Re: [PATCH] pinctrl: meson: a4: Add input enable pin configuration

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] aml_pinconf_get() violates the pinctrl subsystem contract by returning 0 (success) instead of -EINVAL when boolean configurations PIN_CONFIG_OUTPUT_ENABLE and PIN_CONFIG_INPUT_ENABLE are disabled, breaking pinctrl debugfs reporting.
--

--- Patch [1]: [PATCH] pinctrl: meson: a4: Add input enable pin configuration ---
commit c1c005b3aefd6b68087e4e331448ad3792ca002d
Author: Xianwei Zhao <[email protected]>

pinctrl: meson: a4: Add input enable pin configuration

This commit adds support for PIN_CONFIG_INPUT_ENABLE in the Amlogic A4
pinctrl driver. It uses the existing output enable control to configure the
input enable state, and updates PIN_CONFIG_OUTPUT_ENABLE handling.

> diff --git a/drivers/pinctrl/meson/pinctrl-amlogic-a4.c b/drivers/pinctrl/meson/pinctrl-amlogic-a4.c
> --- a/drivers/pinctrl/meson/pinctrl-amlogic-a4.c
> +++ b/drivers/pinctrl/meson/pinctrl-amlogic-a4.c
> @@ -469,9 +469,15 @@ static int aml_pinconf_get(struct pinctrl_dev *pcdev, unsigned int pin,
>  		break;
>  	case PIN_CONFIG_OUTPUT_ENABLE:
>  		ret = aml_pinconf_get_output(info, pin);
> -		if (ret <= 0)
> +		if (ret < 0)
> +			return -EINVAL;
> +		arg = ret;
> +		break;
> +	case PIN_CONFIG_INPUT_ENABLE:
> +		ret = aml_pinconf_get_output(info, pin);
> +		if (ret < 0)
>  			return -EINVAL;
> -		arg = 1;
> +		arg = !ret;
>  		break;

[Severity: Medium]
Does returning 0 instead of -EINVAL when these boolean configurations are
disabled violate the pinctrl subsystem contract in aml_pinconf_get?

The generic pinctrl subsystem relies on the -EINVAL return code to omit
disabled boolean features. Because boolean features ignore the returned
argument value, returning 0 with arg = 0 when the feature is disabled will
cause debugfs (such as /sys/kernel/debug/pinctrl/.../pinconf-pins) to
falsely report both 'output enabled' and 'input enabled' as active for
every pin.

Should the check for PIN_CONFIG_OUTPUT_ENABLE remain ret <= 0, and should
PIN_CONFIG_INPUT_ENABLE also return -EINVAL when input is not enabled?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

_______________________________________________
linux-amlogic mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-amlogic