All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v12 0/4] Watchdog: introduce ARM SBSA watchdog driver
@ 2016-02-16  8:36 ` fu.wei at linaro.org
  0 siblings, 0 replies; 32+ messages in thread
From: fu.wei @ 2016-02-16  8:36 UTC (permalink / raw)
  To: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, wim,
	linux, corbet, catalin.marinas, will.deacon,
	Suravee.Suthikulpanit
  Cc: linux-kernel, linux-watchdog, linux-doc, devicetree,
	linux-arm-kernel, linaro-acpi, rruigrok, harba, cov, timur,
	dyoung, panand, graeme.gregory, al.stone, hanjun.guo, jcm, arnd,
	leo.duran, sudeep.holla, Fu Wei

From: Fu Wei <fu.wei@linaro.org>

This patchset:
    (1)Introduce Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt
    for FDT info of SBSA Generic Watchdog, and give two examples of
    adding SBSA Generic Watchdog device node into the dts files:
    foundation-v8.dts and amd-seattle-soc.dtsi.

    (2)Introduce ARM SBSA watchdog driver:
        a.Use linux kernel watchdog framework;
        b.Work with FDT on ARM64;
        c.Support getting timeout from parameter and FDT at the driver
          init stage.
        d.The driver works in two modes:
          (1) single stage timeout (ignore WS0 interrupt)
          (2) two stages timeout (register WS0 interrupt, do panic in routine)
        e.User can config working mode by module parameter "action".

This patchset has been tested with watchdog daemon
(ACPI/FDT, module/build-in) on the following platforms:
    (1)ARM Foundation v8 model
    (2)AMD Seattle platform

This patchset has been tested with kdump successfully.

Changelog:
v12:Fix a dev_warn message typo
    Remove unnecessary "status" in dts
    Add more *ed-by in commit message.

v11:https://lkml.org/lkml/2016/2/9/577
    Merge patch 4 and 5.
    Improve some comments.
    The driver works in two modes, it's configured by "action"(instead of
    panic_enabled).
    Improve the initialization of the timeout limits.
    Feeding dog by writing "0" to WRR.

v10:https://lkml.org/lkml/2016/2/3/817
    Delete pretimeout support.
    Separate the driver to two parts:
        (1) single stage timeout driver(ignore WS0 interrupt);
        (2) register WS0 interrupt for the half timeout panic.
    timeout == (enable --> WS1).

v9: https://lkml.org/lkml/2015/11/9/57
    Rebase to latest kernel version(4.3).
    Update the Documentation of sbsa-gwdt device node info of FDT:
        (1) move some introduction to pretimeout patch
        (2) delete WS1 value from "interrupts" of binding documentation,
            since WS1 won't be handled by Linux.

v8: https://lkml.org/lkml/2015/10/27/466
    Rebase to latest kernel version(4.3-rc7).
    Separate the patches of GTDT support and arm_arch_timer. This
    clocksource relevant patch will upstreamed in a individual patchset.
    Update all the default timeout and pretimeout to 30s and 60s.
    Improve documentation and inline comments.
    Fix a bug in pretimeout support which makes timeout and pretimeout
    parameters initialization fail.

v7: https://lkml.org/lkml/2015/8/24/611
    Rebase to latest kernel version(4.2-rc7).
    Improve FDT support: geting resource by order, instead of name.
    According to the FDT support, Update the example dts file, gtdt.c
    and sbsa_gwdt.c.
    Pass the sparse test, and fix the warning.
    Fix the max_pretimeout and max_timeout value overflow bug.
    Delete the WCV output value.
    

v6: https://lkml.org/lkml/2015/6/23/359
    Improve the dtb example files: reduce the register frame size to 4K.
    Improve pretimeout support:
        (1) improve watchdog_init_timeouts function
	(2) rename watchdog_check_min_max_timeouts back to the original name
        (1) improve watchdog_timeout_invalid/watchdog_pretimeout_invalid
    Add the new features in the sbsa_gwdt driver:
	(1) In the second stage, user can feed the dog without cleaning WS0.
	(2) In the second stage, user can trigger WS1 by setting pretimeout = 0.
	(3) expand the max value of pretimeout, in case 10 second is not enough
	    for a kdump kernel reboot in panic.

v5: https://lkml.org/lkml/2015/6/10/357
    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: https://lkml.org/lkml/2015/6/2/4
    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: https://lkml.org/lkml/2015/5/25/111
    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: https://lkml.org/lkml/2015/5/21/172
    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: https://lkml.org/lkml/2015/5/15/279
    The first version upstream patchset to linux mailing list.

Fu Wei (4):
  Documentation: add sbsa-gwdt driver 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

 .../devicetree/bindings/watchdog/sbsa-gwdt.txt     |  31 ++
 Documentation/watchdog/watchdog-parameters.txt     |   7 +
 arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi       |   8 +
 arch/arm64/boot/dts/arm/foundation-v8.dts          |   7 +
 drivers/watchdog/Kconfig                           |  20 +
 drivers/watchdog/Makefile                          |   1 +
 drivers/watchdog/sbsa_gwdt.c                       | 403 +++++++++++++++++++++
 7 files changed, 477 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt
 create mode 100644 drivers/watchdog/sbsa_gwdt.c

-- 
2.5.0

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v12 0/4] Watchdog: introduce ARM SBSA watchdog driver
@ 2016-02-16  8:36 ` fu.wei at linaro.org
  0 siblings, 0 replies; 32+ messages in thread
From: fu.wei at linaro.org @ 2016-02-16  8:36 UTC (permalink / raw)
  To: linux-arm-kernel

From: Fu Wei <fu.wei@linaro.org>

This patchset:
    (1)Introduce Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt
    for FDT info of SBSA Generic Watchdog, and give two examples of
    adding SBSA Generic Watchdog device node into the dts files:
    foundation-v8.dts and amd-seattle-soc.dtsi.

    (2)Introduce ARM SBSA watchdog driver:
        a.Use linux kernel watchdog framework;
        b.Work with FDT on ARM64;
        c.Support getting timeout from parameter and FDT at the driver
          init stage.
        d.The driver works in two modes:
          (1) single stage timeout (ignore WS0 interrupt)
          (2) two stages timeout (register WS0 interrupt, do panic in routine)
        e.User can config working mode by module parameter "action".

This patchset has been tested with watchdog daemon
(ACPI/FDT, module/build-in) on the following platforms:
    (1)ARM Foundation v8 model
    (2)AMD Seattle platform

This patchset has been tested with kdump successfully.

Changelog:
v12:Fix a dev_warn message typo
    Remove unnecessary "status" in dts
    Add more *ed-by in commit message.

v11:https://lkml.org/lkml/2016/2/9/577
    Merge patch 4 and 5.
    Improve some comments.
    The driver works in two modes, it's configured by "action"(instead of
    panic_enabled).
    Improve the initialization of the timeout limits.
    Feeding dog by writing "0" to WRR.

v10:https://lkml.org/lkml/2016/2/3/817
    Delete pretimeout support.
    Separate the driver to two parts:
        (1) single stage timeout driver(ignore WS0 interrupt);
        (2) register WS0 interrupt for the half timeout panic.
    timeout == (enable --> WS1).

v9: https://lkml.org/lkml/2015/11/9/57
    Rebase to latest kernel version(4.3).
    Update the Documentation of sbsa-gwdt device node info of FDT:
        (1) move some introduction to pretimeout patch
        (2) delete WS1 value from "interrupts" of binding documentation,
            since WS1 won't be handled by Linux.

v8: https://lkml.org/lkml/2015/10/27/466
    Rebase to latest kernel version(4.3-rc7).
    Separate the patches of GTDT support and arm_arch_timer. This
    clocksource relevant patch will upstreamed in a individual patchset.
    Update all the default timeout and pretimeout to 30s and 60s.
    Improve documentation and inline comments.
    Fix a bug in pretimeout support which makes timeout and pretimeout
    parameters initialization fail.

v7: https://lkml.org/lkml/2015/8/24/611
    Rebase to latest kernel version(4.2-rc7).
    Improve FDT support: geting resource by order, instead of name.
    According to the FDT support, Update the example dts file, gtdt.c
    and sbsa_gwdt.c.
    Pass the sparse test, and fix the warning.
    Fix the max_pretimeout and max_timeout value overflow bug.
    Delete the WCV output value.
    

v6: https://lkml.org/lkml/2015/6/23/359
    Improve the dtb example files: reduce the register frame size to 4K.
    Improve pretimeout support:
        (1) improve watchdog_init_timeouts function
	(2) rename watchdog_check_min_max_timeouts back to the original name
        (1) improve watchdog_timeout_invalid/watchdog_pretimeout_invalid
    Add the new features in the sbsa_gwdt driver:
	(1) In the second stage, user can feed the dog without cleaning WS0.
	(2) In the second stage, user can trigger WS1 by setting pretimeout = 0.
	(3) expand the max value of pretimeout, in case 10 second is not enough
	    for a kdump kernel reboot in panic.

v5: https://lkml.org/lkml/2015/6/10/357
    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: https://lkml.org/lkml/2015/6/2/4
    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: https://lkml.org/lkml/2015/5/25/111
    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: https://lkml.org/lkml/2015/5/21/172
    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: https://lkml.org/lkml/2015/5/15/279
    The first version upstream patchset to linux mailing list.

Fu Wei (4):
  Documentation: add sbsa-gwdt driver 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

 .../devicetree/bindings/watchdog/sbsa-gwdt.txt     |  31 ++
 Documentation/watchdog/watchdog-parameters.txt     |   7 +
 arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi       |   8 +
 arch/arm64/boot/dts/arm/foundation-v8.dts          |   7 +
 drivers/watchdog/Kconfig                           |  20 +
 drivers/watchdog/Makefile                          |   1 +
 drivers/watchdog/sbsa_gwdt.c                       | 403 +++++++++++++++++++++
 7 files changed, 477 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt
 create mode 100644 drivers/watchdog/sbsa_gwdt.c

-- 
2.5.0

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v12 1/4] Documentation: add sbsa-gwdt driver documentation
  2016-02-16  8:36 ` fu.wei at linaro.org
@ 2016-02-16  8:36   ` fu.wei at linaro.org
  -1 siblings, 0 replies; 32+ messages in thread
From: fu.wei @ 2016-02-16  8:36 UTC (permalink / raw)
  To: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, wim,
	linux, corbet, catalin.marinas, will.deacon,
	Suravee.Suthikulpanit
  Cc: linux-kernel, linux-watchdog, linux-doc, devicetree,
	linux-arm-kernel, linaro-acpi, rruigrok, harba, cov, timur,
	dyoung, panand, graeme.gregory, al.stone, hanjun.guo, jcm, arnd,
	leo.duran, sudeep.holla, Fu Wei

From: Fu Wei <fu.wei@linaro.org>

The sbsa-gwdt.txt documentation in devicetree/bindings/watchdog is for
introducing SBSA(Server Base System Architecture) Generic Watchdog
device node info into FDT.

Also add sbsa-gwdt introduction in watchdog-parameters.txt

Acked-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Fu Wei <fu.wei@linaro.org>
---
 .../devicetree/bindings/watchdog/sbsa-gwdt.txt     | 31 ++++++++++++++++++++++
 Documentation/watchdog/watchdog-parameters.txt     |  7 +++++
 2 files changed, 38 insertions(+)

diff --git a/Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt b/Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt
new file mode 100644
index 0000000..6f2d5f9
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt
@@ -0,0 +1,31 @@
+* SBSA (Server Base System Architecture) Generic Watchdog
+
+The SBSA Generic Watchdog Timer is used to force a reset of the system
+after two stages of timeout have elapsed.  A detailed definition of the
+watchdog timer can be found in the ARM document: ARM-DEN-0029 - Server
+Base System Architecture (SBSA)
+
+Required properties:
+- compatible: Should at least contain "arm,sbsa-gwdt".
+
+- reg: Each entry specifies the base physical address of a register frame
+  and the length of that frame; currently, two frames must be defined,
+  in this order:
+  1: Watchdog control frame;
+  2: Refresh frame.
+
+- interrupts: Should contain the Watchdog Signal 0 (WS0) SPI (Shared
+  Peripheral Interrupt) number of SBSA Generic Watchdog.
+
+Optional properties
+- timeout-sec: Watchdog timeout values (in seconds).
+
+Example for FVP Foundation Model v8:
+
+watchdog@2a440000 {
+	compatible = "arm,sbsa-gwdt";
+	reg = <0x0 0x2a440000 0 0x1000>,
+	      <0x0 0x2a450000 0 0x1000>;
+	interrupts = <0 27 4>;
+	timeout-sec = <30>;
+};
diff --git a/Documentation/watchdog/watchdog-parameters.txt b/Documentation/watchdog/watchdog-parameters.txt
index 9f9ec9f..ce7ae4e 100644
--- a/Documentation/watchdog/watchdog-parameters.txt
+++ b/Documentation/watchdog/watchdog-parameters.txt
@@ -284,6 +284,13 @@ sbc_fitpc2_wdt:
 margin: Watchdog margin in seconds (default 60s)
 nowayout: Watchdog cannot be stopped once started
 -------------------------------------------------
