Re: [PATCH 04/11] pinctrl: scmi: report an off setting as not set

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pinctrl: scmi: -EINVAL in pinctrl_scmi_pinconf_get() breaks GPIO get_direction
--

commit 925e342f7e0510b1b9dda63effb86ab92324b2e4
Author: Clément Le Goffic <clegoffic@baylibre.com>

pinctrl: scmi: report an off setting as not set

This commit changes the SCMI pinctrl driver to return -EINVAL when
querying boolean or pull pinconf parameters whose platform value is 0, so
that inactive settings are not reported as set.

> diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c
> index f74ef0952ba9b..355ad26cb2156 100644
> --- a/drivers/pinctrl/pinctrl-scmi.c
> +++ b/drivers/pinctrl/pinctrl-scmi.c
> @@ -153,13 +153,41 @@ static int pinctrl_scmi_map_pinconf_type(enum pin_config_param param,
>  	return 0;
>  }
>  
> -static u32 pinctrl_scmi_map_pinconf_arg_get(enum pin_config_param param,
> -					    u32 value)
> +static bool pinctrl_scmi_pinconf_inactive_flag(enum pin_config_param param,
> +					       u32 value)
>  {
> +	if (value)
> +		return false;
> +
> +	switch (param) {
> +	case PIN_CONFIG_BIAS_BUS_HOLD:
> +	case PIN_CONFIG_BIAS_DISABLE:
> +	case PIN_CONFIG_BIAS_HIGH_IMPEDANCE:
> +	case PIN_CONFIG_BIAS_PULL_DOWN:
> +	case PIN_CONFIG_BIAS_PULL_PIN_DEFAULT:
> +	case PIN_CONFIG_BIAS_PULL_UP:
> +	case PIN_CONFIG_DRIVE_OPEN_DRAIN:
> +	case PIN_CONFIG_DRIVE_OPEN_SOURCE:
> +	case PIN_CONFIG_DRIVE_PUSH_PULL:
> +	case PIN_CONFIG_INPUT_ENABLE:
> +	case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
> +	case PIN_CONFIG_OUTPUT_ENABLE:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static int pinctrl_scmi_map_pinconf_arg_get(enum pin_config_param param,
> +					    u32 *value)
> +{
> +	if (pinctrl_scmi_pinconf_inactive_flag(param, *value))
> +		return -EINVAL;
> +
>  	if (param == PIN_CONFIG_DRIVE_STRENGTH)
> -		return value / MILLI;
> +		*value /= MILLI;
>  
> -	return value;
> +	return 0;
>  }
>  
>  static int pinctrl_scmi_map_pinconf_type_get(enum pin_config_param param,
> @@ -232,7 +260,10 @@ static int pinctrl_scmi_pinconf_get(struct pinctrl_dev *pctldev,
>  	if (ret)
>  		return ret;
>  
> -	config_value = pinctrl_scmi_map_pinconf_arg_get(config_type, config_value);
> +	ret = pinctrl_scmi_map_pinconf_arg_get(config_type, &config_value);
> +	if (ret)
> +		return ret;

[Severity: High]
Does returning -EINVAL for PIN_CONFIG_OUTPUT_ENABLE when config_value is 0
break pin_control_gpio_get_direction() in drivers/gpio/gpio-by-pinctrl.c
for scmi-pinctrl-gpio?

In drivers/gpio/gpio-by-pinctrl.c, pin_control_gpio_get_direction()
queries the pin direction through pinctrl_gpio_get_config() ->
pin_config_get_for_pin() -> pinctrl_scmi_pinconf_get():

drivers/gpio/gpio-by-pinctrl.c:pin_control_gpio_get_direction() {
	config = PIN_CONFIG_OUTPUT_ENABLE;
	ret = pinctrl_gpio_get_config(gc, offset, &config);
	if (ret)
		return ret;
	if (config)
		return GPIO_LINE_DIRECTION_OUT;

	return GPIO_LINE_DIRECTION_IN;
}

When a pin has output mode disabled (SCMI_PIN_OUTPUT_MODE is 0, meaning
the pin is an input), pinctrl_scmi_pinconf_inactive_flag() now returns
true for PIN_CONFIG_OUTPUT_ENABLE, so pinctrl_scmi_pinconf_get() returns
-EINVAL.

Because pin_control_gpio_get_direction() returns ret immediately when it
is non-zero, querying the direction of any input line via
gpiod_get_direction() or gpiochip_lock_as_irq() will fail with -EINVAL,
and GPIO_LINE_DIRECTION_IN becomes unreachable.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030-clegoffic-pinctrl-scmi-gpio-v1-0-52d92843ad97@baylibre.com?part=4