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

[PATCH v4 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-v4

Changes since v3:
 - Improve description of 'timeout-sec' in the binding document, per
recommendation from Guenter Roeck

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

Changes since v3:
 - Improve description of 'timeout-sec' in the binding document, per
recommendation from Guenter Roeck

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 v4 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 v4 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 v4 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..bee6f1f 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 is determined by the driver
 
 Example:
 	watchdog@66090000 {
-- 
2.1.4

[PATCH v4 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..bee6f1f 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 is determined by the driver
 
 Example:
 	watchdog@66090000 {
-- 
2.1.4

[PATCH v4 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..bee6f1f 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 is determined by the driver
 
 Example:
 	watchdog at 66090000 {
-- 
2.1.4

[PATCH v4 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 v4 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 v4 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>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
---
 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 v4 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>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
---
 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 v4 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 v4 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 v4 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 v4 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 v4 1/6] Documentation: DT: Consolidate SP805 binding docs

[Rob Herring]

On Mon, May 28, 2018 at 11:01:32AM -0700, Ray Jui wrote:
> 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

Would be good to get a ACK from FSL/NXP person on this. It looks to me 
like the driver fetches the wrong clock as it gets the first one and the 
driver really wants 'wdog_clk'. In any case, their dts files should be 
updated.

Reviewed-by: Rob Herring <robh@kernel.org>

Rob

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

[Rob Herring]

On Mon, May 28, 2018 at 11:01:32AM -0700, Ray Jui wrote:
> 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

Would be good to get a ACK from FSL/NXP person on this. It looks to me 
like the driver fetches the wrong clock as it gets the first one and the 
driver really wants 'wdog_clk'. In any case, their dts files should be 
updated.

Reviewed-by: Rob Herring <robh@kernel.org>

Rob

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

[Rob Herring]

On Mon, May 28, 2018 at 11:01:33AM -0700, Ray Jui wrote:
> Update the SP805 binding document to add optional 'timeout-sec'
> devicetree property

Note "dt-bindings: watchdog: ..." is the preferred subject prefix.

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

Otherwise,

Reviewed-by: Rob Herring <robh@kernel.org>

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

[Rob Herring]

On Mon, May 28, 2018 at 11:01:33AM -0700, Ray Jui wrote:
> Update the SP805 binding document to add optional 'timeout-sec'
> devicetree property

Note "dt-bindings: watchdog: ..." is the preferred subject prefix.

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

Otherwise,

Reviewed-by: Rob Herring <robh@kernel.org>

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

[Guenter Roeck]

On 06/05/2018 12:41 PM, Rob Herring wrote:
> On Mon, May 28, 2018 at 11:01:32AM -0700, Ray Jui wrote:
>> 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
> 
> Would be good to get a ACK from FSL/NXP person on this. It looks to me
> like the driver fetches the wrong clock as it gets the first one and the
> driver really wants 'wdog_clk'. In any case, their dts files should be
> updated.
>

This is really confusing, since he deleted file lists apb_pclk first.
Does the watchdog driver need apb_pclk or wdog_clk ? That isn't clear to me.
arch/arm64/boot/dts/hisilicon/hi3660.dtsi only provides apb_pclk, or at least
it says so. The fsl dts files all have apb_pclk first.

Either case, why are two clocks asked for in the first place ? Are there
situations where the second clock is actually used/useful ?

Guenter

> Reviewed-by: Rob Herring <robh@kernel.org>
> 
> Rob
> --
> 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 v4 1/6] Documentation: DT: Consolidate SP805 binding docs

[Guenter Roeck]

On 06/05/2018 12:41 PM, Rob Herring wrote:
> On Mon, May 28, 2018 at 11:01:32AM -0700, Ray Jui wrote:
>> 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
> 
> Would be good to get a ACK from FSL/NXP person on this. It looks to me
> like the driver fetches the wrong clock as it gets the first one and the
> driver really wants 'wdog_clk'. In any case, their dts files should be
> updated.
>

This is really confusing, since he deleted file lists apb_pclk first.
Does the watchdog driver need apb_pclk or wdog_clk ? That isn't clear to me.
arch/arm64/boot/dts/hisilicon/hi3660.dtsi only provides apb_pclk, or at least
it says so. The fsl dts files all have apb_pclk first.

Either case, why are two clocks asked for in the first place ? Are there
situations where the second clock is actually used/useful ?

Guenter

> Reviewed-by: Rob Herring <robh@kernel.org>
> 
> Rob
> --
> 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 v4 1/6] Documentation: DT: Consolidate SP805 binding docs

[Rob Herring]

On Wed, Jun 6, 2018 at 11:19 AM, Guenter Roeck <linux@roeck-us.net> wrote:
> On 06/05/2018 12:41 PM, Rob Herring wrote:
>>
>> On Mon, May 28, 2018 at 11:01:32AM -0700, Ray Jui wrote:
>>>
>>> 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
>>
>>
>> Would be good to get a ACK from FSL/NXP person on this. It looks to me
>> like the driver fetches the wrong clock as it gets the first one and the
>> driver really wants 'wdog_clk'. In any case, their dts files should be
>> updated.
>>
>
> This is really confusing, since he deleted file lists apb_pclk first.
> Does the watchdog driver need apb_pclk or wdog_clk ? That isn't clear to me.
> arch/arm64/boot/dts/hisilicon/hi3660.dtsi only provides apb_pclk, or at
> least
> it says so.

Note that that clock source is 32KHz. That is obviously a mistake
because no one clocks their bus/register interface at 32KHz. Someone
just filled in something that happened to work.

> The fsl dts files all have apb_pclk first.

It's all kind of a mess, but fortunately one we should be able to clean-up.

The compatible string changes too, but AMBA bus devices don't actually
use the compatible string as they use the ID registers to match. I
suppose some other OS could do things differently. Worth the risk to
clean-up IMO.

>
> Either case, why are two clocks asked for in the first place ? Are there
> situations where the second clock is actually used/useful ?

For clocks, the bus needs "apb_pclk" and the driver just gets the
first clock. The driver is obviously going to want the functional
clock that determines the counter rate. That should

Primecell peripherals are about the only ones that have clear specs
WRT clock inputs. Yet we've still managed to screw them up. There are
2 clocks in the spec, so the DT has (or should have) 2 clocks.

Rob

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

[Rob Herring]

On Wed, Jun 6, 2018 at 11:19 AM, Guenter Roeck <linux@roeck-us.net> wrote:
> On 06/05/2018 12:41 PM, Rob Herring wrote:
>>
>> On Mon, May 28, 2018 at 11:01:32AM -0700, Ray Jui wrote:
>>>
>>> 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
>>
>>
>> Would be good to get a ACK from FSL/NXP person on this. It looks to me
>> like the driver fetches the wrong clock as it gets the first one and the
>> driver really wants 'wdog_clk'. In any case, their dts files should be
>> updated.
>>
>
> This is really confusing, since he deleted file lists apb_pclk first.
> Does the watchdog driver need apb_pclk or wdog_clk ? That isn't clear to me.
> arch/arm64/boot/dts/hisilicon/hi3660.dtsi only provides apb_pclk, or at
> least
> it says so.

Note that that clock source is 32KHz. That is obviously a mistake
because no one clocks their bus/register interface at 32KHz. Someone
just filled in something that happened to work.

> The fsl dts files all have apb_pclk first.

It's all kind of a mess, but fortunately one we should be able to clean-up.

The compatible string changes too, but AMBA bus devices don't actually
use the compatible string as they use the ID registers to match. I
suppose some other OS could do things differently. Worth the risk to
clean-up IMO.

>
> Either case, why are two clocks asked for in the first place ? Are there
> situations where the second clock is actually used/useful ?

For clocks, the bus needs "apb_pclk" and the driver just gets the
first clock. The driver is obviously going to want the functional
clock that determines the counter rate. That should

Primecell peripherals are about the only ones that have clear specs
WRT clock inputs. Yet we've still managed to screw them up. There are
2 clocks in the spec, so the DT has (or should have) 2 clocks.

Rob

Re: [PATCH v4 0/6] Enhance support for the SP805 WDT

[Florian Fainelli]

On 05/28/2018 11:01 AM, Ray Jui wrote:
> 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-v4
> 
> Changes since v3:
>  - Improve description of 'timeout-sec' in the binding document, per
> recommendation from Guenter Roeck
> 
> 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

I can take the last two patches and Guenter would take the first 4 or
would you want to proceed differently?
-- 
Florian

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

[Florian Fainelli]

On 05/28/2018 11:01 AM, Ray Jui wrote:
> 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-v4
> 
> Changes since v3:
>  - Improve description of 'timeout-sec' in the binding document, per
> recommendation from Guenter Roeck
> 
> 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

I can take the last two patches and Guenter would take the first 4 or
would you want to proceed differently?
-- 
Florian

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

[Ray Jui]

On 6/6/2018 9:33 AM, Rob Herring wrote:
> On Wed, Jun 6, 2018 at 11:19 AM, Guenter Roeck <linux@roeck-us.net> wrote:
>> On 06/05/2018 12:41 PM, Rob Herring wrote:
>>>
>>> On Mon, May 28, 2018 at 11:01:32AM -0700, Ray Jui wrote:
>>>>
>>>> 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
>>>
>>>
>>> Would be good to get a ACK from FSL/NXP person on this. It looks to me
>>> like the driver fetches the wrong clock as it gets the first one and the
>>> driver really wants 'wdog_clk'. In any case, their dts files should be
>>> updated.
>>>
>>
>> This is really confusing, since he deleted file lists apb_pclk first.
>> Does the watchdog driver need apb_pclk or wdog_clk ? That isn't clear to me.
>> arch/arm64/boot/dts/hisilicon/hi3660.dtsi only provides apb_pclk, or at
>> least
>> it says so.
> 
> Note that that clock source is 32KHz. That is obviously a mistake
> because no one clocks their bus/register interface at 32KHz. Someone
> just filled in something that happened to work.
> 
>> The fsl dts files all have apb_pclk first.
> 
> It's all kind of a mess, but fortunately one we should be able to clean-up.
> 

It is indeed a mess. Note the SP805 driver only derive one clock from 
DT, and that's not done based on name. As a result, the first clock 
defined in DT will be fetched and the rate calculation will be carried 
out based on that clock rate.

I assumed the clock entries and their names defined in the binding 
document are just placeholders, at least for the 2nd clock.

Based on how the current driver is, the first clock needs to be the 
WDOGCLK for things to work properly.

According to the SP805 TRM, APB clock is the PCLK, that drives the bus 
for register access.

The relationship between WDOGCLK and PCLK is defined as:

- the rising edges of WDOGCLK must be synchronous and
balanced with a rising edge of PCLK

- the WDOGCLK frequency cannot be greater than the PCLK
frequency

> The compatible string changes too, but AMBA bus devices don't actually
> use the compatible string as they use the ID registers to match. I
> suppose some other OS could do things differently. Worth the risk to
> clean-up IMO.
> 
>>
>> Either case, why are two clocks asked for in the first place ? Are there
>> situations where the second clock is actually used/useful ?
> 
> For clocks, the bus needs "apb_pclk" and the driver just gets the
> first clock. The driver is obviously going to want the functional
> clock that determines the counter rate. That should
> 
> Primecell peripherals are about the only ones that have clear specs
> WRT clock inputs. Yet we've still managed to screw them up. There are
> 2 clocks in the spec, so the DT has (or should have) 2 clocks.
> 
> Rob
> 

Let me know how you guys want to proceed with this?

Thanks,

Ray

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

[Ray Jui]

On 6/6/2018 9:33 AM, Rob Herring wrote:
> On Wed, Jun 6, 2018 at 11:19 AM, Guenter Roeck <linux@roeck-us.net> wrote:
>> On 06/05/2018 12:41 PM, Rob Herring wrote:
>>>
>>> On Mon, May 28, 2018 at 11:01:32AM -0700, Ray Jui wrote:
>>>>
>>>> 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
>>>
>>>
>>> Would be good to get a ACK from FSL/NXP person on this. It looks to me
>>> like the driver fetches the wrong clock as it gets the first one and the
>>> driver really wants 'wdog_clk'. In any case, their dts files should be
>>> updated.
>>>
>>
>> This is really confusing, since he deleted file lists apb_pclk first.
>> Does the watchdog driver need apb_pclk or wdog_clk ? That isn't clear to me.
>> arch/arm64/boot/dts/hisilicon/hi3660.dtsi only provides apb_pclk, or at
>> least
>> it says so.
> 
> Note that that clock source is 32KHz. That is obviously a mistake
> because no one clocks their bus/register interface at 32KHz. Someone
> just filled in something that happened to work.
> 
>> The fsl dts files all have apb_pclk first.
> 
> It's all kind of a mess, but fortunately one we should be able to clean-up.
> 

It is indeed a mess. Note the SP805 driver only derive one clock from 
DT, and that's not done based on name. As a result, the first clock 
defined in DT will be fetched and the rate calculation will be carried 
out based on that clock rate.

I assumed the clock entries and their names defined in the binding 
document are just placeholders, at least for the 2nd clock.

Based on how the current driver is, the first clock needs to be the 
WDOGCLK for things to work properly.

According to the SP805 TRM, APB clock is the PCLK, that drives the bus 
for register access.

The relationship between WDOGCLK and PCLK is defined as:

- the rising edges of WDOGCLK must be synchronous and
balanced with a rising edge of PCLK

- the WDOGCLK frequency cannot be greater than the PCLK
frequency

> The compatible string changes too, but AMBA bus devices don't actually
> use the compatible string as they use the ID registers to match. I
> suppose some other OS could do things differently. Worth the risk to
> clean-up IMO.
> 
>>
>> Either case, why are two clocks asked for in the first place ? Are there
>> situations where the second clock is actually used/useful ?
> 
> For clocks, the bus needs "apb_pclk" and the driver just gets the
> first clock. The driver is obviously going to want the functional
> clock that determines the counter rate. That should
> 
> Primecell peripherals are about the only ones that have clear specs
> WRT clock inputs. Yet we've still managed to screw them up. There are
> 2 clocks in the spec, so the DT has (or should have) 2 clocks.
> 
> Rob
> 

Let me know how you guys want to proceed with this?

Thanks,

Ray

Re: [PATCH v4 0/6] Enhance support for the SP805 WDT

[Ray Jui]

Hi Florian,

On 6/6/2018 12:29 PM, Florian Fainelli wrote:
> On 05/28/2018 11:01 AM, Ray Jui wrote:
>> 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-v4
>>
>> Changes since v3:
>>   - Improve description of 'timeout-sec' in the binding document, per
>> recommendation from Guenter Roeck
>>
>> 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
> 
> I can take the last two patches and Guenter would take the first 4 or
> would you want to proceed differently?
> 

It looks like we still need to figure out how to proceed based on 
discussions with Rob and Guenter on 1/6?

Thanks,

Ray

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

[Ray Jui]

Hi Florian,

On 6/6/2018 12:29 PM, Florian Fainelli wrote:
> On 05/28/2018 11:01 AM, Ray Jui wrote:
>> 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-v4
>>
>> Changes since v3:
>>   - Improve description of 'timeout-sec' in the binding document, per
>> recommendation from Guenter Roeck
>>
>> 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
> 
> I can take the last two patches and Guenter would take the first 4 or
> would you want to proceed differently?
> 

It looks like we still need to figure out how to proceed based on 
discussions with Rob and Guenter on 1/6?

Thanks,

Ray

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

[Ray Jui]

Hi Guenter/Rob,

Kindly let me know how you want to proceed with this?

Thanks,

Ray

On 6/6/2018 4:39 PM, Ray Jui wrote:
> 
> 
> On 6/6/2018 9:33 AM, Rob Herring wrote:
>> On Wed, Jun 6, 2018 at 11:19 AM, Guenter Roeck <linux@roeck-us.net> 
>> wrote:
>>> On 06/05/2018 12:41 PM, Rob Herring wrote:
>>>>
>>>> On Mon, May 28, 2018 at 11:01:32AM -0700, Ray Jui wrote:
>>>>>
>>>>> 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
>>>>
>>>>
>>>> Would be good to get a ACK from FSL/NXP person on this. It looks to me
>>>> like the driver fetches the wrong clock as it gets the first one and 
>>>> the
>>>> driver really wants 'wdog_clk'. In any case, their dts files should be
>>>> updated.
>>>>
>>>
>>> This is really confusing, since he deleted file lists apb_pclk first.
>>> Does the watchdog driver need apb_pclk or wdog_clk ? That isn't clear 
>>> to me.
>>> arch/arm64/boot/dts/hisilicon/hi3660.dtsi only provides apb_pclk, or at
>>> least
>>> it says so.
>>
>> Note that that clock source is 32KHz. That is obviously a mistake
>> because no one clocks their bus/register interface at 32KHz. Someone
>> just filled in something that happened to work.
>>
>>> The fsl dts files all have apb_pclk first.
>>
>> It's all kind of a mess, but fortunately one we should be able to 
>> clean-up.
>>
> 
> It is indeed a mess. Note the SP805 driver only derive one clock from 
> DT, and that's not done based on name. As a result, the first clock 
> defined in DT will be fetched and the rate calculation will be carried 
> out based on that clock rate.
> 
> I assumed the clock entries and their names defined in the binding 
> document are just placeholders, at least for the 2nd clock.
> 
> Based on how the current driver is, the first clock needs to be the 
> WDOGCLK for things to work properly.
> 
> According to the SP805 TRM, APB clock is the PCLK, that drives the bus 
> for register access.
> 
> The relationship between WDOGCLK and PCLK is defined as:
> 
> - the rising edges of WDOGCLK must be synchronous and
> balanced with a rising edge of PCLK
> 
> - the WDOGCLK frequency cannot be greater than the PCLK
> frequency
> 
>> The compatible string changes too, but AMBA bus devices don't actually
>> use the compatible string as they use the ID registers to match. I
>> suppose some other OS could do things differently. Worth the risk to
>> clean-up IMO.
>>
>>>
>>> Either case, why are two clocks asked for in the first place ? Are there
>>> situations where the second clock is actually used/useful ?
>>
>> For clocks, the bus needs "apb_pclk" and the driver just gets the
>> first clock. The driver is obviously going to want the functional
>> clock that determines the counter rate. That should
>>
>> Primecell peripherals are about the only ones that have clear specs
>> WRT clock inputs. Yet we've still managed to screw them up. There are
>> 2 clocks in the spec, so the DT has (or should have) 2 clocks.
>>
>> Rob
>>
> 
> Let me know how you guys want to proceed with this?
> 
> Thanks,
> 
> Ray

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

[Ray Jui]

Hi Guenter/Rob,

Kindly let me know how you want to proceed with this?

Thanks,

Ray

On 6/6/2018 4:39 PM, Ray Jui wrote:
> 
> 
> On 6/6/2018 9:33 AM, Rob Herring wrote:
>> On Wed, Jun 6, 2018 at 11:19 AM, Guenter Roeck <linux@roeck-us.net> 
>> wrote:
>>> On 06/05/2018 12:41 PM, Rob Herring wrote:
>>>>
>>>> On Mon, May 28, 2018 at 11:01:32AM -0700, Ray Jui wrote:
>>>>>
>>>>> 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
>>>>
>>>>
>>>> Would be good to get a ACK from FSL/NXP person on this. It looks to me
>>>> like the driver fetches the wrong clock as it gets the first one and 
>>>> the
>>>> driver really wants 'wdog_clk'. In any case, their dts files should be
>>>> updated.
>>>>
>>>
>>> This is really confusing, since he deleted file lists apb_pclk first.
>>> Does the watchdog driver need apb_pclk or wdog_clk ? That isn't clear 
>>> to me.
>>> arch/arm64/boot/dts/hisilicon/hi3660.dtsi only provides apb_pclk, or at
>>> least
>>> it says so.
>>
>> Note that that clock source is 32KHz. That is obviously a mistake
>> because no one clocks their bus/register interface at 32KHz. Someone
>> just filled in something that happened to work.
>>
>>> The fsl dts files all have apb_pclk first.
>>
>> It's all kind of a mess, but fortunately one we should be able to 
>> clean-up.
>>
> 
> It is indeed a mess. Note the SP805 driver only derive one clock from 
> DT, and that's not done based on name. As a result, the first clock 
> defined in DT will be fetched and the rate calculation will be carried 
> out based on that clock rate.
> 
> I assumed the clock entries and their names defined in the binding 
> document are just placeholders, at least for the 2nd clock.
> 
> Based on how the current driver is, the first clock needs to be the 
> WDOGCLK for things to work properly.
> 
> According to the SP805 TRM, APB clock is the PCLK, that drives the bus 
> for register access.
> 
> The relationship between WDOGCLK and PCLK is defined as:
> 
> - the rising edges of WDOGCLK must be synchronous and
> balanced with a rising edge of PCLK
> 
> - the WDOGCLK frequency cannot be greater than the PCLK
> frequency
> 
>> The compatible string changes too, but AMBA bus devices don't actually
>> use the compatible string as they use the ID registers to match. I
>> suppose some other OS could do things differently. Worth the risk to
>> clean-up IMO.
>>
>>>
>>> Either case, why are two clocks asked for in the first place ? Are there
>>> situations where the second clock is actually used/useful ?
>>
>> For clocks, the bus needs "apb_pclk" and the driver just gets the
>> first clock. The driver is obviously going to want the functional
>> clock that determines the counter rate. That should
>>
>> Primecell peripherals are about the only ones that have clear specs
>> WRT clock inputs. Yet we've still managed to screw them up. There are
>> 2 clocks in the spec, so the DT has (or should have) 2 clocks.
>>
>> Rob
>>
> 
> Let me know how you guys want to proceed with this?
> 
> Thanks,
> 
> Ray

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

[Guenter Roeck]

On Wed, Jun 20, 2018 at 10:39:16AM -0700, Ray Jui wrote:
> Hi Guenter/Rob,
> 
> Kindly let me know how you want to proceed with this?
> 

If I recall correctly, the patch series does not add a new problem
but merely exposes one. Is my recollection correct ? If so, maybe
we should just add a note somewhere indicating what might be wrong
and otherwise apply the series.

Does this make sense ?

Guenter

> Thanks,
> 
> Ray
> 
> On 6/6/2018 4:39 PM, Ray Jui wrote:
> >
> >
> >On 6/6/2018 9:33 AM, Rob Herring wrote:
> >>On Wed, Jun 6, 2018 at 11:19 AM, Guenter Roeck <linux@roeck-us.net>
> >>wrote:
> >>>On 06/05/2018 12:41 PM, Rob Herring wrote:
> >>>>
> >>>>On Mon, May 28, 2018 at 11:01:32AM -0700, Ray Jui wrote:
> >>>>>
> >>>>>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
> >>>>
> >>>>
> >>>>Would be good to get a ACK from FSL/NXP person on this. It looks to me
> >>>>like the driver fetches the wrong clock as it gets the first one and
> >>>>the
> >>>>driver really wants 'wdog_clk'. In any case, their dts files should be
> >>>>updated.
> >>>>
> >>>
> >>>This is really confusing, since he deleted file lists apb_pclk first.
> >>>Does the watchdog driver need apb_pclk or wdog_clk ? That isn't clear
> >>>to me.
> >>>arch/arm64/boot/dts/hisilicon/hi3660.dtsi only provides apb_pclk, or at
> >>>least
> >>>it says so.
> >>
> >>Note that that clock source is 32KHz. That is obviously a mistake
> >>because no one clocks their bus/register interface at 32KHz. Someone
> >>just filled in something that happened to work.
> >>
> >>>The fsl dts files all have apb_pclk first.
> >>
> >>It's all kind of a mess, but fortunately one we should be able to
> >>clean-up.
> >>
> >
> >It is indeed a mess. Note the SP805 driver only derive one clock from DT,
> >and that's not done based on name. As a result, the first clock defined in
> >DT will be fetched and the rate calculation will be carried out based on
> >that clock rate.
> >
> >I assumed the clock entries and their names defined in the binding
> >document are just placeholders, at least for the 2nd clock.
> >
> >Based on how the current driver is, the first clock needs to be the
> >WDOGCLK for things to work properly.
> >
> >According to the SP805 TRM, APB clock is the PCLK, that drives the bus for
> >register access.
> >
> >The relationship between WDOGCLK and PCLK is defined as:
> >
> >- the rising edges of WDOGCLK must be synchronous and
> >balanced with a rising edge of PCLK
> >
> >- the WDOGCLK frequency cannot be greater than the PCLK
> >frequency
> >
> >>The compatible string changes too, but AMBA bus devices don't actually
> >>use the compatible string as they use the ID registers to match. I
> >>suppose some other OS could do things differently. Worth the risk to
> >>clean-up IMO.
> >>
> >>>
> >>>Either case, why are two clocks asked for in the first place ? Are there
> >>>situations where the second clock is actually used/useful ?
> >>
> >>For clocks, the bus needs "apb_pclk" and the driver just gets the
> >>first clock. The driver is obviously going to want the functional
> >>clock that determines the counter rate. That should
> >>
> >>Primecell peripherals are about the only ones that have clear specs
> >>WRT clock inputs. Yet we've still managed to screw them up. There are
> >>2 clocks in the spec, so the DT has (or should have) 2 clocks.
> >>
> >>Rob
> >>
> >
> >Let me know how you guys want to proceed with this?
> >
> >Thanks,
> >
> >Ray
> --
> 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 v4 1/6] Documentation: DT: Consolidate SP805 binding docs

[Guenter Roeck]

On Wed, Jun 20, 2018 at 10:39:16AM -0700, Ray Jui wrote:
> Hi Guenter/Rob,
> 
> Kindly let me know how you want to proceed with this?
> 

If I recall correctly, the patch series does not add a new problem
but merely exposes one. Is my recollection correct ? If so, maybe
we should just add a note somewhere indicating what might be wrong
and otherwise apply the series.

Does this make sense ?

Guenter

> Thanks,
> 
> Ray
> 
> On 6/6/2018 4:39 PM, Ray Jui wrote:
> >
> >
> >On 6/6/2018 9:33 AM, Rob Herring wrote:
> >>On Wed, Jun 6, 2018 at 11:19 AM, Guenter Roeck <linux@roeck-us.net>
> >>wrote:
> >>>On 06/05/2018 12:41 PM, Rob Herring wrote:
> >>>>
> >>>>On Mon, May 28, 2018 at 11:01:32AM -0700, Ray Jui wrote:
> >>>>>
> >>>>>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
> >>>>
> >>>>
> >>>>Would be good to get a ACK from FSL/NXP person on this. It looks to me
> >>>>like the driver fetches the wrong clock as it gets the first one and
> >>>>the
> >>>>driver really wants 'wdog_clk'. In any case, their dts files should be
> >>>>updated.
> >>>>
> >>>
> >>>This is really confusing, since he deleted file lists apb_pclk first.
> >>>Does the watchdog driver need apb_pclk or wdog_clk ? That isn't clear
> >>>to me.
> >>>arch/arm64/boot/dts/hisilicon/hi3660.dtsi only provides apb_pclk, or at
> >>>least
> >>>it says so.
> >>
> >>Note that that clock source is 32KHz. That is obviously a mistake
> >>because no one clocks their bus/register interface at 32KHz. Someone
> >>just filled in something that happened to work.
> >>
> >>>The fsl dts files all have apb_pclk first.
> >>
> >>It's all kind of a mess, but fortunately one we should be able to
> >>clean-up.
> >>
> >
> >It is indeed a mess. Note the SP805 driver only derive one clock from DT,
> >and that's not done based on name. As a result, the first clock defined in
> >DT will be fetched and the rate calculation will be carried out based on
> >that clock rate.
> >
> >I assumed the clock entries and their names defined in the binding
> >document are just placeholders, at least for the 2nd clock.
> >
> >Based on how the current driver is, the first clock needs to be the
> >WDOGCLK for things to work properly.
> >
> >According to the SP805 TRM, APB clock is the PCLK, that drives the bus for
> >register access.
> >
> >The relationship between WDOGCLK and PCLK is defined as:
> >
> >- the rising edges of WDOGCLK must be synchronous and
> >balanced with a rising edge of PCLK
> >
> >- the WDOGCLK frequency cannot be greater than the PCLK
> >frequency
> >
> >>The compatible string changes too, but AMBA bus devices don't actually
> >>use the compatible string as they use the ID registers to match. I
> >>suppose some other OS could do things differently. Worth the risk to
> >>clean-up IMO.
> >>
> >>>
> >>>Either case, why are two clocks asked for in the first place ? Are there
> >>>situations where the second clock is actually used/useful ?
> >>
> >>For clocks, the bus needs "apb_pclk" and the driver just gets the
> >>first clock. The driver is obviously going to want the functional
> >>clock that determines the counter rate. That should
> >>
> >>Primecell peripherals are about the only ones that have clear specs
> >>WRT clock inputs. Yet we've still managed to screw them up. There are
> >>2 clocks in the spec, so the DT has (or should have) 2 clocks.
> >>
> >>Rob
> >>
> >
> >Let me know how you guys want to proceed with this?
> >
> >Thanks,
> >
> >Ray
> --
> 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 v4 1/6] Documentation: DT: Consolidate SP805 binding docs

[Ray Jui]

On 6/27/2018 11:33 AM, Guenter Roeck wrote:
> On Wed, Jun 20, 2018 at 10:39:16AM -0700, Ray Jui wrote:
>> Hi Guenter/Rob,
>>
>> Kindly let me know how you want to proceed with this?
>>
> 
> If I recall correctly, the patch series does not add a new problem
> but merely exposes one. Is my recollection correct ? If so, maybe
> we should just add a note somewhere indicating what might be wrong
> and otherwise apply the series.
> 
> Does this make sense ?

Yes this makes a lot of sense to me. This patch series exposes potential 
problems in some SoCs that they might not be feeding the correct clock 
into WDT, at least based on clock names from their DT entries.

This patch series does not change/affect how SP805 works on those systems.

Where should the note be added?

Many thanks!

Ray

> 
> Guenter
> 
>> Thanks,
>>
>> Ray
>>
>> On 6/6/2018 4:39 PM, Ray Jui wrote:
>>>
>>>
>>> On 6/6/2018 9:33 AM, Rob Herring wrote:
>>>> On Wed, Jun 6, 2018 at 11:19 AM, Guenter Roeck <linux@roeck-us.net>
>>>> wrote:
>>>>> On 06/05/2018 12:41 PM, Rob Herring wrote:
>>>>>>
>>>>>> On Mon, May 28, 2018 at 11:01:32AM -0700, Ray Jui wrote:
>>>>>>>
>>>>>>> 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
>>>>>>
>>>>>>
>>>>>> Would be good to get a ACK from FSL/NXP person on this. It looks to me
>>>>>> like the driver fetches the wrong clock as it gets the first one and
>>>>>> the
>>>>>> driver really wants 'wdog_clk'. In any case, their dts files should be
>>>>>> updated.
>>>>>>
>>>>>
>>>>> This is really confusing, since he deleted file lists apb_pclk first.
>>>>> Does the watchdog driver need apb_pclk or wdog_clk ? That isn't clear
>>>>> to me.
>>>>> arch/arm64/boot/dts/hisilicon/hi3660.dtsi only provides apb_pclk, or at
>>>>> least
>>>>> it says so.
>>>>
>>>> Note that that clock source is 32KHz. That is obviously a mistake
>>>> because no one clocks their bus/register interface at 32KHz. Someone
>>>> just filled in something that happened to work.
>>>>
>>>>> The fsl dts files all have apb_pclk first.
>>>>
>>>> It's all kind of a mess, but fortunately one we should be able to
>>>> clean-up.
>>>>
>>>
>>> It is indeed a mess. Note the SP805 driver only derive one clock from DT,
>>> and that's not done based on name. As a result, the first clock defined in
>>> DT will be fetched and the rate calculation will be carried out based on
>>> that clock rate.
>>>
>>> I assumed the clock entries and their names defined in the binding
>>> document are just placeholders, at least for the 2nd clock.
>>>
>>> Based on how the current driver is, the first clock needs to be the
>>> WDOGCLK for things to work properly.
>>>
>>> According to the SP805 TRM, APB clock is the PCLK, that drives the bus for
>>> register access.
>>>
>>> The relationship between WDOGCLK and PCLK is defined as:
>>>
>>> - the rising edges of WDOGCLK must be synchronous and
>>> balanced with a rising edge of PCLK
>>>
>>> - the WDOGCLK frequency cannot be greater than the PCLK
>>> frequency
>>>
>>>> The compatible string changes too, but AMBA bus devices don't actually
>>>> use the compatible string as they use the ID registers to match. I
>>>> suppose some other OS could do things differently. Worth the risk to
>>>> clean-up IMO.
>>>>
>>>>>
>>>>> Either case, why are two clocks asked for in the first place ? Are there
>>>>> situations where the second clock is actually used/useful ?
>>>>
>>>> For clocks, the bus needs "apb_pclk" and the driver just gets the
>>>> first clock. The driver is obviously going to want the functional
>>>> clock that determines the counter rate. That should
>>>>
>>>> Primecell peripherals are about the only ones that have clear specs
>>>> WRT clock inputs. Yet we've still managed to screw them up. There are
>>>> 2 clocks in the spec, so the DT has (or should have) 2 clocks.
>>>>
>>>> Rob
>>>>
>>>
>>> Let me know how you guys want to proceed with this?
>>>
>>> Thanks,
>>>
>>> Ray
>> --
>> 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 v4 1/6] Documentation: DT: Consolidate SP805 binding docs

[Ray Jui]

On 6/27/2018 11:33 AM, Guenter Roeck wrote:
> On Wed, Jun 20, 2018 at 10:39:16AM -0700, Ray Jui wrote:
>> Hi Guenter/Rob,
>>
>> Kindly let me know how you want to proceed with this?
>>
> 
> If I recall correctly, the patch series does not add a new problem
> but merely exposes one. Is my recollection correct ? If so, maybe
> we should just add a note somewhere indicating what might be wrong
> and otherwise apply the series.
> 
> Does this make sense ?

Yes this makes a lot of sense to me. This patch series exposes potential 
problems in some SoCs that they might not be feeding the correct clock 
into WDT, at least based on clock names from their DT entries.

This patch series does not change/affect how SP805 works on those systems.

Where should the note be added?

Many thanks!

Ray

> 
> Guenter
> 
>> Thanks,
>>
>> Ray
>>
>> On 6/6/2018 4:39 PM, Ray Jui wrote:
>>>
>>>
>>> On 6/6/2018 9:33 AM, Rob Herring wrote:
>>>> On Wed, Jun 6, 2018 at 11:19 AM, Guenter Roeck <linux@roeck-us.net>
>>>> wrote:
>>>>> On 06/05/2018 12:41 PM, Rob Herring wrote:
>>>>>>
>>>>>> On Mon, May 28, 2018 at 11:01:32AM -0700, Ray Jui wrote:
>>>>>>>
>>>>>>> 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
>>>>>>
>>>>>>
>>>>>> Would be good to get a ACK from FSL/NXP person on this. It looks to me
>>>>>> like the driver fetches the wrong clock as it gets the first one and
>>>>>> the
>>>>>> driver really wants 'wdog_clk'. In any case, their dts files should be
>>>>>> updated.
>>>>>>
>>>>>
>>>>> This is really confusing, since he deleted file lists apb_pclk first.
>>>>> Does the watchdog driver need apb_pclk or wdog_clk ? That isn't clear
>>>>> to me.
>>>>> arch/arm64/boot/dts/hisilicon/hi3660.dtsi only provides apb_pclk, or at
>>>>> least
>>>>> it says so.
>>>>
>>>> Note that that clock source is 32KHz. That is obviously a mistake
>>>> because no one clocks their bus/register interface at 32KHz. Someone
>>>> just filled in something that happened to work.
>>>>
>>>>> The fsl dts files all have apb_pclk first.
>>>>
>>>> It's all kind of a mess, but fortunately one we should be able to
>>>> clean-up.
>>>>
>>>
>>> It is indeed a mess. Note the SP805 driver only derive one clock from DT,
>>> and that's not done based on name. As a result, the first clock defined in
>>> DT will be fetched and the rate calculation will be carried out based on
>>> that clock rate.
>>>
>>> I assumed the clock entries and their names defined in the binding
>>> document are just placeholders, at least for the 2nd clock.
>>>
>>> Based on how the current driver is, the first clock needs to be the
>>> WDOGCLK for things to work properly.
>>>
>>> According to the SP805 TRM, APB clock is the PCLK, that drives the bus for
>>> register access.
>>>
>>> The relationship between WDOGCLK and PCLK is defined as:
>>>
>>> - the rising edges of WDOGCLK must be synchronous and
>>> balanced with a rising edge of PCLK
>>>
>>> - the WDOGCLK frequency cannot be greater than the PCLK
>>> frequency
>>>
>>>> The compatible string changes too, but AMBA bus devices don't actually
>>>> use the compatible string as they use the ID registers to match. I
>>>> suppose some other OS could do things differently. Worth the risk to
>>>> clean-up IMO.
>>>>
>>>>>
>>>>> Either case, why are two clocks asked for in the first place ? Are there
>>>>> situations where the second clock is actually used/useful ?
>>>>
>>>> For clocks, the bus needs "apb_pclk" and the driver just gets the
>>>> first clock. The driver is obviously going to want the functional
>>>> clock that determines the counter rate. That should
>>>>
>>>> Primecell peripherals are about the only ones that have clear specs
>>>> WRT clock inputs. Yet we've still managed to screw them up. There are
>>>> 2 clocks in the spec, so the DT has (or should have) 2 clocks.
>>>>
>>>> Rob
>>>>
>>>
>>> Let me know how you guys want to proceed with this?
>>>
>>> Thanks,
>>>
>>> Ray
>> --
>> 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 v4 1/6] Documentation: DT: Consolidate SP805 binding docs

[Guenter Roeck]

On Wed, Jun 27, 2018 at 11:38:48AM -0700, Ray Jui wrote:
> 
> 
> On 6/27/2018 11:33 AM, Guenter Roeck wrote:
> >On Wed, Jun 20, 2018 at 10:39:16AM -0700, Ray Jui wrote:
> >>Hi Guenter/Rob,
> >>
> >>Kindly let me know how you want to proceed with this?
> >>
> >
> >If I recall correctly, the patch series does not add a new problem
> >but merely exposes one. Is my recollection correct ? If so, maybe
> >we should just add a note somewhere indicating what might be wrong
> >and otherwise apply the series.
> >
> >Does this make sense ?
> 
> Yes this makes a lot of sense to me. This patch series exposes potential
> problems in some SoCs that they might not be feeding the correct clock into
> WDT, at least based on clock names from their DT entries.
> 
> This patch series does not change/affect how SP805 works on those systems.
> 
> Where should the note be added?
> 

I would suggest to add a note into the driver where the clock is used,
with the details discussed here.

Does this make sense ?

Thanks,
Guenter

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

[Guenter Roeck]

On Wed, Jun 27, 2018 at 11:38:48AM -0700, Ray Jui wrote:
> 
> 
> On 6/27/2018 11:33 AM, Guenter Roeck wrote:
> >On Wed, Jun 20, 2018 at 10:39:16AM -0700, Ray Jui wrote:
> >>Hi Guenter/Rob,
> >>
> >>Kindly let me know how you want to proceed with this?
> >>
> >
> >If I recall correctly, the patch series does not add a new problem
> >but merely exposes one. Is my recollection correct ? If so, maybe
> >we should just add a note somewhere indicating what might be wrong
> >and otherwise apply the series.
> >
> >Does this make sense ?
> 
> Yes this makes a lot of sense to me. This patch series exposes potential
> problems in some SoCs that they might not be feeding the correct clock into
> WDT, at least based on clock names from their DT entries.
> 
> This patch series does not change/affect how SP805 works on those systems.
> 
> Where should the note be added?
> 

