devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] riscv: add initial support for Canaan Kendryte K230
@ 2024-03-03 13:24 Yangyu Chen
  2024-03-03 13:26 ` [PATCH 1/5] dt-bindings: riscv: Add T-HEAD C908 compatible Yangyu Chen
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Yangyu Chen @ 2024-03-03 13:24 UTC (permalink / raw)
  To: linux-riscv
  Cc: Conor Dooley, Rob Herring, Krzysztof Kozlowski, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Guo Ren, devicetree, linux-kernel,
	Yangyu Chen

K230 is an ideal chip for testing RISC-V Vector 1.0 now. Add initial
support for it to allow more people to participate in building drivers
to mainline for it.

This kernel has been tested upon factory SDK [1] with
k230_evb_only_linux_defconfig and patched mainline opensbi [2] to skip
locked pmp and successfully booted to busybox on initrd with this log [3].

[1] https://github.com/kendryte/k230_sdk
[2] https://github.com/cyyself/opensbi/tree/k230
[3] https://gist.github.com/cyyself/b9445f38cc3ba1094924bd41c9086176

Yangyu Chen (5):
  dt-bindings: riscv: Add T-HEAD C908 compatible
  dt-bindings: add Canaan K230 boards compatible strings
  riscv: Kconfig.socs: Allow SOC_CANAAN with MMU for K230
  riscv: dts: add initial canmv-k230 and k230-evb dts
  riscv: config: enable SOC_CANAAN in defconfig

 .../devicetree/bindings/riscv/canaan.yaml     |  13 +-
 .../devicetree/bindings/riscv/cpus.yaml       |   1 +
 arch/riscv/Kconfig.socs                       |   5 +-
 arch/riscv/boot/dts/canaan/Makefile           |   2 +
 arch/riscv/boot/dts/canaan/canmv-k230.dts     |  23 +++
 arch/riscv/boot/dts/canaan/k230-evb.dts       |  23 +++
 arch/riscv/boot/dts/canaan/k230.dtsi          | 146 ++++++++++++++++++
 arch/riscv/configs/defconfig                  |   1 +
 8 files changed, 210 insertions(+), 4 deletions(-)
 create mode 100644 arch/riscv/boot/dts/canaan/canmv-k230.dts
 create mode 100644 arch/riscv/boot/dts/canaan/k230-evb.dts
 create mode 100644 arch/riscv/boot/dts/canaan/k230.dtsi


base-commit: 45e0b0fd6dc574101825ac2738b890da024e4cda
prerequisite-patch-id: 2374c56c0032e616e45854d2bc2bb1073996313d

Dependencies: https://lore.kernel.org/linux-riscv/tencent_88FEE0A2C5E0852436A2F1A1087E6803380A@qq.com/
-- 
2.43.0


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

* [PATCH 1/5] dt-bindings: riscv: Add T-HEAD C908 compatible
  2024-03-03 13:24 [PATCH 0/5] riscv: add initial support for Canaan Kendryte K230 Yangyu Chen
@ 2024-03-03 13:26 ` Yangyu Chen
  2024-03-04 10:12   ` Conor Dooley
  2024-03-03 13:26 ` [PATCH 2/5] dt-bindings: add Canaan K230 boards compatible strings Yangyu Chen
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Yangyu Chen @ 2024-03-03 13:26 UTC (permalink / raw)
  To: linux-riscv
  Cc: Conor Dooley, Rob Herring, Krzysztof Kozlowski, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Guo Ren, devicetree, linux-kernel,
	Yangyu Chen

The thead,c908 is a RISC-V CPU core from T-HEAD Semiconductor which used
in Canaan Kendryte K230 SoC.

Signed-off-by: Yangyu Chen <cyy@cyyself.name>
---
 Documentation/devicetree/bindings/riscv/cpus.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
index 9d8670c00e3b..e853a7fcee8a 100644
--- a/Documentation/devicetree/bindings/riscv/cpus.yaml
+++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
@@ -47,6 +47,7 @@ properties:
               - sifive,u74
               - sifive,u74-mc
               - thead,c906
+              - thead,c908
               - thead,c910
               - thead,c920
           - const: riscv
-- 
2.43.0


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

* [PATCH 2/5] dt-bindings: add Canaan K230 boards compatible strings
  2024-03-03 13:24 [PATCH 0/5] riscv: add initial support for Canaan Kendryte K230 Yangyu Chen
  2024-03-03 13:26 ` [PATCH 1/5] dt-bindings: riscv: Add T-HEAD C908 compatible Yangyu Chen
@ 2024-03-03 13:26 ` Yangyu Chen
  2024-03-04  8:11   ` Krzysztof Kozlowski
  2024-03-03 13:26 ` [PATCH 3/5] riscv: Kconfig.socs: Allow SOC_CANAAN with MMU for K230 Yangyu Chen
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Yangyu Chen @ 2024-03-03 13:26 UTC (permalink / raw)
  To: linux-riscv
  Cc: Conor Dooley, Rob Herring, Krzysztof Kozlowski, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Guo Ren, devicetree, linux-kernel,
	Yangyu Chen

Since K230 was released, K210 is no longer the only SoC in the Kendryte
series, so remove the K210 string from the description. Also, add two
boards based on k230 to compatible strings to allow them to be used in the
dt.

Signed-off-by: Yangyu Chen <cyy@cyyself.name>
---
 Documentation/devicetree/bindings/riscv/canaan.yaml | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/riscv/canaan.yaml b/Documentation/devicetree/bindings/riscv/canaan.yaml
index 41fd11f70a49..444758db964e 100644
--- a/Documentation/devicetree/bindings/riscv/canaan.yaml
+++ b/Documentation/devicetree/bindings/riscv/canaan.yaml
@@ -10,7 +10,7 @@ maintainers:
   - Damien Le Moal <dlemoal@kernel.org>
 
 description:
-  Canaan Kendryte K210 SoC-based boards
+  Canaan Kendryte SoC-based boards
 
 properties:
   $nodename:
@@ -42,6 +42,17 @@ properties:
       - items:
           - const: canaan,kendryte-k210
 
+      - items:
+          - const: canaan,k230-usip-lp3-evb
+          - const: canaan,kendryte-k230
+
+      - items:
+          - const: canaan,canmv-k230
+          - const: canaan,kendryte-k230
+
+      - items:
+          - const: canaan,kendryte-k230
+
 additionalProperties: true
 
 ...
-- 
2.43.0


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

* [PATCH 3/5] riscv: Kconfig.socs: Allow SOC_CANAAN with MMU for K230
  2024-03-03 13:24 [PATCH 0/5] riscv: add initial support for Canaan Kendryte K230 Yangyu Chen
  2024-03-03 13:26 ` [PATCH 1/5] dt-bindings: riscv: Add T-HEAD C908 compatible Yangyu Chen
  2024-03-03 13:26 ` [PATCH 2/5] dt-bindings: add Canaan K230 boards compatible strings Yangyu Chen
@ 2024-03-03 13:26 ` Yangyu Chen
  2024-03-04 10:18   ` Conor Dooley
  2024-03-03 13:26 ` [PATCH 4/5] riscv: dts: add initial canmv-k230 and k230-evb dts Yangyu Chen
  2024-03-03 13:26 ` [PATCH 5/5] riscv: config: enable SOC_CANAAN in defconfig Yangyu Chen
  4 siblings, 1 reply; 15+ messages in thread
