[PATCH 0/2] spi: pxa2xx: MacBook8,1 quirk and LPSS S3 resume fixes

[PATCH 0/2] spi: pxa2xx: MacBook8,1 quirk and LPSS S3 resume fixes

[Shih-Yuan Lee]

This series fixes two bugs in the spi-pxa2xx driver found during work
to restore MacBook8,1 SPI keyboard/touchpad functionality across S3
suspend/resume cycles.

Patch 1 moves the force-PIO quirk for Apple MacBook8,1 from the client
driver (applespi) to the host controller driver (spi-pxa2xx-pci), where
LPSS setup properly belongs.  It also fixes a related runtime PM bug:
when DMA is disabled, pm_runtime_allow() must not be called, because
runtime autosuspend clock-gates the LPSS block between transfers and
causes PCIe Completion Timeouts when its MMIO registers are subsequently
accessed.

Patch 2 fixes S3 suspend/resume for all Intel LPSS SPI controllers.
The LPSS power domain is fully removed across S3, so all private
registers (BAR0 0x200-0x2ff) and IDMA registers (0x800-0x814) are lost.
Without explicitly de-asserting LPSS_PRIV_RESETS (0x204) on resume,
any MMIO access triggers a PCIe Completion Timeout and watchdog reset.
Additionally, the IDMA block (sharing the SPI interrupt line) must have
its registers restored to prevent spurious interrupts from masking real
SPI transfers.  The CS control register (0x224) is intentionally
re-initialised via lpss_ssp_setup() rather than restored from snapshot,
ensuring CS starts deasserted regardless of its state at suspend time.

These patches are independent of the companion applespi series also
submitted today.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=108331

Shih-Yuan Lee (2):
  spi: pxa2xx: disable DMA and fix runtime PM for Apple MacBook8,1
  spi: pxa2xx: restore LPSS private and IDMA registers on S3 resume

 drivers/spi/spi-pxa2xx-pci.c | 38 ++++++++++++++++++++++--
 drivers/spi/spi-pxa2xx.c     | 57 ++++++++++++++++++++++++++++++++++++
 drivers/spi/spi-pxa2xx.h     |  4 +++
 3 files changed, 96 insertions(+), 3 deletions(-)

-- 
2.39.5

[PATCH 1/2] spi: pxa2xx: disable DMA and fix runtime PM for Apple MacBook8,1

[Shih-Yuan Lee]

On MacBook8,1 (early 2015 12" MacBook), the LPSS SPI controller at
00:15.4 has two related problems:

1. The DMA handshake and interrupt routing frequently fail or time out,
   causing the keyboard and trackpad (driven by the applespi driver via
   SPI) to become unresponsive.  Force PIO mode to avoid this.

2. When DMA is disabled, pm_runtime_allow() must not be called for the
   controller, because runtime autosuspend clock-gates the LPSS block
   between transfers.  Accessing its MMIO registers while clock-gated
   triggers a PCIe Completion Timeout which causes a watchdog reset.

Move the force-PIO quirk to spi-pxa2xx-pci.c (the LPSS host controller
driver) to avoid layering violations in the client driver, and guard
pm_runtime_allow() with the same pxa2xx_spi_pci_can_dma() check used
to set enable_dma so that PIO-mode controllers stay permanently active.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=108331
Signed-off-by: Shih-Yuan Lee <fourdollars@debian.org>
---
 drivers/spi/spi-pxa2xx-pci.c | 38 +++++++++++++++++++++++++++++++++---
 1 file changed, 35 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-pxa2xx-pci.c b/drivers/spi/spi-pxa2xx-pci.c
index cae77ac18520..cb61f1d2d9e6 100644
--- a/drivers/spi/spi-pxa2xx-pci.c
+++ b/drivers/spi/spi-pxa2xx-pci.c
@@ -18,9 +18,14 @@
 
 #include <linux/dmaengine.h>
 #include <linux/platform_data/dma-dw.h>
+#include <linux/dmi.h>
 
 #include "spi-pxa2xx.h"
 
+static bool spi_pxa2xx_force_pio;
+module_param_named(force_pio, spi_pxa2xx_force_pio, bool, 0444);
+MODULE_PARM_DESC(force_pio, "Force PIO mode (disables DMA) for SPI transfers. ([0] = disabled, 1 = enabled)");
+
 #define PCI_DEVICE_ID_INTEL_QUARK_X1000		0x0935
 #define PCI_DEVICE_ID_INTEL_BYT			0x0f0e
 #define PCI_DEVICE_ID_INTEL_MRFLD		0x1194
@@ -93,6 +98,32 @@ static void lpss_dma_put_device(void *dma_dev)
 	pci_dev_put(dma_dev);
 }
 
