Re: [PATCH v4 04/10] gpio: swnode: don't use the swnode's name as the key for GPIO lookup

[Charles Keepax <ckeepax@opensource.cirrus.com>]

On Tue, Nov 18, 2025 at 07:01:24PM +0100, Bartosz Golaszewski wrote:
> On Tue, Nov 18, 2025 at 5:34 PM Charles Keepax
> <ckeepax@opensource.cirrus.com> wrote:
> >
> > On Mon, Nov 03, 2025 at 10:35:24AM +0100, Bartosz Golaszewski wrote:
> > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > >
> > > 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.
> > >
> > > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > > ---
> > >  drivers/gpio/gpiolib-swnode.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpio/gpiolib-swnode.c b/drivers/gpio/gpiolib-swnode.c
> > > index f21dbc28cf2c8c2d06d034b7c89d302cc52bb9b5..e3806db1c0e077d76fcc71a50ca40bbf6872ca40 100644
> > > --- a/drivers/gpio/gpiolib-swnode.c
> > > +++ b/drivers/gpio/gpiolib-swnode.c
> > > @@ -41,7 +41,7 @@ static struct gpio_device *swnode_get_gpio_device(struct fwnode_handle *fwnode)
> > >           !strcmp(gdev_node->name, GPIOLIB_SWNODE_UNDEFINED_NAME))
> > >               return ERR_PTR(-ENOENT);
> > >
> > > -     gdev = gpio_device_find_by_label(gdev_node->name);
> > > +     gdev = gpio_device_find_by_fwnode(fwnode);
> > >       return gdev ?: ERR_PTR(-EPROBE_DEFER);
> > >  }
> >
> > One small problem is this does break drivers/spi/spi-cs42l43.c.
> 
> I'd say it's a big problem. :)
> 
> > That driver has to register some swnodes to specify some GPIO
> > chip selects due to some squiffy ACPI from Windows land. Currently
> > it relies on the sw node being called cs42l43-pinctrl to match
> > the driver.
> >
> 
> What is the problem exactly? The "cs42l43-pinctrl" swnode is
> associated with a GPIO device I suppose? Does it not find it? I'd need
> some more information in order to figure out a way to fix it.

Yeah doesn't find the GPIO device. Apologies the background is
fairly lenghty here but to give a high level summary. The cs42l43
is an audio CODEC but it has a SPI controller on it, in some
configurations there are a couple of speaker amps connected to
this SPI controller. For Window reasons this SPI controller isn't
properly represented in ACPI, so we have to depend on a couple
magic properties and then use software nodes to register the
speaker amps. However, as part of this we need to register a
cs-gpios property for the chip selects used by the amps.

This chip select gpios property is where the problem happens, we
need to refer to the gpio/pinctrl driver of the cs42l43, but
software nodes only seem to allow referring to other software
nodes. Previously this worked as we gave the node the same name
as the real driver, which meant it found the real driver.
However, after switching to look up by fwnode, it is looking for
a gpio controller attached to the software node.

static const struct software_node cs42l43_gpiochip_swnode = {
	/* Previously matched the driver name for the pinctrl driver */
	.name	=  "cs42l43-pinctrl",
};

static const struct software_node_ref_args cs42l43_cs_refs[] = {
	/* This needs to point to the genuine pinctrl driver? */
	SOFTWARE_NODE_REFERENCE(&cs42l43_gpiochip_swnode, 0, GPIO_ACTIVE_LOW),
	/* This is a mark that indicates the native chip select is used*/
	SOFTWARE_NODE_REFERENCE(&swnode_gpio_undefined),
};

The bit that is unclear to me is how we get that software node
property to point to the real pinctrl driver rather than the
dummy software node. I think maybe we need to add a SW node as a
secondary node on the pinctrl driver itself and link to that?

Also happy from my side to spend some time working on this as I
am not convinced what the driver is doing now is totally legit.

Thanks,
Charles