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

[Peter Collingbourne]

On Sun, May 17, 2026 at 7:02 AM David Laight
<david.laight.linux@xxxxxxxxx> wrote:
>
> 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

In include/linux/mtd/spinand.h we have:

#define SPINAND_READ_POLL_DELAY_US 5
#define SPINAND_RESET_POLL_DELAY_US 5
#define SPINAND_WRITE_POLL_DELAY_US 15
#define SPINAND_ERASE_POLL_DELAY_US 50

So we will only delay here for reads and resets.

I would generally expect drivers to arrange for spi_mem_read_status()
to sleep using wait_for_completion*() where possible, so we shouldn't
normally be actually spinning here.

For example, the generic driver:

spi_mem_read_status()
-> spi_mem_exec_op()
  -> spi_sync()
    -> __spi_sync()
      -> __spi_transfer_message_noqueue()
        -> __spi_pump_transfer_message()
          -> spi_transfer_one_message() [via ctlr->transfer_one_message]
            -> spi_transfer_wait()
              -> wait_for_completion_timeout()

For spi-mt65xx:

spi_mem_read_status()
-> spi_mem_exec_op()
  -> mtk_spi_mem_exec_op() [via ctlr->mem_ops->exec_op]
    -> wait_for_completion_timeout()

Peter