+sbsa_gwdt:
+timeout: Watchdog timeout in seconds. (default 10s)
+action: Watchdog action at the first stage timeout,
+	set to 0 to ignore, 1 to panic. (default=0)
+nowayout: Watchdog cannot be stopped once started
+	(default=kernel config parameter)
+-------------------------------------------------
 sc1200wdt:
 isapnp: When set to 0 driver ISA PnP support will be disabled (default=1)
 io: io port
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v12 1/4] Documentation: add sbsa-gwdt driver documentation
@ 2016-02-16  8:36   ` fu.wei at linaro.org
  0 siblings, 0 replies; 32+ messages in thread
From: fu.wei at linaro.org @ 2016-02-16  8:36 UTC (permalink / raw)
  To: linux-arm-kernel

From: Fu Wei <fu.wei@linaro.org>

The sbsa-gwdt.txt documentation in devicetree/bindings/watchdog is for
introducing SBSA(Server Base System Architecture) Generic Watchdog
device node info into FDT.

Also add sbsa-gwdt introduction in watchdog-parameters.txt

Acked-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Fu Wei <fu.wei@linaro.org>
---
 .../devicetree/bindings/watchdog/sbsa-gwdt.txt     | 31 ++++++++++++++++++++++
 Documentation/watchdog/watchdog-parameters.txt     |  7 +++++
 2 files changed, 38 insertions(+)

diff --git a/Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt b/Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt
new file mode 100644
index 0000000..6f2d5f9
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt
@@ -0,0 +1,31 @@
+* SBSA (Server Base System Architecture) Generic Watchdog
+
+The SBSA Generic Watchdog Timer is used to force a reset of the system
+after two stages of timeout have elapsed.  A detailed definition of the
+watchdog timer can be found in the ARM document: ARM-DEN-0029 - Server
+Base System Architecture (SBSA)
+
+Required properties:
+- compatible: Should at least contain "arm,sbsa-gwdt".
+
+- reg: Each entry specifies the base physical address of a register frame
+  and the length of that frame; currently, two frames must be defined,
+  in this order:
+  1: Watchdog control frame;
+  2: Refresh frame.
+
+- interrupts: Should contain the Watchdog Signal 0 (WS0) SPI (Shared
+  Peripheral Interrupt) number of SBSA Generic Watchdog.
+
+Optional properties
+- timeout-sec: Watchdog timeout values (in seconds).
+
+Example for FVP Foundation Model v8:
+
+watchdog at 2a440000 {
+	compatible = "arm,sbsa-gwdt";
+	reg = <0x0 0x2a440000 0 0x1000>,
+	      <0x0 0x2a450000 0 0x1000>;
+	interrupts = <0 27 4>;
+	timeout-sec = <30>;
+};
diff --git a/Documentation/watchdog/watchdog-parameters.txt b/Documentation/watchdog/watchdog-parameters.txt
index 9f9ec9f..ce7ae4e 100644
--- a/Documentation/watchdog/watchdog-parameters.txt
+++ b/Documentation/watchdog/watchdog-parameters.txt
@@ -284,6 +284,13 @@ sbc_fitpc2_wdt:
 margin: Watchdog margin in seconds (default 60s)
 nowayout: Watchdog cannot be stopped once started
 -------------------------------------------------
+sbsa_gwdt:
+timeout: Watchdog timeout in seconds. (default 10s)
+action: Watchdog action at the first stage timeout,
+	set to 0 to ignore, 1 to panic. (default=0)
+nowayout: Watchdog cannot be stopped once started
+	(default=kernel config parameter)
+-------------------------------------------------
 sc1200wdt:
 isapnp: When set to 0 driver ISA PnP support will be disabled (default=1)
 io: io port
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v12 2/4] ARM64: add SBSA Generic Watchdog device node in foundation-v8.dts
  2016-02-16  8:36 ` fu.wei at linaro.org
@ 2016-02-16  8:36   ` fu.wei at linaro.org
  -1 siblings, 0 replies; 32+ messages in thread
From: fu.wei @ 2016-02-16  8:36 UTC (permalink / raw)
  To: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, wim,
	linux, corbet, catalin.marinas, will.deacon,
	Suravee.Suthikulpanit
  Cc: linux-kernel, linux-watchdog, linux-doc, devicetree,
	linux-arm-kernel, linaro-acpi, rruigrok, harba, cov, timur,
	dyoung, panand, graeme.gregory, al.stone, hanjun.guo, jcm, arnd,
	leo.duran, sudeep.holla, Fu Wei

From: Fu Wei <fu.wei@linaro.org>

This can be a example of adding SBSA Generic Watchdog device node
into some dts files for the Soc which contains SBSA Generic Watchdog.

Acked-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Fu Wei <fu.wei@linaro.org>
---
 arch/arm64/boot/dts/arm/foundation-v8.dts | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/arm/foundation-v8.dts b/arch/arm64/boot/dts/arm/foundation-v8.dts
index 4eac8dc..66cb9aa 100644
--- a/arch/arm64/boot/dts/arm/foundation-v8.dts
+++ b/arch/arm64/boot/dts/arm/foundation-v8.dts
@@ -237,4 +237,11 @@
 			};
 		};
 	};
+	watchdog@2a440000 {
+		compatible = "arm,sbsa-gwdt";
+		reg = <0x0 0x2a440000 0 0x1000>,
+			<0x0 0x2a450000 0 0x1000>;
+		interrupts = <0 27 4>;
+		timeout-sec = <30>;
+	};
 };
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v12 2/4] ARM64: add SBSA Generic Watchdog device node in foundation-v8.dts
@ 2016-02-16  8:36   ` fu.wei at linaro.org
  0 siblings, 0 replies; 32+ messages in thread
From: fu.wei at linaro.org @ 2016-02-16  8:36 UTC (permalink / raw)
  To: linux-arm-kernel

From: Fu Wei <fu.wei@linaro.org>

This can be a example of adding SBSA Generic Watchdog device node
into some dts files for the Soc which contains SBSA Generic Watchdog.

Acked-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Fu Wei <fu.wei@linaro.org>
---
 arch/arm64/boot/dts/arm/foundation-v8.dts | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/arm/foundation-v8.dts b/arch/arm64/boot/dts/arm/foundation-v8.dts
index 4eac8dc..66cb9aa 100644
--- a/arch/arm64/boot/dts/arm/foundation-v8.dts
+++ b/arch/arm64/boot/dts/arm/foundation-v8.dts
@@ -237,4 +237,11 @@
 			};
 		};
 	};
+	watchdog at 2a440000 {
+		compatible = "arm,sbsa-gwdt";
+		reg = <0x0 0x2a440000 0 0x1000>,
+			<0x0 0x2a450000 0 0x1000>;
+		interrupts = <0 27 4>;
+		timeout-sec = <30>;
+	};
 };
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v12 3/4] ARM64: add SBSA Generic Watchdog device node in amd-seattle-soc.dtsi
  2016-02-16  8:36 ` fu.wei at linaro.org
@ 2016-02-16  8:36   ` fu.wei at linaro.org
  -1 siblings, 0 replies; 32+ messages in thread
From: fu.wei @ 2016-02-16  8:36 UTC (permalink / raw)
  To: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, wim,
	linux, corbet, catalin.marinas, will.deacon,
	Suravee.Suthikulpanit
  Cc: linux-kernel, linux-watchdog, linux-doc, devicetree,
	linux-arm-kernel, linaro-acpi, rruigrok, harba, cov, timur,
	dyoung, panand, graeme.gregory, al.stone, hanjun.guo, jcm, arnd,
	leo.duran, sudeep.holla, Fu Wei

From: Fu Wei <fu.wei@linaro.org>

This can be a example of adding SBSA Generic Watchdog device node
into some dts files for the Soc which contains SBSA Generic Watchdog.

Acked-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
Signed-off-by: Fu Wei <fu.wei@linaro.org>
---
 arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi | 8 ++++++++
 1 file changed, 8 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..0a8ca1d 100644
--- a/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi
+++ b/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi
@@ -84,6 +84,14 @@
 			clock-names = "uartclk", "apb_pclk";
 		};
 
+		watchdog0: watchdog@e0bb0000 {
+			compatible = "arm,sbsa-gwdt";
+			reg = <0x0 0xe0bc0000 0 0x1000>,
+				<0x0 0xe0bb0000 0 0x1000>;
+			interrupts = <0 337 4>;
+			timeout-sec = <15>;
+		};
+
 		spi0: ssp@e1020000 {
 			status = "disabled";
 			compatible = "arm,pl022", "arm,primecell";
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v12 3/4] ARM64: add SBSA Generic Watchdog device node in amd-seattle-soc.dtsi
@ 2016-02-16  8:36   ` fu.wei at linaro.org
  0 siblings, 0 replies; 32+ messages in thread
From: fu.wei at linaro.org @ 2016-02-16  8:36 UTC (permalink / raw)
  To: linux-arm-kernel

From: Fu Wei <fu.wei@linaro.org>

This can be a example of adding SBSA Generic Watchdog device node
into some dts files for the Soc which contains SBSA Generic Watchdog.

Acked-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
Signed-off-by: Fu Wei <fu.wei@linaro.org>
---
 arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi | 8 ++++++++
 1 file changed, 8 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..0a8ca1d 100644
--- a/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi
+++ b/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi
@@ -84,6 +84,14 @@
 			clock-names = "uartclk", "apb_pclk";
 		};
 
+		watchdog0: watchdog at e0bb0000 {
+			compatible = "arm,sbsa-gwdt";
+			reg = <0x0 0xe0bc0000 0 0x1000>,
+				<0x0 0xe0bb0000 0 0x1000>;
+			interrupts = <0 337 4>;
+			timeout-sec = <15>;
+		};
+
 		spi0: ssp at e1020000 {
 			status = "disabled";
 			compatible = "arm,pl022", "arm,primecell";
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v12 4/4] Watchdog: introduce ARM SBSA watchdog driver
  2016-02-16  8:36 ` fu.wei at linaro.org
