[linux dev-6.6 v2 0/7] pinctrl: npcm8xx: pin configuration changes
Tomer Maimon
tmaimon77 at gmail.com
Fri Jul 26 03:56:10 AEST 2024
Hi Andrew,
Thanks for your comments
On Thu, 25 Jul 2024 at 10:29, Andrew Jeffery
<andrew at 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
More information about the openbmc
mailing list