mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v17 0/6] spi: pxa2xx: PM fixes, teardown overhaul, and LPSS restore for MacBook8,1
@ 2026-09-30 16:06 Shih-Yuan Lee
  2026-09-30 16:06 ` [PATCH v17 1/6] spi: pxa2xx: rename local status variable to ret Shih-Yuan Lee
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Shih-Yuan Lee @ 2026-09-30 16:06 UTC (permalink / raw)
  To: Mark Brown
  Cc: Andy Shevchenko, Mika Westerberg, Lukas Wunner, Daniel Mack,
	Haojian Zhuang, Robert Jarzmik, linux-spi, linux-kernel,
	linux-arm-kernel, Shih-Yuan Lee

This series addresses power management, interrupt synchronization, and
S3 suspend/resume issues on Intel LPSS SPI controllers, particularly
focusing on making PIO mode robust and enabling reliable operation
for the SPI keyboard and touchpad on Apple MacBook8,1.

Patch breakdown:
- Patch 1: Rename status variable to ret in pxa2xx_spi_probe(), suspend,
  and resume to align with coding standards.
- Patch 2: Introduce serialized clock enable/disable helpers with clk_lock
  and a clk_enabled flag.
- Patch 3: Guard MMIO register access in ssp_int() by acquiring an active
  PM runtime reference, properly distinguishing suspended state (0) from
  RPM disabled state (-EINVAL), and synchronize probe IRQ registration.
- Patch 4: Overhaul teardown and suspend sequences to ensure in-flight
  interrupt handlers complete before the SOC clock is gated.
- Patch 5: Restore LPSS private registers and deassert functional, APB, and
  iDMA resets across S3 system resume in the PCI glue layer, scoped to
  Lynxpoint-LP (is_lpt), taking the register snapshot after the queue is
  quiesced, restoring registers and deasserting resets before power/clock
  resumption, and disabling D3cold during runtime PM.
- Patch 6: Disable DMA channel allocation to force PIO mode as a workaround
  and keep the controller in D0 specifically for Apple MacBook8,1 using a
  cached PXA2XX_SPI_QUIRK_NO_AUTOSUSPEND quirk, resolving DMA timeouts and
  eliminating 60-125 Hz input latency spikes while keeping safe autosuspend
  configuration in sysfs.

Changes since v16:
- Series restructured to 6 patches:
  - Merged runtime autosuspend lockout into the MacBook8,1 DMI quirk in
    the PCI glue driver (Patch 6/6), avoiding any generic core driver
    pinning.
  - Used PXA2XX_SPI_QUIRK_NO_AUTOSUSPEND in struct pxa2xx_spi_pci_config
    to keep MacBook8,1 in D0 without calling pm_runtime_allow().

- Clock management helpers (Patch 2/6):
  - Documented that pxa2xx_spi_clk_disable() provides idempotency to
    avoid Common Clock Framework underflow warnings when removing a
    device that is already runtime-suspended with its clock gated.

- Interrupt handler and PM synchronization (Patch 3/6):
  - In ssp_int(), distinguish between RPM suspended (return value 0)
    and RPM disabled / !CONFIG_PM (return value -EINVAL), servicing
    interrupts when RPM is not active and guarding pm_runtime_put*()
    with active > 0.
  - Dropped redundant lockless pm_runtime_status_suspended() pre-check.
  - Pair READ_ONCE(drv_data->clk_enabled) with WRITE_ONCE() updates.

- Teardown and suspend overhaul (Patch 4/6):
  - Documented that pm_runtime_force_resume() failure returns immediately
    without resuming the controller queue to prevent queuing messages to
    unpowered or unclocked hardware.

- LPSS S3 context restoration (Patch 5/6):
  - Renamed 'is_lpss' to 'is_lpt' in struct pxa2xx_spi_pci_config to
    accurately reflect Lynxpoint-LP scoping.
  - Verified structure packing with pahole (152 bytes, 0 internal
    padding holes).
  - In pxa2xx_spi_pci_suspend(), quiesce the controller queue with
    spi_controller_suspend() before capturing the LPSS register
    snapshot to ensure registers are in a clean idle state with chip
    select deasserted.
  - In pxa2xx_spi_pci_resume(), restore LPSS private registers and
    deassert functional, APB, and iDMA resets before calling
    pm_runtime_force_resume(), guaranteeing that the controller is out of
    reset before the clock is enabled and eliminating the shared IRQ
    window against unclocked/reset hardware.
  - Eliminated dead was_suspended branch in resume and kept clock helpers
    static to spi-pxa2xx.c.
  - Added pci_d3cold_disable() in probe for Lynxpoint-LP to guarantee
    registers are retained across S0 idle states.

- Apple MacBook8,1 Quirk (Patch 6/6):
  - Added 'quirks' field in struct pxa2xx_spi_pci_config placed right
    before 'is_lpt' (152 bytes, 0 internal holes verified by pahole).
  - Cached DMI match once during lpss_spi_setup() to avoid duplicate
    scans.
  - Documented forced PIO mode as a workaround for EFI leaving the
    companion DMAC held in reset and unrouted DMA completion interrupts.
  - In pxa2xx_spi_pci_probe(), always configure autosuspend delay to 50 ms
    and arm pm_runtime_use_autosuspend(), but conditionally skip
    pm_runtime_allow() when PXA2XX_SPI_QUIRK_NO_AUTOSUSPEND is set.
    This keeps the device in D0 by default while ensuring that if userspace
    enables autosuspend via sysfs, the safe 50 ms delay is preserved.
  - Maintained factual documentation reflecting OS parity (macOS Big
    Sur and Windows 10 Boot Camp) and logic board schematics.

Shih-Yuan Lee (6):
  spi: pxa2xx: rename local status variable to ret
  spi: pxa2xx: introduce clock enable and disable helper functions
  spi: pxa2xx: acquire active PM runtime reference in interrupt handler
  spi: pxa2xx: overhaul teardown and suspend sequence to synchronize IRQ
    before clock gating
  spi: pxa2xx-pci: restore LPSS private register state across S3 resume
  spi: pxa2xx-pci: disable DMA and runtime autosuspend for Apple
    MacBook8,1

 drivers/spi/spi-pxa2xx-pci.c | 212 ++++++++++++++++++++++++++++++++++-
 drivers/spi/spi-pxa2xx.c     | 177 +++++++++++++++++++----------
 drivers/spi/spi-pxa2xx.h     |   6 +
 3 files changed, 332 insertions(+), 63 deletions(-)

-- 
2.39.5

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v17 1/6] spi: pxa2xx: rename local status variable to ret
  2026-09-30 16:06 [PATCH v17 0/6] spi: pxa2xx: PM fixes, teardown overhaul, and LPSS restore for MacBook8,1 Shih-Yuan Lee
@ 2026-09-30 16:06 ` Shih-Yuan Lee
  2026-09-30 16:06 ` [PATCH v17 2/6] spi: pxa2xx: introduce clock enable and disable helper functions Shih-Yuan Lee
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Shih-Yuan Lee @ 2026-09-30 16:06 UTC (permalink / raw)
  To: Mark Brown
  Cc: Andy Shevchenko, Mika Westerberg, Lukas Wunner, Daniel Mack,
	Haojian Zhuang, Robert Jarzmik, linux-spi, linux-kernel,
	linux-arm-kernel, Shih-Yuan Lee

Rename the return value variable name from 'status' to 'ret' in the
pxa2xx_spi_probe(), pxa2xx_spi_suspend(), and pxa2xx_spi_resume()
functions to conform to standard Linux kernel coding conventions.

Assisted-by: Antigravity:gemini-3.8-flash sparse
Signed-off-by: Shih-Yuan Lee <fourdollars@debian.org>
---
 drivers/spi/spi-pxa2xx.c | 44 ++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index 6291d7c2e06f..b11e30589074 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -1274,7 +1274,7 @@ int pxa2xx_spi_probe(struct device *dev, struct ssp_device *ssp,
 	struct spi_controller *controller;
 	struct driver_data *drv_data;
 	const struct lpss_config *config;
-	int status;
+	int ret;
 	u32 tmp;
 
 	if (platform_info->is_target)
@@ -1330,15 +1330,15 @@ int pxa2xx_spi_probe(struct device *dev, struct ssp_device *ssp,
 						| SSSR_ROR | SSSR_TUR;
 	}
 
-	status = request_irq(ssp->irq, ssp_int, IRQF_SHARED, dev_name(dev),
+	ret = request_irq(ssp->irq, ssp_int, IRQF_SHARED, dev_name(dev),
 			drv_data);
-	if (status < 0)
-		return dev_err_probe(dev, status, "cannot get IRQ %d\n", ssp->irq);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "cannot get IRQ %d\n", ssp->irq);
 
 	/* Setup DMA if requested */
 	if (platform_info->enable_dma) {
-		status = pxa2xx_spi_dma_setup(drv_data);
-		if (status) {
+		ret = pxa2xx_spi_dma_setup(drv_data);
+		if (ret) {
 			dev_warn(dev, "no DMA channels available, using PIO\n");
 			platform_info->enable_dma = false;
 		} else {
@@ -1352,8 +1352,8 @@ int pxa2xx_spi_probe(struct device *dev, struct ssp_device *ssp,
 	}
 
 	/* Enable SOC clock */
-	status = clk_prepare_enable(ssp->clk);
-	if (status)
+	ret = clk_prepare_enable(ssp->clk);
+	if (ret)
 		goto out_error_dma_irq_alloc;
 
 	controller->max_speed_hz = clk_get_rate(ssp->clk);
@@ -1433,20 +1433,20 @@ int pxa2xx_spi_probe(struct device *dev, struct ssp_device *ssp,
 		drv_data->gpiod_ready = devm_gpiod_get_optional(dev,
 						"ready", GPIOD_OUT_LOW);
 		if (IS_ERR(drv_data->gpiod_ready)) {
-			status = PTR_ERR(drv_data->gpiod_ready);
+			ret = PTR_ERR(drv_data->gpiod_ready);
 			goto out_error_clock_enabled;
 		}
 	}
 
 	/* Register with the SPI framework */
 	dev_set_drvdata(dev, drv_data);
-	status = spi_register_controller(controller);
-	if (status) {
-		dev_err_probe(dev, status, "problem registering SPI controller\n");
+	ret = spi_register_controller(controller);
+	if (ret) {
+		dev_err_probe(dev, ret, "problem registering SPI controller\n");
 		goto out_error_clock_enabled;
 	}
 
-	return status;
+	return ret;
 
 out_error_clock_enabled:
 	clk_disable_unprepare(ssp->clk);
@@ -1455,7 +1455,7 @@ int pxa2xx_spi_probe(struct device *dev, struct ssp_device *ssp,
 	pxa2xx_spi_dma_release(drv_data);
 	free_irq(ssp->irq, drv_data);
 
-	return status;
+	return ret;
 }
 EXPORT_SYMBOL_NS_GPL(pxa2xx_spi_probe, "SPI_PXA2xx");
 
@@ -1483,11 +1483,11 @@ 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;
+	int ret;
 
-	status = spi_controller_suspend(drv_data->controller);
-	if (status)
-		return status;
+	ret = spi_controller_suspend(drv_data->controller);
+	if (ret)
+		return ret;
 
 	pxa_ssp_disable(ssp);
 
@@ -1501,13 +1501,13 @@ static int pxa2xx_spi_resume(struct device *dev)
 {
 	struct driver_data *drv_data = dev_get_drvdata(dev);
 	struct ssp_device *ssp = drv_data->ssp;
-	int status;
+	int ret;
 
 	/* Enable the SSP clock */
 	if (!pm_runtime_suspended(dev)) {
-		status = clk_prepare_enable(ssp->clk);
-		if (status)
-			return status;
+		ret = clk_prepare_enable(ssp->clk);
+		if (ret)
+			return ret;
 	}
 
 	/* Start the queue running */
-- 
2.39.5


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v17 2/6] spi: pxa2xx: introduce clock enable and disable helper functions
  2026-09-30 16:06 [PATCH v17 0/6] spi: pxa2xx: PM fixes, teardown overhaul, and LPSS restore for MacBook8,1 Shih-Yuan Lee
  2026-09-30 16:06 ` [PATCH v17 1/6] spi: pxa2xx: rename local status variable to ret Shih-Yuan Lee
