[PATCH v2 2/3] platform/x86: serial-multi-instantiate: Fix SPI chip select on platforms with incomplete ACPI cs-gpios

[PATCH v2 2/3] platform/x86: serial-multi-instantiate: Fix SPI chip select on platforms with incomplete ACPI cs-gpios

[Khalil]

Some HP laptops with Intel Lunar Lake and dual Cirrus Logic CS35L56
amplifiers over SPI have an incomplete cs-gpios property in the SPI
controller's _DSD - it only declares the first chip select. The
remaining chip select GPIOs are defined as GpioIo resources in the
peripheral's ACPI node but are not referenced by the controller.

This causes the SPI framework to reject devices whose chip select
exceeds num_chipselect with -EINVAL, preventing the second amplifier
from probing.

Fix this on known affected platforms by:

1. Adding a DMI quirk table to identify affected systems
2. For devices with chip selects outside the controller's range,
   acquiring the GPIO from the peripheral's ACPI GpioIo resource
3. Extending num_chipselect and reallocating the controller's
   cs_gpiods array to install the GPIO descriptor
4. Setting SPI_CONTROLLER_GPIO_SS so the framework calls both
   the GPIO toggle and controller->set_cs (needed for clock
   gating on Intel LPSS controllers)

Only chip selects beyond the controller's num_chipselect are fixed up.
Chip selects within range with a NULL cs_gpiods entry are left alone,
as NULL means "native chip select" which is intentional.

Tested on HP EliteBook 8 G1i 16 inch (board 8D8A) with 2x CS35L56
Rev B0 amplifiers. Both amplifiers probe and produce audio.

Signed-off-by: Khalil <khalilst@gmail.com>
---
 .../platform/x86/serial-multi-instantiate.c   | 168 ++++++++++++++++++
 1 file changed, 168 insertions(+)

diff --git a/drivers/platform/x86/serial-multi-instantiate.c b/drivers/platform/x86/serial-multi-instantiate.c
index 1a369334f..0d30bd0a7 100644
--- a/drivers/platform/x86/serial-multi-instantiate.c
+++ b/drivers/platform/x86/serial-multi-instantiate.c
@@ -8,6 +8,10 @@
 
 #include <linux/acpi.h>
 #include <linux/bits.h>
+#include <linux/delay.h>
+#include <linux/dmi.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/machine.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
@@ -46,6 +50,53 @@ struct smi {
 	int spi_num;
 	struct i2c_client **i2c_devs;
 	struct spi_device **spi_devs;
+	struct gpio_desc *cs_gpio;
+};
+
+/*
+ * Quirk data for platforms with broken ACPI SPI chip select descriptions.
+ * cs_gpio_idx: index of the GpioIo resource in the ACPI _CRS that provides
+ *              the chip select GPIO for devices missing a proper cs-gpios
+ *              entry on the SPI controller.
+ */
+struct smi_cs_gpio_quirk {
+	int cs_gpio_idx;
+};
+
+static const struct smi_cs_gpio_quirk hp_elitebook_8g1i_quirk = {
+	.cs_gpio_idx = 0,
+};
+
+/*
+ * DMI table of platforms with broken SPI chip select ACPI descriptions.
+ *
+ * These systems have multiple SPI peripherals (e.g., dual CS35L56
+ * amplifiers) but the SPI controller's _DSD cs-gpios property is
+ * incomplete - it only declares the first chip select. The remaining
+ * chip select GPIOs are defined as GpioIo resources in the peripheral's
+ * ACPI node but are not referenced by the controller.
+ */
+static const struct dmi_system_id smi_cs_gpio_dmi_table[] = {
+	{
+		.ident = "HP EliteBook 8 G1i 16 inch",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "HP"),
+			DMI_MATCH(DMI_BOARD_NAME, "8D8A"),
+		},
+		.driver_data = (void *)&hp_elitebook_8g1i_quirk,
+	},
+	{ }
+};
+
+/*
+ * ACPI GPIO mapping for the chip select GpioIo resource.
+ * Maps "cs-gpios" to the GpioIo resource at index 0 of 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 },
+	{ }
 };
 
 static int smi_get_irq(struct platform_device *pdev, struct acpi_device *adev,
@@ -99,6 +150,101 @@ static void smi_devs_unregister(struct smi *smi)
 	}
 }
 
+/*
+ * smi_spi_setup_cs_gpio - Fix up chip select for a device on a platform
+ * with broken ACPI cs-gpios description.
+ *
+ * On affected platforms, the SPI controller's cs-gpios property is incomplete,
+ * so the framework has no GPIO descriptor for some chip selects. This causes
+ * __spi_add_device() to reject the device with -EINVAL (cs >= num_chipselect).
+ *
+ * This function:
+ * 1. Extends num_chipselect if the device's CS is out of range
+ * 2. Reallocates cs_gpiods to match, preventing out-of-bounds access
+ *    in __spi_add_device()
+ * 3. Installs the GPIO from the peripheral's ACPI node into the
+ *    controller's cs_gpiods array so __spi_add_device() propagates
+ *    it to the device
+ * 4. Sets SPI_CONTROLLER_GPIO_SS so the framework calls both the GPIO
+ *    toggle and controller->set_cs (needed for clock gating on Intel
+ *    LPSS controllers)
+ */
+static int smi_spi_setup_cs_gpio(struct device *dev,
+				 struct spi_device *spi_dev,
+				 struct smi *smi,
+				 const struct smi_cs_gpio_quirk *quirk)
+{
+	struct spi_controller *ctlr = spi_dev->controller;
+	struct gpio_desc **new_gpiods;
+	u16 cs = spi_get_chipselect(spi_dev, 0);
+
+	/*
+	 * Only fix up chip selects that the controller doesn't know about.
+	 * A NULL cs_gpiods[cs] within the controller's num_chipselect range
+	 * means "native chip select" - that's intentional, not broken.
+	 * The broken case is when cs >= num_chipselect, meaning the ACPI
+	 * cs-gpios property on the controller was incomplete.
+	 */
+	if (cs < ctlr->num_chipselect)
+		return 0;
+
+	/* Extend num_chipselect to cover this device */
+	dev_info(dev, "Extending num_chipselect from %u to %u for CS%u\n",
+		 ctlr->num_chipselect, cs + 1, cs);
+	ctlr->num_chipselect = cs + 1;
+
+	/* Acquire the CS GPIO from the ACPI GpioIo resource if not yet done */
+	if (!smi->cs_gpio) {
+		int ret;
+
+		ret = devm_acpi_dev_add_driver_gpios(dev, smi_cs_gpio_mapping);
+		if (ret) {
+			dev_warn(dev, "Failed to add CS GPIO mapping: %d\n", ret);
+			return ret;
+		}
+
+		smi->cs_gpio = devm_gpiod_get(dev, "cs", GPIOD_OUT_HIGH);
+		if (IS_ERR(smi->cs_gpio)) {
+			dev_warn(dev, "Failed to get CS GPIO: %ld\n",
+				 PTR_ERR(smi->cs_gpio));
+			smi->cs_gpio = NULL;
+			return -ENOENT;
+		}
+		dev_info(dev, "Acquired CS GPIO for CS%u from ACPI GpioIo[%d]\n",
+			 cs, quirk->cs_gpio_idx);
+	}
+
+	/*
+	 * Reallocate the controller's cs_gpiods array to accommodate the
+	 * new num_chipselect, and install the GPIO descriptor. This is
+	 * necessary because __spi_add_device() unconditionally reads
+	 * ctlr->cs_gpiods[cs] to set the device's cs_gpiod.
+	 */
+	new_gpiods = devm_kcalloc(&ctlr->dev, ctlr->num_chipselect,
+				  sizeof(*new_gpiods), GFP_KERNEL);
+	if (!new_gpiods)
+		return -ENOMEM;
+
+	if (ctlr->cs_gpiods) {
+		unsigned int i;
+
+		for (i = 0; i < cs; i++)
+			new_gpiods[i] = ctlr->cs_gpiods[i];
+	}
+	new_gpiods[cs] = smi->cs_gpio;
+	ctlr->cs_gpiods = new_gpiods;
+
+	/*
+	 * SPI_CONTROLLER_GPIO_SS ensures the framework calls both the
+	 * GPIO CS toggle and controller->set_cs(). This is required on
+	 * Intel LPSS controllers where set_cs handles clock gating.
+	 */
+	ctlr->flags |= SPI_CONTROLLER_GPIO_SS;
+
+	dev_info(dev, "Installed GPIO CS on controller for CS%u\n", cs);
+	return 0;
+}
+
 /**
  * smi_spi_probe - Instantiate multiple SPI devices from inst array
  * @pdev:	Platform device
@@ -112,10 +258,19 @@ static int smi_spi_probe(struct platform_device *pdev, struct smi *smi,
 {
 	struct device *dev = &pdev->dev;
 	struct acpi_device *adev = ACPI_COMPANION(dev);
+	const struct dmi_system_id *dmi_id;
+	const struct smi_cs_gpio_quirk *quirk = NULL;
 	struct spi_controller *ctlr;
 	struct spi_device *spi_dev;
 	char name[50];
 	int i, ret, count;
+	u16 cs;
+
+	dmi_id = dmi_first_match(smi_cs_gpio_dmi_table);
+	if (dmi_id) {
+		quirk = dmi_id->driver_data;
+		dev_info(dev, "Applying CS GPIO quirk for %s\n", dmi_id->ident);
+	}
 
 	ret = acpi_spi_count_resources(adev);
 	if (ret < 0)
@@ -139,6 +294,19 @@ static int smi_spi_probe(struct platform_device *pdev, struct smi *smi,
 		}
 
 		ctlr = spi_dev->controller;
+		cs = spi_get_chipselect(spi_dev, 0);
+
+		/*
+		 * On quirked platforms, fix up the chip select GPIO for
+		 * devices that would otherwise fail due to incomplete
+		 * ACPI cs-gpios on the SPI controller.
+		 */
+		if (quirk) {
+			ret = smi_spi_setup_cs_gpio(dev, spi_dev, smi, quirk);
+			if (ret)
+				dev_dbg(dev, "CS GPIO setup returned %d for CS%u\n",
+					ret, cs);
+		}
 
 		strscpy(spi_dev->modalias, inst_array[i].type);
 
