[PATCH V3 0/4] coresight: trbe: Enable ACPI based devices

[PATCH V3 0/4] coresight: trbe: Enable ACPI based devices

[Anshuman Khandual]

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.com/

- 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.com/

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(-)

-- 
2.25.1

[PATCH V3 0/4] coresight: trbe: Enable ACPI based devices

[Anshuman Khandual]

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.com/

- 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.com/

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(-)

-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V3 1/4] arm_pmu: acpi: Refactor arm_spe_acpi_register_device()

[Anshuman Khandual]

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)
-- 
2.25.1

[PATCH V3 1/4] arm_pmu: acpi: Refactor arm_spe_acpi_register_device()

[Anshuman Khandual]

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)
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V3 2/4] arm_pmu: acpi: Add a representative platform device for TRBE

[Anshuman Khandual]

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__ */
-- 
2.25.1

[PATCH V3 2/4] arm_pmu: acpi: Add a representative platform device for TRBE

[Anshuman Khandual]

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__ */
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V3 3/4] coresight: trbe: Add a representative coresight_platform_data for TRBE

[Anshuman Khandual]

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;
-- 
2.25.1

[PATCH V3 3/4] coresight: trbe: Add a representative coresight_platform_data for TRBE

[Anshuman Khandual]

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;
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V3 4/4] coresight: trbe: Enable ACPI based TRBE devices

[Anshuman Khandual]

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>
 
-- 
2.25.1

[PATCH V3 4/4] coresight: trbe: Enable ACPI based TRBE devices

[Anshuman Khandual]

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>
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V3 1/4] arm_pmu: acpi: Refactor arm_spe_acpi_register_device()

[Anshuman Khandual]

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

Re: [PATCH V3 1/4] arm_pmu: acpi: Refactor arm_spe_acpi_register_device()

[Anshuman Khandual]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V3 2/4] arm_pmu: acpi: Add a representative platform device for TRBE

[Yicong Yang]

On 2023/8/3 13:56, 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)
>  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

Maybe we should use #if IS_ENABLED(CONFIG_CORESIGHT_TRBE) here and other places?

As trbe can be configured as a module, when CONFIG_CORESIGHT_TRBE=m this block
won't be compiled. Referred to

https://github.com/torvalds/linux/blob/c7c90e121e992eefdf07945e5a6e9cf097b29463/include/linux/kconfig.h#L68

> +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__ */
> 

Re: [PATCH V3 2/4] arm_pmu: acpi: Add a representative platform device for TRBE

[Yicong Yang]

On 2023/8/3 13:56, 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)
>  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

Maybe we should use #if IS_ENABLED(CONFIG_CORESIGHT_TRBE) here and other places?

As trbe can be configured as a module, when CONFIG_CORESIGHT_TRBE=m this block
won't be compiled. Referred to

https://github.com/torvalds/linux/blob/c7c90e121e992eefdf07945e5a6e9cf097b29463/include/linux/kconfig.h#L68

> +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__ */
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V3 3/4] coresight: trbe: Add a representative coresight_platform_data for TRBE

[Suzuki K Poulose]

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;

Re: [PATCH V3 3/4] coresight: trbe: Add a representative coresight_platform_data for TRBE

[Suzuki K Poulose]

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;


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V3 3/4] coresight: trbe: Add a representative coresight_platform_data for TRBE

[Anshuman Khandual]

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.
+        */

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V3 3/4] coresight: trbe: Add a representative coresight_platform_data for TRBE

[Anshuman Khandual]

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.
+        */

Re: [PATCH V3 2/4] arm_pmu: acpi: Add a representative platform device for TRBE

[Anshuman Khandual]

On 8/3/23 14:44, Yicong Yang wrote:
> On 2023/8/3 13:56, 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)
>>  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
> Maybe we should use #if IS_ENABLED(CONFIG_CORESIGHT_TRBE) here and other places?
> 
> As trbe can be configured as a module, when CONFIG_CORESIGHT_TRBE=m this block
> won't be compiled. Referred to
> 
> https://github.com/torvalds/linux/blob/c7c90e121e992eefdf07945e5a6e9cf097b29463/include/linux/kconfig.h#L68