I would suggest to add a note into the driver where the clock is used,
with the details discussed here.

Does this make sense ?

Thanks,
Guenter

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

[Ray Jui]

On 6/27/2018 11:42 AM, Guenter Roeck wrote:
> On Wed, Jun 27, 2018 at 11:38:48AM -0700, Ray Jui wrote:
>>
>>
>> On 6/27/2018 11:33 AM, Guenter Roeck wrote:
>>> On Wed, Jun 20, 2018 at 10:39:16AM -0700, Ray Jui wrote:
>>>> Hi Guenter/Rob,
>>>>
>>>> Kindly let me know how you want to proceed with this?
>>>>
>>>
>>> If I recall correctly, the patch series does not add a new problem
>>> but merely exposes one. Is my recollection correct ? If so, maybe
>>> we should just add a note somewhere indicating what might be wrong
>>> and otherwise apply the series.
>>>
>>> Does this make sense ?
>>
>> Yes this makes a lot of sense to me. This patch series exposes potential
>> problems in some SoCs that they might not be feeding the correct clock into
>> WDT, at least based on clock names from their DT entries.
>>
>> This patch series does not change/affect how SP805 works on those systems.
>>
>> Where should the note be added?
>>
> 
> I would suggest to add a note into the driver where the clock is used,
> with the details discussed here.

