[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <35ff1e0e-5810-49c4-8a1a-d58ad7a5cc31@arm.com>
Date: Thu, 21 Aug 2025 17:04:33 +0100
From: Robin Murphy <robin.murphy@....com>
To: Besar Wicaksono <bwicaksono@...dia.com>,
Ilkka Koskinen <ilkka@...amperecomputing.com>
Cc: "will@...nel.org" <will@...nel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
"suzuki.poulose@....com" <suzuki.poulose@....com>,
"mark.rutland@....com" <mark.rutland@....com>,
Thierry Reding <treding@...dia.com>, Jon Hunter <jonathanh@...dia.com>,
Vikram Sethi <vsethi@...dia.com>, Rich Wiley <rwiley@...dia.com>,
Shanker Donthineni <sdonthineni@...dia.com>
Subject: Re: [PATCH 1/5] perf/arm_cspmu: Export arm_cspmu_apmt_node
On 2025-08-20 8:07 pm, Besar Wicaksono wrote:
> Hi Ilkka,
>
> Thanks for the feedback. Please see my comment inline.
>
>> -----Original Message-----
>> From: Ilkka Koskinen <ilkka@...amperecomputing.com>
>> Sent: Tuesday, August 19, 2025 3:16 PM
>> To: Besar Wicaksono <bwicaksono@...dia.com>
>> Cc: will@...nel.org; linux-arm-kernel@...ts.infradead.org; linux-
>> kernel@...r.kernel.org; linux-tegra@...r.kernel.org;
>> suzuki.poulose@....com; robin.murphy@....com;
>> ilkka@...amperecomputing.com; mark.rutland@....com; Thierry Reding
>> <treding@...dia.com>; Jon Hunter <jonathanh@...dia.com>; Vikram Sethi
>> <vsethi@...dia.com>; Rich Wiley <rwiley@...dia.com>; Shanker Donthineni
>> <sdonthineni@...dia.com>
>> Subject: Re: [PATCH 1/5] perf/arm_cspmu: Export arm_cspmu_apmt_node
>>
>> External email: Use caution opening links or attachments
>>
>>
>> Hi Ben,
>>
>> On Tue, 12 Aug 2025, Besar Wicaksono wrote:
>>> Make arm_cspmu_apmt_node API accessible to vendor driver.
>>
>> I think I haven't seen the latest version of the spec. So, I'm curious,
>> what kind of information the table has that the vendor drivers needs to
>> have access to it?
>>
>
> The vendor driver may need the node instance primary and secondary
> fields to get additional properties of the PMU that is not covered
> by the standard properties. For example, the PMU device entry in
> APMT can be defined as ACPI node type. The node instance primary
> and secondary will contain the HID and UID of an ACPI device object
> that is associated with the PMU. This ACPI object can have more info
> to supplement the standard props.
Rather than exposing the raw APMT data, maybe then cspmu should just
encapsulate a method for retrieving the associated ACPI device, if any?
I guess it could be a generalised "firmware device" notion - even though
for DT that should mostly be cspmu->dev already - since that could then
work neatly for generic device properties, but perhaps we don't expect
many sub-drivers to support both ACPI and DT...
Thanks,
Robin.
>>>
>>> Signed-off-by: Besar Wicaksono <bwicaksono@...dia.com>
>>> ---
>>> drivers/perf/arm_cspmu/arm_cspmu.c | 3 ++-
>>> drivers/perf/arm_cspmu/arm_cspmu.h | 4 ++++
>>> 2 files changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c
>> b/drivers/perf/arm_cspmu/arm_cspmu.c
>>> index efa9b229e701..e4b98cfa606c 100644
>>> --- a/drivers/perf/arm_cspmu/arm_cspmu.c
>>> +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
>>> @@ -70,12 +70,13 @@ static void arm_cspmu_set_ev_filter(struct
>> arm_cspmu *cspmu,
>>> static void arm_cspmu_set_cc_filter(struct arm_cspmu *cspmu,
>>> const struct perf_event *event);
>>>
>>> -static struct acpi_apmt_node *arm_cspmu_apmt_node(struct device *dev)
>>> +struct acpi_apmt_node *arm_cspmu_apmt_node(struct device *dev)
>>> {
>>> struct acpi_apmt_node **ptr = dev_get_platdata(dev);
>>>
>>> return ptr ? *ptr : NULL;
>>> }
>>> +EXPORT_SYMBOL_GPL(arm_cspmu_apmt_node);
>>
>> Rather than exporting the function, wouldn't it be better to move it to
>> arm_cspmu.h instead?
>
> Sounds good to me. I will make the change on the next revision.
>
> Thanks,
> Besar
>
>>
>> Cheers, Ilkka
>>
>>>
>>> /*
>>> * In CoreSight PMU architecture, all of the MMIO registers are 32-bit except
>>> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.h
>> b/drivers/perf/arm_cspmu/arm_cspmu.h
>>> index 19684b76bd96..36c1dcce33d6 100644
>>> --- a/drivers/perf/arm_cspmu/arm_cspmu.h
>>> +++ b/drivers/perf/arm_cspmu/arm_cspmu.h
>>> @@ -8,6 +8,7 @@
>>> #ifndef __ARM_CSPMU_H__
>>> #define __ARM_CSPMU_H__
>>>
>>> +#include <linux/acpi.h>
>>> #include <linux/bitfield.h>
>>> #include <linux/cpumask.h>
>>> #include <linux/device.h>
>>> @@ -222,4 +223,7 @@ int arm_cspmu_impl_register(const struct
>> arm_cspmu_impl_match *impl_match);
>>> /* Unregister vendor backend. */
>>> void arm_cspmu_impl_unregister(const struct arm_cspmu_impl_match
>> *impl_match);
>>>
>>> +/* Get ACPI APMT node. */
>>> +struct acpi_apmt_node *arm_cspmu_apmt_node(struct device *dev);
>>> +
>>> #endif /* __ARM_CSPMU_H__ */
>>> --
>>> 2.47.0
>>>
>>>
Powered by blists - more mailing lists