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 <ma...@denx.de>
Cc: Pali Rohar <p...@kernel.org>
Cc: Stefan Roese <s...@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.
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-672e058a8...@denx.de/
$ git describe --contains 5fc094351381c4254098a25404d8712324b6918e
v2021.10-rc1~19^2
commit 5fc094351381c4254098a25404d8712324b6918e
Author: Teresa Remmet <t.rem...@phytec.de>
Date: Thu Jul 15 13:26:32 2021 +0200
drivers: watchdog: wdt-uclass: Use IS_ENABLED for WATCHDOG_AUTOSTART
There is no separate SPL/TPL config for WATCHDOG_AUTOSTART.
So use IS_ENABLED instead of CONFIG_IS_ENABLED to make watchdog
working in SPL again.
Fixes: 830d29ac3721 ("watchdog: Allow to use CONFIG_WDT without
starting watchdog")
Signed-off-by: Teresa Remmet <t.rem...@phytec.de>
Reviewed-by: Stefan Roese <s...@denx.de>
diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
index a0c2429e5a43..17334dbda6c9 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -53,7 +53,7 @@ int initr_watchdog(void)
4 * reset_period) / 4;
}
- if (!CONFIG_IS_ENABLED(WATCHDOG_AUTOSTART)) {
+ if (!IS_ENABLED(CONFIG_WATCHDOG_AUTOSTART)) {
printf("WDT: Not starting\n");
return 0;
}