@ 2026-09-30 16:06 ` Shih-Yuan Lee
  2026-09-30 17:31   ` Mark Brown
  2026-09-30 16:06 ` [PATCH v17 3/6] spi: pxa2xx: acquire active PM runtime reference in interrupt handler Shih-Yuan Lee
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 9+ messages in thread
From: Shih-Yuan Lee @ 2026-09-30 16:06 UTC (permalink / raw)
  To: Mark Brown
  Cc: Andy Shevchenko, Mika Westerberg, Lukas Wunner, Daniel Mack,
	Haojian Zhuang, Robert Jarzmik, linux-spi, linux-kernel,
	linux-arm-kernel, Shih-Yuan Lee

The driver enables and disables the SOC clock during probe, teardown,
and power management callbacks. Directly calling clk_disable_unprepare()
when the clock is already disabled—such as when removing a device that
is runtime-suspended—causes an unbalanced clock disable warning from the
Common Clock Framework.

Introduce pxa2xx_spi_clk_enable() and pxa2xx_spi_clk_disable() helper
functions that track the clock state with a 'clk_enabled' boolean flag
protected by a 'clk_lock' mutex in struct driver_data. These helpers
make clock toggling idempotent: repeated enable or disable invocations
are safe no-ops serialized by clk_lock.

Convert probe, remove, suspend, resume, and runtime PM callbacks to use
these helpers instead of direct clk_prepare_enable() and
clk_disable_unprepare() calls.

Pack 'clk_enabled' immediately after 'n_bytes' into the existing
padding hole in struct driver_data, avoiding additional alignment
padding.

Assisted-by: Antigravity:gemini-3.8-flash sparse
Signed-off-by: Shih-Yuan Lee <fourdollars@debian.org>
---
 drivers/spi/spi-pxa2xx.c | 41 ++++++++++++++++++++++++++++++++--------
 drivers/spi/spi-pxa2xx.h |  2 ++
 2 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index b11e30589074..a9d19d9a5343 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -713,6 +713,31 @@ static void handle_bad_msg(struct driver_data *drv_data)
 	dev_err(drv_data->ssp->dev, "bad message state in interrupt handler\n");
 }
 
+static int pxa2xx_spi_clk_enable(struct driver_data *drv_data)
+{
+	int ret = 0;
+
+	mutex_lock(&drv_data->clk_lock);
+	if (!drv_data->clk_enabled) {
+		ret = clk_prepare_enable(drv_data->ssp->clk);
+		if (!ret)
+			drv_data->clk_enabled = true;
+	}
+	mutex_unlock(&drv_data->clk_lock);
+
+	return ret;
+}
+
+static void pxa2xx_spi_clk_disable(struct driver_data *drv_data)
+{
+	mutex_lock(&drv_data->clk_lock);
+	if (drv_data->clk_enabled) {
+		drv_data->clk_enabled = false;
+		clk_disable_unprepare(drv_data->ssp->clk);
+	}
+	mutex_unlock(&drv_data->clk_lock);
+}
+
 static irqreturn_t ssp_int(int irq, void *dev_id)
 {
 	struct driver_data *drv_data = dev_id;
@@ -1288,6 +1313,7 @@ int pxa2xx_spi_probe(struct device *dev, struct ssp_device *ssp,
 	drv_data->controller = controller;
 	drv_data->controller_info = platform_info;
 	drv_data->ssp = ssp;
+	mutex_init(&drv_data->clk_lock);
 
 	/* The spi->mode bits understood by this driver: */
 	controller->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH | SPI_LOOP;
@@ -1352,7 +1378,7 @@ int pxa2xx_spi_probe(struct device *dev, struct ssp_device *ssp,
 	}
 
 	/* Enable SOC clock */
-	ret = clk_prepare_enable(ssp->clk);
+	ret = pxa2xx_spi_clk_enable(drv_data);
 	if (ret)
 		goto out_error_dma_irq_alloc;
 
@@ -1449,7 +1475,7 @@ int pxa2xx_spi_probe(struct device *dev, struct ssp_device *ssp,
 	return ret;
 
 out_error_clock_enabled:
-	clk_disable_unprepare(ssp->clk);
+	pxa2xx_spi_clk_disable(drv_data);
 
 out_error_dma_irq_alloc:
 	pxa2xx_spi_dma_release(drv_data);
@@ -1468,7 +1494,7 @@ void pxa2xx_spi_remove(struct device *dev)
 
 	/* Disable the SSP at the peripheral and SOC level */
 	pxa_ssp_disable(ssp);
-	clk_disable_unprepare(ssp->clk);
+	pxa2xx_spi_clk_disable(drv_data);
 
 	/* Release DMA */
 	if (drv_data->controller_info->enable_dma)
@@ -1492,7 +1518,7 @@ static int pxa2xx_spi_suspend(struct device *dev)
 	pxa_ssp_disable(ssp);
 
 	if (!pm_runtime_suspended(dev))
-		clk_disable_unprepare(ssp->clk);
+		pxa2xx_spi_clk_disable(drv_data);
 
 	return 0;
 }
@@ -1500,12 +1526,11 @@ static int pxa2xx_spi_suspend(struct device *dev)
 static int pxa2xx_spi_resume(struct device *dev)
 {
 	struct driver_data *drv_data = dev_get_drvdata(dev);
-	struct ssp_device *ssp = drv_data->ssp;
 	int ret;
 
 	/* Enable the SSP clock */
 	if (!pm_runtime_suspended(dev)) {
-		ret = clk_prepare_enable(ssp->clk);
+		ret = pxa2xx_spi_clk_enable(drv_data);
 		if (ret)
 			return ret;
 	}
@@ -1518,7 +1543,7 @@ static int pxa2xx_spi_runtime_suspend(struct device *dev)
 {
 	struct driver_data *drv_data = dev_get_drvdata(dev);
 
-	clk_disable_unprepare(drv_data->ssp->clk);
+	pxa2xx_spi_clk_disable(drv_data);
 	return 0;
 }
 
@@ -1526,7 +1551,7 @@ static int pxa2xx_spi_runtime_resume(struct device *dev)
 {
 	struct driver_data *drv_data = dev_get_drvdata(dev);
 
-	return clk_prepare_enable(drv_data->ssp->clk);
+	return pxa2xx_spi_clk_enable(drv_data);
 }
 
 EXPORT_NS_GPL_DEV_PM_OPS(pxa2xx_spi_pm_ops, SPI_PXA2xx) = {
diff --git a/drivers/spi/spi-pxa2xx.h b/drivers/spi/spi-pxa2xx.h
index 447be0369384..3352239f74a0 100644
--- a/drivers/spi/spi-pxa2xx.h
+++ b/drivers/spi/spi-pxa2xx.h
@@ -66,6 +66,8 @@ struct driver_data {
 	void *rx;
 	void *rx_end;
 	u8 n_bytes;
+	bool clk_enabled;
+	struct mutex clk_lock;
 	int (*write)(struct driver_data *drv_data);
 	int (*read)(struct driver_data *drv_data);
 	irqreturn_t (*transfer_handler)(struct driver_data *drv_data);
-- 
2.39.5


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v17 3/6] spi: pxa2xx: acquire active PM runtime reference in interrupt handler
  2026-09-30 16:06 [PATCH v17 0/6] spi: pxa2xx: PM fixes, teardown overhaul, and LPSS restore for MacBook8,1 Shih-Yuan Lee
  2026-09-30 16:06 ` [PATCH v17 1/6] spi: pxa2xx: rename local status variable to ret Shih-Yuan Lee
  2026-09-30 16:06 ` [PATCH v17 2/6] spi: pxa2xx: introduce clock enable and disable helper functions Shih-Yuan Lee
@ 2026-09-30 16:06 ` Shih-Yuan Lee
  2026-09-30 17:40   ` Mark Brown
  2026-09-30 16:06 ` [PATCH v17 4/6] spi: pxa2xx: overhaul teardown and suspend sequence to synchronize IRQ before clock gating Shih-Yuan Lee
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 9+ messages in thread
From: Shih-Yuan Lee @ 2026-09-30 16:06 UTC (permalink / raw)
  To: Mark Brown
  Cc: Andy Shevchenko, Mika Westerberg, Lukas Wunner, Daniel Mack,
	Haojian Zhuang, Robert Jarzmik, linux-spi, linux-kernel,
	linux-arm-kernel, Shih-Yuan Lee

