linux-kernel - Re: [PATCH] gpio: mmio: do not calculate bgpio

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: <CAOiHx=nssvX-VYySmpLZ8bvBmitT87bX2AYspdkH3y9iWTB+kQ@mail.gmail.com>
Date: Wed, 26 Jun 2024 17:20:50 +0200
From: Jonas Gorski <jonas.gorski@...il.com>
To: Asmaa Mnebhi <asmaa@...dia.com>
Cc: Shiji Yang <yangshiji66@...look.com>, 
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>, Linus Walleij <linus.walleij@...aro.org>, 
	Bartosz Golaszewski <brgl@...ev.pl>, Andy Shevchenko <andy.shevchenko@...il.com>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Mark Mentovai <mark@...tovai.com>, 
	Lóránd Horváth <lorand.horvath82@...il.com>
Subject: Re: [PATCH] gpio: mmio: do not calculate bgpio_bits via "ngpios"

On Wed, 26 Jun 2024 at 17:00, Asmaa Mnebhi <asmaa@...dia.com> wrote:
>
> I am not sure this change is needed?
> When I initially submitted " gpio: mmio: handle "ngpios" properly in bgpio_init() ", It was specifically because I have a 32bit reg access but only 16 gpios. Initially, I did not add the else and so Andy suggested to add it with the roundup_pow_of_two to stay backward compatible. If your system is a 32 bit arch and you only use 16 Gpio bits, why don't you configure that in your DTS?

Because the registers in the datasheet are specified as 32 bit wide,
so defining them as 32 bit in the dts(i) is the most natural way of
defining them, even if they may use less than half of the register for
gpios. And on big endian systems you cannot just use smaller
accessors, you also must shift the register offsets. So this change
broke existing devicetrees.

And as other theoretical arguments against doing that, less than 32
bit accesses may be inefficient, or the bus where the registers are
may require 32 bit accesses. And finally, the caller explicitly passed
a register width via the sz argument, so we should listen to the
caller and use that, and not trying to be clever by changing the
access width based on the number of gpios. At least not unless the
caller explicitly requested that. Like e.g. make 0 a special value for
automatically calculating the number of bits based on the number of
gpios.

If you only use 16 bits of the 32 bit registers and you want to use 16
bit accessors, IMHO it's up to you to pass appropriate values to
bgpio_init(), and not hope that bgpio_init() will fix this magically
up for you.

Best Regards,
Jonas