I assume you meant adding the notes to the SP805 driver where the clock 
is used.

If so, I think that makes sense. That notes deserves its own patch 
because it really has nothing to do with any of the change in this patch 
series.

Do you want me to 1) embed that patch into this patch series and send 
out v5; or 2) leave the patch series as it is and send out a separate 
patch to add the notes to the driver?

Thanks.

> 
> Does this make sense ?
> 
> Thanks,
> Guenter
> 

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

[Ray Jui]

On 6/27/2018 11:42 AM, Guenter Roeck wrote:
> On Wed, Jun 27, 2018 at 11:38:48AM -0700, Ray Jui wrote:
>>
>>
>> On 6/27/2018 11:33 AM, Guenter Roeck wrote:
>>> On Wed, Jun 20, 2018 at 10:39:16AM -0700, Ray Jui wrote:
>>>> Hi Guenter/Rob,
>>>>
>>>> Kindly let me know how you want to proceed with this?
>>>>
>>>
>>> If I recall correctly, the patch series does not add a new problem
>>> but merely exposes one. Is my recollection correct ? If so, maybe
>>> we should just add a note somewhere indicating what might be wrong
>>> and otherwise apply the series.
>>>
>>> Does this make sense ?
>>
>> Yes this makes a lot of sense to me. This patch series exposes potential
>> problems in some SoCs that they might not be feeding the correct clock into
>> WDT, at least based on clock names from their DT entries.
>>
>> This patch series does not change/affect how SP805 works on those systems.
>>
>> Where should the note be added?
>>
> 
> I would suggest to add a note into the driver where the clock is used,
> with the details discussed here.

