| Message ID | 20240524094603.988-3-paul.barker.ct@bp.renesas.com |
|---|---|
| State | New |
| Headers | show |
| Series | Configure GbEth for RGMII on RZ/G2L family | expand |
Hi Paul, On Fri, May 24, 2024 at 11:46 AM Paul Barker <paul.barker.ct@bp.renesas.com> wrote: > We currently support setting OEN (Output ENable) bits only for the > RZ/G3S SoC and so the functions rzg2l_oen_is_supported() and > rzg2l_pin_to_oen_bit() are hardcoded for the RZ/G3S. To prepare for > supporting OEN on SoCs in the RZ/G2L family, we need to make this code > more flexible. > > So, the rzg2l_oen_is_supported() and rzg2l_pin_to_oen_bit() functions > are replaced with a single translation function which is called via a > pin_to_oen_bit function pointer and returns an error code if OEN is not > supported for the given pin. > > Signed-off-by: Paul Barker <paul.barker.ct@bp.renesas.com> Thanks for your patch! > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c > @@ -256,6 +256,8 @@ struct rzg2l_pinctrl_data { > const struct rzg2l_hwcfg *hwcfg; > const struct rzg2l_variable_pin_cfg *variable_pin_cfg; > unsigned int n_variable_pin_cfg; > + int (*pin_to_oen_bit)(const struct rzg2l_hwcfg *hwcfg, > + u32 caps, u32 offset, u8 pin); > }; This definitely needs synchronization with Prabhakar, as he introduces a different set of function pointers to distinguish RZ/G2L (G3S) and RZ/V2H. We really like to end up with something that is consistent, and works for all. Gr{oetje,eeting}s, Geert
On 30/05/2024 13:51, Geert Uytterhoeven wrote: > Hi Paul, > > On Fri, May 24, 2024 at 11:46 AM Paul Barker > <paul.barker.ct@bp.renesas.com> wrote: >> We currently support setting OEN (Output ENable) bits only for the >> RZ/G3S SoC and so the functions rzg2l_oen_is_supported() and >> rzg2l_pin_to_oen_bit() are hardcoded for the RZ/G3S. To prepare for >> supporting OEN on SoCs in the RZ/G2L family, we need to make this code >> more flexible. >> >> So, the rzg2l_oen_is_supported() and rzg2l_pin_to_oen_bit() functions >> are replaced with a single translation function which is called via a >> pin_to_oen_bit function pointer and returns an error code if OEN is not >> supported for the given pin. >> >> Signed-off-by: Paul Barker <paul.barker.ct@bp.renesas.com> > > Thanks for your patch! > >> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c >> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c >> @@ -256,6 +256,8 @@ struct rzg2l_pinctrl_data { >> const struct rzg2l_hwcfg *hwcfg; >> const struct rzg2l_variable_pin_cfg *variable_pin_cfg; >> unsigned int n_variable_pin_cfg; >> + int (*pin_to_oen_bit)(const struct rzg2l_hwcfg *hwcfg, >> + u32 caps, u32 offset, u8 pin); >> }; > > This definitely needs synchronization with Prabhakar, as he introduces > a different set of function pointers to distinguish RZ/G2L (G3S) and > RZ/V2H. We really like to end up with something that is consistent, > and works for all. Apologies that we missed this conflict! We will have to use Prabhakar's approach. The methods for RZ/G2L & RZ/G3S are similar enough to share read/write functions and just have separate functions for determining which bit to set, but for RZ/V2H we're writing to a completely different register. So, please proceed with Prabhakar's patches. I'll rebase this series on top of his and re-work the relevant bits. Thanks,
diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c index 724308cd5a37..08c68b95e67f 100644 --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c @@ -256,6 +256,8 @@ struct rzg2l_pinctrl_data { const struct rzg2l_hwcfg *hwcfg; const struct rzg2l_variable_pin_cfg *variable_pin_cfg; unsigned int n_variable_pin_cfg; + int (*pin_to_oen_bit)(const struct rzg2l_hwcfg *hwcfg, + u32 caps, u32 offset, u8 pin); }; /** @@ -1014,22 +1016,14 @@ static bool rzg2l_ds_is_supported(struct rzg2l_pinctrl *pctrl, u32 caps, return false; } -static bool rzg2l_oen_is_supported(u32 caps, u8 pin, u8 max_pin) -{ - if (!(caps & PIN_CFG_OEN)) - return false; - - if (pin > max_pin) - return false; - - return true; -} - -static u8 rzg2l_pin_to_oen_bit(u32 port, u8 pin, u8 max_port) +static int rzg3s_pin_to_oen_bit(const struct rzg2l_hwcfg *hwcfg, u32 caps, u32 port, u8 pin) { u8 bit = pin * 2; - if (port == max_port) + if (!(caps & PIN_CFG_OEN) || pin > hwcfg->oen_max_pin) + return -EINVAL; + + if (port == hwcfg->oen_max_port) bit += 1; return bit; @@ -1037,29 +1031,30 @@ static u8 rzg2l_pin_to_oen_bit(u32 port, u8 pin, u8 max_port) static u32 rzg2l_read_oen(struct rzg2l_pinctrl *pctrl, u32 caps, u32 port, u8 pin) { - u8 max_port = pctrl->data->hwcfg->oen_max_port; - u8 max_pin = pctrl->data->hwcfg->oen_max_pin; - u8 bit; + int bit; - if (!rzg2l_oen_is_supported(caps, pin, max_pin)) + if (!pctrl->data->pin_to_oen_bit) return 0; - bit = rzg2l_pin_to_oen_bit(port, pin, max_port); + bit = pctrl->data->pin_to_oen_bit(pctrl->data->hwcfg, caps, port, pin); + if (bit < 0) + return 0; return !(readb(pctrl->base + ETH_MODE) & BIT(bit)); } static int rzg2l_write_oen(struct rzg2l_pinctrl *pctrl, u32 caps, u32 port, u8 pin, u8 oen) { - u8 max_port = pctrl->data->hwcfg->oen_max_port; - u8 max_pin = pctrl->data->hwcfg->oen_max_pin; unsigned long flags; - u8 val, bit; + int bit; + u8 val; - if (!rzg2l_oen_is_supported(caps, pin, max_pin)) - return -EINVAL; + if (!pctrl->data->pin_to_oen_bit) + return -EOPNOTSUPP; - bit = rzg2l_pin_to_oen_bit(port, pin, max_port); + bit = pctrl->data->pin_to_oen_bit(pctrl->data->hwcfg, caps, port, pin); + if (bit < 0) + return bit; spin_lock_irqsave(&pctrl->lock, flags); val = readb(pctrl->base + ETH_MODE); @@ -2705,6 +2700,7 @@ static struct rzg2l_pinctrl_data r9a08g045_data = { .n_port_pins = ARRAY_SIZE(r9a08g045_gpio_configs) * RZG2L_PINS_PER_PORT, .n_dedicated_pins = ARRAY_SIZE(rzg3s_dedicated_pins), .hwcfg = &rzg3s_hwcfg, + .pin_to_oen_bit = rzg3s_pin_to_oen_bit, }; static const struct of_device_id rzg2l_pinctrl_of_table[] = {
We currently support setting OEN (Output ENable) bits only for the RZ/G3S SoC and so the functions rzg2l_oen_is_supported() and rzg2l_pin_to_oen_bit() are hardcoded for the RZ/G3S. To prepare for supporting OEN on SoCs in the RZ/G2L family, we need to make this code more flexible. So, the rzg2l_oen_is_supported() and rzg2l_pin_to_oen_bit() functions are replaced with a single translation function which is called via a pin_to_oen_bit function pointer and returns an error code if OEN is not supported for the given pin. Signed-off-by: Paul Barker <paul.barker.ct@bp.renesas.com> --- drivers/pinctrl/renesas/pinctrl-rzg2l.c | 44 +++++++++++-------------- 1 file changed, 20 insertions(+), 24 deletions(-)