On 09. 04. 19 16:22, Stefan Roese wrote: > On 09.04.19 12:45, Michal Simek wrote: >> On 08. 04. 19 11:28, Stefan Roese wrote: >>> This patch tries to implement a generic watchdog_reset() function that >>> can be used by all boards that want to service the watchdog device in >>> U-Boot. This watchdog servicing is enabled via CONFIG_WATCHDOG. >>> >>> Without this approach, new boards or platforms needed to implement a >>> board specific version of this functionality, mostly copy'ing the same >>> code over and over again into their board or platforms code base. >>> >>> With this new generic function, the scattered other functions are now >>> removed to be replaced by the generic one. The new version also enables >>> the configuration of the watchdog timeout via the DT "timeout-sec" >>> property (if enabled via CONFIG_OF_CONTROL). >>> >>> This patch also adds a new flag to the GD flags, to flag that the >>> watchdog is ready to use and adds the pointer to the watchdog device >>> to the GD. This enables us to remove the global "watchdog_dev" >>> variable, which was prone to cause problems because of its potentially >>> very early use in watchdog_reset(), even before the BSS is cleared. >>> >>> Signed-off-by: Stefan Roese <[email protected]> >>> Cc: Heiko Schocher <[email protected]> >>> Cc: Tom Rini <[email protected]> >>> Cc: Michal Simek <[email protected]> >>> Cc: "Marek Behún" <[email protected]> >>> Cc: Daniel Schwierzeck <[email protected]> >>> --- >>> This patch depends on the following patches: >>> >>> [1] watchdog: Move watchdog_dev to data section (BSS may not be cleared) >>> https://patchwork.ozlabs.org/patch/1075500/ >>> >>> [2] watchdog: Handle SPL build with watchdog disabled >>> https://patchwork.ozlabs.org/patch/1074098/ >>> >>> I would like to see [1] applied in this release, since its a real fix on >>> some of the platforms with minimal chances of breakage. >>> >>> This patch now is a bigger rework and is intended for the next merge >>> window. >>> >>> Please note that I didn't remove the "timeout-sec" handling from the >>> driver already reading it from the DT (cdns & at91) in this patch. >>> The reason for this is, that in the cdns case, the removal also brings >>> a functional change, which I wanted to do in a separate patch. And >>> in the at91 case its because there are updates to this driver already >>> queued in the at91 next git branch which would most likely cause merge >>> conflict. I'll send a cleanup patch for this driver later after these >>> patches have been applied. >>> >>> Thanks, >>> Stefan >>> >>> v2: >>> - Rename watchdog_start() to initr_watchdog() and move it to board_r.c. >>> This way its only called once, after the DM subsystem has bound all >>> watchdog drivers. This enables the use of the currently implemented >>> logic of using the correct watchdog in U-Boot (via alias etc). >>> >>> arch/mips/mach-mt7620/cpu.c | 36 ----------------- >>> board/CZ.NIC/turris_mox/turris_mox.c | 30 -------------- >>> board/CZ.NIC/turris_omnia/turris_omnia.c | 35 ---------------- >>> .../microblaze-generic/microblaze-generic.c | 40 ------------------- >>> board/xilinx/zynq/board.c | 39 ------------------ >>> board/xilinx/zynqmp/zynqmp.c | 39 ------------------ >>> common/board_r.c | 40 +++++++++++++++++++ >>> drivers/watchdog/wdt-uclass.c | 26 ++++++++++++ >>> include/asm-generic/global_data.h | 4 ++ >>> 9 files changed, 70 insertions(+), 219 deletions(-) > > <snip> > >>> int board_early_init_r(void) >>> { >>> u32 val; >>> diff --git a/common/board_r.c b/common/board_r.c >>> index 472987d5d5..d80f16c3ed 100644 >>> --- a/common/board_r.c >>> +++ b/common/board_r.c >>> @@ -48,6 +48,7 @@ >>> #include <linux/compiler.h> >>> #include <linux/err.h> >>> #include <efi_loader.h> >>> +#include <wdt.h> >>> DECLARE_GLOBAL_DATA_PTR; >>> @@ -621,6 +622,42 @@ static int initr_bedbug(void) >>> } >>> #endif >>> +#if defined(CONFIG_WATCHDOG) && defined(CONFIG_WDT) >> >> This is not correct. >> Here should be just CONFIG_WDT. >> >> Because there are cases where you want just start watchdog in u-boot but >> never service it by u-boot. > > AFAIK, this would change the current behavior. Currently, only when > CONFIG_WATCHDOG is enabled the watchdog driver is started automatically > in U-Boot (and serviced). If I make the change you suggest above, all > boards defining CONFIG_WDT (DM watchdog support) will automatically > enable the watchdog. This might make sense, but AFAICT this changes > the current behavior.
The question is what default is. Because when I was adding support for Zynq/ZynqMP/Microblaze this logic is used there. I think that WDT should be there and if you think there are boards which want to have both we can cover that by Kconfig. Thanks, Michal _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
