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

[Vishwaroop A <va@nvidia.com>]

On Wed, May 20, 2026 at 08:25:23AM -0700, Breno Leitao wrote:
> > +     status = tegra_qspi_readl(tqspi, QSPI_TRANS_STATUS);
> > +     if (!(status & QSPI_RDY))
> > +             return IRQ_NONE;
> > +
> > +     spin_lock(&tqspi->lock);
>
> Can you help me to understand what the tqspi->lock protects? I am still
> a bit confused by this lock, but at the first glance, I am wondering if
> you don't need to have the lock while reading the status.

Good question. The QSPI_TRANS_STATUS read before the lock is a hardware
register read (MMIO) used as the IRQF_SHARED ownership check -- we read
QSPI_RDY to determine if this interrupt belongs to us. If not, we
return IRQ_NONE immediately. Taking the lock before this check would
serialize against every unrelated interrupt on the shared line for no
benefit, since we're reading hardware state that no software path can
modify concurrently.

The only CPU-side write to QSPI_TRANS_STATUS is the write-1-to-clear
inside tegra_qspi_mask_clear_irq(), which happens under the lock at
line 1655, after we've already confirmed the interrupt is ours. The
register is also cleared at the start of each new message in
tegra_qspi_setup_transfer_one() -> tegra_qspi_mask_clear_irq(), but
that runs before the transfer is started and interrupts are unmasked,
so there's no overlap with the ISR.

The lock itself protects the software state that is shared between
the ISR, the workqueue bottom-half, and the timeout handler running
in the transfer thread. Specifically, curr_xfer is read and NULLed
by handle_cpu_based_xfer/handle_dma_based_xfer under the lock, and
checked by the timeout handler under the lock, so concurrent access
is serialized. The ISR writes status_reg, tx_status, and rx_status
under the lock before scheduling the workqueue; the workqueue and
transfer thread read them after the scheduling barrier or completion
respectively, so the lock provides the write-side ordering.

The trans_status caching (tqspi->trans_status = status) before the
lock is safe because the ISR is the sole writer in interrupt context.
The only reader is handle_timeout, which reads it under
spin_lock_irqsave after wait_for_completion_timeout expires. The
setup path resets it to zero under the lock before starting each
transfer, well before the ISR can fire.

Vishwaroop