All of lore.kernel.org
 help / color / mirror / Atom feed
From: Robin Murphy <robin.murphy@arm.com>
To: will@kernel.org
Cc: mark.rutland@arm.com, suzuki.poulose@arm.com,
	bwicaksono@nvidia.com, ilkka@os.amperecomputing.com,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH v2 4/4] perf/arm_cspmu: Decouple APMT dependency
Date: Mon,  5 Jun 2023 18:01:34 +0100	[thread overview]
Message-ID: <88f97268603e1aa6016d178982a1dc2861f6770d.1685983270.git.robin.murphy@arm.com> (raw)
In-Reply-To: <cover.1685983270.git.robin.murphy@arm.com>

The functional paths of the driver need not care about ACPI, so abstract
the property of atomic doubleword access as its own flag (repacking the
structure for a better fit). We also do not need to go poking directly
at the APMT for standard resources which the ACPI layer has already
dealt with, so deal with the optional MMIO page and interrupt in the
normal firmware-agnostic manner. The few remaining portions of probing
that *are* APMT-specific can still easily retrieve the APMT pointer as
needed without us having to carry a duplicate copy around everywhere.

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>

---
v2: Fix platdata dereferences, clean up now-unused acpi.h include too.
---
 drivers/perf/arm_cspmu/arm_cspmu.c | 54 ++++++++++--------------------
 drivers/perf/arm_cspmu/arm_cspmu.h |  5 ++-
 2 files changed, 19 insertions(+), 40 deletions(-)

diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
index 3b91115c376d..38e1170af347 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.c
+++ b/drivers/perf/arm_cspmu/arm_cspmu.c
@@ -100,10 +100,6 @@
 #define ARM_CSPMU_ACTIVE_CPU_MASK		0x0
 #define ARM_CSPMU_ASSOCIATED_CPU_MASK		0x1
 
