[U-Boot] nand commands missing wtchdog reset

[U-Boot] nand commands missing wtchdog reset

[Jaap de Jong]

Hi all,
On my board (at91sam9263ek) I have enabled the watchdog.
It will reset the processor after about 16 seconds.
It looks like it is working but if I'm writing a large file into nand it 
seems that the watchdog is not reset and finally my processor resets.
I've patched it, but I'm not sure if it is the right way to do it this 
way...

diff -urN a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
--- a/drivers/mtd/nand/nand_base.c    2010-12-22 20:22:14.000000000 +0100
+++ b/drivers/mtd/nand/nand_base.c    2011-01-31 08:45:07.818135600 +0100
@@ -447,6 +447,7 @@
          if (chip->dev_ready)
              if (chip->dev_ready(mtd))
                  break;
+        WATCHDOG_RESET ();
      }
  }

@@ -730,6 +731,7 @@
              if (this->read_byte(mtd) & NAND_STATUS_READY)
                  break;
          }
+        WATCHDOG_RESET ();
      }
  #ifdef PPCHAMELON_NAND_TIMER_HACK
      reset_timer();

Regards,
Jaap

[U-Boot] nand commands missing wtchdog reset

[Scott Wood]

On Mon, 31 Jan 2011 09:05:55 +0100
Jaap de Jong <jaap.dejong@nedap.com> wrote:

> Hi all,
> On my board (at91sam9263ek) I have enabled the watchdog.
> It will reset the processor after about 16 seconds.
> It looks like it is working but if I'm writing a large file into nand it 
> seems that the watchdog is not reset and finally my processor resets.
> I've patched it, but I'm not sure if it is the right way to do it this 
> way...

So far we've been putting the watchdog resets in higher-level
functions.  It looks like the block-skipping versions have them, but
the non-block-skipping versions don't (and the former will call the
latter if it doesn't see any bad blocks).

So I think this should go in nand_read() and nand_write().  If things
hang up inside the low-level wait that should trigger the watchdog.

> diff -urN a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> --- a/drivers/mtd/nand/nand_base.c    2010-12-22 20:22:14.000000000 +0100
> +++ b/drivers/mtd/nand/nand_base.c    2011-01-31 08:45:07.818135600 +0100
> @@ -447,6 +447,7 @@
>           if (chip->dev_ready)
>               if (chip->dev_ready(mtd))
>                   break;
> +        WATCHDOG_RESET ();
>       }
>   }
> 
> @@ -730,6 +731,7 @@
>               if (this->read_byte(mtd) & NAND_STATUS_READY)
>                   break;
>           }
> +        WATCHDOG_RESET ();
>       }
>   #ifdef PPCHAMELON_NAND_TIMER_HACK
>       reset_timer();
> 

This patch is whitespace-mangled.  Try using git send-email.

-Scott

[U-Boot] nand commands missing wtchdog reset

[Scott Wood]

On Mon, 31 Jan 2011 13:16:59 -0600
Scott Wood <scottwood@freescale.com> wrote:

> On Mon, 31 Jan 2011 09:05:55 +0100
> Jaap de Jong <jaap.dejong@nedap.com> wrote:
> 
> > Hi all,
> > On my board (at91sam9263ek) I have enabled the watchdog.
> > It will reset the processor after about 16 seconds.
> > It looks like it is working but if I'm writing a large file into nand it 
> > seems that the watchdog is not reset and finally my processor resets.
> > I've patched it, but I'm not sure if it is the right way to do it this 
> > way...
> 
> So far we've been putting the watchdog resets in higher-level
> functions.  It looks like the block-skipping versions have them, but
> the non-block-skipping versions don't (and the former will call the
> latter if it doesn't see any bad blocks).
> 
> So I think this should go in nand_read() and nand_write().  If things
> hang up inside the low-level wait that should trigger the watchdog.

Oh, and all patches require a sign-off, and the text above the patch
should be what is intended to go in the git changelog, with any
additional comments/greetings/etc below a "---" line.

See http://www.denx.de/wiki/U-Boot/Patches
and also the Developer's Certificate of Origin in
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/SubmittingPatches;h=689e2371095cc5dfea9927120009341f369159aa;hb=HEAD

-Scott

[U-Boot] nand commands missing wtchdog reset

[Jaap de Jong]

thanks for your reply.
You are right, the format was totally wrong; I apologize!
Concerning the patch itself: I agree: low-level hangups should trigger 
the watchdog although in this specific case the hangups will not occur 
due to a timeout construction surrounding it.
Unfortunately I'm not able to investigate this any further
To get this fix done: hopefully some u-boot-guru will do the dirty work...

Jaap

On 01/31/2011 08:25 PM, Scott Wood wrote:
> On Mon, 31 Jan 2011 13:16:59 -0600
> Scott Wood<scottwood@freescale.com>  wrote:
>
>> On Mon, 31 Jan 2011 09:05:55 +0100
>> Jaap de Jong<jaap.dejong@nedap.com>  wrote:
>>
>>> Hi all,
>>> On my board (at91sam9263ek) I have enabled the watchdog.
>>> It will reset the processor after about 16 seconds.
>>> It looks like it is working but if I'm writing a large file into nand it
>>> seems that the watchdog is not reset and finally my processor resets.
>>> I've patched it, but I'm not sure if it is the right way to do it this
>>> way...
>> So far we've been putting the watchdog resets in higher-level
>> functions.  It looks like the block-skipping versions have them, but
>> the non-block-skipping versions don't (and the former will call the
>> latter if it doesn't see any bad blocks).
>>
>> So I think this should go in nand_read() and nand_write().  If things
>> hang up inside the low-level wait that should trigger the watchdog.
> Oh, and all patches require a sign-off, and the text above the patch
> should be what is intended to go in the git changelog, with any
> additional comments/greetings/etc below a "---" line.
>
> See http://www.denx.de/wiki/U-Boot/Patches
> and also the Developer's Certificate of Origin in
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/SubmittingPatches;h=689e2371095cc5dfea9927120009341f369159aa;hb=HEAD
>
> -Scott
>

[U-Boot] nand commands missing wtchdog reset

[Scott Wood]

On Tue, 1 Feb 2011 10:43:56 +0100
Jaap de Jong <jaap.dejong@nedap.com> wrote:

> thanks for your reply.
> You are right, the format was totally wrong; I apologize!
> Concerning the patch itself: I agree: low-level hangups should trigger 
> the watchdog although in this specific case the hangups will not occur 
> due to a timeout construction surrounding it.

And if the timeout mechanism fails, that'd be a really good time for
the watchdog to kick in. :-)

> Unfortunately I'm not able to investigate this any further
> To get this fix done: hopefully some u-boot-guru will do the dirty work...

It's just a couple lines, I'll write up a patch.

-Scott