+static const struct dmi_system_id pxa2xx_spi_pci_dmi_table[] = {
+	{
+		.ident = "Apple MacBook8,1",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "MacBook8,1"),
+		},
+	},
+	{ }
+};
+
+static bool pxa2xx_spi_pci_can_dma(struct pci_dev *dev)
+{
+	if (spi_pxa2xx_force_pio) {
+		pci_info(dev, "Forcing PIO mode (disabling DMA)\n");
+		return false;
+	}
+
+	if (dmi_check_system(pxa2xx_spi_pci_dmi_table)) {
+		pci_info(dev, "MacBook8,1 detected: disabling DMA to force PIO mode\n");
+		return false;
+	}
+
+	return true;
+}
+
 static int lpss_spi_setup(struct pci_dev *dev, struct pxa2xx_spi_controller *c)
 {
 	struct ssp_device *ssp = &c->ssp;
@@ -166,7 +197,7 @@ static int lpss_spi_setup(struct pci_dev *dev, struct pxa2xx_spi_controller *c)
 
 	c->dma_filter = lpss_dma_filter;
 	c->dma_burst_size = 1;
-	c->enable_dma = 1;
+	c->enable_dma = pxa2xx_spi_pci_can_dma(dev);
 	return 0;
 }
 
@@ -238,7 +269,7 @@ static int mrfld_spi_setup(struct pci_dev *dev, struct pxa2xx_spi_controller *c)
 
 	c->dma_filter = lpss_dma_filter;
 	c->dma_burst_size = 8;
-	c->enable_dma = 1;
+	c->enable_dma = pxa2xx_spi_pci_can_dma(dev);
 	return 0;
 }
 
@@ -303,7 +334,8 @@ static int pxa2xx_spi_pci_probe(struct pci_dev *dev,
 	pm_runtime_set_autosuspend_delay(&dev->dev, 50);
 	pm_runtime_use_autosuspend(&dev->dev);
 	pm_runtime_put_autosuspend(&dev->dev);
-	pm_runtime_allow(&dev->dev);
+	if (pxa2xx_spi_pci_can_dma(dev))
+		pm_runtime_allow(&dev->dev);
 
 	return 0;
 }
-- 
2.39.5

[PATCH 2/2] spi: pxa2xx: restore LPSS private and IDMA registers on S3 resume

[Shih-Yuan Lee]

Intel LPSS SPI controllers lose all private register state across S3
suspend because the LPSS power domain is fully removed.  On resume the
driver only re-enables the SSP clock but leaves the LPSS private
registers (BAR0 0x200-0x2ff) and the IDMA registers (0x800-0x814) in
their power-on-reset state, which causes two separate problems:

1. LPSS_PRIV_RESETS (0x204) stays zero, keeping the functional block
   and IDMA in reset.  Writing 7 to de-assert both resets before any
   register access is mandatory; accessing MMIO while in reset causes a
   PCIe Completion Timeout and a watchdog-triggered system reset.

2. The IDMA block shares the SPI interrupt line.  With its registers
   zeroed the IDMA asserts a spurious interrupt that masks the real SPI
   interrupt, causing every subsequent SPI transfer to time out (-110).

3. The LPSS software chip-select control register (0x224) must *not* be
   blindly restored from its suspend-time snapshot: if CS was asserted
   at the moment of suspend, restoring that state corrupts the first
   post-resume SPI transaction.  Instead, call lpss_ssp_setup() which
   unconditionally writes SW_MODE | CS_HIGH (idle/deasserted), matching
   the state established at probe time.