I assume you meant adding the notes to the SP805 driver where the clock 
is used.

If so, I think that makes sense. That notes deserves its own patch 
because it really has nothing to do with any of the change in this patch 
series.

Do you want me to 1) embed that patch into this patch series and send 
out v5; or 2) leave the patch series as it is and send out a separate 
patch to add the notes to the driver?

Thanks.

> 
> Does this make sense ?
> 
> Thanks,
> Guenter
> 

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

[Guenter Roeck]

On Wed, Jun 27, 2018 at 11:47:21AM -0700, Ray Jui wrote:
> 
> 
> On 6/27/2018 11:42 AM, Guenter Roeck wrote:
> >On Wed, Jun 27, 2018 at 11:38:48AM -0700, Ray Jui wrote:
> >>
> >>
> >>On 6/27/2018 11:33 AM, Guenter Roeck wrote:
> >>>On Wed, Jun 20, 2018 at 10:39:16AM -0700, Ray Jui wrote:
> >>>>Hi Guenter/Rob,
> >>>>
> >>>>Kindly let me know how you want to proceed with this?
> >>>>
> >>>
> >>>If I recall correctly, the patch series does not add a new problem
> >>>but merely exposes one. Is my recollection correct ? If so, maybe
> >>>we should just add a note somewhere indicating what might be wrong
> >>>and otherwise apply the series.
> >>>
> >>>Does this make sense ?
> >>
> >>Yes this makes a lot of sense to me. This patch series exposes potential
> >>problems in some SoCs that they might not be feeding the correct clock into
> >>WDT, at least based on clock names from their DT entries.
> >>
> >>This patch series does not change/affect how SP805 works on those systems.
> >>
> >>Where should the note be added?
> >>
> >
> >I would suggest to add a note into the driver where the clock is used,
> >with the details discussed here.
> 
> I assume you meant adding the notes to the SP805 driver where the clock is
> used.
> 
> If so, I think that makes sense. That notes deserves its own patch because
> it really has nothing to do with any of the change in this patch series.
> 
> Do you want me to 1) embed that patch into this patch series and send out
> v5; or 2) leave the patch series as it is and send out a separate patch to
> add the notes to the driver?
> 
2) is fine. I don't have the series here right now; if I recall correctly
all patches in the series are all marked as Reviewed-by: and/or Acked-by:.
If so, I'll apply them to my tree tonight, or at least the ones that will
go in through the watchdog tree.

