[PATCH RFT/RFC 0/2] mfd: cs42l43: fix GPIO lookup for chip selects

[PATCH RFT/RFC 0/2] mfd: cs42l43: fix GPIO lookup for chip selects

[Bartosz Golaszewski]

It wouldn't leave me alone, so here's a proposal to address the software
node issue of this driver using GPIO lookups.

Link to the previous idea:
  https://lore.kernel.org/all/20251119-cs42l43-gpio-swnodes-v1-1-25996afebd97@linaro.org/

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
Bartosz Golaszewski (2):
      gpiolib: support "undefined" GPIO machine lookup
      mfd: cs42l43: use GPIO machine lookup for cs-gpios

 drivers/gpio/gpiolib.c    |  3 +++
 drivers/mfd/cs42l43.c     | 23 +++++++++++++++++++++++
 drivers/spi/spi-cs42l43.c | 35 -----------------------------------
 3 files changed, 26 insertions(+), 35 deletions(-)
---
base-commit: fe4d0dea039f2befb93f27569593ec209843b0f5
change-id: 20251119-cs42l43-gpio-lookup-3e3509bed0b3

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

[PATCH RFT/RFC 1/2] gpiolib: support "undefined" GPIO machine lookup

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

SPI devices can specify a cs-gpios property to enumerate their chip
selects. In device tree, a zero entry in this property can be used to
specify that a particular chip select is using the SPI controllers
native chip select, for example:

    cs-gpios = <&gpio1 0 0>, <0>;

Here, the second chip select is native. Allow users to pass
ERR_PTR(-ENOENT) as the key field in struct gpiod_lookup, indicating a
native chip select.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/gpio/gpiolib.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 91e0c384f34ae5c0ed61ccd3a978685d4f72e867..dbb5f7fe7b661979f559fcd32ad6e1c463431a18 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4557,6 +4557,9 @@ static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id,
 		if (p->con_id && (!con_id || strcmp(p->con_id, con_id)))
 			continue;
 
