* [PATCH] mtd: fix timeout in M25P80 driver
@ 2009-04-13 14:26 Peter Horton
2009-04-13 19:33 ` Martin Michlmayr
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Peter Horton @ 2009-04-13 14:26 UTC (permalink / raw)
To: linux-mtd; +Cc: linux-kernel, dwmw2
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.
Signed-off-by: Peter Horton <zero@colonel-panic.org>
---
Index: linux-2.6.29-git12/drivers/mtd/devices/m25p80.c
===================================================================
--- linux-2.6.29-git12.orig/drivers/mtd/devices/m25p80.c 2009-04-12 21:41:16.000000000 +0000
+++ linux-2.6.29-git12/drivers/mtd/devices/m25p80.c 2009-04-12 21:43:01.000000000 +0000
@@ -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;
}
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] mtd: fix timeout in M25P80 driver
2009-04-13 14:26 [PATCH] mtd: fix timeout in M25P80 driver Peter Horton
@ 2009-04-13 19:33 ` Martin Michlmayr
2009-04-15 23:10 ` Andrew Morton
2009-04-17 7:24 ` Artem Bityutskiy
2 siblings, 0 replies; 6+ messages in thread
From: Martin Michlmayr @ 2009-04-13 19:33 UTC (permalink / raw)
To: Peter Horton; +Cc: linux-mtd, linux-kernel, dwmw2
* Peter Horton <zero@colonel-panic.org> [2009-04-13 15:26]:
> 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.
>
> Signed-off-by: Peter Horton <zero@colonel-panic.org>
Tested-by: Martin Michlmayr <tbm@cyrius.com>
This solves the problem I've seen on the QNAP TS-219.
--
Martin Michlmayr
http://www.cyrius.com/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mtd: fix timeout in M25P80 driver
2009-04-13 14:26 [PATCH] mtd: fix timeout in M25P80 driver Peter Horton
2009-04-13 19:33 ` Martin Michlmayr
@ 2009-04-15 23:10 ` Andrew Morton
2009-04-17 7:24 ` Artem Bityutskiy
2 siblings, 0 replies; 6+ messages in thread
From: Andrew Morton @ 2009-04-15 23:10 UTC (permalink / raw)
To: Peter Horton; +Cc: linux-mtd, linux-kernel, dwmw2
On Mon, 13 Apr 2009 15:26:33 +0100
Peter Horton <zero@colonel-panic.org> 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.
>
> Signed-off-by: Peter Horton <zero@colonel-panic.org>
> ---
> Index: linux-2.6.29-git12/drivers/mtd/devices/m25p80.c
> ===================================================================
> --- linux-2.6.29-git12.orig/drivers/mtd/devices/m25p80.c 2009-04-12 21:41:16.000000000 +0000
> +++ linux-2.6.29-git12/drivers/mtd/devices/m25p80.c 2009-04-12 21:43:01.000000000 +0000
> @@ -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;
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.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] mtd: fix timeout in M25P80 driver
2009-04-13 14:26 [PATCH] mtd: fix timeout in M25P80 driver Peter Horton
2009-04-13 19:33 ` Martin Michlmayr
2009-04-15 23:10 ` Andrew Morton
@ 2009-04-17 7:24 ` Artem Bityutskiy
2009-04-17 8:22 ` Peter Horton
2 siblings, 1 reply; 6+ messages in thread
From: Artem Bityutskiy @ 2009-04-17 7:24 UTC (permalink / raw)
To: Peter Horton; +Cc: linux-mtd, dwmw2, linux-kernel
On Mon, 2009-04-13 at 15:26 +0100, Peter Horton 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.
>
> Signed-off-by: Peter Horton <zero@colonel-panic.org>
> ---
> Index: linux-2.6.29-git12/drivers/mtd/devices/m25p80.c
> ===================================================================
> --- linux-2.6.29-git12.orig/drivers/mtd/devices/m25p80.c 2009-04-12 21:41:16.000000000 +0000
> +++ linux-2.6.29-git12/drivers/mtd/devices/m25p80.c 2009-04-12 21:43:01.000000000 +0000
> @@ -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.
> - */
Why did you remove the comment? Is it wrong or useless?
> - 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;
> }
--
Best regards,
Artem Bityutskiy (Битюцкий Артём)
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] mtd: fix timeout in M25P80 driver
2009-04-17 7:24 ` Artem Bityutskiy
@ 2009-04-17 8:22 ` Peter Horton
2009-04-17 9:21 ` Artem Bityutskiy
0 siblings, 1 reply; 6+ messages in thread
From: Peter Horton @ 2009-04-17 8:22 UTC (permalink / raw)
To: dedekind; +Cc: Peter Horton, dwmw2, linux-mtd, linux-kernel
Artem Bityutskiy wrote:
> On Mon, 2009-04-13 at 15:26 +0100, Peter Horton 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.
>>
>> Signed-off-by: Peter Horton <zero@colonel-panic.org>
>> ---
>> Index: linux-2.6.29-git12/drivers/mtd/devices/m25p80.c
>> ===================================================================
>> --- linux-2.6.29-git12.orig/drivers/mtd/devices/m25p80.c 2009-04-12 21:41:16.000000000 +0000
>> +++ linux-2.6.29-git12/drivers/mtd/devices/m25p80.c 2009-04-12 21:43:01.000000000 +0000
>> @@ -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.
>> - */
> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] mtd: fix timeout in M25P80 driver
2009-04-17 8:22 ` Peter Horton
@ 2009-04-17 9:21 ` Artem Bityutskiy
0 siblings, 0 replies; 6+ messages in thread
From: Artem Bityutskiy @ 2009-04-17 9:21 UTC (permalink / raw)
To: Peter Horton; +Cc: Peter Horton, dwmw2, linux-mtd, linux-kernel
On Fri, 2009-04-17 at 09:22 +0100, Peter Horton wrote:
> Artem Bityutskiy wrote:
> > On Mon, 2009-04-13 at 15:26 +0100, Peter Horton 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.
> >>
> >> Signed-off-by: Peter Horton <zero@colonel-panic.org>
> >> ---
> >> Index: linux-2.6.29-git12/drivers/mtd/devices/m25p80.c
> >> ===================================================================
> >> --- linux-2.6.29-git12.orig/drivers/mtd/devices/m25p80.c 2009-04-12 21:41:16.000000000 +0000
> >> +++ linux-2.6.29-git12/drivers/mtd/devices/m25p80.c 2009-04-12 21:43:01.000000000 +0000
> >> @@ -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.
> >> - */
> > 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 !!!
So I assume you are going to send another patch? Also, how about
Andrew's suggestion not to hog CPU?
--
Best regards,
Artem Bityutskiy (Битюцкий Артём)
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-04-17 9:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-04-13 14:26 [PATCH] mtd: fix timeout in M25P80 driver Peter Horton
2009-04-13 19:33 ` Martin Michlmayr
2009-04-15 23:10 ` Andrew Morton
2009-04-17 7:24 ` Artem Bityutskiy
2009-04-17 8:22 ` Peter Horton
2009-04-17 9:21 ` Artem Bityutskiy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox