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

[Mark Brown <broonie@debian.org>]

[-- Attachment #1: Type: text/plain, Size: 1962 bytes --]

On Wed, Jun 10, 2026 at 06:24:00AM +0000, Vishwaroop A wrote:

> @@ -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)

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?

> +static void tegra_qspi_work_handler(struct work_struct *work)
>  {
> -	struct tegra_qspi *tqspi = context_data;

> +	 * Check if the timeout handler already processed this transfer.
> +	 * Can happen if the workqueue was delayed and the timeout fired
> +	 * first. In that case there is nothing to do: tegra_qspi_start_
> +	 * {cpu,dma}_based_transfer() at the start of the next transfer
> +	 * (or the next message) re-enables interrupts.
>  	 */
> -	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;
>  	}

Can't the timeout handler also be running at the same time as this
rather than already having handled things?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]