On a shared interrupt line, ssp_int() can be invoked while the SPI
controller is suspended or transitioning power states. In ssp_int(),
evaluating device power state without acquiring a reference before
accessing MMIO registers is not atomic: another thread executing
pm_runtime_suspend() can drop the last reference and gate the clock
immediately after the check passes. Accessing unclocked registers then
leads to PCIe Completion Timeouts and system hangs.

In addition, request_irq() is called in pxa2xx_spi_probe() before the
clock is enabled and the controller hardware is initialized, allowing
shared interrupts to fire against partially configured hardware.

Ensure the interrupt handler runs only when the hardware is clocked and
active:
1. In ssp_int(), check READ_ONCE(drv_data->clk_enabled), immediately
   returning IRQ_NONE if the clock is disabled. Update 'clk_enabled'
   with WRITE_ONCE() in the clock helpers to pair with the lockless
   reader.
2. Call pm_runtime_get_if_active(dev) in ssp_int() to atomically check
   if the device is active and acquire a PM runtime reference. If
   runtime PM is enabled and the device is suspended (return value 0),
   return IRQ_NONE without accessing MMIO registers. When runtime PM is
   disabled (return value -EINVAL), the controller is permanently
   active, so proceed with servicing the interrupt.
3. On exit, if an active PM reference was acquired (> 0), release it
   via pm_runtime_put_autosuspend() when an interrupt was handled
   (IRQ_HANDLED) to preserve the autosuspend delay, or via
   pm_runtime_put() on IRQ_NONE to avoid refreshing the autosuspend
   timer on foreign interrupts.
4. In pxa2xx_spi_probe(), move request_irq() after the clock is enabled
   and hardware registers are fully initialized, right before
   spi_register_controller(), ensuring no interrupt can run against
   unconfigured hardware, and update error unwinding accordingly.

Assisted-by: Antigravity:gemini-3.8-flash spin sparse
Signed-off-by: Shih-Yuan Lee <fourdollars@debian.org>
---
 drivers/spi/spi-pxa2xx.c | 68 +++++++++++++++++++++++++++-------------
 1 file changed, 47 insertions(+), 21 deletions(-)

diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index a9d19d9a5343..b091434977af 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -721,7 +721,7 @@ static int pxa2xx_spi_clk_enable(struct driver_data *drv_data)
 	if (!drv_data->clk_enabled) {
 		ret = clk_prepare_enable(drv_data->ssp->clk);
 		if (!ret)
-			drv_data->clk_enabled = true;
+			WRITE_ONCE(drv_data->clk_enabled, true);
 	}
 	mutex_unlock(&drv_data->clk_lock);
 
