Re: [PATCH v12 02/11] gpiolib: regmap: add GPIO_REGMAP_QUIRK_SET_AFTER_DIR

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] gpiolib: regmap: unchecked gpio_regmap_set() in gpio_regmap_direction_output()
--

commit 1a15997a522bf0a981af10f62271be5cd75e972d
Author: Long Zhao <longzhao@ambarella.com>

gpiolib: regmap: add GPIO_REGMAP_QUIRK_SET_AFTER_DIR

This commit adds a quirk flag to allow writing the output value of a GPIO
line after changing its direction to output for controllers that ignore
output writes while the line is still configured as an input.

> diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
> index 51b4d69b87403..0a44deae3d01e 100644
> --- a/drivers/gpio/gpio-regmap.c
> +++ b/drivers/gpio/gpio-regmap.c
[ ... ]
> @@ -271,9 +272,17 @@ static int gpio_regmap_direction_output(struct gpio_chip *chip,
>  			return ret;
>  	}
>  
> -	gpio_regmap_set(chip, offset, value);
> +	if (!(gpio->quirks & GPIO_REGMAP_QUIRK_SET_AFTER_DIR))
> +		gpio_regmap_set(chip, offset, value);
>  
> -	return gpio_regmap_set_direction(chip, offset, true);
> +	ret = gpio_regmap_set_direction(chip, offset, true);
> +	if (ret)
> +		return ret;
> +
> +	if (gpio->quirks & GPIO_REGMAP_QUIRK_SET_AFTER_DIR)
> +		gpio_regmap_set(chip, offset, value);
> +
> +	return 0;

[Severity: Medium]
Should gpio_regmap_direction_output() in drivers/gpio/gpio-regmap.c check
and propagate the return value of both gpio_regmap_set() calls?

In drivers/gpio/gpio-regmap.c, gpio_regmap_set() returns a negative error
code if reg_mask_xlate() or regmap_write_bits() / regmap_update_bits()
fails:

drivers/gpio/gpio-regmap.c:gpio_regmap_set() {
	ret = gpio->reg_mask_xlate(gpio, base, offset, &reg, &mask);
	if (ret)
		return ret;
	...
	if (gpio->reg_dat_base == gpio->reg_set_base)
		ret = regmap_write_bits(gpio->regmap, reg, mask, mask_val);
	else
		ret = regmap_update_bits(gpio->regmap, reg, mask, mask_val);

	return ret;
}

When the quirk is not set and the initial gpio_regmap_set() fails (for
example due to an I2C/SPI bus error or invalid register translation),
ignoring the return value allows execution to proceed to
gpio_regmap_set_direction(), enabling the output driver on the pin with a
stale output value.

When the quirk is set and the post-direction gpio_regmap_set() fails,
gpio_regmap_direction_output() discards the error and returns 0 to the
caller even though setting the output value failed.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027-cv75-gpio-v12-0-c22555400e18@ambarella.com?part=2