Re: [PATCH 5/9] riscv: dts: ultrarisc: Add initial device tree for UltraRISC DP1000

[Conor Dooley <conor.dooley@microchip.com>]

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

On Fri, May 15, 2026 at 09:18:01AM +0800, Jia Wang wrote:
> Add the base device tree for the UltraRISC DP1000 SoC. It describes the
> 8×CP100 CPU cores and essential SoC peripherals including the interrupt
> controller, pinctrl, GPIO, UART, SPI, I2C, PCIe, GMAC and the DMA
> controller.
> 
> Link: https://lore.kernel.org/lkml/20260427-ultrarisc-pcie-v4-2-98935f6cdfb5@ultrarisc.com/
> Link: https://lore.kernel.org/lkml/20260429-ultrarisc-serial-v7-3-e475cce9e274@ultrarisc.com/
> 

No blank line here FYI.

> Signed-off-by: Jia Wang <wangjia@ultrarisc.com>
> ---
>  MAINTAINERS                               |   1 +
>  arch/riscv/boot/dts/ultrarisc/dp1000.dtsi | 851 ++++++++++++++++++++++++++++++
>  2 files changed, 852 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index baaaa46b1a56..832e01898ae5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -23087,6 +23087,7 @@ M:	Jia Wang <wangjia@ultrarisc.com>
>  L:	linux-riscv@lists.infradead.org
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/riscv/ultrarisc.yaml
> +F:	arch/riscv/boot/dts/ultrarisc/
>  
>  RNBD BLOCK DRIVERS
>  M:	Md. Haris Iqbal <haris.iqbal@ionos.com>
> diff --git a/arch/riscv/boot/dts/ultrarisc/dp1000.dtsi b/arch/riscv/boot/dts/ultrarisc/dp1000.dtsi
> new file mode 100644
> index 000000000000..1aae53fc1a2b
> --- /dev/null
> +++ b/arch/riscv/boot/dts/ultrarisc/dp1000.dtsi
> @@ -0,0 +1,851 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright(C) 2026 UltraRISC Technology (Shanghai) Co., Ltd.
> + */
> +
> +/dts-v1/;
> +
> +/ {
> +	compatible = "ultrarisc,dp1000";
> +	#address-cells = <2>;
> +	#size-cells = <2>;
> +
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		timebase-frequency = <10000000>;
> +
> +		cpu0: cpu@0 {
> +			compatible = "ultrarisc,cp100", "riscv";
> +			reg = <0x0>;
> +			device_type = "cpu";
> +			riscv,isa = "rv64imafdcbh";

Drop this property, its replacements (below) should be supported by all
users.

> +			riscv,isa-base = "rv64i";
> +			riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "b", "h",
> +					       "zba", "zbb", "zbc", "zbs", "zicntr",
> +					       "zicsr", "zifencei", "zihpm", "ziccif",
> +					       "ziccrse", "ziccamoa", "za64rs", "zicbom",
> +					       "zicbop", "zicboz", "zkt", "svade",
> +					       "ssccptr", "sstvecd", "sscounterenw",
> +					       "shcounterenw", "shtvala", "shvstvecd",
> +					       "shvsatpa", "svvptc";
> +			mmu-type = "riscv,sv48";
> +			clock-frequency = <2000000000>;
> +			/* L1 I-cache and D-cache:

FYI, the comment format outside of drivers/net/ has a newline after /*.

> +			 * block-size 64B
> +			 * 4-way set associative, size 64KB
> +			 * per-core.
> +			 */
> +			d-cache-block-size = <64>;
> +			d-cache-sets = <256>;
> +			d-cache-size = <0x10000>;
> +			i-cache-block-size = <64>;
> +			i-cache-sets = <256>;
> +			i-cache-size = <0x10000>;
> +			next-level-cache = <&l2_cache0>;
> +			riscv,cbom-block-size = <64>;
> +			riscv,cboz-block-size = <64>;
> +
> +			cpu0_intc: interrupt-controller {
> +				compatible = "riscv,cpu-intc";
> +				interrupt-controller;
> +				#interrupt-cells = <0x01>;
> +			};
> +
> +			l2_cache0: l2-cache {
> +				/* L2 cache:
> +				 * cache-unified, block-size 64B
> +				 * 8-way set associative, size 512KB
> +				 * per-core.
> +				 */
> +				compatible = "cache";
> +				cache-block-size = <64>;
> +				cache-level = <2>;
> +				cache-size = <0x80000>;
> +				cache-sets = <1024>;
> +				cache-unified;
> +				next-level-cache = <&cluster0_l3>;
> +			};
> +		};

> +		cpu4: cpu@4 {
> +			compatible = "ultrarisc,cp100", "riscv";
> +			reg = <0x10>;

Sashiko correctly pointed out that this is wrong.

> +		cpu5: cpu@5 {
> +			compatible = "ultrarisc,cp100", "riscv";
> +			reg = <0x11>;

And so is this.

> +		cpu6: cpu@6 {
> +			compatible = "ultrarisc,cp100", "riscv";
> +			reg = <0x12>;

And this.

> +		cpu7: cpu@7 {
> +			compatible = "ultrarisc,cp100", "riscv";
> +			reg = <0x13>;

And this.

> +	clocks {

I've commented on this in the cover letter.

> +		device_clk: device_clk {
> +			compatible = "fixed-clock";
> +			clock-frequency = <62500000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		timer_clk: timer_clk {
> +			compatible = "fixed-clock";
> +			clock-frequency = <50000000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		csr_clk: csr_clk {
> +			compatible = "fixed-clock";
> +			clock-frequency = <250000000>;
> +			#clock-cells = <0>;
> +		};
> +	};

> +	memory@80000000 {
> +		device_type = "memory";
> +		reg = <0x00 0x80000000 0x4 0x00000000>;
> +	};

This looks incorrectly placed, memory should be in the board dts unless
it is actually a part of the chip.

> +
> +	soc {
> +		compatible = "simple-bus";
> +		ranges;
> +		#address-cells = <0x02>;
> +		#size-cells = <0x02>;
> +
> +		clint: clint@8000000 {
> +			compatible = "sifive,clint0", "riscv,clint0";

Missing a device-specific clint compatible.

> +			reg = <0x00 0x8000000 0x00 0x100000>;
> +			interrupts-extended = <&cpu0_intc 0x03>, <&cpu0_intc 0x07>,
> +					      <&cpu1_intc 0x03>, <&cpu1_intc 0x07>,
> +					      <&cpu2_intc 0x03>, <&cpu2_intc 0x07>,
> +					      <&cpu3_intc 0x03>, <&cpu3_intc 0x07>,
> +					      <&cpu4_intc 0x03>, <&cpu4_intc 0x07>,
> +					      <&cpu5_intc 0x03>, <&cpu5_intc 0x07>,
> +					      <&cpu6_intc 0x03>, <&cpu6_intc 0x07>,
> +					      <&cpu7_intc 0x03>, <&cpu7_intc 0x07>;
> +		};
> +
> +		plic: plic@9000000 {
> +			compatible = "ultrarisc,dp1000-plic", "ultrarisc,cp100-plic";
> +			reg = <0x00 0x9000000 0x00 0x4000000>;
> +			#interrupt-cells = <1>;
> +			#address-cells = <0>;
> +			interrupt-controller;
> +			interrupts-extended = <&cpu0_intc 0xb>, <&cpu0_intc 0x9>, <&cpu0_intc 0xa>,
> +					      <&cpu1_intc 0xb>, <&cpu1_intc 0x9>, <&cpu1_intc 0xa>,
> +					      <&cpu2_intc 0xb>, <&cpu2_intc 0x9>, <&cpu2_intc 0xa>,
> +					      <&cpu3_intc 0xb>, <&cpu3_intc 0x9>, <&cpu3_intc 0xa>,
> +					      <&cpu4_intc 0xb>, <&cpu4_intc 0x9>, <&cpu4_intc 0xa>,
> +					      <&cpu5_intc 0xb>, <&cpu5_intc 0x9>, <&cpu5_intc 0xa>,
> +					      <&cpu6_intc 0xb>, <&cpu6_intc 0x9>, <&cpu6_intc 0xa>,
> +					      <&cpu7_intc 0xb>, <&cpu7_intc 0x9>, <&cpu7_intc 0xa>;
> +			riscv,ndev = <160>;
> +		};
> +
> +		pmx0: pinmux@11081000 {
> +			compatible = "ultrarisc,dp1000-pinctrl";
> +			reg = <0x0 0x11081000  0x0 0x1000>;
                                             ^^ extra space here btw

> +			#pinctrl-cells = <2>;
> +		};
> +
> +		spi0: spi@20320000 {
> +			compatible = "snps,dw-apb-ssi";

Missing a device-specific compatible here, can just fall abck to the
dw-apb-ssi one, so no need to make driver changes.

> +			reg = <0x0 0x20320000 0x0 0x1000>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			clocks = <&device_clk>;
> +			interrupt-parent = <&plic>;
> +			interrupts = <19>;
> +			num-cs = <3>;
> +		};

> +
> +		i2c0: i2c@20330000 {
> +			compatible = "snps,designware-i2c";

Same applies here.

> +			reg = <0x0 0x20330000 0x0 0x100>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			clock-frequency = <400000>;
> +			clocks = <&device_clk>;
> +			interrupt-parent = <&plic>;
> +			interrupts = <20>;
> +		};

> +
> +		ethernet: ethernet@38000000 {
> +			compatible = "snps,dwmac", "snps,dwmac-5.10a";

Surprised this passes dtbs_check. Same here, you need a specific
compatible.

> +			reg = <0x00 0x38000000 0x00 0x1000000>;
> +			clocks = <&csr_clk>;
> +			clock-names = "stmmaceth";
> +			interrupt-parent = <&plic>;
> +			interrupts = <84>;
> +			interrupt-names = "macirq";
> +			local-mac-address = [ff ff ff ff ff ff];
> +			max-speed = <1000>;
> +			phy-mode = "rgmii-id";
> +			snps,txpbl = <8>;
> +			snps,rxpbl = <8>;
> +		};
> +
> +		dmac: dma-controller@39000000 {
> +			compatible = "snps,axi-dma-1.01a";

Same thing here, I'd like to see a device specific compatible too.

> +			reg = <0x0 0x39000000 0x0 0x400>;
> +			clocks = <&device_clk>, <&device_clk>;

This kind of thing is what makes me think that you're over simplifying
your clock tree. Sure, maybe you do use the same clock here for both
ports, but it's suspect.

Cheers,
Conor.

> +			clock-names = "core-clk", "cfgr-clk";
> +			#dma-cells = <1>;
> +			dma-channels = <8>;
> +			interrupt-parent = <&plic>;
> +			interrupts = <152>;
> +			snps,dma-masters = <1>;
> +			snps,data-width = <4>;
> +			snps,block-size = <512 512 512 512 512 512 512 512>;
> +			snps,priority = <0 1 2 3 4 5 6 7>;
> +			snps,axi-max-burst-len = <256>;
> +		};
> +	};
> +};
> 
> -- 
> 2.34.1
> 

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