All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marc Zyngier <maz@kernel.org>
To: "Andreas Färber" <afaerber@suse.de>
Cc: Mark Rutland <mark.rutland@arm.com>,
	devicetree@vger.kernel.org, James Tai <james.tai@realtek.com>,
	linux-realtek-soc@lists.infradead.org,
	linux-kernel@vger.kernel.org, Rob Herring <robh+dt@kernel.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v3 3/8] ARM: dts: Prepare Realtek RTD1195 and MeLE X1000
Date: Sun, 17 Nov 2019 16:22:10 +0000	[thread overview]
Message-ID: <86a78ujwwd.wl-maz@kernel.org> (raw)
In-Reply-To: <61bf74ad-b4a1-f443-bf99-be354b4d942b@suse.de>

On Sun, 17 Nov 2019 15:40:59 +0000,
Andreas Färber <afaerber@suse.de> 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@suse.de> 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@kernel.org>
> >> [AF: Fixed r-bus size, fixed GIC CPU mask, updated memreserve]
> >> Signed-off-by: Andreas Färber <afaerber@suse.de>
> >> ---
> >>  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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <maz@kernel.org>
To: "Andreas Färber" <afaerber@suse.de>
Cc: linux-realtek-soc@lists.infradead.org,
	Mark Rutland <mark.rutland@arm.com>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Rob Herring <robh+dt@kernel.org>,
	linux-arm-kernel@lists.infradead.org,
	James Tai <james.tai@realtek.com>
Subject: Re: [PATCH v3 3/8] ARM: dts: Prepare Realtek RTD1195 and MeLE X1000
Date: Sun, 17 Nov 2019 16:22:10 +0000	[thread overview]
Message-ID: <86a78ujwwd.wl-maz@kernel.org> (raw)
In-Reply-To: <61bf74ad-b4a1-f443-bf99-be354b4d942b@suse.de>

On Sun, 17 Nov 2019 15:40:59 +0000,
Andreas Färber <afaerber@suse.de> 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@suse.de> 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@kernel.org>
> >> [AF: Fixed r-bus size, fixed GIC CPU mask, updated memreserve]
> >> Signed-off-by: Andreas Färber <afaerber@suse.de>
> >> ---
> >>  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.

  reply	other threads:[~2019-11-17 16:22 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-17  7:21 [PATCH v3 0/8] ARM: Initial RTD1195 and MeLE X1000 support Andreas Färber
2019-11-17  7:21 ` Andreas Färber
2019-11-17  7:21 ` [PATCH v3 1/8] dt-bindings: arm: realtek: Add RTD1195 and MeLE X1000 Andreas Färber
2019-11-17  7:21   ` Andreas Färber
2019-11-17  7:21 ` [PATCH v3 2/8] ARM: Prepare Realtek RTD1195 Andreas Färber
2019-11-17  7:21   ` Andreas Färber
2019-11-17  7:21 ` [PATCH v3 3/8] ARM: dts: Prepare Realtek RTD1195 and MeLE X1000 Andreas Färber
2019-11-17  7:21   ` Andreas Färber
2019-11-17 10:47   ` Marc Zyngier
2019-11-17 10:47     ` Marc Zyngier
2019-11-17 15:40     ` Andreas Färber
2019-11-17 15:40       ` Andreas Färber
2019-11-17 16:22       ` Marc Zyngier [this message]
2019-11-17 16:22         ` Marc Zyngier
2019-11-18  1:24         ` Andreas Färber
2019-11-18  1:24           ` Andreas Färber
2019-11-18  9:14           ` Marc Zyngier
2019-11-18  9:14             ` Marc Zyngier
2019-11-17  7:21 ` [PATCH v3 4/8] ARM: dts: rtd1195: Introduce r-bus Andreas Färber
2019-11-17  7:21   ` Andreas Färber
2019-11-17  7:21 ` [PATCH v3 5/8] dt-bindings: reset: Add Realtek RTD1195 Andreas Färber
2019-11-17  7:21   ` Andreas Färber
2019-11-17  7:21 ` [PATCH v3 6/8] ARM: dts: rtd1195: Add reset nodes Andreas Färber
2019-11-17  7:21   ` Andreas Färber
2019-11-18  9:22   ` James Tai
2019-11-18  9:22     ` James Tai
2019-11-19  8:34     ` Andreas Färber
2019-11-19  8:34       ` Andreas Färber
2019-11-20  6:53       ` James Tai
2019-11-20  6:53         ` James Tai
2019-11-17  7:21 ` [PATCH v3 7/8] ARM: dts: rtd1195: Add UART resets Andreas Färber
2019-11-17  7:21   ` Andreas Färber
2019-11-17  7:21 ` [PATCH v3 8/8] ARM: realtek: Enable RTD1195 arch timer Andreas Färber
2019-11-17  7:21   ` Andreas Färber
2019-11-17 11:02   ` Marc Zyngier
2019-11-17 11:02     ` Marc Zyngier
2019-11-17 17:08     ` Andreas Färber
2019-11-17 17:08       ` Andreas Färber
2019-11-18  9:27       ` Marc Zyngier
2019-11-18  9:27         ` Marc Zyngier
2019-11-18 22:48         ` Andreas Färber
2019-11-18 22:48           ` Andreas Färber
2020-03-20 16:16           ` James Tai [戴志峰]
2020-03-20 16:16             ` James Tai [戴志峰]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=86a78ujwwd.wl-maz@kernel.org \
    --to=maz@kernel.org \
    --cc=afaerber@suse.de \
    --cc=devicetree@vger.kernel.org \
    --cc=james.tai@realtek.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-realtek-soc@lists.infradead.org \
    --cc=mark.rutland@arm.com \
    --cc=robh+dt@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.