[PATCH v3 0/6] Enhance support for the SP805 WDT

[PATCH v3 0/6] Enhance support for the SP805 WDT

[Ray Jui]

This patch series enhances the support for the SP805 watchdog timer.
First of all, 'timeout-sec' devicetree property is added. In addition,
support is also added to allow the driver to reset the watchdog if it
has been detected that watchdot has been started in the bootloader. In
this case, the driver will initiate the ping service from the kernel
watchdog subsystem, before a user mode daemon takes over. This series
also enables SP805 in the default ARM64 defconfig

This patch series is based off v4.17-rc5 and is available on GIHUB:
repo: https://github.com/Broadcom/arm64-linux.git
branch: sp805-wdt-v3

Changes since v2:
 - Fix indent and format to make them consistent within arm,sp805.txt

Changes since v1:
 - Consolidate two duplicated SP805 binding documents into one
 - Slight change of the wdt_is_running implementation per discussion

Ray Jui (6):
  Documentation: DT: Consolidate SP805 binding docs
  Documentation: DT: Add optional 'timeout-sec' property for sp805
  watchdog: sp805: add 'timeout-sec' DT property support
  watchdog: sp805: set WDOG_HW_RUNNING when appropriate
  arm64: dt: set initial SR watchdog timeout to 60 seconds
  arm64: defconfig: add CONFIG_ARM_SP805_WATCHDOG

 .../devicetree/bindings/watchdog/arm,sp805.txt     | 29 +++++++++++++++-----
 .../devicetree/bindings/watchdog/sp805-wdt.txt     | 31 ----------------------
 .../arm64/boot/dts/broadcom/stingray/stingray.dtsi |  1 +
 arch/arm64/configs/defconfig                       |  1 +
 drivers/watchdog/sp805_wdt.c                       | 28 ++++++++++++++++++-
 5 files changed, 51 insertions(+), 39 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/watchdog/sp805-wdt.txt

-- 
2.1.4

[PATCH v3 0/6] Enhance support for the SP805 WDT

[Ray Jui]

This patch series enhances the support for the SP805 watchdog timer.
First of all, 'timeout-sec' devicetree property is added. In addition,
support is also added to allow the driver to reset the watchdog if it
has been detected that watchdot has been started in the bootloader. In
this case, the driver will initiate the ping service from the kernel
watchdog subsystem, before a user mode daemon takes over. This series
also enables SP805 in the default ARM64 defconfig

This patch series is based off v4.17-rc5 and is available on GIHUB:
repo: https://github.com/Broadcom/arm64-linux.git
branch: sp805-wdt-v3

Changes since v2:
 - Fix indent and format to make them consistent within arm,sp805.txt

Changes since v1:
 - Consolidate two duplicated SP805 binding documents into one
 - Slight change of the wdt_is_running implementation per discussion

Ray Jui (6):
  Documentation: DT: Consolidate SP805 binding docs
  Documentation: DT: Add optional 'timeout-sec' property for sp805
  watchdog: sp805: add 'timeout-sec' DT property support
  watchdog: sp805: set WDOG_HW_RUNNING when appropriate
  arm64: dt: set initial SR watchdog timeout to 60 seconds
  arm64: defconfig: add CONFIG_ARM_SP805_WATCHDOG

 .../devicetree/bindings/watchdog/arm,sp805.txt     | 29 +++++++++++++++-----
 .../devicetree/bindings/watchdog/sp805-wdt.txt     | 31 ----------------------
 .../arm64/boot/dts/broadcom/stingray/stingray.dtsi |  1 +
 arch/arm64/configs/defconfig                       |  1 +
 drivers/watchdog/sp805_wdt.c                       | 28 ++++++++++++++++++-
 5 files changed, 51 insertions(+), 39 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/watchdog/sp805-wdt.txt

-- 
2.1.4

[PATCH v3 1/6] Documentation: DT: Consolidate SP805 binding docs

[Ray Jui]

Consolidate two SP805 binding documents "arm,sp805.txt" and
"sp805-wdt.txt" into "arm,sp805.txt" that matches the naming of the
desired compatible string to be used

Signed-off-by: Ray Jui <ray.jui@broadcom.com>
---
 .../devicetree/bindings/watchdog/arm,sp805.txt     | 27 ++++++++++++++-----
 .../devicetree/bindings/watchdog/sp805-wdt.txt     | 31 ----------------------
 2 files changed, 20 insertions(+), 38 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/watchdog/sp805-wdt.txt

diff --git a/Documentation/devicetree/bindings/watchdog/arm,sp805.txt b/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
index ca99d64..0fa3629 100644
--- a/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
+++ b/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
@@ -1,17 +1,30 @@
 ARM AMBA Primecell SP805 Watchdog
 
+SP805 WDT is a ARM Primecell Peripheral and has a standard-id register that
+can be used to identify the peripheral type, vendor, and revision.
+This value can be used for driver matching.
+
+As SP805 WDT is a primecell IP, it follows the base bindings specified in
+'arm/primecell.txt'
+
 Required properties:
-- compatible: Should be "arm,sp805" & "arm,primecell"
-- reg: Should contain location and length for watchdog timer register.
-- interrupts: Should contain the list of watchdog timer interrupts.
-- clocks: clocks driving the watchdog timer hardware. This list should be 2
-	clocks. With 2 clocks, the order is wdogclk clock, apb_pclk.
+- compatible:  Should be "arm,sp805" & "arm,primecell"
+- reg:         Should contain location and length for watchdog timer register
+- clocks:      Clocks driving the watchdog timer hardware. This list should be
+               2 clocks. With 2 clocks, the order is wdog_clk, apb_pclk
+               wdog_clk can be equal to or be a sub-multiple of the apb_pclk
+               frequency
+- clock-names: Shall be "wdog_clk" for first clock and "apb_pclk" for the
+               second one
+
+Optional properties:
+- interrupts:  Should specify WDT interrupt number
 
 Example:
 	watchdog@66090000 {
 		compatible = "arm,sp805", "arm,primecell";
 		reg = <0x66090000 0x1000>;
 		interrupts = <GIC_SPI 406 IRQ_TYPE_LEVEL_HIGH>;
-		clocks = <&apb_pclk>,<&apb_pclk>;
-		clock-names = "wdogclk", "apb_pclk";
+		clocks = <&wdt_clk>, <&apb_pclk>;
+		clock-names = "wdog_clk", "apb_pclk";
 	};
