Hi Geert, Thank you for the review. On Thu, Aug 29, 2024 at 2:15 PM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote: > > 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. > Ok, I'll update the commit description to below: 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 most cases within the switch statement. > > 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; > And while at it I'll replace as above and below in the v2. Cheers, Prabhakar > > @@ -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