All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch] cfi: prevent kernel panic with cfi flash
@ 2005-09-27 19:25 ` Jan Pedersen
  0 siblings, 0 replies; 6+ messages in thread
From: Jan Pedersen @ 2005-09-27 19:25 UTC (permalink / raw)
  To: linux-mips

When using the cfi (common flash interface) driver, every word written to
the flash chips is followed by a operation complete poll. This poll is
intended to have a timeout of 1 ms. However this timeout is calculated by
HZ/1000, which happends to be 0 because HZ < 1000. The result of this is
that there will be just one poll for operation complete. If this single poll
fails, the kernel panics. This patch increases this timeout to HZ (1
second). This is far more than needed, but is preferred over a panic. This
fix is well tested and completely avoids the panic.

Signed-off-by: Jan Pedersen <jp@jp-embedded.com>
---
diff -Naur linux-2.4.31.org/drivers/mtd/chips/cfi_cmdset_0002.c
linux-2.4.31/drivers/mtd/chips/cfi_cmdset_0002.c
--- linux-2.4.31.org/drivers/mtd/chips/cfi_cmdset_0002.c	2004-11-17
06:54:21.000000000 -0500
+++ linux-2.4.31/drivers/mtd/chips/cfi_cmdset_0002.c	2005-08-22
12:14:17.000000000 -0400
@@ -510,7 +510,7 @@
 	   or tells us why it failed. */        
 	dq6 = CMD(1<<6);
 	dq5 = CMD(1<<5);
-	timeo = jiffies + (HZ/1000); /* setting timeout to 1ms for now */
+	timeo = jiffies + (HZ); /* setting timeout to 1s for now */
 		
 	oldstatus = cfi_read(map, adr);
 	status = cfi_read(map, adr);

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

* [patch] cfi: prevent kernel panic with cfi flash
@ 2005-09-27 19:25 ` Jan Pedersen
  0 siblings, 0 replies; 6+ messages in thread
From: Jan Pedersen @ 2005-09-27 19:25 UTC (permalink / raw)
  To: linux-mips

When using the cfi (common flash interface) driver, every word written to
the flash chips is followed by a operation complete poll. This poll is
intended to have a timeout of 1 ms. However this timeout is calculated by
HZ/1000, which happends to be 0 because HZ < 1000. The result of this is
that there will be just one poll for operation complete. If this single poll
fails, the kernel panics. This patch increases this timeout to HZ (1
second). This is far more than needed, but is preferred over a panic. This
fix is well tested and completely avoids the panic.

Signed-off-by: Jan Pedersen <jp@jp-embedded.com>
---
diff -Naur linux-2.4.31.org/drivers/mtd/chips/cfi_cmdset_0002.c
linux-2.4.31/drivers/mtd/chips/cfi_cmdset_0002.c
--- linux-2.4.31.org/drivers/mtd/chips/cfi_cmdset_0002.c	2004-11-17
06:54:21.000000000 -0500
+++ linux-2.4.31/drivers/mtd/chips/cfi_cmdset_0002.c	2005-08-22
12:14:17.000000000 -0400
@@ -510,7 +510,7 @@
 	   or tells us why it failed. */        
 	dq6 = CMD(1<<6);
 	dq5 = CMD(1<<5);
-	timeo = jiffies + (HZ/1000); /* setting timeout to 1ms for now */
+	timeo = jiffies + (HZ); /* setting timeout to 1s for now */
 		
 	oldstatus = cfi_read(map, adr);
 	status = cfi_read(map, adr);

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

* Re: [patch] cfi: prevent kernel panic with cfi flash
  2005-09-27 19:25 ` Jan Pedersen
  (?)
@ 2005-09-30  0:20 ` Ralf Baechle
  -1 siblings, 0 replies; 6+ messages in thread
From: Ralf Baechle @ 2005-09-30  0:20 UTC (permalink / raw)
  To: Jan Pedersen; +Cc: linux-mips

On Tue, Sep 27, 2005 at 09:25:42PM +0200, Jan Pedersen wrote:

> When using the cfi (common flash interface) driver, every word written to
> the flash chips is followed by a operation complete poll. This poll is
> intended to have a timeout of 1 ms. However this timeout is calculated by
> HZ/1000, which happends to be 0 because HZ < 1000. The result of this is
> that there will be just one poll for operation complete. If this single poll
> fails, the kernel panics. This patch increases this timeout to HZ (1
> second). This is far more than needed, but is preferred over a panic. This
> fix is well tested and completely avoids the panic.
> 
> Signed-off-by: Jan Pedersen <jp@jp-embedded.com>

Patch looks good to me - but this code isn't maintained by linux-mips.org,
so you may want to resend your patches here:

MEMORY TECHNOLOGY DEVICES
P:      David Woodhouse
M:      dwmw2@infradead.org
W:      http://www.linux-mtd.infradead.org/
L:      linux-mtd@lists.infradead.org
S:      Maintained

Thanks anyway,

  Ralf

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