You are right, just making CONFIG_CORESIGHT_TRBE=m make it fall back on the
stub arm_trbe_acpi_register_device() definition, which does not create the
required dummy platform device thus preventing TRBE probe on the platform.

Will change the above #ifdef as IS_ENABLED() for CONFIG_CORESIGHT_TRBE and
revert back using IS_ENABLED() in the previous patch for CONFIG_ARM_SPE_PMU
as well. Thanks for noticing this problem.

Re: [PATCH V3 2/4] arm_pmu: acpi: Add a representative platform device for TRBE

[Anshuman Khandual]

On 8/3/23 14:44, Yicong Yang wrote:
> On 2023/8/3 13:56, 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)
>>  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
> Maybe we should use #if IS_ENABLED(CONFIG_CORESIGHT_TRBE) here and other places?
> 
> As trbe can be configured as a module, when CONFIG_CORESIGHT_TRBE=m this block
> won't be compiled. Referred to
> 
> https://github.com/torvalds/linux/blob/c7c90e121e992eefdf07945e5a6e9cf097b29463/include/linux/kconfig.h#L68

You are right, just making CONFIG_CORESIGHT_TRBE=m make it fall back on the
stub arm_trbe_acpi_register_device() definition, which does not create the
required dummy platform device thus preventing TRBE probe on the platform.

Will change the above #ifdef as IS_ENABLED() for CONFIG_CORESIGHT_TRBE and
revert back using IS_ENABLED() in the previous patch for CONFIG_ARM_SPE_PMU
as well. Thanks for noticing this problem.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V3 2/4] arm_pmu: acpi: Add a representative platform device for TRBE

[Anshuman Khandual]

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__ */

Re: [PATCH V3 2/4] arm_pmu: acpi: Add a representative platform device for TRBE

[Anshuman Khandual]

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__ */

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V3 3/4] coresight: trbe: Add a representative coresight_platform_data for TRBE

[Suzuki K Poulose]

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

Re: [PATCH V3 3/4] coresight: trbe: Add a representative coresight_platform_data for TRBE

[Suzuki K Poulose]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V3 1/4] arm_pmu: acpi: Refactor arm_spe_acpi_register_device()

[Will Deacon]

On Thu, Aug 03, 2023 at 11:43:27AM +0530, Anshuman Khandual wrote:
> 
> 
> 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.

Wouldn't that still be the case without the check in this hunk? We'd run
into the homogeneous check and return -ENXIO from there, no?

Will

Re: [PATCH V3 1/4] arm_pmu: acpi: Refactor arm_spe_acpi_register_device()

[Will Deacon]

On Thu, Aug 03, 2023 at 11:43:27AM +0530, Anshuman Khandual wrote:
> 
> 
> 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.

Wouldn't that still be the case without the check in this hunk? We'd run
into the homogeneous check and return -ENXIO from there, no?

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V3 4/4] coresight: trbe: Enable ACPI based TRBE devices

[Anshuman Khandual]

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@intel.com/config

 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>
>  

Re: [PATCH V3 4/4] coresight: trbe: Enable ACPI based TRBE devices

[Anshuman Khandual]

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@intel.com/config

 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>
>  

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V3 1/4] arm_pmu: acpi: Refactor arm_spe_acpi_register_device()

[Anshuman Khandual]

On 8/4/23 22:09, Will Deacon wrote:
> On Thu, Aug 03, 2023 at 11:43:27AM +0530, Anshuman Khandual wrote:
>>
>>
>> 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.
> 
> Wouldn't that still be the case without the check in this hunk? We'd run
> into the homogeneous check and return -ENXIO from there, no?
Although the return code will be the same i.e -ENXIO, but not for the same reason.

		this_gsi = parse_gsi(gicc);
		if (!this_gsi)
			return gsi ? -ENXIO : 0;

This returns 0 when IRQ could not be parsed for the first cpu, but returns -ENXIO
for subsequent cpus. Although return code -ENXIO here still indicates IRQ parsing
to have failed.

		} else if (hetid != this_hetid || gsi != this_gsi) {
			pr_warn("ACPI: %s: must be homogeneous\n", pdev->name);
			return -ENXIO;
		} 

