Skip to contentSkip to conversation

sh-pfc: r8a73a4: Support sparse GPIO numbers

PATCH In review Magnus Damm 3 participants 4 messages Mar 26, 2013 – Mar 27, 2013

Subsystems (5)
Arm (32-bit) Arm sub-architectures SuperH Arm/RISC-V/Renesas architecture +1 more subsystems
lore
Share & tools
lore

Magnus Damm

author maintainer
#0 lore raw

PATCH 2/13 sh-pfc: r8a73a4: Support sparse GPIO numbers

From: Magnus Damm <[email protected]>

The r8a73a4 SoC has sparse GPIO numbers. Declare ranges for pin numbers in the PFC SoC data. Pin numbers shall be used with the GPIO API from this point on.

Signed-off-by Magnus Damm <[email protected]>

---

 arch/arm/mach-shmobile/include/mach/r8a73a4.h |    2 +-
 drivers/pinctrl/sh-pfc/pfc-r8a73a4.c          |   16 ++++++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)
arch/arm/mach-shmobile/include/mach/r8a73a4.h +1 -1
@@ -86,7 +86,7 @@ enum {
 	GPIO_PORT325, GPIO_PORT326, GPIO_PORT327, GPIO_PORT328, GPIO_PORT329,
 
 	/* Port0 */
-	GPIO_FN_LCDD0,
+	GPIO_FN_LCDD0 = 330,
 	GPIO_FN_PDM2_CLK_0,
 	GPIO_FN_DU0_DR0,
 	GPIO_FN_IRQ0,
drivers/pinctrl/sh-pfc/pfc-r8a73a4.c +16 -0
@@ -1424,6 +1424,20 @@ static struct sh_pfc_pin pinmux_pins[]  	GPIO_PORT_ALL(),
 };
 
+static const struct pinmux_range pinmux_ranges[] = {
+	{.begin = 0, .end = 30,},
+	{.begin = 32, .end = 40,},
+	{.begin = 64, .end = 85,},
+	{.begin = 96, .end = 126,},
+	{.begin = 128, .end = 134,},
+	{.begin = 160, .end = 178,},
+	{.begin = 192, .end = 222,},
+	{.begin = 224, .end = 250,},
+	{.begin = 256, .end = 283,},
+	{.begin = 288, .end = 308,},
+	{.begin = 320, .end = 329,},
+};
+
 #define PINMUX_FN_BASE	ARRAY_SIZE(pinmux_pins)
 
 static const struct pinmux_func pinmux_func_gpios[] = {

@@ -2815,6 +2829,8 @@ const struct sh_pfc_soc_info r8a73a4_pin
 
 	.pins = pinmux_pins,
 	.nr_pins = ARRAY_SIZE(pinmux_pins),
+	.ranges = pinmux_ranges,
+	.nr_ranges = ARRAY_SIZE(pinmux_ranges),
 	.func_gpios = pinmux_func_gpios,
 	.nr_func_gpios = ARRAY_SIZE(pinmux_func_gpios),
Acked-by on v1 2/13
#1 lore raw

Hi Magnus,

Thanks for the patch.

Quoted 7 lines · On Tuesday 26 March 2013 22:49:59 Magnus Damm wrote:

From: Magnus Damm <[email protected]>

The r8a73a4 SoC has sparse GPIO numbers. Declare ranges for pin numbers in the PFC SoC data. Pin numbers shall be used with the GPIO API from this point on.

Signed-off-by Magnus Damm <[email protected]>
Acked-by Laurent Pinchart <[email protected]>

drivers/pinctrl/sh-pfc/pfc-r8a73a4.c

Quoted 19 lines

---

 arch/arm/mach-shmobile/include/mach/r8a73a4.h |    2 +-
 drivers/pinctrl/sh-pfc/pfc-r8a73a4.c          |   16 ++++++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)
arch/arm/mach-shmobile/include/mach/r8a73a4.h +0 -0

20:20:54.000000000 +0900 @@ -86,7 +86,7 @@ enum {

arch/arm/mach-shmobile/include/mach/r8a73a4.h +1 -1
 	GPIO_PORT325, GPIO_PORT326, GPIO_PORT327, GPIO_PORT328, GPIO_PORT329,
 
 	/* Port0 */
-	GPIO_FN_LCDD0,
+	GPIO_FN_LCDD0 = 330,
 	GPIO_FN_PDM2_CLK_0,
 	GPIO_FN_DU0_DR0,
 	GPIO_FN_IRQ0,
drivers/pinctrl/sh-pfc/pfc-r8a73a4.c +0 -0

21:12:07.000000000

Quoted 6 lines

+0900 @@ -1424,6 +1424,20 @@ static struct sh_pfc_pin pinmux_pins[] > GPIO_PORT_ALL(),
};

drivers/pinctrl/sh-pfc/pfc-r8a73a4.c +3 -0
+static const struct pinmux_range pinmux_ranges[] = {
+	{.begin = 0, .end = 30,},
+	{.begin = 32, .end = 40,},

drivers/pinctrl/sh-pfc/pfc-r8a73a4.c static struct sh_pfc_pin pinmux_pins[] GPIO_PORT_ALL(), · line 1438

Quoted 10 lines
drivers/pinctrl/sh-pfc/pfc-r8a73a4.c +10 -0
+	{.begin = 64, .end = 85,},
+	{.begin = 96, .end = 126,},
+	{.begin = 128, .end = 134,},
+	{.begin = 160, .end = 178,},
+	{.begin = 192, .end = 222,},
+	{.begin = 224, .end = 250,},
+	{.begin = 256, .end = 283,},
+	{.begin = 288, .end = 308,},
+	{.begin = 320, .end = 329,},
+};