+		if (p->key == PTR_ERR(-ENOENT))
+			return ERR_PTR(-ENOENT);
+
 		if (p->chip_hwnum == U16_MAX) {
 			desc = gpio_name_to_desc(p->key);
 			if (desc) {

-- 
2.51.0

Re: [PATCH RFT/RFC 1/2] gpiolib: support "undefined" GPIO machine lookup

[kernel test robot]

Hi Bartosz,

[This is a private test report for your RFC patch.]
kernel test robot noticed the following build errors:

[auto build test ERROR on fe4d0dea039f2befb93f27569593ec209843b0f5]

url:    https://github.com/intel-lab-lkp/linux/commits/Bartosz-Golaszewski/gpiolib-support-undefined-GPIO-machine-lookup/20251119-233054
base:   fe4d0dea039f2befb93f27569593ec209843b0f5
patch link:    https://lore.kernel.org/r/20251119-cs42l43-gpio-lookup-v1-1-029b1d9e1843%40linaro.org
patch subject: [PATCH RFT/RFC 1/2] gpiolib: support "undefined" GPIO machine lookup
config: openrisc-allnoconfig (https://download.01.org/0day-ci/archive/20251120/202511200043.AUTK7uH5-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251120/202511200043.AUTK7uH5-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511200043.AUTK7uH5-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   drivers/gpio/gpiolib.c: In function 'gpiod_find':
>> drivers/gpio/gpiolib.c:4560:39: error: passing argument 1 of 'PTR_ERR' makes pointer from integer without a cast [-Wint-conversion]
    4560 |                 if (p->key == PTR_ERR(-ENOENT))
   In file included from include/linux/cleanup.h:6,
                    from include/linux/preempt.h:11,
                    from include/linux/spinlock.h:56,
                    from include/linux/mmzone.h:8,
                    from include/linux/gfp.h:7,
                    from include/linux/slab.h:16,
                    from include/linux/resource_ext.h:11,
                    from include/linux/acpi.h:13,
                    from drivers/gpio/gpiolib.c:3:
   include/linux/err.h:63:61: note: expected 'const void *' but argument is of type 'int'
      63 | static inline long __must_check PTR_ERR(__force const void *ptr)
         |                                                 ~~~~~~~~~~~~^~~
>> drivers/gpio/gpiolib.c:4560:28: warning: comparison between pointer and integer
    4560 |                 if (p->key == PTR_ERR(-ENOENT))
         |                            ^~


vim +/PTR_ERR +4560 drivers/gpio/gpiolib.c

  4536	
  4537	static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id,
  4538					    unsigned int idx, unsigned long *flags)
  4539	{
  4540		struct gpio_desc *desc = ERR_PTR(-ENOENT);
  4541		struct gpiod_lookup_table *table;
  4542		struct gpiod_lookup *p;
  4543		struct gpio_chip *gc;
  4544	
  4545		guard(mutex)(&gpio_lookup_lock);
  4546	
  4547		table = gpiod_find_lookup_table(dev);
  4548		if (!table)
  4549			return desc;
  4550	
  4551		for (p = &table->table[0]; p->key; p++) {
  4552			/* idx must always match exactly */
  4553			if (p->idx != idx)
  4554				continue;
  4555	
  4556			/* If the lookup entry has a con_id, require exact match */
  4557			if (p->con_id && (!con_id || strcmp(p->con_id, con_id)))
  4558				continue;
  4559	
> 4560			if (p->key == PTR_ERR(-ENOENT))
  4561				return ERR_PTR(-ENOENT);
  4562	
  4563			if (p->chip_hwnum == U16_MAX) {
  4564				desc = gpio_name_to_desc(p->key);
  4565				if (desc) {
  4566					*flags = p->flags;
  4567					return desc;
  4568				}
  4569	
  4570				dev_warn(dev, "cannot find GPIO line %s, deferring\n",
  4571					 p->key);
  4572				return ERR_PTR(-EPROBE_DEFER);
  4573			}
  4574	
  4575			struct gpio_device *gdev __free(gpio_device_put) =
  4576						gpio_device_find_by_label(p->key);
  4577			if (!gdev) {
  4578				/*
  4579				 * As the lookup table indicates a chip with
  4580				 * p->key should exist, assume it may
  4581				 * still appear later and let the interested
  4582				 * consumer be probed again or let the Deferred
  4583				 * Probe infrastructure handle the error.
  4584				 */
  4585				dev_warn(dev, "cannot find GPIO chip %s, deferring\n",
  4586					 p->key);
  4587				return ERR_PTR(-EPROBE_DEFER);
  4588			}
  4589	
  4590			gc = gpio_device_get_chip(gdev);
  4591	
  4592			if (gc->ngpio <= p->chip_hwnum) {
  4593				dev_err(dev,
  4594					"requested GPIO %u (%u) is out of range [0..%u] for chip %s\n",
  4595					idx, p->chip_hwnum, gc->ngpio - 1,
  4596					gc->label);
  4597				return ERR_PTR(-EINVAL);
  4598			}
  4599	
  4600			desc = gpio_device_get_desc(gdev, p->chip_hwnum);
  4601			*flags = p->flags;
  4602	
  4603			return desc;
  4604		}
  4605	
  4606		return desc;
  4607	}
  4608	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH RFT/RFC 1/2] gpiolib: support "undefined" GPIO machine lookup

[Bartosz Golaszewski]

On Wed, Nov 19, 2025 at 4:21 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> SPI devices can specify a cs-gpios property to enumerate their chip
> selects. In device tree, a zero entry in this property can be used to
> specify that a particular chip select is using the SPI controllers
> native chip select, for example:
>
>     cs-gpios = <&gpio1 0 0>, <0>;
>
> Here, the second chip select is native. Allow users to pass
> ERR_PTR(-ENOENT) as the key field in struct gpiod_lookup, indicating a
> native chip select.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
>  drivers/gpio/gpiolib.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 91e0c384f34ae5c0ed61ccd3a978685d4f72e867..dbb5f7fe7b661979f559fcd32ad6e1c463431a18 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -4557,6 +4557,9 @@ static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id,
>                 if (p->con_id && (!con_id || strcmp(p->con_id, con_id)))
>                         continue;
>
> +               if (p->key == PTR_ERR(-ENOENT))

As the build bot pointed out - this should have been ERR_PTR(-ENOENT).

Bart

> +                       return ERR_PTR(-ENOENT);
> +
>                 if (p->chip_hwnum == U16_MAX) {
>                         desc = gpio_name_to_desc(p->key);
>                         if (desc) {
>
> --
> 2.51.0
>

Re: [PATCH RFT/RFC 1/2] gpiolib: support "undefined" GPIO machine lookup

[kernel test robot]

Hi Bartosz,

[This is a private test report for your RFC patch.]
kernel test robot noticed the following build errors:

[auto build test ERROR on fe4d0dea039f2befb93f27569593ec209843b0f5]

url:    https://github.com/intel-lab-lkp/linux/commits/Bartosz-Golaszewski/gpiolib-support-undefined-GPIO-machine-lookup/20251119-233054
base:   fe4d0dea039f2befb93f27569593ec209843b0f5
patch link:    https://lore.kernel.org/r/20251119-cs42l43-gpio-lookup-v1-1-029b1d9e1843%40linaro.org
patch subject: [PATCH RFT/RFC 1/2] gpiolib: support "undefined" GPIO machine lookup
config: loongarch-allnoconfig (https://download.01.org/0day-ci/archive/20251120/202511200320.9ITKS8Am-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 0bba1e76581bad04e7d7f09f5115ae5e2989e0d9)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251120/202511200320.9ITKS8Am-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511200320.9ITKS8Am-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

>> drivers/gpio/gpiolib.c:4560:25: error: incompatible integer to pointer conversion passing 'int' to parameter of type 'const void *' [-Wint-conversion]
    4560 |                 if (p->key == PTR_ERR(-ENOENT))
         |                                       ^~~~~~~
   include/linux/err.h:63:61: note: passing argument to parameter 'ptr' here
      63 | static inline long __must_check PTR_ERR(__force const void *ptr)
         |                                                             ^
>> drivers/gpio/gpiolib.c:4560:14: warning: comparison between pointer and integer ('const char *' and 'long') [-Wpointer-integer-compare]
    4560 |                 if (p->key == PTR_ERR(-ENOENT))
         |                     ~~~~~~ ^  ~~~~~~~~~~~~~~~~
   1 warning and 1 error generated.


vim +4560 drivers/gpio/gpiolib.c

  4536	
  4537	static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id,
  4538					    unsigned int idx, unsigned long *flags)
  4539	{
  4540		struct gpio_desc *desc = ERR_PTR(-ENOENT);
  4541		struct gpiod_lookup_table *table;
  4542		struct gpiod_lookup *p;
  4543		struct gpio_chip *gc;
  4544	
  4545		guard(mutex)(&gpio_lookup_lock);
  4546	
  4547		table = gpiod_find_lookup_table(dev);
  4548		if (!table)
  4549			return desc;
  4550	
  4551		for (p = &table->table[0]; p->key; p++) {
  4552			/* idx must always match exactly */
  4553			if (p->idx != idx)
  4554				continue;
  4555	
  4556			/* If the lookup entry has a con_id, require exact match */
  4557			if (p->con_id && (!con_id || strcmp(p->con_id, con_id)))
  4558				continue;
  4559	
> 4560			if (p->key == PTR_ERR(-ENOENT))
  4561				return ERR_PTR(-ENOENT);
  4562	
  4563			if (p->chip_hwnum == U16_MAX) {
  4564				desc = gpio_name_to_desc(p->key);
  4565				if (desc) {
  4566					*flags = p->flags;
  4567					return desc;
  4568				}
  4569	
  4570				dev_warn(dev, "cannot find GPIO line %s, deferring\n",
  4571					 p->key);
  4572				return ERR_PTR(-EPROBE_DEFER);
  4573			}
  4574	
  4575			struct gpio_device *gdev __free(gpio_device_put) =
  4576						gpio_device_find_by_label(p->key);
  4577			if (!gdev) {
  4578				/*
  4579				 * As the lookup table indicates a chip with
  4580				 * p->key should exist, assume it may
  4581				 * still appear later and let the interested
  4582				 * consumer be probed again or let the Deferred
  4583				 * Probe infrastructure handle the error.
  4584				 */
  4585				dev_warn(dev, "cannot find GPIO chip %s, deferring\n",
  4586					 p->key);
  4587				return ERR_PTR(-EPROBE_DEFER);
  4588			}
  4589	
  4590			gc = gpio_device_get_chip(gdev);
  4591	
  4592			if (gc->ngpio <= p->chip_hwnum) {
  4593				dev_err(dev,
  4594					"requested GPIO %u (%u) is out of range [0..%u] for chip %s\n",
  4595					idx, p->chip_hwnum, gc->ngpio - 1,
  4596					gc->label);
  4597				return ERR_PTR(-EINVAL);
  4598			}
  4599	
  4600			desc = gpio_device_get_desc(gdev, p->chip_hwnum);
  4601			*flags = p->flags;
  4602	
  4603			return desc;
  4604		}
  4605	
  4606		return desc;
  4607	}
  4608	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH RFT/RFC 2/2] mfd: cs42l43: use GPIO machine lookup for cs-gpios

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Currently the SPI driver sets up a software node referencing the GPIO
controller exposing the chip-select GPIO but this node never gets
attached to the actual GPIO provider. The lookup uses the weird way GPIO
core performs the software node lookup by the swnode's name. We want to
switch to a true firmware node lookup in GPIO core but without the true
link, this driver will break.

We can't use software nodes as only one software node per device is
allowed and the ACPI node the MFD device uses has a secondary node
already attached.

Let's switch to GPIO machine lookup instead.

Fixes: 439fbc97502a ("spi: cs42l43: Add bridged cs35l56 amplifiers")
Reported-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Closes: https://lore.kernel.org/all/aRyf7qDdHKABppP8@opensource.cirrus.com/
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/mfd/cs42l43.c     | 23 +++++++++++++++++++++++
 drivers/spi/spi-cs42l43.c | 35 -----------------------------------
 2 files changed, 23 insertions(+), 35 deletions(-)

diff --git a/drivers/mfd/cs42l43.c b/drivers/mfd/cs42l43.c
index 107cfb983fec416bbdd31caa1e6a569026467735..098b95a21b38158b5ea765ad55fdcf703a2454c0 100644
--- a/drivers/mfd/cs42l43.c
+++ b/drivers/mfd/cs42l43.c
@@ -14,6 +14,7 @@
 #include <linux/err.h>
 #include <linux/firmware.h>
 #include <linux/gpio/consumer.h>
+#include <linux/gpio/machine.h>
 #include <linux/jiffies.h>
 #include <linux/mfd/core.h>
 #include <linux/mfd/cs42l43.h>
@@ -522,6 +523,15 @@ static const struct mfd_cell cs42l43_devs[] = {
 	},
 };
 
+static struct gpiod_lookup_table cs42l43_gpio_lookup = {
+	.dev_id = "cs42l43-spi",
+	.table = {
+		GPIO_LOOKUP("cs42l43-pinctrl", 0, "cs", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP(INIT_ERR_PTR(-ENOENT), 0, "cs", 0),
+		{ }
+	},
+};
+
 /*
  * If the device is connected over Soundwire, as well as soft resetting the
  * device, this function will also way for the device to detach from the bus
@@ -900,6 +910,11 @@ static int cs42l43_irq_config(struct cs42l43 *cs42l43)
 	return 0;
 }
 
+static void cs42l43_remove_gpio_lookup(void *data)
+{
+	gpiod_remove_lookup_table(&cs42l43_gpio_lookup);
+}
+
 static void cs42l43_boot_work(struct work_struct *work)
 {
 	struct cs42l43 *cs42l43 = container_of(work, struct cs42l43, boot_work);
@@ -954,6 +969,14 @@ static void cs42l43_boot_work(struct work_struct *work)
 	if (ret)
 		goto err;
 
+	gpiod_add_lookup_table(&cs42l43_gpio_lookup);
+
+	ret = devm_add_action_or_reset(cs42l43->dev, cs42l43_remove_gpio_lookup, NULL);
+	if (ret) {
+		dev_err(cs42l43->dev, "Failed to schedule a devres action: %d\n", ret);
+		goto err;
+	}
+
 	ret = devm_mfd_add_devices(cs42l43->dev, PLATFORM_DEVID_NONE,
 				   cs42l43_devs, ARRAY_SIZE(cs42l43_devs),
 				   NULL, 0, NULL);
diff --git a/drivers/spi/spi-cs42l43.c b/drivers/spi/spi-cs42l43.c
index 14307dd800b744fee17edd864688a68c65666c68..7430a1ba829cbd68d4bfdb1cf9cf8fa7a1cf23a6 100644
--- a/drivers/spi/spi-cs42l43.c
+++ b/drivers/spi/spi-cs42l43.c
@@ -13,8 +13,6 @@
 #include <linux/device.h>
 #include <linux/errno.h>
 #include <linux/gpio/consumer.h>
-#include <linux/gpio/machine.h>
-#include <linux/gpio/property.h>
 #include <linux/mfd/cs42l43.h>
 #include <linux/mfd/cs42l43-regs.h>
 #include <linux/mod_devicetable.h>
@@ -52,20 +50,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 +308,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);
@@ -402,20 +381,6 @@ static int cs42l43_spi_probe(struct platform_device *pdev)
 				return dev_err_probe(priv->dev, ret,
 						     "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);
-		if (ret)
-			return dev_err_probe(priv->dev, ret, "Failed to add swnode\n");
 	} else {
 		device_set_node(&priv->ctlr->dev, fwnode);
 	}

-- 
2.51.0

Re: [PATCH RFT/RFC 2/2] mfd: cs42l43: use GPIO machine lookup for cs-gpios

[Bartosz Golaszewski]

On Wed, Nov 19, 2025 at 4:21 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> Currently the SPI driver sets up a software node referencing the GPIO
> controller exposing the chip-select GPIO but this node never gets
> attached to the actual GPIO provider. The lookup uses the weird way GPIO
> core performs the software node lookup by the swnode's name. We want to
> switch to a true firmware node lookup in GPIO core but without the true
> link, this driver will break.
>
> We can't use software nodes as only one software node per device is
> allowed and the ACPI node the MFD device uses has a secondary node
> already attached.
>
> Let's switch to GPIO machine lookup instead.
>
> Fixes: 439fbc97502a ("spi: cs42l43: Add bridged cs35l56 amplifiers")
> Reported-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> Closes: https://lore.kernel.org/all/aRyf7qDdHKABppP8@opensource.cirrus.com/
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
>
> +static struct gpiod_lookup_table cs42l43_gpio_lookup = {
> +       .dev_id = "cs42l43-spi",
> +       .table = {
> +               GPIO_LOOKUP("cs42l43-pinctrl", 0, "cs", GPIO_ACTIVE_LOW),
> +               GPIO_LOOKUP(INIT_ERR_PTR(-ENOENT), 0, "cs", 0),

I sent the wrong version, sorry. This should have been:

GPIO_LOOKUP_IDX("cs42l43-pinctrl", 0, "cs", 0, GPIO_ACTIVE_LOW),
GPIO_LOOKUP_IDX(INIT_ERR_PTR(-ENOENT), 0, "cs", 1, 0),

Charles: Can you fix it up yourself before testing?

Bart

Re: [PATCH RFT/RFC 2/2] mfd: cs42l43: use GPIO machine lookup for cs-gpios

[Charles Keepax]

On Wed, Nov 19, 2025 at 04:35:07PM +0100, Bartosz Golaszewski wrote:
> On Wed, Nov 19, 2025 at 4:21 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > Currently the SPI driver sets up a software node referencing the GPIO
> > controller exposing the chip-select GPIO but this node never gets
> > attached to the actual GPIO provider. The lookup uses the weird way GPIO
> > core performs the software node lookup by the swnode's name. We want to
> > switch to a true firmware node lookup in GPIO core but without the true
> > link, this driver will break.
> >
> > We can't use software nodes as only one software node per device is
> > allowed and the ACPI node the MFD device uses has a secondary node
> > already attached.
> >
> > Let's switch to GPIO machine lookup instead.
> >
> > Fixes: 439fbc97502a ("spi: cs42l43: Add bridged cs35l56 amplifiers")
> > Reported-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> > Closes: https://lore.kernel.org/all/aRyf7qDdHKABppP8@opensource.cirrus.com/
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > ---
> >
> > +static struct gpiod_lookup_table cs42l43_gpio_lookup = {
> > +       .dev_id = "cs42l43-spi",
> > +       .table = {
> > +               GPIO_LOOKUP("cs42l43-pinctrl", 0, "cs", GPIO_ACTIVE_LOW),
> > +               GPIO_LOOKUP(INIT_ERR_PTR(-ENOENT), 0, "cs", 0),
> 
> I sent the wrong version, sorry. This should have been:
> 
> GPIO_LOOKUP_IDX("cs42l43-pinctrl", 0, "cs", 0, GPIO_ACTIVE_LOW),
> GPIO_LOOKUP_IDX(INIT_ERR_PTR(-ENOENT), 0, "cs", 1, 0),
> 
> Charles: Can you fix it up yourself before testing?

Yeah can do, but I am still very nervous about how this approach
interacts with device tree and ACPI systems doing things
normally. Is this also ignored if the firmware specifies the
properties correctly? I feel like if we go this route I am going
to have to bring up a few extra things to test on as its quite a
big change.

Apologies for the delay on my suggestion but something weird is
happening deep in the swnode stuff and its taking me ages to peel
back all the layers of in abstraction there. It seems something
copies all the properties and somehow the fwnode I want doesn't
make it through that process. But the basic idea is like:

props = devm_kmemdup(priv->dev, cs42l43_cs_props,
		     sizeof(cs42l43_cs_props), GFP_KERNEL);
if (!props)
	return -ENOMEM;

args = devm_kmemdup(priv->dev, cs42l43_cs_refs,
		    sizeof(cs42l43_cs_refs), GFP_KERNEL);
if (!args)
	return -ENOMEM;

args[0].fwnode = fwnode;
props->pointer = props;

ie. As your patches add support for using the geniune firmware
node just do so.

Thanks,
Charles

Re: [PATCH RFT/RFC 2/2] mfd: cs42l43: use GPIO machine lookup for cs-gpios

[Charles Keepax]

On Wed, Nov 19, 2025 at 03:52:01PM +0000, Charles Keepax wrote:
> On Wed, Nov 19, 2025 at 04:35:07PM +0100, Bartosz Golaszewski wrote:
> > On Wed, Nov 19, 2025 at 4:21 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > >
> > > Currently the SPI driver sets up a software node referencing the GPIO
> > > controller exposing the chip-select GPIO but this node never gets
> > > attached to the actual GPIO provider. The lookup uses the weird way GPIO
> > > core performs the software node lookup by the swnode's name. We want to
> > > switch to a true firmware node lookup in GPIO core but without the true
> > > link, this driver will break.
> > >
> > > We can't use software nodes as only one software node per device is
> > > allowed and the ACPI node the MFD device uses has a secondary node
> > > already attached.
> > >
> > > Let's switch to GPIO machine lookup instead.
> > >
> > > Fixes: 439fbc97502a ("spi: cs42l43: Add bridged cs35l56 amplifiers")
> > > Reported-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> > > Closes: https://lore.kernel.org/all/aRyf7qDdHKABppP8@opensource.cirrus.com/
> > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > > ---
> > >
> > > +static struct gpiod_lookup_table cs42l43_gpio_lookup = {
> > > +       .dev_id = "cs42l43-spi",
> > > +       .table = {
> > > +               GPIO_LOOKUP("cs42l43-pinctrl", 0, "cs", GPIO_ACTIVE_LOW),
> > > +               GPIO_LOOKUP(INIT_ERR_PTR(-ENOENT), 0, "cs", 0),
> > 
> > I sent the wrong version, sorry. This should have been:
> > 
> > GPIO_LOOKUP_IDX("cs42l43-pinctrl", 0, "cs", 0, GPIO_ACTIVE_LOW),
> > GPIO_LOOKUP_IDX(INIT_ERR_PTR(-ENOENT), 0, "cs", 1, 0),
> > 
> > Charles: Can you fix it up yourself before testing?
> 
> Yeah can do, but I am still very nervous about how this approach
> interacts with device tree and ACPI systems doing things
> normally. Is this also ignored if the firmware specifies the
> properties correctly? I feel like if we go this route I am going
> to have to bring up a few extra things to test on as its quite a
> big change.
> 
> Apologies for the delay on my suggestion but something weird is
> happening deep in the swnode stuff and its taking me ages to peel
> back all the layers of in abstraction there. It seems something
> copies all the properties and somehow the fwnode I want doesn't
> make it through that process. But the basic idea is like:
> 
> props = devm_kmemdup(priv->dev, cs42l43_cs_props,
> 		     sizeof(cs42l43_cs_props), GFP_KERNEL);
> if (!props)
> 	return -ENOMEM;
> 
> args = devm_kmemdup(priv->dev, cs42l43_cs_refs,
> 		    sizeof(cs42l43_cs_refs), GFP_KERNEL);
> if (!args)
> 	return -ENOMEM;
> 
> args[0].fwnode = fwnode;
> props->pointer = props;

blargh... and of course that should be = args;

Thanks,
Charles

Re: [PATCH RFT/RFC 2/2] mfd: cs42l43: use GPIO machine lookup for cs-gpios

[Bartosz Golaszewski]

On Wed, Nov 19, 2025 at 4:52 PM Charles Keepax
<ckeepax@opensource.cirrus.com> wrote:
>
> On Wed, Nov 19, 2025 at 04:35:07PM +0100, Bartosz Golaszewski wrote:
> > On Wed, Nov 19, 2025 at 4:21 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > >
> > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > >
> > > Currently the SPI driver sets up a software node referencing the GPIO
> > > controller exposing the chip-select GPIO but this node never gets
> > > attached to the actual GPIO provider. The lookup uses the weird way GPIO
> > > core performs the software node lookup by the swnode's name. We want to
> > > switch to a true firmware node lookup in GPIO core but without the true
> > > link, this driver will break.
> > >
> > > We can't use software nodes as only one software node per device is
> > > allowed and the ACPI node the MFD device uses has a secondary node
> > > already attached.
> > >
> > > Let's switch to GPIO machine lookup instead.
> > >
> > > Fixes: 439fbc97502a ("spi: cs42l43: Add bridged cs35l56 amplifiers")
> > > Reported-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> > > Closes: https://lore.kernel.org/all/aRyf7qDdHKABppP8@opensource.cirrus.com/
> > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > > ---
> > >
> > > +static struct gpiod_lookup_table cs42l43_gpio_lookup = {
> > > +       .dev_id = "cs42l43-spi",
> > > +       .table = {
> > > +               GPIO_LOOKUP("cs42l43-pinctrl", 0, "cs", GPIO_ACTIVE_LOW),
> > > +               GPIO_LOOKUP(INIT_ERR_PTR(-ENOENT), 0, "cs", 0),
> >
> > I sent the wrong version, sorry. This should have been:
> >
> > GPIO_LOOKUP_IDX("cs42l43-pinctrl", 0, "cs", 0, GPIO_ACTIVE_LOW),
> > GPIO_LOOKUP_IDX(INIT_ERR_PTR(-ENOENT), 0, "cs", 1, 0),
> >
> > Charles: Can you fix it up yourself before testing?
>
> Yeah can do, but I am still very nervous about how this approach
> interacts with device tree and ACPI systems doing things
> normally. Is this also ignored if the firmware specifies the
> properties correctly? I feel like if we go this route I am going
> to have to bring up a few extra things to test on as its quite a
> big change.
>

The machine lookup always comes after the firmware node lookup. Even
after the secondary node. It's the last-ditch effort to find a match
actually.

> Apologies for the delay on my suggestion but something weird is
> happening deep in the swnode stuff and its taking me ages to peel
> back all the layers of in abstraction there. It seems something
> copies all the properties and somehow the fwnode I want doesn't
> make it through that process. But the basic idea is like:
>
> props = devm_kmemdup(priv->dev, cs42l43_cs_props,
>                      sizeof(cs42l43_cs_props), GFP_KERNEL);
> if (!props)
>         return -ENOMEM;
>
> args = devm_kmemdup(priv->dev, cs42l43_cs_refs,
>                     sizeof(cs42l43_cs_refs), GFP_KERNEL);
> if (!args)
>         return -ENOMEM;
>
> args[0].fwnode = fwnode;
> props->pointer = props;
>
> ie. As your patches add support for using the geniune firmware
> node just do so.
>

But do you have the firmware node at the time of doing the above? The
software node must first be registered with the swnode subsystem to
become an actual firmware node. Only then can you reference it by its
firmware node address.

Bart

Re: [PATCH RFT/RFC 2/2] mfd: cs42l43: use GPIO machine lookup for cs-gpios

[Charles Keepax]

On Wed, Nov 19, 2025 at 05:28:54PM +0100, Bartosz Golaszewski wrote:
> On Wed, Nov 19, 2025 at 4:52 PM Charles Keepax
> <ckeepax@opensource.cirrus.com> wrote:
> > On Wed, Nov 19, 2025 at 04:35:07PM +0100, Bartosz Golaszewski wrote:
> > > On Wed, Nov 19, 2025 at 4:21 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > props = devm_kmemdup(priv->dev, cs42l43_cs_props,
> >                      sizeof(cs42l43_cs_props), GFP_KERNEL);
> > if (!props)
> >         return -ENOMEM;
> >
> > args = devm_kmemdup(priv->dev, cs42l43_cs_refs,
> >                     sizeof(cs42l43_cs_refs), GFP_KERNEL);
> > if (!args)
> >         return -ENOMEM;
> >
> > args[0].fwnode = fwnode;
> > props->pointer = props;
> >
> > ie. As your patches add support for using the geniune firmware
> > node just do so.
> >
> 
> But do you have the firmware node at the time of doing the above? The
> software node must first be registered with the swnode subsystem to
> become an actual firmware node. Only then can you reference it by its
> firmware node address.

The firmware node here isn't referring to a software node it is
referring to the actual ACPI node. As in we are now correctly
specifying the GPIO using the ACPI node rather than depending on
the name based lookup to find the driver. The problem was before
your patches we couldn't do that as it wasn't possible.

I have sent a patch for you guys to have a look at and see what
you think. I am slightly inclined to favour that approach since
it changes a lot less about the system and once we are no longer
relying on name based look up I think everything the driver is
doing is all above board and legit.

Thanks,
Charels