Linux SPI subsystem development
 help / color / mirror / Atom feed
From: David Laight <david.laight.linux@gmail.com>
To: Peter Collingbourne <peter@pcc.me.uk>
Cc: Mark Brown <broonie@kernel.org>,
	Patrice Chotard <patrice.chotard@foss.st.com>,
	Boris Brezillon <boris.brezillon@collabora.com>,
	Christophe Kerello <christophe.kerello@foss.st.com>,
	linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] spi: use udelay() for short polling delays
Date: Sun, 17 May 2026 15:02:53 +0100	[thread overview]
Message-ID: <20260517150253.031dec09@pumpkin> (raw)
In-Reply-To: <20260517062554.117448-1-peter@pcc.me.uk>

On Sat, 16 May 2026 23:25:54 -0700
Peter Collingbourne <peter@pcc.me.uk> 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@pcc.me.uk>
> ---
>  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;
>  	}


  reply	other threads:[~2026-05-17 14:02 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-17  6:25 [PATCH] spi: use udelay() for short polling delays Peter Collingbourne
2026-05-17 14:02 ` David Laight [this message]
2026-05-17 23:15   ` Peter Collingbourne
2026-05-18  9:04     ` David Laight
2026-05-18 10:46 ` Mark Brown
2026-05-19 10:26   ` Peter Collingbourne

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260517150253.031dec09@pumpkin \
    --to=david.laight.linux@gmail.com \
    --cc=boris.brezillon@collabora.com \
    --cc=broonie@kernel.org \
    --cc=christophe.kerello@foss.st.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=patrice.chotard@foss.st.com \
    --cc=peter@pcc.me.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox