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

From: Robin Murphy
Date: Mon Jul 03 2023 - 06:56:30 EST


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@xxxxxxx> 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@xxxxxxx>
Reviewed-by: Ilkka Koskinen <ilkka@xxxxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Robin Murphy <robin.murphy@xxxxxxx>

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