Fix all three issues by:
  - Saving the four LPSS clock/LTR/SSP private registers and six IDMA
    registers in pxa2xx_spi_suspend().
  - In pxa2xx_spi_resume(), writing LPSS_PRIV_RESETS first, then
    restoring the saved private registers (excluding 0x204 and 0x224),
    then calling lpss_ssp_setup() to re-initialise CS to idle-high,
    and finally restoring the IDMA registers.

Signed-off-by: Shih-Yuan Lee <fourdollars@debian.org>
---
 drivers/spi/spi-pxa2xx.c | 57 ++++++++++++++++++++++++++++++++++++++++
 drivers/spi/spi-pxa2xx.h |  4 +++
 2 files changed, 61 insertions(+)

diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index 6291d7c2e06f..e8a83c818328 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -1479,12 +1479,35 @@ void pxa2xx_spi_remove(struct device *dev)
 }
 EXPORT_SYMBOL_NS_GPL(pxa2xx_spi_remove, "SPI_PXA2xx");
 
+/*
+ * LPSS private registers to save across S3 suspend.
+ * NOTE: 0x224 (CS control) is intentionally excluded - it is re-initialised
+ * by lpss_ssp_setup() on resume to ensure CS starts deasserted (idle-high).
+ */
+static const unsigned int lpss_saved_regs[] = {
+	0x200,
+	0x204,
+	0x220,
+	0x238,
+};
+
 static int pxa2xx_spi_suspend(struct device *dev)
 {
 	struct driver_data *drv_data = dev_get_drvdata(dev);
 	struct ssp_device *ssp = drv_data->ssp;
 	int status;
 
+	if (is_lpss_ssp(drv_data) && !pm_runtime_suspended(dev)) {
+		struct pxa2xx_spi_controller *pdata = drv_data->controller_info;
+		int i;
+
+		for (i = 0; i < ARRAY_SIZE(lpss_saved_regs); i++)
+			pdata->lpss_priv_ctx[i] = readl(ssp->mmio_base + lpss_saved_regs[i]);
+
+		for (i = 0; i < 6; i++)
+			pdata->lpss_idma_ctx[i] = readl(ssp->mmio_base + 0x800 + i * 4);
+	}
+
 	status = spi_controller_suspend(drv_data->controller);
 	if (status)
 		return status;
@@ -1508,6 +1531,40 @@ static int pxa2xx_spi_resume(struct device *dev)
 		status = clk_prepare_enable(ssp->clk);
 		if (status)
 			return status;
+
+		if (is_lpss_ssp(drv_data)) {
+			struct pxa2xx_spi_controller *pdata = drv_data->controller_info;
+			int i;
+
+			/* First de-assert resets by writing 7 to 0x204 (LPSS_PRIV_RESETS) */
+			writel(7, ssp->mmio_base + 0x204);
+
+			/*
+			 * Restore clock/LTR/SSP private registers.
+			 * 0x204 (resets) is skipped - already written above.
+			 * 0x224 (CS control) is skipped - re-initialised by
+			 * lpss_ssp_setup() below to ensure CS starts idle-high.
+			 */
+			for (i = 0; i < ARRAY_SIZE(lpss_saved_regs); i++) {
+				if (lpss_saved_regs[i] == 0x204)
+					continue;
+				writel(pdata->lpss_priv_ctx[i],
+				       ssp->mmio_base + lpss_saved_regs[i]);
+			}
+
+			/*
+			 * Re-initialise SW chip-select control so CS starts
+			 * deasserted (SW_MODE | CS_HIGH) regardless of the
+			 * state it was in at suspend time.
+			 */
+			lpss_ssp_setup(drv_data);
+
+			/* Restore IDMA registers */
+			for (i = 0; i < 6; i++) {
+				writel(pdata->lpss_idma_ctx[i],
+				       ssp->mmio_base + 0x800 + i * 4);
+			}
+		}
 	}
 
 	/* Start the queue running */