@ 2016-02-16  8:36   ` fu.wei at linaro.org
  -1 siblings, 0 replies; 32+ messages in thread
From: fu.wei @ 2016-02-16  8:36 UTC (permalink / raw)
  To: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, wim,
	linux, corbet, catalin.marinas, will.deacon,
	Suravee.Suthikulpanit
  Cc: linux-kernel, linux-watchdog, linux-doc, devicetree,
	linux-arm-kernel, linaro-acpi, rruigrok, harba, cov, timur,
	dyoung, panand, graeme.gregory, al.stone, hanjun.guo, jcm, arnd,
	leo.duran, sudeep.holla, Fu Wei

From: Fu Wei <fu.wei@linaro.org>

According to Server Base System Architecture (SBSA) specification,
the SBSA Generic Watchdog has two stage timeouts: the first signal (WS0)
is for alerting the system by interrupt, the second one (WS1) is a real
hardware reset.
More details about the hardware specification of this device:
ARM DEN0029B - Server Base System Architecture (SBSA)

This driver can operate ARM SBSA Generic Watchdog as a single stage watchdog
or a two stages watchdog, it's set up by the module parameter "action".
In the single stage mode, when the timeout is reached, your system
will be reset by WS1. The first signal (WS0) is ignored.
In the two stages mode, when the timeout is reached, the first signal (WS0)
will trigger panic. If the system is getting into trouble and cannot be reset
by panic or restart properly by the kdump kernel(if supported), then the
second stage (as long as the first stage) will be reached, system will be
reset by WS1. This function can help administrator to backup the system
context info by panic console output or kdump.

This driver bases on linux kernel watchdog framework, so it can get
timeout from module parameter and FDT at the driver init stage.

Signed-off-by: Fu Wei <fu.wei@linaro.org>
Reviewed-by: Graeme Gregory <graeme.gregory@linaro.org>
Tested-by: Pratyush Anand <panand@redhat.com>
Acked-by: Timur Tabi <timur@codeaurora.org>
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/watchdog/Kconfig     |  20 +++
 drivers/watchdog/Makefile    |   1 +
 drivers/watchdog/sbsa_gwdt.c | 403 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 424 insertions(+)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 0f6d851..ed9a5cb 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -202,6 +202,26 @@ 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 has two stage timeouts:
+	  the first signal (WS0) is for alerting the system by interrupt,
+	  the second one (WS1) is a real hardware reset.
+	  More details: ARM DEN0029B - Server Base System Architecture (SBSA)
+
+	  This driver can operate ARM SBSA Generic Watchdog as a single stage
+	  or a two stages watchdog, it depends on the module parameter "action".
+
+	  Note: the maximum timeout in the two stages mode is half of that in
+	  the single stage mode.
+
+	  To compile this driver as module, choose M here: The module
+	  will be called sbsa_gwdt.
+
 config ASM9260_WATCHDOG
 	tristate "Alphascale ASM9260 watchdog"
 	depends on MACH_ASM9260
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index f566753..f9826d4 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_ASM9260_WATCHDOG) += asm9260_wdt.o
 obj-$(CONFIG_AT91RM9200_WATCHDOG) += at91rm9200_wdt.o
 obj-$(CONFIG_AT91SAM9X_WATCHDOG) += at91sam9_wdt.o
diff --git a/drivers/watchdog/sbsa_gwdt.c b/drivers/watchdog/sbsa_gwdt.c
new file mode 100644
index 0000000..789444e
--- /dev/null
+++ b/drivers/watchdog/sbsa_gwdt.c
@@ -0,0 +1,403 @@
+/*
+ * SBSA(Server Base System Architecture) Generic Watchdog driver
+ *
+ * Copyright (c) 2015, Linaro Ltd.
+ * Author: Fu Wei <fu.wei@linaro.org>
+ *         Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
+ *         Al Stone <al.stone@linaro.org>
+ *         Timur Tabi <timur@codeaurora.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License 2 as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * ARM SBSA Generic Watchdog has two stage timeouts:
+ * the first signal (WS0) is for alerting the system by interrupt,
+ * the second one (WS1) is a real hardware reset.
+ * More details about the hardware specification of this device:
+ * ARM DEN0029B - Server Base System Architecture (SBSA)
+ *
+ * This driver can operate ARM SBSA Generic Watchdog as a single stage watchdog
+ * or a two stages watchdog, it's set up by the module parameter "action".
+ * In the single stage mode, when the timeout is reached, your system
+ * will be reset by WS1. The first signal (WS0) is ignored.
+ * In the two stages mode, when the timeout is reached, the first signal (WS0)
+ * will trigger panic. If the system is getting into trouble and cannot be reset
+ * by panic or restart properly by the kdump kernel(if supported), then the
+ * second stage (as long as the first stage) will be reached, system will be
+ * reset by WS1. This function can help administrator to backup the system
+ * context info by panic console output or kdump.
+ *
+ * SBSA GWDT:
+ * if action is 1 (the two stages mode):
+ * |--------WOR-------WS0--------WOR-------WS1
+ * |----timeout-----(panic)----timeout-----reset
+ *
+ * if action is 0 (the single stage mode):
+ * |------WOR-----WS0(ignored)-----WOR------WS1
+ * |--------------timeout-------------------reset
+ *
+ * Note: Since this watchdog timer has two stages, and each stage is determined
+ * by WOR, in the single stage mode, the timeout is (WOR * 2); in the two
+ * stages mode, the timeout is WOR. The maximum timeout in the two stages mode
+ * is half of that in the single stage mode.
+ *
+ */
+
+#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				0x010
+
+/* 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.
+ * @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;
+	void __iomem		*refresh_base;
+	void __iomem		*control_base;
+};
+
+#define to_sbsa_gwdt(e) container_of(e, struct sbsa_gwdt, wdd)
+
+#define DEFAULT_TIMEOUT		10 /* seconds, the 1st + 2nd watch periods*/
+
+static unsigned int timeout;
+module_param(timeout, uint, 0);
+MODULE_PARM_DESC(timeout,
+		 "Watchdog timeout in seconds. (>=0, default="
+		 __MODULE_STRING(DEFAULT_TIMEOUT) ")");
+
+/*
+ * action refers to action taken when watchdog gets WS0
+ * 0 = skip
+ * 1 = panic
+ * defaults to skip (0)
+ */
+static int action;
+module_param(action, int, 0);
+MODULE_PARM_DESC(action, "after watchdog gets WS0 interrupt, do: "
+		 "0 = skip(*)  1 = panic");
+
+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) ")");
+
+/*
+ * watchdog operation functions
+ */
+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 (action)
+		writel(gwdt->clk * timeout,
+		       gwdt->control_base + SBSA_GWDT_WOR);
+	else
+		/*
+		 * In the single stage mode, The first signal (WS0) is ignored,
+		 * the timeout is (WOR * 2), so the WOR should be configured
+		 * to half value of timeout.
+		 */
+		writel(gwdt->clk / 2 * timeout,
+		       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 = 0;
+
+	/*
+	 * In the single stage mode, if WS0 is deasserted
+	 * (watchdog is in the first stage),
+	 * timeleft = WOR + (WCV - system counter)
+	 */
+	if (!action &&
+	    !(readl(gwdt->control_base + SBSA_GWDT_WCS) & SBSA_GWDT_WCS_WS0))
+		timeleft += readl(gwdt->control_base + SBSA_GWDT_WOR);
+
+	timeleft += readq(gwdt->control_base + SBSA_GWDT_WCV) -
+		    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);
+
+	/*
+	* Writing WRR for an explicit watchdog refresh.
+	* You can write anyting (like 0).
+	*/
+	writel(0, gwdt->refresh_base + SBSA_GWDT_WRR);
+
+	return 0;
+}
+
+static unsigned int sbsa_gwdt_status(struct watchdog_device *wdd)
+{
+	struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
+	u32 status = readl(gwdt->control_base + SBSA_GWDT_WCS);
+
+	/* is the watchdog timer running? */
+	return (status & SBSA_GWDT_WCS_EN) << WDOG_ACTIVE;
+}
+
+static int sbsa_gwdt_start(struct watchdog_device *wdd)
+{
+	struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
+
+	/* writing WCS will cause an explicit watchdog refresh */
+	writel(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);
+
+	/* Simply write 0 to WCS to clean WCS_EN bit */
+	writel(0, gwdt->control_base + SBSA_GWDT_WCS);
+
+	return 0;
+}
+
+static irqreturn_t sbsa_gwdt_interrupt(int irq, void *dev_id)
+{
+	panic("SBSA Watchdog 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,
+	.status		= sbsa_gwdt_status,
+	.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)
+{
+	void __iomem *rf_base, *cf_base;
+	struct device *dev = &pdev->dev;
+	struct watchdog_device *wdd;
+	struct sbsa_gwdt *gwdt;
+	struct resource *res;
+	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(pdev, IORESOURCE_MEM, 0);
+	cf_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(cf_base))
+		return PTR_ERR(cf_base);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	rf_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(rf_base))
+		return PTR_ERR(rf_base);
+
+	if (action) {
+		irq = platform_get_irq(pdev, 0);
+		if (irq < 0) {
+			action = 0;
+			dev_warn(dev, "unable to get ws0 interrupt.\n");
+		} else {
+			if (devm_request_irq(dev, irq, sbsa_gwdt_interrupt, 0,
+					     pdev->name, gwdt)) {
+				action = 0;
+				dev_warn(dev, "unable to request IRQ %d.\n",
+					 irq);
+			}
+		}
+		if (!action)
+			dev_warn(dev, "falling back to signle stage mode.\n");
+	}
+
+	/*
+	 * 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;
+
+	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;
+	wdd->max_timeout = U32_MAX / gwdt->clk;
+	/*
+	 * In the single stage mode, The first signal (WS0) is ignored,
+	 * the timeout is (WOR * 2), so the maximum timeout should be double.
+	 */
+	if (!action)
+		wdd->max_timeout *= 2;
+
+	wdd->timeout = DEFAULT_TIMEOUT;
+	watchdog_init_timeout(wdd, timeout, dev);
+
+	status = readl(gwdt->control_base + SBSA_GWDT_WCS);
+	if (status & SBSA_GWDT_WCS_WS1) {
+		dev_warn(dev, "System reset by WDT.\n");
+		wdd->bootstatus |= WDIOF_CARDRESET;
+	}
+
+	ret = watchdog_register_device(wdd);
+	if (ret)
+		return ret;
+
+	/*
+	 * Update timeout to WOR.
+	 * Because of the explicit watchdog refresh mechanism,
+	 * it's also a ping, if watchdog is enabled.
+	 */
+	sbsa_gwdt_set_timeout(wdd, wdd->timeout);
+
+	dev_info(dev, "Initialized with %ds timeout @ %u Hz, action=%d.%s\n",
+		 wdd->timeout, gwdt->clk, action,
+		 status & SBSA_GWDT_WCS_EN ? " [enabled]" : "");
+
+	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_AUTHOR("Fu Wei <fu.wei@linaro.org>");
+MODULE_AUTHOR("Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>");
+MODULE_AUTHOR("Al Stone <al.stone@linaro.org>");
+MODULE_AUTHOR("Timur Tabi <timur@codeaurora.org>");
+MODULE_LICENSE("GPL v2");
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v12 4/4] Watchdog: introduce ARM SBSA watchdog driver
@ 2016-02-16  8:36   ` fu.wei at linaro.org
  0 siblings, 0 replies; 32+ messages in thread
From: fu.wei at linaro.org @ 2016-02-16  8:36 UTC (permalink / raw)
  To: linux-arm-kernel

From: Fu Wei <fu.wei@linaro.org>

According to Server Base System Architecture (SBSA) specification,
the SBSA Generic Watchdog has two stage timeouts: the first signal (WS0)
is for alerting the system by interrupt, the second one (WS1) is a real
hardware reset.
More details about the hardware specification of this device:
ARM DEN0029B - Server Base System Architecture (SBSA)

This driver can operate ARM SBSA Generic Watchdog as a single stage watchdog
or a two stages watchdog, it's set up by the module parameter "action".
In the single stage mode, when the timeout is reached, your system
will be reset by WS1. The first signal (WS0) is ignored.
In the two stages mode, when the timeout is reached, the first signal (WS0)
will trigger panic. If the system is getting into trouble and cannot be reset
by panic or restart properly by the kdump kernel(if supported), then the
second stage (as long as the first stage) will be reached, system will be
reset by WS1. This function can help administrator to backup the system
context info by panic console output or kdump.

This driver bases on linux kernel watchdog framework, so it can get
timeout from module parameter and FDT at the driver init stage.

Signed-off-by: Fu Wei <fu.wei@linaro.org>
Reviewed-by: Graeme Gregory <graeme.gregory@linaro.org>
Tested-by: Pratyush Anand <panand@redhat.com>
Acked-by: Timur Tabi <timur@codeaurora.org>
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/watchdog/Kconfig     |  20 +++
 drivers/watchdog/Makefile    |   1 +
 drivers/watchdog/sbsa_gwdt.c | 403 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 424 insertions(+)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 0f6d851..ed9a5cb 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -202,6 +202,26 @@ 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 has two stage timeouts:
+	  the first signal (WS0) is for alerting the system by interrupt,
+	  the second one (WS1) is a real hardware reset.
+	  More details: ARM DEN0029B - Server Base System Architecture (SBSA)
+
+	  This driver can operate ARM SBSA Generic Watchdog as a single stage
+	  or a two stages watchdog, it depends on the module parameter "action".
+
+	  Note: the maximum timeout in the two stages mode is half of that in
+	  the single stage mode.
+
+	  To compile this driver as module, choose M here: The module
+	  will be called sbsa_gwdt.
+
 config ASM9260_WATCHDOG
 	tristate "Alphascale ASM9260 watchdog"
 	depends on MACH_ASM9260
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index f566753..f9826d4 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_ASM9260_WATCHDOG) += asm9260_wdt.o
 obj-$(CONFIG_AT91RM9200_WATCHDOG) += at91rm9200_wdt.o
 obj-$(CONFIG_AT91SAM9X_WATCHDOG) += at91sam9_wdt.o
