Re: [PATCH v2 3/4] arm64: dts: renesas: Add R8A78000 X5H DTs

[Marc Zyngier <maz@kernel.org>]

On Fri, 12 Sep 2025 08:37:29 +0100,
Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> 
> Hi Morimoto-san,
> 
> CC maz
> 
> On Fri, 12 Sept 2025 at 02:39, Kuninori Morimoto
> <kuninori.morimoto.gx@renesas.com> wrote:
> > > > +               /* The Arm GIC-700AE - View 1 */
> > >
> > > s/700/720/
> >
> > Oops, thanks. Will fix
> >
> > > > +               gic: interrupt-controller@39000000 {
> > > > +                       compatible = "arm,gic-v3";
> > >
> > > The documentation states it is compliant with GICv4.1?
> >
> > I'm not familiar with GIC. And I think there is no v4 support on Linux yet ?
> > If my understanding was correct, GICv4 have GICv3 compatible.
> > We can use v3 driver so far, and can be replaced to v4 driver if it was
> > supported in Linux?
> 
> 'git grep -i "\<gic.*v4.1"' does show support.
> 
> Marc?

We don't need to differentiate the various GICv3.{0,1,2,3} and
GICv4.{0,1,2} in DT. GICv3 is enough, and everything is else can be
probed.

And yes, we support everything (but that's not relevant for DT).

> 
> > > > +                       #interrupt-cells = <3>;
> > > > +                       #address-cells = <0>;
> > > > +                       interrupt-controller;
> > > > +                       redistributor-stride = <0x0 0x40000>;

No. That's the architected value, and doesn't need to be described.
This property is solely designed to support broken HW, and I really
hope this is not the case here.

> > > > +                       #redistributor-regions = <32>;
> > > > +                       reg = <0 0x39000000 0 0x20000>, // GICD
> > >
> > > The base address is 0x38000000, according to the docs?
> >
> > It is indicated in very deep place in datasheet. I will indicate
> > detail in v2.
> >
> > > > +                             <0 0x397C0000 0 0x40000>, // GICR Core29
> > > > +                             <0 0x39800000 0 0x40000>, // GICR Core30
> > > > +                             <0 0x39840000 0 0x40000>; // GICR Core31

This really is silly. You have *one* RD region, not 32. This single
region covers all the RDs that your system has.

> > >
> > > No GICC, GICH, and GICV?
> >
> > will be added later ?
> 
> OK.

I seriously doubt you can have these regions, unless you have attached
a GIC700 to an ancient core such as A53. Here, you seem to have a
bunch of A720, which will *not* have the GICv2 compat regions.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.