[v2,3/4] perf/arm_cspmu: Clean up ACPI dependency

Message ID 9d126711c7498b199b3e6f5cf48ca60ffb9df54c.1685983270.git.robin.murphy@arm.com
State New
Headers
Series perf/arm_cspmu: Fixes and cleanups |

Commit Message

Robin Murphy June 5, 2023, 5:01 p.m. UTC
  Build-wise, the ACPI dependency consists of only a couple of things
which could probably stand being factored out into ACPI helpers anyway.
However for the immediate concern of working towards Devicetree support
here, it's easy enough to make a few tweaks to contain the affected code
locally, such that we can relax the Kconfig dependency.

Reviewed-and-Tested-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Reviewed-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/perf/arm_cspmu/Kconfig     |  3 +--
 drivers/perf/arm_cspmu/arm_cspmu.c | 17 +++++++++++++++--
 2 files changed, 16 insertions(+), 4 deletions(-)
  

Comments

Geert Uytterhoeven July 3, 2023, 9:21 a.m. UTC | #1
Hi Robin,

On Mon, Jun 5, 2023 at 7:05 PM Robin Murphy <robin.murphy@arm.com> wrote:
> Build-wise, the ACPI dependency consists of only a couple of things
> which could probably stand being factored out into ACPI helpers anyway.
> However for the immediate concern of working towards Devicetree support
> here, it's easy enough to make a few tweaks to contain the affected code
> locally, such that we can relax the Kconfig dependency.
>
> Reviewed-and-Tested-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> Reviewed-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>

Thanks for your patch, which is now commit f9bd34e3753ea8f1
("perf/arm_cspmu: Clean up ACPI dependency") upstream.

> --- a/drivers/perf/arm_cspmu/Kconfig
> +++ b/drivers/perf/arm_cspmu/Kconfig
> @@ -4,8 +4,7 @@
>
>  config ARM_CORESIGHT_PMU_ARCH_SYSTEM_PMU
>         tristate "ARM Coresight Architecture PMU"
> -       depends on ARM64 && ACPI
> -       depends on ACPI_APMT || COMPILE_TEST
> +       depends on ARM64 || COMPILE_TEST

From looking at the code, the "arm-cs-arch-pmu" platform device can
be instantiated only through ACPI.  So I think it is a bit premature to
relax the dependency, and expose this question to people configuring
an ARM64 kernel without ACPI/APMT support.

Am I missing something?
Thanks!

>         help
>           Provides support for performance monitoring unit (PMU) devices
>           based on ARM CoreSight PMU architecture. Note that this PMU

Gr{oetje,eeting}s,

                        Geert
  
Robin Murphy July 3, 2023, 10:56 a.m. UTC | #2
Hi Geert,

On 2023-07-03 10:21, Geert Uytterhoeven wrote:
> Hi Robin,
> 
> On Mon, Jun 5, 2023 at 7:05 PM Robin Murphy <robin.murphy@arm.com> wrote:
>> Build-wise, the ACPI dependency consists of only a couple of things
>> which could probably stand being factored out into ACPI helpers anyway.
>> However for the immediate concern of working towards Devicetree support
>> here, it's easy enough to make a few tweaks to contain the affected code
>> locally, such that we can relax the Kconfig dependency.
>>
>> Reviewed-and-Tested-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> Reviewed-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> 
> Thanks for your patch, which is now commit f9bd34e3753ea8f1
> ("perf/arm_cspmu: Clean up ACPI dependency") upstream.
> 
>> --- a/drivers/perf/arm_cspmu/Kconfig
>> +++ b/drivers/perf/arm_cspmu/Kconfig
>> @@ -4,8 +4,7 @@
>>
>>   config ARM_CORESIGHT_PMU_ARCH_SYSTEM_PMU
>>          tristate "ARM Coresight Architecture PMU"
>> -       depends on ARM64 && ACPI
>> -       depends on ACPI_APMT || COMPILE_TEST
>> +       depends on ARM64 || COMPILE_TEST
> 
>  From looking at the code, the "arm-cs-arch-pmu" platform device can
> be instantiated only through ACPI.  So I think it is a bit premature to
> relax the dependency, and expose this question to people configuring
> an ARM64 kernel without ACPI/APMT support.
> 
> Am I missing something?