diff --git a/drivers/watchdog/sbsa_gwdt.c b/drivers/watchdog/sbsa_gwdt.c
new file mode 100644
index 0000000..789444e
--- /dev/null
+++ b/drivers/watchdog/sbsa_gwdt.c
@@ -0,0 +1,403 @@
+/*
+ * SBSA(Server Base System Architecture) Generic Watchdog driver
+ *
+ * Copyright (c) 2015, Linaro Ltd.
+ * Author: Fu Wei <fu.wei@linaro.org>
+ *         Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
+ *         Al Stone <al.stone@linaro.org>
+ *         Timur Tabi <timur@codeaurora.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License 2 as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * ARM SBSA Generic Watchdog has two stage timeouts:
+ * the first signal (WS0) is for alerting the system by interrupt,
+ * the second one (WS1) is a real hardware reset.
+ * More details about the hardware specification of this device:
+ * ARM DEN0029B - Server Base System Architecture (SBSA)
+ *
+ * This driver can operate ARM SBSA Generic Watchdog as a single stage watchdog
+ * or a two stages watchdog, it's set up by the module parameter "action".
+ * In the single stage mode, when the timeout is reached, your system
+ * will be reset by WS1. The first signal (WS0) is ignored.
+ * In the two stages mode, when the timeout is reached, the first signal (WS0)
+ * will trigger panic. If the system is getting into trouble and cannot be reset
+ * by panic or restart properly by the kdump kernel(if supported), then the
+ * second stage (as long as the first stage) will be reached, system will be
+ * reset by WS1. This function can help administrator to backup the system
+ * context info by panic console output or kdump.
+ *
+ * SBSA GWDT:
+ * if action is 1 (the two stages mode):
+ * |--------WOR-------WS0--------WOR-------WS1
+ * |----timeout-----(panic)----timeout-----reset
+ *
+ * if action is 0 (the single stage mode):
+ * |------WOR-----WS0(ignored)-----WOR------WS1
+ * |--------------timeout-------------------reset
+ *
+ * Note: Since this watchdog timer has two stages, and each stage is determined
+ * by WOR, in the single stage mode, the timeout is (WOR * 2); in the two
+ * stages mode, the timeout is WOR. The maximum timeout in the two stages mode
+ * is half of that in the single stage mode.
+ *
+ */
+
+#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				0x010
+
+/* 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.
+ * @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;
+	void __iomem		*refresh_base;
+	void __iomem		*control_base;
+};
+
+#define to_sbsa_gwdt(e) container_of(e, struct sbsa_gwdt, wdd)
+
+#define DEFAULT_TIMEOUT		10 /* seconds, the 1st + 2nd watch periods*/
+
+static unsigned int timeout;
+module_param(timeout, uint, 0);
+MODULE_PARM_DESC(timeout,
+		 "Watchdog timeout in seconds. (>=0, default="
+		 __MODULE_STRING(DEFAULT_TIMEOUT) ")");
+
+/*
+ * action refers to action taken when watchdog gets WS0
+ * 0 = skip
+ * 1 = panic
+ * defaults to skip (0)
+ */
+static int action;
+module_param(action, int, 0);
+MODULE_PARM_DESC(action, "after watchdog gets WS0 interrupt, do: "
+		 "0 = skip(*)  1 = panic");
+
+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) ")");
+
+/*
+ * watchdog operation functions
+ */
+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 (action)
+		writel(gwdt->clk * timeout,
+		       gwdt->control_base + SBSA_GWDT_WOR);
+	else
+		/*
+		 * In the single stage mode, The first signal (WS0) is ignored,
+		 * the timeout is (WOR * 2), so the WOR should be configured
+		 * to half value of timeout.
+		 */
+		writel(gwdt->clk / 2 * timeout,
+		       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 = 0;
+
+	/*
+	 * In the single stage mode, if WS0 is deasserted
+	 * (watchdog is in the first stage),
+	 * timeleft = WOR + (WCV - system counter)
+	 */
+	if (!action &&
+	    !(readl(gwdt->control_base + SBSA_GWDT_WCS) & SBSA_GWDT_WCS_WS0))
+		timeleft += readl(gwdt->control_base + SBSA_GWDT_WOR);
+
+	timeleft += readq(gwdt->control_base + SBSA_GWDT_WCV) -
+		    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);
+
+	/*
+	* Writing WRR for an explicit watchdog refresh.
+	* You can write anyting (like 0).
+	*/
+	writel(0, gwdt->refresh_base + SBSA_GWDT_WRR);
+
+	return 0;
+}
+
+static unsigned int sbsa_gwdt_status(struct watchdog_device *wdd)
+{
+	struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
+	u32 status = readl(gwdt->control_base + SBSA_GWDT_WCS);
+
+	/* is the watchdog timer running? */
+	return (status & SBSA_GWDT_WCS_EN) << WDOG_ACTIVE;
+}
+
+static int sbsa_gwdt_start(struct watchdog_device *wdd)
+{
+	struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
+
+	/* writing WCS will cause an explicit watchdog refresh */
+	writel(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);
+
+	/* Simply write 0 to WCS to clean WCS_EN bit */
+	writel(0, gwdt->control_base + SBSA_GWDT_WCS);
+
+	return 0;
+}
+
+static irqreturn_t sbsa_gwdt_interrupt(int irq, void *dev_id)
+{
+	panic("SBSA Watchdog 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,
+	.status		= sbsa_gwdt_status,
+	.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)
+{
+	void __iomem *rf_base, *cf_base;
+	struct device *dev = &pdev->dev;
+	struct watchdog_device *wdd;
+	struct sbsa_gwdt *gwdt;
+	struct resource *res;
+	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(pdev, IORESOURCE_MEM, 0);
+	cf_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(cf_base))
+		return PTR_ERR(cf_base);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	rf_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(rf_base))
+		return PTR_ERR(rf_base);
+
+	if (action) {
+		irq = platform_get_irq(pdev, 0);
+		if (irq < 0) {
+			action = 0;
+			dev_warn(dev, "unable to get ws0 interrupt.\n");
+		} else {
+			if (devm_request_irq(dev, irq, sbsa_gwdt_interrupt, 0,
+					     pdev->name, gwdt)) {
+				action = 0;
+				dev_warn(dev, "unable to request IRQ %d.\n",
+					 irq);
+			}
+		}
+		if (!action)
+			dev_warn(dev, "falling back to signle stage mode.\n");
+	}
+
+	/*
+	 * 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;
+
+	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;
+	wdd->max_timeout = U32_MAX / gwdt->clk;
+	/*
+	 * In the single stage mode, The first signal (WS0) is ignored,
+	 * the timeout is (WOR * 2), so the maximum timeout should be double.
+	 */
+	if (!action)
+		wdd->max_timeout *= 2;
+
+	wdd->timeout = DEFAULT_TIMEOUT;
+	watchdog_init_timeout(wdd, timeout, dev);
+
+	status = readl(gwdt->control_base + SBSA_GWDT_WCS);
+	if (status & SBSA_GWDT_WCS_WS1) {
+		dev_warn(dev, "System reset by WDT.\n");
+		wdd->bootstatus |= WDIOF_CARDRESET;
+	}
+
+	ret = watchdog_register_device(wdd);
+	if (ret)
+		return ret;
+
+	/*
+	 * Update timeout to WOR.
+	 * Because of the explicit watchdog refresh mechanism,
+	 * it's also a ping, if watchdog is enabled.
+	 */
+	sbsa_gwdt_set_timeout(wdd, wdd->timeout);
+
+	dev_info(dev, "Initialized with %ds timeout @ %u Hz, action=%d.%s\n",
+		 wdd->timeout, gwdt->clk, action,
+		 status & SBSA_GWDT_WCS_EN ? " [enabled]" : "");
+
+	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_AUTHOR("Fu Wei <fu.wei@linaro.org>");
+MODULE_AUTHOR("Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>");
+MODULE_AUTHOR("Al Stone <al.stone@linaro.org>");
+MODULE_AUTHOR("Timur Tabi <timur@codeaurora.org>");
+MODULE_LICENSE("GPL v2");
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH v12 4/4] Watchdog: introduce ARM SBSA watchdog driver
  2016-02-16  8:36   ` fu.wei at linaro.org
@ 2016-02-16 15:29     ` Guenter Roeck
  -1 siblings, 0 replies; 32+ messages in thread
From: Guenter Roeck @ 2016-02-16 15:29 UTC (permalink / raw)
  To: fu.wei, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
	wim, corbet, catalin.marinas, will.deacon, Suravee.Suthikulpanit
  Cc: linux-kernel, linux-watchdog, linux-doc, devicetree,
	linux-arm-kernel, linaro-acpi, rruigrok, harba, cov, timur,
	dyoung, panand, graeme.gregory, al.stone, hanjun.guo, jcm, arnd,
	leo.duran, sudeep.holla

On 02/16/2016 12:36 AM, fu.wei@linaro.org wrote:
> From: Fu Wei <fu.wei@linaro.org>
>
> According to Server Base System Architecture (SBSA) specification,
> the SBSA Generic Watchdog has two stage timeouts: the first signal (WS0)
> is for alerting the system by interrupt, the second one (WS1) is a real
> hardware reset.
> More details about the hardware specification of this device:
> ARM DEN0029B - Server Base System Architecture (SBSA)
>
> This driver can operate ARM SBSA Generic Watchdog as a single stage watchdog
> or a two stages watchdog, it's set up by the module parameter "action".
> In the single stage mode, when the timeout is reached, your system
> will be reset by WS1. The first signal (WS0) is ignored.
> In the two stages mode, when the timeout is reached, the first signal (WS0)
> will trigger panic. If the system is getting into trouble and cannot be reset
> by panic or restart properly by the kdump kernel(if supported), then the
> second stage (as long as the first stage) will be reached, system will be
> reset by WS1. This function can help administrator to backup the system
> context info by panic console output or kdump.
>
> This driver bases on linux kernel watchdog framework, so it can get
> timeout from module parameter and FDT at the driver init stage.
>
> Signed-off-by: Fu Wei <fu.wei@linaro.org>
> Reviewed-by: Graeme Gregory <graeme.gregory@linaro.org>
> Tested-by: Pratyush Anand <panand@redhat.com>
> Acked-by: Timur Tabi <timur@codeaurora.org>
> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> ---
>   drivers/watchdog/Kconfig     |  20 +++
>   drivers/watchdog/Makefile    |   1 +
>   drivers/watchdog/sbsa_gwdt.c | 403 +++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 424 insertions(+)
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 0f6d851..ed9a5cb 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig

[ ... ]

> +
> +static int sbsa_gwdt_probe(struct platform_device *pdev)
> +{

[ ... ]

> +		if (!action)
> +			dev_warn(dev, "falling back to signle stage mode.\n");

Still:

s/signle/single/

[ ... ]

> +
> +MODULE_DESCRIPTION("SBSA Generic Watchdog Driver");
> +MODULE_AUTHOR("Fu Wei <fu.wei@linaro.org>");
> +MODULE_AUTHOR("Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>");
> +MODULE_AUTHOR("Al Stone <al.stone@linaro.org>");
> +MODULE_AUTHOR("Timur Tabi <timur@codeaurora.org>");
> +MODULE_LICENSE("GPL v2");
>
Do you need a MODULE_ALIAS ?

Guenter

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v12 4/4] Watchdog: introduce ARM SBSA watchdog driver
@ 2016-02-16 15:29     ` Guenter Roeck
  0 siblings, 0 replies; 32+ messages in thread
From: Guenter Roeck @ 2016-02-16 15:29 UTC (permalink / raw)
  To: linux-arm-kernel

On 02/16/2016 12:36 AM, fu.wei at linaro.org wrote:
> From: Fu Wei <fu.wei@linaro.org>
>
> According to Server Base System Architecture (SBSA) specification,
> the SBSA Generic Watchdog has two stage timeouts: the first signal (WS0)
> is for alerting the system by interrupt, the second one (WS1) is a real
> hardware reset.
> More details about the hardware specification of this device:
> ARM DEN0029B - Server Base System Architecture (SBSA)
>
> This driver can operate ARM SBSA Generic Watchdog as a single stage watchdog
> or a two stages watchdog, it's set up by the module parameter "action".
> In the single stage mode, when the timeout is reached, your system
> will be reset by WS1. The first signal (WS0) is ignored.
> In the two stages mode, when the timeout is reached, the first signal (WS0)
> will trigger panic. If the system is getting into trouble and cannot be reset
> by panic or restart properly by the kdump kernel(if supported), then the
> second stage (as long as the first stage) will be reached, system will be
> reset by WS1. This function can help administrator to backup the system
> context info by panic console output or kdump.
>
> This driver bases on linux kernel watchdog framework, so it can get
> timeout from module parameter and FDT at the driver init stage.
>
> Signed-off-by: Fu Wei <fu.wei@linaro.org>
> Reviewed-by: Graeme Gregory <graeme.gregory@linaro.org>
> Tested-by: Pratyush Anand <panand@redhat.com>
> Acked-by: Timur Tabi <timur@codeaurora.org>
> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> ---
>   drivers/watchdog/Kconfig     |  20 +++
>   drivers/watchdog/Makefile    |   1 +
>   drivers/watchdog/sbsa_gwdt.c | 403 +++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 424 insertions(+)
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 0f6d851..ed9a5cb 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig

[ ... ]

> +
> +static int sbsa_gwdt_probe(struct platform_device *pdev)
> +{

[ ... ]

> +		if (!action)
> +			dev_warn(dev, "falling back to signle stage mode.\n");

Still:

s/signle/single/

[ ... ]

> +
> +MODULE_DESCRIPTION("SBSA Generic Watchdog Driver");
> +MODULE_AUTHOR("Fu Wei <fu.wei@linaro.org>");
> +MODULE_AUTHOR("Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>");
> +MODULE_AUTHOR("Al Stone <al.stone@linaro.org>");
> +MODULE_AUTHOR("Timur Tabi <timur@codeaurora.org>");
> +MODULE_LICENSE("GPL v2");
>
Do you need a MODULE_ALIAS ?

Guenter

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v12 1/4] Documentation: add sbsa-gwdt driver documentation
  2016-02-16  8:36   ` fu.wei at linaro.org
@ 2016-02-16 15:30     ` Guenter Roeck
  -1 siblings, 0 replies; 32+ messages in thread
From: Guenter Roeck @ 2016-02-16 15:30 UTC (permalink / raw)
  To: fu.wei, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
	wim, corbet, catalin.marinas, will.deacon, Suravee.Suthikulpanit
  Cc: linux-kernel, linux-watchdog, linux-doc, devicetree,
	linux-arm-kernel, linaro-acpi, rruigrok, harba, cov, timur,
	dyoung, panand, graeme.gregory, al.stone, hanjun.guo, jcm, arnd,
	leo.duran, sudeep.holla

On 02/16/2016 12:36 AM, fu.wei@linaro.org wrote:
> From: Fu Wei <fu.wei@linaro.org>
>
> The sbsa-gwdt.txt documentation in devicetree/bindings/watchdog is for
> introducing SBSA(Server Base System Architecture) Generic Watchdog
> device node info into FDT.
>
> Also add sbsa-gwdt introduction in watchdog-parameters.txt
>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Fu Wei <fu.wei@linaro.org>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   .../devicetree/bindings/watchdog/sbsa-gwdt.txt     | 31 ++++++++++++++++++++++
>   Documentation/watchdog/watchdog-parameters.txt     |  7 +++++
>   2 files changed, 38 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt b/Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt
> new file mode 100644
> index 0000000..6f2d5f9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt
> @@ -0,0 +1,31 @@
> +* SBSA (Server Base System Architecture) Generic Watchdog
> +
> +The SBSA Generic Watchdog Timer is used to force a reset of the system
> +after two stages of timeout have elapsed.  A detailed definition of the
> +watchdog timer can be found in the ARM document: ARM-DEN-0029 - Server
> +Base System Architecture (SBSA)
> +
> +Required properties:
> +- compatible: Should at least contain "arm,sbsa-gwdt".
> +
> +- reg: Each entry specifies the base physical address of a register frame
> +  and the length of that frame; currently, two frames must be defined,
> +  in this order:
> +  1: Watchdog control frame;
> +  2: Refresh frame.
> +
> +- interrupts: Should contain the Watchdog Signal 0 (WS0) SPI (Shared
> +  Peripheral Interrupt) number of SBSA Generic Watchdog.
> +
> +Optional properties
> +- timeout-sec: Watchdog timeout values (in seconds).
> +
> +Example for FVP Foundation Model v8:
> +
> +watchdog@2a440000 {
> +	compatible = "arm,sbsa-gwdt";
> +	reg = <0x0 0x2a440000 0 0x1000>,
> +	      <0x0 0x2a450000 0 0x1000>;
> +	interrupts = <0 27 4>;
> +	timeout-sec = <30>;
> +};
> diff --git a/Documentation/watchdog/watchdog-parameters.txt b/Documentation/watchdog/watchdog-parameters.txt
> index 9f9ec9f..ce7ae4e 100644
> --- a/Documentation/watchdog/watchdog-parameters.txt
> +++ b/Documentation/watchdog/watchdog-parameters.txt
> @@ -284,6 +284,13 @@ sbc_fitpc2_wdt:
>   margin: Watchdog margin in seconds (default 60s)
>   nowayout: Watchdog cannot be stopped once started
>   -------------------------------------------------
> +sbsa_gwdt:
> +timeout: Watchdog timeout in seconds. (default 10s)
> +action: Watchdog action at the first stage timeout,
> +	set to 0 to ignore, 1 to panic. (default=0)
> +nowayout: Watchdog cannot be stopped once started
> +	(default=kernel config parameter)
> +-------------------------------------------------
>   sc1200wdt:
>   isapnp: When set to 0 driver ISA PnP support will be disabled (default=1)
>   io: io port
>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v12 1/4] Documentation: add sbsa-gwdt driver documentation
@ 2016-02-16 15:30     ` Guenter Roeck
  0 siblings, 0 replies; 32+ messages in thread
