Hi Tom,

Am 2020-09-22 16:41, schrieb Tom Rini:
On Tue, Sep 22, 2020 at 03:18:58PM +0200, Michael Walle wrote:
[..]
> But all that said, since we have "wdt stop", perhaps you can find a
> place to put that in the boot script?  Or just declare that if we get
> far enough to run preboot cmd then it's good enough, and update your
> call to "wdt expire 1" to be "wdt start && wdt expire 1" ?

"wdt expire 1" will automatically start the watchdog, won't it? Anyway,
it was just an example why I need the CONFIG_WDT.

Right, OK.  I'm just wondering if we can use the existing "wdt stop"
functionality to cover what you're aiming for.

See below.

Just to get your opinion correct on this topic: you say as soon as the
CONFIG_WDT is enabled u-boot will start it and never stop it, although
CONFIG_WDT is just "Enable driver model watchdog timer drivers" and the
help text is just about that, too.

I will agree that the help text and symbols can use further cleaning up
still.  CONFIG_WDT implies in CONFIG_WATCHDOG which says:

This option enables U-Boot watchdog support where U-Boot is using watchdog_reset function to service watchdog device in U-Boot. Enable this option if you want to service enabled watchdog by U-Boot. Disable this option if you want U-Boot to start watchdog but never service it.

Which is what we've done (to the best of my knowledge) "forever".

Yes CONFIG_WATCHDOG is implied, but if you disable CONFIG_WATCHDOG it will
still be started. Just not serviced.

IMHO it is wrong to enable the watchdog together with that option. There should be another one (even defaulting to 'yes') which tells u-boot whether
it should be enabled by default.

config WDT_AUTOSTART
   boot "Start the (first) watchdog by default"
   default y
   help
Upon u-boot startup the first watchdog will be started automatically. Be aware, that it will also kept enabled after the bootloader starts
     the operation system!

Now, given what I said above looking at commit 06985289d452 ("watchdog:
Implement generic watchdog_reset() version") is where we get the current behavior, symbol-wise. At this point, I'm not quite sure how best to do
what you're looking for, or if we just have a bug in terms of which
symbols are used.  It sounds like you just want to stop the watchdog in
U-Boot (outside of a specific start-and-trigger case) and let the OS
decide if it's going to enable it.  And if the OS is going to enable it
and you want the watchdog started before OS boot, preboot could be setf
in the environment to "wdt start" to get it going again (and you enable
CONFIG_USE_PREBOOT and set CONFIG_PREBOOT to an empty string, or wdt
stop?).  Or does that still not cover what you're trying to do?

There are two things I want(ed) to achieve:
 (1) While booting the d-i I noticed my board does watchdog resets. So
     I digged into this and noticed that since the commit in question,
     any watchdog will be started unconditionally, which looked wrong
     to me and is a bit of a suprising behaviour. Esp. when you inherit
     the device trees from linux where the SoC watchdogs are usually
     enabled.
     Also I looked into how you could disable this behavior per-board.
     I didn't find a reliable method that worked for any boot command.
     Therefore, I've started this discussion, to find out if this
     is the intended behavior.

 (2) To be able to boot an operating system with the boards default
     environment, that isn't aware of any watchdog.

For my specific use-case there are the following solutions so far:
 (a) stop the watchdog via "wdt stop" sometime
 (b) disable CONFIG_WDT
 (c) don't start the watchdog at all
 (d) have a runtime switch in the environment to stop it
     before booting the OS.

(a) and (b) is currently possible, (c) and (d) would need a patch.
(b) is out of question for me, because I need the u-boot wdt commands.
(a) sounds like a hack to me, why would you stop it even if I don't
want it to be started in the first place. So I'd prefer (c).

I see that someone might prefer (d) as it gives the user the choice
without having to recompile u-boot.

But apart from my use case, I could think of others and IMHO we
should leave the choice up to the board user (and making it as easy
as possible to configure it). So what do about the following:

choice
  prompt "Watchdog behaviour"
  default WDT_SUPERVISE_OS

config WDT_SUPERVISE_NOTHING
   boot "Supervise nothing"
   help
     No watchdog will be started.

config WDT_SUPERVISE_U_BOOT
   boot "Supervise u-boot"
   help
Upon u-boot startup the first watchdog will be started automatically
     and stopped as soon as an operating system is booted.

config WDT_SUPERVISE_OS
   boot "Supervise u-boot and operating system"
   help
Upon u-boot startup the first watchdog will be started automatically
     and kept running even after booting the operating system.
     Be aware, that the operating system needs to service the watchdog!

endchoice

-michael

Reply via email to