From: Fu Wei fu.wei@linaro.org
This patchset: (1)Introduce Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt for FDT info of SBSA Generic Watchdog, and give two examples of adding SBSA Generic Watchdog device node into the dts files: foundation-v8.dts and amd-seattle-soc.dtsi.
(2)Introduce "pretimeout" into the watchdog framework, and update Documentation/watchdog/watchdog-kernel-api.txt to introduce: (1)the new elements in the watchdog_device and watchdog_ops struct; (2)the new API "watchdog_init_timeouts".
(3)Introduce ARM SBSA watchdog driver: a.Use linux kernel watchdog framework; b.Work with FDT on ARM64; c.Use "pretimeout" in watchdog framework; d.Support getting timeout and pretimeout from parameter and FDT at the driver init stage. e.In the first timeout, do panic to save system context; f.In the second stage, user can still feed the dog without cleaning WS0. By this feature, we can avoid the panic infinite loops, while backing up a large system context in a server. g.In the second stage, can trigger WS1 by setting pretimeout = 0 if necessary.
(4)Introduce ACPI GTDT parser: drivers/acpi/gtdt.c Parse SBSA Generic Watchdog Structure in GTDT table of ACPI, and create a platform device with that information. This platform device can be used by This Watchdog driver. drivers/clocksource/arm_arch_timer.c is simplified by this GTDT support.
This patchset has been tested with watchdog daemon (ACPI/FDT, module/build-in) on the following platforms: (1)ARM Foundation v8 model
Changelog: v6: Improve the dtb example files: reduce the register frame size to 4K. Improve pretimeout support: (1) improve watchdog_init_timeouts function (2) rename watchdog_check_min_max_timeouts back to the original name (1) improve watchdog_timeout_invalid/watchdog_pretimeout_invalid Add the new features in the sbsa_gwdt driver: (1) In the second stage, user can feed the dog without cleaning WS0. (2) In the second stage, user can trigger WS1 by setting pretimeout = 0. (3) expand the max value of pretimeout, in case 10 second is not enough for a kdump kernel reboot in panic.
v5: Improve pretimeout support: (1)fix typo in documentation and comments. (2)fix the timeout limits validation bug. Simplify sbsa_gwdt driver: (1)integrate all the registers access functions into caller.
v4: Refactor GTDT support code: remove it from arch/arm64/kernel/acpi.c, put it into drivers/acpi/gtdt.c file. Integrate the GTDT code of drivers/clocksource/arm_arch_timer.c into drivers/acpi/gtdt.c. Improve pretimeout support, fix "pretimeout == 0" problem. Simplify sbsa_gwdt driver: (1)timeout/pretimeout limits setup; (2)keepalive function; (3)delete "clk == 0" check; (4)delete WS0 status bit check in interrupt routine; (5)sbsa_gwdt_set_wcv function.
v3: Delete "export arch_timer_get_rate" patch. Driver back to use arch_timer_get_cntfrq. Improve watchdog_init_timeouts function and update relevant documentation. Improve watchdog_timeout_invalid and watchdog_pretimeout_invalid. Improve foundation-v8.dts: delete the unnecessary tag of device node. Remove "ARM64 || COMPILE_TEST" from Kconfig. Add comments in arch/arm64/kernel/acpi.c Fix typoes and incorrect comments.
v2: Improve watchdog-kernel-api.txt documentation for pretimeout support. Export "arch_timer_get_rate" in arm_arch_timer.c. Add watchdog_init_timeouts API for pretimeout support in framework. Improve suspend and resume foundation in driver Improve timeout/pretimeout values init code in driver. Delete unnecessary items of the sbsa_gwdt struct and #define. Delete all unnecessary debug info in driver. Fix 64bit division bug. Use the arch_timer interface to get watchdog clock rate. Add MODULE_DEVICE_TABLE for platform device id. Fix typoes.
v1: The first version upstream patchset to linux mailing list.
Fu Wei (8): Documentation: add sbsa-gwdt.txt documentation ARM64: add SBSA Generic Watchdog device node in foundation-v8.dts ARM64: add SBSA Generic Watchdog device node in amd-seattle-soc.dtsi Watchdog: introdouce "pretimeout" into framework Watchdog: introduce ARM SBSA watchdog driver ACPI: add GTDT table parse driver into ACPI driver Watchdog: enable ACPI GTDT support for ARM SBSA watchdog driver clocksource: simplify ACPI code in arm_arch_timer.c
.../devicetree/bindings/watchdog/sbsa-gwdt.txt | 36 ++ Documentation/watchdog/watchdog-kernel-api.txt | 47 ++- arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi | 11 + arch/arm64/boot/dts/arm/foundation-v8.dts | 10 + arch/arm64/kernel/time.c | 4 +- drivers/acpi/Kconfig | 9 + drivers/acpi/Makefile | 1 + drivers/acpi/gtdt.c | 180 ++++++++ drivers/clocksource/Kconfig | 1 + drivers/clocksource/arm_arch_timer.c | 60 +-- drivers/watchdog/Kconfig | 15 + drivers/watchdog/Makefile | 1 + drivers/watchdog/sbsa_gwdt.c | 455 +++++++++++++++++++++ drivers/watchdog/watchdog_core.c | 98 +++-- drivers/watchdog/watchdog_dev.c | 53 +++ include/clocksource/arm_arch_timer.h | 8 + include/linux/acpi.h | 5 + include/linux/clocksource.h | 4 +- include/linux/watchdog.h | 39 +- 19 files changed, 947 insertions(+), 90 deletions(-) create mode 100644 Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt create mode 100644 drivers/acpi/gtdt.c create mode 100644 drivers/watchdog/sbsa_gwdt.c
From: Fu Wei fu.wei@linaro.org
The sbsa-gwdt.txt documentation in devicetree/bindings/watchdog is for introducing SBSA(Server Base System Architecture) Generic Watchdog device node info into FDT.
Acked-by: Arnd Bergmann arnd@arndb.de Signed-off-by: Fu Wei fu.wei@linaro.org --- .../devicetree/bindings/watchdog/sbsa-gwdt.txt | 36 ++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt
diff --git a/Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt b/Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt new file mode 100644 index 0000000..010e5c4 --- /dev/null +++ b/Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt @@ -0,0 +1,36 @@ +* SBSA(Server Base System Architecture) Generic Watchdog + +The SBSA Generic Watchdog Timer is used for resetting the system after +two stages of timeout. +More details: ARM-DEN-0029 - Server Base System Architecture (SBSA) + +Required properties: +- compatible : Should at least contain "arm,sbsa-gwdt". + +- reg : base physical address of the frames and length of memory mapped region. + +- reg-names : Should contain the resource reg names to show the order of + the values in "reg". + Must include the following entries : "refresh", "control". + +- interrupts : Should at least contain WS0 interrupt, + the WS1 Signal is optional. + +- interrupt-names : Should contain the resource interrupt names. + Must include the following entries : "ws0". "ws1" is optional. + +Optional properties +- timeout-sec : Watchdog pre-timeout and timeout values (in seconds). + The first is timeout values, then pre-timeout. + +Example for FVP Foundation Model v8: + +watchdog@2a440000 { + compatible = "arm,sbsa-gwdt"; + reg = <0x0 0x2a440000 0 0x1000>, + <0x0 0x2a450000 0 0x1000>; + reg-names = "control", "refresh"; + interrupts = <0 27 4>; + interrupt-names = "ws0"; + timeout-sec = <10 5>; +};
On Tue, Jun 23, 2015 at 9:16 AM, fu.wei@linaro.org wrote:
From: Fu Wei fu.wei@linaro.org
The sbsa-gwdt.txt documentation in devicetree/bindings/watchdog is for introducing SBSA(Server Base System Architecture) Generic Watchdog device node info into FDT.
Acked-by: Arnd Bergmann arnd@arndb.de Signed-off-by: Fu Wei fu.wei@linaro.org
.../devicetree/bindings/watchdog/sbsa-gwdt.txt | 36 ++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt
diff --git a/Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt b/Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt new file mode 100644 index 0000000..010e5c4 --- /dev/null +++ b/Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt @@ -0,0 +1,36 @@ +* SBSA(Server Base System Architecture) Generic Watchdog
+The SBSA Generic Watchdog Timer is used for resetting the system after +two stages of timeout. +More details: ARM-DEN-0029 - Server Base System Architecture (SBSA)
+Required properties: +- compatible : Should at least contain "arm,sbsa-gwdt".
+- reg : base physical address of the frames and length of memory mapped region.
This needs to specify the ordering of refresh and control.
+- reg-names : Should contain the resource reg names to show the order of
- the values in "reg".
- Must include the following entries : "refresh", "control".
This should be optional.
+- interrupts : Should at least contain WS0 interrupt,
- the WS1 Signal is optional.
s/Signal/interrupt/ ?
+- interrupt-names : Should contain the resource interrupt names.
- Must include the following entries : "ws0". "ws1" is optional.
This should be optional.
+Optional properties +- timeout-sec : Watchdog pre-timeout and timeout values (in seconds).
The first is timeout values, then pre-timeout.
+Example for FVP Foundation Model v8:
+watchdog@2a440000 {
compatible = "arm,sbsa-gwdt";
reg = <0x0 0x2a440000 0 0x1000>,
<0x0 0x2a450000 0 0x1000>;
reg-names = "control", "refresh";
interrupts = <0 27 4>;
interrupt-names = "ws0";
timeout-sec = <10 5>;
+};
1.9.1
-- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Rob,
Great thanks for your review :-)
On 14 July 2015 at 22:49, Rob Herring robherring2@gmail.com wrote:
On Tue, Jun 23, 2015 at 9:16 AM, fu.wei@linaro.org wrote:
From: Fu Wei fu.wei@linaro.org
The sbsa-gwdt.txt documentation in devicetree/bindings/watchdog is for introducing SBSA(Server Base System Architecture) Generic Watchdog device node info into FDT.
Acked-by: Arnd Bergmann arnd@arndb.de Signed-off-by: Fu Wei fu.wei@linaro.org
.../devicetree/bindings/watchdog/sbsa-gwdt.txt | 36 ++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt
diff --git a/Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt b/Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt new file mode 100644 index 0000000..010e5c4 --- /dev/null +++ b/Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt @@ -0,0 +1,36 @@ +* SBSA(Server Base System Architecture) Generic Watchdog
+The SBSA Generic Watchdog Timer is used for resetting the system after +two stages of timeout. +More details: ARM-DEN-0029 - Server Base System Architecture (SBSA)
+Required properties: +- compatible : Should at least contain "arm,sbsa-gwdt".
+- reg : base physical address of the frames and length of memory mapped region.
This needs to specify the ordering of refresh and control.
In the FDT binding, Do we have to use ordering? Is the name always optional? Sorry for questioning this, I just want to confirm that is a rule in the FDT binding. If that is a rule, would you let me know : where is it?
In my driver, I get these resource by name, but not ordering, for now
Thanks for your help :-)
+- reg-names : Should contain the resource reg names to show the order of
- the values in "reg".
- Must include the following entries : "refresh", "control".
This should be optional.
+- interrupts : Should at least contain WS0 interrupt,
- the WS1 Signal is optional.
s/Signal/interrupt/ ?
yes, thanks, will fix it.
+- interrupt-names : Should contain the resource interrupt names.
- Must include the following entries : "ws0". "ws1" is optional.
This should be optional.
+Optional properties +- timeout-sec : Watchdog pre-timeout and timeout values (in seconds).
The first is timeout values, then pre-timeout.
+Example for FVP Foundation Model v8:
+watchdog@2a440000 {
compatible = "arm,sbsa-gwdt";
reg = <0x0 0x2a440000 0 0x1000>,
<0x0 0x2a450000 0 0x1000>;
reg-names = "control", "refresh";
interrupts = <0 27 4>;
interrupt-names = "ws0";
timeout-sec = <10 5>;
+};
1.9.1
-- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Rob,
I have got your point by chatting with you though IRC, I will improve my patch according to your suggestion.
Thanks for your help.
On 14 July 2015 at 23:48, Fu Wei fu.wei@linaro.org wrote:
Hi Rob,
Great thanks for your review :-)
On 14 July 2015 at 22:49, Rob Herring robherring2@gmail.com wrote:
On Tue, Jun 23, 2015 at 9:16 AM, fu.wei@linaro.org wrote:
From: Fu Wei fu.wei@linaro.org
The sbsa-gwdt.txt documentation in devicetree/bindings/watchdog is for introducing SBSA(Server Base System Architecture) Generic Watchdog device node info into FDT.
Acked-by: Arnd Bergmann arnd@arndb.de Signed-off-by: Fu Wei fu.wei@linaro.org
.../devicetree/bindings/watchdog/sbsa-gwdt.txt | 36 ++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt
diff --git a/Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt b/Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt new file mode 100644 index 0000000..010e5c4 --- /dev/null +++ b/Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt @@ -0,0 +1,36 @@ +* SBSA(Server Base System Architecture) Generic Watchdog
+The SBSA Generic Watchdog Timer is used for resetting the system after +two stages of timeout. +More details: ARM-DEN-0029 - Server Base System Architecture (SBSA)
+Required properties: +- compatible : Should at least contain "arm,sbsa-gwdt".
+- reg : base physical address of the frames and length of memory mapped region.
This needs to specify the ordering of refresh and control.
In the FDT binding, Do we have to use ordering? Is the name always optional? Sorry for questioning this, I just want to confirm that is a rule in the FDT binding. If that is a rule, would you let me know : where is it?
In my driver, I get these resource by name, but not ordering, for now
Thanks for your help :-)
+- reg-names : Should contain the resource reg names to show the order of
- the values in "reg".
- Must include the following entries : "refresh", "control".
This should be optional.
+- interrupts : Should at least contain WS0 interrupt,
- the WS1 Signal is optional.
s/Signal/interrupt/ ?
yes, thanks, will fix it.
+- interrupt-names : Should contain the resource interrupt names.
- Must include the following entries : "ws0". "ws1" is optional.
This should be optional.
+Optional properties +- timeout-sec : Watchdog pre-timeout and timeout values (in seconds).
The first is timeout values, then pre-timeout.
+Example for FVP Foundation Model v8:
+watchdog@2a440000 {
compatible = "arm,sbsa-gwdt";
reg = <0x0 0x2a440000 0 0x1000>,
<0x0 0x2a450000 0 0x1000>;
reg-names = "control", "refresh";
interrupts = <0 27 4>;
interrupt-names = "ws0";
timeout-sec = <10 5>;
+};
1.9.1
-- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
-- Best regards,
Fu Wei Software Engineer Red Hat Software (Beijing) Co.,Ltd.Shanghai Branch Ph: +86 21 61221326(direct) Ph: +86 186 2020 4684 (mobile) Room 1512, Regus One Corporate Avenue,Level 15, One Corporate Avenue,222 Hubin Road,Huangpu District, Shanghai,China 200021
From: Fu Wei fu.wei@linaro.org
This can be a example of adding SBSA Generic Watchdog device node into some dts files for the Soc which contains SBSA Generic Watchdog.
Acked-by: Arnd Bergmann arnd@arndb.de Signed-off-by: Fu Wei fu.wei@linaro.org --- arch/arm64/boot/dts/arm/foundation-v8.dts | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/arch/arm64/boot/dts/arm/foundation-v8.dts b/arch/arm64/boot/dts/arm/foundation-v8.dts index 4eac8dc..7b80203 100644 --- a/arch/arm64/boot/dts/arm/foundation-v8.dts +++ b/arch/arm64/boot/dts/arm/foundation-v8.dts @@ -237,4 +237,14 @@ }; }; }; + watchdog@2a440000 { + compatible = "arm,sbsa-gwdt"; + reg = <0x0 0x2a440000 0 0x1000>, + <0x0 0x2a450000 0 0x1000>; + reg-names = "control", + "refresh"; + interrupts = <0 27 4>; + interrupt-names = "ws0"; + timeout-sec = <10 5>; + }; };
From: Fu Wei fu.wei@linaro.org
This can be a example of adding SBSA Generic Watchdog device node into some dts files for the Soc which contains SBSA Generic Watchdog.
Acked-by: Arnd Bergmann arnd@arndb.de Acked-by: Suravee Suthikulpanit Suravee.Suthikulpanit@amd.com Tested-by: Suravee Suthikulpanit Suravee.Suthikulpanit@amd.com Signed-off-by: Fu Wei fu.wei@linaro.org --- arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi b/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi index 2874d92..4be7019 100644 --- a/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi +++ b/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi @@ -84,6 +84,17 @@ clock-names = "uartclk", "apb_pclk"; };
+ watchdog0: watchdog@e0bb0000 { + compatible = "arm,sbsa-gwdt"; + reg = <0x0 0xe0bb0000 0 0x1000>, + <0x0 0xe0bc0000 0 0x1000>; + reg-names = "refresh", + "control"; + interrupts = <0 337 4>; + interrupt-names = "ws0"; + timeout-sec = <10 5>; + }; + spi0: ssp@e1020000 { status = "disabled"; compatible = "arm,pl022", "arm,primecell";
From: Fu Wei fu.wei@linaro.org
Also update Documentation/watchdog/watchdog-kernel-api.txt to introduce: (1)the new elements in the watchdog_device and watchdog_ops struct; (2)the new API "watchdog_init_timeouts"
Reasons: (1)kernel already has two watchdog drivers are using "pretimeout": drivers/char/ipmi/ipmi_watchdog.c drivers/watchdog/kempld_wdt.c(but the definition is different) (2)some other drivers are going to use this: ARM SBSA Generic Watchdog
Signed-off-by: Fu Wei fu.wei@linaro.org --- Documentation/watchdog/watchdog-kernel-api.txt | 47 ++++++++++-- drivers/watchdog/watchdog_core.c | 98 ++++++++++++++++++-------- drivers/watchdog/watchdog_dev.c | 53 ++++++++++++++ include/linux/watchdog.h | 39 ++++++++-- 4 files changed, 200 insertions(+), 37 deletions(-)
diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt index a0438f3..d1b1238 100644 --- a/Documentation/watchdog/watchdog-kernel-api.txt +++ b/Documentation/watchdog/watchdog-kernel-api.txt @@ -49,6 +49,9 @@ struct watchdog_device { unsigned int timeout; unsigned int min_timeout; unsigned int max_timeout; + unsigned int pretimeout; + unsigned int min_pretimeout; + unsigned int max_pretimeout; void *driver_data; struct mutex lock; unsigned long status; @@ -70,6 +73,9 @@ It contains following fields: * timeout: the watchdog timer's timeout value (in seconds). * min_timeout: the watchdog timer's minimum timeout value (in seconds). * max_timeout: the watchdog timer's maximum timeout value (in seconds). +* pretimeout: the watchdog timer's pretimeout value (in seconds). +* min_pretimeout: the watchdog timer's minimum pretimeout value (in seconds). +* max_pretimeout: the watchdog timer's maximum pretimeout value (in seconds). * bootstatus: status of the device after booting (reported with watchdog WDIOF_* status bits). * driver_data: a pointer to the drivers private data of a watchdog device. @@ -92,6 +98,7 @@ struct watchdog_ops { int (*ping)(struct watchdog_device *); unsigned int (*status)(struct watchdog_device *); int (*set_timeout)(struct watchdog_device *, unsigned int); + int (*set_pretimeout)(struct watchdog_device *, unsigned int); unsigned int (*get_timeleft)(struct watchdog_device *); void (*ref)(struct watchdog_device *); void (*unref)(struct watchdog_device *); @@ -153,9 +160,19 @@ they are supported. These optional routines/operations are: and -EIO for "could not write value to the watchdog". On success this routine should set the timeout value of the watchdog_device to the achieved timeout value (which may be different from the requested one - because the watchdog does not necessarily has a 1 second resolution). + because the watchdog does not necessarily has a 1 second resolution; + If the driver supports pretimeout, then the timeout value must be greater + than that). (Note: the WDIOF_SETTIMEOUT needs to be set in the options field of the watchdog's info structure). +* set_pretimeout: this routine checks and changes the pretimeout of the + watchdog timer device. It returns 0 on success, -EINVAL for "parameter out of + range" and -EIO for "could not write value to the watchdog". On success this + routine should set the pretimeout value of the watchdog_device to the + achieved pretimeout value (which may be different from the requested one + because the watchdog does not necessarily has a 1 second resolution). + (Note: the WDIOF_PRETIMEOUT needs to be set in the options field of the + watchdog's info structure). * get_timeleft: this routines returns the time that's left before a reset. * ref: the operation that calls kref_get on the kref of a dynamically allocated watchdog_device struct. @@ -219,8 +236,28 @@ extern int watchdog_init_timeout(struct watchdog_device *wdd, unsigned int timeout_parm, struct device *dev);
The watchdog_init_timeout function allows you to initialize the timeout field -using the module timeout parameter or by retrieving the timeout-sec property from -the device tree (if the module timeout parameter is invalid). Best practice is -to set the default timeout value as timeout value in the watchdog_device and -then use this function to set the user "preferred" timeout value. +using the module timeout parameter or by retrieving the first element of +the timeout-sec property from the device tree (if the module timeout parameter +is invalid). Best practice is to set the default timeout value as timeout value +in the watchdog_device and then use this function to set the user "preferred" +timeout value. +This routine returns zero on success and a negative errno code for failure. + +Some watchdog timers have two stage of timeouts (timeout and pretimeout), +to initialize the timeout and pretimeout fields at the same time, the following +function can be used: + +int watchdog_init_timeouts(struct watchdog_device *wdd, + unsigned int pretimeout_parm, + unsigned int timeout_parm, + struct device *dev); + +The watchdog_init_timeouts function allows you to initialize the pretimeout and +timeout fields using the module pretimeout and timeout parameter or by +retrieving the elements in the timeout-sec property (the first element is for +timeout, the second one is for pretimeout) from the device tree (if the module +pretimeout and timeout parameter are invalid). +Best practice is to set the default pretimeout and timeout value as pretimeout +and timeout value in the watchdog_device and then use this function to set the +user "preferred" pretimeout value. This routine returns zero on success and a negative errno code for failure. diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c index cec9b55..a45e8bb 100644 --- a/drivers/watchdog/watchdog_core.c +++ b/drivers/watchdog/watchdog_core.c @@ -46,57 +46,99 @@ static struct class *watchdog_class; static void watchdog_check_min_max_timeout(struct watchdog_device *wdd) { /* - * Check that we have valid min and max timeout values, if - * not reset them both to 0 (=not used or unknown) + * Check that we have valid min and max pretimeout and timeout values, + * if not, reset them all to 0 (=not used or unknown) */ - if (wdd->min_timeout > wdd->max_timeout) { - pr_info("Invalid min and max timeout values, resetting to 0!\n"); + if (wdd->min_pretimeout > wdd->max_pretimeout || + wdd->min_timeout > wdd->max_timeout || + wdd->min_timeout < wdd->min_pretimeout || + wdd->max_timeout < wdd->max_pretimeout) { + pr_info("Invalid min or max timeouts, resetting to 0\n"); + wdd->min_pretimeout = 0; + wdd->max_pretimeout = 0; wdd->min_timeout = 0; wdd->max_timeout = 0; } }
/** - * watchdog_init_timeout() - initialize the timeout field + * watchdog_init_timeouts() - initialize the pretimeout and timeout field + * @pretimeout_parm: pretimeout module parameter * @timeout_parm: timeout module parameter * @dev: Device that stores the timeout-sec property * - * Initialize the timeout field of the watchdog_device struct with either the - * timeout module parameter (if it is valid value) or the timeout-sec property - * (only if it is a valid value and the timeout_parm is out of bounds). - * If none of them are valid then we keep the old value (which should normally - * be the default timeout value. + * Initialize the pretimeout and timeout field of the watchdog_device struct + * with both the pretimeout and timeout module parameters (if they are valid) or + * the timeout-sec property (only if they are valid and the pretimeout_parm or + * timeout_parm is out of bounds). If one of them is invalid, then we keep + * the old value (which should normally be the default timeout value). * * A zero is returned on success and -EINVAL for failure. */ -int watchdog_init_timeout(struct watchdog_device *wdd, - unsigned int timeout_parm, struct device *dev) +int watchdog_init_timeouts(struct watchdog_device *wdd, + unsigned int pretimeout_parm, + unsigned int timeout_parm, + struct device *dev) { - unsigned int t = 0; - int ret = 0; + int ret = 0, length = 0; + u32 timeouts[2] = {0}; + struct property *prop;
watchdog_check_min_max_timeout(wdd);
- /* try to get the timeout module parameter first */ - if (!watchdog_timeout_invalid(wdd, timeout_parm) && timeout_parm) { - wdd->timeout = timeout_parm; - return ret; - } - if (timeout_parm) + /* + * Try to get the pretimeout module parameter first. + * Note: zero is a valid value for pretimeout. + */ + if (watchdog_pretimeout_invalid(wdd, pretimeout_parm)) + ret = -EINVAL; + + /* + * Try to get the timeout module parameter, + * if it's valid and pretimeout is valid(ret == 0), + * assignment and return zero. Otherwise, try dtb. + */ + if (timeout_parm && !ret) { + if (!watchdog_timeout_invalid(wdd, timeout_parm)) { + wdd->timeout = timeout_parm; + wdd->pretimeout = pretimeout_parm; + return 0; + } ret = -EINVAL; + }
- /* try to get the timeout_sec property */ + /* + * Either at least one of the module parameters is invalid, + * or timeout_parm is 0. Try to get the timeout_sec property. + */ if (dev == NULL || dev->of_node == NULL) return ret; - of_property_read_u32(dev->of_node, "timeout-sec", &t); - if (!watchdog_timeout_invalid(wdd, t) && t) - wdd->timeout = t; - else - ret = -EINVAL;
- return ret; + prop = of_find_property(dev->of_node, "timeout-sec", &length); + if (prop && length > 0 && length <= sizeof(u32) * 2) { + of_property_read_u32_array(dev->of_node, + "timeout-sec", timeouts, + length / sizeof(u32)); + if (length == sizeof(u32) * 2) { + if (watchdog_pretimeout_invalid(wdd, timeouts[1])) + return -EINVAL; + ret = 0; + } else { + ret = -EINVAL; + } + + if (!watchdog_timeout_invalid(wdd, timeouts[0]) && + timeouts[0]) { + wdd->timeout = timeouts[0]; + if (!ret) + wdd->pretimeout = timeouts[1]; + return 0; + } + } + + return -EINVAL; } -EXPORT_SYMBOL_GPL(watchdog_init_timeout); +EXPORT_SYMBOL_GPL(watchdog_init_timeouts);
/** * watchdog_register_device() - register a watchdog device diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c index 6aaefba..af0777e 100644 --- a/drivers/watchdog/watchdog_dev.c +++ b/drivers/watchdog/watchdog_dev.c @@ -218,6 +218,38 @@ out_timeout: }
/* + * watchdog_set_pretimeout: set the watchdog timer pretimeout + * @wddev: the watchdog device to set the timeout for + * @pretimeout: pretimeout to set in seconds + */ + +static int watchdog_set_pretimeout(struct watchdog_device *wddev, + unsigned int pretimeout) +{ + int err; + + if (!wddev->ops->set_pretimeout || + !(wddev->info->options & WDIOF_PRETIMEOUT)) + return -EOPNOTSUPP; + + if (watchdog_pretimeout_invalid(wddev, pretimeout)) + return -EINVAL; + + mutex_lock(&wddev->lock); + + if (test_bit(WDOG_UNREGISTERED, &wddev->status)) { + err = -ENODEV; + goto out_pretimeout; + } + + err = wddev->ops->set_pretimeout(wddev, pretimeout); + +out_pretimeout: + mutex_unlock(&wddev->lock); + return err; +} + +/* * watchdog_get_timeleft: wrapper to get the time left before a reboot * @wddev: the watchdog device to get the remaining time from * @timeleft: the time that's left @@ -388,6 +420,27 @@ static long watchdog_ioctl(struct file *file, unsigned int cmd, if (wdd->timeout == 0) return -EOPNOTSUPP; return put_user(wdd->timeout, p); + case WDIOC_SETPRETIMEOUT: + /* check if we support the pretimeout */ + if (!(wdd->info->options & WDIOF_PRETIMEOUT)) + return -EOPNOTSUPP; + if (get_user(val, p)) + return -EFAULT; + err = watchdog_set_pretimeout(wdd, val); + if (err < 0) + return err; + /* + * If the watchdog is active then we send a keepalive ping + * to make sure that the watchdog keeps running (and if + * possible that it takes the new pretimeout) + */ + watchdog_ping(wdd); + /* Fall */ + case WDIOC_GETPRETIMEOUT: + /* check if we support the pretimeout */ + if (wdd->info->options & WDIOF_PRETIMEOUT) + return put_user(wdd->pretimeout, p); + return -EOPNOTSUPP; case WDIOC_GETTIMELEFT: err = watchdog_get_timeleft(wdd, &val); if (err) diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h index a746bf5..47a87cb 100644 --- a/include/linux/watchdog.h +++ b/include/linux/watchdog.h @@ -25,6 +25,7 @@ struct watchdog_device; * @ping: The routine that sends a keepalive ping to the watchdog device. * @status: The routine that shows the status of the watchdog device. * @set_timeout:The routine for setting the watchdog devices timeout value. + * @set_pretimeout:The routine for setting the watchdog devices pretimeout value * @get_timeleft:The routine that get's the time that's left before a reset. * @ref: The ref operation for dyn. allocated watchdog_device structs * @unref: The unref operation for dyn. allocated watchdog_device structs @@ -44,6 +45,7 @@ struct watchdog_ops { int (*ping)(struct watchdog_device *); unsigned int (*status)(struct watchdog_device *); int (*set_timeout)(struct watchdog_device *, unsigned int); + int (*set_pretimeout)(struct watchdog_device *, unsigned int); unsigned int (*get_timeleft)(struct watchdog_device *); void (*ref)(struct watchdog_device *); void (*unref)(struct watchdog_device *); @@ -62,6 +64,9 @@ struct watchdog_ops { * @timeout: The watchdog devices timeout value. * @min_timeout:The watchdog devices minimum timeout value. * @max_timeout:The watchdog devices maximum timeout value. + * @pretimeout: The watchdog devices pretimeout value. + * @min_pretimeout:The watchdog devices minimum pretimeout value. + * @max_pretimeout:The watchdog devices maximum pretimeout value. * @driver-data:Pointer to the drivers private data. * @lock: Lock for watchdog core internal use only. * @status: Field that contains the devices internal status bits. @@ -86,6 +91,9 @@ struct watchdog_device { unsigned int timeout; unsigned int min_timeout; unsigned int max_timeout; + unsigned int pretimeout; + unsigned int min_pretimeout; + unsigned int max_pretimeout; void *driver_data; struct mutex lock; unsigned long status; @@ -116,8 +124,22 @@ static inline void watchdog_set_nowayout(struct watchdog_device *wdd, bool noway /* Use the following function to check if a timeout value is invalid */ static inline bool watchdog_timeout_invalid(struct watchdog_device *wdd, unsigned int t) { - return ((wdd->max_timeout != 0) && - (t < wdd->min_timeout || t > wdd->max_timeout)); + return (wdd->max_timeout && + (t < wdd->min_timeout || t > wdd->max_timeout)) || + (wdd->pretimeout && t <= wdd->pretimeout); +} + +/* + * Use the following function to check if a pretimeout value is invalid. + * It can be "0", that means we don't use pretimeout. + * This function returns false, when pretimeout is 0. + */ +static inline bool watchdog_pretimeout_invalid(struct watchdog_device *wdd, + unsigned int t) +{ + return t && ((wdd->max_pretimeout && + (t < wdd->min_pretimeout || t > wdd->max_pretimeout)) || + (wdd->timeout && t >= wdd->timeout)); }
/* Use the following functions to manipulate watchdog driver specific data */ @@ -132,8 +154,17 @@ static inline void *watchdog_get_drvdata(struct watchdog_device *wdd) }
/* drivers/watchdog/watchdog_core.c */ -extern int watchdog_init_timeout(struct watchdog_device *wdd, - unsigned int timeout_parm, struct device *dev); +int watchdog_init_timeouts(struct watchdog_device *wdd, + unsigned int pretimeout_parm, + unsigned int timeout_parm, + struct device *dev); +static inline int watchdog_init_timeout(struct watchdog_device *wdd, + unsigned int timeout_parm, + struct device *dev) +{ + return watchdog_init_timeouts(wdd, 0, timeout_parm, dev); +} + extern int watchdog_register_device(struct watchdog_device *); extern void watchdog_unregister_device(struct watchdog_device *);
Hi Guenter,
Any suggestion on this v6 patchset, for now , I only got : (1) delete WCV output: diff --git a/drivers/watchdog/sbsa_gwdt.c b/drivers/watchdog/sbsa_gwdt.c index 2de5899..c42883b 100644 --- a/drivers/watchdog/sbsa_gwdt.c +++ b/drivers/watchdog/sbsa_gwdt.c @@ -344,8 +342,7 @@ static int sbsa_gwdt_probe(struct platform_device *pdev)
status = readl_relaxed(gwdt->control_base + SBSA_GWDT_WCS); if (status & SBSA_GWDT_WCS_WS1) { - dev_warn(dev, "System reset by WDT(WCV: %llx)\n", - sbsa_gwdt_get_wcv(wdd)); + dev_warn(dev, "System reset by WDT.\n"); wdd->bootstatus |= WDIOF_CARDRESET; } else if (status == (SBSA_GWDT_WCS_WS0 | SBSA_GWDT_WCS_EN)) { gwdt->ws0_mode = true;
any question or anything I can improve for this patchset?
Great thanks for your help.
On 23 June 2015 at 22:16, fu.wei@linaro.org wrote:
From: Fu Wei fu.wei@linaro.org
This patchset: (1)Introduce Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt for FDT info of SBSA Generic Watchdog, and give two examples of adding SBSA Generic Watchdog device node into the dts files: foundation-v8.dts and amd-seattle-soc.dtsi.
(2)Introduce "pretimeout" into the watchdog framework, and update Documentation/watchdog/watchdog-kernel-api.txt to introduce: (1)the new elements in the watchdog_device and watchdog_ops struct; (2)the new API "watchdog_init_timeouts". (3)Introduce ARM SBSA watchdog driver: a.Use linux kernel watchdog framework; b.Work with FDT on ARM64; c.Use "pretimeout" in watchdog framework; d.Support getting timeout and pretimeout from parameter and FDT at the driver init stage. e.In the first timeout, do panic to save system context; f.In the second stage, user can still feed the dog without cleaning WS0. By this feature, we can avoid the panic infinite loops, while backing up a large system context in a server. g.In the second stage, can trigger WS1 by setting pretimeout = 0 if necessary. (4)Introduce ACPI GTDT parser: drivers/acpi/gtdt.c Parse SBSA Generic Watchdog Structure in GTDT table of ACPI, and create a platform device with that information. This platform device can be used by This Watchdog driver. drivers/clocksource/arm_arch_timer.c is simplified by this GTDT support.
This patchset has been tested with watchdog daemon (ACPI/FDT, module/build-in) on the following platforms: (1)ARM Foundation v8 model
Changelog: v6: Improve the dtb example files: reduce the register frame size to 4K. Improve pretimeout support: (1) improve watchdog_init_timeouts function (2) rename watchdog_check_min_max_timeouts back to the original name (1) improve watchdog_timeout_invalid/watchdog_pretimeout_invalid Add the new features in the sbsa_gwdt driver: (1) In the second stage, user can feed the dog without cleaning WS0. (2) In the second stage, user can trigger WS1 by setting pretimeout = 0. (3) expand the max value of pretimeout, in case 10 second is not enough for a kdump kernel reboot in panic.
v5: Improve pretimeout support: (1)fix typo in documentation and comments. (2)fix the timeout limits validation bug. Simplify sbsa_gwdt driver: (1)integrate all the registers access functions into caller.
v4: Refactor GTDT support code: remove it from arch/arm64/kernel/acpi.c, put it into drivers/acpi/gtdt.c file. Integrate the GTDT code of drivers/clocksource/arm_arch_timer.c into drivers/acpi/gtdt.c. Improve pretimeout support, fix "pretimeout == 0" problem. Simplify sbsa_gwdt driver: (1)timeout/pretimeout limits setup; (2)keepalive function; (3)delete "clk == 0" check; (4)delete WS0 status bit check in interrupt routine; (5)sbsa_gwdt_set_wcv function.
v3: Delete "export arch_timer_get_rate" patch. Driver back to use arch_timer_get_cntfrq. Improve watchdog_init_timeouts function and update relevant documentation. Improve watchdog_timeout_invalid and watchdog_pretimeout_invalid. Improve foundation-v8.dts: delete the unnecessary tag of device node. Remove "ARM64 || COMPILE_TEST" from Kconfig. Add comments in arch/arm64/kernel/acpi.c Fix typoes and incorrect comments.
v2: Improve watchdog-kernel-api.txt documentation for pretimeout support. Export "arch_timer_get_rate" in arm_arch_timer.c. Add watchdog_init_timeouts API for pretimeout support in framework. Improve suspend and resume foundation in driver Improve timeout/pretimeout values init code in driver. Delete unnecessary items of the sbsa_gwdt struct and #define. Delete all unnecessary debug info in driver. Fix 64bit division bug. Use the arch_timer interface to get watchdog clock rate. Add MODULE_DEVICE_TABLE for platform device id. Fix typoes.
v1: The first version upstream patchset to linux mailing list.
Fu Wei (8): Documentation: add sbsa-gwdt.txt documentation ARM64: add SBSA Generic Watchdog device node in foundation-v8.dts ARM64: add SBSA Generic Watchdog device node in amd-seattle-soc.dtsi Watchdog: introdouce "pretimeout" into framework Watchdog: introduce ARM SBSA watchdog driver ACPI: add GTDT table parse driver into ACPI driver Watchdog: enable ACPI GTDT support for ARM SBSA watchdog driver clocksource: simplify ACPI code in arm_arch_timer.c
.../devicetree/bindings/watchdog/sbsa-gwdt.txt | 36 ++ Documentation/watchdog/watchdog-kernel-api.txt | 47 ++- arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi | 11 + arch/arm64/boot/dts/arm/foundation-v8.dts | 10 + arch/arm64/kernel/time.c | 4 +- drivers/acpi/Kconfig | 9 + drivers/acpi/Makefile | 1 + drivers/acpi/gtdt.c | 180 ++++++++ drivers/clocksource/Kconfig | 1 + drivers/clocksource/arm_arch_timer.c | 60 +-- drivers/watchdog/Kconfig | 15 + drivers/watchdog/Makefile | 1 + drivers/watchdog/sbsa_gwdt.c | 455 +++++++++++++++++++++ drivers/watchdog/watchdog_core.c | 98 +++-- drivers/watchdog/watchdog_dev.c | 53 +++ include/clocksource/arm_arch_timer.h | 8 + include/linux/acpi.h | 5 + include/linux/clocksource.h | 4 +- include/linux/watchdog.h | 39 +- 19 files changed, 947 insertions(+), 90 deletions(-) create mode 100644 Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt create mode 100644 drivers/acpi/gtdt.c create mode 100644 drivers/watchdog/sbsa_gwdt.c
-- 1.9.1
On 06/29/2015 09:53 AM, Fu Wei wrote:
Hi Guenter,
Any suggestion on this v6 patchset, for now , I only got :
Problem is that each version of your patchset tends to introduce substantially new or different functionality, meaning the review has to pretty much start from scratch. Right now I just don't have time to do that, so you'll have to be a bit patient.
Guenter
Hi Guenter,
Great thanks for your time, I can't tell you how much I appreciate your review! :-)
np, I will improve my patchset, once I get your suggestion. :-)
On 30 June 2015 at 03:16, Guenter Roeck linux@roeck-us.net wrote:
On 06/29/2015 09:53 AM, Fu Wei wrote:
Hi Guenter,
Any suggestion on this v6 patchset, for now , I only got :
Problem is that each version of your patchset tends to introduce substantially new or different functionality, meaning the review has to pretty much start from scratch. Right now I just don't have time to do that, so you'll have to be a bit patient.
Guenter
Hi Guenter,
If you get some time, could you help me on this patchset again? Great thanks for your help!
On 30 June 2015 at 03:16, Guenter Roeck linux@roeck-us.net wrote:
On 06/29/2015 09:53 AM, Fu Wei wrote:
Hi Guenter,
Any suggestion on this v6 patchset, for now , I only got :
Problem is that each version of your patchset tends to introduce substantially new or different functionality, meaning the review has to pretty much start from scratch. Right now I just don't have time to do that, so you'll have to be a bit patient.
Guenter
On Mon, Jul 13, 2015 at 05:09:57PM +0800, Fu Wei wrote:
Hi Guenter,
If you get some time, could you help me on this patchset again? Great thanks for your help!
I am on vacation this week. Hopefully next week or two weeks from now, depending on my work load.
Guenter
Hi Guenter,
Great thanks for your info, looking forward to your feedback. :-)
Have a good vacation! :-)
On 13 July 2015 at 23:34, Guenter Roeck linux@roeck-us.net wrote:
On Mon, Jul 13, 2015 at 05:09:57PM +0800, Fu Wei wrote:
Hi Guenter,
If you get some time, could you help me on this patchset again? Great thanks for your help!
I am on vacation this week. Hopefully next week or two weeks from now, depending on my work load.
Guenter