From: Guenter Roeck @ 2016-02-16 15:30 UTC (permalink / raw)
  To: linux-arm-kernel

On 02/16/2016 12:36 AM, fu.wei at linaro.org wrote:
> From: Fu Wei <fu.wei@linaro.org>
>
> The sbsa-gwdt.txt documentation in devicetree/bindings/watchdog is for
> introducing SBSA(Server Base System Architecture) Generic Watchdog
> device node info into FDT.
>
> Also add sbsa-gwdt introduction in watchdog-parameters.txt
>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Fu Wei <fu.wei@linaro.org>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   .../devicetree/bindings/watchdog/sbsa-gwdt.txt     | 31 ++++++++++++++++++++++
>   Documentation/watchdog/watchdog-parameters.txt     |  7 +++++
>   2 files changed, 38 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt b/Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt
> new file mode 100644
> index 0000000..6f2d5f9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/sbsa-gwdt.txt
> @@ -0,0 +1,31 @@
> +* SBSA (Server Base System Architecture) Generic Watchdog
> +
> +The SBSA Generic Watchdog Timer is used to force a reset of the system
> +after two stages of timeout have elapsed.  A detailed definition of the
> +watchdog timer can be found in the ARM document: ARM-DEN-0029 - Server
> +Base System Architecture (SBSA)
> +
> +Required properties:
> +- compatible: Should at least contain "arm,sbsa-gwdt".
> +
> +- reg: Each entry specifies the base physical address of a register frame
> +  and the length of that frame; currently, two frames must be defined,
> +  in this order:
> +  1: Watchdog control frame;
> +  2: Refresh frame.
> +
> +- interrupts: Should contain the Watchdog Signal 0 (WS0) SPI (Shared
> +  Peripheral Interrupt) number of SBSA Generic Watchdog.
> +
> +Optional properties
> +- timeout-sec: Watchdog timeout values (in seconds).
> +
> +Example for FVP Foundation Model v8:
> +
> +watchdog at 2a440000 {
> +	compatible = "arm,sbsa-gwdt";
> +	reg = <0x0 0x2a440000 0 0x1000>,
> +	      <0x0 0x2a450000 0 0x1000>;
> +	interrupts = <0 27 4>;
> +	timeout-sec = <30>;
> +};
> diff --git a/Documentation/watchdog/watchdog-parameters.txt b/Documentation/watchdog/watchdog-parameters.txt
> index 9f9ec9f..ce7ae4e 100644
> --- a/Documentation/watchdog/watchdog-parameters.txt
> +++ b/Documentation/watchdog/watchdog-parameters.txt
> @@ -284,6 +284,13 @@ sbc_fitpc2_wdt:
>   margin: Watchdog margin in seconds (default 60s)
>   nowayout: Watchdog cannot be stopped once started
>   -------------------------------------------------
> +sbsa_gwdt:
> +timeout: Watchdog timeout in seconds. (default 10s)
> +action: Watchdog action at the first stage timeout,
> +	set to 0 to ignore, 1 to panic. (default=0)
> +nowayout: Watchdog cannot be stopped once started
> +	(default=kernel config parameter)
> +-------------------------------------------------
>   sc1200wdt:
>   isapnp: When set to 0 driver ISA PnP support will be disabled (default=1)
>   io: io port
>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v12 2/4] ARM64: add SBSA Generic Watchdog device node in foundation-v8.dts
  2016-02-16  8:36   ` fu.wei at linaro.org
@ 2016-02-16 15:31     ` Guenter Roeck
  -1 siblings, 0 replies; 32+ messages in thread
From: Guenter Roeck @ 2016-02-16 15:31 UTC (permalink / raw)
  To: fu.wei, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
	wim, corbet, catalin.marinas, will.deacon, Suravee.Suthikulpanit
  Cc: linux-kernel, linux-watchdog, linux-doc, devicetree,
	linux-arm-kernel, linaro-acpi, rruigrok, harba, cov, timur,
	dyoung, panand, graeme.gregory, al.stone, hanjun.guo, jcm, arnd,
	leo.duran, sudeep.holla

On 02/16/2016 12:36 AM, fu.wei@linaro.org wrote:
> From: Fu Wei <fu.wei@linaro.org>
>
> This can be a example of adding SBSA Generic Watchdog device node
> into some dts files for the Soc which contains SBSA Generic Watchdog.
>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Fu Wei <fu.wei@linaro.org>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   arch/arm64/boot/dts/arm/foundation-v8.dts | 7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/arm/foundation-v8.dts b/arch/arm64/boot/dts/arm/foundation-v8.dts
> index 4eac8dc..66cb9aa 100644
> --- a/arch/arm64/boot/dts/arm/foundation-v8.dts
> +++ b/arch/arm64/boot/dts/arm/foundation-v8.dts
> @@ -237,4 +237,11 @@
>   			};
>   		};
>   	};
> +	watchdog@2a440000 {
> +		compatible = "arm,sbsa-gwdt";
> +		reg = <0x0 0x2a440000 0 0x1000>,
> +			<0x0 0x2a450000 0 0x1000>;
> +		interrupts = <0 27 4>;
> +		timeout-sec = <30>;
> +	};
>   };
>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v12 2/4] ARM64: add SBSA Generic Watchdog device node in foundation-v8.dts
@ 2016-02-16 15:31     ` Guenter Roeck
  0 siblings, 0 replies; 32+ messages in thread
From: Guenter Roeck @ 2016-02-16 15:31 UTC (permalink / raw)
  To: linux-arm-kernel

On 02/16/2016 12:36 AM, fu.wei at linaro.org wrote:
> From: Fu Wei <fu.wei@linaro.org>
>
> This can be a example of adding SBSA Generic Watchdog device node
> into some dts files for the Soc which contains SBSA Generic Watchdog.
>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Fu Wei <fu.wei@linaro.org>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   arch/arm64/boot/dts/arm/foundation-v8.dts | 7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/arm/foundation-v8.dts b/arch/arm64/boot/dts/arm/foundation-v8.dts
> index 4eac8dc..66cb9aa 100644
> --- a/arch/arm64/boot/dts/arm/foundation-v8.dts
> +++ b/arch/arm64/boot/dts/arm/foundation-v8.dts
> @@ -237,4 +237,11 @@
>   			};
>   		};
>   	};
> +	watchdog at 2a440000 {
> +		compatible = "arm,sbsa-gwdt";
> +		reg = <0x0 0x2a440000 0 0x1000>,
> +			<0x0 0x2a450000 0 0x1000>;
> +		interrupts = <0 27 4>;
> +		timeout-sec = <30>;
> +	};
>   };
>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v12 3/4] ARM64: add SBSA Generic Watchdog device node in amd-seattle-soc.dtsi
  2016-02-16  8:36   ` fu.wei at linaro.org
@ 2016-02-16 15:31     ` Guenter Roeck
  -1 siblings, 0 replies; 32+ messages in thread
From: Guenter Roeck @ 2016-02-16 15:31 UTC (permalink / raw)
  To: fu.wei, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
	wim, corbet, catalin.marinas, will.deacon, Suravee.Suthikulpanit
  Cc: linux-kernel, linux-watchdog, linux-doc, devicetree,
	linux-arm-kernel, linaro-acpi, rruigrok, harba, cov, timur,
	dyoung, panand, graeme.gregory, al.stone, hanjun.guo, jcm, arnd,
	leo.duran, sudeep.holla

On 02/16/2016 12:36 AM, fu.wei@linaro.org wrote:
> From: Fu Wei <fu.wei@linaro.org>
>
> This can be a example of adding SBSA Generic Watchdog device node
> into some dts files for the Soc which contains SBSA Generic Watchdog.
>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> Signed-off-by: Fu Wei <fu.wei@linaro.org>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi | 8 ++++++++
>   1 file changed, 8 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..0a8ca1d 100644
> --- a/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi
> +++ b/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi
> @@ -84,6 +84,14 @@
>   			clock-names = "uartclk", "apb_pclk";
>   		};
>
> +		watchdog0: watchdog@e0bb0000 {
> +			compatible = "arm,sbsa-gwdt";
> +			reg = <0x0 0xe0bc0000 0 0x1000>,
> +				<0x0 0xe0bb0000 0 0x1000>;
> +			interrupts = <0 337 4>;
> +			timeout-sec = <15>;
> +		};
> +
>   		spi0: ssp@e1020000 {
>   			status = "disabled";
>   			compatible = "arm,pl022", "arm,primecell";
>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v12 3/4] ARM64: add SBSA Generic Watchdog device node in amd-seattle-soc.dtsi
@ 2016-02-16 15:31     ` Guenter Roeck
  0 siblings, 0 replies; 32+ messages in thread
From: Guenter Roeck @ 2016-02-16 15:31 UTC (permalink / raw)
  To: linux-arm-kernel

On 02/16/2016 12:36 AM, fu.wei at linaro.org wrote:
> From: Fu Wei <fu.wei@linaro.org>
>
> This can be a example of adding SBSA Generic Watchdog device node
> into some dts files for the Soc which contains SBSA Generic Watchdog.
>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> Signed-off-by: Fu Wei <fu.wei@linaro.org>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi | 8 ++++++++
>   1 file changed, 8 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..0a8ca1d 100644
> --- a/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi
> +++ b/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi
> @@ -84,6 +84,14 @@
>   			clock-names = "uartclk", "apb_pclk";
>   		};
>
> +		watchdog0: watchdog at e0bb0000 {
> +			compatible = "arm,sbsa-gwdt";
> +			reg = <0x0 0xe0bc0000 0 0x1000>,
> +				<0x0 0xe0bb0000 0 0x1000>;
> +			interrupts = <0 337 4>;
> +			timeout-sec = <15>;
> +		};
> +
>   		spi0: ssp at e1020000 {
>   			status = "disabled";
>   			compatible = "arm,pl022", "arm,primecell";
>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v12 4/4] Watchdog: introduce ARM SBSA watchdog driver
  2016-02-16 15:29     ` Guenter Roeck
  (?)
@ 2016-02-16 15:54       ` Fu Wei
  -1 siblings, 0 replies; 32+ messages in thread
From: Fu Wei @ 2016-02-16 15:54 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Rob Herring, Paweł Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Wim Van Sebroeck, Jon Corbet, Catalin Marinas,
	Will Deacon, Suravee Suthikulpanit, LKML, linux-watchdog,
	linux-doc, devicetree, linux-arm-kernel,
	Linaro ACPI Mailman List, Richard Ruigrok, Abdulhamid, Harb,
	Christopher Covington, Timur Tabi, Dave Young, Pratyush Anand,
	G Gregory, Al Stone, Hanjun Guo, Jon Masters, Arnd Bergmann,
	Leo Duran, Sudeep Holla

On 16 February 2016 at 23:29, Guenter Roeck <linux@roeck-us.net> wrote:
> On 02/16/2016 12:36 AM, fu.wei@linaro.org wrote:
>>
>> From: Fu Wei <fu.wei@linaro.org>
>>
>> According to Server Base System Architecture (SBSA) specification,
>> the SBSA Generic Watchdog has two stage timeouts: the first signal (WS0)
>> is for alerting the system by interrupt, the second one (WS1) is a real
>> hardware reset.
>> More details about the hardware specification of this device:
>> ARM DEN0029B - Server Base System Architecture (SBSA)
>>
>> This driver can operate ARM SBSA Generic Watchdog as a single stage
>> watchdog
>> or a two stages watchdog, it's set up by the module parameter "action".
>> In the single stage mode, when the timeout is reached, your system
>> will be reset by WS1. The first signal (WS0) is ignored.
>> In the two stages mode, when the timeout is reached, the first signal
>> (WS0)
>> will trigger panic. If the system is getting into trouble and cannot be
>> reset
>> by panic or restart properly by the kdump kernel(if supported), then the
>> second stage (as long as the first stage) will be reached, system will be
>> reset by WS1. This function can help administrator to backup the system
>> context info by panic console output or kdump.
>>
>> This driver bases on linux kernel watchdog framework, so it can get
>> timeout from module parameter and FDT at the driver init stage.
>>
>> Signed-off-by: Fu Wei <fu.wei@linaro.org>
>> Reviewed-by: Graeme Gregory <graeme.gregory@linaro.org>
>> Tested-by: Pratyush Anand <panand@redhat.com>
>> Acked-by: Timur Tabi <timur@codeaurora.org>
>> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>> ---
>>   drivers/watchdog/Kconfig     |  20 +++
>>   drivers/watchdog/Makefile    |   1 +
>>   drivers/watchdog/sbsa_gwdt.c | 403
>> +++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 424 insertions(+)
>>
>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>> index 0f6d851..ed9a5cb 100644
>> --- a/drivers/watchdog/Kconfig
>> +++ b/drivers/watchdog/Kconfig
>
>
> [ ... ]
>
>> +
>> +static int sbsa_gwdt_probe(struct platform_device *pdev)
>> +{
>
>
> [ ... ]
>
>> +               if (!action)
>> +                       dev_warn(dev, "falling back to signle stage
>> mode.\n");
>
>
> Still:
>
> s/signle/single/

sorry, my bad, will fix it

>
> [ ... ]
>
>> +
>> +MODULE_DESCRIPTION("SBSA Generic Watchdog Driver");
>> +MODULE_AUTHOR("Fu Wei <fu.wei@linaro.org>");
>> +MODULE_AUTHOR("Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>");
>> +MODULE_AUTHOR("Al Stone <al.stone@linaro.org>");
>> +MODULE_AUTHOR("Timur Tabi <timur@codeaurora.org>");
>> +MODULE_LICENSE("GPL v2");
>>
> Do you need a MODULE_ALIAS ?

For now, I thinks we don't need it, Hope I didn't miss something :-)
Because this module can be mounted automatically with dtb or ACPI(if
apply my GTDT patch).
Do you have any suggestion or concern? :-)

