Skip to contentSkip to conversation

mtd: fix timeout in M25P80 driver

PATCH In review Peter Horton 5 participants 6 messages Apr 13, 2009 – Apr 17, 2009 Memory technology devices (MTD)

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 mtd: fix timeout in M25P80 driver

Extend erase timeout in M25P80 SPI Flash driver.

The M25P80 drivers fails erasing sectors on a M25P128 because the ready wait timeout is too short. Change the timeout from a simple loop count to a suitable number of seconds.

---

drivers/mtd/devices/m25p80.c +8 -8
Index: linux-2.6.29-git12/drivers/mtd/devices/m25p80.c
===================================================================
@@ -54,7 +54,7 @@
 #define	SR_SRWD			0x80	/* SR write protect */
 
 /* Define max times to check status register before we give up. */
-#define	MAX_READY_WAIT_COUNT	100000
+#define	MAX_READY_WAIT_JIFFIES	(10 * HZ)	/* eg. M25P128 specs 6s max sector erase */
 #define	CMD_SIZE		4
 
 #ifdef CONFIG_M25PXX_USE_FAST_READ
@@ -145,20 +145,20 @@
  */
 static int wait_till_ready(struct m25p *flash)
 {
-	int count;
+	unsigned long deadline;
 	int sr;
 
-	/* one chip guarantees max 5 msec wait here after page writes,
-	 * but potentially three seconds (!) after page erase.
-	 */
-	for (count = 0; count < MAX_READY_WAIT_COUNT; count++) {
+	deadline = jiffies + MAX_READY_WAIT_JIFFIES;
+
+	do {
 		if ((sr = read_sr(flash)) < 0)
 			break;
 		else if (!(sr & SR_WIP))
 			return 0;
 
-		/* REVISIT sometimes sleeping would be best */
-	}
+		cond_resched();
+
+	} while (!time_after_eq(jiffies, deadline));
 
 	return 1;
 }
Tested-by on v1 1/1
#1 lore raw
Quoted 7 lines · Peter Horton

Extend erase timeout in M25P80 SPI Flash driver.

The M25P80 drivers fails erasing sectors on a M25P128 because the ready wait timeout is too short. Change the timeout from a simple loop count to a suitable number of seconds.

This solves the problem I've seen on the QNAP TS-219.

Signature · 2 lines
Martin Michlmayr
http://www.cyrius.com/
#2 lore raw
In reply to Peter Horton
Quoted 38 lines · On Mon, 13 Apr 2009 15:26:33 +0100 Peter Horton <[email protected]> wrote:

Extend erase timeout in M25P80 SPI Flash driver.

The M25P80 drivers fails erasing sectors on a M25P128 because the ready wait timeout is too short. Change the timeout from a simple loop count to a suitable number of seconds.

---
Index: linux-2.6.29-git12/drivers/mtd/devices/m25p80.c
===================================================================

drivers/mtd/devices/m25p80.c +5 -6
@@ -54,7 +54,7 @@
 #define	SR_SRWD			0x80	/* SR write protect */
 
 /* Define max times to check status register before we give up. */
-#define	MAX_READY_WAIT_COUNT	100000
+#define	MAX_READY_WAIT_JIFFIES	(10 * HZ)	/* eg. M25P128 specs 6s max sector erase */
 #define	CMD_SIZE		4
 
 #ifdef CONFIG_M25PXX_USE_FAST_READ
