Received-SPF: pass (google.com: domain of linux-kernel+bounces-92635-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Tue, 05 Mar 2024 07:54:05 -0800 (PST)
Subject:     Re: [PATCH v3 6/7] riscv: dts: add initial canmv-k230 and k230-evb dts
In-Reply-To: <tencent_1B7642BA1B77F81E0423B41B1DD770E36209@qq.com>
CC: linux-riscv@lists.infradead.org, Conor Dooley <conor@kernel.org>,
  dlemoal@kernel.org, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
  Paul Walmsley <paul.walmsley@sifive.com>, aou@eecs.berkeley.edu, guoren@kernel.org, devicetree@vger.kernel.org,
  linux-kernel@vger.kernel.org, cyy@cyyself.name
From: Palmer Dabbelt <palmer@dabbelt.com>
To: cyy@cyyself.name
Message-ID: <mhng-f1aa91d4-211f-4eb4-a94d-a9d88b0d3f6a@palmer-ri-x1c9>
Precedence: bulk
Mime-Version: 1.0 (MHng)
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit

On Mon, 04 Mar 2024 13:06:00 PST (-0800), cyy@cyyself.name wrote:
> Add initial dts for CanMV-K230 and K230-EVB powered by Canaan Kendryte
> K230 SoC [1].
>
> Some key consideration:
> - Only place BigCore which is 1.6GHz RV64GCBV
>
> The existence of cache coherence between the two cores remains unknown
> since they have dedicated L2 caches. And the factory SDK uses it for
> other OS by default. I don't know whether the two CPUs on K230 SoC
> can be used in one system. So only place BigCore here.
>
> 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]

I'm seeing

    3.1.6 Vector instruction set (RVV)
    For more information, see RISC-V “V”Vector Extension, Version 
    1.0-rc1-20210608
    URL：https://github.com/riscv/riscv-v-spec/releases/download/v1.0-rc1/riscv-v-spec-1.0-rc1.pdf

    3.1.7 Bit operation instruction set (RV64B)
    For more information, see ：RISC-V Bit-Manipulation ISA-extensions, 
    Version 1.0.0, 2021-06-12: public review
    URL：https://github.com/riscv/riscv-bitmanip/releases/download/1.0.0/bitmanip-1.0.0.pdf

so I think maybe that's vector 1.0-rc1 and the final bitmanip?

> I have tested it by using this [3] which attempts to execute "add.uw",
> "andn", "clmulr", "bclr" and they doesn't traps on K230. But on JH7110,
> "clmulr" and "bclr" will trap.
>
> - Support for "zicbom" is tested by hand
>
> Have tested with some out-of-tree drivers from [4] that need DMA and they
> do not come to the dts currently.
>
> - Cache parameters are inferred from T-Head docs [2] and Canaan docs [1]
>
> L1i: 32KB, VIPT 4-Way set-associative, 64B Cacheline
> L1d: 32KB, VIPT 4-Way set-associative, 64B Cacheline
> L2: 256KB, PIPT 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] say C908 should support Sv48. However, it will fail
> during the kernel probe when running Linux on K230. I also tested it by
> hand on M-Mode software, writing Sv48 to satp.mode will not trap but will
> leave the csr unchanged. While writing Sv39 it will take effect. It shows
> that this CPU does not support Sv48.

I think that only works due to a bug in our sv48 probing routines -- 
unless I'm missing something, we mixed up the sv57 and sv48 probing and 
if sv57 fails to probe then we disable sv48 as well.  So I think we 
probably need some cleanup in there.

> - Svpbmt and T-Head MAEE both supported
>
> T-Head C908 does support both Svpbmt and T-Head MAEE for page-based memory
> attributes and is controlled by csr.mxstatus. If the kernel wants to use
> svpbmt, the m-mode software should set BIT(21) of csr.mxstatus to zero
> before entering the s-mode kernel. Otherwise, the kernel will not boot as 0
> on T-Head MAEE represent to NonCachable Memory and it will lose dirty cache
> lines modification that haven't been written back to the memory.