@@ -732,7 +732,7 @@ static void pxa2xx_spi_clk_disable(struct driver_data *drv_data)
 {
 	mutex_lock(&drv_data->clk_lock);
 	if (drv_data->clk_enabled) {
-		drv_data->clk_enabled = false;
+		WRITE_ONCE(drv_data->clk_enabled, false);
 		clk_disable_unprepare(drv_data->ssp->clk);
 	}
 	mutex_unlock(&drv_data->clk_lock);
@@ -741,17 +741,25 @@ static void pxa2xx_spi_clk_disable(struct driver_data *drv_data)
 static irqreturn_t ssp_int(int irq, void *dev_id)
 {
 	struct driver_data *drv_data = dev_id;
+	struct device *dev = drv_data->ssp->dev;
 	u32 sccr1_reg;
 	u32 mask = drv_data->mask_sr;
 	u32 status;
+	irqreturn_t ret;
+	int active;
 
 	/*
-	 * The IRQ might be shared with other peripherals so we must first
-	 * check that are we RPM suspended or not. If we are we assume that
-	 * the IRQ was not for us (we shouldn't be RPM suspended when the
-	 * interrupt is enabled).
+	 * The IRQ might be shared with other peripherals or trigger during
+	 * power state transitions. Ensure the clock is active, and acquire
+	 * an active PM runtime reference while ssp_int() runs. If runtime
+	 * PM is enabled and the device is suspended (active == 0), return
+	 * IRQ_NONE immediately without touching MMIO registers.
 	 */
-	if (pm_runtime_suspended(drv_data->ssp->dev))
+	if (!READ_ONCE(drv_data->clk_enabled))
+		return IRQ_NONE;
+
+	active = pm_runtime_get_if_active(dev);
+	if (active == 0)
 		return IRQ_NONE;
 
 	/*
@@ -762,7 +770,7 @@ static irqreturn_t ssp_int(int irq, void *dev_id)
 	 */
 	status = pxa2xx_spi_read(drv_data, SSSR);
 	if (status == ~0)
-		return IRQ_NONE;
+		goto out_put;
 
 	sccr1_reg = pxa2xx_spi_read(drv_data, SSCR1);
 
@@ -775,18 +783,32 @@ static irqreturn_t ssp_int(int irq, void *dev_id)
 		mask &= ~SSSR_TINT;
 
 	if (!(status & mask))
-		return IRQ_NONE;
+		goto out_put;
 
 	pxa2xx_spi_write(drv_data, SSCR1, sccr1_reg & ~drv_data->int_cr1);
 	pxa2xx_spi_write(drv_data, SSCR1, sccr1_reg);
 
 	if (!drv_data->controller->cur_msg) {
 		handle_bad_msg(drv_data);
-		/* Never fail */
-		return IRQ_HANDLED;
+		ret = IRQ_HANDLED;
+		goto out;
 	}
 
-	return drv_data->transfer_handler(drv_data);
+	ret = drv_data->transfer_handler(drv_data);
+	goto out;
+
+out_put:
+	ret = IRQ_NONE;
+
+out:
+	if (active > 0) {
+		if (ret == IRQ_HANDLED)
+			pm_runtime_put_autosuspend(dev);
+		else
+			pm_runtime_put(dev);
+	}
+
+	return ret;
 }
 
 /*
@@ -1356,11 +1378,6 @@ int pxa2xx_spi_probe(struct device *dev, struct ssp_device *ssp,
 						| SSSR_ROR | SSSR_TUR;
 	}
 
-	ret = request_irq(ssp->irq, ssp_int, IRQF_SHARED, dev_name(dev),
-			drv_data);
-	if (ret < 0)
-		return dev_err_probe(dev, ret, "cannot get IRQ %d\n", ssp->irq);
-
 	/* Setup DMA if requested */
 	if (platform_info->enable_dma) {
 		ret = pxa2xx_spi_dma_setup(drv_data);
@@ -1380,7 +1397,7 @@ int pxa2xx_spi_probe(struct device *dev, struct ssp_device *ssp,
 	/* Enable SOC clock */
 	ret = pxa2xx_spi_clk_enable(drv_data);
 	if (ret)
-		goto out_error_dma_irq_alloc;
+		goto out_error_dma_alloc;
 
 	controller->max_speed_hz = clk_get_rate(ssp->clk);
 	/*
@@ -1464,22 +1481,31 @@ int pxa2xx_spi_probe(struct device *dev, struct ssp_device *ssp,
 		}
 	}
 
+	ret = request_irq(ssp->irq, ssp_int, IRQF_SHARED, dev_name(dev),
+			drv_data);
+	if (ret < 0) {
+		ret = dev_err_probe(dev, ret, "cannot get IRQ %d\n", ssp->irq);
+		goto out_error_clock_enabled;
+	}
+
 	/* Register with the SPI framework */
 	dev_set_drvdata(dev, drv_data);
 	ret = spi_register_controller(controller);
 	if (ret) {
 		dev_err_probe(dev, ret, "problem registering SPI controller\n");
-		goto out_error_clock_enabled;
+		goto out_error_irq_alloc;
 	}
 
 	return ret;
 
+out_error_irq_alloc:
+	free_irq(ssp->irq, drv_data);
+
 out_error_clock_enabled:
 	pxa2xx_spi_clk_disable(drv_data);
 
-out_error_dma_irq_alloc:
+out_error_dma_alloc:
 	pxa2xx_spi_dma_release(drv_data);
-	free_irq(ssp->irq, drv_data);
 
 	return ret;
 }
-- 
2.39.5


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v17 4/6] spi: pxa2xx: overhaul teardown and suspend sequence to synchronize IRQ before clock gating
  2026-09-30 16:06 [PATCH v17 0/6] spi: pxa2xx: PM fixes, teardown overhaul, and LPSS restore for MacBook8,1 Shih-Yuan Lee
                   ` (2 preceding siblings ...)
  2026-09-30 16:06 ` [PATCH v17 3/6] spi: pxa2xx: acquire active PM runtime reference in interrupt handler Shih-Yuan Lee
@ 2026-09-30 16:06 ` Shih-Yuan Lee
  2026-09-30 16:06 ` [PATCH v17 5/6] spi: pxa2xx-pci: restore LPSS private register state across S3 resume Shih-Yuan Lee
  2026-09-30 16:06 ` [PATCH v17 6/6] spi: pxa2xx-pci: disable DMA and runtime autosuspend for Apple MacBook8,1 Shih-Yuan Lee
  5 siblings, 0 replies; 9+ messages in thread
From: Shih-Yuan Lee @ 2026-09-30 16:06 UTC (permalink / raw)
  To: Mark Brown
  Cc: Andy Shevchenko, Mika Westerberg, Lukas Wunner, Daniel Mack,
	Haojian Zhuang, Robert Jarzmik, linux-spi, linux-kernel,
	linux-arm-kernel, Shih-Yuan Lee

When removing the driver or suspending the device, the clock must not
be disabled while shared interrupts are still active. Gating the clock
before waiting for in-flight interrupt handlers to complete results
in race conditions where the handler performs unclocked MMIO accesses,
causing PCIe Completion Timeouts.

Overhaul the remove, suspend, and runtime_suspend paths to use a strict
synchronized teardown order:
1. In remove, call free_irq() (which internally synchronizes any in-flight
   handlers) before disabling the clock via pxa2xx_spi_clk_disable().
2. In runtime_suspend, under clk_lock and only when the clock is enabled,
   disable the SSP peripheral via pxa_ssp_disable(), clear 'clk_enabled'
   via WRITE_ONCE() so that new interrupts immediately bail out with
   IRQ_NONE, drain in-flight handlers via synchronize_irq() while the
   clock is still running, and finally gate the clock with
   clk_disable_unprepare().
3. In system suspend, suspend the controller queue and use
   pm_runtime_force_suspend() to invoke runtime_suspend, ensuring
   in-flight interrupts are drained before the clock is gated. If
   pm_runtime_force_suspend() fails, resume the controller queue so
   the controller remains operational since the system will stay awake.
4. In system resume, restore the device state using
   pm_runtime_force_resume() before restarting the controller queue.
   If pm_runtime_force_resume() fails, return the error immediately
   without calling spi_controller_resume(), keeping the queue stopped
   to prevent transferring messages against unclocked or unpowered
   hardware. If the device was already runtime-suspended prior to
   system sleep, pm_runtime_force_resume() leaves the clock gated until
   the next transfer resumes it, optimizing idle power.

Throughout suspended states, 'clk_enabled' being false serves as the
primary invariant ensuring that any subsequent interrupt handler
invocation safely returns IRQ_NONE without accessing hardware registers.

Assisted-by: Antigravity:gemini-3.8-flash spin sparse
Signed-off-by: Shih-Yuan Lee <fourdollars@debian.org>
---
 drivers/spi/spi-pxa2xx.c | 39 +++++++++++++++++++++++----------------
 1 file changed, 23 insertions(+), 16 deletions(-)

diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index b091434977af..2a3fa9ca7213 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -1520,31 +1520,33 @@ void pxa2xx_spi_remove(struct device *dev)
 
 	/* Disable the SSP at the peripheral and SOC level */
 	pxa_ssp_disable(ssp);
+
+	/* Release IRQ before gating the SOC clock */
+	free_irq(ssp->irq, drv_data);
+
+	/* Safe to disable the SSP clock now */
 	pxa2xx_spi_clk_disable(drv_data);
 
 	/* Release DMA */
 	if (drv_data->controller_info->enable_dma)
 		pxa2xx_spi_dma_release(drv_data);
-
-	/* Release IRQ */
-	free_irq(ssp->irq, drv_data);
 }
 EXPORT_SYMBOL_NS_GPL(pxa2xx_spi_remove, "SPI_PXA2xx");
 
 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 ret;
 
 	ret = spi_controller_suspend(drv_data->controller);
 	if (ret)
 		return ret;
 
-	pxa_ssp_disable(ssp);
-
-	if (!pm_runtime_suspended(dev))
-		pxa2xx_spi_clk_disable(drv_data);
+	ret = pm_runtime_force_suspend(dev);
+	if (ret) {
+		spi_controller_resume(drv_data->controller);
+		return ret;
+	}
 
 	return 0;
 }
@@ -1554,14 +1556,10 @@ static int pxa2xx_spi_resume(struct device *dev)
 	struct driver_data *drv_data = dev_get_drvdata(dev);
 	int ret;
 
-	/* Enable the SSP clock */
-	if (!pm_runtime_suspended(dev)) {
-		ret = pxa2xx_spi_clk_enable(drv_data);
-		if (ret)
-			return ret;
-	}
+	ret = pm_runtime_force_resume(dev);
+	if (ret)
+		return ret;
 
-	/* Start the queue running */
 	return spi_controller_resume(drv_data->controller);
 }
 
@@ -1569,7 +1567,16 @@ static int pxa2xx_spi_runtime_suspend(struct device *dev)
 {
 	struct driver_data *drv_data = dev_get_drvdata(dev);
 
-	pxa2xx_spi_clk_disable(drv_data);
+	mutex_lock(&drv_data->clk_lock);
+	if (drv_data->clk_enabled) {
+		pxa_ssp_disable(drv_data->ssp);
+		WRITE_ONCE(drv_data->clk_enabled, false);
+		mutex_unlock(&drv_data->clk_lock);
+		synchronize_irq(drv_data->ssp->irq);
+		clk_disable_unprepare(drv_data->ssp->clk);
+	} else {
+		mutex_unlock(&drv_data->clk_lock);
+	}
 	return 0;
 }
 
-- 
2.39.5


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v17 5/6] spi: pxa2xx-pci: restore LPSS private register state across S3 resume
  2026-09-30 16:06 [PATCH v17 0/6] spi: pxa2xx: PM fixes, teardown overhaul, and LPSS restore for MacBook8,1 Shih-Yuan Lee
                   ` (3 preceding siblings ...)
  2026-09-30 16:06 ` [PATCH v17 4/6] spi: pxa2xx: overhaul teardown and suspend sequence to synchronize IRQ before clock gating Shih-Yuan Lee
@ 2026-09-30 16:06 ` Shih-Yuan Lee
  2026-09-30 16:06 ` [PATCH v17 6/6] spi: pxa2xx-pci: disable DMA and runtime autosuspend for Apple MacBook8,1 Shih-Yuan Lee
  5 siblings, 0 replies; 9+ messages in thread
From: Shih-Yuan Lee @ 2026-09-30 16:06 UTC (permalink / raw)
  To: Mark Brown
  Cc: Andy Shevchenko, Mika Westerberg, Lukas Wunner, Daniel Mack,
	Haojian Zhuang, Robert Jarzmik, linux-spi, linux-kernel,
	linux-arm-kernel, Shih-Yuan Lee

On platforms where Intel LPSS SPI is enumerated as a bare PCI device
(such as the Apple MacBook8,1 on Lynxpoint-LP), the LPSS power island
loses power during system sleep and powers up with the controller held
in reset. Because the device lacks ACPI companion objects or MFD
binding, neither drivers/acpi/x86/lpss.c nor drivers/mfd/intel-lpss.c
restore the private register context across S3 resume.

This causes memory-mapped I/O reads on resume to return ~0, resulting
in PCIe Completion Timeouts, missing interrupts, and dead
keyboard/touchpad.

Restore LPSS private registers and deassert functional, APB, and iDMA
resets in the PCI glue layer:
1. In suspend, quiesce the controller queue via spi_controller_suspend()
   so no transfers remain in flight and chip select is deasserted.
   Then save the LPSS private registers (LPSS_PRIV_REG_COUNT 9 covering
   up to offset 0x20 including reg_cs_ctrl at 0x18). Ensure the clock is
   active for reading MMIO registers using pm_runtime_resume_and_get(),
   balanced by pm_runtime_put_noidle(), before calling
   pm_runtime_force_suspend().
2. In resume, restore LPSS private registers and deassert functional,
   APB, and iDMA resets via pxa2xx_spi_pci_lpss_restore_ctx() before
   calling pm_runtime_force_resume(). Because the controller clock
   remains gated (clk_enabled is false) during this restoration, any
   interrupt arriving on a shared line bails out early in ssp_int()
   without accessing registers on a controller held in reset. Once
   resets are deasserted and private registers are restored, call
   pm_runtime_force_resume() to enable the clock and mark the device
   active, and restart the queue with spi_controller_resume().
3. In probe, call pci_d3cold_disable() for Lynxpoint-LP (is_lpt) to
   ensure the PCI device cannot enter D3cold during runtime PM,
   guaranteeing that LPSS private registers are retained during S0 idle
   periods and only require restoration across S3 system sleep. Set
   DPM_FLAG_NO_DIRECT_COMPLETE to prevent PCI subsystem direct-complete
   bypassing system sleep callbacks.
4. Scope LPSS context restoration specifically to Lynxpoint-LP
   (PCI_DEVICE_ID_INTEL_LPT*) devices to avoid perturbing other LPSS
   PCI platforms (BYT, BSW) or non-LPSS platforms.

Export lpss_ssp_setup() and core runtime PM ops so the PCI glue layer
can invoke them around private register restoration.

Assisted-by: Antigravity:gemini-3.8-flash sparse
Signed-off-by: Shih-Yuan Lee <fourdollars@debian.org>
---
 drivers/spi/spi-pxa2xx-pci.c | 151 ++++++++++++++++++++++++++++++++++-
 drivers/spi/spi-pxa2xx.c     |   9 ++-
 drivers/spi/spi-pxa2xx.h     |   4 +
 3 files changed, 157 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-pxa2xx-pci.c b/drivers/spi/spi-pxa2xx-pci.c
index cae77ac18520..f8d71362e840 100644
--- a/drivers/spi/spi-pxa2xx-pci.c
+++ b/drivers/spi/spi-pxa2xx-pci.c
@@ -12,6 +12,7 @@
 #include <linux/pci.h>
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
+#include <linux/spi/spi.h>
 #include <linux/sprintf.h>
 #include <linux/string.h>
 #include <linux/types.h>
@@ -33,10 +34,22 @@
 #define PCI_DEVICE_ID_INTEL_LPT1_0		0x9ce5
 #define PCI_DEVICE_ID_INTEL_LPT1_1		0x9ce6
 
+#define LPSS_PRIV_RESETS			0x04
+#define LPSS_PRIV_RESETS_FUNC			BIT(0)
+#define LPSS_PRIV_RESETS_APB			BIT(1)
+#define LPSS_PRIV_RESETS_IDMA			BIT(2)
+#define LPSS_PRIV_REG_COUNT			9
+
 struct pxa_spi_info {
 	int (*setup)(struct pci_dev *pdev, struct pxa2xx_spi_controller *c);
 };
 
+struct pxa2xx_spi_pci_config {
+	struct pxa2xx_spi_controller pdata;
+	u32 lpss_priv_ctx[LPSS_PRIV_REG_COUNT];
+	bool is_lpt;
+};
+
 static struct dw_dma_slave byt_tx_param = { .dst_id = 0 };
 static struct dw_dma_slave byt_rx_param = { .src_id = 1 };
 
@@ -95,6 +108,7 @@ static void lpss_dma_put_device(void *dma_dev)
 
 static int lpss_spi_setup(struct pci_dev *dev, struct pxa2xx_spi_controller *c)
 {
+	struct pxa2xx_spi_pci_config *cfg = container_of(c, struct pxa2xx_spi_pci_config, pdata);
 	struct ssp_device *ssp = &c->ssp;
 	struct dw_dma_slave *tx, *rx;
 	struct pci_dev *dma_dev;
@@ -131,6 +145,7 @@ static int lpss_spi_setup(struct pci_dev *dev, struct pxa2xx_spi_controller *c)
 		ssp->port_id = 0;
 		c->tx_param = &lpt0_tx_param;
 		c->rx_param = &lpt0_rx_param;
+		cfg->is_lpt = true;
 		break;
 	case PCI_DEVICE_ID_INTEL_LPT0_1:
 	case PCI_DEVICE_ID_INTEL_LPT1_1:
@@ -138,6 +153,7 @@ static int lpss_spi_setup(struct pci_dev *dev, struct pxa2xx_spi_controller *c)
 		ssp->port_id = 1;
 		c->tx_param = &lpt1_tx_param;
 		c->rx_param = &lpt1_rx_param;
+		cfg->is_lpt = true;
 		break;
 	default:
 		return -ENODEV;
@@ -261,22 +277,143 @@ static const struct pxa_spi_info qrk_info_config = {
 	.setup = qrk_spi_setup,
 };
 
+static void pxa2xx_spi_pci_lpss_save_ctx(struct device *dev)
+{
+	struct driver_data *drv_data = dev_get_drvdata(dev);
+	struct pxa2xx_spi_pci_config *cfg;
+	unsigned int i;
+
+	if (!drv_data || !drv_data->controller_info)
+		return;
+
+	cfg = container_of(drv_data->controller_info,
+			   struct pxa2xx_spi_pci_config, pdata);
+
+	if (cfg->is_lpt && drv_data->lpss_base) {
+		for (i = 0; i < LPSS_PRIV_REG_COUNT; i++)
+			cfg->lpss_priv_ctx[i] = readl(drv_data->lpss_base + i * 4);
+	}
+}
+
+static void pxa2xx_spi_pci_lpss_restore_ctx(struct device *dev)
+{
+	struct driver_data *drv_data = dev_get_drvdata(dev);
+	struct pxa2xx_spi_pci_config *cfg;
+	unsigned int i;
+	u32 resets;
+
+	if (!drv_data || !drv_data->controller_info)
+		return;
+
+	cfg = container_of(drv_data->controller_info,
+			   struct pxa2xx_spi_pci_config, pdata);
+
+	if (cfg->is_lpt && drv_data->lpss_base) {
+		/*
+		 * Deassert functional, APB, and iDMA resets, preserving any
+		 * platform-specific bits in LPSS_PRIV_RESETS.
+		 */
+		resets = cfg->lpss_priv_ctx[LPSS_PRIV_RESETS / 4] |
+			 LPSS_PRIV_RESETS_FUNC | LPSS_PRIV_RESETS_APB |
+			 LPSS_PRIV_RESETS_IDMA;
+		writel(resets, drv_data->lpss_base + LPSS_PRIV_RESETS);
+
+		for (i = 0; i < LPSS_PRIV_REG_COUNT; i++) {
+			if (i == LPSS_PRIV_RESETS / 4)
+				continue;
+			writel(cfg->lpss_priv_ctx[i], drv_data->lpss_base + i * 4);
+		}
+
+		/*
+		 * Program chip select to deasserted state before resuming the
+		 * controller queue, preventing races with the message pump kthread.
+		 */
+		lpss_ssp_setup(drv_data);
+	}
+}
+
+static int pxa2xx_spi_pci_suspend(struct device *dev)
+{
+	struct driver_data *drv_data = dev_get_drvdata(dev);
+	struct pxa2xx_spi_pci_config *cfg;
+	int ret;
+
+	if (!drv_data || !drv_data->controller_info)
+		return 0;
+
+	cfg = container_of(drv_data->controller_info,
+			   struct pxa2xx_spi_pci_config, pdata);
+
+	ret = spi_controller_suspend(drv_data->controller);
+	if (ret)
+		return ret;
+
+	if (cfg->is_lpt) {
+		ret = pm_runtime_resume_and_get(dev);
+		if (ret < 0) {
+			spi_controller_resume(drv_data->controller);
+			return ret;
+		}
+
+		pxa2xx_spi_pci_lpss_save_ctx(dev);
+		pm_runtime_put_noidle(dev);
+	}
+
+	ret = pm_runtime_force_suspend(dev);
+	if (ret) {
+		spi_controller_resume(drv_data->controller);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int pxa2xx_spi_pci_resume(struct device *dev)
+{
+	struct driver_data *drv_data = dev_get_drvdata(dev);
+	struct pxa2xx_spi_pci_config *cfg;
+	int ret;
+
+	if (!drv_data || !drv_data->controller_info)
+		return 0;
+
+	cfg = container_of(drv_data->controller_info,
+			   struct pxa2xx_spi_pci_config, pdata);
+
+	if (cfg->is_lpt)
+		pxa2xx_spi_pci_lpss_restore_ctx(dev);
+
+	ret = pm_runtime_force_resume(dev);
+	if (ret)
+		return ret;
+
+	return spi_controller_resume(drv_data->controller);
+}
+
+static const struct dev_pm_ops pxa2xx_spi_pci_pm_ops = {
+	SYSTEM_SLEEP_PM_OPS(pxa2xx_spi_pci_suspend, pxa2xx_spi_pci_resume)
+	RUNTIME_PM_OPS(pxa2xx_spi_runtime_suspend, pxa2xx_spi_runtime_resume, NULL)
+};
+
 static int pxa2xx_spi_pci_probe(struct pci_dev *dev,
 		const struct pci_device_id *ent)
 {
 	const struct pxa_spi_info *info;
-	int ret;
+	struct pxa2xx_spi_pci_config *cfg;
 	struct pxa2xx_spi_controller *pdata;
 	struct ssp_device *ssp;
+	int ret;
 
 	ret = pcim_enable_device(dev);
 	if (ret)
 		return ret;
 
-	pdata = devm_kzalloc(&dev->dev, sizeof(*pdata), GFP_KERNEL);
-	if (!pdata)
+	cfg = devm_kzalloc(&dev->dev, sizeof(*cfg), GFP_KERNEL);
+	if (!cfg)
 		return -ENOMEM;
 
+	pdata = &cfg->pdata;
+
 	ssp = &pdata->ssp;
 	ssp->dev = &dev->dev;
 	ssp->phys_base = pci_resource_start(dev, 0);
@@ -300,6 +437,12 @@ static int pxa2xx_spi_pci_probe(struct pci_dev *dev,
 	if (ret)
 		return ret;
 
+	if (cfg->is_lpt) {
+		pci_d3cold_disable(dev);
+		dev_pm_set_driver_flags(&dev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
+		pxa2xx_spi_pci_lpss_save_ctx(&dev->dev);
+	}
+
 	pm_runtime_set_autosuspend_delay(&dev->dev, 50);
 	pm_runtime_use_autosuspend(&dev->dev);
 	pm_runtime_put_autosuspend(&dev->dev);
@@ -336,7 +479,7 @@ static struct pci_driver pxa2xx_spi_pci_driver = {
 	.name           = "pxa2xx_spi_pci",
 	.id_table       = pxa2xx_spi_pci_devices,
 	.driver = {
-		.pm	= pm_ptr(&pxa2xx_spi_pm_ops),
+		.pm	= pm_ptr(&pxa2xx_spi_pci_pm_ops),
 	},
 	.probe          = pxa2xx_spi_pci_probe,
 	.remove         = pxa2xx_spi_pci_remove,
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index 2a3fa9ca7213..56ddfa67708e 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -343,7 +343,7 @@ static bool __lpss_ssp_update_priv(struct driver_data *drv_data, unsigned int of
  * Perform LPSS SSP specific setup. This function must be called first if
  * one is going to use LPSS SSP private registers.
  */
-static void lpss_ssp_setup(struct driver_data *drv_data)
+void lpss_ssp_setup(struct driver_data *drv_data)
 {
 	const struct lpss_config *config;
 	u32 value;
@@ -365,6 +365,7 @@ static void lpss_ssp_setup(struct driver_data *drv_data)
 		}
 	}
 }
+EXPORT_SYMBOL_NS_GPL(lpss_ssp_setup, "SPI_PXA2xx");
 
 static void lpss_ssp_select_cs(struct spi_device *spi,
 			       const struct lpss_config *config)
@@ -1563,7 +1564,7 @@ static int pxa2xx_spi_resume(struct device *dev)
 	return spi_controller_resume(drv_data->controller);
 }
 
-static int pxa2xx_spi_runtime_suspend(struct device *dev)
+int pxa2xx_spi_runtime_suspend(struct device *dev)
 {
 	struct driver_data *drv_data = dev_get_drvdata(dev);
 
@@ -1579,13 +1580,15 @@ static int pxa2xx_spi_runtime_suspend(struct device *dev)
 	}
 	return 0;
 }
+EXPORT_SYMBOL_NS_GPL(pxa2xx_spi_runtime_suspend, "SPI_PXA2xx");
 
-static int pxa2xx_spi_runtime_resume(struct device *dev)
+int pxa2xx_spi_runtime_resume(struct device *dev)
 {
 	struct driver_data *drv_data = dev_get_drvdata(dev);
 
 	return pxa2xx_spi_clk_enable(drv_data);
 }
+EXPORT_SYMBOL_NS_GPL(pxa2xx_spi_runtime_resume, "SPI_PXA2xx");
 
 EXPORT_NS_GPL_DEV_PM_OPS(pxa2xx_spi_pm_ops, SPI_PXA2xx) = {
 	SYSTEM_SLEEP_PM_OPS(pxa2xx_spi_suspend, pxa2xx_spi_resume)
diff --git a/drivers/spi/spi-pxa2xx.h b/drivers/spi/spi-pxa2xx.h
index 3352239f74a0..a31022256202 100644
--- a/drivers/spi/spi-pxa2xx.h
+++ b/drivers/spi/spi-pxa2xx.h
@@ -138,6 +138,10 @@ int pxa2xx_spi_probe(struct device *dev, struct ssp_device *ssp,
 		     struct pxa2xx_spi_controller *platform_info);
 void pxa2xx_spi_remove(struct device *dev);
 
+void lpss_ssp_setup(struct driver_data *drv_data);
+int pxa2xx_spi_runtime_suspend(struct device *dev);
+int pxa2xx_spi_runtime_resume(struct device *dev);
+
 extern const struct dev_pm_ops pxa2xx_spi_pm_ops;
 
 #endif /* SPI_PXA2XX_H */
-- 
2.39.5


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v17 6/6] spi: pxa2xx-pci: disable DMA and runtime autosuspend for Apple MacBook8,1
  2026-09-30 16:06 [PATCH v17 0/6] spi: pxa2xx: PM fixes, teardown overhaul, and LPSS restore for MacBook8,1 Shih-Yuan Lee
                   ` (4 preceding siblings ...)
  2026-09-30 16:06 ` [PATCH v17 5/6] spi: pxa2xx-pci: restore LPSS private register state across S3 resume Shih-Yuan Lee
@ 2026-09-30 16:06 ` Shih-Yuan Lee
  5 siblings, 0 replies; 9+ messages in thread
From: Shih-Yuan Lee @ 2026-09-30 16:06 UTC (permalink / raw)
  To: Mark Brown
  Cc: Andy Shevchenko, Mika Westerberg, Lukas Wunner, Daniel Mack,
	Haojian Zhuang, Robert Jarzmik, linux-spi, linux-kernel,
	linux-arm-kernel, Shih-Yuan Lee

On MacBook8,1 (early 2015 12" MacBook), the LPSS SPI controller at
00:15.4 (PCI ID 8086:9ce6, PCI_DEVICE_ID_INTEL_LPT1_1) registers and
allocates DMA channels successfully. However, during actual SPI
transfers, the DMA handshake synchronization fails.

This leads to continuous transfer timeouts and locks up the driver
queue, making the connected keyboard and touchpad completely dead:

[ 1582.050494] spi_master spi1: failed to transfer one message from queue
[ 1582.258387] applespi spi-APP000D:00: SPI transfer timed out
[ 1582.258498] applespi spi-APP000D:00: Error reading from device: -110

Empirical hardware register inspection reveals that the companion LPSS
DMA controller at 00:15.0 has its LPSS Private Reset Register (BAR0 +
0x204) left strictly at 0x00000000 by the EFI firmware. Even after
deasserting this reset and overriding ACPI _PRT to assign shared IRQ 21,
the DMA interrupt counter remains strictly at 0, confirming that DMA
completion interrupts are not delivered on this platform. Furthermore,
the SPI controller driver at 00:15.4 cannot safely reach across the PCI
bus to manipulate private registers of the separate DMAC function at
00:15.0 without violating driver layering.

Reverse engineering of the official OS drivers on the same hardware
corroborates that DMA is deliberately bypassed:

  1. On macOS (Big Sur), IOKit registry shows that while the LPSS DMAC
     driver (AppleIntelLpssDmac) is loaded, the allocated channel count
     (AppleIntelLpssDmacChannel) is 0 when the SPI device driver
     (AppleHSSPIHIDDriver) is active.
  2. On Windows 10 (Boot Camp), dynamic MMIO monitoring of BAR1 shows
     that the LPSS DMA control registers (offset 0x800+) remain
     inactive (all zeros) during active keyboard and touchpad
     transactions, and transfers are handled solely via the FIFO data
     registers at the beginning of BAR1.
  3. Logic board schematics confirm that a dedicated out-of-band GPIO
     interrupt line (TPAD_SPI_INT_L) is routed directly to the PCH for
     input events.

Additionally, when operating in PIO mode servicing high-frequency
keyboard and touchpad input streams operating at 60-125 Hz, the default
50 ms autosuspend delay causes the controller to repeatedly transition
between D0 and D3hot between bursts of user input. These runtime resume
transitions introduce latency spikes that drop key presses and cause
erratic touchpad motion.

Implement the forced PIO mode and runtime autosuspend lockout DMI quirk
in spi-pxa2xx-pci.c (the LPSS host controller PCI glue driver) scoped
strictly to MacBook8,1:
1. Include <linux/dmi.h> in alphabetical order for DMI system matching.
2. Add 'quirks' field in struct pxa2xx_spi_pci_config and define
   PXA2XX_SPI_QUIRK_NO_AUTOSUSPEND.
3. Add pxa2xx_spi_pci_dmi_table matching Apple MacBook8,1 and query it
   once in lpss_spi_setup() scoped to PCI_DEVICE_ID_INTEL_LPT1_1
   (0x9ce6). When matched, log the system ident with pci_info(), set
   enable_dma = 0 as a workaround, cache PXA2XX_SPI_QUIRK_NO_AUTOSUSPEND
   in cfg->quirks, and return immediately to skip unused DMA channel
   setup.
4. In pxa2xx_spi_pci_probe(), configure autosuspend with a 50 ms delay
   for all devices, but conditionally skip pm_runtime_allow() when
   PXA2XX_SPI_QUIRK_NO_AUTOSUSPEND is set. This keeps the controller in
   D0 by default (as initialized by PCI core pci_pm_init()) while bound,
   eliminating D3hot resume latency spikes, while ensuring that if
   userspace explicitly enables autosuspend via sysfs, the safe 50 ms
   delay is preserved rather than defaulting to 0. Standard system
   sleep (S3) remains fully operational via pm_runtime_force_suspend().

Assisted-by: Antigravity:gemini-3.8-flash sparse
Signed-off-by: Shih-Yuan Lee <fourdollars@debian.org>
---
 drivers/spi/spi-pxa2xx-pci.c | 61 +++++++++++++++++++++++++++++++++++-
 1 file changed, 60 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-pxa2xx-pci.c b/drivers/spi/spi-pxa2xx-pci.c
index f8d71362e840..f12a8ef12b17 100644
--- a/drivers/spi/spi-pxa2xx-pci.c
+++ b/drivers/spi/spi-pxa2xx-pci.c
@@ -7,6 +7,7 @@
  */
 #include <linux/clk-provider.h>
 #include <linux/device.h>
+#include <linux/dmi.h>
 #include <linux/err.h>
 #include <linux/module.h>
 #include <linux/pci.h>
@@ -40,6 +41,8 @@
 #define LPSS_PRIV_RESETS_IDMA			BIT(2)
 #define LPSS_PRIV_REG_COUNT			9
 
+#define PXA2XX_SPI_QUIRK_NO_AUTOSUSPEND		BIT(0)
+
 struct pxa_spi_info {
 	int (*setup)(struct pci_dev *pdev, struct pxa2xx_spi_controller *c);
 };
@@ -47,6 +50,7 @@ struct pxa_spi_info {
 struct pxa2xx_spi_pci_config {
 	struct pxa2xx_spi_controller pdata;
 	u32 lpss_priv_ctx[LPSS_PRIV_REG_COUNT];
+	u8 quirks;
 	bool is_lpt;
 };
 
@@ -106,8 +110,53 @@ static void lpss_dma_put_device(void *dma_dev)
 	pci_dev_put(dma_dev);
 }
 
+/*
+ * Apple MacBook8,1 (early 2015 12" MacBook):
+ *
+ * The LPSS SPI controller at 00:15.4 connects to the internal SPI keyboard
+ * and touchpad (applespi). When DMA mode is enabled, DMA transfers consistently
+ * time out (-110) because DMA completion interrupts are never delivered.
+ *
+ * Empirical hardware inspection and OS analysis show that:
+ * 1. Hardware/firmware configuration: EFI leaves companion LPSS DMA (00:15.0)
+ *    held in reset (LPSS_PRIV_RESETS == 0x0 / BAR0 + 0x204). Even after
+ *    deasserting the reset and overriding ACPI _PRT to assign shared IRQ 21,
+ *    the DMA interrupt counter remains strictly 0, confirming that DMA
+ *    completion interrupts are not delivered on this platform.
+ * 2. Operating system parity: Both macOS (AppleIntelLpssDmac channel count
+ *    is 0) and Windows 10 Boot Camp bypass DMA and handle keyboard/touchpad
+ *    transactions strictly in PIO mode via FIFO registers.
+ * 3. Layering: The SPI controller driver (00:15.4) cannot safely reconfigure
+ *    the private registers of the separate DMAC PCI function (00:15.0).
+ * 4. Runtime autosuspend: In PIO mode servicing 60-125 Hz input events, the
+ *    default 50 ms autosuspend delay introduces D3hot <-> D0 transition
+ *    latency spikes that drop key presses and cause erratic touchpad motion.
+ *
+ * Therefore, as a workaround, disable DMA channel allocation to force PIO mode
+ * and keep the device in D0 without enabling runtime autosuspend.
+ */
+static const struct dmi_system_id pxa2xx_spi_pci_dmi_table[] = {
+	{
+		.ident = "Apple MacBook8,1",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
+			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "MacBook8,1"),
+		},
+	},
+	{ }
+};
+
+static const struct dmi_system_id *pxa2xx_spi_pci_get_dmi_id(struct pci_dev *dev)
+{
+	if (dev->device == PCI_DEVICE_ID_INTEL_LPT1_1)
+		return dmi_first_match(pxa2xx_spi_pci_dmi_table);
+
+	return NULL;
+}
+
 static int lpss_spi_setup(struct pci_dev *dev, struct pxa2xx_spi_controller *c)
 {
+	const struct dmi_system_id *dmi_id;
 	struct pxa2xx_spi_pci_config *cfg = container_of(c, struct pxa2xx_spi_pci_config, pdata);
 	struct ssp_device *ssp = &c->ssp;
 	struct dw_dma_slave *tx, *rx;
@@ -165,6 +214,14 @@ static int lpss_spi_setup(struct pci_dev *dev, struct pxa2xx_spi_controller *c)
 	if (ret)
 		return ret;
 
+	dmi_id = pxa2xx_spi_pci_get_dmi_id(dev);
+	if (dmi_id) {
+		pci_info(dev, "%s detected: disabling DMA to force PIO mode\n", dmi_id->ident);
+		c->enable_dma = 0;
+		cfg->quirks |= PXA2XX_SPI_QUIRK_NO_AUTOSUSPEND;
+		return 0;
+	}
+
 	dma_dev = pci_get_slot(dev->bus, PCI_DEVFN(PCI_SLOT(dev->devfn), 0));
 	ret = devm_add_action_or_reset(&dev->dev, lpss_dma_put_device, dma_dev);
 	if (ret)
@@ -446,7 +503,9 @@ 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 (!(cfg->quirks & PXA2XX_SPI_QUIRK_NO_AUTOSUSPEND))
+		pm_runtime_allow(&dev->dev);
 
 	return 0;
 }
