Message-ID: <CAMRc=MdcL0f9aE5emAsFLmwZoN5_-qM4JCSzP6D3J8D1PrsaEg@mail.gmail.com>
Date: Thu, 20 Nov 2025 10:29:41 +0100
From: Bartosz Golaszewski <brgl@...ev.pl>
To: Charles Keepax <ckeepax@...nsource.cirrus.com>
Cc: broonie@...nel.org, linus.walleij@...aro.org, andy@...nel.org, 
	p.zabel@...gutronix.de, linux-gpio@...r.kernel.org, linux-spi@...r.kernel.org, 
	bartosz.golaszewski@...aro.org, linux-kernel@...r.kernel.org, 
	patches@...nsource.cirrus.com
Subject: Re: [PATCH] spi: cs42l43: Use actual ACPI firmware node for chip selects

On Wed, Nov 19, 2025 at 5:40 PM Charles Keepax
<ckeepax@...nsource.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@...nsource.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

Powered by blists - more mailing lists