[PATCH/RFC 0/4] sh: sh7722/sh7757i/sh7264/sh7269: Fix pinctrl registration

[PATCH/RFC 0/4] sh: sh7722/sh7757i/sh7264/sh7269: Fix pinctrl registration

[Geert Uytterhoeven]

	Hi all,

Magnus reported that on sh7722/Migo-R, pinctrl registration fails with:

    sh-pfc pfc-sh7722: pin 0 already registered
    sh-pfc pfc-sh7722: error during pin registration
    sh-pfc pfc-sh7722: could not register: -22
    sh-pfc: probe of pfc-sh7722 failed with error -22

pinmux_pins[] is initialized through PINMUX_GPIO(), using designated
array initializers, where the GPIO_* enums serve as indices.
Apparently GPIO_PTQ7 was defined in the enum, but never used.
If enum values are defined, but never used, pinmux_pins[] contains
(zero-filled) holes.  Hence such entries are treated as pin zero, which
was registered before, and initialization fails.

I can't see how this ever worked, as at the time of commit f5e25ae52feff2dc
("sh-pfc: Add sh7722 pinmux support"), pinmux_gpios[] in
drivers/pinctrl/sh-pfc/pfc-sh7722.c already had the hole, and
drivers/pinctrl/core.c already had the check.

Some scripting revealed a few more broken drivers:
  - sh7757 has four holes, due to nonexistent GPIO_PT[JLNQ]7_RESV.
  - sh7264 and sh7269 define GPIO_PH[0-7], but don't use it with
    PINMUX_GPIO().

Patch 1 fixes the issue on sh7722, and was tested.
Patches 3-4 should fix the issue on the other 3 SoCs, but was untested due
to lack of hardware.

Thanks for your comments!

Geert Uytterhoeven (4):
  sh: sh7722: Remove nonexistent GPIO_PTQ7 to fix pinctrl registration
  [RFC] sh: sh7757: Remove nonexistent GPIO_PT[JLNQ]7_RESV to fix
    pinctrl registration
  sh: sh7264: Remove nonexistent GPIO_PH[0-7] to fix pinctrl
    registration
  [RFC] sh: sh7269: Remove nonexistent GPIO_PH[0-7] to fix pinctrl
    registration

 arch/sh/include/cpu-sh2a/cpu/sh7264.h | 4 +---
 arch/sh/include/cpu-sh2a/cpu/sh7269.h | 4 +---
 arch/sh/include/cpu-sh4/cpu/sh7722.h  | 2 +-
 arch/sh/include/cpu-sh4/cpu/sh7757.h  | 8 ++++----
 4 files changed, 7 insertions(+), 11 deletions(-)

-- 
2.7.4

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

[PATCH 1/4] sh: sh7722: Remove nonexistent GPIO_PTQ7 to fix pinctrl registration

[Geert Uytterhoeven]

On sh7722/Migo-R, pinctrl registration fails with:

    sh-pfc pfc-sh7722: pin 0 already registered
    sh-pfc pfc-sh7722: error during pin registration
    sh-pfc pfc-sh7722: could not register: -22
    sh-pfc: probe of pfc-sh7722 failed with error -22

pinmux_pins[] is initialized through PINMUX_GPIO(), using designated
array initializers, where the GPIO_* enums serve as indices.
As GPIO_PTQ7 is defined in the enum, but never used, pinmux_pins[]
contains a (zero-filled) hole. Hence this entry is treated as pin zero,
which was registered before, and initialization fails.

According to the datasheet, port PTQ7 does not exist. Hence remove
GPIO_PTQ7 from the enum to fix this.

Fixes: 8d7b5b0af7e070b9 ("sh: Add sh7722 pinmux code")
Reported-by: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Tested on sh7722/Migo-R.
---
 arch/sh/include/cpu-sh4/cpu/sh7722.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/sh/include/cpu-sh4/cpu/sh7722.h b/arch/sh/include/cpu-sh4/cpu/sh7722.h
