Re: [PATCH 3/5] arm64: perf: Support new DT compatibles

From: Mark Rutland <mark.rutland@arm.com>
To: Robin Murphy <robin.murphy@arm.com>
Cc: will@kernel.org, catalin.marinas@arm.com, robh+dt@kernel.org,
	suzuki.poulose@arm.com, thierry.reding@gmail.com,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org
Subject: Re: [PATCH 3/5] arm64: perf: Support new DT compatibles
Date: Tue, 14 Dec 2021 13:36:18 +0000	[thread overview]
Message-ID: <Ybid0mUjLI8uJV+R@FVFF77S0Q05N> (raw)
In-Reply-To: <428b2e70-0ab1-8acc-3cbd-4df131976e27@arm.com>

On Tue, Dec 07, 2021 at 07:14:29PM +0000, Robin Murphy wrote:
> On 2021-12-07 18:44, Mark Rutland wrote:
> > On Tue, Dec 07, 2021 at 06:20:41PM +0000, Robin Murphy wrote:
> > > Wire up the new DT compatibles so we can present appropriate
> > > PMU names to userspace for the latest and greatest CPUs.
> > > 
> > > Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> > > ---
> > >   arch/arm64/kernel/perf_event.c | 36 ++++++++++++++++++++++++++++++++++
> > >   1 file changed, 36 insertions(+)
> > > 
> > > diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> > > index 57720372da62..3fe4dcfc28d4 100644
> > > --- a/arch/arm64/kernel/perf_event.c
> > > +++ b/arch/arm64/kernel/perf_event.c
> > > @@ -1215,6 +1215,26 @@ static int armv8_a78_pmu_init(struct arm_pmu *cpu_pmu)
> > >   	return armv8_pmu_init_nogroups(cpu_pmu, "armv8_cortex_a78", NULL);
> > >   }
> > > +static int armv9_a510_pmu_init(struct arm_pmu *cpu_pmu)
> > > +{
> > > +	return armv8_pmu_init_nogroups(cpu_pmu, "armv9_cortex_a510", NULL);
> > > +}
> > > +
> > > +static int armv9_a710_pmu_init(struct arm_pmu *cpu_pmu)
> > > +{
> > > +	return armv8_pmu_init_nogroups(cpu_pmu, "armv9_cortex_a710", NULL);
> > > +}
> > > +
> > > +static int armv8_x1_pmu_init(struct arm_pmu *cpu_pmu)
> > > +{
> > > +	return armv8_pmu_init_nogroups(cpu_pmu, "armv8_cortex_x1", NULL);
> > > +}
> > > +
> > > +static int armv9_x2_pmu_init(struct arm_pmu *cpu_pmu)
> > > +{
> > > +	return armv8_pmu_init_nogroups(cpu_pmu, "armv9_cortex_x2", NULL);
> > > +}
> > 
> > I wonder if it'd be better to do something like:
> > 
> > #define PMU_INIT_SIMPLE(name)						\
> > static int name##_pmu_init(struct arm_pmu *cpu_pmu)			\
> > {
> > 	return armv8_pmu_init_nogroups(cpu_pmu, #name, NULL);		\
> > }
> > 
> > PMU_INIT_SIMPLE(armv9_cortex_a510)
> > PMU_INIT_SIMPLE(armv9_cortex_a710)
> > PMU_INIT_SIMPLE(armv8_xortex_x1)
> > PMU_INIT_SIMPLE(armv9_xortex_x2)
> > 
> > ... and fix up the armv8_pmu_of_device_ids[] table to use the longer init names
> > that results in?
> 
> Indeed I did ponder doing almost exactly that, but at that point I'd rather
> try refactoring a bit deeper to convert most of the arm_pmu init business to
> pure data, so I figured I'd chuck in the simple tweak to mitigate these new
> additions with minimal churn, then have a go at the bigger change in its own
> right.

Sure; that makes sense to me, so for this as-is:

Acked-by: Mark Rutland <mark.rutland@arm.com>

... and I'll leave it to Will to have the final say on whether we want the
"armv9_" prefix or whether we stick with "armv8_" for consistenct, when he
chooses to pick this.

One thing I've just realised is that for the ACPI case, we're stuck with
"armv8_pmuv3_%d" regardless, which I think is fine itself, but we might want to
call that out.

Thanks,
Mark.

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