Re: [PATCH v10 06/36] pinctrl: airoha: an7581: fix mux/conf of pcie_reset pins

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Missing DT Binding Update for new pinmux function and groups.
- [Low] Unused macro AIROHA_PINCTRL_GPIO.
--

--- Patch [6]: [PATCH v10 06/36] pinctrl: airoha: an7581: fix mux/conf of pcie_reset pins ---
commit 54ed88634fb21e810e128e4d6842ad8043183d6b
Author: Mikhail Kshevetskiy <[email protected]>

pinctrl: airoha: an7581: fix mux/conf of pcie_reset pins

This commit fixes the pin numbering for pcie_reset pins, adjusting them
from 61-63 to 60-62. It also introduces proper pinmux implementations for
setting pcie_reset pins as GPIOs or PWM.

> diff --git a/drivers/pinctrl/airoha/pinctrl-airoha.c b/drivers/pinctrl/airoha/pinctrl-airoha.c
> index ffbc7b3c61812..4be694b8ea778 100644
> --- a/drivers/pinctrl/airoha/pinctrl-airoha.c
> +++ b/drivers/pinctrl/airoha/pinctrl-airoha.c
[ ... ]
> @@ -907,7 +914,8 @@ static const char *const pwm_groups[] = { "gpio0", "gpio1",
>  					  "gpio40", "gpio41",
>  					  "gpio42", "gpio43",
>  					  "gpio44", "gpio45",
> -					  "gpio46" };
> +					  "gpio46", "gpio47",
> +					  "gpio48", "gpio49" };

[Severity: Medium]
Does adding the new gpio47, gpio48 and gpio49 groups to the pwm function
without updating the device tree binding schema cause dtbs_check validation
errors?

[ ... ]
> @@ -1406,6 +1414,45 @@ static const struct airoha_pinctrl_func_group pnand_func_group[] = {
>  	},
>  };
>  
> +#define AIROHA_PINCTRL_GPIO(gpio, mux_val)			\
> +	{							\
> +		.name = (gpio),					\
> +		.regmap[0] = {					\
> +			AIROHA_FUNC_MUX,			\
> +			REG_GPIO_PON_MODE,			\
> +			(mux_val),				\
> +			(mux_val)				\
> +		},						\
> +		.regmap_size = 1,				\
> +	}

[Severity: Low]
Is this macro needed? It appears AIROHA_PINCTRL_GPIO is never used in the
driver, as all configurations rely on the AIROHA_PINCTRL_GPIO_EXT macro
instead.

[ ... ]
> @@ -1804,6 +1875,7 @@ static const struct airoha_pinctrl_func en7581_pinctrl_funcs[] = {
>  	PINCTRL_FUNC_DESC("i2s", i2s),
>  	PINCTRL_FUNC_DESC("emmc", emmc),
>  	PINCTRL_FUNC_DESC("pnand", pnand),
> +	PINCTRL_FUNC_DESC("gpio", gpio),

[Severity: Medium]
Are there corresponding schema updates in the yaml for the newly introduced
gpio function? Without them, a user configuring the gpio function in their
board's device tree might encounter schema validation errors.

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