On 9/2/21 9:49 AM, Stefan Roese wrote:
On 02.09.21 09:30, Marcel Ziswiler wrote:
On Tue, 2021-08-31 at 11:17 +0200, Marek Vasut wrote:
On 8/31/21 8:49 AM, Marcel Ziswiler wrote:
Hi Marek
On Tue, 2021-08-31 at 00:03 +0200, Marek Vasut wrote:
Commit 830d29ac372 ("watchdog: Allow to use CONFIG_WDT without starting watchdog") completely broke WDT operation in both SPL and TPL, in either case those WDTs are never enabled. Fix it by filling in the missing Kconfig options for SPL and TPL.
Fixes: 830d29ac372 ("watchdog: Allow to use CONFIG_WDT without starting watchdog") Signed-off-by: Marek Vasut marex@denx.de Cc: Pali Rohar pali@kernel.org Cc: Stefan Roese sr@denx.de
drivers/watchdog/Kconfig | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index f0ff2612a6b..65d974c4dd5 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -273,4 +273,24 @@ config SPL_WDT Enable driver model for watchdog timer in SPL. This is similar to CONFIG_WDT in U-Boot. +config SPL_WATCHDOG_AUTOSTART + bool "Automatically start watchdog timer in SPL" + depends on SPL && WDT + default y + help + Automatically start watchdog timer and start servicing it during + SPL phase. Enabled by default. Disable this option if you want + to compile U-Boot with CONFIG_WDT support but do not want to + activate watchdog, like when CONFIG_WDT option is disabled.
+config TPL_WATCHDOG_AUTOSTART + bool "Automatically start watchdog timer in TPL" + depends on TPL && WDT + default y + help + Automatically start watchdog timer and start servicing it during + TPL phase. Enabled by default. Disable this option if you want + to compile U-Boot with CONFIG_WDT support but do not want to + activate watchdog, like when CONFIG_WDT option is disabled.
endmenu
Those Kconfig entries look fine. However, I am wondering where exactly they get used. Am I missing anything?
Have a look at the patch this Fixes:, if those Kconfig entries are not present, the WDT is disabled in SPL and TPL unconditionally.
Yes, I did. But I guess I was not aware of how exactly that CONFIG_IS_ENABLED(WATCHDOG_AUTOSTART) behaves. While I have not really found this documented or the implementation thereof I assume it pre-fixes config stuff with SPL_ resp. TPL_ when building those flavors, correct?
Yes.
I am wondering how many others are unaware of this (just like the author of the patch this fixes). Maybe we should at least document this properly somewhere, not?
If this is not the case (I did not check this), then yes. A documentation for these CONFIG_IS_ENABLED() and IS_ENABLED() helpers would be very good.
You can possibly even auto-lint for CONFIG_IS_ENABLED(...)/IS_ENABLED(...) usage that is missing the Kconfig entries and warn about that, to prevent patches that break SPL/TPL .
It could be that if you're using some non-free or proprietary preloader instead of SPL, you won't run into this problem.
No, don't worry. I am not running anything evil (;-p).
You won't run into any issues with this problem, as it is fixed in a different way in master already, as Rasmus already pointed out:
https://lore.kernel.org/u-boot/f015f55e-3225-655e-2e8b-672e058a8ae5@denx.de/
$ git describe --contains 5fc094351381c4254098a25404d8712324b6918e v2021.10-rc1~19^2
commit 5fc094351381c4254098a25404d8712324b6918e Author: Teresa Remmet t.remmet@phytec.de Date: Thu Jul 15 13:26:32 2021 +0200
Yes.