Re: [PATCH v7 13/34] pinctrl: airoha: add set_direction() helper for gpio_chip

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Unintended modification of adjacent bits in the GPIO direction register due to incorrect mask size, violating the 'No functional changes' claim.
--

--- Patch [13]: [PATCH v7 13/34] pinctrl: airoha: add set_direction() helper for gpio_chip ---
commit e729e6b2ed1ea68da146d1ea646d1135a1f27b5e
Author: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>

pinctrl: airoha: add set_direction() helper for gpio_chip
    
The patch creates a set_direction() helper for the gpio_chip abstraction and
reimplements existing direction configuration to use this helper. The commit
message claims this is purely a cosmetic patch with no functional changes.

> diff --git a/drivers/pinctrl/airoha/pinctrl-airoha.c b/drivers/pinctrl/airoha/pinctrl-airoha.c
> index 6a0ba54053fe7..fcf498f7720c9 100644
> --- a/drivers/pinctrl/airoha/pinctrl-airoha.c
> +++ b/drivers/pinctrl/airoha/pinctrl-airoha.c
[ ... ]
> @@ -2523,12 +2523,43 @@ static int airoha_gpio_get_direction(struct gpio_chip *chip, unsigned int gpio)
[ ... ]
> +static int airoha_gpio_set_direction(struct gpio_chip *chip, unsigned int gpio,
> +				     bool input)
> +{
[ ... ]
> +	/* set direction */
> +	field_shift = 2 * (gpio % AIROHA_REG_GPIOCTRL_NUM_PIN);
> +	mask = GENMASK(field_shift + 1, field_shift);
> +	index = gpio / AIROHA_REG_GPIOCTRL_NUM_PIN;
> +
> +	return regmap_update_bits(pinctrl->regmap,
> +				  pinctrl->gpiochip.dir[index],
> +				  mask, !input ? BIT(field_shift) : 0);
> +}

[Severity: Medium]
Does this new helper introduce a functional change contrary to the commit
message?

In the removed airoha_pinmux_set_direction() code shown below, the mask
covered exactly 1 bit:

    mask = BIT(2 * (pin % AIROHA_REG_GPIOCTRL_NUM_PIN));

Here in the new airoha_gpio_set_direction(), the mask spans 2 bits via
GENMASK(field_shift + 1, field_shift). The regmap_update_bits() call now
explicitly clears the upper bit of this 2-bit field.

Could this zeroing of the upper bit, which was previously preserved,
alter undocumented hardware state?

[ ... ]
> @@ -2752,27 +2783,13 @@ static int airoha_pinmux_set_direction(struct pinctrl_dev *pctrl_dev,
[ ... ]
> -	/* set direction */
> -	mask = BIT(2 * (pin % AIROHA_REG_GPIOCTRL_NUM_PIN));
> -	index = pin / AIROHA_REG_GPIOCTRL_NUM_PIN;
> -	return regmap_update_bits(pinctrl->regmap,
> -				  pinctrl->gpiochip.dir[index], mask,
> -				  !input ? mask : 0);
> +	return airoha_gpio_set_direction(&pinctrl->gpiochip.chip, gpio, input);
>  }

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