Re: [PATCH 03/10] arm64: dts: sophgo: Add initial SG2000 SoC device tree

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

Hi Krzysztof!

On Mon, 2025-02-10 at 09:45 +0100, Krzysztof Kozlowski wrote:
> On 09/02/2025 23:06, Alexander Sverdlin wrote:
> > Add initial device tree for the SG2000 SoC by SOPHGO (from ARM64 PoV).
> > 
> > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
> > ---
> >   arch/arm64/boot/dts/sophgo/sg2000.dtsi | 79 ++++++++++++++++++++++++++
> >   1 file changed, 79 insertions(+)
> >   create mode 100644 arch/arm64/boot/dts/sophgo/sg2000.dtsi
> > 
> > diff --git a/arch/arm64/boot/dts/sophgo/sg2000.dtsi b/arch/arm64/boot/dts/sophgo/sg2000.dtsi
> > new file mode 100644
> > index 000000000000..4e520486cbe5
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/sophgo/sg2000.dtsi
> > @@ -0,0 +1,79 @@
> > +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> > +
> > +#define SOC_PERIPHERAL_IRQ(nr)		GIC_SPI (nr)
> > +
> > +#include <dt-bindings/interrupt-controller/arm-gic.h>
> > +#include <riscv/sophgo/cv18xx-periph.dtsi>
> > +#include <riscv/sophgo/cv181x.dtsi>
> > +
> > +/ {

[...]

> > +	gic: interrupt-controller@1f01000 {
> 
> MMIO nodes are always in the soc.

I think I've looked a wrong example (or a counter-example)...
$ grep -R -P '^\t\tcompatible = "arm,cortex-a15-gic";' *
arm/boot/dts/intel/axm/axm55xx.dtsi:		compatible = "arm,cortex-a15-gic";
arm/boot/dts/ti/omap/dra7.dtsi:		compatible = "arm,cortex-a15-gic";
arm/boot/dts/ti/omap/omap5.dtsi:		compatible = "arm,cortex-a15-gic";
arm/boot/dts/nvidia/tegra124.dtsi:		compatible = "arm,cortex-a15-gic";
arm/boot/dts/nvidia/tegra114.dtsi:		compatible = "arm,cortex-a15-gic";
arm64/boot/dts/nvidia/tegra132.dtsi:		compatible = "arm,cortex-a15-gic";
arm64/boot/dts/freescale/s32v234.dtsi:		compatible = "arm,cortex-a15-gic";
arm64/boot/dts/apm/apm-storm.dtsi:		compatible = "arm,cortex-a15-gic";
arm64/boot/dts/apm/apm-shadowcat.dtsi:		compatible = "arm,cortex-a15-gic";

But thanks for clarification!

> > +		compatible = "arm,cortex-a15-gic";
> > +		interrupt-controller;
> > +		#interrupt-cells = <3>;
> > +		reg = <0x01f01000 0x1000>,
> > +		      <0x01f02000 0x2000>;
> > +	};
> > +
> > +	soc {
> 
> Override by phandle/label instead of duplicating.
> 
> > +		ranges;
> > +
> > +		pinctrl: pinctrl@3001000 {
> > +			compatible = "sophgo,sg2000-pinctrl";
> > +			reg = <0x03001000 0x1000>,
> > +			      <0x05027000 0x1000>;
> > +			reg-names = "sys", "rtc";
> > +		};
> > +	};
> > +};
> > +
> > +
> > +&clk {
> > +	compatible = "sophgo,sg2000-clk";
> 
> 
> That's discouraged practice. If you need to define compatible, it means
> the block is not shared between designs and must not be in common DTSI.

That doesn't come from my series, that's how original cv18xx.dtsi has been
designed. Same question as before: do I need to rework it if I will not be able to
test the changes (I don't even posess the relevant HW). But if not, I have
to adapt the same pattern into the new sg2000.dtsi.

-- 
Alexander Sverdlin.