From: Charles Keepax <ckeepax@opensource.cirrus.com>
On some systems the cs42l43 has amplifiers attached to its SPI
controller that are not properly defined in ACPI. Currently
software nodes are added to support this case, however, the chip
selects for these devices are specified using a hack. A software
node is added with the same name as the pinctrl driver, as the
look up was name based, this allowed the GPIO look up to return
the pinctrl driver even though the swnode was not owned by it.
This was necessary as the swnodes did not support directly
linking to real firmware nodes.
Since commit e5d527be7e69 ("gpio: swnode: don't use the swnode's
name as the key for GPIO lookup") changed the lookup to be
fwnode based this hack will no longer find the pinctrl driver,
resulting in the driver not probing. There is no pinctrl driver
attached to the swnode itself. But other patches did add support
for linking a swnode to a real fwnode node [1]. As such the hack
is no longer needed, so switch over to just passing the real
fwnode for the pinctrl property to avoid any issues.
Link: https://lore.kernel.org/linux-gpio/20251106-reset-gpios-swnodes-v6-0-69aa852de9e4@linaro.org/ [1]
Fixes: 439fbc97502a ("spi: cs42l43: Add bridged cs35l56 amplifiers")
Cc: stable+noautosel@kernel.org # Don't backport, previous approach works, fix relies on swnode changes
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
[Bartosz:
- remove unneeded Fixes: tag,
- use PROPERTY_ENTRY_REF_ARRAY() instead of PROPERTY_ENTRY_REF_ARRAY_LEN()]
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/spi/spi-cs42l43.c | 40 ++++++++++------------------------------
1 file changed, 10 insertions(+), 30 deletions(-)
diff --git a/drivers/spi/spi-cs42l43.c b/drivers/spi/spi-cs42l43.c
index 14307dd800b744fee17edd864688a68c65666c68..4b6b65f450a86c238ba0c66e03462c775d12b8cd 100644
--- a/drivers/spi/spi-cs42l43.c
+++ b/drivers/spi/spi-cs42l43.c
@@ -52,20 +52,6 @@ static struct spi_board_info amp_info_template = {
.mode = SPI_MODE_0,
};
-static const struct software_node cs42l43_gpiochip_swnode = {
- .name = "cs42l43-pinctrl",
-};
-
-static const struct software_node_ref_args cs42l43_cs_refs[] = {
- SOFTWARE_NODE_REFERENCE(&cs42l43_gpiochip_swnode, 0, GPIO_ACTIVE_LOW),
- SOFTWARE_NODE_REFERENCE(&swnode_gpio_undefined),
-};
-
-static const struct property_entry cs42l43_cs_props[] = {
- PROPERTY_ENTRY_REF_ARRAY("cs-gpios", cs42l43_cs_refs),
- {}
-};
-
static int cs42l43_spi_tx(struct regmap *regmap, const u8 *buf, unsigned int len)
{
const u8 *end = buf + len;
@@ -324,11 +310,6 @@ static void cs42l43_release_of_node(void *data)
fwnode_handle_put(data);
}
-static void cs42l43_release_sw_node(void *data)
-{
- software_node_unregister(&cs42l43_gpiochip_swnode);
-}
-
static int cs42l43_spi_probe(struct platform_device *pdev)
{
struct cs42l43 *cs42l43 = dev_get_drvdata(pdev->dev.parent);
@@ -391,6 +372,15 @@ static int cs42l43_spi_probe(struct platform_device *pdev)
fwnode_property_read_u32(xu_fwnode, "01fa-sidecar-instances", &nsidecars);
if (nsidecars) {
+ struct software_node_ref_args args[] = {
+ SOFTWARE_NODE_REFERENCE(fwnode, 0, GPIO_ACTIVE_LOW),
+ SOFTWARE_NODE_REFERENCE(&swnode_gpio_undefined),
+ };
+ struct property_entry props[] = {
+ PROPERTY_ENTRY_REF_ARRAY("cs-gpios", args),
+ { }
+ };
+
ret = fwnode_property_read_u32(xu_fwnode, "01fa-spk-id-val", &spkid);
if (!ret) {
dev_dbg(priv->dev, "01fa-spk-id-val = %d\n", spkid);
@@ -403,17 +393,7 @@ static int cs42l43_spi_probe(struct platform_device *pdev)
"Failed to get spk-id-gpios\n");
}
- ret = software_node_register(&cs42l43_gpiochip_swnode);
- if (ret)
- return dev_err_probe(priv->dev, ret,
- "Failed to register gpio swnode\n");
-
- ret = devm_add_action_or_reset(priv->dev, cs42l43_release_sw_node, NULL);
- if (ret)
- return ret;
-
- ret = device_create_managed_software_node(&priv->ctlr->dev,
- cs42l43_cs_props, NULL);
+ ret = device_create_managed_software_node(&priv->ctlr->dev, props, NULL);
if (ret)
return dev_err_probe(priv->dev, ret, "Failed to add swnode\n");
} else {
--
2.51.0On Thu, Nov 20, 2025 at 02:23:59PM +0100, Bartosz Golaszewski wrote:
> From: Charles Keepax <ckeepax@opensource.cirrus.com>
>
> On some systems the cs42l43 has amplifiers attached to its SPI
> controller that are not properly defined in ACPI. Currently
> software nodes are added to support this case, however, the chip
> selects for these devices are specified using a hack. A software
> node is added with the same name as the pinctrl driver, as the
> look up was name based, this allowed the GPIO look up to return
> the pinctrl driver even though the swnode was not owned by it.
> This was necessary as the swnodes did not support directly
> linking to real firmware nodes.
>
> Since commit e5d527be7e69 ("gpio: swnode: don't use the swnode's
> name as the key for GPIO lookup") changed the lookup to be
> fwnode based this hack will no longer find the pinctrl driver,
> resulting in the driver not probing. There is no pinctrl driver
> attached to the swnode itself. But other patches did add support
> for linking a swnode to a real fwnode node [1]. As such the hack
> is no longer needed, so switch over to just passing the real
> fwnode for the pinctrl property to avoid any issues.
>
> Link: https://lore.kernel.org/linux-gpio/20251106-reset-gpios-swnodes-v6-0-69aa852de9e4@linaro.org/ [1]
> Fixes: 439fbc97502a ("spi: cs42l43: Add bridged cs35l56 amplifiers")
> Cc: stable+noautosel@kernel.org # Don't backport, previous approach works, fix relies on swnode changes
Just wanted to check what the thinking is on backports here. I
see we have marked this as do not backport. Which I think is
sensible the changes in the preceeding patches are a bit much for
a backport. However, the patch has caused the regression has gone
to a few stable branches (v6.17, v6.12):
commit e5d527be7e69 ("gpio: swnode: don't use the swnode's name as the key for GPIO lookup")
Are you guys ok if I send a revert for that patch to the stable
branches it has gone to? It doesn't actually fix any bugs on
those kernels, and we are starting to see issues coming in that I
think are related to this:
https://github.com/thesofproject/linux/issues/5599
Thanks,
Charles
On Mon, Nov 24, 2025 at 5:58 PM Charles Keepax
<ckeepax@opensource.cirrus.com> wrote:
>
> On Thu, Nov 20, 2025 at 02:23:59PM +0100, Bartosz Golaszewski wrote:
> > From: Charles Keepax <ckeepax@opensource.cirrus.com>
> >
> > On some systems the cs42l43 has amplifiers attached to its SPI
> > controller that are not properly defined in ACPI. Currently
> > software nodes are added to support this case, however, the chip
> > selects for these devices are specified using a hack. A software
> > node is added with the same name as the pinctrl driver, as the
> > look up was name based, this allowed the GPIO look up to return
> > the pinctrl driver even though the swnode was not owned by it.
> > This was necessary as the swnodes did not support directly
> > linking to real firmware nodes.
> >
> > Since commit e5d527be7e69 ("gpio: swnode: don't use the swnode's
> > name as the key for GPIO lookup") changed the lookup to be
> > fwnode based this hack will no longer find the pinctrl driver,
> > resulting in the driver not probing. There is no pinctrl driver
> > attached to the swnode itself. But other patches did add support
> > for linking a swnode to a real fwnode node [1]. As such the hack
> > is no longer needed, so switch over to just passing the real
> > fwnode for the pinctrl property to avoid any issues.
> >
> > Link: https://lore.kernel.org/linux-gpio/20251106-reset-gpios-swnodes-v6-0-69aa852de9e4@linaro.org/ [1]
> > Fixes: 439fbc97502a ("spi: cs42l43: Add bridged cs35l56 amplifiers")
> > Cc: stable+noautosel@kernel.org # Don't backport, previous approach works, fix relies on swnode changes
>
> Just wanted to check what the thinking is on backports here. I
> see we have marked this as do not backport. Which I think is
> sensible the changes in the preceeding patches are a bit much for
> a backport. However, the patch has caused the regression has gone
> to a few stable branches (v6.17, v6.12):
>
I totally forgot about this having gone into stable.
> commit e5d527be7e69 ("gpio: swnode: don't use the swnode's name as the key for GPIO lookup")
>
> Are you guys ok if I send a revert for that patch to the stable
> branches it has gone to? It doesn't actually fix any bugs on
> those kernels, and we are starting to see issues coming in that I
> think are related to this:
>
Yes, definitely, please do. You can add:
Acked-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Bart
On Thu, Nov 20, 2025 at 02:23:59PM +0100, Bartosz Golaszewski wrote: > From: Charles Keepax <ckeepax@opensource.cirrus.com> > > On some systems the cs42l43 has amplifiers attached to its SPI > controller that are not properly defined in ACPI. Currently > software nodes are added to support this case, however, the chip > selects for these devices are specified using a hack. A software > node is added with the same name as the pinctrl driver, as the > look up was name based, this allowed the GPIO look up to return > the pinctrl driver even though the swnode was not owned by it. > This was necessary as the swnodes did not support directly > linking to real firmware nodes. Acked-by: Mark Brown <broonie@kernel.org>
On Thu, Nov 20, 2025 at 02:23:59PM +0100, Bartosz Golaszewski wrote:
> On some systems the cs42l43 has amplifiers attached to its SPI
> controller that are not properly defined in ACPI. Currently
> software nodes are added to support this case, however, the chip
> selects for these devices are specified using a hack. A software
> node is added with the same name as the pinctrl driver, as the
> look up was name based, this allowed the GPIO look up to return
> the pinctrl driver even though the swnode was not owned by it.
> This was necessary as the swnodes did not support directly
> linking to real firmware nodes.
>
> Since commit e5d527be7e69 ("gpio: swnode: don't use the swnode's
> name as the key for GPIO lookup") changed the lookup to be
> fwnode based this hack will no longer find the pinctrl driver,
> resulting in the driver not probing. There is no pinctrl driver
> attached to the swnode itself. But other patches did add support
> for linking a swnode to a real fwnode node [1]. As such the hack
> is no longer needed, so switch over to just passing the real
> fwnode for the pinctrl property to avoid any issues.
I very much like this solution.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
--
With Best Regards,
Andy Shevchenko
© 2016 - 2025 Red Hat, Inc.