All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marc Zyngier <maz@kernel.org>
To: James Tai <james.tai@realtek.com>
Cc: linux-realtek-soc@lists.infradead.org, mark.rutland@arm.com,
	devicetree@vger.kernel.org,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	linux-kernel@vger.kernel.org, robh+dt@kernel.org,
	Robin Murphy <robin.murphy@arm.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 2/2] arm64: dts: realtek: Add RTD1319 SoC and Realtek PymParticle EVB
Date: Sun, 29 Dec 2019 11:29:16 +0000	[thread overview]
Message-ID: <718082aebcc3ab4d9169a4abbe968ec1@kernel.org> (raw)
In-Reply-To: <68b6541e1f4b447cb6845d16fdab28d9@realtek.com>

On 2019-12-29 07:46, James Tai wrote:
> Hi Marc,
> 
> Thanks for review.
> 
>> > +	timer {
>> > +		compatible = "arm,armv8-timer";
>> > +		interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
>> > +			     <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
>> > +			     <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
>> > +			     <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
>> 
>> Nit: At some point, it'd be good to be able to describe the EL2 
>> virtual timer
>> interrupt too. Not specially important, but since these ARMv8.2 CPUs 
>> have it...
> 
> I will add the EL2 virtual timer interrupt to timer node.

If you do this, please update the binding first, as this interrupt
is not described there yet.

> 
>> > +		gic: interrupt-controller@ff100000 {
>> > +			compatible = "arm,gic-v3";
>> > +			reg = <0xff100000 0x10000>,
>> > +			      <0xff140000 0xc0000>;
>> 
>> Are you sure about the size of the GICR region? For 4 CPUs, it should 
>> be
>> 0x80000. Here, you have a range for 6 CPUs.
> 
> The GICR region should be 0x80000 because the RTD1319 SoC have only 4 
> CPUs.

OK. Please verify that this is actually the case, and that the last
redistributor (at offset 0x60000) has GICR_TYPER.Last set. I have
recently seen GICs configured for a larger number of CPUs where
some of them were disabled in HW, and the DT was wrongly describing
some of the redistributors only, leading to SW crashes.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <maz@kernel.org>
To: James Tai <james.tai@realtek.com>
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	linux-realtek-soc@lists.infradead.org,
	linux-kernel@vger.kernel.org, robh+dt@kernel.org,
	Robin Murphy <robin.murphy@arm.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 2/2] arm64: dts: realtek: Add RTD1319 SoC and Realtek PymParticle EVB
Date: Sun, 29 Dec 2019 11:29:16 +0000	[thread overview]
Message-ID: <718082aebcc3ab4d9169a4abbe968ec1@kernel.org> (raw)
In-Reply-To: <68b6541e1f4b447cb6845d16fdab28d9@realtek.com>

On 2019-12-29 07:46, James Tai wrote:
> Hi Marc,
> 
> Thanks for review.
> 
>> > +	timer {
>> > +		compatible = "arm,armv8-timer";
>> > +		interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
>> > +			     <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
>> > +			     <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
>> > +			     <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
>> 
>> Nit: At some point, it'd be good to be able to describe the EL2 
>> virtual timer
>> interrupt too. Not specially important, but since these ARMv8.2 CPUs 
>> have it...
> 
> I will add the EL2 virtual timer interrupt to timer node.

If you do this, please update the binding first, as this interrupt
is not described there yet.

> 
>> > +		gic: interrupt-controller@ff100000 {
>> > +			compatible = "arm,gic-v3";
>> > +			reg = <0xff100000 0x10000>,
>> > +			      <0xff140000 0xc0000>;
>> 
>> Are you sure about the size of the GICR region? For 4 CPUs, it should 
>> be
>> 0x80000. Here, you have a range for 6 CPUs.
> 
> The GICR region should be 0x80000 because the RTD1319 SoC have only 4 
> CPUs.

OK. Please verify that this is actually the case, and that the last
redistributor (at offset 0x60000) has GICR_TYPER.Last set. I have
recently seen GICs configured for a larger number of CPUs where
some of them were disabled in HW, and the DT was wrongly describing
some of the redistributors only, leading to SW crashes.

Thanks,

         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

  reply	other threads:[~2019-12-29 11:29 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-28 15:05 [PATCH v2 0/2] Initial RTD1319 SoC and Realtek PymParticle EVB support James Tai
2019-12-28 15:05 ` James Tai
2019-12-28 15:05 ` [PATCH v2 1/2] dt-bindings: arm: realtek: Document RTD1319 and Realtek PymParticle EVB James Tai
2019-12-28 15:05   ` James Tai
2020-01-04 21:07   ` Rob Herring
2020-01-04 21:07     ` Rob Herring
2019-12-28 15:05 ` [PATCH v2 2/2] arm64: dts: realtek: Add RTD1319 SoC " James Tai
2019-12-28 15:05   ` James Tai
2019-12-28 18:57   ` Marc Zyngier
2019-12-28 18:57     ` Marc Zyngier
2019-12-29  7:46     ` James Tai
2019-12-29  7:46       ` James Tai
2019-12-29 11:29       ` Marc Zyngier [this message]
2019-12-29 11:29         ` Marc Zyngier
2019-12-29  2:48 ` [PATCH v2 0/2] Initial RTD1319 SoC and Realtek PymParticle EVB support Andreas Färber
2019-12-29  2:48   ` Andreas Färber
2019-12-29  7:57   ` James Tai
2019-12-29  7:57     ` 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=718082aebcc3ab4d9169a4abbe968ec1@kernel.org \
    --to=maz@kernel.org \
    --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=lorenzo.pieralisi@arm.com \
    --cc=mark.rutland@arm.com \
    --cc=robh+dt@kernel.org \
    --cc=robin.murphy@arm.com \
    /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.