Re: [PATCH v4 01/20] platform/x86: x86-android-tablets: convert Goodix devices to GPIO references

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Mon, Feb 09, 2026 at 10:25:47AM -0600, Bartosz Golaszewski wrote:
> On Mon, 9 Feb 2026 15:41:54 +0100, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> said:
> > Hi Bartosz, Hans,
> >
> 
> [snip]
> 
> >> >>>
> >> >>> Could somebody advise on how to fix this, or does a fix already exist? I am
> >> >>> not familiar with the swnode/fwnode framework but can do some investigation to
> >> >>> resolve this.
> >> >>>
> >> >
> >> > I really don't want to revert to the label lookup and string comparison as this
> >> > is totally broken
> >
> > I would start with explaining why exactly this is broken. So far the
> > explanation has been:
> >
> > "
> >     Looking up a GPIO controller by label that is the name of the software
> >     node is wonky at best - the GPIO controller driver is free to set
> >     a different label than the name of its firmware node. We're already being
> >     passed a firmware node handle attached to the GPIO device to
> >     swnode_get_gpio_device() so use it instead for a more precise lookup.
> > "
> >
> > This is weak. Indeed the controller is free to select a different label,
> > breaking the link. This is a weakness of software nodes in general,
> > where we do not have a mechanism to validate the scheme.
> >
> 
> I would even say that if we keep comparing strings, then it's no better
> than GPIO lookup tables.

I would like to stop using lookup tables so that we always excessive the
common (fwnode-based) paths for finding gpios in consumers, and not an
exception like we have with the lookup tables.

> 
> > However switching to matching strictly by fwnode requires knowledge of
> > the instance of the fwnode. This may work OK-ish for some drivers but
> > breaks for the main intended users of this: legacy board files, where we
> > want to specify static mappings and do not want to "reach" into
> > individual gpiochip drivers to fish out the actual firmware node.
> >
> 
> I'm not sure I'm following. We can (and do a lot!) reference static struct
> software_node instances before we register them with driver core, creating
> whole trees of property nodes. I get that here we're talking about *real*
> firmware nodes but see below:

If your fwnode pointer is not a compile-time constant (since you have to
look it up somehow) you can not make corresponding property entries
static constant arrays, and have to build them dynamically (or fix them
up in place after doing the lookup).

> 
> > From the docs that were accepted (Documentation/driver-api/gpio/board.rst)
> 
> Quick `git blame`, `git log`, ah, yes, I signed off on this. I guess it's on
> me now too. :)
> 
> >
> > "
> >     The software node representing a GPIO controller need not be attached to the
> >     GPIO controller device. The only requirement is that the node must be
> >     registered and its name must match the GPIO controller's label.
> > "
> >
> > Note that this is different from OF or ACPI nodes because there parsing
> > and creating nodes is a separate process. You get the whole graph
> > available to you right away, so it is possible to know the exact node
> > you are referencing.
> >
> 
> The device that creates devices not tied to a real OF or ACPI node, is
> typically responsible for creating the software nodes for its children. It
> can check if given GPIO controller already exists (gpio_device_find_by_label())
> and if not - just return -EPROBE_DEFER and not create children. If it does,
> then it can pass its fwnode to the children. A bit like so:
> 
>   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/reset/core.c#n910
> 
> Just with the lookup done by label and not fwnode.

As I mentioned another email, in case of old legacy boards you do not
really have ACPI or OF nodes. If they were there then we would need to
build trees of software nodes, we would simply extend the DTs for them.

> 
> I may have not paid attention to the docs and signed off on it but it doesn't
> make it correct. The strength of software nodes is that we have an actual
> fwnode link between devices. If we just fake this link, then why are we even
> using software nodes at all?

To allow drivers drop hard coded platform data and switch to generic
device properties. This goes beyond GPIOs.

> 
> > This is what I wrote to Hans when he raised his concerns:
> >
> > "
> >     I agree it is a bit weird, but this allows to disconnect the board file
> >     from the GPIO driver and makes it easier to convert to device tree down
> >     the road as it can be done in a piecemeal fashion. If you want fwnode
> >     actually attached to the gpiochip then:
> >
> >     1. You can't really have static/const initializers in most of the cases
> >     2. Fishing it out from an unrelated subsystem is much harder than
> >     matching on a name.
> > "
> 
> Ok but if we match names then we already have a mechanism for that: lookup
> tables. I really fail to see how creating software nodes with a specific name
> in consumer code is any better.

I do not want to have 2 mechanisms, one for GPIOs, and one for
everything else. Let's take gpio_keys driver. It uses platform data or
device tree properties (generic device properties) to describe the
hardware. I want to drop platform data, and I do not want to encumber
the driver itself with setting up the board specific lookup tables, I
want the configuration be contained in board files. I also do not want
to have to configure all properties but GPIOs through one mechanism
(software nodes) and then have an exception for GPIOs to use lookup
tables.

I understand that matching on firmware node is nice and may even be
preferable in some cases, but I think matching on name has its own
place given all practicalities.

> 
> [snip]
> 
> >
> > I think the hardest breakages are here:
> >
> > arch/arm/mach-omap1/board-nokia770.c
> > arch/arm/mach-pxa/gumstix.c
> > arch/arm/mach-pxa/spitz.c
> > arch/arm/mach-tegra/board-paz00.c
> >
> > If we check out at nokia, it uses drivers/gpio/gpio-omap.c. As far as I
> > can see it does not even have a fwnode assigned: it either does not have
> > parent or its parent is a platform device instantiated by the driver
> > itself and does not have a firmware node associated with it.
> >
> 
> In that case using software nodes makes absolutely no sense and users should
> stick to GPIO lookup tables. The provider does not have an fwnode attached, how
> can we just fake it in the consumer?

Why not? It allows us to use generic device properties for describing
everything that we need.

> 
> I would rather just attach a software node to the omap controller in
> arch/arm/mach-omap1/gpio16xx.c and use its address in these board files.

That might work, just a bigger change than I wanted to make originally.

> 
> > So in the end name matching allows weakly referencing objects elsewhere
> > in the kernel for legacy and one-off devices.
> >
> > Maybe we should allow for both? Do a fwnode lookup and when it fails
> > fall back to matching on name?
> >
> 
> Isn't it GPIO lookup tables with extra steps and more confusing too? Because
> that's how it works in GPIO core: if you can't find anything using the fwnode
> lookup, you fall back to comparing names in GPIO lookup tables.

What you are missing is that it allows to describe everything, not only
GPIOs, through the common mechanism.

> 
> My point is: it only makes sense to use software nodes if we're creating
> links between them. If we fake them and compare their names instead, then
> it's just abuse of the API. It says PROPERTY_ENTRY_REF() for a reason: it's
> supposed to store an address leading you to the actual device on the other
> side.

No, it is supposed to store a "reference". It does not have to be a
pointer, it is a reference to another object. Could be an IP address or
USB port number, or something else that can uniquely identify that
object. In DTS it is not a pointer, it is "&node". How it is
implemented underneath is an implementation detail.

Thanks.

-- 
Dmitry