Re: [PATCH] arm64: dts: renesas: r9a07g044: Add Cortex-A55 PMU node

[Mark Rutland <mark.rutland@arm.com>]

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 <biju.das.jz@bp.renesas.com> 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 <biju.das.jz@bp.renesas.com> wrote:
> > > >
> > > > Hi Prabhakar,
> > > >
> > > > Thanks for the patch.
> > > >
> > > > > Subject: [PATCH] arm64: dts: renesas: r9a07g044: Add Cortex-A55 PMU
> > > > > node
> > > > >
> > > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > >
> > > > > Enable the performance monitor unit for the Cortex-A55 cores on the
> > > > > RZ/G2L
> > > > > (r9a07g044) SoC.
> > > > >
> > > > > Signed-off-by: Lad Prabhakar
> > > > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > ---
> > > > >  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.