Re: [PATCH] spi: use udelay() for short polling delays

[David Laight]

On Sat, 16 May 2026 23:25:54 -0700
Peter Collingbourne <peter@xxxxxxxxx> wrote:

> A short polling delay, such as the delay of 5us
> (SPINAND_READ_POLL_DELAY_US) provided by the SPI NAND driver,
> can become a 1/HZ (order of ms) delay caused by the usleep_range()
> call in read_poll_timeout(), significantly reducing SPI NAND access
> performance. Fix it by implementing the polling delay with udelay()
> (via read_poll_timeout_atomic()) if it is short enough, matching how
> the initial delay is handled.
> 
> Fixes: c955a0cc8a28 ("spi: spi-mem: add automatic poll status functions")
> Signed-off-by: Peter Collingbourne <peter@xxxxxxxxx>
> ---
>  drivers/spi/spi-mem.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
> index a09371a075d2..914e35e51cea 100644
> --- a/drivers/spi/spi-mem.c
> +++ b/drivers/spi/spi-mem.c
> @@ -1005,10 +1005,19 @@ int spi_mem_poll_status(struct spi_mem *mem,
>  			usleep_range((initial_delay_us >> 2) + 1,
>  				     initial_delay_us);
>  
> -		ret = read_poll_timeout(spi_mem_read_status, read_status_ret,
> -					(read_status_ret || ((status) & mask) == match),
> -					polling_delay_us, timeout_ms * 1000, false, mem,
> -					op, &status);
> +		if (polling_delay_us < 10)
> +			ret = read_poll_timeout_atomic(
> +				spi_mem_read_status, read_status_ret,
> +				(read_status_ret || ((status)&mask) == match),
> +				polling_delay_us, timeout_ms * 1000, false, mem,
> +				op, &status);
> +		else
> +			ret = read_poll_timeout(
> +				spi_mem_read_status, read_status_ret,
> +				(read_status_ret || ((status)&mask) == match),
> +				polling_delay_us, timeout_ms * 1000, false, mem,
> +				op, &status);
> +

That looks rather sub-optional.
Even if the interval is short you want to drop to sleeps if the device isn't
responding.
(Or this code is used for erases and you are doing a full device erase.)

I doubt you want to spin for more than 1ms.

-David


>  		if (read_status_ret)
>  			return read_status_ret;
>  	}