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);
}
--
2.51.0
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. 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. I guess that is not quite the right way to handle that but its not clear to me how to link the software node properties to the pinctrl otherwise, anyone have any pointers there? Note: There are a reasonable amount of shipping laptops this will break. Thanks, Charles
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. > I guess that is not quite the right way to handle that but its > not clear to me how to link the software node properties to the > pinctrl otherwise, anyone have any pointers there? > Depends what you mean. Creating software nodes is fine, depending on some arbitrary string not so much. As I said: I need more information but I'm willing to help you fix it ASAP. Bart
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
On Tue, Nov 18, 2025 at 7:16 PM Charles Keepax
<ckeepax@opensource.cirrus.com> wrote:
>
> 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.
>
As mentioned by Andy: this sounds precisely like what this series
addresses with the reset-gpio driver. We now CAN reference fwnodes
from software nodes.
> 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.
Well, it is sketchy. We register the cs42l43_gpiochip_swnode and
reference it but never assign it to the GPIO device. If you assigned
it as a secondary fwnode to the relevant GPIO device, it would have
been found during the lookup.
Right now, with how the lookup-by-label works in gpiolib-swnode.c, you
get the referenced software node and read its name but there's no real
link between the swnode and the GPIO device. It's just a big hack.
I have an idea for fixing it, let me cook up a patch. It'll still be a
bit hacky but will at least create a true link.
Bart
On Wed, Nov 19, 2025 at 9:35 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > I have an idea for fixing it, let me cook up a patch. It'll still be a > bit hacky but will at least create a true link. > Scratch that, I didn't notice before but we register both devices from MFD core. We can just set up software nodes there. Bart
On Wed, Nov 19, 2025 at 9:41 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > On Wed, Nov 19, 2025 at 9:35 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > I have an idea for fixing it, let me cook up a patch. It'll still be a > > bit hacky but will at least create a true link. > > > > Scratch that, I didn't notice before but we register both devices from > MFD core. We can just set up software nodes there. > Here you go: https://lore.kernel.org/all/20251119-cs42l43-gpio-swnodes-v1-1-25996afebd97@linaro.org/ Please give it a try. This is independent from this series and should probably be backported to stable. Bartosz
On Wed, Nov 19, 2025 at 10:13:30AM +0100, Bartosz Golaszewski wrote: > On Wed, Nov 19, 2025 at 9:41 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > On Wed, Nov 19, 2025 at 9:35 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > > > I have an idea for fixing it, let me cook up a patch. It'll still be a > > > bit hacky but will at least create a true link. > > > > > > > Scratch that, I didn't notice before but we register both devices from > > MFD core. We can just set up software nodes there. > > > > Here you go: https://lore.kernel.org/all/20251119-cs42l43-gpio-swnodes-v1-1-25996afebd97@linaro.org/ > > Please give it a try. This is independent from this series and should > probably be backported to stable. So think breaks more drivers: https://lore.kernel.org/all/aYkdKfP5fg6iywgr@jekhomev/ I think it may also break: arch/arm/mach-omap1/board-nokia770.c arch/arm/mach-pxa/devices.c arch/arm/mach-pxa/devices.h arch/arm/mach-pxa/gumstix.c arch/arm/mach-pxa/pxa25x.c arch/arm/mach-pxa/pxa27x.c arch/arm/mach-pxa/spitz.c arch/arm/mach-tegra/board-paz00.c arch/x86/platform/geode/geode-common.c drivers/platform/x86/barco-p50-gpio.c drivers/platform/x86/pcengines-apuv2.c Thanks. -- Dmitry
On Mon, Feb 9, 2026 at 7:44 AM Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > > On Wed, Nov 19, 2025 at 10:13:30AM +0100, Bartosz Golaszewski wrote: > > On Wed, Nov 19, 2025 at 9:41 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > > > On Wed, Nov 19, 2025 at 9:35 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > > > > > I have an idea for fixing it, let me cook up a patch. It'll still be a > > > > bit hacky but will at least create a true link. > > > > > > > > > > Scratch that, I didn't notice before but we register both devices from > > > MFD core. We can just set up software nodes there. > > > > > > > Here you go: https://lore.kernel.org/all/20251119-cs42l43-gpio-swnodes-v1-1-25996afebd97@linaro.org/ > > > > Please give it a try. This is independent from this series and should > > probably be backported to stable. > > So think breaks more drivers: > > https://lore.kernel.org/all/aYkdKfP5fg6iywgr@jekhomev/ > > I think it may also break: > > arch/arm/mach-omap1/board-nokia770.c > arch/arm/mach-pxa/devices.c > arch/arm/mach-pxa/devices.h > arch/arm/mach-pxa/gumstix.c > arch/arm/mach-pxa/pxa25x.c > arch/arm/mach-pxa/pxa27x.c > arch/arm/mach-pxa/spitz.c > arch/arm/mach-tegra/board-paz00.c Most of them seem to use software nodes correctly. Nokia 770 could potentially break depending on the timing but the lookup uses the right string. > arch/x86/platform/geode/geode-common.c Looks like a correct case of referencing the software node to me. > drivers/platform/x86/barco-p50-gpio.c > drivers/platform/x86/pcengines-apuv2.c > Same here. Nothing here seems to depend on a label and there are real links between the GPIO chip's and consumer's software nodes. The problem we triggered here was caused by a GPIO consumer who would create a bogus software node locally without any real link to the provider. It would then depend on the provider being named a certain way to look up its GPIO. That doesn't seem to be the case in the above files. Bartosz
пн, 9 февр. 2026 г. в 14:38, Bartosz Golaszewski <brgl@kernel.org>: > > On Mon, Feb 9, 2026 at 7:44 AM Dmitry Torokhov > <dmitry.torokhov@gmail.com> wrote: > > > > On Wed, Nov 19, 2025 at 10:13:30AM +0100, Bartosz Golaszewski wrote: > > > On Wed, Nov 19, 2025 at 9:41 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > > > > > On Wed, Nov 19, 2025 at 9:35 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > > > > > > > I have an idea for fixing it, let me cook up a patch. It'll still be a > > > > > bit hacky but will at least create a true link. > > > > > > > > > > > > > Scratch that, I didn't notice before but we register both devices from > > > > MFD core. We can just set up software nodes there. > > > > > > > > > > Here you go: https://lore.kernel.org/all/20251119-cs42l43-gpio-swnodes-v1-1-25996afebd97@linaro.org/ > > > > > > Please give it a try. This is independent from this series and should > > > probably be backported to stable. > > > > So think breaks more drivers: > > > > https://lore.kernel.org/all/aYkdKfP5fg6iywgr@jekhomev/ > > > > I think it may also break: > > > > arch/arm/mach-omap1/board-nokia770.c > > arch/arm/mach-pxa/devices.c > > arch/arm/mach-pxa/devices.h > > arch/arm/mach-pxa/gumstix.c > > arch/arm/mach-pxa/pxa25x.c > > arch/arm/mach-pxa/pxa27x.c > > arch/arm/mach-pxa/spitz.c > > arch/arm/mach-tegra/board-paz00.c > > Most of them seem to use software nodes correctly. Nokia 770 could > potentially break depending on the timing but the lookup uses the > right string. > > > arch/x86/platform/geode/geode-common.c > > Looks like a correct case of referencing the software node to me. > > > drivers/platform/x86/barco-p50-gpio.c > > drivers/platform/x86/pcengines-apuv2.c > > > > Same here. Nothing here seems to depend on a label and there are real > links between the GPIO chip's and consumer's software nodes. > > The problem we triggered here was caused by a GPIO consumer who would > create a bogus software node locally without any real link to the > provider. It would then depend on the provider being named a certain > way to look up its GPIO. That doesn't seem to be the case in the above > files. The driver drivers/platform/x86/x86-android-tablets/core.c also creates fake gpiochip software nodes to use them in PROPERTY_ENTRY_GPIO definitions (see drivers/platform/x86/x86-android-tablets/lenovo.c for examples). I use the same approach for rt5677 gpiochip in my changes for Lenovo YB1-X90 tablet (not mainlined yet) and this is broken also now. -- Yauhen Kharuzhy
[ resending to the right addresses for Bartosz and Linus ] On Sun, Feb 08, 2026 at 10:44:45PM -0800, Dmitry Torokhov wrote: > On Wed, Nov 19, 2025 at 10:13:30AM +0100, Bartosz Golaszewski wrote: > > On Wed, Nov 19, 2025 at 9:41 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > > > On Wed, Nov 19, 2025 at 9:35 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > > > > > I have an idea for fixing it, let me cook up a patch. It'll still be a > > > > bit hacky but will at least create a true link. > > > > > > > > > > Scratch that, I didn't notice before but we register both devices from > > > MFD core. We can just set up software nodes there. > > > > > > > Here you go: https://lore.kernel.org/all/20251119-cs42l43-gpio-swnodes-v1-1-25996afebd97@linaro.org/ > > > > Please give it a try. This is independent from this series and should > > probably be backported to stable. > > So think breaks more drivers: > > https://lore.kernel.org/all/aYkdKfP5fg6iywgr@jekhomev/ > > I think it may also break: > > arch/arm/mach-omap1/board-nokia770.c > arch/arm/mach-pxa/devices.c > arch/arm/mach-pxa/devices.h > arch/arm/mach-pxa/gumstix.c > arch/arm/mach-pxa/pxa25x.c > arch/arm/mach-pxa/pxa27x.c > arch/arm/mach-pxa/spitz.c > arch/arm/mach-tegra/board-paz00.c > arch/x86/platform/geode/geode-common.c > drivers/platform/x86/barco-p50-gpio.c > drivers/platform/x86/pcengines-apuv2.c > > Thanks. > -- Dmitry
On Tue, Nov 18, 2025 at 06:15:49PM +0000, Charles Keepax wrote:
> 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:
...
> > > 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.
Interestingly that Bart's series from where this patch came has more patches
targeting exactly this scenario, i.e. to allow to refer any type of fwnode from
swnode. Maybe we need those too and something on top?
> 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.
--
With Best Regards,
Andy Shevchenko
On Mon, Nov 03, 2025 at 10:35:24AM +0100, Bartosz Golaszewski wrote: > > 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. Sounds to me like a ready-to-go patch and even maybe with a Fixes tags, but it's up to you. So, why not apply it so we have less churn in the next version of the series? -- With Best Regards, Andy Shevchenko
On Mon, Nov 3, 2025 at 10:51 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Mon, Nov 03, 2025 at 10:35:24AM +0100, Bartosz Golaszewski wrote:
> >
> > 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.
>
> Sounds to me like a ready-to-go patch and even maybe with a Fixes tags, but
> it's up to you. So, why not apply it so we have less churn in the next version
> of the series?
>
Yeah, makes sense.
Fixes: e7f9ff5dc90c ("gpiolib: add support for software nodes")
© 2016 - 2026 Red Hat, Inc.