From: Yangyu Chen @ 2024-03-03 13:26 UTC (permalink / raw)
  To: linux-riscv
  Cc: Conor Dooley, Rob Herring, Krzysztof Kozlowski, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Guo Ren, devicetree, linux-kernel,
	Yangyu Chen

Since K230 was released, SOC_CANAAN is no longer only referred to the K210.
Remove it depends on !MMU will allow building dts for K230 and remove the
K210 string from the help message.

Signed-off-by: Yangyu Chen <cyy@cyyself.name>
---
 arch/riscv/Kconfig.socs | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
index 623de5f8a208..b4e9b7f75510 100644
--- a/arch/riscv/Kconfig.socs
+++ b/arch/riscv/Kconfig.socs
@@ -75,13 +75,12 @@ config ARCH_CANAAN
 	def_bool SOC_CANAAN
 
 config SOC_CANAAN
-	bool "Canaan Kendryte K210 SoC"
-	depends on !MMU
+	bool "Canaan Kendryte SoC"
 	select CLINT_TIMER if RISCV_M_MODE
 	select ARCH_HAS_RESET_CONTROLLER
 	select PINCTRL
 	select COMMON_CLK
 	help
-	  This enables support for Canaan Kendryte K210 SoC platform hardware.
+	  This enables support for Canaan Kendryte SoC platform hardware.
 
 endmenu # "SoC selection"
-- 
2.43.0


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

* [PATCH 4/5] riscv: dts: add initial canmv-k230 and k230-evb dts
  2024-03-03 13:24 [PATCH 0/5] riscv: add initial support for Canaan Kendryte K230 Yangyu Chen
                   ` (2 preceding siblings ...)
  2024-03-03 13:26 ` [PATCH 3/5] riscv: Kconfig.socs: Allow SOC_CANAAN with MMU for K230 Yangyu Chen
@ 2024-03-03 13:26 ` Yangyu Chen
  2024-03-04  8:13   ` Krzysztof Kozlowski
  2024-03-04 19:00   ` Conor Dooley
  2024-03-03 13:26 ` [PATCH 5/5] riscv: config: enable SOC_CANAAN in defconfig Yangyu Chen
  4 siblings, 2 replies; 15+ messages in thread
From: Yangyu Chen @ 2024-03-03 13:26 UTC (permalink / raw)
  To: linux-riscv
  Cc: Conor Dooley, Rob Herring, Krzysztof Kozlowski, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Guo Ren, devicetree, linux-kernel,
	Yangyu Chen

Add initial dts for CanMV-K230 and K230-EVB powered by Canaan Kendryte
K230 SoC [1].

Some key considerations:
- Only enable BigCore which is 1.6GHz RV64GCBV

Since is there cache coherence between two cores remains a mystery since
they have a dedicated L2 Cache. And the factory SDK uses it for other OS
by default.

Meanwhile, although docs from Canaan said 1.6GHz Core with Vector is
CPU1, the csr.mhartid of this core is 0.

- Support for "zba" "zbb" "zbc" "zbs" are tested by hand

The user manual of C908 from T-Head does not document it specifically.
It just said it supports B extension V1.0-rc1. [2]

- Support for "zicbom" is tested by hand

Have tested with some out-of-tree drivers that need DMA and they do not
come to the dts currently.

- Cache parameters are inferred from T-Head docs [2] and Cannan docs [1]

L1i: 32KB, VIPT 4-Way set-associative, 64B Cacheline
L1d: 32KB, VIPT 4-Way set-associative, 64B Cacheline
L2: 256KB, PIPI 16-way set-associative, 64B Cacheline

The numbers of cache sets are calculated from these parameters.

- MMU only supports Sv39

Since T-Head docs [2] says C908 should support sv48. However, it will fail
during the kernel probe. I also tested it by hand on M-Mode software,
writing sv48 to satp.mode will not trap but will leave the csr unchanged.

[1] https://developer.canaan-creative.com/k230/dev/zh/00_hardware/K230_datasheet.html#chapter-1-introduction
[2] https://occ-intl-prod.oss-ap-southeast-1.aliyuncs.com/resource//1699268369347/XuanTie-C908-UserManual.pdf

Signed-off-by: Yangyu Chen <cyy@cyyself.name>
---
 arch/riscv/boot/dts/canaan/Makefile       |   2 +
 arch/riscv/boot/dts/canaan/canmv-k230.dts |  23 ++++
 arch/riscv/boot/dts/canaan/k230-evb.dts   |  23 ++++
 arch/riscv/boot/dts/canaan/k230.dtsi      | 146 ++++++++++++++++++++++
 4 files changed, 194 insertions(+)
 create mode 100644 arch/riscv/boot/dts/canaan/canmv-k230.dts
 create mode 100644 arch/riscv/boot/dts/canaan/k230-evb.dts
 create mode 100644 arch/riscv/boot/dts/canaan/k230.dtsi

diff --git a/arch/riscv/boot/dts/canaan/Makefile b/arch/riscv/boot/dts/canaan/Makefile
index 987d1f0c41f0..b4a0ec668f9a 100644
--- a/arch/riscv/boot/dts/canaan/Makefile
+++ b/arch/riscv/boot/dts/canaan/Makefile
@@ -5,3 +5,5 @@ dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_bit.dtb
 dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_dock.dtb
 dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_go.dtb
 dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maixduino.dtb
