2024-01-25 09:48:09

by AnnanLiu

[permalink] [raw]
Subject: [PATCH v2] riscv: dts: sophgo: add watchdog dt node for CV1800

Add the watchdog device tree node to cv1800 SoC.

Signed-off-by: AnnanLiu <[email protected]>
---
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/[email protected]/

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 <[email protected]>
+ * Copyright (C) 2024 Annan Liu <[email protected]>
*/

#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



2024-01-25 12:15:54

by Chen Wang

[permalink] [raw]
Subject: Re: [PATCH v2] riscv: dts: sophgo: add watchdog dt node for CV1800

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:
> Add the watchdog device tree node to cv1800 SoC.
>
> Signed-off-by: AnnanLiu <[email protected]>
> ---
> 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/[email protected]/
>
> 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 <[email protected]>
> + * Copyright (C) 2024 Annan Liu <[email protected]>
> */
>
> #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>;

2024-01-25 12:40:40

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2] riscv: dts: sophgo: add watchdog dt node for CV1800

On 25/01/2024 10:46, AnnanLiu wrote:
> +
> + 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


2024-01-25 17:17:06

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2] riscv: dts: sophgo: add watchdog dt node for CV1800

On Thu, Jan 25, 2024 at 01:39:51PM +0100, Krzysztof Kozlowski wrote:
> On 25/01/2024 10:46, AnnanLiu wrote:
> > +
> > + 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.


Attachments:
(No filename) (773.00 B)
signature.asc (235.00 B)
Download all attachments

2024-01-25 17:22:50

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2] riscv: dts: sophgo: add watchdog dt node for CV1800

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.


Attachments:
(No filename) (285.00 B)
signature.asc (235.00 B)
Download all attachments

2024-01-25 18:10:33

by Emil Renner Berthing

[permalink] [raw]
Subject: Re: [PATCH v2] riscv: dts: sophgo: add watchdog dt node for CV1800

AnnanLiu wrote:
> Add the watchdog device tree node to cv1800 SoC.
>
> Signed-off-by: AnnanLiu <[email protected]>
> ---
> 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/[email protected]/
>
> 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 <[email protected]>
> + * Copyright (C) 2024 Annan Liu <[email protected]>
> */
>
> #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";
> + };

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
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv

2024-01-29 07:58:00

by Inochi Amaoto

[permalink] [raw]
Subject: Re: [PATCH v2] riscv: dts: sophgo: add watchdog dt node for CV1800

>On Thu, Jan 25, 2024 at 01:39:51PM +0100, Krzysztof Kozlowski wrote:
>> On 25/01/2024 10:46, AnnanLiu wrote:
>> > +
>> > + 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.

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

2024-02-17 16:10:24

by Jisheng Zhang

[permalink] [raw]
Subject: Re: [PATCH v2] riscv: dts: sophgo: add watchdog dt node for CV1800

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 <[email protected]>
> ---
> 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/[email protected]/

I will update reset series soon

>
> 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 <[email protected]>
> + * Copyright (C) 2024 Annan Liu <[email protected]>

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

> */
>
> #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
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv

2024-03-25 09:33:16

by Inochi Amaoto

[permalink] [raw]
Subject: Re: [PATCH v2] riscv: dts: sophgo: add watchdog dt node for CV1800

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 <[email protected]>
> ---
> 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/[email protected]/
>
> 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 <[email protected]>
> + * Copyright (C) 2024 Annan Liu <[email protected]>
> */
>
> #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>;
> + };
> +

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
>