Re: [PATCH v9 10/36] pinctrl: airoha: fix I2C1 pin mux config for AN7581

[email protected]
Newsgroups org.kernel.vger.linux-devicetree
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The I2C1 pinmux configuration mask sets the master mode bit but fails to include the mutually exclusive slave mode bit, potentially leaving the hardware in an undefined state if slave mode was previously enabled.
--

--- Patch [10]: [PATCH v9 10/36] pinctrl: airoha: fix I2C1 pin mux config for AN7581 ---
commit 7aaf099c9130097177ef6d5edb5a5f51b5fd1d25
Author: Daniel Schwierzeck <[email protected]>
pinctrl: airoha: fix I2C1 pin mux config for AN7581

This commit updates the I2C1 pin group configuration to set bits 0 and 13,
enabling I2C1 master mode by default on the AN7581.

> diff --git a/drivers/pinctrl/airoha/pinctrl-airoha.c b/drivers/pinctrl/airoha/pinctrl-airoha.c
> index ee2bd0ee8c40f..2695342adfc76 100644
> --- a/drivers/pinctrl/airoha/pinctrl-airoha.c
> +++ b/drivers/pinctrl/airoha/pinctrl-airoha.c
> @@ -1158,8 +1158,8 @@ static const struct airoha_pinctrl_func_group i2c_func_group[] = {
>  		.regmap[0] = {
>  			AIROHA_FUNC_MUX,
>  			REG_GPIO_2ND_I2C_MODE,
> -			GPIO_2ND_I2C_MODE_MASK,
> -			GPIO_2ND_I2C_MODE_MASK
> +			GPIO_2ND_I2C_MODE_MASK | GPIO_I2C_MASTER_MODE_MODE,
> +			GPIO_2ND_I2C_MODE_MASK | GPIO_I2C_MASTER_MODE_MODE,

[Severity: Medium]
Will this leave the hardware in an undefined state if the bootloader
previously set the slave mode bit?

The commit message notes that bit 13 is for master and bit 11 is for slave.
By omitting the slave mode bit (GPIO_I2C_SLAVE_MODE_MODE) from the mask here
in i2c_func_group, the regmap update won't clear bit 11 if it is already set.

Does this mask need to include the slave mode bit to ensure it is properly
cleared when enforcing master mode?

>  		},
>  		.regmap_size = 1,
>  	},

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=10
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.