diff mbox series

[v2,10/12] pinctrl: renesas: rzg2l: Add support for RZ/V2N SoC

Message ID 20250407191628.323613-11-prabhakar.mahadev-lad.rj@bp.renesas.com
State New
Headers show
Series Add support for Renesas RZ/V2N SoC and EVK | expand

Commit Message

Lad, Prabhakar April 7, 2025, 7:16 p.m. UTC 
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Add pinctrl support for the Renesas RZ/V2N SoC by reusing the existing
RZ/V2H(P) pin configuration data. The PFC block is nearly identical, with
the only difference being the absence of `PCIE1_RSTOUTB` on RZ/V2N.

To accommodate this, move the `PCIE1_RSTOUTB` entry to the end of the
`rzv2h_dedicated_pins` array and set `.n_dedicated_pins` to
`ARRAY_SIZE(rzv2h_dedicated_pins) - 1` in the RZ/V2N OF data.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v1->v2:
- No changes in the code.
---
 drivers/pinctrl/renesas/Kconfig         |  1 +
 drivers/pinctrl/renesas/pinctrl-rzg2l.c | 36 ++++++++++++++++++++++++-
 2 files changed, 36 insertions(+), 1 deletion(-)

Comments

Geert Uytterhoeven April 10, 2025, 10:19 a.m. UTC | #1 
Hi Prabhakar,

On Mon, 7 Apr 2025 at 21:16, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Add pinctrl support for the Renesas RZ/V2N SoC by reusing the existing
> RZ/V2H(P) pin configuration data. The PFC block is nearly identical, with
> the only difference being the absence of `PCIE1_RSTOUTB` on RZ/V2N.
>
> To accommodate this, move the `PCIE1_RSTOUTB` entry to the end of the
> `rzv2h_dedicated_pins` array and set `.n_dedicated_pins` to
> `ARRAY_SIZE(rzv2h_dedicated_pins) - 1` in the RZ/V2N OF data.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Suggestion for improvement below.

> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> @@ -2304,7 +2304,6 @@ static struct rzg2l_dedicated_configs rzv2h_dedicated_pins[] = {
>         { "SD1DAT3", RZG2L_SINGLE_PIN_PACK(0xc, 3, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
>                                                     PIN_CFG_IEN | PIN_CFG_PUPD)) },
>         { "PCIE0_RSTOUTB", RZG2L_SINGLE_PIN_PACK(0xe, 0, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR)) },
> -       { "PCIE1_RSTOUTB", RZG2L_SINGLE_PIN_PACK(0xe, 1, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR)) },
>         { "ET0_MDIO", RZG2L_SINGLE_PIN_PACK(0xf, 0, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
>                                                      PIN_CFG_IEN | PIN_CFG_PUPD)) },
>         { "ET0_MDC", RZG2L_SINGLE_PIN_PACK(0xf, 1, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
> @@ -2359,6 +2358,14 @@ static struct rzg2l_dedicated_configs rzv2h_dedicated_pins[] = {
>         { "ET1_RXD1", RZG2L_SINGLE_PIN_PACK(0x14, 5, (PIN_CFG_PUPD)) },
>         { "ET1_RXD2", RZG2L_SINGLE_PIN_PACK(0x14, 6, (PIN_CFG_PUPD)) },
>         { "ET1_RXD3", RZG2L_SINGLE_PIN_PACK(0x14, 7, (PIN_CFG_PUPD)) },
> +
> +       /*
> +        * This pin is only available on the RZ/V2H(P) SoC and not on the RZ/V2N.
> +        * Since this array is shared with the RZ/V2N SoC, this entry should be placed
> +        * at the end. This ensures that on the RZ/V2N, we can set
> +        * `.n_dedicated_pins = ARRAY_SIZE(rzv2h_dedicated_pins) - 1,`.
> +        */
> +       { "PCIE1_RSTOUTB", RZG2L_SINGLE_PIN_PACK(0xe, 1, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR)) },
>  };

Alternatively, you can replace the single array by a structure
containing two arrays, one for common pins, and a second
for V2H-only pins, like the common and automotive arrays in
e.g. drivers/pinctrl/renesas/pfc-r8a7791.c.  That would get rid of
the literal "- 1" (and the need for a comment ;-), and would protect
against future mistakes.

Gr{oetje,eeting}s,

                        Geert
Lad, Prabhakar April 14, 2025, 8:34 a.m. UTC | #2 
Hi Geert,

Thank you for the review.

On Thu, Apr 10, 2025 at 11:20 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Mon, 7 Apr 2025 at 21:16, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Add pinctrl support for the Renesas RZ/V2N SoC by reusing the existing
> > RZ/V2H(P) pin configuration data. The PFC block is nearly identical, with
> > the only difference being the absence of `PCIE1_RSTOUTB` on RZ/V2N.
> >
> > To accommodate this, move the `PCIE1_RSTOUTB` entry to the end of the
> > `rzv2h_dedicated_pins` array and set `.n_dedicated_pins` to
> > `ARRAY_SIZE(rzv2h_dedicated_pins) - 1` in the RZ/V2N OF data.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Thanks for your patch!
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> Suggestion for improvement below.
>
> > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > @@ -2304,7 +2304,6 @@ static struct rzg2l_dedicated_configs rzv2h_dedicated_pins[] = {
> >         { "SD1DAT3", RZG2L_SINGLE_PIN_PACK(0xc, 3, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
> >                                                     PIN_CFG_IEN | PIN_CFG_PUPD)) },
> >         { "PCIE0_RSTOUTB", RZG2L_SINGLE_PIN_PACK(0xe, 0, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR)) },
> > -       { "PCIE1_RSTOUTB", RZG2L_SINGLE_PIN_PACK(0xe, 1, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR)) },
> >         { "ET0_MDIO", RZG2L_SINGLE_PIN_PACK(0xf, 0, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
> >                                                      PIN_CFG_IEN | PIN_CFG_PUPD)) },
> >         { "ET0_MDC", RZG2L_SINGLE_PIN_PACK(0xf, 1, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
> > @@ -2359,6 +2358,14 @@ static struct rzg2l_dedicated_configs rzv2h_dedicated_pins[] = {
> >         { "ET1_RXD1", RZG2L_SINGLE_PIN_PACK(0x14, 5, (PIN_CFG_PUPD)) },
> >         { "ET1_RXD2", RZG2L_SINGLE_PIN_PACK(0x14, 6, (PIN_CFG_PUPD)) },
> >         { "ET1_RXD3", RZG2L_SINGLE_PIN_PACK(0x14, 7, (PIN_CFG_PUPD)) },
> > +
> > +       /*
> > +        * This pin is only available on the RZ/V2H(P) SoC and not on the RZ/V2N.
> > +        * Since this array is shared with the RZ/V2N SoC, this entry should be placed
> > +        * at the end. This ensures that on the RZ/V2N, we can set
> > +        * `.n_dedicated_pins = ARRAY_SIZE(rzv2h_dedicated_pins) - 1,`.
> > +        */
> > +       { "PCIE1_RSTOUTB", RZG2L_SINGLE_PIN_PACK(0xe, 1, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR)) },
> >  };
>
> Alternatively, you can replace the single array by a structure
> containing two arrays, one for common pins, and a second
> for V2H-only pins, like the common and automotive arrays in
> e.g. drivers/pinctrl/renesas/pfc-r8a7791.c.  That would get rid of
> the literal "- 1" (and the need for a comment ;-), and would protect
> against future mistakes.
>
My initial intention was to do the above, but it was generating a lot
of diff so choose this approach. I'll switch back to above in v3.

Cheers,
Prabhakar
diff mbox series

Patch

diff --git a/drivers/pinctrl/renesas/Kconfig b/drivers/pinctrl/renesas/Kconfig
index 3c18d908b21e..e16034fc1bbf 100644
--- a/drivers/pinctrl/renesas/Kconfig
+++ b/drivers/pinctrl/renesas/Kconfig
@@ -42,6 +42,7 @@  config PINCTRL_RENESAS
 	select PINCTRL_RZG2L if ARCH_RZG2L
 	select PINCTRL_RZV2M if ARCH_R9A09G011
 	select PINCTRL_RZG2L if ARCH_R9A09G047
+	select PINCTRL_RZG2L if ARCH_R9A09G056
 	select PINCTRL_RZG2L if ARCH_R9A09G057
 	select PINCTRL_PFC_SH7203 if CPU_SUBTYPE_SH7203
 	select PINCTRL_PFC_SH7264 if CPU_SUBTYPE_SH7264
diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index c72e250f4a15..ae5e040f3276 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -2304,7 +2304,6 @@  static struct rzg2l_dedicated_configs rzv2h_dedicated_pins[] = {
 	{ "SD1DAT3", RZG2L_SINGLE_PIN_PACK(0xc, 3, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
 						    PIN_CFG_IEN | PIN_CFG_PUPD)) },
 	{ "PCIE0_RSTOUTB", RZG2L_SINGLE_PIN_PACK(0xe, 0, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR)) },
-	{ "PCIE1_RSTOUTB", RZG2L_SINGLE_PIN_PACK(0xe, 1, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR)) },
 	{ "ET0_MDIO", RZG2L_SINGLE_PIN_PACK(0xf, 0, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
 						     PIN_CFG_IEN | PIN_CFG_PUPD)) },
 	{ "ET0_MDC", RZG2L_SINGLE_PIN_PACK(0xf, 1, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR |
@@ -2359,6 +2358,14 @@  static struct rzg2l_dedicated_configs rzv2h_dedicated_pins[] = {
 	{ "ET1_RXD1", RZG2L_SINGLE_PIN_PACK(0x14, 5, (PIN_CFG_PUPD)) },
 	{ "ET1_RXD2", RZG2L_SINGLE_PIN_PACK(0x14, 6, (PIN_CFG_PUPD)) },
 	{ "ET1_RXD3", RZG2L_SINGLE_PIN_PACK(0x14, 7, (PIN_CFG_PUPD)) },
+
+	/*
+	 * This pin is only available on the RZ/V2H(P) SoC and not on the RZ/V2N.
+	 * Since this array is shared with the RZ/V2N SoC, this entry should be placed
+	 * at the end. This ensures that on the RZ/V2N, we can set
+	 * `.n_dedicated_pins = ARRAY_SIZE(rzv2h_dedicated_pins) - 1,`.
+	 */
+	{ "PCIE1_RSTOUTB", RZG2L_SINGLE_PIN_PACK(0xe, 1, (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR)) },
 };
 
 static struct rzg2l_dedicated_configs rzg3e_dedicated_pins[] = {
@@ -3349,6 +3356,29 @@  static struct rzg2l_pinctrl_data r9a09g047_data = {
 	.bias_param_to_hw = &rzv2h_bias_param_to_hw,
 };
 
+static struct rzg2l_pinctrl_data r9a09g056_data = {
+	.port_pins = rzv2h_gpio_names,
+	.port_pin_configs = r9a09g057_gpio_configs,
+	.n_ports = ARRAY_SIZE(r9a09g057_gpio_configs),
+	.dedicated_pins = rzv2h_dedicated_pins,
+	.n_port_pins = ARRAY_SIZE(r9a09g057_gpio_configs) * RZG2L_PINS_PER_PORT,
+	.n_dedicated_pins = ARRAY_SIZE(rzv2h_dedicated_pins) - 1,
+	.hwcfg = &rzv2h_hwcfg,
+	.variable_pin_cfg = r9a09g057_variable_pin_cfg,
+	.n_variable_pin_cfg = ARRAY_SIZE(r9a09g057_variable_pin_cfg),
+	.num_custom_params = ARRAY_SIZE(renesas_rzv2h_custom_bindings),
+	.custom_params = renesas_rzv2h_custom_bindings,
+#ifdef CONFIG_DEBUG_FS
+	.custom_conf_items = renesas_rzv2h_conf_items,
+#endif
+	.pwpr_pfc_lock_unlock = &rzv2h_pwpr_pfc_lock_unlock,
+	.pmc_writeb = &rzv2h_pmc_writeb,
+	.oen_read = &rzv2h_oen_read,
+	.oen_write = &rzv2h_oen_write,
+	.hw_to_bias_param = &rzv2h_hw_to_bias_param,
+	.bias_param_to_hw = &rzv2h_bias_param_to_hw,
+};
+
 static struct rzg2l_pinctrl_data r9a09g057_data = {
 	.port_pins = rzv2h_gpio_names,
 	.port_pin_configs = r9a09g057_gpio_configs,
@@ -3389,6 +3419,10 @@  static const struct of_device_id rzg2l_pinctrl_of_table[] = {
 		.compatible = "renesas,r9a09g047-pinctrl",
 		.data = &r9a09g047_data,
 	},
+	{
+		.compatible = "renesas,r9a09g056-pinctrl",
+		.data = &r9a09g056_data,
+	},
 	{
 		.compatible = "renesas,r9a09g057-pinctrl",
 		.data = &r9a09g057_data,