[PATCH] arm: atmel_qspi: fix race condition in transfer completion check

[Eugen Hristev]

Hello Ramin,

Thank you for your patch.

Can you please change the subject to have "spi: atmel_qspi: ..."

On 7/13/25 19:08, Ramin Moussavi wrote:
> In atmel_qspi_transfer(), the status register is polled with:
> 
>   imr = QSPI_SR_INSTRE | QSPI_SR_CSR;
>   return readl_poll_timeout(aq->regs + QSPI_SR, sr,
>                             (sr & imr) == imr,
>                             ATMEL_QSPI_TIMEOUT);
> 
> However, this is racy: QSPI_SR_INSTRE can be set before QSPI_SR_CSR,
> and will then be cleared by the read. If that happens, the condition
> "(sr & imr) == imr" can never be true, and the function times out.
> 
> This race condition is avoided in at91bootstrap by accumulating the
> status bits across reads until both bits have been observed:
> 
>   /* Poll INSTruction End and Chip Select Rise flags. */
>   imr = (QSPI_SR_INSTRE | QSPI_SR_CSR);
>   sr = 0;
>   do {
>     udelay(1);
>     sr |= qspi_readl(qspi, QSPI_SR) & imr;
>   } while ((--timeout) && (sr != imr));
> 
> Update U-Boot's atmel_qspi_transfer() to use the same pattern,
> ensuring that both flags are observed even if they are not set
> simultaneously.
> 
> Signed-off-by: Ramin Seyed-Mousavi <lordrasmus at gmail.com>
> ---
>  drivers/spi/atmel-quadspi.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
> index 8aa7a83aef4..fba32876f8f 100644
> --- a/drivers/spi/atmel-quadspi.c
> +++ b/drivers/spi/atmel-quadspi.c
> @@ -616,6 +616,8 @@ static int atmel_qspi_transfer(struct atmel_qspi *aq,
>  			       const struct spi_mem_op *op, u32 offset)
>  {
>  	u32 sr, imr;
> +	uint32_t val = 0;
> +	unsigned long timeout;
>  
>  	/* Skip to the final steps if there is no data */
>  	if (op->data.nbytes) {
> @@ -636,8 +638,19 @@ static int atmel_qspi_transfer(struct atmel_qspi *aq,
>  
>  	/* Poll INSTruction End and Chip Select Rise flags. */
>  	imr = QSPI_SR_INSTRE | QSPI_SR_CSR;
> -	return readl_poll_timeout(aq->regs + QSPI_SR, sr, (sr & imr) == imr,
> -				  ATMEL_QSPI_TIMEOUT);
> +
> +	timeout = timer_get_us() + ATMEL_QSPI_TIMEOUT;
> +	while(1){

There is a missing space, can you please run checkpatch tool and fix the
warnings given ?

> +		val |= readl( aq->regs + QSPI_SR ) & imr;
> +		if( ( val & imr ) == imr ){

Remove unnecessary brackets please, also spacing is odd

> +			return 0;
> +		}
> +
> +		if ( time_after(timer_get_us(), timeout)) {
Same here
> +			return -ETIMEDOUT;
> +		}
> +	}
> +
>  }
>  
>  static int atmel_qspi_sama7g5_set_cfg(struct atmel_qspi *aq,


Eugen