>
> 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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v12 4/4] Watchdog: introduce ARM SBSA watchdog driver
@ 2016-02-16 15:54       ` Fu Wei
  0 siblings, 0 replies; 32+ messages in thread
From: Fu Wei @ 2016-02-16 15:54 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Rob Herring, Paweł Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Wim Van Sebroeck, Jon Corbet, Catalin Marinas,
	Will Deacon, Suravee Suthikulpanit, LKML, linux-watchdog,
	linux-doc, devicetree, linux-arm-kernel,
	Linaro ACPI Mailman List, Richard Ruigrok, Abdulhamid, Harb,
	Christopher Covington, Timur Tabi, Dave Young, Pratyush Anand

On 16 February 2016 at 23:29, Guenter Roeck <linux@roeck-us.net> wrote:
> On 02/16/2016 12:36 AM, fu.wei@linaro.org wrote:
>>
>> From: Fu Wei <fu.wei@linaro.org>
>>
>> According to Server Base System Architecture (SBSA) specification,
>> the SBSA Generic Watchdog has two stage timeouts: the first signal (WS0)
>> is for alerting the system by interrupt, the second one (WS1) is a real
>> hardware reset.
>> More details about the hardware specification of this device:
>> ARM DEN0029B - Server Base System Architecture (SBSA)
>>
>> This driver can operate ARM SBSA Generic Watchdog as a single stage
>> watchdog
>> or a two stages watchdog, it's set up by the module parameter "action".
>> In the single stage mode, when the timeout is reached, your system
>> will be reset by WS1. The first signal (WS0) is ignored.
>> In the two stages mode, when the timeout is reached, the first signal
>> (WS0)
>> will trigger panic. If the system is getting into trouble and cannot be
>> reset
>> by panic or restart properly by the kdump kernel(if supported), then the
>> second stage (as long as the first stage) will be reached, system will be
>> reset by WS1. This function can help administrator to backup the system
>> context info by panic console output or kdump.
>>
>> This driver bases on linux kernel watchdog framework, so it can get
>> timeout from module parameter and FDT at the driver init stage.
>>
>> Signed-off-by: Fu Wei <fu.wei@linaro.org>
>> Reviewed-by: Graeme Gregory <graeme.gregory@linaro.org>
>> Tested-by: Pratyush Anand <panand@redhat.com>
>> Acked-by: Timur Tabi <timur@codeaurora.org>
>> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>> ---
>>   drivers/watchdog/Kconfig     |  20 +++
>>   drivers/watchdog/Makefile    |   1 +
>>   drivers/watchdog/sbsa_gwdt.c | 403
>> +++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 424 insertions(+)
>>
>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>> index 0f6d851..ed9a5cb 100644
>> --- a/drivers/watchdog/Kconfig
>> +++ b/drivers/watchdog/Kconfig
>
>
> [ ... ]
>
>> +
>> +static int sbsa_gwdt_probe(struct platform_device *pdev)
>> +{
>
>
> [ ... ]
>
>> +               if (!action)
>> +                       dev_warn(dev, "falling back to signle stage
>> mode.\n");
>
>
> Still:
>
> s/signle/single/

sorry, my bad, will fix it

>
> [ ... ]
>
>> +
>> +MODULE_DESCRIPTION("SBSA Generic Watchdog Driver");
>> +MODULE_AUTHOR("Fu Wei <fu.wei@linaro.org>");
>> +MODULE_AUTHOR("Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>");
>> +MODULE_AUTHOR("Al Stone <al.stone@linaro.org>");
>> +MODULE_AUTHOR("Timur Tabi <timur@codeaurora.org>");
>> +MODULE_LICENSE("GPL v2");
>>
> Do you need a MODULE_ALIAS ?

For now, I thinks we don't need it, Hope I didn't miss something :-)
Because this module can be mounted automatically with dtb or ACPI(if
apply my GTDT patch).
Do you have any suggestion or concern? :-)

>
> 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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v12 4/4] Watchdog: introduce ARM SBSA watchdog driver
@ 2016-02-16 15:54       ` Fu Wei
  0 siblings, 0 replies; 32+ messages in thread
From: Fu Wei @ 2016-02-16 15:54 UTC (permalink / raw)
  To: linux-arm-kernel

On 16 February 2016 at 23:29, Guenter Roeck <linux@roeck-us.net> wrote:
> On 02/16/2016 12:36 AM, fu.wei at linaro.org wrote:
>>
>> From: Fu Wei <fu.wei@linaro.org>
>>
>> According to Server Base System Architecture (SBSA) specification,
>> the SBSA Generic Watchdog has two stage timeouts: the first signal (WS0)
>> is for alerting the system by interrupt, the second one (WS1) is a real
>> hardware reset.
>> More details about the hardware specification of this device:
>> ARM DEN0029B - Server Base System Architecture (SBSA)
>>
>> This driver can operate ARM SBSA Generic Watchdog as a single stage
>> watchdog
>> or a two stages watchdog, it's set up by the module parameter "action".
>> In the single stage mode, when the timeout is reached, your system
>> will be reset by WS1. The first signal (WS0) is ignored.
>> In the two stages mode, when the timeout is reached, the first signal
>> (WS0)
>> will trigger panic. If the system is getting into trouble and cannot be
>> reset
>> by panic or restart properly by the kdump kernel(if supported), then the
>> second stage (as long as the first stage) will be reached, system will be
>> reset by WS1. This function can help administrator to backup the system
>> context info by panic console output or kdump.
>>
>> This driver bases on linux kernel watchdog framework, so it can get
>> timeout from module parameter and FDT at the driver init stage.
>>
>> Signed-off-by: Fu Wei <fu.wei@linaro.org>
>> Reviewed-by: Graeme Gregory <graeme.gregory@linaro.org>
>> Tested-by: Pratyush Anand <panand@redhat.com>
>> Acked-by: Timur Tabi <timur@codeaurora.org>
>> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>> ---
>>   drivers/watchdog/Kconfig     |  20 +++
>>   drivers/watchdog/Makefile    |   1 +
>>   drivers/watchdog/sbsa_gwdt.c | 403
>> +++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 424 insertions(+)
>>
>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>> index 0f6d851..ed9a5cb 100644
>> --- a/drivers/watchdog/Kconfig
>> +++ b/drivers/watchdog/Kconfig
>
>
> [ ... ]
>
>> +
>> +static int sbsa_gwdt_probe(struct platform_device *pdev)
>> +{
>
>
> [ ... ]
>
>> +               if (!action)
>> +                       dev_warn(dev, "falling back to signle stage
>> mode.\n");
>
>
> Still:
>
> s/signle/single/

sorry, my bad, will fix it

>
> [ ... ]
>
>> +
>> +MODULE_DESCRIPTION("SBSA Generic Watchdog Driver");
>> +MODULE_AUTHOR("Fu Wei <fu.wei@linaro.org>");
>> +MODULE_AUTHOR("Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>");
>> +MODULE_AUTHOR("Al Stone <al.stone@linaro.org>");
>> +MODULE_AUTHOR("Timur Tabi <timur@codeaurora.org>");
>> +MODULE_LICENSE("GPL v2");
>>
> Do you need a MODULE_ALIAS ?

For now, I thinks we don't need it, Hope I didn't miss something :-)
Because this module can be mounted automatically with dtb or ACPI(if
apply my GTDT patch).
Do you have any suggestion or concern? :-)

>
> 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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v12 4/4] Watchdog: introduce ARM SBSA watchdog driver
  2016-02-16 15:54       ` Fu Wei
  (?)
@ 2016-02-16 16:33         ` Fu Wei
  -1 siblings, 0 replies; 32+ messages in thread
From: Fu Wei @ 2016-02-16 16:33 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Rob Herring, Paweł Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Wim Van Sebroeck, Jon Corbet, Catalin Marinas,
	Will Deacon, Suravee Suthikulpanit, LKML, linux-watchdog,
	linux-doc, devicetree, linux-arm-kernel,
	Linaro ACPI Mailman List, Richard Ruigrok, Abdulhamid, Harb,
	Christopher Covington, Timur Tabi, Dave Young, Pratyush Anand,
	G Gregory, Al Stone, Hanjun Guo, Jon Masters, Arnd Bergmann,
	Leo Duran, Sudeep Holla

On 16 February 2016 at 23:54, Fu Wei <fu.wei@linaro.org> wrote:
> On 16 February 2016 at 23:29, Guenter Roeck <linux@roeck-us.net> wrote:
>> On 02/16/2016 12:36 AM, fu.wei@linaro.org wrote:
>>>
>>> From: Fu Wei <fu.wei@linaro.org>
>>>
>>> According to Server Base System Architecture (SBSA) specification,
>>> the SBSA Generic Watchdog has two stage timeouts: the first signal (WS0)
>>> is for alerting the system by interrupt, the second one (WS1) is a real
>>> hardware reset.
>>> More details about the hardware specification of this device:
>>> ARM DEN0029B - Server Base System Architecture (SBSA)
>>>
>>> This driver can operate ARM SBSA Generic Watchdog as a single stage
>>> watchdog
>>> or a two stages watchdog, it's set up by the module parameter "action".
>>> In the single stage mode, when the timeout is reached, your system
>>> will be reset by WS1. The first signal (WS0) is ignored.
>>> In the two stages mode, when the timeout is reached, the first signal
>>> (WS0)
>>> will trigger panic. If the system is getting into trouble and cannot be
>>> reset
>>> by panic or restart properly by the kdump kernel(if supported), then the
>>> second stage (as long as the first stage) will be reached, system will be
>>> reset by WS1. This function can help administrator to backup the system
>>> context info by panic console output or kdump.
>>>
>>> This driver bases on linux kernel watchdog framework, so it can get
>>> timeout from module parameter and FDT at the driver init stage.
>>>
>>> Signed-off-by: Fu Wei <fu.wei@linaro.org>
>>> Reviewed-by: Graeme Gregory <graeme.gregory@linaro.org>
>>> Tested-by: Pratyush Anand <panand@redhat.com>
>>> Acked-by: Timur Tabi <timur@codeaurora.org>
>>> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>>> ---
>>>   drivers/watchdog/Kconfig     |  20 +++
>>>   drivers/watchdog/Makefile    |   1 +
>>>   drivers/watchdog/sbsa_gwdt.c | 403
>>> +++++++++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 424 insertions(+)
>>>
>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>>> index 0f6d851..ed9a5cb 100644
>>> --- a/drivers/watchdog/Kconfig
>>> +++ b/drivers/watchdog/Kconfig
>>
>>
>> [ ... ]
>>
>>> +
>>> +static int sbsa_gwdt_probe(struct platform_device *pdev)
>>> +{
>>
>>
>> [ ... ]
>>
>>> +               if (!action)
>>> +                       dev_warn(dev, "falling back to signle stage
>>> mode.\n");
>>
>>
>> Still:
>>
>> s/signle/single/
>
> sorry, my bad, will fix it
>
>>
>> [ ... ]
>>
>>> +
>>> +MODULE_DESCRIPTION("SBSA Generic Watchdog Driver");
>>> +MODULE_AUTHOR("Fu Wei <fu.wei@linaro.org>");
>>> +MODULE_AUTHOR("Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>");
>>> +MODULE_AUTHOR("Al Stone <al.stone@linaro.org>");
>>> +MODULE_AUTHOR("Timur Tabi <timur@codeaurora.org>");
>>> +MODULE_LICENSE("GPL v2");
>>>
>> Do you need a MODULE_ALIAS ?
>
> For now, I thinks we don't need it, Hope I didn't miss something :-)
> Because this module can be mounted automatically with dtb or ACPI(if
> apply my GTDT patch).
> Do you have any suggestion or concern? :-)

re-think  about that , will add
MODULE_ALIAS("platform:sbsa-gwdt");

Do you agree ?  :-)

>
>>
>> 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