index 3bb74e534d0f8ca4..78961ab78a5a9c83 100644
--- a/arch/sh/include/cpu-sh4/cpu/sh7722.h
+++ b/arch/sh/include/cpu-sh4/cpu/sh7722.h
@@ -67,7 +67,7 @@ enum {
 	GPIO_PTN3, GPIO_PTN2, GPIO_PTN1, GPIO_PTN0,
 
 	/* PTQ */
-	GPIO_PTQ7, GPIO_PTQ6, GPIO_PTQ5, GPIO_PTQ4,
+	GPIO_PTQ6, GPIO_PTQ5, GPIO_PTQ4,
 	GPIO_PTQ3, GPIO_PTQ2, GPIO_PTQ1, GPIO_PTQ0,
 
 	/* PTR */
-- 
2.7.4

Re: [PATCH 1/4] sh: sh7722: Remove nonexistent GPIO_PTQ7 to fix pinctrl registration

[jmondi]

Hi Geert,

On Tue, May 09, 2017 at 04:11:54PM +0200, Geert Uytterhoeven wrote:
> On sh7722/Migo-R, pinctrl registration fails with:
>
>     sh-pfc pfc-sh7722: pin 0 already registered
>     sh-pfc pfc-sh7722: error during pin registration
>     sh-pfc pfc-sh7722: could not register: -22
>     sh-pfc: probe of pfc-sh7722 failed with error -22
>
> pinmux_pins[] is initialized through PINMUX_GPIO(), using designated
> array initializers, where the GPIO_* enums serve as indices.
> As GPIO_PTQ7 is defined in the enum, but never used, pinmux_pins[]
> contains a (zero-filled) hole. Hence this entry is treated as pin zero,
> which was registered before, and initialization fails.
>
> According to the datasheet, port PTQ7 does not exist. Hence remove
> GPIO_PTQ7 from the enum to fix this.
>
> Fixes: 8d7b5b0af7e070b9 ("sh: Add sh7722 pinmux code")
> Reported-by: Magnus Damm <magnus.damm@gmail.com>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Tested-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Thank you
   j

> ---
> Tested on sh7722/Migo-R.
> ---
>  arch/sh/include/cpu-sh4/cpu/sh7722.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/sh/include/cpu-sh4/cpu/sh7722.h b/arch/sh/include/cpu-sh4/cpu/sh7722.h
> index 3bb74e534d0f8ca4..78961ab78a5a9c83 100644
> --- a/arch/sh/include/cpu-sh4/cpu/sh7722.h
> +++ b/arch/sh/include/cpu-sh4/cpu/sh7722.h
> @@ -67,7 +67,7 @@ enum {
>  	GPIO_PTN3, GPIO_PTN2, GPIO_PTN1, GPIO_PTN0,
>
>  	/* PTQ */
> -	GPIO_PTQ7, GPIO_PTQ6, GPIO_PTQ5, GPIO_PTQ4,
> +	GPIO_PTQ6, GPIO_PTQ5, GPIO_PTQ4,
>  	GPIO_PTQ3, GPIO_PTQ2, GPIO_PTQ1, GPIO_PTQ0,
>
>  	/* PTR */
> --
> 2.7.4
>

[PATCH 2/4] [RFC] sh: sh7757: Remove nonexistent GPIO_PT[JLNQ]7_RESV to fix pinctrl registration

[Geert Uytterhoeven]

Commit 3810e96056ffddf6 ("sh: modify pinmux for SH7757 2nd cut") renamed
GPIO_PT[JLNQ]7 to GPIO_PT[JLNQ]7_RESV, and removed the existing users
from the pinmux_pins[] array.

However, pinmux_pins[] is initialized through PINMUX_GPIO(), using
designated array initializers, where the GPIO_* enums serve as indices.
Hence entries were not really removed, but replaced by (zero-filled)
holes.  Such entries are treated as pin zero, which was registered
before, thus leading to initialization failures:

    sh-pfc pfc-sh7757: pin 0 already registered
    sh-pfc pfc-sh7757: error during pin registration
    sh-pfc pfc-sh7757: could not register: -22
    sh-pfc: probe of pfc-sh7757 failed with error -22

Remove GPIO_PT[JLNQ]7_RESV from the enum to fix this.

Fixes: 3810e96056ffddf6 ("sh: modify pinmux for SH7757 2nd cut")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Untested due to lack of hardware and datasheet.
---
 arch/sh/include/cpu-sh4/cpu/sh7757.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/sh/include/cpu-sh4/cpu/sh7757.h b/arch/sh/include/cpu-sh4/cpu/sh7757.h
index 5340f3bc1863c389..b40fb541e72a78ae 100644
--- a/arch/sh/include/cpu-sh4/cpu/sh7757.h
+++ b/arch/sh/include/cpu-sh4/cpu/sh7757.h
@@ -40,7 +40,7 @@ enum {
 
 	/* PTJ */
 	GPIO_PTJ0, GPIO_PTJ1, GPIO_PTJ2, GPIO_PTJ3,
-	GPIO_PTJ4, GPIO_PTJ5, GPIO_PTJ6, GPIO_PTJ7_RESV,
+	GPIO_PTJ4, GPIO_PTJ5, GPIO_PTJ6,
 
 	/* PTK */
 	GPIO_PTK0, GPIO_PTK1, GPIO_PTK2, GPIO_PTK3,
@@ -48,7 +48,7 @@ enum {
 
 	/* PTL */
 	GPIO_PTL0, GPIO_PTL1, GPIO_PTL2, GPIO_PTL3,
-	GPIO_PTL4, GPIO_PTL5, GPIO_PTL6, GPIO_PTL7_RESV,
+	GPIO_PTL4, GPIO_PTL5, GPIO_PTL6,
 
 	/* PTM */
 	GPIO_PTM0, GPIO_PTM1, GPIO_PTM2, GPIO_PTM3,
@@ -56,7 +56,7 @@ enum {
 
 	/* PTN */
 	GPIO_PTN0, GPIO_PTN1, GPIO_PTN2, GPIO_PTN3,
-	GPIO_PTN4, GPIO_PTN5, GPIO_PTN6, GPIO_PTN7_RESV,
+	GPIO_PTN4, GPIO_PTN5, GPIO_PTN6,
 
 	/* PTO */
 	GPIO_PTO0, GPIO_PTO1, GPIO_PTO2, GPIO_PTO3,
@@ -68,7 +68,7 @@ enum {
 
 	/* PTQ */
 	GPIO_PTQ0, GPIO_PTQ1, GPIO_PTQ2, GPIO_PTQ3,
-	GPIO_PTQ4, GPIO_PTQ5, GPIO_PTQ6, GPIO_PTQ7_RESV,
+	GPIO_PTQ4, GPIO_PTQ5, GPIO_PTQ6,
 
 	/* PTR */
 	GPIO_PTR0, GPIO_PTR1, GPIO_PTR2, GPIO_PTR3,
-- 
2.7.4

[PATCH 3/4] sh: sh7264: Remove nonexistent GPIO_PH[0-7] to fix pinctrl registration

[Geert Uytterhoeven]

Pinmux_pins[] is initialized through PINMUX_GPIO(), using designated
array initializers, where the GPIO_* enums serve as indices.
If enum values are defined, but never used, pinmux_pins[] contains
(zero-filled) holes.  Such entries are treated as pin zero, which was
registered before, thus leading to pinctrl registration failures:

    sh-pfc pfc-sh7264: pin 0 already registered
    sh-pfc pfc-sh7264: error during pin registration
    sh-pfc pfc-sh7264: could not register: -22
    sh-pfc: probe of pfc-sh7264 failed with error -22

Remove GPIO_PH[0-7] from the enum to fix this.

Fixes: 41797f75486d8ca3 ("sh: Add pinmux for sh7264")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Untested due to lack of hardware.
---
 arch/sh/include/cpu-sh2a/cpu/sh7264.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/sh/include/cpu-sh2a/cpu/sh7264.h b/arch/sh/include/cpu-sh2a/cpu/sh7264.h
index 4d1ef6d74bd6050c..2ae0e938b657a109 100644
--- a/arch/sh/include/cpu-sh2a/cpu/sh7264.h
+++ b/arch/sh/include/cpu-sh2a/cpu/sh7264.h
@@ -43,9 +43,7 @@ enum {
 	GPIO_PG7, GPIO_PG6, GPIO_PG5, GPIO_PG4,
 	GPIO_PG3, GPIO_PG2, GPIO_PG1, GPIO_PG0,
 
-	/* Port H */
-	GPIO_PH7, GPIO_PH6, GPIO_PH5, GPIO_PH4,
-	GPIO_PH3, GPIO_PH2, GPIO_PH1, GPIO_PH0,
+	/* Port H - Port H does not have a Data Register */
 
 	/* Port I - not on device */
 
-- 
2.7.4

[PATCH 4/4] [RFC] sh: sh7269: Remove nonexistent GPIO_PH[0-7] to fix pinctrl registration

[Geert Uytterhoeven]

Pinmux_pins[] is initialized through PINMUX_GPIO(), using designated
array initializers, where the GPIO_* enums serve as indices.
If enum values are defined, but never used, pinmux_pins[] contains
(zero-filled) holes.  Such entries are treated as pin zero, which was
registered before, thus leading to pinctrl registration failures:

    sh-pfc pfc-sh7269: pin 0 already registered
    sh-pfc pfc-sh7269: error during pin registration
    sh-pfc pfc-sh7269: could not register: -22
    sh-pfc: probe of pfc-sh7269 failed with error -22

Remove GPIO_PH[0-7] from the enum to fix this.

Fixes: ef0fa5331a73e479 ("sh: Add pinmux for sh7269")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Untested due to lack of hardware.
---
 arch/sh/include/cpu-sh2a/cpu/sh7269.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/sh/include/cpu-sh2a/cpu/sh7269.h b/arch/sh/include/cpu-sh2a/cpu/sh7269.h
index 2a0ca8780f0d8c34..13c495a9fc0007b4 100644
--- a/arch/sh/include/cpu-sh2a/cpu/sh7269.h
+++ b/arch/sh/include/cpu-sh2a/cpu/sh7269.h
@@ -45,9 +45,7 @@ enum {
 	GPIO_PG7, GPIO_PG6, GPIO_PG5, GPIO_PG4,
 	GPIO_PG3, GPIO_PG2, GPIO_PG1, GPIO_PG0,
 
-	/* Port H */
-	GPIO_PH7, GPIO_PH6, GPIO_PH5, GPIO_PH4,
-	GPIO_PH3, GPIO_PH2, GPIO_PH1, GPIO_PH0,
+	/* Port H - Port H does not have a Data Register */
 
 	/* Port I - not on device */
 
-- 
2.7.4

Re: [PATCH/RFC 0/4] sh: sh7722/sh7757i/sh7264/sh7269: Fix pinctrl registration

[Laurent Pinchart]

Hi Geert,

Thank you for the patches.

On Tuesday 09 May 2017 16:11:53 Geert Uytterhoeven wrote:
> 	Hi all,
> 
> Magnus reported that on sh7722/Migo-R, pinctrl registration fails with:
> 
>     sh-pfc pfc-sh7722: pin 0 already registered
>     sh-pfc pfc-sh7722: error during pin registration
>     sh-pfc pfc-sh7722: could not register: -22
>     sh-pfc: probe of pfc-sh7722 failed with error -22
> 
> pinmux_pins[] is initialized through PINMUX_GPIO(), using designated
> array initializers, where the GPIO_* enums serve as indices.
> Apparently GPIO_PTQ7 was defined in the enum, but never used.
> If enum values are defined, but never used, pinmux_pins[] contains
> (zero-filled) holes.  Hence such entries are treated as pin zero, which
> was registered before, and initialization fails.
> 
> I can't see how this ever worked, as at the time of commit f5e25ae52feff2dc
> ("sh-pfc: Add sh7722 pinmux support"), pinmux_gpios[] in
> drivers/pinctrl/sh-pfc/pfc-sh7722.c already had the hole, and
> drivers/pinctrl/core.c already had the check.
> 
> Some scripting revealed a few more broken drivers:
>   - sh7757 has four holes, due to nonexistent GPIO_PT[JLNQ]7_RESV.
>   - sh7264 and sh7269 define GPIO_PH[0-7], but don't use it with
>     PINMUX_GPIO().
> 
> Patch 1 fixes the issue on sh7722, and was tested.
> Patches 3-4 should fix the issue on the other 3 SoCs, but was untested due
> to lack of hardware.

This all looks good to me, even if I can't test or verify patches 2/4 to 4/4 
as I don't have access to the hardware or datasheet either. Given that they're 
untested I wouldn't fake the error message in the commit log though, but just 
refer to the problem noticed on sh7722.

Apart from that, for the whole series,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> Thanks for your comments!
> 
> Geert Uytterhoeven (4):
>   sh: sh7722: Remove nonexistent GPIO_PTQ7 to fix pinctrl registration
>   [RFC] sh: sh7757: Remove nonexistent GPIO_PT[JLNQ]7_RESV to fix
>     pinctrl registration
>   sh: sh7264: Remove nonexistent GPIO_PH[0-7] to fix pinctrl
>     registration
>   [RFC] sh: sh7269: Remove nonexistent GPIO_PH[0-7] to fix pinctrl
>     registration
> 
>  arch/sh/include/cpu-sh2a/cpu/sh7264.h | 4 +---
>  arch/sh/include/cpu-sh2a/cpu/sh7269.h | 4 +---
>  arch/sh/include/cpu-sh4/cpu/sh7722.h  | 2 +-
>  arch/sh/include/cpu-sh4/cpu/sh7757.h  | 8 ++++----
>  4 files changed, 7 insertions(+), 11 deletions(-)

-- 
Regards,

Laurent Pinchart

RE: [PATCH/RFC 0/4] sh: sh7722/sh7757i/sh7264/sh7269: Fix pinctrl registration

[Phil Edworthy]

Hi Geert,

Thanks for the patches, sorry I can't test these either... It's been quite a few
years since I had an sh7264 or sh7269 board!

Thanks
Phil

-----Original Message-----
On: 09 May 2017 16:44, Laurent Pinchart wrote:
Subject: Re: [PATCH/RFC 0/4] sh: sh7722/sh7757i/sh7264/sh7269: Fix pinctrl registration

Hi Geert,

Thank you for the patches.

On Tuesday 09 May 2017 16:11:53 Geert Uytterhoeven wrote:
> 	Hi all,
> 
> Magnus reported that on sh7722/Migo-R, pinctrl registration fails with:
> 
>     sh-pfc pfc-sh7722: pin 0 already registered
>     sh-pfc pfc-sh7722: error during pin registration
>     sh-pfc pfc-sh7722: could not register: -22
>     sh-pfc: probe of pfc-sh7722 failed with error -22
> 
> pinmux_pins[] is initialized through PINMUX_GPIO(), using designated 
> array initializers, where the GPIO_* enums serve as indices.
> Apparently GPIO_PTQ7 was defined in the enum, but never used.
> If enum values are defined, but never used, pinmux_pins[] contains
> (zero-filled) holes.  Hence such entries are treated as pin zero, 
> which was registered before, and initialization fails.
> 
> I can't see how this ever worked, as at the time of commit 
> f5e25ae52feff2dc
> ("sh-pfc: Add sh7722 pinmux support"), pinmux_gpios[] in 
> drivers/pinctrl/sh-pfc/pfc-sh7722.c already had the hole, and 
> drivers/pinctrl/core.c already had the check.
> 
> Some scripting revealed a few more broken drivers:
>   - sh7757 has four holes, due to nonexistent GPIO_PT[JLNQ]7_RESV.
>   - sh7264 and sh7269 define GPIO_PH[0-7], but don't use it with
>     PINMUX_GPIO().
> 
> Patch 1 fixes the issue on sh7722, and was tested.
> Patches 3-4 should fix the issue on the other 3 SoCs, but was untested 
> due to lack of hardware.

This all looks good to me, even if I can't test or verify patches 2/4 to 4/4 as I don't have access to the hardware or datasheet either. Given that they're untested I wouldn't fake the error message in the commit log though, but just refer to the problem noticed on sh7722.

Apart from that, for the whole series,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> Thanks for your comments!
> 
> Geert Uytterhoeven (4):
>   sh: sh7722: Remove nonexistent GPIO_PTQ7 to fix pinctrl registration
>   [RFC] sh: sh7757: Remove nonexistent GPIO_PT[JLNQ]7_RESV to fix
>     pinctrl registration
>   sh: sh7264: Remove nonexistent GPIO_PH[0-7] to fix pinctrl
>     registration
>   [RFC] sh: sh7269: Remove nonexistent GPIO_PH[0-7] to fix pinctrl
>     registration
> 
>  arch/sh/include/cpu-sh2a/cpu/sh7264.h | 4 +---  
> arch/sh/include/cpu-sh2a/cpu/sh7269.h | 4 +---  
> arch/sh/include/cpu-sh4/cpu/sh7722.h  | 2 +-  
> arch/sh/include/cpu-sh4/cpu/sh7757.h  | 8 ++++----
>  4 files changed, 7 insertions(+), 11 deletions(-)

--
Regards,

Laurent Pinchart