[PATCH v2 1/7] riscv: dts: sophgo: cv18xx: Move RiscV-specific part into SoCs' .dtsi files

[Inochi Amaoto]

On Wed, Feb 12, 2025 at 08:24:55PM +0100, Krzysztof Kozlowski wrote:
> On 12/02/2025 18:44, Alexander Sverdlin wrote:
> > Hi 
> > 
> > On Wed, 2025-02-12 at 17:46 +0100, Krzysztof Kozlowski wrote:
> >>>>> Make the peripheral device tree re-usable on ARM64 platform by moving CPU
> >>>>> core and interrupt controllers' parts into the respective per-SoC .dtsi
> >>>>> files.
> >>>>>
> >>>>> Add SOC_PERIPHERAL_IRQ() macro which explicitly maps peripheral nubering
> >>>>> into "plic" interrupt-controller numbering.
> >>>>>
> >>>>> Have a nice refactoring side-effect that "plic" and "clint" "compatible"
> >>>>> property is not specified outside of the corresponding device itself.
> >>>>>
> >>>>> Signed-off-by: Alexander Sverdlin <alexander.sverdlin at gmail.com>
> >>>>> ---
> >>>>> Changelog:
> >>>>> v2:
> >>>>> - instead of carving out peripherals' part, carve out ARCH-specifics (CPU
> >>>>> core, interrupt controllers) and spread them among 3 SoC .dtsi files which
> >>>>> included cv18xx.dtsi;
> >>>>> - define a label for the "soc" node and use it in the newly introduced DTs;
> >>>>>
> >>>>>  arch/riscv/boot/dts/sophgo/cv1800b.dtsi    | 64 ++++++++++++---
> >>>>>  arch/riscv/boot/dts/sophgo/cv1812h.dtsi    | 64 ++++++++++++---
> >>>>>  arch/riscv/boot/dts/sophgo/cv181x.dtsi     |  2 +-
> >>>>>  arch/riscv/boot/dts/sophgo/cv18xx-cpu.dtsi | 57 ++++++++++++++
> >>>>>  arch/riscv/boot/dts/sophgo/cv18xx.dtsi     | 91 ++++++----------------
> >>>>>  arch/riscv/boot/dts/sophgo/sg2002.dtsi     | 64 ++++++++++++---
> >>>>>  6 files changed, 240 insertions(+), 102 deletions(-)
> >>>>>  create mode 100644 arch/riscv/boot/dts/sophgo/cv18xx-cpu.dtsi
> >>>>>
> >>>>> diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
> >>>>> index aa1f5df100f0..eef2884b36f9 100644
> >>>>> --- a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
> >>>>> +++ b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
> >>>>> @@ -3,6 +3,8 @@
> >>>>>   * Copyright (C) 2023 Jisheng Zhang <jszhang at kernel.org>
> >>>>>   */
> >>>>>  
> >>>>> +#define SOC_PERIPHERAL_IRQ(nr)	((nr) + 16)
> >>>>> +
> >>>>>  #include <dt-bindings/pinctrl/pinctrl-cv1800b.h>
> >>>>>  #include "cv18xx.dtsi"
> >>>>>  
> >>>>> @@ -14,22 +16,62 @@ memory at 80000000 {
> >>>>>  		reg = <0x80000000 0x4000000>;
> >>>>>  	};
> >>>>>  
> >>>>
> >>>>> -	soc {
> >>>>> -		pinctrl: pinctrl at 3001000 {
> >>>>> -			compatible = "sophgo,cv1800b-pinctrl";
> >>>>> -			reg = <0x03001000 0x1000>,
> >>>>> -			      <0x05027000 0x1000>;
> >>>>> -			reg-names = "sys", "rtc";
> >>>>
> >>>>
> >>>>> +	cpus: cpus {
> >>>>> +		#address-cells = <1>;
> >>>>> +		#size-cells = <0>;
> >>>>> +		timebase-frequency = <25000000>;
> >>>>> +
> >>>>> +		cpu0: cpu at 0 {
> >>>>> +			compatible = "thead,c906", "riscv";
> >>>>> +			device_type = "cpu";
> >>>>> +			reg = <0>;
> >>>>> +			d-cache-block-size = <64>;
> >>>>> +			d-cache-sets = <512>;
> >>>>> +			d-cache-size = <65536>;
> >>>>> +			i-cache-block-size = <64>;
> >>>>> +			i-cache-sets = <128>;
> >>>>> +			i-cache-size = <32768>;
> >>>>> +			mmu-type = "riscv,sv39";
> >>>>> +			riscv,isa = "rv64imafdc";
> >>>>> +			riscv,isa-base = "rv64i";
> >>>>> +			riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "zicntr", "zicsr",
> >>>>> +					       "zifencei", "zihpm";
> >>>>> +
> >>>>> +			cpu0_intc: interrupt-controller {
> >>>>> +				compatible = "riscv,cpu-intc";
> >>>>> +				interrupt-controller;
> >>>>> +				#interrupt-cells = <1>;
> >>>>> +			};
> >>>>>  		};
> >>>>>  	};
> >>>>>  };
> >>>>
> >>>> Make all soc definition include the common cpu file. 
> >>>> Not just copy it.
> >>>
> >>> I was acting according to Krzysztof's suggestion:
> >>> https://lore.kernel.org/soc/d3ba0ea5-0491-42d5-a18e-64cf21df696c@kernel.org/
> >>>
> >>> Krzysztof, I can name the file cv18xx-cpu-intc.dtsi and pack CPU core + interrupt
> >>> controllers into it. Would it make sense?
> >>
> >>
> >> I don't understand the original suggestion.
> > 
> > This is the snippet in question:
> > 
> > ---[ cut ]---
> > #define SOC_PERIPHERAL_IRQ(nr)	((nr) + 16)
> > 
> > / {
> > 	cpus: cpus {
> > 		#address-cells = <1>;
> > 		#size-cells = <0>;
> > 		timebase-frequency = <25000000>;
> > 
> > 		cpu0: cpu at 0 {
> > 			compatible = "thead,c906", "riscv";
> > 			device_type = "cpu";
> > 			reg = <0>;
> > 			d-cache-block-size = <64>;
> > 			d-cache-sets = <512>;
> > 			d-cache-size = <65536>;
> > 			i-cache-block-size = <64>;
> > 			i-cache-sets = <128>;
> > 			i-cache-size = <32768>;
> > 			mmu-type = "riscv,sv39";
> > 			riscv,isa = "rv64imafdc";
> > 			riscv,isa-base = "rv64i";
> > 			riscv,isa-extensions = "i", "m", "a", "f", "d", "c", "zicntr", "zicsr",
> > 					       "zifencei", "zihpm";
> > 
> > 			cpu0_intc: interrupt-controller {
> > 				compatible = "riscv,cpu-intc";
> > 				interrupt-controller;
> > 				#interrupt-cells = <1>;
> > 			};
> > 		};
> > 	};
> > };
> > 
> > &soc {
> > 	interrupt-parent = <&plic>;
> > 	dma-noncoherent;
> > 
> > 	plic: interrupt-controller at 70000000 {
> > 		reg = <0x70000000 0x4000000>;
> > 		interrupts-extended = <&cpu0_intc 11>, <&cpu0_intc 9>;
> > 		interrupt-controller;
> > 		#address-cells = <0>;
> > 		#interrupt-cells = <2>;
> > 		riscv,ndev = <101>;
> > 	};
> > 
> > 	clint: timer at 74000000 {
> > 		reg = <0x74000000 0x10000>;
> > 		interrupts-extended = <&cpu0_intc 3>, <&cpu0_intc 7>;
> > 	};
> > };
> > ---[ cut ]---
> > 
> > Inochi's proposal is to put it into separate cv18xx-cpu-intc.dtsi and
> > include the latter in 3 other SoC-specific .dtsis. In v2 I've just
> > duplicated the above snippet 3 times (refer to diffstat above).
> > 
> > What are your thoughts? In Renesas everything is duplicated, I believe.
> > Sophgo outsources much smaller snippets into .dtsi (refer to cv181x.dtsi). 
> 
> If it represents some shared design/part, then it feels good.
> Best regards,
> Krzysztof

Yes, it does share the same design.

Regards,
Inochi