-- 
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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v12 4/4] Watchdog: introduce ARM SBSA watchdog driver
@ 2016-02-16 16:33         ` Fu Wei
  0 siblings, 0 replies; 32+ messages in thread
From: Fu Wei @ 2016-02-16 16:33 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Rob Herring, Paweł Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Wim Van Sebroeck, Jon Corbet, Catalin Marinas,
	Will Deacon, Suravee Suthikulpanit, LKML, linux-watchdog,
	linux-doc, devicetree, linux-arm-kernel,
	Linaro ACPI Mailman List, Richard Ruigrok, Abdulhamid, Harb,
	Christopher Covington, Timur Tabi, Dave Young, Pratyush Anand

On 16 February 2016 at 23:54, Fu Wei <fu.wei@linaro.org> wrote:
> On 16 February 2016 at 23:29, Guenter Roeck <linux@roeck-us.net> wrote:
>> On 02/16/2016 12:36 AM, fu.wei@linaro.org wrote:
>>>
>>> From: Fu Wei <fu.wei@linaro.org>
>>>
>>> According to Server Base System Architecture (SBSA) specification,
>>> the SBSA Generic Watchdog has two stage timeouts: the first signal (WS0)
>>> is for alerting the system by interrupt, the second one (WS1) is a real
>>> hardware reset.
>>> More details about the hardware specification of this device:
>>> ARM DEN0029B - Server Base System Architecture (SBSA)
>>>
>>> This driver can operate ARM SBSA Generic Watchdog as a single stage
>>> watchdog
>>> or a two stages watchdog, it's set up by the module parameter "action".
>>> In the single stage mode, when the timeout is reached, your system
>>> will be reset by WS1. The first signal (WS0) is ignored.
>>> In the two stages mode, when the timeout is reached, the first signal
>>> (WS0)
>>> will trigger panic. If the system is getting into trouble and cannot be
>>> reset
>>> by panic or restart properly by the kdump kernel(if supported), then the
>>> second stage (as long as the first stage) will be reached, system will be
>>> reset by WS1. This function can help administrator to backup the system
>>> context info by panic console output or kdump.
>>>
>>> This driver bases on linux kernel watchdog framework, so it can get
>>> timeout from module parameter and FDT at the driver init stage.
>>>
>>> Signed-off-by: Fu Wei <fu.wei@linaro.org>
>>> Reviewed-by: Graeme Gregory <graeme.gregory@linaro.org>
>>> Tested-by: Pratyush Anand <panand@redhat.com>
>>> Acked-by: Timur Tabi <timur@codeaurora.org>
>>> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>>> ---
>>>   drivers/watchdog/Kconfig     |  20 +++
>>>   drivers/watchdog/Makefile    |   1 +
>>>   drivers/watchdog/sbsa_gwdt.c | 403
>>> +++++++++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 424 insertions(+)
>>>
>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>>> index 0f6d851..ed9a5cb 100644
>>> --- a/drivers/watchdog/Kconfig
>>> +++ b/drivers/watchdog/Kconfig
>>
>>
>> [ ... ]
>>
>>> +
>>> +static int sbsa_gwdt_probe(struct platform_device *pdev)
>>> +{
>>
>>
>> [ ... ]
>>
>>> +               if (!action)
>>> +                       dev_warn(dev, "falling back to signle stage
>>> mode.\n");
>>
>>
>> Still:
>>
>> s/signle/single/
>
> sorry, my bad, will fix it
>
>>
>> [ ... ]
>>
>>> +
>>> +MODULE_DESCRIPTION("SBSA Generic Watchdog Driver");
>>> +MODULE_AUTHOR("Fu Wei <fu.wei@linaro.org>");
>>> +MODULE_AUTHOR("Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>");
>>> +MODULE_AUTHOR("Al Stone <al.stone@linaro.org>");
>>> +MODULE_AUTHOR("Timur Tabi <timur@codeaurora.org>");
>>> +MODULE_LICENSE("GPL v2");
>>>
>> Do you need a MODULE_ALIAS ?
>
> For now, I thinks we don't need it, Hope I didn't miss something :-)
> Because this module can be mounted automatically with dtb or ACPI(if
> apply my GTDT patch).
> Do you have any suggestion or concern? :-)

re-think  about that , will add
MODULE_ALIAS("platform:sbsa-gwdt");

Do you agree ?  :-)

>
>>
>> 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



-- 
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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v12 4/4] Watchdog: introduce ARM SBSA watchdog driver
@ 2016-02-16 16:33         ` Fu Wei
  0 siblings, 0 replies; 32+ messages in thread
From: Fu Wei @ 2016-02-16 16:33 UTC (permalink / raw)
  To: linux-arm-kernel

On 16 February 2016 at 23:54, Fu Wei <fu.wei@linaro.org> wrote:
> On 16 February 2016 at 23:29, Guenter Roeck <linux@roeck-us.net> wrote:
>> On 02/16/2016 12:36 AM, fu.wei at linaro.org wrote:
>>>
>>> From: Fu Wei <fu.wei@linaro.org>
>>>
>>> According to Server Base System Architecture (SBSA) specification,
>>> the SBSA Generic Watchdog has two stage timeouts: the first signal (WS0)
>>> is for alerting the system by interrupt, the second one (WS1) is a real
>>> hardware reset.
>>> More details about the hardware specification of this device:
>>> ARM DEN0029B - Server Base System Architecture (SBSA)
>>>
>>> This driver can operate ARM SBSA Generic Watchdog as a single stage
>>> watchdog
>>> or a two stages watchdog, it's set up by the module parameter "action".
>>> In the single stage mode, when the timeout is reached, your system
>>> will be reset by WS1. The first signal (WS0) is ignored.
>>> In the two stages mode, when the timeout is reached, the first signal
>>> (WS0)
>>> will trigger panic. If the system is getting into trouble and cannot be
>>> reset
>>> by panic or restart properly by the kdump kernel(if supported), then the
>>> second stage (as long as the first stage) will be reached, system will be
>>> reset by WS1. This function can help administrator to backup the system
>>> context info by panic console output or kdump.
>>>
>>> This driver bases on linux kernel watchdog framework, so it can get
>>> timeout from module parameter and FDT at the driver init stage.
>>>
>>> Signed-off-by: Fu Wei <fu.wei@linaro.org>
>>> Reviewed-by: Graeme Gregory <graeme.gregory@linaro.org>
>>> Tested-by: Pratyush Anand <panand@redhat.com>
>>> Acked-by: Timur Tabi <timur@codeaurora.org>
>>> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>>> ---
>>>   drivers/watchdog/Kconfig     |  20 +++
>>>   drivers/watchdog/Makefile    |   1 +
>>>   drivers/watchdog/sbsa_gwdt.c | 403
>>> +++++++++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 424 insertions(+)
>>>
>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>>> index 0f6d851..ed9a5cb 100644
>>> --- a/drivers/watchdog/Kconfig
>>> +++ b/drivers/watchdog/Kconfig
>>
>>
>> [ ... ]
>>
>>> +
>>> +static int sbsa_gwdt_probe(struct platform_device *pdev)
>>> +{
>>
>>
>> [ ... ]
>>
>>> +               if (!action)
>>> +                       dev_warn(dev, "falling back to signle stage
>>> mode.\n");
>>
>>
>> Still:
>>
>> s/signle/single/
>
> sorry, my bad, will fix it
>
>>
>> [ ... ]
>>
>>> +
>>> +MODULE_DESCRIPTION("SBSA Generic Watchdog Driver");
>>> +MODULE_AUTHOR("Fu Wei <fu.wei@linaro.org>");
>>> +MODULE_AUTHOR("Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>");
>>> +MODULE_AUTHOR("Al Stone <al.stone@linaro.org>");
>>> +MODULE_AUTHOR("Timur Tabi <timur@codeaurora.org>");
>>> +MODULE_LICENSE("GPL v2");
>>>
>> Do you need a MODULE_ALIAS ?
>
> For now, I thinks we don't need it, Hope I didn't miss something :-)
> Because this module can be mounted automatically with dtb or ACPI(if
> apply my GTDT patch).
> Do you have any suggestion or concern? :-)

re-think  about that , will add
MODULE_ALIAS("platform:sbsa-gwdt");

Do you agree ?  :-)

>
>>
>> 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



-- 
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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v12 4/4] Watchdog: introduce ARM SBSA watchdog driver
  2016-02-16 16:33         ` Fu Wei
  (?)
@ 2016-02-17  0:09           ` Guenter Roeck
  -1 siblings, 0 replies; 32+ messages in thread
From: Guenter Roeck @ 2016-02-17  0:09 UTC (permalink / raw)
  To: Fu Wei
  Cc: Rob Herring, Paweł Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Wim Van Sebroeck, Jon Corbet, Catalin Marinas,
	Will Deacon, Suravee Suthikulpanit, LKML, linux-watchdog,
	linux-doc, devicetree, linux-arm-kernel,
	Linaro ACPI Mailman List, Richard Ruigrok, Abdulhamid, Harb,
	Christopher Covington, Timur Tabi, Dave Young, Pratyush Anand,
	G Gregory, Al Stone, Hanjun Guo, Jon Masters, Arnd Bergmann,
	Leo Duran, Sudeep Holla

On Wed, Feb 17, 2016 at 12:33:24AM +0800, Fu Wei wrote:
[ ... ]
> >>> +MODULE_AUTHOR("Fu Wei <fu.wei@linaro.org>");
> >>> +MODULE_AUTHOR("Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>");
> >>> +MODULE_AUTHOR("Al Stone <al.stone@linaro.org>");
> >>> +MODULE_AUTHOR("Timur Tabi <timur@codeaurora.org>");
> >>> +MODULE_LICENSE("GPL v2");
> >>>
> >> Do you need a MODULE_ALIAS ?
> >
> > For now, I thinks we don't need it, Hope I didn't miss something :-)
> > Because this module can be mounted automatically with dtb or ACPI(if
> > apply my GTDT patch).
> > Do you have any suggestion or concern? :-)
> 
> re-think  about that , will add
> MODULE_ALIAS("platform:sbsa-gwdt");
> 
> Do you agree ?  :-)
> 
Sure.

Guenter

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v12 4/4] Watchdog: introduce ARM SBSA watchdog driver
@ 2016-02-17  0:09           ` Guenter Roeck
  0 siblings, 0 replies; 32+ messages in thread
From: Guenter Roeck @ 2016-02-17  0:09 UTC (permalink / raw)
  To: Fu Wei
  Cc: Rob Herring, Paweł Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Wim Van Sebroeck, Jon Corbet, Catalin Marinas,
	Will Deacon, Suravee Suthikulpanit, LKML, linux-watchdog,
	linux-doc, devicetree, linux-arm-kernel,
	Linaro ACPI Mailman List, Richard Ruigrok, Abdulhamid, Harb,
	Christopher Covington, Timur Tabi, Dave Young, Pratyush Anand,
	G Gregory

On Wed, Feb 17, 2016 at 12:33:24AM +0800, Fu Wei wrote:
[ ... ]
> >>> +MODULE_AUTHOR("Fu Wei <fu.wei@linaro.org>");
> >>> +MODULE_AUTHOR("Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>");
> >>> +MODULE_AUTHOR("Al Stone <al.stone@linaro.org>");
> >>> +MODULE_AUTHOR("Timur Tabi <timur@codeaurora.org>");
> >>> +MODULE_LICENSE("GPL v2");
> >>>
> >> Do you need a MODULE_ALIAS ?
> >
> > For now, I thinks we don't need it, Hope I didn't miss something :-)
> > Because this module can be mounted automatically with dtb or ACPI(if
> > apply my GTDT patch).
> > Do you have any suggestion or concern? :-)
> 
> re-think  about that , will add
> MODULE_ALIAS("platform:sbsa-gwdt");
> 
> Do you agree ?  :-)
> 
Sure.

Guenter

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v12 4/4] Watchdog: introduce ARM SBSA watchdog driver
@ 2016-02-17  0:09           ` Guenter Roeck
  0 siblings, 0 replies; 32+ messages in thread
From: Guenter Roeck @ 2016-02-17  0:09 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Feb 17, 2016 at 12:33:24AM +0800, Fu Wei wrote:
[ ... ]
> >>> +MODULE_AUTHOR("Fu Wei <fu.wei@linaro.org>");
> >>> +MODULE_AUTHOR("Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>");
> >>> +MODULE_AUTHOR("Al Stone <al.stone@linaro.org>");
> >>> +MODULE_AUTHOR("Timur Tabi <timur@codeaurora.org>");
> >>> +MODULE_LICENSE("GPL v2");
> >>>
> >> Do you need a MODULE_ALIAS ?
> >
> > For now, I thinks we don't need it, Hope I didn't miss something :-)
> > Because this module can be mounted automatically with dtb or ACPI(if
> > apply my GTDT patch).
> > Do you have any suggestion or concern? :-)
> 
> re-think  about that , will add
> MODULE_ALIAS("platform:sbsa-gwdt");
> 
> Do you agree ?  :-)
> 
Sure.

Guenter

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v12 4/4] Watchdog: introduce ARM SBSA watchdog driver
  2016-02-16  8:36   ` fu.wei at linaro.org
@ 2016-02-26 19:27     ` Timur Tabi
  -1 siblings, 0 replies; 32+ messages in thread
From: Timur Tabi @ 2016-02-26 19:27 UTC (permalink / raw)
  To: fu.wei, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
	wim, linux, corbet, catalin.marinas, will.deacon,
	Suravee.Suthikulpanit
  Cc: linux-kernel, linux-watchdog, linux-doc, devicetree,
	linux-arm-kernel, linaro-acpi, rruigrok, harba, cov, dyoung,
	panand, graeme.gregory, al.stone, hanjun.guo, jcm, arnd,
	leo.duran, sudeep.holla

