This series enables detection of ACPI based TRBE devices via a stand alone purpose built representative platform device. But as a pre-requisite this changes coresight_platform_data structure assignment for the TRBE device.
This series is based on v6.5-rc4 kernel, is also dependent on the following EDK2 changes posted earlier by Sami.
https://edk2.groups.io/g/devel/message/107239 https://edk2.groups.io/g/devel/message/107241
Changes in V3:
- Changed ARMV8_TRBE_PDEV_NAME from "arm-trbe-acpi" into "arm,trbe" - Dropped local variable 'matched' - Replaced 'matched' with 'valid gsi' as being already matched once - Moved find_acpi_cpu_topology_hetero_id() outside conditional check
Changes in V2:
https://lore.kernel.org/all/20230801094052.750416-1-anshuman.khandual@arm.co...
- Refactored arm_spe_acpi_register_device() in a separate patch - Renamed trbe_acpi_resources as trbe_resources - Renamed trbe_acpi_dev as trbe_dev
Changes in V1:
https://lore.kernel.org/all/20230728112733.359620-1-anshuman.khandual@arm.co...
Cc: Sami Mujawar sami.mujawar@arm.com Cc: Catalin Marinas catalin.marinas@arm.com Cc: Will Deacon will@kernel.org Cc: Mark Rutland mark.rutland@arm.com Cc: Suzuki K Poulose suzuki.poulose@arm.com Cc: Mike Leach mike.leach@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: James Clark james.clark@arm.com Cc: coresight@lists.linaro.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org
Anshuman Khandual (4): arm_pmu: acpi: Refactor arm_spe_acpi_register_device() arm_pmu: acpi: Add a representative platform device for TRBE coresight: trbe: Add a representative coresight_platform_data for TRBE coresight: trbe: Enable ACPI based TRBE devices
arch/arm64/include/asm/acpi.h | 3 + drivers/hwtracing/coresight/coresight-trbe.c | 15 +- drivers/hwtracing/coresight/coresight-trbe.h | 1 + drivers/perf/arm_pmu_acpi.c | 142 +++++++++++++------ include/linux/perf/arm_pmu.h | 1 + 5 files changed, 119 insertions(+), 43 deletions(-)
Sanity checking all the GICC tables for same interrupt number, and ensuring a homogeneous ACPI based machine, could be used for other platform devices as well. Hence this refactors arm_spe_acpi_register_device() into a common helper arm_acpi_register_pmu_device().
Cc: Catalin Marinas catalin.marinas@arm.com Cc: Will Deacon will@kernel.org Cc: Mark Rutland mark.rutland@arm.com Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Co-developed-by: Will Deacon will@kernel.org Signed-off-by: Will Deacon will@kernel.org Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com --- drivers/perf/arm_pmu_acpi.c | 107 ++++++++++++++++++++++-------------- 1 file changed, 67 insertions(+), 40 deletions(-)
diff --git a/drivers/perf/arm_pmu_acpi.c b/drivers/perf/arm_pmu_acpi.c index 90815ad762eb..235c14766a36 100644 --- a/drivers/perf/arm_pmu_acpi.c +++ b/drivers/perf/arm_pmu_acpi.c @@ -70,6 +70,65 @@ static void arm_pmu_acpi_unregister_irq(int cpu) }
#if IS_ENABLED(CONFIG_ARM_SPE_PMU) +static int +arm_acpi_register_pmu_device(struct platform_device *pdev, u8 len, + u16 (*parse_gsi)(struct acpi_madt_generic_interrupt *)) +{ + int cpu, this_hetid, hetid, irq, ret; + u16 this_gsi, gsi = 0; + + /* + * Ensure that platform device must have IORESOURCE_IRQ + * resource to hold gsi interrupt. + */ + if (pdev->num_resources != 1) + return -ENXIO; + + if (pdev->resource[0].flags != IORESOURCE_IRQ) + return -ENXIO; + + /* + * Sanity check all the GICC tables for the same interrupt + * number. For now, only support homogeneous ACPI machines. + */ + for_each_possible_cpu(cpu) { + struct acpi_madt_generic_interrupt *gicc; + + gicc = acpi_cpu_get_madt_gicc(cpu); + if (gicc->header.length < len) + return gsi ? -ENXIO : 0; + + this_gsi = parse_gsi(gicc); + if (!this_gsi) + return gsi ? -ENXIO : 0; + + this_hetid = find_acpi_cpu_topology_hetero_id(cpu); + if (!gsi) { + hetid = this_hetid; + gsi = this_gsi; + } else if (hetid != this_hetid || gsi != this_gsi) { + pr_warn("ACPI: %s: must be homogeneous\n", pdev->name); + return -ENXIO; + } + } + + irq = acpi_register_gsi(NULL, gsi, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_HIGH); + if (irq < 0) { + pr_warn("ACPI: %s Unable to register interrupt: %d\n", pdev->name, gsi); + return -ENXIO; + } + + pdev->resource[0].start = irq; + ret = platform_device_register(pdev); + if (ret < 0) { + pr_warn("ACPI: %s: Unable to register device\n", pdev->name); + acpi_unregister_gsi(gsi); + } + return ret; +} +#endif + +#ifdef CONFIG_ARM_SPE_PMU static struct resource spe_resources[] = { { /* irq */ @@ -84,6 +143,11 @@ static struct platform_device spe_dev = { .num_resources = ARRAY_SIZE(spe_resources) };
+static u16 arm_spe_parse_gsi(struct acpi_madt_generic_interrupt *gicc) +{ + return gicc->spe_interrupt; +} + /* * For lack of a better place, hook the normal PMU MADT walk * and create a SPE device if we detect a recent MADT with @@ -91,47 +155,10 @@ static struct platform_device spe_dev = { */ static void arm_spe_acpi_register_device(void) { - int cpu, hetid, irq, ret; - bool first = true; - u16 gsi = 0; - - /* - * Sanity check all the GICC tables for the same interrupt number. - * For now, we only support homogeneous ACPI/SPE machines. - */ - for_each_possible_cpu(cpu) { - struct acpi_madt_generic_interrupt *gicc; - - gicc = acpi_cpu_get_madt_gicc(cpu); - if (gicc->header.length < ACPI_MADT_GICC_SPE) - return; - - if (first) { - gsi = gicc->spe_interrupt; - if (!gsi) - return; - hetid = find_acpi_cpu_topology_hetero_id(cpu); - first = false; - } else if ((gsi != gicc->spe_interrupt) || - (hetid != find_acpi_cpu_topology_hetero_id(cpu))) { - pr_warn("ACPI: SPE must be homogeneous\n"); - return; - } - } - - irq = acpi_register_gsi(NULL, gsi, ACPI_LEVEL_SENSITIVE, - ACPI_ACTIVE_HIGH); - if (irq < 0) { - pr_warn("ACPI: SPE Unable to register interrupt: %d\n", gsi); - return; - } - - spe_resources[0].start = irq; - ret = platform_device_register(&spe_dev); - if (ret < 0) { + int ret = arm_acpi_register_pmu_device(&spe_dev, ACPI_MADT_GICC_SPE, + arm_spe_parse_gsi); + if (ret) pr_warn("ACPI: SPE: Unable to register device\n"); - acpi_unregister_gsi(gsi); - } } #else static inline void arm_spe_acpi_register_device(void)
On 8/3/23 11:26, Anshuman Khandual wrote:
- /*
* Sanity check all the GICC tables for the same interrupt* number. For now, only support homogeneous ACPI machines.*/- for_each_possible_cpu(cpu) {
struct acpi_madt_generic_interrupt *gicc;gicc = acpi_cpu_get_madt_gicc(cpu);if (gicc->header.length < len)return gsi ? -ENXIO : 0;this_gsi = parse_gsi(gicc);if (!this_gsi)return gsi ? -ENXIO : 0;
Hello Will,
Moved parse_gsi() return code checking to its original place just to make it similar in semantics to existing 'gicc->header.length check'. If 'gsi' is valid i.e atleast a single cpu has been probed, return -ENXIO indicating mismatch, otherwise just return 0.
- Anshuman
ACPI TRBE does not have a HID for identification which could create and add a platform device into the platform bus. Also without a platform device, it cannot be probed and bound to a platform driver.
This creates a dummy platform device for TRBE after ascertaining that ACPI provides required interrupts uniformly across all cpus on the system. This device gets created inside drivers/perf/arm_pmu_acpi.c to accommodate TRBE being built as a module.
Cc: Catalin Marinas catalin.marinas@arm.com Cc: Will Deacon will@kernel.org Cc: Mark Rutland mark.rutland@arm.com Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com --- arch/arm64/include/asm/acpi.h | 3 +++ drivers/perf/arm_pmu_acpi.c | 37 ++++++++++++++++++++++++++++++++++- include/linux/perf/arm_pmu.h | 1 + 3 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h index bd68e1b7f29f..4d537d56eb84 100644 --- a/arch/arm64/include/asm/acpi.h +++ b/arch/arm64/include/asm/acpi.h @@ -42,6 +42,9 @@ #define ACPI_MADT_GICC_SPE (offsetof(struct acpi_madt_generic_interrupt, \ spe_interrupt) + sizeof(u16))
+#define ACPI_MADT_GICC_TRBE (offsetof(struct acpi_madt_generic_interrupt, \ + trbe_interrupt) + sizeof(u16)) + /* Basic configuration for ACPI */ #ifdef CONFIG_ACPI pgprot_t __acpi_get_mem_attribute(phys_addr_t addr); diff --git a/drivers/perf/arm_pmu_acpi.c b/drivers/perf/arm_pmu_acpi.c index 235c14766a36..79feea548e6e 100644 --- a/drivers/perf/arm_pmu_acpi.c +++ b/drivers/perf/arm_pmu_acpi.c @@ -69,7 +69,7 @@ static void arm_pmu_acpi_unregister_irq(int cpu) acpi_unregister_gsi(gsi); }
-#if IS_ENABLED(CONFIG_ARM_SPE_PMU) +#if IS_ENABLED(CONFIG_ARM_SPE_PMU) || IS_ENABLED(CONFIG_CORESIGHT_TRBE) static int arm_acpi_register_pmu_device(struct platform_device *pdev, u8 len, u16 (*parse_gsi)(struct acpi_madt_generic_interrupt *)) @@ -166,6 +166,40 @@ static inline void arm_spe_acpi_register_device(void) } #endif /* CONFIG_ARM_SPE_PMU */
+#ifdef CONFIG_CORESIGHT_TRBE +static struct resource trbe_resources[] = { + { + /* irq */ + .flags = IORESOURCE_IRQ, + } +}; + +static struct platform_device trbe_dev = { + .name = ARMV8_TRBE_PDEV_NAME, + .id = -1, + .resource = trbe_resources, + .num_resources = ARRAY_SIZE(trbe_resources) +}; + +static u16 arm_trbe_parse_gsi(struct acpi_madt_generic_interrupt *gicc) +{ + return gicc->trbe_interrupt; +} + +static void arm_trbe_acpi_register_device(void) +{ + int ret = arm_acpi_register_pmu_device(&trbe_dev, ACPI_MADT_GICC_TRBE, + arm_trbe_parse_gsi); + if (ret) + pr_warn("ACPI: TRBE: Unable to register device\n"); +} +#else +static inline void arm_trbe_acpi_register_device(void) +{ + +} +#endif /* CONFIG_CORESIGHT_TRBE */ + static int arm_pmu_acpi_parse_irqs(void) { int irq, cpu, irq_cpu, err; @@ -401,6 +435,7 @@ static int arm_pmu_acpi_init(void) return 0;
arm_spe_acpi_register_device(); + arm_trbe_acpi_register_device();
return 0; } diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h index a0801f68762b..143fbc10ecfe 100644 --- a/include/linux/perf/arm_pmu.h +++ b/include/linux/perf/arm_pmu.h @@ -187,5 +187,6 @@ void armpmu_free_irq(int irq, int cpu); #endif /* CONFIG_ARM_PMU */
#define ARMV8_SPE_PDEV_NAME "arm,spe-v1" +#define ARMV8_TRBE_PDEV_NAME "arm,trbe"
#endif /* __ARM_PMU_H__ */
On 8/3/23 11:26, Anshuman Khandual wrote:
ACPI TRBE does not have a HID for identification which could create and add a platform device into the platform bus. Also without a platform device, it cannot be probed and bound to a platform driver.
This creates a dummy platform device for TRBE after ascertaining that ACPI provides required interrupts uniformly across all cpus on the system. This device gets created inside drivers/perf/arm_pmu_acpi.c to accommodate TRBE being built as a module.
Cc: Catalin Marinas catalin.marinas@arm.com Cc: Will Deacon will@kernel.org Cc: Mark Rutland mark.rutland@arm.com Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com
arch/arm64/include/asm/acpi.h | 3 +++ drivers/perf/arm_pmu_acpi.c | 37 ++++++++++++++++++++++++++++++++++- include/linux/perf/arm_pmu.h | 1 + 3 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h index bd68e1b7f29f..4d537d56eb84 100644 --- a/arch/arm64/include/asm/acpi.h +++ b/arch/arm64/include/asm/acpi.h @@ -42,6 +42,9 @@ #define ACPI_MADT_GICC_SPE (offsetof(struct acpi_madt_generic_interrupt, \ spe_interrupt) + sizeof(u16)) +#define ACPI_MADT_GICC_TRBE (offsetof(struct acpi_madt_generic_interrupt, \
- trbe_interrupt) + sizeof(u16))
/* Basic configuration for ACPI */ #ifdef CONFIG_ACPI pgprot_t __acpi_get_mem_attribute(phys_addr_t addr); diff --git a/drivers/perf/arm_pmu_acpi.c b/drivers/perf/arm_pmu_acpi.c index 235c14766a36..79feea548e6e 100644 --- a/drivers/perf/arm_pmu_acpi.c +++ b/drivers/perf/arm_pmu_acpi.c @@ -69,7 +69,7 @@ static void arm_pmu_acpi_unregister_irq(int cpu) acpi_unregister_gsi(gsi); } -#if IS_ENABLED(CONFIG_ARM_SPE_PMU) +#if IS_ENABLED(CONFIG_ARM_SPE_PMU) || IS_ENABLED(CONFIG_CORESIGHT_TRBE)
Rather than adding IS_ENABLED() checks for all applicable configs in future which will need to call arm_acpi_register_pmu_device() for a dummy platform device, could we instead just add __maybe_unused for the function to prevent build warning when there are no call sites ? Seems bit better and simpler.
static int arm_acpi_register_pmu_device(struct platform_device *pdev, u8 len, u16 (*parse_gsi)(struct acpi_madt_generic_interrupt *)) @@ -166,6 +166,40 @@ static inline void arm_spe_acpi_register_device(void) } #endif /* CONFIG_ARM_SPE_PMU */ +#ifdef CONFIG_CORESIGHT_TRBE +static struct resource trbe_resources[] = {
- {
/* irq */.flags = IORESOURCE_IRQ,- }
+};
+static struct platform_device trbe_dev = {
- .name = ARMV8_TRBE_PDEV_NAME,
- .id = -1,
- .resource = trbe_resources,
- .num_resources = ARRAY_SIZE(trbe_resources)
+};
+static u16 arm_trbe_parse_gsi(struct acpi_madt_generic_interrupt *gicc) +{
- return gicc->trbe_interrupt;
+}
+static void arm_trbe_acpi_register_device(void) +{
- int ret = arm_acpi_register_pmu_device(&trbe_dev, ACPI_MADT_GICC_TRBE,
arm_trbe_parse_gsi);- if (ret)
pr_warn("ACPI: TRBE: Unable to register device\n");+} +#else +static inline void arm_trbe_acpi_register_device(void) +{
+} +#endif /* CONFIG_CORESIGHT_TRBE */
static int arm_pmu_acpi_parse_irqs(void) { int irq, cpu, irq_cpu, err; @@ -401,6 +435,7 @@ static int arm_pmu_acpi_init(void) return 0; arm_spe_acpi_register_device();
- arm_trbe_acpi_register_device();
return 0; } diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h index a0801f68762b..143fbc10ecfe 100644 --- a/include/linux/perf/arm_pmu.h +++ b/include/linux/perf/arm_pmu.h @@ -187,5 +187,6 @@ void armpmu_free_irq(int irq, int cpu); #endif /* CONFIG_ARM_PMU */ #define ARMV8_SPE_PDEV_NAME "arm,spe-v1" +#define ARMV8_TRBE_PDEV_NAME "arm,trbe" #endif /* __ARM_PMU_H__ */
TRBE coresight devices do not need regular connections information, as the paths get built between all percpu source and their respective percpu sink devices. Please refer 'commit 2cd87a7b293d ("coresight: core: Add support for dedicated percpu sinks")' which added support for percpu sink devices.
coresight_register() expect device connections via the platform_data. TRBE devices do not have any graph connections and thus is empty. With upcoming ACPI support for TRBE, we do not get a real acpi_device and thus coresight_get_platform_dat() will end up in failures. Hence this allocates a zeroed coresight_platform_data structure and assigns that back into the device.
Cc: Suzuki K Poulose suzuki.poulose@arm.com Cc: Mike Leach mike.leach@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: coresight@lists.linaro.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com --- drivers/hwtracing/coresight/coresight-trbe.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 7720619909d6..e1d9d06e7725 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -1494,9 +1494,9 @@ static int arm_trbe_device_probe(struct platform_device *pdev) if (!drvdata) return -ENOMEM;
- pdata = coresight_get_platform_data(dev); - if (IS_ERR(pdata)) - return PTR_ERR(pdata); + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) + return -ENOMEM;
dev_set_drvdata(dev, drvdata); dev->platform_data = pdata;
On 03/08/2023 06:56, Anshuman Khandual wrote:
TRBE coresight devices do not need regular connections information, as the paths get built between all percpu source and their respective percpu sink devices. Please refer 'commit 2cd87a7b293d ("coresight: core: Add support for dedicated percpu sinks")' which added support for percpu sink devices.
coresight_register() expect device connections via the platform_data. TRBE devices do not have any graph connections and thus is empty. With upcoming ACPI support for TRBE, we do not get a real acpi_device and thus coresight_get_platform_dat() will end up in failures. Hence this allocates a zeroed coresight_platform_data structure and assigns that back into the device.
Cc: Suzuki K Poulose suzuki.poulose@arm.com Cc: Mike Leach mike.leach@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: coresight@lists.linaro.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com
drivers/hwtracing/coresight/coresight-trbe.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 7720619909d6..e1d9d06e7725 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -1494,9 +1494,9 @@ static int arm_trbe_device_probe(struct platform_device *pdev) if (!drvdata) return -ENOMEM;
- pdata = coresight_get_platform_data(dev);
- if (IS_ERR(pdata))
return PTR_ERR(pdata);
- pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
- if (!pdata)
return -ENOMEM;
Please could you add a comment in here, on why we use a dummy platform data ? It is good to have documented it in the code too.
Suzuki
dev_set_drvdata(dev, drvdata); dev->platform_data = pdata;
On 8/3/23 19:25, Suzuki K Poulose wrote:
On 03/08/2023 06:56, Anshuman Khandual wrote:
TRBE coresight devices do not need regular connections information, as the paths get built between all percpu source and their respective percpu sink devices. Please refer 'commit 2cd87a7b293d ("coresight: core: Add support for dedicated percpu sinks")' which added support for percpu sink devices.
coresight_register() expect device connections via the platform_data. TRBE devices do not have any graph connections and thus is empty. With upcoming ACPI support for TRBE, we do not get a real acpi_device and thus coresight_get_platform_dat() will end up in failures. Hence this allocates a zeroed coresight_platform_data structure and assigns that back into the device.
Cc: Suzuki K Poulose suzuki.poulose@arm.com Cc: Mike Leach mike.leach@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: coresight@lists.linaro.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com
drivers/hwtracing/coresight/coresight-trbe.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 7720619909d6..e1d9d06e7725 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -1494,9 +1494,9 @@ static int arm_trbe_device_probe(struct platform_device *pdev) if (!drvdata) return -ENOMEM; - pdata = coresight_get_platform_data(dev); - if (IS_ERR(pdata)) - return PTR_ERR(pdata); + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) + return -ENOMEM;
Please could you add a comment in here, on why we use a dummy platform data ? It is good to have documented it in the code too.
Sure, will add the following in-code documentation.
+ /* + * TRBE coresight devices do not need regular connections + * information, as the paths get built between all percpu + * source and their respective percpu sink devices. Though + * coresight_register() expect device connections via the + * platform_data, which TRBE devices do not have. As they + * are not real ACPI devices, coresight_get_platform_dat() + * ends up failing. Instead let's allocate a dummy zeroed + * coresight_platform_data structure and assign that back + * into the device for that purpose. + */
On 04/08/2023 10:18, Anshuman Khandual wrote:
On 8/3/23 19:25, Suzuki K Poulose wrote:
On 03/08/2023 06:56, Anshuman Khandual wrote:
TRBE coresight devices do not need regular connections information, as the paths get built between all percpu source and their respective percpu sink devices. Please refer 'commit 2cd87a7b293d ("coresight: core: Add support for dedicated percpu sinks")' which added support for percpu sink devices.
coresight_register() expect device connections via the platform_data. TRBE devices do not have any graph connections and thus is empty. With upcoming ACPI support for TRBE, we do not get a real acpi_device and thus coresight_get_platform_dat() will end up in failures. Hence this allocates a zeroed coresight_platform_data structure and assigns that back into the device.
Cc: Suzuki K Poulose suzuki.poulose@arm.com Cc: Mike Leach mike.leach@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: coresight@lists.linaro.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com
drivers/hwtracing/coresight/coresight-trbe.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index 7720619909d6..e1d9d06e7725 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -1494,9 +1494,9 @@ static int arm_trbe_device_probe(struct platform_device *pdev) if (!drvdata) return -ENOMEM; - pdata = coresight_get_platform_data(dev); - if (IS_ERR(pdata)) - return PTR_ERR(pdata); + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) + return -ENOMEM;
Please could you add a comment in here, on why we use a dummy platform data ? It is good to have documented it in the code too.
Sure, will add the following in-code documentation.
/** TRBE coresight devices do not need regular connections* information, as the paths get built between all percpu* source and their respective percpu sink devices. Though* coresight_register() expect device connections via the* platform_data, which TRBE devices do not have. As they* are not real ACPI devices, coresight_get_platform_dat()
minor nit: s/coresight_get_platform_dat/coresight_get_platform_data/ here and above in the description.
Otherwise, looks good.
Suzuki
This detects and enables ACPI based TRBE devices via the dummy platform device created earlier for this purpose.
Cc: Suzuki K Poulose suzuki.poulose@arm.com Cc: Mike Leach mike.leach@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: coresight@lists.linaro.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com --- drivers/hwtracing/coresight/coresight-trbe.c | 9 +++++++++ drivers/hwtracing/coresight/coresight-trbe.h | 1 + 2 files changed, 10 insertions(+)
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index e1d9d06e7725..f884883e9018 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -1537,7 +1537,16 @@ static const struct of_device_id arm_trbe_of_match[] = { }; MODULE_DEVICE_TABLE(of, arm_trbe_of_match);
+#ifdef CONFIG_ACPI +static const struct platform_device_id arm_trbe_acpi_match[] = { + { ARMV8_TRBE_PDEV_NAME, 0 }, + { } +}; +MODULE_DEVICE_TABLE(platform, arm_trbe_acpi_match); +#endif + static struct platform_driver arm_trbe_driver = { + .id_table = arm_trbe_acpi_match, .driver = { .name = DRVNAME, .of_match_table = of_match_ptr(arm_trbe_of_match), diff --git a/drivers/hwtracing/coresight/coresight-trbe.h b/drivers/hwtracing/coresight/coresight-trbe.h index 77cbb5c63878..94e67009848a 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.h +++ b/drivers/hwtracing/coresight/coresight-trbe.h @@ -12,6 +12,7 @@ #include <linux/irq.h> #include <linux/kernel.h> #include <linux/of.h> +#include <linux/perf/arm_pmu.h> #include <linux/platform_device.h> #include <linux/smp.h>
On 8/3/23 11:26, Anshuman Khandual wrote:
This detects and enables ACPI based TRBE devices via the dummy platform device created earlier for this purpose.
Cc: Suzuki K Poulose suzuki.poulose@arm.com Cc: Mike Leach mike.leach@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: coresight@lists.linaro.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com
drivers/hwtracing/coresight/coresight-trbe.c | 9 +++++++++ drivers/hwtracing/coresight/coresight-trbe.h | 1 + 2 files changed, 10 insertions(+)
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index e1d9d06e7725..f884883e9018 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -1537,7 +1537,16 @@ static const struct of_device_id arm_trbe_of_match[] = { }; MODULE_DEVICE_TABLE(of, arm_trbe_of_match); +#ifdef CONFIG_ACPI +static const struct platform_device_id arm_trbe_acpi_match[] = {
- { ARMV8_TRBE_PDEV_NAME, 0 },
- { }
+}; +MODULE_DEVICE_TABLE(platform, arm_trbe_acpi_match); +#endif
static struct platform_driver arm_trbe_driver = {
- .id_table = arm_trbe_acpi_match,
The build problem [1] reported on the first version of the series still exists here i.e arm_trbe_acpi_match is hidden without CONFIG_ACPI. I had assumed that CONFIG_CORESIGHT always enables CONFIG_ACPI, which is not the case. Following random config (with CONFIG_ACPI=n and CONFIG_CORESIGHT_TRBE=y) easily triggers the build problem.
https://download.01.org/0day-ci/archive/20230805/202308052123.uqR35d19-lkp@i...
make CROSS_COMPILE=aarch64-linux-gnu- ARCH=arm64 -s -j 128 drivers/hwtracing/coresight/coresight-trbe.c:1563:23: error: implicit declaration of function ‘ACPI_PTR’ [-Werror=implicit-function-declaration] 1563 | .acpi_match_table = ACPI_PTR(arm_trbe_acpi_match), | ^~~~~~~~ drivers/hwtracing/coresight/coresight-trbe.c:1563:32: error: ‘arm_trbe_acpi_match’ undeclared here (not in a function); did you mean ‘arm_trbe_of_match’? 1563 | .acpi_match_table = ACPI_PTR(arm_trbe_acpi_match), | ^~~~~~~~~~~~~~~~~~~ | arm_trbe_of_match
Following config wrap around fixes the problem.
--- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -1557,7 +1557,9 @@ MODULE_DEVICE_TABLE(platform, arm_trbe_acpi_match); #endif
static struct platform_driver arm_trbe_driver = { +#ifdef CONFIG_ACPI .id_table = arm_trbe_acpi_match, +#endif .driver = { .name = DRVNAME, .of_match_table = of_match_ptr(arm_trbe_of_match),
Please not that unlike other coresight drivers, TRBE is not using 'acpi_device_id' based "acpi_match_table = ACPI_PTR" construct. But regardless, ACPI_PTR() seems to be an alternate (probably better) solution as well.
--- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -1557,7 +1557,7 @@ MODULE_DEVICE_TABLE(platform, arm_trbe_acpi_match); #endif
static struct platform_driver arm_trbe_driver = { - .id_table = arm_trbe_acpi_match, + .id_table = ACPI_PTR(arm_trbe_acpi_match), .driver = { .name = DRVNAME, .of_match_table = of_match_ptr(arm_trbe_of_match), diff --git a/drivers/hwtracing/coresight/coresight-trbe.h b/drivers/hwtracing/coresight/coresight-trbe.h index 94e67009848a..fce1735d5c58 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.h +++ b/drivers/hwtracing/coresight/coresight-trbe.h @@ -7,6 +7,7 @@ * * Author: Anshuman Khandual anshuman.khandual@arm.com */ +#include <linux/acpi.h> #include <linux/coresight.h> #include <linux/device.h> #include <linux/irq.h>
[1] https://lore.kernel.org/all/202308052123.uqR35d19-lkp@intel.com/
.driver = { .name = DRVNAME, .of_match_table = of_match_ptr(arm_trbe_of_match), diff --git a/drivers/hwtracing/coresight/coresight-trbe.h b/drivers/hwtracing/coresight/coresight-trbe.h index 77cbb5c63878..94e67009848a 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.h +++ b/drivers/hwtracing/coresight/coresight-trbe.h @@ -12,6 +12,7 @@ #include <linux/irq.h> #include <linux/kernel.h> #include <linux/of.h> +#include <linux/perf/arm_pmu.h> #include <linux/platform_device.h> #include <linux/smp.h>
On 07/08/2023 05:43, Anshuman Khandual wrote:
On 8/3/23 11:26, Anshuman Khandual wrote:
This detects and enables ACPI based TRBE devices via the dummy platform device created earlier for this purpose.
Cc: Suzuki K Poulose suzuki.poulose@arm.com Cc: Mike Leach mike.leach@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: coresight@lists.linaro.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com
drivers/hwtracing/coresight/coresight-trbe.c | 9 +++++++++ drivers/hwtracing/coresight/coresight-trbe.h | 1 + 2 files changed, 10 insertions(+)
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index e1d9d06e7725..f884883e9018 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -1537,7 +1537,16 @@ static const struct of_device_id arm_trbe_of_match[] = { }; MODULE_DEVICE_TABLE(of, arm_trbe_of_match); +#ifdef CONFIG_ACPI +static const struct platform_device_id arm_trbe_acpi_match[] = {
- { ARMV8_TRBE_PDEV_NAME, 0 },
- { }
+}; +MODULE_DEVICE_TABLE(platform, arm_trbe_acpi_match); +#endif
- static struct platform_driver arm_trbe_driver = {
- .id_table = arm_trbe_acpi_match,
The build problem [1] reported on the first version of the series still exists here i.e arm_trbe_acpi_match is hidden without CONFIG_ACPI. I had assumed that CONFIG_CORESIGHT always enables CONFIG_ACPI, which is not the case. Following random config (with CONFIG_ACPI=n and CONFIG_CORESIGHT_TRBE=y) easily triggers the build problem.
https://download.01.org/0day-ci/archive/20230805/202308052123.uqR35d19-lkp@i...
make CROSS_COMPILE=aarch64-linux-gnu- ARCH=arm64 -s -j 128 drivers/hwtracing/coresight/coresight-trbe.c:1563:23: error: implicit declaration of function ‘ACPI_PTR’ [-Werror=implicit-function-declaration] 1563 | .acpi_match_table = ACPI_PTR(arm_trbe_acpi_match), | ^~~~~~~~ drivers/hwtracing/coresight/coresight-trbe.c:1563:32: error: ‘arm_trbe_acpi_match’ undeclared here (not in a function); did you mean ‘arm_trbe_of_match’? 1563 | .acpi_match_table = ACPI_PTR(arm_trbe_acpi_match), | ^~~~~~~~~~~~~~~~~~~ | arm_trbe_of_match
Following config wrap around fixes the problem.
--- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -1557,7 +1557,9 @@ MODULE_DEVICE_TABLE(platform, arm_trbe_acpi_match); #endif static struct platform_driver arm_trbe_driver = { +#ifdef CONFIG_ACPI .id_table = arm_trbe_acpi_match, +#endif .driver = { .name = DRVNAME, .of_match_table = of_match_ptr(arm_trbe_of_match),
Please not that unlike other coresight drivers, TRBE is not using 'acpi_device_id' based "acpi_match_table = ACPI_PTR" construct. But regardless, ACPI_PTR() seems to be an alternate (probably better) solution as well.
--- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -1557,7 +1557,7 @@ MODULE_DEVICE_TABLE(platform, arm_trbe_acpi_match); #endif static struct platform_driver arm_trbe_driver = {
.id_table = arm_trbe_acpi_match,
.id_table = ACPI_PTR(arm_trbe_acpi_match),
This is preferred.
.driver = { .name = DRVNAME, .of_match_table = of_match_ptr(arm_trbe_of_match),diff --git a/drivers/hwtracing/coresight/coresight-trbe.h b/drivers/hwtracing/coresight/coresight-trbe.h index 94e67009848a..fce1735d5c58 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.h +++ b/drivers/hwtracing/coresight/coresight-trbe.h @@ -7,6 +7,7 @@
- Author: Anshuman Khandual anshuman.khandual@arm.com
*/ +#include <linux/acpi.h>
Shouldn't this be added in trbe.c ? Does trbe.h depend on any ACPI headers ?
Suzuki
On 8/7/23 17:07, Suzuki K Poulose wrote:
On 07/08/2023 05:43, Anshuman Khandual wrote:
On 8/3/23 11:26, Anshuman Khandual wrote:
This detects and enables ACPI based TRBE devices via the dummy platform device created earlier for this purpose.
Cc: Suzuki K Poulose suzuki.poulose@arm.com Cc: Mike Leach mike.leach@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: coresight@lists.linaro.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Anshuman Khandual anshuman.khandual@arm.com
drivers/hwtracing/coresight/coresight-trbe.c | 9 +++++++++ drivers/hwtracing/coresight/coresight-trbe.h | 1 + 2 files changed, 10 insertions(+)
diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c index e1d9d06e7725..f884883e9018 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -1537,7 +1537,16 @@ static const struct of_device_id arm_trbe_of_match[] = { }; MODULE_DEVICE_TABLE(of, arm_trbe_of_match); +#ifdef CONFIG_ACPI +static const struct platform_device_id arm_trbe_acpi_match[] = { + { ARMV8_TRBE_PDEV_NAME, 0 }, + { } +}; +MODULE_DEVICE_TABLE(platform, arm_trbe_acpi_match); +#endif
static struct platform_driver arm_trbe_driver = { + .id_table = arm_trbe_acpi_match,
The build problem [1] reported on the first version of the series still exists here i.e arm_trbe_acpi_match is hidden without CONFIG_ACPI. I had assumed that CONFIG_CORESIGHT always enables CONFIG_ACPI, which is not the case. Following random config (with CONFIG_ACPI=n and CONFIG_CORESIGHT_TRBE=y) easily triggers the build problem.
https://download.01.org/0day-ci/archive/20230805/202308052123.uqR35d19-lkp@i...
make CROSS_COMPILE=aarch64-linux-gnu- ARCH=arm64 -s -j 128 drivers/hwtracing/coresight/coresight-trbe.c:1563:23: error: implicit declaration of function ‘ACPI_PTR’ [-Werror=implicit-function-declaration] 1563 | .acpi_match_table = ACPI_PTR(arm_trbe_acpi_match), | ^~~~~~~~ drivers/hwtracing/coresight/coresight-trbe.c:1563:32: error: ‘arm_trbe_acpi_match’ undeclared here (not in a function); did you mean ‘arm_trbe_of_match’? 1563 | .acpi_match_table = ACPI_PTR(arm_trbe_acpi_match), | ^~~~~~~~~~~~~~~~~~~ | arm_trbe_of_match
Following config wrap around fixes the problem.
--- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -1557,7 +1557,9 @@ MODULE_DEVICE_TABLE(platform, arm_trbe_acpi_match); #endif static struct platform_driver arm_trbe_driver = { +#ifdef CONFIG_ACPI .id_table = arm_trbe_acpi_match, +#endif .driver = { .name = DRVNAME, .of_match_table = of_match_ptr(arm_trbe_of_match),
Please not that unlike other coresight drivers, TRBE is not using 'acpi_device_id' based "acpi_match_table = ACPI_PTR" construct. But regardless, ACPI_PTR() seems to be an alternate (probably better) solution as well.
--- a/drivers/hwtracing/coresight/coresight-trbe.c +++ b/drivers/hwtracing/coresight/coresight-trbe.c @@ -1557,7 +1557,7 @@ MODULE_DEVICE_TABLE(platform, arm_trbe_acpi_match); #endif static struct platform_driver arm_trbe_driver = { - .id_table = arm_trbe_acpi_match, + .id_table = ACPI_PTR(arm_trbe_acpi_match),
This is preferred.
.driver = { .name = DRVNAME, .of_match_table = of_match_ptr(arm_trbe_of_match), diff --git a/drivers/hwtracing/coresight/coresight-trbe.h b/drivers/hwtracing/coresight/coresight-trbe.h index 94e67009848a..fce1735d5c58 100644 --- a/drivers/hwtracing/coresight/coresight-trbe.h +++ b/drivers/hwtracing/coresight/coresight-trbe.h @@ -7,6 +7,7 @@ * * Author: Anshuman Khandual anshuman.khandual@arm.com */ +#include <linux/acpi.h>
Shouldn't this be added in trbe.c ? Does trbe.h depend on any ACPI headers ?
The convention we have followed till now is all include/linux/ headers required in TRBE driver is included via trbe.h not directly in trbe.c, just followed the same principle this time around for acpi.h and perf/arm_pmu.h as well.
-
Anshuman Khandual -
Suzuki K Poulose