| Message ID | 9d126711c7498b199b3e6f5cf48ca60ffb9df54c.1685983270.git.robin.murphy@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | perf/arm_cspmu: Fixes and cleanups | expand |
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
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 >
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) {