From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f54.google.com (mail-wr1-f54.google.com [209.85.221.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E6A1D3E559F for ; Mon, 18 May 2026 09:04:53 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.54 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779095095; cv=none; b=q2xeTk6QSqcTNL1TLNKGVo/EAeEx2difRgzpr4TxI0poCjyLw8Y0SGiZPBoMBxz1S0yLay+l3FuIeWQkt6N5trHN14hIwF51LMiElKUvaR4/dlgvNCtDzaTuu5K6JhjDXJrl2QmuMC18WFsSHExaZQ1WNrGXdDYElwSbguYmwZE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779095095; c=relaxed/simple; bh=uakw6O/aSxeiq7FiH6COxnqTbF2HQ/jSmQP7xiHYsq0=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=ohk9+mjHrJuvOZ2SSggeWQ9aqxwTX3NCSUBBARsxOEBxMCPrZfZ4IIWG6J10JphqGkJYQbeMCdkVRwXXPcvs4HmnAOxSZvUTxJ1B0XhZcjXH92wVsmJ0dhqdTNypAMHWcBnE4tQo6hl/7ySw/PGEmduHEI0dySqyl0OwrF27EnY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=MBj6heJd; arc=none smtp.client-ip=209.85.221.54 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="MBj6heJd" Received: by mail-wr1-f54.google.com with SMTP id ffacd0b85a97d-449d6c68ed8so1865253f8f.0 for ; Mon, 18 May 2026 02:04:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1779095092; x=1779699892; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=gw/wuvCTs9a0Y5MJYOGwOreCV9s8UlS12seGF2T/0X0=; b=MBj6heJd1S6iZWzcrb4f1OoZuqGnFWRUNc0B+w3ZYVCmu1848XRrELNEXwfN8VE99f 3pqbUot4ky/AjSf/nykFKYeIz8kt8So1MeixNAbQCqYDZfVy7hYVM0gFPlOw/O8SbXWV IlRNSOjGP0ehsqYeFO2UeZz+r4G45iO9tvOlNM6ZQbnfQH0VWTbEIAPyWaYTBgqbDiYX yKPgPDVWLd80FpOGbGj06oHFYwig+Rd4ZiqSIY6zYt3DioekkFD8uuUcBQoPtLsUZz8a C+/ckFlFZEWLIPwioPs6CM+GqN7PUJdPBVeePfi/T/U2VPC1yxb79u1z1gG/g6Ktfdgf Lsfg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1779095092; x=1779699892; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=gw/wuvCTs9a0Y5MJYOGwOreCV9s8UlS12seGF2T/0X0=; b=AeTfn4Vqax3OWCLV4DyRdEy2d7gj69+bn3ZF33Y2053vuL4LKVrMyjXtE8S+VOBZEG Fvs5ki44zfSrJPyX4fPaVyz6eTq3rqgLo5P0t4CCbT5UsYKHeDTuHihHaTzUVO6/qiTz hgqTyfi3zD45xAoX1rF0uuqi+VLAacOXlkAPO4fL3rE0CUI2QI1KjKJSBQAvRQuVFFcZ A1lsmpkLeIuLi3/lZn9fYrOpwhh4lhBjJxUAX/ZaPJmoXIjnxw57cCj8p6yrYzBrBqRP bPyNMGse2WAFzAizggY1loS/I0A6QomvrKfdV3iIP/tTTI736tJs3+TbY8THHIFyYEwx KQgg== X-Forwarded-Encrypted: i=1; AFNElJ9gNTbmGh3PqDwHcoSv1R5ZhfT7aBhaE4O530BaeYbPcankLBd+4jtEXUbXGaQhtNBT4trehSnuUL5GEB4=@vger.kernel.org X-Gm-Message-State: AOJu0YwGLzG4XRnf52CQyC6o+CD7A3TJx8cM2i1NX49MgqGCrLp0rOAA 6eka1gvD3mW/FCKHuGoYMYTgAunpGx0KbTd3ftalfNTXECn9s2OXU9QT X-Gm-Gg: Acq92OHduuKuwe0JSPhP38fDnvvAD9Y9b14TjexvHeIS2Ucds7kI2DAAbQNgpPlAr+R U2QbCKuWpnApeDxEGTKayU4P7/7q8W0qduVnxa1S7Jtqww/fNw+gIe5UiiDkCEhiBOkGcWEY3D7 YvQtMCAZceuigX2gHlxgPD+RgUw9kbA2rqEbG1TioxU1ID2UqRAn7rDwHUqWy280hpfAqKWWpvt uu08lEhGCKEGpb/wZvN4I1DooGqnppyBbgpIIcUVtdpD8PN+4/4nPLul1qvBy3r09ylZGzKJW8Q vlWHxlo0m1dHAIoj/8Xort7yCL6t/h4EgQrihRsFfxNRtfR7R9h22tOGt6D0KqlQSz8G1SxCssM TmLqp6hNCLD53AaYXVqOmIgyeVRuPhvEHUtLAJsbf2+hvFSF7lcjtYws+4zTdqGs6twOzldggqJ wykllUxe6ei3HlOTwjC3xpIYG8i47z7IcpZnwylQfuJWYyLBk1YnP0+I9/dC8yRkCY X-Received: by 2002:a05:600c:8210:b0:48a:5c23:cab with SMTP id 5b1f17b1804b1-48fe6322447mr202576835e9.19.1779095091999; Mon, 18 May 2026 02:04:51 -0700 (PDT) Received: from pumpkin (82-69-66-36.dsl.in-addr.zen.co.uk. [82.69.66.36]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-48fe5ab527asm266838295e9.11.2026.05.18.02.04.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 18 May 2026 02:04:51 -0700 (PDT) Date: Mon, 18 May 2026 10:04:49 +0100 From: David Laight To: Peter Collingbourne Cc: Mark Brown , Patrice Chotard , Boris Brezillon , Christophe Kerello , linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] spi: use udelay() for short polling delays Message-ID: <20260518100449.0595079d@pumpkin> In-Reply-To: References: <20260517062554.117448-1-peter@pcc.me.uk> <20260517150253.031dec09@pumpkin> X-Mailer: Claws Mail 4.1.1 (GTK 3.24.38; arm-unknown-linux-gnueabihf) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On Sun, 17 May 2026 16:15:01 -0700 Peter Collingbourne wrote: > On Sun, May 17, 2026 at 7:02=E2=80=AFAM David Laight > wrote: > > > > On Sat, 16 May 2026 23:25:54 -0700 > > Peter Collingbourne wrote: > > =20 > > > 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 functio= ns") > > > Signed-off-by: Peter Collingbourne > > > --- > > > 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 =3D read_poll_timeout(spi_mem_read_status, read_sta= tus_ret, > > > - (read_status_ret || ((status) &= mask) =3D=3D match), > > > - polling_delay_us, timeout_ms * = 1000, false, mem, > > > - op, &status); > > > + if (polling_delay_us < 10) > > > + ret =3D read_poll_timeout_atomic( > > > + spi_mem_read_status, read_status_ret, > > > + (read_status_ret || ((status)&mask) =3D= =3D match), > > > + polling_delay_us, timeout_ms * 1000, fa= lse, mem, > > > + op, &status); > > > + else > > > + ret =3D read_poll_timeout( > > > + spi_mem_read_status, read_status_ret, > > > + (read_status_ret || ((status)&mask) =3D= =3D match), > > > + polling_delay_us, timeout_ms * 1000, fa= lse, mem, > > > + op, &status); > > > + =20 > > > > 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 =20 >=20 > In include/linux/mtd/spinand.h we have: >=20 > #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 >=20 > So we will only delay here for reads and resets. The timeout_ms values are equally relevant. If anything goes wrong you don't want to be spinning for a long time (and the timeout in the polling/atomic case is likely to be significantly longer than you might expect - it only counts time in delay_us()). So sort of want something like the following - but it is too hacky! int spins =3D polling_delay_us < 10 ? 10 : 0; #define usleep_range(min, max) (--spins < 0 ? usleep_range(min, max) : udel= ay(max)) ret =3D read_poll_timeout(....); #undef usleep_range -- David >=20 > 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. >=20 > For example, the generic driver: >=20 > 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() =20 >=20 > For spi-mt65xx: >=20 > spi_mem_read_status() > -> spi_mem_exec_op() > -> mtk_spi_mem_exec_op() [via ctlr->mem_ops->exec_op] > -> wait_for_completion_timeout() =20 >=20 > Peter >=20