-- 
2.43.0

[PATCH v2 0/3] Fix CS35L56 amplifier on Intel LPSS SPI with broken ACPI cs-gpios

[Khalil]

Hi,

Resend of v2 with the version tag now present on all patches in the
series (previous send only had v2 on the cover letter - apologies for
the noise).

This is v2 of the patch series fixing dual CS35L56 amplifiers on HP
laptops with Intel Lunar Lake and broken ACPI cs-gpios.

Changes since RFC v1 (addressing feedback from Richard Fitzgerald):

- Added DMI quirk table: GPIO CS fixup is now only applied on known
  affected platforms (HP EliteBook 8 G1i, board 8D8A), not generically
  for all devices with CS > 0.

- Fixed chip select range check: only chip selects outside the
  controller's num_chipselect range are fixed up. A NULL cs_gpiods[cs]
  within range means "native chip select", not broken. This avoids
  incorrectly replacing native CS0 with the GPIO descriptor.

- Removed ASUS reference (GU605C was fixed by BIOS update).

- Added Patch 1/3 (spi core): __spi_add_device() now preserves preset
  cs_gpiod on the device instead of unconditionally overwriting from
  ctlr->cs_gpiods. This was suggested by Richard as the preferred
  approach to avoid modifying controller state post-probe.

- Patch 2/3 (serial-multi-instantiate): Extracted GPIO fixup into a
  dedicated helper function. Currently still installs the GPIO on the
  controller's cs_gpiods array because __spi_add_device() overwrites
  device-level cs_gpiod. With Patch 1/3 applied, a future cleanup
  could set the GPIO directly on the device instead.

- Patch 3/3 (spi-pxa2xx): Unchanged from v1. Handles clock gating
  on Intel LPSS controllers when GPIO chip select is active.

Note: While Richard also suggested the cs35l41_hda self-fixup approach
(setting cs_gpiod on the device post-probe via spi_setup), this doesn't
work here because the device can't even be added to the bus - SPI core
rejects it with -EINVAL when cs >= num_chipselect. The serial-multi-
instantiate approach is necessary to extend num_chipselect first.

Regarding modifying the probed controller: I acknowledge this is not
ideal. Patch 1/3 is a step toward the cleaner approach (preset on
device). However, num_chipselect still needs to be extended, and
cs_gpiods needs to be expanded to prevent out-of-bounds access in
__spi_add_device(). I'm open to suggestions for a cleaner way to
handle this.

Tested on HP EliteBook 8 G1i 16 inch (Intel Core Ultra 7 258V,
Lunar Lake-M) with 2x CS35L56 Rev B0. Both amplifiers probe
successfully, load calibration and tuning, and produce audio.

Related bug reports:
- https://bugs.launchpad.net/ubuntu/+source/alsa-driver/+bug/2131138
- https://bugzilla.kernel.org/show_bug.cgi?id=221064
- https://github.com/thesofproject/linux/issues/5621

Khalil (3):
  spi: Preserve preset cs_gpiod in __spi_add_device()
  platform/x86: serial-multi-instantiate: Fix SPI chip select on
    platforms with incomplete ACPI cs-gpios
  spi: pxa2xx: Handle clock gating for GPIO chip select devices

 .../platform/x86/serial-multi-instantiate.c   | 168 ++++++++++++++++++
 drivers/spi/spi-pxa2xx.c                      |  45 ++++-
 drivers/spi/spi.c                             |   3 +-
 3 files changed, 211 insertions(+), 5 deletions(-)

-- 
2.43.0

[PATCH v2 1/3] spi: Preserve preset cs_gpiod in __spi_add_device()

[Khalil]

__spi_add_device() unconditionally overwrites spi->cs_gpiod[] from
ctlr->cs_gpiods[cs], even if the caller has already set a GPIO
descriptor on the device. This prevents drivers like
serial-multi-instantiate from pre-configuring a GPIO chip select
acquired from ACPI before adding the device.

Skip the overwrite when the device already has a cs_gpiod set for
the given index, allowing callers to preset GPIO chip selects that
aren't described in the controller's cs-gpios property.

This is useful on platforms where the ACPI _DSD cs-gpios property
on the SPI controller is incomplete, but the peripheral's ACPI node
does contain the correct GpioIo resource for its chip select.

Suggested-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Signed-off-by: Khalil <khalilst@gmail.com>
---
 drivers/spi/spi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index a0b2bd3b8..64be99d13 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -734,7 +734,8 @@ static int __spi_add_device(struct spi_device *spi, struct spi_device *parent)
 
 		for (idx = 0; idx < spi->num_chipselect; idx++) {
 			cs = spi_get_chipselect(spi, idx);
-			spi_set_csgpiod(spi, idx, ctlr->cs_gpiods[cs]);
+			if (!spi_get_csgpiod(spi, idx))
+				spi_set_csgpiod(spi, idx, ctlr->cs_gpiods[cs]);
 		}
 	}
 