diff --git a/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
deleted file mode 100644
index edc4f0e..0000000
--- a/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
+++ /dev/null
@@ -1,31 +0,0 @@
-* ARM SP805 Watchdog Timer (WDT) Controller
-
-SP805 WDT is a ARM Primecell Peripheral and has a standard-id register that
-can be used to identify the peripheral type, vendor, and revision.
-This value can be used for driver matching.
-
-As SP805 WDT is a primecell IP, it follows the base bindings specified in
-'arm/primecell.txt'
-
-Required properties:
-- compatible : Should be "arm,sp805-wdt", "arm,primecell"
-- reg : Base address and size of the watchdog timer registers.
-- clocks : From common clock binding.
-           First clock is PCLK and the second is WDOGCLK.
-           WDOGCLK can be equal to or be a sub-multiple of the PCLK frequency.
-- clock-names : From common clock binding.
-                Shall be "apb_pclk" for first clock and "wdog_clk" for the
-                second one.
-
-Optional properties:
-- interrupts : Should specify WDT interrupt number.
-
-Examples:
-
-		cluster1_core0_watchdog: wdt@c000000 {
-			compatible = "arm,sp805-wdt", "arm,primecell";
-			reg = <0x0 0xc000000 0x0 0x1000>;
-			clocks = <&clockgen 4 3>, <&clockgen 4 3>;
-			clock-names = "apb_pclk", "wdog_clk";
-		};
-
-- 
2.1.4

[PATCH v3 1/6] Documentation: DT: Consolidate SP805 binding docs

[Ray Jui]

Consolidate two SP805 binding documents "arm,sp805.txt" and
"sp805-wdt.txt" into "arm,sp805.txt" that matches the naming of the
desired compatible string to be used

Signed-off-by: Ray Jui <ray.jui@broadcom.com>
---
 .../devicetree/bindings/watchdog/arm,sp805.txt     | 27 ++++++++++++++-----
 .../devicetree/bindings/watchdog/sp805-wdt.txt     | 31 ----------------------
 2 files changed, 20 insertions(+), 38 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/watchdog/sp805-wdt.txt

diff --git a/Documentation/devicetree/bindings/watchdog/arm,sp805.txt b/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
index ca99d64..0fa3629 100644
--- a/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
+++ b/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
@@ -1,17 +1,30 @@
 ARM AMBA Primecell SP805 Watchdog
 
+SP805 WDT is a ARM Primecell Peripheral and has a standard-id register that
+can be used to identify the peripheral type, vendor, and revision.
+This value can be used for driver matching.
+
+As SP805 WDT is a primecell IP, it follows the base bindings specified in
+'arm/primecell.txt'
+
 Required properties:
-- compatible: Should be "arm,sp805" & "arm,primecell"
-- reg: Should contain location and length for watchdog timer register.
-- interrupts: Should contain the list of watchdog timer interrupts.
-- clocks: clocks driving the watchdog timer hardware. This list should be 2
-	clocks. With 2 clocks, the order is wdogclk clock, apb_pclk.
+- compatible:  Should be "arm,sp805" & "arm,primecell"
+- reg:         Should contain location and length for watchdog timer register
+- clocks:      Clocks driving the watchdog timer hardware. This list should be
+               2 clocks. With 2 clocks, the order is wdog_clk, apb_pclk
+               wdog_clk can be equal to or be a sub-multiple of the apb_pclk
+               frequency
+- clock-names: Shall be "wdog_clk" for first clock and "apb_pclk" for the
+               second one
+
+Optional properties:
+- interrupts:  Should specify WDT interrupt number
 
 Example:
 	watchdog at 66090000 {
 		compatible = "arm,sp805", "arm,primecell";
 		reg = <0x66090000 0x1000>;
 		interrupts = <GIC_SPI 406 IRQ_TYPE_LEVEL_HIGH>;
-		clocks = <&apb_pclk>,<&apb_pclk>;
-		clock-names = "wdogclk", "apb_pclk";
+		clocks = <&wdt_clk>, <&apb_pclk>;
+		clock-names = "wdog_clk", "apb_pclk";
 	};
diff --git a/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
deleted file mode 100644
index edc4f0e..0000000
--- a/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
+++ /dev/null
@@ -1,31 +0,0 @@
-* ARM SP805 Watchdog Timer (WDT) Controller
-
-SP805 WDT is a ARM Primecell Peripheral and has a standard-id register that
-can be used to identify the peripheral type, vendor, and revision.
-This value can be used for driver matching.
-
-As SP805 WDT is a primecell IP, it follows the base bindings specified in
-'arm/primecell.txt'
-
-Required properties:
-- compatible : Should be "arm,sp805-wdt", "arm,primecell"
-- reg : Base address and size of the watchdog timer registers.
-- clocks : From common clock binding.
-           First clock is PCLK and the second is WDOGCLK.
-           WDOGCLK can be equal to or be a sub-multiple of the PCLK frequency.
-- clock-names : From common clock binding.
-                Shall be "apb_pclk" for first clock and "wdog_clk" for the
-                second one.
-
-Optional properties:
-- interrupts : Should specify WDT interrupt number.
-
-Examples:
-
-		cluster1_core0_watchdog: wdt at c000000 {
-			compatible = "arm,sp805-wdt", "arm,primecell";
-			reg = <0x0 0xc000000 0x0 0x1000>;
-			clocks = <&clockgen 4 3>, <&clockgen 4 3>;
-			clock-names = "apb_pclk", "wdog_clk";
-		};
-
-- 
2.1.4

[PATCH v3 2/6] Documentation: DT: Add optional 'timeout-sec' property for sp805

