From 0ddc47db89b19baf6c085fc86537631126f3ff5f Mon Sep 17 00:00:00 2001 From: Ahmad Fatoum Date: Wed, 23 Oct 2019 18:55:56 +0200 Subject: Documentation: efi: emphasize watchdog deactivation on ExitBootServices The UEFI specification paragraph quoted above notes: > The watchdog timer is only used during boot services. On successful > completion of ExitBootServices() the watchdog timer is disabled. Thus disabling the watchdog is _the_ only proper behavior. Adjust the wording accordingly. Cc: Oleksij Rempel Signed-off-by: Ahmad Fatoum Reviewed-by: Signed-off-by: Sascha Hauer --- Documentation/boards/efi.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/boards/efi.rst b/Documentation/boards/efi.rst index 2178c9ab42..f04b1d3237 100644 --- a/Documentation/boards/efi.rst +++ b/Documentation/boards/efi.rst @@ -350,7 +350,7 @@ https://uefi.org/sites/default/files/resources/UEFI_Spec_2_1_D.pdf Current linux kernel (v5.0) will execute ExitBootServices() during the early boot stage and thus will automatically disable the (U)EFI watchdog. Since it is -a proper behavior according to the (U)EFI specification, it is impossible to +the proper behavior according to the (U)EFI specification, it is impossible to protect full boot chain by using this watchdog only. It is recommended to use an alternative hardware watchdog, preferably started before the bootloader. If (U)EFI firmware lacks this feature, the bootloader should be able to start an alternative -- cgit v1.2.3 From 60101d8efc8036853345061c288b4789be4c3132 Mon Sep 17 00:00:00 2001 From: Ahmad Fatoum Date: Wed, 23 Oct 2019 18:55:57 +0200 Subject: efi: efi-image: don't mask x86 interrupts on boot 55da0cf1 ("efi: add support for initrd loading") introduced support for the Linux v3.6+ handover protocol[1]. As part of this change a x86 cli (Clear Interrupt Flag) instruction was introduced just prior to the jump into the kernel's EFI handover protocol entry point. While the normal Linux x86 boot protocols require that interrupts are masked on entry, this doesn't apply to the EFI stub, because the EFI stub itself is the one implementing the boot protocol and as such masks the interrupts itself[2]. EFI watchdogs may, and often are, implemented using a timer interrupt. Dropping the cli will allow monitoring the boot of the kernel up to it calling ExitBootServices. In absence of a hardware watchdog, this is the only watchdog available to users with EFI 1.0+, so it seems prudent to not make it even more useless. [1]: https://www.kernel.org/doc/Documentation/x86/boot.txt [2]: Linux v5.4-rc4, arch/x86/boot/compressed/eboot.c Cc: Michael Olbrich Fixes: 55da0cf1 ("efi: add support for initrd loading") Signed-off-by: Ahmad Fatoum Signed-off-by: Sascha Hauer --- common/efi/efi-image.c | 1 - 1 file changed, 1 deletion(-) diff --git a/common/efi/efi-image.c b/common/efi/efi-image.c index 939663a6e2..9c66c9f882 100644 --- a/common/efi/efi-image.c +++ b/common/efi/efi-image.c @@ -174,7 +174,6 @@ static inline void linux_efi_handover(efi_handle_t handle, { handover_fn handover; - asm volatile ("cli"); handover = (handover_fn)((long)header->code32_start + 512 + header->handover_offset); handover(handle, efi_sys_table, header); -- cgit v1.2.3 From 2731cc09db884b5089d027a412e0396d07d23f42 Mon Sep 17 00:00:00 2001 From: Ahmad Fatoum Date: Wed, 23 Oct 2019 18:55:58 +0200 Subject: watchdog: efi: bump down priority below default The barebox EFI documentation notes: > Current linux kernel (v5.0) will execute ExitBootServices() during the > early boot stage and thus will automatically disable the (U)EFI watchdog. > Since it is the proper behavior according to the (U)EFI specification, it > is impossible to protect full boot chain by using this watchdog only. > It is recommended to use an alternative hardware watchdog Heed the advice and bump down the EFI watchdog priority below the watchdog priority default. This ensures the EFI watchdog isn't inadvertently used if other watchdogs are registered. Signed-off-by: Ahmad Fatoum Signed-off-by: Sascha Hauer --- drivers/watchdog/efi_wdt.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/watchdog/efi_wdt.c b/drivers/watchdog/efi_wdt.c index 8e3e51b7a9..ea1ede1381 100644 --- a/drivers/watchdog/efi_wdt.c +++ b/drivers/watchdog/efi_wdt.c @@ -41,6 +41,7 @@ static int efi_wdt_probe(struct device_d *dev) priv->wd.set_timeout = efi_wdt_set_timeout; priv->wd.hwdev = dev; priv->dev = dev; + priv->wd.priority = WATCHDOG_DEFAULT_PRIORITY - 50; dev->priv = priv; -- cgit v1.2.3 From c2d167823588708c9f8f68c7db111ce146e19e3f Mon Sep 17 00:00:00 2001 From: Ahmad Fatoum Date: Wed, 23 Oct 2019 18:55:59 +0200 Subject: watchdog: export priority as device parameter 8f4cf30903 ("watchdog: Allow multiple watchdogs") introduced the ability to set a per-watchdog priority from within drivers, which is usually populated with of_get_watchdog_priority. For debugging, it can be useful to query and override this priority on the fly. Provide a device parameter to do so. As watchdog_get_default only considers priorities > 0, it makes sense to have a newly set priority of 0 disable the watchdog. Signed-off-by: Ahmad Fatoum Signed-off-by: Sascha Hauer --- drivers/watchdog/wd_core.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/drivers/watchdog/wd_core.c b/drivers/watchdog/wd_core.c index e6e5ddecd2..ae29a76064 100644 --- a/drivers/watchdog/wd_core.c +++ b/drivers/watchdog/wd_core.c @@ -41,6 +41,16 @@ static int _watchdog_set_timeout(struct watchdog *wd, unsigned timeout) return wd->set_timeout(wd, timeout); } +static int watchdog_set_priority(struct param_d *param, void *priv) +{ + struct watchdog *wd = priv; + + if (wd->priority == 0) + return _watchdog_set_timeout(wd, 0); + + return 0; +} + static int watchdog_set_cur(struct param_d *param, void *priv) { struct watchdog *wd = priv; @@ -130,6 +140,12 @@ int watchdog_register(struct watchdog *wd) if (!wd->priority) wd->priority = WATCHDOG_DEFAULT_PRIORITY; + p = dev_add_param_uint32(&wd->dev, "priority", + watchdog_set_priority, NULL, + &wd->priority, "%u", wd); + if (IS_ERR(p)) + return PTR_ERR(p); + /* set some default sane value */ if (!wd->timeout_max) wd->timeout_max = 60 * 60 * 24; -- cgit v1.2.3 From 9ba3943ce613c179740957e85405b4c832323ddb Mon Sep 17 00:00:00 2001 From: Ahmad Fatoum Date: Wed, 23 Oct 2019 18:56:00 +0200 Subject: watchdog: export API to configure watchdogs by name So far watchdog users could only configure the watchdog with the highest priority. In preparation for having the wd command configure a watchdog by name, extend watchdog_set_timeout with a struct watchdog *parameter and export functions to query default watchdog and to find watchdog by name. No functional change. Signed-off-by: Ahmad Fatoum Signed-off-by: Sascha Hauer --- commands/wd.c | 2 +- common/boot.c | 3 ++- drivers/watchdog/wd_core.c | 40 ++++++++++++++++++++++++---------------- include/watchdog.h | 18 ++++++++++++++++-- 4 files changed, 43 insertions(+), 20 deletions(-) diff --git a/commands/wd.c b/commands/wd.c index 26823f869a..f857291f03 100644 --- a/commands/wd.c +++ b/commands/wd.c @@ -34,7 +34,7 @@ static int do_wd(int argc, char *argv[]) } } - rc = watchdog_set_timeout(timeout); + rc = watchdog_set_timeout(watchdog_get_default(), timeout); if (rc < 0) { switch (rc) { case -EINVAL: diff --git a/common/boot.c b/common/boot.c index 14d4fe9d64..dcbe5cc2ec 100644 --- a/common/boot.c +++ b/common/boot.c @@ -146,7 +146,8 @@ int boot_entry(struct bootentry *be, int verbose, int dryrun) printf("Booting entry '%s'\n", be->title); if (IS_ENABLED(CONFIG_WATCHDOG) && boot_watchdog_timeout) { - ret = watchdog_set_timeout(boot_watchdog_timeout); + ret = watchdog_set_timeout(watchdog_get_default(), + boot_watchdog_timeout); if (ret) pr_warn("Failed to enable watchdog: %s\n", strerror(-ret)); } diff --git a/drivers/watchdog/wd_core.c b/drivers/watchdog/wd_core.c index ae29a76064..80bde82f7c 100644 --- a/drivers/watchdog/wd_core.c +++ b/drivers/watchdog/wd_core.c @@ -31,8 +31,15 @@ static const char *watchdog_name(struct watchdog *wd) return "unknown"; } -static int _watchdog_set_timeout(struct watchdog *wd, unsigned timeout) +/* + * start, stop or retrigger the watchdog + * timeout in [seconds]. timeout of '0' will disable the watchdog (if possible) + */ +int watchdog_set_timeout(struct watchdog *wd, unsigned timeout) { + if (!wd) + return -ENODEV; + if (timeout > wd->timeout_max) return -EINVAL; @@ -40,13 +47,14 @@ static int _watchdog_set_timeout(struct watchdog *wd, unsigned timeout) return wd->set_timeout(wd, timeout); } +EXPORT_SYMBOL(watchdog_set_timeout); static int watchdog_set_priority(struct param_d *param, void *priv) { struct watchdog *wd = priv; if (wd->priority == 0) - return _watchdog_set_timeout(wd, 0); + return watchdog_set_timeout(wd, 0); return 0; } @@ -65,7 +73,7 @@ static void watchdog_poller_cb(void *priv); static void watchdog_poller_start(struct watchdog *wd) { - _watchdog_set_timeout(wd, wd->timeout_cur); + watchdog_set_timeout(wd, wd->timeout_cur); poller_call_async(&wd->poller, 500 * MSECOND, watchdog_poller_cb, wd); @@ -192,7 +200,7 @@ int watchdog_deregister(struct watchdog *wd) } EXPORT_SYMBOL(watchdog_deregister); -static struct watchdog *watchdog_get_default(void) +struct watchdog *watchdog_get_default(void) { struct watchdog *tmp, *wd = NULL; int priority = 0; @@ -206,23 +214,23 @@ static struct watchdog *watchdog_get_default(void) return wd; } +EXPORT_SYMBOL(watchdog_get_default); -/* - * start, stop or retrigger the watchdog - * timeout in [seconds]. timeout of '0' will disable the watchdog (if possible) - */ -int watchdog_set_timeout(unsigned timeout) +struct watchdog *watchdog_get_by_name(const char *name) { - struct watchdog *wd; + struct watchdog *tmp; + struct device_d *dev = get_device_by_name(name); + if (!dev) + return NULL; - wd = watchdog_get_default(); - - if (!wd) - return -ENODEV; + list_for_each_entry(tmp, &watchdog_list, list) { + if (dev == tmp->hwdev || dev == &tmp->dev) + return tmp; + } - return _watchdog_set_timeout(wd, timeout); + return NULL; } -EXPORT_SYMBOL(watchdog_set_timeout); +EXPORT_SYMBOL(watchdog_get_by_name); /** * of_get_watchdog_priority() - get the desired watchdog priority from device tree diff --git a/include/watchdog.h b/include/watchdog.h index 0db4263a31..891a0920e4 100644 --- a/include/watchdog.h +++ b/include/watchdog.h @@ -14,6 +14,7 @@ # define INCLUDE_WATCHDOG_H #include +#include struct watchdog { int (*set_timeout)(struct watchdog *, unsigned); @@ -31,7 +32,9 @@ struct watchdog { #ifdef CONFIG_WATCHDOG int watchdog_register(struct watchdog *); int watchdog_deregister(struct watchdog *); -int watchdog_set_timeout(unsigned); +struct watchdog *watchdog_get_default(void); +struct watchdog *watchdog_get_by_name(const char *name); +int watchdog_set_timeout(struct watchdog*, unsigned); unsigned int of_get_watchdog_priority(struct device_node *node); #else static inline int watchdog_register(struct watchdog *w) @@ -44,11 +47,22 @@ static inline int watchdog_deregister(struct watchdog *w) return 0; } -static inline int watchdog_set_timeout(unsigned t) +static inline struct watchdog *watchdog_get_default(void) +{ + return NULL; +} + +static inline struct watchdog *watchdog_get_by_name(const char *name) +{ + return NULL; +} + +static inline int watchdog_set_timeout(struct watchdog*w, unsigned t) { return 0; } + static inline unsigned int of_get_watchdog_priority(struct device_node *node) { return 0; -- cgit v1.2.3 From b7a06921166d44f0bc3c6638144351382fef409d Mon Sep 17 00:00:00 2001 From: Ahmad Fatoum Date: Wed, 23 Oct 2019 18:56:01 +0200 Subject: commands: wd: support configuring watchdog by name So far, wd has always configured the highest-priority watchdog when multiple are available. Add an optional -d parameter to support configuring the other watchdogs as well. The name passed can be either the watchdog device name (e.g. wdog0) or the hardware device name (e.g. efi-wdt). Signed-off-by: Ahmad Fatoum Signed-off-by: Sascha Hauer --- commands/wd.c | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/commands/wd.c b/commands/wd.c index f857291f03..8029bab1ce 100644 --- a/commands/wd.c +++ b/commands/wd.c @@ -16,6 +16,8 @@ #include #include #include +#include +#include #include /* default timeout in [sec] */ @@ -23,18 +25,30 @@ static unsigned timeout = CONFIG_CMD_WD_DEFAULT_TIMOUT; static int do_wd(int argc, char *argv[]) { + struct watchdog *wd = watchdog_get_default(); + int opt; int rc; - if (argc > 1) { - if (isdigit(*argv[1])) { - timeout = simple_strtoul(argv[1], NULL, 0); + while ((opt = getopt(argc, argv, "d:")) > 0) { + switch (opt) { + case 'd': + wd = watchdog_get_by_name(optarg); + break; + default: + return COMMAND_ERROR_USAGE; + } + } + + if (optind < argc) { + if (isdigit(*argv[optind])) { + timeout = simple_strtoul(argv[optind], NULL, 0); } else { printf("numerical parameter expected\n"); - return 1; + return COMMAND_ERROR_USAGE; } } - rc = watchdog_set_timeout(watchdog_get_default(), timeout); + rc = watchdog_set_timeout(wd, timeout); if (rc < 0) { switch (rc) { case -EINVAL: @@ -43,12 +57,15 @@ static int do_wd(int argc, char *argv[]) case -ENOSYS: printf("Watchdog cannot be disabled\n"); break; + case -ENODEV: + printf("Watchdog device doesn't exist.\n"); + break; default: printf("Watchdog fails: '%s'\n", strerror(-rc)); break; } - return 1; + return COMMAND_ERROR; } return 0; @@ -58,12 +75,15 @@ BAREBOX_CMD_HELP_START(wd) BAREBOX_CMD_HELP_TEXT("Enable the watchdog to bark in TIME seconds.") BAREBOX_CMD_HELP_TEXT("When TIME is 0, the watchdog gets disabled,") BAREBOX_CMD_HELP_TEXT("Without a parameter the watchdog will be re-triggered.") +BAREBOX_CMD_HELP_TEXT("Options:") +BAREBOX_CMD_HELP_OPT("-d DEVICE\t", "watchdog name (default is highest priority watchdog)") BAREBOX_CMD_HELP_END BAREBOX_CMD_START(wd) .cmd = do_wd, BAREBOX_CMD_DESC("enable/disable/trigger the watchdog") - BAREBOX_CMD_OPTS("[TIME]") + BAREBOX_CMD_OPTS("[-d DEVICE] [TIME]") BAREBOX_CMD_GROUP(CMD_GRP_HWMANIP) BAREBOX_CMD_HELP(cmd_wd_help) + BAREBOX_CMD_COMPLETE(device_complete) BAREBOX_CMD_END -- cgit v1.2.3 From e857e4930aef1aa11dfbb1c45069d0fe562eeedc Mon Sep 17 00:00:00 2001 From: Oleksij Rempel Date: Thu, 24 Oct 2019 08:03:41 +0200 Subject: Documentation: add watchdog documentation Signed-off-by: Oleksij Rempel Signed-off-by: Sascha Hauer --- Documentation/user/user-manual.rst | 1 + Documentation/user/watchdog.rst | 85 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+) create mode 100644 Documentation/user/watchdog.rst diff --git a/Documentation/user/user-manual.rst b/Documentation/user/user-manual.rst index f04981c3f0..41fdb8805c 100644 --- a/Documentation/user/user-manual.rst +++ b/Documentation/user/user-manual.rst @@ -34,6 +34,7 @@ Contents: state random debugging + watchdog * :ref:`search` * :ref:`genindex` diff --git a/Documentation/user/watchdog.rst b/Documentation/user/watchdog.rst new file mode 100644 index 0000000000..26e347d5f6 --- /dev/null +++ b/Documentation/user/watchdog.rst @@ -0,0 +1,85 @@ +Watchdog Support +================ + +Barebox Watchdog Functionality +------------------------------ + +In some cases we are not able to influence the hardware design anymore or while +developing one needs to be able to feed the watchdog to disable it from within +the bootloader. For these scenarios barebox provides the watchdog framework +with the following functionality and at least ``CONFIG_WATCHDOG`` should be +enabled. + +Polling +~~~~~~~ + +Watchdog polling/feeding allows to feed the watchdog and keep it running on one +side and to not reset the system on the other side. It is needed on hardware +with short-time watchdogs. For example the Atheros ar9331 watchdog has a +maximal timeout of 7 seconds, so it may reset even on netboot. +Or it can be used on systems where the watchdog is already running and can't be +disabled, an example for that is the watchdog of the i.MX2 series. +This functionally can be seen as a threat, since in error cases barebox will +continue to feed the watchdog even if that is not desired. So, depending on +your needs ``CONFIG_WATCHDOG_POLLER`` can be enabled or disabled at compile +time. Even if barebox was built with watchdog polling support, it is not +enabled by default. To start polling from command line run: + +.. code-block:: console + + wdog0.autoping=1 + +The poller interval is not configurable, but fixed at 500ms and the watchdog +timeout is configured by default to the maximum of the supported values by +hardware. To change the timeout used by the poller, run: + +.. code-block:: console + + wdog0.timeout_cur=7 + +To read the current watchdog's configuration, run: + +.. code-block:: console + + devinfo wdog0 + +The output may look as follows where ``timeout_cur`` and ``timeout_max`` are +measured in seconds: + +.. code-block:: console + + barebox@DPTechnics DPT-Module:/ devinfo wdog0 + Parameters: + autoping: 1 (type: bool) + timeout_cur: 7 (type: uint32) + timeout_max: 10 (type: uint32) + +Use barebox' environment to persist these changes between reboots: + +.. code-block:: console + + nv dev.wdog0.autoping=1 + nv dev.wdog0.timeout_cur=7 + +Boot Watchdog Timeout +~~~~~~~~~~~~~~~~~~~~~ + +With this functionality barebox may start a watchdog or update the timeout of +an already-running one, just before kicking the boot image. It can be +configured temporarily via + +.. code-block:: console + + global boot.watchdog_timeout=10 + +or persistently by + +.. code-block:: console + + nv boot.watchdog_timeout=10 + +where the used value again is measured in seconds. + +On a system with multiple watchdogs, the watchdog with the highest positive +priority is the one affected by the ``boot.watchdog_timeout`` parameter. If +multiple watchdogs share the same priority, only one will be started. -- cgit v1.2.3 From 3c38c8470e60cda83cd059b826d8a627725643fc Mon Sep 17 00:00:00 2001 From: Oleksij Rempel Date: Thu, 24 Oct 2019 08:03:42 +0200 Subject: doc: watchdog: add note about danger of autoping Signed-off-by: Oleksij Rempel Signed-off-by: Sascha Hauer --- Documentation/user/watchdog.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Documentation/user/watchdog.rst b/Documentation/user/watchdog.rst index 26e347d5f6..02bd576a89 100644 --- a/Documentation/user/watchdog.rst +++ b/Documentation/user/watchdog.rst @@ -29,6 +29,12 @@ enabled by default. To start polling from command line run: wdog0.autoping=1 +**NOTE** Using this feature might have the effect that the watchdog is +effectively disabled. In case barebox is stuck in a loop that includes feeding +the watchdog, then the watchdog will never trigger. Only use this feature +during development or when a bad watchdog design (Short watchdog timeout +enabled as boot default) doesn't give you another choice. + The poller interval is not configurable, but fixed at 500ms and the watchdog timeout is configured by default to the maximum of the supported values by hardware. To change the timeout used by the poller, run: -- cgit v1.2.3 From 07dc9dfd190da4caaed828fd1e1b6b024133614f Mon Sep 17 00:00:00 2001 From: Ahmad Fatoum Date: Thu, 24 Oct 2019 17:24:26 +0200 Subject: watchdog: remove wrong uses of timeout_cur The barebox watchdog poller uses the struct watchdog.timeout_cur as the timeout value to configure the watchdog with. There's no need for the device driver to set this. I didn't know that when I wrote the drivers, but I do now, hence this commit. Signed-off-by: Ahmad Fatoum Signed-off-by: Sascha Hauer --- drivers/watchdog/stm32_iwdg.c | 1 - drivers/watchdog/stpmic1_wdt.c | 1 - 2 files changed, 2 deletions(-) diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c index 20536cb4ab..4d252e558c 100644 --- a/drivers/watchdog/stm32_iwdg.c +++ b/drivers/watchdog/stm32_iwdg.c @@ -256,7 +256,6 @@ static int stm32_iwdg_probe(struct device_d *dev) wdd->set_timeout = stm32_iwdg_set_timeout; wdd->timeout_max = (RLR_MAX + 1) * data->max_prescaler * 1000; wdd->timeout_max /= wd->rate * 1000; - wdd->timeout_cur = wdd->timeout_max; ret = watchdog_register(wdd); if (ret) { diff --git a/drivers/watchdog/stpmic1_wdt.c b/drivers/watchdog/stpmic1_wdt.c index eb8c43f716..f79b7e8c27 100644 --- a/drivers/watchdog/stpmic1_wdt.c +++ b/drivers/watchdog/stpmic1_wdt.c @@ -175,7 +175,6 @@ static int stpmic1_wdt_probe(struct device_d *dev) wdd->hwdev = dev; wdd->set_timeout = stpmic1_wdt_set_timeout; wdd->timeout_max = PMIC_WDT_MAX_TIMEOUT; - wdd->timeout_cur = PMIC_WDT_DEFAULT_TIMEOUT; /* have the watchdog reset, not power-off the system */ regmap_write_bits(wdt->regmap, MAIN_CR, RREQ_EN, RREQ_EN); -- cgit v1.2.3 From e1ef5d828320cec35232126497b1b8dd4c22d540 Mon Sep 17 00:00:00 2001 From: Ahmad Fatoum Date: Thu, 24 Oct 2019 17:24:27 +0200 Subject: watchdog: rename timeout_curr to poller_timeout_curr internally timeout_curr is the timeout programmed into the watchdog hardware every 500 milliseconds. If watchdog poller support is disabled, it serves no purpose, prefix it with poller_ to better communicate this fact. No functional change. Signed-off-by: Ahmad Fatoum Signed-off-by: Sascha Hauer --- drivers/watchdog/wd_core.c | 10 +++++----- include/watchdog.h | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/watchdog/wd_core.c b/drivers/watchdog/wd_core.c index 80bde82f7c..07fe9b030e 100644 --- a/drivers/watchdog/wd_core.c +++ b/drivers/watchdog/wd_core.c @@ -63,7 +63,7 @@ static int watchdog_set_cur(struct param_d *param, void *priv) { struct watchdog *wd = priv; - if (wd->timeout_cur > wd->timeout_max) + if (wd->poller_timeout_cur > wd->timeout_max) return -EINVAL; return 0; @@ -73,7 +73,7 @@ static void watchdog_poller_cb(void *priv); static void watchdog_poller_start(struct watchdog *wd) { - watchdog_set_timeout(wd, wd->timeout_cur); + watchdog_set_timeout(wd, wd->poller_timeout_cur); poller_call_async(&wd->poller, 500 * MSECOND, watchdog_poller_cb, wd); @@ -158,8 +158,8 @@ int watchdog_register(struct watchdog *wd) if (!wd->timeout_max) wd->timeout_max = 60 * 60 * 24; - if (!wd->timeout_cur || wd->timeout_cur > wd->timeout_max) - wd->timeout_cur = wd->timeout_max; + if (!wd->poller_timeout_cur || wd->poller_timeout_cur > wd->timeout_max) + wd->poller_timeout_cur = wd->timeout_max; p = dev_add_param_uint32_ro(&wd->dev, "timeout_max", &wd->timeout_max, "%u"); @@ -167,7 +167,7 @@ int watchdog_register(struct watchdog *wd) return PTR_ERR(p); p = dev_add_param_uint32(&wd->dev, "timeout_cur", watchdog_set_cur, NULL, - &wd->timeout_cur, "%u", wd); + &wd->poller_timeout_cur, "%u", wd); if (IS_ERR(p)) return PTR_ERR(p); diff --git a/include/watchdog.h b/include/watchdog.h index 891a0920e4..184a218916 100644 --- a/include/watchdog.h +++ b/include/watchdog.h @@ -23,7 +23,7 @@ struct watchdog { struct device_d dev; unsigned int priority; unsigned int timeout_max; - unsigned int timeout_cur; + unsigned int poller_timeout_cur; unsigned int poller_enable; struct poller_async poller; struct list_head list; -- cgit v1.2.3 From d0c0d45237fbeda2e2dc96123eb315df544c27ac Mon Sep 17 00:00:00 2001 From: Ahmad Fatoum Date: Thu, 24 Oct 2019 17:24:28 +0200 Subject: watchdog: add timeout_cur parameter only when poller is enabled timeout_curr is the timeout programmed into the watchdog hardware every 500 milliseconds. If watchdog poller support is disabled, it still shows up as a configurable device parameter, but has no effect. Improve user experience by having it show up only if watchdog poller support was compiled in. This is already the case for the autoping parameter. The timeout_max parameter is a generic parameter and will remain unchanged. Signed-off-by: Ahmad Fatoum Signed-off-by: Sascha Hauer --- drivers/watchdog/wd_core.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/drivers/watchdog/wd_core.c b/drivers/watchdog/wd_core.c index 07fe9b030e..8b13950238 100644 --- a/drivers/watchdog/wd_core.c +++ b/drivers/watchdog/wd_core.c @@ -158,20 +158,21 @@ int watchdog_register(struct watchdog *wd) if (!wd->timeout_max) wd->timeout_max = 60 * 60 * 24; - if (!wd->poller_timeout_cur || wd->poller_timeout_cur > wd->timeout_max) - wd->poller_timeout_cur = wd->timeout_max; - p = dev_add_param_uint32_ro(&wd->dev, "timeout_max", &wd->timeout_max, "%u"); if (IS_ERR(p)) return PTR_ERR(p); - p = dev_add_param_uint32(&wd->dev, "timeout_cur", watchdog_set_cur, NULL, - &wd->poller_timeout_cur, "%u", wd); - if (IS_ERR(p)) - return PTR_ERR(p); - if (IS_ENABLED(CONFIG_WATCHDOG_POLLER)) { + if (!wd->poller_timeout_cur || + wd->poller_timeout_cur > wd->timeout_max) + wd->poller_timeout_cur = wd->timeout_max; + + p = dev_add_param_uint32(&wd->dev, "timeout_cur", watchdog_set_cur, + NULL, &wd->poller_timeout_cur, "%u", wd); + if (IS_ERR(p)) + return PTR_ERR(p); + ret = watchdog_register_poller(wd); if (ret) return ret; -- cgit v1.2.3 From 7d3b130af9d282e9a5b788bd44d1ad6528a3bde1 Mon Sep 17 00:00:00 2001 From: Marco Felsch Date: Mon, 4 Nov 2019 11:28:05 +0100 Subject: mfd: da9063: fix TWDSCALE debug message The TWDSCALE is the found scale + 1 as described in the datasheets for the DA9062/3 devices. The driver logic is correct just the debug message is wrong. Signed-off-by: Marco Felsch Signed-off-by: Sascha Hauer --- drivers/mfd/da9063.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mfd/da9063.c b/drivers/mfd/da9063.c index b61e764876..7099c48703 100644 --- a/drivers/mfd/da9063.c +++ b/drivers/mfd/da9063.c @@ -270,7 +270,7 @@ static int da9063_watchdog_set_timeout(struct watchdog *wd, unsigned timeout) while (timeout > (2048 << scale) && scale <= 6) scale++; dev_dbg(dev, "calculated TWDSCALE=%u (req=%ims calc=%ims)\n", - scale, timeout, 2048 << scale); + scale + 1, timeout, 2048 << scale); scale++; /* scale 0 disables the WD */ } -- cgit v1.2.3 From cbfdbf38c190bc9197f85c9633f103cdc15de571 Mon Sep 17 00:00:00 2001 From: Marco Felsch Date: Mon, 4 Nov 2019 11:28:06 +0100 Subject: mfd: da9063: fix watchdog ping execution The watchdog resets the system if the watchdog gets pinged to fast. Between each watchdog ping must be a pause of at least 200ms. This commit fixes that by rejecting two fast requests. Signed-off-by: Marco Felsch Signed-off-by: Sascha Hauer --- drivers/mfd/da9063.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/mfd/da9063.c b/drivers/mfd/da9063.c index 7099c48703..e1343bac76 100644 --- a/drivers/mfd/da9063.c +++ b/drivers/mfd/da9063.c @@ -14,6 +14,7 @@ */ #include +#include #include #include #include @@ -33,6 +34,7 @@ struct da9063 { struct i2c_client *client1; struct device_d *dev; unsigned int timeout; + uint64_t last_ping; }; /* forbidden/impossible value; timeout will be set to this value initially to @@ -237,6 +239,14 @@ static int da9063_watchdog_ping(struct da9063 *priv) int ret; u8 val; + /* + * The watchdog has a cool down phase of 200ms and if we ping to fast + * the da9062/3 resets the system. Reject those requests has a maximum + * failure of 10% if the watchdog timeout is set to 2.048s. + */ + if (!is_timeout(priv->last_ping, 200 * MSECOND)) + return 0; + dev_dbg(priv->dev, "ping\n"); /* reset watchdog timer; register is self clearing */ @@ -245,6 +255,8 @@ static int da9063_watchdog_ping(struct da9063 *priv) if (ret < 0) return ret; + priv->last_ping = get_time_ns(); + return 0; } -- cgit v1.2.3