Re: [PATCH v10 2/8] gpiolib: regmap: add GPIO_REGMAP_QUIRK_SET_AFTER_DIR

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] gpio: regmap: direction_output bypasses set_with_clear callback
- [Medium] gpio: regmap: silent masking of regmap I/O errors in direction_output
--

commit 3bcfc79235e7b203090c7124352804ddb1bcdde7
Author: Long Zhao <longzhao@ambarella.com>

gpiolib: regmap: add GPIO_REGMAP_QUIRK_SET_AFTER_DIR

This commit introduces a new quirk flag, GPIO_REGMAP_QUIRK_SET_AFTER_DIR,
which allows the output value to be written after the direction change.
This supports legacy controllers that ignore output writes while a line
is still configured as an input.

> diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
> --- 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);

[Severity: High]
Does this bypass the chip->set callback for hardware that uses separate set
and clear registers?

When setting the initial value during a direction change to output,
hardcoding a call to gpio_regmap_set() instead of using the properly assigned
callback (such as gpio_regmap_set_with_clear) means that if value == 0, it
writes 0 to the reg_set_base register.

On set/clear hardware, this is typically a no-op instead of properly writing
the mask to reg_clr_base. This could leave the pin incorrectly driven high.

[Severity: Medium]
Is it safe to ignore the return value of gpio_regmap_set() here?

If the underlying bus (like I2C or SPI) encounters a failure or timeout
during the register write, the negative error code is silently dropped.

The gpiolib core would be falsely informed that the direction and value were
successfully configured, leading to state inconsistencies where software
believes a line is driven high or low but the hardware register write failed.

> +
> +	return 0;
>  }
>  
>  void *gpio_regmap_get_drvdata(struct gpio_regmap *gpio)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015-cv75-gpio-v10-0-3fe3eb315a55@ambarella.com?part=2