On Mon, Mar 20, 2023 at 6:39 PM Chester Lin <clin@xxxxxxxx> wrote: > > Move common codes into smaller inline functions and remove some argument > handlings that are not actually used by either S32 MSCR register or generic > config params. ... > case PIN_CONFIG_OUTPUT_ENABLE: > - if (arg) > - *config |= S32_MSCR_OBE; > - else > - *config &= ~S32_MSCR_OBE; > + *config |= S32_MSCR_OBE; > *mask |= S32_MSCR_OBE; > break; > case PIN_CONFIG_INPUT_ENABLE: > - if (arg) > - *config |= S32_MSCR_IBE; > - else > - *config &= ~S32_MSCR_IBE; > + *config |= S32_MSCR_IBE; > *mask |= S32_MSCR_IBE; > break; Isn't it a regression here? Otherwise needs an explicit explanation in the commit message on what's going on here and why it's not a regression. ... > case PIN_CONFIG_BIAS_DISABLE: > - *config &= ~(S32_MSCR_PUS | S32_MSCR_PUE); > - *mask |= S32_MSCR_PUS | S32_MSCR_PUE; > + s32_pin_set_pull(param, mask, config); > break; This now can be unified with PU and PD cases above. -- With Best Regards, Andy Shevchenko