Re: [PATCH 3/5] ARM: dts: Add basic support for EcoNet EN7523

From: Marc Zyngier <maz@kernel.org>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Bert Vermeulen <bert@biot.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
	<devicetree@vger.kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Arnd Bergmann <arnd@arndb.de>, Olof Johansson <olof@lixom.net>,
	SoC Team <soc@kernel.org>, Rob Herring <robh+dt@kernel.org>,
	John Crispin <john@phrozen.org>, Felix Fietkau <nbd@nbd.name>
Subject: Re: [PATCH 3/5] ARM: dts: Add basic support for EcoNet EN7523
Date: Fri, 30 Jul 2021 15:53:55 +0100	[thread overview]
Message-ID: <87y29n26po.wl-maz@kernel.org> (raw)
In-Reply-To: <CACRpkdYvMtE8b-Xiy6=Aiz20jvY0M0Bz9XmcEQDHhqeS+LErEA@mail.gmail.com>

On Fri, 30 Jul 2021 15:31:36 +0100,
Linus Walleij <linus.walleij@linaro.org> wrote:
> 
> Paging Marc Z and Catalin just so they can see this:
> 
> On Fri, Jul 30, 2021 at 3:49 PM Bert Vermeulen <bert@biot.com> wrote:
> 
> > From: John Crispin <john@phrozen.org>
> >
> > Add basic support for EcoNet EN7523, enough for booting to console.
> >
> > The UART is basically 8250-compatible, except for the clock selection.
> > A clock-frequency value is synthesized to get this to run at 115200 bps.
> >
> > Signed-off-by: John Crispin <john@phrozen.org>
> > Signed-off-by: Bert Vermeulen <bert@biot.com>
> (...)
> > +       gic: interrupt-controller@09000000 {
> > +               compatible = "arm,gic-v3";
> > +               interrupt-controller;
> > +               #interrupt-cells = <3>;
> > +               #address-cells = <1>;
> > +               #size-cells = <1>;
> > +               reg = <0x09000000 0x20000>,
> > +                         <0x09080000 0x80000>;
> > +               interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_LOW>;
> > +
> > +               its: gic-its@09020000 {
> > +                       compatible = "arm,gic-v3-its";
> > +                       msi-controller;
> > +                       #msi-cell = <1>;
> > +                       reg = <0x090200000 0x20000>;
> > +               };
> > +       };
> 
> Yup GICv3 on ARM32-only silicon.

Hey, why not. But that's very unlikely, as Cortex-A7 doesn't have a
GICv3 CPU interface built in (it only has the memory mapped version),
and A53/57 were the first CPUs to ever support GICv3. I don't believe
the description of the CPU in the DT is accurate.

Bert, please send a kernel boot log.

> 
> > +       timer {
> > +               compatible = "arm,armv8-timer";
> > +               interrupt-parent = <&gic>;
> > +               interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> > +                            <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> > +                            <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
> > +                            <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;

Copy paste bug. These are not valid intspecs for GICv3.

> > +               clock-frequency = <25000000>;
> > +       };
> 
> Also arm,armv8-timer on ARM32-only silicon.

Probably because that's not what it actually is. My bet is on A53 with
a crippled firmware.

> This is kind of a first.

Thanks,

	M.

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