Patchwork mtd: nand: docg4: add udelay(1) to polling loop

login
register
mail settings
Submitter Mike Dunn
Date April 22, 2013, 6:23 p.m.
Message ID <1366654990-15392-1-git-send-email-mikedunn@newsguy.com>
Download mbox | patch
Permalink /patch/238615/
State New
Headers show

Comments

Mike Dunn - April 22, 2013, 6:23 p.m.
Add call to udelay(1) in the busy-wait loop that polls the device status
register.  This will make the timeout independent of cpu clock rate. This
sloppiness came back to bite me when I increased the cpu clock rate, and
timeouts started occurring during long block erasure operations.

Signed-off-by: Mike Dunn <mikedunn@newsguy.com>
---
 drivers/mtd/nand/docg4.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
Artem Bityutskiy - May 16, 2013, 7:43 a.m.
On Mon, 2013-04-22 at 11:23 -0700, Mike Dunn wrote:
>  	timeo = 1000;
>  	do {
> +		udelay(1);
>  		cpu_relax();
>  		flash_status = readb(docptr + DOC_FLASHCONTROL);
>  	} while (!(flash_status & DOC_CTRL_FLASHREADY) && --timeo);

How about using jiffies instead? Something like:

timeout = jiffies + msecs_to_jiffies(delay);
do {
} while (time_before(jiffies, timeout))
Mike Dunn - May 16, 2013, 7:49 p.m.
On 05/16/2013 12:43 AM, Artem Bityutskiy wrote:
> On Mon, 2013-04-22 at 11:23 -0700, Mike Dunn wrote:
>>  	timeo = 1000;
>>  	do {
>> +		udelay(1);
>>  		cpu_relax();
>>  		flash_status = readb(docptr + DOC_FLASHCONTROL);
>>  	} while (!(flash_status & DOC_CTRL_FLASHREADY) && --timeo);
> 
> How about using jiffies instead? Something like:
> 
> timeout = jiffies + msecs_to_jiffies(delay);
> do {
> } while (time_before(jiffies, timeout))
> 


This works too (just tested it).  But the reason I did not do this originally is
because the timeout is very short.  With udelay(1) and timeo=1000, the timeout
is 1ms (plus processing time), and I think this is well in excess of the actual
time needed by the hardware.  Using jiffies and with HZ=100, my minimum timeout
is 10ms.  But there's no harm in having an excessive timeout since it's a
busy-wait loop, so your suggestion is probably fine.

Is use of udelay() frowned upon?  Will using jiffies cause a problem with a
"tickless" configuration?  Hoping to learn something...

Thanks Artem!
Mike

Patch

diff --git a/drivers/mtd/nand/docg4.c b/drivers/mtd/nand/docg4.c
index 18fa448..fd353d2 100644
--- a/drivers/mtd/nand/docg4.c
+++ b/drivers/mtd/nand/docg4.c
@@ -279,6 +279,7 @@  static int poll_status(struct docg4_priv *doc)
 
 	timeo = 1000;
 	do {
+		udelay(1);
 		cpu_relax();
 		flash_status = readb(docptr + DOC_FLASHCONTROL);
 	} while (!(flash_status & DOC_CTRL_FLASHREADY) && --timeo);