[Ray Jui]

Update the SP805 binding document to add optional 'timeout-sec'
devicetree property

Signed-off-by: Ray Jui <ray.jui@broadcom.com>
---
 Documentation/devicetree/bindings/watchdog/arm,sp805.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/watchdog/arm,sp805.txt b/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
index 0fa3629..1debea3 100644
--- a/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
+++ b/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
@@ -19,6 +19,8 @@ Required properties:
 
 Optional properties:
 - interrupts:  Should specify WDT interrupt number
+- timeout-sec: Should specify default WDT timeout in seconds. If unset, the
+               default timeout in the driver is 30 seconds
 
 Example:
 	watchdog@66090000 {
-- 
2.1.4

[PATCH v3 2/6] Documentation: DT: Add optional 'timeout-sec' property for sp805

[Ray Jui]

Update the SP805 binding document to add optional 'timeout-sec'
devicetree property

Signed-off-by: Ray Jui <ray.jui@broadcom.com>
---
 Documentation/devicetree/bindings/watchdog/arm,sp805.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/watchdog/arm,sp805.txt b/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
index 0fa3629..1debea3 100644
--- a/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
+++ b/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
@@ -19,6 +19,8 @@ Required properties:
 
 Optional properties:
 - interrupts:  Should specify WDT interrupt number
+- timeout-sec: Should specify default WDT timeout in seconds. If unset, the
+               default timeout in the driver is 30 seconds
 
 Example:
 	watchdog at 66090000 {
-- 
2.1.4

[PATCH v3 3/6] watchdog: sp805: add 'timeout-sec' DT property support

[Ray Jui]

Add support for optional devicetree property 'timeout-sec'.
'timeout-sec' is used in the driver if specified in devicetree.
Otherwise, fall back to driver default, i.e., 60 seconds

Signed-off-by: Ray Jui <ray.jui@broadcom.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/watchdog/sp805_wdt.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
index 03805bc..1484609 100644
--- a/drivers/watchdog/sp805_wdt.c
+++ b/drivers/watchdog/sp805_wdt.c
@@ -230,7 +230,14 @@ sp805_wdt_probe(struct amba_device *adev, const struct amba_id *id)
 	spin_lock_init(&wdt->lock);
 	watchdog_set_nowayout(&wdt->wdd, nowayout);
 	watchdog_set_drvdata(&wdt->wdd, wdt);
-	wdt_setload(&wdt->wdd, DEFAULT_TIMEOUT);
+
+	/*
+	 * If 'timeout-sec' devicetree property is specified, use that.
+	 * Otherwise, use DEFAULT_TIMEOUT
+	 */
+	wdt->wdd.timeout = DEFAULT_TIMEOUT;
+	watchdog_init_timeout(&wdt->wdd, 0, &adev->dev);
+	wdt_setload(&wdt->wdd, wdt->wdd.timeout);
 
 	ret = watchdog_register_device(&wdt->wdd);
 	if (ret) {
-- 
2.1.4

[PATCH v3 3/6] watchdog: sp805: add 'timeout-sec' DT property support

[Ray Jui]

Add support for optional devicetree property 'timeout-sec'.
'timeout-sec' is used in the driver if specified in devicetree.
Otherwise, fall back to driver default, i.e., 60 seconds

Signed-off-by: Ray Jui <ray.jui@broadcom.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/watchdog/sp805_wdt.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
index 03805bc..1484609 100644
--- a/drivers/watchdog/sp805_wdt.c
+++ b/drivers/watchdog/sp805_wdt.c
@@ -230,7 +230,14 @@ sp805_wdt_probe(struct amba_device *adev, const struct amba_id *id)
 	spin_lock_init(&wdt->lock);
 	watchdog_set_nowayout(&wdt->wdd, nowayout);
 	watchdog_set_drvdata(&wdt->wdd, wdt);
-	wdt_setload(&wdt->wdd, DEFAULT_TIMEOUT);
+
+	/*
+	 * If 'timeout-sec' devicetree property is specified, use that.
+	 * Otherwise, use DEFAULT_TIMEOUT
+	 */
+	wdt->wdd.timeout = DEFAULT_TIMEOUT;
+	watchdog_init_timeout(&wdt->wdd, 0, &adev->dev);
+	wdt_setload(&wdt->wdd, wdt->wdd.timeout);
 
 	ret = watchdog_register_device(&wdt->wdd);
 	if (ret) {
-- 
2.1.4

[PATCH v3 4/6] watchdog: sp805: set WDOG_HW_RUNNING when appropriate

[Ray Jui]

If the watchdog hardware is already enabled during the boot process,
when the Linux watchdog driver loads, it should reset the watchdog and
tell the watchdog framework. As a result, ping can be generated from
the watchdog framework, until the userspace watchdog daemon takes over
control

Signed-off-by: Ray Jui <ray.jui@broadcom.com>
Reviewed-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
---
 drivers/watchdog/sp805_wdt.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
index 1484609..d662a6f 100644
--- a/drivers/watchdog/sp805_wdt.c
+++ b/drivers/watchdog/sp805_wdt.c
@@ -42,6 +42,7 @@
 	/* control register masks */
 	#define	INT_ENABLE	(1 << 0)
 	#define	RESET_ENABLE	(1 << 1)
+	#define	ENABLE_MASK	(INT_ENABLE | RESET_ENABLE)
 #define WDTINTCLR		0x00C
 #define WDTRIS			0x010
 #define WDTMIS			0x014
@@ -74,6 +75,15 @@ module_param(nowayout, bool, 0);
 MODULE_PARM_DESC(nowayout,
 		"Set to 1 to keep watchdog running after device release");
 
+/* returns true if wdt is running; otherwise returns false */
+static bool wdt_is_running(struct watchdog_device *wdd)
+{
+	struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
+	u32 wdtcontrol = readl_relaxed(wdt->base + WDTCONTROL);
+
+	return (wdtcontrol & ENABLE_MASK) == ENABLE_MASK;
+}
+
 /* This routine finds load value that will reset system in required timout */
 static int wdt_setload(struct watchdog_device *wdd, unsigned int timeout)
 {
@@ -239,6 +249,15 @@ sp805_wdt_probe(struct amba_device *adev, const struct amba_id *id)
 	watchdog_init_timeout(&wdt->wdd, 0, &adev->dev);
 	wdt_setload(&wdt->wdd, wdt->wdd.timeout);
 
+	/*
+	 * If HW is already running, enable/reset the wdt and set the running
+	 * bit to tell the wdt subsystem
+	 */
+	if (wdt_is_running(&wdt->wdd)) {
+		wdt_enable(&wdt->wdd);
+		set_bit(WDOG_HW_RUNNING, &wdt->wdd.status);
+	}
+
 	ret = watchdog_register_device(&wdt->wdd);
 	if (ret) {
 		dev_err(&adev->dev, "watchdog_register_device() failed: %d\n",
-- 
2.1.4

[PATCH v3 4/6] watchdog: sp805: set WDOG_HW_RUNNING when appropriate

[Ray Jui]

If the watchdog hardware is already enabled during the boot process,
when the Linux watchdog driver loads, it should reset the watchdog and
tell the watchdog framework. As a result, ping can be generated from
the watchdog framework, until the userspace watchdog daemon takes over
control

Signed-off-by: Ray Jui <ray.jui@broadcom.com>
Reviewed-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
---
 drivers/watchdog/sp805_wdt.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
index 1484609..d662a6f 100644
--- a/drivers/watchdog/sp805_wdt.c
+++ b/drivers/watchdog/sp805_wdt.c
@@ -42,6 +42,7 @@
 	/* control register masks */
 	#define	INT_ENABLE	(1 << 0)
 	#define	RESET_ENABLE	(1 << 1)
+	#define	ENABLE_MASK	(INT_ENABLE | RESET_ENABLE)
 #define WDTINTCLR		0x00C
 #define WDTRIS			0x010
 #define WDTMIS			0x014
@@ -74,6 +75,15 @@ module_param(nowayout, bool, 0);
 MODULE_PARM_DESC(nowayout,
 		"Set to 1 to keep watchdog running after device release");
 
+/* returns true if wdt is running; otherwise returns false */
+static bool wdt_is_running(struct watchdog_device *wdd)
+{
+	struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
+	u32 wdtcontrol = readl_relaxed(wdt->base + WDTCONTROL);
+
+	return (wdtcontrol & ENABLE_MASK) == ENABLE_MASK;
+}
+
 /* This routine finds load value that will reset system in required timout */
 static int wdt_setload(struct watchdog_device *wdd, unsigned int timeout)
 {
@@ -239,6 +249,15 @@ sp805_wdt_probe(struct amba_device *adev, const struct amba_id *id)
 	watchdog_init_timeout(&wdt->wdd, 0, &adev->dev);
 	wdt_setload(&wdt->wdd, wdt->wdd.timeout);
 
+	/*
+	 * If HW is already running, enable/reset the wdt and set the running
+	 * bit to tell the wdt subsystem
+	 */
+	if (wdt_is_running(&wdt->wdd)) {
+		wdt_enable(&wdt->wdd);
+		set_bit(WDOG_HW_RUNNING, &wdt->wdd.status);
+	}
+
 	ret = watchdog_register_device(&wdt->wdd);
 	if (ret) {
 		dev_err(&adev->dev, "watchdog_register_device() failed: %d\n",
-- 
2.1.4

[PATCH v3 5/6] arm64: dt: set initial SR watchdog timeout to 60 seconds

[Ray Jui]

Set initial Stingray watchdog timeout to 60 seconds

By the time when the userspace watchdog daemon is ready and taking
control over, the watchdog timeout will then be reset to what's
configured in the daemon

Signed-off-by: Ray Jui <ray.jui@broadcom.com>
Reviewed-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
---
 arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi b/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi
index 99aaff0..1e1cf49 100644
--- a/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi
+++ b/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi
@@ -420,6 +420,7 @@
 			interrupts = <GIC_SPI 189 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&hsls_25m_div2_clk>, <&hsls_div4_clk>;
 			clock-names = "wdogclk", "apb_pclk";
+			timeout-sec = <60>;
 		};
 
 		gpio_hsls: gpio@d0000 {
-- 
2.1.4

[PATCH v3 5/6] arm64: dt: set initial SR watchdog timeout to 60 seconds

[Ray Jui]

Set initial Stingray watchdog timeout to 60 seconds

By the time when the userspace watchdog daemon is ready and taking
control over, the watchdog timeout will then be reset to what's
configured in the daemon

Signed-off-by: Ray Jui <ray.jui@broadcom.com>
Reviewed-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
---
 arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi b/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi
index 99aaff0..1e1cf49 100644
--- a/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi
+++ b/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi
@@ -420,6 +420,7 @@
 			interrupts = <GIC_SPI 189 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&hsls_25m_div2_clk>, <&hsls_div4_clk>;
 			clock-names = "wdogclk", "apb_pclk";
+			timeout-sec = <60>;
 		};
 
 		gpio_hsls: gpio at d0000 {
-- 
2.1.4

[PATCH v3 6/6] arm64: defconfig: add CONFIG_ARM_SP805_WATCHDOG

[Ray Jui]

Enable the SP805 watchdog timer

Signed-off-by: Ray Jui <ray.jui@broadcom.com>
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index ecf6137..3fe5eb5 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -351,6 +351,7 @@ CONFIG_ROCKCHIP_THERMAL=m
 CONFIG_TEGRA_BPMP_THERMAL=m
 CONFIG_UNIPHIER_THERMAL=y
 CONFIG_WATCHDOG=y
+CONFIG_ARM_SP805_WATCHDOG=y
 CONFIG_S3C2410_WATCHDOG=y
 CONFIG_MESON_GXBB_WATCHDOG=m
 CONFIG_MESON_WATCHDOG=m
-- 
2.1.4

[PATCH v3 6/6] arm64: defconfig: add CONFIG_ARM_SP805_WATCHDOG

[Ray Jui]

Enable the SP805 watchdog timer

Signed-off-by: Ray Jui <ray.jui@broadcom.com>
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index ecf6137..3fe5eb5 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -351,6 +351,7 @@ CONFIG_ROCKCHIP_THERMAL=m
 CONFIG_TEGRA_BPMP_THERMAL=m
 CONFIG_UNIPHIER_THERMAL=y
 CONFIG_WATCHDOG=y
+CONFIG_ARM_SP805_WATCHDOG=y
 CONFIG_S3C2410_WATCHDOG=y
 CONFIG_MESON_GXBB_WATCHDOG=m
 CONFIG_MESON_WATCHDOG=m
-- 
2.1.4

Re: [PATCH v3 2/6] Documentation: DT: Add optional 'timeout-sec' property for sp805

[Guenter Roeck]

On Wed, May 23, 2018 at 05:15:20PM -0700, Ray Jui wrote:
> Update the SP805 binding document to add optional 'timeout-sec'
> devicetree property
> 
> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> ---
>  Documentation/devicetree/bindings/watchdog/arm,sp805.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/arm,sp805.txt b/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
> index 0fa3629..1debea3 100644
> --- a/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
> +++ b/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
> @@ -19,6 +19,8 @@ Required properties:
>  
>  Optional properties:
>  - interrupts:  Should specify WDT interrupt number
> +- timeout-sec: Should specify default WDT timeout in seconds. If unset, the
> +               default timeout in the driver is 30 seconds

"... the default timeout is determined by the driver" might be better.
If you want to mandate a default here (not sure if that is a good idea),
I would suggest to use something like "should be 30 seconds".

Guenter

>  
>  Example:
>  	watchdog@66090000 {
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 2/6] Documentation: DT: Add optional 'timeout-sec' property for sp805

[Guenter Roeck]

On Wed, May 23, 2018 at 05:15:20PM -0700, Ray Jui wrote:
> Update the SP805 binding document to add optional 'timeout-sec'
> devicetree property
> 
> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> ---
>  Documentation/devicetree/bindings/watchdog/arm,sp805.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/arm,sp805.txt b/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
> index 0fa3629..1debea3 100644
> --- a/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
> +++ b/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
> @@ -19,6 +19,8 @@ Required properties:
>  
>  Optional properties:
>  - interrupts:  Should specify WDT interrupt number
> +- timeout-sec: Should specify default WDT timeout in seconds. If unset, the
> +               default timeout in the driver is 30 seconds

"... the default timeout is determined by the driver" might be better.
If you want to mandate a default here (not sure if that is a good idea),
I would suggest to use something like "should be 30 seconds".

Guenter

>  
>  Example:
>  	watchdog at 66090000 {
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 4/6] watchdog: sp805: set WDOG_HW_RUNNING when appropriate

[Guenter Roeck]

On Wed, May 23, 2018 at 05:15:22PM -0700, Ray Jui wrote:
> If the watchdog hardware is already enabled during the boot process,
> when the Linux watchdog driver loads, it should reset the watchdog and
> tell the watchdog framework. As a result, ping can be generated from
> the watchdog framework, until the userspace watchdog daemon takes over
> control
> 
> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> Reviewed-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
> Reviewed-by: Scott Branden <scott.branden@broadcom.com>

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

I have one question, though: Is it really correct that both
INT_ENABLE _and_ RESET_ENABLE have to be set to enable the watdog ?
What if only RESET_ENABLE is set ?

Thanks,
Guenter

> ---
>  drivers/watchdog/sp805_wdt.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
> index 1484609..d662a6f 100644
> --- a/drivers/watchdog/sp805_wdt.c
> +++ b/drivers/watchdog/sp805_wdt.c
> @@ -42,6 +42,7 @@
>  	/* control register masks */
>  	#define	INT_ENABLE	(1 << 0)
>  	#define	RESET_ENABLE	(1 << 1)
> +	#define	ENABLE_MASK	(INT_ENABLE | RESET_ENABLE)
>  #define WDTINTCLR		0x00C
>  #define WDTRIS			0x010
>  #define WDTMIS			0x014
> @@ -74,6 +75,15 @@ module_param(nowayout, bool, 0);
>  MODULE_PARM_DESC(nowayout,
>  		"Set to 1 to keep watchdog running after device release");
>  
> +/* returns true if wdt is running; otherwise returns false */
> +static bool wdt_is_running(struct watchdog_device *wdd)
> +{
> +	struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
> +	u32 wdtcontrol = readl_relaxed(wdt->base + WDTCONTROL);
> +
> +	return (wdtcontrol & ENABLE_MASK) == ENABLE_MASK;
> +}
> +
>  /* This routine finds load value that will reset system in required timout */
>  static int wdt_setload(struct watchdog_device *wdd, unsigned int timeout)
>  {
> @@ -239,6 +249,15 @@ sp805_wdt_probe(struct amba_device *adev, const struct amba_id *id)
>  	watchdog_init_timeout(&wdt->wdd, 0, &adev->dev);
>  	wdt_setload(&wdt->wdd, wdt->wdd.timeout);
>  
> +	/*
> +	 * If HW is already running, enable/reset the wdt and set the running
> +	 * bit to tell the wdt subsystem
> +	 */
> +	if (wdt_is_running(&wdt->wdd)) {
> +		wdt_enable(&wdt->wdd);
> +		set_bit(WDOG_HW_RUNNING, &wdt->wdd.status);
> +	}
> +
>  	ret = watchdog_register_device(&wdt->wdd);
>  	if (ret) {
>  		dev_err(&adev->dev, "watchdog_register_device() failed: %d\n",
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 4/6] watchdog: sp805: set WDOG_HW_RUNNING when appropriate

[Guenter Roeck]

On Wed, May 23, 2018 at 05:15:22PM -0700, Ray Jui wrote:
> If the watchdog hardware is already enabled during the boot process,
> when the Linux watchdog driver loads, it should reset the watchdog and
> tell the watchdog framework. As a result, ping can be generated from
> the watchdog framework, until the userspace watchdog daemon takes over
> control
> 
> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> Reviewed-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
> Reviewed-by: Scott Branden <scott.branden@broadcom.com>

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

I have one question, though: Is it really correct that both
INT_ENABLE _and_ RESET_ENABLE have to be set to enable the watdog ?
What if only RESET_ENABLE is set ?

Thanks,
Guenter

> ---
>  drivers/watchdog/sp805_wdt.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
> index 1484609..d662a6f 100644
> --- a/drivers/watchdog/sp805_wdt.c
> +++ b/drivers/watchdog/sp805_wdt.c
> @@ -42,6 +42,7 @@
>  	/* control register masks */
>  	#define	INT_ENABLE	(1 << 0)
>  	#define	RESET_ENABLE	(1 << 1)
> +	#define	ENABLE_MASK	(INT_ENABLE | RESET_ENABLE)
>  #define WDTINTCLR		0x00C
>  #define WDTRIS			0x010
>  #define WDTMIS			0x014
> @@ -74,6 +75,15 @@ module_param(nowayout, bool, 0);
>  MODULE_PARM_DESC(nowayout,
>  		"Set to 1 to keep watchdog running after device release");
>  
> +/* returns true if wdt is running; otherwise returns false */
> +static bool wdt_is_running(struct watchdog_device *wdd)
> +{
> +	struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
> +	u32 wdtcontrol = readl_relaxed(wdt->base + WDTCONTROL);
> +
> +	return (wdtcontrol & ENABLE_MASK) == ENABLE_MASK;
> +}
> +
>  /* This routine finds load value that will reset system in required timout */
>  static int wdt_setload(struct watchdog_device *wdd, unsigned int timeout)
>  {
> @@ -239,6 +249,15 @@ sp805_wdt_probe(struct amba_device *adev, const struct amba_id *id)
>  	watchdog_init_timeout(&wdt->wdd, 0, &adev->dev);
>  	wdt_setload(&wdt->wdd, wdt->wdd.timeout);
>  
> +	/*
> +	 * If HW is already running, enable/reset the wdt and set the running
> +	 * bit to tell the wdt subsystem
> +	 */
> +	if (wdt_is_running(&wdt->wdd)) {
> +		wdt_enable(&wdt->wdd);
> +		set_bit(WDOG_HW_RUNNING, &wdt->wdd.status);
> +	}
> +
>  	ret = watchdog_register_device(&wdt->wdd);
>  	if (ret) {
>  		dev_err(&adev->dev, "watchdog_register_device() failed: %d\n",
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 4/6] watchdog: sp805: set WDOG_HW_RUNNING when appropriate

[Ray Jui]

On 5/24/2018 9:19 AM, Guenter Roeck wrote:
> On Wed, May 23, 2018 at 05:15:22PM -0700, Ray Jui wrote:
>> If the watchdog hardware is already enabled during the boot process,
>> when the Linux watchdog driver loads, it should reset the watchdog and
>> tell the watchdog framework. As a result, ping can be generated from
>> the watchdog framework, until the userspace watchdog daemon takes over
>> control
>>
>> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
>> Reviewed-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> 
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> 
> I have one question, though: Is it really correct that both
> INT_ENABLE _and_ RESET_ENABLE have to be set to enable the watdog ?
> What if only RESET_ENABLE is set ?

According to the SP805 TRM, INT_ENABLE needs to be set to high to enable 
the counter and the interrupt. Counter will be stopped if INT_ENABLE is 
cleared. So yes, INT_ENABLE needs to be set.

Thanks,

Ray

> 
> Thanks,
> Guenter
> 
>> ---
>>   drivers/watchdog/sp805_wdt.c | 19 +++++++++++++++++++
>>   1 file changed, 19 insertions(+)
>>
>> diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
>> index 1484609..d662a6f 100644
>> --- a/drivers/watchdog/sp805_wdt.c
>> +++ b/drivers/watchdog/sp805_wdt.c
>> @@ -42,6 +42,7 @@
>>   	/* control register masks */
>>   	#define	INT_ENABLE	(1 << 0)
>>   	#define	RESET_ENABLE	(1 << 1)
>> +	#define	ENABLE_MASK	(INT_ENABLE | RESET_ENABLE)
>>   #define WDTINTCLR		0x00C
>>   #define WDTRIS			0x010
>>   #define WDTMIS			0x014
>> @@ -74,6 +75,15 @@ module_param(nowayout, bool, 0);
>>   MODULE_PARM_DESC(nowayout,
>>   		"Set to 1 to keep watchdog running after device release");
>>   
>> +/* returns true if wdt is running; otherwise returns false */
>> +static bool wdt_is_running(struct watchdog_device *wdd)
>> +{
>> +	struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
>> +	u32 wdtcontrol = readl_relaxed(wdt->base + WDTCONTROL);
>> +
>> +	return (wdtcontrol & ENABLE_MASK) == ENABLE_MASK;
>> +}
>> +
>>   /* This routine finds load value that will reset system in required timout */
>>   static int wdt_setload(struct watchdog_device *wdd, unsigned int timeout)
>>   {
>> @@ -239,6 +249,15 @@ sp805_wdt_probe(struct amba_device *adev, const struct amba_id *id)
>>   	watchdog_init_timeout(&wdt->wdd, 0, &adev->dev);
>>   	wdt_setload(&wdt->wdd, wdt->wdd.timeout);
>>   
>> +	/*
>> +	 * If HW is already running, enable/reset the wdt and set the running
>> +	 * bit to tell the wdt subsystem
>> +	 */
>> +	if (wdt_is_running(&wdt->wdd)) {
>> +		wdt_enable(&wdt->wdd);
>> +		set_bit(WDOG_HW_RUNNING, &wdt->wdd.status);
>> +	}
>> +
>>   	ret = watchdog_register_device(&wdt->wdd);
>>   	if (ret) {
>>   		dev_err(&adev->dev, "watchdog_register_device() failed: %d\n",
>> -- 
>> 2.1.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 4/6] watchdog: sp805: set WDOG_HW_RUNNING when appropriate

[Ray Jui]

On 5/24/2018 9:19 AM, Guenter Roeck wrote:
> On Wed, May 23, 2018 at 05:15:22PM -0700, Ray Jui wrote:
>> If the watchdog hardware is already enabled during the boot process,
>> when the Linux watchdog driver loads, it should reset the watchdog and
>> tell the watchdog framework. As a result, ping can be generated from
>> the watchdog framework, until the userspace watchdog daemon takes over
>> control
>>
>> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
>> Reviewed-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> 
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> 
> I have one question, though: Is it really correct that both
> INT_ENABLE _and_ RESET_ENABLE have to be set to enable the watdog ?
> What if only RESET_ENABLE is set ?

According to the SP805 TRM, INT_ENABLE needs to be set to high to enable 
the counter and the interrupt. Counter will be stopped if INT_ENABLE is 
cleared. So yes, INT_ENABLE needs to be set.

Thanks,

Ray

> 
> Thanks,
> Guenter
> 
>> ---
>>   drivers/watchdog/sp805_wdt.c | 19 +++++++++++++++++++
>>   1 file changed, 19 insertions(+)
>>
>> diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
>> index 1484609..d662a6f 100644
>> --- a/drivers/watchdog/sp805_wdt.c
>> +++ b/drivers/watchdog/sp805_wdt.c
>> @@ -42,6 +42,7 @@
>>   	/* control register masks */
>>   	#define	INT_ENABLE	(1 << 0)
>>   	#define	RESET_ENABLE	(1 << 1)
>> +	#define	ENABLE_MASK	(INT_ENABLE | RESET_ENABLE)
>>   #define WDTINTCLR		0x00C
>>   #define WDTRIS			0x010
>>   #define WDTMIS			0x014
>> @@ -74,6 +75,15 @@ module_param(nowayout, bool, 0);
>>   MODULE_PARM_DESC(nowayout,
>>   		"Set to 1 to keep watchdog running after device release");
>>   
>> +/* returns true if wdt is running; otherwise returns false */
>> +static bool wdt_is_running(struct watchdog_device *wdd)
>> +{
>> +	struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
>> +	u32 wdtcontrol = readl_relaxed(wdt->base + WDTCONTROL);
>> +
>> +	return (wdtcontrol & ENABLE_MASK) == ENABLE_MASK;
>> +}
>> +
>>   /* This routine finds load value that will reset system in required timout */
>>   static int wdt_setload(struct watchdog_device *wdd, unsigned int timeout)
>>   {
>> @@ -239,6 +249,15 @@ sp805_wdt_probe(struct amba_device *adev, const struct amba_id *id)
>>   	watchdog_init_timeout(&wdt->wdd, 0, &adev->dev);
>>   	wdt_setload(&wdt->wdd, wdt->wdd.timeout);
>>   
>> +	/*
>> +	 * If HW is already running, enable/reset the wdt and set the running
>> +	 * bit to tell the wdt subsystem
>> +	 */
>> +	if (wdt_is_running(&wdt->wdd)) {
>> +		wdt_enable(&wdt->wdd);
>> +		set_bit(WDOG_HW_RUNNING, &wdt->wdd.status);
>> +	}
>> +
>>   	ret = watchdog_register_device(&wdt->wdd);
>>   	if (ret) {
>>   		dev_err(&adev->dev, "watchdog_register_device() failed: %d\n",
>> -- 
>> 2.1.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/6] Documentation: DT: Add optional 'timeout-sec' property for sp805

[Ray Jui]

On 5/24/2018 9:16 AM, Guenter Roeck wrote:
> On Wed, May 23, 2018 at 05:15:20PM -0700, Ray Jui wrote:
>> Update the SP805 binding document to add optional 'timeout-sec'
>> devicetree property
>>
>> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
>> ---
>>   Documentation/devicetree/bindings/watchdog/arm,sp805.txt | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/watchdog/arm,sp805.txt b/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
>> index 0fa3629..1debea3 100644
>> --- a/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
>> +++ b/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
>> @@ -19,6 +19,8 @@ Required properties:
>>   
>>   Optional properties:
>>   - interrupts:  Should specify WDT interrupt number
>> +- timeout-sec: Should specify default WDT timeout in seconds. If unset, the
>> +               default timeout in the driver is 30 seconds
> 
> "... the default timeout is determined by the driver" might be better.
> If you want to mandate a default here (not sure if that is a good idea),
> I would suggest to use something like "should be 30 seconds".
> 

Okay. This can be changed to:

- timeout-sec: Should specify default WDT timeout in seconds. If unset, 
the default timeout is determined by the driver.

Please advise how to proceed with this patch series. Should I make the 
above modification and send out v4?

Thanks,

Ray

> Guenter
> 
>>   
>>   Example:
>>   	watchdog@66090000 {
>> -- 
>> 2.1.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 2/6] Documentation: DT: Add optional 'timeout-sec' property for sp805

[Ray Jui]

On 5/24/2018 9:16 AM, Guenter Roeck wrote:
> On Wed, May 23, 2018 at 05:15:20PM -0700, Ray Jui wrote:
>> Update the SP805 binding document to add optional 'timeout-sec'
>> devicetree property
>>
>> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
>> ---
>>   Documentation/devicetree/bindings/watchdog/arm,sp805.txt | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/watchdog/arm,sp805.txt b/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
>> index 0fa3629..1debea3 100644
>> --- a/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
>> +++ b/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
>> @@ -19,6 +19,8 @@ Required properties:
>>   
>>   Optional properties:
>>   - interrupts:  Should specify WDT interrupt number
>> +- timeout-sec: Should specify default WDT timeout in seconds. If unset, the
>> +               default timeout in the driver is 30 seconds
> 
> "... the default timeout is determined by the driver" might be better.
> If you want to mandate a default here (not sure if that is a good idea),
> I would suggest to use something like "should be 30 seconds".
> 

Okay. This can be changed to:

- timeout-sec: Should specify default WDT timeout in seconds. If unset, 
the default timeout is determined by the driver.

Please advise how to proceed with this patch series. Should I make the 
above modification and send out v4?

Thanks,

Ray

> Guenter
> 
>>   
>>   Example:
>>   	watchdog at 66090000 {
>> -- 
>> 2.1.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 4/6] watchdog: sp805: set WDOG_HW_RUNNING when appropriate

[Guenter Roeck]

On Thu, May 24, 2018 at 09:36:25AM -0700, Ray Jui wrote:
> 
> 
> On 5/24/2018 9:19 AM, Guenter Roeck wrote:
> >On Wed, May 23, 2018 at 05:15:22PM -0700, Ray Jui wrote:
> >>If the watchdog hardware is already enabled during the boot process,
> >>when the Linux watchdog driver loads, it should reset the watchdog and
> >>tell the watchdog framework. As a result, ping can be generated from
> >>the watchdog framework, until the userspace watchdog daemon takes over
> >>control
> >>
> >>Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> >>Reviewed-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
> >>Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> >
> >Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> >
> >I have one question, though: Is it really correct that both
> >INT_ENABLE _and_ RESET_ENABLE have to be set to enable the watdog ?
> >What if only RESET_ENABLE is set ?
> 
> According to the SP805 TRM, INT_ENABLE needs to be set to high to enable the
> counter and the interrupt. Counter will be stopped if INT_ENABLE is cleared.
> So yes, INT_ENABLE needs to be set.
> 
Excellent, thanks for the clarification.

Guenter

[PATCH v3 4/6] watchdog: sp805: set WDOG_HW_RUNNING when appropriate

[Guenter Roeck]

On Thu, May 24, 2018 at 09:36:25AM -0700, Ray Jui wrote:
> 
> 
> On 5/24/2018 9:19 AM, Guenter Roeck wrote:
> >On Wed, May 23, 2018 at 05:15:22PM -0700, Ray Jui wrote:
> >>If the watchdog hardware is already enabled during the boot process,
> >>when the Linux watchdog driver loads, it should reset the watchdog and
> >>tell the watchdog framework. As a result, ping can be generated from
> >>the watchdog framework, until the userspace watchdog daemon takes over
> >>control
> >>
> >>Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> >>Reviewed-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
> >>Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> >
> >Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> >
> >I have one question, though: Is it really correct that both
> >INT_ENABLE _and_ RESET_ENABLE have to be set to enable the watdog ?
> >What if only RESET_ENABLE is set ?
> 
> According to the SP805 TRM, INT_ENABLE needs to be set to high to enable the
> counter and the interrupt. Counter will be stopped if INT_ENABLE is cleared.
> So yes, INT_ENABLE needs to be set.
> 
Excellent, thanks for the clarification.

Guenter

Re: [PATCH v3 2/6] Documentation: DT: Add optional 'timeout-sec' property for sp805

[Guenter Roeck]

On Thu, May 24, 2018 at 09:42:20AM -0700, Ray Jui wrote:
> 
> 
> On 5/24/2018 9:16 AM, Guenter Roeck wrote:
> >On Wed, May 23, 2018 at 05:15:20PM -0700, Ray Jui wrote:
> >>Update the SP805 binding document to add optional 'timeout-sec'
> >>devicetree property
> >>
> >>Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> >>---
> >>  Documentation/devicetree/bindings/watchdog/arm,sp805.txt | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >>diff --git a/Documentation/devicetree/bindings/watchdog/arm,sp805.txt b/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
> >>index 0fa3629..1debea3 100644
> >>--- a/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
> >>+++ b/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
> >>@@ -19,6 +19,8 @@ Required properties:
> >>  Optional properties:
> >>  - interrupts:  Should specify WDT interrupt number
> >>+- timeout-sec: Should specify default WDT timeout in seconds. If unset, the
> >>+               default timeout in the driver is 30 seconds
> >
> >"... the default timeout is determined by the driver" might be better.
> >If you want to mandate a default here (not sure if that is a good idea),
> >I would suggest to use something like "should be 30 seconds".
> >
> 
> Okay. This can be changed to:
> 
> - timeout-sec: Should specify default WDT timeout in seconds. If unset, the
> default timeout is determined by the driver.
> 
> Please advise how to proceed with this patch series. Should I make the above
> modification and send out v4?

I would suggest to wait a day or two, then send out v4 if there are no further
comments.

Thanks,
Guenter

[PATCH v3 2/6] Documentation: DT: Add optional 'timeout-sec' property for sp805

[Guenter Roeck]

On Thu, May 24, 2018 at 09:42:20AM -0700, Ray Jui wrote:
> 
> 
> On 5/24/2018 9:16 AM, Guenter Roeck wrote:
> >On Wed, May 23, 2018 at 05:15:20PM -0700, Ray Jui wrote:
> >>Update the SP805 binding document to add optional 'timeout-sec'
> >>devicetree property
> >>
> >>Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> >>---
> >>  Documentation/devicetree/bindings/watchdog/arm,sp805.txt | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >>diff --git a/Documentation/devicetree/bindings/watchdog/arm,sp805.txt b/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
> >>index 0fa3629..1debea3 100644
> >>--- a/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
> >>+++ b/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
> >>@@ -19,6 +19,8 @@ Required properties:
> >>  Optional properties:
> >>  - interrupts:  Should specify WDT interrupt number
> >>+- timeout-sec: Should specify default WDT timeout in seconds. If unset, the
> >>+               default timeout in the driver is 30 seconds
> >
> >"... the default timeout is determined by the driver" might be better.
> >If you want to mandate a default here (not sure if that is a good idea),
> >I would suggest to use something like "should be 30 seconds".
> >
> 
> Okay. This can be changed to:
> 
> - timeout-sec: Should specify default WDT timeout in seconds. If unset, the
> default timeout is determined by the driver.
> 
> Please advise how to proceed with this patch series. Should I make the above
> modification and send out v4?

I would suggest to wait a day or two, then send out v4 if there are no further
comments.

Thanks,
Guenter