fu.wei@linaro.org wrote:
> +	if (action) {
> +		irq = platform_get_irq(pdev, 0);
> +		if (irq < 0) {
> +			action = 0;
> +			dev_warn(dev, "unable to get ws0 interrupt.\n");
> +		} else {
> +			if (devm_request_irq(dev, irq, sbsa_gwdt_interrupt, 0,
> +					     pdev->name, gwdt)) {
> +				action = 0;
> +				dev_warn(dev, "unable to request IRQ %d.\n",
> +					 irq);
> +			}
> +		}
> +		if (!action)
> +			dev_warn(dev, "falling back to signle stage mode.\n");
> +	}
> +
> +	/*
> +	 * 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;

I think you need to ping the watchdog before enabling the interrupt, in 
case there is a pending interrupt.  This just happened to me in testing, 
so I recommend this:

> 	/*
> 	 * 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;
>
> 	if (action) {
> 		irq = platform_get_irq(pdev, 0);
> 		if (irq < 0) {
> 			action = 0;
> 			dev_warn(dev, "unable to get ws0 interrupt.\n");
> 		} else {
> 			sbsa_gwdt_keepalive(&gwdt->wdd);
> 			if (devm_request_irq(dev, irq, sbsa_gwdt_interrupt, 0,
> 					     pdev->name, gwdt)) {
> 				action = 0;
> 				dev_warn(dev, "unable to request IRQ %d.\n",
> 					 irq);
> 			}
> 		}
> 		if (!action)
> 			dev_warn(dev, "falling back to single stage mode.\n");
> 	}

In fact, I think you need to move the "if (action) {" block near the end 
of sbsa_gwdt_probe().  We don't want to enable the interrupt until the 
watchdog is fully initialized.

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum, a Linux Foundation collaborative project.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v12 4/4] Watchdog: introduce ARM SBSA watchdog driver
@ 2016-02-26 19:27     ` Timur Tabi
  0 siblings, 0 replies; 32+ messages in thread
From: Timur Tabi @ 2016-02-26 19:27 UTC (permalink / raw)
  To: linux-arm-kernel

fu.wei at linaro.org wrote:
> +	if (action) {
> +		irq = platform_get_irq(pdev, 0);
> +		if (irq < 0) {
> +			action = 0;
> +			dev_warn(dev, "unable to get ws0 interrupt.\n");
> +		} else {
> +			if (devm_request_irq(dev, irq, sbsa_gwdt_interrupt, 0,
> +					     pdev->name, gwdt)) {
> +				action = 0;
> +				dev_warn(dev, "unable to request IRQ %d.\n",
> +					 irq);
> +			}
> +		}
> +		if (!action)
> +			dev_warn(dev, "falling back to signle stage mode.\n");
> +	}
> +
> +	/*
> +	 * 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;

I think you need to ping the watchdog before enabling the interrupt, in 
case there is a pending interrupt.  This just happened to me in testing, 
so I recommend this:

> 	/*
> 	 * 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;
>
> 	if (action) {
> 		irq = platform_get_irq(pdev, 0);
> 		if (irq < 0) {
> 			action = 0;
> 			dev_warn(dev, "unable to get ws0 interrupt.\n");
> 		} else {
> 			sbsa_gwdt_keepalive(&gwdt->wdd);
> 			if (devm_request_irq(dev, irq, sbsa_gwdt_interrupt, 0,
> 					     pdev->name, gwdt)) {
> 				action = 0;
> 				dev_warn(dev, "unable to request IRQ %d.\n",
> 					 irq);
> 			}
> 		}
> 		if (!action)
> 			dev_warn(dev, "falling back to single stage mode.\n");
> 	}

In fact, I think you need to move the "if (action) {" block near the end 
of sbsa_gwdt_probe().  We don't want to enable the interrupt until the 
watchdog is fully initialized.

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum, a Linux Foundation collaborative project.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v12 4/4] Watchdog: introduce ARM SBSA watchdog driver
@ 2016-02-28 14:01       ` Fu Wei
  0 siblings, 0 replies; 32+ messages in thread
From: Fu Wei @ 2016-02-28 14:01 UTC (permalink / raw)
  To: Timur Tabi
  Cc: Rob Herring, Paweł Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Wim Van Sebroeck, Guenter Roeck, Jon Corbet,
	Catalin Marinas, Will Deacon, Suravee Suthikulpanit, LKML,
	linux-watchdog, linux-doc, devicetree, linux-arm-kernel,
	Linaro ACPI Mailman List, Richard Ruigrok, Abdulhamid, Harb,
	Christopher Covington, Dave Young, Pratyush Anand, G Gregory,
	Al Stone, Hanjun Guo, Jon Masters, Arnd Bergmann, Leo Duran,
	Sudeep Holla

Hi Timur,

On 27 February 2016 at 03:27, Timur Tabi <timur@codeaurora.org> wrote:
> fu.wei@linaro.org wrote:
>>
>> +       if (action) {
>> +               irq = platform_get_irq(pdev, 0);
>> +               if (irq < 0) {
>> +                       action = 0;
>> +                       dev_warn(dev, "unable to get ws0 interrupt.\n");
>> +               } else {
>> +                       if (devm_request_irq(dev, irq,
>> sbsa_gwdt_interrupt, 0,
>> +                                            pdev->name, gwdt)) {
>> +                               action = 0;
>> +                               dev_warn(dev, "unable to request IRQ
>> %d.\n",
>> +                                        irq);
>> +                       }
>> +               }
>> +               if (!action)
>> +                       dev_warn(dev, "falling back to signle stage
>> mode.\n");
>> +       }
>> +
>> +       /*
>> +        * 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;
>
>
> I think you need to ping the watchdog before enabling the interrupt, in case
> there is a pending interrupt.  This just happened to me in testing, so I
> recommend this:
>
>>         /*
>>          * 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;
>>
>>         if (action) {
>>                 irq = platform_get_irq(pdev, 0);
>>                 if (irq < 0) {
>>                         action = 0;
>>                         dev_warn(dev, "unable to get ws0 interrupt.\n");
>>                 } else {
>>                         sbsa_gwdt_keepalive(&gwdt->wdd);
>>                         if (devm_request_irq(dev, irq,
>> sbsa_gwdt_interrupt, 0,
>>                                              pdev->name, gwdt)) {
>>                                 action = 0;
>>                                 dev_warn(dev, "unable to request IRQ
>> %d.\n",
>>                                          irq);
>>                         }
>>                 }
>>                 if (!action)
>>                         dev_warn(dev, "falling back to single stage
>> mode.\n");
>>         }
>
>
> In fact, I think you need to move the "if (action) {" block near the end of
> sbsa_gwdt_probe().  We don't want to enable the interrupt until the watchdog
> is fully initialized.
>

Good point! Thanks for your testing :-)

Will post v14 for this change.

> --
> 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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v12 4/4] Watchdog: introduce ARM SBSA watchdog driver
@ 2016-02-28 14:01       ` Fu Wei
  0 siblings, 0 replies; 32+ messages in thread
From: Fu Wei @ 2016-02-28 14:01 UTC (permalink / raw)
  To: Timur Tabi
  Cc: Rob Herring, Paweł Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Wim Van Sebroeck, Guenter Roeck, Jon Corbet,
	Catalin Marinas, Will Deacon, Suravee Suthikulpanit, LKML,
	linux-watchdog-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Linaro ACPI Mailman List, Richard Ruigrok, Abdulhamid, Harb,
	Christopher Covington, Dave Young, Pratyush Anand

Hi Timur,

On 27 February 2016 at 03:27, Timur Tabi <timur-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> fu.wei-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org wrote:
>>
>> +       if (action) {
>> +               irq = platform_get_irq(pdev, 0);
>> +               if (irq < 0) {
>> +                       action = 0;
>> +                       dev_warn(dev, "unable to get ws0 interrupt.\n");
>> +               } else {
>> +                       if (devm_request_irq(dev, irq,
>> sbsa_gwdt_interrupt, 0,
>> +                                            pdev->name, gwdt)) {
>> +                               action = 0;
>> +                               dev_warn(dev, "unable to request IRQ
>> %d.\n",
>> +                                        irq);
>> +                       }
>> +               }
>> +               if (!action)
>> +                       dev_warn(dev, "falling back to signle stage
>> mode.\n");
>> +       }
>> +
>> +       /*
>> +        * 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;
>
>
> I think you need to ping the watchdog before enabling the interrupt, in case
> there is a pending interrupt.  This just happened to me in testing, so I
> recommend this:
>
>>         /*
>>          * 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;
>>
>>         if (action) {
>>                 irq = platform_get_irq(pdev, 0);
>>                 if (irq < 0) {
>>                         action = 0;
>>                         dev_warn(dev, "unable to get ws0 interrupt.\n");
>>                 } else {
>>                         sbsa_gwdt_keepalive(&gwdt->wdd);
>>                         if (devm_request_irq(dev, irq,
>> sbsa_gwdt_interrupt, 0,
>>                                              pdev->name, gwdt)) {
>>                                 action = 0;
>>                                 dev_warn(dev, "unable to request IRQ
>> %d.\n",
>>                                          irq);
>>                         }
>>                 }
>>                 if (!action)
>>                         dev_warn(dev, "falling back to single stage
>> mode.\n");
>>         }
>
>
> In fact, I think you need to move the "if (action) {" block near the end of
> sbsa_gwdt_probe().  We don't want to enable the interrupt until the watchdog
> is fully initialized.
>

Good point! Thanks for your testing :-)

Will post v14 for this change.

> --
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v12 4/4] Watchdog: introduce ARM SBSA watchdog driver
@ 2016-02-28 14:01       ` Fu Wei
  0 siblings, 0 replies; 32+ messages in thread
From: Fu Wei @ 2016-02-28 14:01 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Timur,

On 27 February 2016 at 03:27, Timur Tabi <timur@codeaurora.org> wrote:
> fu.wei at linaro.org wrote:
>>
>> +       if (action) {
>> +               irq = platform_get_irq(pdev, 0);
>> +               if (irq < 0) {
>> +                       action = 0;
>> +                       dev_warn(dev, "unable to get ws0 interrupt.\n");
>> +               } else {
>> +                       if (devm_request_irq(dev, irq,
>> sbsa_gwdt_interrupt, 0,
>> +                                            pdev->name, gwdt)) {
>> +                               action = 0;
>> +                               dev_warn(dev, "unable to request IRQ
>> %d.\n",
>> +                                        irq);
>> +                       }
>> +               }
>> +               if (!action)
>> +                       dev_warn(dev, "falling back to signle stage
>> mode.\n");
>> +       }
>> +
>> +       /*
>> +        * 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;
>
>
> I think you need to ping the watchdog before enabling the interrupt, in case
> there is a pending interrupt.  This just happened to me in testing, so I
> recommend this:
>
>>         /*
>>          * 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;
>>
>>         if (action) {
>>                 irq = platform_get_irq(pdev, 0);
>>                 if (irq < 0) {
>>                         action = 0;
>>                         dev_warn(dev, "unable to get ws0 interrupt.\n");
>>                 } else {
>>                         sbsa_gwdt_keepalive(&gwdt->wdd);
>>                         if (devm_request_irq(dev, irq,
>> sbsa_gwdt_interrupt, 0,
>>                                              pdev->name, gwdt)) {
>>                                 action = 0;
>>                                 dev_warn(dev, "unable to request IRQ
>> %d.\n",
>>                                          irq);
>>                         }
>>                 }
>>                 if (!action)
>>                         dev_warn(dev, "falling back to single stage
>> mode.\n");
>>         }
>
>
> In fact, I think you need to move the "if (action) {" block near the end of
> sbsa_gwdt_probe().  We don't want to enable the interrupt until the watchdog
> is fully initialized.
>

Good point! Thanks for your testing :-)

Will post v14 for this change.

> --
> 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

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2016-02-28 14:01 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-16  8:36 [PATCH v12 0/4] Watchdog: introduce ARM SBSA watchdog driver fu.wei
2016-02-16  8:36 ` fu.wei at linaro.org
2016-02-16  8:36 ` [PATCH v12 1/4] Documentation: add sbsa-gwdt driver documentation fu.wei
2016-02-16  8:36   ` fu.wei at linaro.org
2016-02-16 15:30   ` Guenter Roeck
2016-02-16 15:30     ` Guenter Roeck
2016-02-16  8:36 ` [PATCH v12 2/4] ARM64: add SBSA Generic Watchdog device node in foundation-v8.dts fu.wei
2016-02-16  8:36   ` fu.wei at linaro.org
2016-02-16 15:31   ` Guenter Roeck
2016-02-16 15:31     ` Guenter Roeck
2016-02-16  8:36 ` [PATCH v12 3/4] ARM64: add SBSA Generic Watchdog device node in amd-seattle-soc.dtsi fu.wei
2016-02-16  8:36   ` fu.wei at linaro.org
2016-02-16 15:31   ` Guenter Roeck
2016-02-16 15:31     ` Guenter Roeck
2016-02-16  8:36 ` [PATCH v12 4/4] Watchdog: introduce ARM SBSA watchdog driver fu.wei
2016-02-16  8:36   ` fu.wei at linaro.org
2016-02-16 15:29   ` Guenter Roeck
2016-02-16 15:29     ` Guenter Roeck
2016-02-16 15:54     ` Fu Wei
2016-02-16 15:54       ` Fu Wei
2016-02-16 15:54       ` Fu Wei
2016-02-16 16:33       ` Fu Wei
2016-02-16 16:33         ` Fu Wei
2016-02-16 16:33         ` Fu Wei
2016-02-17  0:09         ` Guenter Roeck
2016-02-17  0:09           ` Guenter Roeck
2016-02-17  0:09           ` Guenter Roeck
2016-02-26 19:27   ` Timur Tabi
2016-02-26 19:27     ` Timur Tabi
2016-02-28 14:01     ` Fu Wei
2016-02-28 14:01       ` Fu Wei
2016-02-28 14:01       ` Fu Wei

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.