Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230323144833.28562-1-clin@suse.com> <20230323144833.28562-4-clin@suse.com>
In-Reply-To: <20230323144833.28562-4-clin@suse.com>
From:   Andy Shevchenko <andy.shevchenko@gmail.com>
Date:   Thu, 23 Mar 2023 20:44:28 +0200
Message-ID: <CAHp75Vfue6TNNbNh8b_fCFKF7+rACd=hgXdoQd7drfojDWOvUw@mail.gmail.com>
Subject: Re: [PATCH v3 3/6] pinctrl: s32cc: refactor pin config parsing
To:     Chester Lin <clin@suse.com>
Cc:     Linus Walleij <linus.walleij@linaro.org>,
        NXP S32 Linux Team <s32@nxp.com>,
        linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org,
        Ghennadi Procopciuc <Ghennadi.Procopciuc@oss.nxp.com>,
        Andrei Stefanescu <andrei.stefanescu@nxp.com>,
        Radu Pirea <radu-nicolae.pirea@nxp.com>,
        =?UTF-8?Q?Andreas_F=C3=A4rber?= <afaerber@suse.de>,
        Matthias Brugger <mbrugger@suse.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Mar 23, 2023 at 4:49=E2=80=AFPM Chester Lin <clin@suse.com> wrote:
>
> Move common codes into smaller inline functions and remove argument check=
s
> that are not actually used by pull up/down bits in S32 MSCR register.

in the S32

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Chester Lin <clin@suse.com>
> ---
> Changes v3:
> - Move the PIN_CONFIG_BIAS_DISABLE case to be with PU and PD cases since
>   they have the same function call.
> - Roll back the argument checks of OUTPUT_ENABLE and INPUT_ENABLE cases
>   since they were wrongly removed with the PU & PD parts in v2.
>
>  drivers/pinctrl/nxp/pinctrl-s32cc.c | 52 ++++++++++++++++++-----------
>  1 file changed, 33 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/pinctrl/nxp/pinctrl-s32cc.c b/drivers/pinctrl/nxp/pi=
nctrl-s32cc.c
> index f698e1a240ef..36f323f87785 100644
> --- a/drivers/pinctrl/nxp/pinctrl-s32cc.c
> +++ b/drivers/pinctrl/nxp/pinctrl-s32cc.c
> @@ -474,11 +474,38 @@ static int s32_get_slew_regval(int arg)
>         return -EINVAL;
>  }
>
> -static int s32_get_pin_conf(enum pin_config_param param, u32 arg,
> -                           unsigned int *mask, unsigned int *config)
> +static inline void s32_pin_set_pull(enum pin_config_param param,
> +                                  unsigned int *mask, unsigned int *conf=
ig)
>  {
> +       switch (param) {
> +       case PIN_CONFIG_BIAS_DISABLE:
> +       case PIN_CONFIG_BIAS_HIGH_IMPEDANCE:
> +               *config &=3D ~(S32_MSCR_PUS | S32_MSCR_PUE);
> +               break;
> +       case PIN_CONFIG_BIAS_PULL_UP:
> +               *config |=3D S32_MSCR_PUS | S32_MSCR_PUE;
> +               break;
> +       case PIN_CONFIG_BIAS_PULL_DOWN:
> +               *config &=3D ~S32_MSCR_PUS;
> +               *config |=3D S32_MSCR_PUE;
> +               break;
> +       default:
> +               return;
> +       }
> +
> +       *mask |=3D S32_MSCR_PUS | S32_MSCR_PUE;
> +}
> +
> +static int s32_parse_pincfg(unsigned long pincfg, unsigned int *mask,
> +                           unsigned int *config)
> +{
> +       enum pin_config_param param;
> +       u32 arg;
>         int ret;
>
> +       param =3D pinconf_to_config_param(pincfg);
> +       arg =3D pinconf_to_config_argument(pincfg);
> +
>         switch (param) {
>         /* All pins are persistent over suspend */
>         case PIN_CONFIG_PERSIST_STATE:
> @@ -508,26 +535,15 @@ static int s32_get_pin_conf(enum pin_config_param p=
aram, u32 arg,
>                 *config |=3D S32_MSCR_SRE((u32)ret);
>                 *mask |=3D S32_MSCR_SRE(~0);
>                 break;
> +       case PIN_CONFIG_BIAS_DISABLE:
>         case PIN_CONFIG_BIAS_PULL_UP:
> -               if (arg)
> -                       *config |=3D S32_MSCR_PUS;
> -               else
> -                       *config &=3D ~S32_MSCR_PUS;
> -               fallthrough;
>         case PIN_CONFIG_BIAS_PULL_DOWN:
> -               if (arg)
> -                       *config |=3D S32_MSCR_PUE;
> -               else
> -                       *config &=3D ~S32_MSCR_PUE;
> -               *mask |=3D S32_MSCR_PUE | S32_MSCR_PUS;
> +               s32_pin_set_pull(param, mask, config);
>                 break;
>         case PIN_CONFIG_BIAS_HIGH_IMPEDANCE:
>                 *config &=3D ~(S32_MSCR_ODE | S32_MSCR_OBE | S32_MSCR_IBE=
);
>                 *mask |=3D S32_MSCR_ODE | S32_MSCR_OBE | S32_MSCR_IBE;
> -               fallthrough;
> -       case PIN_CONFIG_BIAS_DISABLE:
> -               *config &=3D ~(S32_MSCR_PUS | S32_MSCR_PUE);
> -               *mask |=3D S32_MSCR_PUS | S32_MSCR_PUE;
> +               s32_pin_set_pull(param, mask, config);
>                 break;
>         default:
>                 return -EOPNOTSUPP;
> @@ -553,9 +569,7 @@ static int s32_pinconf_mscr_update(struct pinctrl_dev=
 *pctldev,
>                 pin_get_name(pctldev, pin_id), num_configs);
>
>         for (i =3D 0; i < num_configs; i++) {
> -               ret =3D s32_get_pin_conf(pinconf_to_config_param(configs[=
i]),
> -                                      pinconf_to_config_argument(configs=
[i]),
> -                                      &mask, &config);
> +               ret =3D s32_parse_pincfg(configs[i], &mask, &config);
>                 if (ret)
>                         return ret;
>         }
> --
> 2.37.3
>


--=20
With Best Regards,
Andy Shevchenko