| Message ID | 20240718182720.2721427-1-tmaimon77@gmail.com |
|---|---|
| Headers | show |
| Series | pinctrl: npcm8xx: pin configuration changes | expand |
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
Hi Andrew, Thanks for your comments On Thu, 25 Jul 2024 at 10:29, Andrew Jeffery <andrew@codeconstruct.com.au> wrote: > > 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. > ``` > will fixed in V3 > 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 ] Thanks for raising I have an error here, I will fixed it in the vanilla and the OpenBMC, I should remove and add enums. > > 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 it removed in the newer version, we can keep it > minItems: 1 > maxItems: 4 > > @@ -157,7 +159,6 @@ patternProperties: > description: | > 0: Low rate > 1: High rate > - $ref: /schemas/types.yaml#/definitions/uint32 it removed in the newer version, we can keep it > 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); Modify in newer version, will revert the change in V3. since kernel 6.6 doesn't support it > 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); same as above > 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); same as above > 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); > -} > - same as above > 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; same as above > 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 Best regards, Tomer
On Thu, 2024-07-25 at 20:56 +0300, Tomer Maimon wrote: > Hi Andrew, > > Thanks for your comments > > On Thu, 25 Jul 2024 at 10:29, Andrew Jeffery > <andrew@codeconstruct.com.au> wrote: > > > > 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. > > ``` > > > will fixed in V3 > > 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 ] > Thanks for raising I have an error here, I will fixed it in the > vanilla and the OpenBMC, I should remove and add enums. > > > > 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 > it removed in the newer version, we can keep it The outcome I'm after is we have the least difference in the patches for dev-6.6 to those upstream. If it's correct to remove it, and that is what you've done in the upstream patches, then we should remove it in the dev-6.6 patches as well. > > minItems: 1 > > maxItems: 4 > > > > @@ -157,7 +159,6 @@ patternProperties: > > description: | > > 0: Low rate > > 1: High rate > > - $ref: /schemas/types.yaml#/definitions/uint32 > it removed in the newer version, we can keep it > > 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); > Modify in newer version, will revert the change in V3. since kernel > 6.6 doesn't support it Just to clarify, the dev-6.6 patches have the "-" content, and the upstream patches have the "+" content. Perhaps how I generated the diff is the confusing bit here. If you need me to unpack that a bit more please ask, it's important that we both have the same understanding of what's going on :D Andrew
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(-)