+dtb-$(CONFIG_ARCH_CANAAN) += k230-evb.dtb
+dtb-$(CONFIG_ARCH_CANAAN) += canmv-k230.dtb
\ No newline at end of file
diff --git a/arch/riscv/boot/dts/canaan/canmv-k230.dts b/arch/riscv/boot/dts/canaan/canmv-k230.dts
new file mode 100644
index 000000000000..09777616d30e
--- /dev/null
+++ b/arch/riscv/boot/dts/canaan/canmv-k230.dts
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2024 Yangyu Chen <cyy@cyyself.name>
+ */
+#include "k230.dtsi"
+
+/ {
+	model = "Canaan CanMV-K230";
+	compatible = "canaan,canmv-k230", "canaan,kendryte-k230";
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+
+	ddr: memory@0 {
+		device_type = "memory";
+		reg = <0x0 0x0 0x0 0x1fdff000>;
+	};
+};
+
+&uart0 {
+	status = "okay";
+};
diff --git a/arch/riscv/boot/dts/canaan/k230-evb.dts b/arch/riscv/boot/dts/canaan/k230-evb.dts
new file mode 100644
index 000000000000..dfdf8b3e99eb
--- /dev/null
+++ b/arch/riscv/boot/dts/canaan/k230-evb.dts
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2024 Yangyu Chen <cyy@cyyself.name>
+ */
+#include "k230.dtsi"
+
+/ {
+	model = "Kendryte K230 EVB";
+	compatible = "canaan,k230-usip-lp3-evb", "canaan,kendryte-k230";
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+
+	ddr: memory@0 {
+		device_type = "memory";
+		reg = <0x0 0x0 0x0 0x1fdff000>;
+	};
+};
+
+&uart0 {
+	status = "okay";
+};
diff --git a/arch/riscv/boot/dts/canaan/k230.dtsi b/arch/riscv/boot/dts/canaan/k230.dtsi
new file mode 100644
index 000000000000..4317bda38142
--- /dev/null
+++ b/arch/riscv/boot/dts/canaan/k230.dtsi
@@ -0,0 +1,146 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2024 Yangyu Chen <cyy@cyyself.name>
+ */
+
+#include <dt-bindings/interrupt-controller/irq.h>
+
+/dts-v1/;
+/ {
+	#address-cells = <2>;
+	#size-cells = <2>;
+	compatible = "canaan,kendryte-k230";
+
+	aliases {
+		serial0 = &uart0;
+	};
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		timebase-frequency = <27000000>;
+
+		cpu@0 {
+			compatible = "thead,c908", "riscv";
+			device_type = "cpu";
+			reg = <0>;
+			riscv,isa = "rv64imafdcv_zba_zbb_zbc_zbs_zicbom_svpbmt";
+			riscv,isa-base = "rv64i";
+			riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "v", "zba", "zbb",
+					       "zbc", "zbs", "zicbom", "zicntr", "zicsr",
+					       "zifencei", "zihpm", "svpbmt";
+			riscv,cbom-block-size = <64>;
+			d-cache-block-size = <64>;
+			d-cache-sets = <128>;
+			d-cache-size = <32768>;
+			i-cache-block-size = <64>;
+			i-cache-sets = <128>;
+			i-cache-size = <32768>;
+			next-level-cache = <&l2_cache>;
+			mmu-type = "riscv,sv39";
+
+			cpu0_intc: interrupt-controller {
+				compatible = "riscv,cpu-intc";
+				interrupt-controller;
+				#interrupt-cells = <1>;
+			};
+		};
+
+		l2_cache: l2-cache {
+			compatible = "cache";
+			cache-block-size = <64>;
+			cache-level = <2>;
+			cache-size = <262144>;
+			cache-sets = <256>;
+			cache-unified;
+		};
+	};
+
+	soc {
+		compatible = "simple-bus";
+		interrupt-parent = <&plic>;
+		#address-cells = <2>;
+		#size-cells = <2>;
+		dma-noncoherent;
+		ranges;
+
+		plic: interrupt-controller@f00000000 {
+			compatible = "thead,c900-plic";
+			reg = <0xf 0x00000000 0x0 0x04000000>;
+			interrupts-extended = <&cpu0_intc 11>, <&cpu0_intc 9>;
+			interrupt-controller;
+			reg-names = "control";
+			#address-cells = <0>;
+			#interrupt-cells = <2>;
+			riscv,ndev = <208>;
+		};
+
+		clint: timer@f04000000 {
+			compatible = "thead,c900-clint";
+			reg = <0xf 0x04000000 0x0 0x04000000>;
+			interrupts-extended = <&cpu0_intc 3>, <&cpu0_intc 7>;
+		};
+
+		apb_clk: apb-clk-clock {
+			compatible = "fixed-clock";
+			clock-frequency = <50000000>;
+			clock-output-names = "apb_clk";
+			#clock-cells = <0>;
+		};
+
+		uart0: serial@91400000 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x0 0x91400000 0x0 0x1000>;
+			clocks = <&apb_clk>;
+			clock-names = "baudclk";
+			interrupts = <16 IRQ_TYPE_LEVEL_HIGH>;
+			reg-io-width = <4>;
+			reg-shift = <2>;
+			status = "disabled";
+		};
+
+		uart1: serial@91401000 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x0 0x91401000 0x0 0x1000>;
+			clocks = <&apb_clk>;
+			clock-names = "baudclk";
+			interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
+			reg-io-width = <4>;
+			reg-shift = <2>;
+			status = "disabled";
+		};
+
+		uart2: serial@91402000 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x0 0x91402000 0x0 0x1000>;
+			clocks = <&apb_clk>;
+			clock-names = "baudclk";
+			interrupts = <18 IRQ_TYPE_LEVEL_HIGH>;
+			reg-io-width = <4>;
+			reg-shift = <2>;
+			status = "disabled";
+		};
+
+		uart3: serial@91403000 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x0 0x91403000 0x0 0x1000>;
+			clocks = <&apb_clk>;
+			clock-names = "baudclk";
+			interrupts = <19 IRQ_TYPE_LEVEL_HIGH>;
+			reg-io-width = <4>;
+			reg-shift = <2>;
+			status = "disabled";
+		};
+
+		uart4: serial@91404000 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x0 0x91404000 0x0 0x1000>;
+			clocks = <&apb_clk>;
+			clock-names = "baudclk";
+			interrupts = <20 IRQ_TYPE_LEVEL_HIGH>;
+			reg-io-width = <4>;
+			reg-shift = <2>;
+			status = "disabled";
+		};
+	};
+};
-- 
2.43.0


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

* [PATCH 5/5] riscv: config: enable SOC_CANAAN in defconfig
  2024-03-03 13:24 [PATCH 0/5] riscv: add initial support for Canaan Kendryte K230 Yangyu Chen
                   ` (3 preceding siblings ...)
  2024-03-03 13:26 ` [PATCH 4/5] riscv: dts: add initial canmv-k230 and k230-evb dts Yangyu Chen
@ 2024-03-03 13:26 ` Yangyu Chen
  4 siblings, 0 replies; 15+ messages in thread
From: Yangyu Chen @ 2024-03-03 13:26 UTC (permalink / raw)
  To: linux-riscv
  Cc: Conor Dooley, Rob Herring, Krzysztof Kozlowski, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Guo Ren, devicetree, linux-kernel,
	Yangyu Chen

Since K230 has been supported, allow SOC_CANAAN to be selected to build dt
and drivers for it in defconfig.

Signed-off-by: Yangyu Chen <cyy@cyyself.name>
---
 arch/riscv/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig
index 89a009a580fe..20b557ec28df 100644
--- a/arch/riscv/configs/defconfig
+++ b/arch/riscv/configs/defconfig
@@ -33,6 +33,7 @@ CONFIG_SOC_STARFIVE=y
 CONFIG_ARCH_SUNXI=y
 CONFIG_ARCH_THEAD=y
 CONFIG_SOC_VIRT=y
+CONFIG_SOC_CANAAN=y
 CONFIG_SMP=y
 CONFIG_HOTPLUG_CPU=y
 CONFIG_PM=y
-- 
2.43.0


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

* Re: [PATCH 2/5] dt-bindings: add Canaan K230 boards compatible strings
  2024-03-03 13:26 ` [PATCH 2/5] dt-bindings: add Canaan K230 boards compatible strings Yangyu Chen
@ 2024-03-04  8:11   ` Krzysztof Kozlowski
  2024-03-04  8:51     ` Yangyu Chen
  0 siblings, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2024-03-04  8:11 UTC (permalink / raw)
  To: Yangyu Chen, linux-riscv
  Cc: Conor Dooley, Rob Herring, Krzysztof Kozlowski, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Guo Ren, devicetree, linux-kernel

