riscv: dts: sophgo: add watchdog dt node for CV1800

Started by AnnanLiu on Jan 25, 2024 · 11 messages from 9 authors · Last reply Feb 14, 2025

Subsystems (2)

Maintainers (4)

STALE630d

Message details and related content

Open message details and related content

Add the watchdog device tree node to cv1800 SoC.

Signed-off-by: AnnanLiu <redacted>
---
This patch depends on the clk driver and reset driver.
Clk driver link:
https://lore.kernel.org/all/IA1PR20MB49539CDAD9A268CBF6CA184BBB9FA@IA1PR20MB4953.namprd20.prod.outlook.com/
Reset driver link:
https://lore.kernel.org/all/20231113005503.2423-1-jszhang@kernel.org/

Changes since v1:
- Change the name of the watchdog from watchdog0 to watchdog.
- Change the status of watchdog.
v1 link:
https://lore.kernel.org/all/DM6PR20MB23160B8499CC2BFDAE6FCACDAB9EA@DM6PR20MB2316.namprd20.prod.outlook.com/


 arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts |  4 ++++
 arch/riscv/boot/dts/sophgo/cv1800b.dtsi          | 16 ++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts b/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts
index 3af9e34b3bc7..75469161bfff 100644
--- a/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts
+++ b/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts

