u-boot leaves watchdog enabled by default

[Michael Walle]

Hi Stefan,

Am 2020-09-21 11:01, schrieb Stefan Roese:
> 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
>>      >> 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?

If I read the patch correctly, it was per board, wasn't it?

At the moment you end up with a watchdog enabled if you have
  (a) CONFIG_WDT set,
  (b) A watchdog defined in the device tree

And no way to stop it reliably (well you could use some kind of bootcmd,
but I don't think that is the way to go).

>> 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.

I'm fine with either one. At least I can stop the watchdog reliably.

OTOH a configuration option to not start a watchdog in the first place,
should still be a valid choice.

> 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.

Keep in mind that my board is a "generic" module. I.e. there might be
customers which may want to start a watchdog and there might be 
customers
(that is also the debian-installer, for example) which doesn't want it
or have no driver for it. This makes even more sense with EFI in place,
which provides a universal boot method for any distribution/os (that
might not be aware that a watchdog is running).

-michael