linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V2 RFC] mtd: spi-nor: intel: provide a range for poll_timout
@ 2017-02-13  8:13 Nicholas Mc Guire
  2017-02-14  9:47 ` Mika Westerberg
  0 siblings, 1 reply; 3+ messages in thread
From: Nicholas Mc Guire @ 2017-02-13  8:13 UTC (permalink / raw)
  To: Cyrille Pitchen
  Cc: Marek Vasut, David Woodhouse, Brian Norris, Boris Brezillon,
	Richard Weinberger, Mika Westerberg, linux-mtd, linux-kernel,
	Nicholas Mc Guire

The overall poll time here is INTEL_SPI_TIMEOUT * 1000 which is
5000 * 1000 - so 5seconds and it is coded as a tight loop here delay_us
to readl_poll_timeout() is set to 0. As this is never called in an atomic
context sleeping should be no issue and there is no reasons for the
tight-loop here.

Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
---

V2: fixed prefix as pointed out by Boris Brezillon
    <boris.brezillon@free-electrons.com>

Problem located by experimental coccinelle script:
./drivers/mtd/spi-nor/intel-spi.c:265:8-26: WARNING: usleep_range min=0 for delay INTEL_SPI_TIMEOUT * 1000
./drivers/mtd/spi-nor/intel-spi.c:274:8-26: WARNING: usleep_range min=0 for delay INTEL_SPI_TIMEOUT * 1000

The rational for setting the delay_us here to 40 is that readx_poll_timeout()
will take delay_us >> 2 + 1 as min value and that should be at least 10us (see
Documentation/timers/timers-howto.txt). Ideally the delay would be made
even larger to keep the load on the hrtimer subsystem low as these delays
here do not seem to be critical. Someone that knows the details of this device
would need to check if a larger delay would be ok here.

Patch was compile tested with: multi_v7_defconfig (implies CONFIG_MTD_SPI_NOR=y)
one coccicheck finding reported and one spars finding (in separate patches)

Patch is against 4.10-rc7 (localversion-next is next-20170213)

 drivers/mtd/spi-nor/intel-spi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/intel-spi.c b/drivers/mtd/spi-nor/intel-spi.c
index a10f602..4630716 100644
--- a/drivers/mtd/spi-nor/intel-spi.c
+++ b/drivers/mtd/spi-nor/intel-spi.c
@@ -263,7 +263,7 @@ static int intel_spi_wait_hw_busy(struct intel_spi *ispi)
 	u32 val;
 
 	return readl_poll_timeout(ispi->base + HSFSTS_CTL, val,
-				  !(val & HSFSTS_CTL_SCIP), 0,
+				  !(val & HSFSTS_CTL_SCIP), 40,
 				  INTEL_SPI_TIMEOUT * 1000);
 }
 
@@ -272,7 +272,7 @@ static int intel_spi_wait_sw_busy(struct intel_spi *ispi)
 	u32 val;
 
 	return readl_poll_timeout(ispi->sregs + SSFSTS_CTL, val,
-				  !(val & SSFSTS_CTL_SCIP), 0,
+				  !(val & SSFSTS_CTL_SCIP), 40,
 				  INTEL_SPI_TIMEOUT * 1000);
 }
 
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH V2 RFC] mtd: spi-nor: intel: provide a range for poll_timout
  2017-02-13  8:13 [PATCH V2 RFC] mtd: spi-nor: intel: provide a range for poll_timout Nicholas Mc Guire
@ 2017-02-14  9:47 ` Mika Westerberg
  2018-05-18 20:02   ` Boris Brezillon
  0 siblings, 1 reply; 3+ messages in thread
From: Mika Westerberg @ 2017-02-14  9:47 UTC (permalink / raw)
  To: Nicholas Mc Guire
  Cc: Cyrille Pitchen, Marek Vasut, David Woodhouse, Brian Norris,
	Boris Brezillon, Richard Weinberger, linux-mtd, linux-kernel

On Mon, Feb 13, 2017 at 09:13:42AM +0100, Nicholas Mc Guire wrote:
> The overall poll time here is INTEL_SPI_TIMEOUT * 1000 which is
> 5000 * 1000 - so 5seconds and it is coded as a tight loop here delay_us
> to readl_poll_timeout() is set to 0. As this is never called in an atomic
> context sleeping should be no issue and there is no reasons for the
> tight-loop here.
> 
> Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH V2 RFC] mtd: spi-nor: intel: provide a range for poll_timout
  2017-02-14  9:47 ` Mika Westerberg
@ 2018-05-18 20:02   ` Boris Brezillon
  0 siblings, 0 replies; 3+ messages in thread
From: Boris Brezillon @ 2018-05-18 20:02 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: Nicholas Mc Guire, Boris Brezillon, Richard Weinberger,
	linux-kernel, Marek Vasut, linux-mtd, Cyrille Pitchen,
	Brian Norris, David Woodhouse

On Tue, 14 Feb 2017 11:47:24 +0200
Mika Westerberg <mika.westerberg@linux.intel.com> wrote:

> On Mon, Feb 13, 2017 at 09:13:42AM +0100, Nicholas Mc Guire wrote:
> > The overall poll time here is INTEL_SPI_TIMEOUT * 1000 which is
> > 5000 * 1000 - so 5seconds and it is coded as a tight loop here delay_us
> > to readl_poll_timeout() is set to 0. As this is never called in an atomic
> > context sleeping should be no issue and there is no reasons for the
> > tight-loop here.
> > 
> > Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>  
> 
> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Queued to spi-nor/next.

Thanks,

Boris

> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-05-18 20:03 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-13  8:13 [PATCH V2 RFC] mtd: spi-nor: intel: provide a range for poll_timout Nicholas Mc Guire
2017-02-14  9:47 ` Mika Westerberg
2018-05-18 20:02   ` Boris Brezillon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).