[linux dev-6.6 v2 0/7] pinctrl: npcm8xx: pin configuration changes

Andrew Jeffery andrew at codeconstruct.com.au
Thu Jul 25 17:29:34 AEST 2024


Hi Tomer,

On Thu, 2024-07-18 at 21:27 +0300, Tomer Maimon wrote:
> This patch set addresses various pin configuration changes for the 
> Nuvoton NPCM8XX BMC SoC. The patches aim to enhance functionality, 
> remove unused pins, and improve overall pin management.
> 
> This patchset is on the upstream process to the Linux vanilla.
> https://lore.kernel.org/lkml/20240716194008.3502068-1-tmaimon77@gmail.com/
> 
> Changes since version 1:
>         - Squash the non-existent pins, groups and functions.
>         - Remove non-existent groups and functions from dt-bindings.
>         - Modify the commit message.
> 
> Tomer Maimon (7):
>   dt-bindings: pinctrl: npcm8xx: remove non-existent groups and
>     functions
>   pinctrl: nuvoton: npcm8xx: remove non-existent pins, groups, functions
>   pinctrl: nuvoton: npcm8xx: clear polarity before set both edge
>   pinctrl: nuvoton: npcm8xx: add gpi35 and gpi36
>   pinctrl: nuvoton: npcm8xx: add pin 250 to DDR pins group
>   pinctrl: nuvoton: npcm8xx: modify clkrun and serirq pin configuration
>   pinctrl: nuvoton: npcm8xx: modify pins flags
> 
>  .../pinctrl/nuvoton,npcm845-pinctrl.yaml      | 74 +++++++++----------
>  drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c     | 64 ++++++++--------
>  2 files changed, 67 insertions(+), 71 deletions(-)
> 

I looked at applying this series. A few notes:

1. I did get some whitespace warnings during patch application:

```
...
Applying: pinctrl: nuvoton: npcm8xx: modify pins flags                                                                                                         
/home/andrew/src/kernel.org/linux.git/worktrees/openbmc/rebase-apply/patch:47: trailing whitespace.                                                            
                  nprd_smi, smb0b, smb0c, smb0den, smb0d, ddc, rg2mdio, wdog1  
/home/andrew/src/kernel.org/linux.git/worktrees/openbmc/rebase-apply/patch:89: trailing whitespace.                                                            
                nprd_smi, smb0b, smb0c, smb0den, smb0d, ddc, rg2mdio, wdog1                                                                                    
warning: 2 lines add whitespace errors.
```

2. There's a reasonable difference to the patches upstream. The
devicetree binding in particular seems to have some odd differences in
the enums that make me wonder whether we're missing other patches
there. There are also some minor differences in the driver which I
assume are explained by intervening changes between v6.6 and upstream.
Here's the diff, can you give a quick comment on each hunk?