@@ -36,3 +36,7 @@ &osc {
 &uart0 {
 	status = "okay";
 };
+
+&watchdog {
+	status = "okay";
+};

diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
index aec6401a467b..03ca32cd37b6 100644
--- a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
+++ b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi

@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: (GPL-2.0 OR MIT)
 /*
  * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org>
+ * Copyright (C) 2024 Annan Liu <annan.liu.xdu@outlook.com>
  */
 
 #include <dt-bindings/interrupt-controller/irq.h>

@@ -103,6 +104,21 @@ uart4: serial@41c0000 {
 			status = "disabled";
 		};
 
+		watchdog: watchdog@3010000{
+			compatible = "snps,dw-wdt";
+			reg = <0x3010000 0x100>;
+			interrupts = <58 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&pclk>;
+			resets = <&rst RST_WDT>;
+			status = "disabled";
+		};
+
+		pclk: pclk {
+			#clock-cells = <0>;
+			compatible = "fixed-clock";
+			clock-frequency = <25000000>;
+		};
+
 		plic: interrupt-controller@70000000 {
 			compatible = "sophgo,cv1800b-plic", "thead,c900-plic";
 			reg = <0x70000000 0x4000000>;

-- 
2.34.1

Chen Wang replied to AnnanLiu Jan 25, 12:15 #

Message details and related content

Open message details and related content

hi, Annan, I see another email with same title, any difference between them two? Which one you want us to review? Maybe you should void one of them.

On 2024/1/25 17:46, AnnanLiu wrote:

quoted hunk

Add the watchdog device tree node to cv1800 SoC.

Signed-off-by: AnnanLiu <redacted>
---
This patch depends on the clk driver and reset driver.
Clk driver link:
https://lore.kernel.org/all/IA1PR20MB49539CDAD9A268CBF6CA184BBB9FA@IA1PR20MB4953.namprd20.prod.outlook.com/
Reset driver link:
https://lore.kernel.org/all/20231113005503.2423-1-jszhang@kernel.org/

Changes since v1:
- Change the name of the watchdog from watchdog0 to watchdog.
- Change the status of watchdog.
v1 link:
https://lore.kernel.org/all/DM6PR20MB23160B8499CC2BFDAE6FCACDAB9EA@DM6PR20MB2316.namprd20.prod.outlook.com/


  arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts |  4 ++++
  arch/riscv/boot/dts/sophgo/cv1800b.dtsi          | 16 ++++++++++++++++
  2 files changed, 20 insertions(+)

diff --git a/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts b/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts
index 3af9e34b3bc7..75469161bfff 100644
--- a/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts
+++ b/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts

@@ -36,3 +36,7 @@ &osc {
  &uart0 {
  	status = "okay";
  };
+
+&watchdog {
+	status = "okay";
+};

diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
index aec6401a467b..03ca32cd37b6 100644
--- a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
+++ b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi

@@ -1,6 +1,7 @@
  // SPDX-License-Identifier: (GPL-2.0 OR MIT)
  /*

27 more quoted lines

Krzysztof Kozlowski replied to AnnanLiu Jan 25, 12:39 #

Message details and related content

Open message details and related content

On 25/01/2024 10:46, AnnanLiu wrote:

quoted hunk

+
+		pclk: pclk {
+			#clock-cells = <0>;
+			compatible = "fixed-clock";
+			clock-frequency = <25000000>;

It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).

Also, why do you describe internal clock as stub?

Best regards,
Krzysztof

Conor Dooley replied to Chen Wang Jan 25, 17:09 #

Message details and related content

Open message details and related content

On Thu, Jan 25, 2024 at 08:15:32PM +0800, Chen Wang wrote:

hi, Annan, I see another email with same title, any difference between them
two? Which one you want us to review? Maybe you should void one of them.

This one I think, the other did not get sent to the mailing lists.

Conor Dooley replied to Krzysztof Kozlowski Jan 25, 17:10 #

Message details and related content

Open message details and related content

On Thu, Jan 25, 2024 at 01:39:51PM +0100, Krzysztof Kozlowski wrote:

On 25/01/2024 10:46, AnnanLiu wrote:

quoted

+
+		pclk: pclk {
+			#clock-cells = <0>;
+			compatible = "fixed-clock";
+			clock-frequency = <25000000>;

It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).

Also, why do you describe internal clock as stub?

Under the --- line it says the patch depends on the clock series, but
as you pointed out the clock is a "fake" stub, so I don't understand
what the dependency would be.

Emil Renner Berthing replied to AnnanLiu Jan 25, 18:04 #

Message details and related content

Open message details and related content

AnnanLiu wrote:

quoted hunk

Add the watchdog device tree node to cv1800 SoC.

Signed-off-by: AnnanLiu <redacted>
---
This patch depends on the clk driver and reset driver.
Clk driver link:
https://lore.kernel.org/all/IA1PR20MB49539CDAD9A268CBF6CA184BBB9FA@IA1PR20MB4953.namprd20.prod.outlook.com/
Reset driver link:
https://lore.kernel.org/all/20231113005503.2423-1-jszhang@kernel.org/

Changes since v1:
- Change the name of the watchdog from watchdog0 to watchdog.
- Change the status of watchdog.
v1 link:
https://lore.kernel.org/all/DM6PR20MB23160B8499CC2BFDAE6FCACDAB9EA@DM6PR20MB2316.namprd20.prod.outlook.com/


 arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts |  4 ++++
 arch/riscv/boot/dts/sophgo/cv1800b.dtsi          | 16 ++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts b/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts
index 3af9e34b3bc7..75469161bfff 100644
--- a/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts
+++ b/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts

@@ -36,3 +36,7 @@ &osc {
 &uart0 {
 	status = "okay";
 };
+
+&watchdog {
+	status = "okay";
+};

diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
index aec6401a467b..03ca32cd37b6 100644
--- a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
+++ b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi

@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: (GPL-2.0 OR MIT)
 /*

17 more quoted lines

Before this patch the nodes seems to be ordered by their address. This
patch breaks that.

+
+		pclk: pclk {
+			#clock-cells = <0>;
+			compatible = "fixed-clock";
+			clock-frequency = <25000000>;
+		};
+
 		plic: interrupt-controller@70000000 {
 			compatible = "sophgo,cv1800b-plic", "thead,c900-plic";
 			reg = <0x70000000 0x4000000>;
--
2.34.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Inochi Amaoto replied to Krzysztof Kozlowski Jan 29, 07:55 #

Message details and related content

Open message details and related content

On Thu, Jan 25, 2024 at 01:39:51PM +0100, Krzysztof Kozlowski wrote:

quoted

On 25/01/2024 10:46, AnnanLiu wrote:

quoted

+
+		pclk: pclk {
+			#clock-cells = <0>;
+			compatible = "fixed-clock";
+			clock-frequency = <25000000>;

It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).

quoted

Also, why do you describe internal clock as stub?

Under the --- line it says the patch depends on the clock series, but
as you pointed out the clock is a "fake" stub, so I don't understand
what the dependency would be.

This clock should not exist. The watchdog may depend on the clock from
clock controller.

Jisheng Zhang replied to AnnanLiu Feb 17, 16:10 #

Message details and related content

Open message details and related content

On Thu, Jan 25, 2024 at 05:46:40PM +0800, AnnanLiu wrote:

Add the watchdog device tree node to cv1800 SoC.

Signed-off-by: AnnanLiu <redacted>
---
This patch depends on the clk driver and reset driver.
Clk driver link:
https://lore.kernel.org/all/IA1PR20MB49539CDAD9A268CBF6CA184BBB9FA@IA1PR20MB4953.namprd20.prod.outlook.com/
Reset driver link:
https://lore.kernel.org/all/20231113005503.2423-1-jszhang@kernel.org/

I will update reset series soon

quoted hunk

Changes since v1:
- Change the name of the watchdog from watchdog0 to watchdog.
- Change the status of watchdog.
v1 link:
https://lore.kernel.org/all/DM6PR20MB23160B8499CC2BFDAE6FCACDAB9EA@DM6PR20MB2316.namprd20.prod.outlook.com/


 arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts |  4 ++++
 arch/riscv/boot/dts/sophgo/cv1800b.dtsi          | 16 ++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts b/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts
index 3af9e34b3bc7..75469161bfff 100644
--- a/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts
+++ b/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts

@@ -36,3 +36,7 @@ &osc {
 &uart0 {
 	status = "okay";
 };
+
+&watchdog {
+	status = "okay";
+};

diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
index aec6401a467b..03ca32cd37b6 100644
--- a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
+++ b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi

@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: (GPL-2.0 OR MIT)
 /*
  * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org>
+ * Copyright (C) 2024 Annan Liu <annan.liu.xdu@outlook.com>

No, I don't think every commit author needs to add this line.

quoted hunk

  */
 
 #include <dt-bindings/interrupt-controller/irq.h>

@@ -103,6 +104,21 @@ uart4: serial@41c0000 {
 			status = "disabled";
 		};
 
+		watchdog: watchdog@3010000{
+			compatible = "snps,dw-wdt";
+			reg = <0x3010000 0x100>;
+			interrupts = <58 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&pclk>;
+			resets = <&rst RST_WDT>;
+			status = "disabled";
+		};
+
+		pclk: pclk {

what's this clk?

+			#clock-cells = <0>;
+			compatible = "fixed-clock";
+			clock-frequency = <25000000>;
+		};
+
 		plic: interrupt-controller@70000000 {
 			compatible = "sophgo,cv1800b-plic", "thead,c900-plic";
 			reg = <0x70000000 0x4000000>;
-- 
2.34.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Inochi Amaoto replied to AnnanLiu Mar 25, 00:58 #

Message details and related content

Open message details and related content

On Thu, Jan 25, 2024 at 05:46:40PM +0800, AnnanLiu wrote:

quoted hunk

Add the watchdog device tree node to cv1800 SoC.

Signed-off-by: AnnanLiu <redacted>
---
This patch depends on the clk driver and reset driver.
Clk driver link:
https://lore.kernel.org/all/IA1PR20MB49539CDAD9A268CBF6CA184BBB9FA@IA1PR20MB4953.namprd20.prod.outlook.com/
Reset driver link:
https://lore.kernel.org/all/20231113005503.2423-1-jszhang@kernel.org/

Changes since v1:
- Change the name of the watchdog from watchdog0 to watchdog.
- Change the status of watchdog.
v1 link:
https://lore.kernel.org/all/DM6PR20MB23160B8499CC2BFDAE6FCACDAB9EA@DM6PR20MB2316.namprd20.prod.outlook.com/


 arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts |  4 ++++
 arch/riscv/boot/dts/sophgo/cv1800b.dtsi          | 16 ++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts b/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts
index 3af9e34b3bc7..75469161bfff 100644
--- a/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts
+++ b/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts

@@ -36,3 +36,7 @@ &osc {
 &uart0 {
 	status = "okay";
 };
+
+&watchdog {
+	status = "okay";
+};

diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
index aec6401a467b..03ca32cd37b6 100644
--- a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
+++ b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi

@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: (GPL-2.0 OR MIT)
 /*

18 more quoted lines

quoted hunk

+		pclk: pclk {
+			#clock-cells = <0>;
+			compatible = "fixed-clock";
+			clock-frequency = <25000000>;
+		};
+

Remove this stub clock and use osc instead.

Also, please rebase to v6.9-rc1.

 		plic: interrupt-controller@70000000 {
 			compatible = "sophgo,cv1800b-plic", "thead,c900-plic";
 			reg = <0x70000000 0x4000000>;
-- 
2.34.1

Alexander Sverdlin replied to AnnanLiu Feb 14, 2025, 12:58 #

Message details and related content

Open message details and related content

Hi Annan, Inochi,

On Thu, 2024-01-25 at 17:46 +0800, AnnanLiu wrote:

quoted hunk

Add the watchdog device tree node to cv1800 SoC.

Signed-off-by: AnnanLiu <redacted>
---
This patch depends on the clk driver and reset driver.
Clk driver link:
https://lore.kernel.org/all/IA1PR20MB49539CDAD9A268CBF6CA184BBB9FA@IA1PR20MB4953.namprd20.prod.outlook.com/
Reset driver link:
https://lore.kernel.org/all/20231113005503.2423-1-jszhang@kernel.org/

Changes since v1:
- Change the name of the watchdog from watchdog0 to watchdog.
- Change the status of watchdog.
v1 link:
https://lore.kernel.org/all/DM6PR20MB23160B8499CC2BFDAE6FCACDAB9EA@DM6PR20MB2316.namprd20.prod.outlook.com/


 arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts |  4 ++++
 arch/riscv/boot/dts/sophgo/cv1800b.dtsi          | 16 ++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts b/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts
index 3af9e34b3bc7..75469161bfff 100644
--- a/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts
+++ b/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts

@@ -36,3 +36,7 @@ &osc {

 &uart0 {
 	status = "okay";
 };
+
+&watchdog {
+	status = "okay";
+};

It would be necessary in all SoCs unless... [1]

quoted hunk

diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
index aec6401a467b..03ca32cd37b6 100644
--- a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
+++ b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi

@@ -1,6 +1,7 @@

 // SPDX-License-Identifier: (GPL-2.0 OR MIT)
 /*
  * Copyright (C) 2023 Jisheng Zhang [off-list ref]
+ * Copyright (C) 2024 Annan Liu [off-list ref]
  */
 
 #include <dt-bindings/interrupt-controller/irq.h>

@@ -103,6 +104,21 @@ uart4: serial@41c0000 {

 			status = "disabled";
 		};
 
+		watchdog: watchdog@3010000{
+			compatible = "snps,dw-wdt";
+			reg = <0x3010000 0x100>;
+			interrupts = <58 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&pclk>;

&osc ?

+			resets = <&rst RST_WDT>;
+			status = "disabled";

[1] ... the status here is dropped. What is the reason for disabling it in the first place?

+		};
+
+		pclk: pclk {
+			#clock-cells = <0>;
+			compatible = "fixed-clock";
+			clock-frequency = <25000000>;
+		};
+
 		plic: interrupt-controller@70000000 {
 			compatible = "sophgo,cv1800b-plic", "thead,c900-plic";
 			reg = <0x70000000 0x4000000>;

Why didn't it go into cv18xx.dtsi?

-- 
Alexander Sverdlin.

Inochi Amaoto replied to Alexander Sverdlin Feb 14, 2025, 22:42 #

Message details and related content

Open message details and related content

On Fri, Feb 14, 2025 at 01:58:15PM +0100, Alexander Sverdlin wrote:

Hi Annan, Inochi,

On Thu, 2024-01-25 at 17:46 +0800, AnnanLiu wrote:

quoted

Add the watchdog device tree node to cv1800 SoC.

Signed-off-by: AnnanLiu <redacted>
---
This patch depends on the clk driver and reset driver.
Clk driver link:
https://lore.kernel.org/all/IA1PR20MB49539CDAD9A268CBF6CA184BBB9FA@IA1PR20MB4953.namprd20.prod.outlook.com/
Reset driver link:
https://lore.kernel.org/all/20231113005503.2423-1-jszhang@kernel.org/

Changes since v1:
- Change the name of the watchdog from watchdog0 to watchdog.
- Change the status of watchdog.
v1 link:
https://lore.kernel.org/all/DM6PR20MB23160B8499CC2BFDAE6FCACDAB9EA@DM6PR20MB2316.namprd20.prod.outlook.com/


 arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts |  4 ++++
 arch/riscv/boot/dts/sophgo/cv1800b.dtsi          | 16 ++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts b/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts
index 3af9e34b3bc7..75469161bfff 100644
--- a/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts
+++ b/arch/riscv/boot/dts/sophgo/cv1800b-milkv-duo.dts

@@ -36,3 +36,7 @@ &osc {

 &uart0 {
 	status = "okay";
 };
+
+&watchdog {
+	status = "okay";
+};

It would be necessary in all SoCs unless... [1]

quoted

diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi

23 more quoted lines

This patch is submitted to early, before the clock driver is
applied. This reason applies to your all question.

quoted

+			resets = <&rst RST_WDT>;
+			status = "disabled";

[1] ... the status here is dropped. What is the reason for disabling it in the first place?

quoted

+		};
+
+		pclk: pclk {
+			#clock-cells = <0>;
+			compatible = "fixed-clock";
+			clock-frequency = <25000000>;
+		};
+
 		plic: interrupt-controller@70000000 {
 			compatible = "sophgo,cv1800b-plic", "thead,c900-plic";
 			reg = <0x70000000 0x4000000>;

Why didn't it go into cv18xx.dtsi?

-- 
Alexander Sverdlin.

`h`	back out one level
`j`	next message in thread / next series-diff position
`k`	previous message in thread / previous series-diff position
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help