All of lore.kernel.org
 help / color / mirror / Atom feed
From: Shih-Yuan Lee <fourdollars@debian.org>
To: Mark Brown <broonie@kernel.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	Lukas Wunner <lukas@wunner.de>, Daniel Mack <daniel@zonque.org>,
	Haojian Zhuang <haojian.zhuang@gmail.com>,
	Robert Jarzmik <robert.jarzmik@free.fr>,
	linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Shih-Yuan Lee <fourdollars@debian.org>
Subject: [PATCH v17 4/6] spi: pxa2xx: overhaul teardown and suspend sequence to synchronize IRQ before clock gating
Date: Thu,  1 Oct 2026 00:06:27 +0800	[thread overview]
Message-ID: <20260930160629.1822-5-fourdollars@debian.org> (raw)
In-Reply-To: <20260930160629.1822-1-fourdollars@debian.org>

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



  parent reply	other threads:[~2026-09-30 16:07 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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-10-01  7:10   ` Andy Shevchenko
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-10-01  7:13   ` Andy Shevchenko
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 ` Shih-Yuan Lee [this message]
2026-09-30 16:06 ` [PATCH v17 5/6] spi: pxa2xx-pci: restore LPSS private register state across S3 resume Shih-Yuan Lee
2026-10-01  4:13   ` Mika Westerberg
2026-10-01  7:20   ` Andy Shevchenko
2026-09-30 16:06 ` [PATCH v17 6/6] spi: pxa2xx-pci: disable DMA and runtime autosuspend for Apple MacBook8,1 Shih-Yuan Lee

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260930160629.1822-5-fourdollars@debian.org \
    --to=fourdollars@debian.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=broonie@kernel.org \
    --cc=daniel@zonque.org \
    --cc=haojian.zhuang@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=lukas@wunner.de \
    --cc=mika.westerberg@linux.intel.com \
    --cc=robert.jarzmik@free.fr \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.