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

[Alexander Sverdlin <alexander.sverdlin@gmail.com>]

Hi Inochi!

On Thu, 2025-02-13 at 06:54 +0800, Inochi Amaoto wrote:
> On Wed, Feb 12, 2025 at 11:43:33PM +0100, Alexander Sverdlin wrote:
> > Make the peripheral device tree re-usable on ARM64 platform by moving CPU
> > core and interrupt controllers' parts into new cv18xx-cpu.dtsi and
> > cv18xx-intc.dtsi.
> > 
> > Add SOC_PERIPHERAL_IRQ() macro which explicitly maps peripheral nubering
> > into "plic" interrupt-controller numbering.
> > 
> > On RiscV side this patch has been dtc-compile-tested only. IRQ numbers
> > substitution with SOC_PERIPHERAL_IRQ() has been scripted in vim.
> > 
> > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
> > ---
> > Changelog:
> > v3:
> > - &cpus node has been moved into cv18xx-cpu.dtsi, &plic and &clint nodes
> > were moved into cv18xx-intc.dtsi to reduce code duplication;
> > 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     | 28 ++++---
> >   arch/riscv/boot/dts/sophgo/cv1812h.dtsi     | 28 ++++---
> >   arch/riscv/boot/dts/sophgo/cv181x.dtsi      |  2 +-
> >   arch/riscv/boot/dts/sophgo/cv18xx-cpu.dtsi  | 36 ++++++++
> >   arch/riscv/boot/dts/sophgo/cv18xx-intc.dtsi | 23 ++++++
> >   arch/riscv/boot/dts/sophgo/cv18xx.dtsi      | 91 ++++++---------------
> >   arch/riscv/boot/dts/sophgo/sg2002.dtsi      | 34 ++++----
> >   7 files changed, 137 insertions(+), 105 deletions(-)
> >   create mode 100644 arch/riscv/boot/dts/sophgo/cv18xx-cpu.dtsi
> >   create mode 100644 arch/riscv/boot/dts/sophgo/cv18xx-intc.dtsi
> > 
> > diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
> > index aa1f5df100f0..9fb7dd2dab18 100644
> > --- a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
> > +++ b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
> > @@ -3,8 +3,12 @@
> >    * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org>
> >    */
> >   
> > +#define SOC_PERIPHERAL_IRQ(nr)	((nr) + 16)
> > +
> >   #include <dt-bindings/pinctrl/pinctrl-cv1800b.h>
> > +#include "cv18xx-cpu.dtsi"
> >   #include "cv18xx.dtsi"
> > +#include "cv18xx-intc.dtsi"
> >   
> >   / {
> >   	compatible = "sophgo,cv1800b";
> > @@ -13,17 +17,23 @@ memory@80000000 {
> >   		device_type = "memory";
> >   		reg = <0x80000000 0x4000000>;
> >   	};
> > +};
> > +
> 
> > +&soc {
> > +	dma-noncoherent;
> >   
> > -	soc {
> > -		pinctrl: pinctrl@3001000 {
> > -			compatible = "sophgo,cv1800b-pinctrl";
> > -			reg = <0x03001000 0x1000>,
> > -			      <0x05027000 0x1000>;
> > -			reg-names = "sys", "rtc";
> > -		};
> > +	pinctrl: pinctrl@3001000 {
> > +		compatible = "sophgo,cv1800b-pinctrl";
> > +		reg = <0x03001000 0x1000>,
> > +		      <0x05027000 0x1000>;
> > +		reg-names = "sys", "rtc";
> >   	};
> >   };
> >   
> 
> The origianl /soc path can be used to override things, 
> why change it to the reference archor? I see no change
> for it.

https://lore.kernel.org/soc/d3ba0ea5-0491-42d5-a18e-64cf21df696c@kernel.org/
The corresponding label is being introduced in cv18xx.dtsi.

> > +&clk {
> > +	compatible = "sophgo,cv1800-clk";
> > +};
> > +
> >   &plic {
> >   	compatible = "sophgo,cv1800b-plic", "thead,c900-plic";
> >   };
> > @@ -31,7 +41,3 @@ &plic {
> >   &clint {
> >   	compatible = "sophgo,cv1800b-clint", "thead,c900-clint";
> >   };
> > -
> > -&clk {
> > -	compatible = "sophgo,cv1800-clk";
> > -};
> > diff --git a/arch/riscv/boot/dts/sophgo/cv1812h.dtsi b/arch/riscv/boot/dts/sophgo/cv1812h.dtsi
> > index 8a1b95c5116b..f23c4dd6687d 100644
> > --- a/arch/riscv/boot/dts/sophgo/cv1812h.dtsi
> > +++ b/arch/riscv/boot/dts/sophgo/cv1812h.dtsi
> > @@ -3,10 +3,14 @@
> >    * Copyright (C) 2023 Inochi Amaoto <inochiama@outlook.com>
> >    */
> >   
> > +#define SOC_PERIPHERAL_IRQ(nr)	((nr) + 16)
> > +
> >   #include <dt-bindings/interrupt-controller/irq.h>
> >   #include <dt-bindings/pinctrl/pinctrl-cv1812h.h>
> > +#include "cv18xx-cpu.dtsi"
> >   #include "cv18xx.dtsi"
> >   #include "cv181x.dtsi"
> > +#include "cv18xx-intc.dtsi"
> >   
> >   / {
> >   	compatible = "sophgo,cv1812h";
> > @@ -15,17 +19,23 @@ memory@80000000 {
> >   		device_type = "memory";
> >   		reg = <0x80000000 0x10000000>;
> >   	};
> > +};
> > +
> > +&soc {
> 
> > +	dma-noncoherent;
> 
> I think this can be moved into cv18xx-cpu.dtsi file, and let
> all SoCs share it.

https://lore.kernel.org/soc/d3ba0ea5-0491-42d5-a18e-64cf21df696c@kernel.org/

If SOC_PERIPHERAL_IRQ() doesn't belong to -cpu.dtsi, then "dma-noncoherent;"
probably neither...

> > -	soc {
> > -		pinctrl: pinctrl@3001000 {
> > -			compatible = "sophgo,cv1812h-pinctrl";
> > -			reg = <0x03001000 0x1000>,
> > -			      <0x05027000 0x1000>;
> > -			reg-names = "sys", "rtc";
> > -		};
> > +	pinctrl: pinctrl@3001000 {
> > +		compatible = "sophgo,cv1812h-pinctrl";
> > +		reg = <0x03001000 0x1000>,
> > +		      <0x05027000 0x1000>;
> > +		reg-names = "sys", "rtc";
> >   	};
> >   };
> >   
> > +&clk {
> > +	compatible = "sophgo,cv1810-clk";
> > +};
> > +
> >   &plic {
> >   	compatible = "sophgo,cv1812h-plic", "thead,c900-plic";
> >   };
> > @@ -33,7 +43,3 @@ &plic {
> >   &clint {
> >   	compatible = "sophgo,cv1812h-clint", "thead,c900-clint";
> >   };
> > -
> > -&clk {
> > -	compatible = "sophgo,cv1810-clk";
> > -};
> > 
> > [...]
> > 
> 
> > diff --git a/arch/riscv/boot/dts/sophgo/cv18xx-intc.dtsi b/arch/riscv/boot/dts/sophgo/cv18xx-intc.dtsi
> > new file mode 100644
> > index 000000000000..90817993a326
> > --- /dev/null
> > +++ b/arch/riscv/boot/dts/sophgo/cv18xx-intc.dtsi
> > @@ -0,0 +1,23 @@
> > +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> > +/*
> > + * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org>
> > + * Copyright (C) 2023 Inochi Amaoto <inochiama@outlook.com>
> > + */
> > +
> > +&soc {
> > +	interrupt-parent = <&plic>;
> > +
> > +	plic: interrupt-controller@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@74000000 {
> > +		reg = <0x74000000 0x10000>;
> > +		interrupts-extended = <&cpu0_intc 3>, <&cpu0_intc 7>;
> > +	};
> > +};
> 
> 
> I think this file can be merged in to cv18xx-cpu.dtsi.
> clint and plic is more like a core specific device.

Yes, this can be done now, as long as SOC_PERIPHERAL_IRQ() remains
outside of the -cpu-intc.dtsi...

-- 
Alexander Sverdlin.