-- 
2.43.0

[PATCH v2 2/3] platform/x86: serial-multi-instantiate: Fix SPI chip select on platforms with incomplete ACPI cs-gpios

[Khalil]

Some HP laptops with Intel Lunar Lake and dual Cirrus Logic CS35L56
amplifiers over SPI have an incomplete cs-gpios property in the SPI
controller's _DSD - it only declares the first chip select. The
remaining chip select GPIOs are defined as GpioIo resources in the
peripheral's ACPI node but are not referenced by the controller.

This causes the SPI framework to reject devices whose chip select
exceeds num_chipselect with -EINVAL, preventing the second amplifier
from probing.

Fix this on known affected platforms by:

1. Adding a DMI quirk table to identify affected systems
2. For devices with chip selects outside the controller's range,
   acquiring the GPIO from the peripheral's ACPI GpioIo resource
3. Extending num_chipselect and reallocating the controller's
   cs_gpiods array to install the GPIO descriptor
4. Setting SPI_CONTROLLER_GPIO_SS so the framework calls both
   the GPIO toggle and controller->set_cs (needed for clock
   gating on Intel LPSS controllers)

Only chip selects beyond the controller's num_chipselect are fixed up.
Chip selects within range with a NULL cs_gpiods entry are left alone,
as NULL means "native chip select" which is intentional.

Tested on HP EliteBook 8 G1i 16 inch (board 8D8A) with 2x CS35L56
Rev B0 amplifiers. Both amplifiers probe and produce audio.

Signed-off-by: Khalil <khalilst@gmail.com>
---
 .../platform/x86/serial-multi-instantiate.c   | 168 ++++++++++++++++++
 1 file changed, 168 insertions(+)

diff --git a/drivers/platform/x86/serial-multi-instantiate.c b/drivers/platform/x86/serial-multi-instantiate.c
index 1a369334f..0d30bd0a7 100644
--- a/drivers/platform/x86/serial-multi-instantiate.c
+++ b/drivers/platform/x86/serial-multi-instantiate.c
@@ -8,6 +8,10 @@
 
 #include <linux/acpi.h>
 #include <linux/bits.h>
+#include <linux/delay.h>
+#include <linux/dmi.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/machine.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
@@ -46,6 +50,53 @@ struct smi {
 	int spi_num;
 	struct i2c_client **i2c_devs;
 	struct spi_device **spi_devs;
+	struct gpio_desc *cs_gpio;
+};
+
+/*
+ * Quirk data for platforms with broken ACPI SPI chip select descriptions.
+ * cs_gpio_idx: index of the GpioIo resource in the ACPI _CRS that provides
+ *              the chip select GPIO for devices missing a proper cs-gpios
+ *              entry on the SPI controller.
+ */
+struct smi_cs_gpio_quirk {
+	int cs_gpio_idx;
+};
+
+static const struct smi_cs_gpio_quirk hp_elitebook_8g1i_quirk = {
+	.cs_gpio_idx = 0,
+};
+
+/*
+ * DMI table of platforms with broken SPI chip select ACPI descriptions.
+ *
+ * These systems have multiple SPI peripherals (e.g., dual CS35L56
+ * amplifiers) but the SPI controller's _DSD cs-gpios property is
+ * incomplete - it only declares the first chip select. The remaining
+ * chip select GPIOs are defined as GpioIo resources in the peripheral's
+ * ACPI node but are not referenced by the controller.
+ */
+static const struct dmi_system_id smi_cs_gpio_dmi_table[] = {
+	{
+		.ident = "HP EliteBook 8 G1i 16 inch",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "HP"),
+			DMI_MATCH(DMI_BOARD_NAME, "8D8A"),
+		},
+		.driver_data = (void *)&hp_elitebook_8g1i_quirk,
+	},
+	{ }
+};
+
+/*
+ * ACPI GPIO mapping for the chip select GpioIo resource.
+ * Maps "cs-gpios" to the GpioIo resource at index 0 of 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 },
+	{ }
 };
 
 static int smi_get_irq(struct platform_device *pdev, struct acpi_device *adev,
@@ -99,6 +150,101 @@ static void smi_devs_unregister(struct smi *smi)
 	}
 }
 
+/*
+ * smi_spi_setup_cs_gpio - Fix up chip select for a device on a platform
+ * with broken ACPI cs-gpios description.
+ *
+ * On affected platforms, the SPI controller's cs-gpios property is incomplete,
+ * so the framework has no GPIO descriptor for some chip selects. This causes
+ * __spi_add_device() to reject the device with -EINVAL (cs >= num_chipselect).
+ *
+ * This function:
+ * 1. Extends num_chipselect if the device's CS is out of range
+ * 2. Reallocates cs_gpiods to match, preventing out-of-bounds access
+ *    in __spi_add_device()
+ * 3. Installs the GPIO from the peripheral's ACPI node into the
+ *    controller's cs_gpiods array so __spi_add_device() propagates
+ *    it to the device
+ * 4. Sets SPI_CONTROLLER_GPIO_SS so the framework calls both the GPIO
+ *    toggle and controller->set_cs (needed for clock gating on Intel
+ *    LPSS controllers)
+ */
+static int smi_spi_setup_cs_gpio(struct device *dev,
+				 struct spi_device *spi_dev,
+				 struct smi *smi,
+				 const struct smi_cs_gpio_quirk *quirk)
+{
+	struct spi_controller *ctlr = spi_dev->controller;
+	struct gpio_desc **new_gpiods;
+	u16 cs = spi_get_chipselect(spi_dev, 0);
+
+	/*
+	 * Only fix up chip selects that the controller doesn't know about.
+	 * A NULL cs_gpiods[cs] within the controller's num_chipselect range
+	 * means "native chip select" - that's intentional, not broken.
+	 * The broken case is when cs >= num_chipselect, meaning the ACPI
+	 * cs-gpios property on the controller was incomplete.
+	 */
+	if (cs < ctlr->num_chipselect)
+		return 0;
+
+	/* Extend num_chipselect to cover this device */
+	dev_info(dev, "Extending num_chipselect from %u to %u for CS%u\n",
+		 ctlr->num_chipselect, cs + 1, cs);
+	ctlr->num_chipselect = cs + 1;
+
+	/* Acquire the CS GPIO from the ACPI GpioIo resource if not yet done */
+	if (!smi->cs_gpio) {
+		int ret;
+
+		ret = devm_acpi_dev_add_driver_gpios(dev, smi_cs_gpio_mapping);
+		if (ret) {
+			dev_warn(dev, "Failed to add CS GPIO mapping: %d\n", ret);
+			return ret;
+		}
+
+		smi->cs_gpio = devm_gpiod_get(dev, "cs", GPIOD_OUT_HIGH);
+		if (IS_ERR(smi->cs_gpio)) {
+			dev_warn(dev, "Failed to get CS GPIO: %ld\n",
+				 PTR_ERR(smi->cs_gpio));
+			smi->cs_gpio = NULL;
+			return -ENOENT;
+		}
+		dev_info(dev, "Acquired CS GPIO for CS%u from ACPI GpioIo[%d]\n",
+			 cs, quirk->cs_gpio_idx);
+	}
+
+	/*
+	 * Reallocate the controller's cs_gpiods array to accommodate the
+	 * new num_chipselect, and install the GPIO descriptor. This is
+	 * necessary because __spi_add_device() unconditionally reads
+	 * ctlr->cs_gpiods[cs] to set the device's cs_gpiod.
+	 */
+	new_gpiods = devm_kcalloc(&ctlr->dev, ctlr->num_chipselect,
+				  sizeof(*new_gpiods), GFP_KERNEL);
+	if (!new_gpiods)
+		return -ENOMEM;
+
+	if (ctlr->cs_gpiods) {
+		unsigned int i;
+
+		for (i = 0; i < cs; i++)
+			new_gpiods[i] = ctlr->cs_gpiods[i];
+	}
+	new_gpiods[cs] = smi->cs_gpio;
+	ctlr->cs_gpiods = new_gpiods;
+
+	/*
+	 * SPI_CONTROLLER_GPIO_SS ensures the framework calls both the
+	 * GPIO CS toggle and controller->set_cs(). This is required on
+	 * Intel LPSS controllers where set_cs handles clock gating.
+	 */
+	ctlr->flags |= SPI_CONTROLLER_GPIO_SS;
+
+	dev_info(dev, "Installed GPIO CS on controller for CS%u\n", cs);
+	return 0;
+}
+
 /**
  * smi_spi_probe - Instantiate multiple SPI devices from inst array
  * @pdev:	Platform device
@@ -112,10 +258,19 @@ static int smi_spi_probe(struct platform_device *pdev, struct smi *smi,
 {
 	struct device *dev = &pdev->dev;
 	struct acpi_device *adev = ACPI_COMPANION(dev);
+	const struct dmi_system_id *dmi_id;
+	const struct smi_cs_gpio_quirk *quirk = NULL;
 	struct spi_controller *ctlr;
 	struct spi_device *spi_dev;
 	char name[50];
 	int i, ret, count;
+	u16 cs;
+
+	dmi_id = dmi_first_match(smi_cs_gpio_dmi_table);
+	if (dmi_id) {
+		quirk = dmi_id->driver_data;
+		dev_info(dev, "Applying CS GPIO quirk for %s\n", dmi_id->ident);
+	}
 
 	ret = acpi_spi_count_resources(adev);
 	if (ret < 0)
@@ -139,6 +294,19 @@ static int smi_spi_probe(struct platform_device *pdev, struct smi *smi,
 		}
 
 		ctlr = spi_dev->controller;
+		cs = spi_get_chipselect(spi_dev, 0);
+
+		/*
+		 * On quirked platforms, fix up the chip select GPIO for
+		 * devices that would otherwise fail due to incomplete
+		 * ACPI cs-gpios on the SPI controller.
+		 */
+		if (quirk) {
+			ret = smi_spi_setup_cs_gpio(dev, spi_dev, smi, quirk);
+			if (ret)
+				dev_dbg(dev, "CS GPIO setup returned %d for CS%u\n",
+					ret, cs);
+		}
 
 		strscpy(spi_dev->modalias, inst_array[i].type);
 
