| Message ID | 1429701102-22320-9-git-send-email-timo.kokkonen@offcode.fi (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello, On Wed, Apr 22, 2015 at 02:11:42PM +0300, Timo Kokkonen wrote: > Use the new watchdog core extensions to let watchdog core take over > boot time watchdog behavior. The difference is that early-timeout-sec > device tree property becomes available for this driver and a running > watchdog is not stopped unless the core decides to stop it. > > Omap watchdog is running by default in the boot up but bootloader > might have stopped it. Therefore we fill the WDOG_HW_RUNNING_AT_BOOT > bit depending on the actual watchdog state so that the watchdog core > can act properly. > > Signed-off-by: Timo Kokkonen <timo.kokkonen@offcode.fi> > --- > drivers/watchdog/omap_wdt.c | 28 +++++++++++++++++++++++++++- > 1 file changed, 27 insertions(+), 1 deletion(-) > > diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c > index bbaf39a..7164f2e 100644 > --- a/drivers/watchdog/omap_wdt.c > +++ b/drivers/watchdog/omap_wdt.c > @@ -78,6 +78,13 @@ static void omap_wdt_reload(struct omap_wdt_dev *wdev) > /* reloaded WCRR from WLDR */ > } > > +static int omap_wdt_is_running(struct omap_wdt_dev *wdev) > +{ > + void __iomem *base = wdev->base; > + > + return readl_relaxed(base + OMAP_WATCHDOG_SPR) == 0x4444; > +} This isn't reliable. The sequence needed to enable the watchdog is writel(0xbbbb, base + OMAP_WATCHDOG_SPR); writel(0x4444, base + OMAP_WATCHDOG_SPR); The sequence to stop is: writel(0xaaaa, base + OMAP_WATCHDOG_SPR); writel(0x5555, base + OMAP_WATCHDOG_SPR); But: barebox@TI AM335x BeagleBone black:/ md 0x44e35048+4 44e35048: 00005555 UU.. barebox@TI AM335x BeagleBone black:/ mw 0x44e35048 0x4444 barebox@TI AM335x BeagleBone black:/ md 0x44e35048+4 44e35048: 00004444 DD.. So the register contains 0x4444 but the timer doesn't run. So at best testing for 0x4444 is a good heuristic. Best regards Uwe
Hi, 03.05.2015 21:56, Uwe Kleine-König wrote: > Hello, > > On Wed, Apr 22, 2015 at 02:11:42PM +0300, Timo Kokkonen wrote: >> Use the new watchdog core extensions to let watchdog core take over >> boot time watchdog behavior. The difference is that early-timeout-sec >> device tree property becomes available for this driver and a running >> watchdog is not stopped unless the core decides to stop it. >> >> Omap watchdog is running by default in the boot up but bootloader >> might have stopped it. Therefore we fill the WDOG_HW_RUNNING_AT_BOOT >> bit depending on the actual watchdog state so that the watchdog core >> can act properly. >> >> Signed-off-by: Timo Kokkonen <timo.kokkonen@offcode.fi> >> --- >> drivers/watchdog/omap_wdt.c | 28 +++++++++++++++++++++++++++- >> 1 file changed, 27 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c >> index bbaf39a..7164f2e 100644 >> --- a/drivers/watchdog/omap_wdt.c >> +++ b/drivers/watchdog/omap_wdt.c >> @@ -78,6 +78,13 @@ static void omap_wdt_reload(struct omap_wdt_dev *wdev) >> /* reloaded WCRR from WLDR */ >> } >> >> +static int omap_wdt_is_running(struct omap_wdt_dev *wdev) >> +{ >> + void __iomem *base = wdev->base; >> + >> + return readl_relaxed(base + OMAP_WATCHDOG_SPR) == 0x4444; >> +} > This isn't reliable. The sequence needed to enable the watchdog is > writel(0xbbbb, base + OMAP_WATCHDOG_SPR); > writel(0x4444, base + OMAP_WATCHDOG_SPR); > > The sequence to stop is: > writel(0xaaaa, base + OMAP_WATCHDOG_SPR); > writel(0x5555, base + OMAP_WATCHDOG_SPR); > > But: > > barebox@TI AM335x BeagleBone black:/ md 0x44e35048+4 > 44e35048: 00005555 UU.. > barebox@TI AM335x BeagleBone black:/ mw 0x44e35048 0x4444 > barebox@TI AM335x BeagleBone black:/ md 0x44e35048+4 > 44e35048: 00004444 DD.. > > So the register contains 0x4444 but the timer doesn't run. So at best > testing for 0x4444 is a good heuristic. Yeah.. I don't think we can get any better than that. Unless we start checking the counter register and see whether it really counts or not, and I think that's a bit overkill.. So I'd say we should be safe when assuming bootloader is doing things correctly. Although, we could add a comment to the code that the test may not be 100% reliable in case the start sequence have not been issued properly. Thanks for pointing this out! -Timo
Hello Timo, On Mon, May 04, 2015 at 08:59:03AM +0300, Timo Kokkonen wrote: > Hi, 03.05.2015 21:56, Uwe Kleine-König wrote: > >On Wed, Apr 22, 2015 at 02:11:42PM +0300, Timo Kokkonen wrote: > >>+static int omap_wdt_is_running(struct omap_wdt_dev *wdev) > >>+{ > >>+ void __iomem *base = wdev->base; > >>+ > >>+ return readl_relaxed(base + OMAP_WATCHDOG_SPR) == 0x4444; > >>+} > >This isn't reliable. The sequence needed to enable the watchdog is > > writel(0xbbbb, base + OMAP_WATCHDOG_SPR); > > writel(0x4444, base + OMAP_WATCHDOG_SPR); > > > >The sequence to stop is: > > writel(0xaaaa, base + OMAP_WATCHDOG_SPR); > > writel(0x5555, base + OMAP_WATCHDOG_SPR); > > > >But: > > > >barebox@TI AM335x BeagleBone black:/ md 0x44e35048+4 > >44e35048: 00005555 UU.. > >barebox@TI AM335x BeagleBone black:/ mw 0x44e35048 0x4444 > >barebox@TI AM335x BeagleBone black:/ md 0x44e35048+4 > >44e35048: 00004444 DD.. > > > >So the register contains 0x4444 but the timer doesn't run. So at best > >testing for 0x4444 is a good heuristic. > > Yeah.. I don't think we can get any better than that. Unless we > start checking the counter register and see whether it really counts > or not, and I think that's a bit overkill.. So I'd say we should be > safe when assuming bootloader is doing things correctly. Although, > we could add a comment to the code that the test may not be 100% > reliable in case the start sequence have not been issued properly. > > Thanks for pointing this out! It doesn't seem to much overhead to do: /* * There is no register that tells us if the timer is running, * so we have to resort to sample twice. The minimal frequency * is 256 Hz (32768 Hz prescaled with 2**7). */ counter1 = readl(base + OMAP_WATCHDOG_CCR); mdelay(4); counter2 = readl(base + OMAP_WATCHDOG_CCR); return counter1 != counter2; I'd say it's even worth to do: cntrl = readl(base + OMAP_WATCHDOG_CNTRL); if (cntrl & (1 << 5)) shift = (cntrl >> 2) & 0x7; else shift = 0; counter1 = readl(base + OMAP_WATCHDOG_CCR); udelay(31 << shift); counter2 = readl(base + OMAP_WATCHDOG_CCR); return counter1 != counter2; For some bonus points add some defines for the magic constants. This is save as the OMAP_WATCHDOG_CNTRL doesn't seem to accept reads while the counter is running. Maybe even this could be used to detect a running timer?: - enable timer: barebox@TI AM335x BeagleBone black:/ mw 0x44e35048 0xbbbb barebox@TI AM335x BeagleBone black:/ mw 0x44e35048 0x4444 - read out WCLR barebox@TI AM335x BeagleBone black:/ md 0x44e35024+4 44e35024: 00000020 ... - write to WCLR barebox@TI AM335x BeagleBone black:/ mw 0x44e35024 0x0 - check result; didn't work barebox@TI AM335x BeagleBone black:/ md 0x44e35024+4 44e35024: 00000020 ... - stop timer barebox@TI AM335x BeagleBone black:/ mw 0x44e35048 0xaaaa barebox@TI AM335x BeagleBone black:/ mw 0x44e35048 0x5555 - recheck WCLR barebox@TI AM335x BeagleBone black:/ md 0x44e35024+4 44e35024: 00000020 ... - write to WCLR barebox@TI AM335x BeagleBone black:/ mw 0x44e35024 0x0 - check result; write succeeded barebox@TI AM335x BeagleBone black:/ md 0x44e35024+4 44e35024: 00000000 .... (This is was tested on an AM335x.) Best regards Uwe
On 04.05.2015 10:04, Uwe Kleine-König wrote: > Hello Timo, > > On Mon, May 04, 2015 at 08:59:03AM +0300, Timo Kokkonen wrote: >> Hi, 03.05.2015 21:56, Uwe Kleine-König wrote: >>> On Wed, Apr 22, 2015 at 02:11:42PM +0300, Timo Kokkonen wrote: >>>> +static int omap_wdt_is_running(struct omap_wdt_dev *wdev) >>>> +{ >>>> + void __iomem *base = wdev->base; >>>> + >>>> + return readl_relaxed(base + OMAP_WATCHDOG_SPR) == 0x4444; >>>> +} >>> This isn't reliable. The sequence needed to enable the watchdog is >>> writel(0xbbbb, base + OMAP_WATCHDOG_SPR); >>> writel(0x4444, base + OMAP_WATCHDOG_SPR); >>> >>> The sequence to stop is: >>> writel(0xaaaa, base + OMAP_WATCHDOG_SPR); >>> writel(0x5555, base + OMAP_WATCHDOG_SPR); >>> >>> But: >>> >>> barebox@TI AM335x BeagleBone black:/ md 0x44e35048+4 >>> 44e35048: 00005555 UU.. >>> barebox@TI AM335x BeagleBone black:/ mw 0x44e35048 0x4444 >>> barebox@TI AM335x BeagleBone black:/ md 0x44e35048+4 >>> 44e35048: 00004444 DD.. >>> >>> So the register contains 0x4444 but the timer doesn't run. So at best >>> testing for 0x4444 is a good heuristic. >> >> Yeah.. I don't think we can get any better than that. Unless we >> start checking the counter register and see whether it really counts >> or not, and I think that's a bit overkill.. So I'd say we should be >> safe when assuming bootloader is doing things correctly. Although, >> we could add a comment to the code that the test may not be 100% >> reliable in case the start sequence have not been issued properly. >> >> Thanks for pointing this out! > It doesn't seem to much overhead to do: > > /* > * There is no register that tells us if the timer is running, > * so we have to resort to sample twice. The minimal frequency > * is 256 Hz (32768 Hz prescaled with 2**7). > */ > counter1 = readl(base + OMAP_WATCHDOG_CCR); > mdelay(4); > counter2 = readl(base + OMAP_WATCHDOG_CCR); > return counter1 != counter2; > > I'd say it's even worth to do: > > cntrl = readl(base + OMAP_WATCHDOG_CNTRL); > if (cntrl & (1 << 5)) > shift = (cntrl >> 2) & 0x7; > else > shift = 0; > counter1 = readl(base + OMAP_WATCHDOG_CCR); > udelay(31 << shift); > counter2 = readl(base + OMAP_WATCHDOG_CCR); > return counter1 != counter2; > I think it is very rare that the watchdog is running in any other than 32kHz clock, so the above should be sufficient in any case. The worst case delay of 3968us is a bit long, but even that is happening only during probe. And vast majority of user are going to have the 31us delay, so that should be fine. Thanks, -Timo > For some bonus points add some defines for the magic constants. > > This is save as the OMAP_WATCHDOG_CNTRL doesn't seem to accept reads > while the counter is running. Maybe even this could be used to detect a > running timer?: > > - enable timer: > barebox@TI AM335x BeagleBone black:/ mw 0x44e35048 0xbbbb > barebox@TI AM335x BeagleBone black:/ mw 0x44e35048 0x4444 > > - read out WCLR > barebox@TI AM335x BeagleBone black:/ md 0x44e35024+4 > 44e35024: 00000020 ... > > - write to WCLR > barebox@TI AM335x BeagleBone black:/ mw 0x44e35024 0x0 > > - check result; didn't work > barebox@TI AM335x BeagleBone black:/ md 0x44e35024+4 > 44e35024: 00000020 ... > > - stop timer > barebox@TI AM335x BeagleBone black:/ mw 0x44e35048 0xaaaa > barebox@TI AM335x BeagleBone black:/ mw 0x44e35048 0x5555 > > - recheck WCLR > barebox@TI AM335x BeagleBone black:/ md 0x44e35024+4 > 44e35024: 00000020 ... > > - write to WCLR > barebox@TI AM335x BeagleBone black:/ mw 0x44e35024 0x0 > > - check result; write succeeded > barebox@TI AM335x BeagleBone black:/ md 0x44e35024+4 > 44e35024: 00000000 .... > > (This is was tested on an AM335x.) > > Best regards > Uwe >
On Mon, May 04, 2015 at 08:59:03AM +0300, Timo Kokkonen wrote: > Hi, 03.05.2015 21:56, Uwe Kleine-König wrote: > >Hello, > > > >On Wed, Apr 22, 2015 at 02:11:42PM +0300, Timo Kokkonen wrote: > >>Use the new watchdog core extensions to let watchdog core take over > >>boot time watchdog behavior. The difference is that early-timeout-sec > >>device tree property becomes available for this driver and a running > >>watchdog is not stopped unless the core decides to stop it. > >> > >>Omap watchdog is running by default in the boot up but bootloader > >>might have stopped it. Therefore we fill the WDOG_HW_RUNNING_AT_BOOT > >>bit depending on the actual watchdog state so that the watchdog core > >>can act properly. > >> > >>Signed-off-by: Timo Kokkonen <timo.kokkonen@offcode.fi> > >>--- > >> drivers/watchdog/omap_wdt.c | 28 +++++++++++++++++++++++++++- > >> 1 file changed, 27 insertions(+), 1 deletion(-) > >> > >>diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c > >>index bbaf39a..7164f2e 100644 > >>--- a/drivers/watchdog/omap_wdt.c > >>+++ b/drivers/watchdog/omap_wdt.c > >>@@ -78,6 +78,13 @@ static void omap_wdt_reload(struct omap_wdt_dev *wdev) > >> /* reloaded WCRR from WLDR */ > >> } > >> > >>+static int omap_wdt_is_running(struct omap_wdt_dev *wdev) > >>+{ > >>+ void __iomem *base = wdev->base; > >>+ > >>+ return readl_relaxed(base + OMAP_WATCHDOG_SPR) == 0x4444; > >>+} > >This isn't reliable. The sequence needed to enable the watchdog is > > writel(0xbbbb, base + OMAP_WATCHDOG_SPR); > > writel(0x4444, base + OMAP_WATCHDOG_SPR); > > > >The sequence to stop is: > > writel(0xaaaa, base + OMAP_WATCHDOG_SPR); > > writel(0x5555, base + OMAP_WATCHDOG_SPR); > > > >But: > > > >barebox@TI AM335x BeagleBone black:/ md 0x44e35048+4 > >44e35048: 00005555 UU.. > >barebox@TI AM335x BeagleBone black:/ mw 0x44e35048 0x4444 > >barebox@TI AM335x BeagleBone black:/ md 0x44e35048+4 > >44e35048: 00004444 DD.. > > > >So the register contains 0x4444 but the timer doesn't run. So at best > >testing for 0x4444 is a good heuristic. > > Yeah.. I don't think we can get any better than that. Unless we start > checking the counter register and see whether it really counts or not, and I > think that's a bit overkill.. So I'd say we should be safe when assuming You sure ? I would prefer Uwe's suggestion to check the counter > bootloader is doing things correctly. Although, we could add a comment to ... because I don't really trust the bootloader that much. If people are not happy with the resulting 4ms added boot time, they should fix the hardware. Otherwise we'll have to deal with people submitting bug reports because their system experiences "unexpected" resets, and I don't really want to have to deal with this. Guenter
On 04.05.2015 10:04, Uwe Kleine-König wrote: > Hello Timo, > > On Mon, May 04, 2015 at 08:59:03AM +0300, Timo Kokkonen wrote: >> Hi, 03.05.2015 21:56, Uwe Kleine-König wrote: >>> On Wed, Apr 22, 2015 at 02:11:42PM +0300, Timo Kokkonen wrote: >>>> +static int omap_wdt_is_running(struct omap_wdt_dev *wdev) >>>> +{ >>>> + void __iomem *base = wdev->base; >>>> + >>>> + return readl_relaxed(base + OMAP_WATCHDOG_SPR) == 0x4444; >>>> +} >>> This isn't reliable. The sequence needed to enable the watchdog is >>> writel(0xbbbb, base + OMAP_WATCHDOG_SPR); >>> writel(0x4444, base + OMAP_WATCHDOG_SPR); >>> >>> The sequence to stop is: >>> writel(0xaaaa, base + OMAP_WATCHDOG_SPR); >>> writel(0x5555, base + OMAP_WATCHDOG_SPR); >>> >>> But: >>> >>> barebox@TI AM335x BeagleBone black:/ md 0x44e35048+4 >>> 44e35048: 00005555 UU.. >>> barebox@TI AM335x BeagleBone black:/ mw 0x44e35048 0x4444 >>> barebox@TI AM335x BeagleBone black:/ md 0x44e35048+4 >>> 44e35048: 00004444 DD.. >>> >>> So the register contains 0x4444 but the timer doesn't run. So at best >>> testing for 0x4444 is a good heuristic. >> >> Yeah.. I don't think we can get any better than that. Unless we >> start checking the counter register and see whether it really counts >> or not, and I think that's a bit overkill.. So I'd say we should be >> safe when assuming bootloader is doing things correctly. Although, >> we could add a comment to the code that the test may not be 100% >> reliable in case the start sequence have not been issued properly. >> >> Thanks for pointing this out! > It doesn't seem to much overhead to do: > > /* > * There is no register that tells us if the timer is running, > * so we have to resort to sample twice. The minimal frequency > * is 256 Hz (32768 Hz prescaled with 2**7). > */ > counter1 = readl(base + OMAP_WATCHDOG_CCR); > mdelay(4); > counter2 = readl(base + OMAP_WATCHDOG_CCR); > return counter1 != counter2; > > I'd say it's even worth to do: > > cntrl = readl(base + OMAP_WATCHDOG_CNTRL); > if (cntrl & (1 << 5)) > shift = (cntrl >> 2) & 0x7; > else > shift = 0; > counter1 = readl(base + OMAP_WATCHDOG_CCR); > udelay(31 << shift); > counter2 = readl(base + OMAP_WATCHDOG_CCR); > return counter1 != counter2; > > For some bonus points add some defines for the magic constants. > > This is save as the OMAP_WATCHDOG_CNTRL doesn't seem to accept reads > while the counter is running. Maybe even this could be used to detect a > running timer?: Maybe, but you will get a data abort when the HW doesn't accept a read. How do you recover from that? Also, it seems that for some reason the watchdog HW is very picky about which registers can be read in what condition. If I add the code to read watchdog counter register, I will get a data abort later at the end of the probe when the watchdog revision is read. I don't understand why that happens, this does not seem logica. Maybe it has got something to do with the fact that the watchdog is stopped? If I can't read the counter register, I can't really implement the above heuristic.. Any ideas? -Timo > > - enable timer: > barebox@TI AM335x BeagleBone black:/ mw 0x44e35048 0xbbbb > barebox@TI AM335x BeagleBone black:/ mw 0x44e35048 0x4444 > > - read out WCLR > barebox@TI AM335x BeagleBone black:/ md 0x44e35024+4 > 44e35024: 00000020 ... > > - write to WCLR > barebox@TI AM335x BeagleBone black:/ mw 0x44e35024 0x0 > > - check result; didn't work > barebox@TI AM335x BeagleBone black:/ md 0x44e35024+4 > 44e35024: 00000020 ... > > - stop timer > barebox@TI AM335x BeagleBone black:/ mw 0x44e35048 0xaaaa > barebox@TI AM335x BeagleBone black:/ mw 0x44e35048 0x5555 > > - recheck WCLR > barebox@TI AM335x BeagleBone black:/ md 0x44e35024+4 > 44e35024: 00000020 ... > > - write to WCLR > barebox@TI AM335x BeagleBone black:/ mw 0x44e35024 0x0 > > - check result; write succeeded > barebox@TI AM335x BeagleBone black:/ md 0x44e35024+4 > 44e35024: 00000000 .... > > (This is was tested on an AM335x.) > > Best regards > Uwe >
Hello Timo, On Thu, May 07, 2015 at 09:42:44AM +0300, Timo Kokkonen wrote: > On 04.05.2015 10:04, Uwe Kleine-König wrote: > >On Mon, May 04, 2015 at 08:59:03AM +0300, Timo Kokkonen wrote: > >>Hi, 03.05.2015 21:56, Uwe Kleine-König wrote: > >>>On Wed, Apr 22, 2015 at 02:11:42PM +0300, Timo Kokkonen wrote: > >>>>+static int omap_wdt_is_running(struct omap_wdt_dev *wdev) > >>>>+{ > >>>>+ void __iomem *base = wdev->base; > >>>>+ > >>>>+ return readl_relaxed(base + OMAP_WATCHDOG_SPR) == 0x4444; > >>>>+} > >>>This isn't reliable. The sequence needed to enable the watchdog is > >>> writel(0xbbbb, base + OMAP_WATCHDOG_SPR); > >>> writel(0x4444, base + OMAP_WATCHDOG_SPR); > >>> > >>>The sequence to stop is: > >>> writel(0xaaaa, base + OMAP_WATCHDOG_SPR); > >>> writel(0x5555, base + OMAP_WATCHDOG_SPR); > >>> > >>>But: > >>> > >>>barebox@TI AM335x BeagleBone black:/ md 0x44e35048+4 > >>>44e35048: 00005555 UU.. > >>>barebox@TI AM335x BeagleBone black:/ mw 0x44e35048 0x4444 > >>>barebox@TI AM335x BeagleBone black:/ md 0x44e35048+4 > >>>44e35048: 00004444 DD.. > >>> > >>>So the register contains 0x4444 but the timer doesn't run. So at best > >>>testing for 0x4444 is a good heuristic. > >> > >>Yeah.. I don't think we can get any better than that. Unless we > >>start checking the counter register and see whether it really counts > >>or not, and I think that's a bit overkill.. So I'd say we should be > >>safe when assuming bootloader is doing things correctly. Although, > >>we could add a comment to the code that the test may not be 100% > >>reliable in case the start sequence have not been issued properly. > >> > >>Thanks for pointing this out! > >It doesn't seem to much overhead to do: > > > > /* > > * There is no register that tells us if the timer is running, > > * so we have to resort to sample twice. The minimal frequency > > * is 256 Hz (32768 Hz prescaled with 2**7). > > */ > > counter1 = readl(base + OMAP_WATCHDOG_CCR); > > mdelay(4); > > counter2 = readl(base + OMAP_WATCHDOG_CCR); > > return counter1 != counter2; > > > >I'd say it's even worth to do: > > > > cntrl = readl(base + OMAP_WATCHDOG_CNTRL); > > if (cntrl & (1 << 5)) > > shift = (cntrl >> 2) & 0x7; > > else > > shift = 0; > > counter1 = readl(base + OMAP_WATCHDOG_CCR); > > udelay(31 << shift); > > counter2 = readl(base + OMAP_WATCHDOG_CCR); > > return counter1 != counter2; > > > >For some bonus points add some defines for the magic constants. > > > >This is save as the OMAP_WATCHDOG_CNTRL doesn't seem to accept reads s/reads/writes/ in the above line. > >while the counter is running. Maybe even this could be used to detect a > >running timer?: > > Maybe, but you will get a data abort when the HW doesn't accept a > read. How do you recover from that? > > Also, it seems that for some reason the watchdog HW is very picky > about which registers can be read in what condition. If I add the > code to read watchdog counter register, I will get a data abort > later at the end of the probe when the watchdog revision is read. I > don't understand why that happens, this does not seem logica. Maybe > it has got something to do with the fact that the watchdog is > stopped? If I can't read the counter register, I can't really > implement the above heuristic.. Any ideas? Which SoC do you use for your tests? Sounds strange. If you provide me your wip patch I can take a look. Best regards Uwe
diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c index bbaf39a..7164f2e 100644 --- a/drivers/watchdog/omap_wdt.c +++ b/drivers/watchdog/omap_wdt.c @@ -78,6 +78,13 @@ static void omap_wdt_reload(struct omap_wdt_dev *wdev) /* reloaded WCRR from WLDR */ } +static int omap_wdt_is_running(struct omap_wdt_dev *wdev) +{ + void __iomem *base = wdev->base; + + return readl_relaxed(base + OMAP_WATCHDOG_SPR) == 0x4444; +} + static void omap_wdt_enable(struct omap_wdt_dev *wdev) { void __iomem *base = wdev->base; @@ -183,6 +190,7 @@ static int omap_wdt_set_timeout(struct watchdog_device *wdog, omap_wdt_enable(wdev); omap_wdt_reload(wdev); wdog->timeout = timeout; + wdog->hw_heartbeat = timeout * HZ / 2; mutex_unlock(&wdev->lock); return 0; @@ -235,12 +243,15 @@ static int omap_wdt_probe(struct platform_device *pdev) omap_wdt->ops = &omap_wdt_ops; omap_wdt->min_timeout = TIMER_MARGIN_MIN; omap_wdt->max_timeout = TIMER_MARGIN_MAX; + omap_wdt->hw_max_timeout = TIMER_MARGIN_MAX * HZ; + omap_wdt->hw_features = WDOG_HW_IS_STOPPABLE; if (timer_margin >= TIMER_MARGIN_MIN && timer_margin <= TIMER_MARGIN_MAX) omap_wdt->timeout = timer_margin; else omap_wdt->timeout = TIMER_MARGIN_DEFAULT; + omap_wdt->hw_heartbeat = omap_wdt->timeout * HZ / 2; watchdog_set_drvdata(omap_wdt, wdev); watchdog_set_nowayout(omap_wdt, nowayout); @@ -250,6 +261,18 @@ static int omap_wdt_probe(struct platform_device *pdev) pm_runtime_enable(wdev->dev); pm_runtime_get_sync(wdev->dev); + if (omap_wdt_is_running(wdev)) + omap_wdt->hw_features |= WDOG_HW_RUNNING_AT_BOOT; + else + /* + * The watchdog should be stopped already by + * bootloader. But unless we call disable here, the + * timeout might not be set correctly on the first + * start. So call disable anyway to make sure the + * watchdog really is stopped properly. + */ + omap_wdt_disable(wdev); + if (pdata && pdata->read_reset_sources) rs = pdata->read_reset_sources(); else @@ -257,7 +280,9 @@ static int omap_wdt_probe(struct platform_device *pdev) omap_wdt->bootstatus = (rs & (1 << OMAP_MPU_WD_RST_SRC_ID_SHIFT)) ? WDIOF_CARDRESET : 0; - omap_wdt_disable(wdev); + ret = watchdog_init_params(omap_wdt, &pdev->dev); + if (ret) + goto err_init_params; ret = watchdog_register_device(omap_wdt); if (ret) @@ -271,6 +296,7 @@ static int omap_wdt_probe(struct platform_device *pdev) return 0; +err_init_params: err_register_device: pm_runtime_disable(wdev->dev); err_ioremap:
Use the new watchdog core extensions to let watchdog core take over boot time watchdog behavior. The difference is that early-timeout-sec device tree property becomes available for this driver and a running watchdog is not stopped unless the core decides to stop it. Omap watchdog is running by default in the boot up but bootloader might have stopped it. Therefore we fill the WDOG_HW_RUNNING_AT_BOOT bit depending on the actual watchdog state so that the watchdog core can act properly. Signed-off-by: Timo Kokkonen <timo.kokkonen@offcode.fi> --- drivers/watchdog/omap_wdt.c | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-)