On 03/03/2024 14:26, Yangyu Chen wrote:
> Since K230 was released, K210 is no longer the only SoC in the Kendryte
> series, so remove the K210 string from the description. Also, add two
> boards based on k230 to compatible strings to allow them to be used in the
> dt.
> 
> Signed-off-by: Yangyu Chen <cyy@cyyself.name>
> ---
>  Documentation/devicetree/bindings/riscv/canaan.yaml | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/riscv/canaan.yaml b/Documentation/devicetree/bindings/riscv/canaan.yaml
> index 41fd11f70a49..444758db964e 100644
> --- a/Documentation/devicetree/bindings/riscv/canaan.yaml
> +++ b/Documentation/devicetree/bindings/riscv/canaan.yaml
> @@ -10,7 +10,7 @@ maintainers:
>    - Damien Le Moal <dlemoal@kernel.org>
>  
>  description:
> -  Canaan Kendryte K210 SoC-based boards
> +  Canaan Kendryte SoC-based boards
>  
>  properties:
>    $nodename:
> @@ -42,6 +42,17 @@ properties:
>        - items:
>            - const: canaan,kendryte-k210
>  
> +      - items:
> +          - const: canaan,k230-usip-lp3-evb
> +          - const: canaan,kendryte-k230
> +
> +      - items:
> +          - const: canaan,canmv-k230

Why this is not part of previous entry in an enum?

> +          - const: canaan,kendryte-k230
> +
> +      - items:
> +          - const: canaan,kendryte-k230

Usually you cannot run SoCs alone. What does it represent (in real life)?

Best regards,
Krzysztof


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

* Re: [PATCH 4/5] riscv: dts: add initial canmv-k230 and k230-evb dts
  2024-03-03 13:26 ` [PATCH 4/5] riscv: dts: add initial canmv-k230 and k230-evb dts Yangyu Chen
@ 2024-03-04  8:13   ` Krzysztof Kozlowski
  2024-03-04 19:00   ` Conor Dooley
  1 sibling, 0 replies; 15+ messages in thread
From: Krzysztof Kozlowski @ 2024-03-04  8:13 UTC (permalink / raw)
  To: Yangyu Chen, linux-riscv
  Cc: Conor Dooley, Rob Herring, Krzysztof Kozlowski, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Guo Ren, devicetree, linux-kernel

On 03/03/2024 14:26, Yangyu Chen wrote:
> Add initial dts for CanMV-K230 and K230-EVB powered by Canaan Kendryte
> K230 SoC [1].
> 
> Some key considerations:
> - Only enable BigCore which is 1.6GHz RV64GCBV
> 
> Since is there cache coherence between two cores remains a mystery since
> they have a dedicated L2 Cache. And the factory SDK uses it for other OS
> by default.
> 
> Meanwhile, although docs from Canaan said 1.6GHz Core with Vector is
> CPU1, the csr.mhartid of this core is 0.
> 
> - Support for "zba" "zbb" "zbc" "zbs" are tested by hand
> 
> The user manual of C908 from T-Head does not document it specifically.
> It just said it supports B extension V1.0-rc1. [2]
> 
> - Support for "zicbom" is tested by hand
> 
> Have tested with some out-of-tree drivers that need DMA and they do not
> come to the dts currently.
> 
> - Cache parameters are inferred from T-Head docs [2] and Cannan docs [1]
> 
> L1i: 32KB, VIPT 4-Way set-associative, 64B Cacheline
> L1d: 32KB, VIPT 4-Way set-associative, 64B Cacheline
> L2: 256KB, PIPI 16-way set-associative, 64B Cacheline
> 
> The numbers of cache sets are calculated from these parameters.
> 
> - MMU only supports Sv39
> 
> Since T-Head docs [2] says C908 should support sv48. However, it will fail
> during the kernel probe. I also tested it by hand on M-Mode software,
> writing sv48 to satp.mode will not trap but will leave the csr unchanged.
> 
> [1] https://developer.canaan-creative.com/k230/dev/zh/00_hardware/K230_datasheet.html#chapter-1-introduction
> [2] https://occ-intl-prod.oss-ap-southeast-1.aliyuncs.com/resource//1699268369347/XuanTie-C908-UserManual.pdf
> 
> Signed-off-by: Yangyu Chen <cyy@cyyself.name>
> ---
>  arch/riscv/boot/dts/canaan/Makefile       |   2 +
>  arch/riscv/boot/dts/canaan/canmv-k230.dts |  23 ++++
>  arch/riscv/boot/dts/canaan/k230-evb.dts   |  23 ++++
>  arch/riscv/boot/dts/canaan/k230.dtsi      | 146 ++++++++++++++++++++++
>  4 files changed, 194 insertions(+)
>  create mode 100644 arch/riscv/boot/dts/canaan/canmv-k230.dts
>  create mode 100644 arch/riscv/boot/dts/canaan/k230-evb.dts
>  create mode 100644 arch/riscv/boot/dts/canaan/k230.dtsi
> 
> diff --git a/arch/riscv/boot/dts/canaan/Makefile b/arch/riscv/boot/dts/canaan/Makefile
> index 987d1f0c41f0..b4a0ec668f9a 100644
> --- a/arch/riscv/boot/dts/canaan/Makefile
> +++ b/arch/riscv/boot/dts/canaan/Makefile
> @@ -5,3 +5,5 @@ dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_bit.dtb
>  dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_dock.dtb
>  dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_go.dtb
>  dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maixduino.dtb
> +dtb-$(CONFIG_ARCH_CANAAN) += k230-evb.dtb
> +dtb-$(CONFIG_ARCH_CANAAN) += canmv-k230.dtb
> \ No newline at end of file

Please fix patch errors.

> diff --git a/arch/riscv/boot/dts/canaan/canmv-k230.dts b/arch/riscv/boot/dts/canaan/canmv-k230.dts
> new file mode 100644
> index 000000000000..09777616d30e


...

> +&uart0 {
> +	status = "okay";
> +};
> diff --git a/arch/riscv/boot/dts/canaan/k230.dtsi b/arch/riscv/boot/dts/canaan/k230.dtsi
> new file mode 100644
> index 000000000000..4317bda38142
> --- /dev/null
> +++ b/arch/riscv/boot/dts/canaan/k230.dtsi
> @@ -0,0 +1,146 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2024 Yangyu Chen <cyy@cyyself.name>
> + */
> +
> +#include <dt-bindings/interrupt-controller/irq.h>
> +
> +/dts-v1/;
> +/ {
> +	#address-cells = <2>;
> +	#size-cells = <2>;
> +	compatible = "canaan,kendryte-k230";
> +
> +	aliases {
> +		serial0 = &uart0;
> +	};
> +
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		timebase-frequency = <27000000>;
> +
> +		cpu@0 {
> +			compatible = "thead,c908", "riscv";
> +			device_type = "cpu";
> +			reg = <0>;
> +			riscv,isa = "rv64imafdcv_zba_zbb_zbc_zbs_zicbom_svpbmt";
> +			riscv,isa-base = "rv64i";
> +			riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "v", "zba", "zbb",
> +					       "zbc", "zbs", "zicbom", "zicntr", "zicsr",
> +					       "zifencei", "zihpm", "svpbmt";
> +			riscv,cbom-block-size = <64>;
> +			d-cache-block-size = <64>;
> +			d-cache-sets = <128>;
> +			d-cache-size = <32768>;
> +			i-cache-block-size = <64>;
> +			i-cache-sets = <128>;
> +			i-cache-size = <32768>;
> +			next-level-cache = <&l2_cache>;
> +			mmu-type = "riscv,sv39";
> +
> +			cpu0_intc: interrupt-controller {
> +				compatible = "riscv,cpu-intc";
> +				interrupt-controller;
> +				#interrupt-cells = <1>;
> +			};
> +		};
> +
> +		l2_cache: l2-cache {
> +			compatible = "cache";
> +			cache-block-size = <64>;
> +			cache-level = <2>;
> +			cache-size = <262144>;
> +			cache-sets = <256>;
> +			cache-unified;
> +		};
> +	};
> +
> +	soc {
> +		compatible = "simple-bus";
> +		interrupt-parent = <&plic>;
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		dma-noncoherent;
> +		ranges;
> +
> +		plic: interrupt-controller@f00000000 {
> +			compatible = "thead,c900-plic";
> +			reg = <0xf 0x00000000 0x0 0x04000000>;
> +			interrupts-extended = <&cpu0_intc 11>, <&cpu0_intc 9>;
> +			interrupt-controller;
> +			reg-names = "control";
> +			#address-cells = <0>;
> +			#interrupt-cells = <2>;
> +			riscv,ndev = <208>;
> +		};
> +
> +		clint: timer@f04000000 {
> +			compatible = "thead,c900-clint";
> +			reg = <0xf 0x04000000 0x0 0x04000000>;
> +			interrupts-extended = <&cpu0_intc 3>, <&cpu0_intc 7>;
> +		};
> +
> +		apb_clk: apb-clk-clock {

Only MMIO-nodes go to soc. Such stubs go outside.

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

> +			compatible = "fixed-clock";
> +			clock-frequency = <50000000>;
> +			clock-output-names = "apb_clk";
> +			#clock-cells = <0>;
> +		};
> +


Best regards,
Krzysztof


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

* Re: [PATCH 2/5] dt-bindings: add Canaan K230 boards compatible strings
  2024-03-04  8:11   ` Krzysztof Kozlowski