-- 
2.43.0

[PATCH v2 3/3] spi: pxa2xx: Handle clock gating for GPIO chip select devices

[Khalil]

On Intel LPSS SPI controllers (Cannon Lake and later) with dynamic
clock gating (cs_clk_stays_gated=true), the SPI clock is gated when
no native chip select is asserted. When using a GPIO chip select
(via SPI_CONTROLLER_GPIO_SS), the SPI framework toggles the GPIO
and also calls the controller's set_cs callback.

Handle this in the pxa2xx cs_assert/cs_deassert functions: when the
device uses a GPIO chip select on an LPSS controller, assert native
CS in the control register to enable the clock, and force the clock
gate on. On deassert, restore both.

This is needed on platforms where serial-multi-instantiate installs
a GPIO chip select from the peripheral's ACPI GpioIo resource to
work around an incomplete cs-gpios property on the SPI controller.

Signed-off-by: Khalil <khalilst@gmail.com>
---
 drivers/spi/spi-pxa2xx.c | 45 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 41 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index 6291d7c2e..fe5762063 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -419,20 +419,43 @@ static void cs_assert(struct spi_device *spi)
 {
 	struct driver_data *drv_data =
 		spi_controller_get_devdata(spi->controller);
+	const struct lpss_config *config;
 
 	if (drv_data->ssp_type == CE4100_SSP) {
 		pxa2xx_spi_write(drv_data, SSSR, spi_get_chipselect(spi, 0));
 		return;
 	}
 
-	if (is_lpss_ssp(drv_data))
-		lpss_ssp_cs_control(spi, true);
+	if (is_lpss_ssp(drv_data)) {
+		config = lpss_get_config(drv_data);
+
+		if (spi_is_csgpiod(spi)) {
+			/*
+			 * GPIO handles the actual chip select to the device.
+			 * On LPSS controllers with dynamic clock gating, the
+			 * SPI clock won't run unless the native CS state says
+			 * "asserted" in the CS control register. Assert native
+			 * CS in the register to enable the clock, and force
+			 * the clock gate on.
+			 */
+			lpss_ssp_cs_control(spi, true);
+			if (config->cs_clk_stays_gated) {
+				__lpss_ssp_update_priv(drv_data,
+					LPSS_PRIV_CLOCK_GATE,
+					LPSS_PRIV_CLOCK_GATE_CLK_CTL_MASK,
+					LPSS_PRIV_CLOCK_GATE_CLK_CTL_FORCE_ON);
+			}
+		} else {
+			lpss_ssp_cs_control(spi, true);
+		}
+	}
 }
 
 static void cs_deassert(struct spi_device *spi)
 {
 	struct driver_data *drv_data =
 		spi_controller_get_devdata(spi->controller);
+	const struct lpss_config *config;
 	unsigned long timeout;
 
 	if (drv_data->ssp_type == CE4100_SSP)
@@ -444,8 +467,22 @@ static void cs_deassert(struct spi_device *spi)
 	       !time_after(jiffies, timeout))
 		cpu_relax();
 
-	if (is_lpss_ssp(drv_data))
-		lpss_ssp_cs_control(spi, false);
+	if (is_lpss_ssp(drv_data)) {
+		config = lpss_get_config(drv_data);
+
+		if (spi_is_csgpiod(spi)) {
+			/* Deassert native CS and restore clock gating */
+			lpss_ssp_cs_control(spi, false);
+			if (config->cs_clk_stays_gated) {
+				__lpss_ssp_update_priv(drv_data,
+					LPSS_PRIV_CLOCK_GATE,
+					LPSS_PRIV_CLOCK_GATE_CLK_CTL_MASK,
+					LPSS_PRIV_CLOCK_GATE_CLK_CTL_FORCE_OFF);
+			}
+		} else {
+			lpss_ssp_cs_control(spi, false);
+		}
+	}
 }
 
 static void pxa2xx_spi_set_cs(struct spi_device *spi, bool level)
-- 
2.43.0

Re: [PATCH v2 2/3] platform/x86: serial-multi-instantiate: Fix SPI chip select on platforms with incomplete ACPI cs-gpios

[Ilpo Järvinen]

On Mon, 13 Apr 2026, Khalil wrote:

