* [PATCH] spi: cs42l43: Use actual ACPI firmware node for chip selects
@ 2025-11-19 16:40 Charles Keepax
2025-11-19 16:45 ` Charles Keepax
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Charles Keepax @ 2025-11-19 16:40 UTC (permalink / raw)
To: broonie, brgl, linus.walleij
Cc: andy, p.zabel, linux-gpio, linux-spi, bartosz.golaszewski,
linux-kernel, patches
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 bit of a hack. A software
node is added with the same name as the pinctrl driver, as the look
up was name based this caused the GPIO looks to return the pinctrl
driver even though the swnode is not associated with the pinctrl
driver. 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. But other patches also add support
for linking a swnode to a real fwnode node [1]. As such switch over to
just passing the real fwnode for the pinctrl property to avoid any
issues.
[1] https://lore.kernel.org/linux-gpio/20251106-reset-gpios-swnodes-v6-0-69aa852de9e4@linaro.org/
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
Ok here is what I would propose to fix this one, IMPORTANT NOTE: this
does depend on the first four patches of the linked chain which I don't
think are merged yet. But I would argue if we are removing the name
based look up, we should add support for fwnodes at the same time.
drivers/spi/spi-cs42l43.c | 35 ++++++++++++++++-------------------
1 file changed, 16 insertions(+), 19 deletions(-)
diff --git a/drivers/spi/spi-cs42l43.c b/drivers/spi/spi-cs42l43.c
index 14307dd800b74..b7dd5f6ecc30e 100644
--- a/drivers/spi/spi-cs42l43.c
+++ b/drivers/spi/spi-cs42l43.c
@@ -52,12 +52,8 @@ 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(NULL, 0, GPIO_ACTIVE_LOW),
SOFTWARE_NODE_REFERENCE(&swnode_gpio_undefined),
};
@@ -324,11 +320,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 +382,9 @@ 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;
+ struct property_entry *props;
+
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 +397,20 @@ 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");
+ props = devm_kmemdup(priv->dev, cs42l43_cs_props, sizeof(cs42l43_cs_props),
+ GFP_KERNEL);
+ if (!props)
+ return -ENOMEM;
- ret = devm_add_action_or_reset(priv->dev, cs42l43_release_sw_node, NULL);
- if (ret)
- return ret;
+ args = devm_kmemdup(priv->dev, cs42l43_cs_refs, sizeof(cs42l43_cs_refs),
+ GFP_KERNEL);
+ if (!args)
+ return -ENOMEM;
+
+ args[0].fwnode = fwnode;
+ props->pointer = args;
- 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.47.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] spi: cs42l43: Use actual ACPI firmware node for chip selects
2025-11-19 16:40 [PATCH] spi: cs42l43: Use actual ACPI firmware node for chip selects Charles Keepax
@ 2025-11-19 16:45 ` Charles Keepax
2025-11-19 16:51 ` Andy Shevchenko
2025-11-19 16:49 ` Andy Shevchenko
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Charles Keepax @ 2025-11-19 16:45 UTC (permalink / raw)
To: broonie, brgl, linus.walleij
Cc: andy, p.zabel, linux-gpio, linux-spi, bartosz.golaszewski,
linux-kernel, patches
On Wed, Nov 19, 2025 at 04:40:17PM +0000, Charles Keepax 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 bit of a hack. A software
> node is added with the same name as the pinctrl driver, as the look
> up was name based this caused the GPIO looks to return the pinctrl
> driver even though the swnode is not associated with the pinctrl
> driver. 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. But other patches also add support
> for linking a swnode to a real fwnode node [1]. As such switch over to
> just passing the real fwnode for the pinctrl property to avoid any
> issues.
>
> [1] https://lore.kernel.org/linux-gpio/20251106-reset-gpios-swnodes-v6-0-69aa852de9e4@linaro.org/
>
> Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> ---
Apologies this probably should have a fixes tag, or two and I
probably should have marked it RFC. Lets have some discussion and
if people like the approach I will send a v2 with the tags
included.
Thanks,
Charles
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] spi: cs42l43: Use actual ACPI firmware node for chip selects
2025-11-19 16:40 [PATCH] spi: cs42l43: Use actual ACPI firmware node for chip selects Charles Keepax
2025-11-19 16:45 ` Charles Keepax
@ 2025-11-19 16:49 ` Andy Shevchenko
2025-11-19 17:04 ` Charles Keepax
2025-11-20 9:23 ` kernel test robot
2025-11-20 9:29 ` Bartosz Golaszewski
3 siblings, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2025-11-19 16:49 UTC (permalink / raw)
To: Charles Keepax
Cc: broonie, brgl, linus.walleij, andy, p.zabel, linux-gpio,
linux-spi, bartosz.golaszewski, linux-kernel, patches
On Wed, Nov 19, 2025 at 6:40 PM Charles Keepax
<ckeepax@opensource.cirrus.com> 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 bit of a hack. A software
> node is added with the same name as the pinctrl driver, as the look
> up was name based this caused the GPIO looks to return the pinctrl
> driver even though the swnode is not associated with the pinctrl
> driver. 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. But other patches also add support
> for linking a swnode to a real fwnode node [1]. As such switch over to
> just passing the real fwnode for the pinctrl property to avoid any
> issues.
> [1] https://lore.kernel.org/linux-gpio/20251106-reset-gpios-swnodes-v6-0-69aa852de9e4@linaro.org/
>
This can be
Link: ... [1]
actually.
> Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> ---
>
> Ok here is what I would propose to fix this one, IMPORTANT NOTE: this
> does depend on the first four patches of the linked chain which I don't
> think are merged yet. But I would argue if we are removing the name
> based look up, we should add support for fwnodes at the same time.
You mean it has functional dependency and not a compile-time one?
...
> static const struct software_node_ref_args cs42l43_cs_refs[] = {
> - SOFTWARE_NODE_REFERENCE(&cs42l43_gpiochip_swnode, 0, GPIO_ACTIVE_LOW),
> + SOFTWARE_NODE_REFERENCE(NULL, 0, GPIO_ACTIVE_LOW),
> SOFTWARE_NODE_REFERENCE(&swnode_gpio_undefined),
> };
Since it's a placeholder, we don't need it at all. See below.
...
> + args = devm_kmemdup(priv->dev, cs42l43_cs_refs, sizeof(cs42l43_cs_refs),
> + GFP_KERNEL);
> + if (!args)
> + return -ENOMEM;
> +
> + args[0].fwnode = fwnode;
You can assign entries directly here as
args = devm_kmalloc_array(...);
...
args[0] = SOFTWARE_NODE_REFERENCE(...);
args[1] = SOFTWARE_NODE_REFERENCE(&swnode_gpio_undefined),
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] spi: cs42l43: Use actual ACPI firmware node for chip selects
2025-11-19 16:45 ` Charles Keepax
@ 2025-11-19 16:51 ` Andy Shevchenko
0 siblings, 0 replies; 8+ messages in thread
From: Andy Shevchenko @ 2025-11-19 16:51 UTC (permalink / raw)
To: Charles Keepax
Cc: broonie, brgl, linus.walleij, andy, p.zabel, linux-gpio,
linux-spi, bartosz.golaszewski, linux-kernel, patches
On Wed, Nov 19, 2025 at 6:45 PM Charles Keepax
<ckeepax@opensource.cirrus.com> wrote:
> On Wed, Nov 19, 2025 at 04:40:17PM +0000, Charles Keepax wrote:
...
> Apologies this probably should have a fixes tag, or two and I
> probably should have marked it RFC. Lets have some discussion and
> if people like the approach I will send a v2 with the tags
> included.
At least I don't see anything quite wrong with it. But I probably need
to have a fresh look when you send a v2.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] spi: cs42l43: Use actual ACPI firmware node for chip selects
2025-11-19 16:49 ` Andy Shevchenko
@ 2025-11-19 17:04 ` Charles Keepax
0 siblings, 0 replies; 8+ messages in thread
From: Charles Keepax @ 2025-11-19 17:04 UTC (permalink / raw)
To: Andy Shevchenko
Cc: broonie, brgl, linus.walleij, andy, p.zabel, linux-gpio,
linux-spi, bartosz.golaszewski, linux-kernel, patches
On Wed, Nov 19, 2025 at 06:49:57PM +0200, Andy Shevchenko wrote:
> On Wed, Nov 19, 2025 at 6:40 PM Charles Keepax
> <ckeepax@opensource.cirrus.com> 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 bit of a hack. A software
> > node is added with the same name as the pinctrl driver, as the look
> > up was name based this caused the GPIO looks to return the pinctrl
> > driver even though the swnode is not associated with the pinctrl
> > driver. 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. But other patches also add support
> > for linking a swnode to a real fwnode node [1]. As such switch over to
> > just passing the real fwnode for the pinctrl property to avoid any
> > issues.
>
> > [1] https://lore.kernel.org/linux-gpio/20251106-reset-gpios-swnodes-v6-0-69aa852de9e4@linaro.org/
> >
>
> This can be
>
> Link: ... [1]
>
> actually.
Thanks will fixup for v2.
> > Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> > ---
> >
> > Ok here is what I would propose to fix this one, IMPORTANT NOTE: this
> > does depend on the first four patches of the linked chain which I don't
> > think are merged yet. But I would argue if we are removing the name
> > based look up, we should add support for fwnodes at the same time.
>
> You mean it has functional dependency and not a compile-time one?
Apologies that wasn't clear, both.
> > + args[0].fwnode = fwnode;
>
> You can assign entries directly here as
>
> args = devm_kmalloc_array(...);
> ...
> args[0] = SOFTWARE_NODE_REFERENCE(...);
> args[1] = SOFTWARE_NODE_REFERENCE(&swnode_gpio_undefined),
Yes thank you I will fix that up for v2 as well, I was a little
rushing this one out.
Thanks,
Charles
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] spi: cs42l43: Use actual ACPI firmware node for chip selects
2025-11-19 16:40 [PATCH] spi: cs42l43: Use actual ACPI firmware node for chip selects Charles Keepax
2025-11-19 16:45 ` Charles Keepax
2025-11-19 16:49 ` Andy Shevchenko
@ 2025-11-20 9:23 ` kernel test robot
2025-11-20 9:29 ` Bartosz Golaszewski
3 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2025-11-20 9:23 UTC (permalink / raw)
To: Charles Keepax, broonie, brgl, linus.walleij
Cc: oe-kbuild-all, andy, p.zabel, linux-gpio, linux-spi,
bartosz.golaszewski, linux-kernel, patches
Hi Charles,
kernel test robot noticed the following build errors:
[auto build test ERROR on broonie-spi/for-next]
[also build test ERROR on linus/master v6.18-rc6 next-20251119]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Charles-Keepax/spi-cs42l43-Use-actual-ACPI-firmware-node-for-chip-selects/20251120-005808
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
patch link: https://lore.kernel.org/r/20251119164017.1115791-1-ckeepax%40opensource.cirrus.com
patch subject: [PATCH] spi: cs42l43: Use actual ACPI firmware node for chip selects
config: sparc-randconfig-6002-20251120 (https://download.01.org/0day-ci/archive/20251120/202511201700.S7ieteHi-lkp@intel.com/config)
compiler: sparc-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251120/202511201700.S7ieteHi-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/202511201700.S7ieteHi-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/spi/spi-cs42l43.c: In function 'cs42l43_spi_probe':
>> drivers/spi/spi-cs42l43.c:410:25: error: 'struct software_node_ref_args' has no member named 'fwnode'; did you mean 'node'?
410 | args[0].fwnode = fwnode;
| ^~~~~~
| node
vim +410 drivers/spi/spi-cs42l43.c
322
323 static int cs42l43_spi_probe(struct platform_device *pdev)
324 {
325 struct cs42l43 *cs42l43 = dev_get_drvdata(pdev->dev.parent);
326 struct cs42l43_spi *priv;
327 struct fwnode_handle *fwnode = dev_fwnode(cs42l43->dev);
328 struct fwnode_handle *xu_fwnode __free(fwnode_handle) = cs42l43_find_xu_node(fwnode);
329 int nsidecars = 0;
330 int spkid = -EINVAL;
331 int ret;
332
333 priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
334 if (!priv)
335 return -ENOMEM;
336
337 priv->ctlr = devm_spi_alloc_host(&pdev->dev, sizeof(*priv->ctlr));
338 if (!priv->ctlr)
339 return -ENOMEM;
340
341 spi_controller_set_devdata(priv->ctlr, priv);
342
343 priv->dev = &pdev->dev;
344 priv->regmap = cs42l43->regmap;
345
346 priv->ctlr->prepare_message = cs42l43_prepare_message;
347 priv->ctlr->prepare_transfer_hardware = cs42l43_prepare_transfer_hardware;
348 priv->ctlr->unprepare_transfer_hardware = cs42l43_unprepare_transfer_hardware;
349 priv->ctlr->transfer_one = cs42l43_transfer_one;
350 priv->ctlr->set_cs = cs42l43_set_cs;
351 priv->ctlr->max_transfer_size = cs42l43_spi_max_length;
352 priv->ctlr->mode_bits = SPI_3WIRE | SPI_MODE_X_MASK;
353 priv->ctlr->flags = SPI_CONTROLLER_HALF_DUPLEX;
354 priv->ctlr->bits_per_word_mask = SPI_BPW_MASK(8) | SPI_BPW_MASK(16) |
355 SPI_BPW_MASK(32);
356 priv->ctlr->min_speed_hz = CS42L43_SPI_ROOT_HZ /
357 cs42l43_clock_divs[ARRAY_SIZE(cs42l43_clock_divs) - 1];
358 priv->ctlr->max_speed_hz = CS42L43_SPI_ROOT_HZ / cs42l43_clock_divs[0];
359 priv->ctlr->use_gpio_descriptors = true;
360 priv->ctlr->auto_runtime_pm = true;
361
362 ret = devm_pm_runtime_enable(priv->dev);
363 if (ret)
364 return ret;
365
366 pm_runtime_idle(priv->dev);
367
368 regmap_write(priv->regmap, CS42L43_TRAN_CONFIG6, CS42L43_FIFO_SIZE - 1);
369 regmap_write(priv->regmap, CS42L43_TRAN_CONFIG7, CS42L43_FIFO_SIZE - 1);
370
371 // Disable Watchdog timer and enable stall
372 regmap_write(priv->regmap, CS42L43_SPI_CONFIG3, 0);
373 regmap_write(priv->regmap, CS42L43_SPI_CONFIG4, CS42L43_SPI_STALL_ENA_MASK);
374
375 if (is_of_node(fwnode)) {
376 fwnode = fwnode_get_named_child_node(fwnode, "spi");
377 ret = devm_add_action_or_reset(priv->dev, cs42l43_release_of_node, fwnode);
378 if (ret)
379 return ret;
380 }
381
382 fwnode_property_read_u32(xu_fwnode, "01fa-sidecar-instances", &nsidecars);
383
384 if (nsidecars) {
385 struct software_node_ref_args *args;
386 struct property_entry *props;
387
388 ret = fwnode_property_read_u32(xu_fwnode, "01fa-spk-id-val", &spkid);
389 if (!ret) {
390 dev_dbg(priv->dev, "01fa-spk-id-val = %d\n", spkid);
391 } else if (ret != -EINVAL) {
392 return dev_err_probe(priv->dev, ret, "Failed to get spk-id-val\n");
393 } else {
394 ret = cs42l43_get_speaker_id_gpios(priv, &spkid);
395 if (ret < 0)
396 return dev_err_probe(priv->dev, ret,
397 "Failed to get spk-id-gpios\n");
398 }
399
400 props = devm_kmemdup(priv->dev, cs42l43_cs_props, sizeof(cs42l43_cs_props),
401 GFP_KERNEL);
402 if (!props)
403 return -ENOMEM;
404
405 args = devm_kmemdup(priv->dev, cs42l43_cs_refs, sizeof(cs42l43_cs_refs),
406 GFP_KERNEL);
407 if (!args)
408 return -ENOMEM;
409
> 410 args[0].fwnode = fwnode;
411 props->pointer = args;
412
413 ret = device_create_managed_software_node(&priv->ctlr->dev, props, NULL);
414 if (ret)
415 return dev_err_probe(priv->dev, ret, "Failed to add swnode\n");
416 } else {
417 device_set_node(&priv->ctlr->dev, fwnode);
418 }
419
420 ret = devm_spi_register_controller(priv->dev, priv->ctlr);
421 if (ret)
422 return dev_err_probe(priv->dev, ret,
423 "Failed to register SPI controller\n");
424
425 if (nsidecars) {
426 struct spi_board_info *ampl_info;
427 struct spi_board_info *ampr_info;
428
429 ampl_info = cs42l43_create_bridge_amp(priv, "cs35l56-left", 0, spkid);
430 if (!ampl_info)
431 return -ENOMEM;
432
433 ampr_info = cs42l43_create_bridge_amp(priv, "cs35l56-right", 1, spkid);
434 if (!ampr_info)
435 return -ENOMEM;
436
437 if (!spi_new_device(priv->ctlr, ampl_info))
438 return dev_err_probe(priv->dev, -ENODEV,
439 "Failed to create left amp slave\n");
440
441 if (!spi_new_device(priv->ctlr, ampr_info))
442 return dev_err_probe(priv->dev, -ENODEV,
443 "Failed to create right amp slave\n");
444 }
445
446 return 0;
447 }
448
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] spi: cs42l43: Use actual ACPI firmware node for chip selects
2025-11-19 16:40 [PATCH] spi: cs42l43: Use actual ACPI firmware node for chip selects Charles Keepax
` (2 preceding siblings ...)
2025-11-20 9:23 ` kernel test robot
@ 2025-11-20 9:29 ` Bartosz Golaszewski
2025-11-20 10:06 ` Charles Keepax
3 siblings, 1 reply; 8+ messages in thread
From: Bartosz Golaszewski @ 2025-11-20 9:29 UTC (permalink / raw)
To: Charles Keepax
Cc: broonie, linus.walleij, andy, p.zabel, linux-gpio, linux-spi,
bartosz.golaszewski, linux-kernel, patches
On Wed, Nov 19, 2025 at 5:40 PM Charles Keepax
<ckeepax@opensource.cirrus.com> 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 bit of a hack. A software
> node is added with the same name as the pinctrl driver, as the look
> up was name based this caused the GPIO looks to return the pinctrl
> driver even though the swnode is not associated with the pinctrl
> driver. 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. But other patches also add support
> for linking a swnode to a real fwnode node [1]. As such switch over to
> just passing the real fwnode for the pinctrl property to avoid any
> issues.
>
> [1] https://lore.kernel.org/linux-gpio/20251106-reset-gpios-swnodes-v6-0-69aa852de9e4@linaro.org/
>
> Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> ---
>
> Ok here is what I would propose to fix this one, IMPORTANT NOTE: this
> does depend on the first four patches of the linked chain which I don't
> think are merged yet. But I would argue if we are removing the name
> based look up, we should add support for fwnodes at the same time.
>
I would then suggest an ack from Mark and making it part of the larger
series coming after swnode changes but before GPIO.
If it works for you better than the ones I proposed then I'm fine with it.
> drivers/spi/spi-cs42l43.c | 35 ++++++++++++++++-------------------
> 1 file changed, 16 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/spi/spi-cs42l43.c b/drivers/spi/spi-cs42l43.c
> index 14307dd800b74..b7dd5f6ecc30e 100644
> --- a/drivers/spi/spi-cs42l43.c
> +++ b/drivers/spi/spi-cs42l43.c
> @@ -52,12 +52,8 @@ 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(NULL, 0, GPIO_ACTIVE_LOW),
> SOFTWARE_NODE_REFERENCE(&swnode_gpio_undefined),
> };
>
> @@ -324,11 +320,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 +382,9 @@ 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;
> + struct property_entry *props;
> +
> 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 +397,20 @@ 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");
> + props = devm_kmemdup(priv->dev, cs42l43_cs_props, sizeof(cs42l43_cs_props),
> + GFP_KERNEL);
> + if (!props)
> + return -ENOMEM;
You don't need to allocate it for more than the duration of this
function, device_create_managed_software_node() makes a deep copy of
the properties. They can be on the stack.
>
> - ret = devm_add_action_or_reset(priv->dev, cs42l43_release_sw_node, NULL);
> - if (ret)
> - return ret;
> + args = devm_kmemdup(priv->dev, cs42l43_cs_refs, sizeof(cs42l43_cs_refs),
> + GFP_KERNEL);
> + if (!args)
> + return -ENOMEM;
Same here.
> +
> + args[0].fwnode = fwnode;
> + props->pointer = args;
>
> - 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.47.3
>
This is looking good, if you post a v2 and it's reviewed, I can resend
my series with this included and maybe it'll still make v6.19.
Bart
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] spi: cs42l43: Use actual ACPI firmware node for chip selects
2025-11-20 9:29 ` Bartosz Golaszewski
@ 2025-11-20 10:06 ` Charles Keepax
0 siblings, 0 replies; 8+ messages in thread
From: Charles Keepax @ 2025-11-20 10:06 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: broonie, linus.walleij, andy, p.zabel, linux-gpio, linux-spi,
bartosz.golaszewski, linux-kernel, patches
On Thu, Nov 20, 2025 at 10:29:41AM +0100, Bartosz Golaszewski wrote:
> On Wed, Nov 19, 2025 at 5:40 PM Charles Keepax
> <ckeepax@opensource.cirrus.com> wrote:
> > + props = devm_kmemdup(priv->dev, cs42l43_cs_props, sizeof(cs42l43_cs_props),
> > + GFP_KERNEL);
> > + if (!props)
> > + return -ENOMEM;
>
> You don't need to allocate it for more than the duration of this
> function, device_create_managed_software_node() makes a deep copy of
> the properties. They can be on the stack.
Good point, thanks will fixup for v2. Should be able to send that
later today.
> This is looking good, if you post a v2 and it's reviewed, I can resend
> my series with this included and maybe it'll still make v6.19.
Cool yeah I am fine with you pulling this into your series once I
have sent the v2.
Thanks,
Charles
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-11-20 10:07 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-19 16:40 [PATCH] spi: cs42l43: Use actual ACPI firmware node for chip selects Charles Keepax
2025-11-19 16:45 ` Charles Keepax
2025-11-19 16:51 ` Andy Shevchenko
2025-11-19 16:49 ` Andy Shevchenko
2025-11-19 17:04 ` Charles Keepax
2025-11-20 9:23 ` kernel test robot
2025-11-20 9:29 ` Bartosz Golaszewski
2025-11-20 10:06 ` Charles Keepax
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).