Re: [RFC PATCH 1/2] platform/x86: serial-multi-instantiate: Add GPIO chip select support for SPI devices

[Richard Fitzgerald]

On 15/02/2026 1:55 pm, Khalil wrote:
> To: broonie@xxxxxxxxxx, hdegoede@xxxxxxxxxx, ilpo.jarvinen@xxxxxxxxxxxxxxx
> Cc: rf@xxxxxxxxxxxxxxxxxxxxx, linux-spi@xxxxxxxxxxxxxxx,
>      platform-driver-x86@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx
>
> Some ACPI platforms (e.g., HP/ASUS laptops with Intel Lunar Lake and
> dual CS35L56 amplifiers) have a broken cs-gpios property in the SPI

The only Asus model we are aware of with CS35L56 and a cs-gpios problem
is the GU605C. That was fixed by a BIOS update so doesn't need a kernel
workaround. Is this the Asus you are referring to? Or is there another?

> controller's _DSD that only declares the first chip select. The second
> chip select GPIO is defined as a GpioIo resource in the peripheral's

Maybe the second. Maybe the first. It depends on the hardware.
Some laptops have four amps and four chip selects.

> ACPI node but is not referenced by the controller.
>
> This causes the SPI framework to have no GPIO descriptor for CS > 0,
> so the chip select line is never toggled for the second device.

It fails before that matters. The driver for that device will never be
probed because SPI core rejects the invalid chip select.

> Fix this by acquiring the chip select GPIO from the ACPI GpioIo resource
> in serial-multi-instantiate and installing it on the controller's
> cs_gpiods array. The GPIO must be set on the controller (not the device)
> because __spi_add_device() unconditionally overwrites spi->cs_gpiod[]
> from ctlr->cs_gpiods[cs].

If you change __spi_add_device() so it doesn't overwrite preset cs_gpiod
you could set it on the device. You wouldn't have to mess around with
the controller configuration after the controller has probed.

Alternatively, if you can make the amp driver probe, it can fix itself
up like the cs35l41_hda driver does. Maybe you can use this
special case in the SPI core:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/spi/spi.c?h=linux-6.19.y#n616

...

> +/*
> + * ACPI GPIO mapping for the chip select GpioIo resource.
> + * Maps "cs-gpios" to the first GpioIo resource in the ACPI _CRS.
> + */
> +static const struct acpi_gpio_params smi_cs_gpio_params = { 0, 0, false };
> +
> +static const struct acpi_gpio_mapping smi_cs_gpio_mapping[] = {
> +	{ "cs-gpios", &smi_cs_gpio_params, 1 },
> +	{ }
> +};
> +
>   #define IRQ_RESOURCE_TYPE	GENMASK(1, 0)
>   #define IRQ_RESOURCE_NONE	0
>   #define IRQ_RESOURCE_GPIO	1
> @@ -114,8 +128,11 @@
>   	struct acpi_device *adev = ACPI_COMPANION(dev);
>   	struct spi_controller *ctlr;
>   	struct spi_device *spi_dev;
> +	struct gpio_desc *cs_gpio = NULL;
>   	char name[50];
>   	int i, ret, count;
> +	bool gpio_mapped = false;
> +	u16 cs;
>
>   	ret = acpi_spi_count_resources(adev);
>   	if (ret < 0)
> @@ -139,6 +156,72 @@
>   		}
>
>   		ctlr = spi_dev->controller;
> +		cs = spi_get_chipselect(spi_dev, 0);
> +
> +		if (cs >= ctlr->num_chipselect) {
> +			dev_info(dev, "Increasing num_chipselect from %u to %u for CS%u\n",
> +				 ctlr->num_chipselect, cs + 1, cs);
> +			ctlr->num_chipselect = cs + 1;
> +		}
> +
> +		/*
> +		 * For devices with CS > 0, use GPIO chip select from ACPI
> +		 * GpioIo resource. The GPIO must be set on the controller's
> +		 * cs_gpiods array (not the device's cs_gpiod) because
> +		 * __spi_add_device() overwrites spi->cs_gpiod from
> +		 * ctlr->cs_gpiods[cs]. Also reallocate the array since
> +		 * a broken ACPI cs-gpios property may have undersized it.
> +		 *
> +		 * SPI_CONTROLLER_GPIO_SS ensures the framework calls both
> +		 * the GPIO toggle and controller->set_cs (for clock gate
> +		 * management on Intel LPSS controllers).
> +		 */

Only do this fixup for models where you know what the cs-gpios mappings
should be.

> +		if (cs > 0 && !gpio_mapped) {
> +			ret = devm_acpi_dev_add_driver_gpios(dev, smi_cs_gpio_mapping);
> +			if (ret)
> +				dev_warn(dev, "Failed to add GPIO mapping: %d\n", ret);
> +			else
> +				gpio_mapped = true;
> +		}
> +
> +		if (cs > 0 && gpio_mapped && !cs_gpio) {
> +			cs_gpio = devm_gpiod_get(dev, "cs", GPIOD_OUT_HIGH);
> +			if (IS_ERR(cs_gpio)) {
> +				dev_warn(dev, "Failed to get CS GPIO: %ld\n",
> +					 PTR_ERR(cs_gpio));
> +				cs_gpio = NULL;
> +			} else {
> +				dev_info(dev, "Got CS GPIO for amp CS%u\n", cs);
> +			}
> +		}
> +
> +		if (cs > 0 && cs_gpio) {
> +			/*
> +			 * Set GPIO on the controller's cs_gpiods array so
> +			 * __spi_add_device() will propagate it to the device.
> +			 * Reallocate the array to fit the new num_chipselect.
> +			 */
> +			struct gpio_desc **new_gpiods;
> +
> +			new_gpiods = devm_kcalloc(&ctlr->dev,
> +						  ctlr->num_chipselect,
> +						  sizeof(*new_gpiods),
> +						  GFP_KERNEL);
> +			if (new_gpiods) {
> +				if (ctlr->cs_gpiods)
> +					new_gpiods[0] = ctlr->cs_gpiods[0];
> +				new_gpiods[cs] = cs_gpio;
> +				ctlr->cs_gpiods = new_gpiods;
> +				ctlr->flags |= SPI_CONTROLLER_GPIO_SS;

Messing with the configuration of a probed SPI controller seems risky.