[U-Boot] [PATCH 1/2 v2] watchdog: Implement generic watchdog_reset() version
Stefan Roese
sr at denx.de
Tue Apr 9 14:22:02 UTC 2019
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 <sr at denx.de>
>> Cc: Heiko Schocher <hs at denx.de>
>> Cc: Tom Rini <trini at konsulko.com>
>> Cc: Michal Simek <michal.simek at xilinx.com>
>> Cc: "Marek Behún" <marek.behun at nic.cz>
>> Cc: Daniel Schwierzeck <daniel.schwierzeck at gmail.com>
>> ---
>> 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.
Thanks,
Stefan
>
>> +#ifndef WDT_DEFAULT_TIMEOUT
>> +#define WDT_DEFAULT_TIMEOUT 60
>> +#endif
>> +
>> +static int initr_watchdog(void)
>> +{
>> + u32 timeout = WDT_DEFAULT_TIMEOUT;
>> +
>> + /*
>> + * Init watchdog: This will call the probe function of the
>> + * watchdog driver, enabling the use of the device
>> + */
>> + if (uclass_get_device_by_seq(UCLASS_WDT, 0,
>> + (struct udevice **)&gd->watchdog_dev)) {
>> + debug("WDT: Not found by seq!\n");
>> + if (uclass_get_device(UCLASS_WDT, 0,
>> + (struct udevice **)&gd->watchdog_dev)) {
>> + printf("WDT: Not found!\n");
>> + return 0;
>> + }
>> + }
>> +
>> + if (CONFIG_IS_ENABLED(OF_CONTROL)) {
>> + timeout = dev_read_u32_default(gd->watchdog_dev, "timeout-sec",
>> + WDT_DEFAULT_TIMEOUT);
>> + }
>> +
>> + wdt_start(gd->watchdog_dev, timeout * 1000, 0);
>> + gd->flags |= GD_FLG_WDT_READY;
>> + printf("WDT: Started (%ds timeout)\n", timeout);
>> +
>> + return 0;
>> +}
>> +#endif
>> +
>> static int run_main_loop(void)
>> {
>> #ifdef CONFIG_SANDBOX
>> @@ -670,6 +707,9 @@ static init_fnc_t init_sequence_r[] = {
>> #ifdef CONFIG_DM
>> initr_dm,
>> #endif
>> +#if defined(CONFIG_WATCHDOG) && defined(CONFIG_WDT)
>
> ditto.
>
>> + initr_watchdog,
>> +#endif
>> #if defined(CONFIG_ARM) || defined(CONFIG_NDS32) || defined(CONFIG_RISCV) || \
>> defined(CONFIG_SANDBOX)
>> board_init, /* Setup chipselects */
>> diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
>> index 23b7e3360d..bbfac4f0f9 100644
>> --- a/drivers/watchdog/wdt-uclass.c
>> +++ b/drivers/watchdog/wdt-uclass.c
>> @@ -10,6 +10,8 @@
>> #include <dm/device-internal.h>
>> #include <dm/lists.h>
>>
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags)
>> {
>> const struct wdt_ops *ops = device_get_ops(dev);
>> @@ -63,6 +65,30 @@ int wdt_expire_now(struct udevice *dev, ulong flags)
>> return ret;
>> }
>>
>> +#if defined(CONFIG_WATCHDOG)
>> +/*
>> + * Called by macro WATCHDOG_RESET. This function be called *very* early,
>> + * so we need to make sure, that the watchdog driver is ready before using
>> + * it in this function.
>> + */
>> +void watchdog_reset(void)
>> +{
>> + static ulong next_reset;
>> + ulong now;
>> +
>> + /* Exit if GD is not ready or watchdog is not initialized yet */
>> + if (!gd || !(gd->flags & GD_FLG_WDT_READY))
>> + return;
>> +
>> + /* Do not reset the watchdog too often */
>> + now = get_timer(0);
>> + if (now > next_reset) {
>> + next_reset = now + 1000; /* reset every 1000ms */
>> + wdt_reset(gd->watchdog_dev);
>> + }
>> +}
>> +#endif
>> +
>> static int wdt_post_bind(struct udevice *dev)
>> {
>> #if defined(CONFIG_NEEDS_MANUAL_RELOC)
>> diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
>> index 78dcf40bff..3cb583cbb1 100644
>> --- a/include/asm-generic/global_data.h
>> +++ b/include/asm-generic/global_data.h
>> @@ -133,6 +133,9 @@ typedef struct global_data {
>> struct spl_handoff *spl_handoff;
>> # endif
>> #endif
>> +#ifdef CONFIG_WATCHDOG
>
> CONFIG_WDT here.
>
> nit: don't know if you should use #if defined() or if CONFIG_IS_ENABLED
> here.
>
>> + struct udevice *watchdog_dev;
>> +#endif
>> } gd_t;
>> #endif
>>
>> @@ -161,5 +164,6 @@ typedef struct global_data {
>> #define GD_FLG_ENV_DEFAULT 0x02000 /* Default variable flag */
>> #define GD_FLG_SPL_EARLY_INIT 0x04000 /* Early SPL init is done */
>> #define GD_FLG_LOG_READY 0x08000 /* Log system is ready for use */
>> +#define GD_FLG_WDT_READY 0x10000 /* Watchdog is ready for use */
>>
>> #endif /* __ASM_GENERIC_GBL_DATA_H */
>>
>
> M
>
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de
More information about the U-Boot
mailing list