[v3,3/6] pinctrl: s32cc: refactor pin config parsing

Message ID 20230323144833.28562-4-clin@suse.com
State New
Headers
Series pinctrl: s32: driver improvements and generic struct use |

Commit Message

Chester Lin March 23, 2023, 2:48 p.m. UTC
  Move common codes into smaller inline functions and remove argument checks
that are not actually used by pull up/down bits in S32 MSCR register.

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(-)
  

Comments

Andy Shevchenko March 23, 2023, 6:44 p.m. UTC | #1
On Thu, Mar 23, 2023 at 4:49 PM Chester Lin <clin@suse.com> wrote:
>
> Move common codes into smaller inline functions and remove argument checks
> 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/pinctrl-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 *config)
>  {
> +       switch (param) {
> +       case PIN_CONFIG_BIAS_DISABLE:
> +       case PIN_CONFIG_BIAS_HIGH_IMPEDANCE:
> +               *config &= ~(S32_MSCR_PUS | S32_MSCR_PUE);
> +               break;
> +       case PIN_CONFIG_BIAS_PULL_UP:
> +               *config |= S32_MSCR_PUS | S32_MSCR_PUE;
> +               break;
> +       case PIN_CONFIG_BIAS_PULL_DOWN:
> +               *config &= ~S32_MSCR_PUS;
> +               *config |= S32_MSCR_PUE;
> +               break;
> +       default:
> +               return;
> +       }
> +
> +       *mask |= 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 = pinconf_to_config_param(pincfg);
> +       arg = 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 param, u32 arg,
>                 *config |= S32_MSCR_SRE((u32)ret);
>                 *mask |= S32_MSCR_SRE(~0);
>                 break;
> +       case PIN_CONFIG_BIAS_DISABLE:
>         case PIN_CONFIG_BIAS_PULL_UP:
> -               if (arg)
> -                       *config |= S32_MSCR_PUS;
> -               else
> -                       *config &= ~S32_MSCR_PUS;
> -               fallthrough;
>         case PIN_CONFIG_BIAS_PULL_DOWN:
> -               if (arg)
> -                       *config |= S32_MSCR_PUE;
> -               else
> -                       *config &= ~S32_MSCR_PUE;
> -               *mask |= S32_MSCR_PUE | S32_MSCR_PUS;
> +               s32_pin_set_pull(param, mask, config);
>                 break;
>         case PIN_CONFIG_BIAS_HIGH_IMPEDANCE:
>                 *config &= ~(S32_MSCR_ODE | S32_MSCR_OBE | S32_MSCR_IBE);
>                 *mask |= S32_MSCR_ODE | S32_MSCR_OBE | S32_MSCR_IBE;
> -               fallthrough;
> -       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;
>         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 = 0; i < num_configs; i++) {
> -               ret = s32_get_pin_conf(pinconf_to_config_param(configs[i]),
> -                                      pinconf_to_config_argument(configs[i]),
> -                                      &mask, &config);
> +               ret = s32_parse_pincfg(configs[i], &mask, &config);
>                 if (ret)
>                         return ret;
>         }
> --
> 2.37.3
>
  

Patch

diff --git a/drivers/pinctrl/nxp/pinctrl-s32cc.c b/drivers/pinctrl/nxp/pinctrl-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 *config)
 {
+	switch (param) {
+	case PIN_CONFIG_BIAS_DISABLE:
+	case PIN_CONFIG_BIAS_HIGH_IMPEDANCE:
+		*config &= ~(S32_MSCR_PUS | S32_MSCR_PUE);
+		break;
+	case PIN_CONFIG_BIAS_PULL_UP:
+		*config |= S32_MSCR_PUS | S32_MSCR_PUE;
+		break;
+	case PIN_CONFIG_BIAS_PULL_DOWN:
+		*config &= ~S32_MSCR_PUS;
+		*config |= S32_MSCR_PUE;
+		break;
+	default:
+		return;
+	}
+
+	*mask |= 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 = pinconf_to_config_param(pincfg);
+	arg = 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 param, u32 arg,
 		*config |= S32_MSCR_SRE((u32)ret);
 		*mask |= S32_MSCR_SRE(~0);
 		break;
+	case PIN_CONFIG_BIAS_DISABLE:
 	case PIN_CONFIG_BIAS_PULL_UP:
-		if (arg)
-			*config |= S32_MSCR_PUS;
-		else
-			*config &= ~S32_MSCR_PUS;
-		fallthrough;
 	case PIN_CONFIG_BIAS_PULL_DOWN:
-		if (arg)
-			*config |= S32_MSCR_PUE;
-		else
-			*config &= ~S32_MSCR_PUE;
-		*mask |= S32_MSCR_PUE | S32_MSCR_PUS;
+		s32_pin_set_pull(param, mask, config);
 		break;
 	case PIN_CONFIG_BIAS_HIGH_IMPEDANCE:
 		*config &= ~(S32_MSCR_ODE | S32_MSCR_OBE | S32_MSCR_IBE);
 		*mask |= S32_MSCR_ODE | S32_MSCR_OBE | S32_MSCR_IBE;
-		fallthrough;
-	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;
 	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 = 0; i < num_configs; i++) {
-		ret = s32_get_pin_conf(pinconf_to_config_param(configs[i]),
-				       pinconf_to_config_argument(configs[i]),
-				       &mask, &config);
+		ret = s32_parse_pincfg(configs[i], &mask, &config);
 		if (ret)
 			return ret;
 	}