| Message ID | 20220805122202.23174-1-pali@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/4] pinctrl: armada-37xx: Add missing GPIO-only pins | expand |
On Fri, Aug 05, 2022 at 02:21:59PM +0200, Pali Rohár wrote: > gpio1_5 and gpio2_2 are GPIO-only pins. Add them into MPP groups table > so they are properly exported as valid pin numbers. > > Fixes: 87466ccd9401 ("pinctrl: armada-37xx: Add pin controller support for Armada 37xx") > Signed-off-by: Pali Rohár <pali@kernel.org> > --- > drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c > index a140b6bfbfaa..2b44c634ccb5 100644 > --- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c > +++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c > @@ -122,6 +122,16 @@ struct armada_37xx_pinctrl { > .funcs = {_func1, _func2} \ > } > > +#define PIN_GRP_GPIO_0(_name, _start, _nr) \ The naming of these macros are a bit odd, but this does not make it any worse. Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
On Tuesday 09 August 2022 22:19:33 Andrew Lunn wrote: > On Fri, Aug 05, 2022 at 02:21:59PM +0200, Pali Rohár wrote: > > gpio1_5 and gpio2_2 are GPIO-only pins. Add them into MPP groups table > > so they are properly exported as valid pin numbers. > > > > Fixes: 87466ccd9401 ("pinctrl: armada-37xx: Add pin controller support for Armada 37xx") > > Signed-off-by: Pali Rohár <pali@kernel.org> > > --- > > drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c > > index a140b6bfbfaa..2b44c634ccb5 100644 > > --- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c > > +++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c > > @@ -122,6 +122,16 @@ struct armada_37xx_pinctrl { > > .funcs = {_func1, _func2} \ > > } > > > > +#define PIN_GRP_GPIO_0(_name, _start, _nr) \ > > The naming of these macros are a bit odd, but this does not make it > any worse. Yea... but I'm continuing to use existing naming convention. Suffix _N means how many non-gpio functions have particular group. > Reviewed-by: Andrew Lunn <andrew@lunn.ch> > > Andrew
On Fri, Aug 05, 2022 at 02:21:59PM +0200, Pali Rohár wrote: > gpio1_5 and gpio2_2 are GPIO-only pins. Add them into MPP groups table > so they are properly exported as valid pin numbers. > > Fixes: 87466ccd9401 ("pinctrl: armada-37xx: Add pin controller support for Armada 37xx") Does this actually break anything? Are there boards in mainline that require this? Does this need to be part of stable? Andrew
On Tuesday 09 August 2022 22:24:12 Andrew Lunn wrote: > On Fri, Aug 05, 2022 at 02:21:59PM +0200, Pali Rohár wrote: > > gpio1_5 and gpio2_2 are GPIO-only pins. Add them into MPP groups table > > so they are properly exported as valid pin numbers. > > > > Fixes: 87466ccd9401 ("pinctrl: armada-37xx: Add pin controller support for Armada 37xx") > > Does this actually break anything? Are there boards in mainline that > require this? Does this need to be part of stable? > > Andrew I'm not adding CC:stable tag for automatic stable backporting. I'm adding just Fixes tag to indicate that this patch fixes above mentioned commit. On Turris Mox board this at least one of those GPIOs available on PIN header and ready for (GPIO) using. These GPIOs are not specified in pinmuxing part of Turris Mox dts files included in kernel. But theoretically they can be added via dts overlay with bootloader. Probably same applies for Espressobin which also have lot of GPIOs exported on pin header -- but I have not investigated it. So I would say currently there is no known or reported breakage and that is why I have not added CC:stable tag, only Fixes. Originally I discovered this issue during debugging of U-Boot which has copy of this driver and U-Boot supports pinmuxing and gpio requesting via commands. And U-Boot gpio command refused to change status of these two GPIOs, for explained reasons.
On Tue, Aug 09, 2022 at 10:36:48PM +0200, Pali Rohár wrote: > On Tuesday 09 August 2022 22:24:12 Andrew Lunn wrote: > > On Fri, Aug 05, 2022 at 02:21:59PM +0200, Pali Rohár wrote: > > > gpio1_5 and gpio2_2 are GPIO-only pins. Add them into MPP groups table > > > so they are properly exported as valid pin numbers. > > > > > > Fixes: 87466ccd9401 ("pinctrl: armada-37xx: Add pin controller support for Armada 37xx") > > > > Does this actually break anything? Are there boards in mainline that > > require this? Does this need to be part of stable? > > > > Andrew > > I'm not adding CC:stable tag for automatic stable backporting. I'm > adding just Fixes tag to indicate that this patch fixes above mentioned > commit. O.K. It might get back ported anyway, due to the fuzzy logic bot thinking its a valuable fix. Andrew
On Fri, Aug 5, 2022 at 2:22 PM Pali Rohár <pali@kernel.org> wrote: > gpio1_5 and gpio2_2 are GPIO-only pins. Add them into MPP groups table > so they are properly exported as valid pin numbers. > > Fixes: 87466ccd9401 ("pinctrl: armada-37xx: Add pin controller support for Armada 37xx") > Signed-off-by: Pali Rohár <pali@kernel.org> All 4 patches applied for next (v6.1), deemed as non-urgent fixes also I guess the patches depend on each other, any other handling desired: suggest. Yours, Linus Walleij
diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c index a140b6bfbfaa..2b44c634ccb5 100644 --- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c +++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c @@ -122,6 +122,16 @@ struct armada_37xx_pinctrl { .funcs = {_func1, _func2} \ } +#define PIN_GRP_GPIO_0(_name, _start, _nr) \ + { \ + .name = _name, \ + .start_pin = _start, \ + .npins = _nr, \ + .reg_mask = 0, \ + .val = {0}, \ + .funcs = {"gpio"} \ + } + #define PIN_GRP_GPIO(_name, _start, _nr, _mask, _func1) \ { \ .name = _name, \ @@ -179,6 +189,7 @@ static struct armada_37xx_pin_group armada_37xx_nb_groups[] = { "pwm", "led"), PIN_GRP_GPIO("pmic1", 7, 1, BIT(7), "pmic"), PIN_GRP_GPIO("pmic0", 6, 1, BIT(8), "pmic"), + PIN_GRP_GPIO_0("gpio1_5", 5, 1), PIN_GRP_GPIO("i2c2", 2, 2, BIT(9), "i2c"), PIN_GRP_GPIO("i2c1", 0, 2, BIT(10), "i2c"), PIN_GRP_GPIO("spi_cs1", 17, 1, BIT(12), "spi"), @@ -195,6 +206,7 @@ static struct armada_37xx_pin_group armada_37xx_nb_groups[] = { static struct armada_37xx_pin_group armada_37xx_sb_groups[] = { PIN_GRP_GPIO("usb32_drvvbus0", 0, 1, BIT(0), "drvbus"), PIN_GRP_GPIO("usb2_drvvbus1", 1, 1, BIT(1), "drvbus"), + PIN_GRP_GPIO_0("gpio2_2", 2, 1), PIN_GRP_GPIO("sdio_sb", 24, 6, BIT(2), "sdio"), PIN_GRP_GPIO("rgmii", 6, 12, BIT(3), "mii"), PIN_GRP_GPIO("smi", 18, 2, BIT(4), "smi"),
gpio1_5 and gpio2_2 are GPIO-only pins. Add them into MPP groups table so they are properly exported as valid pin numbers. Fixes: 87466ccd9401 ("pinctrl: armada-37xx: Add pin controller support for Armada 37xx") Signed-off-by: Pali Rohár <pali@kernel.org> --- drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)