@ 2024-03-04  8:51     ` Yangyu Chen
  2024-03-04 10:11       ` Conor Dooley
  2024-03-04 10:27       ` Krzysztof Kozlowski
  0 siblings, 2 replies; 15+ messages in thread
From: Yangyu Chen @ 2024-03-04  8:51 UTC (permalink / raw)
  To: Krzysztof Kozlowski, linux-riscv
  Cc: Conor Dooley, Rob Herring, Krzysztof Kozlowski, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Guo Ren, devicetree, linux-kernel

On 2024/3/4 16:11, Krzysztof Kozlowski wrote:
> On 03/03/2024 14:26, Yangyu Chen wrote:
>> Since K230 was released, K210 is no longer the only SoC in the Kendryte
>> series, so remove the K210 string from the description. Also, add two
>> boards based on k230 to compatible strings to allow them to be used in the
>> dt.
>>
>> Signed-off-by: Yangyu Chen <cyy@cyyself.name>
>> ---
>>   Documentation/devicetree/bindings/riscv/canaan.yaml | 13 ++++++++++++-
>>   1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/riscv/canaan.yaml b/Documentation/devicetree/bindings/riscv/canaan.yaml
>> index 41fd11f70a49..444758db964e 100644
>> --- a/Documentation/devicetree/bindings/riscv/canaan.yaml
>> +++ b/Documentation/devicetree/bindings/riscv/canaan.yaml
>> @@ -10,7 +10,7 @@ maintainers:
>>     - Damien Le Moal <dlemoal@kernel.org>
>>   
>>   description:
>> -  Canaan Kendryte K210 SoC-based boards
>> +  Canaan Kendryte SoC-based boards
>>   
>>   properties:
>>     $nodename:
>> @@ -42,6 +42,17 @@ properties:
>>         - items:
>>             - const: canaan,kendryte-k210
>>   
>> +      - items:
>> +          - const: canaan,k230-usip-lp3-evb
>> +          - const: canaan,kendryte-k230
>> +
>> +      - items:
>> +          - const: canaan,canmv-k230
> 
> Why this is not part of previous entry in an enum?
> 
>> +          - const: canaan,kendryte-k230
>> +
>> +      - items:
>> +          - const: canaan,kendryte-k230
> 
> Usually you cannot run SoCs alone. What does it represent (in real life)?
> 

I'm not sure what it means.

If you wonder why should I add a compatible string for soc, that is 
although we cannot run SoCs alone, adding a soc compatible will allow 
some bootloaders or SBI on RISC-V to choose an errata for a soc. Such as 
this opensbi patch. [1]

If you wonder why I should allow a soc-compatible string with soc alone, 
that is because k210 did it previously. And provide a k210_generic.dts 
to use it. I haven't provided generic dts now but allowing only 
soc-compatible string alone would also be acceptable I think.

[1] 
https://github.com/cyyself/opensbi/commit/b113c1c01d700314a4a696297ec09031a9399354

Thanks,
Yangyu Chen

> Best regards,
> Krzysztof
> 


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

* Re: [PATCH 2/5] dt-bindings: add Canaan K230 boards compatible strings
  2024-03-04  8:51     ` Yangyu Chen
@ 2024-03-04 10:11       ` Conor Dooley
  2024-03-04 10:27       ` Krzysztof Kozlowski
  1 sibling, 0 replies; 15+ messages in thread
From: Conor Dooley @ 2024-03-04 10:11 UTC (permalink / raw)
  To: Yangyu Chen
  Cc: Krzysztof Kozlowski, linux-riscv, Conor Dooley, Rob Herring,
	Krzysztof Kozlowski, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Guo Ren, devicetree, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2953 bytes --]

On Mon, Mar 04, 2024 at 04:51:05PM +0800, Yangyu Chen wrote:
> On 2024/3/4 16:11, Krzysztof Kozlowski wrote:
> > On 03/03/2024 14:26, Yangyu Chen wrote:
> > > Since K230 was released, K210 is no longer the only SoC in the Kendryte
> > > series, so remove the K210 string from the description. Also, add two
> > > boards based on k230 to compatible strings to allow them to be used in the
> > > dt.
> > > 
> > > Signed-off-by: Yangyu Chen <cyy@cyyself.name>
> > > ---
> > >   Documentation/devicetree/bindings/riscv/canaan.yaml | 13 ++++++++++++-
> > >   1 file changed, 12 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/riscv/canaan.yaml b/Documentation/devicetree/bindings/riscv/canaan.yaml
> > > index 41fd11f70a49..444758db964e 100644
> > > --- a/Documentation/devicetree/bindings/riscv/canaan.yaml
> > > +++ b/Documentation/devicetree/bindings/riscv/canaan.yaml
> > > @@ -10,7 +10,7 @@ maintainers:
> > >     - Damien Le Moal <dlemoal@kernel.org>
> > >   description:
> > > -  Canaan Kendryte K210 SoC-based boards
> > > +  Canaan Kendryte SoC-based boards
> > >   properties:
> > >     $nodename:
> > > @@ -42,6 +42,17 @@ properties:
> > >         - items:
> > >             - const: canaan,kendryte-k210
> > > +      - items:
> > > +          - const: canaan,k230-usip-lp3-evb
> > > +          - const: canaan,kendryte-k230
> > > +
> > > +      - items:
> > > +          - const: canaan,canmv-k230
> > 
> > Why this is not part of previous entry in an enum?
> > 
> > > +          - const: canaan,kendryte-k230
> > > +
> > > +      - items:
> > > +          - const: canaan,kendryte-k230
> > 
> > Usually you cannot run SoCs alone. What does it represent (in real life)?
> > 
> 
> I'm not sure what it means.