Guenter

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

[Guenter Roeck]

On Wed, Jun 27, 2018 at 11:47:21AM -0700, Ray Jui wrote:
> 
> 
> On 6/27/2018 11:42 AM, Guenter Roeck wrote:
> >On Wed, Jun 27, 2018 at 11:38:48AM -0700, Ray Jui wrote:
> >>
> >>
> >>On 6/27/2018 11:33 AM, Guenter Roeck wrote:
> >>>On Wed, Jun 20, 2018 at 10:39:16AM -0700, Ray Jui wrote:
> >>>>Hi Guenter/Rob,
> >>>>
> >>>>Kindly let me know how you want to proceed with this?
> >>>>
> >>>
> >>>If I recall correctly, the patch series does not add a new problem
> >>>but merely exposes one. Is my recollection correct ? If so, maybe
> >>>we should just add a note somewhere indicating what might be wrong
> >>>and otherwise apply the series.
> >>>
> >>>Does this make sense ?
> >>
> >>Yes this makes a lot of sense to me. This patch series exposes potential
> >>problems in some SoCs that they might not be feeding the correct clock into
> >>WDT, at least based on clock names from their DT entries.
> >>
> >>This patch series does not change/affect how SP805 works on those systems.
> >>
> >>Where should the note be added?
> >>
> >
> >I would suggest to add a note into the driver where the clock is used,
> >with the details discussed here.
> 
> I assume you meant adding the notes to the SP805 driver where the clock is
> used.
> 
> If so, I think that makes sense. That notes deserves its own patch because
> it really has nothing to do with any of the change in this patch series.
> 
> Do you want me to 1) embed that patch into this patch series and send out
> v5; or 2) leave the patch series as it is and send out a separate patch to
> add the notes to the driver?
> 
2) is fine. I don't have the series here right now; if I recall correctly
all patches in the series are all marked as Reviewed-by: and/or Acked-by:.
If so, I'll apply them to my tree tonight, or at least the ones that will
go in through the watchdog tree.

