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: <bfab980197bfd845cce360d2dbd12ef716f8b727.camel@svanheule.net>
Date: Tue, 21 Oct 2025 11:00:10 +0200
From: Sander Vanheule <sander@...nheule.net>
To: Michael Walle <mwalle@...nel.org>, 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 Michael,

On Tue, 2025-10-21 at 09:33 +0200, Michael Walle wrote:
> > +	/* 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.

It would certainly make the code simpler, but it may impact performance a bit.
E.g. a bit-banged I2C bus doesn't need to update the output value (only the
direction), so using regmap_update_bits() saves half the writes when the output
data register can be properly cached.

Similar to gpio-mmio.c, gpio-regmap.c could also provide multiple setters, so
the branch(es) in this call would only occur once at init, to sacrifice code
size for a bit of performance. Feel free to let me know what your preference is,
otherwise I'll keep the patch as-is.

> 
> 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.

I was referring to bgpio_set(). AFAICT gpiod_direction_input() and
gpiod_direction_output() call the driver unconditionally, without checking if
the gpiolib state would change for this pin.

> 
> In any case, feel free to add.
> 
> Reviewed-by: Michael Walle <mwalle@...nel.org>

Thanks!

Best,
Sander