You have a SoC compatible but no board compatible. You cannot run a SoC
without some sort of board connected to it, so this should be removed.

> If you wonder why should I add a compatible string for soc, that is although
> we cannot run SoCs alone, adding a soc compatible will allow some
> bootloaders or SBI on RISC-V to choose an errata for a soc. Such as this
> opensbi patch. [1]

You don't need to add an isolated compatible like this to be able to
apply that "erratum", the compatible is already documented from the
"usip-l3-evb" and "canmv-k230" entries.

> If you wonder why I should allow a soc-compatible string with soc alone,
> that is because k210 did it previously.

The k210 is not really a beacon of quality in the DT department, copying
from there is likely to be misleading unfortunately.

> And provide a k210_generic.dts to
> use it. I haven't provided generic dts now but allowing only soc-compatible
> string alone would also be acceptable I think.

To be honest, I would like to delete the generic dts for the k210, I
don't think it should exist, at least not in the current form.

Thanks,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* Re: [PATCH 1/5] dt-bindings: riscv: Add T-HEAD C908 compatible
  2024-03-03 13:26 ` [PATCH 1/5] dt-bindings: riscv: Add T-HEAD C908 compatible Yangyu Chen
@ 2024-03-04 10:12   ` Conor Dooley
  0 siblings, 0 replies; 15+ messages in thread
From: Conor Dooley @ 2024-03-04 10:12 UTC (permalink / raw)
  To: Yangyu Chen
  Cc: linux-riscv, Conor Dooley, Rob Herring, Krzysztof Kozlowski,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Guo Ren, devicetree,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 956 bytes --]

On Sun, Mar 03, 2024 at 09:26:23PM +0800, Yangyu Chen wrote:
> The thead,c908 is a RISC-V CPU core from T-HEAD Semiconductor which used
> in Canaan Kendryte K230 SoC.
> 
> Signed-off-by: Yangyu Chen <cyy@cyyself.name>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

> ---
>  Documentation/devicetree/bindings/riscv/cpus.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
> index 9d8670c00e3b..e853a7fcee8a 100644
> --- a/Documentation/devicetree/bindings/riscv/cpus.yaml
> +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
> @@ -47,6 +47,7 @@ properties:
>                - sifive,u74
>                - sifive,u74-mc
>                - thead,c906
> +              - thead,c908
>                - thead,c910
>                - thead,c920
>            - const: riscv
> -- 
> 2.43.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* Re: [PATCH 3/5] riscv: Kconfig.socs: Allow SOC_CANAAN with MMU for K230
  2024-03-03 13:26 ` [PATCH 3/5] riscv: Kconfig.socs: Allow SOC_CANAAN with MMU for K230 Yangyu Chen
@ 2024-03-04 10:18   ` Conor Dooley
  0 siblings, 0 replies; 15+ messages in thread
From: Conor Dooley @ 2024-03-04 10:18 UTC (permalink / raw)
  To: Yangyu Chen
  Cc: linux-riscv, Conor Dooley, Rob Herring, Krzysztof Kozlowski,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Guo Ren, devicetree,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1182 bytes --]

On Sun, Mar 03, 2024 at 09:26:25PM +0800, Yangyu Chen wrote:
> Since K230 was released, SOC_CANAAN is no longer only referred to the K210.
> Remove it depends on !MMU will allow building dts for K230 and remove the
> K210 string from the help message.
> 
> Signed-off-by: Yangyu Chen <cyy@cyyself.name>

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

> ---
>  arch/riscv/Kconfig.socs | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
> index 623de5f8a208..b4e9b7f75510 100644
> --- a/arch/riscv/Kconfig.socs
> +++ b/arch/riscv/Kconfig.socs
> @@ -75,13 +75,12 @@ config ARCH_CANAAN
>  	def_bool SOC_CANAAN
>  
>  config SOC_CANAAN
> -	bool "Canaan Kendryte K210 SoC"
> -	depends on !MMU
> +	bool "Canaan Kendryte SoC"
>  	select CLINT_TIMER if RISCV_M_MODE
>  	select ARCH_HAS_RESET_CONTROLLER
>  	select PINCTRL
>  	select COMMON_CLK
>  	help
> -	  This enables support for Canaan Kendryte K210 SoC platform hardware.
> +	  This enables support for Canaan Kendryte SoC platform hardware.
>  
>  endmenu # "SoC selection"
> -- 
> 2.43.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* Re: [PATCH 2/5] dt-bindings: add Canaan K230 boards compatible strings
  2024-03-04  8:51     ` Yangyu Chen
  2024-03-04 10:11       ` Conor Dooley