-- 
2.39.5


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v17 2/6] spi: pxa2xx: introduce clock enable and disable helper functions
  2026-09-30 16:06 ` [PATCH v17 2/6] spi: pxa2xx: introduce clock enable and disable helper functions Shih-Yuan Lee
@ 2026-09-30 17:31   ` Mark Brown
  0 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2026-09-30 17:31 UTC (permalink / raw)
  To: Shih-Yuan Lee
  Cc: Andy Shevchenko, Mika Westerberg, Lukas Wunner, Daniel Mack,
	Haojian Zhuang, Robert Jarzmik, linux-spi, linux-kernel,
	linux-arm-kernel

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

On Thu, Oct 01, 2026 at 12:06:25AM +0800, Shih-Yuan Lee wrote:
> The driver enables and disables the SOC clock during probe, teardown,
> and power management callbacks. Directly calling clk_disable_unprepare()
> when the clock is already disabled—such as when removing a device that
> is runtime-suspended—causes an unbalanced clock disable warning from the
> Common Clock Framework.

> Introduce pxa2xx_spi_clk_enable() and pxa2xx_spi_clk_disable() helper
> functions that track the clock state with a 'clk_enabled' boolean flag
> protected by a 'clk_lock' mutex in struct driver_data. These helpers
> make clock toggling idempotent: repeated enable or disable invocations
> are safe no-ops serialized by clk_lock.