> Some HP laptops with Intel Lunar Lake and dual Cirrus Logic CS35L56
> amplifiers over SPI have an incomplete cs-gpios property in the SPI
> controller's _DSD - it only declares the first chip select. The
> remaining chip select GPIOs are defined as GpioIo resources in the
> peripheral's ACPI node but are not referenced by the controller.
> 
> This causes the SPI framework to reject devices whose chip select
> exceeds num_chipselect with -EINVAL, preventing the second amplifier
> from probing.
> 
> Fix this on known affected platforms by:
> 
> 1. Adding a DMI quirk table to identify affected systems
> 2. For devices with chip selects outside the controller's range,
>    acquiring the GPIO from the peripheral's ACPI GpioIo resource
> 3. Extending num_chipselect and reallocating the controller's
>    cs_gpiods array to install the GPIO descriptor
> 4. Setting SPI_CONTROLLER_GPIO_SS so the framework calls both
>    the GPIO toggle and controller->set_cs (needed for clock
>    gating on Intel LPSS controllers)
> 
> Only chip selects beyond the controller's num_chipselect are fixed up.
> Chip selects within range with a NULL cs_gpiods entry are left alone,
> as NULL means "native chip select" which is intentional.
> 
> Tested on HP EliteBook 8 G1i 16 inch (board 8D8A) with 2x CS35L56
> Rev B0 amplifiers. Both amplifiers probe and produce audio.
> 
> Signed-off-by: Khalil <khalilst@gmail.com>
> ---
>  .../platform/x86/serial-multi-instantiate.c   | 168 ++++++++++++++++++
>  1 file changed, 168 insertions(+)
> 
> diff --git a/drivers/platform/x86/serial-multi-instantiate.c b/drivers/platform/x86/serial-multi-instantiate.c
> index 1a369334f..0d30bd0a7 100644
> --- a/drivers/platform/x86/serial-multi-instantiate.c
> +++ b/drivers/platform/x86/serial-multi-instantiate.c
> @@ -8,6 +8,10 @@
>  
>  #include <linux/acpi.h>
>  #include <linux/bits.h>
> +#include <linux/delay.h>
> +#include <linux/dmi.h>

Will this build with all configs since you didn't add any depends on?

> +#include <linux/gpio/consumer.h>
> +#include <linux/gpio/machine.h>
>  #include <linux/i2c.h>
>  #include <linux/interrupt.h>
>  #include <linux/kernel.h>
> @@ -46,6 +50,53 @@ struct smi {
>  	int spi_num;
>  	struct i2c_client **i2c_devs;
>  	struct spi_device **spi_devs;
> +	struct gpio_desc *cs_gpio;
> +};
> +
> +/*
> + * Quirk data for platforms with broken ACPI SPI chip select descriptions.
> + * cs_gpio_idx: index of the GpioIo resource in the ACPI _CRS that provides
> + *              the chip select GPIO for devices missing a proper cs-gpios
> + *              entry on the SPI controller.
> + */
> +struct smi_cs_gpio_quirk {
> +	int cs_gpio_idx;
> +};
> +
> +static const struct smi_cs_gpio_quirk hp_elitebook_8g1i_quirk = {
> +	.cs_gpio_idx = 0,
> +};
> +
> +/*
> + * DMI table of platforms with broken SPI chip select ACPI descriptions.
> + *
> + * These systems have multiple SPI peripherals (e.g., dual CS35L56
> + * amplifiers) but the SPI controller's _DSD cs-gpios property is
> + * incomplete - it only declares the first chip select. The remaining
> + * chip select GPIOs are defined as GpioIo resources in the peripheral's
> + * ACPI node but are not referenced by the controller.
> + */
> +static const struct dmi_system_id smi_cs_gpio_dmi_table[] = {
> +	{
> +		.ident = "HP EliteBook 8 G1i 16 inch",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "HP"),
> +			DMI_MATCH(DMI_BOARD_NAME, "8D8A"),
> +		},
> +		.driver_data = (void *)&hp_elitebook_8g1i_quirk,

Cast looks unnecessary.

> +	},
> +	{ }
> +};
> +
> +/*
> + * ACPI GPIO mapping for the chip select GpioIo resource.
> + * Maps "cs-gpios" to the GpioIo resource at index 0 of 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 },
> +	{ }
>  };
>  
>  static int smi_get_irq(struct platform_device *pdev, struct acpi_device *adev,
> @@ -99,6 +150,101 @@ static void smi_devs_unregister(struct smi *smi)
>  	}
>  }
>  
> +/*
> + * smi_spi_setup_cs_gpio - Fix up chip select for a device on a platform
> + * with broken ACPI cs-gpios description.
> + *
> + * On affected platforms, the SPI controller's cs-gpios property is incomplete,
> + * so the framework has no GPIO descriptor for some chip selects. This causes
> + * __spi_add_device() to reject the device with -EINVAL (cs >= num_chipselect).
> + *
> + * This function:
> + * 1. Extends num_chipselect if the device's CS is out of range
> + * 2. Reallocates cs_gpiods to match, preventing out-of-bounds access
> + *    in __spi_add_device()
> + * 3. Installs the GPIO from the peripheral's ACPI node into the
> + *    controller's cs_gpiods array so __spi_add_device() propagates
> + *    it to the device
> + * 4. Sets SPI_CONTROLLER_GPIO_SS so the framework calls both the GPIO
> + *    toggle and controller->set_cs (needed for clock gating on Intel
> + *    LPSS controllers)
> + */
> +static int smi_spi_setup_cs_gpio(struct device *dev,
> +				 struct spi_device *spi_dev,
> +				 struct smi *smi,
> +				 const struct smi_cs_gpio_quirk *quirk)
> +{
> +	struct spi_controller *ctlr = spi_dev->controller;
> +	struct gpio_desc **new_gpiods;
> +	u16 cs = spi_get_chipselect(spi_dev, 0);

Could you place this assignment right before the if () below.

> +
> +	/*
> +	 * Only fix up chip selects that the controller doesn't know about.
> +	 * A NULL cs_gpiods[cs] within the controller's num_chipselect range
> +	 * means "native chip select" - that's intentional, not broken.
> +	 * The broken case is when cs >= num_chipselect, meaning the ACPI
> +	 * cs-gpios property on the controller was incomplete.
> +	 */
> +	if (cs < ctlr->num_chipselect)
> +		return 0;
> +
> +	/* Extend num_chipselect to cover this device */
> +	dev_info(dev, "Extending num_chipselect from %u to %u for CS%u\n",

Add include for dev_*() printing (I know it's also a pre-existing problem 
but lets add it finally here).

> +		 ctlr->num_chipselect, cs + 1, cs);
> +	ctlr->num_chipselect = cs + 1;
> +
> +	/* Acquire the CS GPIO from the ACPI GpioIo resource if not yet done */
> +	if (!smi->cs_gpio) {
> +		int ret;
> +
> +		ret = devm_acpi_dev_add_driver_gpios(dev, smi_cs_gpio_mapping);
> +		if (ret) {
> +			dev_warn(dev, "Failed to add CS GPIO mapping: %d\n", ret);
> +			return ret;
> +		}
> +
> +		smi->cs_gpio = devm_gpiod_get(dev, "cs", GPIOD_OUT_HIGH);
> +		if (IS_ERR(smi->cs_gpio)) {

Add include.

> +			dev_warn(dev, "Failed to get CS GPIO: %ld\n",
> +				 PTR_ERR(smi->cs_gpio));
> +			smi->cs_gpio = NULL;
> +			return -ENOENT;

Should this pass the error code instead?

> +		}
> +		dev_info(dev, "Acquired CS GPIO for CS%u from ACPI GpioIo[%d]\n",
> +			 cs, quirk->cs_gpio_idx);
> +	}
> +
> +	/*
> +	 * Reallocate the controller's cs_gpiods array to accommodate the
> +	 * new num_chipselect, and install the GPIO descriptor. This is
> +	 * necessary because __spi_add_device() unconditionally reads
> +	 * ctlr->cs_gpiods[cs] to set the device's cs_gpiod.
> +	 */
> +	new_gpiods = devm_kcalloc(&ctlr->dev, ctlr->num_chipselect,
> +				  sizeof(*new_gpiods), GFP_KERNEL);
> +	if (!new_gpiods)
> +		return -ENOMEM;
> +
> +	if (ctlr->cs_gpiods) {
> +		unsigned int i;
> +
> +		for (i = 0; i < cs; i++)
> +			new_gpiods[i] = ctlr->cs_gpiods[i];
> +	}
> +	new_gpiods[cs] = smi->cs_gpio;
> +	ctlr->cs_gpiods = new_gpiods;
> +
> +	/*
> +	 * SPI_CONTROLLER_GPIO_SS ensures the framework calls both the
> +	 * GPIO CS toggle and controller->set_cs(). This is required on
> +	 * Intel LPSS controllers where set_cs handles clock gating.
> +	 */
> +	ctlr->flags |= SPI_CONTROLLER_GPIO_SS;
> +
> +	dev_info(dev, "Installed GPIO CS on controller for CS%u\n", cs);

Normally, the expectation is that success remains silent.

> +	return 0;
> +}
> +
>  /**
>   * smi_spi_probe - Instantiate multiple SPI devices from inst array
>   * @pdev:	Platform device
> @@ -112,10 +258,19 @@ static int smi_spi_probe(struct platform_device *pdev, struct smi *smi,
>  {
>  	struct device *dev = &pdev->dev;
>  	struct acpi_device *adev = ACPI_COMPANION(dev);
> +	const struct dmi_system_id *dmi_id;
> +	const struct smi_cs_gpio_quirk *quirk = NULL;
>  	struct spi_controller *ctlr;
>  	struct spi_device *spi_dev;
>  	char name[50];
>  	int i, ret, count;
> +	u16 cs;
> +
> +	dmi_id = dmi_first_match(smi_cs_gpio_dmi_table);
> +	if (dmi_id) {
> +		quirk = dmi_id->driver_data;
> +		dev_info(dev, "Applying CS GPIO quirk for %s\n", dmi_id->ident);
> +	}
>  
>  	ret = acpi_spi_count_resources(adev);
>  	if (ret < 0)
> @@ -139,6 +294,19 @@ static int smi_spi_probe(struct platform_device *pdev, struct smi *smi,
>  		}
>  
>  		ctlr = spi_dev->controller;
> +		cs = spi_get_chipselect(spi_dev, 0);
> +
> +		/*
> +		 * On quirked platforms, fix up the chip select GPIO for
> +		 * devices that would otherwise fail due to incomplete
> +		 * ACPI cs-gpios on the SPI controller.
> +		 */
> +		if (quirk) {
> +			ret = smi_spi_setup_cs_gpio(dev, spi_dev, smi, quirk);
> +			if (ret)
> +				dev_dbg(dev, "CS GPIO setup returned %d for CS%u\n",
> +					ret, cs);
> +		}
>  
>  		strscpy(spi_dev->modalias, inst_array[i].type);
>  
> 

