u-boot leaves watchdog enabled by default

[Tom Rini]

On Mon, Sep 21, 2020 at 10:56:14PM +0200, Michael Walle wrote:
> Hi,
> 
> Am 2020-09-21 20:50, schrieb Tom Rini:
> > On Mon, Sep 21, 2020 at 08:29:00PM +0200, Heinrich Schuchardt wrote:
> > > On 9/21/20 7:30 PM, Tom Rini wrote:
> > > > On Mon, Sep 21, 2020 at 11:01:37AM +0200, Stefan Roese wrote:
> > > >> Hi Michael,
> > > >> Hi Chris,
> > > >>
> > > >> On 15.09.20 12:44, Chris Packham wrote:
> > > >>>
> > > >>>
> > > >>> On Tue, 15 Sep 2020, 7:54 PM Michael Walle, <michael at walle.cc> wrote:
> > > >>>
> > > >>>     Am 2020-09-15 09:44, schrieb Rayagonda Kokatanur:
> > > >>>      > On Tue, Sep 15, 2020 at 12:56 PM Michael Walle <michael at walle.cc>
> > > >>>      > wrote:
> > > >>>      >>
> > > >>>      >> Hi Stefan,
> > > >>>      >>
> > > >>>      >> it appears that since commit 06985289d45 ("watchdog: Implement
> > > >>>     generic
> > > >>>      >> watchdog_reset() version") - by default - the first watchdog is
> > > >>>      >> started
> > > >>>      >> unconditionally if CONFIG_WDT is set but never stopped before
> > > >>>     booting
> > > >>>      >> the operating system.
> > > >>>      >>
> > > >>>      >> Shouldn't it also be stopped uncondionally? What's worse is that on
> > > >>>      >> one
> > > >>>      >> board/arch the watchdog is stopped in arch_preboot_os() which is
> > > >>>     never
> > > 
> > > Which board are you referring to?
> 
> See the commit above. It is board/alliedtelesis/x530/x530.c. It might not
> use
> EFI, but I tried to use it as a blueprint to disable the watchdog by default
> and then noticed it won't work in the bootefi case (and I guess the 'go'
> case).
> 
> > > 
> > > >>>      >> called in the bootefi case. So even if I'd do a workaround and
> > > >>>     stop it
> > > >>>      >> manually in my board code, I couldn't do that consistently for
> > > >>>      >> bootm/bootefi.
> > > >>>      >>
> > > >>>      >> Or am I missing something here?
> > > >>>      >
> > > >>>      > Define CONFIG_WATCHDOG.
> > > >>>      > This takes care of resetting wdt.
> > > >>>
> > > >>>     Yes as along as you're inside the bootloader, but when u-boot hands
> > > >>>     control over the OS the watchdog is not serviced anymore; which wouldn't
> > > >>>     be a problem per se, but it is enabled unconditionally by u-boot.
> > > >>>
> > > >>>
> > > >>> Just to add some data. At $dayjob we use this behaviour as a failsafe to
> > > >>> make sure our userspace gets to a point where it is servicing the
> > > >>> watchdog.
> > > >>
> > > >> Yes, this is exactly how this is supposed to work AFAIK.
> > > >>
> > > >> Michael, are you sure that the watchdog was disabled in U-Boot when
> > > >> booting into the OS before this patch?
> > > >>
> > > >>> That said having a leave-wdt-running environment variable would work for
> > > >>> our use case.
> > > >>
> > > >> I would rather use it the other way around. Something like "wdt-stop-
> > > >> pre-os" to optionally stop the WDT before booting into the OS.
> > > >>
> > > >> Remark:
> > > >> IMHO, if you don't use the WDT in the OS, it does not make much sense
> > > >> to enable the WDT in U-Boot.
> > > >
> > > > Yes, we need to be very careful about making it so that a watchdog is
> > > > disabled and not re-enabled before moving on for a whole bunch of
> > > > reasons.  And the best option would be to just disable the watchdog if
> > > > it won't be used while the device is running the OS.
> > > >
> > > 
> > > The requirement of the UEFI specification is that if booting fails a
> > > system should reset after five minutes by default. We ensure this in
> > > the
> > > UEFI sub-system before ExitBootServices() using an EFI timer event.
> > > 
> > > In the UEFI sub-system we currently call in ExitBootServices():
> > > 
> > >         efi_set_watchdog(0); /* this disables the EFI timer */
> > >         WATCHDOG_RESET();
> > > 
> > > Is there any requirement to do more?
> > 
> > For EFI or ?  What I'm saying is that the watchdog must be left running
> > and not stopped, if we either:
> > - Came in to the world with the watchdog running AND were not
> >   specifically told to disable the watching.
> > - Came in to the world and were told to enable a watchdog.
> 
> My reason to start this thread was the fact that a watchdog is started
> by default in a generic way (i.e. initr_watchdog()) but there is _no_
> way to disable it. I'm having a minimal board configuration and I want

OK, but why is CONFIG_WDT enabled if you don't want to use the watchdog?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200921/e4407325/attachment.sig>