Re: [PATCH 2/4] perf/arm-cmn: Add CMN-650 support

From: Ilkka Koskinen <ilkka@os.amperecomputing.com>
To: Ilkka Koskinen <ilkka@os.amperecomputing.com>
Cc: Robin Murphy <robin.murphy@arm.com>,
	will@kernel.org, mark.rutland@arm.com,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 2/4] perf/arm-cmn: Add CMN-650 support
Date: Tue, 19 Apr 2022 16:47:11 -0700 (PDT)	[thread overview]
Message-ID: <alpine.DEB.2.22.394.2204191637120.1991@ubuntu200401> (raw)
In-Reply-To: <alpine.DEB.2.22.394.2204191446440.2855@ubuntu200401>

On Tue, 19 Apr 2022, Ilkka Koskinen wrote:
>
> Hi Robin,
>
> I need to go through your patches more carefully, but I do have a couple of 
> comments already:
>
> On Mon, 18 Apr 2022, Robin Murphy wrote:
>> Add the identifiers and events for CMN-650, which slots into its
>> evolutionary position between CMN-600 and the 700-series products.
>> Imagine CMN-600 made bigger, and with most of the rough edges smoothed
>> off, but that then balanced out by some bonkers PMU functionality for
>> the new HN-P enhancement in CMN-650r2.
>> 
>> Most of the CXG events are actually common to newer revisions of CMN-600
>> too, so they're arguably a little late; oh well.
>> 
>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>> ---
>> drivers/perf/arm-cmn.c | 222 ++++++++++++++++++++++++++++++++---------
>> 1 file changed, 176 insertions(+), 46 deletions(-)
>> 
>> diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c
>> index 9c1d82be7a2f..cce8516d465c 100644
>> --- a/drivers/perf/arm-cmn.c
>> +++ b/drivers/perf/arm-cmn.c

>> @@ -1979,6 +2108,7 @@ MODULE_DEVICE_TABLE(of, arm_cmn_of_match);
>> #ifdef CONFIG_ACPI
>> static const struct acpi_device_id arm_cmn_acpi_match[] = {
>> 	{ "ARMHC600", CMN600 },
>> +	{ "ARMHC650", CMN650 },
>
> Not the great place for this comment but there probably isn't any better.
>
> Based on DEN0093 v1.1, CMN's DSDT entries have been changed since CMN-600. 
> ROOTNODEBASE seems to be specific for CMN-600. Moreover, if you compare the 
> address maps in TRMs' Discovery chapters, you can see the difference.
>
> I'm thinking, at the minimal the second platform_get_resource() call has to 
> be skipped and zero returned in arm_cmn600_acpi_probe(), if the model is 
> cmn650 (probably also for cmn-700)

Uh, if only I had read the code more carefully, I had noticed that's what 
the driver does indeed.

Anyway, so far it everything works fine. I test the driver a little more 
(and review the patches more carefully) and will let know how it goes.

--Ilkka

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