This returns -ENXIO when there is a IRQ mismatch. But if the above check is not
there, -ENXIO return code here could not be classified into IRQ parse problem or
mismatch without looking into the IRQ value.

Re: [PATCH V3 1/4] arm_pmu: acpi: Refactor arm_spe_acpi_register_device()

[Anshuman Khandual]

On 8/4/23 22:09, Will Deacon wrote:
> On Thu, Aug 03, 2023 at 11:43:27AM +0530, Anshuman Khandual wrote:
>>
>>
>> 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.
> 
> Wouldn't that still be the case without the check in this hunk? We'd run
> into the homogeneous check and return -ENXIO from there, no?
Although the return code will be the same i.e -ENXIO, but not for the same reason.

		this_gsi = parse_gsi(gicc);
		if (!this_gsi)
			return gsi ? -ENXIO : 0;

This returns 0 when IRQ could not be parsed for the first cpu, but returns -ENXIO
for subsequent cpus. Although return code -ENXIO here still indicates IRQ parsing
to have failed.

		} else if (hetid != this_hetid || gsi != this_gsi) {
			pr_warn("ACPI: %s: must be homogeneous\n", pdev->name);
			return -ENXIO;
		} 

This returns -ENXIO when there is a IRQ mismatch. But if the above check is not
there, -ENXIO return code here could not be classified into IRQ parse problem or
mismatch without looking into the IRQ value.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V3 4/4] coresight: trbe: Enable ACPI based TRBE devices

[Suzuki K Poulose]

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@intel.com/config
> 
>   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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V3 4/4] coresight: trbe: Enable ACPI based TRBE devices

[Suzuki K Poulose]

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@intel.com/config
> 
>   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

Re: [PATCH V3 4/4] coresight: trbe: Enable ACPI based TRBE devices

[Anshuman Khandual]

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@intel.com/config
>>
>>   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.

Re: [PATCH V3 4/4] coresight: trbe: Enable ACPI based TRBE devices

[Anshuman Khandual]

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@intel.com/config
>>
>>   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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V3 1/4] arm_pmu: acpi: Refactor arm_spe_acpi_register_device()

[Will Deacon]

On Mon, Aug 07, 2023 at 11:03:40AM +0530, Anshuman Khandual wrote:
> On 8/4/23 22:09, Will Deacon wrote:
> > On Thu, Aug 03, 2023 at 11:43:27AM +0530, Anshuman Khandual wrote:
> >> 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;
> >>
> >> 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.
> > 
> > Wouldn't that still be the case without the check in this hunk? We'd run
> > into the homogeneous check and return -ENXIO from there, no?
> Although the return code will be the same i.e -ENXIO, but not for the same reason.
> 
> 		this_gsi = parse_gsi(gicc);
> 		if (!this_gsi)
> 			return gsi ? -ENXIO : 0;
> 
> This returns 0 when IRQ could not be parsed for the first cpu, but returns -ENXIO
> for subsequent cpus. Although return code -ENXIO here still indicates IRQ parsing
> to have failed.
> 
> 		} else if (hetid != this_hetid || gsi != this_gsi) {
> 			pr_warn("ACPI: %s: must be homogeneous\n", pdev->name);
> 			return -ENXIO;
> 		} 
> 
> This returns -ENXIO when there is a IRQ mismatch. But if the above check is not
> there, -ENXIO return code here could not be classified into IRQ parse problem or
> mismatch without looking into the IRQ value.

Sorry, but I don't understand your point here. If any of this fails, there's
going to be some debugging needed to look at the ACPI tables; the only
difference with my suggestion is that you'll get a message indicating that
the devices aren't homogeneous, which I think is helpful.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V3 1/4] arm_pmu: acpi: Refactor arm_spe_acpi_register_device()

[Will Deacon]

