Re: [PATCH v7 26/34] pinctrl: airoha: an7583: add support for pon_alt pinmux

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The introduction of the `pon_alt` pinmux introduces a new mux bit for pins 36-40 (gpio34-gpio38) but fails to update the GPIO and PWM functions to clear this bit, rendering these pins permanently stuck in `pon_alt` mode if left enabled by a bootloader or dynamically switched by Linux.
--

--- Patch [26]: [PATCH v7 26/34] pinctrl: airoha: an7583: add support for pon_alt pinmux ---
commit b1dabfb03ae8c035084ec1f24b413e2c3334d17a
Author: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
Subject: pinctrl: airoha: an7583: add support for pon_alt pinmux

This commit adds support for the pon pin function using the pon_alt pin group.

> diff --git a/drivers/pinctrl/airoha/pinctrl-an7583.c b/drivers/pinctrl/airoha/pinctrl-an7583.c
> index 9b5c7ea5e5472..7e94dcb3b61a5 100644
> --- a/drivers/pinctrl/airoha/pinctrl-an7583.c
> +++ b/drivers/pinctrl/airoha/pinctrl-an7583.c

[ ... ]

> @@ -629,10 +632,19 @@ static const struct airoha_pinctrl_func_group pon_func_group[] = {
>  		.regmap[0] = {
>  			AIROHA_FUNC_MUX,
>  			REG_GPIO_PON_MODE,
> -			GPIO_PON_MODE_MASK,
> +			GPIO_PON_MODE_MASK | GPIO_PON_ALT_MODE_MASK,
>  			GPIO_PON_MODE_MASK
>  		},
>  		.regmap_size = 1,
> +	}, {
> +		.name = "pon_alt",
> +		.regmap[0] = {
> +			AIROHA_FUNC_MUX,
> +			REG_GPIO_PON_MODE,
> +			GPIO_PON_MODE_MASK | GPIO_PON_ALT_MODE_MASK,
> +			GPIO_PON_ALT_MODE_MASK
> +		},
> +		.regmap_size = 1,
>  	},
>  };

[Severity: High]
Since pon_alt uses pins 36-40 (gpio34-gpio38) and sets GPIO_PON_ALT_MODE_MASK
in REG_GPIO_PON_MODE, do we also need to add these pins to gpio_groups[]?

Currently, gpio34 through gpio38 appear to be missing from gpio_groups[]:

drivers/pinctrl/airoha/pinctrl-an7583.c:gpio_groups[] {
    ...
    static const char *const gpio_groups[] = {
    	"gpio39", "gpio40", "gpio41", "gpio42", "gpio43",
    ...
}

Without being in gpio_groups[], it seems impossible to select the "gpio" 
function in the device tree to clear the pon_alt mux bit.

Additionally, will these pins get stuck in pon_alt mode if requested as PWM?

The PWM mapping for pins 36-38 uses AIROHA_PINCTRL_PWM_EXT:

drivers/pinctrl/airoha/pinctrl-an7583.c:pwm_func_group[] {
    ...
	AIROHA_PINCTRL_PWM_EXT("gpio36", GPIO36_FLASH_MODE_CFG),
	AIROHA_PINCTRL_PWM_EXT("gpio37", GPIO37_FLASH_MODE_CFG),
	AIROHA_PINCTRL_PWM_EXT("gpio38", GPIO38_FLASH_MODE_CFG),
    ...
}

This macro only seems to modify REG_GPIO_FLASH_MODE_CFG_EXT and does not clear
the GPIO_PON_ALT_MODE_MASK bit in REG_GPIO_PON_MODE. If a bootloader leaves
these pins in pon_alt mode, or if they are dynamically switched, won't the PWM
output fail silently because the pins remain multiplexed to pon_alt?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727074234.3761170-1-mikhail.kshevetskiy@iopsys.eu?part=26