[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMRc=Mc==X3wERStdXmr5A9p0sPe7wdrPE4GZuqPLaKoBb9O9w@mail.gmail.com>
Date: Wed, 19 Nov 2025 10:40:30 +0100
From: Bartosz Golaszewski <brgl@...ev.pl>
To: Charles Keepax <ckeepax@...nsource.cirrus.com>
Cc: David Rhodes <david.rhodes@...rus.com>, Richard Fitzgerald <rf@...nsource.cirrus.com>,
Lee Jones <lee@...nel.org>, Mark Brown <broonie@...nel.org>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, Philipp Zabel <p.zabel@...gutronix.de>,
Linus Walleij <linus.walleij@...aro.org>, Maciej Strozek <mstrozek@...nsource.cirrus.com>,
Andy Shevchenko <andy@...nel.org>, linux-sound@...r.kernel.org,
patches@...nsource.cirrus.com, linux-kernel@...r.kernel.org,
linux-spi@...r.kernel.org,
Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Subject: Re: [PATCH RFT/RFC] mfd: cs42l43: setup true links with software nodes
On Wed, Nov 19, 2025 at 10:31 AM Charles Keepax
<ckeepax@...nsource.cirrus.com> wrote:
>
> On Wed, Nov 19, 2025 at 10:10:24AM +0100, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@...aro.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@...nsource.cirrus.com>
> > Closes: https://lore.kernel.org/all/aRyf7qDdHKABppP8@opensource.cirrus.com/
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@...aro.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?
> > +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?
> > +static const struct software_node cs42l43_spi_swnode = {
> > + .properties = cs42l43_cs_props,
> > +};
> > +
> > static const struct mfd_cell cs42l43_devs[] = {
> > - { .name = "cs42l43-pinctrl", },
> > - { .name = "cs42l43-spi", },
> > + {
> > + .name = "cs42l43-pinctrl",
> > + .swnode = &cs42l43_gpiochip_swnode,
> > + },
> > + {
> > + .name = "cs42l43-spi",
> > + .swnode = &cs42l43_spi_swnode,
>
> I will need to think about this a little more but I suspect this
> no longer being conditional on the magic ACPI properties could
> cause issues if a system was to use the SPI controller more
> traditionally and actually have CS GPIOs in the firmware.
> Although maybe that real node would take preference over the
> swnode?
>
The lookup goes to the primary firmware node first, it would typically
be DT or ACPI node. Then if no match, it will go to the secondary
node.
Bart
Powered by blists - more mailing lists