[PATCH v6 0/3] spi: tegra210-quad: Improve interrupt handling for loaded systems

[PATCH v6 0/3] spi: tegra210-quad: Improve interrupt handling for loaded systems

[Vishwaroop A]

The current threaded IRQ implementation in spi-tegra210-quad suffers
from scheduler-induced latency on heavily loaded systems. The old
irq_thread() runs SCHED_FIFO but is pinned by the kernel to the IRQ
affinity mask (typically one CPU); when that CPU is saturated by RT
workloads (e.g. NCCL multicast) or by an SPI transaction coming from
a higher-priority context, the sleeping DMA/PIO wait inside the IRQ
thread cannot progress and wait_for_completion_timeout() in
transfer_one_message expires - even though the QSPI hardware
finished on time. This results in false timeout errors and WARN_ON
splats during normal operation.

This series addresses the problem in three steps:

1. Convert the threaded IRQ handler to a hard IRQ + high-priority
   unbound workqueue model. The hard IRQ does the minimum: capture
   FIFO status, mask and clear the controller IRQ, then schedule the
   bottom half. The workqueue handler runs in process context (can
   sleep for DMA completion) and runs on any CPU in the WQ_UNBOUND
   pool, so the bottom half can migrate off the interrupt-taking CPU
   that the previous threaded IRQ pinned to via
   set_cpus_allowed_ptr(irq_affinity).

