| Message ID | 1457000979-15717-1-git-send-email-romain.izard.pro@gmail.com (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Headers | show |
On 03/03/2016 02:29 AM, Romain Izard wrote: > If the internal counter is not refreshed when the watchdog is started > for the first time, the watchdog will trigger very rapidly. For example, > opening /dev/watchdog without writing in it will immediately trigger a > reboot, instead of waiting for the delay to expire. > > To avoid this problem, reload the timer on opening the watchdog device. > > Command: "while sleep 5; do echo 1; done > /dev/watchdog" > Before: system reset > After: the watchdog runs correctly > > Signed-off-by: Romain Izard <romain.izard.pro@gmail.com> Subject might better read "ping watchdog on start" or similar. Does the watchdog have to be pinged before it is enabled ? I am a bit concerned that there may still be a 125 uS window during which the system could restart. Thanks, Guenter > --- > drivers/watchdog/sama5d4_wdt.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/watchdog/sama5d4_wdt.c b/drivers/watchdog/sama5d4_wdt.c > index a49634cdc1cc..e162fe140ae1 100644 > --- a/drivers/watchdog/sama5d4_wdt.c > +++ b/drivers/watchdog/sama5d4_wdt.c > @@ -15,6 +15,7 @@ > #include <linux/platform_device.h> > #include <linux/reboot.h> > #include <linux/watchdog.h> > +#include <linux/delay.h> > > #include "at91sam9_wdt.h" > > @@ -58,6 +59,8 @@ static int sama5d4_wdt_start(struct watchdog_device *wdd) > reg = wdt_read(wdt, AT91_WDT_MR); > reg &= ~AT91_WDT_WDDIS; > wdt_write(wdt, AT91_WDT_MR, reg); > + udelay(125); /* > 4 cycles at 32,768 Hz */ > + wdt_write(wdt, AT91_WDT_CR, AT91_WDT_KEY | AT91_WDT_WDRSTT); > > return 0; > } >
Hi Guenter, 2016-03-03 13:10 GMT+01:00 Guenter Roeck <linux@roeck-us.net>: > On 03/03/2016 02:29 AM, Romain Izard wrote: >> >> If the internal counter is not refreshed when the watchdog is started >> for the first time, the watchdog will trigger very rapidly. For >> example, opening /dev/watchdog without writing in it will immediately >> trigger a reboot, instead of waiting for the delay to expire. >> >> To avoid this problem, reload the timer on opening the watchdog >> device. >> >> Command: "while sleep 5; do echo 1; done > /dev/watchdog" >> Before: system reset >> After: the watchdog runs correctly >> >> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com> > > > Subject might better read "ping watchdog on start" or similar. > OK. I'll change it for a v2. > Does the watchdog have to be pinged before it is enabled ? I am a bit > concerned that there may still be a 125 uS window during which the > system could restart. > According to the SAMA5D2 & SAMA5D4 datasheets, the timer ought to be reloaded when the watchdog is enabled by a write in the MR register. Unfortunately, it does not work as described, as I encountered the problem on a SAMA5D2 Xplained board. The 4 clock delay is not in the datasheet either, but without any delay the timer is clearly not reloaded, as my issue stays the same. As there is a required delay before writing to MR after writing to CR, I applied the same type of delay in the reverse case. Perhaps Nicolas or Wenyou have more information on this. Best regards,
On Thu, Mar 03, 2016 at 01:53:47PM +0100, Romain Izard wrote: > Hi Guenter, > > 2016-03-03 13:10 GMT+01:00 Guenter Roeck <linux@roeck-us.net>: > > On 03/03/2016 02:29 AM, Romain Izard wrote: > >> > >> If the internal counter is not refreshed when the watchdog is started > >> for the first time, the watchdog will trigger very rapidly. For > >> example, opening /dev/watchdog without writing in it will immediately > >> trigger a reboot, instead of waiting for the delay to expire. > >> > >> To avoid this problem, reload the timer on opening the watchdog > >> device. > >> > >> Command: "while sleep 5; do echo 1; done > /dev/watchdog" > >> Before: system reset > >> After: the watchdog runs correctly > >> > >> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com> > > > > > > Subject might better read "ping watchdog on start" or similar. > > > OK. I'll change it for a v2. > > > Does the watchdog have to be pinged before it is enabled ? I am a bit > > concerned that there may still be a 125 uS window during which the > > system could restart. > > > > According to the SAMA5D2 & SAMA5D4 datasheets, the timer ought to be > reloaded when the watchdog is enabled by a write in the MR register. > Unfortunately, it does not work as described, as I encountered the > problem on a SAMA5D2 Xplained board. > > The 4 clock delay is not in the datasheet either, but without any delay > the timer is clearly not reloaded, as my issue stays the same. As there > is a required delay before writing to MR after writing to CR, I applied > the same type of delay in the reverse case. > Question is if there is now a 4 clock window where the watchdog can still reset the system. Can you reload the timer before enabling the watchdog ? Thanks, Guenter > Perhaps Nicolas or Wenyou have more information on this. > > Best regards, > -- > Romain Izard
Hi Romain, On 2016/3/3 18:29, Romain Izard wrote: > If the internal counter is not refreshed when the watchdog is started > for the first time, the watchdog will trigger very rapidly. For example, > opening /dev/watchdog without writing in it will immediately trigger a > reboot, instead of waiting for the delay to expire. > > To avoid this problem, reload the timer on opening the watchdog device. > > Command: "while sleep 5; do echo 1; done > /dev/watchdog" > Before: system reset > After: the watchdog runs correctly I didn't reproduce your issue on my side, run the your commands as follows, it works fine, the system reset doesn't happen. ---8<---- #!/bin/sh while [ 1 ] do sleep 5; echo 1 > /dev/watchdog done --->8---- I also check the WDT_MR register before and after enabling watchdog, the WDV and WDD fields are correct. Can you check it again? thank you. > > Signed-off-by: Romain Izard <romain.izard.pro@gmail.com> > --- > drivers/watchdog/sama5d4_wdt.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/watchdog/sama5d4_wdt.c b/drivers/watchdog/sama5d4_wdt.c > index a49634cdc1cc..e162fe140ae1 100644 > --- a/drivers/watchdog/sama5d4_wdt.c > +++ b/drivers/watchdog/sama5d4_wdt.c > @@ -15,6 +15,7 @@ > #include <linux/platform_device.h> > #include <linux/reboot.h> > #include <linux/watchdog.h> > +#include <linux/delay.h> > > #include "at91sam9_wdt.h" > > @@ -58,6 +59,8 @@ static int sama5d4_wdt_start(struct watchdog_device *wdd) > reg = wdt_read(wdt, AT91_WDT_MR); > reg &= ~AT91_WDT_WDDIS; > wdt_write(wdt, AT91_WDT_MR, reg); > + udelay(125); /* > 4 cycles at 32,768 Hz */ > + wdt_write(wdt, AT91_WDT_CR, AT91_WDT_KEY | AT91_WDT_WDRSTT); > > return 0; > } Best Regards, Wenyou Yang
On 03/03/2016 05:35 PM, Yang, Wenyou wrote: > Hi Romain, > > On 2016/3/3 18:29, Romain Izard wrote: >> If the internal counter is not refreshed when the watchdog is started >> for the first time, the watchdog will trigger very rapidly. For example, >> opening /dev/watchdog without writing in it will immediately trigger a >> reboot, instead of waiting for the delay to expire. >> >> To avoid this problem, reload the timer on opening the watchdog device. >> >> Command: "while sleep 5; do echo 1; done > /dev/watchdog" >> Before: system reset >> After: the watchdog runs correctly > I didn't reproduce your issue on my side, > > run the your commands as follows, it works fine, the system reset doesn't happen. Different chip revision ? Different chip type ? Different chip initialization by ROMMON ? Can we get exact chip revisions and types for both cases (working and not working), and (if it might be relevant) a dump of all associated chip registers ? Thanks, Guenter > ---8<---- > #!/bin/sh > > while [ 1 ] > do > sleep 5; > echo 1 > /dev/watchdog > done > --->8---- > > I also check the WDT_MR register before and after enabling watchdog, the WDV and WDD fields are correct. > > Can you check it again? thank you. > >> >> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com> >> --- >> drivers/watchdog/sama5d4_wdt.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/watchdog/sama5d4_wdt.c b/drivers/watchdog/sama5d4_wdt.c >> index a49634cdc1cc..e162fe140ae1 100644 >> --- a/drivers/watchdog/sama5d4_wdt.c >> +++ b/drivers/watchdog/sama5d4_wdt.c >> @@ -15,6 +15,7 @@ >> #include <linux/platform_device.h> >> #include <linux/reboot.h> >> #include <linux/watchdog.h> >> +#include <linux/delay.h> >> #include "at91sam9_wdt.h" >> @@ -58,6 +59,8 @@ static int sama5d4_wdt_start(struct watchdog_device *wdd) >> reg = wdt_read(wdt, AT91_WDT_MR); >> reg &= ~AT91_WDT_WDDIS; >> wdt_write(wdt, AT91_WDT_MR, reg); >> + udelay(125); /* > 4 cycles at 32,768 Hz */ >> + wdt_write(wdt, AT91_WDT_CR, AT91_WDT_KEY | AT91_WDT_WDRSTT); >> return 0; >> } > > Best Regards, > Wenyou Yang >
Hi Wenyou, Guenter, 2016-03-04 6:23 GMT+01:00 Guenter Roeck <linux@roeck-us.net>: > On 03/03/2016 05:35 PM, Yang, Wenyou wrote: >> On 2016/3/3 18:29, Romain Izard wrote: >>> >>> If the internal counter is not refreshed when the watchdog is >>> started for the first time, the watchdog will trigger very rapidly. >>> For example, opening /dev/watchdog without writing in it will >>> immediately trigger a reboot, instead of waiting for the delay to >>> expire. >>> >>> To avoid this problem, reload the timer on opening the watchdog >>> device. >>> >>> Command: "while sleep 5; do echo 1; done > /dev/watchdog" Before: >>> system reset After: the watchdog runs correctly >> >> I didn't reproduce your issue on my side, >> >> run the your commands as follows, it works fine, the system reset >> doesn't happen. I've just verified with the factory image provided on the SAMA5D2 Xplained board. It does not display this behaviour. But the difference is that in the case without the issue, I'm using the AT91bootstrap SPL, U-Boot, and the kernel from the QSPI chip. When I have the issue, I have a U-Boot based SPL, U-Boot itself and the kernel that come from the FAT partition of an SD-Card. Userspace does not seem to be involved in the issue, as I can reproduce it both with my buildroot environment, and the Yocto environment from the factory image. > Different chip revision ? Different chip type ? Different chip > initialization by ROMMON ? > > Can we get exact chip revisions and types for both cases (working and > not working), and (if it might be relevant) a dump of all associated > chip registers ? >> I also check the WDT_MR register before and after enabling watchdog, >> the WDV and WDD fields are correct. >> >> Can you check it again? thank you. Working case: MR on kernel startup: 0x3fffafff MR after watchdog init: 0x0fffafff MR after start: 0x0fff2fff Problem case: MR on kernel startup: 0x00008000 MR after watchdog init: 0x0fffafff MR after start: 0x0fff2fff So this means that the counter reload does not seem to work very well if WDD/WDV have been set to 0 in the past. The other question is why does U-Boot (from the Atmel branch based on 2015.1) put this stange value in this register. Best regards,
On 03/04/2016 01:06 AM, Romain Izard wrote: > Hi Wenyou, Guenter, > > 2016-03-04 6:23 GMT+01:00 Guenter Roeck <linux@roeck-us.net>: >> On 03/03/2016 05:35 PM, Yang, Wenyou wrote: >>> On 2016/3/3 18:29, Romain Izard wrote: >>>> >>>> If the internal counter is not refreshed when the watchdog is >>>> started for the first time, the watchdog will trigger very rapidly. >>>> For example, opening /dev/watchdog without writing in it will >>>> immediately trigger a reboot, instead of waiting for the delay to >>>> expire. >>>> >>>> To avoid this problem, reload the timer on opening the watchdog >>>> device. >>>> >>>> Command: "while sleep 5; do echo 1; done > /dev/watchdog" Before: >>>> system reset After: the watchdog runs correctly >>> >>> I didn't reproduce your issue on my side, >>> >>> run the your commands as follows, it works fine, the system reset >>> doesn't happen. > > I've just verified with the factory image provided on the SAMA5D2 > Xplained board. It does not display this behaviour. > > But the difference is that in the case without the issue, I'm using the > AT91bootstrap SPL, U-Boot, and the kernel from the QSPI chip. When I > have the issue, I have a U-Boot based SPL, U-Boot itself and the kernel > that come from the FAT partition of an SD-Card. > > Userspace does not seem to be involved in the issue, as I can reproduce > it both with my buildroot environment, and the Yocto environment from > the factory image. > >> Different chip revision ? Different chip type ? Different chip >> initialization by ROMMON ? >> >> Can we get exact chip revisions and types for both cases (working and >> not working), and (if it might be relevant) a dump of all associated >> chip registers ? > > >>> I also check the WDT_MR register before and after enabling watchdog, >>> the WDV and WDD fields are correct. >>> >>> Can you check it again? thank you. > > Working case: > MR on kernel startup: 0x3fffafff > MR after watchdog init: 0x0fffafff > MR after start: 0x0fff2fff > > Problem case: > MR on kernel startup: 0x00008000 > MR after watchdog init: 0x0fffafff > MR after start: 0x0fff2fff > > So this means that the counter reload does not seem to work very well if > WDD/WDV have been set to 0 in the past. The other question is why does > U-Boot (from the Atmel branch based on 2015.1) put this stange value in > this register. > Can you check the value of AT91_WDT_SR ? Maybe it tells us something. Also, in the error case, can you check if the watchdog times out at all after you applied your patch ? Thanks, Guenter > Best regards, >
2016-03-04 14:09 GMT+01:00 Guenter Roeck <linux@roeck-us.net>: > On 03/04/2016 01:06 AM, Romain Izard wrote: >> 2016-03-04 6:23 GMT+01:00 Guenter Roeck <linux@roeck-us.net>: >>> On 03/03/2016 05:35 PM, Yang, Wenyou wrote: >>>> On 2016/3/3 18:29, Romain Izard wrote: >>>>> >>>>> If the internal counter is not refreshed when the watchdog is >>>>> started for the first time, the watchdog will trigger very >>>>> rapidly. For example, opening /dev/watchdog without writing in it >>>>> will immediately trigger a reboot, instead of waiting for the >>>>> delay to expire. >>>>> >>>>> To avoid this problem, reload the timer on opening the watchdog >>>>> device. >>>>> >>>>> Command: "while sleep 5; do echo 1; done > /dev/watchdog" Before: >>>>> system reset After: the watchdog runs correctly >>>> >>>> >>>> I didn't reproduce your issue on my side, >>>> >>>> run the your commands as follows, it works fine, the system reset >>>> doesn't happen. >> >> >> I've just verified with the factory image provided on the SAMA5D2 >> Xplained board. It does not display this behaviour. >> >> But the difference is that in the case without the issue, I'm using >> the AT91bootstrap SPL, U-Boot, and the kernel from the QSPI chip. >> When I have the issue, I have a U-Boot based SPL, U-Boot itself and >> the kernel that come from the FAT partition of an SD-Card. >> >> Userspace does not seem to be involved in the issue, as I can >> reproduce it both with my buildroot environment, and the Yocto >> environment from the factory image. >> >>> Different chip revision ? Different chip type ? Different chip >>> initialization by ROMMON ? >>> >>> Can we get exact chip revisions and types for both cases (working >>> and not working), and (if it might be relevant) a dump of all >>> associated chip registers ? >> >> >> >>>> I also check the WDT_MR register before and after enabling >>>> watchdog, the WDV and WDD fields are correct. >>>> >>>> Can you check it again? thank you. >> >> >> Working case: >> MR on kernel startup: 0x3fffafff >> MR after watchdog init: 0x0fffafff >> MR after start: 0x0fff2fff >> >> Problem case: >> MR on kernel startup: 0x00008000 >> MR after watchdog init: 0x0fffafff >> MR after start: 0x0fff2fff >> >> So this means that the counter reload does not seem to work very well >> if WDD/WDV have been set to 0 in the past. The other question is why >> does U-Boot (from the Atmel branch based on 2015.1) put this stange >> value in this register. >> > > Can you check the value of AT91_WDT_SR ? Maybe it tells us something. > I didn't report it because it contained 0 at all times. So no information. > Also, in the error case, can you check if the watchdog times out at all > after you applied your patch ? It times out after 16s as expected, and reboot occurs correctly.
On 03/04/2016 05:26 AM, Romain Izard wrote: > 2016-03-04 14:09 GMT+01:00 Guenter Roeck <linux@roeck-us.net>: >> On 03/04/2016 01:06 AM, Romain Izard wrote: >>> 2016-03-04 6:23 GMT+01:00 Guenter Roeck <linux@roeck-us.net>: >>>> On 03/03/2016 05:35 PM, Yang, Wenyou wrote: >>>>> On 2016/3/3 18:29, Romain Izard wrote: >>>>>> >>>>>> If the internal counter is not refreshed when the watchdog is >>>>>> started for the first time, the watchdog will trigger very >>>>>> rapidly. For example, opening /dev/watchdog without writing in it >>>>>> will immediately trigger a reboot, instead of waiting for the >>>>>> delay to expire. >>>>>> >>>>>> To avoid this problem, reload the timer on opening the watchdog >>>>>> device. >>>>>> >>>>>> Command: "while sleep 5; do echo 1; done > /dev/watchdog" Before: >>>>>> system reset After: the watchdog runs correctly >>>>> >>>>> >>>>> I didn't reproduce your issue on my side, >>>>> >>>>> run the your commands as follows, it works fine, the system reset >>>>> doesn't happen. >>> >>> >>> I've just verified with the factory image provided on the SAMA5D2 >>> Xplained board. It does not display this behaviour. >>> >>> But the difference is that in the case without the issue, I'm using >>> the AT91bootstrap SPL, U-Boot, and the kernel from the QSPI chip. >>> When I have the issue, I have a U-Boot based SPL, U-Boot itself and >>> the kernel that come from the FAT partition of an SD-Card. >>> >>> Userspace does not seem to be involved in the issue, as I can >>> reproduce it both with my buildroot environment, and the Yocto >>> environment from the factory image. >>> >>>> Different chip revision ? Different chip type ? Different chip >>>> initialization by ROMMON ? >>>> >>>> Can we get exact chip revisions and types for both cases (working >>>> and not working), and (if it might be relevant) a dump of all >>>> associated chip registers ? >>> >>> >>> >>>>> I also check the WDT_MR register before and after enabling >>>>> watchdog, the WDV and WDD fields are correct. >>>>> >>>>> Can you check it again? thank you. >>> >>> >>> Working case: >>> MR on kernel startup: 0x3fffafff >>> MR after watchdog init: 0x0fffafff >>> MR after start: 0x0fff2fff >>> >>> Problem case: >>> MR on kernel startup: 0x00008000 >>> MR after watchdog init: 0x0fffafff >>> MR after start: 0x0fff2fff >>> >>> So this means that the counter reload does not seem to work very well >>> if WDD/WDV have been set to 0 in the past. The other question is why >>> does U-Boot (from the Atmel branch based on 2015.1) put this stange >>> value in this register. >>> >> >> Can you check the value of AT91_WDT_SR ? Maybe it tells us something. >> > I didn't report it because it contained 0 at all times. So no information. > >> Also, in the error case, can you check if the watchdog times out at all >> after you applied your patch ? > > It times out after 16s as expected, and reboot occurs correctly. > Interesting. So it looks like AT91_WDT_WDRSTT has to be set if the timer values in MR are changed from 0 to another value, or maybe after each timer value change. Wonder if that should be done in the init function, after MR is set (with the watchdog disabled). Thoughts, anyone ? Thanks, Guenter
Hi, On Fri, 4 Mar 2016 05:56:20 -0800 Guenter Roeck wrote: > On 03/04/2016 05:26 AM, Romain Izard wrote: > > 2016-03-04 14:09 GMT+01:00 Guenter Roeck <linux@roeck-us.net>: > >> On 03/04/2016 01:06 AM, Romain Izard wrote: > >>> 2016-03-04 6:23 GMT+01:00 Guenter Roeck <linux@roeck-us.net>: > >>>> On 03/03/2016 05:35 PM, Yang, Wenyou wrote: > >>>>> On 2016/3/3 18:29, Romain Izard wrote: > >>>>>> > >>>>>> If the internal counter is not refreshed when the watchdog is > >>>>>> started for the first time, the watchdog will trigger very > >>>>>> rapidly. For example, opening /dev/watchdog without writing in it > >>>>>> will immediately trigger a reboot, instead of waiting for the > >>>>>> delay to expire. > >>>>>> > >>>>>> To avoid this problem, reload the timer on opening the watchdog > >>>>>> device. > >>>>>> > >>>>>> Command: "while sleep 5; do echo 1; done > /dev/watchdog" Before: > >>>>>> system reset After: the watchdog runs correctly > >>>>> > >>>>> > >>>>> I didn't reproduce your issue on my side, > >>>>> > >>>>> run the your commands as follows, it works fine, the system reset > >>>>> doesn't happen. > >>> > >>> > >>> I've just verified with the factory image provided on the SAMA5D2 > >>> Xplained board. It does not display this behaviour. > >>> > >>> But the difference is that in the case without the issue, I'm using > >>> the AT91bootstrap SPL, U-Boot, and the kernel from the QSPI chip. > >>> When I have the issue, I have a U-Boot based SPL, U-Boot itself and > >>> the kernel that come from the FAT partition of an SD-Card. > >>> > >>> Userspace does not seem to be involved in the issue, as I can > >>> reproduce it both with my buildroot environment, and the Yocto > >>> environment from the factory image. > >>> > >>>> Different chip revision ? Different chip type ? Different chip > >>>> initialization by ROMMON ? > >>>> > >>>> Can we get exact chip revisions and types for both cases (working > >>>> and not working), and (if it might be relevant) a dump of all > >>>> associated chip registers ? > >>> > >>> > >>> > >>>>> I also check the WDT_MR register before and after enabling > >>>>> watchdog, the WDV and WDD fields are correct. > >>>>> > >>>>> Can you check it again? thank you. > >>> > >>> > >>> Working case: > >>> MR on kernel startup: 0x3fffafff > >>> MR after watchdog init: 0x0fffafff > >>> MR after start: 0x0fff2fff > >>> > >>> Problem case: > >>> MR on kernel startup: 0x00008000 > >>> MR after watchdog init: 0x0fffafff > >>> MR after start: 0x0fff2fff > >>> > >>> So this means that the counter reload does not seem to work very well > >>> if WDD/WDV have been set to 0 in the past. The other question is why > >>> does U-Boot (from the Atmel branch based on 2015.1) put this stange > >>> value in this register. > >>> > >> > >> Can you check the value of AT91_WDT_SR ? Maybe it tells us something. > >> > > I didn't report it because it contained 0 at all times. So no information. > > > >> Also, in the error case, can you check if the watchdog times out at all > >> after you applied your patch ? > > > > It times out after 16s as expected, and reboot occurs correctly. > > > > Interesting. So it looks like AT91_WDT_WDRSTT has to be set if the timer > values in MR are changed from 0 to another value, or maybe after each > timer value change. Wonder if that should be done in the init function, > after MR is set (with the watchdog disabled). > > Thoughts, anyone ? > Are you aware of the Notes in the SAMA5D4 Reference Manual (Chapter 19.5.2 Watchdog Timer Mode Register): |Note: The first write access prevents any further modification of | the value of this register. Read accesses remain possible. |Note: The WDD and WDV values must not be modified within three slow | clock periods following a restart of the watchdog performed by | a write access in WDT_CR. Any modification will cause the watchdog to trigger an end of period earlier than expected. Lothar Waßmann
Hi Lothar, 2016-03-04 15:59 GMT+01:00 Lothar Waßmann <LW@karo-electronics.de>: >> >>>>> I also check the WDT_MR register before and after enabling >> >>>>> watchdog, the WDV and WDD fields are correct. >> >>>>> >> >>>>> Can you check it again? thank you. >> >>> >> >>> >> >>> Working case: >> >>> MR on kernel startup: 0x3fffafff >> >>> MR after watchdog init: 0x0fffafff >> >>> MR after start: 0x0fff2fff >> >>> >> >>> Problem case: >> >>> MR on kernel startup: 0x00008000 >> >>> MR after watchdog init: 0x0fffafff >> >>> MR after start: 0x0fff2fff >> >>> >> >>> So this means that the counter reload does not seem to work very well >> >>> if WDD/WDV have been set to 0 in the past. The other question is why >> >>> does U-Boot (from the Atmel branch based on 2015.1) put this stange >> >>> value in this register. >> >>> >> >> >> >> Can you check the value of AT91_WDT_SR ? Maybe it tells us something. >> >> >> > I didn't report it because it contained 0 at all times. So no information. >> > >> >> Also, in the error case, can you check if the watchdog times out at all >> >> after you applied your patch ? >> > >> > It times out after 16s as expected, and reboot occurs correctly. >> > >> >> Interesting. So it looks like AT91_WDT_WDRSTT has to be set if the timer >> values in MR are changed from 0 to another value, or maybe after each >> timer value change. Wonder if that should be done in the init function, >> after MR is set (with the watchdog disabled). >> >> Thoughts, anyone ? >> > Are you aware of the Notes in the SAMA5D4 Reference Manual (Chapter > 19.5.2 Watchdog Timer Mode Register): > > |Note: The first write access prevents any further modification of > | the value of this register. Read accesses remain possible. > |Note: The WDD and WDV values must not be modified within three slow > | clock periods following a restart of the watchdog performed by > | a write access in WDT_CR. Any modification will cause the watchdog > | to trigger an end of period earlier than expected. This text is valid for older versions of the Watchdog controller, found in AT91SAM9 and SAMA5D3 chips. But SAMA5D4 & SAMA5D2 have a newer revision, which supports multiple writes to the MR register. Are you sure about your datasheet? I have this in the latest version found on Atmel's site. > Atmel-11238B-ATARM-SAMA5D4-Datasheet_24-Aug-15 > Section 18.5.2 > > Note: Write access to this register has no effect if the LOCKMR > command is issued in WDT_CR (unlocked on hardware reset). > Note: The WDT_MR register values must not be modified within three slow > clock periods following a restart of the watchdog performed by > a write access in WDT_CR. Any modification will cause the watchdog > to trigger an end of period earlier than expected. > It matches the comments from Wenyou when he committed the sama5d4 watchdog driver to replace the existing at91sam9 watchdog. Best regards,
Hi, On Fri, 4 Mar 2016 16:26:59 +0100 Romain Izard wrote: > Hi Lothar, > > 2016-03-04 15:59 GMT+01:00 Lothar Waßmann <LW@karo-electronics.de>: > >> >>>>> I also check the WDT_MR register before and after enabling > >> >>>>> watchdog, the WDV and WDD fields are correct. > >> >>>>> > >> >>>>> Can you check it again? thank you. > >> >>> > >> >>> > >> >>> Working case: > >> >>> MR on kernel startup: 0x3fffafff > >> >>> MR after watchdog init: 0x0fffafff > >> >>> MR after start: 0x0fff2fff > >> >>> > >> >>> Problem case: > >> >>> MR on kernel startup: 0x00008000 > >> >>> MR after watchdog init: 0x0fffafff > >> >>> MR after start: 0x0fff2fff > >> >>> > >> >>> So this means that the counter reload does not seem to work very well > >> >>> if WDD/WDV have been set to 0 in the past. The other question is why > >> >>> does U-Boot (from the Atmel branch based on 2015.1) put this stange > >> >>> value in this register. > >> >>> > >> >> > >> >> Can you check the value of AT91_WDT_SR ? Maybe it tells us something. > >> >> > >> > I didn't report it because it contained 0 at all times. So no information. > >> > > >> >> Also, in the error case, can you check if the watchdog times out at all > >> >> after you applied your patch ? > >> > > >> > It times out after 16s as expected, and reboot occurs correctly. > >> > > >> > >> Interesting. So it looks like AT91_WDT_WDRSTT has to be set if the timer > >> values in MR are changed from 0 to another value, or maybe after each > >> timer value change. Wonder if that should be done in the init function, > >> after MR is set (with the watchdog disabled). > >> > >> Thoughts, anyone ? > >> > > > Are you aware of the Notes in the SAMA5D4 Reference Manual (Chapter > > 19.5.2 Watchdog Timer Mode Register): > > > > |Note: The first write access prevents any further modification of > > | the value of this register. Read accesses remain possible. > > |Note: The WDD and WDV values must not be modified within three slow > > | clock periods following a restart of the watchdog performed by > > | a write access in WDT_CR. Any modification will cause the watchdog > > | to trigger an end of period earlier than expected. > > This text is valid for older versions of the Watchdog controller, found > in AT91SAM9 and SAMA5D3 chips. But SAMA5D4 & SAMA5D2 have a newer > revision, which supports multiple writes to the MR register. > > Are you sure about your datasheet? I have this in the latest version > found on Atmel's site. > > > Atmel-11238B-ATARM-SAMA5D4-Datasheet_24-Aug-15 > > Section 18.5.2 > > OK, I obviously had an outdated Manual. Lothar Waßmann
diff --git a/drivers/watchdog/sama5d4_wdt.c b/drivers/watchdog/sama5d4_wdt.c index a49634cdc1cc..e162fe140ae1 100644 --- a/drivers/watchdog/sama5d4_wdt.c +++ b/drivers/watchdog/sama5d4_wdt.c @@ -15,6 +15,7 @@ #include <linux/platform_device.h> #include <linux/reboot.h> #include <linux/watchdog.h> +#include <linux/delay.h> #include "at91sam9_wdt.h" @@ -58,6 +59,8 @@ static int sama5d4_wdt_start(struct watchdog_device *wdd) reg = wdt_read(wdt, AT91_WDT_MR); reg &= ~AT91_WDT_WDDIS; wdt_write(wdt, AT91_WDT_MR, reg); + udelay(125); /* > 4 cycles at 32,768 Hz */ + wdt_write(wdt, AT91_WDT_CR, AT91_WDT_KEY | AT91_WDT_WDRSTT); return 0; }
If the internal counter is not refreshed when the watchdog is started for the first time, the watchdog will trigger very rapidly. For example, opening /dev/watchdog without writing in it will immediately trigger a reboot, instead of waiting for the delay to expire. To avoid this problem, reload the timer on opening the watchdog device. Command: "while sleep 5; do echo 1; done > /dev/watchdog" Before: system reset After: the watchdog runs correctly Signed-off-by: Romain Izard <romain.izard.pro@gmail.com> --- drivers/watchdog/sama5d4_wdt.c | 3 +++ 1 file changed, 3 insertions(+)