-- 
 i.

Re: [PATCH v2 2/3] platform/x86: serial-multi-instantiate: Fix SPI chip select on platforms with incomplete ACPI cs-gpios

[Hans de Goede]

Hi,

On 13-Apr-26 12:05 PM, Khalil wrote:
> Some HP laptops with Intel Lunar Lake and dual Cirrus Logic CS35L56
> amplifiers over SPI have an incomplete cs-gpios property in the SPI
> controller's _DSD - it only declares the first chip select. The
> remaining chip select GPIOs are defined as GpioIo resources in the
> peripheral's ACPI node but are not referenced by the controller.
> 
> This causes the SPI framework to reject devices whose chip select
> exceeds num_chipselect with -EINVAL, preventing the second amplifier
> from probing.
> 
> Fix this on known affected platforms by:
> 
> 1. Adding a DMI quirk table to identify affected systems
> 2. For devices with chip selects outside the controller's range,
>    acquiring the GPIO from the peripheral's ACPI GpioIo resource
> 3. Extending num_chipselect and reallocating the controller's
>    cs_gpiods array to install the GPIO descriptor
> 4. Setting SPI_CONTROLLER_GPIO_SS so the framework calls both
>    the GPIO toggle and controller->set_cs (needed for clock
>    gating on Intel LPSS controllers)
> 
> Only chip selects beyond the controller's num_chipselect are fixed up.
> Chip selects within range with a NULL cs_gpiods entry are left alone,
> as NULL means "native chip select" which is intentional.
> 
> Tested on HP EliteBook 8 G1i 16 inch (board 8D8A) with 2x CS35L56
> Rev B0 amplifiers. Both amplifiers probe and produce audio.
> 
> Signed-off-by: Khalil <khalilst@gmail.com>
> ---
>  .../platform/x86/serial-multi-instantiate.c   | 168 ++++++++++++++++++
>  1 file changed, 168 insertions(+)
> 
> diff --git a/drivers/platform/x86/serial-multi-instantiate.c b/drivers/platform/x86/serial-multi-instantiate.c
> index 1a369334f..0d30bd0a7 100644
> --- a/drivers/platform/x86/serial-multi-instantiate.c
> +++ b/drivers/platform/x86/serial-multi-instantiate.c
> @@ -8,6 +8,10 @@
>  
>  #include <linux/acpi.h>
>  #include <linux/bits.h>
> +#include <linux/delay.h>
> +#include <linux/dmi.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/gpio/machine.h>
>  #include <linux/i2c.h>
>  #include <linux/interrupt.h>
>  #include <linux/kernel.h>
> @@ -46,6 +50,53 @@ struct smi {
>  	int spi_num;
>  	struct i2c_client **i2c_devs;
>  	struct spi_device **spi_devs;
> +	struct gpio_desc *cs_gpio;
> +};
> +
> +/*
> + * Quirk data for platforms with broken ACPI SPI chip select descriptions.
> + * cs_gpio_idx: index of the GpioIo resource in the ACPI _CRS that provides
> + *              the chip select GPIO for devices missing a proper cs-gpios
> + *              entry on the SPI controller.
> + */
> +struct smi_cs_gpio_quirk {
> +	int cs_gpio_idx;
> +};
> +
> +static const struct smi_cs_gpio_quirk hp_elitebook_8g1i_quirk = {
> +	.cs_gpio_idx = 0,
> +};
> +
> +/*
> + * DMI table of platforms with broken SPI chip select ACPI descriptions.
> + *
> + * These systems have multiple SPI peripherals (e.g., dual CS35L56
> + * amplifiers) but the SPI controller's _DSD cs-gpios property is
> + * incomplete - it only declares the first chip select. The remaining
> + * chip select GPIOs are defined as GpioIo resources in the peripheral's
> + * ACPI node but are not referenced by the controller.
> + */
> +static const struct dmi_system_id smi_cs_gpio_dmi_table[] = {
> +	{
> +		.ident = "HP EliteBook 8 G1i 16 inch",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "HP"),
> +			DMI_MATCH(DMI_BOARD_NAME, "8D8A"),
> +		},
> +		.driver_data = (void *)&hp_elitebook_8g1i_quirk,
> +	},
> +	{ }
> +};

Do we really need a DMI quirk here? I would expect on existing models
there already is a cs for both amplifiers and the "if (cs < ctlr->num_chipselect)"
+ early return 0 will trigger so this will be a no-op there.

