Re: [PATCH v11 17/37] pinctrl: airoha: minor improvements

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver completely ignores the `arg` value for `PIN_CONFIG_OUTPUT_ENABLE` and `PIN_CONFIG_INPUT_ENABLE`, incorrectly enabling the feature even when the device tree requests it to be disabled (e.g., via `output-disable`).
--

--- Patch [17]: [PATCH v11 17/37] pinctrl: airoha: minor improvements ---
commit 32427c5ad4b8147aba5933507740e1938010aa75
Author: Mikhail Kshevetskiy <[email protected]>

pinctrl: airoha: minor improvements

This commit improves error handling by checking return values of various
configuration functions in the Airoha pinctrl driver. It also switches to 
using field_get() and field_prep() macros instead of direct bit operations.

> diff --git a/drivers/pinctrl/airoha/pinctrl-airoha.c b/drivers/pinctrl/airoha/pinctrl-airoha.c
> --- a/drivers/pinctrl/airoha/pinctrl-airoha.c
> +++ b/drivers/pinctrl/airoha/pinctrl-airoha.c

[ ... ]

>  		case PIN_CONFIG_OUTPUT_ENABLE:
>  		case PIN_CONFIG_INPUT_ENABLE:
>  		case PIN_CONFIG_LEVEL: {
>  			bool input = param == PIN_CONFIG_INPUT_ENABLE;
> -			int err;
>  
>  			err = airoha_pinmux_set_direction(pctrl_dev, NULL, pin,
>  							  input);

[Severity: High]
This is a pre-existing issue, but does this logic handle the arg value
correctly when a feature is requested to be disabled?

The code calculates the input direction boolean purely based on the param
type, completely ignoring the arg value. This means if a device tree
requests a pin to be disabled (e.g., via output-disable where arg=0),
the function will still enable it as an output because
param == PIN_CONFIG_INPUT_ENABLE will be false.

Could this lead to unintended hardware states, such as pin contention if an
external device is actively driving a line that was meant to be disabled?

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