linux-kernel - Re: [PATCH v1 1/2] spi: tegra210-quad: Fix timeout handling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <dhbvszhpgmgxqdatbbou3z2mj5bu7vnbr7ko4ufunlycpudsml@6xnz57kowygw>
Date: Tue, 14 Oct 2025 12:15:25 +0200
From: Thierry Reding <treding@...dia.com>
To: Vishwaroop A <va@...dia.com>
Cc: Mark Brown <broonie@...nel.org>, 
	Thierry Reding <thierry.reding@...il.com>, Jonathan Hunter <jonathanh@...dia.com>, 
	Sowjanya Komatineni <skomatineni@...dia.com>, Laxman Dewangan <ldewangan@...dia.com>, smangipudi@...dia.com, 
	kyarlagadda@...dia.com, linux-spi@...r.kernel.org, linux-tegra@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 1/2] spi: tegra210-quad: Fix timeout handling

On Fri, Oct 10, 2025 at 03:20:00PM +0000, Vishwaroop A wrote:
> When the CPU that the QSPI interrupt handler runs on (typically CPU 0)
> is excessively busy, it can lead to rare cases of the IRQ thread not
> running before the transfer timeout is reached.
> 
> While handling the timeouts, any pending transfers are cleaned up and
> the message that they correspond to is marked as failed, which leaves
> the curr_xfer field pointing at stale memory.
> 
> To avoid this, clear curr_xfer to NULL upon timeout and check for this
> condition when the IRQ thread is finally run.
> 
> While at it, also make sure to clear interrupts on failure so that new
> interrupts can be run.
> 
> A better, more involved, fix would move the interrupt clearing into a
> hard IRQ handler. Ideally we would also want to signal that the IRQ
> thread no longer needs to be run after the timeout is hit to avoid the
> extra check for a valid transfer.
> 
> Fixes: 921fc1838fb0 ("spi: tegra210-quad: Add support for Tegra210 QSPI controller")
> Signed-off-by: Thierry Reding <treding@...dia.com>
> Signed-off-by: Vishwaroop A <va@...dia.com>
> ---
>  drivers/spi/spi-tegra210-quad.c | 31 ++++++++++++++++++++++++-------
>  1 file changed, 24 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
> index 3be7499db21e..c2f880d08109 100644
> --- a/drivers/spi/spi-tegra210-quad.c
> +++ b/drivers/spi/spi-tegra210-quad.c
> @@ -1024,8 +1024,10 @@ static void tegra_qspi_handle_error(struct tegra_qspi *tqspi)
>  	dev_err(tqspi->dev, "error in transfer, fifo status 0x%08x\n", tqspi->status_reg);
>  	tegra_qspi_dump_regs(tqspi);
>  	tegra_qspi_flush_fifos(tqspi, true);
> -	if (device_reset(tqspi->dev) < 0)
> +	if (device_reset(tqspi->dev) < 0) {
>  		dev_warn_once(tqspi->dev, "device reset failed\n");
> +		tegra_qspi_mask_clear_irq(tqspi);
> +	}
>  }
>  
>  static void tegra_qspi_transfer_end(struct spi_device *spi)
> @@ -1175,12 +1177,14 @@ static int tegra_qspi_combined_seq_xfer(struct tegra_qspi *tqspi,
>  							  QSPI_DMA_CTL);
>  				}
>  
> -				/* Reset controller if timeout happens */
> -				if (device_reset(tqspi->dev) < 0)
> -					dev_warn_once(tqspi->dev,
> -						      "device reset failed\n");
> -				ret = -EIO;
> -				goto exit;
> +			/* Reset controller if timeout happens */
> +			if (device_reset(tqspi->dev) < 0) {
> +				dev_warn_once(tqspi->dev,
> +					      "device reset failed\n");
> +				tegra_qspi_mask_clear_irq(tqspi);
> +			}
> +			ret = -EIO;
> +			goto exit;

Vishwaroop,

It looks like somehow this hunk got messed up when you applied the patch
to your tree. The entire block above needs to remain indented like it
was. The only addition was the curly brackets and the call to
tegra_qspi_mask_clear_irq().

Thierry

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)