-/* Check if field f in flags is set with value v */
-#define CHECK_APMT_FLAG(flags, f, v) \
-	((flags & (ACPI_APMT_FLAGS_ ## f)) == (ACPI_APMT_FLAGS_ ## f ## _ ## v))
-
 /* Check and use default if implementer doesn't provide attribute callback */
 #define CHECK_DEFAULT_IMPL_OPS(ops, callback)			\
 	do {							\
@@ -121,6 +117,11 @@
 
 static unsigned long arm_cspmu_cpuhp_state;
 
+static struct acpi_apmt_node *arm_cspmu_apmt_node(struct device *dev)
+{
+	return *(struct acpi_apmt_node **)dev_get_platdata(dev);
+}
+
 /*
  * In CoreSight PMU architecture, all of the MMIO registers are 32-bit except
  * counter register. The counter register can be implemented as 32-bit or 64-bit
@@ -155,12 +156,6 @@ static u64 read_reg64_hilohi(const void __iomem *addr, u32 max_poll_count)
 	return val;
 }
 
-/* Check if PMU supports 64-bit single copy atomic. */
-static inline bool supports_64bit_atomics(const struct arm_cspmu *cspmu)
-{
-	return CHECK_APMT_FLAG(cspmu->apmt_node->flags, ATOMIC, SUPP);
-}
-
 /* Check if cycle counter is supported. */
 static inline bool supports_cycle_counter(const struct arm_cspmu *cspmu)
 {
@@ -319,7 +314,7 @@ static const char *arm_cspmu_get_name(const struct arm_cspmu *cspmu)
 	static atomic_t pmu_idx[ACPI_APMT_NODE_TYPE_COUNT] = { 0 };
 
 	dev = cspmu->dev;
-	apmt_node = cspmu->apmt_node;
+	apmt_node = arm_cspmu_apmt_node(dev);
 	pmu_type = apmt_node->type;
 
 	if (pmu_type >= ACPI_APMT_NODE_TYPE_COUNT) {
@@ -396,8 +391,8 @@ static const struct impl_match impl_match[] = {
 static int arm_cspmu_init_impl_ops(struct arm_cspmu *cspmu)
 {
 	int ret;
-	struct acpi_apmt_node *apmt_node = cspmu->apmt_node;
 	struct arm_cspmu_impl_ops *impl_ops = &cspmu->impl.ops;
+	struct acpi_apmt_node *apmt_node = arm_cspmu_apmt_node(cspmu->dev);
 	const struct impl_match *match = impl_match;
 
 	/*
@@ -719,7 +714,7 @@ static u64 arm_cspmu_read_counter(struct perf_event *event)
 		offset = counter_offset(sizeof(u64), event->hw.idx);
 		counter_addr = cspmu->base1 + offset;
 
-		return supports_64bit_atomics(cspmu) ?
+		return cspmu->has_atomic_dword ?
 			       readq(counter_addr) :
 			       read_reg64_hilohi(counter_addr, HILOHI_MAX_POLL);
 	}
@@ -910,24 +905,18 @@ static struct arm_cspmu *arm_cspmu_alloc(struct platform_device *pdev)
 {
 	struct acpi_apmt_node *apmt_node;
 	struct arm_cspmu *cspmu;
-	struct device *dev;
-
-	dev = &pdev->dev;
-	apmt_node = *(struct acpi_apmt_node **)dev_get_platdata(dev);
-	if (!apmt_node) {
-		dev_err(dev, "failed to get APMT node\n");
-		return NULL;
-	}
+	struct device *dev = &pdev->dev;
 
 	cspmu = devm_kzalloc(dev, sizeof(*cspmu), GFP_KERNEL);
 	if (!cspmu)
 		return NULL;
 
 	cspmu->dev = dev;
-	cspmu->apmt_node = apmt_node;
-
 	platform_set_drvdata(pdev, cspmu);
 
+	apmt_node = arm_cspmu_apmt_node(dev);
+	cspmu->has_atomic_dword = apmt_node->flags & ACPI_APMT_FLAGS_ATOMIC;
+
 	return cspmu;
 }
 
@@ -935,11 +924,9 @@ static int arm_cspmu_init_mmio(struct arm_cspmu *cspmu)
 {
 	struct device *dev;
 	struct platform_device *pdev;
-	struct acpi_apmt_node *apmt_node;
 
 	dev = cspmu->dev;
 	pdev = to_platform_device(dev);
-	apmt_node = cspmu->apmt_node;
 
 	/* Base address for page 0. */
 	cspmu->base0 = devm_platform_ioremap_resource(pdev, 0);
@@ -950,7 +937,7 @@ static int arm_cspmu_init_mmio(struct arm_cspmu *cspmu)
 
 	/* Base address for page 1 if supported. Otherwise point to page 0. */
 	cspmu->base1 = cspmu->base0;
-	if (CHECK_APMT_FLAG(apmt_node->flags, DUAL_PAGE, SUPP)) {
+	if (platform_get_resource(pdev, IORESOURCE_MEM, 1)) {
 		cspmu->base1 = devm_platform_ioremap_resource(pdev, 1);
 		if (IS_ERR(cspmu->base1)) {
 			dev_err(dev, "ioremap failed for page-1 resource\n");
@@ -1047,19 +1034,14 @@ static int arm_cspmu_request_irq(struct arm_cspmu *cspmu)
 	int irq, ret;
 	struct device *dev;
 	struct platform_device *pdev;
-	struct acpi_apmt_node *apmt_node;
 
 	dev = cspmu->dev;
 	pdev = to_platform_device(dev);
-	apmt_node = cspmu->apmt_node;
 
 	/* Skip IRQ request if the PMU does not support overflow interrupt. */
-	if (apmt_node->ovflw_irq == 0)
-		return 0;
-
-	irq = platform_get_irq(pdev, 0);
+	irq = platform_get_irq_optional(pdev, 0);
 	if (irq < 0)
-		return irq;
+		return irq == -ENXIO ? 0 : irq;
 
 	ret = devm_request_irq(dev, irq, arm_cspmu_handle_irq,
 			       IRQF_NOBALANCING | IRQF_NO_THREAD, dev_name(dev),
@@ -1103,13 +1085,11 @@ static inline int arm_cspmu_find_cpu_container(int cpu, u32 container_uid)
 
 static int arm_cspmu_acpi_get_cpus(struct arm_cspmu *cspmu)
 {
-	struct device *dev;
 	struct acpi_apmt_node *apmt_node;
 	int affinity_flag;
 	int cpu;
 
-	dev = cspmu->pmu.dev;
-	apmt_node = cspmu->apmt_node;
+	apmt_node = arm_cspmu_apmt_node(cspmu->dev);
 	affinity_flag = apmt_node->flags & ACPI_APMT_FLAGS_AFFINITY;
 
 	if (affinity_flag == ACPI_APMT_FLAGS_AFFINITY_PROC) {
@@ -1131,7 +1111,7 @@ static int arm_cspmu_acpi_get_cpus(struct arm_cspmu *cspmu)
 	}
 
 	if (cpumask_empty(&cspmu->associated_cpus)) {
-		dev_dbg(dev, "No cpu associated with the PMU\n");
+		dev_dbg(cspmu->dev, "No cpu associated with the PMU\n");
 		return -ENODEV;
 	}
 
diff --git a/drivers/perf/arm_cspmu/arm_cspmu.h b/drivers/perf/arm_cspmu/arm_cspmu.h
index 51323b175a4a..83df53d1c132 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.h
+++ b/drivers/perf/arm_cspmu/arm_cspmu.h
@@ -8,7 +8,6 @@
 #ifndef __ARM_CSPMU_H__
 #define __ARM_CSPMU_H__
 
-#include <linux/acpi.h>
 #include <linux/bitfield.h>
 #include <linux/cpumask.h>
 #include <linux/device.h>
@@ -118,16 +117,16 @@ struct arm_cspmu_impl {
 struct arm_cspmu {
 	struct pmu pmu;
 	struct device *dev;
-	struct acpi_apmt_node *apmt_node;
 	const char *name;
 	const char *identifier;
 	void __iomem *base0;
 	void __iomem *base1;
-	int irq;
 	cpumask_t associated_cpus;
 	cpumask_t active_cpu;
 	struct hlist_node cpuhp_node;
+	int irq;
 
+	bool has_atomic_dword;
 	u32 pmcfgr;
 	u32 num_logical_ctrs;
 	u32 num_set_clr_reg;
-- 
2.39.2.101.g768bb238c484.dirty


WARNING: multiple messages have this Message-ID (diff)
From: Robin Murphy <robin.murphy@arm.com>
To: will@kernel.org
Cc: mark.rutland@arm.com, suzuki.poulose@arm.com,
	bwicaksono@nvidia.com, ilkka@os.amperecomputing.com,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH v2 4/4] perf/arm_cspmu: Decouple APMT dependency
Date: Mon,  5 Jun 2023 18:01:34 +0100	[thread overview]
Message-ID: <88f97268603e1aa6016d178982a1dc2861f6770d.1685983270.git.robin.murphy@arm.com> (raw)
In-Reply-To: <cover.1685983270.git.robin.murphy@arm.com>

The functional paths of the driver need not care about ACPI, so abstract
the property of atomic doubleword access as its own flag (repacking the
structure for a better fit). We also do not need to go poking directly
at the APMT for standard resources which the ACPI layer has already
dealt with, so deal with the optional MMIO page and interrupt in the
normal firmware-agnostic manner. The few remaining portions of probing
that *are* APMT-specific can still easily retrieve the APMT pointer as
needed without us having to carry a duplicate copy around everywhere.

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>

---
v2: Fix platdata dereferences, clean up now-unused acpi.h include too.
---
 drivers/perf/arm_cspmu/arm_cspmu.c | 54 ++++++++++--------------------
 drivers/perf/arm_cspmu/arm_cspmu.h |  5 ++-
 2 files changed, 19 insertions(+), 40 deletions(-)

diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
index 3b91115c376d..38e1170af347 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.c
+++ b/drivers/perf/arm_cspmu/arm_cspmu.c
@@ -100,10 +100,6 @@
 #define ARM_CSPMU_ACTIVE_CPU_MASK		0x0
 #define ARM_CSPMU_ASSOCIATED_CPU_MASK		0x1
 
-/* Check if field f in flags is set with value v */
-#define CHECK_APMT_FLAG(flags, f, v) \
-	((flags & (ACPI_APMT_FLAGS_ ## f)) == (ACPI_APMT_FLAGS_ ## f ## _ ## v))
-
 /* Check and use default if implementer doesn't provide attribute callback */
 #define CHECK_DEFAULT_IMPL_OPS(ops, callback)			\
 	do {							\
@@ -121,6 +117,11 @@
 
 static unsigned long arm_cspmu_cpuhp_state;
 
+static struct acpi_apmt_node *arm_cspmu_apmt_node(struct device *dev)
+{
+	return *(struct acpi_apmt_node **)dev_get_platdata(dev);
+}
+
 /*
  * In CoreSight PMU architecture, all of the MMIO registers are 32-bit except
  * counter register. The counter register can be implemented as 32-bit or 64-bit
@@ -155,12 +156,6 @@ static u64 read_reg64_hilohi(const void __iomem *addr, u32 max_poll_count)
 	return val;
 }
 
-/* Check if PMU supports 64-bit single copy atomic. */
-static inline bool supports_64bit_atomics(const struct arm_cspmu *cspmu)
-{
-	return CHECK_APMT_FLAG(cspmu->apmt_node->flags, ATOMIC, SUPP);
-}
-
 /* Check if cycle counter is supported. */
 static inline bool supports_cycle_counter(const struct arm_cspmu *cspmu)
 {
@@ -319,7 +314,7 @@ static const char *arm_cspmu_get_name(const struct arm_cspmu *cspmu)
 	static atomic_t pmu_idx[ACPI_APMT_NODE_TYPE_COUNT] = { 0 };
 
 	dev = cspmu->dev;
-	apmt_node = cspmu->apmt_node;
+	apmt_node = arm_cspmu_apmt_node(dev);
 	pmu_type = apmt_node->type;
 
 	if (pmu_type >= ACPI_APMT_NODE_TYPE_COUNT) {
@@ -396,8 +391,8 @@ static const struct impl_match impl_match[] = {
 static int arm_cspmu_init_impl_ops(struct arm_cspmu *cspmu)
 {
 	int ret;
-	struct acpi_apmt_node *apmt_node = cspmu->apmt_node;
 	struct arm_cspmu_impl_ops *impl_ops = &cspmu->impl.ops;
+	struct acpi_apmt_node *apmt_node = arm_cspmu_apmt_node(cspmu->dev);
 	const struct impl_match *match = impl_match;
 
 	/*
@@ -719,7 +714,7 @@ static u64 arm_cspmu_read_counter(struct perf_event *event)
 		offset = counter_offset(sizeof(u64), event->hw.idx);
 		counter_addr = cspmu->base1 + offset;
 
-		return supports_64bit_atomics(cspmu) ?
+		return cspmu->has_atomic_dword ?
 			       readq(counter_addr) :
 			       read_reg64_hilohi(counter_addr, HILOHI_MAX_POLL);
 	}
@@ -910,24 +905,18 @@ static struct arm_cspmu *arm_cspmu_alloc(struct platform_device *pdev)
 {
 	struct acpi_apmt_node *apmt_node;
 	struct arm_cspmu *cspmu;
-	struct device *dev;
-
-	dev = &pdev->dev;
-	apmt_node = *(struct acpi_apmt_node **)dev_get_platdata(dev);
-	if (!apmt_node) {
-		dev_err(dev, "failed to get APMT node\n");
-		return NULL;
-	}
+	struct device *dev = &pdev->dev;
 
 	cspmu = devm_kzalloc(dev, sizeof(*cspmu), GFP_KERNEL);
 	if (!cspmu)
 		return NULL;
 
 	cspmu->dev = dev;
-	cspmu->apmt_node = apmt_node;
-
 	platform_set_drvdata(pdev, cspmu);
 
+	apmt_node = arm_cspmu_apmt_node(dev);
+	cspmu->has_atomic_dword = apmt_node->flags & ACPI_APMT_FLAGS_ATOMIC;
+
 	return cspmu;
 }
 
@@ -935,11 +924,9 @@ static int arm_cspmu_init_mmio(struct arm_cspmu *cspmu)
 {
 	struct device *dev;
 	struct platform_device *pdev;
-	struct acpi_apmt_node *apmt_node;
 
 	dev = cspmu->dev;
 	pdev = to_platform_device(dev);
-	apmt_node = cspmu->apmt_node;
 
 	/* Base address for page 0. */
 	cspmu->base0 = devm_platform_ioremap_resource(pdev, 0);
@@ -950,7 +937,7 @@ static int arm_cspmu_init_mmio(struct arm_cspmu *cspmu)
 
 	/* Base address for page 1 if supported. Otherwise point to page 0. */
 	cspmu->base1 = cspmu->base0;
-	if (CHECK_APMT_FLAG(apmt_node->flags, DUAL_PAGE, SUPP)) {
+	if (platform_get_resource(pdev, IORESOURCE_MEM, 1)) {
 		cspmu->base1 = devm_platform_ioremap_resource(pdev, 1);
 		if (IS_ERR(cspmu->base1)) {
 			dev_err(dev, "ioremap failed for page-1 resource\n");
@@ -1047,19 +1034,14 @@ static int arm_cspmu_request_irq(struct arm_cspmu *cspmu)
 	int irq, ret;
 	struct device *dev;
 	struct platform_device *pdev;
-	struct acpi_apmt_node *apmt_node;
 
 	dev = cspmu->dev;
 	pdev = to_platform_device(dev);
-	apmt_node = cspmu->apmt_node;
 
 	/* Skip IRQ request if the PMU does not support overflow interrupt. */
-	if (apmt_node->ovflw_irq == 0)
-		return 0;
-
-	irq = platform_get_irq(pdev, 0);
+	irq = platform_get_irq_optional(pdev, 0);
 	if (irq < 0)
-		return irq;
+		return irq == -ENXIO ? 0 : irq;
 
 	ret = devm_request_irq(dev, irq, arm_cspmu_handle_irq,
 			       IRQF_NOBALANCING | IRQF_NO_THREAD, dev_name(dev),
@@ -1103,13 +1085,11 @@ static inline int arm_cspmu_find_cpu_container(int cpu, u32 container_uid)
 
 static int arm_cspmu_acpi_get_cpus(struct arm_cspmu *cspmu)
 {
-	struct device *dev;
 	struct acpi_apmt_node *apmt_node;
 	int affinity_flag;
 	int cpu;
 
-	dev = cspmu->pmu.dev;
-	apmt_node = cspmu->apmt_node;
+	apmt_node = arm_cspmu_apmt_node(cspmu->dev);
 	affinity_flag = apmt_node->flags & ACPI_APMT_FLAGS_AFFINITY;
 
 	if (affinity_flag == ACPI_APMT_FLAGS_AFFINITY_PROC) {
@@ -1131,7 +1111,7 @@ static int arm_cspmu_acpi_get_cpus(struct arm_cspmu *cspmu)
 	}
 
 	if (cpumask_empty(&cspmu->associated_cpus)) {
-		dev_dbg(dev, "No cpu associated with the PMU\n");
+		dev_dbg(cspmu->dev, "No cpu associated with the PMU\n");
 		return -ENODEV;
 	}
 
diff --git a/drivers/perf/arm_cspmu/arm_cspmu.h b/drivers/perf/arm_cspmu/arm_cspmu.h
index 51323b175a4a..83df53d1c132 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.h
+++ b/drivers/perf/arm_cspmu/arm_cspmu.h
@@ -8,7 +8,6 @@
 #ifndef __ARM_CSPMU_H__
 #define __ARM_CSPMU_H__
 
-#include <linux/acpi.h>
 #include <linux/bitfield.h>
 #include <linux/cpumask.h>
 #include <linux/device.h>
@@ -118,16 +117,16 @@ struct arm_cspmu_impl {
 struct arm_cspmu {
 	struct pmu pmu;
 	struct device *dev;
-	struct acpi_apmt_node *apmt_node;
 	const char *name;
 	const char *identifier;
 	void __iomem *base0;
 	void __iomem *base1;
-	int irq;
 	cpumask_t associated_cpus;
 	cpumask_t active_cpu;
 	struct hlist_node cpuhp_node;
+	int irq;
 
+	bool has_atomic_dword;
 	u32 pmcfgr;
 	u32 num_logical_ctrs;
 	u32 num_set_clr_reg;
-- 
2.39.2.101.g768bb238c484.dirty


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

  parent reply	other threads:[~2023-06-05 17:02 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-05 17:01 [PATCH v2 0/4] perf/arm_cspmu: Fixes and cleanups Robin Murphy
2023-06-05 17:01 ` Robin Murphy
2023-06-05 17:01 ` [PATCH v2 1/4] perf/arm_cspmu: Fix event attribute type Robin Murphy
2023-06-05 17:01   ` Robin Murphy
2023-06-05 17:01 ` [PATCH v2 2/4] ACPI/APMT: Don't register invalid resource Robin Murphy
2023-06-05 17:01   ` Robin Murphy
2023-06-07 14:21   ` Lorenzo Pieralisi
2023-06-07 14:21     ` Lorenzo Pieralisi
2023-06-05 17:01 ` [PATCH v2 3/4] perf/arm_cspmu: Clean up ACPI dependency Robin Murphy
2023-06-05 17:01   ` Robin Murphy
2023-07-03  9:21   ` Geert Uytterhoeven
2023-07-03  9:21     ` Geert Uytterhoeven
2023-07-03 10:56     ` Robin Murphy
2023-07-03 10:56       ` Robin Murphy
2023-06-05 17:01 ` Robin Murphy [this message]
2023-06-05 17:01   ` [PATCH v2 4/4] perf/arm_cspmu: Decouple APMT dependency Robin Murphy
2023-06-05 20:29   ` Ilkka Koskinen
2023-06-05 20:29     ` Ilkka Koskinen
2023-06-09 11:16 ` [PATCH v2 0/4] perf/arm_cspmu: Fixes and cleanups Will Deacon
2023-06-09 11:16   ` Will Deacon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=88f97268603e1aa6016d178982a1dc2861f6770d.1685983270.git.robin.murphy@arm.com \
    --to=robin.murphy@arm.com \
    --cc=bwicaksono@nvidia.com \
    --cc=ilkka@os.amperecomputing.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=suzuki.poulose@arm.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.