Re: [PATCH RFT/RFC] mfd: cs42l43: setup true links with software nodes

[Charles Keepax <ckeepax@opensource.cirrus.com>]

On Wed, Nov 19, 2025 at 10:40:30AM +0100, Bartosz Golaszewski wrote:
> On Wed, Nov 19, 2025 at 10:31 AM Charles Keepax
> <ckeepax@opensource.cirrus.com> wrote:
> >
> > On Wed, Nov 19, 2025 at 10:10:24AM +0100, Bartosz Golaszewski wrote:
> > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > >
> > > Currently the SPI driver sets up a software node rerefencing 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 so the actual link must exist.
> > > Move the configuration to the MFD core and connect the SPI and pinctrl
> > > sub-devices with software node references.
> > >
> > > 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>
> > > ---
> > > Hi Charles!
> > >
> > > Please give this a try. It's only build-tested so far. I hope I
> > > understood correctly that it's the SPI driver that needs the "cs" GPIO
> > > from the pinctrl.
> >
> > Unfortunately it fails with an -EBUSY on adding the MFD children.
> > I will investigate a little more and report back.
> 
> Does it fail in device_add_software_node() in MFD core? Is it possible
> that the device already has a software node for some reason?

Yeah it seems there is already a software node, although somewhat
at a loss as to why, the only ones we add are inside the SPI
driver. I will poke further and see if I can find out how it
acquires that node.

> > > +static const struct software_node cs42l43_gpiochip_swnode = {
> > > +     .name = "cs42l43-pinctrl",
> > > +};
> > > +
> > > +static const struct property_entry cs42l43_cs_props[] = {
> > > +     PROPERTY_ENTRY_GPIO("cs-gpios", &cs42l43_gpiochip_swnode, 0, GPIO_ACTIVE_LOW),
> > > +     { }
> > > +};
> >
> > This drops the undefined node, that is important as it lets the
> > SPI core know that device is using an internal chip select rather
> > than a GPIO.
> 
> I really dislike the whole concept of this undefined software node.
> This driver is literally the only user and I'd love to just get rid of
> it. HOW exactly does it let the driver know to use internal chip
> select? Can we do it differently?

The SPI code lives in spi_get_gpio_descs(). The basic gist is
it will use a native chip select if the gpiod_get_index return
NULL. The system came from device tree (see Documentation
spi-controller.yaml) initially, then I added an analogue to
swnodes a while back in 9d50f95bc0d5 ("gpio: swnode: Add
ability to specify native chip selects for SPI").  I have no
great attachment to this way of doing it, however, it does
seem logical to me that the system is at least fairly similar
between the different firmware types.

Thanks,
Charles