@@ -145,20 +145,20 @@
  */
 static int wait_till_ready(struct m25p *flash)
 {
-	int count;
+	unsigned long deadline;
 	int sr;
 
-	/* one chip guarantees max 5 msec wait here after page writes,
-	 * but potentially three seconds (!) after page erase.
-	 */
-	for (count = 0; count < MAX_READY_WAIT_COUNT; count++) {
+	deadline = jiffies + MAX_READY_WAIT_JIFFIES;
+
+	do {
 		if ((sr = read_sr(flash)) < 0)
 			break;

drivers/mtd/devices/m25p80.c line 163

Quoted 10 lines
drivers/mtd/devices/m25p80.c +3 -2
 		else if (!(sr & SR_WIP))
 			return 0;
 
-		/* REVISIT sometimes sleeping would be best */
-	}
+		cond_resched();
+
+	} while (!time_after_eq(jiffies, deadline));
 
 	return 1;

ow, so it sits there chewing electricity for up to ten seconds.

How much time does this really take, in the real world?

It would be better to fall back to (say) msleep(1) if we find out that the device is being slow to respond.

#3 lore raw
In reply to Peter Horton
Quoted 22 lines · Peter Horton

Extend erase timeout in M25P80 SPI Flash driver.

The M25P80 drivers fails erasing sectors on a M25P128 because the ready wait timeout is too short. Change the timeout from a simple loop count to a suitable number of seconds.

---
Index: linux-2.6.29-git12/drivers/mtd/devices/m25p80.c
===================================================================

drivers/mtd/devices/m25p80.c +1 -1
@@ -54,7 +54,7 @@
 #define	SR_SRWD			0x80	/* SR write protect */
 
 /* Define max times to check status register before we give up. */
-#define	MAX_READY_WAIT_COUNT	100000
+#define	MAX_READY_WAIT_JIFFIES	(10 * HZ)	/* eg. M25P128 specs 6s max sector erase */
 #define	CMD_SIZE		4
 
 #ifdef CONFIG_M25PXX_USE_FAST_READ

drivers/mtd/devices/m25p80.c line 153

Quoted 10 lines
drivers/mtd/devices/m25p80.c +1 -4
@@ -145,20 +145,20 @@
  */
 static int wait_till_ready(struct m25p *flash)
 {
-	int count;
+	unsigned long deadline;
 	int sr;
 
-	/* one chip guarantees max 5 msec wait here after page writes,
-	 * but potentially three seconds (!) after page erase.
-	 */

Why did you remove the comment? Is it wrong or useless?

Quoted 17 lines
drivers/mtd/devices/m25p80.c +6 -3
-	for (count = 0; count < MAX_READY_WAIT_COUNT; count++) {
+	deadline = jiffies + MAX_READY_WAIT_JIFFIES;
+
+	do {
 		if ((sr = read_sr(flash)) < 0)
 			break;
 		else if (!(sr & SR_WIP))
 			return 0;
 
-		/* REVISIT sometimes sleeping would be best */
-	}
+		cond_resched();
+
+	} while (!time_after_eq(jiffies, deadline));
 
 	return 1;
 }
Signature · 2 lines
Best regards,
Artem Bityutskiy (Битюцкий Артём)
#4 lore raw
Quoted 30 lines · Artem Bityutskiy
Quoted 29 lines · Peter Horton

Extend erase timeout in M25P80 SPI Flash driver.

The M25P80 drivers fails erasing sectors on a M25P128 because the ready wait timeout is too short. Change the timeout from a simple loop count to a suitable number of seconds.

---
Index: linux-2.6.29-git12/drivers/mtd/devices/m25p80.c
===================================================================

drivers/mtd/devices/m25p80.c +2 -2
@@ -54,7 +54,7 @@
 #define	SR_SRWD			0x80	/* SR write protect */
 
 /* Define max times to check status register before we give up. */
-#define	MAX_READY_WAIT_COUNT	100000
+#define	MAX_READY_WAIT_JIFFIES	(10 * HZ)	/* eg. M25P128 specs 6s max sector erase */
 #define	CMD_SIZE		4
 
 #ifdef CONFIG_M25PXX_USE_FAST_READ
@@ -145,20 +145,20 @@
  */
 static int wait_till_ready(struct m25p *flash)
 {
-	int count;
+	unsigned long deadline;
 	int sr;
 
Quoted 5 lines

drivers/mtd/devices/m25p80.c line 153

Quoted 3 lines
drivers/mtd/devices/m25p80.c +0 -3
-	/* one chip guarantees max 5 msec wait here after page writes,
-	 * but potentially three seconds (!) after page erase.
-	 */

Why did you remove the comment? Is it wrong or useless?

I moved the comment up to the definition of MAX_READY_WAIT_JIFFIES. Looking through the code I missed the fact the driver can generate "chip erase", on the M25P128 the maximum ready timeout for this is 250s !!!

P.

#5 lore raw
Quoted 37 lines · Peter Horton
Quoted 30 lines · Artem Bityutskiy
Quoted 29 lines · Peter Horton

Extend erase timeout in M25P80 SPI Flash driver.

The M25P80 drivers fails erasing sectors on a M25P128 because the ready wait timeout is too short. Change the timeout from a simple loop count to a suitable number of seconds.

---
Index: linux-2.6.29-git12/drivers/mtd/devices/m25p80.c
===================================================================

drivers/mtd/devices/m25p80.c +2 -2
@@ -54,7 +54,7 @@
 #define	SR_SRWD			0x80	/* SR write protect */
 
 /* Define max times to check status register before we give up. */
-#define	MAX_READY_WAIT_COUNT	100000
+#define	MAX_READY_WAIT_JIFFIES	(10 * HZ)	/* eg. M25P128 specs 6s max sector erase */
 #define	CMD_SIZE		4
 
 #ifdef CONFIG_M25PXX_USE_FAST_READ
@@ -145,20 +145,20 @@
  */
 static int wait_till_ready(struct m25p *flash)
 {
-	int count;
+	unsigned long deadline;
 	int sr;
 
Quoted 5 lines

drivers/mtd/devices/m25p80.c line 153

Quoted 3 lines
drivers/mtd/devices/m25p80.c +0 -3
-	/* one chip guarantees max 5 msec wait here after page writes,
-	 * but potentially three seconds (!) after page erase.
-	 */

Why did you remove the comment? Is it wrong or useless?

Quoted 3 lines

I moved the comment up to the definition of MAX_READY_WAIT_JIFFIES. Looking through the code I missed the fact the driver can generate "chip erase", on the M25P128 the maximum ready timeout for this is 250s !!!

So I assume you are going to send another patch? Also, how about Andrew's suggestion not to hog CPU?

Signature · 2 lines
Best regards,
Artem Bityutskiy (Битюцкий Артём)