Can't you just enable this through a flag in smi_instance.flags and drop
the DMI quirk?


> +
> +/*
> + * ACPI GPIO mapping for the chip select GpioIo resource.
> + * Maps "cs-gpios" to the GpioIo resource at index 0 of 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 },
> +	{ }
>  };

I'm confused here you say that the chip-select for the second amplifier
is missing, but I would expect that to be the GpioIo resource at index 1
not 0?

I would expect the GpioIo resource at index 0 to be for the first
amplifier which does have a GPIO already in the _DSD cs-gpios property ?

Since this is a multi-serial device ACPI node, there is only one
set of resources for both amplifiers (with their also being 2 SPI
resources in the _CRS list).

So I would expect there to also be multiple GPIO entries in the
_CRS list, with the order of the chip-selects GPIOs resources matching
the order of the SPI resources.



>  
>  static int smi_get_irq(struct platform_device *pdev, struct acpi_device *adev,
> @@ -99,6 +150,101 @@ static void smi_devs_unregister(struct smi *smi)
>  	}
>  }
>  
> +/*
> + * smi_spi_setup_cs_gpio - Fix up chip select for a device on a platform
> + * with broken ACPI cs-gpios description.
> + *
> + * On affected platforms, the SPI controller's cs-gpios property is incomplete,
> + * so the framework has no GPIO descriptor for some chip selects. This causes
> + * __spi_add_device() to reject the device with -EINVAL (cs >= num_chipselect).
> + *
> + * This function:
> + * 1. Extends num_chipselect if the device's CS is out of range
> + * 2. Reallocates cs_gpiods to match, preventing out-of-bounds access
> + *    in __spi_add_device()
> + * 3. Installs the GPIO from the peripheral's ACPI node into the
> + *    controller's cs_gpiods array so __spi_add_device() propagates
> + *    it to the device
> + * 4. Sets SPI_CONTROLLER_GPIO_SS so the framework calls both the GPIO
> + *    toggle and controller->set_cs (needed for clock gating on Intel
> + *    LPSS controllers)
> + */
> +static int smi_spi_setup_cs_gpio(struct device *dev,
> +				 struct spi_device *spi_dev,
> +				 struct smi *smi,
> +				 const struct smi_cs_gpio_quirk *quirk)
> +{
> +	struct spi_controller *ctlr = spi_dev->controller;
> +	struct gpio_desc **new_gpiods;
> +	u16 cs = spi_get_chipselect(spi_dev, 0);
> +
> +	/*
> +	 * Only fix up chip selects that the controller doesn't know about.
> +	 * A NULL cs_gpiods[cs] within the controller's num_chipselect range
> +	 * means "native chip select" - that's intentional, not broken.
> +	 * The broken case is when cs >= num_chipselect, meaning the ACPI
> +	 * cs-gpios property on the controller was incomplete.
> +	 */
> +	if (cs < ctlr->num_chipselect)
> +		return 0;
> +
> +	/* Extend num_chipselect to cover this device */
> +	dev_info(dev, "Extending num_chipselect from %u to %u for CS%u\n",
> +		 ctlr->num_chipselect, cs + 1, cs);
> +	ctlr->num_chipselect = cs + 1;
> +
> +	/* Acquire the CS GPIO from the ACPI GpioIo resource if not yet done */
> +	if (!smi->cs_gpio) {

I'm confused shouldn't the gpio be a per spi_device thing not something
which you lookup only once. What if neither amplifier has a cs GPIO in
the _DSD should this then not do 2 lookups:

1. For the first spi_dev (using the first SPI resource in the _CRS list)
   lookup the first GPIO resources in the _CRS list 
2. For the second spi_dev lookup the second GPIO resources in the _CRS list 

?


> +		int ret;
> +
> +		ret = devm_acpi_dev_add_driver_gpios(dev, smi_cs_gpio_mapping);
> +		if (ret) {
> +			dev_warn(dev, "Failed to add CS GPIO mapping: %d\n", ret);
> +			return ret;
> +		}

So here I think you should dynamically fill the lookup using the index of
the spi_dev ('i' in the caller of this function) to fill in the offset of
the GPIO resource which should be looked up.

You can dynamically fill a local lookup and then non devm register the GPIO
lookup + remove it after the devm_gpiod_get() call.


> +
> +		smi->cs_gpio = devm_gpiod_get(dev, "cs", GPIOD_OUT_HIGH);
> +		if (IS_ERR(smi->cs_gpio)) {
> +			dev_warn(dev, "Failed to get CS GPIO: %ld\n",
> +				 PTR_ERR(smi->cs_gpio));
> +			smi->cs_gpio = NULL;
> +			return -ENOENT;
> +		}
> +		dev_info(dev, "Acquired CS GPIO for CS%u from ACPI GpioIo[%d]\n",
> +			 cs, quirk->cs_gpio_idx);
> +	}
> +
> +	/*
> +	 * Reallocate the controller's cs_gpiods array to accommodate the
> +	 * new num_chipselect, and install the GPIO descriptor. This is
> +	 * necessary because __spi_add_device() unconditionally reads
> +	 * ctlr->cs_gpiods[cs] to set the device's cs_gpiod.
> +	 */
> +	new_gpiods = devm_kcalloc(&ctlr->dev, ctlr->num_chipselect,
> +				  sizeof(*new_gpiods), GFP_KERNEL);
> +	if (!new_gpiods)
> +		return -ENOMEM;
> +
> +	if (ctlr->cs_gpiods) {
> +		unsigned int i;
> +
> +		for (i = 0; i < cs; i++)
> +			new_gpiods[i] = ctlr->cs_gpiods[i];
> +	}
> +	new_gpiods[cs] = smi->cs_gpio;
> +	ctlr->cs_gpiods = new_gpiods;
> +
> +	/*
> +	 * SPI_CONTROLLER_GPIO_SS ensures the framework calls both the
> +	 * GPIO CS toggle and controller->set_cs(). This is required on
> +	 * Intel LPSS controllers where set_cs handles clock gating.
> +	 */
> +	ctlr->flags |= SPI_CONTROLLER_GPIO_SS;
> +
> +	dev_info(dev, "Installed GPIO CS on controller for CS%u\n", cs);
> +	return 0;
> +}
> +
>  /**
>   * smi_spi_probe - Instantiate multiple SPI devices from inst array
>   * @pdev:	Platform device
> @@ -112,10 +258,19 @@ static int smi_spi_probe(struct platform_device *pdev, struct smi *smi,
>  {
>  	struct device *dev = &pdev->dev;
>  	struct acpi_device *adev = ACPI_COMPANION(dev);
> +	const struct dmi_system_id *dmi_id;
> +	const struct smi_cs_gpio_quirk *quirk = NULL;
>  	struct spi_controller *ctlr;
>  	struct spi_device *spi_dev;
>  	char name[50];
>  	int i, ret, count;
> +	u16 cs;
> +
> +	dmi_id = dmi_first_match(smi_cs_gpio_dmi_table);
> +	if (dmi_id) {
> +		quirk = dmi_id->driver_data;
> +		dev_info(dev, "Applying CS GPIO quirk for %s\n", dmi_id->ident);
> +	}
>  
>  	ret = acpi_spi_count_resources(adev);
>  	if (ret < 0)
> @@ -139,6 +294,19 @@ static int smi_spi_probe(struct platform_device *pdev, struct smi *smi,
>  		}
>  
>  		ctlr = spi_dev->controller;
> +		cs = spi_get_chipselect(spi_dev, 0);
> +
> +		/*
> +		 * On quirked platforms, fix up the chip select GPIO for
> +		 * devices that would otherwise fail due to incomplete
> +		 * ACPI cs-gpios on the SPI controller.
> +		 */
> +		if (quirk) {
> +			ret = smi_spi_setup_cs_gpio(dev, spi_dev, smi, quirk);
> +			if (ret)
> +				dev_dbg(dev, "CS GPIO setup returned %d for CS%u\n",
> +					ret, cs);
> +		}
>  
>  		strscpy(spi_dev->modalias, inst_array[i].type);
>  