I wonder whether it wouldn't be a better idea to add the pin number to the sh_pfc_pin structure and compute the ranges array at runtime. Pin numbers would be hidden in the GPIO_PORT_ALL() macro. What's your opinion ?

I'm of course not asking you to implement that solution, your patch is fine as it is.

Quoted 12 lines
drivers/pinctrl/sh-pfc/pfc-r8a73a4.c +3 -0
+
 #define PINMUX_FN_BASE	ARRAY_SIZE(pinmux_pins)
 
 static const struct pinmux_func pinmux_func_gpios[] = {

@@ -2815,6 +2829,8 @@ const struct sh_pfc_soc_info r8a73a4_pin
 
 	.pins = pinmux_pins,
 	.nr_pins = ARRAY_SIZE(pinmux_pins),
+	.ranges = pinmux_ranges,
+	.nr_ranges = ARRAY_SIZE(pinmux_ranges),
 	.func_gpios = pinmux_func_gpios,
 	.nr_func_gpios = ARRAY_SIZE(pinmux_func_gpios),
Signature · 3 lines
Regards,

Laurent Pinchart
author maintainer
#2 lore raw
In reply to Magnus Damm

Hi Laurent,

Quoted 47 lines · Laurent Pinchart

Hi Magnus,

Thanks for the patch.

Quoted 7 lines · On Tuesday 26 March 2013 22:49:59 Magnus Damm wrote:

From: Magnus Damm <[email protected]>

The r8a73a4 SoC has sparse GPIO numbers. Declare ranges for pin numbers in the PFC SoC data. Pin numbers shall be used with the GPIO API from this point on.

Signed-off-by Magnus Damm <[email protected]>
Acked-by Laurent Pinchart <[email protected]>

drivers/pinctrl/sh-pfc/pfc-r8a73a4.c 2013-03-26

Quoted 19 lines

---

 arch/arm/mach-shmobile/include/mach/r8a73a4.h |    2 +-
 drivers/pinctrl/sh-pfc/pfc-r8a73a4.c          |   16 ++++++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)
arch/arm/mach-shmobile/include/mach/r8a73a4.h 2013-03-26 renamed from arch/arm/mach-shmobile/include/mach/r8a73a4.h +0 -0
20:20:54.000000000 +0900 @@ -86,7 +86,7 @@ enum {
      GPIO_PORT325, GPIO_PORT326, GPIO_PORT327, GPIO_PORT328, GPIO_PORT329,

      /* Port0 */
arch/arm/mach-shmobile/include/mach/r8a73a4.h 2013-03-26 renamed from arch/arm/mach-shmobile/include/mach/r8a73a4.h +1 -1
-     GPIO_FN_LCDD0,
+     GPIO_FN_LCDD0 = 330,
      GPIO_FN_PDM2_CLK_0,
      GPIO_FN_DU0_DR0,
      GPIO_FN_IRQ0,
drivers/pinctrl/sh-pfc/pfc-r8a73a4.c 2013-03-26 renamed from drivers/pinctrl/sh-pfc/pfc-r8a73a4.c +0 -0

21:12:07.000000000

Quoted 12 lines

+0900 @@ -1424,6 +1424,20 @@ static struct sh_pfc_pin pinmux_pins[] >> GPIO_PORT_ALL(),
};

drivers/pinctrl/sh-pfc/pfc-r8a73a4.c 2013-03-26 renamed from drivers/pinctrl/sh-pfc/pfc-r8a73a4.c +9 -0
+static const struct pinmux_range pinmux_ranges[] = {
+     {.begin = 0, .end = 30,},
+     {.begin = 32, .end = 40,},
+     {.begin = 64, .end = 85,},
+     {.begin = 96, .end = 126,},
+     {.begin = 128, .end = 134,},
+     {.begin = 160, .end = 178,},
+     {.begin = 192, .end = 222,},
+     {.begin = 224, .end = 250,},
Quoted 8 lines

drivers/pinctrl/sh-pfc/pfc-r8a73a4.c 2013-03-26

Quoted 4 lines
drivers/pinctrl/sh-pfc/pfc-r8a73a4.c 2013-03-26 renamed from drivers/pinctrl/sh-pfc/pfc-r8a73a4.c +4 -0
+     {.begin = 256, .end = 283,},
+     {.begin = 288, .end = 308,},
+     {.begin = 320, .end = 329,},
+};

I wonder whether it wouldn't be a better idea to add the pin number to the sh_pfc_pin structure and compute the ranges array at runtime. Pin numbers would be hidden in the GPIO_PORT_ALL() macro. What's your opinion ?

I think the current duplication is unfortunate, so some other way to handle this would be nice. Keeping it with sh_pfc_pin sounds like a good place to me.

At the same time, I'm not sure what our current memory footprint is. It may be rather pointless these days with development boards with 4 GiB on them, but still. The old PFC code and the INTC were developed to be both efficient in hot paths and as small as possible from a memory foot print point of view.

So I'd like this new-and-improved code to be as small as possible too. But from a priority point of view, avoiding duplication of information must be much more important. So please try to figure out something good.

Quoted 2 lines

I'm of course not asking you to implement that solution, your patch is fine as it is.

Thanks!

/ magnus