linux-hardening - Re: [PATCH v3 03/10] gpiolib: implement low-level, shared GPIO support

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] [day] [month] [year] [list]

Message-ID: <CAMRc=Mfz+Se0Wq4cXjgP=DBOMkTqSBdoV-CcC-+Ma0hfELARzA@mail.gmail.com>
Date: Wed, 29 Oct 2025 16:57:02 +0100
From: Bartosz Golaszewski <brgl@...ev.pl>
To: Andy Shevchenko <andriy.shevchenko@...el.com>
Cc: Kees Cook <kees@...nel.org>, Mika Westerberg <westeri@...nel.org>, 
	Dmitry Torokhov <dmitry.torokhov@...il.com>, Andrew Morton <akpm@...ux-foundation.org>, 
	Linus Walleij <linus.walleij@...aro.org>, Manivannan Sadhasivam <mani@...nel.org>, 
	Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Saravana Kannan <saravanak@...gle.com>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
	Andy Shevchenko <andy@...nel.org>, Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>, 
	Srinivas Kandagatla <srini@...nel.org>, Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>, 
	Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>, 
	Alexey Klimov <alexey.klimov@...aro.org>, linux-hardening@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-gpio@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, linux-sound@...r.kernel.org, 
	linux-arm-msm@...r.kernel.org, 
	Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Subject: Re: [PATCH v3 03/10] gpiolib: implement low-level, shared GPIO support

On Wed, Oct 29, 2025 at 4:19 PM Andy Shevchenko
<andriy.shevchenko@...el.com> wrote:
>
> On Wed, Oct 29, 2025 at 01:39:34PM +0100, Bartosz Golaszewski wrote:
> > On Wed, Oct 29, 2025 at 12:45 PM Andy Shevchenko
> > <andriy.shevchenko@...el.com> wrote:
> > > On Wed, Oct 29, 2025 at 12:20:39PM +0100, Bartosz Golaszewski wrote:
> > > >
> > > > This module scans the device tree (for now only OF nodes are supported
> > > > but care is taken to make other fwnode implementations easy to
> > > > integrate) and determines which GPIO lines are shared by multiple users.
> > > > It stores that information in memory. When the GPIO chip exposing shared
> > > > lines is registered, the shared GPIO descriptors it exposes are marked
> > > > as shared and virtual "proxy" devices that mediate access to the shared
> > > > lines are created. When a consumer of a shared GPIO looks it up, its
> > > > fwnode lookup is redirected to a just-in-time machine lookup that points
> > > > to this proxy device.
> > > >
> > > > This code can be compiled out on platforms which don't use shared GPIOs.
> > >
> > > Besides strcmp_suffix() that already exists in OF core, there are also some
> > > existing pieces that seems being repeated here (again). Can we reduce amount
> > > of duplication?
> >
> > I'm afraid you need to be more specific here.
>
> You can simply browse the file, it's not long to find and think about it.
> I'm _thinking_ that it's possible to improve the situation overall by
> try our best of deduplicating (or rather not duplicating) things.
>

Sorry, but this is not how reviewing works. You can't just say: "I
think this can be improved, go figure out what can and fix it, you can
browse this file for reference". You need to specifically point out
issues in code and propose alternatives.

> ...
>
> > > > +#if IS_ENABLED(CONFIG_OF)
> > > > +static int gpio_shared_of_traverse(struct device_node *curr)
> > > > +{
> > >
> > > I believe parts of this code may be resided somewhere in drivers/of/property.c
> > > or nearby as it has the similar parsing routines.
> >
> > I don't think this is a good idea, I want to keep it within the
> > confines of drivers/gpio/ and the use-case is so specific, there's
> > really no point in putting parts of it under drivers/of/.
> >
> > If I could only iterate over all properties of an fwnode, I'd have
> > skipped using OF-specific routines altogether.
>
> The problem is that every subsystem considers "it's not a good idea" or
> "historical reasons" or other excuses. Since you are adding OF-specific
> stuff that has something already done inside OF specific code, why to
> spread it over the kernel by duplicating in another place(s)?
>

Well, point me to the things that have been done already and I'll see
about reusing them.

Bartosz