Re: [PATCH v3 3/8] ARM: dts: Prepare Realtek RTD1195 and MeLE X1000

[Marc Zyngier]

On Sun, 17 Nov 2019 15:40:59 +0000,
Andreas FÃrber <afaerber@xxxxxxx> wrote:
> 
> Hi Marc,
> 
> Am 17.11.19 um 11:47 schrieb Marc Zyngier:
> > On Sun, 17 Nov 2019 08:21:04 +0100
> > Andreas FÃrber <afaerber@xxxxxxx> wrote:
> >> Add Device Trees for Realtek RTD1195 SoC and MeLE X1000 TV box.
> >>
> >> Reuse the existing RTD1295 watchdog compatible for now.
> >>
> >> Reviewed-by: Rob Herring <robh@xxxxxxxxxx>
> >> [AF: Fixed r-bus size, fixed GIC CPU mask, updated memreserve]
> >> Signed-off-by: Andreas FÃrber <afaerber@xxxxxxx>
> >> ---
> >>  v2 -> v3:
> >>  * Fixed r-bus size in /soc ranges from 0x1000000 to 0x70000 (James)
> >>  * Adjusted /memreserve/ to close gap from 0xa800 to 0xc000 for full 0x100000
> >>  * Changed arch timer from GIC_CPU_MASK_RAW(0xf) to GIC_CPU_MASK_SIMPLE(2)
> >>    squashed from RTD1395 v1 series
> >>  
> >>  v1 -> v2:
> >>  * Dropped /memreserve/ and reserved-memory nodes for peripherals and NOR (Rob)
> >>  * Carved them out from memory reg instead (Rob)
> >>  * Converted some /memreserve/s to reserved-memory nodes
> >>  
> >>  arch/arm/boot/dts/Makefile               |   2 +
> >>  arch/arm/boot/dts/rtd1195-mele-x1000.dts |  31 ++++++++
> >>  arch/arm/boot/dts/rtd1195.dtsi           | 127 +++++++++++++++++++++++++++++++
> >>  3 files changed, 160 insertions(+)
> >>  create mode 100644 arch/arm/boot/dts/rtd1195-mele-x1000.dts
> >>  create mode 100644 arch/arm/boot/dts/rtd1195.dtsi

[...]

> >> +	timer {
> >> +		compatible = "arm,armv7-timer";
> >> +		interrupts = <GIC_PPI 13
> >> +			(GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>,
> >> +			     <GIC_PPI 14
> >> +			(GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>,
> >> +			     <GIC_PPI 11
> >> +			(GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>,
> >> +			     <GIC_PPI 10
> >> +			(GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>;
> >> +		clock-frequency = <27000000>;
> > 
> > This is 2019, and yet it feels like 2011. This should be setup in the
> > bootloader, not in DT...
> 
> What exactly - the whole node, the GIC CPU mask, the
> clock-frequency?

The clock frequency. Having to rely on such hacks 8 years down the
line makes me feel like we've achieved nothing...
</depressed>

> Please compare previous submissions: It's a v2012.07 based downstream
> U-Boot that I don't have GPL sources of. It doesn't even fill in the
> /memory@0 node.

Qualeetee...

> >> +		gic: interrupt-controller@ff011000 {
> >> +			compatible = "arm,cortex-a7-gic";
> >> +			reg = <0xff011000 0x1000>,
> >> +			      <0xff012000 0x2000>;
> >> +			interrupt-controller;
> >> +			#interrupt-cells = <3>;
> > 
> > You know what I'm going to say: GICH and GI[C]V are missing, as well as
> > the maintenance interrupt. This is all bog-standard HW (most probably a
> > GIC400), so there is no reason for this information not to be present.
> 
> Yes, and if you look at my rtd1295-next branch referenced in the cover
> letter, you will find that I do have follow-up patches adding GICH and
> GICV, also a guess for the GICV interrupt, and in a different patch [1]
> I have specifically reminded Realtek to review the v2 of this patch
> please, which still hasn't happened yet...
> 
> I inquired for the RTD1619 patch, and James replied that for its GICv3
> they supposedly do _not_ have the optional GICH and GICV [1].

Which is expected. Cortex-A55 doesn't have a GICv2 CPU interface
built-in, and thus doesn't not have the compatibility interface when
coupled with a GICv3 implementation.

In your case, Cortex A7 has all the required HW, and the required
values can be derived from the public TRM.

> Thus I am waiting on their input for whether they have it on RTD1195.
> The U-Boot that I have on this device does not boot the kernel in HYP
> mode, so I cannot test KVM myself. Same issue on the Horseradish
> EVB.

Given the vintage of the bootloader, I'm pretty sure he system boots
in secure mode, so it'd just be a matter of switching to non-secure.
Just use the existing bootloader as something that initialises memory
for you and boot a modern u-boot from there.

	M.

-- 
Jazz is not dead, it just smells funny.