diff --git a/drivers/spi/spi-pxa2xx.h b/drivers/spi/spi-pxa2xx.h
index 447be0369384..423cef5118e7 100644
--- a/drivers/spi/spi-pxa2xx.h
+++ b/drivers/spi/spi-pxa2xx.h
@@ -34,6 +34,10 @@ struct pxa2xx_spi_controller {
 
 	/* For non-PXA arches */
 	struct ssp_device ssp;
+
+	/* LPSS private registers context */
+	u32 lpss_priv_ctx[4];
+	u32 lpss_idma_ctx[6];
 };
 
 struct spi_controller;
-- 
2.39.5

Re: [PATCH 2/2] spi: pxa2xx: restore LPSS private and IDMA registers on S3 resume

[Mark Brown]

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

On Mon, Jul 13, 2026 at 12:24:20AM +0800, Shih-Yuan Lee wrote:
> Intel LPSS SPI controllers lose all private register state across S3
> suspend because the LPSS power domain is fully removed.  On resume the
> driver only re-enables the SSP clock but leaves the LPSS private
> registers (BAR0 0x200-0x2ff) and the IDMA registers (0x800-0x814) in
> their power-on-reset state, which causes two separate problems:

> 1. LPSS_PRIV_RESETS (0x204) stays zero, keeping the functional block
>    and IDMA in reset.  Writing 7 to de-assert both resets before any
>    register access is mandatory; accessing MMIO while in reset causes a
>    PCIe Completion Timeout and a watchdog-triggered system reset.
> 
> 2. The IDMA block shares the SPI interrupt line.  With its registers
>    zeroed the IDMA asserts a spurious interrupt that masks the real SPI
>    interrupt, causing every subsequent SPI transfer to time out (-110).
> 
> 3. The LPSS software chip-select control register (0x224) must *not* be
>    blindly restored from its suspend-time snapshot: if CS was asserted
>    at the moment of suspend, restoring that state corrupts the first
>    post-resume SPI transaction.  Instead, call lpss_ssp_setup() which
>    unconditionally writes SW_MODE | CS_HIGH (idle/deasserted), matching
>    the state established at probe time.

That's three problems, not two.

> +/*
> + * LPSS private registers to save across S3 suspend.
> + * NOTE: 0x224 (CS control) is intentionally excluded - it is re-initialised
> + * by lpss_ssp_setup() on resume to ensure CS starts deasserted (idle-high).
> + */
> +static const unsigned int lpss_saved_regs[] = {
> +	0x200,
> +	0x204,
> +	0x220,
> +	0x238,
> +};

Unnamed registers?

>  static int pxa2xx_spi_suspend(struct device *dev)
>  {
>  	struct driver_data *drv_data = dev_get_drvdata(dev);
>  	struct ssp_device *ssp = drv_data->ssp;
>  	int status;
>  
> +	if (is_lpss_ssp(drv_data) && !pm_runtime_suspended(dev)) {
> +		struct pxa2xx_spi_controller *pdata = drv_data->controller_info;
> +		int i;
> +
> +		for (i = 0; i < ARRAY_SIZE(lpss_saved_regs); i++)
> +			pdata->lpss_priv_ctx[i] = readl(ssp->mmio_base + lpss_saved_regs[i]);
> +
> +		for (i = 0; i < 6; i++)
> +			pdata->lpss_idma_ctx[i] = readl(ssp->mmio_base + 0x800 + i * 4);
> +	}
> +
>  	status = spi_controller_suspend(drv_data->controller);

This is saving the registers before we quiesce the controller, the
values might change underneath us.

> +		if (is_lpss_ssp(drv_data)) {
> +			struct pxa2xx_spi_controller *pdata = drv_data->controller_info;
> +			int i;
> +
> +			/* First de-assert resets by writing 7 to 0x204 (LPSS_PRIV_RESETS) */
> +			writel(7, ssp->mmio_base + 0x204);

Do all the is_lpss_spi() devices have the same base offset?

These magic numbers are not good.

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