2. Cache QSPI_TRANS_STATUS in the ISR before clearing it. This lets
   the timeout handler distinguish between a real hardware timeout
   (QSPI_RDY not set) and a delayed workqueue (QSPI_RDY set),
   preventing false timeout errors when hardware has already
   completed. Pair the cache publication with
   smp_store_release()/smp_load_acquire() so the timeout handler
   observes a coherent set of cached fields on weakly-ordered
   architectures. In v6 the timeout handler is additionally serialised
   with the workqueue via cancel_work_sync() and only runs the manual
   completion fallback on the last chunk of a transfer (see "Changes
   since v5" below for the multi-chunk DMA race Mark identified).

3. Process small PIO transfers (those that complete the whole
   spi_transfer in a single chunk) directly in hard IRQ context,
   eliminating workqueue scheduling latency for TPM-style short reads.

Runtime PM lifetime note (unchanged from v4): the work handler only
touches QSPI MMIO when curr_xfer is non-NULL. While curr_xfer is set,
the transfer thread is blocked in wait_for_completion_timeout() with
the SPI core's runtime PM reference held, so the clocks are
guaranteed on. When the work handler runs late after the timeout path
has already processed the transfer, it sees curr_xfer == NULL and
returns without any MMIO. With this invariant no additional PM
reference handoff between the ISR and the work handler is needed.

Changes since v5 (addressing review by Mark Brown [1][2]):

  Patch 1 ("Convert to hard IRQ with high-priority workqueue"):
    - Rewrote the commit message to be honest about the priority
      trade-off. WQ_HIGHPRI worker runs SCHED_NORMAL at nice -20
      (HIGHPRI_NICE_LEVEL = MIN_NICE in kernel/workqueue.c) whereas
      irq_thread() runs SCHED_FIFO. A real-time userspace task will
      therefore preempt the bottom half where it would not have
      preempted the old irq_thread. The changelog now spells out
      what is gained in exchange - hard-IRQ status latching that is
      independent of the process scheduler, WQ_UNBOUND worker
      migration off the interrupt-taking CPU that irq_thread() could
      not leave, and the small-PIO fastpath (patch 3) that removes
      the worker from the latency-sensitive TPM path entirely.
      Addresses Mark's "The high priority workqueue is a SCHED_NORMAL
      task with priority -20 so it can be starved by real time tasks
      running at SCHED_FIFO" review comment.

      The wording explicitly notes that the hard IRQ can still be
      delayed by higher-priority IRQ handling or local IRQ-disabled /
      non-preemptible sections; it is not subject to the process
      scheduler, but it is not literally immediate either.

    - Removed the reference to cancel_work_sync() from the
      tegra_qspi_work_handler() comment in this patch. That call is
      introduced in patch 2, so the v5 comment described behaviour
      that did not exist at patch 1's bisect point. Patch 2 now
      updates the same comment to describe the serialisation once
      cancel_work_sync() is actually in place. Addresses Mark's
      "That's actually in patch 2 so we have a bisect issue here"
      comment.

    - Reworded the tegra_qspi_work_handler() kernel-doc: threaded
      IRQ pins to the IRQ affinity mask (not necessarily CPU0).

  Patch 2 ("Cache TRANS_STATUS in ISR for timeout handler"):
    - Rewrote tegra_qspi_handle_timeout() to close the multi-chunk
      DMA race Mark identified. After cancel_work_sync() drains the
      bottom half, try_wait_for_completion(&xfer_completion) is
      called first; a full-transfer completion signalled by the
      work handler during the drain is consumed and the handler
      returns 0. Otherwise the fallback that invokes
      handle_{cpu,dma}_based_xfer() from process context is now
      gated on "is the current chunk the last chunk of the
      transfer?", computed under tqspi->lock as

          cur_pos + curr_dma_words * bytes_per_word >= t->len

      which is the same expression tegra_qspi_start_cpu_based_
      transfer() already uses to set is_last_pio_chunk. On an
      intermediate chunk of a multi-chunk DMA transfer the work
      handler may have processed the current chunk and armed the
      next chunk (unmasked the IRQ and kicked HW) before
      cancel_work_sync() returned; running another handler from
      handle_timeout() there would let seq_xfer() clear curr_xfer
      and finalise the message while the DMA engine is still
      moving the next chunk into the client buffer. Multi-chunk
      continuation timeouts now return -ETIMEDOUT and the caller's
      existing dma_stop() + reset() path cleans up. Addresses
      Mark's "handle_dma_based_xfer() starts a new DMA after the
      current one completes if there's more work to do but it
      looks like _combined_seq_xfer() will clear curr_xfer if we
      didn't get an error from handling the timeout" review
      comment.

    - Added a recovery_in_progress guard. tegra_qspi_handle_timeout()
      publishes recovery_in_progress under tqspi->lock; the ISR
      checks it under the same lock and skips both the small-PIO
      fastpath dispatch and queue_work() while recovery runs. Both
      dispatch decisions inside the ISR now happen while still
      holding tqspi->lock (queue_work() is safe to call from
      spinlock context), so the guard is atomic with the dispatch
      decision. Any ISR that had already released the lock and is
      about to run its fastpath / queue_work() is drained by
      synchronize_irq(tqspi->irq), which handle_timeout() calls
      immediately after masking the controller IRQ. This closes the
      residual re-enqueue race where a running worker armed the next
      chunk during the drain, unmasked the controller, and let a
      subsequent RDY IRQ enqueue a fresh worker after
      cancel_work_sync() returned.

    - Reordered the ISR so trans_status is published via
      smp_store_release() *before* tegra_qspi_mask_clear_irq()
      clears QSPI_TRANS_STATUS in hardware. In the previous ordering
      a timeout handler on another CPU that saw the cache still zero
      (release not yet visible) and fell back to a live
      QSPI_TRANS_STATUS read could observe the hardware bit already
      cleared, reporting a false timeout on a transfer that had in
      fact just completed. Publishing the cache first keeps the
      "cache miss -> live read" fallback consistent: the live read
      still sees QSPI_RDY until the cache is visible.

    - Re-mask and synchronize_irq() at the exit of
      tegra_qspi_handle_timeout() before clearing
      recovery_in_progress. The drained worker may have unmasked the
      controller IRQ when arming the next chunk of a multi-chunk
      transfer; without re-masking here a lingering RDY IRQ that
      arrives after this function returns could enter the ISR after
      recovery_in_progress has been cleared, queue a fresh worker
      and race the caller's dma_stop() + device_reset() + curr_xfer
      clear path.

    - handle_timeout() now enters recovery unconditionally rather
      than returning -ETIMEDOUT before serialising against the ISR
      and worker. Every expired wait_for_completion_timeout() masks
      the controller IRQ, synchronize_irq()s to drain any in-flight
      hard IRQ (including the small-PIO fastpath), and
      cancel_work_sync()s the workqueue before deciding whether the
      hardware finished. The RDY classification runs *after* this
      quiesce, so a genuine hardware timeout still ends up as
      -ETIMEDOUT but the caller's dma_stop() + device_reset() +
      curr_xfer clear no longer races a delayed ISR that fires
      immediately after the entry status sample.

    - Added a cache-live-cache retry to the entry trans_status
      classification. The initial smp_load_acquire() may miss a
      concurrent smp_store_release() from an ISR on another CPU; if
      the subsequent live QSPI_TRANS_STATUS read also returns zero
      (because the ISR W1C'd it between our two loads), a second
      cache load observes the now-visible release. Without this
      retry the timeout path could report -ETIMEDOUT on a transfer
      that actually completed but whose cache publication was still
      in flight.

    - Added a lost-IRQ FIFO snapshot. When the ISR cache is empty
      but the live QSPI_TRANS_STATUS shows RDY (a genuine lost or
      severely delayed IRQ), snapshot QSPI_FIFO_STATUS *before*
      tegra_qspi_mask_clear_irq() W1Cs the FIFO error bits, then
      publish that snapshot into tqspi->{status_reg,tx_status,
      rx_status} after the drain. Without this the manual final-
      chunk handler would operate on stale tx_status / rx_status
      fields from a previous chunk's ISR.

    - Updated the tegra_qspi_handle_timeout() kernel-doc to
      describe the last-chunk gate and the reason multi-chunk
      continuation timeouts must surface -ETIMEDOUT.

    - Updated the tegra_qspi_work_handler() comment to describe the
      cancel_work_sync() + recovery_in_progress serialisation (now
      in this patch, where both live).

    - Corrected the cancel_work_sync() description in the
      handle_timeout() comment and commit message: cancel_work_sync()
      cancels a pending worker without executing it and waits for a
      currently running worker to finish (v5 wording incorrectly
      said "executes a pending work synchronously").

    - Changed the is_curr_dma_xfer read in handle_timeout() to
      READ_ONCE(), matching the WRITE_ONCE() used in the writers.
      Consistency cleanup inside the function this patch is
      already touching.

  Patch 3 ("Process small PIO transfers in hard IRQ context"):
    - The small-PIO fastpath is now dispatched while still holding
      tqspi->lock (the lock is dropped only immediately before the
      call to handle_cpu_based_xfer(), which takes the lock
      internally). This puts the fastpath decision under the same
      recovery_in_progress guard as queue_work() in patch 2, so
      tegra_qspi_handle_timeout() cannot race a hard-IRQ fastpath
      run: either the ISR observes recovery_in_progress == true
      under the lock and returns early, or it commits to the
      fastpath dispatch before handle_timeout() can proceed past
      synchronize_irq(). No functional change to the fastpath
      itself.

Changes since v4 (addressing review by Mark Brown [3]):

  Patch 1 ("Convert to hard IRQ with high-priority workqueue"):
    - Rewrote the tegra_qspi_work_handler() comment to describe the
      serialisation invariant honestly (superseded by patch-1 fixes
      in v6 above; the current wording lives in patch 2 where
      cancel_work_sync() exists). Addresses Mark's "Can't the
      timeout handler also be running at the same time as this?"
      comment on v4.
    - Converted the tegra_qspi_isr() header comment to a proper
      kernel-doc block, with @irq / @context_data / Return: fields.
      No functional change.

  Patch 2 ("Cache TRANS_STATUS in ISR for timeout handler"):
    - Serialise tegra_qspi_handle_timeout() against the workqueue.
      Mask the controller IRQ (tegra_qspi_mask_clear_irq()) and then
      cancel_work_sync(&tqspi->irq_work) at the top of the recovery
      path. Once cancel_work_sync() returns the bottom half is
      neither running nor pending. Addresses Mark's "This can be
      called from both tegra_qspi_work_handler() and
      tegra_qspi_handle_timeout() - I can't see what stops them
      both handling and completing the same transfer simultaneously?"
      v4 review comment.
    - Clear the cached trans_status per chunk. Add
      smp_store_release(&trans_status, 0) immediately before
      tegra_qspi_unmask_irq() in both tegra_qspi_start_cpu_based_
      transfer() and tegra_qspi_start_dma_based_transfer(), so a
      multi-chunk DMA transfer (or the DMA -> PIO tail-chunk
      transition) cannot leave a stale RDY from chunk N in the cache
      when chunk N+1's completion times out. Addresses Mark's "It
      looks like the CPU based transfer function supports multiple
      interrupts per transfer ... don't we need to clear
      trans_status when we handle the interrupt as well?" v4
      review comment.
    - Take tqspi->lock across the ISR's status snapshot and cache
      publish sequence, and move tegra_qspi_mask_clear_irq() inside
      the locked region. Closes the QSPI_INTR_MASK RMW race Mark
      had already flagged on v3.

  Patch 3 ("Process small PIO transfers in hard IRQ context"):
    - Comment-only reflows to match the surrounding text.

Changes since v3 (addressing review by Mark Brown):

  Patch 1 ("Convert to hard IRQ with high-priority workqueue"):
    - Dropped IRQF_SHARED. Tegra QSPI uses a dedicated GIC SPI line
      on every SoC that uses this driver, so the ISR does not need
      a runtime PM reference. Addresses Mark's "Since we now have
      IRQF_SHARED we need to take a runtime PM reference here"
      comment.
    - Switched from devm_request_irq() to plain request_irq() in
      probe() and added explicit free_irq() in remove(), in the
      order: spi_unregister_controller -> free_irq ->
      destroy_workqueue -> pm_runtime_dont_use_autosuspend ->
      pm_runtime_force_suspend -> tegra_qspi_deinit_dma. Addresses
      Mark's "devm + non-devm mix seems likely to be racy"
      comments on probe() and remove().
    - Removed the tegra_qspi_unmask_irq() call from the
      work_handler NULL-bail path. Addresses Mark's "unmask after
      dropping the lock feels like it opens up races" comment.
    - Snapshot tqspi->curr_xfer under tqspi->lock at the top of
      handle_dma_based_xfer() so the DMA waits operate on a stable
      transfer pointer even if the timeout path clears curr_xfer
      concurrently. Integrates cleanly with Breno Leitao's
      recently merged protect-curr_xfer series.

  Patch 2 ("Cache TRANS_STATUS in ISR for timeout handler"):
    - Cached status_reg / tx_status / rx_status / trans_status are
      now published with WRITE_ONCE() and smp_store_release() and
      consumed with smp_load_acquire() in
      tegra_qspi_handle_timeout(). Live MMIO fallback via
      tegra_qspi_readl() only runs when the cache is still zero
      (the ISR never ran).

  Patch 3 ("Process small PIO transfers in hard IRQ context"):
    - Replaced the previous "curr_dma_words <= QSPI_FIFO_DEPTH"
      check with a tqspi->is_last_pio_chunk scalar computed in
      tegra_qspi_start_cpu_based_transfer() before it unmasks the
      IRQ. Addresses Mark's "Is cur_dma_words always in the same
      units as QSPI_FIFO_DEPTH - I see there's packed transfer
      support in the driver?" comment.
    - Fastpath additionally gates on tx_status == 0 &&
      rx_status == 0 because handle_cpu_based_xfer()'s error path
      calls tegra_qspi_handle_error() -> device_reset(), which can
      sleep and must not run from hard IRQ context.
    - is_curr_dma_xfer and is_last_pio_chunk are written from
      process context and read lock-free from the hard IRQ handler
      and the workqueue handler, so the writers use WRITE_ONCE()
      and the readers use READ_ONCE().

Changes since v2:
  - Added cancel_work_sync() in remove to flush pending work
    before devm tore down the workqueue (Jon Hunter). v4 has
    replaced devm altogether per Mark Brown's comment, so the
    explicit teardown now relies on free_irq() preventing new
    work being queued, followed by destroy_workqueue() draining
    what is in-flight.
  - Rewrote patch 2 commit message to describe the race in terms
    of the workqueue model rather than referencing the old
    threaded IRQ (Jon).
  - s/NULLed/cleared/ in code comment (Jon).

Changes since v1:
  - Switched to devm_alloc_workqueue() and devm_request_irq() for
    resource management (Jon Hunter). v4 has since reverted to
    non-devm for the IRQ and workqueue per Mark Brown's review,
    so teardown order can be made explicit.
  - Improved patch 2 commit message to explain the timeout race
    scenario and clarify that the issue pre-exists the workqueue
    conversion (Jon).
  - Removed unnecessary local variable in tegra_qspi_handle_timeout
    (Jon).
  - Moved "workqueue was delayed" comment updates from patch 2 to
    patch 1, since patch 1 introduces the workqueue (Jon).

The series is based on linux-next (next-20260810).

[1] https://lore.kernel.org/linux-spi/bbc6a709-7c83-4866-8905-39ab4d8b3b77@sirena.org.uk/
[2] https://lore.kernel.org/linux-spi/c1100cde-ffe9-42b7-9f21-827cae7248e7@sirena.org.uk/
[3] https://lore.kernel.org/linux-spi/20260610062400.1502354-1-va@nvidia.com/

Vishwaroop A (3):
  spi: tegra210-quad: Convert to hard IRQ with high-priority workqueue
  spi: tegra210-quad: Cache TRANS_STATUS in ISR for timeout handler
  spi: tegra210-quad: Process small PIO transfers in hard IRQ context

 drivers/spi/spi-tegra210-quad.c | 509 +++++++++++++++++++++++++++-----
 1 file changed, 441 insertions(+), 68 deletions(-)

-- 
2.17.1

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

[Vishwaroop A]

Threaded IRQ handlers can be delayed by the scheduler on heavily loaded
systems, causing wait_for_completion_timeout() to expire before the
handler runs and producing false transfer timeouts. On GB200 with TPM
hwrng traffic running alongside a NCCL multicast workload this shows up
as a WARN in tegra_qspi_transfer_one_message() even though the hardware
has already signalled QSPI_RDY.

irq_thread() runs SCHED_FIFO but set_cpus_allowed_ptr()s to the IRQ
affinity mask (typically a single CPU). When that CPU is saturated by
non-preemptible kernel work on the same interrupt line (softirqs,
spinlock contention, network RX processing), the FIFO priority alone
does not help - there is nothing at lower priority to preempt. The
bottom half sits on the runqueue for milliseconds and occasionally
seconds.

Convert to a hard IRQ handler that schedules work on a WQ_HIGHPRI |
WQ_UNBOUND workqueue:

  - The hard IRQ handler runs outside process-scheduler control - it
    can still be delayed by higher-priority IRQ handling or local
    IRQ-disabled / non-preemptible sections, but not by CFS or
    RT-userspace backpressure. tegra_qspi_isr() captures FIFO / trans
    status and masks the controller IRQ synchronously with the
    hardware event, so the subsequent timeout classification (added
    in the following patch) always sees the true state.

  - The workqueue worker runs SCHED_NORMAL with HIGHPRI_NICE_LEVEL
    (nice -20). A real-time SCHED_FIFO userspace task will preempt it
    where it would not have preempted the old irq_thread; that is a
    real trade-off. In exchange, WQ_UNBOUND lets the worker migrate
    off the interrupt-taking CPU that the threaded IRQ could not
    leave, which is the actual failure mode observed in the field.
    The following patch (small-PIO fastpath) further removes the
    worker from the latency-sensitive TPM path entirely.

The work handler only touches QSPI MMIO when curr_xfer is non-NULL.
curr_xfer is cleared only after the transfer thread has processed the
completion, and while it is set the transfer thread is blocked in
wait_for_completion_timeout() with the SPI core's runtime PM reference
held, so the clocks are guaranteed on.

The ISR returns IRQ_HANDLED unconditionally. Tegra QSPI has a
dedicated, non-shared GIC SPI line on every SoC that uses this driver,
so any spurious / late IRQ (for example after the timeout path has
cleared curr_xfer) must still be acked and re-masked here; otherwise
the level-triggered line could stay asserted and trip the kernel
spurious-IRQ detector into disabling the line ("nobody cared, try to
disable"). The lock-free curr_xfer NULL check lets the ISR bail
without touching FIFO / status when there is no transfer to drive
forward.

handle_dma_based_xfer() snapshots curr_xfer under the spinlock at
function entry and bails immediately when the timeout path has already
cleared it. This avoids waiting up to QSPI_DMA_TIMEOUT on a DMA
completion that belongs to a transfer the synchronous path has already
torn down, and keeps the subsequent dma_unmap / FIFO-drain operations
consistent with the transfer that actually started.

Resources are allocated and torn down manually so that remove() can
stop the controller, free the IRQ (preventing new work from being
queued), then destroy the workqueue (which drains any already-queued
work while the clocks are still on) before runtime PM is disabled.

Signed-off-by: Vishwaroop A <va@nvidia.com>
---
 drivers/spi/spi-tegra210-quad.c | 161 ++++++++++++++++++++++----------
 1 file changed, 113 insertions(+), 48 deletions(-)

diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
index 8ede864c3d3c..7c09a1fe0d41 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;
@@ -1232,9 +1234,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) {
@@ -1351,8 +1353,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) {
@@ -1506,6 +1508,19 @@ static irqreturn_t handle_dma_based_xfer(struct tegra_qspi *tqspi)
 	long wait_status;
 	int num_errors = 0;
 
+	/*
+	 * Snapshot curr_xfer under the lock before the (potentially long)
+	 * DMA waits below. The timeout path can clear tqspi->curr_xfer
+	 * concurrently; using the local copy keeps the subsequent dma_unmap
+	 * and FIFO-drain steps consistent with the transfer that actually
+	 * started, and lets us bail safely if cleanup already happened.
+	 */
+	spin_lock_irqsave(&tqspi->lock, flags);
+	t = tqspi->curr_xfer;
+	spin_unlock_irqrestore(&tqspi->lock, flags);
+	if (!t)
+		return IRQ_HANDLED;
+
 	if (tqspi->cur_direction & DATA_DIR_TX) {
 		if (tqspi->tx_status) {
 			if (tqspi->tx_dma_chan)
@@ -1539,12 +1554,6 @@ static irqreturn_t handle_dma_based_xfer(struct tegra_qspi *tqspi)
 	}
 
 	spin_lock_irqsave(&tqspi->lock, flags);
-	t = tqspi->curr_xfer;
-
-	if (!t) {
-		spin_unlock_irqrestore(&tqspi->lock, flags);
-		return IRQ_HANDLED;
-	}
 
 	if (num_errors) {
 		tegra_qspi_dma_unmap_xfer(tqspi, t);
@@ -1581,46 +1590,38 @@ 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).
+ * Runs on any CPU in the WQ_UNBOUND pool, so the bottom half can migrate off
+ * the interrupt-taking CPU that the previous threaded IRQ pinned to
+ * (irq_thread() calls set_cpus_allowed_ptr() with the IRQ affinity mask).
+ *
+ * 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.
+	 * The timeout path can clear curr_xfer between the ISR queuing
+	 * this work and the worker actually running, so re-check under
+	 * the lock and bail if there is nothing to do.
 	 */
-	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;
+		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);
 
 	/*
@@ -1630,9 +1631,55 @@ 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.
+ *
+ * Tegra QSPI uses a dedicated, non-shared GIC SPI line on every SoC that
+ * uses this driver. The handler always returns IRQ_HANDLED and always
+ * acknowledges/re-masks the controller IRQ, so the level-triggered line
+ * cannot stay asserted and trip the kernel spurious-IRQ detector into
+ * disabling the line. On a stray IRQ where curr_xfer is NULL (e.g. the
+ * timeout path has already torn the transfer down) the FIFO/status
+ * processing and bottom-half scheduling are skipped because there is no
+ * transfer to drive forward.
+ *
+ * Return: IRQ_HANDLED.
+ */
+static irqreturn_t tegra_qspi_isr(int irq, void *context_data)
+{
+	struct tegra_qspi *tqspi = context_data;
+
+	if (!READ_ONCE(tqspi->curr_xfer)) {
+		tegra_qspi_mask_clear_irq(tqspi);
+		return IRQ_HANDLED;
+	}
+
+	spin_lock(&tqspi->lock);
+	tqspi->status_reg = tegra_qspi_readl(tqspi, QSPI_FIFO_STATUS);
+	tegra_qspi_mask_clear_irq(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);
 
-	return handle_dma_based_xfer(tqspi);
+	queue_work(tqspi->wq, &tqspi->irq_work);
+
+	return IRQ_HANDLED;
 }
 
 static struct tegra_qspi_soc_data tegra210_qspi_soc_data = {
@@ -1800,12 +1847,21 @@ 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 = alloc_workqueue("%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 = request_irq(tqspi->irq, tegra_qspi_isr, 0,
+			  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;
+		goto exit_destroy_wq;
 	}
 
 	ret = spi_register_controller(host);
@@ -1817,7 +1873,9 @@ static int tegra_qspi_probe(struct platform_device *pdev)
 	return 0;
 
 exit_free_irq:
-	free_irq(qspi_irq, tqspi);
+	free_irq(tqspi->irq, tqspi);
+exit_destroy_wq:
+	destroy_workqueue(tqspi->wq);
 exit_pm_disable:
 	pm_runtime_dont_use_autosuspend(&pdev->dev);
 	pm_runtime_force_suspend(&pdev->dev);
@@ -1830,8 +1888,15 @@ static void tegra_qspi_remove(struct platform_device *pdev)
 	struct spi_controller *host = platform_get_drvdata(pdev);
 	struct tegra_qspi *tqspi = spi_controller_get_devdata(host);
 
+	/*
+	 * Tear down in reverse order of probe() so that the controller stops
+	 * accepting transfers before the IRQ is released, no new work can be
+	 * queued after the IRQ is freed, and any work already queued is
+	 * drained while the clocks are still running.
+	 */
 	spi_unregister_controller(host);
 	free_irq(tqspi->irq, tqspi);
+	destroy_workqueue(tqspi->wq);
 	pm_runtime_dont_use_autosuspend(&pdev->dev);
 	pm_runtime_force_suspend(&pdev->dev);
 	tegra_qspi_deinit_dma(tqspi);
-- 
2.17.1

[PATCH v6 2/3] spi: tegra210-quad: Cache TRANS_STATUS in ISR for timeout handler

[Vishwaroop A]

On heavily loaded systems the workqueue bottom half can be delayed
long enough for wait_for_completion_timeout() to expire before the
ISR's queued work actually runs. Reading QSPI_TRANS_STATUS directly
from the controller in the timeout handler races with both the
workqueue handler and the controller itself, and can mis-classify a
transfer that genuinely timed out as having "completed".

Cache the controller status captured by the hard IRQ before it is
acked, and let the timeout handler consume that cache:

  - tegra_qspi_isr() reads QSPI_FIFO_STATUS and QSPI_TRANS_STATUS,
    derives tx_status / rx_status, publishes them via WRITE_ONCE(),
    and then publishes the trans_status cache via
    smp_store_release() *before* masking and acking the controller
    IRQ. Publish-before-clear is required so that a timeout handler
    that fell back to a live QSPI_TRANS_STATUS read (because it saw
    the cache still zero on another CPU) also sees the hardware
    RDY bit that has not been cleared yet.

  - tegra_qspi_handle_timeout() consumes trans_status with a paired
    smp_load_acquire() and a cache-live-cache retry pattern. If the
    initial cache load returns zero, the handler reads the live
    QSPI_TRANS_STATUS register; if that also returns zero it retries
    the cache once more. That closes the interleaving where an ISR
    publishes trans_status with release semantics and then W1Cs the
    hardware between the timeout handler's cache load and its live
    load, otherwise leaving the timeout handler with cache = 0 and
    HW = 0 (a false timeout on a transfer that has in fact just
    completed).

  - tegra_qspi_setup_transfer_one() and both
    tegra_qspi_start_{cpu,dma}_based_transfer() paths clear the cache
    with smp_store_release() under the spinlock before unmasking the
    IRQ for the new chunk, so a stale RDY bit from a previous chunk
    of a multi-chunk transfer cannot fool the handler.

Serialise handle_timeout with the workqueue and the ISR
unconditionally. Every expired wait_for_completion_timeout() enters
the recovery state: publish recovery_in_progress under tqspi->lock,
mask the controller IRQ, synchronize_irq() to drain any in-flight
hard IRQ (including the small-PIO fastpath), and cancel_work_sync()
to drain the workqueue. This holds regardless of whether the
hardware finished, because a genuine hardware timeout still races the
caller's dma_stop() + device_reset() + curr_xfer clear against a
delayed ISR or worker that arrives immediately after the status
sample. Classifying the timeout as -ETIMEDOUT only *after*
serialisation gives the caller a stable state to clean up.

Snapshot the live FIFO error status *before* entering recovery when
the ISR cache is empty and the live QSPI_TRANS_STATUS shows RDY (the
lost-IRQ path). tegra_qspi_mask_clear_irq() W1Cs QSPI_TRANS_STATUS
and the QSPI_FIFO_STATUS error bits, so a lost-IRQ recovery that
called it first would erase the very error state the manual handler
downstream needs to see. Capturing the snapshot before the mask and
publishing it into tqspi->{status_reg,tx_status,rx_status} after the
drain keeps the manual final-chunk handler operating on fresh error
data rather than stale fields from an earlier ISR run.

cancel_work_sync() cancels a pending worker without executing it and
waits for a currently running one to finish. The recovery_in_progress
guard is checked inside tegra_qspi_isr() under the same tqspi->lock
as its queue_work() and small-PIO fastpath dispatch decisions, so no
new bottom-half work is enqueued once we publish the flag.
synchronize_irq() closes the window where an ISR observed
recovery_in_progress == false, released the lock, and is about to
call queue_work(): we wait for that ISR to finish before draining
the workqueue, so its queued work is caught by cancel_work_sync().

After the drain, re-check the cache once more (the drained worker
may have published a completion status the entry snapshot did not
observe). If try_wait_for_completion() reports the whole transfer
completed, return success.

Restrict the manual fallback that invokes handle_{cpu,dma}_based_xfer()
from process context to the last chunk of a transfer. On an
intermediate chunk of a multi-chunk DMA transfer the work handler
may have processed the current chunk and armed the next chunk
(unmasked the IRQ and kicked HW) before cancel_work_sync() returned;
running another handler here would let the caller's seq_xfer clear
curr_xfer and finalise the message while the DMA engine is still
moving the next chunk into the client buffer. Return -ETIMEDOUT in
that case and let the caller's existing dma_stop() + reset() path
clean up.

Before returning, re-mask the controller IRQ and synchronize_irq()
one more time. The drained bottom half may have unmasked the IRQ
when arming a subsequent chunk; without the re-mask a lingering
RDY IRQ that arrives after this function returns could invoke the
ISR and queue a worker after recovery_in_progress has been cleared,
racing the caller's cleanup of curr_xfer.

Signed-off-by: Vishwaroop A <va@nvidia.com>
---
 drivers/spi/spi-tegra210-quad.c | 294 +++++++++++++++++++++++++++++---
 1 file changed, 266 insertions(+), 28 deletions(-)

diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
index 7c09a1fe0d41..c242f56a09fd 100644
--- a/drivers/spi/spi-tegra210-quad.c
+++ b/drivers/spi/spi-tegra210-quad.c
@@ -193,6 +193,13 @@ struct tegra_qspi {
 	unsigned int				irq;
 	struct work_struct			irq_work;
 	struct workqueue_struct			*wq;
+	/*
+	 * Set by tegra_qspi_handle_timeout() while it drains the bottom
+	 * half so tegra_qspi_isr() suppresses new queue_work() calls
+	 * that would otherwise race the recovery path or the caller's
+	 * cleanup of curr_xfer.
+	 */
+	bool					recovery_in_progress;
 
 	u32					cur_speed;
 	unsigned int				cur_pos;
@@ -214,6 +221,7 @@ struct tegra_qspi {
 	u32					tx_status;
 	u32					rx_status;
 	u32					status_reg;
+	u32					trans_status;
 	bool					is_packed;
 	bool					use_dma;
 
@@ -624,6 +632,17 @@ static int tegra_qspi_start_dma_based_transfer(struct tegra_qspi *tqspi, struct
 	val = QSPI_DMA_BLK_SET(tqspi->curr_dma_words - 1);
 	tegra_qspi_writel(tqspi, val, QSPI_DMA_BLK);
 
+	/*
+	 * Reset the cached transfer status before unmasking the IRQ for
+	 * this chunk. The cache must represent only the IRQ for THIS
+	 * chunk; a stale RDY from the previous chunk of a multi-chunk
+	 * transfer would otherwise mislead tegra_qspi_handle_timeout()
+	 * into a false-positive recovery while the new chunk is still in
+	 * flight. Pairs with smp_load_acquire() in
+	 * tegra_qspi_handle_timeout(). The new chunk's IRQ cannot fire
+	 * until QSPI_DMA_CTL is written below.
+	 */
+	smp_store_release(&tqspi->trans_status, 0);
 	tegra_qspi_unmask_irq(tqspi);
 
 	if (tqspi->is_packed)
@@ -736,6 +755,16 @@ static int tegra_qspi_start_cpu_based_transfer(struct tegra_qspi *qspi, struct s
 	val = QSPI_DMA_BLK_SET(cur_words - 1);
 	tegra_qspi_writel(qspi, val, QSPI_DMA_BLK);
 
+	/*
+	 * Reset the cached transfer status before unmasking the IRQ for
+	 * this chunk so the cache represents only the IRQ for THIS chunk;
+	 * a stale RDY from the previous chunk would otherwise mislead
+	 * tegra_qspi_handle_timeout() into a false-positive recovery
+	 * while the new chunk is still in flight. Pairs with
+	 * smp_load_acquire() in tegra_qspi_handle_timeout(). The new
+	 * chunk's IRQ cannot fire until QSPI_COMMAND1 is written below.
+	 */
+	smp_store_release(&qspi->trans_status, 0);
 	tegra_qspi_unmask_irq(qspi);
 
 	qspi->is_curr_dma_xfer = false;
@@ -861,6 +890,13 @@ static u32 tegra_qspi_setup_transfer_one(struct spi_device *spi, struct spi_tran
 	tqspi->cur_rx_pos = 0;
 	tqspi->cur_tx_pos = 0;
 	tqspi->curr_xfer = t;
+	/*
+	 * Pairs with smp_load_acquire() in tegra_qspi_handle_timeout().
+	 * Clearing the cached trans_status before unmasking the IRQ for
+	 * the new transfer prevents a stale RDY bit from the previous
+	 * transfer fooling the timeout handler into a false recovery.
+	 */
+	smp_store_release(&tqspi->trans_status, 0);
 	spin_unlock_irqrestore(&tqspi->lock, flags);
 
 	if (is_first_of_msg) {
@@ -1067,40 +1103,206 @@ static irqreturn_t handle_dma_based_xfer(struct tegra_qspi *tqspi);
  * tegra_qspi_handle_timeout - Handle transfer timeout with hardware check
  * @tqspi: QSPI controller instance
  *
- * When a timeout occurs but hardware has completed the transfer (interrupt
- * was lost or delayed), manually trigger transfer completion processing.
- * This avoids failing transfers that actually succeeded.
+ * When wait_for_completion_timeout() expires the hardware may still have
+ * finished the current chunk. Drain the pending bottom half and, if the
+ * whole transfer really did complete during the drain, consume the
+ * completion and report success.
+ *
+ * When the bottom half advanced the transfer by only one chunk of a
+ * multi-chunk DMA/PIO transfer without signalling xfer_completion, a
+ * fallback that ran handle_{cpu,dma}_based_xfer() here would race with
+ * the DMA engine already moving the next chunk into the client buffer
+ * (spi_finalize_current_message() would then release the buffer while
+ * the controller is still writing memory). Fake completion is therefore
+ * only attempted when the current chunk is the last chunk of the
+ * transfer; multi-chunk continuation timeouts return -ETIMEDOUT and
+ * let the caller reset the controller.
  *
- * Returns: 0 if transfer was completed, -ETIMEDOUT if real timeout
+ * Returns: 0 if the transfer completed, -ETIMEDOUT otherwise.
  */
 static int tegra_qspi_handle_timeout(struct tegra_qspi *tqspi)
 {
+	struct spi_transfer *t;
+	unsigned long flags;
+	bool is_last_chunk;
+	bool lost_irq_snapshot = false;
 	irqreturn_t ret;
-	u32 status;
+	int retval;
+	u32 status, refreshed;
+	u32 lost_fifo_status = 0;
+	u32 lost_tx_status = 0;
+	u32 lost_rx_status = 0;
 
-	/* Check if hardware actually completed the transfer */
-	status = tegra_qspi_readl(tqspi, QSPI_TRANS_STATUS);
-	if (!(status & QSPI_RDY))
-		return -ETIMEDOUT;
+	/*
+	 * Snapshot both the ISR cache and (if the cache is empty) the
+	 * live status registers BEFORE entering recovery. The recovery
+	 * path calls tegra_qspi_mask_clear_irq() below, which performs
+	 * W1Cs on QSPI_TRANS_STATUS and on the QSPI_FIFO_STATUS error
+	 * bits: a lost-IRQ recovery must capture the current FIFO error
+	 * state before the mask erases it.
+	 *
+	 * Cache-live-cache retry: if the initial cache load returns zero
+	 * we fall back to a live QSPI_TRANS_STATUS read, and if that also
+	 * returns zero we retry the cache once more. That closes the
+	 * interleaving where an ISR on another CPU publishes trans_status
+	 * with release semantics and then W1Cs the hardware between our
+	 * cache load and our live load: the second cache load observes
+	 * the now-visible release and we correctly classify the transfer
+	 * as complete rather than reporting a false timeout.
+	 *
+	 * The trans_status cache is reset to zero in
+	 * tegra_qspi_start_{cpu,dma}_based_transfer() before unmasking
+	 * the IRQ for every chunk, so a stale RDY from the previous
+	 * chunk of a multi-chunk transfer cannot survive into this
+	 * check.
+	 */
+	status = smp_load_acquire(&tqspi->trans_status);
+	if (!status) {
+		status = tegra_qspi_readl(tqspi, QSPI_TRANS_STATUS);
+		if (!status) {
+			/* Retry cache; pairs with release in ISR post-store. */
+			status = smp_load_acquire(&tqspi->trans_status);
+		} else {
+			/*
+			 * Live register shows RDY but the ISR cache is
+			 * empty: either the ISR ran and cleared HW between
+			 * our two loads (the cache retry above would have
+			 * observed it, so we would not be here), or the IRQ
+			 * was genuinely lost. Snapshot the live FIFO error
+			 * status now so tegra_qspi_mask_clear_irq() below
+			 * does not W1C it away before the manual handler
+			 * downstream can see it.
+			 */
+			lost_fifo_status = tegra_qspi_readl(tqspi,
+							    QSPI_FIFO_STATUS);
+			lost_tx_status = lost_fifo_status &
+					 (QSPI_TX_FIFO_UNF | QSPI_TX_FIFO_OVF);
+			lost_rx_status = lost_fifo_status &
+					 (QSPI_RX_FIFO_OVF | QSPI_RX_FIFO_UNF);
+			lost_irq_snapshot = true;
+		}
+	}
 
 	/*
-	 * Hardware completed but interrupt was lost/delayed. Manually
-	 * process the completion by calling the appropriate handler.
+	 * Enter recovery unconditionally. Every expired
+	 * wait_for_completion_timeout() must serialise against a delayed
+	 * ISR or worker before the caller runs dma_stop() +
+	 * device_reset() + curr_xfer clear: publishing
+	 * recovery_in_progress under tqspi->lock, masking the controller
+	 * IRQ, calling synchronize_irq() to drain any in-flight ISR
+	 * (including the small-PIO hard-IRQ fastpath), and finally
+	 * cancel_work_sync() to drain the workqueue gives us that
+	 * serialisation regardless of whether the hardware finished. A
+	 * genuine hardware timeout still ends up as -ETIMEDOUT further
+	 * down, but only after ISR and workqueue activity are quiesced.
+	 *
+	 * cancel_work_sync() cancels a pending worker without executing
+	 * it and waits for a currently running one to finish; the
+	 * recovery_in_progress guard checked inside tegra_qspi_isr()
+	 * under tqspi->lock is atomic with its queue_work() and small-PIO
+	 * fastpath dispatch decisions, so no new bottom-half work is
+	 * enqueued once we publish the flag.
+	 *
+	 * tegra_qspi_mask_clear_irq() is idempotent: its read-modify-write
+	 * of QSPI_INTR_MASK and W1C of QSPI_TRANS_STATUS / FIFO error
+	 * status all tolerate a double-write, so it is safe whether or
+	 * not the ISR has already run for this transfer.
 	 */
+	spin_lock_irqsave(&tqspi->lock, flags);
+	WRITE_ONCE(tqspi->recovery_in_progress, true);
+	spin_unlock_irqrestore(&tqspi->lock, flags);
+
+	tegra_qspi_mask_clear_irq(tqspi);
+	synchronize_irq(tqspi->irq);
+	cancel_work_sync(&tqspi->irq_work);
+
+	if (try_wait_for_completion(&tqspi->xfer_completion)) {
+		retval = 0;
+		goto out;
+	}
+
+	/*
+	 * Re-check the cache after the drain: the worker we just drained
+	 * may have published a completion status the entry snapshot did
+	 * not observe (for example the ISR fired on another CPU after we
+	 * loaded the cache but before we masked).
+	 */
+	refreshed = smp_load_acquire(&tqspi->trans_status);
+	if (refreshed)
+		status = refreshed;
+
+	if (!(status & QSPI_RDY)) {
+		retval = -ETIMEDOUT;
+		goto out;
+	}
+
+	/*
+	 * If the ISR never ran (lost IRQ path) publish the FIFO error
+	 * snapshot we captured before mask_clear_irq() so the manual
+	 * handler downstream has fresh error state rather than stale
+	 * fields from a previous chunk's ISR.
+	 */
+	if (lost_irq_snapshot) {
+		WRITE_ONCE(tqspi->status_reg, lost_fifo_status);
+		WRITE_ONCE(tqspi->tx_status, lost_tx_status);
+		WRITE_ONCE(tqspi->rx_status, lost_rx_status);
+	}
+
+	/*
+	 * The bottom half did not signal full completion. Either the work
+	 * ran and advanced the transfer by one chunk (possibly arming the
+	 * next chunk of a multi-chunk transfer), or it was cancelled
+	 * before it could run, or the current chunk really did not
+	 * complete. Only fake completion when the current chunk is the
+	 * last chunk of the transfer; otherwise the DMA engine may still
+	 * be moving the next chunk into memory, and returning 0 here would
+	 * let seq_xfer clear curr_xfer and finalise the message while the
+	 * hardware is still writing.
+	 *
+	 * The last-chunk arithmetic mirrors tegra_qspi_start_cpu_based_
+	 * transfer(), which uses cur_pos + curr_dma_words * bytes_per_word
+	 * >= t->len to set is_last_pio_chunk before arming the IRQ.
+	 */
+	spin_lock_irqsave(&tqspi->lock, flags);
+	t = tqspi->curr_xfer;
+	if (!t) {
+		/* CPU-path handler already cleared curr_xfer */
+		spin_unlock_irqrestore(&tqspi->lock, flags);
+		retval = 0;
+		goto out;
+	}
+	is_last_chunk = (tqspi->cur_pos +
+			 tqspi->curr_dma_words * tqspi->bytes_per_word) >= t->len;
+	spin_unlock_irqrestore(&tqspi->lock, flags);
+
+	if (!is_last_chunk) {
+		retval = -ETIMEDOUT;
+		goto out;
+	}
+
 	dev_warn_ratelimited(tqspi->dev,
 			     "QSPI interrupt timeout, but transfer complete\n");
 
-	/* Clear the transfer status */
-	status = tegra_qspi_readl(tqspi, QSPI_TRANS_STATUS);
-	tegra_qspi_writel(tqspi, status, QSPI_TRANS_STATUS);
-
-	/* Manually trigger completion handler */
-	if (!tqspi->is_curr_dma_xfer)
+	if (!READ_ONCE(tqspi->is_curr_dma_xfer))
 		ret = handle_cpu_based_xfer(tqspi);
 	else
 		ret = handle_dma_based_xfer(tqspi);
 
-	return (ret == IRQ_HANDLED) ? 0 : -EIO;
+	retval = (ret == IRQ_HANDLED) ? 0 : -EIO;
+
+out:
+	/*
+	 * The drained bottom half may have unmasked the controller IRQ
+	 * to arm the next chunk of a multi-chunk transfer. Re-mask and
+	 * synchronize before clearing recovery_in_progress so that no
+	 * lingering ISR can queue fresh work behind the caller's back
+	 * (the caller's dma_stop() + device_reset() + curr_xfer clear
+	 * runs immediately after we return on the error path).
+	 */
+	tegra_qspi_mask_clear_irq(tqspi);
+	synchronize_irq(tqspi->irq);
+	WRITE_ONCE(tqspi->recovery_in_progress, false);
+	return retval;
 }
 
 static u32 tegra_qspi_cmd_config(bool is_ddr, u8 bus_width, u8 len)
@@ -1613,9 +1815,12 @@ static void tegra_qspi_work_handler(struct work_struct *work)
 	spin_lock_irqsave(&tqspi->lock, flags);
 
 	/*
-	 * The timeout path can clear curr_xfer between the ISR queuing
-	 * this work and the worker actually running, so re-check under
-	 * the lock and bail if there is nothing to do.
+	 * tegra_qspi_handle_timeout() sets recovery_in_progress under
+	 * tqspi->lock and then calls cancel_work_sync(), so any running
+	 * worker is drained and tegra_qspi_isr() cannot enqueue a new
+	 * one while recovery runs. The curr_xfer NULL check catches the
+	 * case where the timeout path already tore the transfer down
+	 * before this work got a chance to run.
 	 */
 	if (!tqspi->curr_xfer) {
 		spin_unlock_irqrestore(&tqspi->lock, flags);
@@ -1657,6 +1862,7 @@ static void tegra_qspi_work_handler(struct work_struct *work)
 static irqreturn_t tegra_qspi_isr(int irq, void *context_data)
 {
 	struct tegra_qspi *tqspi = context_data;
+	u32 status_reg, trans_status;
 
 	if (!READ_ONCE(tqspi->curr_xfer)) {
 		tegra_qspi_mask_clear_irq(tqspi);
@@ -1664,21 +1870,53 @@ static irqreturn_t tegra_qspi_isr(int irq, void *context_data)
 	}
 
 	spin_lock(&tqspi->lock);
-	tqspi->status_reg = tegra_qspi_readl(tqspi, QSPI_FIFO_STATUS);
-	tegra_qspi_mask_clear_irq(tqspi);
+	status_reg = tegra_qspi_readl(tqspi, QSPI_FIFO_STATUS);
+	trans_status = tegra_qspi_readl(tqspi, QSPI_TRANS_STATUS);
 
 	if (tqspi->cur_direction & DATA_DIR_TX)
-		tqspi->tx_status = tqspi->status_reg &
-				    (QSPI_TX_FIFO_UNF | QSPI_TX_FIFO_OVF);
+		WRITE_ONCE(tqspi->tx_status,
+			   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);
+		WRITE_ONCE(tqspi->rx_status,
+			   status_reg & (QSPI_RX_FIFO_OVF | QSPI_RX_FIFO_UNF));
 
-	spin_unlock(&tqspi->lock);
+	WRITE_ONCE(tqspi->status_reg, status_reg);
+	/*
+	 * Publish trans_status with release semantics before we clear
+	 * the hardware status in tegra_qspi_mask_clear_irq() below. That
+	 * ordering matters for the lock-free cache read in
+	 * tegra_qspi_handle_timeout(): if the timeout path sees the
+	 * released trans_status it also observes the matching status_reg
+	 * / tx_status / rx_status; if it does not yet see the released
+	 * value it falls back to a live QSPI_TRANS_STATUS read, and that
+	 * live read still returns QSPI_RDY because we have not cleared
+	 * the register yet. Reversing this order would open a window
+	 * where the cache is still zero but the hardware bit has already
+	 * been cleared, making the fallback report a false timeout.
+	 */
+	smp_store_release(&tqspi->trans_status, trans_status);
+
+	tegra_qspi_mask_clear_irq(tqspi);
+
+	/*
+	 * If tegra_qspi_handle_timeout() is draining the bottom half,
+	 * skip queueing new work. The flag is set under tqspi->lock and
+	 * queue_work() below happens while we still hold the lock, so
+	 * the guard is atomic with the queue decision. Any ISR that had
+	 * already passed this check is drained by the synchronize_irq()
+	 * call that tegra_qspi_handle_timeout() issues after publishing
+	 * the flag.
+	 */
+	if (READ_ONCE(tqspi->recovery_in_progress)) {
+		spin_unlock(&tqspi->lock);
+		return IRQ_HANDLED;
+	}
 
 	queue_work(tqspi->wq, &tqspi->irq_work);
 
+	spin_unlock(&tqspi->lock);
+
 	return IRQ_HANDLED;
 }
 
-- 
2.17.1

[PATCH v6 3/3] spi: tegra210-quad: Process small PIO transfers in hard IRQ context

[Vishwaroop A]

On heavily loaded systems, workqueue scheduling delays can exceed
transfer timeouts even for high-priority queues, causing false
timeouts for latency-sensitive devices like TPM despite hardware
completing in microseconds.

Process small PIO transfers (those that complete the whole spi_transfer
in a single chunk) directly in hard IRQ context instead of deferring to
the workqueue. This reduces completion latency from 1000ms+ to
microseconds and matches the pattern used by other SPI drivers.

To avoid touching the spi_transfer object from hard IRQ context (which
would race with the synchronous teardown path that clears curr_xfer on
timeout), tegra_qspi_start_cpu_based_transfer() caches the "this PIO
chunk completes the whole transfer" decision into a scalar
tqspi->is_last_pio_chunk *before* unmasking the IRQ. The hard-IRQ
fastpath consumes that scalar with READ_ONCE() and never dereferences
curr_xfer or any spi_transfer fields. Multi-chunk PIO transfers are
intentionally kept on the workqueue (only the final chunk sets the
flag) so the fastpath can never recurse into
tegra_qspi_start_cpu_based_transfer() from hard IRQ context, and DMA
transfers always go through the workqueue because their completion
path sleeps on the DMA engine.

The fastpath also gates on the per-IRQ tx_status / rx_status locals
being zero, because handle_cpu_based_xfer()'s error path calls
tegra_qspi_reset() -> device_reset(), which can sleep and must not run
from hard IRQ context.

is_curr_dma_xfer and is_last_pio_chunk are written from process
context (the transfer-start functions) and read lock-free from the
hard IRQ handler and the workqueue handler, so the writes use
WRITE_ONCE() and the reads use READ_ONCE() to prevent compiler tearing
and silence KCSAN data-race warnings.

Signed-off-by: Vishwaroop A <va@nvidia.com>
---
 drivers/spi/spi-tegra210-quad.c | 88 +++++++++++++++++++++++++++++----
 1 file changed, 79 insertions(+), 9 deletions(-)

diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
index c242f56a09fd..d8cca3aa276b 100644
--- a/drivers/spi/spi-tegra210-quad.c
+++ b/drivers/spi/spi-tegra210-quad.c
@@ -214,6 +214,18 @@ struct tegra_qspi {
 	unsigned int				dma_buf_size;
 	unsigned int				max_buf_size;
 	bool					is_curr_dma_xfer;
+	/*
+	 * Cached "this PIO chunk completes the whole transfer" decision,
+	 * computed by tegra_qspi_start_cpu_based_transfer() before it
+	 * unmasks the IRQ. Used by the hard IRQ small-PIO fastpath in
+	 * place of dereferencing curr_xfer->len, so the ISR cannot touch
+	 * the spi_transfer object even on a late IRQ that races with the
+	 * synchronous teardown path. Multi-chunk PIO transfers always go
+	 * through the workqueue (this flag is only set on the final
+	 * chunk), so the fastpath cannot recurse into
+	 * tegra_qspi_start_cpu_based_transfer() from hard IRQ context.
+	 */
+	bool					is_last_pio_chunk;
 
 	struct completion			rx_dma_complete;
 	struct completion			tx_dma_complete;
@@ -734,7 +746,13 @@ static int tegra_qspi_start_dma_based_transfer(struct tegra_qspi *tqspi, struct
 
 	tegra_qspi_writel(tqspi, tqspi->command1_reg, QSPI_COMMAND1);
 
-	tqspi->is_curr_dma_xfer = true;
+	/*
+	 * WRITE_ONCE() pairs with READ_ONCE() in tegra_qspi_isr() and
+	 * tegra_qspi_work_handler(); the flag is read lock-free across
+	 * the hard-IRQ / process-context boundary so the annotation
+	 * prevents compiler tearing and silences KCSAN.
+	 */
+	WRITE_ONCE(tqspi->is_curr_dma_xfer, true);
 	tqspi->dma_control_reg = val;
 	val |= QSPI_DMA_EN;
 	tegra_qspi_writel(tqspi, val, QSPI_DMA_CTL);
@@ -755,6 +773,20 @@ static int tegra_qspi_start_cpu_based_transfer(struct tegra_qspi *qspi, struct s
 	val = QSPI_DMA_BLK_SET(cur_words - 1);
 	tegra_qspi_writel(qspi, val, QSPI_DMA_BLK);
 
+	/*
+	 * Snapshot whether this PIO chunk completes the whole transfer
+	 * before unmasking the IRQ, so the hard IRQ small-PIO fastpath
+	 * can decide whether to drain inline without dereferencing the
+	 * spi_transfer object. cur_pos / curr_dma_words / bytes_per_word
+	 * are stable here: they are written by
+	 * tegra_qspi_calculate_curr_xfer_param() earlier in this code
+	 * path. The IRQ cannot fire until the QSPI_COMMAND1 write below
+	 * kicks the transfer off, so this store happens-before any ISR
+	 * that observes the unmask.
+	 */
+	WRITE_ONCE(qspi->is_last_pio_chunk,
+		   qspi->cur_pos + qspi->curr_dma_words * qspi->bytes_per_word >= t->len);
+
 	/*
 	 * Reset the cached transfer status before unmasking the IRQ for
 	 * this chunk so the cache represents only the IRQ for THIS chunk;
@@ -767,7 +799,7 @@ static int tegra_qspi_start_cpu_based_transfer(struct tegra_qspi *qspi, struct s
 	smp_store_release(&qspi->trans_status, 0);
 	tegra_qspi_unmask_irq(qspi);
 
-	qspi->is_curr_dma_xfer = false;
+	WRITE_ONCE(qspi->is_curr_dma_xfer, false);
 	val = qspi->command1_reg;
 	val |= QSPI_PIO;
 	tegra_qspi_writel(qspi, val, QSPI_COMMAND1);
@@ -1835,7 +1867,7 @@ static void tegra_qspi_work_handler(struct work_struct *work)
 	 * DMA handler also needs to sleep in wait_for_completion_*(), which
 	 * cannot be done while holding spinlock.
 	 */
-	if (!tqspi->is_curr_dma_xfer)
+	if (!READ_ONCE(tqspi->is_curr_dma_xfer))
 		handle_cpu_based_xfer(tqspi);
 	else
 		handle_dma_based_xfer(tqspi);
@@ -1863,6 +1895,7 @@ static irqreturn_t tegra_qspi_isr(int irq, void *context_data)
 {
 	struct tegra_qspi *tqspi = context_data;
 	u32 status_reg, trans_status;
+	u32 tx_status = 0, rx_status = 0;
 
 	if (!READ_ONCE(tqspi->curr_xfer)) {
 		tegra_qspi_mask_clear_irq(tqspi);
@@ -1873,13 +1906,15 @@ static irqreturn_t tegra_qspi_isr(int irq, void *context_data)
 	status_reg = tegra_qspi_readl(tqspi, QSPI_FIFO_STATUS);
 	trans_status = tegra_qspi_readl(tqspi, QSPI_TRANS_STATUS);
 
-	if (tqspi->cur_direction & DATA_DIR_TX)
-		WRITE_ONCE(tqspi->tx_status,
-			   status_reg & (QSPI_TX_FIFO_UNF | QSPI_TX_FIFO_OVF));
+	if (tqspi->cur_direction & DATA_DIR_TX) {
+		tx_status = status_reg & (QSPI_TX_FIFO_UNF | QSPI_TX_FIFO_OVF);
+		WRITE_ONCE(tqspi->tx_status, tx_status);
+	}
 
-	if (tqspi->cur_direction & DATA_DIR_RX)
-		WRITE_ONCE(tqspi->rx_status,
-			   status_reg & (QSPI_RX_FIFO_OVF | QSPI_RX_FIFO_UNF));
+	if (tqspi->cur_direction & DATA_DIR_RX) {
+		rx_status = status_reg & (QSPI_RX_FIFO_OVF | QSPI_RX_FIFO_UNF);
+		WRITE_ONCE(tqspi->rx_status, rx_status);
+	}
 
 	WRITE_ONCE(tqspi->status_reg, status_reg);
 	/*
@@ -1913,6 +1948,41 @@ static irqreturn_t tegra_qspi_isr(int irq, void *context_data)
 		return IRQ_HANDLED;
 	}
 
+	/*
+	 * Small-PIO fastpath: drain the FIFO inline only when this chunk
+	 * completes the entire outstanding transfer and no error bit was
+	 * latched, to avoid workqueue scheduling latency for TPM-style
+	 * short reads.
+	 *
+	 * The "last chunk" decision is computed and cached as a scalar by
+	 * tegra_qspi_start_cpu_based_transfer() before it unmasks the IRQ,
+	 * so the hard-IRQ fastpath never dereferences the spi_transfer
+	 * pointer here. That keeps the ISR safe against any teardown race
+	 * where the synchronous path could clear curr_xfer concurrently.
+	 *
+	 * The fastpath dispatch decision is made while still holding
+	 * tqspi->lock, so the recovery_in_progress guard above covers it
+	 * atomically with queue_work() below: an ISR that reaches the
+	 * fastpath cannot race a tegra_qspi_handle_timeout() that
+	 * subsequently observes recovery_in_progress == true, because
+	 * that path calls synchronize_irq() before proceeding. We drop
+	 * the lock before calling handle_cpu_based_xfer() so it can take
+	 * tqspi->lock internally without deadlocking.
+	 *
+	 * Multi-chunk PIO continuation stays on the workqueue so that
+	 * tegra_qspi_start_cpu_based_transfer() can re-arm the IRQ from
+	 * process context. DMA transfers also stay on the workqueue
+	 * because their completion path sleeps on the DMA engine.
+	 * tegra_qspi_handle_error() -> device_reset() can sleep, so the
+	 * fastpath only runs when both status words are clean.
+	 */
+	if (!READ_ONCE(tqspi->is_curr_dma_xfer) &&
+	    READ_ONCE(tqspi->is_last_pio_chunk) &&
+	    !tx_status && !rx_status) {
+		spin_unlock(&tqspi->lock);
+		return handle_cpu_based_xfer(tqspi);
+	}
+
 	queue_work(tqspi->wq, &tqspi->irq_work);
 
 	spin_unlock(&tqspi->lock);
-- 
2.17.1