@ 2024-03-04 10:27       ` Krzysztof Kozlowski
  1 sibling, 0 replies; 15+ messages in thread
From: Krzysztof Kozlowski @ 2024-03-04 10:27 UTC (permalink / raw)
  To: Yangyu Chen, linux-riscv
  Cc: Conor Dooley, Rob Herring, Krzysztof Kozlowski, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Guo Ren, devicetree, linux-kernel

On 04/03/2024 09:51, Yangyu Chen wrote:
> On 2024/3/4 16:11, Krzysztof Kozlowski wrote:
>> On 03/03/2024 14:26, Yangyu Chen wrote:
>>> Since K230 was released, K210 is no longer the only SoC in the Kendryte
>>> series, so remove the K210 string from the description. Also, add two
>>> boards based on k230 to compatible strings to allow them to be used in the
>>> dt.
>>>
>>> Signed-off-by: Yangyu Chen <cyy@cyyself.name>
>>> ---
>>>   Documentation/devicetree/bindings/riscv/canaan.yaml | 13 ++++++++++++-
>>>   1 file changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/riscv/canaan.yaml b/Documentation/devicetree/bindings/riscv/canaan.yaml
>>> index 41fd11f70a49..444758db964e 100644
>>> --- a/Documentation/devicetree/bindings/riscv/canaan.yaml
>>> +++ b/Documentation/devicetree/bindings/riscv/canaan.yaml
>>> @@ -10,7 +10,7 @@ maintainers:
>>>     - Damien Le Moal <dlemoal@kernel.org>
>>>   
>>>   description:
>>> -  Canaan Kendryte K210 SoC-based boards
>>> +  Canaan Kendryte SoC-based boards
>>>   
>>>   properties:
>>>     $nodename:
>>> @@ -42,6 +42,17 @@ properties:
>>>         - items:
>>>             - const: canaan,kendryte-k210
>>>   
>>> +      - items:
>>> +          - const: canaan,k230-usip-lp3-evb
>>> +          - const: canaan,kendryte-k230
>>> +
>>> +      - items:
>>> +          - const: canaan,canmv-k230
>>
>> Why this is not part of previous entry in an enum?
>>
>>> +          - const: canaan,kendryte-k230
>>> +
>>> +      - items:
>>> +          - const: canaan,kendryte-k230
>>
>> Usually you cannot run SoCs alone. What does it represent (in real life)?
>>
> 
> I'm not sure what it means.
> 
> If you wonder why should I add a compatible string for soc, that is 
> although we cannot run SoCs alone, adding a soc compatible will allow 
> some bootloaders or SBI on RISC-V to choose an errata for a soc. Such as 
> this opensbi patch. [1]

No, this piece of code will not allow this. They choose errata
regardless of this change.

> 
> If you wonder why I should allow a soc-compatible string with soc alone, 
> that is because k210 did it previously. And provide a k210_generic.dts 

I don't remember background behind k210_generic. Any SoC-compatible
alone is exception, so needs serious justification. Drop it or provide
proper rationale.

> to use it. I haven't provided generic dts now but allowing only 
> soc-compatible string alone would also be acceptable I think.

No, it is not. Stop making own rules.


Best regards,
Krzysztof


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

* Re: [PATCH 4/5] riscv: dts: add initial canmv-k230 and k230-evb dts
  2024-03-03 13:26 ` [PATCH 4/5] riscv: dts: add initial canmv-k230 and k230-evb dts Yangyu Chen
  2024-03-04  8:13   ` Krzysztof Kozlowski
@ 2024-03-04 19:00   ` Conor Dooley
  2024-03-04 19:51     ` Yangyu Chen
  1 sibling, 1 reply; 15+ messages in thread
From: Conor Dooley @ 2024-03-04 19:00 UTC (permalink / raw)
  To: Yangyu Chen
  Cc: linux-riscv, Rob Herring, Krzysztof Kozlowski, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Guo Ren, devicetree, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 4127 bytes --]

Hey,

Meant to reply here earlier but I got distracted.

On Sun, Mar 03, 2024 at 09:26:26PM +0800, Yangyu Chen wrote:
> Add initial dts for CanMV-K230 and K230-EVB powered by Canaan Kendryte
> K230 SoC [1].
> 
> Some key considerations:
> - Only enable BigCore which is 1.6GHz RV64GCBV
> 
> Since is there cache coherence between two cores remains a mystery since
> they have a dedicated L2 Cache. And the factory SDK uses it for other OS
> by default.
> 
> Meanwhile, although docs from Canaan said 1.6GHz Core with Vector is
> CPU1, the csr.mhartid of this core is 0.
> 
> - Support for "zba" "zbb" "zbc" "zbs" are tested by hand
> 
> The user manual of C908 from T-Head does not document it specifically.
> It just said it supports B extension V1.0-rc1. [2]
> 
> - Support for "zicbom" is tested by hand
> 
> Have tested with some out-of-tree drivers that need DMA and they do not
> come to the dts currently.
> 
> - Cache parameters are inferred from T-Head docs [2] and Cannan docs [1]
> 
> L1i: 32KB, VIPT 4-Way set-associative, 64B Cacheline
> L1d: 32KB, VIPT 4-Way set-associative, 64B Cacheline
> L2: 256KB, PIPI 16-way set-associative, 64B Cacheline
> 
> The numbers of cache sets are calculated from these parameters.
> 
> - MMU only supports Sv39
> 
> Since T-Head docs [2] says C908 should support sv48. However, it will fail
> during the kernel probe. I also tested it by hand on M-Mode software,
> writing sv48 to satp.mode will not trap but will leave the csr unchanged.
> 
> [1] https://developer.canaan-creative.com/k230/dev/zh/00_hardware/K230_datasheet.html#chapter-1-introduction
> [2] https://occ-intl-prod.oss-ap-southeast-1.aliyuncs.com/resource//1699268369347/XuanTie-C908-UserManual.pdf
> 
> Signed-off-by: Yangyu Chen <cyy@cyyself.name>
> ---
>  arch/riscv/boot/dts/canaan/Makefile       |   2 +
>  arch/riscv/boot/dts/canaan/canmv-k230.dts |  23 ++++

Could you name this file "k230-canmv.dts" please, so that the soc comes
first?

>  arch/riscv/boot/dts/canaan/k230-evb.dts   |  23 ++++
>  arch/riscv/boot/dts/canaan/k230.dtsi      | 146 ++++++++++++++++++++++
>  4 files changed, 194 insertions(+)
>  create mode 100644 arch/riscv/boot/dts/canaan/canmv-k230.dts
>  create mode 100644 arch/riscv/boot/dts/canaan/k230-evb.dts
>  create mode 100644 arch/riscv/boot/dts/canaan/k230.dtsi
> 
> diff --git a/arch/riscv/boot/dts/canaan/Makefile b/arch/riscv/boot/dts/canaan/Makefile
> index 987d1f0c41f0..b4a0ec668f9a 100644
> --- a/arch/riscv/boot/dts/canaan/Makefile
> +++ b/arch/riscv/boot/dts/canaan/Makefile
> @@ -5,3 +5,5 @@ dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_bit.dtb
>  dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_dock.dtb
>  dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_go.dtb
>  dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maixduino.dtb
> +dtb-$(CONFIG_ARCH_CANAAN) += k230-evb.dtb
> +dtb-$(CONFIG_ARCH_CANAAN) += canmv-k230.dtb
> \ No newline at end of file
> diff --git a/arch/riscv/boot/dts/canaan/canmv-k230.dts b/arch/riscv/boot/dts/canaan/canmv-k230.dts
> new file mode 100644
> index 000000000000..09777616d30e
> --- /dev/null
> +++ b/arch/riscv/boot/dts/canaan/canmv-k230.dts
> @@ -0,0 +1,23 @@
> +// SPDX-License-Identifier: GPL-2.0+

Is there a reason that you only put these under GPL-2.0+?
The usual license for DT stuff is (GPL-2.0 OR BSD-2-Clause), dual
licensing makes it easier for other projects to use the devicetrees.

> +
> +		plic: interrupt-controller@f00000000 {
> +			compatible = "thead,c900-plic";
> +			reg = <0xf 0x00000000 0x0 0x04000000>;
> +			interrupts-extended = <&cpu0_intc 11>, <&cpu0_intc 9>;
> +			interrupt-controller;
> +			reg-names = "control";
> +			#address-cells = <0>;
> +			#interrupt-cells = <2>;
> +			riscv,ndev = <208>;
> +		};
> +
> +		clint: timer@f04000000 {
> +			compatible = "thead,c900-clint";
> +			reg = <0xf 0x04000000 0x0 0x04000000>;
> +			interrupts-extended = <&cpu0_intc 3>, <&cpu0_intc 7>;
> +		};

Both of these should have SoC-specific compatibles. Without them, this
should not pass dtbs_check. Did you run it?

Cheers,
Conor.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* Re: [PATCH 4/5] riscv: dts: add initial canmv-k230 and k230-evb dts
  2024-03-04 19:00   ` Conor Dooley
