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

[Conor Dooley <conor@kernel.org>]

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

On Tue, Oct 13, 2026 at 10:36:18AM +0000, sashiko-bot@kernel.org wrote:
> 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?

This seems like a fairly reasonable comment from Sashiko. Does keeping it
here lead to hwloc incorrectly displaying your topology?

> 
> > +				/*
> > +				 * 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>;
> > +			};
> > +		};

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