From: Fu Wei <[email protected]>
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 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.
(3)Introduce ARM SBSA watchdog driver:
a.Use linux kernel watchdog framework;
b.Work with FDT on ARM64;
c.In first timeout, do panic to save system context;
d.Support getting timeout from parameter and FDT at the driver init stage.
This patchset has been tested with watchdog daemon
(ACPI/FDT, module/build-in) on the following platforms:
(1)ARM Foundation v8 model
Changelog:
non_pretimeout: Delete all pretimeout code
The time from enabling watchdog to WS0 is the same with
the time from WS0 to WS1, they are both timeout value.
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 (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: 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 | 35 ++
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 | 12 +
drivers/watchdog/Makefile | 1 +
drivers/watchdog/sbsa_gwdt.c | 383 +++++++++++++++++++++
include/clocksource/arm_arch_timer.h | 8 +
include/linux/acpi.h | 5 +
include/linux/clocksource.h | 4 +-
15 files changed, 671 insertions(+), 53 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
From: Fu Wei <[email protected]>
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 <[email protected]>
Signed-off-by: Fu Wei <[email protected]>
---
.../devicetree/bindings/watchdog/sbsa-gwdt.txt | 35 ++++++++++++++++++++++
1 file changed, 35 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..164ee98
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt
@@ -0,0 +1,35 @@
+* 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 timeout value (in seconds).
+
+Example for FVP Foundation Model v8:
+
+watchdog@2a440000 {
+ compatible = "arm,sbsa-gwdt";
+ reg = <0x0 0x2a440000 0 0x10000>,
+ <0x0 0x2a450000 0 0x10000>;
+ reg-names = "control", "refresh";
+ interrupts = <0 27 4>;
+ interrupt-names = "ws0";
+ timeout-sec = <10>;
+};
--
1.9.1
From: Fu Wei <[email protected]>
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 <[email protected]>
Signed-off-by: Fu Wei <[email protected]>
---
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..dfc799e 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 0x10000>,
+ <0x0 0x2a450000 0 0x10000>;
+ reg-names = "control",
+ "refresh";
+ interrupts = <0 27 4>;
+ interrupt-names = "ws0";
+ timeout-sec = <10>;
+ };
};
--
1.9.1
From: Fu Wei <[email protected]>
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 <[email protected]>
Acked-by: Suravee Suthikulpanit <[email protected]>
Tested-by: Suravee Suthikulpanit <[email protected]>
Signed-off-by: Fu Wei <[email protected]>
---
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..3fc2746 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>;
+ };
+
spi0: ssp@e1020000 {
status = "disabled";
compatible = "arm,pl022", "arm,primecell";
--
1.9.1
From: Fu Wei <[email protected]>
This driver bases on linux kernel watchdog framework.
It supports getting timeout from parameter and FDT
at the driver init stage.
The first timeout period expires, the interrupt routine
got another timeout period to run panic for saving
system context.
Signed-off-by: Fu Wei <[email protected]>
---
drivers/watchdog/Kconfig | 11 ++
drivers/watchdog/Makefile | 1 +
drivers/watchdog/sbsa_gwdt.c | 383 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 395 insertions(+)
create mode 100644 drivers/watchdog/sbsa_gwdt.c
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index e5e7c55..554f18a 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -152,6 +152,17 @@ 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
+ depends on ARM_ARCH_TIMER
+ select WATCHDOG_CORE
+ help
+ ARM SBSA Generic Watchdog. This watchdog has two Watchdog timeouts.
+ The first timeout will trigger a panic; the second timeout will
+ trigger a system reset.
+ More details: ARM 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..1ddc10f
--- /dev/null
+++ b/drivers/watchdog/sbsa_gwdt.c
@@ -0,0 +1,383 @@
+/*
+ * SBSA(Server Base System Architecture) Generic Watchdog driver
+ *
+ * Copyright (c) 2015, Linaro Ltd.
+ * Author: Fu Wei <[email protected]>
+ * Suravee Suthikulpanit <[email protected]>
+ *
+ * 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 has two stage timeouts,
+ * When the first timeout occurs, WS0(SPI or LPI) is triggered,
+ * the second timeout period(as long as the first timeout period) starts.
+ * In WS0 interrupt routine, panic() will be called for collecting
+ * crashdown info.
+ * If system can not recover from WS0 interrupt routine, then second
+ * timeout occurs, WS1(reset or higher level interrupt) is triggered.
+ * The two timeout period can be set by WOR(32bit).
+ * WOR gives a maximum watch period of around 10s at the maximum
+ * system counter frequency.
+ * The System Counter shall run at maximum of 400MHz.
+ *
+ * But If we need a larger timeout period, this driver will programme WCV
+ * directly. That can support more than 10s timeout at the maximum
+ * system counter frequency.
+ * More details: ARM DEN0029B - Server Base System Architecture (SBSA)
+ *
+ * SBSA GWDT: |---WOR(or WCV)---WS0---WOR(or WCV)---WS1
+ * |-----timeout-----WS0-----timeout-----WS1
+ */
+
+#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/uaccess.h>
+#include <linux/watchdog.h>
+#include <asm/arch_timer.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)
+
+/**
+ * struct sbsa_gwdt - Internal representation of the SBSA GWDT
+ * @wdd: kernel watchdog_device structure
+ * @clk: store the System Counter clock frequency, in Hz.
+ * @max_wor_timeout: the maximum timeout value for WOR (in seconds).
+ * @refresh_base: Virtual address of the watchdog refresh frame
+ * @control_base: Virtual address of the watchdog control frame
+ */
+struct sbsa_gwdt {
+ struct watchdog_device wdd;
+ u32 clk;
+ int max_wor_timeout;
+ void __iomem *refresh_base;
+ void __iomem *control_base;
+};
+
+#define to_sbsa_gwdt(e) container_of(e, struct sbsa_gwdt, wdd)
+
+#define DEFAULT_TIMEOUT 30 /* seconds */
+
+static unsigned int timeout;
+module_param(timeout, uint, 0);
+MODULE_PARM_DESC(timeout,
+ "Watchdog timeout in seconds. (>=0, default="
+ __MODULE_STRING(DEFAULT_TIMEOUT) ")");
+
+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) ")");
+
+/*
+ * help functions for accessing 64bit WCV register
+ */
+static u64 sbsa_gwdt_get_wcv(struct watchdog_device *wdd)
+{
+ u32 wcv_lo, wcv_hi;
+ struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
+
+ do {
+ wcv_hi = readl_relaxed(gwdt->control_base + SBSA_GWDT_WCV_HI);
+ wcv_lo = readl_relaxed(gwdt->control_base + SBSA_GWDT_WCV_LO);
+ } while (wcv_hi != readl_relaxed(gwdt->control_base +
+ SBSA_GWDT_WCV_HI));
+
+ return (((u64)wcv_hi << 32) | wcv_lo);
+}
+
+static void reload_timeout_to_wcv(struct watchdog_device *wdd)
+{
+ struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
+ u64 wcv;
+
+ wcv = arch_counter_get_cntvct() + (u64)wdd->timeout * gwdt->clk;
+
+ writel_relaxed(upper_32_bits(wcv),
+ gwdt->control_base + SBSA_GWDT_WCV_HI);
+ writel_relaxed(lower_32_bits(wcv),
+ gwdt->control_base + SBSA_GWDT_WCV_LO);
+}
+
+static int sbsa_gwdt_set_timeout(struct watchdog_device *wdd,
+ unsigned int timeout)
+{
+ struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
+
+ wdd->timeout = timeout;
+
+ if (timeout <= gwdt->max_wor_timeout)
+ writel_relaxed(timeout * gwdt->clk,
+ gwdt->control_base + SBSA_GWDT_WOR);
+ else
+ writel_relaxed(gwdt->max_wor_timeout * gwdt->clk,
+ gwdt->control_base + SBSA_GWDT_WOR);
+
+ 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();
+
+ do_div(timeleft, gwdt->clk);
+
+ return timeleft;
+}
+
+static int sbsa_gwdt_keepalive(struct watchdog_device *wdd)
+{
+ struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
+
+ if (wdd->timeout <= gwdt->max_wor_timeout)
+ /*
+ * Writing WRR for an explicit watchdog refresh.
+ * You can write anyting(like 0xc0ffee).
+ */
+ writel_relaxed(0xc0ffee, gwdt->refresh_base + SBSA_GWDT_WRR);
+ else
+ reload_timeout_to_wcv(wdd);
+
+ return 0;
+}
+
+static int sbsa_gwdt_start(struct watchdog_device *wdd)
+{
+ struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
+ /* Force refresh due to hardware bug found in certain Soc. */
+ writel_relaxed(0xc0ffee, gwdt->refresh_base + SBSA_GWDT_WRR);
+ /* writing WCS will cause an explicit watchdog refresh */
+ writel_relaxed(SBSA_GWDT_WCS_EN, gwdt->control_base + SBSA_GWDT_WCS);
+
+ return sbsa_gwdt_keepalive(wdd);
+}
+
+static int sbsa_gwdt_stop(struct watchdog_device *wdd)
+{
+ struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
+
+ writel_relaxed(0, gwdt->control_base + SBSA_GWDT_WCS);
+
+ return 0;
+}
+
+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;
+
+ if (wdd->timeout > gwdt->max_wor_timeout)
+ reload_timeout_to_wcv(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_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,
+ .get_timeleft = sbsa_gwdt_get_timeleft,
+};
+
+static int sbsa_gwdt_probe(struct platform_device *pdev)
+{
+ u64 first_period_max = U64_MAX;
+ struct device *dev = &pdev->dev;
+ struct watchdog_device *wdd;
+ struct sbsa_gwdt *gwdt;
+ struct resource *res;
+ void *rf_base, *cf_base;
+ int ret, irq;
+ u32 status;
+
+ gwdt = devm_kzalloc(dev, sizeof(*gwdt), GFP_KERNEL);
+ if (!gwdt)
+ return -ENOMEM;
+ platform_set_drvdata(pdev, gwdt);
+
+ 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);
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "control");
+ cf_base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(cf_base))
+ return PTR_ERR(cf_base);
+
+ irq = platform_get_irq_byname(pdev, "ws0");
+ if (irq < 0) {
+ dev_err(dev, "unable to get ws0 interrupt.\n");
+ return irq;
+ }
+
+ /*
+ * Get the frequency of system counter from the cp15 interface of ARM
+ * Generic timer. We don't need to check it, because if it returns "0",
+ * system would panic in very early stage.
+ */
+ gwdt->clk = arch_timer_get_cntfrq();
+ gwdt->refresh_base = rf_base;
+ gwdt->control_base = cf_base;
+ gwdt->max_wor_timeout = U32_MAX / gwdt->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);
+
+ wdd->min_timeout = 1;
+ do_div(first_period_max, gwdt->clk);
+ wdd->max_timeout = first_period_max;
+
+ wdd->timeout = DEFAULT_TIMEOUT;
+ watchdog_init_timeout(wdd, timeout, dev);
+
+ 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));
+ 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);
+ }
+
+ /* update timeout to WOR */
+ sbsa_gwdt_set_timeout(wdd, wdd->timeout);
+
+ ret = devm_request_irq(dev, irq, sbsa_gwdt_interrupt, 0,
+ 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;
+
+ dev_info(dev, "Initialized with %ds timeout @ %u Hz\n", wdd->timeout,
+ 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);
+
+ watchdog_unregister_device(&gwdt->wdd);
+
+ return 0;
+}
+
+/* Disable watchdog if it is active during suspend */
+static int __maybe_unused sbsa_gwdt_suspend(struct device *dev)
+{
+ struct sbsa_gwdt *gwdt = dev_get_drvdata(dev);
+
+ if (watchdog_active(&gwdt->wdd))
+ sbsa_gwdt_stop(&gwdt->wdd);
+
+ return 0;
+}
+
+/* Enable watchdog and configure it if necessary */
+static int __maybe_unused sbsa_gwdt_resume(struct device *dev)
+{
+ struct sbsa_gwdt *gwdt = dev_get_drvdata(dev);
+
+ if (watchdog_active(&gwdt->wdd))
+ sbsa_gwdt_start(&gwdt->wdd);
+
+ return 0;
+}
+
+static const struct dev_pm_ops sbsa_gwdt_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(sbsa_gwdt_suspend, sbsa_gwdt_resume)
+};
+
+static const struct of_device_id sbsa_gwdt_of_match[] = {
+ { .compatible = "arm,sbsa-gwdt", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, sbsa_gwdt_of_match);
+
+static const struct platform_device_id sbsa_gwdt_pdev_match[] = {
+ { .name = "sbsa-gwdt", },
+ {},
+};
+MODULE_DEVICE_TABLE(platform, sbsa_gwdt_pdev_match);
+
+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,
+ .id_table = sbsa_gwdt_pdev_match,
+};
+
+module_platform_driver(sbsa_gwdt_driver);
+
+MODULE_DESCRIPTION("SBSA Generic Watchdog Driver");
+MODULE_VERSION("v1.0");
+MODULE_AUTHOR("Fu Wei <[email protected]>");
+MODULE_AUTHOR("Suravee Suthikulpanit <[email protected]>");
+MODULE_LICENSE("GPL v2");
--
1.9.1
From: Fu Wei <[email protected]>
This driver adds support for parsing SBSA Generic Watchdog
Structure in GTDT, and creating a platform device with that
information. This allows the operating system to obtain device
data from the resource of platform device.
The platform device named "sbsa-gwdt" can be used by the
ARM SBSA Generic Watchdog driver.
Signed-off-by: Fu Wei <[email protected]>
Signed-off-by: Hanjun Guo <[email protected]>
---
drivers/acpi/Kconfig | 9 ++++
drivers/acpi/Makefile | 1 +
drivers/acpi/gtdt.c | 137 ++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 147 insertions(+)
create mode 100644 drivers/acpi/gtdt.c
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 1bbaa3d..e125698 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -433,4 +433,13 @@ config XPOWER_PMIC_OPREGION
endif
+config ACPI_GTDT
+ bool "ACPI GTDT Support"
+ depends on ARM64
+ help
+ GTDT (Generic Timer Description Table) provides information
+ for per-processor timers and Platform (memory-mapped) timers
+ for ARM platforms. Select this option to provide information
+ needed for the timers init.
+
endif # ACPI
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 431e587..2c5a194 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -96,3 +96,4 @@ obj-$(CONFIG_ACPI_EXTLOG) += acpi_extlog.o
obj-$(CONFIG_PMIC_OPREGION) += pmic/intel_pmic.o
obj-$(CONFIG_CRC_PMIC_OPREGION) += pmic/intel_pmic_crc.o
obj-$(CONFIG_XPOWER_PMIC_OPREGION) += pmic/intel_pmic_xpower.o
+obj-$(CONFIG_ACPI_GTDT) += gtdt.o
diff --git a/drivers/acpi/gtdt.c b/drivers/acpi/gtdt.c
new file mode 100644
index 0000000..a92abf2
--- /dev/null
+++ b/drivers/acpi/gtdt.c
@@ -0,0 +1,137 @@
+/*
+ * ARM Specific GTDT table Support
+ *
+ * Copyright (C) 2015, Linaro Ltd.
+ * Author: Fu Wei <[email protected]>
+ * Hanjun Guo <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/acpi.h>
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+static int __init map_generic_timer_interrupt(u32 interrupt, u32 flags)
+{
+ int trigger, polarity;
+
+ if (!interrupt)
+ return 0;
+
+ trigger = (flags & ACPI_GTDT_INTERRUPT_MODE) ? ACPI_EDGE_SENSITIVE
+ : ACPI_LEVEL_SENSITIVE;
+
+ polarity = (flags & ACPI_GTDT_INTERRUPT_POLARITY) ? ACPI_ACTIVE_LOW
+ : ACPI_ACTIVE_HIGH;
+
+ return acpi_register_gsi(NULL, interrupt, trigger, polarity);
+}
+
+/*
+ * Initialize a SBSA generic Watchdog platform device info from GTDT
+ * According to SBSA specification the size of refresh and control
+ * frames of SBSA Generic Watchdog is SZ_4K(Offset 0x000 – 0xFFF).
+ */
+static int __init gtdt_import_sbsa_gwdt(struct acpi_gtdt_watchdog *wd,
+ int index)
+{
+ struct platform_device *pdev;
+ int irq = map_generic_timer_interrupt(wd->timer_interrupt,
+ wd->timer_flags);
+ struct resource res[] = {
+ DEFINE_RES_IRQ_NAMED(irq, "ws0"),
+ DEFINE_RES_MEM_NAMED(wd->refresh_frame_address, SZ_4K,
+ "refresh"),
+ DEFINE_RES_MEM_NAMED(wd->control_frame_address, SZ_4K,
+ "control"),
+ };
+
+ pr_debug("GTDT: a Watchdog GT(0x%llx/0x%llx gsi:%u flags:0x%x)\n",
+ wd->refresh_frame_address, wd->control_frame_address,
+ wd->timer_interrupt, wd->timer_flags);
+
+ if (!(wd->refresh_frame_address &&
+ wd->control_frame_address &&
+ wd->timer_interrupt)) {
+ pr_err("GTDT: failed geting the device info.\n");
+ return -EINVAL;
+ }
+
+ if (irq < 0) {
+ pr_err("GTDT: failed to register GSI of the Watchdog GT.\n");
+ return -EINVAL;
+ }
+
+ /*
+ * Add a platform device named "sbsa-gwdt" to match the platform driver.
+ * "sbsa-gwdt": SBSA(Server Base System Architecture) Generic Watchdog
+ * The platform driver (like drivers/watchdog/sbsa_gwdt.c)can get device
+ * info below by matching this name.
+ */
+ pdev = platform_device_register_simple("sbsa-gwdt", index, res,
+ ARRAY_SIZE(res));
+ if (IS_ERR(pdev)) {
+ acpi_unregister_gsi(wd->timer_interrupt);
+ return PTR_ERR(pdev);
+ }
+
+ return 0;
+}
+
+static int __init gtdt_platform_timer_parse(struct acpi_table_header *table)
+{
+ struct acpi_gtdt_header *header;
+ struct acpi_table_gtdt *gtdt;
+ void *gtdt_subtable;
+ int i, gwdt_index;
+ int ret = 0;
+
+ if (table->revision < 2) {
+ pr_warn("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");
+ return -EINVAL;
+ }
+ header = (struct acpi_gtdt_header *)gtdt_subtable;
+ if (header->type == ACPI_GTDT_TYPE_WATCHDOG) {
+ ret = gtdt_import_sbsa_gwdt(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;
+}
+
+static int __init gtdt_platform_timer_init(void)
+{
+ if (acpi_disabled)
+ return 0;
+
+ return acpi_table_parse(ACPI_SIG_GTDT, gtdt_platform_timer_parse);
+}
+
+device_initcall(gtdt_platform_timer_init);
--
1.9.1
From: Fu Wei <[email protected]>
This patch enables ACPI GTDT support for ARM SBSA
watchdog driver automatically, if ACPI support is enabled.
Signed-off-by: Fu Wei <[email protected]>
---
drivers/watchdog/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 554f18a..20f9980 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -157,6 +157,7 @@ config ARM_SBSA_WATCHDOG
depends on ARM64
depends on ARM_ARCH_TIMER
select WATCHDOG_CORE
+ select ACPI_GTDT if ACPI
help
ARM SBSA Generic Watchdog. This watchdog has two Watchdog timeouts.
The first timeout will trigger a panic; the second timeout will
--
1.9.1
From: Fu Wei <[email protected]>
The patch update arm_arch_timer driver to use the function
provided by the new GTDT driver of ACPI.
By this way, arm_arch_timer.c can be simplified, and separate
all the ACPI GTDT knowledge from this timer driver.
Signed-off-by: Fu Wei <[email protected]>
Signed-off-by: Hanjun Guo <[email protected]>
---
arch/arm64/kernel/time.c | 4 +--
drivers/acpi/gtdt.c | 43 ++++++++++++++++++++++++++
drivers/clocksource/Kconfig | 1 +
drivers/clocksource/arm_arch_timer.c | 60 +++++++-----------------------------
include/clocksource/arm_arch_timer.h | 8 +++++
include/linux/acpi.h | 5 +++
include/linux/clocksource.h | 4 +--
7 files changed, 72 insertions(+), 53 deletions(-)
diff --git a/arch/arm64/kernel/time.c b/arch/arm64/kernel/time.c
index 42f9195..2cabea6 100644
--- a/arch/arm64/kernel/time.c
+++ b/arch/arm64/kernel/time.c
@@ -75,9 +75,9 @@ void __init time_init(void)
/*
* Since ACPI or FDT will only one be available in the system,
- * we can use acpi_generic_timer_init() here safely
+ * we can use arch_timer_acpi_init() here safely
*/
- acpi_generic_timer_init();
+ arch_timer_acpi_init();
arch_timer_rate = arch_timer_get_rate();
if (!arch_timer_rate)
diff --git a/drivers/acpi/gtdt.c b/drivers/acpi/gtdt.c
index a92abf2..699c9fd 100644
--- a/drivers/acpi/gtdt.c
+++ b/drivers/acpi/gtdt.c
@@ -17,6 +17,8 @@
#include <linux/module.h>
#include <linux/platform_device.h>
+#include <clocksource/arm_arch_timer.h>
+
static int __init map_generic_timer_interrupt(u32 interrupt, u32 flags)
{
int trigger, polarity;
@@ -33,6 +35,47 @@ static int __init map_generic_timer_interrupt(u32 interrupt, u32 flags)
return acpi_register_gsi(NULL, interrupt, trigger, polarity);
}
+struct arch_timer_data __initdata *arch_timer_data_p;
+
+static int __init arch_timer_data_init(struct acpi_table_header *table)
+{
+ struct acpi_table_gtdt *gtdt;
+
+ gtdt = container_of(table, struct acpi_table_gtdt, header);
+
+ arch_timer_data_p->phys_secure_ppi =
+ map_generic_timer_interrupt(gtdt->secure_el1_interrupt,
+ gtdt->secure_el1_flags);
+
+ arch_timer_data_p->phys_nonsecure_ppi =
+ map_generic_timer_interrupt(gtdt->non_secure_el1_interrupt,
+ gtdt->non_secure_el1_flags);
+
+ arch_timer_data_p->virt_ppi =
+ map_generic_timer_interrupt(gtdt->virtual_timer_interrupt,
+ gtdt->virtual_timer_flags);
+
+ arch_timer_data_p->hyp_ppi =
+ map_generic_timer_interrupt(gtdt->non_secure_el2_interrupt,
+ gtdt->non_secure_el2_flags);
+
+ arch_timer_data_p->c3stop = !(gtdt->non_secure_el1_flags &
+ ACPI_GTDT_ALWAYS_ON);
+
+ return 0;
+}
+
+/* Initialize the arch_timer_data struct for arm_arch_timer by GTDT info */
+int __init gtdt_arch_timer_data_init(struct arch_timer_data *data)
+{
+ if (acpi_disabled || !data)
+ return -EINVAL;
+
+ arch_timer_data_p = data;
+
+ return acpi_table_parse(ACPI_SIG_GTDT, arch_timer_data_init);
+}
+
/*
* Initialize a SBSA generic Watchdog platform device info from GTDT
* According to SBSA specification the size of refresh and control
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 51d7865f..ea94a6f 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -109,6 +109,7 @@ config CLKSRC_EFM32
config ARM_ARCH_TIMER
bool
select CLKSRC_OF if OF
+ select ACPI_GTDT if ACPI
config ARM_ARCH_TIMER_EVTSTREAM
bool "Support for ARM architected timer event stream generation"
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 0aa135d..99505bb 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -817,68 +817,30 @@ CLOCKSOURCE_OF_DECLARE(armv7_arch_timer_mem, "arm,armv7-timer-mem",
arch_timer_mem_init);
#ifdef CONFIG_ACPI
-static int __init map_generic_timer_interrupt(u32 interrupt, u32 flags)
-{
- int trigger, polarity;
-
- if (!interrupt)
- return 0;
-
- trigger = (flags & ACPI_GTDT_INTERRUPT_MODE) ? ACPI_EDGE_SENSITIVE
- : ACPI_LEVEL_SENSITIVE;
-
- polarity = (flags & ACPI_GTDT_INTERRUPT_POLARITY) ? ACPI_ACTIVE_LOW
- : ACPI_ACTIVE_HIGH;
-
- return acpi_register_gsi(NULL, interrupt, trigger, polarity);
-}
-
/* Initialize per-processor generic timer */
-static int __init arch_timer_acpi_init(struct acpi_table_header *table)
+void __init arch_timer_acpi_init(void)
{
- struct acpi_table_gtdt *gtdt;
+ struct arch_timer_data data;
if (arch_timers_present & ARCH_CP15_TIMER) {
pr_warn("arch_timer: already initialized, skipping\n");
- return -EINVAL;
+ return;
}
- gtdt = container_of(table, struct acpi_table_gtdt, header);
-
arch_timers_present |= ARCH_CP15_TIMER;
- arch_timer_ppi[PHYS_SECURE_PPI] =
- map_generic_timer_interrupt(gtdt->secure_el1_interrupt,
- gtdt->secure_el1_flags);
-
- arch_timer_ppi[PHYS_NONSECURE_PPI] =
- map_generic_timer_interrupt(gtdt->non_secure_el1_interrupt,
- gtdt->non_secure_el1_flags);
-
- arch_timer_ppi[VIRT_PPI] =
- map_generic_timer_interrupt(gtdt->virtual_timer_interrupt,
- gtdt->virtual_timer_flags);
+ if (gtdt_arch_timer_data_init(&data))
+ return;
- arch_timer_ppi[HYP_PPI] =
- map_generic_timer_interrupt(gtdt->non_secure_el2_interrupt,
- gtdt->non_secure_el2_flags);
+ arch_timer_ppi[PHYS_SECURE_PPI] = data.phys_secure_ppi;
+ arch_timer_ppi[PHYS_NONSECURE_PPI] = data.phys_nonsecure_ppi;
+ arch_timer_ppi[VIRT_PPI] = data.virt_ppi;
+ arch_timer_ppi[HYP_PPI] = data.hyp_ppi;
+ /* Always-on capability */
+ arch_timer_c3stop = data.c3stop;
/* Get the frequency from CNTFRQ */
arch_timer_detect_rate(NULL, NULL);
-
- /* Always-on capability */
- arch_timer_c3stop = !(gtdt->non_secure_el1_flags & ACPI_GTDT_ALWAYS_ON);
-
arch_timer_init();
- return 0;
-}
-
-/* Initialize all the generic timers presented in GTDT */
-void __init acpi_generic_timer_init(void)
-{
- if (acpi_disabled)
- return;
-
- acpi_table_parse(ACPI_SIG_GTDT, arch_timer_acpi_init);
}
#endif
diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h
index 9916d0e..5deed9d 100644
--- a/include/clocksource/arm_arch_timer.h
+++ b/include/clocksource/arm_arch_timer.h
@@ -43,6 +43,14 @@ enum arch_timer_reg {
#define ARCH_TIMER_EVT_STREAM_FREQ 10000 /* 100us */
+struct arch_timer_data {
+ int phys_secure_ppi;
+ int phys_nonsecure_ppi;
+ int virt_ppi;
+ int hyp_ppi;
+ bool c3stop;
+};
+
#ifdef CONFIG_ARM_ARCH_TIMER
extern u32 arch_timer_get_rate(void);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 598f0f1..7213c0d 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -458,6 +458,11 @@ void acpi_walk_dep_device_list(acpi_handle handle);
struct platform_device *acpi_create_platform_device(struct acpi_device *);
#define ACPI_PTR(_ptr) (_ptr)
+#ifdef CONFIG_ACPI_GTDT
+struct arch_timer_data;
+int __init gtdt_arch_timer_data_init(struct arch_timer_data *data);
+#endif
+
#else /* !CONFIG_ACPI */
#define acpi_disabled 1
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index d27d015..264e553 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -254,9 +254,9 @@ static inline void clocksource_of_init(void) {}
#endif
#ifdef CONFIG_ACPI
-void acpi_generic_timer_init(void);
+void arch_timer_acpi_init(void);
#else
-static inline void acpi_generic_timer_init(void) { }
+static inline void arch_timer_acpi_init(void) { }
#endif
#endif /* _LINUX_CLOCKSOURCE_H */
--
1.9.1
On 06/10/2015 10:47 AM, [email protected] wrote:
> From: Fu Wei <[email protected]>
>
> This driver bases on linux kernel watchdog framework.
> It supports getting timeout from parameter and FDT
> at the driver init stage.
> The first timeout period expires, the interrupt routine
> got another timeout period to run panic for saving
> system context.
>
> Signed-off-by: Fu Wei <[email protected]>
> ---
> drivers/watchdog/Kconfig | 11 ++
> drivers/watchdog/Makefile | 1 +
> drivers/watchdog/sbsa_gwdt.c | 383 +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 395 insertions(+)
> create mode 100644 drivers/watchdog/sbsa_gwdt.c
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index e5e7c55..554f18a 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -152,6 +152,17 @@ 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
> + depends on ARM_ARCH_TIMER
> + select WATCHDOG_CORE
> + help
> + ARM SBSA Generic Watchdog. This watchdog has two Watchdog timeouts.
> + The first timeout will trigger a panic; the second timeout will
> + trigger a system reset.
> + More details: ARM 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..1ddc10f
> --- /dev/null
> +++ b/drivers/watchdog/sbsa_gwdt.c
> @@ -0,0 +1,383 @@
> +/*
> + * SBSA(Server Base System Architecture) Generic Watchdog driver
> + *
> + * Copyright (c) 2015, Linaro Ltd.
> + * Author: Fu Wei <[email protected]>
> + * Suravee Suthikulpanit <[email protected]>
> + *
> + * 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 has two stage timeouts,
> + * When the first timeout occurs, WS0(SPI or LPI) is triggered,
> + * the second timeout period(as long as the first timeout period) starts.
> + * In WS0 interrupt routine, panic() will be called for collecting
> + * crashdown info.
> + * If system can not recover from WS0 interrupt routine, then second
> + * timeout occurs, WS1(reset or higher level interrupt) is triggered.
> + * The two timeout period can be set by WOR(32bit).
> + * WOR gives a maximum watch period of around 10s at the maximum
> + * system counter frequency.
> + * The System Counter shall run at maximum of 400MHz.
> + *
> + * But If we need a larger timeout period, this driver will programme WCV
> + * directly. That can support more than 10s timeout at the maximum
> + * system counter frequency.
> + * More details: ARM DEN0029B - Server Base System Architecture (SBSA)
> + *
> + * SBSA GWDT: |---WOR(or WCV)---WS0---WOR(or WCV)---WS1
> + * |-----timeout-----WS0-----timeout-----WS1
If we use WCV at all, I would like to see something like
* SBSA GWDT: |---WOR(or WCV)---WS0--------WOR------WS1
* |-----timeout-----WS0-----------------WS1
* panic hw reset
where WOR would be used up to its maximum, to be replaced by WCV
(but kept at maximum) if the selected timeout is larger than the
maximum timeout selectable with WOR. Would this be possible ?
Guenter
Hi Guenter,
On 11 June 2015 at 13:33, Guenter Roeck <[email protected]> wrote:
> On 06/10/2015 10:47 AM, [email protected] wrote:
>>
>> From: Fu Wei <[email protected]>
>>
>> This driver bases on linux kernel watchdog framework.
>> It supports getting timeout from parameter and FDT
>> at the driver init stage.
>> The first timeout period expires, the interrupt routine
>> got another timeout period to run panic for saving
>> system context.
>>
>> Signed-off-by: Fu Wei <[email protected]>
>> ---
>> drivers/watchdog/Kconfig | 11 ++
>> drivers/watchdog/Makefile | 1 +
>> drivers/watchdog/sbsa_gwdt.c | 383
>> +++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 395 insertions(+)
>> create mode 100644 drivers/watchdog/sbsa_gwdt.c
>>
>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>> index e5e7c55..554f18a 100644
>> --- a/drivers/watchdog/Kconfig
>> +++ b/drivers/watchdog/Kconfig
>> @@ -152,6 +152,17 @@ 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
>> + depends on ARM_ARCH_TIMER
>> + select WATCHDOG_CORE
>> + help
>> + ARM SBSA Generic Watchdog. This watchdog has two Watchdog
>> timeouts.
>> + The first timeout will trigger a panic; the second timeout will
>> + trigger a system reset.
>> + More details: ARM 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..1ddc10f
>> --- /dev/null
>> +++ b/drivers/watchdog/sbsa_gwdt.c
>> @@ -0,0 +1,383 @@
>> +/*
>> + * SBSA(Server Base System Architecture) Generic Watchdog driver
>> + *
>> + * Copyright (c) 2015, Linaro Ltd.
>> + * Author: Fu Wei <[email protected]>
>> + * Suravee Suthikulpanit <[email protected]>
>> + *
>> + * 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 has two stage timeouts,
>> + * When the first timeout occurs, WS0(SPI or LPI) is triggered,
>> + * the second timeout period(as long as the first timeout period)
>> starts.
>> + * In WS0 interrupt routine, panic() will be called for collecting
>> + * crashdown info.
>> + * If system can not recover from WS0 interrupt routine, then
>> second
>> + * timeout occurs, WS1(reset or higher level interrupt) is
>> triggered.
>> + * The two timeout period can be set by WOR(32bit).
>> + * WOR gives a maximum watch period of around 10s at the maximum
>> + * system counter frequency.
>> + * The System Counter shall run at maximum of 400MHz.
>> + *
>> + * But If we need a larger timeout period, this driver will
>> programme WCV
>> + * directly. That can support more than 10s timeout at the maximum
>> + * system counter frequency.
>> + * More details: ARM DEN0029B - Server Base System Architecture
>> (SBSA)
>> + *
>> + * SBSA GWDT: |---WOR(or WCV)---WS0---WOR(or WCV)---WS1
>> + * |-----timeout-----WS0-----timeout-----WS1
>
>
> If we use WCV at all, I would like to see something like
>
> * SBSA GWDT: |---WOR(or WCV)---WS0--------WOR------WS1
> * |-----timeout-----WS0-----------------WS1
> * panic hw reset
>
> where WOR would be used up to its maximum, to be replaced by WCV
> (but kept at maximum) if the selected timeout is larger than the
> maximum timeout selectable with WOR. Would this be possible ?
for this part |-----timeout-----WS0, I am doing this way in
non-pretimeout version.
but for WS0-----------------WS1, do you mean WOR would always be used
up to its maximum???
I don't see any variable attached on it. So I would like to confirm
what is this value
>
> Guenter
>
--
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 06/10/2015 10:44 PM, Fu Wei wrote:
> Hi Guenter,
>
> On 11 June 2015 at 13:33, Guenter Roeck <[email protected]> wrote:
>> On 06/10/2015 10:47 AM, [email protected] wrote:
>>>
>>> From: Fu Wei <[email protected]>
>>>
>>> This driver bases on linux kernel watchdog framework.
>>> It supports getting timeout from parameter and FDT
>>> at the driver init stage.
>>> The first timeout period expires, the interrupt routine
>>> got another timeout period to run panic for saving
>>> system context.
>>>
>>> Signed-off-by: Fu Wei <[email protected]>
>>> ---
>>> drivers/watchdog/Kconfig | 11 ++
>>> drivers/watchdog/Makefile | 1 +
>>> drivers/watchdog/sbsa_gwdt.c | 383
>>> +++++++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 395 insertions(+)
>>> create mode 100644 drivers/watchdog/sbsa_gwdt.c
>>>
>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>>> index e5e7c55..554f18a 100644
>>> --- a/drivers/watchdog/Kconfig
>>> +++ b/drivers/watchdog/Kconfig
>>> @@ -152,6 +152,17 @@ 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
>>> + depends on ARM_ARCH_TIMER
>>> + select WATCHDOG_CORE
>>> + help
>>> + ARM SBSA Generic Watchdog. This watchdog has two Watchdog
>>> timeouts.
>>> + The first timeout will trigger a panic; the second timeout will
>>> + trigger a system reset.
>>> + More details: ARM 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..1ddc10f
>>> --- /dev/null
>>> +++ b/drivers/watchdog/sbsa_gwdt.c
>>> @@ -0,0 +1,383 @@
>>> +/*
>>> + * SBSA(Server Base System Architecture) Generic Watchdog driver
>>> + *
>>> + * Copyright (c) 2015, Linaro Ltd.
>>> + * Author: Fu Wei <[email protected]>
>>> + * Suravee Suthikulpanit <[email protected]>
>>> + *
>>> + * 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 has two stage timeouts,
>>> + * When the first timeout occurs, WS0(SPI or LPI) is triggered,
>>> + * the second timeout period(as long as the first timeout period)
>>> starts.
>>> + * In WS0 interrupt routine, panic() will be called for collecting
>>> + * crashdown info.
>>> + * If system can not recover from WS0 interrupt routine, then
>>> second
>>> + * timeout occurs, WS1(reset or higher level interrupt) is
>>> triggered.
>>> + * The two timeout period can be set by WOR(32bit).
>>> + * WOR gives a maximum watch period of around 10s at the maximum
>>> + * system counter frequency.
>>> + * The System Counter shall run at maximum of 400MHz.
>>> + *
>>> + * But If we need a larger timeout period, this driver will
>>> programme WCV
>>> + * directly. That can support more than 10s timeout at the maximum
>>> + * system counter frequency.
>>> + * More details: ARM DEN0029B - Server Base System Architecture
>>> (SBSA)
>>> + *
>>> + * SBSA GWDT: |---WOR(or WCV)---WS0---WOR(or WCV)---WS1
>>> + * |-----timeout-----WS0-----timeout-----WS1
>>
>>
>> If we use WCV at all, I would like to see something like
>>
>> * SBSA GWDT: |---WOR(or WCV)---WS0--------WOR------WS1
>> * |-----timeout-----WS0-----------------WS1
>> * panic hw reset
>>
>> where WOR would be used up to its maximum, to be replaced by WCV
>> (but kept at maximum) if the selected timeout is larger than the
>> maximum timeout selectable with WOR. Would this be possible ?
>
> for this part |-----timeout-----WS0, I am doing this way in
> non-pretimeout version.
>
> but for WS0-----------------WS1, do you mean WOR would always be used
> up to its maximum???
yes
> I don't see any variable attached on it. So I would like to confirm
> what is this value
>
min(timeout, max_wor_timeout)
Guenter
Hi Guenter,
On 11 June 2015 at 13:49, Guenter Roeck <[email protected]> wrote:
> On 06/10/2015 10:44 PM, Fu Wei wrote:
>>
>> Hi Guenter,
>>
>> On 11 June 2015 at 13:33, Guenter Roeck <[email protected]> wrote:
>>>
>>> On 06/10/2015 10:47 AM, [email protected] wrote:
>>>>
>>>>
>>>> From: Fu Wei <[email protected]>
>>>>
>>>> This driver bases on linux kernel watchdog framework.
>>>> It supports getting timeout from parameter and FDT
>>>> at the driver init stage.
>>>> The first timeout period expires, the interrupt routine
>>>> got another timeout period to run panic for saving
>>>> system context.
>>>>
>>>> Signed-off-by: Fu Wei <[email protected]>
>>>> ---
>>>> drivers/watchdog/Kconfig | 11 ++
>>>> drivers/watchdog/Makefile | 1 +
>>>> drivers/watchdog/sbsa_gwdt.c | 383
>>>> +++++++++++++++++++++++++++++++++++++++++++
>>>> 3 files changed, 395 insertions(+)
>>>> create mode 100644 drivers/watchdog/sbsa_gwdt.c
>>>>
>>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>>>> index e5e7c55..554f18a 100644
>>>> --- a/drivers/watchdog/Kconfig
>>>> +++ b/drivers/watchdog/Kconfig
>>>> @@ -152,6 +152,17 @@ 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
>>>> + depends on ARM_ARCH_TIMER
>>>> + select WATCHDOG_CORE
>>>> + help
>>>> + ARM SBSA Generic Watchdog. This watchdog has two Watchdog
>>>> timeouts.
>>>> + The first timeout will trigger a panic; the second timeout
>>>> will
>>>> + trigger a system reset.
>>>> + More details: ARM 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..1ddc10f
>>>> --- /dev/null
>>>> +++ b/drivers/watchdog/sbsa_gwdt.c
>>>> @@ -0,0 +1,383 @@
>>>> +/*
>>>> + * SBSA(Server Base System Architecture) Generic Watchdog driver
>>>> + *
>>>> + * Copyright (c) 2015, Linaro Ltd.
>>>> + * Author: Fu Wei <[email protected]>
>>>> + * Suravee Suthikulpanit <[email protected]>
>>>> + *
>>>> + * 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 has two stage timeouts,
>>>> + * When the first timeout occurs, WS0(SPI or LPI) is triggered,
>>>> + * the second timeout period(as long as the first timeout period)
>>>> starts.
>>>> + * In WS0 interrupt routine, panic() will be called for
>>>> collecting
>>>> + * crashdown info.
>>>> + * If system can not recover from WS0 interrupt routine, then
>>>> second
>>>> + * timeout occurs, WS1(reset or higher level interrupt) is
>>>> triggered.
>>>> + * The two timeout period can be set by WOR(32bit).
>>>> + * WOR gives a maximum watch period of around 10s at the maximum
>>>> + * system counter frequency.
>>>> + * The System Counter shall run at maximum of 400MHz.
>>>> + *
>>>> + * But If we need a larger timeout period, this driver will
>>>> programme WCV
>>>> + * directly. That can support more than 10s timeout at the
>>>> maximum
>>>> + * system counter frequency.
>>>> + * More details: ARM DEN0029B - Server Base System Architecture
>>>> (SBSA)
>>>> + *
>>>> + * SBSA GWDT: |---WOR(or WCV)---WS0---WOR(or WCV)---WS1
>>>> + * |-----timeout-----WS0-----timeout-----WS1
>>>
>>>
>>>
>>> If we use WCV at all, I would like to see something like
>>>
>>> * SBSA GWDT: |---WOR(or WCV)---WS0--------WOR------WS1
>>> * |-----timeout-----WS0-----------------WS1
>>> * panic hw reset
>>>
>>> where WOR would be used up to its maximum, to be replaced by WCV
>>> (but kept at maximum) if the selected timeout is larger than the
>>> maximum timeout selectable with WOR. Would this be possible ?
>>
>>
>> for this part |-----timeout-----WS0, I am doing this way in
>> non-pretimeout version.
>>
>> but for WS0-----------------WS1, do you mean WOR would always be used
>> up to its maximum???
>
>
> yes
>
>> I don't see any variable attached on it. So I would like to confirm
>> what is this value
>>
>
> min(timeout, max_wor_timeout)
Sure, no problem. just need a very little fix in non-pretimout version
Just delete several lines in "sbsa_gwdt_interrupt", become like this:
+static irqreturn_t sbsa_gwdt_interrupt(int irq, void *dev_id)
+{
+ panic("SBSA Watchdog pre-timeout");
+
+ return IRQ_HANDLED;
+}
this matches your thought.
Any other code I need to improve ? If It is OK for you, I can sent a
new patchset soon.
Please let me know anything else I need to improve.
Great thanks for your time
>
> Guenter
>
--
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 Rafael,
Any comments on patch 5,6,7/7 of this patch set?
Thanks
Hanjun
On 06/11/2015 01:47 AM, [email protected] wrote:
> From: Fu Wei <[email protected]>
>
> This driver adds support for parsing SBSA Generic Watchdog
> Structure in GTDT, and creating a platform device with that
> information. This allows the operating system to obtain device
> data from the resource of platform device.
>
> The platform device named "sbsa-gwdt" can be used by the
> ARM SBSA Generic Watchdog driver.
>
> Signed-off-by: Fu Wei <[email protected]>
> Signed-off-by: Hanjun Guo <[email protected]>
> ---
> drivers/acpi/Kconfig | 9 ++++
> drivers/acpi/Makefile | 1 +
> drivers/acpi/gtdt.c | 137 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 147 insertions(+)
> create mode 100644 drivers/acpi/gtdt.c
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 1bbaa3d..e125698 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -433,4 +433,13 @@ config XPOWER_PMIC_OPREGION
>
> endif
>
> +config ACPI_GTDT
> + bool "ACPI GTDT Support"
> + depends on ARM64
> + help
> + GTDT (Generic Timer Description Table) provides information
> + for per-processor timers and Platform (memory-mapped) timers
> + for ARM platforms. Select this option to provide information
> + needed for the timers init.
> +
> endif # ACPI
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 431e587..2c5a194 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -96,3 +96,4 @@ obj-$(CONFIG_ACPI_EXTLOG) += acpi_extlog.o
> obj-$(CONFIG_PMIC_OPREGION) += pmic/intel_pmic.o
> obj-$(CONFIG_CRC_PMIC_OPREGION) += pmic/intel_pmic_crc.o
> obj-$(CONFIG_XPOWER_PMIC_OPREGION) += pmic/intel_pmic_xpower.o
> +obj-$(CONFIG_ACPI_GTDT) += gtdt.o
> diff --git a/drivers/acpi/gtdt.c b/drivers/acpi/gtdt.c
> new file mode 100644
> index 0000000..a92abf2
> --- /dev/null
> +++ b/drivers/acpi/gtdt.c
> @@ -0,0 +1,137 @@
> +/*
> + * ARM Specific GTDT table Support
> + *
> + * Copyright (C) 2015, Linaro Ltd.
> + * Author: Fu Wei <[email protected]>
> + * Hanjun Guo <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/device.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +
> +static int __init map_generic_timer_interrupt(u32 interrupt, u32 flags)
> +{
> + int trigger, polarity;
> +
> + if (!interrupt)
> + return 0;
> +
> + trigger = (flags & ACPI_GTDT_INTERRUPT_MODE) ? ACPI_EDGE_SENSITIVE
> + : ACPI_LEVEL_SENSITIVE;
> +
> + polarity = (flags & ACPI_GTDT_INTERRUPT_POLARITY) ? ACPI_ACTIVE_LOW
> + : ACPI_ACTIVE_HIGH;
> +
> + return acpi_register_gsi(NULL, interrupt, trigger, polarity);
> +}
> +
> +/*
> + * Initialize a SBSA generic Watchdog platform device info from GTDT
> + * According to SBSA specification the size of refresh and control
> + * frames of SBSA Generic Watchdog is SZ_4K(Offset 0x000 – 0xFFF).
> + */
> +static int __init gtdt_import_sbsa_gwdt(struct acpi_gtdt_watchdog *wd,
> + int index)
> +{
> + struct platform_device *pdev;
> + int irq = map_generic_timer_interrupt(wd->timer_interrupt,
> + wd->timer_flags);
> + struct resource res[] = {
> + DEFINE_RES_IRQ_NAMED(irq, "ws0"),
> + DEFINE_RES_MEM_NAMED(wd->refresh_frame_address, SZ_4K,
> + "refresh"),
> + DEFINE_RES_MEM_NAMED(wd->control_frame_address, SZ_4K,
> + "control"),
> + };
> +
> + pr_debug("GTDT: a Watchdog GT(0x%llx/0x%llx gsi:%u flags:0x%x)\n",
> + wd->refresh_frame_address, wd->control_frame_address,
> + wd->timer_interrupt, wd->timer_flags);
> +
> + if (!(wd->refresh_frame_address &&
> + wd->control_frame_address &&
> + wd->timer_interrupt)) {
> + pr_err("GTDT: failed geting the device info.\n");
> + return -EINVAL;
> + }
> +
> + if (irq < 0) {
> + pr_err("GTDT: failed to register GSI of the Watchdog GT.\n");
> + return -EINVAL;
> + }
> +
> + /*
> + * Add a platform device named "sbsa-gwdt" to match the platform driver.
> + * "sbsa-gwdt": SBSA(Server Base System Architecture) Generic Watchdog
> + * The platform driver (like drivers/watchdog/sbsa_gwdt.c)can get device
> + * info below by matching this name.
> + */
> + pdev = platform_device_register_simple("sbsa-gwdt", index, res,
> + ARRAY_SIZE(res));
> + if (IS_ERR(pdev)) {
> + acpi_unregister_gsi(wd->timer_interrupt);
> + return PTR_ERR(pdev);
> + }
> +
> + return 0;
> +}
> +
> +static int __init gtdt_platform_timer_parse(struct acpi_table_header *table)
> +{
> + struct acpi_gtdt_header *header;
> + struct acpi_table_gtdt *gtdt;
> + void *gtdt_subtable;
> + int i, gwdt_index;
> + int ret = 0;
> +
> + if (table->revision < 2) {
> + pr_warn("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");
> + return -EINVAL;
> + }
> + header = (struct acpi_gtdt_header *)gtdt_subtable;
> + if (header->type == ACPI_GTDT_TYPE_WATCHDOG) {
> + ret = gtdt_import_sbsa_gwdt(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;
> +}
> +
> +static int __init gtdt_platform_timer_init(void)
> +{
> + if (acpi_disabled)
> + return 0;
> +
> + return acpi_table_parse(ACPI_SIG_GTDT, gtdt_platform_timer_parse);
> +}
> +
> +device_initcall(gtdt_platform_timer_init);
>
On Thu, Jun 11, 2015 at 01:47:29AM +0800, [email protected] wrote:
> From: Fu Wei <[email protected]>
>
> This driver bases on linux kernel watchdog framework.
> It supports getting timeout from parameter and FDT
> at the driver init stage.
> The first timeout period expires, the interrupt routine
> got another timeout period to run panic for saving
> system context.
>
Comments inline.
Thanks,
Guenter
> Signed-off-by: Fu Wei <[email protected]>
> ---
> drivers/watchdog/Kconfig | 11 ++
> drivers/watchdog/Makefile | 1 +
> drivers/watchdog/sbsa_gwdt.c | 383 +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 395 insertions(+)
> create mode 100644 drivers/watchdog/sbsa_gwdt.c
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index e5e7c55..554f18a 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -152,6 +152,17 @@ 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
> + depends on ARM_ARCH_TIMER
> + select WATCHDOG_CORE
> + help
> + ARM SBSA Generic Watchdog. This watchdog has two Watchdog timeouts.
> + The first timeout will trigger a panic; the second timeout will
> + trigger a system reset.
> + More details: ARM DEN0029B - Server Base System Architecture (SBSA)
> +
To compile this driver as module, choose M here: The module
will be called sbsa_gwdt.
> 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..1ddc10f
> --- /dev/null
> +++ b/drivers/watchdog/sbsa_gwdt.c
> @@ -0,0 +1,383 @@
> +/*
> + * SBSA(Server Base System Architecture) Generic Watchdog driver
> + *
> + * Copyright (c) 2015, Linaro Ltd.
> + * Author: Fu Wei <[email protected]>
> + * Suravee Suthikulpanit <[email protected]>
> + *
> + * 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 has two stage timeouts,
s/This/The/
"has two stages".
I would suggest to drop "Note:", but that is up to you.
> + * When the first timeout occurs, WS0(SPI or LPI) is triggered,
> + * the second timeout period(as long as the first timeout period) starts.
no longer accurate if WOR is used for the second period.
> + * In WS0 interrupt routine, panic() will be called for collecting
> + * crashdown info.
> + * If system can not recover from WS0 interrupt routine, then second
> + * timeout occurs, WS1(reset or higher level interrupt) is triggered.
> + * The two timeout period can be set by WOR(32bit).
The second timeout period is determined by ...
> + * WOR gives a maximum watch period of around 10s at the maximum
> + * system counter frequency.
> + * The System Counter shall run at maximum of 400MHz.
"... at the maximum system counter frequency of 400 MHz.", and drop the
last sentence.
Please uses spaces before '('.
> + *
> + * But If we need a larger timeout period, this driver will programme WCV
s/But //
s/this/the/
s/programme/program/
> + * directly. That can support more than 10s timeout at the maximum
> + * system counter frequency.
Drop the last sentence.
> + * More details: ARM DEN0029B - Server Base System Architecture (SBSA)
> + *
> + * SBSA GWDT: |---WOR(or WCV)---WS0---WOR(or WCV)---WS1
> + * |-----timeout-----WS0-----timeout-----WS1
> + */
> +
> +#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/uaccess.h>
> +#include <linux/watchdog.h>
> +#include <asm/arch_timer.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)
> +
> +/**
> + * struct sbsa_gwdt - Internal representation of the SBSA GWDT
> + * @wdd: kernel watchdog_device structure
> + * @clk: store the System Counter clock frequency, in Hz.
> + * @max_wor_timeout: the maximum timeout value for WOR (in seconds).
> + * @refresh_base: Virtual address of the watchdog refresh frame
> + * @control_base: Virtual address of the watchdog control frame
> + */
> +struct sbsa_gwdt {
> + struct watchdog_device wdd;
> + u32 clk;
> + int max_wor_timeout;
> + void __iomem *refresh_base;
> + void __iomem *control_base;
> +};
> +
> +#define to_sbsa_gwdt(e) container_of(e, struct sbsa_gwdt, wdd)
> +
> +#define DEFAULT_TIMEOUT 30 /* seconds */
> +
> +static unsigned int timeout;
> +module_param(timeout, uint, 0);
> +MODULE_PARM_DESC(timeout,
> + "Watchdog timeout in seconds. (>=0, default="
> + __MODULE_STRING(DEFAULT_TIMEOUT) ")");
> +
> +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) ")");
> +
> +/*
> + * help functions for accessing 64bit WCV register
> + */
> +static u64 sbsa_gwdt_get_wcv(struct watchdog_device *wdd)
> +{
> + u32 wcv_lo, wcv_hi;
> + struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
> +
> + do {
> + wcv_hi = readl_relaxed(gwdt->control_base + SBSA_GWDT_WCV_HI);
> + wcv_lo = readl_relaxed(gwdt->control_base + SBSA_GWDT_WCV_LO);
> + } while (wcv_hi != readl_relaxed(gwdt->control_base +
> + SBSA_GWDT_WCV_HI));
> +
> + return (((u64)wcv_hi << 32) | wcv_lo);
> +}
> +
> +static void reload_timeout_to_wcv(struct watchdog_device *wdd)
> +{
> + struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
> + u64 wcv;
> +
> + wcv = arch_counter_get_cntvct() + (u64)wdd->timeout * gwdt->clk;
> +
> + writel_relaxed(upper_32_bits(wcv),
> + gwdt->control_base + SBSA_GWDT_WCV_HI);
> + writel_relaxed(lower_32_bits(wcv),
> + gwdt->control_base + SBSA_GWDT_WCV_LO);
> +}
> +
> +static int sbsa_gwdt_set_timeout(struct watchdog_device *wdd,
> + unsigned int timeout)
> +{
> + struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
> +
> + wdd->timeout = timeout;
> +
> + if (timeout <= gwdt->max_wor_timeout)
> + writel_relaxed(timeout * gwdt->clk,
> + gwdt->control_base + SBSA_GWDT_WOR);
> + else
> + writel_relaxed(gwdt->max_wor_timeout * gwdt->clk,
> + gwdt->control_base + SBSA_GWDT_WOR);
> +
This can be simplified a bit to
if (timeout > gwdt->max_wor_timeout)
timeout = gwdt->max_wor_timeout;
writel_relaxed(timeout * gwdt->clk,
gwdt->control_base + SBSA_GWDT_WOR);
> + 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();
> +
> + do_div(timeleft, gwdt->clk);
> +
> + return timeleft;
> +}
> +
> +static int sbsa_gwdt_keepalive(struct watchdog_device *wdd)
> +{
> + struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
> +
> + if (wdd->timeout <= gwdt->max_wor_timeout)
> + /*
> + * Writing WRR for an explicit watchdog refresh.
> + * You can write anyting(like 0xc0ffee).
> + */
> + writel_relaxed(0xc0ffee, gwdt->refresh_base + SBSA_GWDT_WRR);
> + else
> + reload_timeout_to_wcv(wdd);
> +
> + return 0;
> +}
> +
> +static int sbsa_gwdt_start(struct watchdog_device *wdd)
> +{
> + struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
> + /* Force refresh due to hardware bug found in certain Soc. */
Can you specify which SOC(s) are known to need this, and explain the bug
a bit better ?
> + writel_relaxed(0xc0ffee, gwdt->refresh_base + SBSA_GWDT_WRR);
> + /* writing WCS will cause an explicit watchdog refresh */
> + writel_relaxed(SBSA_GWDT_WCS_EN, gwdt->control_base + SBSA_GWDT_WCS);
> +
> + return sbsa_gwdt_keepalive(wdd);
> +}
> +
> +static int sbsa_gwdt_stop(struct watchdog_device *wdd)
> +{
> + struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
> +
> + writel_relaxed(0, gwdt->control_base + SBSA_GWDT_WCS);
> +
> + return 0;
> +}
> +
> +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;
> +
> + if (wdd->timeout > gwdt->max_wor_timeout)
> + reload_timeout_to_wcv(wdd);
> +
Please drop the above.
> + 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_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,
> + .get_timeleft = sbsa_gwdt_get_timeleft,
> +};
> +
> +static int sbsa_gwdt_probe(struct platform_device *pdev)
> +{
> + u64 first_period_max = U64_MAX;
> + struct device *dev = &pdev->dev;
> + struct watchdog_device *wdd;
> + struct sbsa_gwdt *gwdt;
> + struct resource *res;
> + void *rf_base, *cf_base;
> + int ret, irq;
> + u32 status;
> +
> + gwdt = devm_kzalloc(dev, sizeof(*gwdt), GFP_KERNEL);
> + if (!gwdt)
> + return -ENOMEM;
> + platform_set_drvdata(pdev, gwdt);
> +
> + 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);
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "control");
> + cf_base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(cf_base))
> + return PTR_ERR(cf_base);
> +
> + irq = platform_get_irq_byname(pdev, "ws0");
> + if (irq < 0) {
> + dev_err(dev, "unable to get ws0 interrupt.\n");
> + return irq;
> + }
> +
> + /*
> + * Get the frequency of system counter from the cp15 interface of ARM
> + * Generic timer. We don't need to check it, because if it returns "0",
> + * system would panic in very early stage.
> + */
> + gwdt->clk = arch_timer_get_cntfrq();
> + gwdt->refresh_base = rf_base;
> + gwdt->control_base = cf_base;
> + gwdt->max_wor_timeout = U32_MAX / gwdt->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);
> +
> + wdd->min_timeout = 1;
> + do_div(first_period_max, gwdt->clk);
> + wdd->max_timeout = first_period_max;
> +
> + wdd->timeout = DEFAULT_TIMEOUT;
> + watchdog_init_timeout(wdd, timeout, dev);
> +
> + 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));
WCV here only tells us how many clock cycles were executed since the
system started (or something like that). So I still don't understand
why it is valuable to print that number.
> + 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);
> + }
Can you merge the message with the info message below ?
Something like
dev_info(dev, "Initialized with %ds timeout @ %u Hz%s\n", wdd->timeout,
gwdt->clk, status & SBSA_GWDT_WCS_EN ? " [enabled]" : "");
I don't think that should be a warning.
> +
> + /* update timeout to WOR */
> + sbsa_gwdt_set_timeout(wdd, wdd->timeout);
> +
That will trigger a refresh if the watchdog is active, meaning the timeout
will occur at time + WOR, not at time + timeout. I think keepalive has to be
called later, preferrably after calling watchdog_register_device().
> + ret = devm_request_irq(dev, irq, sbsa_gwdt_interrupt, 0,
> + 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;
> +
> + dev_info(dev, "Initialized with %ds timeout @ %u Hz\n", wdd->timeout,
> + 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);
> +
> + watchdog_unregister_device(&gwdt->wdd);
> +
> + return 0;
> +}
> +
> +/* Disable watchdog if it is active during suspend */
> +static int __maybe_unused sbsa_gwdt_suspend(struct device *dev)
> +{
> + struct sbsa_gwdt *gwdt = dev_get_drvdata(dev);
> +
> + if (watchdog_active(&gwdt->wdd))
> + sbsa_gwdt_stop(&gwdt->wdd);
> +
> + return 0;
> +}
> +
> +/* Enable watchdog and configure it if necessary */
> +static int __maybe_unused sbsa_gwdt_resume(struct device *dev)
> +{
> + struct sbsa_gwdt *gwdt = dev_get_drvdata(dev);
> +
> + if (watchdog_active(&gwdt->wdd))
> + sbsa_gwdt_start(&gwdt->wdd);
> +
> + return 0;
> +}
> +
> +static const struct dev_pm_ops sbsa_gwdt_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(sbsa_gwdt_suspend, sbsa_gwdt_resume)
> +};
> +
> +static const struct of_device_id sbsa_gwdt_of_match[] = {
> + { .compatible = "arm,sbsa-gwdt", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, sbsa_gwdt_of_match);
> +
> +static const struct platform_device_id sbsa_gwdt_pdev_match[] = {
> + { .name = "sbsa-gwdt", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(platform, sbsa_gwdt_pdev_match);
> +
> +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,
> + .id_table = sbsa_gwdt_pdev_match,
> +};
> +
> +module_platform_driver(sbsa_gwdt_driver);
> +
> +MODULE_DESCRIPTION("SBSA Generic Watchdog Driver");
> +MODULE_VERSION("v1.0");
Version numbers tend to be out of date constantly, and there is no well
defined mechanism or protocol when increase them. I would suggest to drop it.
> +MODULE_AUTHOR("Fu Wei <[email protected]>");
> +MODULE_AUTHOR("Suravee Suthikulpanit <[email protected]>");
> +MODULE_LICENSE("GPL v2");
[email protected] wrote:
> + if (timeout <= gwdt->max_wor_timeout)
> + writel_relaxed(timeout * gwdt->clk,
> + gwdt->control_base + SBSA_GWDT_WOR);
> + else
> + writel_relaxed(gwdt->max_wor_timeout * gwdt->clk,
> + gwdt->control_base + SBSA_GWDT_WOR);
You pre-calculate the maximum timeout possible already, so why do you
need the if-statement?
Frankly, your non-pretimeout driver is almost identical to mine, which
was posted weeks ago. At this point, you're really just copying my
driver but putting your name on it.
--
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.
[email protected] wrote:
> This patch enables ACPI GTDT support for ARM SBSA
> watchdog driver automatically, if ACPI support is enabled.
You don't need this patch if you reorder your patches, like this:
#4 ACPI: add GTDT table parse driver into ACPI driver
#5 Watchdog: introduce ARM SBSA watchdog driver
And then this patch becomes part of patch #5.
--
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.
On 06/10/2015 12:47 PM, [email protected] wrote:
> + reg = <0x0 0xe0bb0000 0 0x10000>,
> + <0x0 0xe0bc0000 0 0x10000>;
I think the sizes are wrong. They should be 0x1000 instead of 0x10000.
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
On 13 June 2015 at 04:54, Timur Tabi <[email protected]> wrote:
> On 06/10/2015 12:47 PM, [email protected] wrote:
>>
>> + reg = <0x0 0xe0bb0000 0 0x10000>,
>> + <0x0 0xe0bc0000 0 0x10000>;
>
>
> I think the sizes are wrong. They should be 0x1000 instead of 0x10000.
This has been proved by test, it works well on Seattle
Foundation model has same value. So I don't think it is wrong
otherwise someone has the data sheet of Seattle B0, and it shows that is wrong.
>
> --
> 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
Hi Timur
On 12 June 2015 at 11:57, Timur Tabi <[email protected]> wrote:
> [email protected] wrote:
>>
>> + if (timeout <= gwdt->max_wor_timeout)
>> + writel_relaxed(timeout * gwdt->clk,
>> + gwdt->control_base + SBSA_GWDT_WOR);
>> + else
>> + writel_relaxed(gwdt->max_wor_timeout * gwdt->clk,
>> + gwdt->control_base + SBSA_GWDT_WOR);
>
>
> You pre-calculate the maximum timeout possible already, so why do you need
> the if-statement?
Have took Guenter's suggestion on this.
>
> Frankly, your non-pretimeout driver is almost identical to mine, which was
> posted weeks ago. At this point, you're really just copying my driver but
> putting your name on it.
Everyone can see how this driver become to this one gradually. For
non-pretimeout, if there is not pretimeout variable, I can only use
timeout to config both of them.
This is definitely not the copy of yours(check yours again, you never
programme WCV, and from the comment from you , you didn't believe
driver can do that. you use panic because of my patch), and I posted
my patchset(with pretimeout) to linaro-acpi list before you posted
yours to linux mailing list. And I always focus on mine.
Let people judge from all these patchset, I don't want to argue with
you on this any more.
>
>
> --
> 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
Fu Wei wrote:
>> >I think the sizes are wrong. They should be 0x1000 instead of 0x10000.
> This has been proved by test, it works well on Seattle
> Foundation model has same value. So I don't think it is wrong
>
> otherwise someone has the data sheet of Seattle B0, and it shows that is wrong.
The registers block is only 0x1000 bytes long. So you're wasting 63KB
of virtual address space for each block.
--
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.
On 06/14/2015 03:05 AM, Fu Wei wrote:
> On 13 June 2015 at 04:54, Timur Tabi <[email protected]> wrote:
>> On 06/10/2015 12:47 PM, [email protected] wrote:
>>>
>>> + reg = <0x0 0xe0bb0000 0 0x10000>,
>>> + <0x0 0xe0bc0000 0 0x10000>;
>>
>>
>> I think the sizes are wrong. They should be 0x1000 instead of 0x10000.
>
> This has been proved by test, it works well on Seattle
> Foundation model has same value. So I don't think it is wrong
>
> otherwise someone has the data sheet of Seattle B0, and it shows that is wrong.
>
If only 0x1000 is used, why would you have to reserve 0x10000 ?
You never access any higher addresses, so no matter what the datasheet
says, 0x1000 should be sufficient. What matters is what the driver uses.
Guenter
Hi Guenter,
On 14 June 2015 at 21:57, Guenter Roeck <[email protected]> wrote:
> On 06/14/2015 03:05 AM, Fu Wei wrote:
>>
>> On 13 June 2015 at 04:54, Timur Tabi <[email protected]> wrote:
>>>
>>> On 06/10/2015 12:47 PM, [email protected] wrote:
>>>>
>>>>
>>>> + reg = <0x0 0xe0bb0000 0 0x10000>,
>>>> + <0x0 0xe0bc0000 0 0x10000>;
>>>
>>>
>>>
>>> I think the sizes are wrong. They should be 0x1000 instead of 0x10000.
>>
>>
>> This has been proved by test, it works well on Seattle
>> Foundation model has same value. So I don't think it is wrong
>>
>> otherwise someone has the data sheet of Seattle B0, and it shows that is
>> wrong.
>>
>
> If only 0x1000 is used, why would you have to reserve 0x10000 ?
> You never access any higher addresses, so no matter what the datasheet
> says, 0x1000 should be sufficient. What matters is what the driver uses.
the reason for using 0x10000 before is :
(1)in <ARM v8-A Foundation Platform User Guide(Version: 9.1)> page 20
0x00_2A44_0000 0x00_2A44_FFFF - EL2 Generic Watchdog Control 64KB S/NS
0x00_2A45_0000 0x00_2A45_FFFF - EL2 Generic Watchdog Refresh 64KB S/NS
(2)It is also confirmed by Suravee, in Seattle, the size is 64KB also.
(3) these configuration has passed the test in Foundation model and Seattle B0.
Not sure the reason, maybe because of the 64KB page size.
But, yes, in SBSA, the device only has 4KB register area, and the
driver only need to use 4KB.
So you are right, "What matters is what the driver uses." , so will use 0x1000.
I think there is not problem, because ACPI uses this size.
>
> Guenter
>
>
--
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 Guenter,
On 12 June 2015 at 00:28, Guenter Roeck <[email protected]> wrote:
> On Thu, Jun 11, 2015 at 01:47:29AM +0800, [email protected] wrote:
>> From: Fu Wei <[email protected]>
>>
>> This driver bases on linux kernel watchdog framework.
>> It supports getting timeout from parameter and FDT
>> at the driver init stage.
>> The first timeout period expires, the interrupt routine
>> got another timeout period to run panic for saving
>> system context.
>>
> Comments inline.
>
> Thanks,
> Guenter
>
>> Signed-off-by: Fu Wei <[email protected]>
>> ---
>> drivers/watchdog/Kconfig | 11 ++
>> drivers/watchdog/Makefile | 1 +
>> drivers/watchdog/sbsa_gwdt.c | 383 +++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 395 insertions(+)
>> create mode 100644 drivers/watchdog/sbsa_gwdt.c
>>
>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>> index e5e7c55..554f18a 100644
>> --- a/drivers/watchdog/Kconfig
>> +++ b/drivers/watchdog/Kconfig
>> @@ -152,6 +152,17 @@ 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
>> + depends on ARM_ARCH_TIMER
>> + select WATCHDOG_CORE
>> + help
>> + ARM SBSA Generic Watchdog. This watchdog has two Watchdog timeouts.
>> + The first timeout will trigger a panic; the second timeout will
>> + trigger a system reset.
>> + More details: ARM DEN0029B - Server Base System Architecture (SBSA)
>> +
> To compile this driver as module, choose M here: The module
> will be called sbsa_gwdt.
Thanks! added it.
>
>> 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..1ddc10f
>> --- /dev/null
>> +++ b/drivers/watchdog/sbsa_gwdt.c
>> @@ -0,0 +1,383 @@
>> +/*
>> + * SBSA(Server Base System Architecture) Generic Watchdog driver
>> + *
>> + * Copyright (c) 2015, Linaro Ltd.
>> + * Author: Fu Wei <[email protected]>
>> + * Suravee Suthikulpanit <[email protected]>
>> + *
>> + * 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 has two stage timeouts,
>
> s/This/The/
>
> "has two stages".
>
> I would suggest to drop "Note:", but that is up to you.
Thanks :-) fixed it
>
>> + * When the first timeout occurs, WS0(SPI or LPI) is triggered,
>> + * the second timeout period(as long as the first timeout period) starts.
>
> no longer accurate if WOR is used for the second period.
>
>> + * In WS0 interrupt routine, panic() will be called for collecting
>> + * crashdown info.
>> + * If system can not recover from WS0 interrupt routine, then second
>> + * timeout occurs, WS1(reset or higher level interrupt) is triggered.
>> + * The two timeout period can be set by WOR(32bit).
>
> The second timeout period is determined by ...
>
>> + * WOR gives a maximum watch period of around 10s at the maximum
>> + * system counter frequency.
>> + * The System Counter shall run at maximum of 400MHz.
>
> "... at the maximum system counter frequency of 400 MHz.", and drop the
> last sentence.
For the second timeout period, I have discussed with a kdump developers,
(1)10s maybe not good enough for all the case of panic + kdump, so
maybe we still need to use WCV in the second timeout period
(2)in the second timeout period, maybe we need to programme WCV for
two reason: a, trigger WS1 to reboot system ASAP; b, feed the watchdog
without cleanning WS0 flag.
WHY we want to feed the watchdog (keepalive) without cleanning WS0 flag??
REASON:
(1)if the system context is large, we may need to feed the dog until
we get all the things backed up.
(2)if system goes wrong, WS0 triggered, then panic--> kdump. if we
feed the dog by WRR or programming WOR, WS0 flag will be cleaned. Once
system goes wrong again, then panic again.....
So this system will be in a panic--kdump--panic--kdump loop, have not
chance to reset.
So if we are in the second timeout period, we may need to always programme WCV.
>
> Please uses spaces before '('.
>
>> + *
>> + * But If we need a larger timeout period, this driver will programme WCV
>
> s/But //
> s/this/the/
> s/programme/program/
>
>> + * directly. That can support more than 10s timeout at the maximum
>> + * system counter frequency.
>
> Drop the last sentence.
Thanks , fixed it
>
>> + * More details: ARM DEN0029B - Server Base System Architecture (SBSA)
>> + *
>> + * SBSA GWDT: |---WOR(or WCV)---WS0---WOR(or WCV)---WS1
>> + * |-----timeout-----WS0-----timeout-----WS1
>> + */
>> +
>> +#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/uaccess.h>
>> +#include <linux/watchdog.h>
>> +#include <asm/arch_timer.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)
>> +
>> +/**
>> + * struct sbsa_gwdt - Internal representation of the SBSA GWDT
>> + * @wdd: kernel watchdog_device structure
>> + * @clk: store the System Counter clock frequency, in Hz.
>> + * @max_wor_timeout: the maximum timeout value for WOR (in seconds).
>> + * @refresh_base: Virtual address of the watchdog refresh frame
>> + * @control_base: Virtual address of the watchdog control frame
>> + */
>> +struct sbsa_gwdt {
>> + struct watchdog_device wdd;
>> + u32 clk;
>> + int max_wor_timeout;
>> + void __iomem *refresh_base;
>> + void __iomem *control_base;
>> +};
>> +
>> +#define to_sbsa_gwdt(e) container_of(e, struct sbsa_gwdt, wdd)
>> +
>> +#define DEFAULT_TIMEOUT 30 /* seconds */
>> +
>> +static unsigned int timeout;
>> +module_param(timeout, uint, 0);
>> +MODULE_PARM_DESC(timeout,
>> + "Watchdog timeout in seconds. (>=0, default="
>> + __MODULE_STRING(DEFAULT_TIMEOUT) ")");
>> +
>> +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) ")");
>> +
>> +/*
>> + * help functions for accessing 64bit WCV register
>> + */
>> +static u64 sbsa_gwdt_get_wcv(struct watchdog_device *wdd)
>> +{
>> + u32 wcv_lo, wcv_hi;
>> + struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
>> +
>> + do {
>> + wcv_hi = readl_relaxed(gwdt->control_base + SBSA_GWDT_WCV_HI);
>> + wcv_lo = readl_relaxed(gwdt->control_base + SBSA_GWDT_WCV_LO);
>> + } while (wcv_hi != readl_relaxed(gwdt->control_base +
>> + SBSA_GWDT_WCV_HI));
>> +
>> + return (((u64)wcv_hi << 32) | wcv_lo);
>> +}
>> +
>> +static void reload_timeout_to_wcv(struct watchdog_device *wdd)
>> +{
>> + struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
>> + u64 wcv;
>> +
>> + wcv = arch_counter_get_cntvct() + (u64)wdd->timeout * gwdt->clk;
>> +
>> + writel_relaxed(upper_32_bits(wcv),
>> + gwdt->control_base + SBSA_GWDT_WCV_HI);
>> + writel_relaxed(lower_32_bits(wcv),
>> + gwdt->control_base + SBSA_GWDT_WCV_LO);
>> +}
>> +
>> +static int sbsa_gwdt_set_timeout(struct watchdog_device *wdd,
>> + unsigned int timeout)
>> +{
>> + struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
>> +
>> + wdd->timeout = timeout;
>> +
>> + if (timeout <= gwdt->max_wor_timeout)
>> + writel_relaxed(timeout * gwdt->clk,
>> + gwdt->control_base + SBSA_GWDT_WOR);
>> + else
>> + writel_relaxed(gwdt->max_wor_timeout * gwdt->clk,
>> + gwdt->control_base + SBSA_GWDT_WOR);
>> +
>
> This can be simplified a bit to
> if (timeout > gwdt->max_wor_timeout)
> timeout = gwdt->max_wor_timeout;
> writel_relaxed(timeout * gwdt->clk,
> gwdt->control_base + SBSA_GWDT_WOR);
yes, good idea, thanks , fixed
>
>> + 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();
>> +
>> + do_div(timeleft, gwdt->clk);
>> +
>> + return timeleft;
>> +}
>> +
>> +static int sbsa_gwdt_keepalive(struct watchdog_device *wdd)
>> +{
>> + struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
>> +
>> + if (wdd->timeout <= gwdt->max_wor_timeout)
>> + /*
>> + * Writing WRR for an explicit watchdog refresh.
>> + * You can write anyting(like 0xc0ffee).
>> + */
>> + writel_relaxed(0xc0ffee, gwdt->refresh_base + SBSA_GWDT_WRR);
>> + else
>> + reload_timeout_to_wcv(wdd);
>> +
>> + return 0;
>> +}
>> +
>> +static int sbsa_gwdt_start(struct watchdog_device *wdd)
>> +{
>> + struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
>> + /* Force refresh due to hardware bug found in certain Soc. */
>
> Can you specify which SOC(s) are known to need this, and explain the bug
> a bit better ?
please ignore this, I have deleted it after discussing this with the
engineer of that chip vendor.
we don't need it now.
>
>> + writel_relaxed(0xc0ffee, gwdt->refresh_base + SBSA_GWDT_WRR);
>> + /* writing WCS will cause an explicit watchdog refresh */
>> + writel_relaxed(SBSA_GWDT_WCS_EN, gwdt->control_base + SBSA_GWDT_WCS);
>> +
>> + return sbsa_gwdt_keepalive(wdd);
>> +}
>> +
>> +static int sbsa_gwdt_stop(struct watchdog_device *wdd)
>> +{
>> + struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
>> +
>> + writel_relaxed(0, gwdt->control_base + SBSA_GWDT_WCS);
>> +
>> + return 0;
>> +}
>> +
>> +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;
>> +
>> + if (wdd->timeout > gwdt->max_wor_timeout)
>> + reload_timeout_to_wcv(wdd);
>> +
> Please drop the above.
as I mentioned above, I thinks we can keep this.
But please check my new patchset for this support
>
>> + 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_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,
>> + .get_timeleft = sbsa_gwdt_get_timeleft,
>> +};
>> +
>> +static int sbsa_gwdt_probe(struct platform_device *pdev)
>> +{
>> + u64 first_period_max = U64_MAX;
>> + struct device *dev = &pdev->dev;
>> + struct watchdog_device *wdd;
>> + struct sbsa_gwdt *gwdt;
>> + struct resource *res;
>> + void *rf_base, *cf_base;
>> + int ret, irq;
>> + u32 status;
>> +
>> + gwdt = devm_kzalloc(dev, sizeof(*gwdt), GFP_KERNEL);
>> + if (!gwdt)
>> + return -ENOMEM;
>> + platform_set_drvdata(pdev, gwdt);
>> +
>> + 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);
>> +
>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "control");
>> + cf_base = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(cf_base))
>> + return PTR_ERR(cf_base);
>> +
>> + irq = platform_get_irq_byname(pdev, "ws0");
>> + if (irq < 0) {
>> + dev_err(dev, "unable to get ws0 interrupt.\n");
>> + return irq;
>> + }
>> +
>> + /*
>> + * Get the frequency of system counter from the cp15 interface of ARM
>> + * Generic timer. We don't need to check it, because if it returns "0",
>> + * system would panic in very early stage.
>> + */
>> + gwdt->clk = arch_timer_get_cntfrq();
>> + gwdt->refresh_base = rf_base;
>> + gwdt->control_base = cf_base;
>> + gwdt->max_wor_timeout = U32_MAX / gwdt->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);
>> +
>> + wdd->min_timeout = 1;
>> + do_div(first_period_max, gwdt->clk);
>> + wdd->max_timeout = first_period_max;
>> +
>> + wdd->timeout = DEFAULT_TIMEOUT;
>> + watchdog_init_timeout(wdd, timeout, dev);
>> +
>> + 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));
>
> WCV here only tells us how many clock cycles were executed since the
> system started (or something like that). So I still don't understand
> why it is valuable to print that number.
this number provides the time of system reset, I thinks that may help
admin to analyse the system failure.
>
>> + 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);
>> + }
>
> Can you merge the message with the info message below ?
> Something like
> dev_info(dev, "Initialized with %ds timeout @ %u Hz%s\n", wdd->timeout,
> gwdt->clk, status & SBSA_GWDT_WCS_EN ? " [enabled]" : "");
>
> I don't think that should be a warning.
yes, good idea, will do
>
>> +
>> + /* update timeout to WOR */
>> + sbsa_gwdt_set_timeout(wdd, wdd->timeout);
>> +
>
> That will trigger a refresh if the watchdog is active, meaning the timeout
> will occur at time + WOR, not at time + timeout. I think keepalive has to be
> called later, preferrably after calling watchdog_register_device().
yes, you are right, will fix it
>
>> + ret = devm_request_irq(dev, irq, sbsa_gwdt_interrupt, 0,
>> + 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;
>> +
>> + dev_info(dev, "Initialized with %ds timeout @ %u Hz\n", wdd->timeout,
>> + 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);
>> +
>> + watchdog_unregister_device(&gwdt->wdd);
>> +
>> + return 0;
>> +}
>> +
>> +/* Disable watchdog if it is active during suspend */
>> +static int __maybe_unused sbsa_gwdt_suspend(struct device *dev)
>> +{
>> + struct sbsa_gwdt *gwdt = dev_get_drvdata(dev);
>> +
>> + if (watchdog_active(&gwdt->wdd))
>> + sbsa_gwdt_stop(&gwdt->wdd);
>> +
>> + return 0;
>> +}
>> +
>> +/* Enable watchdog and configure it if necessary */
>> +static int __maybe_unused sbsa_gwdt_resume(struct device *dev)
>> +{
>> + struct sbsa_gwdt *gwdt = dev_get_drvdata(dev);
>> +
>> + if (watchdog_active(&gwdt->wdd))
>> + sbsa_gwdt_start(&gwdt->wdd);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct dev_pm_ops sbsa_gwdt_pm_ops = {
>> + SET_SYSTEM_SLEEP_PM_OPS(sbsa_gwdt_suspend, sbsa_gwdt_resume)
>> +};
>> +
>> +static const struct of_device_id sbsa_gwdt_of_match[] = {
>> + { .compatible = "arm,sbsa-gwdt", },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, sbsa_gwdt_of_match);
>> +
>> +static const struct platform_device_id sbsa_gwdt_pdev_match[] = {
>> + { .name = "sbsa-gwdt", },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(platform, sbsa_gwdt_pdev_match);
>> +
>> +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,
>> + .id_table = sbsa_gwdt_pdev_match,
>> +};
>> +
>> +module_platform_driver(sbsa_gwdt_driver);
>> +
>> +MODULE_DESCRIPTION("SBSA Generic Watchdog Driver");
>> +MODULE_VERSION("v1.0");
>
> Version numbers tend to be out of date constantly, and there is no well
> defined mechanism or protocol when increase them. I would suggest to drop it.
Ok, will drop it
>
>> +MODULE_AUTHOR("Fu Wei <[email protected]>");
>> +MODULE_AUTHOR("Suravee Suthikulpanit <[email protected]>");
>> +MODULE_LICENSE("GPL v2");
--
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, Jun 23, 2015 at 09:26:35PM +0800, Fu Wei wrote:
> Hi Guenter,
[ ...]
> >
> >> + * When the first timeout occurs, WS0(SPI or LPI) is triggered,
> >> + * the second timeout period(as long as the first timeout period) starts.
> >
> > no longer accurate if WOR is used for the second period.
> >
> >> + * In WS0 interrupt routine, panic() will be called for collecting
> >> + * crashdown info.
> >> + * If system can not recover from WS0 interrupt routine, then second
> >> + * timeout occurs, WS1(reset or higher level interrupt) is triggered.
> >> + * The two timeout period can be set by WOR(32bit).
> >
> > The second timeout period is determined by ...
> >
> >> + * WOR gives a maximum watch period of around 10s at the maximum
> >> + * system counter frequency.
> >> + * The System Counter shall run at maximum of 400MHz.
> >
> > "... at the maximum system counter frequency of 400 MHz.", and drop the
> > last sentence.
>
> For the second timeout period, I have discussed with a kdump developers,
> (1)10s maybe not good enough for all the case of panic + kdump, so
> maybe we still need to use WCV in the second timeout period
> (2)in the second timeout period, maybe we need to programme WCV for
> two reason: a, trigger WS1 to reboot system ASAP; b, feed the watchdog
> without cleanning WS0 flag.
>
> WHY we want to feed the watchdog (keepalive) without cleanning WS0 flag??
> REASON:
> (1)if the system context is large, we may need to feed the dog until
> we get all the things backed up.
> (2)if system goes wrong, WS0 triggered, then panic--> kdump. if we
> feed the dog by WRR or programming WOR, WS0 flag will be cleaned. Once
> system goes wrong again, then panic again.....
> So this system will be in a panic--kdump--panic--kdump loop, have not
> chance to reset.
>
> So if we are in the second timeout period, we may need to always programme WCV.
>
The crashdump kernel is supposed to reload the watchdog driver, which will ping
the watchdog. If it isn't able to do that in 10 seconds, something is wrong.
> >> +
> >> + 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));
> >
> > WCV here only tells us how many clock cycles were executed since the
> > system started (or something like that). So I still don't understand
> > why it is valuable to print that number.
>
> this number provides the time of system reset, I thinks that may help
> admin to analyse the system failure.
>
It doesn't mean anything to anyone but you since it is not in a well defined
time scale. Also, I would be somewhat surprised if WCV would retain its value
on reset. Much more likely it is the time (in clock cycles) since reset.
Guenter
Hi Guenter,
you always can provide help very quickly, thank you very much :-)
On 23 June 2015 at 23:21, Guenter Roeck <[email protected]> wrote:
> On Tue, Jun 23, 2015 at 09:26:35PM +0800, Fu Wei wrote:
>> Hi Guenter,
> [ ...]
>
>> >
>> >> + * When the first timeout occurs, WS0(SPI or LPI) is triggered,
>> >> + * the second timeout period(as long as the first timeout period) starts.
>> >
>> > no longer accurate if WOR is used for the second period.
>> >
>> >> + * In WS0 interrupt routine, panic() will be called for collecting
>> >> + * crashdown info.
>> >> + * If system can not recover from WS0 interrupt routine, then second
>> >> + * timeout occurs, WS1(reset or higher level interrupt) is triggered.
>> >> + * The two timeout period can be set by WOR(32bit).
>> >
>> > The second timeout period is determined by ...
>> >
>> >> + * WOR gives a maximum watch period of around 10s at the maximum
>> >> + * system counter frequency.
>> >> + * The System Counter shall run at maximum of 400MHz.
>> >
>> > "... at the maximum system counter frequency of 400 MHz.", and drop the
>> > last sentence.
>>
>> For the second timeout period, I have discussed with a kdump developers,
>> (1)10s maybe not good enough for all the case of panic + kdump, so
>> maybe we still need to use WCV in the second timeout period
>> (2)in the second timeout period, maybe we need to programme WCV for
>> two reason: a, trigger WS1 to reboot system ASAP; b, feed the watchdog
>> without cleanning WS0 flag.
>>
>> WHY we want to feed the watchdog (keepalive) without cleanning WS0 flag??
>> REASON:
>> (1)if the system context is large, we may need to feed the dog until
>> we get all the things backed up.
>> (2)if system goes wrong, WS0 triggered, then panic--> kdump. if we
>> feed the dog by WRR or programming WOR, WS0 flag will be cleaned. Once
>> system goes wrong again, then panic again.....
>> So this system will be in a panic--kdump--panic--kdump loop, have not
>> chance to reset.
>>
>> So if we are in the second timeout period, we may need to always programme WCV.
>>
> The crashdump kernel is supposed to reload the watchdog driver, which will ping
> the watchdog. If it isn't able to do that in 10 seconds, something is wrong.
yes, 10s maybe not enough for all case.
When I tested kdump on arm64, sometimes , it took 20s. So I am
thinking : can we make the max value of pretimeout > 10s in this
driver.
>
>> >> +
>> >> + 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));
>> >
>> > WCV here only tells us how many clock cycles were executed since the
>> > system started (or something like that). So I still don't understand
>> > why it is valuable to print that number.
>>
>> this number provides the time of system reset, I thinks that may help
>> admin to analyse the system failure.
>>
> It doesn't mean anything to anyone but you since it is not in a well defined
> time scale.
maybe I should convert it to second?
I think the original value is better?
> Also, I would be somewhat surprised if WCV would retain its value
> on reset. Much more likely it is the time (in clock cycles) since reset.
yes, It has been mentioned in SBSA:
---------------------
If WS0 is asserted and a timeout refresh occurs then the following must occur:
If the system is compliant to SBSA level 0 or level 1 then it is
IMPLEMENTATION DEFINED as to whether the
compare value is loaded with the sum of the zero-extended watchdog
offset register and the current
generic timer system count value, or whether it retains its current value.
If the system is compliant to SBSA level 2 or higher the compare
value must retain its current value. This
means that the compare value records the time that WS1 is asserted.
---------------------
Hope I understand it correctly. please let me know , if I
misunderstand something, thanks
>
> Guenter
--
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 Wed, Jun 24, 2015 at 12:17:19AM +0800, Fu Wei wrote:
> Hi Guenter,
>
> you always can provide help very quickly, thank you very much :-)
>
> On 23 June 2015 at 23:21, Guenter Roeck <[email protected]> wrote:
> > On Tue, Jun 23, 2015 at 09:26:35PM +0800, Fu Wei wrote:
> >> Hi Guenter,
> > [ ...]
> >
> >> >
> >> >> + * When the first timeout occurs, WS0(SPI or LPI) is triggered,
> >> >> + * the second timeout period(as long as the first timeout period) starts.
> >> >
> >> > no longer accurate if WOR is used for the second period.
> >> >
> >> >> + * In WS0 interrupt routine, panic() will be called for collecting
> >> >> + * crashdown info.
> >> >> + * If system can not recover from WS0 interrupt routine, then second
> >> >> + * timeout occurs, WS1(reset or higher level interrupt) is triggered.
> >> >> + * The two timeout period can be set by WOR(32bit).
> >> >
> >> > The second timeout period is determined by ...
> >> >
> >> >> + * WOR gives a maximum watch period of around 10s at the maximum
> >> >> + * system counter frequency.
> >> >> + * The System Counter shall run at maximum of 400MHz.
> >> >
> >> > "... at the maximum system counter frequency of 400 MHz.", and drop the
> >> > last sentence.
> >>
> >> For the second timeout period, I have discussed with a kdump developers,
> >> (1)10s maybe not good enough for all the case of panic + kdump, so
> >> maybe we still need to use WCV in the second timeout period
> >> (2)in the second timeout period, maybe we need to programme WCV for
> >> two reason: a, trigger WS1 to reboot system ASAP; b, feed the watchdog
> >> without cleanning WS0 flag.
> >>
> >> WHY we want to feed the watchdog (keepalive) without cleanning WS0 flag??
> >> REASON:
> >> (1)if the system context is large, we may need to feed the dog until
> >> we get all the things backed up.
> >> (2)if system goes wrong, WS0 triggered, then panic--> kdump. if we
> >> feed the dog by WRR or programming WOR, WS0 flag will be cleaned. Once
> >> system goes wrong again, then panic again.....
> >> So this system will be in a panic--kdump--panic--kdump loop, have not
> >> chance to reset.
> >>
> >> So if we are in the second timeout period, we may need to always programme WCV.
> >>
> > The crashdump kernel is supposed to reload the watchdog driver, which will ping
> > the watchdog. If it isn't able to do that in 10 seconds, something is wrong.
>
> yes, 10s maybe not enough for all case.
> When I tested kdump on arm64, sometimes , it took 20s. So I am
> thinking : can we make the max value of pretimeout > 10s in this
> driver.
>
It takes more than 10 seconds to load the crashdump kernel,
or it takes more than 10 seconds to complete the dump ?
>
> >
> >> >> +
> >> >> + 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));
> >> >
> >> > WCV here only tells us how many clock cycles were executed since the
> >> > system started (or something like that). So I still don't understand
> >> > why it is valuable to print that number.
> >>
> >> this number provides the time of system reset, I thinks that may help
> >> admin to analyse the system failure.
> >>
> > It doesn't mean anything to anyone but you since it is not in a well defined
> > time scale.
>
> maybe I should convert it to second?
> I think the original value is better?
>
I think you should drop it.
Guenter
Hi Guenter
On 24 June 2015 at 00:43, Guenter Roeck <[email protected]> wrote:
> On Wed, Jun 24, 2015 at 12:17:19AM +0800, Fu Wei wrote:
>> Hi Guenter,
>>
>> you always can provide help very quickly, thank you very much :-)
>>
>> On 23 June 2015 at 23:21, Guenter Roeck <[email protected]> wrote:
>> > On Tue, Jun 23, 2015 at 09:26:35PM +0800, Fu Wei wrote:
>> >> Hi Guenter,
>> > [ ...]
>> >
>> >> >
>> >> >> + * When the first timeout occurs, WS0(SPI or LPI) is triggered,
>> >> >> + * the second timeout period(as long as the first timeout period) starts.
>> >> >
>> >> > no longer accurate if WOR is used for the second period.
>> >> >
>> >> >> + * In WS0 interrupt routine, panic() will be called for collecting
>> >> >> + * crashdown info.
>> >> >> + * If system can not recover from WS0 interrupt routine, then second
>> >> >> + * timeout occurs, WS1(reset or higher level interrupt) is triggered.
>> >> >> + * The two timeout period can be set by WOR(32bit).
>> >> >
>> >> > The second timeout period is determined by ...
>> >> >
>> >> >> + * WOR gives a maximum watch period of around 10s at the maximum
>> >> >> + * system counter frequency.
>> >> >> + * The System Counter shall run at maximum of 400MHz.
>> >> >
>> >> > "... at the maximum system counter frequency of 400 MHz.", and drop the
>> >> > last sentence.
>> >>
>> >> For the second timeout period, I have discussed with a kdump developers,
>> >> (1)10s maybe not good enough for all the case of panic + kdump, so
>> >> maybe we still need to use WCV in the second timeout period
>> >> (2)in the second timeout period, maybe we need to programme WCV for
>> >> two reason: a, trigger WS1 to reboot system ASAP; b, feed the watchdog
>> >> without cleanning WS0 flag.
>> >>
>> >> WHY we want to feed the watchdog (keepalive) without cleanning WS0 flag??
>> >> REASON:
>> >> (1)if the system context is large, we may need to feed the dog until
>> >> we get all the things backed up.
>> >> (2)if system goes wrong, WS0 triggered, then panic--> kdump. if we
>> >> feed the dog by WRR or programming WOR, WS0 flag will be cleaned. Once
>> >> system goes wrong again, then panic again.....
>> >> So this system will be in a panic--kdump--panic--kdump loop, have not
>> >> chance to reset.
>> >>
>> >> So if we are in the second timeout period, we may need to always programme WCV.
>> >>
>> > The crashdump kernel is supposed to reload the watchdog driver, which will ping
>> > the watchdog. If it isn't able to do that in 10 seconds, something is wrong.
>>
>> yes, 10s maybe not enough for all case.
>> When I tested kdump on arm64, sometimes , it took 20s. So I am
>> thinking : can we make the max value of pretimeout > 10s in this
>> driver.
>>
> It takes more than 10 seconds to load the crashdump kernel,
> or it takes more than 10 seconds to complete the dump ?
It takes more than 10 seconds to boot into kernel(from panic to finish
devices init in crashdump kernel).
I thinks that maybe depend on hardware or soc.
As I said, 10 seconds maybe not enough for all cases.
For completing the dump, 10 seconds maybe not enough for some case(big
RAM, dump to network and so on),
that is why I added "ping without cleaning WS0" support in the second stage.
>
>>
>> >
>> >> >> +
>> >> >> + 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));
>> >> >
>> >> > WCV here only tells us how many clock cycles were executed since the
>> >> > system started (or something like that). So I still don't understand
>> >> > why it is valuable to print that number.
>> >>
>> >> this number provides the time of system reset, I thinks that may help
>> >> admin to analyse the system failure.
>> >>
>> > It doesn't mean anything to anyone but you since it is not in a well defined
>> > time scale.
>>
>> maybe I should convert it to second?
>> I think the original value is better?
>>
>
> I think you should drop it.
OK, will do in my next patchset.
But my option is if hardware provide this info, and it can let admin
know the crash time. maybe it can help to debug.
:-)
>
> Guenter
--
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