Hi Prabhakar, Thanks for your patch! On Tue, Aug 27, 2024 at 3:17 PM Prabhakar <prabhakar.csengg@xxxxxxxxx> wrote: > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > > Refactor the `rzg2l_pinctrl_pinconf_set()` function by moving the call to > `arg = pinconf_to_config_argument(_configs[i])` to the beginning of the > loop. Previously, this call was redundantly made in each case of the > switch statement. This is not 100% true: the PIN_CONFIG_BIAS_* cases do not call pinconf_to_config_argument(). But I agree that calling it unconditionally doesn't harm. > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c > @@ -1395,7 +1395,6 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev, > break; > > case PIN_CONFIG_OUTPUT_ENABLE: > - arg = pinconf_to_config_argument(_configs[i]); > if (!(cfg & PIN_CFG_OEN)) > return -EINVAL; > if (!pctrl->data->oen_write) Missed opportunity for simplification: case PIN_CONFIG_POWER_SOURCE: - settings.power_source = pinconf_to_config_argument(_configs[i]); + settings.power_source = arg; break; > @@ -1432,8 +1429,6 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev, > break; > > case PIN_CONFIG_DRIVE_STRENGTH: > - arg = pinconf_to_config_argument(_configs[i]); > - > if (!(cfg & PIN_CFG_IOLH_A) || hwcfg->drive_strength_ua) > return -EINVAL; > case PIN_CONFIG_DRIVE_STRENGTH_UA: ... - settings.drive_strength_ua = pinconf_to_config_argument(_configs[i]); + settings.drive_strength_ua = arg; break; The rest 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