diff mbox series

[v2,1/4] pinctrl: armada-37xx: Add missing GPIO-only pins

Message ID 20220805122202.23174-1-pali@kernel.org
State New
Headers show
Series [v2,1/4] pinctrl: armada-37xx: Add missing GPIO-only pins | expand

Commit Message

Pali Rohár Aug. 5, 2022, 12:21 p.m. UTC
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(+)

Comments

Andrew Lunn Aug. 9, 2022, 8:19 p.m. UTC | #1
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
Pali Rohár Aug. 9, 2022, 8:23 p.m. UTC | #2
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
Andrew Lunn Aug. 9, 2022, 8:24 p.m. UTC | #3
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
Pali Rohár Aug. 9, 2022, 8:36 p.m. UTC | #4
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.
Andrew Lunn Aug. 9, 2022, 8:41 p.m. UTC | #5
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
Linus Walleij Aug. 22, 2022, 8:47 a.m. UTC | #6
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 mbox series

Patch

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"),