From: Fu Wei fu.wei@linaro.org
Changelog: v3: improve sbsa-gwdt.txt documentation delete #address-cells and #size-cells simplify resource name, fix typo update foundation-v8.dts and amd-seattle-soc.dtsi add "pretimeout" into Watchdog framework improve driver delete unnecessary check, add debug messages. fix unnecessary pr_crit (to pr_debug) in WS0 irq routine simplify date struct add debug option into Kconfig/Makefile simplify ACPI GTDT support code fix code style problem by "checkpatch --strict"
v2: enable modularize support both ACPI and FDT separate ACPI support code from driver to arch/arm64 acpi supprt move a function to header file for reusing the code simplify WS0 irq routine: using panic() add example dts code for foundation-v8 and AMD seattle Soc add sbsa-gwdt.txt documentation for FDT
Test on ARM Foundation v8 model with watchdog daemon (ACPI/FDT, module/build-in)
v1: draft patch for RFC and review
Fu Wei (7): 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: introdouce ARM SBSA watchdog driver Watchdog: add debug option into Kconfig/Makefile ACPI: import watchdog info of GTDT into platform device
.../devicetree/bindings/watchdog/sbsa-gwdt.txt | 36 ++ arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi | 11 + arch/arm64/boot/dts/arm/foundation-v8.dts | 10 + arch/arm64/kernel/acpi.c | 131 +++++ drivers/watchdog/Kconfig | 17 + drivers/watchdog/Makefile | 3 + drivers/watchdog/sbsa_gwdt.c | 560 +++++++++++++++++++++ drivers/watchdog/watchdog_core.c | 66 +++ drivers/watchdog/watchdog_dev.c | 48 ++ include/linux/watchdog.h | 19 + 10 files changed, 901 insertions(+) create mode 100644 Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt 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.
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..7fffb42 --- /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 resuming system operation +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". the "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@2a450000 { + compatible = "arm,sbsa-gwdt"; + reg = <0x0 0x2a450000 0 0x10000>, + <0x0 0x2a440000 0 0x10000>; + reg-names = "refresh", "control"; + interrupts = <0 27 4>; + interrupt-names = "ws0" + timeout-sec = <10 5>; +};
On 05/12/2015 03:56 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.
The subject line does not need a '.'.
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..7fffb42 --- /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 resuming system operation +after two stages of timeout.
"resuming system operation" ? Isn't it more like "resetting the system" ?
+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". the "ws1" is optional.
s/the//
+Optional properties +- timeout-sec : Watchdog pre-timeout and timeout values (in seconds).
- The first is timeout values, then pre-timeout.
You'll need to copy the devicetree mailing list to get feedback on this file, and to get feedback on the modified meaning of timeout-sec. Specific question to answer is if timeout-sec should be used for both timeout and pre-timeout, or if a new property such as pretimeout-sec should be introduced. This is especially important since another property, early-timeout-sec, is in the process of being added.
+Example for FVP Foundation Model v8:
+watchdog@2a450000 {
- compatible = "arm,sbsa-gwdt";
- reg = <0x0 0x2a450000 0 0x10000>,
<0x0 0x2a440000 0 0x10000>;
Does this warrant a more detailed explanation, or are people expected to know how this is formatted ?
- reg-names = "refresh", "control";
- interrupts = <0 27 4>;
- interrupt-names = "ws0"
Is there a missing ';' ?
- timeout-sec = <10 5>;
+};
Hi Guenter,
Great thanks for your review, comment inline below :
On 12 May 2015 at 21:31, Guenter Roeck linux@roeck-us.net wrote:
On 05/12/2015 03:56 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.
The subject line does not need a '.'.
OK, will delete it :-)
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..7fffb42 --- /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 resuming system operation +after two stages of timeout.
"resuming system operation" ? Isn't it more like "resetting the system" ?
yes, that makes sense, I just copied this from another doc, so ... I think "resetting" is better.
+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". the "ws1" is optional.
s/the//
Ok, thanks
+Optional properties
+- timeout-sec : Watchdog pre-timeout and timeout values (in seconds).
The first is timeout values, then pre-timeout.
You'll need to copy the devicetree mailing list to get feedback on this file, and to get feedback on the modified meaning of timeout-sec. Specific question to answer is if timeout-sec should be used for both timeout and pre-timeout, or if a new property such as pretimeout-sec should be introduced. This is especially important since another property, early-timeout-sec, is in the process of being added.
yes, good idea, as you know I just send it into linaro-acpi list for review first, but will send to dt mailing list once I fix this patch :-)
+Example for FVP Foundation Model v8:
+watchdog@2a450000 {
compatible = "arm,sbsa-gwdt";
reg = <0x0 0x2a450000 0 0x10000>,
<0x0 0x2a440000 0 0x10000>;
Does this warrant a more detailed explanation, or are people expected to know how this is formatted ?
I think people should know that , because is a address of device reg just like other devices. for AArch64 Foundation model #address-cells = <2>; #size-cells = <2>;
reg-names = "refresh", "control";
interrupts = <0 27 4>;
interrupt-names = "ws0"
Is there a missing ';' ?
yes, my fault, will fix it
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 file for the Soc which contains SBSA Generic Watchdog.
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..e3fae16 100644 --- a/arch/arm64/boot/dts/arm/foundation-v8.dts +++ b/arch/arm64/boot/dts/arm/foundation-v8.dts @@ -237,4 +237,14 @@ }; }; }; + watchdog0: watchdog@2a450000 { + compatible = "arm,sbsa-gwdt"; + reg = <0x0 0x2a450000 0 0x10000>, + <0x0 0x2a440000 0 0x10000>; + reg-names = "refresh", + "control"; + 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 file for the Soc which contains SBSA Generic Watchdog.
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..95994eb 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 0x10000>, + <0x0 0xe0bc0000 0 0x10000>; + 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
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 dirvers are going to use this: ARM SBSA Generic Watchdog
Signed-off-by: Fu Wei fu.wei@linaro.org --- drivers/watchdog/watchdog_core.c | 66 ++++++++++++++++++++++++++++++++++++++++ drivers/watchdog/watchdog_dev.c | 48 +++++++++++++++++++++++++++++ include/linux/watchdog.h | 19 ++++++++++++ 3 files changed, 133 insertions(+)
diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c index cec9b55..6ca9578 100644 --- a/drivers/watchdog/watchdog_core.c +++ b/drivers/watchdog/watchdog_core.c @@ -54,6 +54,14 @@ static void watchdog_check_min_max_timeout(struct watchdog_device *wdd) wdd->min_timeout = 0; wdd->max_timeout = 0; } + if (wdd->min_pretimeout && wdd->min_timeout < wdd->min_pretimeout) { + pr_info("Invalid min timeout, resetting to min pretimeout!\n"); + wdd->min_timeout = wdd->min_pretimeout; + } + if (wdd->max_pretimeout && wdd->max_timeout < wdd->max_pretimeout) { + pr_info("Invalid max timeout, resetting to max pretimeout!\n"); + wdd->max_timeout = wdd->max_pretimeout; + } }
/** @@ -98,6 +106,63 @@ int watchdog_init_timeout(struct watchdog_device *wdd, } EXPORT_SYMBOL_GPL(watchdog_init_timeout);
+static void watchdog_check_min_max_pretimeout(struct watchdog_device *wdd) +{ + /* + * Check that we have valid min and max pretimeout values, if + * not reset them both to 0 (=not used or unknown) + */ + if (wdd->min_pretimeout > wdd->max_pretimeout) { + pr_info("Invalid min and max pretimeout, resetting to 0!\n"); + wdd->min_pretimeout = 0; + wdd->max_pretimeout = 0; + } +} + +/** + * watchdog_init_pretimeout() - initialize the pretimeout field + * @pretimeout_parm: pretimeout module parameter + * @dev: Device that stores the timeout-sec property + * + * Initialize the pretimeout field of the watchdog_device struct with either + * the pretimeout 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 pretimeout value. + * + * A zero is returned on success and -EINVAL for failure. + */ +int watchdog_init_pretimeout(struct watchdog_device *wdd, + unsigned int pretimeout_parm, struct device *dev) +{ + int ret = 0; + u32 timeouts[2]; + + watchdog_check_min_max_pretimeout(wdd); + + /* try to get the timeout module parameter first */ + if (!watchdog_pretimeout_invalid(wdd, pretimeout_parm) && + pretimeout_parm) { + wdd->pretimeout = pretimeout_parm; + return ret; + } + if (pretimeout_parm) + ret = -EINVAL; + + /* try to get the timeout_sec property */ + if (!dev || !dev->of_node) + return ret; + ret = of_property_read_u32_array(dev->of_node, + "timeout-sec", timeouts, 2); + if (!watchdog_pretimeout_invalid(wdd, timeouts[1]) && timeouts[1]) + wdd->pretimeout = timeouts[1]; + else + ret = -EINVAL; + + return ret; +} +EXPORT_SYMBOL_GPL(watchdog_init_pretimeout); + /** * watchdog_register_device() - register a watchdog device * @wdd: watchdog device @@ -119,6 +184,7 @@ int watchdog_register_device(struct watchdog_device *wdd) if (wdd->ops->start == NULL || wdd->ops->stop == NULL) return -EINVAL;
+ watchdog_check_min_max_pretimeout(wdd); watchdog_check_min_max_timeout(wdd);
/* diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c index 6aaefba..b519257 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,22 @@ 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: + 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 keep's running (and if + * possible that it takes the new timeout) */ + watchdog_ping(wdd); + /* Fall */ + case WDIOC_GETPRETIMEOUT: + /* timeout == 0 means that we don't use the pretimeout */ + if (wdd->pretimeout == 0) + return -EOPNOTSUPP; + return put_user(wdd->pretimeout, p); 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..f0a3c5b 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; @@ -120,6 +128,14 @@ static inline bool watchdog_timeout_invalid(struct watchdog_device *wdd, unsigne (t < wdd->min_timeout || t > wdd->max_timeout)); }
+/* Use the following function to check if a pretimeout value is invalid */ +static inline bool watchdog_pretimeout_invalid(struct watchdog_device *wdd, + unsigned int t) +{ + return ((wdd->max_pretimeout != 0) && + (t < wdd->min_pretimeout || t > wdd->max_pretimeout)); +} + /* Use the following functions to manipulate watchdog driver specific data */ static inline void watchdog_set_drvdata(struct watchdog_device *wdd, void *data) { @@ -134,6 +150,9 @@ 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); +extern int watchdog_init_pretimeout(struct watchdog_device *wdd, + unsigned int pretimeout_parm, + struct device *dev); extern int watchdog_register_device(struct watchdog_device *); extern void watchdog_unregister_device(struct watchdog_device *);
From: Fu Wei fu.wei@linaro.org
(1)Use linux kernel watchdog framework (2)Work with FDT on ARM64 (3)Use "pretimeout" in watchdog framework. (4)In first timeout(WS0), do panic to save system context.
Signed-off-by: Fu Wei fu.wei@linaro.org --- drivers/watchdog/Kconfig | 10 + drivers/watchdog/Makefile | 1 + drivers/watchdog/sbsa_gwdt.c | 560 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 571 insertions(+) create mode 100644 drivers/watchdog/sbsa_gwdt.c
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index e5e7c55..46d6f46 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -152,6 +152,16 @@ config ARM_SP805_WATCHDOG ARM Primecell SP805 Watchdog timer. This will reboot your system when the timeout is reached.
+config ARM_SBSA_WATCHDOG + tristate "ARM SBSA Generic Watchdog" + depends on ARM64 && ARM_AMBA + select WATCHDOG_CORE + help + ARM SBSA Generic Watchdog timer. This has two Watchdog Signal(WS0/WS1), + will trigger a warnning interrupt(do panic) in the first timeout(WS0); + will reboot your system when the second timeout(WS1) is reached. + More details: DEN0029B - Server Base System Architecture (SBSA) + config AT91RM9200_WATCHDOG tristate "AT91RM9200 watchdog" depends on SOC_AT91RM9200 && MFD_SYSCON diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index 5c19294..471f1b7c 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -30,6 +30,7 @@ obj-$(CONFIG_USBPCWATCHDOG) += pcwd_usb.o
# ARM Architecture obj-$(CONFIG_ARM_SP805_WATCHDOG) += sp805_wdt.o +obj-$(CONFIG_ARM_SBSA_WATCHDOG) += sbsa_gwdt.o obj-$(CONFIG_AT91RM9200_WATCHDOG) += at91rm9200_wdt.o obj-$(CONFIG_AT91SAM9X_WATCHDOG) += at91sam9_wdt.o obj-$(CONFIG_CADENCE_WATCHDOG) += cadence_wdt.o diff --git a/drivers/watchdog/sbsa_gwdt.c b/drivers/watchdog/sbsa_gwdt.c new file mode 100644 index 0000000..ef9d689 --- /dev/null +++ b/drivers/watchdog/sbsa_gwdt.c @@ -0,0 +1,560 @@ +/* + * SBSA(Server Base System Architecture) Generic Watchdog driver + * + * Copyright (c) 2015, Linaro Ltd. + * Author: Fu Wei fu.wei@linaro.org + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License 2 as published + * by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * Note: This SBSA Generic watchdog driver is compatible with + * the pretimeout concept of Linux kernel. + * But timeout and pretimeout are set by the different REGs. + * The first watch period is set by writing WCV directly, + * that can support more than 10s timeout at the maximum + * system counter frequency. + * And the second watch period is set by WOR(32bit) which will be loaded + * automatically by hardware, when WS0 is triggered. + * This gives a maximum watch period of around 10s at the maximum + * system counter frequency. + * The System Counter shall run at maximum of 400MHz. + * More details: DEN0029B - Server Base System Architecture (SBSA) + * + * Kernel/API: P---------| pretimeout + * |-------------------------------T timeout + * SBSA GWDT: P--WOR---WS1 pretimeout + * |-------WCV----------WS0~~~~~~~~T timeout + */ + +#include <asm/arch_timer.h> + +#include <linux/acpi.h> +#include <linux/io.h> +#include <linux/interrupt.h> +#include <linux/module.h> +#include <linux/moduleparam.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> +#include <linux/spinlock.h> +#include <linux/uaccess.h> +#include <linux/watchdog.h> + +/* SBSA Generic Watchdog register definitions */ +/* refresh frame */ +#define SBSA_GWDT_WRR 0x000 + +/* control frame */ +#define SBSA_GWDT_WCS 0x000 +#define SBSA_GWDT_WOR 0x008 +#define SBSA_GWDT_WCV_LO 0x010 +#define SBSA_GWDT_WCV_HI 0x014 + +/* refresh/control frame */ +#define SBSA_GWDT_W_IIDR 0xfcc +#define SBSA_GWDT_IDR 0xfd0 + +/* Watchdog Control and Status Register */ +#define SBSA_GWDT_WCS_EN BIT(0) +#define SBSA_GWDT_WCS_WS0 BIT(1) +#define SBSA_GWDT_WCS_WS1 BIT(2) + +/* Watchdog Interface Identification Register */ +#define SBSA_GWDT_W_IIDR_PID(x) ((x >> 20) & 0xfff) +#define SBSA_GWDT_W_IIDR_ARCH_VER(x) ((x >> 16) & 0xf) +#define SBSA_GWDT_W_IIDR_REV(x) ((x >> 12) & 0xf) +#define SBSA_GWDT_W_IIDR_IMPLEMENTER(x) (x & 0xfff) +#define SBSA_GWDT_W_IIDR_VID_BANK(x) ((x >> 8) & 0xf) +#define SBSA_GWDT_W_IIDR_VID(x) (x & 0x7f) + +/* Watchdog Identification Register */ +#define SBSA_GWDT_IDR_W_PIDR2 0xfe8 +#define SBSA_GWDT_IDR_W_PIDR2_ARCH_VER(x) ((x >> 4) & 0xf) + +/** + * struct sbsa_gwdt - Internal representation of the SBSA GWDT + * @wdd: kernel watchdog_device structure + * @clk: store the System Counter clock frequency, in Hz. + * @refresh_base: VA of the watchdog refresh frame + * @control_base: VA of the watchdog control frame + * @lock: struct sbsa_gwdt spinlock + * @pm_status_store: store the PM info of WDT + */ +struct sbsa_gwdt { + struct watchdog_device wdd; + u32 clk; + void __iomem *refresh_base; + void __iomem *control_base; +#ifdef CONFIG_PM_SLEEP + spinlock_t lock; + u8 pm_status_store; +#endif +}; + +#define to_sbsa_gwdt(e) container_of(e, struct sbsa_gwdt, wdd) + +#define DEFAULT_TIMEOUT 15 /* seconds, the 1st + 2nd watch period*/ +#define DEFAULT_PRETIMEOUT 5 /* seconds, the 2nd watch period*/ + +static unsigned int timeout = DEFAULT_TIMEOUT; +module_param(timeout, uint, 0); +MODULE_PARM_DESC(timeout, + "Watchdog timeout in seconds. (>=0, default=" + __MODULE_STRING(DEFAULT_TIMEOUT) ")"); + +static unsigned int max_timeout = UINT_MAX; +module_param(max_timeout, uint, 0); +MODULE_PARM_DESC(max_timeout, + "Watchdog max timeout in seconds. (>=0, default=" + __MODULE_STRING(UINT_MAX) ")"); + +static unsigned int max_pretimeout = U32_MAX; +module_param(max_pretimeout, uint, 0); +MODULE_PARM_DESC(max_pretimeout, + "Watchdog max pretimeout in seconds. (>=0, default=" + __MODULE_STRING(U32_MAX) ")"); + +static unsigned int pretimeout = DEFAULT_PRETIMEOUT; +module_param(pretimeout, uint, 0); +MODULE_PARM_DESC(pretimeout, + "Watchdog pretimeout in seconds. (>=0, default=" + __MODULE_STRING(DEFAULT_PRETIMEOUT) ")"); + +static bool nowayout = WATCHDOG_NOWAYOUT; +module_param(nowayout, bool, S_IRUGO); +MODULE_PARM_DESC(nowayout, + "Watchdog cannot be stopped once started (default=" + __MODULE_STRING(WATCHDOG_NOWAYOUT) ")"); + +/* + * Architected system timer support. + */ +static void sbsa_gwdt_cf_write(unsigned int reg, u32 val, + struct watchdog_device *wdd) +{ + struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd); + + writel_relaxed(val, gwdt->control_base + reg); +} + +static void sbsa_gwdt_rf_write(unsigned int reg, u32 val, + struct watchdog_device *wdd) +{ + struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd); + + writel_relaxed(val, gwdt->refresh_base + reg); +} + +static u32 sbsa_gwdt_cf_read(unsigned int reg, struct watchdog_device *wdd) +{ + struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd); + + return readl_relaxed(gwdt->control_base + reg); +} + +/* +static u32 sbsa_gwdt_rf_read(unsigned int reg, struct watchdog_device *wdd) +{ + struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd); + + return readl_relaxed(gwdt->refresh_base + reg); +} +*/ + +/* + * help founctions for accessing 64bit WCV register + * mutex_lock must be called prior to calling this function. + */ +static u64 sbsa_gwdt_get_wcv(struct watchdog_device *wdd) +{ + u32 wcv_lo, wcv_hi; + + wcv_lo = sbsa_gwdt_cf_read(SBSA_GWDT_WCV_LO, wdd); + wcv_hi = sbsa_gwdt_cf_read(SBSA_GWDT_WCV_HI, wdd); + + return (((u64)wcv_hi << 32) | wcv_lo); +} + +static int sbsa_gwdt_set_wcv(struct watchdog_device *wdd, u64 value) +{ + u32 wcv_lo, wcv_hi; + + wcv_lo = value & U32_MAX; + wcv_hi = (value >> 32) & U32_MAX; + + sbsa_gwdt_cf_write(SBSA_GWDT_WCV_LO, wcv_lo, wdd); + sbsa_gwdt_cf_write(SBSA_GWDT_WCV_HI, wcv_hi, wdd); + + pr_debug("sbsa_gwdt: set WCV to %llu, result: %llu\n", + value, sbsa_gwdt_get_wcv(wdd)); + + return 0; +} + +static int reload_timeout_to_wcv(struct watchdog_device *wdd) +{ + struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd); + u64 wcv; + + wcv = arch_counter_get_cntvct() + + (wdd->timeout - wdd->pretimeout) * gwdt->clk; + + /* refresh the wcv */ + return sbsa_gwdt_set_wcv(wdd, wcv); +} + +/* Use the following function to set the limit of timeout + * after updating pretimeout */ +void sbsa_gwdt_set_timeout_limits(struct sbsa_gwdt *gwdt) +{ + unsigned int first_period_max = (U64_MAX / gwdt->clk); + struct watchdog_device *wdd = &gwdt->wdd; + + wdd->min_timeout = wdd->pretimeout + 1; + wdd->max_timeout = min(wdd->pretimeout + first_period_max, + max_timeout); + + pr_debug("sbsa_gwdt: timeout (%u-%u), pretimeout (%u-%u)\n", + wdd->min_timeout, wdd->max_timeout, + wdd->min_pretimeout, wdd->max_pretimeout); +} + +static int sbsa_gwdt_set_timeout(struct watchdog_device *wdd, + unsigned int timeout) +{ + wdd->timeout = timeout; + + return reload_timeout_to_wcv(wdd); /* refresh the wcv */ +} + +static int sbsa_gwdt_set_pretimeout(struct watchdog_device *wdd, + unsigned int pretimeout) +{ + struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd); + u32 wor; + + wdd->pretimeout = pretimeout; + sbsa_gwdt_set_timeout_limits(gwdt); + + /* refresh the WOR, that will cause an explicit watchdog refresh */ + wor = pretimeout * gwdt->clk; + sbsa_gwdt_cf_write(SBSA_GWDT_WOR, wor, wdd); + + pr_debug("sbsa_gwdt: set WOR to %x(%us), result: %x\n", + wor, pretimeout, sbsa_gwdt_cf_read(SBSA_GWDT_WOR, wdd)); + + return 0; +} + +static unsigned int sbsa_gwdt_get_timeleft(struct watchdog_device *wdd) +{ + struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd); + u64 timeleft = sbsa_gwdt_get_wcv(wdd) - arch_counter_get_cntvct(); + + return (unsigned int)(timeleft / gwdt->clk); +} + +static int sbsa_gwdt_start(struct watchdog_device *wdd) +{ + u32 status; + int ret; + + ret = reload_timeout_to_wcv(wdd); + if (ret) + return ret; + + status = sbsa_gwdt_cf_read(SBSA_GWDT_WCS, wdd); + status |= SBSA_GWDT_WCS_EN; + /* writing WCS will cause an explicit watchdog refresh */ + sbsa_gwdt_cf_write(SBSA_GWDT_WCS, status, wdd); + + pr_debug("sbsa_gwdt: WCS is %x(%s)\n", + sbsa_gwdt_cf_read(SBSA_GWDT_WCS, wdd), __func__); + + return 0; +} + +static int sbsa_gwdt_stop(struct watchdog_device *wdd) +{ + u32 status; + + status = sbsa_gwdt_cf_read(SBSA_GWDT_WCS, wdd); + status &= ~SBSA_GWDT_WCS_EN; + /* writing WCS will cause an explicit watchdog refresh */ + sbsa_gwdt_cf_write(SBSA_GWDT_WCS, status, wdd); + + pr_debug("sbsa_gwdt: WCS is %x(%s)\n", + sbsa_gwdt_cf_read(SBSA_GWDT_WCS, wdd), __func__); + + return 0; +} + +static int sbsa_gwdt_keepalive(struct watchdog_device *wdd) +{ + /* Writing WRR for an explicit watchdog refresh + * You can write anyting(like 0xc0ffee) + */ + sbsa_gwdt_rf_write(SBSA_GWDT_WRR, 0xc0ffee, wdd); + + pr_debug("sbsa_gwdt: ping, %xs left.\n", sbsa_gwdt_get_timeleft(wdd)); + + return reload_timeout_to_wcv(wdd); +} + +static irqreturn_t sbsa_gwdt_interrupt(int irq, void *dev_id) +{ + struct sbsa_gwdt *gwdt = (struct sbsa_gwdt *)dev_id; + struct watchdog_device *wdd = &gwdt->wdd; + u32 status; + + status = sbsa_gwdt_cf_read(SBSA_GWDT_WCS, wdd); + + pr_debug("sbsa_gwdt: interrupt routine, WCS@%x\n", status); + + if (status & SBSA_GWDT_WCS_WS0) { + pr_debug("sbsa_gwdt WS0: trigger WS1 in %ds!\n", + sbsa_gwdt_get_timeleft(wdd)); + panic("SBSA Watchdog pre-timeout"); + } + + return IRQ_HANDLED; +} + +static struct watchdog_info sbsa_gwdt_info = { + .identity = "SBSA Generic Watchdog", + .options = WDIOF_SETTIMEOUT | + WDIOF_KEEPALIVEPING | + WDIOF_MAGICCLOSE | + WDIOF_PRETIMEOUT | + WDIOF_CARDRESET, +}; + +static struct watchdog_ops sbsa_gwdt_ops = { + .owner = THIS_MODULE, + .start = sbsa_gwdt_start, + .stop = sbsa_gwdt_stop, + .ping = sbsa_gwdt_keepalive, + .set_timeout = sbsa_gwdt_set_timeout, + .set_pretimeout = sbsa_gwdt_set_pretimeout, + .get_timeleft = sbsa_gwdt_get_timeleft, +}; + +static int sbsa_gwdt_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + + struct sbsa_gwdt *gwdt; + struct watchdog_device *wdd; + + struct resource *res; + void *rf_base, *cf_base; + int irq; + u32 clk, status, w_iidr; + + int ret = 0; + + /* + * Try to determine the frequency from the cp15 interface + */ + clk = arch_timer_get_cntfrq(); + if (!clk) { + dev_err(dev, "System Counter frequency not available\n"); + return -EINVAL; + } + + gwdt = devm_kzalloc(dev, sizeof(*gwdt), GFP_KERNEL); + if (!gwdt) + return -ENOMEM; + + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "refresh"); + rf_base = devm_ioremap_resource(dev, res); + if (IS_ERR(rf_base)) + return PTR_ERR(rf_base); + + pr_debug("sbsa_gwdt: ioremap %s frame 0x%llx(size: %llu)-->%p.\n", + res->name, (unsigned long long)res->start, + (unsigned long long)(res->end - res->start + 1), rf_base); + + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "control"); + cf_base = devm_ioremap_resource(dev, res); + if (IS_ERR(rf_base)) + return PTR_ERR(rf_base); + + pr_debug("sbsa_gwdt: ioremap %s frame 0x%llx(size: %llu)-->%p.\n", + res->name, (unsigned long long)res->start, + (unsigned long long)(res->end - res->start + 1), cf_base); + + irq = platform_get_irq_byname(pdev, "ws0"); + if (irq < 0) { + dev_err(dev, "unable to get ws0 interrupt.\n"); + return irq; + } + + pr_debug("sbsa_gwdt: ws0 irq %d.\n", irq); + + gwdt->refresh_base = rf_base; + gwdt->control_base = cf_base; + gwdt->clk = clk; +#ifdef CONFIG_PM_SLEEP + spin_lock_init(&gwdt->lock); +#endif + platform_set_drvdata(pdev, gwdt); + + pr_debug("sbsa_gwdt: hw clk: %uHz--> WOR(32bit) max timeout is %us.\n", + gwdt->clk, U32_MAX / clk); + + wdd = &gwdt->wdd; + wdd->parent = dev; + wdd->info = &sbsa_gwdt_info; + wdd->ops = &sbsa_gwdt_ops; + watchdog_set_drvdata(wdd, gwdt); + watchdog_set_nowayout(wdd, nowayout); + + status = sbsa_gwdt_cf_read(SBSA_GWDT_WCS, wdd); + if (status & SBSA_GWDT_WCS_WS1) { + dev_warn(dev, "System was reseted by WDT(WCS: %x, WCV: %llx)\n", + status, sbsa_gwdt_get_wcv(wdd)); + wdd->bootstatus |= WDIOF_CARDRESET; + } + /* Check if watchdog is already enabled */ + if (status & SBSA_GWDT_WCS_EN) { + dev_warn(dev, "already enabled!\n"); + sbsa_gwdt_keepalive(wdd); + } + + pr_debug("sbsa_gwdt: WCS: %x(%s)\n", status, __func__); + + wdd->min_pretimeout = 0; + wdd->max_pretimeout = min(U32_MAX / clk, max_timeout - 1); + sbsa_gwdt_set_timeout_limits(gwdt); + + ret = watchdog_init_pretimeout(wdd, pretimeout, dev); + if (ret) { + pretimeout = DEFAULT_PRETIMEOUT; + dev_info(dev, "can't get pretimeout param, set default %us.\n", + pretimeout); + } + ret = watchdog_init_timeout(wdd, timeout, dev); + if (ret) { + timeout = DEFAULT_TIMEOUT; + dev_info(dev, "can't get timeout param, set default: %us.\n", + timeout); + } + + sbsa_gwdt_set_pretimeout(wdd, pretimeout); + sbsa_gwdt_set_timeout(wdd, timeout); + + ret = devm_request_irq(dev, irq, sbsa_gwdt_interrupt, IRQF_TIMER, + pdev->name, gwdt); + if (ret) { + dev_err(dev, "unable to request IRQ %d\n", irq); + return ret; + } + + ret = watchdog_register_device(wdd); + if (ret) + return ret; + + /* get device information from control frame and display */ + w_iidr = sbsa_gwdt_cf_read(SBSA_GWDT_W_IIDR, &gwdt->wdd); + dev_info(dev, "PID:%u(JEP106 %u-%u), Arch v%u Rev %u.", + SBSA_GWDT_W_IIDR_PID(w_iidr), + SBSA_GWDT_W_IIDR_VID_BANK(w_iidr), + SBSA_GWDT_W_IIDR_VID(w_iidr), + SBSA_GWDT_W_IIDR_ARCH_VER(w_iidr), + SBSA_GWDT_W_IIDR_REV(w_iidr)); + + dev_info(dev, "Initialized with %ds timeout, %ds pretimeout @ %uHz\n", + wdd->timeout, wdd->pretimeout, gwdt->clk); + + return 0; +} + +static void sbsa_gwdt_shutdown(struct platform_device *pdev) +{ + struct sbsa_gwdt *gwdt = platform_get_drvdata(pdev); + + sbsa_gwdt_stop(&gwdt->wdd); +} + +static int sbsa_gwdt_remove(struct platform_device *pdev) +{ + struct sbsa_gwdt *gwdt = platform_get_drvdata(pdev); + int ret = 0; + + if (!nowayout) + ret = sbsa_gwdt_stop(&gwdt->wdd); + + watchdog_unregister_device(&gwdt->wdd); + + return ret; +} + +#ifdef CONFIG_PM_SLEEP +/* Disable watchdog if it is active during suspend */ +static int sbsa_gwdt_suspend(struct device *dev) +{ + struct sbsa_gwdt *gwdt = dev_get_drvdata(dev); + struct watchdog_device *wdd = &gwdt->wdd; + + spin_lock(&gwdt->lock); + gwdt->pm_status_store = sbsa_gwdt_cf_read(SBSA_GWDT_WCS, wdd); + + if (gwdt->pm_status_store & SBSA_GWDT_WCS_EN) + sbsa_gwdt_stop(wdd); + spin_unlock(&gwdt->lock); + + return 0; +} + +/* Enable watchdog and configure it if necessary */ +static int sbsa_gwdt_resume(struct device *dev) +{ + struct sbsa_gwdt *gwdt = dev_get_drvdata(dev); + struct watchdog_device *wdd = &gwdt->wdd; + + spin_lock(&gwdt->lock); + if (gwdt->pm_status_store & SBSA_GWDT_WCS_EN) + sbsa_gwdt_start(wdd); + else + sbsa_gwdt_stop(wdd); + spin_unlock(&gwdt->lock); + + return 0; +} +#endif + +#if IS_BUILTIN(CONFIG_OF) +static const struct of_device_id sbsa_gwdt_of_match[] = { + { .compatible = "arm,sbsa-gwdt", }, + {}, +}; +MODULE_DEVICE_TABLE(of, sbsa_gwdt_of_match); +#endif + +static const struct dev_pm_ops sbsa_gwdt_pm_ops = { + SET_SYSTEM_SLEEP_PM_OPS(sbsa_gwdt_suspend, sbsa_gwdt_resume) +}; + +static struct platform_driver sbsa_gwdt_driver = { + .driver = { + .name = "sbsa-gwdt", + .pm = &sbsa_gwdt_pm_ops, + .of_match_table = sbsa_gwdt_of_match, + }, + .probe = sbsa_gwdt_probe, + .remove = sbsa_gwdt_remove, + .shutdown = sbsa_gwdt_shutdown, +}; + +module_platform_driver(sbsa_gwdt_driver); + +MODULE_DESCRIPTION("SBSA Generic Watchdog Driver"); +MODULE_AUTHOR("Fu Wei fu.wei@linaro.org"); +MODULE_LICENSE("GPL v2");
On Tuesday 12 May 2015, fu.wei@linaro.org wrote:
From: Fu Wei fu.wei@linaro.org
(1)Use linux kernel watchdog framework (2)Work with FDT on ARM64 (3)Use "pretimeout" in watchdog framework. (4)In first timeout(WS0), do panic to save system context.
Signed-off-by: Fu Wei fu.wei@linaro.org
drivers/watchdog/Kconfig | 10 + drivers/watchdog/Makefile | 1 + drivers/watchdog/sbsa_gwdt.c | 560 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 571 insertions(+) create mode 100644 drivers/watchdog/sbsa_gwdt.c
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index e5e7c55..46d6f46 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -152,6 +152,16 @@ config ARM_SP805_WATCHDOG ARM Primecell SP805 Watchdog timer. This will reboot your system when the timeout is reached. +config ARM_SBSA_WATCHDOG
- tristate "ARM SBSA Generic Watchdog"
- depends on ARM64 && ARM_AMBA
- select WATCHDOG_CORE
Ideally make it possible to build this on all architectures, so we get compile coverage on x86 builds.
You should also drop the ARM_AMBA dependency because this is regular platform_driver.
ARM_AMBA is only used for primecell devices, which this is not.
+#if IS_BUILTIN(CONFIG_OF) +static const struct of_device_id sbsa_gwdt_of_match[] = {
- { .compatible = "arm,sbsa-gwdt", },
- {},
+}; +MODULE_DEVICE_TABLE(of, sbsa_gwdt_of_match); +#endif
+static const struct dev_pm_ops sbsa_gwdt_pm_ops = {
- SET_SYSTEM_SLEEP_PM_OPS(sbsa_gwdt_suspend, sbsa_gwdt_resume)
+};
+static struct platform_driver sbsa_gwdt_driver = {
- .driver = {
.name = "sbsa-gwdt",
.pm = &sbsa_gwdt_pm_ops,
.of_match_table = sbsa_gwdt_of_match,
- },
But fix the undefined reference here first: sbsa_gwdt_of_match is hidden behind CONFIG_OF, which is always set on ARM64. Just remove the #ifdef and it will build on other architectures as well.
arnd
Hi Arnd,
Great thanks, will fix those: (1)drop ARM_AMBA (2)#if IS_BUILTIN(CONFIG_OF) and #endif (but can I set depends on OF ??)
Are you saying I can drop ARM64 also? this IP core maybe in ARM , but is that possible on other platform(x68/mips/ppc)?
On 12 May 2015 at 21:13, Arnd Bergmann arnd@arndb.de wrote:
On Tuesday 12 May 2015, fu.wei@linaro.org wrote:
From: Fu Wei fu.wei@linaro.org
(1)Use linux kernel watchdog framework (2)Work with FDT on ARM64 (3)Use "pretimeout" in watchdog framework. (4)In first timeout(WS0), do panic to save system context.
Signed-off-by: Fu Wei fu.wei@linaro.org
drivers/watchdog/Kconfig | 10 + drivers/watchdog/Makefile | 1 + drivers/watchdog/sbsa_gwdt.c | 560
+++++++++++++++++++++++++++++++++++++++++++
3 files changed, 571 insertions(+) create mode 100644 drivers/watchdog/sbsa_gwdt.c
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index e5e7c55..46d6f46 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -152,6 +152,16 @@ config ARM_SP805_WATCHDOG ARM Primecell SP805 Watchdog timer. This will reboot your system
when
the timeout is reached.
+config ARM_SBSA_WATCHDOG
tristate "ARM SBSA Generic Watchdog"
depends on ARM64 && ARM_AMBA
select WATCHDOG_CORE
Ideally make it possible to build this on all architectures, so we get compile coverage on x86 builds.
You should also drop the ARM_AMBA dependency because this is regular platform_driver.
ARM_AMBA is only used for primecell devices, which this is not.
+#if IS_BUILTIN(CONFIG_OF) +static const struct of_device_id sbsa_gwdt_of_match[] = {
{ .compatible = "arm,sbsa-gwdt", },
{},
+}; +MODULE_DEVICE_TABLE(of, sbsa_gwdt_of_match); +#endif
+static const struct dev_pm_ops sbsa_gwdt_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(sbsa_gwdt_suspend, sbsa_gwdt_resume)
+};
+static struct platform_driver sbsa_gwdt_driver = {
.driver = {
.name = "sbsa-gwdt",
.pm = &sbsa_gwdt_pm_ops,
.of_match_table = sbsa_gwdt_of_match,
},
But fix the undefined reference here first: sbsa_gwdt_of_match is hidden behind CONFIG_OF, which is always set on ARM64. Just remove the #ifdef and it will build on other architectures as well.
arnd
Arnd Bergmann wrote:
+config ARM_SBSA_WATCHDOG
- tristate "ARM SBSA Generic Watchdog"
- depends on ARM64 && ARM_AMBA
- select WATCHDOG_CORE
Ideally make it possible to build this on all architectures, so we get compile coverage on x86 builds.
I never understood this. This driver is only useful on an ARM64 Server system. Why do we care if it can't be built on x86?
Hi Arnd,
Sorry, I just found out that , "undefined reference " issue is stupid mistake, you won't see that in my next patchset
On 12 May 2015 at 21:13, Arnd Bergmann arnd@arndb.de wrote:
On Tuesday 12 May 2015, fu.wei@linaro.org wrote:
From: Fu Wei fu.wei@linaro.org
(1)Use linux kernel watchdog framework (2)Work with FDT on ARM64 (3)Use "pretimeout" in watchdog framework. (4)In first timeout(WS0), do panic to save system context.
Signed-off-by: Fu Wei fu.wei@linaro.org
drivers/watchdog/Kconfig | 10 + drivers/watchdog/Makefile | 1 + drivers/watchdog/sbsa_gwdt.c | 560 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 571 insertions(+) create mode 100644 drivers/watchdog/sbsa_gwdt.c
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index e5e7c55..46d6f46 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -152,6 +152,16 @@ config ARM_SP805_WATCHDOG ARM Primecell SP805 Watchdog timer. This will reboot your system when the timeout is reached.
+config ARM_SBSA_WATCHDOG
tristate "ARM SBSA Generic Watchdog"
depends on ARM64 && ARM_AMBA
select WATCHDOG_CORE
Ideally make it possible to build this on all architectures, so we get compile coverage on x86 builds.
You should also drop the ARM_AMBA dependency because this is regular platform_driver.
ARM_AMBA is only used for primecell devices, which this is not.
+#if IS_BUILTIN(CONFIG_OF) +static const struct of_device_id sbsa_gwdt_of_match[] = {
{ .compatible = "arm,sbsa-gwdt", },
{},
+}; +MODULE_DEVICE_TABLE(of, sbsa_gwdt_of_match); +#endif
+static const struct dev_pm_ops sbsa_gwdt_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(sbsa_gwdt_suspend, sbsa_gwdt_resume)
+};
+static struct platform_driver sbsa_gwdt_driver = {
.driver = {
.name = "sbsa-gwdt",
.pm = &sbsa_gwdt_pm_ops,
.of_match_table = sbsa_gwdt_of_match,
},
But fix the undefined reference here first: sbsa_gwdt_of_match is hidden behind CONFIG_OF, which is always set on ARM64. Just remove the #ifdef and it will build on other architectures as well.
arnd
From: Fu Wei fu.wei@linaro.org
This patch adds a debug config option which can add "-DDEBUG" flag in the build for development reason.
Reasons: There are some drivers of Watchdog using pr_debug, dev_debug or some other similar function which depend on "DEBUG". Making a config option to enable those message is convenient for development.
Signed-off-by: Fu Wei fu.wei@linaro.org --- drivers/watchdog/Kconfig | 7 +++++++ drivers/watchdog/Makefile | 2 ++ 2 files changed, 9 insertions(+)
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index 46d6f46..4bfb67d 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -36,6 +36,13 @@ config WATCHDOG_CORE and gives them the /dev/watchdog interface (and later also the sysfs interface).
+config WATCHDOG_DEBUG + bool "Debug support for WATCHDOG drivers" + depends on DEBUG_KERNEL + help + Say "yes" to enable debug messaging (like dev_dbg and pr_debug), + sysfs, and debugfs support in WatchDog controller and protocol drivers. + config WATCHDOG_NOWAYOUT bool "Disable watchdog shutdown on close" help diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index 471f1b7c..854b81d 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -2,6 +2,8 @@ # Makefile for the WatchDog device drivers. #
+ccflags-$(CONFIG_WATCHDOG_DEBUG) := -DDEBUG + # The WatchDog Timer Driver Core. watchdog-objs += watchdog_core.o watchdog_dev.o obj-$(CONFIG_WATCHDOG_CORE) += watchdog.o
fu.wei@linaro.org wrote:
+config WATCHDOG_DEBUG
- bool "Debug support for WATCHDOG drivers"
- depends on DEBUG_KERNEL
- help
Say "yes" to enable debug messaging (like dev_dbg and pr_debug),
sysfs, and debugfs support in WatchDog controller and protocol drivers.
I personally don't like Kconfig debugging options like that. If I want to debug a driver, I'll just add "#define DEBUG" to the top of the file and rebuild.
Hi Timur,
thanks , I just feel it's convenient if we have that, it is a option, it can be ignore or merge, :-)
see if people like it :-)
On 12 May 2015 at 22:40, Timur Tabi timur@codeaurora.org wrote:
fu.wei@linaro.org wrote:
+config WATCHDOG_DEBUG
bool "Debug support for WATCHDOG drivers"
depends on DEBUG_KERNEL
help
Say "yes" to enable debug messaging (like dev_dbg and pr_debug),
sysfs, and debugfs support in WatchDog controller and protocol
drivers.
I personally don't like Kconfig debugging options like that. If I want to debug a driver, I'll just add "#define DEBUG" to the top of the file and rebuild.
-- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation.
But the only thing that this Kconfig option does is add the -DDEBUG command-line parameter. If every driver did that, the Kconfigs would be a mess. And then, all that does is enable a bunch of pr_debugs in your driver.
The watchdog maintainer should decide, of course, but I think this kind of Kconfig option sets a bad precedent.
On 05/12/2015 10:03 AM, Fu Wei wrote:
Hi Timur,
thanks , I just feel it's convenient if we have that, it is a option, it can be ignore or merge, :-)
see if people like it :-)
On 12 May 2015 at 22:40, Timur Tabi <timur@codeaurora.org mailto:timur@codeaurora.org> wrote:
fu.wei@linaro.org <mailto:fu.wei@linaro.org> wrote: +config WATCHDOG_DEBUG + bool "Debug support for WATCHDOG drivers" + depends on DEBUG_KERNEL + help + Say "yes" to enable debug messaging (like dev_dbg and pr_debug), + sysfs, and debugfs support in WatchDog controller and protocol drivers. + I personally don't like Kconfig debugging options like that. If I want to debug a driver, I'll just add "#define DEBUG" to the top of the file and rebuild. -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation.
-- 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
Hi Timur,
yes, I am not 100% sure that is good, but I just learned it from other Kconfigs, they are doing this and I didn't see the bad side of it. because if we want to debug WDT , we may just debug one module ,and we wouldn't turn on it in production :-)
but just let watchdog maintainer decide it. for this patchset ,it is just a side-product :-)
Thanks
On 13 May 2015 at 01:25, Timur Tabi timur@codeaurora.org wrote:
But the only thing that this Kconfig option does is add the -DDEBUG command-line parameter. If every driver did that, the Kconfigs would be a mess. And then, all that does is enable a bunch of pr_debugs in your driver.
The watchdog maintainer should decide, of course, but I think this kind of Kconfig option sets a bad precedent.
On 05/12/2015 10:03 AM, Fu Wei wrote:
Hi Timur,
thanks , I just feel it's convenient if we have that, it is a option, it can be ignore or merge, :-)
see if people like it :-)
On 12 May 2015 at 22:40, Timur Tabi <timur@codeaurora.org mailto:timur@codeaurora.org> wrote:
fu.wei@linaro.org <mailto:fu.wei@linaro.org> wrote: +config WATCHDOG_DEBUG + bool "Debug support for WATCHDOG drivers" + depends on DEBUG_KERNEL + help + Say "yes" to enable debug messaging (like dev_dbg and pr_debug), + sysfs, and debugfs support in WatchDog controller and protocol drivers. + I personally don't like Kconfig debugging options like that. If I want to debug a driver, I'll just add "#define DEBUG" to the top of the file and rebuild. -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation.
-- 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
-- Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
On 12 May 2015 at 18:37, Fu Wei fu.wei@linaro.org wrote:
Hi Timur,
yes, I am not 100% sure that is good, but I just learned it from other Kconfigs, they are doing this and I didn't see the bad side of it. because if we want to debug WDT , we may just debug one module ,and we wouldn't turn on it in production :-)
but just let watchdog maintainer decide it. for this patchset ,it is just a side-product :-)
With this statement you answered the question, this is completely independent of your patchset and should go upstream separately.
Graeme
Thanks
On 13 May 2015 at 01:25, Timur Tabi timur@codeaurora.org wrote:
But the only thing that this Kconfig option does is add the -DDEBUG command-line parameter. If every driver did that, the Kconfigs would be a mess. And then, all that does is enable a bunch of pr_debugs in your driver.
The watchdog maintainer should decide, of course, but I think this kind of Kconfig option sets a bad precedent.
On 05/12/2015 10:03 AM, Fu Wei wrote:
Hi Timur,
thanks , I just feel it's convenient if we have that, it is a option, it can be ignore or merge, :-)
see if people like it :-)
On 12 May 2015 at 22:40, Timur Tabi <timur@codeaurora.org mailto:timur@codeaurora.org> wrote:
fu.wei@linaro.org <mailto:fu.wei@linaro.org> wrote: +config WATCHDOG_DEBUG + bool "Debug support for WATCHDOG drivers" + depends on DEBUG_KERNEL + help + Say "yes" to enable debug messaging (like dev_dbg and pr_debug), + sysfs, and debugfs support in WatchDog controller and protocol drivers. + I personally don't like Kconfig debugging options like that. If I want to debug a driver, I'll just add "#define DEBUG" to the top of the file and rebuild. -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation.
-- 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
-- Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
-- 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
On Tue, May 12, 2015 at 06:56:34PM +0800, fu.wei@linaro.org wrote:
From: Fu Wei fu.wei@linaro.org
This patch adds a debug config option which can add "-DDEBUG" flag in the build for development reason.
Reasons: There are some drivers of Watchdog using pr_debug, dev_debug or some other similar function which depend on "DEBUG". Making a config option to enable those message is convenient for development.
Signed-off-by: Fu Wei fu.wei@linaro.org
If I were the maintainer, I would NACK this one. I don't believe that cluttering the kernel configuration with debug options is a good idea.
Guenter
From: Fu Wei fu.wei@linaro.org
Import SBSA Generic watchdog info in GTDT into platform device. That make ARM SBSA watchdog driver can work on the platform which boot with ACPI.
Signed-off-by: Fu Wei fu.wei@linaro.org --- arch/arm64/kernel/acpi.c | 131 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 131 insertions(+)
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c index 8b83955..90422b5 100644 --- a/arch/arm64/kernel/acpi.c +++ b/arch/arm64/kernel/acpi.c @@ -23,6 +23,7 @@ #include <linux/irqdomain.h> #include <linux/memblock.h> #include <linux/of_fdt.h> +#include <linux/platform_device.h> #include <linux/smp.h>
#include <asm/cputype.h> @@ -343,3 +344,133 @@ void __init acpi_gic_init(void)
early_acpi_os_unmap_memory((char *)table, tbl_size); } + +static int __init acpi_gtdt_import_sbsa_gwdt(struct acpi_gtdt_watchdog *wd, + int index) +{ + struct platform_device *pdev; + struct resource *res; + u32 gsi, flags; + int trigger, polarity; + resource_size_t rf_base_phy, cf_base_phy; + int err; + + /* Get SBSA Generic Watchdog info + * from a watchdog type subtable in GTDT + */ + rf_base_phy = (resource_size_t)wd->refresh_frame_address; + cf_base_phy = (resource_size_t)wd->control_frame_address; + gsi = wd->timer_interrupt; + flags = wd->timer_flags; + trigger = (flags & ACPI_GTDT_INTERRUPT_MODE) ? ACPI_EDGE_SENSITIVE + : ACPI_LEVEL_SENSITIVE; + polarity = (flags & ACPI_GTDT_INTERRUPT_POLARITY) ? ACPI_ACTIVE_LOW + : ACPI_ACTIVE_HIGH; + + pr_info("GTDT: found a sbsa-gwdt device @0x%llx/0x%llx gsi:%u flags:0x%x\n", + rf_base_phy, cf_base_phy, gsi, flags); + + if (!(rf_base_phy && cf_base_phy && gsi)) { + pr_err("GTDT: failed geting the device info.\n"); + return -EINVAL; + } + + pdev = platform_device_alloc("sbsa-gwdt", index); + if (!pdev) + return -ENOMEM; + + res = kcalloc(3, sizeof(*res), GFP_KERNEL); + if (!res) { + err = -ENOMEM; + goto err_free_device; + } + + res[0].start = rf_base_phy; + res[0].end = rf_base_phy + SZ_4K - 1; + res[0].name = "refresh"; + res[0].flags = IORESOURCE_MEM; + + res[1].start = cf_base_phy; + res[1].end = cf_base_phy + SZ_4K - 1; + res[1].name = "control"; + res[1].flags = IORESOURCE_MEM; + + res[2].start = acpi_register_gsi(NULL, gsi, trigger, polarity); + res[2].end = res[2].start; + res[2].name = "ws0"; + res[2].flags = IORESOURCE_IRQ; + + err = platform_device_add_resources(pdev, res, 3); + if (err) + goto err_free_res; + + err = platform_device_add(pdev); + if (err) + goto err_free_res; + + return 0; + +err_free_res: + kfree(res); +err_free_device: + platform_device_put(pdev); + + return err; +} + +/* Initialize SBSA generic Watchdog platform device info from GTDT */ +static int __init acpi_gtdt_sbsa_gwdt_init(struct acpi_table_header *table) +{ + struct acpi_table_gtdt *gtdt; + struct acpi_gtdt_header *header; + void *gtdt_subtable; + int i, gwdt_index; + int ret = 0; + + if (table->revision < 2) { + pr_info("GTDT: Revision:%d doesn't support Platform Timers.\n", + table->revision); + return 0; + } + + gtdt = container_of(table, struct acpi_table_gtdt, header); + if (!gtdt->platform_timer_count) { + pr_info("GTDT: No Platform Timer structures.\n"); + return 0; + } + + gtdt_subtable = (void *)gtdt + gtdt->platform_timer_offset; + + for (i = 0, gwdt_index = 0; i < gtdt->platform_timer_count; i++) { + if (gtdt_subtable > (void *)table + table->length) { + pr_err("GTDT: subtable pointer overflows, bad table\n"); + ret = -EINVAL; + break; + } + header = (struct acpi_gtdt_header *)gtdt_subtable; + if (header->type == ACPI_GTDT_TYPE_WATCHDOG) { + ret = acpi_gtdt_import_sbsa_gwdt( + (struct acpi_gtdt_watchdog *)gtdt_subtable, + gwdt_index); + if (ret) + pr_err("GTDT: failed to import subtable %d\n", + i); + else + ++gwdt_index; + } + gtdt_subtable += header->length; + } + + return ret; +} + +/* Initialize the SBSA Generic Watchdog presented in GTDT */ +static int __init acpi_gtdt_init(void) +{ + if (acpi_disabled) + return 0; + + return acpi_table_parse(ACPI_SIG_GTDT, acpi_gtdt_sbsa_gwdt_init); +} + +arch_initcall(acpi_gtdt_init);
On 05/12/2015 05:56 AM, fu.wei@linaro.org wrote:
From: Fu Wei fu.wei@linaro.org
Import SBSA Generic watchdog info in GTDT into platform device. That make ARM SBSA watchdog driver can work on the platform which boot with ACPI.
"Parse the SBSA Generic Watchdog ACPI table, and create a platform device with that information. This platform device is used by the ARM SBSA Generic Watchdog driver."
Signed-off-by: Fu Wei fu.wei@linaro.org
arch/arm64/kernel/acpi.c | 131 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 131 insertions(+)
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c index 8b83955..90422b5 100644 --- a/arch/arm64/kernel/acpi.c +++ b/arch/arm64/kernel/acpi.c @@ -23,6 +23,7 @@ #include <linux/irqdomain.h> #include <linux/memblock.h> #include <linux/of_fdt.h> +#include <linux/platform_device.h> #include <linux/smp.h>
#include <asm/cputype.h> @@ -343,3 +344,133 @@ void __init acpi_gic_init(void)
early_acpi_os_unmap_memory((char *)table, tbl_size); }
+static int __init acpi_gtdt_import_sbsa_gwdt(struct acpi_gtdt_watchdog *wd,
int index)
+{
- struct platform_device *pdev;
- struct resource *res;
- u32 gsi, flags;
- int trigger, polarity;
- resource_size_t rf_base_phy, cf_base_phy;
- int err;
- /* Get SBSA Generic Watchdog info
* from a watchdog type subtable in GTDT
*/
Please reformat.
/* * ..... */
- rf_base_phy = (resource_size_t)wd->refresh_frame_address;
- cf_base_phy = (resource_size_t)wd->control_frame_address;
- gsi = wd->timer_interrupt;
- flags = wd->timer_flags;
- trigger = (flags & ACPI_GTDT_INTERRUPT_MODE) ? ACPI_EDGE_SENSITIVE
: ACPI_LEVEL_SENSITIVE;
- polarity = (flags & ACPI_GTDT_INTERRUPT_POLARITY) ? ACPI_ACTIVE_LOW
: ACPI_ACTIVE_HIGH;
- pr_info("GTDT: found a sbsa-gwdt device @0x%llx/0x%llx gsi:%u flags:0x%x\n",
"an", not "a"
rf_base_phy, cf_base_phy, gsi, flags);
- if (!(rf_base_phy && cf_base_phy && gsi)) {
pr_err("GTDT: failed geting the device info.\n");
return -EINVAL;
- }
- pdev = platform_device_alloc("sbsa-gwdt", index);
- if (!pdev)
return -ENOMEM;
- res = kcalloc(3, sizeof(*res), GFP_KERNEL);
- if (!res) {
err = -ENOMEM;
goto err_free_device;
- }
- res[0].start = rf_base_phy;
- res[0].end = rf_base_phy + SZ_4K - 1;
- res[0].name = "refresh";
- res[0].flags = IORESOURCE_MEM;
- res[1].start = cf_base_phy;
- res[1].end = cf_base_phy + SZ_4K - 1;
- res[1].name = "control";
- res[1].flags = IORESOURCE_MEM;
- res[2].start = acpi_register_gsi(NULL, gsi, trigger, polarity);
What happened to moving the acpi_register_gsi() call into the driver, so that res[2].start contains the physical IRQ?
- res[2].end = res[2].start;
- res[2].name = "ws0";
- res[2].flags = IORESOURCE_IRQ;
- err = platform_device_add_resources(pdev, res, 3);
- if (err)
goto err_free_res;
FYI, platform_device_add_resources() uses kmemdup(), so you don't need to use kcalloc() to allocate the res[] array. Instead, you can do this:
struct resource res[3];
- err = platform_device_add(pdev);
- if (err)
goto err_free_res;
- return 0;
+err_free_res:
- kfree(res);
+err_free_device:
- platform_device_put(pdev);
If you call acpi_register_gsi() in this function, then you need to call acpi_unregister_gsi() on the ex
- return err;
+}
+/* Initialize SBSA generic Watchdog platform device info from GTDT */ +static int __init acpi_gtdt_sbsa_gwdt_init(struct acpi_table_header *table) +{
- struct acpi_table_gtdt *gtdt;
- struct acpi_gtdt_header *header;
- void *gtdt_subtable;
- int i, gwdt_index;
- int ret = 0;
- if (table->revision < 2) {
I wonder if this should be != 2. Do you think that this code would work with version 3 without change?
pr_info("GTDT: Revision:%d doesn't support Platform Timers.\n",
table->revision);
return 0;
- }
- gtdt = container_of(table, struct acpi_table_gtdt, header);
- if (!gtdt->platform_timer_count) {
pr_info("GTDT: No Platform Timer structures.\n");
return 0;
- }
- gtdt_subtable = (void *)gtdt + gtdt->platform_timer_offset;
- for (i = 0, gwdt_index = 0; i < gtdt->platform_timer_count; i++) {
if (gtdt_subtable > (void *)table + table->length) {
pr_err("GTDT: subtable pointer overflows, bad table\n");
ret = -EINVAL;
break;
Just do "return -EINVAL" here.
}
header = (struct acpi_gtdt_header *)gtdt_subtable;
Unnecessary typecast.
if (header->type == ACPI_GTDT_TYPE_WATCHDOG) {
How about
if (header->type != ACPI_GTDT_TYPE_WATCHDOG) continue;
That would make the rest of the block format better.
ret = acpi_gtdt_import_sbsa_gwdt(
(struct acpi_gtdt_watchdog *)gtdt_subtable,
Unnecessary typecast.
gwdt_index);
if (ret)
pr_err("GTDT: failed to import subtable %d\n",
i);
else
++gwdt_index;
gwdt_index++;
}
gtdt_subtable += header->length;
- }
- return ret;
+}
+/* Initialize the SBSA Generic Watchdog presented in GTDT */ +static int __init acpi_gtdt_init(void) +{
- if (acpi_disabled)
return 0;
- return acpi_table_parse(ACPI_SIG_GTDT, acpi_gtdt_sbsa_gwdt_init);
+}
+arch_initcall(acpi_gtdt_init);
Hi Timur,
On 13 May 2015 at 06:44, Timur Tabi timur@codeaurora.org wrote:
On 05/12/2015 05:56 AM, fu.wei@linaro.org wrote:
From: Fu Wei fu.wei@linaro.org
Import SBSA Generic watchdog info in GTDT into platform device. That make ARM SBSA watchdog driver can work on the platform which boot with ACPI.
"Parse the SBSA Generic Watchdog ACPI table, and create a platform device with that information. This platform device is used by the ARM SBSA Generic Watchdog driver."
OK , thanks
Signed-off-by: Fu Wei fu.wei@linaro.org
arch/arm64/kernel/acpi.c | 131 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 131 insertions(+)
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c index 8b83955..90422b5 100644 --- a/arch/arm64/kernel/acpi.c +++ b/arch/arm64/kernel/acpi.c @@ -23,6 +23,7 @@ #include <linux/irqdomain.h> #include <linux/memblock.h> #include <linux/of_fdt.h> +#include <linux/platform_device.h> #include <linux/smp.h>
#include <asm/cputype.h> @@ -343,3 +344,133 @@ void __init acpi_gic_init(void)
early_acpi_os_unmap_memory((char *)table, tbl_size);
}
+static int __init acpi_gtdt_import_sbsa_gwdt(struct acpi_gtdt_watchdog *wd,
int index)
+{
struct platform_device *pdev;
struct resource *res;
u32 gsi, flags;
int trigger, polarity;
resource_size_t rf_base_phy, cf_base_phy;
int err;
/* Get SBSA Generic Watchdog info
* from a watchdog type subtable in GTDT
*/
Please reformat.
/* * ..... */
Fixed , thanks
rf_base_phy = (resource_size_t)wd->refresh_frame_address;
cf_base_phy = (resource_size_t)wd->control_frame_address;
gsi = wd->timer_interrupt;
flags = wd->timer_flags;
trigger = (flags & ACPI_GTDT_INTERRUPT_MODE) ? ACPI_EDGE_SENSITIVE
: ACPI_LEVEL_SENSITIVE;
polarity = (flags & ACPI_GTDT_INTERRUPT_POLARITY) ?
ACPI_ACTIVE_LOW
: ACPI_ACTIVE_HIGH;
pr_info("GTDT: found a sbsa-gwdt device @0x%llx/0x%llx gsi:%u
flags:0x%x\n",
"an", not "a"
thanks
rf_base_phy, cf_base_phy, gsi, flags);
if (!(rf_base_phy && cf_base_phy && gsi)) {
pr_err("GTDT: failed geting the device info.\n");
return -EINVAL;
}
pdev = platform_device_alloc("sbsa-gwdt", index);
if (!pdev)
return -ENOMEM;
res = kcalloc(3, sizeof(*res), GFP_KERNEL);
if (!res) {
err = -ENOMEM;
goto err_free_device;
}
res[0].start = rf_base_phy;
res[0].end = rf_base_phy + SZ_4K - 1;
res[0].name = "refresh";
res[0].flags = IORESOURCE_MEM;
res[1].start = cf_base_phy;
res[1].end = cf_base_phy + SZ_4K - 1;
res[1].name = "control";
res[1].flags = IORESOURCE_MEM;
res[2].start = acpi_register_gsi(NULL, gsi, trigger, polarity);
What happened to moving the acpi_register_gsi() call into the driver, so that res[2].start contains the physical IRQ?
it just do the mapping, config gic with flags, return irq number. and in FDT (OF) driver, they do the same thing , so it makes sense to put the acpi_register_gsi() here.
that make driver get the same kind of value. is that right? let me know if i misunderstand some thing.
res[2].end = res[2].start;
res[2].name = "ws0";
res[2].flags = IORESOURCE_IRQ;
err = platform_device_add_resources(pdev, res, 3);
if (err)
goto err_free_res;
FYI, platform_device_add_resources() uses kmemdup(), so you don't need to use kcalloc() to allocate the res[] array. Instead, you can do this:
struct resource res[3];
I have tried , but fail, not sure why , but once I use kcalloc() again, this func works.
err = platform_device_add(pdev);
if (err)
goto err_free_res;
return 0;
+err_free_res:
kfree(res);
+err_free_device:
platform_device_put(pdev);
If you call acpi_register_gsi() in this function, then you need to call acpi_unregister_gsi() on the ex
yes, you are right, fixed
return err;
+}
+/* Initialize SBSA generic Watchdog platform device info from GTDT */ +static int __init acpi_gtdt_sbsa_gwdt_init(struct acpi_table_header *table) +{
struct acpi_table_gtdt *gtdt;
struct acpi_gtdt_header *header;
void *gtdt_subtable;
int i, gwdt_index;
int ret = 0;
if (table->revision < 2) {
I wonder if this should be != 2. Do you think that this code would work with version 3 without change?
that may work or may not. but we can fix this if we get revision 3 or higher.
pr_info("GTDT: Revision:%d doesn't support Platform
Timers.\n",
table->revision);
return 0;
}
gtdt = container_of(table, struct acpi_table_gtdt, header);
if (!gtdt->platform_timer_count) {
pr_info("GTDT: No Platform Timer structures.\n");
return 0;
}
gtdt_subtable = (void *)gtdt + gtdt->platform_timer_offset;
for (i = 0, gwdt_index = 0; i < gtdt->platform_timer_count; i++) {
if (gtdt_subtable > (void *)table + table->length) {
pr_err("GTDT: subtable pointer overflows, bad
table\n");
ret = -EINVAL;
break;
Just do "return -EINVAL" here.
yes, fixed
}
header = (struct acpi_gtdt_header *)gtdt_subtable;
Unnecessary typecast.
no, we need it
if (header->type == ACPI_GTDT_TYPE_WATCHDOG) {
How about
if (header->type != ACPI_GTDT_TYPE_WATCHDOG) continue;
That would make the rest of the block format better.
no, we may reuse this func soon.
ret = acpi_gtdt_import_sbsa_gwdt(
(struct acpi_gtdt_watchdog
*)gtdt_subtable,
Unnecessary typecast.
yes, thanks
gwdt_index);
if (ret)
pr_err("GTDT: failed to import subtable
%d\n",
i);
else
++gwdt_index;
gwdt_index++;
same effect, but OK, we can use that .
}
gtdt_subtable += header->length;
}
return ret;
+}
+/* Initialize the SBSA Generic Watchdog presented in GTDT */ +static int __init acpi_gtdt_init(void) +{
if (acpi_disabled)
return 0;
return acpi_table_parse(ACPI_SIG_GTDT, acpi_gtdt_sbsa_gwdt_init);
+}
+arch_initcall(acpi_gtdt_init);
-- Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Hi Timur,
On 14 May 2015 at 16:27, Fu Wei fu.wei@linaro.org wrote:
Hi Timur,
On 13 May 2015 at 06:44, Timur Tabi timur@codeaurora.org wrote:
On 05/12/2015 05:56 AM, fu.wei@linaro.org wrote:
From: Fu Wei fu.wei@linaro.org
Import SBSA Generic watchdog info in GTDT into platform device. That make ARM SBSA watchdog driver can work on the platform which boot with ACPI.
"Parse the SBSA Generic Watchdog ACPI table, and create a platform device with that information. This platform device is used by the ARM SBSA Generic Watchdog driver."
OK , thanks
Signed-off-by: Fu Wei fu.wei@linaro.org
arch/arm64/kernel/acpi.c | 131 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 131 insertions(+)
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c index 8b83955..90422b5 100644 --- a/arch/arm64/kernel/acpi.c +++ b/arch/arm64/kernel/acpi.c @@ -23,6 +23,7 @@ #include <linux/irqdomain.h> #include <linux/memblock.h> #include <linux/of_fdt.h> +#include <linux/platform_device.h> #include <linux/smp.h>
#include <asm/cputype.h> @@ -343,3 +344,133 @@ void __init acpi_gic_init(void)
early_acpi_os_unmap_memory((char *)table, tbl_size);
}
+static int __init acpi_gtdt_import_sbsa_gwdt(struct acpi_gtdt_watchdog *wd,
int index)
+{
struct platform_device *pdev;
struct resource *res;
u32 gsi, flags;
int trigger, polarity;
resource_size_t rf_base_phy, cf_base_phy;
int err;
/* Get SBSA Generic Watchdog info
* from a watchdog type subtable in GTDT
*/
Please reformat.
/* * ..... */
Fixed , thanks
rf_base_phy = (resource_size_t)wd->refresh_frame_address;
cf_base_phy = (resource_size_t)wd->control_frame_address;
gsi = wd->timer_interrupt;
flags = wd->timer_flags;
trigger = (flags & ACPI_GTDT_INTERRUPT_MODE) ? ACPI_EDGE_SENSITIVE
: ACPI_LEVEL_SENSITIVE;
polarity = (flags & ACPI_GTDT_INTERRUPT_POLARITY) ?
ACPI_ACTIVE_LOW
: ACPI_ACTIVE_HIGH;
pr_info("GTDT: found a sbsa-gwdt device @0x%llx/0x%llx gsi:%u
flags:0x%x\n",
"an", not "a"
thanks
rf_base_phy, cf_base_phy, gsi, flags);
if (!(rf_base_phy && cf_base_phy && gsi)) {
pr_err("GTDT: failed geting the device info.\n");
return -EINVAL;
}
pdev = platform_device_alloc("sbsa-gwdt", index);
if (!pdev)
return -ENOMEM;
res = kcalloc(3, sizeof(*res), GFP_KERNEL);
if (!res) {
err = -ENOMEM;
goto err_free_device;
}
res[0].start = rf_base_phy;
res[0].end = rf_base_phy + SZ_4K - 1;
res[0].name = "refresh";
res[0].flags = IORESOURCE_MEM;
res[1].start = cf_base_phy;
res[1].end = cf_base_phy + SZ_4K - 1;
res[1].name = "control";
res[1].flags = IORESOURCE_MEM;
res[2].start = acpi_register_gsi(NULL, gsi, trigger, polarity);
What happened to moving the acpi_register_gsi() call into the driver, so that res[2].start contains the physical IRQ?
it just do the mapping, config gic with flags, return irq number. and in FDT (OF) driver, they do the same thing , so it makes sense to put the acpi_register_gsi() here.
that make driver get the same kind of value. is that right? let me know if i misunderstand some thing.
res[2].end = res[2].start;
res[2].name = "ws0";
res[2].flags = IORESOURCE_IRQ;
err = platform_device_add_resources(pdev, res, 3);
if (err)
goto err_free_res;
FYI, platform_device_add_resources() uses kmemdup(), so you don't need to use kcalloc() to allocate the res[] array. Instead, you can do this:
struct resource res[3];
I have tried , but fail, not sure why , but once I use kcalloc() again, this func works.
err = platform_device_add(pdev);
if (err)
goto err_free_res;
return 0;
+err_free_res:
kfree(res);
+err_free_device:
platform_device_put(pdev);
If you call acpi_register_gsi() in this function, then you need to call acpi_unregister_gsi() on the ex
yes, you are right, fixed
return err;
+}
+/* Initialize SBSA generic Watchdog platform device info from GTDT */ +static int __init acpi_gtdt_sbsa_gwdt_init(struct acpi_table_header *table) +{
struct acpi_table_gtdt *gtdt;
struct acpi_gtdt_header *header;
void *gtdt_subtable;
int i, gwdt_index;
int ret = 0;
if (table->revision < 2) {
I wonder if this should be != 2. Do you think that this code would work with version 3 without change?
that may work or may not. but we can fix this if we get revision 3 or higher.
pr_info("GTDT: Revision:%d doesn't support Platform
Timers.\n",
table->revision);
return 0;
}
gtdt = container_of(table, struct acpi_table_gtdt, header);
if (!gtdt->platform_timer_count) {
pr_info("GTDT: No Platform Timer structures.\n");
return 0;
}
gtdt_subtable = (void *)gtdt + gtdt->platform_timer_offset;
for (i = 0, gwdt_index = 0; i < gtdt->platform_timer_count; i++) {
if (gtdt_subtable > (void *)table + table->length) {
pr_err("GTDT: subtable pointer overflows, bad
table\n");
ret = -EINVAL;
break;
Just do "return -EINVAL" here.
yes, fixed
}
header = (struct acpi_gtdt_header *)gtdt_subtable;
Unnecessary typecast.
no, we need it
sorry, misunderstood this, we don't need it , Fixed it
if (header->type == ACPI_GTDT_TYPE_WATCHDOG) {
How about
if (header->type != ACPI_GTDT_TYPE_WATCHDOG) continue;
That would make the rest of the block format better.
no, we may reuse this func soon.
ret = acpi_gtdt_import_sbsa_gwdt(
(struct acpi_gtdt_watchdog
*)gtdt_subtable,
Unnecessary typecast.
yes, thanks
gwdt_index);
if (ret)
pr_err("GTDT: failed to import subtable
%d\n",
i);
else
++gwdt_index;
gwdt_index++;
same effect, but OK, we can use that .
}
gtdt_subtable += header->length;
}
return ret;
+}
+/* Initialize the SBSA Generic Watchdog presented in GTDT */ +static int __init acpi_gtdt_init(void) +{
if (acpi_disabled)
return 0;
return acpi_table_parse(ACPI_SIG_GTDT, acpi_gtdt_sbsa_gwdt_init);
+}
+arch_initcall(acpi_gtdt_init);
-- Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
-- 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
On 2015年05月12日 18:56, fu.wei@linaro.org wrote:
From: Fu Wei fu.wei@linaro.org
Import SBSA Generic watchdog info in GTDT into platform device. That make ARM SBSA watchdog driver can work on the platform which boot with ACPI.
Signed-off-by: Fu Wei fu.wei@linaro.org
arch/arm64/kernel/acpi.c | 131 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 131 insertions(+)
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c index 8b83955..90422b5 100644 --- a/arch/arm64/kernel/acpi.c +++ b/arch/arm64/kernel/acpi.c @@ -23,6 +23,7 @@ #include <linux/irqdomain.h> #include <linux/memblock.h> #include <linux/of_fdt.h> +#include <linux/platform_device.h> #include <linux/smp.h>
#include <asm/cputype.h> @@ -343,3 +344,133 @@ void __init acpi_gic_init(void)
early_acpi_os_unmap_memory((char *)table, tbl_size); }
+static int __init acpi_gtdt_import_sbsa_gwdt(struct acpi_gtdt_watchdog *wd,
int index)
+{
- struct platform_device *pdev;
- struct resource *res;
- u32 gsi, flags;
- int trigger, polarity;
- resource_size_t rf_base_phy, cf_base_phy;
- int err;
- /* Get SBSA Generic Watchdog info
* from a watchdog type subtable in GTDT
*/
- rf_base_phy = (resource_size_t)wd->refresh_frame_address;
- cf_base_phy = (resource_size_t)wd->control_frame_address;
- gsi = wd->timer_interrupt;
- flags = wd->timer_flags;
- trigger = (flags & ACPI_GTDT_INTERRUPT_MODE) ? ACPI_EDGE_SENSITIVE
: ACPI_LEVEL_SENSITIVE;
- polarity = (flags & ACPI_GTDT_INTERRUPT_POLARITY) ? ACPI_ACTIVE_LOW
: ACPI_ACTIVE_HIGH;
I think we need to reuse the map_generic_timer_interrupt() in arm_arch_timer.c, but we can do that on top of this patch set.
- pr_info("GTDT: found a sbsa-gwdt device @0x%llx/0x%llx gsi:%u flags:0x%x\n",
rf_base_phy, cf_base_phy, gsi, flags);
...
- if (!(rf_base_phy && cf_base_phy && gsi)) {
pr_err("GTDT: failed geting the device info.\n");
return -EINVAL;
- }
Should the pr_info() put it here?
- pdev = platform_device_alloc("sbsa-gwdt", index);
- if (!pdev)
return -ENOMEM;
- res = kcalloc(3, sizeof(*res), GFP_KERNEL);
- if (!res) {
err = -ENOMEM;
goto err_free_device;
- }
- res[0].start = rf_base_phy;
- res[0].end = rf_base_phy + SZ_4K - 1;
- res[0].name = "refresh";
- res[0].flags = IORESOURCE_MEM;
- res[1].start = cf_base_phy;
- res[1].end = cf_base_phy + SZ_4K - 1;
- res[1].name = "control";
- res[1].flags = IORESOURCE_MEM;
- res[2].start = acpi_register_gsi(NULL, gsi, trigger, polarity);
- res[2].end = res[2].start;
- res[2].name = "ws0";
- res[2].flags = IORESOURCE_IRQ;
- err = platform_device_add_resources(pdev, res, 3);
- if (err)
goto err_free_res;
- err = platform_device_add(pdev);
- if (err)
goto err_free_res;
- return 0;
+err_free_res:
- kfree(res);
+err_free_device:
- platform_device_put(pdev);
- return err;
+}
+/* Initialize SBSA generic Watchdog platform device info from GTDT */ +static int __init acpi_gtdt_sbsa_gwdt_init(struct acpi_table_header *table) +{
- struct acpi_table_gtdt *gtdt;
- struct acpi_gtdt_header *header;
- void *gtdt_subtable;
- int i, gwdt_index;
- int ret = 0;
- if (table->revision < 2) {
pr_info("GTDT: Revision:%d doesn't support Platform Timers.\n",
table->revision);
return 0;
- }
No, please don't count on the table revision, lots of BIOS vendors just ignore that field, so we should count on real content in this table, which means we should scan for the table to find if there are information we needed.
- gtdt = container_of(table, struct acpi_table_gtdt, header);
- if (!gtdt->platform_timer_count) {
pr_info("GTDT: No Platform Timer structures.\n");
return 0;
- }
- gtdt_subtable = (void *)gtdt + gtdt->platform_timer_offset;
I like this part, it removes the introduction of new table handler for GTDT.
- for (i = 0, gwdt_index = 0; i < gtdt->platform_timer_count; i++) {
if (gtdt_subtable > (void *)table + table->length) {
pr_err("GTDT: subtable pointer overflows, bad table\n");
ret = -EINVAL;
break;
}
header = (struct acpi_gtdt_header *)gtdt_subtable;
if (header->type == ACPI_GTDT_TYPE_WATCHDOG) {
ret = acpi_gtdt_import_sbsa_gwdt(
(struct acpi_gtdt_watchdog *)gtdt_subtable,
gwdt_index);
Can we refactor this a little bit to make this function can be reused for memory-mapped timer init?
Thanks Hanjun
Hi Hanjun,
Thanks for discussing this with me about this , I have fixed some problem base on our discuss.
please check v4 patch :-)
On 13 May 2015 at 12:55, Hanjun Guo hanjun.guo@linaro.org wrote:
On 2015年05月12日 18:56, fu.wei@linaro.org wrote:
From: Fu Wei fu.wei@linaro.org
Import SBSA Generic watchdog info in GTDT into platform device. That make ARM SBSA watchdog driver can work on the platform which boot with ACPI.
Signed-off-by: Fu Wei fu.wei@linaro.org
arch/arm64/kernel/acpi.c | 131 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 131 insertions(+)
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c index 8b83955..90422b5 100644 --- a/arch/arm64/kernel/acpi.c +++ b/arch/arm64/kernel/acpi.c @@ -23,6 +23,7 @@ #include <linux/irqdomain.h> #include <linux/memblock.h> #include <linux/of_fdt.h> +#include <linux/platform_device.h> #include <linux/smp.h>
#include <asm/cputype.h> @@ -343,3 +344,133 @@ void __init acpi_gic_init(void)
early_acpi_os_unmap_memory((char *)table, tbl_size);
}
+static int __init acpi_gtdt_import_sbsa_gwdt(struct acpi_gtdt_watchdog *wd,
int index)
+{
struct platform_device *pdev;
struct resource *res;
u32 gsi, flags;
int trigger, polarity;
resource_size_t rf_base_phy, cf_base_phy;
int err;
/* Get SBSA Generic Watchdog info
* from a watchdog type subtable in GTDT
*/
rf_base_phy = (resource_size_t)wd->refresh_frame_address;
cf_base_phy = (resource_size_t)wd->control_frame_address;
gsi = wd->timer_interrupt;
flags = wd->timer_flags;
trigger = (flags & ACPI_GTDT_INTERRUPT_MODE) ? ACPI_EDGE_SENSITIVE
: ACPI_LEVEL_SENSITIVE;
polarity = (flags & ACPI_GTDT_INTERRUPT_POLARITY) ?
ACPI_ACTIVE_LOW
: ACPI_ACTIVE_HIGH;
I think we need to reuse the map_generic_timer_interrupt() in arm_arch_timer.c, but we can do that on top of this patch set.
pr_info("GTDT: found a sbsa-gwdt device @0x%llx/0x%llx gsi:%u
flags:0x%x\n",
rf_base_phy, cf_base_phy, gsi, flags);
...
if (!(rf_base_phy && cf_base_phy && gsi)) {
pr_err("GTDT: failed geting the device info.\n");
return -EINVAL;
}
Should the pr_info() put it here?
pdev = platform_device_alloc("sbsa-gwdt", index);
if (!pdev)
return -ENOMEM;
res = kcalloc(3, sizeof(*res), GFP_KERNEL);
if (!res) {
err = -ENOMEM;
goto err_free_device;
}
res[0].start = rf_base_phy;
res[0].end = rf_base_phy + SZ_4K - 1;
res[0].name = "refresh";
res[0].flags = IORESOURCE_MEM;
res[1].start = cf_base_phy;
res[1].end = cf_base_phy + SZ_4K - 1;
res[1].name = "control";
res[1].flags = IORESOURCE_MEM;
res[2].start = acpi_register_gsi(NULL, gsi, trigger, polarity);
res[2].end = res[2].start;
res[2].name = "ws0";
res[2].flags = IORESOURCE_IRQ;
err = platform_device_add_resources(pdev, res, 3);
if (err)
goto err_free_res;
err = platform_device_add(pdev);
if (err)
goto err_free_res;
return 0;
+err_free_res:
kfree(res);
+err_free_device:
platform_device_put(pdev);
return err;
+}
+/* Initialize SBSA generic Watchdog platform device info from GTDT */ +static int __init acpi_gtdt_sbsa_gwdt_init(struct acpi_table_header *table) +{
struct acpi_table_gtdt *gtdt;
struct acpi_gtdt_header *header;
void *gtdt_subtable;
int i, gwdt_index;
int ret = 0;
if (table->revision < 2) {
pr_info("GTDT: Revision:%d doesn't support Platform
Timers.\n",
table->revision);
return 0;
}
No, please don't count on the table revision, lots of BIOS vendors just ignore that field, so we should count on real content in this table, which means we should scan for the table to find if there are information we needed.
gtdt = container_of(table, struct acpi_table_gtdt, header);
if (!gtdt->platform_timer_count) {
pr_info("GTDT: No Platform Timer structures.\n");
return 0;
}
gtdt_subtable = (void *)gtdt + gtdt->platform_timer_offset;
I like this part, it removes the introduction of new table handler for GTDT.
for (i = 0, gwdt_index = 0; i < gtdt->platform_timer_count; i++) {
if (gtdt_subtable > (void *)table + table->length) {
pr_err("GTDT: subtable pointer overflows, bad
table\n");
ret = -EINVAL;
break;
}
header = (struct acpi_gtdt_header *)gtdt_subtable;
if (header->type == ACPI_GTDT_TYPE_WATCHDOG) {
ret = acpi_gtdt_import_sbsa_gwdt(
(struct acpi_gtdt_watchdog
*)gtdt_subtable,
gwdt_index);
Can we refactor this a little bit to make this function can be reused for memory-mapped timer init?
Thanks Hanjun