What problem is this solving?  Usually if something is dropping a
reference to a shared resource like a clock without knowing if it took
it then whatever else might have been using the resource is going to be
broken when the clock suddenly vanishes underneath it.  If we are
coordinating properly we shouldn't need the flag, the refcount should be
good enough.


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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v17 3/6] spi: pxa2xx: acquire active PM runtime reference in interrupt handler
  2026-09-30 16:06 ` [PATCH v17 3/6] spi: pxa2xx: acquire active PM runtime reference in interrupt handler Shih-Yuan Lee
@ 2026-09-30 17:40   ` Mark Brown
  0 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2026-09-30 17:40 UTC (permalink / raw)
  To: Shih-Yuan Lee
  Cc: Andy Shevchenko, Mika Westerberg, Lukas Wunner, Daniel Mack,
	Haojian Zhuang, Robert Jarzmik, linux-spi, linux-kernel,
	linux-arm-kernel

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

On Thu, Oct 01, 2026 at 12:06:26AM +0800, Shih-Yuan Lee wrote:
> On a shared interrupt line, ssp_int() can be invoked while the SPI
> controller is suspended or transitioning power states. In ssp_int(),
> evaluating device power state without acquiring a reference before
> accessing MMIO registers is not atomic: another thread executing
> pm_runtime_suspend() can drop the last reference and gate the clock
> immediately after the check passes. Accessing unclocked registers then
> leads to PCIe Completion Timeouts and system hangs.

