Re: [PATCH v13 02/13] 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 eae7ad18ef30360e98f853dbc86b39ea56d2da3f
Author: Long Zhao <longzhao@ambarella.com>

gpiolib: regmap: add GPIO_REGMAP_QUIRK_SET_AFTER_DIR

This commit adds a quirk flag to struct gpio_regmap_config and struct
gpio_regmap so that gpio_regmap_direction_output() writes the output value
after switching the line direction to output for controllers that ignore
output writes while a line is 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]
As reported by Sashiko on v11 [1], 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 if gpio->reg_mask_xlate() fails or if 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 the quirk is not set, a failure in gpio_regmap_set() is ignored and
gpio_regmap_direction_output() still proceeds to call
gpio_regmap_set_direction(), enabling the output with a stale value. When
the quirk is set, a failure in gpio_regmap_set() after changing the
direction is also discarded and 0 is returned to the caller.

[1] https://lore.kernel.org/r/20261020-cv75-gpio-v11-2-e058bae19571@ambarella.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102-cv75-gpio-v13-0-4f93114ea204@ambarella.com?part=2