Regards,

Hans

Re: [PATCH v2 2/3] platform/x86: serial-multi-instantiate: Fix SPI chip select on platforms with incomplete ACPI cs-gpios

[Richard Fitzgerald]

On 13/04/2026 12:43 pm, Hans de Goede wrote:
> Hi,
> 
> On 13-Apr-26 12:05 PM, Khalil wrote:
>> Some HP laptops with Intel Lunar Lake and dual Cirrus Logic CS35L56
>> amplifiers over SPI have an incomplete cs-gpios property in the SPI
>> controller's _DSD - it only declares the first chip select. The
>> remaining chip select GPIOs are defined as GpioIo resources in the
>> peripheral's ACPI node but are not referenced by the controller.
>>
>> This causes the SPI framework to reject devices whose chip select
>> exceeds num_chipselect with -EINVAL, preventing the second amplifier
>> from probing.
>>
>> Fix this on known affected platforms by:
>>

<SNIP>

>> +
>> +/*
>> + * ACPI GPIO mapping for the chip select GpioIo resource.
>> + * Maps "cs-gpios" to the GpioIo resource at index 0 of 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 },
>> +	{ }
>>   };
> 
> I'm confused here you say that the chip-select for the second amplifier
> is missing, but I would expect that to be the GpioIo resource at index 1
> not 0?
> 
> I would expect the GpioIo resource at index 0 to be for the first
> amplifier which does have a GPIO already in the _DSD cs-gpios property ?
> 
> Since this is a multi-serial device ACPI node, there is only one
> set of resources for both amplifiers (with their also being 2 SPI
> resources in the _CRS list).
> 
> So I would expect there to also be multiple GPIO entries in the
> _CRS list, with the order of the chip-selects GPIOs resources matching
> the order of the SPI resources.

You can't assume that. There is no ACPI reason why the GpioIo should be
in order of chip selects. In fact, some manufacturers scramble them
(e.g. CS2, CS1, CS3). You have to remember that the ACPI is written so
that it works with Windows (which is the reason we need this serial
multi-instantiate driver.) so it may contain Windowsisms.

Re: [PATCH v2 1/3] spi: Preserve preset cs_gpiod in __spi_add_device()

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 853 bytes --]

On Mon, Apr 13, 2026 at 12:05:28PM +0200, Khalil wrote:
> __spi_add_device() unconditionally overwrites spi->cs_gpiod[] from
> ctlr->cs_gpiods[cs], even if the caller has already set a GPIO
> descriptor on the device. This prevents drivers like
> serial-multi-instantiate from pre-configuring a GPIO chip select
> acquired from ACPI before adding the device.
> 
> Skip the overwrite when the device already has a cs_gpiod set for
> the given index, allowing callers to preset GPIO chip selects that
> aren't described in the controller's cs-gpios property.

This seems like it's almost certainly going to break something since we
just silently discard whatever the controller had set.  We at the very
least want to warn about collisions here, and ideally teach the
controller about bindings so we don't get two bits of software fighting.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 2/3] platform/x86: serial-multi-instantiate: Fix SPI chip select on platforms with incomplete ACPI cs-gpios

[Hans de Goede]

Hi,

On 13-Apr-26 2:51 PM, Richard Fitzgerald wrote:
> On 13/04/2026 12:43 pm, Hans de Goede wrote:
>> Hi,
>>
>> On 13-Apr-26 12:05 PM, Khalil wrote:
>>> Some HP laptops with Intel Lunar Lake and dual Cirrus Logic CS35L56
>>> amplifiers over SPI have an incomplete cs-gpios property in the SPI
>>> controller's _DSD - it only declares the first chip select. The
>>> remaining chip select GPIOs are defined as GpioIo resources in the
>>> peripheral's ACPI node but are not referenced by the controller.
>>>
>>> This causes the SPI framework to reject devices whose chip select
>>> exceeds num_chipselect with -EINVAL, preventing the second amplifier
>>> from probing.
>>>
>>> Fix this on known affected platforms by:
>>>
> 
> <SNIP>
> 
>>> +
>>> +/*
>>> + * ACPI GPIO mapping for the chip select GpioIo resource.
>>> + * Maps "cs-gpios" to the GpioIo resource at index 0 of 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 },
>>> +    { }
>>>   };
>>
>> I'm confused here you say that the chip-select for the second amplifier
>> is missing, but I would expect that to be the GpioIo resource at index 1
>> not 0?
>>
>> I would expect the GpioIo resource at index 0 to be for the first
>> amplifier which does have a GPIO already in the _DSD cs-gpios property ?
>>
>> Since this is a multi-serial device ACPI node, there is only one
>> set of resources for both amplifiers (with their also being 2 SPI
>> resources in the _CRS list).
>>
>> So I would expect there to also be multiple GPIO entries in the
>> _CRS list, with the order of the chip-selects GPIOs resources matching
>> the order of the SPI resources.
> 
> You can't assume that. There is no ACPI reason why the GpioIo should be
> in order of chip selects. In fact, some manufacturers scramble them
> (e.g. CS2, CS1, CS3). You have to remember that the ACPI is written so
> that it works with Windows (which is the reason we need this serial
> multi-instantiate driver.) so it may contain Windowsisms.

Right, in that case maybe we do need the DMI quirk and the DMI quirk
needs to point to a specific struct acpi_gpio_mapping for that model
laptop ?

Either there is some logic and the code can be generic, or there
is no logic and then we need to not pretend that the code is generic.

And in my mind the current code at least pretends to be somewhat
generic.

Thinking more about this, I do not think that the current patch does
what it is supposed to do at all.

According to the commit msg there is 1 GPIO listed in the _DSD
props and the ACPI lookup added is using the same "cs-gpios" conid
as the _DSD dependent code in drivers/spi/spi.c and it is getting
the gpio with:

smi->cs_gpio = devm_gpiod_get(dev, "cs", GPIOD_OUT_HIGH);

which is equivalent to:

smi->cs_gpio = devm_gpiod_get_index(dev, "cs", 0, GPIOD_OUT_HIGH);

which is the same lookup as the already succeeded lookup of CS0
in drivers/spi/spi.c .

for CS1 the lookup should be:

smi->cs_gpio = devm_gpiod_get_index(dev, "cs", 1, GPIOD_OUT_HIGH);

and the struct acpi_gpio_mapping should point to an array of
acpi_gpio_params[[] with 2 entries, with the first entry
duplicating the GPIO from the _DSD info and the second entry
pointing to the actual CS1 chip-select.


Andy, you know the ACPI gpio-lookup code better then me, can
you confirm that since the con_id between _DSD props and
the added acpi_gpio_mapping is the same that calling:

smi->cs_gpio = devm_gpiod_get(dev, "cs", GPIOD_OUT_HIGH);

aka:

smi->cs_gpio = devm_gpiod_get_index(dev, "cs", 0, GPIOD_OUT_HIGH);

will just return the GPIO from the _DSD again ?

Regards,

Hans