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 3/6] spi: pxa2xx: acquire active PM runtime reference in interrupt handler
Date: Thu, 1 Oct 2026 00:06:26 +0800 [thread overview]
Message-ID: <20260930160629.1822-4-fourdollars@debian.org> (raw)
In-Reply-To: <20260930160629.1822-1-fourdollars@debian.org>
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
next prev parent reply other threads:[~2026-09-30 16:07 UTC|newest]
Thread overview: 9+ 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-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 ` Shih-Yuan Lee [this message]
2026-09-30 17:40 ` [PATCH v17 3/6] spi: pxa2xx: acquire active PM runtime reference in interrupt handler 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
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-4-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 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®