```
$ git diff <backported patches on dev-6.6> <patches proposed upstream on c33ffdb70cc6 ("Merge tag 'phy-for-6.11' of git://git.kernel.org/pub/scm/linux/kernel/git/phy/linux-phy")> -- drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.yaml
diff --git a/Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.yaml
index 4496658006ab..8cd1f442240e 100644
--- a/Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/nuvoton,npcm845-pinctrl.yaml
@@ -35,6 +35,7 @@ properties:
 patternProperties:
   '^gpio@':
     type: object
+    additionalProperties: false

     description:
       Eight GPIO banks that each contain 32 GPIOs.
@@ -80,37 +81,39 @@ patternProperties:
                   fanin7, fanin8, fanin9, fanin10, fanin11, fanin12, fanin13,
                   fanin14, fanin15, pwm0, pwm1, pwm2, pwm3, r2, r2err, r2md,
                   r3rxer, ga20kbc, smb5d, lpc, espi, rg2, ddr, i3c0, i3c1,
-                  i3c2, i3c3, i3c4, i3c5, smb0, smb1, smb2, smb2c, smb2b,
-                  smb1c, smb1b, smb8, smb9, smb10, smb11, sd1, sd1pwr, pwm4,
-                  pwm5, pwm6, pwm7, pwm8, pwm9, pwm10, pwm11, mmc8, mmc, mmcwp,
-                  mmccd, mmcrst, clkout, serirq, scipme, smi, smb6, smb7, spi1,
-                  faninx, r1, spi3, spi3cs1, spi3quad, spi3cs2, spi3cs3,
-                  nprd_smi, smb0b, smb0c, smb0den, smb0d, ddc, rg2mdio, wdog1
-                  wdog2, smb12, smb13, spix, spixcs1, clkreq, hgpio0, hgpio1,
-                  hgpio2, hgpio3, hgpio4, hgpio5, hgpio6, hgpio7 ]
+                  i3c2, i3c3, i3c4, i3c5, smb0, smb1, smb2, smb2c, smb2b, smb1c,
+                  smb1b, smb8, smb9, smb10, smb11, sd1, sd1pwr, pwm4, pwm5,
+                  pwm6, pwm7, pwm8, pwm9, pwm10, pwm11, mmc8, mmc, mmcwp, mmccd,
+                  mmcrst, clkout, serirq, scipme, smi, smb6, smb6b, smb6c,
+                  smb6d, smb7, smb7b, smb7c, smb7d, spi1, faninx, r1, spi3,
+                  spi3cs1, spi3quad, spi3cs2, spi3cs3, nprd_smi, smb0b, smb0c,
+                  smb0den, smb0d, ddc, rg2mdio, wdog1, wdog2, smb12, smb13,
+                  spix, spixcs1, clkreq, hgpio0, hgpio1, hgpio2, hgpio3, hgpio4,
+                  hgpio5, hgpio6, hgpio7, bu4, bu4b, bu5, bu5b, bu6, gpo187 ]

       function:
         description:
           The function that a group of pins is muxed to
-        enum: [ iox1, iox2, smb1d, smb2d, lkgpo1, lkgpo2, ioxh, gspi,
-                smb5b, smb5c, lkgpo0, pspi, jm1, jm2, smb4b, smb4c, smb15,
-                smb16, smb17, smb18, smb19, smb20, smb21, smb22, smb23,
-                smb23b, smb4d, smb14, smb5, smb4, smb3, spi0cs1, spi0cs2,
-                spi0cs3, spi1cs0, spi1cs1, spi1cs2, spi1cs3, spi1cs23, smb3c,
-                smb3b, bmcuart0a, uart1, jtag2, bmcuart1, uart2, sg1mdio,
-                bmcuart0b, r1err, r1md, r1oen, r2oen, rmii3, r3oen, smb3d,
-                fanin0, fanin1, fanin2, fanin3, fanin4, fanin5, fanin6,
-                fanin7, fanin8, fanin9, fanin10, fanin11, fanin12, fanin13,
-                fanin14, fanin15, pwm0, pwm1, pwm2, pwm3, r2, r2err, r2md,
-                r3rxer, ga20kbc, smb5d, lpc, espi, rg2, ddr, i3c0, i3c1,
-                i3c2, i3c3, i3c4, i3c5, smb0, smb1, smb2, smb2c, smb2b,
-                smb1c, smb1b, smb8, smb9, smb10, smb11, sd1, sd1pwr, pwm4,
-                pwm5, pwm6, pwm7, pwm8, pwm9, pwm10, pwm11, mmc8, mmc, mmcwp,
-                mmccd, mmcrst, clkout, serirq, scipme, smi, smb6, smb7, spi1,
-                faninx, r1, spi3, spi3cs1, spi3quad, spi3cs2, spi3cs3,
-                nprd_smi, smb0b, smb0c, smb0den, smb0d, ddc, rg2mdio, wdog1
-                wdog2, smb12, smb13, spix, spixcs1, clkreq, hgpio0, hgpio1,
-                hgpio2, hgpio3, hgpio4, hgpio5, hgpio6, hgpio7 ]
+        enum: [ iox1, iox2, smb1d, smb2d, lkgpo1, lkgpo2, ioxh, gspi, smb5b,
+                smb5c, lkgpo0, pspi, jm1, jm2, smb4b, smb4c, smb15, smb16,
+                smb17, smb18, smb19, smb20, smb21, smb22, smb23, smb23b, smb4d,
+                smb14, smb5, smb4, smb3, spi0cs1, spi0cs2, spi0cs3, spi1cs0,
+                spi1cs1, spi1cs2, spi1cs3, spi1cs23, smb3c, smb3b, bmcuart0a,
+                uart1, jtag2, bmcuart1, uart2, sg1mdio, bmcuart0b, r1err, r1md,
+                r1oen, r2oen, rmii3, r3oen, smb3d, fanin0, fanin1, fanin2,
+                fanin3, fanin4, fanin5, fanin6, fanin7, fanin8, fanin9, fanin10,
+                fanin11, fanin12, fanin13, fanin14, fanin15, pwm0, pwm1, pwm2,
+                pwm3, r2, r2err, r2md, r3rxer, ga20kbc, smb5d, lpc, espi, rg2,
+                ddr, i3c0, i3c1, i3c2, i3c3, i3c4, i3c5, smb0, smb1, smb2,
+                smb2c, smb2b, smb1c, smb1b, smb8, smb9, smb10, smb11, sd1,
+                sd1pwr, pwm4, pwm5, pwm6, pwm7, pwm8, pwm9, pwm10, pwm11,
+                mmc8, mmc, mmcwp, mmccd, mmcrst, clkout, serirq, scipme, smi,
+                smb6, smb6b, smb6c, smb6d, smb7, smb7b, smb7c, smb7d, spi1,
+                faninx, r1, spi3, spi3cs1, spi3quad, spi3cs2, spi3cs3, nprd_smi,
+                smb0b, smb0c, smb0den, smb0d, ddc, rg2mdio, wdog1, wdog2,
+                smb12, smb13, spix, spixcs1, clkreq, hgpio0, hgpio1, hgpio2,
+                hgpio3, hgpio4, hgpio5, hgpio6, hgpio7, bu4, bu4b, bu5, bu5b,
+                bu6, gpo187 ]

     dependencies:
       groups: [ function ]
@@ -149,7 +152,6 @@ patternProperties:
         description:
           Debouncing periods in microseconds, one period per interrupt
           bank found in the controller
-        $ref: /schemas/types.yaml#/definitions/uint32-array
         minItems: 1
         maxItems: 4

@@ -157,7 +159,6 @@ patternProperties:
         description: |
           0: Low rate
           1: High rate
-        $ref: /schemas/types.yaml#/definitions/uint32
         enum: [0, 1]

       drive-strength:
diff --git a/drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c b/drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c
index 607960fdbc40..471f644c5eef 100644
--- a/drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c
+++ b/drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c
@@ -173,7 +173,7 @@ static int npcmgpio_direction_input(struct gpio_chip *chip, unsigned int offset)
        struct npcm8xx_gpio *bank = gpiochip_get_data(chip);
        int ret;

-       ret = pinctrl_gpio_direction_input(offset + chip->base);
+       ret = pinctrl_gpio_direction_input(chip, offset);
        if (ret)
                return ret;

@@ -186,7 +186,7 @@ static int npcmgpio_direction_output(struct gpio_chip *chip,
        struct npcm8xx_gpio *bank = gpiochip_get_data(chip);
        int ret;

-       ret = pinctrl_gpio_direction_output(offset + chip->base);
+       ret = pinctrl_gpio_direction_output(chip, offset);
        if (ret)
                return ret;

@@ -198,18 +198,13 @@ static int npcmgpio_gpio_request(struct gpio_chip *chip, unsigned int offset)
        struct npcm8xx_gpio *bank = gpiochip_get_data(chip);
        int ret;

-       ret = pinctrl_gpio_request(offset + chip->base);
+       ret = pinctrl_gpio_request(chip, offset);
        if (ret)
                return ret;

        return bank->request(chip, offset);
 }

-static void npcmgpio_gpio_free(struct gpio_chip *chip, unsigned int offset)
-{
-       pinctrl_gpio_free(offset + chip->base);
-}
-
 static void npcmgpio_irq_handler(struct irq_desc *desc)
 {
        unsigned long sts, en, bit;
@@ -2386,7 +2381,7 @@ static int npcm8xx_gpio_fw(struct npcm8xx_pinctrl *pctrl)
                pctrl->gpio_bank[id].gc.direction_output = npcmgpio_direction_output;
                pctrl->gpio_bank[id].request = pctrl->gpio_bank[id].gc.request;
                pctrl->gpio_bank[id].gc.request = npcmgpio_gpio_request;
-               pctrl->gpio_bank[id].gc.free = npcmgpio_gpio_free;
+               pctrl->gpio_bank[id].gc.free = pinctrl_gpio_free;
                for (i = 0 ; i < NPCM8XX_DEBOUNCE_MAX ; i++)
                        pctrl->gpio_bank[id].debounce.set_val[i] = false;
                pctrl->gpio_bank[id].gc.add_pin_ranges = npcmgpio_add_pin_ranges;
```

Andrew


More information about the openbmc mailing list