@ 2024-03-04 19:51     ` Yangyu Chen
  0 siblings, 0 replies; 15+ messages in thread
From: Yangyu Chen @ 2024-03-04 19:51 UTC (permalink / raw)
  To: Conor Dooley
  Cc: linux-riscv, Rob Herring, Krzysztof Kozlowski, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Guo Ren, devicetree, linux-kernel

On 2024/3/5 03:00, Conor Dooley wrote:
> Hey,
> 
> Meant to reply here earlier but I got distracted.
> 
> On Sun, Mar 03, 2024 at 09:26:26PM +0800, Yangyu Chen wrote:
>> Add initial dts for CanMV-K230 and K230-EVB powered by Canaan Kendryte
>> K230 SoC [1].
>>
>> Some key considerations:
>> - Only enable BigCore which is 1.6GHz RV64GCBV
>>
>> Since is there cache coherence between two cores remains a mystery since
>> they have a dedicated L2 Cache. And the factory SDK uses it for other OS
>> by default.
>>
>> Meanwhile, although docs from Canaan said 1.6GHz Core with Vector is
>> CPU1, the csr.mhartid of this core is 0.
>>
>> - Support for "zba" "zbb" "zbc" "zbs" are tested by hand
>>
>> The user manual of C908 from T-Head does not document it specifically.
>> It just said it supports B extension V1.0-rc1. [2]
>>
>> - Support for "zicbom" is tested by hand
>>
>> Have tested with some out-of-tree drivers that need DMA and they do not
>> come to the dts currently.
>>
>> - Cache parameters are inferred from T-Head docs [2] and Cannan docs [1]
>>
>> L1i: 32KB, VIPT 4-Way set-associative, 64B Cacheline
>> L1d: 32KB, VIPT 4-Way set-associative, 64B Cacheline
>> L2: 256KB, PIPI 16-way set-associative, 64B Cacheline
>>
>> The numbers of cache sets are calculated from these parameters.
>>
>> - MMU only supports Sv39
>>
>> Since T-Head docs [2] says C908 should support sv48. However, it will fail
>> during the kernel probe. I also tested it by hand on M-Mode software,
>> writing sv48 to satp.mode will not trap but will leave the csr unchanged.
>>
>> [1] https://developer.canaan-creative.com/k230/dev/zh/00_hardware/K230_datasheet.html#chapter-1-introduction
>> [2] https://occ-intl-prod.oss-ap-southeast-1.aliyuncs.com/resource//1699268369347/XuanTie-C908-UserManual.pdf
>>
>> Signed-off-by: Yangyu Chen <cyy@cyyself.name>
>> ---
>>   arch/riscv/boot/dts/canaan/Makefile       |   2 +
>>   arch/riscv/boot/dts/canaan/canmv-k230.dts |  23 ++++
> 
> Could you name this file "k230-canmv.dts" please, so that the soc comes
> first?
> 

OK. For patch v3.

>>   arch/riscv/boot/dts/canaan/k230-evb.dts   |  23 ++++
>>   arch/riscv/boot/dts/canaan/k230.dtsi      | 146 ++++++++++++++++++++++
>>   4 files changed, 194 insertions(+)
>>   create mode 100644 arch/riscv/boot/dts/canaan/canmv-k230.dts
>>   create mode 100644 arch/riscv/boot/dts/canaan/k230-evb.dts
>>   create mode 100644 arch/riscv/boot/dts/canaan/k230.dtsi
>>
>> diff --git a/arch/riscv/boot/dts/canaan/Makefile b/arch/riscv/boot/dts/canaan/Makefile
>> index 987d1f0c41f0..b4a0ec668f9a 100644
>> --- a/arch/riscv/boot/dts/canaan/Makefile
>> +++ b/arch/riscv/boot/dts/canaan/Makefile
>> @@ -5,3 +5,5 @@ dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_bit.dtb
>>   dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_dock.dtb
>>   dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_go.dtb
>>   dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maixduino.dtb
>> +dtb-$(CONFIG_ARCH_CANAAN) += k230-evb.dtb
>> +dtb-$(CONFIG_ARCH_CANAAN) += canmv-k230.dtb
>> \ No newline at end of file
>> diff --git a/arch/riscv/boot/dts/canaan/canmv-k230.dts b/arch/riscv/boot/dts/canaan/canmv-k230.dts
>> new file mode 100644
>> index 000000000000..09777616d30e
>> --- /dev/null
>> +++ b/arch/riscv/boot/dts/canaan/canmv-k230.dts
>> @@ -0,0 +1,23 @@
>> +// SPDX-License-Identifier: GPL-2.0+
> 
> Is there a reason that you only put these under GPL-2.0+?
> The usual license for DT stuff is (GPL-2.0 OR BSD-2-Clause), dual
> licensing makes it easier for other projects to use the devicetrees.
> 

No. Just choose the same license from K210. I will change to both 
GPL-2.0 or BSD-2-Caluse on patchv3.

>> +
>> +		plic: interrupt-controller@f00000000 {
>> +			compatible = "thead,c900-plic";
>> +			reg = <0xf 0x00000000 0x0 0x04000000>;
>> +			interrupts-extended = <&cpu0_intc 11>, <&cpu0_intc 9>;
>> +			interrupt-controller;
>> +			reg-names = "control";
>> +			#address-cells = <0>;
>> +			#interrupt-cells = <2>;
>> +			riscv,ndev = <208>;
>> +		};
>> +
>> +		clint: timer@f04000000 {
>> +			compatible = "thead,c900-clint";
>> +			reg = <0xf 0x04000000 0x0 0x04000000>;
>> +			interrupts-extended = <&cpu0_intc 3>, <&cpu0_intc 7>;
>> +		};
> 
> Both of these should have SoC-specific compatibles. Without them, this
> should not pass dtbs_check. Did you run it?
> 

Sorry. I haven't run it before submitting patch v1. But I have run it 
and got something fixed on patchv2.

> Cheers,
> Conor.
> 

To be honest, I want some review comments on the CPU node. As we know 
K230 is a dual-core soc. But I didn't know the details of the bus, even 
for is there was cache coherence between two cores. The factory SDK also 
provides a linux dts having only one core. I don't know whether it is 
acceptable.


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

end of thread, other threads:[~2024-03-04 19:51 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-03 13:24 [PATCH 0/5] riscv: add initial support for Canaan Kendryte K230 Yangyu Chen
2024-03-03 13:26 ` [PATCH 1/5] dt-bindings: riscv: Add T-HEAD C908 compatible Yangyu Chen
2024-03-04 10:12   ` Conor Dooley
2024-03-03 13:26 ` [PATCH 2/5] dt-bindings: add Canaan K230 boards compatible strings Yangyu Chen
2024-03-04  8:11   ` Krzysztof Kozlowski
2024-03-04  8:51     ` Yangyu Chen
2024-03-04 10:11       ` Conor Dooley
2024-03-04 10:27       ` Krzysztof Kozlowski
2024-03-03 13:26 ` [PATCH 3/5] riscv: Kconfig.socs: Allow SOC_CANAAN with MMU for K230 Yangyu Chen
2024-03-04 10:18   ` Conor Dooley
2024-03-03 13:26 ` [PATCH 4/5] riscv: dts: add initial canmv-k230 and k230-evb dts Yangyu Chen
2024-03-04  8:13   ` Krzysztof Kozlowski
2024-03-04 19:00   ` Conor Dooley
2024-03-04 19:51     ` Yangyu Chen
2024-03-03 13:26 ` [PATCH 5/5] riscv: config: enable SOC_CANAAN in defconfig Yangyu Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).