As was mentioned in the original cover letter on v1, these patches were 
actually the bottom half of a branch adding DT support - the DT parts 
are still untested and not quite complete (there's a property I don't 
need for the thing I'm looking at, but still deserves to be hooked up in 
general), but it seemed worth landing these prep patches since they 
impact what Besar and Ilkka are also working on in parallel.

At this point, the kconfig could indeed be "depends on (ARM64 && 
ACPI_APMT) || COMPILE_TEST". I can't recall why I didn't change that 
when splitting these patches out for posting - I may have decided the 
impact was negligible (i.e. even with DT support, it's still going to be 
a driver most people won't care about anyway), or the visibility vs. 
functional dependency aspect may have just slipped my mind entirely. 
You're welcome to make that change for now if you'd like to.

(I'm not sure how soon I'll be posting the follow-up DT patches, since 
I'm dependent on other people to provide testing and feedback, and 
haven't heard any news yet)

Thanks,
Robin.


> Thanks!
> 
>>          help
>>            Provides support for performance monitoring unit (PMU) devices
>>            based on ARM CoreSight PMU architecture. Note that this PMU
> 
> Gr{oetje,eeting}s,
> 
>                          Geert
>
  

Patch

diff --git a/drivers/perf/arm_cspmu/Kconfig b/drivers/perf/arm_cspmu/Kconfig
index 0b316fe69a45..25d25ded0983 100644
--- a/drivers/perf/arm_cspmu/Kconfig
+++ b/drivers/perf/arm_cspmu/Kconfig
@@ -4,8 +4,7 @@ 
 
 config ARM_CORESIGHT_PMU_ARCH_SYSTEM_PMU
 	tristate "ARM Coresight Architecture PMU"
-	depends on ARM64 && ACPI
-	depends on ACPI_APMT || COMPILE_TEST
+	depends on ARM64 || COMPILE_TEST
 	help
 	  Provides support for performance monitoring unit (PMU) devices
 	  based on ARM CoreSight PMU architecture. Note that this PMU
diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
index 72dc7a9e1ca8..3b91115c376d 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.c
+++ b/drivers/perf/arm_cspmu/arm_cspmu.c
@@ -28,7 +28,6 @@ 
 #include <linux/module.h>
 #include <linux/perf_event.h>
 #include <linux/platform_device.h>
-#include <acpi/processor.h>
 
 #include "arm_cspmu.h"
 #include "nvidia_cspmu.h"
@@ -1075,6 +1074,9 @@  static int arm_cspmu_request_irq(struct arm_cspmu *cspmu)
 	return 0;
 }
 
+#if defined(CONFIG_ACPI) && defined(CONFIG_ARM64)
+#include <acpi/processor.h>
+
 static inline int arm_cspmu_find_cpu_container(int cpu, u32 container_uid)
 {
 	u32 acpi_uid;
@@ -1099,7 +1101,7 @@  static inline int arm_cspmu_find_cpu_container(int cpu, u32 container_uid)
 	return -ENODEV;
 }
 
-static int arm_cspmu_get_cpus(struct arm_cspmu *cspmu)
+static int arm_cspmu_acpi_get_cpus(struct arm_cspmu *cspmu)
 {
 	struct device *dev;
 	struct acpi_apmt_node *apmt_node;
@@ -1135,6 +1137,17 @@  static int arm_cspmu_get_cpus(struct arm_cspmu *cspmu)
 
 	return 0;
 }
+#else
+static int arm_cspmu_acpi_get_cpus(struct arm_cspmu *cspmu)
+{
+	return -ENODEV;
+}
+#endif
+
+static int arm_cspmu_get_cpus(struct arm_cspmu *cspmu)
+{
+	return arm_cspmu_acpi_get_cpus(cspmu);
+}
 
 static int arm_cspmu_register_pmu(struct arm_cspmu *cspmu)
 {