On Mon, Aug 07, 2023 at 11:03:40AM +0530, Anshuman Khandual wrote:
> On 8/4/23 22:09, Will Deacon wrote:
> > On Thu, Aug 03, 2023 at 11:43:27AM +0530, Anshuman Khandual wrote:
> >> 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;
> >>
> >> 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.
> > 
> > Wouldn't that still be the case without the check in this hunk? We'd run
> > into the homogeneous check and return -ENXIO from there, no?
> Although the return code will be the same i.e -ENXIO, but not for the same reason.
> 
> 		this_gsi = parse_gsi(gicc);
> 		if (!this_gsi)
> 			return gsi ? -ENXIO : 0;
> 
> This returns 0 when IRQ could not be parsed for the first cpu, but returns -ENXIO
> for subsequent cpus. Although return code -ENXIO here still indicates IRQ parsing
> to have failed.
> 
> 		} else if (hetid != this_hetid || gsi != this_gsi) {
> 			pr_warn("ACPI: %s: must be homogeneous\n", pdev->name);
> 			return -ENXIO;
> 		} 
> 
> This returns -ENXIO when there is a IRQ mismatch. But if the above check is not
> there, -ENXIO return code here could not be classified into IRQ parse problem or
> mismatch without looking into the IRQ value.

Sorry, but I don't understand your point here. If any of this fails, there's
going to be some debugging needed to look at the ACPI tables; the only
difference with my suggestion is that you'll get a message indicating that
the devices aren't homogeneous, which I think is helpful.

Will

Re: [PATCH V3 1/4] arm_pmu: acpi: Refactor arm_spe_acpi_register_device()

[Anshuman Khandual]

On 8/8/23 18:51, Will Deacon wrote:
> On Mon, Aug 07, 2023 at 11:03:40AM +0530, Anshuman Khandual wrote:
>> On 8/4/23 22:09, Will Deacon wrote:
>>> On Thu, Aug 03, 2023 at 11:43:27AM +0530, Anshuman Khandual wrote:
>>>> 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;
>>>>
>>>> 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.
>>>
>>> Wouldn't that still be the case without the check in this hunk? We'd run
>>> into the homogeneous check and return -ENXIO from there, no?
>> Although the return code will be the same i.e -ENXIO, but not for the same reason.
>>
>> 		this_gsi = parse_gsi(gicc);
>> 		if (!this_gsi)
>> 			return gsi ? -ENXIO : 0;
>>
>> This returns 0 when IRQ could not be parsed for the first cpu, but returns -ENXIO
>> for subsequent cpus. Although return code -ENXIO here still indicates IRQ parsing
>> to have failed.
>>
>> 		} else if (hetid != this_hetid || gsi != this_gsi) {
>> 			pr_warn("ACPI: %s: must be homogeneous\n", pdev->name);
>> 			return -ENXIO;
>> 		} 
>>
>> This returns -ENXIO when there is a IRQ mismatch. But if the above check is not
>> there, -ENXIO return code here could not be classified into IRQ parse problem or
>> mismatch without looking into the IRQ value.
> 
> Sorry, but I don't understand your point here. If any of this fails, there's
> going to be some debugging needed to look at the ACPI tables; the only
> difference with my suggestion is that you'll get a message indicating that
> the devices aren't homogeneous, which I think is helpful.

I dont have strong opinion either way. Hence will move 'this_gsi' check inside the
!gsi conditional check like you had suggested earlier.

Re: [PATCH V3 1/4] arm_pmu: acpi: Refactor arm_spe_acpi_register_device()

[Anshuman Khandual]

On 8/8/23 18:51, Will Deacon wrote:
> On Mon, Aug 07, 2023 at 11:03:40AM +0530, Anshuman Khandual wrote:
>> On 8/4/23 22:09, Will Deacon wrote:
>>> On Thu, Aug 03, 2023 at 11:43:27AM +0530, Anshuman Khandual wrote:
>>>> 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;
>>>>
>>>> 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.
>>>
>>> Wouldn't that still be the case without the check in this hunk? We'd run
>>> into the homogeneous check and return -ENXIO from there, no?
>> Although the return code will be the same i.e -ENXIO, but not for the same reason.
>>
>> 		this_gsi = parse_gsi(gicc);
>> 		if (!this_gsi)
>> 			return gsi ? -ENXIO : 0;
>>
>> This returns 0 when IRQ could not be parsed for the first cpu, but returns -ENXIO
>> for subsequent cpus. Although return code -ENXIO here still indicates IRQ parsing
>> to have failed.
>>
>> 		} else if (hetid != this_hetid || gsi != this_gsi) {
>> 			pr_warn("ACPI: %s: must be homogeneous\n", pdev->name);
>> 			return -ENXIO;
>> 		} 
>>
>> This returns -ENXIO when there is a IRQ mismatch. But if the above check is not
>> there, -ENXIO return code here could not be classified into IRQ parse problem or
>> mismatch without looking into the IRQ value.
> 
> Sorry, but I don't understand your point here. If any of this fails, there's
> going to be some debugging needed to look at the ACPI tables; the only
> difference with my suggestion is that you'll get a message indicating that
> the devices aren't homogeneous, which I think is helpful.

