Re: [PATCH v2 1/3] spi: tegra210-quad: Convert to hard IRQ with high-priority workqueue

From: Jon Hunter

Date: Wed May 20 2026 - 05:26:09 EST




On 19/05/2026 16:51, Vishwaroop A wrote:
Threaded IRQ handlers suffer from scheduler latency on heavily loaded
systems, causing false transfer timeouts. Convert to hard IRQ handler
that schedules work on a high-priority unbound workqueue.

The hard IRQ handler verifies the interrupt, caches FIFO status,
clears and masks interrupts, then schedules bottom-half processing.
The workqueue handler runs in process context (can sleep for DMA)
and can execute on any CPU, avoiding CPU0 bottlenecks.

Signed-off-by: Vishwaroop A <va@xxxxxxxxxx>
---
drivers/spi/spi-tegra210-quad.c | 128 +++++++++++++++++++++-----------
1 file changed, 84 insertions(+), 44 deletions(-)

diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
index db28dd556484..17d0b511af1d 100644
--- a/drivers/spi/spi-tegra210-quad.c
+++ b/drivers/spi/spi-tegra210-quad.c
@@ -191,6 +191,8 @@ struct tegra_qspi {
void __iomem *base;
phys_addr_t phys;
unsigned int irq;
+ struct work_struct irq_work;
+ struct workqueue_struct *wq;
u32 cur_speed;
unsigned int cur_pos;
@@ -1225,9 +1227,9 @@ static int tegra_qspi_combined_seq_xfer(struct tegra_qspi *tqspi,
if (ret == 0) {
/*
- * Check if hardware completed the transfer
- * even though interrupt was lost or delayed.
- * If so, process the completion and continue.
+ * Check if hardware completed the transfer even though
+ * workqueue was delayed. If so, process completion and
+ * continue.
*/
ret = tegra_qspi_handle_timeout(tqspi);
if (ret < 0) {
@@ -1344,8 +1346,8 @@ static int tegra_qspi_non_combined_seq_xfer(struct tegra_qspi *tqspi,
if (ret == 0) {
/*
* Check if hardware completed the transfer even though
- * interrupt was lost or delayed. If so, process the
- * completion and continue.
+ * workqueue was delayed. If so, process completion and
+ * continue.
*/
ret = tegra_qspi_handle_timeout(tqspi);
if (ret < 0) {
@@ -1574,46 +1576,40 @@ static irqreturn_t handle_dma_based_xfer(struct tegra_qspi *tqspi)
return IRQ_HANDLED;
}
-static irqreturn_t tegra_qspi_isr_thread(int irq, void *context_data)
+/**
+ * tegra_qspi_work_handler - Workqueue handler for interrupt bottom-half
+ * @work: work_struct embedded in tegra_qspi
+ *
+ * Runs in process context and can sleep (needed for DMA completion waits).
+ * Can run on any available CPU, avoiding CPU0 bottleneck that occurs with
+ * threaded IRQ handlers which are pinned to the IRQ's CPU.
+ *
+ * The hard IRQ handler has already:
+ * - Verified this is our interrupt (QSPI_RDY was set)
+ * - Cached FIFO status in tqspi->status_reg
+ * - Parsed tx_status / rx_status from FIFO status
+ * - Masked further interrupts
+ */
+static void tegra_qspi_work_handler(struct work_struct *work)
{
- struct tegra_qspi *tqspi = context_data;
+ struct tegra_qspi *tqspi = container_of(work, struct tegra_qspi, irq_work);
unsigned long flags;
- u32 status;
- /*
- * Read transfer status to check if interrupt was triggered by transfer
- * completion
- */
- status = tegra_qspi_readl(tqspi, QSPI_TRANS_STATUS);
+ spin_lock_irqsave(&tqspi->lock, flags);
/*
- * Occasionally the IRQ thread takes a long time to wake up (usually
- * when the CPU that it's running on is excessively busy) and we have
- * already reached the timeout before and cleaned up the timed out
- * transfer. Avoid any processing in that case and bail out early.
- *
- * If no transfer is in progress, check if this was a real interrupt
- * that the timeout handler already processed, or a spurious one.
+ * Check if timeout handler already processed this transfer.
+ * Can happen if work was delayed and timeout fired first. If
+ * so, we must unmask interrupts before returning, otherwise
+ * they remain masked from the hard IRQ handler and the next
+ * transfer will timeout.
*/
- spin_lock_irqsave(&tqspi->lock, flags);
if (!tqspi->curr_xfer) {
spin_unlock_irqrestore(&tqspi->lock, flags);
- /* Spurious interrupt - transfer not ready */
- if (!(status & QSPI_RDY))
- return IRQ_NONE;
- /* Real interrupt, already handled by timeout path */
- return IRQ_HANDLED;
+ tegra_qspi_unmask_irq(tqspi);
+ return;
}
- tqspi->status_reg = tegra_qspi_readl(tqspi, QSPI_FIFO_STATUS);
-
- if (tqspi->cur_direction & DATA_DIR_TX)
- tqspi->tx_status = tqspi->status_reg & (QSPI_TX_FIFO_UNF | QSPI_TX_FIFO_OVF);
-
- if (tqspi->cur_direction & DATA_DIR_RX)
- tqspi->rx_status = tqspi->status_reg & (QSPI_RX_FIFO_OVF | QSPI_RX_FIFO_UNF);
-
- tegra_qspi_mask_clear_irq(tqspi);
spin_unlock_irqrestore(&tqspi->lock, flags);
/*
@@ -1623,9 +1619,46 @@ static irqreturn_t tegra_qspi_isr_thread(int irq, void *context_data)
* cannot be done while holding spinlock.
*/
if (!tqspi->is_curr_dma_xfer)
- return handle_cpu_based_xfer(tqspi);
+ handle_cpu_based_xfer(tqspi);
+ else
+ handle_dma_based_xfer(tqspi);
+}
+
+/**
+ * tegra_qspi_isr - Hard IRQ handler
+ * @irq: IRQ number
+ * @context_data: QSPI controller instance
+ *
+ * Runs in hard IRQ context with minimal latency. Cannot sleep.
+ *
+ * Return: IRQ_NONE if not our interrupt, IRQ_HANDLED if handled
+ */
+static irqreturn_t tegra_qspi_isr(int irq, void *context_data)
+{
+ struct tegra_qspi *tqspi = context_data;
+ u32 status;
+
+ status = tegra_qspi_readl(tqspi, QSPI_TRANS_STATUS);
+ if (!(status & QSPI_RDY))
+ return IRQ_NONE;
+
+ spin_lock(&tqspi->lock);
+ tqspi->status_reg = tegra_qspi_readl(tqspi, QSPI_FIFO_STATUS);
+ tegra_qspi_mask_clear_irq(tqspi);
- return handle_dma_based_xfer(tqspi);
+ if (tqspi->cur_direction & DATA_DIR_TX)
+ tqspi->tx_status = tqspi->status_reg &
+ (QSPI_TX_FIFO_UNF | QSPI_TX_FIFO_OVF);
+
+ if (tqspi->cur_direction & DATA_DIR_RX)
+ tqspi->rx_status = tqspi->status_reg &
+ (QSPI_RX_FIFO_OVF | QSPI_RX_FIFO_UNF);
+
+ spin_unlock(&tqspi->lock);
+
+ queue_work(tqspi->wq, &tqspi->irq_work);
+
+ return IRQ_HANDLED;
}
static struct tegra_qspi_soc_data tegra210_qspi_soc_data = {
@@ -1793,9 +1826,19 @@ static int tegra_qspi_probe(struct platform_device *pdev)
pm_runtime_put_autosuspend(&pdev->dev);
- ret = request_threaded_irq(tqspi->irq, NULL,
- tegra_qspi_isr_thread, IRQF_ONESHOT,
- dev_name(&pdev->dev), tqspi);
+ tqspi->wq = devm_alloc_workqueue(&pdev->dev, "%s",
+ WQ_HIGHPRI | WQ_UNBOUND, 0,
+ dev_name(&pdev->dev));
+ if (!tqspi->wq) {
+ dev_err(&pdev->dev, "failed to allocate workqueue\n");
+ ret = -ENOMEM;
+ goto exit_pm_disable;
+ }
+
+ INIT_WORK(&tqspi->irq_work, tegra_qspi_work_handler);
+
+ ret = devm_request_irq(&pdev->dev, tqspi->irq, tegra_qspi_isr,
+ IRQF_SHARED, dev_name(&pdev->dev), tqspi);
if (ret < 0) {
dev_err(&pdev->dev, "failed to request IRQ#%u: %d\n", tqspi->irq, ret);
goto exit_pm_disable;
@@ -1804,13 +1847,11 @@ static int tegra_qspi_probe(struct platform_device *pdev)
ret = spi_register_controller(host);
if (ret < 0) {
dev_err(&pdev->dev, "failed to register host: %d\n", ret);
- goto exit_free_irq;
+ goto exit_pm_disable;
}
return 0;
-exit_free_irq:
- free_irq(qspi_irq, tqspi);
exit_pm_disable:
pm_runtime_dont_use_autosuspend(&pdev->dev);
pm_runtime_force_suspend(&pdev->dev);
@@ -1824,7 +1865,6 @@ static void tegra_qspi_remove(struct platform_device *pdev)
struct tegra_qspi *tqspi = spi_controller_get_devdata(host);
spi_unregister_controller(host);
- free_irq(tqspi->irq, tqspi);
pm_runtime_dont_use_autosuspend(&pdev->dev);
pm_runtime_force_suspend(&pdev->dev);
tegra_qspi_deinit_dma(tqspi);

Looks like you dropped the ...

flush_workqueue(tqspi->wq);

Don't we still need this?

Jon

--
nvpublic