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@xxxxxxxx> wrote:
> > > From: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>
> > >
> > > 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@xxxxxxxxxxxxxxxxxxxxx>
> > > Closes: https://lore.kernel.org/all/aRyf7qDdHKABppP8@xxxxxxxxxxxxxxxxxxxxx/
> > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>
> > > ---
> > >
> > > +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