On Thu, Feb 13, 2025 at 12:04:39AM +0100, Alexander Sverdlin wrote:
> 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@xxxxxxxxx>
> > > ---
> > > 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@xxxxxxxxxx>
> > > */
> > >
> > > +#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@xxxxxxxxxx/
> The corresponding label is being introduced in cv18xx.dtsi.
>
Yes I know, but I wonder why you change it here, the url you provide
told nothing about change existing structure.
The origianl struct:
/ {
soc {
...
};
};
it can also be used to add things in /soc, renesa uses this in that
form is not the reason for this change. I prefer to keep original
structure so less change is introducted.
> > > +&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@xxxxxxxxxxx>
> > > */
> > >
> > > +#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@xxxxxxxxxx/
>
> If SOC_PERIPHERAL_IRQ() doesn't belong to -cpu.dtsi, then "dma-noncoherent;"
> probably neither...
>
This is OK for me.
> > > - 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@xxxxxxxxxx>
> > > + * Copyright (C) 2023 Inochi Amaoto <inochiama@xxxxxxxxxxx>
> > > + */
> > > +
> > > +&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.
>
