[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aR3slKbafBPFrv6P@opensource.cirrus.com>
Date: Wed, 19 Nov 2025 16:13:08 +0000
From: Charles Keepax <ckeepax@...nsource.cirrus.com>
To: Bartosz Golaszewski <brgl@...ev.pl>
Cc: Linus Walleij <linus.walleij@...aro.org>,
David Rhodes <david.rhodes@...rus.com>,
Richard Fitzgerald <rf@...nsource.cirrus.com>,
Lee Jones <lee@...nel.org>, Mark Brown <broonie@...nel.org>,
Maciej Strozek <mstrozek@...nsource.cirrus.com>,
Andy Shevchenko <andy@...nel.org>,
Philipp Zabel <p.zabel@...gutronix.de>, linux-gpio@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-sound@...r.kernel.org,
patches@...nsource.cirrus.com, linux-spi@...r.kernel.org,
Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Subject: Re: [PATCH RFT/RFC 2/2] mfd: cs42l43: use GPIO machine lookup for
cs-gpios
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@...ev.pl> wrote:
> > > From: Bartosz Golaszewski <bartosz.golaszewski@...aro.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@...nsource.cirrus.com>
> > > Closes: https://lore.kernel.org/all/aRyf7qDdHKABppP8@opensource.cirrus.com/
> > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@...aro.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
Powered by blists - more mailing lists