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

From: sashiko-bot@kernel.org
To: "Xianwei Zhao" <xianwei.zhao@amlogic.com>
Cc: linux-amlogic@lists.infradead.org, neil.armstrong@linaro.org
Subject: Re: [PATCH] pinctrl: meson: a4: Add input enable pin configuration
Date: Wed, 12 Aug 2026 10:28:06 +0000	[thread overview]
Message-ID: <20260812102806.E8B1F1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260812-input-enable-v1-1-81e5ca7772aa@amlogic.com>

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 <xianwei.zhao@amlogic.com>

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/20260812-input-enable-v1-1-81e5ca7772aa@amlogic.com?part=1

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic