linux-kernel - Re: [RFC PATCH 1/2] gpio: regmap: Force writes for aliased data regs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <DDNTQLB5YRM3.39C226E0QO6X9@kernel.org>
Date: Tue, 21 Oct 2025 09:33:04 +0200
From: "Michael Walle" <mwalle@...nel.org>
To: "Sander Vanheule" <sander@...nheule.net>, "Linus Walleij"
 <linus.walleij@...aro.org>, "Bartosz Golaszewski" <brgl@...ev.pl>,
 <linux-gpio@...r.kernel.org>
Cc: <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH 1/2] gpio: regmap: Force writes for aliased data
 regs

Hi,

On Mon Oct 20, 2025 at 1:56 PM CEST, Sander Vanheule wrote:
> GPIO chips often have data input and output fields aliased to the same
> offset. Since gpio-regmap performs a value update before the direction
> update (to prevent glitches), a pin currently configured as input may
> cause regmap_update_bits() to not perform a write.
>
> This may cause unexpected line states when the current input state
> equals the requested output state:
>
>         OUT   IN      OUT
>     DIR ''''''\...|.../''''''
>
>     pin ....../'''|'''\......
>              (1) (2) (3)
>
>     1. Line was configurad as out-low, but is reconfigured to input.
>        External logic results in high value.
>     2. Set output value high. regmap_update_bits() sees the value is
>        already high and discards the register write.
>     3. Line is switched to output, maintaining the stale output config
>        (low) instead of the requested config (high).
>
> By switching to regmap_write_bits(), a write of the requested output
> value can be forced, irrespective of the read state. Do this only for
> aliased registers, so the more efficient regmap_update_bits() can still
> be used for distinct registers.
>
> Signed-off-by: Sander Vanheule <sander@...nheule.net>
> ---
>  drivers/gpio/gpio-regmap.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
> index ab9e4077fa60..ba3c19206ccf 100644
> --- a/drivers/gpio/gpio-regmap.c
> +++ b/drivers/gpio/gpio-regmap.c
> @@ -93,7 +93,7 @@ static int gpio_regmap_set(struct gpio_chip *chip, unsigned int offset,
>  {
>  	struct gpio_regmap *gpio = gpiochip_get_data(chip);
>  	unsigned int base = gpio_regmap_addr(gpio->reg_set_base);
> -	unsigned int reg, mask;
> +	unsigned int reg, mask, mask_val;
>  	int ret;
>  
>  	ret = gpio->reg_mask_xlate(gpio, base, offset, &reg, &mask);
> @@ -101,9 +101,15 @@ static int gpio_regmap_set(struct gpio_chip *chip, unsigned int offset,
>  		return ret;
>  
>  	if (val)
> -		ret = regmap_update_bits(gpio->regmap, reg, mask, mask);
> +		mask_val = mask;
>  	else
> -		ret = regmap_update_bits(gpio->regmap, reg, mask, 0);
> +		mask_val = 0;
> +
> +	/* ignore input values which shadow the old output value */
> +	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);

I wonder if we should just switch to regmap_write_bits() entirely.

In patch 2, you've wrote:

> The generic gpiochip implementation stores a shadow value of the
> pin output data, which is updated and written to hardware on output
> data changes. Pin input values are always obtained by reading the
> aliased data register from hardware.

I couldn't find that in the code though. But if the gpiolib only
updates the output register on changes, the write part in
regmap_update_bits() would always occur.

In any case, feel free to add.

Reviewed-by: Michael Walle <mwalle@...nel.org>

-michael

Download attachment "signature.asc" of type "application/pgp-signature" (298 bytes)