So I guess we need the bootloader to just be accurate here?  ie: 
whatever extension it tells S-mode kernels is enabled is how S-mode 
behaves, and then this should just work itself out.

> [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
> [3] https://github.com/cyyself/rvb_test
> [4] https://github.com/cyyself/linux/tree/k230-mainline
>
> Signed-off-by: Yangyu Chen <cyy@cyyself.name>
> ---
>  arch/riscv/boot/dts/canaan/Makefile       |   2 +
>  arch/riscv/boot/dts/canaan/k230-canmv.dts |  24 ++++
>  arch/riscv/boot/dts/canaan/k230-evb.dts   |  24 ++++
>  arch/riscv/boot/dts/canaan/k230.dtsi      | 140 ++++++++++++++++++++++
>  4 files changed, 190 insertions(+)
>  create mode 100644 arch/riscv/boot/dts/canaan/k230-canmv.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..7d54ea5c6f3d 100644
> --- a/arch/riscv/boot/dts/canaan/Makefile
> +++ b/arch/riscv/boot/dts/canaan/Makefile
> @@ -1,6 +1,8 @@
>  # SPDX-License-Identifier: GPL-2.0
>  dtb-$(CONFIG_ARCH_CANAAN) += canaan_kd233.dtb
>  dtb-$(CONFIG_ARCH_CANAAN) += k210_generic.dtb
> +dtb-$(CONFIG_ARCH_CANAAN) += k230-canmv.dtb
> +dtb-$(CONFIG_ARCH_CANAAN) += k230-evb.dtb
>  dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_bit.dtb
>  dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_dock.dtb
>  dtb-$(CONFIG_ARCH_CANAAN) += sipeed_maix_go.dtb
> diff --git a/arch/riscv/boot/dts/canaan/k230-canmv.dts b/arch/riscv/boot/dts/canaan/k230-canmv.dts
> new file mode 100644
> index 000000000000..3ab5c8de11a8
> --- /dev/null
> +++ b/arch/riscv/boot/dts/canaan/k230-canmv.dts
> @@ -0,0 +1,24 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +/*
> + * 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..42720113c566
> --- /dev/null
> +++ b/arch/riscv/boot/dts/canaan/k230-evb.dts
> @@ -0,0 +1,24 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +/*
> + * 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..0bcff67b78a8
> --- /dev/null
> +++ b/arch/riscv/boot/dts/canaan/k230.dtsi
> @@ -0,0 +1,140 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +/*
> + * 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;
> +		};
> +	};
> +
> +	apb_clk: apb-clk-clock {
> +		compatible = "fixed-clock";
> +		clock-frequency = <50000000>;
> +		clock-output-names = "apb_clk";
> +		#clock-cells = <0>;
> +	};
> +
> +	soc {
> +		compatible = "simple-bus";
> +		interrupt-parent = <&plic>;
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		dma-noncoherent;
> +		ranges;
> +
> +		plic: interrupt-controller@f00000000 {
> +			compatible = "canaan,k230-plic" ,"thead,c900-plic";
> +			reg = <0xf 0x00000000 0x0 0x04000000>;
> +			interrupts-extended = <&cpu0_intc 11>, <&cpu0_intc 9>;
> +			interrupt-controller;
> +			#address-cells = <0>;
> +			#interrupt-cells = <2>;
> +			riscv,ndev = <208>;
> +		};
> +
> +		clint: timer@f04000000 {
> +			compatible = "canaan,k230-clint", "thead,c900-clint";
> +			reg = <0xf 0x04000000 0x0 0x04000000>;
> +			interrupts-extended = <&cpu0_intc 3>, <&cpu0_intc 7>;
> +		};
> +
> +		uart0: serial@91400000 {
> +			compatible = "snps,dw-apb-uart";
> +			reg = <0x0 0x91400000 0x0 0x1000>;
> +			clocks = <&apb_clk>;
> +			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>;
> +			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>;
> +			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>;
> +			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>;
> +			interrupts = <20 IRQ_TYPE_LEVEL_HIGH>;
> +			reg-io-width = <4>;
> +			reg-shift = <2>;
> +			status = "disabled";
> +		};
> +	};
> +};