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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dts: L2 cache nodes are incorrectly nested as children of cpu nodes
--

commit 72d6d58d85f9aab9e46d9c7167175451c2551346
Author: Jia Wang <wangjia@ultrarisc.com>

riscv: dts: ultrarisc: Add initial device tree for UltraRISC DP1000

Add the base device tree for the UltraRISC DP1000 SoC. It describes
eight CP100 CPU cores, their cache topology, and essential peripherals.

> diff --git a/arch/riscv/boot/dts/ultrarisc/dp1000.dtsi b/arch/riscv/boot/dts/ultrarisc/dp1000.dtsi
> new file mode 100644
> index 0000000000000..86d7f5533aa84
> --- /dev/null
> +++ b/arch/riscv/boot/dts/ultrarisc/dp1000.dtsi
> @@ -0,0 +1,899 @@
[ ... ]
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		timebase-frequency = <10000000>;
> +
> +		cpu0: cpu@0 {
> +			compatible = "ultrarisc,cp100", "riscv";
[ ... ]
> +			cpu0_intc: interrupt-controller {
> +				compatible = "riscv,cpu-intc";
> +				interrupt-controller;
> +				#interrupt-cells = <1>;
> +			};
> +
> +			l2_cache0: l2-cache {

[Severity: Medium]
Does this incorrectly nest the L2 cache nodes as children of the cpu nodes?

Standard Devicetree topologies for CPU schemas require cache nodes to be
defined as siblings under the /cpus node or globally, rather than as direct
children of the cpu nodes. Instantiating l2-cache here can cause
dt_binding_check validation failures and break generic kernel cache topology
parsers that expect the cache nodes at the root of /cpus and referenced via
the next-level-cache phandle.

Should these cache nodes be moved outside of the cpu@0 node?

> +				/*
> +				 * 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@10 {
> +			compatible = "ultrarisc,cp100", "riscv";
[ ... ]
> +			cpu4_intc: interrupt-controller {
> +				compatible = "riscv,cpu-intc";
> +				interrupt-controller;
> +				#interrupt-cells = <1>;
> +			};
> +
> +			l2_cache4: l2-cache {

[Severity: Medium]
The same structural error appears to be repeated across all CPU nodes in the
cluster. Could these cache nodes be moved outside their respective cpu nodes
as well?

> +				/*
> +				 * L2 cache:
> +				 * cache-unified, block-size 64B
> +				 * 8-way set associative, size 512KB

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013-ultrarisc-dts-v3-0-5ff3ebb2a90d@ultrarisc.com?part=5