Re: [PATCH v11 2/9] 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] gpio: regmap: unchecked gpio_regmap_set() in gpio_regmap_direction_output()
--

commit 08236aadae56fa844fef879953d0665dc5a56ed0
Author: Long Zhao <longzhao@ambarella.com>

gpiolib: regmap: add GPIO_REGMAP_QUIRK_SET_AFTER_DIR

This commit adds the GPIO_REGMAP_QUIRK_SET_AFTER_DIR flag to gpio-regmap so
controllers that ignore output writes while a line is configured as an input
can write the output value after changing the line direction.

> 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() check and propagate the return value of
gpio_regmap_set()?

In drivers/gpio/gpio-regmap.c, gpio_regmap_set() returns a negative error code
when gpio->reg_mask_xlate(), regmap_write_bits(), or 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 GPIO_REGMAP_QUIRK_SET_AFTER_DIR is not set, ignoring a failure from
gpio_regmap_set() causes gpio_regmap_direction_output() to still call
gpio_regmap_set_direction(chip, offset, true), enabling the output buffer with
a stale output value.

When GPIO_REGMAP_QUIRK_SET_AFTER_DIR is set, a failure in gpio_regmap_set()
after gpio_regmap_set_direction() is discarded and 0 is returned to
gpiod_direction_output_raw_commit(), leaving the line configured as an output
with an unwritten value while reporting success to the caller.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261020-cv75-gpio-v11-0-e058bae19571@ambarella.com?part=2