Re: [PATCH v2 3/3] pinctrl: airoha: add support of en7528 SoC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing pinmux support for routing multiplexed pins 0-39 back to GPIO or PWM functions.
--

commit 0726ac69786f503cb07357f6d0de45f2418a4be1
Author: Benjamin Larsson <benjamin.larsson@genexis.eu>

pinctrl: airoha: add support of en7528 SoC

The commit adds pin controller and GPIO driver support for the EcoNet EN7528
SoC, which shares driver code with Airoha SoCs but has a different register
layout.

> diff --git a/drivers/pinctrl/airoha/pinctrl-en7528.c b/drivers/pinctrl/airoha/pinctrl-en7528.c
> new file mode 100644
> index 0000000000000..f6c8f834421f7
> --- /dev/null
> +++ b/drivers/pinctrl/airoha/pinctrl-en7528.c
[ ... ]
> +static const struct airoha_pinctrl_func_group gpio_func_group[] = {
> +	AIROHA_PINCTRL_GPIO("gpio40", GPIO_PCIE_RESET0_MASK),
> +	AIROHA_PINCTRL_GPIO("gpio41", GPIO_PCIE_RESET1_MASK),
> +};

[Severity: Medium]
What happens if the bootloader configures pins 0-39 to an alternate function?

Since the driver only defines the GPIO function for pins 40 and 41 in
gpio_func_group, there doesn't seem to be a way to clear the IOMUX bits in
REG_PON_I2C_MODE to route multiplexed pins 0-39 back to their GPIO function
if they were left in an alternate state by the bootloader.

[ ... ]
> +/* PWM - AIROHA_PINCTRL_PWM and AIROHA_PINCTRL_PWM_EXT are shared */
> +#define AIROHA_PINCTRL_PWM(gpio, mux_val)		\
> +	{						\
> +		.name = (gpio),				\
> +		.regmap[0] = {				\
> +			AIROHA_FUNC_PWM_MUX,		\
> +			REG_GPIO_FLASH_MODE_CFG,	\
> +			(mux_val),			\
> +			(mux_val)			\
> +		},					\
> +		.regmap_size = 1,			\
> +	}						\
> +
> +#define AIROHA_PINCTRL_PWM_EXT(gpio, mux_val)		\
> +	{						\
> +		.name = (gpio),				\
> +		.regmap[0] = {				\
> +			AIROHA_FUNC_PWM_EXT_MUX,	\
> +			REG_GPIO_FLASH_MODE_CFG_EXT,	\
> +			(mux_val),			\
> +			(mux_val)			\
> +		},					\
> +		.regmap_size = 1,			\
> +	}						\
[ ... ]
> +static const struct airoha_pinctrl_func_group pwm_func_group[] = {
> +	AIROHA_PINCTRL_PWM("gpio0", GPIO0_FLASH_MODE_CFG),
> +	AIROHA_PINCTRL_PWM("gpio1", GPIO1_FLASH_MODE_CFG),
[ ... ]
> +	AIROHA_PINCTRL_PWM_EXT("gpio28", GPIO28_FLASH_MODE_CFG),
> +	AIROHA_PINCTRL_PWM_EXT("gpio29", GPIO29_FLASH_MODE_CFG),

[Severity: Medium]
Will PWM override the alternate function for these pins?

The macros AIROHA_PINCTRL_PWM and AIROHA_PINCTRL_PWM_EXT only set the PWM
enable bit but fail to clear the primary IOMUX bit. 

If the bootloader sets an alternate function on one of these pins, the driver
fails to clear the alternate function bit in REG_PON_I2C_MODE, leaving the
pad disconnected from the PWM controller.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1787931313.git.naseefkm@gmail.com?part=3