> @@ -1464,22 +1481,31 @@ int pxa2xx_spi_probe(struct device *dev, struct ssp_device *ssp,
>  		}
>  	}
>  
> +	ret = request_irq(ssp->irq, ssp_int, IRQF_SHARED, dev_name(dev),
> +			drv_data);
> +	if (ret < 0) {
> +		ret = dev_err_probe(dev, ret, "cannot get IRQ %d\n", ssp->irq);
> +		goto out_error_clock_enabled;
> +	}
> +
>  	/* Register with the SPI framework */
>  	dev_set_drvdata(dev, drv_data);

We need the driver data before we request the interrupt, the interrupt
handler does runtime PM and the runtime PM operations use driver data.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-09-30 17:40 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-30 16:06 [PATCH v17 0/6] spi: pxa2xx: PM fixes, teardown overhaul, and LPSS restore for MacBook8,1 Shih-Yuan Lee
2026-09-30 16:06 ` [PATCH v17 1/6] spi: pxa2xx: rename local status variable to ret Shih-Yuan Lee
2026-09-30 16:06 ` [PATCH v17 2/6] spi: pxa2xx: introduce clock enable and disable helper functions Shih-Yuan Lee
2026-09-30 17:31   ` Mark Brown
2026-09-30 16:06 ` [PATCH v17 3/6] spi: pxa2xx: acquire active PM runtime reference in interrupt handler Shih-Yuan Lee
2026-09-30 17:40   ` Mark Brown
2026-09-30 16:06 ` [PATCH v17 4/6] spi: pxa2xx: overhaul teardown and suspend sequence to synchronize IRQ before clock gating Shih-Yuan Lee
2026-09-30 16:06 ` [PATCH v17 5/6] spi: pxa2xx-pci: restore LPSS private register state across S3 resume Shih-Yuan Lee
2026-09-30 16:06 ` [PATCH v17 6/6] spi: pxa2xx-pci: disable DMA and runtime autosuspend for Apple MacBook8,1 Shih-Yuan Lee

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®