Re: [PATCH] spi: cs42l43: Workaround for wrong speaker ID on Dell XPS 13 DX13260

[Richard Fitzgerald]

On 20/9/26 16:14, Mark Brown wrote:
> On Sat, Sep 19, 2026 at 02:47:30PM +0100, Richard Fitzgerald wrote:
> > Create an acpi_gpio_mapping so that speaker ID GPIOs can be read directly
> > from the ACPI GpioIo() instead of relying on the spk-id-gpios property to
...
> Is there an overlap with cs35l56_try_get_broken_sdca_spkid_gpio() here?
> Didn't check thoroughly, and if they are doing the same thing it's
> probably reasonable to factor out incrementally.
Only slightly. That is fixing a completely different problem that 
affects a couple of old Dell models. The normal non-broken speaker ID in
the cs35l56 driver will need the same change. As this is a workaround I
didn't want to complicate backporting by entangling it with the codec
driver just to avoid code duplication. I'll worry about that later.

> kzalloc_obj() needs slab.h (which will be implicitly included anyway but
> still).
Acked

> Could we see an -EBUSY if there's something else using another GPIO from
> the same provider?

I'm not sure what you mean by "another GPIO from the same provider".
If you mean can the codec driver also try to read the same GPIOs:

1. No. It's read here in the SPI bus driver because on these systems the
cs42l43 is the only device that appears in ACPI, so it's the only one
that has access to the ACPI GpioIo(). And there's only one instance.

2. The code only creates the mapping temporarily while it reads the GPIO
state and then removes the mapping

3. If it could return -EBUSY it could have returned that with the _DSD
spk-id-gpios property and the original code would have returned an
error.

> I'm a bit fuzzy on the ACPI mappings so possibly
> that can't happen.

Linux used to have a function to read the ACPI GpioIo() definitions,
same way Windows does. That was removed a few years ago so that GPIOs
now must use the DT-style "something-gpios" way, and an ACPI _DSD
property provides the mapping from that property to the GpioIo().
Of course, that _DSD property is Linux-specific so won't be in any ACPI
written for Windows, so the acpi_gpio_mapping had to be added to provide
a way to read the GpioIo(), but in a more complex way than before.
It's just a mapping from Linux-specific named GPIOs to the actual ACPI
GPIO definition. So we've gone in a circle removing the ACPI-specific
API and then adding a ACPI-specific API, but I assume something was
fixed by doing it the new wat.

The ACPI property takes precedence and overrides the mapping, but the
specific case of the cs42l43 SPI driver we know that the node containing
the defective _DSD property is always different from the node containing
the GpioIo() definition.

On the Sashiko complaints:
1. Code doesn't handle the singular form "spk-id-gpio". The script that
generates this ACPI is hardcoded to the plural form, so this isn't a
issue I'll do a change for that anyway just for completeness, but I'd
prefer to do that as a separate patch so I can keep the "tested-by" on
this patch. (I'll do a spin to add it to this patch if you prefer.)

2. "Does this ignore the ACPI pin offset". Yes, that's precisely what
the patch is intended to do.

3. The stuff about overwriting a GPIO provider's node. Sashiko seems to
be assuming that the spk-id-gpios is pointing at the GPIO driver node,
which it would be on DT. On ACPI it's just pointing at our node that
contains the ACPI GpioIo() that points to the GPIO driver node. adev
is the adev of our child node.

4. The "use-after-free" seems be the same misunderstanding, it thinks
we're changing the node of the GPIO provider driver. We're not, we're
adding the mapping to our child node and only the cs42l43 driver uses
that.