[U-Boot] [PATCH] nand_base: Add timeout for NAND reset command

Without the timeout present an infinite loop can occur if the NAND device is broken or not present.
Signed-off-by: Peter Tyser ptyser@xes-inc.com --- drivers/mtd/nand/nand_base.c | 19 +++++++++++++++++-- 1 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index bfa7874..a2656eb 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -75,6 +75,17 @@ #include <jffs2/jffs2.h> #endif
+/* + * CONFIG_SYS_NAND_RESET_CNT is used as a timeout mechanism when resetting + * a flash. NAND flash is initialized prior to interrupts so standard timers + * can't be used. CONFIG_SYS_NAND_RESET_CNT should be set to a value + * which is greater than (max NAND reset time / NAND status read time). + * A conservative default of 200000 (500 us / 25 ns) is used as a default. + */ +#ifndef CONFIG_SYS_NAND_RESET_CNT +#define CONFIG_SYS_NAND_RESET_CNT 200000 +#endif + /* Define default oob placement schemes for large and small page devices */ static struct nand_ecclayout nand_oob_8 = { .eccbytes = 3, @@ -524,6 +535,7 @@ static void nand_command(struct mtd_info *mtd, unsigned int command, { register struct nand_chip *chip = mtd->priv; int ctrl = NAND_CTRL_CLE | NAND_CTRL_CHANGE; + uint32_t rst_sts_cnt = CONFIG_SYS_NAND_RESET_CNT;
/* * Write out the command to the device. @@ -590,7 +602,8 @@ static void nand_command(struct mtd_info *mtd, unsigned int command, NAND_CTRL_CLE | NAND_CTRL_CHANGE); chip->cmd_ctrl(mtd, NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE); - while (!(chip->read_byte(mtd) & NAND_STATUS_READY)) ; + while (!(chip->read_byte(mtd) & NAND_STATUS_READY) && + (rst_sts_cnt--)); return;
/* This applies to read commands */ @@ -626,6 +639,7 @@ static void nand_command_lp(struct mtd_info *mtd, unsigned int command, int column, int page_addr) { register struct nand_chip *chip = mtd->priv; + uint32_t rst_sts_cnt = CONFIG_SYS_NAND_RESET_CNT;
/* Emulate NAND_CMD_READOOB */ if (command == NAND_CMD_READOOB) { @@ -696,7 +710,8 @@ static void nand_command_lp(struct mtd_info *mtd, unsigned int command, NAND_NCE | NAND_CLE | NAND_CTRL_CHANGE); chip->cmd_ctrl(mtd, NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE); - while (!(chip->read_byte(mtd) & NAND_STATUS_READY)) ; + while (!(chip->read_byte(mtd) & NAND_STATUS_READY) && + (rst_sts_cnt--)); return;
case NAND_CMD_RNDOUT:

Peter Tyser wrote:
+/*
- CONFIG_SYS_NAND_RESET_CNT is used as a timeout mechanism when resetting
- a flash. NAND flash is initialized prior to interrupts so standard timers
- can't be used. CONFIG_SYS_NAND_RESET_CNT should be set to a value
- which is greater than (max NAND reset time / NAND status read time).
- A conservative default of 200000 (500 us / 25 ns) is used as a default.
- */
+#ifndef CONFIG_SYS_NAND_RESET_CNT +#define CONFIG_SYS_NAND_RESET_CNT 200000 +#endif
Where does 25 ns come from? Should the timeout be in terms of real time rather than iterations (we use get_ticks() for this purpose in fsl_elbc_nand.c)?
-Scott

On Wed, 2009-02-04 at 13:54 -0600, Scott Wood wrote:
Peter Tyser wrote:
+/*
- CONFIG_SYS_NAND_RESET_CNT is used as a timeout mechanism when resetting
- a flash. NAND flash is initialized prior to interrupts so standard timers
- can't be used. CONFIG_SYS_NAND_RESET_CNT should be set to a value
- which is greater than (max NAND reset time / NAND status read time).
- A conservative default of 200000 (500 us / 25 ns) is used as a default.
- */
+#ifndef CONFIG_SYS_NAND_RESET_CNT +#define CONFIG_SYS_NAND_RESET_CNT 200000 +#endif
Where does 25 ns come from? Should the timeout be in terms of real time rather than iterations (we use get_ticks() for this purpose in fsl_elbc_nand.c)?
The 25ns was calculated based on the addition of trp and trhoh from the Micron MT29F8G08 datasheet. Based on the timing diagram for a "Read Status" cycle I thought this would be the minimum cycle time needed to read the chip's status. Other chips (ST, Samsung) I glanced at had > 25 ns read status times as well.
I had tried using get_timer() (I believe nand_wait() would have been perfect to use), but that didn't work due to interrupts being disabled when NAND is probed. I didn't consider using get_ticks()... That seems much better. Is get_ticks() available for all platforms when NAND is initialized?
Assuming get_ticks() is available for all platforms, would you prefer I: 1. re-do the patch using get_ticks() 2. update nand_wait() to use get_ticks instead of get_timer() and use it
Thanks, Peter

Peter Tyser wrote:
The 25ns was calculated based on the addition of trp and trhoh from the Micron MT29F8G08 datasheet. Based on the timing diagram for a "Read Status" cycle I thought this would be the minimum cycle time needed to read the chip's status. Other chips (ST, Samsung) I glanced at had > 25 ns read status times as well.
I had tried using get_timer() (I believe nand_wait() would have been perfect to use), but that didn't work due to interrupts being disabled when NAND is probed. I didn't consider using get_ticks()... That seems much better. Is get_ticks() available for all platforms when NAND is initialized?
Probably, but who knows what weirdness is out there.
Assuming get_ticks() is available for all platforms, would you prefer I:
- re-do the patch using get_ticks()
- update nand_wait() to use get_ticks instead of get_timer() and use it
#2 looks better.
-Scott

Dear Scott Wood,
In message 4989FE57.80404@freescale.com you wrote:
Peter Tyser wrote:
The 25ns was calculated based on the addition of trp and trhoh from the Micron MT29F8G08 datasheet. Based on the timing diagram for a "Read Status" cycle I thought this would be the minimum cycle time needed to read the chip's status. Other chips (ST, Samsung) I glanced at had > 25 ns read status times as well.
I had tried using get_timer() (I believe nand_wait() would have been perfect to use), but that didn't work due to interrupts being disabled when NAND is probed. I didn't consider using get_ticks()... That seems much better. Is get_ticks() available for all platforms when NAND is initialized?
get_ticks() not a public interface. It should not be used in any common code.
Please use get_timer().
Best regards,
Wolfgang Denk

On Wed, 2009-02-04 at 22:22 +0100, Wolfgang Denk wrote:
Dear Scott Wood,
In message 4989FE57.80404@freescale.com you wrote:
Peter Tyser wrote:
The 25ns was calculated based on the addition of trp and trhoh from the Micron MT29F8G08 datasheet. Based on the timing diagram for a "Read Status" cycle I thought this would be the minimum cycle time needed to read the chip's status. Other chips (ST, Samsung) I glanced at had > 25 ns read status times as well.
I had tried using get_timer() (I believe nand_wait() would have been perfect to use), but that didn't work due to interrupts being disabled when NAND is probed. I didn't consider using get_ticks()... That seems much better. Is get_ticks() available for all platforms when NAND is initialized?
get_ticks() not a public interface. It should not be used in any common code.
Please use get_timer().
The problem is that the NAND code is used prior to interrupts being enabled, thus we can't use get_timer(). I used a hokey delay based on (read times * number of iterations). Whats worse, my hokey loop or get_ticks()?
Thanks, Peter

Dear Peter Tyser,
In message 1233782967.7067.338.camel@localhost.localdomain you wrote:
get_ticks() not a public interface. It should not be used in any common code.
Please use get_timer().
Ah, I see.
The problem is that the NAND code is used prior to interrupts being enabled, thus we can't use get_timer(). I used a hokey delay based on (read times * number of iterations). Whats worse, my hokey loop or get_ticks()?
I can't tell. Thing is that get_ticks() may or may not exist. I don't think all architectures have it. I don't see it for ixp, leon*, mcf*, microblaze, mips, nios*, ...
Best regards,
Wolfgang Denk

Wolfgang Denk wrote:
get_ticks() not a public interface. It should not be used in any common code.
Then what is it doing in include/common.h?
Please use get_timer().
But as was pointed out, that depends on interrupts, and thus does not work until very late in the boot process.
-Scott

Dear Scott,
In message 498A0917.8080404@freescale.com you wrote:
get_ticks() not a public interface. It should not be used in any common code.
Then what is it doing in include/common.h?
Please use get_timer().
But as was pointed out, that depends on interrupts, and thus does not work until very late in the boot process.
I see.
But get_ticks() is an internal interface that may, or may not be preset. There are architectures where it does not exist.
Best regards,
Wolfgang Denk

Hi Scott,
On Wed, 2009-02-04 at 22:54 +0100, Wolfgang Denk wrote:
Dear Scott,
In message 498A0917.8080404@freescale.com you wrote:
get_ticks() not a public interface. It should not be used in any common code.
Then what is it doing in include/common.h?
Please use get_timer().
But as was pointed out, that depends on interrupts, and thus does not work until very late in the boot process.
I see.
But get_ticks() is an internal interface that may, or may not be preset. There are architectures where it does not exist.
Since get_ticks() and get_timer() are not available, is the patch (hokey as it is) acceptable?
Thanks, Peter

Peter Tyser wrote:
Hi Scott,
On Wed, 2009-02-04 at 22:54 +0100, Wolfgang Denk wrote:
Dear Scott,
In message 498A0917.8080404@freescale.com you wrote:
get_ticks() not a public interface. It should not be used in any common code.
Then what is it doing in include/common.h?
Please use get_timer().
But as was pointed out, that depends on interrupts, and thus does not work until very late in the boot process.
I see.
But get_ticks() is an internal interface that may, or may not be preset. There are architectures where it does not exist.
Since get_ticks() and get_timer() are not available, is the patch (hokey as it is) acceptable?
Yes, I'll apply it.
-Scott

On Wed, Feb 04, 2009 at 01:47:22PM -0600, Peter Tyser wrote:
Without the timeout present an infinite loop can occur if the NAND device is broken or not present.
Signed-off-by: Peter Tyser ptyser@xes-inc.com
Applied to u-boot-nand-flash.
-Scott
participants (3)
-
Peter Tyser
-
Scott Wood
-
Wolfgang Denk