I dont have strong opinion either way. Hence will move 'this_gsi' check inside the
!gsi conditional check like you had suggested earlier.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V3 4/4] coresight: trbe: Enable ACPI based TRBE devices

[kernel test robot]

Hi Anshuman,

kernel test robot noticed the following build errors:

[auto build test ERROR on arm64/for-next/core]
[also build test ERROR on arm/for-next soc/for-next linus/master arm/fixes v6.5-rc5 next-20230809]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Anshuman-Khandual/arm_pmu-acpi-Refactor-arm_spe_acpi_register_device/20230803-135907
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
patch link:    https://lore.kernel.org/r/20230803055652.1322801-5-anshuman.khandual%40arm.com
patch subject: [PATCH V3 4/4] coresight: trbe: Enable ACPI based TRBE devices
config: arm64-randconfig-r071-20230813 (https://download.01.org/0day-ci/archive/20230814/202308140529.wEpy3fPK-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230814/202308140529.wEpy3fPK-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308140529.wEpy3fPK-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/hwtracing/coresight/coresight-trbe.c:1549:21: error: 'arm_trbe_acpi_match' undeclared here (not in a function); did you mean 'arm_trbe_of_match'?
    1549 |         .id_table = arm_trbe_acpi_match,
         |                     ^~~~~~~~~~~~~~~~~~~
         |                     arm_trbe_of_match


vim +1549 drivers/hwtracing/coresight/coresight-trbe.c

  1547	
  1548	static struct platform_driver arm_trbe_driver = {
> 1549		.id_table = arm_trbe_acpi_match,
  1550		.driver	= {
  1551			.name = DRVNAME,
  1552			.of_match_table = of_match_ptr(arm_trbe_of_match),
  1553			.suppress_bind_attrs = true,
  1554		},
  1555		.probe	= arm_trbe_device_probe,
  1556		.remove	= arm_trbe_device_remove,
  1557	};
  1558	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH V3 4/4] coresight: trbe: Enable ACPI based TRBE devices

[kernel test robot]

Hi Anshuman,

kernel test robot noticed the following build errors:

[auto build test ERROR on arm64/for-next/core]
[also build test ERROR on arm/for-next soc/for-next linus/master arm/fixes v6.5-rc5 next-20230809]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Anshuman-Khandual/arm_pmu-acpi-Refactor-arm_spe_acpi_register_device/20230803-135907
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
patch link:    https://lore.kernel.org/r/20230803055652.1322801-5-anshuman.khandual%40arm.com
patch subject: [PATCH V3 4/4] coresight: trbe: Enable ACPI based TRBE devices
config: arm64-randconfig-r071-20230813 (https://download.01.org/0day-ci/archive/20230814/202308140529.wEpy3fPK-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230814/202308140529.wEpy3fPK-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308140529.wEpy3fPK-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/hwtracing/coresight/coresight-trbe.c:1549:21: error: 'arm_trbe_acpi_match' undeclared here (not in a function); did you mean 'arm_trbe_of_match'?
    1549 |         .id_table = arm_trbe_acpi_match,
         |                     ^~~~~~~~~~~~~~~~~~~
         |                     arm_trbe_of_match


vim +1549 drivers/hwtracing/coresight/coresight-trbe.c

  1547	
  1548	static struct platform_driver arm_trbe_driver = {
> 1549		.id_table = arm_trbe_acpi_match,
  1550		.driver	= {
  1551			.name = DRVNAME,
  1552			.of_match_table = of_match_ptr(arm_trbe_of_match),
  1553			.suppress_bind_attrs = true,
  1554		},
  1555		.probe	= arm_trbe_device_probe,
  1556		.remove	= arm_trbe_device_remove,
  1557	};
  1558	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel