Hi John, Prabhakar, On Wed, 2 Jul 2025 at 02:57, John Madieu <john.madieu.xa@xxxxxxxxxxxxxx> wrote: > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > > Add support to configure the PFC_OEN register on the RZ/G3E SoC for > specific pins that require direction control via output-enable. > > On the RZ/G3E SoC, certain pins such as TXC_TXCLK must be switchable > between input and output modes depending on the PHY interface mode > (MII or RGMII). This behavior maps to the `output-enable` property in > the device tree and requires configuring the PFC_OEN register. > > Update the r9a09g047_variable_pin_cfg array to include PB1, PE1, PL0, > PL1, PL2, and PL4 with PIN_CFG_OEN flags to indicate support for this > feature. Define a new rzg3e_hwcfg structure with SoC-specific pin names > used for OEN bit mapping. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> Thanks for your patch! > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c > @@ -3283,6 +3307,19 @@ static const char * const rzv2h_oen_pin_names[] = { > "XSPI0_CKN", "XSPI0_CKP" > }; > > +static const char * const rzg3e_oen_pin_names[] = { > + "PB1", "PE1", "PL4", "PL1", "PL2", "PL0" > +}; > + > +static const struct rzg2l_hwcfg rzg3e_hwcfg = { > + .regs = { > + .pwpr = 0x3c04, > + }, > + .tint_start_index = 17, > + .oen_pin_names = rzg3e_oen_pin_names, > + .oen_pin_names_len = ARRAY_SIZE(rzg3e_oen_pin_names), > +}; > + > static const struct rzg2l_hwcfg rzv2h_hwcfg = { > .regs = { > .pwpr = 0x3c04, > @@ -3352,7 +3389,7 @@ static struct rzg2l_pinctrl_data r9a09g047_data = { > .dedicated_pins = rzg3e_dedicated_pins, > .n_port_pins = ARRAY_SIZE(r9a09g047_gpio_configs) * RZG2L_PINS_PER_PORT, > .n_dedicated_pins = ARRAY_SIZE(rzg3e_dedicated_pins), > - .hwcfg = &rzv2h_hwcfg, > + .hwcfg = &rzg3e_hwcfg, > .variable_pin_cfg = r9a09g047_variable_pin_cfg, > .n_variable_pin_cfg = ARRAY_SIZE(r9a09g047_variable_pin_cfg), > .num_custom_params = ARRAY_SIZE(renesas_rzv2h_custom_bindings), I would rather use the existing .oen_{read,write}() abstraction, and thus provide new rzg3e_oen_{read,write}() implementations: - .oen_read = &rzv2h_oen_read, - .oen_write = &rzv2h_oen_write, + .oen_read = &rzg3e_oen_read, + .oen_write = &rzg3e_oen_write, Of course this requires refactoring the existing rzv2h_pin_to_oen_bit() and rzv2h_oen_{read,write}() functions to avoid duplication. Do you agree? The actual pin parts LGTM. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx 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