From: Lad Prabhakar <[email protected]>
Enable the performance monitor unit for the Cortex-A55 cores on the
RZ/G2L (r9a07g044) SoC.
Signed-off-by: Lad Prabhakar <[email protected]>
---
arch/arm64/boot/dts/renesas/r9a07g044.dtsi | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
index 80b2332798d9..ff9bdc03a3ed 100644
--- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
@@ -161,6 +161,11 @@ opp-50000000 {
};
};
+ pmu_a55 {
+ compatible = "arm,cortex-a55-pmu";
+ interrupts-extended = <&gic GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH>;
+ };
+
psci {
compatible = "arm,psci-1.0", "arm,psci-0.2";
method = "smc";
--
2.25.1
On 27/01/2023 18:40, Prabhakar wrote:
> From: Lad Prabhakar <[email protected]>
>
> Enable the performance monitor unit for the Cortex-A55 cores on the
> RZ/G2L (r9a07g044) SoC.
>
> Signed-off-by: Lad Prabhakar <[email protected]>
> ---
> arch/arm64/boot/dts/renesas/r9a07g044.dtsi | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> index 80b2332798d9..ff9bdc03a3ed 100644
> --- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> @@ -161,6 +161,11 @@ opp-50000000 {
> };
> };
>
> + pmu_a55 {
No underscores in node names. This is usually called just 'pmu'.
Best regards,
Krzysztof
Hi Prabhakar,
Thanks for the patch.
> Subject: [PATCH] arm64: dts: renesas: r9a07g044: Add Cortex-A55 PMU node
>
> From: Lad Prabhakar <[email protected]>
>
> Enable the performance monitor unit for the Cortex-A55 cores on the RZ/G2L
> (r9a07g044) SoC.
>
> Signed-off-by: Lad Prabhakar <[email protected]>
> ---
> arch/arm64/boot/dts/renesas/r9a07g044.dtsi | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> index 80b2332798d9..ff9bdc03a3ed 100644
> --- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> @@ -161,6 +161,11 @@ opp-50000000 {
> };
> };
>
> + pmu_a55 {
> + compatible = "arm,cortex-a55-pmu";
> + interrupts-extended = <&gic GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH>;
Just a question, Is it tested? timer node[1] defines irq type as LOW, here it is high.
Also do we need to define (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW) as it has 2 cores??
[1]
timer {
compatible = "arm,armv8-timer";
interrupts-extended = <&gic GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>,
<&gic GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>,
<&gic GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>,
<&gic GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>;
};
Cheers,
Biju
> + };
> +
> psci {
> compatible = "arm,psci-1.0", "arm,psci-0.2";
> method = "smc";
> --
> 2.25.1
Hi Biju,
Thank you for the review.
On Fri, Jan 27, 2023 at 6:38 PM Biju Das <[email protected]> wrote:
>
> Hi Prabhakar,
>
> Thanks for the patch.
>
> > Subject: [PATCH] arm64: dts: renesas: r9a07g044: Add Cortex-A55 PMU node
> >
> > From: Lad Prabhakar <[email protected]>
> >
> > Enable the performance monitor unit for the Cortex-A55 cores on the RZ/G2L
> > (r9a07g044) SoC.
> >
> > Signed-off-by: Lad Prabhakar <[email protected]>
> > ---
> > arch/arm64/boot/dts/renesas/r9a07g044.dtsi | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > index 80b2332798d9..ff9bdc03a3ed 100644
> > --- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > @@ -161,6 +161,11 @@ opp-50000000 {
> > };
> > };
> >
> > + pmu_a55 {
> > + compatible = "arm,cortex-a55-pmu";
> > + interrupts-extended = <&gic GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH>;
>
> Just a question, Is it tested?
Yes this was tested with perf test (https://pastebin.com/dkckcYHr)
> timer node[1] defines irq type as LOW, here it is high.
You are right looking at the RZG2L_InterruptMapping_rev01.xlsx this
should be LOW. (I followed the SPI IRQS where all the LEVEL interrupts
are HIGH)
> Also do we need to define (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW) as it has 2 cores??
>
No this is not required for example here [0] where it has 6 cores.
[0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/renesas/r8a779f0.dtsi?h=v6.2-rc5#n203
Cheers,
Prabhakar
Hi Krzysztof,
Thank you for the review.
On Fri, Jan 27, 2023 at 5:54 PM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 27/01/2023 18:40, Prabhakar wrote:
> > From: Lad Prabhakar <[email protected]>
> >
> > Enable the performance monitor unit for the Cortex-A55 cores on the
> > RZ/G2L (r9a07g044) SoC.
> >
> > Signed-off-by: Lad Prabhakar <[email protected]>
> > ---
> > arch/arm64/boot/dts/renesas/r9a07g044.dtsi | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > index 80b2332798d9..ff9bdc03a3ed 100644
> > --- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > @@ -161,6 +161,11 @@ opp-50000000 {
> > };
> > };
> >
> > + pmu_a55 {
>
> No underscores in node names. This is usually called just 'pmu'.
>
Ok, I will update it in the next version.
Cheers,
Prabhakar
Hi Prabhakar,
> Subject: Re: [PATCH] arm64: dts: renesas: r9a07g044: Add Cortex-A55 PMU node
>
> Hi Biju,
>
> Thank you for the review.
>
> On Fri, Jan 27, 2023 at 6:38 PM Biju Das <[email protected]> wrote:
> >
> > Hi Prabhakar,
> >
> > Thanks for the patch.
> >
> > > Subject: [PATCH] arm64: dts: renesas: r9a07g044: Add Cortex-A55 PMU
> > > node
> > >
> > > From: Lad Prabhakar <[email protected]>
> > >
> > > Enable the performance monitor unit for the Cortex-A55 cores on the
> > > RZ/G2L
> > > (r9a07g044) SoC.
> > >
> > > Signed-off-by: Lad Prabhakar
> > > <[email protected]>
> > > ---
> > > arch/arm64/boot/dts/renesas/r9a07g044.dtsi | 5 +++++
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > index 80b2332798d9..ff9bdc03a3ed 100644
> > > --- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > @@ -161,6 +161,11 @@ opp-50000000 {
> > > };
> > > };
> > >
> > > + pmu_a55 {
> > > + compatible = "arm,cortex-a55-pmu";
> > > + interrupts-extended = <&gic GIC_PPI 7
> > > + IRQ_TYPE_LEVEL_HIGH>;
> >
> > Just a question, Is it tested?
> Yes this was tested with perf test
>
> > timer node[1] defines irq type as LOW, here it is high.
> You are right looking at the RZG2L_InterruptMapping_rev01.xlsx this should
> be LOW. (I followed the SPI IRQS where all the LEVEL interrupts are HIGH)
>
> > Also do we need to define (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW) as
> it has 2 cores??
> >
> No this is not required for example here [0] where it has 6 cores.
I may be wrong, That is the only example[1], where the A55 PMU per cpu interrupts and number of a55 cores in the DT
are not matching.
[1]
https://elixir.bootlin.com/linux/latest/B/ident/arm%2Ccortex-a55-pmu
Cheers,
Biju
On Fri, Jan 27, 2023 at 10:48 PM Biju Das <[email protected]> wrote:
> > Subject: Re: [PATCH] arm64: dts: renesas: r9a07g044: Add Cortex-A55 PMU node
> > On Fri, Jan 27, 2023 at 6:38 PM Biju Das <[email protected]> wrote:
> > > > Subject: [PATCH] arm64: dts: renesas: r9a07g044: Add Cortex-A55 PMU
> > > > node
> > > >
> > > > From: Lad Prabhakar <[email protected]>
> > > >
> > > > Enable the performance monitor unit for the Cortex-A55 cores on the
> > > > RZ/G2L
> > > > (r9a07g044) SoC.
> > > >
> > > > Signed-off-by: Lad Prabhakar
> > > > <[email protected]>
> > > > ---
> > > > arch/arm64/boot/dts/renesas/r9a07g044.dtsi | 5 +++++
> > > > 1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > > b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > > index 80b2332798d9..ff9bdc03a3ed 100644
> > > > --- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > > +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > > @@ -161,6 +161,11 @@ opp-50000000 {
> > > > };
> > > > };
> > > >
> > > > + pmu_a55 {
> > > > + compatible = "arm,cortex-a55-pmu";
> > > > + interrupts-extended = <&gic GIC_PPI 7
> > > > + IRQ_TYPE_LEVEL_HIGH>;
> > >
> > > Just a question, Is it tested?
> > Yes this was tested with perf test
> >
> > > timer node[1] defines irq type as LOW, here it is high.
> > You are right looking at the RZG2L_InterruptMapping_rev01.xlsx this should
> > be LOW. (I followed the SPI IRQS where all the LEVEL interrupts are HIGH)
> >
> > > Also do we need to define (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW) as
> > it has 2 cores??
> > >
> > No this is not required for example here [0] where it has 6 cores.
>
> I may be wrong, That is the only example[1], where the A55 PMU per cpu interrupts and number of a55 cores in the DT
> are not matching.
>
> [1]
> https://elixir.bootlin.com/linux/latest/B/ident/arm%2Ccortex-a55-pmu
Indeed, this looks like an omission, propagated through
arch/arm64/boot/dts/renesas/r8a779[afg]0.dtsi.
And doesn't this apply to all PPI interrupts, i.e. shouldn't the GIC
in arch/arm64/boot/dts/renesas/r9a07g0{43u,44u,54}.dtsi specify the
mask in their interrupts properties, too?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Geert,
On Mon, Jan 30, 2023 at 10:05 AM Geert Uytterhoeven
<[email protected]> wrote:
>
> On Fri, Jan 27, 2023 at 10:48 PM Biju Das <[email protected]> wrote:
> > > Subject: Re: [PATCH] arm64: dts: renesas: r9a07g044: Add Cortex-A55 PMU node
> > > On Fri, Jan 27, 2023 at 6:38 PM Biju Das <[email protected]> wrote:
> > > > > Subject: [PATCH] arm64: dts: renesas: r9a07g044: Add Cortex-A55 PMU
> > > > > node
> > > > >
> > > > > From: Lad Prabhakar <[email protected]>
> > > > >
> > > > > Enable the performance monitor unit for the Cortex-A55 cores on the
> > > > > RZ/G2L
> > > > > (r9a07g044) SoC.
> > > > >
> > > > > Signed-off-by: Lad Prabhakar
> > > > > <[email protected]>
> > > > > ---
> > > > > arch/arm64/boot/dts/renesas/r9a07g044.dtsi | 5 +++++
> > > > > 1 file changed, 5 insertions(+)
> > > > >
> > > > > diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > > > b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > > > index 80b2332798d9..ff9bdc03a3ed 100644
> > > > > --- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > > > +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > > > @@ -161,6 +161,11 @@ opp-50000000 {
> > > > > };
> > > > > };
> > > > >
> > > > > + pmu_a55 {
> > > > > + compatible = "arm,cortex-a55-pmu";
> > > > > + interrupts-extended = <&gic GIC_PPI 7
> > > > > + IRQ_TYPE_LEVEL_HIGH>;
> > > >
> > > > Just a question, Is it tested?
> > > Yes this was tested with perf test
> > >
> > > > timer node[1] defines irq type as LOW, here it is high.
> > > You are right looking at the RZG2L_InterruptMapping_rev01.xlsx this should
> > > be LOW. (I followed the SPI IRQS where all the LEVEL interrupts are HIGH)
> > >
> > > > Also do we need to define (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW) as
> > > it has 2 cores??
> > > >
> > > No this is not required for example here [0] where it has 6 cores.
> >
> > I may be wrong, That is the only example[1], where the A55 PMU per cpu interrupts and number of a55 cores in the DT
> > are not matching.
> >
> > [1]
> > https://elixir.bootlin.com/linux/latest/B/ident/arm%2Ccortex-a55-pmu
>
> Indeed, this looks like an omission, propagated through
> arch/arm64/boot/dts/renesas/r8a779[afg]0.dtsi.
>
> And doesn't this apply to all PPI interrupts, i.e. shouldn't the GIC
> in arch/arm64/boot/dts/renesas/r9a07g0{43u,44u,54}.dtsi specify the
> mask in their interrupts properties, too?
>
I was under the impression that the GIC_CPU_MASK_SIMPLE(x) was only
needed if the driver handled per-cpu stuff.
Marc, what should be the correct usage?
Cheers,
Prabhakar
On Mon, 30 Jan 2023 13:13:26 +0000,
"Lad, Prabhakar" <[email protected]> wrote:
>
> Hi Geert,
>
> On Mon, Jan 30, 2023 at 10:05 AM Geert Uytterhoeven
> <[email protected]> wrote:
> >
> > On Fri, Jan 27, 2023 at 10:48 PM Biju Das <[email protected]> wrote:
> > > > Subject: Re: [PATCH] arm64: dts: renesas: r9a07g044: Add Cortex-A55 PMU node
> > > > On Fri, Jan 27, 2023 at 6:38 PM Biju Das <[email protected]> wrote:
> > > > > > Subject: [PATCH] arm64: dts: renesas: r9a07g044: Add Cortex-A55 PMU
> > > > > > node
> > > > > >
> > > > > > From: Lad Prabhakar <[email protected]>
> > > > > >
> > > > > > Enable the performance monitor unit for the Cortex-A55 cores on the
> > > > > > RZ/G2L
> > > > > > (r9a07g044) SoC.
> > > > > >
> > > > > > Signed-off-by: Lad Prabhakar
> > > > > > <[email protected]>
> > > > > > ---
> > > > > > arch/arm64/boot/dts/renesas/r9a07g044.dtsi | 5 +++++
> > > > > > 1 file changed, 5 insertions(+)
> > > > > >
> > > > > > diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > > > > b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > > > > index 80b2332798d9..ff9bdc03a3ed 100644
> > > > > > --- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > > > > +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > > > > @@ -161,6 +161,11 @@ opp-50000000 {
> > > > > > };
> > > > > > };
> > > > > >
> > > > > > + pmu_a55 {
> > > > > > + compatible = "arm,cortex-a55-pmu";
> > > > > > + interrupts-extended = <&gic GIC_PPI 7
> > > > > > + IRQ_TYPE_LEVEL_HIGH>;
> > > > >
> > > > > Just a question, Is it tested?
> > > > Yes this was tested with perf test
> > > >
> > > > > timer node[1] defines irq type as LOW, here it is high.
> > > > You are right looking at the RZG2L_InterruptMapping_rev01.xlsx this should
> > > > be LOW. (I followed the SPI IRQS where all the LEVEL interrupts are HIGH)
> > > >
> > > > > Also do we need to define (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW) as
> > > > it has 2 cores??
> > > > >
> > > > No this is not required for example here [0] where it has 6 cores.
> > >
> > > I may be wrong, That is the only example[1], where the A55 PMU per cpu interrupts and number of a55 cores in the DT
> > > are not matching.
> > >
> > > [1]
> > > https://elixir.bootlin.com/linux/latest/B/ident/arm%2Ccortex-a55-pmu
> >
> > Indeed, this looks like an omission, propagated through
> > arch/arm64/boot/dts/renesas/r8a779[afg]0.dtsi.
> >
> > And doesn't this apply to all PPI interrupts, i.e. shouldn't the GIC
> > in arch/arm64/boot/dts/renesas/r9a07g0{43u,44u,54}.dtsi specify the
> > mask in their interrupts properties, too?
> >
> I was under the impression that the GIC_CPU_MASK_SIMPLE(x) was only
> needed if the driver handled per-cpu stuff.
>
> Marc, what should be the correct usage?
I'm reading the DT correctly, this system has a GICv3, which is quite
natural for an A55-based system. For this configuration, no mask is
required.
The CPU mask stuff only applies to pre-GICv3. With GICv3+, you simply
cannot express such a mask, as there is no practical limit to the
number of CPUs.
M.
--
Without deviation from the norm, progress is not possible.
Hi Marc,
On Mon, Jan 30, 2023 at 1:26 PM Marc Zyngier <[email protected]> wrote:
>
> On Mon, 30 Jan 2023 13:13:26 +0000,
> "Lad, Prabhakar" <[email protected]> wrote:
> >
> > Hi Geert,
> >
> > On Mon, Jan 30, 2023 at 10:05 AM Geert Uytterhoeven
> > <[email protected]> wrote:
> > >
> > > On Fri, Jan 27, 2023 at 10:48 PM Biju Das <[email protected]> wrote:
> > > > > Subject: Re: [PATCH] arm64: dts: renesas: r9a07g044: Add Cortex-A55 PMU node
> > > > > On Fri, Jan 27, 2023 at 6:38 PM Biju Das <[email protected]> wrote:
> > > > > > > Subject: [PATCH] arm64: dts: renesas: r9a07g044: Add Cortex-A55 PMU
> > > > > > > node
> > > > > > >
> > > > > > > From: Lad Prabhakar <[email protected]>
> > > > > > >
> > > > > > > Enable the performance monitor unit for the Cortex-A55 cores on the
> > > > > > > RZ/G2L
> > > > > > > (r9a07g044) SoC.
> > > > > > >
> > > > > > > Signed-off-by: Lad Prabhakar
> > > > > > > <[email protected]>
> > > > > > > ---
> > > > > > > arch/arm64/boot/dts/renesas/r9a07g044.dtsi | 5 +++++
> > > > > > > 1 file changed, 5 insertions(+)
> > > > > > >
> > > > > > > diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > > > > > b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > > > > > index 80b2332798d9..ff9bdc03a3ed 100644
> > > > > > > --- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > > > > > +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > > > > > @@ -161,6 +161,11 @@ opp-50000000 {
> > > > > > > };
> > > > > > > };
> > > > > > >
> > > > > > > + pmu_a55 {
> > > > > > > + compatible = "arm,cortex-a55-pmu";
> > > > > > > + interrupts-extended = <&gic GIC_PPI 7
> > > > > > > + IRQ_TYPE_LEVEL_HIGH>;
> > > > > >
> > > > > > Just a question, Is it tested?
> > > > > Yes this was tested with perf test
> > > > >
> > > > > > timer node[1] defines irq type as LOW, here it is high.
> > > > > You are right looking at the RZG2L_InterruptMapping_rev01.xlsx this should
> > > > > be LOW. (I followed the SPI IRQS where all the LEVEL interrupts are HIGH)
> > > > >
> > > > > > Also do we need to define (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW) as
> > > > > it has 2 cores??
> > > > > >
> > > > > No this is not required for example here [0] where it has 6 cores.
> > > >
> > > > I may be wrong, That is the only example[1], where the A55 PMU per cpu interrupts and number of a55 cores in the DT
> > > > are not matching.
> > > >
> > > > [1]
> > > > https://elixir.bootlin.com/linux/latest/B/ident/arm%2Ccortex-a55-pmu
> > >
> > > Indeed, this looks like an omission, propagated through
> > > arch/arm64/boot/dts/renesas/r8a779[afg]0.dtsi.
> > >
> > > And doesn't this apply to all PPI interrupts, i.e. shouldn't the GIC
> > > in arch/arm64/boot/dts/renesas/r9a07g0{43u,44u,54}.dtsi specify the
> > > mask in their interrupts properties, too?
> > >
> > I was under the impression that the GIC_CPU_MASK_SIMPLE(x) was only
> > needed if the driver handled per-cpu stuff.
> >
> > Marc, what should be the correct usage?
>
> I'm reading the DT correctly, this system has a GICv3, which is quite
> natural for an A55-based system. For this configuration, no mask is
> required.
>
> The CPU mask stuff only applies to pre-GICv3. With GICv3+, you simply
> cannot express such a mask, as there is no practical limit to the
> number of CPUs.
>
Thank you for the clarification.
Cheers,
Prabhakar
Hi Mark and Will,
On Fri, Jan 27, 2023 at 9:48 PM Biju Das <[email protected]> wrote:
>
> Hi Prabhakar,
>
>
> > Subject: Re: [PATCH] arm64: dts: renesas: r9a07g044: Add Cortex-A55 PMU node
> >
> > Hi Biju,
> >
> > Thank you for the review.
> >
> > On Fri, Jan 27, 2023 at 6:38 PM Biju Das <[email protected]> wrote:
> > >
> > > Hi Prabhakar,
> > >
> > > Thanks for the patch.
> > >
> > > > Subject: [PATCH] arm64: dts: renesas: r9a07g044: Add Cortex-A55 PMU
> > > > node
> > > >
> > > > From: Lad Prabhakar <[email protected]>
> > > >
> > > > Enable the performance monitor unit for the Cortex-A55 cores on the
> > > > RZ/G2L
> > > > (r9a07g044) SoC.
> > > >
> > > > Signed-off-by: Lad Prabhakar
> > > > <[email protected]>
> > > > ---
> > > > arch/arm64/boot/dts/renesas/r9a07g044.dtsi | 5 +++++
> > > > 1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > > b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > > index 80b2332798d9..ff9bdc03a3ed 100644
> > > > --- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > > +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > > @@ -161,6 +161,11 @@ opp-50000000 {
> > > > };
> > > > };
> > > >
> > > > + pmu_a55 {
> > > > + compatible = "arm,cortex-a55-pmu";
> > > > + interrupts-extended = <&gic GIC_PPI 7
> > > > + IRQ_TYPE_LEVEL_HIGH>;
> > >
> > > Just a question, Is it tested?
> > Yes this was tested with perf test
> >
> > > timer node[1] defines irq type as LOW, here it is high.
> > You are right looking at the RZG2L_InterruptMapping_rev01.xlsx this should
> > be LOW. (I followed the SPI IRQS where all the LEVEL interrupts are HIGH)
> >
> > > Also do we need to define (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW) as
> > it has 2 cores??
> > >
> > No this is not required for example here [0] where it has 6 cores.
>
> I may be wrong, That is the only example[1], where the A55 PMU per cpu interrupts and number of a55 cores in the DT
> are not matching.
>
Some SoCs specify the GIC_CPU_MASK_SIMPLE(x) while describing the PPI
interrupt for the PMU and some dont [1]. What should be the correct
usage when specifying the PPI interrupts for the PMU with multiple CPU
cores (we are using
arm,cortex-a55-pmu)?
[1] https://elixir.bootlin.com/linux/latest/B/ident/arm%2Ccortex-a55-pmu
Cheers,
Prabhakar
On Mon, Jan 30, 2023 at 02:04:44PM +0000, Lad, Prabhakar wrote:
> Hi Mark and Will,
>
> On Fri, Jan 27, 2023 at 9:48 PM Biju Das <[email protected]> wrote:
> >
> > Hi Prabhakar,
> >
> >
> > > Subject: Re: [PATCH] arm64: dts: renesas: r9a07g044: Add Cortex-A55 PMU node
> > >
> > > Hi Biju,
> > >
> > > Thank you for the review.
> > >
> > > On Fri, Jan 27, 2023 at 6:38 PM Biju Das <[email protected]> wrote:
> > > >
> > > > Hi Prabhakar,
> > > >
> > > > Thanks for the patch.
> > > >
> > > > > Subject: [PATCH] arm64: dts: renesas: r9a07g044: Add Cortex-A55 PMU
> > > > > node
> > > > >
> > > > > From: Lad Prabhakar <[email protected]>
> > > > >
> > > > > Enable the performance monitor unit for the Cortex-A55 cores on the
> > > > > RZ/G2L
> > > > > (r9a07g044) SoC.
> > > > >
> > > > > Signed-off-by: Lad Prabhakar
> > > > > <[email protected]>
> > > > > ---
> > > > > arch/arm64/boot/dts/renesas/r9a07g044.dtsi | 5 +++++
> > > > > 1 file changed, 5 insertions(+)
> > > > >
> > > > > diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > > > b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > > > index 80b2332798d9..ff9bdc03a3ed 100644
> > > > > --- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > > > +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > > > @@ -161,6 +161,11 @@ opp-50000000 {
> > > > > };
> > > > > };
> > > > >
> > > > > + pmu_a55 {
> > > > > + compatible = "arm,cortex-a55-pmu";
> > > > > + interrupts-extended = <&gic GIC_PPI 7
> > > > > + IRQ_TYPE_LEVEL_HIGH>;
> > > >
> > > > Just a question, Is it tested?
> > > Yes this was tested with perf test
> > >
> > > > timer node[1] defines irq type as LOW, here it is high.
> > > You are right looking at the RZG2L_InterruptMapping_rev01.xlsx this should
> > > be LOW. (I followed the SPI IRQS where all the LEVEL interrupts are HIGH)
> > >
> > > > Also do we need to define (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW) as
> > > it has 2 cores??
> > > >
> > > No this is not required for example here [0] where it has 6 cores.
> >
> > I may be wrong, That is the only example[1], where the A55 PMU per cpu interrupts and number of a55 cores in the DT
> > are not matching.
> >
> Some SoCs specify the GIC_CPU_MASK_SIMPLE(x) while describing the PPI
> interrupt for the PMU and some dont [1]. What should be the correct
> usage when specifying the PPI interrupts for the PMU with multiple CPU
> cores (we are using
> arm,cortex-a55-pmu)?
>
> [1] https://elixir.bootlin.com/linux/latest/B/ident/arm%2Ccortex-a55-pmu
This is a GICv3 system. the GICv3 interrupts binding *does not* have a cpumask,
and it's always wrong to use GIC_CPU_MASK_SIMPLE() (or any mask, for that
matter) for GICv3
The GICv2 binding has the mask, but even there it's arguably pointless.
Please do not add the mask here, since it would violate the GICv3 binding.
Thanks,
Mark.
Hi Mark,
On Mon, Jan 30, 2023 at 2:10 PM Mark Rutland <[email protected]> wrote:
>
> On Mon, Jan 30, 2023 at 02:04:44PM +0000, Lad, Prabhakar wrote:
> > Hi Mark and Will,
> >
> > On Fri, Jan 27, 2023 at 9:48 PM Biju Das <[email protected]> wrote:
> > >
> > > Hi Prabhakar,
> > >
> > >
> > > > Subject: Re: [PATCH] arm64: dts: renesas: r9a07g044: Add Cortex-A55 PMU node
> > > >
> > > > Hi Biju,
> > > >
> > > > Thank you for the review.
> > > >
> > > > On Fri, Jan 27, 2023 at 6:38 PM Biju Das <[email protected]> wrote:
> > > > >
> > > > > Hi Prabhakar,
> > > > >
> > > > > Thanks for the patch.
> > > > >
> > > > > > Subject: [PATCH] arm64: dts: renesas: r9a07g044: Add Cortex-A55 PMU
> > > > > > node
> > > > > >
> > > > > > From: Lad Prabhakar <[email protected]>
> > > > > >
> > > > > > Enable the performance monitor unit for the Cortex-A55 cores on the
> > > > > > RZ/G2L
> > > > > > (r9a07g044) SoC.
> > > > > >
> > > > > > Signed-off-by: Lad Prabhakar
> > > > > > <[email protected]>
> > > > > > ---
> > > > > > arch/arm64/boot/dts/renesas/r9a07g044.dtsi | 5 +++++
> > > > > > 1 file changed, 5 insertions(+)
> > > > > >
> > > > > > diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > > > > b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > > > > index 80b2332798d9..ff9bdc03a3ed 100644
> > > > > > --- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > > > > +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > > > > > @@ -161,6 +161,11 @@ opp-50000000 {
> > > > > > };
> > > > > > };
> > > > > >
> > > > > > + pmu_a55 {
> > > > > > + compatible = "arm,cortex-a55-pmu";
> > > > > > + interrupts-extended = <&gic GIC_PPI 7
> > > > > > + IRQ_TYPE_LEVEL_HIGH>;
> > > > >
> > > > > Just a question, Is it tested?
> > > > Yes this was tested with perf test
> > > >
> > > > > timer node[1] defines irq type as LOW, here it is high.
> > > > You are right looking at the RZG2L_InterruptMapping_rev01.xlsx this should
> > > > be LOW. (I followed the SPI IRQS where all the LEVEL interrupts are HIGH)
> > > >
> > > > > Also do we need to define (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW) as
> > > > it has 2 cores??
> > > > >
> > > > No this is not required for example here [0] where it has 6 cores.
> > >
> > > I may be wrong, That is the only example[1], where the A55 PMU per cpu interrupts and number of a55 cores in the DT
> > > are not matching.
> > >
> > Some SoCs specify the GIC_CPU_MASK_SIMPLE(x) while describing the PPI
> > interrupt for the PMU and some dont [1]. What should be the correct
> > usage when specifying the PPI interrupts for the PMU with multiple CPU
> > cores (we are using
> > arm,cortex-a55-pmu)?
> >
> > [1] https://elixir.bootlin.com/linux/latest/B/ident/arm%2Ccortex-a55-pmu
>
> This is a GICv3 system. the GICv3 interrupts binding *does not* have a cpumask,
> and it's always wrong to use GIC_CPU_MASK_SIMPLE() (or any mask, for that
> matter) for GICv3
>
> The GICv2 binding has the mask, but even there it's arguably pointless.
>
> Please do not add the mask here, since it would violate the GICv3 binding.
>
Thank you for the clarification.
(Note to myself, to drop GIC_CPU_MASK_SIMPLE() from timer nodes from
rzg2l family)
Cheers,
Prabhakar