[U-Boot] [PATCH v4] at91: Don't initialize watchdog if CONFIG_SKIP_WATCHDOG_INIT is defined
On AT91 the watchdog mode register can only be written once after reset. If this register is written by u-boot e.g. a Linux driver can't reconfigure the watchdog later. If the watchdog is left untouched this is possible. Without touching the mode register the watchdog has a default setup and u-boot is still able to trigger the watchdog. Signed-off-by: Alexander Stein --- Changes in v2: * Add a new specific option CONFIG_SKIP_WATCHDOG_INIT * Add some documentation Changes in v3: * Be more verbose in describing the problem in the commit message and README Changes in v4: * Fix commit message README | 11 +++ arch/arm/cpu/arm926ejs/at91/lowlevel_init.S |2 ++ 2 files changed, 13 insertions(+), 0 deletions(-) diff --git a/README b/README index b6bf451..9fbb6c9 100644 --- a/README +++ b/README @@ -2827,6 +2827,17 @@ Low Level (hardware related) configuration options: some other boot loader or by a debugger which performs these initializations itself. +- CONFIG_SKIP_WATCHDOG_INIT + + [arm AT91 only] If this variable is defined, then the + watchdog will not be programmed upon u-boot start. + On AT91 the watchdog mode register can only be written + once after reset. If this register is written by u-boot + e.g. a Linux driver can't reconfigure the watchdog later. If + the watchdog is left untouched this is possible. + Without touching the mode register the watchdog has a default + setup and u-boot is still able to trigger the watchdog. + - CONFIG_PRELOADER Modifies the behaviour of start.S when compiling a loader diff --git a/arch/arm/cpu/arm926ejs/at91/lowlevel_init.S b/arch/arm/cpu/arm926ejs/at91/lowlevel_init.S index 559c35c..0645ba8 100644 --- a/arch/arm/cpu/arm926ejs/at91/lowlevel_init.S +++ b/arch/arm/cpu/arm926ejs/at91/lowlevel_init.S @@ -186,8 +186,10 @@ SDRAM_setup_end: .ltorg SMRDATA: +#ifndef CONFIG_SKIP_WATCHDOG_INIT .word AT91_ASM_WDT_MR .word CONFIG_SYS_WDTC_WDMR_VAL +#endif /* configure PIOx as EBI0 D[16-31] */ #if defined(CONFIG_AT91SAM9263) .word AT91_ASM_PIOD_PDR -- 1.7.1 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4] at91: Don't initialize watchdog if CONFIG_SKIP_WATCHDOG_INIT is defined
On Mon, Aug 9, 2010 at 2:40 AM, Alexander Stein wrote: > On AT91 the watchdog mode register can only be written once after reset. > If this register is written by u-boot e.g. a Linux driver can't > reconfigure the watchdog later. If the watchdog is left untouched this > is possible. Without touching the mode register the watchdog has a default > setup and u-boot is still able to trigger the watchdog. > > Signed-off-by: Alexander Stein > --- > Changes in v2: > * Add a new specific option CONFIG_SKIP_WATCHDOG_INIT > * Add some documentation > > Changes in v3: > * Be more verbose in describing the problem in the commit message and README > > Changes in v4: > * Fix commit message > > README | 11 +++ > arch/arm/cpu/arm926ejs/at91/lowlevel_init.S | 2 ++ > 2 files changed, 13 insertions(+), 0 deletions(-) > > diff --git a/README b/README > index b6bf451..9fbb6c9 100644 > --- a/README > +++ b/README > @@ -2827,6 +2827,17 @@ Low Level (hardware related) configuration options: > some other boot loader or by a debugger which > performs these initializations itself. > > +- CONFIG_SKIP_WATCHDOG_INIT > + > + [arm AT91 only] If this variable is defined, then the > + watchdog will not be programmed upon u-boot start. > + On AT91 the watchdog mode register can only be written > + once after reset. If this register is written by u-boot > + e.g. a Linux driver can't reconfigure the watchdog later. If > + the watchdog is left untouched this is possible. > + Without touching the mode register the watchdog has a default > + setup and u-boot is still able to trigger the watchdog. isnt the at91 logic inverted ? shouldnt the watchdog programming only be done when someone has opted in to it via some watchdog define ? -mike ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4] at91: Don't initialize watchdog if CONFIG_SKIP_WATCHDOG_INIT is defined
Hello Mike, Am Montag, 9. August 2010, 09:13:45 schrieb Mike Frysinger: > > On AT91 the watchdog mode register can only be written once after reset. > > If this register is written by u-boot e.g. a Linux driver can't > > reconfigure the watchdog later. If the watchdog is left untouched this > > is possible. Without touching the mode register the watchdog has a > > default setup and u-boot is still able to trigger the watchdog. > > > > [...] > > > > +- CONFIG_SKIP_WATCHDOG_INIT > > + > > + [arm AT91 only] If this variable is defined, then the > > + watchdog will not be programmed upon u-boot start. > > + On AT91 the watchdog mode register can only be written > > + once after reset. If this register is written by u-boot > > + e.g. a Linux driver can't reconfigure the watchdog later. > > If + the watchdog is left untouched this is possible. + > > Without touching the mode register the watchdog has a > > default + setup and u-boot is still able to trigger the > > watchdog. > > isnt the at91 logic inverted ? shouldnt the watchdog programming only > be done when someone has opted in to it via some watchdog define ? This was my first version, but Wolfgang NAK'ed it as he see this as the wrong approach. See http://article.gmane.org/gmane.comp.boot-loaders.u-boot/81589 Best regards, Alexander ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4] at91: Don't initialize watchdog if CONFIG_SKIP_WATCHDOG_INIT is defined
On Mon, Aug 9, 2010 at 7:42 AM, Alexander Stein wrote: > Am Montag, 9. August 2010, 09:13:45 schrieb Mike Frysinger: >> > On AT91 the watchdog mode register can only be written once after reset. >> > If this register is written by u-boot e.g. a Linux driver can't >> > reconfigure the watchdog later. If the watchdog is left untouched this >> > is possible. Without touching the mode register the watchdog has a >> > default setup and u-boot is still able to trigger the watchdog. >> > >> > [...] >> > >> > +- CONFIG_SKIP_WATCHDOG_INIT >> > + >> > + [arm AT91 only] If this variable is defined, then the >> > + watchdog will not be programmed upon u-boot start. >> > + On AT91 the watchdog mode register can only be written >> > + once after reset. If this register is written by u-boot >> > + e.g. a Linux driver can't reconfigure the watchdog later. >> > If + the watchdog is left untouched this is possible. + >> > Without touching the mode register the watchdog has a >> > default + setup and u-boot is still able to trigger the >> > watchdog. >> >> isnt the at91 logic inverted ? shouldnt the watchdog programming only >> be done when someone has opted in to it via some watchdog define ? > > This was my first version, but Wolfgang NAK'ed it as he see this as the wrong > approach. See http://article.gmane.org/gmane.comp.boot-loaders.u-boot/81589 i think you interpreted his statement incorrectly ? he said it should raise an error, not that supporting CONFIG_HW_WATCHDOG is wrong. -mike ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4] at91: Don't initialize watchdog if CONFIG_SKIP_WATCHDOG_INIT is defined
Dear Alexander Stein, just to bring in my thoughts to this watchdog issue, and to explain what I think the issue is here: 1. on (all?) AT91SAM9 devices the watchdog is initially enabled (after Reset) with a 16 second timeout (provides a 32kHz Xtal is used). 2. the watchdog mode register can only be written once, then it becomes read-only. 3. on (all?) systems without NOR flash u-boot is a secondary boot loader. That primary bootloader in that case _could_ have written the mode register. 4. usually systems would leave the watchdog untouched until the final operating systems takes over. That means that we should have two, positively acting defines that 1. make u-boot retrigger the watchdog within the 16 second interval (if NOT defined, u-boot will NOT retrigger the watchdog) 2. make u-boot write the mode register with any user defined value (watchdog disabled (forever), or enabled with different timeout or action) The define for 1. could essentially be on for every system, because it would not hurt to retrigger a disabled watchdog; the define for 2. would require the define 1., if the watchdog stays enabled. So... that being said, can we go forward as follows: CONFIG_AT91SAM9_WATCHDOG and CONFIG_HW_WATCHDOG need both be defined so u-boot will periodically retrigger the watchdog independant of its mode. CONFIG_SYS_WDTC_WDMR_VAL, _IF_ defined will make u-boot write that value into the watchdog mode register. I know that is exactly Alexander's original proposal, and with proper README it should be understandable that this is the right way to do it. Best Regards, Reinhard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4] at91: Don't initialize watchdog if CONFIG_SKIP_WATCHDOG_INIT is defined
On Friday, August 20, 2010 08:31:22 Reinhard Meyer wrote: > just to bring in my thoughts to this watchdog issue, > and to explain what I think the issue is here: > > 1. on (all?) AT91SAM9 devices the watchdog is initially enabled > (after Reset) with a 16 second timeout (provides a 32kHz Xtal > is used). ah, i think that's the kicker and what needs to be noted. i wasnt aware of devices that did this sort of thing. -mike signature.asc Description: This is a digitally signed message part. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4] at91: Don't initialize watchdog if CONFIG_SKIP_WATCHDOG_INIT is defined
Dear Mike Frysinger, > On Friday, August 20, 2010 08:31:22 Reinhard Meyer wrote: >> just to bring in my thoughts to this watchdog issue, >> and to explain what I think the issue is here: >> >> 1. on (all?) AT91SAM9 devices the watchdog is initially enabled >> (after Reset) with a 16 second timeout (provides a 32kHz Xtal >> is used). > > ah, i think that's the kicker and what needs to be noted. i wasnt aware of > devices that did this sort of thing. I think that's rather ingenious, with a little hardware one could toggle between boot devices or toggle some higher address line of a NOR flash to try several bootloaders. So... lets proceed here... For the doc/README.at91-watchdog (new file): " If CONFIG_SYS_WDTC_WDMR_VAL is defined u-boot will write that value into the watchdog mode register. Otherwise the watchdog is left in its initial state: active with 16 second timeout. Note that the watchdog mode register can only be written once. If the watchdog is left running or programmed to be running, you need to enable its retriggering by defining CONFIG_AT91SAM9_WATCHDOG and CONFIG_HW_WATCHDOG." Alexander, can you prepare a revised patch with README like above and a better (positive logic) subject like "AT91: program WDMR only if CONFIG_SYS_WDTC_WDMR_VAL is defined"? Maybe change CONFIG_SYS_WDTC_WDMR_VAL to CONFIG_AT91_WDTC_WDMR_VAL, because its AT91 specific but user changeable? Best Regards, Reinhard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