Guenter

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

[Ray Jui]

Hi Guenter/Florian,

On 6/27/2018 11:55 AM, Guenter Roeck wrote:
> On Wed, Jun 27, 2018 at 11:47:21AM -0700, Ray Jui wrote:
>>
>>
>> On 6/27/2018 11:42 AM, Guenter Roeck wrote:
>>> On Wed, Jun 27, 2018 at 11:38:48AM -0700, Ray Jui wrote:
>>>>
>>>>
>>>> On 6/27/2018 11:33 AM, Guenter Roeck wrote:
>>>>> On Wed, Jun 20, 2018 at 10:39:16AM -0700, Ray Jui wrote:
>>>>>> Hi Guenter/Rob,
>>>>>>
>>>>>> Kindly let me know how you want to proceed with this?
>>>>>>
>>>>>
>>>>> If I recall correctly, the patch series does not add a new problem
>>>>> but merely exposes one. Is my recollection correct ? If so, maybe
>>>>> we should just add a note somewhere indicating what might be wrong
>>>>> and otherwise apply the series.
>>>>>
>>>>> Does this make sense ?
>>>>
>>>> Yes this makes a lot of sense to me. This patch series exposes potential
>>>> problems in some SoCs that they might not be feeding the correct clock into
>>>> WDT, at least based on clock names from their DT entries.
>>>>
>>>> This patch series does not change/affect how SP805 works on those systems.
>>>>
>>>> Where should the note be added?
>>>>
>>>
>>> I would suggest to add a note into the driver where the clock is used,
>>> with the details discussed here.
>>
>> I assume you meant adding the notes to the SP805 driver where the clock is
>> used.
>>
>> If so, I think that makes sense. That notes deserves its own patch because
>> it really has nothing to do with any of the change in this patch series.
>>
>> Do you want me to 1) embed that patch into this patch series and send out
>> v5; or 2) leave the patch series as it is and send out a separate patch to
>> add the notes to the driver?
>>
> 2) is fine. I don't have the series here right now; if I recall correctly
> all patches in the series are all marked as Reviewed-by: and/or Acked-by:.
> If so, I'll apply them to my tree tonight, or at least the ones that will
> go in through the watchdog tree.
> 
> Guenter
> 

Guenter, please let us know once you take the first 4 patches.

Florian, could you please help with the last 2, once Guenter confirmed 
the first 4 are merged.

Thanks!

Ray

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

[Ray Jui]

Hi Guenter/Florian,

On 6/27/2018 11:55 AM, Guenter Roeck wrote:
> On Wed, Jun 27, 2018 at 11:47:21AM -0700, Ray Jui wrote:
>>
>>
>> On 6/27/2018 11:42 AM, Guenter Roeck wrote:
>>> On Wed, Jun 27, 2018 at 11:38:48AM -0700, Ray Jui wrote:
>>>>
>>>>
>>>> On 6/27/2018 11:33 AM, Guenter Roeck wrote:
>>>>> On Wed, Jun 20, 2018 at 10:39:16AM -0700, Ray Jui wrote:
>>>>>> Hi Guenter/Rob,
>>>>>>
>>>>>> Kindly let me know how you want to proceed with this?
>>>>>>
>>>>>
>>>>> If I recall correctly, the patch series does not add a new problem
>>>>> but merely exposes one. Is my recollection correct ? If so, maybe
>>>>> we should just add a note somewhere indicating what might be wrong
>>>>> and otherwise apply the series.
>>>>>
>>>>> Does this make sense ?
>>>>
>>>> Yes this makes a lot of sense to me. This patch series exposes potential
>>>> problems in some SoCs that they might not be feeding the correct clock into
>>>> WDT, at least based on clock names from their DT entries.
>>>>
>>>> This patch series does not change/affect how SP805 works on those systems.
>>>>
>>>> Where should the note be added?
>>>>
>>>
>>> I would suggest to add a note into the driver where the clock is used,
>>> with the details discussed here.
>>
>> I assume you meant adding the notes to the SP805 driver where the clock is
>> used.
>>
>> If so, I think that makes sense. That notes deserves its own patch because
>> it really has nothing to do with any of the change in this patch series.
>>
>> Do you want me to 1) embed that patch into this patch series and send out
>> v5; or 2) leave the patch series as it is and send out a separate patch to
>> add the notes to the driver?
>>
> 2) is fine. I don't have the series here right now; if I recall correctly
> all patches in the series are all marked as Reviewed-by: and/or Acked-by:.
> If so, I'll apply them to my tree tonight, or at least the ones that will
> go in through the watchdog tree.
> 
> Guenter
> 

Guenter, please let us know once you take the first 4 patches.

Florian, could you please help with the last 2, once Guenter confirmed 
the first 4 are merged.

Thanks!

Ray

Re: [v4,1/6] Documentation: DT: Consolidate SP805 binding docs

[Guenter Roeck]

On Mon, May 28, 2018 at 11:01:32AM -0700, Ray Jui wrote:
> 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>
> Reviewed-by: Rob Herring <robh@kernel.org>