* [patch] cfi: prevent kernel panic with cfi flash
@ 2005-10-04 20:47 Jan Pedersen
  2005-10-05 13:25 ` Jörn Engel
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Pedersen @ 2005-10-04 20:47 UTC (permalink / raw)
  To: linux-mtd

When using the cfi (common flash interface) driver, every word written to
the flash chips is followed by a operation complete poll. This poll is
intended to have a timeout of 1 ms. However this timeout is calculated by
HZ/1000, which happends to be 0 because HZ < 1000. The result of this is
that there will be just one poll for operation complete. If this single poll
fails, the kernel panics. This patch increases this timeout to HZ (1
second). This is far more than needed, but is preferred over a panic. This
fix is well tested and completely avoids the panic.

Signed-off-by: Jan Pedersen <jp@jp-embedded.com>
---
diff -Naur linux-2.4.31.org/drivers/mtd/chips/cfi_cmdset_0002.c
linux-2.4.31/drivers/mtd/chips/cfi_cmdset_0002.c
--- linux-2.4.31.org/drivers/mtd/chips/cfi_cmdset_0002.c	2004-11-17
06:54:21.000000000 -0500
+++ linux-2.4.31/drivers/mtd/chips/cfi_cmdset_0002.c	2005-08-22
12:14:17.000000000 -0400
@@ -510,7 +510,7 @@
 	   or tells us why it failed. */        
 	dq6 = CMD(1<<6);
 	dq5 = CMD(1<<5);
-	timeo = jiffies + (HZ/1000); /* setting timeout to 1ms for now */
+	timeo = jiffies + (HZ); /* setting timeout to 1s for now */
 		
 	oldstatus = cfi_read(map, adr);
 	status = cfi_read(map, adr);

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

* Re: [patch] cfi: prevent kernel panic with cfi flash
  2005-10-04 20:47 Jan Pedersen
@ 2005-10-05 13:25 ` Jörn Engel
  2005-10-06 20:19   ` Jan Pedersen
  0 siblings, 1 reply; 6+ messages in thread
From: Jörn Engel @ 2005-10-05 13:25 UTC (permalink / raw)
  To: Jan Pedersen; +Cc: linux-mtd

On Tue, 4 October 2005 22:47:29 +0200, Jan Pedersen wrote:
> 
> When using the cfi (common flash interface) driver, every word written to
> the flash chips is followed by a operation complete poll. This poll is
> intended to have a timeout of 1 ms. However this timeout is calculated by
> HZ/1000, which happends to be 0 because HZ < 1000. The result of this is
> that there will be just one poll for operation complete. If this single poll
> fails, the kernel panics. This patch increases this timeout to HZ (1
> second). This is far more than needed, but is preferred over a panic. This
> fix is well tested and completely avoids the panic.

Hmm.  Is there a good reason to pick (HZ) instead of (HZ/1000 + 1)?
The latter would be one jiffy more than 1ms, much closer to intended
value and still always nonzero.

> Signed-off-by: Jan Pedersen <jp@jp-embedded.com>
> ---
> diff -Naur linux-2.4.31.org/drivers/mtd/chips/cfi_cmdset_0002.c
> linux-2.4.31/drivers/mtd/chips/cfi_cmdset_0002.c
> --- linux-2.4.31.org/drivers/mtd/chips/cfi_cmdset_0002.c	2004-11-17
> 06:54:21.000000000 -0500
> +++ linux-2.4.31/drivers/mtd/chips/cfi_cmdset_0002.c	2005-08-22
> 12:14:17.000000000 -0400
> @@ -510,7 +510,7 @@
>  	   or tells us why it failed. */        
>  	dq6 = CMD(1<<6);
>  	dq5 = CMD(1<<5);
> -	timeo = jiffies + (HZ/1000); /* setting timeout to 1ms for now */
> +	timeo = jiffies + (HZ); /* setting timeout to 1s for now */
>  		
>  	oldstatus = cfi_read(map, adr);
>  	status = cfi_read(map, adr);
> 

Jörn

-- 
Measure. Don't tune for speed until you've measured, and even then
don't unless one part of the code overwhelms the rest.
-- Rob Pike

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

* RE: [patch] cfi: prevent kernel panic with cfi flash
  2005-10-05 13:25 ` Jörn Engel
@ 2005-10-06 20:19   ` Jan Pedersen
  0 siblings, 0 replies; 6+ messages in thread
From: Jan Pedersen @ 2005-10-06 20:19 UTC (permalink / raw)
  To: 'Jörn Engel'; +Cc: linux-mtd

> Hmm.  Is there a good reason to pick (HZ) instead of (HZ/1000 + 1)?
> The latter would be one jiffy more than 1ms, much closer to intended
> value and still always nonzero.

It is not that important what the value is set to. I just checked the
datasheet, and my flash chips have a word write timing of maximum 210 uS
(typical 9 uS). After the 210 uS, dq5 is raised to indicate a write timeout.
This is detected by the sw, so the 1 ms timeout is never reached unless
there is a bug in hw or sw. Assuming that other flash chips have similar
write timings, 1 ms is enough, however, since the kernel panics instead
doing some retry, reaching the timeout means a dead box. Since the timeout
is initialized BEFORE the first poll, and the last time_after() call is done
AFTER the last poll, the actual timeout might be lower than intended if
jiffies ticks just after timeout initialization. So I simply picked the
value HZ.

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

end of thread, other threads:[~2005-10-06 20:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-09-27 19:25 [patch] cfi: prevent kernel panic with cfi flash Jan Pedersen
2005-09-27 19:25 ` Jan Pedersen
2005-09-30  0:20 ` Ralf Baechle
  -- strict thread matches above, loose matches on Subject: below --
2005-10-04 20:47 Jan Pedersen
2005-10-05 13:25 ` Jörn Engel
2005-10-06 20:19   ` Jan Pedersen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.