RE: [PATCH v2 3/8] perf/arm_cspmu: Add arm_cspmu_acpi_dev_get

[Besar Wicaksono]

> -----Original Message-----
> From: Jonathan Cameron <jonathan.cameron@xxxxxxxxxx>
> Sent: Thursday, February 19, 2026 3:40 AM
> To: Besar Wicaksono <bwicaksono@xxxxxxxxxx>
> Cc: will@xxxxxxxxxx; suzuki.poulose@xxxxxxx; robin.murphy@xxxxxxx;
> ilkka@xxxxxxxxxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; linux-tegra@xxxxxxxxxxxxxxx; mark.rutland@xxxxxxx;
> Thierry Reding <treding@xxxxxxxxxx>; Jon Hunter <jonathanh@xxxxxxxxxx>;
> Vikram Sethi <vsethi@xxxxxxxxxx>; Rich Wiley <rwiley@xxxxxxxxxx>; Shanker
> Donthineni <sdonthineni@xxxxxxxxxx>; Sean Kelley <skelley@xxxxxxxxxx>;
> Yifei Wan <ywan@xxxxxxxxxx>; Matt Ochs <mochs@xxxxxxxxxx>; Nirmoy Das
> <nirmoyd@xxxxxxxxxx>
> Subject: Re: [PATCH v2 3/8] perf/arm_cspmu: Add arm_cspmu_acpi_dev_get
> 
> External email: Use caution opening links or attachments
> 
> 
> On Wed, 18 Feb 2026 14:58:04 +0000
> Besar Wicaksono <bwicaksono@xxxxxxxxxx> wrote:
> 
> > Add interface to get ACPI device associated with the
> > PMU. This ACPI device may contain additional properties
> > not covered by the standard properties.
> >
> > Reviewed-by: Ilkka Koskinen <ilkka@xxxxxxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Besar Wicaksono <bwicaksono@xxxxxxxxxx>
> Hi Besar,
> 
> A drive by review as I was curious.
> 
> A few comments inline.
> > ---
> >  drivers/perf/arm_cspmu/arm_cspmu.c | 22 +++++++++++++++++++++-
> >  drivers/perf/arm_cspmu/arm_cspmu.h | 17 ++++++++++++++++-
> >  2 files changed, 37 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c
> b/drivers/perf/arm_cspmu/arm_cspmu.c
> > index 34430b68f602..ab2479c048bb 100644
> > --- a/drivers/perf/arm_cspmu/arm_cspmu.c
> > +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
> > @@ -16,7 +16,7 @@
> >   * The user should refer to the vendor technical documentation to get details
> >   * about the supported events.
> >   *
> > - * Copyright (c) 2022-2023, NVIDIA CORPORATION & AFFILIATES. All rights
> reserved.
> > + * Copyright (c) 2022-2026, NVIDIA CORPORATION & AFFILIATES. All rights
> reserved.
> >   *
> >   */
> >
> > @@ -1132,6 +1132,26 @@ static int arm_cspmu_acpi_get_cpus(struct
> arm_cspmu *cspmu)
> >
> >       return 0;
> >  }
> > +
> > +struct acpi_device *arm_cspmu_acpi_dev_get(const struct arm_cspmu
> *cspmu)
> > +{
> > +     char hid[16];
> > +     char uid[16];
> 
> Might as well do
>         char hid[16] = { };
>         char uid[16] = { };
> 

Sure, will do on V3.

> and drop the memsets below.
> 
> > +     const struct acpi_apmt_node *apmt_node;
> > +
> > +     apmt_node = arm_cspmu_apmt_node(cspmu->dev);
> > +     if (!apmt_node || apmt_node->type != ACPI_APMT_NODE_TYPE_ACPI)
> > +             return NULL;
> > +
> > +     memset(hid, 0, sizeof(hid));
> > +     memset(uid, 0, sizeof(uid));
> > +
> > +     memcpy(hid, &apmt_node->inst_primary, sizeof(apmt_node-
> >inst_primary));
> > +     snprintf(uid, sizeof(uid), "%u", apmt_node->inst_secondary);
> > +
> > +     return acpi_dev_get_first_match_dev(hid, uid, -1);
> > +}
> > +EXPORT_SYMBOL_GPL(arm_cspmu_acpi_dev_get);
> >  #else
> >  static int arm_cspmu_acpi_get_cpus(struct arm_cspmu *cspmu)
> >  {
> > diff --git a/drivers/perf/arm_cspmu/arm_cspmu.h
> b/drivers/perf/arm_cspmu/arm_cspmu.h
> > index cd65a58dbd88..320096673200 100644
> > --- a/drivers/perf/arm_cspmu/arm_cspmu.h
> > +++ b/drivers/perf/arm_cspmu/arm_cspmu.h
> > @@ -1,13 +1,14 @@
> >  /* SPDX-License-Identifier: GPL-2.0
> >   *
> >   * ARM CoreSight Architecture PMU driver.
> > - * Copyright (c) 2022-2023, NVIDIA CORPORATION & AFFILIATES. All rights
> reserved.
> > + * Copyright (c) 2022-2026, NVIDIA CORPORATION & AFFILIATES. All rights
> reserved.
> >   *
> >   */
> >
> >  #ifndef __ARM_CSPMU_H__
> >  #define __ARM_CSPMU_H__
> >
> > +#include <linux/acpi.h>
> >  #include <linux/bitfield.h>
> >  #include <linux/cpumask.h>
> >  #include <linux/device.h>
> > @@ -255,4 +256,18 @@ 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);
> >
> > +#if defined(CONFIG_ACPI)

Thanks for spotting. Will fix it on V3.

Regards,
Besar

> This isn't the same gate as used for whether the function is built. I think that's
> #if defined(CONFIG_ACPI) && defined(CONFIG_ARM64)
> 
> Whilst it might work to have them different today I think this is a little more
> fragile than would be ideal.
> 
> The ARM64 bit seems to be there to allow COMPILE_TEST for
> ARM_CORESIGHT_PMU_ARCH_SYSTEM_PMU  and to me that smells like a
> stub or Kconfig
> dependency missing.
> 
> > +/**
> > + * Get ACPI device associated with the PMU.
> > + * The caller is responsible for calling acpi_dev_put() on the returned device.
> > + */
> > +struct acpi_device *arm_cspmu_acpi_dev_get(const struct arm_cspmu
> *cspmu);
> > +#else
> > +static inline struct acpi_device *
> > +arm_cspmu_acpi_dev_get(const struct arm_cspmu *cspmu)
> > +{
> > +     return NULL;
> > +}
> > +#endif
> > +
> >  #endif /* __ARM_CSPMU_H__ */