For the record:

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

> ---
>  .../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";
> -		};
> -

[v4,1/6] Documentation: DT: Consolidate SP805 binding docs

[Guenter Roeck]

On Mon, May 28, 2018 at 11:01:32AM -0700, Ray Jui wrote:
> 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>
> Reviewed-by: Rob Herring <robh@kernel.org>

For the record:

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

> ---
>  .../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";
> -		};
> -

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

[Guenter Roeck]

On Mon, May 28, 2018 at 11:01:33AM -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>
> Reviewed-by: Rob Herring <robh@kernel.org>

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

> ---
>  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..bee6f1f 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 is determined by the driver
>  
>  Example:
>  	watchdog@66090000 {

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

[Guenter Roeck]

On Mon, May 28, 2018 at 11:01:33AM -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>
> Reviewed-by: Rob Herring <robh@kernel.org>

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

> ---
>  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..bee6f1f 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 is determined by the driver
>  
>  Example:
>  	watchdog at 66090000 {

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

[Guenter Roeck]

On 06/28/2018 04:50 PM, Ray Jui wrote:
> Hi Guenter/Florian,
> 
> On 6/27/2018 11:55 AM, Guenter Roeck wrote:
>> On Wed, Jun 27, 2018 at 11:47:21AM -0700, Ray Jui wrote:
>>>
>>>
>>> On 6/27/2018 11:42 AM, Guenter Roeck wrote:
>>>> On Wed, Jun 27, 2018 at 11:38:48AM -0700, Ray Jui wrote:
>>>>>
>>>>>
>>>>> On 6/27/2018 11:33 AM, Guenter Roeck wrote:
>>>>>> On Wed, Jun 20, 2018 at 10:39:16AM -0700, Ray Jui wrote:
>>>>>>> Hi Guenter/Rob,
>>>>>>>
>>>>>>> Kindly let me know how you want to proceed with this?
>>>>>>>
>>>>>>
>>>>>> If I recall correctly, the patch series does not add a new problem
>>>>>> but merely exposes one. Is my recollection correct ? If so, maybe
>>>>>> we should just add a note somewhere indicating what might be wrong
>>>>>> and otherwise apply the series.
>>>>>>
>>>>>> Does this make sense ?
>>>>>
>>>>> Yes this makes a lot of sense to me. This patch series exposes potential
>>>>> problems in some SoCs that they might not be feeding the correct clock into
>>>>> WDT, at least based on clock names from their DT entries.
>>>>>
>>>>> This patch series does not change/affect how SP805 works on those systems.
>>>>>
>>>>> Where should the note be added?
>>>>>
>>>>
>>>> I would suggest to add a note into the driver where the clock is used,
>>>> with the details discussed here.
>>>
>>> I assume you meant adding the notes to the SP805 driver where the clock is
>>> used.
>>>
>>> If so, I think that makes sense. That notes deserves its own patch because
>>> it really has nothing to do with any of the change in this patch series.
>>>
>>> Do you want me to 1) embed that patch into this patch series and send out
>>> v5; or 2) leave the patch series as it is and send out a separate patch to
>>> add the notes to the driver?
>>>
>> 2) is fine. I don't have the series here right now; if I recall correctly
>> all patches in the series are all marked as Reviewed-by: and/or Acked-by:.
>> If so, I'll apply them to my tree tonight, or at least the ones that will
>> go in through the watchdog tree.
>>
>> Guenter
>>
> 
> Guenter, please let us know once you take the first 4 patches.
> 

Patches are in my tree. Wim, it would be great if you can pick up the tree
so it shows up in -next.

Thanks,
Guenter

> Florian, could you please help with the last 2, once Guenter confirmed the first 4 are merged.
> 
> Thanks!
> 
> Ray
> -- 
> 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 v4 1/6] Documentation: DT: Consolidate SP805 binding docs

[Guenter Roeck]

On 06/28/2018 04:50 PM, Ray Jui wrote:
> Hi Guenter/Florian,
> 
> On 6/27/2018 11:55 AM, Guenter Roeck wrote:
>> On Wed, Jun 27, 2018 at 11:47:21AM -0700, Ray Jui wrote:
>>>
>>>
>>> On 6/27/2018 11:42 AM, Guenter Roeck wrote:
>>>> On Wed, Jun 27, 2018 at 11:38:48AM -0700, Ray Jui wrote:
>>>>>
>>>>>
>>>>> On 6/27/2018 11:33 AM, Guenter Roeck wrote:
>>>>>> On Wed, Jun 20, 2018 at 10:39:16AM -0700, Ray Jui wrote:
>>>>>>> Hi Guenter/Rob,
>>>>>>>
>>>>>>> Kindly let me know how you want to proceed with this?
>>>>>>>
>>>>>>
>>>>>> If I recall correctly, the patch series does not add a new problem
>>>>>> but merely exposes one. Is my recollection correct ? If so, maybe
>>>>>> we should just add a note somewhere indicating what might be wrong
>>>>>> and otherwise apply the series.
>>>>>>
>>>>>> Does this make sense ?
>>>>>
>>>>> Yes this makes a lot of sense to me. This patch series exposes potential
>>>>> problems in some SoCs that they might not be feeding the correct clock into
>>>>> WDT, at least based on clock names from their DT entries.
>>>>>
>>>>> This patch series does not change/affect how SP805 works on those systems.
>>>>>
>>>>> Where should the note be added?
>>>>>
>>>>
>>>> I would suggest to add a note into the driver where the clock is used,
>>>> with the details discussed here.
>>>
>>> I assume you meant adding the notes to the SP805 driver where the clock is
>>> used.
>>>
>>> If so, I think that makes sense. That notes deserves its own patch because
>>> it really has nothing to do with any of the change in this patch series.
>>>
>>> Do you want me to 1) embed that patch into this patch series and send out
>>> v5; or 2) leave the patch series as it is and send out a separate patch to
>>> add the notes to the driver?
>>>
>> 2) is fine. I don't have the series here right now; if I recall correctly
>> all patches in the series are all marked as Reviewed-by: and/or Acked-by:.
>> If so, I'll apply them to my tree tonight, or at least the ones that will
>> go in through the watchdog tree.
>>
>> Guenter
>>
> 
> Guenter, please let us know once you take the first 4 patches.
> 

Patches are in my tree. Wim, it would be great if you can pick up the tree
so it shows up in -next.

Thanks,
Guenter

> Florian, could you please help with the last 2, once Guenter confirmed the first 4 are merged.
> 
> Thanks!
> 
> Ray
> -- 
> 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 v4 6/6] arm64: defconfig: add CONFIG_ARM_SP805_WATCHDOG

[Florian Fainelli]

On Mon, 28 May 2018 11:01:37 -0700, Ray Jui <ray.jui@broadcom.com> wrote:
> Enable the SP805 watchdog timer
> 
> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> ---

Applied to defconfig-arm64/next, thanks!
--
Florian

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

[Florian Fainelli]

On Mon, 28 May 2018 11:01:37 -0700, Ray Jui <ray.jui@broadcom.com> wrote:
> Enable the SP805 watchdog timer
> 
> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> ---

Applied to defconfig-arm64/next, thanks!
--
Florian

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

[Florian Fainelli]

On 05/28/2018 11:01 AM, Ray Jui wrote:
> 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>

Applied to devicetree-arm64/next, thanks!
-- 
Florian

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

[Florian Fainelli]

On 05/28/2018 11:01 AM, Ray Jui wrote:
> 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>

Applied to devicetree-arm64/next, thanks!
-- 
Florian