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

[Vishwaroop A <va@nvidia.com>]

On Thu, May 21, 2026 at 08:04:08AM -0700, Breno Leitao wrote:
> If the lock is really only guarding curr_xfer / status_reg / tx_status
> / rx_status, why hold it across those register accesses at all?

Thanks for pushing on this, it's a fair question and worth spelling
out more carefully.

The lock spans the register accesses because the FIFO_STATUS read and
the tx_status / rx_status / status_reg writes form one update -- the
software fields are populated directly from that read. The bottom-half
(handle_cpu_based_xfer at line 1467) and the timeout handler (which
calls into handle_cpu_based_xfer at line 1098 after releasing its own
acquisition at line 1092) both read tx_status and rx_status under the
same lock. Splitting the lock so the read is outside would let those
callers observe partially updated software state.

The mask_clear_irq call is inside the same critical section because
it is the W1C that retires the interrupt source the ISR just consumed.
Pairing it with the FIFO_STATUS read and the software writes ensures
that any subsequent acquirer of the lock sees either "interrupt is
pending and software fields are stale" or "interrupt is retired and
software fields are current" -- never the middle state.

handle_cpu_based_xfer holds the lock across its FIFO drain and
sub-transfer restart for the same reason: the restart calls
tegra_qspi_unmask_irq, which can re-trigger the ISR before the
handler finishes setting curr_xfer = NULL at line 1497. Releasing
the lock only after that assignment lets the next ISR's critical
section start from a consistent state.

TRANS_STATUS is read outside the lock because that read precedes the
sequence -- it is the IRQF_SHARED ownership check, a single readl
with no associated software state and no RMW. The two CPU-side paths
that clear it are tegra_qspi_mask_clear_irq inside the ISR's locked
section (line 1655) and tegra_qspi_mask_clear_irq from
setup_transfer_one (line 862), which runs before the controller
unmasks its interrupts, so it cannot overlap with an ISR.

Vishwaroop