[PATCH v3 0/5] perf/arm_cspmu: Add devicetree support

[PATCH v3 0/5] perf/arm_cspmu: Add devicetree support

[Robin Murphy]

v2: https://lore.kernel.org/linux-arm-kernel/cover.1702571292.git.robin.murphy@arm.com/

Hi all,

Third time lucky... here's a hopefully-final spin to address the
functional issues Besar pointed out, and give a little final polish to
the binding (although I stand firm on teh point of a trivial example
not being valuable :))

Cheers,
Robin.


Robin Murphy (5):
  perf/arm_cspmu: Simplify initialisation
  perf/arm_cspmu: Simplify attribute groups
  perf/arm_cspmu: Simplify counter reset
  dt-bindings/perf: Add Arm CoreSight PMU
  perf/arm_cspmu: Add devicetree support

 .../bindings/perf/arm,coresight-pmu.yaml      |  39 +++++
 drivers/perf/arm_cspmu/arm_cspmu.c            | 153 ++++++++++--------
 drivers/perf/arm_cspmu/arm_cspmu.h            |   1 +
 drivers/perf/arm_cspmu/nvidia_cspmu.c         |   6 -
 4 files changed, 126 insertions(+), 73 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/perf/arm,coresight-pmu.yaml

-- 
2.39.2.101.g768bb238c484.dirty

[PATCH v3 0/5] perf/arm_cspmu: Add devicetree support

[Robin Murphy]

v2: https://lore.kernel.org/linux-arm-kernel/cover.1702571292.git.robin.murphy@arm.com/

Hi all,

Third time lucky... here's a hopefully-final spin to address the
functional issues Besar pointed out, and give a little final polish to
the binding (although I stand firm on teh point of a trivial example
not being valuable :))

Cheers,
Robin.


Robin Murphy (5):
  perf/arm_cspmu: Simplify initialisation
  perf/arm_cspmu: Simplify attribute groups
  perf/arm_cspmu: Simplify counter reset
  dt-bindings/perf: Add Arm CoreSight PMU
  perf/arm_cspmu: Add devicetree support

 .../bindings/perf/arm,coresight-pmu.yaml      |  39 +++++
 drivers/perf/arm_cspmu/arm_cspmu.c            | 153 ++++++++++--------
 drivers/perf/arm_cspmu/arm_cspmu.h            |   1 +
 drivers/perf/arm_cspmu/nvidia_cspmu.c         |   6 -
 4 files changed, 126 insertions(+), 73 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/perf/arm,coresight-pmu.yaml

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

[PATCH v3 1/5] perf/arm_cspmu: Simplify initialisation

[Robin Murphy]

It's far simpler for implementations to literally override whichever
default ops they want to, by initialising to the default ops first. This
saves all the bother of checking what the impl_init_ops call has or
hasn't touched. Make the same clear distinction for the PMIIDR override
as well, in case we gain more sources for overriding that in future.

Reviewed-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/perf/arm_cspmu/arm_cspmu.c    | 55 +++++++++++----------------
 drivers/perf/arm_cspmu/nvidia_cspmu.c |  6 ---
 2 files changed, 22 insertions(+), 39 deletions(-)

diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
index 50b89b989ce7..7c8a6bd940f4 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.c
+++ b/drivers/perf/arm_cspmu/arm_cspmu.c
@@ -100,13 +100,6 @@
 #define ARM_CSPMU_ACTIVE_CPU_MASK		0x0
 #define ARM_CSPMU_ASSOCIATED_CPU_MASK		0x1
 
-/* Check and use default if implementer doesn't provide attribute callback */
-#define CHECK_DEFAULT_IMPL_OPS(ops, callback)			\
-	do {							\
-		if (!ops->callback)				\
-			ops->callback = arm_cspmu_ ## callback;	\
-	} while (0)
-
 /*
  * Maximum poll count for reading counter value using high-low-high sequence.
  */
@@ -408,21 +401,32 @@ static struct arm_cspmu_impl_match *arm_cspmu_impl_match_get(u32 pmiidr)
 	return NULL;
 }
 
+#define DEFAULT_IMPL_OP(name)	.name = arm_cspmu_##name
+
 static int arm_cspmu_init_impl_ops(struct arm_cspmu *cspmu)
 {
 	int ret = 0;
-	struct arm_cspmu_impl_ops *impl_ops = &cspmu->impl.ops;
 	struct acpi_apmt_node *apmt_node = arm_cspmu_apmt_node(cspmu->dev);
 	struct arm_cspmu_impl_match *match;
 
-	/*
-	 * Get PMU implementer and product id from APMT node.
-	 * If APMT node doesn't have implementer/product id, try get it
-	 * from PMIIDR.
-	 */
-	cspmu->impl.pmiidr =
-		(apmt_node->impl_id) ? apmt_node->impl_id :
-				       readl(cspmu->base0 + PMIIDR);
+	/* Start with a default PMU implementation */
+	cspmu->impl.module = THIS_MODULE;
+	cspmu->impl.pmiidr = readl(cspmu->base0 + PMIIDR);
+	cspmu->impl.ops = (struct arm_cspmu_impl_ops) {
+		DEFAULT_IMPL_OP(get_event_attrs),
+		DEFAULT_IMPL_OP(get_format_attrs),
+		DEFAULT_IMPL_OP(get_identifier),
+		DEFAULT_IMPL_OP(get_name),
+		DEFAULT_IMPL_OP(is_cycle_counter_event),
+		DEFAULT_IMPL_OP(event_type),
+		DEFAULT_IMPL_OP(event_filter),
+		DEFAULT_IMPL_OP(set_ev_filter),
+		DEFAULT_IMPL_OP(event_attr_is_visible),
+	};
+
+	/* Firmware may override implementer/product ID from PMIIDR */
+	if (apmt_node->impl_id)
+		cspmu->impl.pmiidr = apmt_node->impl_id;
 
 	/* Find implementer specific attribute ops. */
 	match = arm_cspmu_impl_match_get(cspmu->impl.pmiidr);
@@ -450,24 +454,9 @@ static int arm_cspmu_init_impl_ops(struct arm_cspmu *cspmu)
 		}
 
 		mutex_unlock(&arm_cspmu_lock);
+	}
 
-		if (ret)
-			return ret;
-	} else
-		cspmu->impl.module = THIS_MODULE;
-
-	/* Use default callbacks if implementer doesn't provide one. */
-	CHECK_DEFAULT_IMPL_OPS(impl_ops, get_event_attrs);
-	CHECK_DEFAULT_IMPL_OPS(impl_ops, get_format_attrs);
-	CHECK_DEFAULT_IMPL_OPS(impl_ops, get_identifier);
-	CHECK_DEFAULT_IMPL_OPS(impl_ops, get_name);
-	CHECK_DEFAULT_IMPL_OPS(impl_ops, is_cycle_counter_event);
-	CHECK_DEFAULT_IMPL_OPS(impl_ops, event_type);
-	CHECK_DEFAULT_IMPL_OPS(impl_ops, event_filter);
-	CHECK_DEFAULT_IMPL_OPS(impl_ops, event_attr_is_visible);
-	CHECK_DEFAULT_IMPL_OPS(impl_ops, set_ev_filter);
-
-	return 0;
+	return ret;
 }
 
 static struct attribute_group *
diff --git a/drivers/perf/arm_cspmu/nvidia_cspmu.c b/drivers/perf/arm_cspmu/nvidia_cspmu.c
index 0382b702f092..5b84b701ad62 100644
--- a/drivers/perf/arm_cspmu/nvidia_cspmu.c
+++ b/drivers/perf/arm_cspmu/nvidia_cspmu.c
@@ -388,12 +388,6 @@ static int nv_cspmu_init_ops(struct arm_cspmu *cspmu)
 	impl_ops->get_format_attrs		= nv_cspmu_get_format_attrs;
 	impl_ops->get_name			= nv_cspmu_get_name;
 
-	/* Set others to NULL to use default callback. */
-	impl_ops->event_type			= NULL;
-	impl_ops->event_attr_is_visible		= NULL;
-	impl_ops->get_identifier		= NULL;
-	impl_ops->is_cycle_counter_event	= NULL;
-
 	return 0;
 }
 
-- 
2.39.2.101.g768bb238c484.dirty

[PATCH v3 1/5] perf/arm_cspmu: Simplify initialisation

[Robin Murphy]

It's far simpler for implementations to literally override whichever
default ops they want to, by initialising to the default ops first. This
saves all the bother of checking what the impl_init_ops call has or
hasn't touched. Make the same clear distinction for the PMIIDR override
as well, in case we gain more sources for overriding that in future.

Reviewed-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/perf/arm_cspmu/arm_cspmu.c    | 55 +++++++++++----------------
 drivers/perf/arm_cspmu/nvidia_cspmu.c |  6 ---
 2 files changed, 22 insertions(+), 39 deletions(-)

diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
index 50b89b989ce7..7c8a6bd940f4 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.c
+++ b/drivers/perf/arm_cspmu/arm_cspmu.c
@@ -100,13 +100,6 @@
 #define ARM_CSPMU_ACTIVE_CPU_MASK		0x0
 #define ARM_CSPMU_ASSOCIATED_CPU_MASK		0x1
 
-/* Check and use default if implementer doesn't provide attribute callback */
-#define CHECK_DEFAULT_IMPL_OPS(ops, callback)			\
-	do {							\
-		if (!ops->callback)				\
-			ops->callback = arm_cspmu_ ## callback;	\
-	} while (0)
-
 /*
  * Maximum poll count for reading counter value using high-low-high sequence.
  */
@@ -408,21 +401,32 @@ static struct arm_cspmu_impl_match *arm_cspmu_impl_match_get(u32 pmiidr)
 	return NULL;
 }
 
+#define DEFAULT_IMPL_OP(name)	.name = arm_cspmu_##name
+
 static int arm_cspmu_init_impl_ops(struct arm_cspmu *cspmu)
 {
 	int ret = 0;
-	struct arm_cspmu_impl_ops *impl_ops = &cspmu->impl.ops;
 	struct acpi_apmt_node *apmt_node = arm_cspmu_apmt_node(cspmu->dev);
 	struct arm_cspmu_impl_match *match;
 
-	/*
-	 * Get PMU implementer and product id from APMT node.
-	 * If APMT node doesn't have implementer/product id, try get it
-	 * from PMIIDR.
-	 */
-	cspmu->impl.pmiidr =
-		(apmt_node->impl_id) ? apmt_node->impl_id :
-				       readl(cspmu->base0 + PMIIDR);
+	/* Start with a default PMU implementation */
+	cspmu->impl.module = THIS_MODULE;
+	cspmu->impl.pmiidr = readl(cspmu->base0 + PMIIDR);
+	cspmu->impl.ops = (struct arm_cspmu_impl_ops) {
+		DEFAULT_IMPL_OP(get_event_attrs),
+		DEFAULT_IMPL_OP(get_format_attrs),
+		DEFAULT_IMPL_OP(get_identifier),
+		DEFAULT_IMPL_OP(get_name),
+		DEFAULT_IMPL_OP(is_cycle_counter_event),
+		DEFAULT_IMPL_OP(event_type),
+		DEFAULT_IMPL_OP(event_filter),
+		DEFAULT_IMPL_OP(set_ev_filter),
+		DEFAULT_IMPL_OP(event_attr_is_visible),
+	};
+
+	/* Firmware may override implementer/product ID from PMIIDR */
+	if (apmt_node->impl_id)
+		cspmu->impl.pmiidr = apmt_node->impl_id;
 
 	/* Find implementer specific attribute ops. */
 	match = arm_cspmu_impl_match_get(cspmu->impl.pmiidr);
@@ -450,24 +454,9 @@ static int arm_cspmu_init_impl_ops(struct arm_cspmu *cspmu)
 		}
 
 		mutex_unlock(&arm_cspmu_lock);
+	}
 
-		if (ret)
-			return ret;
-	} else
-		cspmu->impl.module = THIS_MODULE;
-
-	/* Use default callbacks if implementer doesn't provide one. */
-	CHECK_DEFAULT_IMPL_OPS(impl_ops, get_event_attrs);
-	CHECK_DEFAULT_IMPL_OPS(impl_ops, get_format_attrs);
-	CHECK_DEFAULT_IMPL_OPS(impl_ops, get_identifier);
-	CHECK_DEFAULT_IMPL_OPS(impl_ops, get_name);
-	CHECK_DEFAULT_IMPL_OPS(impl_ops, is_cycle_counter_event);
-	CHECK_DEFAULT_IMPL_OPS(impl_ops, event_type);
-	CHECK_DEFAULT_IMPL_OPS(impl_ops, event_filter);
-	CHECK_DEFAULT_IMPL_OPS(impl_ops, event_attr_is_visible);
-	CHECK_DEFAULT_IMPL_OPS(impl_ops, set_ev_filter);
-
-	return 0;
+	return ret;
 }
 
 static struct attribute_group *
diff --git a/drivers/perf/arm_cspmu/nvidia_cspmu.c b/drivers/perf/arm_cspmu/nvidia_cspmu.c
index 0382b702f092..5b84b701ad62 100644
--- a/drivers/perf/arm_cspmu/nvidia_cspmu.c
+++ b/drivers/perf/arm_cspmu/nvidia_cspmu.c
@@ -388,12 +388,6 @@ static int nv_cspmu_init_ops(struct arm_cspmu *cspmu)
 	impl_ops->get_format_attrs		= nv_cspmu_get_format_attrs;
 	impl_ops->get_name			= nv_cspmu_get_name;
 
-	/* Set others to NULL to use default callback. */
-	impl_ops->event_type			= NULL;
-	impl_ops->event_attr_is_visible		= NULL;
-	impl_ops->get_identifier		= NULL;
-	impl_ops->is_cycle_counter_event	= NULL;
-
 	return 0;
 }
 
-- 
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

[PATCH v3 2/5] perf/arm_cspmu: Simplify attribute groups

[Robin Murphy]

The attribute group array itself is always the same, so there's no
need to allocate it separately. Storing it directly in our instance
data saves memory and gives us one less point of failure.

Reviewed-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/perf/arm_cspmu/arm_cspmu.c | 26 +++++++++-----------------
 drivers/perf/arm_cspmu/arm_cspmu.h |  1 +
 2 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
index 7c8a6bd940f4..b888473ec71f 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.c
+++ b/drivers/perf/arm_cspmu/arm_cspmu.c
@@ -501,23 +501,16 @@ arm_cspmu_alloc_format_attr_group(struct arm_cspmu *cspmu)
 	return format_group;
 }
 
-static struct attribute_group **
-arm_cspmu_alloc_attr_group(struct arm_cspmu *cspmu)
+static int arm_cspmu_alloc_attr_groups(struct arm_cspmu *cspmu)
 {
-	struct attribute_group **attr_groups = NULL;
-	struct device *dev = cspmu->dev;
+	const struct attribute_group **attr_groups = cspmu->attr_groups;
 	const struct arm_cspmu_impl_ops *impl_ops = &cspmu->impl.ops;
 
 	cspmu->identifier = impl_ops->get_identifier(cspmu);
 	cspmu->name = impl_ops->get_name(cspmu);
 
 	if (!cspmu->identifier || !cspmu->name)
-		return NULL;
-
-	attr_groups = devm_kcalloc(dev, 5, sizeof(struct attribute_group *),
-				   GFP_KERNEL);
-	if (!attr_groups)
-		return NULL;
+		return -ENOMEM;
 
 	attr_groups[0] = arm_cspmu_alloc_event_attr_group(cspmu);
 	attr_groups[1] = arm_cspmu_alloc_format_attr_group(cspmu);
@@ -525,9 +518,9 @@ arm_cspmu_alloc_attr_group(struct arm_cspmu *cspmu)
 	attr_groups[3] = &arm_cspmu_cpumask_attr_group;
 
 	if (!attr_groups[0] || !attr_groups[1])
-		return NULL;
+		return -ENOMEM;
 
-	return attr_groups;
+	return 0;
 }
 
 static inline void arm_cspmu_reset_counters(struct arm_cspmu *cspmu)
@@ -1164,11 +1157,10 @@ static int arm_cspmu_get_cpus(struct arm_cspmu *cspmu)
 static int arm_cspmu_register_pmu(struct arm_cspmu *cspmu)
 {
 	int ret, capabilities;
-	struct attribute_group **attr_groups;
 
-	attr_groups = arm_cspmu_alloc_attr_group(cspmu);
-	if (!attr_groups)
-		return -ENOMEM;
+	ret = arm_cspmu_alloc_attr_groups(cspmu);
+	if (ret)
+		return ret;
 
 	ret = cpuhp_state_add_instance(arm_cspmu_cpuhp_state,
 				       &cspmu->cpuhp_node);
@@ -1190,7 +1182,7 @@ static int arm_cspmu_register_pmu(struct arm_cspmu *cspmu)
 		.start		= arm_cspmu_start,
 		.stop		= arm_cspmu_stop,
 		.read		= arm_cspmu_read,
-		.attr_groups	= (const struct attribute_group **)attr_groups,
+		.attr_groups	= cspmu->attr_groups,
 		.capabilities	= capabilities,
 	};
 
diff --git a/drivers/perf/arm_cspmu/arm_cspmu.h b/drivers/perf/arm_cspmu/arm_cspmu.h
index 2fe723555a6b..c9163acfe810 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.h
+++ b/drivers/perf/arm_cspmu/arm_cspmu.h
@@ -157,6 +157,7 @@ struct arm_cspmu {
 	int cycle_counter_logical_idx;
 
 	struct arm_cspmu_hw_events hw_events;
+	const struct attribute_group *attr_groups[5];
 
 	struct arm_cspmu_impl impl;
 };
-- 
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

[PATCH v3 2/5] perf/arm_cspmu: Simplify attribute groups

[Robin Murphy]

The attribute group array itself is always the same, so there's no
need to allocate it separately. Storing it directly in our instance
data saves memory and gives us one less point of failure.

Reviewed-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/perf/arm_cspmu/arm_cspmu.c | 26 +++++++++-----------------
 drivers/perf/arm_cspmu/arm_cspmu.h |  1 +
 2 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
index 7c8a6bd940f4..b888473ec71f 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.c
+++ b/drivers/perf/arm_cspmu/arm_cspmu.c
@@ -501,23 +501,16 @@ arm_cspmu_alloc_format_attr_group(struct arm_cspmu *cspmu)
 	return format_group;
 }
 
-static struct attribute_group **
-arm_cspmu_alloc_attr_group(struct arm_cspmu *cspmu)
+static int arm_cspmu_alloc_attr_groups(struct arm_cspmu *cspmu)
 {
-	struct attribute_group **attr_groups = NULL;
-	struct device *dev = cspmu->dev;
+	const struct attribute_group **attr_groups = cspmu->attr_groups;
 	const struct arm_cspmu_impl_ops *impl_ops = &cspmu->impl.ops;
 
 	cspmu->identifier = impl_ops->get_identifier(cspmu);
 	cspmu->name = impl_ops->get_name(cspmu);
 
 	if (!cspmu->identifier || !cspmu->name)
-		return NULL;
-
-	attr_groups = devm_kcalloc(dev, 5, sizeof(struct attribute_group *),
-				   GFP_KERNEL);
-	if (!attr_groups)
-		return NULL;
+		return -ENOMEM;
 
 	attr_groups[0] = arm_cspmu_alloc_event_attr_group(cspmu);
 	attr_groups[1] = arm_cspmu_alloc_format_attr_group(cspmu);
@@ -525,9 +518,9 @@ arm_cspmu_alloc_attr_group(struct arm_cspmu *cspmu)
 	attr_groups[3] = &arm_cspmu_cpumask_attr_group;
 
 	if (!attr_groups[0] || !attr_groups[1])
-		return NULL;
+		return -ENOMEM;
 
-	return attr_groups;
+	return 0;
 }
 
 static inline void arm_cspmu_reset_counters(struct arm_cspmu *cspmu)
@@ -1164,11 +1157,10 @@ static int arm_cspmu_get_cpus(struct arm_cspmu *cspmu)
 static int arm_cspmu_register_pmu(struct arm_cspmu *cspmu)
 {
 	int ret, capabilities;
-	struct attribute_group **attr_groups;
 
-	attr_groups = arm_cspmu_alloc_attr_group(cspmu);
-	if (!attr_groups)
-		return -ENOMEM;
+	ret = arm_cspmu_alloc_attr_groups(cspmu);
+	if (ret)
+		return ret;
 
 	ret = cpuhp_state_add_instance(arm_cspmu_cpuhp_state,
 				       &cspmu->cpuhp_node);
@@ -1190,7 +1182,7 @@ static int arm_cspmu_register_pmu(struct arm_cspmu *cspmu)
 		.start		= arm_cspmu_start,
 		.stop		= arm_cspmu_stop,
 		.read		= arm_cspmu_read,
-		.attr_groups	= (const struct attribute_group **)attr_groups,
+		.attr_groups	= cspmu->attr_groups,
 		.capabilities	= capabilities,
 	};
 
diff --git a/drivers/perf/arm_cspmu/arm_cspmu.h b/drivers/perf/arm_cspmu/arm_cspmu.h
index 2fe723555a6b..c9163acfe810 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.h
+++ b/drivers/perf/arm_cspmu/arm_cspmu.h
@@ -157,6 +157,7 @@ struct arm_cspmu {
 	int cycle_counter_logical_idx;
 
 	struct arm_cspmu_hw_events hw_events;
+	const struct attribute_group *attr_groups[5];
 
 	struct arm_cspmu_impl impl;
 };
-- 
2.39.2.101.g768bb238c484.dirty

[PATCH v3 3/5] perf/arm_cspmu: Simplify counter reset

[Robin Murphy]

arm_cspmu_reset_counters() inherently also stops them since it is
writing 0 to PMCR.E, so there should be no need to do that twice.
Also tidy up the reset routine itself for consistency with the start
and stop routines, and to be clear at first glance that it is simply
writing a constant value.

Reviewed-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/perf/arm_cspmu/arm_cspmu.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
index b888473ec71f..b54dc7fd6ca5 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.c
+++ b/drivers/perf/arm_cspmu/arm_cspmu.c
@@ -525,11 +525,7 @@ static int arm_cspmu_alloc_attr_groups(struct arm_cspmu *cspmu)
 
 static inline void arm_cspmu_reset_counters(struct arm_cspmu *cspmu)
 {
-	u32 pmcr = 0;
-
-	pmcr |= PMCR_P;
-	pmcr |= PMCR_C;
-	writel(pmcr, cspmu->base0 + PMCR);
+	writel(PMCR_C | PMCR_P, cspmu->base0 + PMCR);
 }
 
 static inline void arm_cspmu_start_counters(struct arm_cspmu *cspmu)
@@ -1187,7 +1183,6 @@ static int arm_cspmu_register_pmu(struct arm_cspmu *cspmu)
 	};
 
 	/* Hardware counter init */
-	arm_cspmu_stop_counters(cspmu);
 	arm_cspmu_reset_counters(cspmu);
 
 	ret = perf_pmu_register(&cspmu->pmu, cspmu->name, -1);
-- 
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

[PATCH v3 3/5] perf/arm_cspmu: Simplify counter reset

[Robin Murphy]

arm_cspmu_reset_counters() inherently also stops them since it is
writing 0 to PMCR.E, so there should be no need to do that twice.
Also tidy up the reset routine itself for consistency with the start
and stop routines, and to be clear at first glance that it is simply
writing a constant value.

Reviewed-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/perf/arm_cspmu/arm_cspmu.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
index b888473ec71f..b54dc7fd6ca5 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.c
+++ b/drivers/perf/arm_cspmu/arm_cspmu.c
@@ -525,11 +525,7 @@ static int arm_cspmu_alloc_attr_groups(struct arm_cspmu *cspmu)
 
 static inline void arm_cspmu_reset_counters(struct arm_cspmu *cspmu)
 {
-	u32 pmcr = 0;
-
-	pmcr |= PMCR_P;
-	pmcr |= PMCR_C;
-	writel(pmcr, cspmu->base0 + PMCR);
+	writel(PMCR_C | PMCR_P, cspmu->base0 + PMCR);
 }
 
 static inline void arm_cspmu_start_counters(struct arm_cspmu *cspmu)
@@ -1187,7 +1183,6 @@ static int arm_cspmu_register_pmu(struct arm_cspmu *cspmu)
 	};
 
 	/* Hardware counter init */
-	arm_cspmu_stop_counters(cspmu);
 	arm_cspmu_reset_counters(cspmu);
 
 	ret = perf_pmu_register(&cspmu->pmu, cspmu->name, -1);
-- 
2.39.2.101.g768bb238c484.dirty

[PATCH v3 4/5] dt-bindings/perf: Add Arm CoreSight PMU

[Robin Murphy]

Add a binding for implementations of the Arm CoreSight Performance
Monitoring Unit Architecture. Not to be confused with CoreSight debug
and trace, the PMU architecture defines a standard MMIO interface for
event counters following a similar design to the CPU PMU architecture,
where the implementation and most of its features are discoverable
through ID registers.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
v2: Use reg-io-width instead of a new property; tweak descriptions
v3: Tweak formatting
---
 .../bindings/perf/arm,coresight-pmu.yaml      | 39 +++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/perf/arm,coresight-pmu.yaml

diff --git a/Documentation/devicetree/bindings/perf/arm,coresight-pmu.yaml b/Documentation/devicetree/bindings/perf/arm,coresight-pmu.yaml
new file mode 100644
index 000000000000..985b62990f80
--- /dev/null
+++ b/Documentation/devicetree/bindings/perf/arm,coresight-pmu.yaml
@@ -0,0 +1,39 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/perf/arm,coresight-pmu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Arm Coresight Performance Monitoring Unit Architecture
+
+maintainers:
+  - Robin Murphy <robin.murphy@arm.com>
+
+properties:
+  compatible:
+    const: arm,coresight-pmu
+
+  reg:
+    items:
+      - description: Register page 0
+      - description: Register page 1, if the PMU implements the dual-page extension
+    minItems: 1
+
+  interrupts:
+    items:
+      - description: Overflow interrupt
+
+  cpus:
+    description: If the PMU is associated with a particular CPU or subset of CPUs,
+      array of phandles to the appropriate CPU node(s)
+
+  reg-io-width:
+    description: Granularity at which PMU register accesses are single-copy atomic
+    default: 4
+    enum: [4, 8]
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
-- 
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

[PATCH v3 4/5] dt-bindings/perf: Add Arm CoreSight PMU

[Robin Murphy]

Add a binding for implementations of the Arm CoreSight Performance
Monitoring Unit Architecture. Not to be confused with CoreSight debug
and trace, the PMU architecture defines a standard MMIO interface for
event counters following a similar design to the CPU PMU architecture,
where the implementation and most of its features are discoverable
through ID registers.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
v2: Use reg-io-width instead of a new property; tweak descriptions
v3: Tweak formatting
---
 .../bindings/perf/arm,coresight-pmu.yaml      | 39 +++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/perf/arm,coresight-pmu.yaml

diff --git a/Documentation/devicetree/bindings/perf/arm,coresight-pmu.yaml b/Documentation/devicetree/bindings/perf/arm,coresight-pmu.yaml
new file mode 100644
index 000000000000..985b62990f80
--- /dev/null
+++ b/Documentation/devicetree/bindings/perf/arm,coresight-pmu.yaml
@@ -0,0 +1,39 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/perf/arm,coresight-pmu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Arm Coresight Performance Monitoring Unit Architecture
+
+maintainers:
+  - Robin Murphy <robin.murphy@arm.com>
+
+properties:
+  compatible:
+    const: arm,coresight-pmu
+
+  reg:
+    items:
+      - description: Register page 0
+      - description: Register page 1, if the PMU implements the dual-page extension
+    minItems: 1
+
+  interrupts:
+    items:
+      - description: Overflow interrupt
+
+  cpus:
+    description: If the PMU is associated with a particular CPU or subset of CPUs,
+      array of phandles to the appropriate CPU node(s)
+
+  reg-io-width:
+    description: Granularity at which PMU register accesses are single-copy atomic
+    default: 4
+    enum: [4, 8]
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
-- 
2.39.2.101.g768bb238c484.dirty

[PATCH v3 5/5] perf/arm_cspmu: Add devicetree support

[Robin Murphy]

Hook up devicetree probing support. For now let's hope that people
implement PMIIDR properly and we don't need an override property or
match data mechanism.

Reviewed-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
v2: Use APMT node to distinguish ACPI; adjust for binding change
v3: Handle NULL APMT node properly, fix phandle iteration.
---
 drivers/perf/arm_cspmu/arm_cspmu.c | 67 ++++++++++++++++++++++++------
 1 file changed, 55 insertions(+), 12 deletions(-)

diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
index b54dc7fd6ca5..6bc21ef96250 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.c
+++ b/drivers/perf/arm_cspmu/arm_cspmu.c
@@ -27,6 +27,7 @@
 #include <linux/io-64-nonatomic-lo-hi.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/of.h>
 #include <linux/perf_event.h>
 #include <linux/platform_device.h>
 
@@ -114,7 +115,9 @@ static void arm_cspmu_set_ev_filter(struct arm_cspmu *cspmu,
 
 static struct acpi_apmt_node *arm_cspmu_apmt_node(struct device *dev)
 {
-	return *(struct acpi_apmt_node **)dev_get_platdata(dev);
+	struct acpi_apmt_node **ptr = dev_get_platdata(dev);
+
+	return ptr ? *ptr : NULL;
 }
 
 /*
@@ -310,6 +313,10 @@ static const char *arm_cspmu_get_name(const struct arm_cspmu *cspmu)
 
 	dev = cspmu->dev;
 	apmt_node = arm_cspmu_apmt_node(dev);
+	if (!apmt_node)
+		return devm_kasprintf(dev, GFP_KERNEL, PMUNAME "_%u",
+				      atomic_fetch_inc(&pmu_idx[0]));
+
 	pmu_type = apmt_node->type;
 
 	if (pmu_type >= ACPI_APMT_NODE_TYPE_COUNT) {
@@ -425,7 +432,7 @@ static int arm_cspmu_init_impl_ops(struct arm_cspmu *cspmu)
 	};
 
 	/* Firmware may override implementer/product ID from PMIIDR */
-	if (apmt_node->impl_id)
+	if (apmt_node && apmt_node->impl_id)
 		cspmu->impl.pmiidr = apmt_node->impl_id;
 
 	/* Find implementer specific attribute ops. */
@@ -940,7 +947,14 @@ static struct arm_cspmu *arm_cspmu_alloc(struct platform_device *pdev)
 	platform_set_drvdata(pdev, cspmu);
 
 	apmt_node = arm_cspmu_apmt_node(dev);
-	cspmu->has_atomic_dword = apmt_node->flags & ACPI_APMT_FLAGS_ATOMIC;
+	if (apmt_node) {
+		cspmu->has_atomic_dword = apmt_node->flags & ACPI_APMT_FLAGS_ATOMIC;
+	} else {
+		u32 width = 0;
+
+		device_property_read_u32(dev, "reg-io-width", &width);
+		cspmu->has_atomic_dword = (width == 8);
+	}
 
 	return cspmu;
 }
@@ -1131,11 +1145,6 @@ static int arm_cspmu_acpi_get_cpus(struct arm_cspmu *cspmu)
 		}
 	}
 
-	if (cpumask_empty(&cspmu->associated_cpus)) {
-		dev_dbg(cspmu->dev, "No cpu associated with the PMU\n");
-		return -ENODEV;
-	}
-
 	return 0;
 }
 #else
@@ -1145,9 +1154,36 @@ static int arm_cspmu_acpi_get_cpus(struct arm_cspmu *cspmu)
 }
 #endif
 
+static int arm_cspmu_of_get_cpus(struct arm_cspmu *cspmu)
+{
+	struct of_phandle_iterator it;
+	int ret, cpu;
+
+	of_for_each_phandle(&it, ret, dev_of_node(cspmu->dev), "cpus", NULL, 0) {
+		cpu = of_cpu_node_to_id(it.node);
+		if (cpu < 0)
+			continue;
+		cpumask_set_cpu(cpu, &cspmu->associated_cpus);
+	}
+	return ret == -ENOENT ? 0 : ret;
+}
+
 static int arm_cspmu_get_cpus(struct arm_cspmu *cspmu)
 {
-	return arm_cspmu_acpi_get_cpus(cspmu);
+	int ret = 0;
+
+	if (arm_cspmu_apmt_node(cspmu->dev))
+		ret = arm_cspmu_acpi_get_cpus(cspmu);
+	else if (device_property_present(cspmu->dev, "cpus"))
+		ret = arm_cspmu_of_get_cpus(cspmu);
+	else
+		cpumask_copy(&cspmu->associated_cpus, cpu_possible_mask);
+
+	if (!ret && cpumask_empty(&cspmu->associated_cpus)) {
+		dev_dbg(cspmu->dev, "No cpu associated with the PMU\n");
+		ret = -ENODEV;
+	}
+	return ret;
 }
 
 static int arm_cspmu_register_pmu(struct arm_cspmu *cspmu)
@@ -1244,11 +1280,18 @@ static const struct platform_device_id arm_cspmu_id[] = {
 };
 MODULE_DEVICE_TABLE(platform, arm_cspmu_id);
 
+static const struct of_device_id arm_cspmu_of_match[] = {
+	{ .compatible = "arm,coresight-pmu" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, arm_cspmu_of_match);
+
 static struct platform_driver arm_cspmu_driver = {
 	.driver = {
-			.name = DRVNAME,
-			.suppress_bind_attrs = true,
-		},
+		.name = DRVNAME,
+		.of_match_table = arm_cspmu_of_match,
+		.suppress_bind_attrs = true,
+	},
 	.probe = arm_cspmu_device_probe,
 	.remove = arm_cspmu_device_remove,
 	.id_table = arm_cspmu_id,
-- 
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

[PATCH v3 5/5] perf/arm_cspmu: Add devicetree support

[Robin Murphy]

Hook up devicetree probing support. For now let's hope that people
implement PMIIDR properly and we don't need an override property or
match data mechanism.

Reviewed-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
v2: Use APMT node to distinguish ACPI; adjust for binding change
v3: Handle NULL APMT node properly, fix phandle iteration.
---
 drivers/perf/arm_cspmu/arm_cspmu.c | 67 ++++++++++++++++++++++++------
 1 file changed, 55 insertions(+), 12 deletions(-)

diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
index b54dc7fd6ca5..6bc21ef96250 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.c
+++ b/drivers/perf/arm_cspmu/arm_cspmu.c
@@ -27,6 +27,7 @@
 #include <linux/io-64-nonatomic-lo-hi.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/of.h>
 #include <linux/perf_event.h>
 #include <linux/platform_device.h>
 
@@ -114,7 +115,9 @@ static void arm_cspmu_set_ev_filter(struct arm_cspmu *cspmu,
 
 static struct acpi_apmt_node *arm_cspmu_apmt_node(struct device *dev)
 {
-	return *(struct acpi_apmt_node **)dev_get_platdata(dev);
+	struct acpi_apmt_node **ptr = dev_get_platdata(dev);
+
+	return ptr ? *ptr : NULL;
 }
 
 /*
@@ -310,6 +313,10 @@ static const char *arm_cspmu_get_name(const struct arm_cspmu *cspmu)
 
 	dev = cspmu->dev;
 	apmt_node = arm_cspmu_apmt_node(dev);
+	if (!apmt_node)
+		return devm_kasprintf(dev, GFP_KERNEL, PMUNAME "_%u",
+				      atomic_fetch_inc(&pmu_idx[0]));
+
 	pmu_type = apmt_node->type;
 
 	if (pmu_type >= ACPI_APMT_NODE_TYPE_COUNT) {
@@ -425,7 +432,7 @@ static int arm_cspmu_init_impl_ops(struct arm_cspmu *cspmu)
 	};
 
 	/* Firmware may override implementer/product ID from PMIIDR */
-	if (apmt_node->impl_id)
+	if (apmt_node && apmt_node->impl_id)
 		cspmu->impl.pmiidr = apmt_node->impl_id;
 
 	/* Find implementer specific attribute ops. */
@@ -940,7 +947,14 @@ static struct arm_cspmu *arm_cspmu_alloc(struct platform_device *pdev)
 	platform_set_drvdata(pdev, cspmu);
 
 	apmt_node = arm_cspmu_apmt_node(dev);
-	cspmu->has_atomic_dword = apmt_node->flags & ACPI_APMT_FLAGS_ATOMIC;
+	if (apmt_node) {
+		cspmu->has_atomic_dword = apmt_node->flags & ACPI_APMT_FLAGS_ATOMIC;
+	} else {
+		u32 width = 0;
+
+		device_property_read_u32(dev, "reg-io-width", &width);
+		cspmu->has_atomic_dword = (width == 8);
+	}
 
 	return cspmu;
 }
@@ -1131,11 +1145,6 @@ static int arm_cspmu_acpi_get_cpus(struct arm_cspmu *cspmu)
 		}
 	}
 
-	if (cpumask_empty(&cspmu->associated_cpus)) {
-		dev_dbg(cspmu->dev, "No cpu associated with the PMU\n");
-		return -ENODEV;
-	}
-
 	return 0;
 }
 #else
@@ -1145,9 +1154,36 @@ static int arm_cspmu_acpi_get_cpus(struct arm_cspmu *cspmu)
 }
 #endif
 
+static int arm_cspmu_of_get_cpus(struct arm_cspmu *cspmu)
+{
+	struct of_phandle_iterator it;
+	int ret, cpu;
+
+	of_for_each_phandle(&it, ret, dev_of_node(cspmu->dev), "cpus", NULL, 0) {
+		cpu = of_cpu_node_to_id(it.node);
+		if (cpu < 0)
+			continue;
+		cpumask_set_cpu(cpu, &cspmu->associated_cpus);
+	}
+	return ret == -ENOENT ? 0 : ret;
+}
+
 static int arm_cspmu_get_cpus(struct arm_cspmu *cspmu)
 {
-	return arm_cspmu_acpi_get_cpus(cspmu);
+	int ret = 0;
+
+	if (arm_cspmu_apmt_node(cspmu->dev))
+		ret = arm_cspmu_acpi_get_cpus(cspmu);
+	else if (device_property_present(cspmu->dev, "cpus"))
+		ret = arm_cspmu_of_get_cpus(cspmu);
+	else
+		cpumask_copy(&cspmu->associated_cpus, cpu_possible_mask);
+
+	if (!ret && cpumask_empty(&cspmu->associated_cpus)) {
+		dev_dbg(cspmu->dev, "No cpu associated with the PMU\n");
+		ret = -ENODEV;
+	}
+	return ret;
 }
 
 static int arm_cspmu_register_pmu(struct arm_cspmu *cspmu)
@@ -1244,11 +1280,18 @@ static const struct platform_device_id arm_cspmu_id[] = {
 };
 MODULE_DEVICE_TABLE(platform, arm_cspmu_id);
 
+static const struct of_device_id arm_cspmu_of_match[] = {
+	{ .compatible = "arm,coresight-pmu" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, arm_cspmu_of_match);
+
 static struct platform_driver arm_cspmu_driver = {
 	.driver = {
-			.name = DRVNAME,
-			.suppress_bind_attrs = true,
-		},
+		.name = DRVNAME,
+		.of_match_table = arm_cspmu_of_match,
+		.suppress_bind_attrs = true,
+	},
 	.probe = arm_cspmu_device_probe,
 	.remove = arm_cspmu_device_remove,
 	.id_table = arm_cspmu_id,
-- 
2.39.2.101.g768bb238c484.dirty

Re: [PATCH v3 0/5] perf/arm_cspmu: Add devicetree support

[Suzuki K Poulose]

On 06/02/2024 10:27, Robin Murphy wrote:
> v2: https://lore.kernel.org/linux-arm-kernel/cover.1702571292.git.robin.murphy@arm.com/
> 
> Hi all,
> 
> Third time lucky... here's a hopefully-final spin to address the
> functional issues Besar pointed out, and give a little final polish to
> the binding (although I stand firm on teh point of a trivial example
> not being valuable :))
> 
> Cheers,
> Robin.
> 
> 
> Robin Murphy (5):
>    perf/arm_cspmu: Simplify initialisation
>    perf/arm_cspmu: Simplify attribute groups
>    perf/arm_cspmu: Simplify counter reset
>    dt-bindings/perf: Add Arm CoreSight PMU
>    perf/arm_cspmu: Add devicetree support
> 
>   .../bindings/perf/arm,coresight-pmu.yaml      |  39 +++++
>   drivers/perf/arm_cspmu/arm_cspmu.c            | 153 ++++++++++--------
>   drivers/perf/arm_cspmu/arm_cspmu.h            |   1 +
>   drivers/perf/arm_cspmu/nvidia_cspmu.c         |   6 -
>   4 files changed, 126 insertions(+), 73 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/perf/arm,coresight-pmu.yaml
> 

For the series:

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

Re: [PATCH v3 0/5] perf/arm_cspmu: Add devicetree support

[Suzuki K Poulose]

On 06/02/2024 10:27, Robin Murphy wrote:
> v2: https://lore.kernel.org/linux-arm-kernel/cover.1702571292.git.robin.murphy@arm.com/
> 
> Hi all,
> 
> Third time lucky... here's a hopefully-final spin to address the
> functional issues Besar pointed out, and give a little final polish to
> the binding (although I stand firm on teh point of a trivial example
> not being valuable :))
> 
> Cheers,
> Robin.
> 
> 
> Robin Murphy (5):
>    perf/arm_cspmu: Simplify initialisation
>    perf/arm_cspmu: Simplify attribute groups
>    perf/arm_cspmu: Simplify counter reset
>    dt-bindings/perf: Add Arm CoreSight PMU
>    perf/arm_cspmu: Add devicetree support
> 
>   .../bindings/perf/arm,coresight-pmu.yaml      |  39 +++++
>   drivers/perf/arm_cspmu/arm_cspmu.c            | 153 ++++++++++--------
>   drivers/perf/arm_cspmu/arm_cspmu.h            |   1 +
>   drivers/perf/arm_cspmu/nvidia_cspmu.c         |   6 -
>   4 files changed, 126 insertions(+), 73 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/perf/arm,coresight-pmu.yaml
> 

For the series:

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

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

RE: [PATCH v3 5/5] perf/arm_cspmu: Add devicetree support

[Besar Wicaksono]

> -----Original Message-----
> From: Robin Murphy <robin.murphy@arm.com>
> Sent: Tuesday, February 6, 2024 4:28 AM
> To: will@kernel.org
> Cc: mark.rutland@arm.com; linux-arm-kernel@lists.infradead.org;
> devicetree@vger.kernel.org; suzuki.poulose@arm.com;
> ilkka@os.amperecomputing.com; Besar Wicaksono
> <bwicaksono@nvidia.com>; Yifei Wan <YWan@nvidia.com>; Richard Wiley
> <rwiley@nvidia.com>
> Subject: [PATCH v3 5/5] perf/arm_cspmu: Add devicetree support
> 
> External email: Use caution opening links or attachments
> 
> 
> Hook up devicetree probing support. For now let's hope that people
> implement PMIIDR properly and we don't need an override property or
> match data mechanism.
> 
> Reviewed-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>

Reviewed-by: Besar Wicaksono <bwicaksono@nvidia.com>
Tested-by: Besar Wicaksono <bwicaksono@nvidia.com>

Regards,
Besar

> ---
> v2: Use APMT node to distinguish ACPI; adjust for binding change
> v3: Handle NULL APMT node properly, fix phandle iteration.
> ---
>  drivers/perf/arm_cspmu/arm_cspmu.c | 67 ++++++++++++++++++++++++-
> -----
>  1 file changed, 55 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c
> b/drivers/perf/arm_cspmu/arm_cspmu.c
> index b54dc7fd6ca5..6bc21ef96250 100644
> --- a/drivers/perf/arm_cspmu/arm_cspmu.c
> +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
> @@ -27,6 +27,7 @@
>  #include <linux/io-64-nonatomic-lo-hi.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
> +#include <linux/of.h>
>  #include <linux/perf_event.h>
>  #include <linux/platform_device.h>
> 
> @@ -114,7 +115,9 @@ static void arm_cspmu_set_ev_filter(struct
> arm_cspmu *cspmu,
> 
>  static struct acpi_apmt_node *arm_cspmu_apmt_node(struct device *dev)
>  {
> -       return *(struct acpi_apmt_node **)dev_get_platdata(dev);
> +       struct acpi_apmt_node **ptr = dev_get_platdata(dev);
> +
> +       return ptr ? *ptr : NULL;
>  }
> 
>  /*
> @@ -310,6 +313,10 @@ static const char *arm_cspmu_get_name(const
> struct arm_cspmu *cspmu)
> 
>         dev = cspmu->dev;
>         apmt_node = arm_cspmu_apmt_node(dev);
> +       if (!apmt_node)
> +               return devm_kasprintf(dev, GFP_KERNEL, PMUNAME "_%u",
> +                                     atomic_fetch_inc(&pmu_idx[0]));
> +
>         pmu_type = apmt_node->type;
> 
>         if (pmu_type >= ACPI_APMT_NODE_TYPE_COUNT) {
> @@ -425,7 +432,7 @@ static int arm_cspmu_init_impl_ops(struct
> arm_cspmu *cspmu)
>         };
> 
>         /* Firmware may override implementer/product ID from PMIIDR */
> -       if (apmt_node->impl_id)
> +       if (apmt_node && apmt_node->impl_id)
>                 cspmu->impl.pmiidr = apmt_node->impl_id;
> 
>         /* Find implementer specific attribute ops. */
> @@ -940,7 +947,14 @@ static struct arm_cspmu *arm_cspmu_alloc(struct
> platform_device *pdev)
>         platform_set_drvdata(pdev, cspmu);
> 
>         apmt_node = arm_cspmu_apmt_node(dev);
> -       cspmu->has_atomic_dword = apmt_node->flags &
> ACPI_APMT_FLAGS_ATOMIC;
> +       if (apmt_node) {
> +               cspmu->has_atomic_dword = apmt_node->flags &
> ACPI_APMT_FLAGS_ATOMIC;
> +       } else {
> +               u32 width = 0;
> +
> +               device_property_read_u32(dev, "reg-io-width", &width);
> +               cspmu->has_atomic_dword = (width == 8);
> +       }
> 
>         return cspmu;
>  }
> @@ -1131,11 +1145,6 @@ static int arm_cspmu_acpi_get_cpus(struct
> arm_cspmu *cspmu)
>                 }
>         }
> 
> -       if (cpumask_empty(&cspmu->associated_cpus)) {
> -               dev_dbg(cspmu->dev, "No cpu associated with the PMU\n");
> -               return -ENODEV;
> -       }
> -
>         return 0;
>  }
>  #else
> @@ -1145,9 +1154,36 @@ static int arm_cspmu_acpi_get_cpus(struct
> arm_cspmu *cspmu)
>  }
>  #endif
> 
> +static int arm_cspmu_of_get_cpus(struct arm_cspmu *cspmu)
> +{
> +       struct of_phandle_iterator it;
> +       int ret, cpu;
> +
> +       of_for_each_phandle(&it, ret, dev_of_node(cspmu->dev), "cpus", NULL,
> 0) {
> +               cpu = of_cpu_node_to_id(it.node);
> +               if (cpu < 0)
> +                       continue;
> +               cpumask_set_cpu(cpu, &cspmu->associated_cpus);
> +       }
> +       return ret == -ENOENT ? 0 : ret;
> +}
> +
>  static int arm_cspmu_get_cpus(struct arm_cspmu *cspmu)
>  {
> -       return arm_cspmu_acpi_get_cpus(cspmu);
> +       int ret = 0;
> +
> +       if (arm_cspmu_apmt_node(cspmu->dev))
> +               ret = arm_cspmu_acpi_get_cpus(cspmu);
> +       else if (device_property_present(cspmu->dev, "cpus"))
> +               ret = arm_cspmu_of_get_cpus(cspmu);
> +       else
> +               cpumask_copy(&cspmu->associated_cpus, cpu_possible_mask);
> +
> +       if (!ret && cpumask_empty(&cspmu->associated_cpus)) {
> +               dev_dbg(cspmu->dev, "No cpu associated with the PMU\n");
> +               ret = -ENODEV;
> +       }
> +       return ret;
>  }
> 
>  static int arm_cspmu_register_pmu(struct arm_cspmu *cspmu)
> @@ -1244,11 +1280,18 @@ static const struct platform_device_id
> arm_cspmu_id[] = {
>  };
>  MODULE_DEVICE_TABLE(platform, arm_cspmu_id);
> 
> +static const struct of_device_id arm_cspmu_of_match[] = {
> +       { .compatible = "arm,coresight-pmu" },
> +       {}
> +};
> +MODULE_DEVICE_TABLE(of, arm_cspmu_of_match);
> +
>  static struct platform_driver arm_cspmu_driver = {
>         .driver = {
> -                       .name = DRVNAME,
> -                       .suppress_bind_attrs = true,
> -               },
> +               .name = DRVNAME,
> +               .of_match_table = arm_cspmu_of_match,
> +               .suppress_bind_attrs = true,
> +       },
>         .probe = arm_cspmu_device_probe,
>         .remove = arm_cspmu_device_remove,
>         .id_table = arm_cspmu_id,
> --
> 2.39.2.101.g768bb238c484.dirty

RE: [PATCH v3 5/5] perf/arm_cspmu: Add devicetree support

[Besar Wicaksono]

> -----Original Message-----
> From: Robin Murphy <robin.murphy@arm.com>
> Sent: Tuesday, February 6, 2024 4:28 AM
> To: will@kernel.org
> Cc: mark.rutland@arm.com; linux-arm-kernel@lists.infradead.org;
> devicetree@vger.kernel.org; suzuki.poulose@arm.com;
> ilkka@os.amperecomputing.com; Besar Wicaksono
> <bwicaksono@nvidia.com>; Yifei Wan <YWan@nvidia.com>; Richard Wiley
> <rwiley@nvidia.com>
> Subject: [PATCH v3 5/5] perf/arm_cspmu: Add devicetree support
> 
> External email: Use caution opening links or attachments
> 
> 
> Hook up devicetree probing support. For now let's hope that people
> implement PMIIDR properly and we don't need an override property or
> match data mechanism.
> 
> Reviewed-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>

Reviewed-by: Besar Wicaksono <bwicaksono@nvidia.com>
Tested-by: Besar Wicaksono <bwicaksono@nvidia.com>

Regards,
Besar

> ---
> v2: Use APMT node to distinguish ACPI; adjust for binding change
> v3: Handle NULL APMT node properly, fix phandle iteration.
> ---
>  drivers/perf/arm_cspmu/arm_cspmu.c | 67 ++++++++++++++++++++++++-
> -----
>  1 file changed, 55 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c
> b/drivers/perf/arm_cspmu/arm_cspmu.c
> index b54dc7fd6ca5..6bc21ef96250 100644
> --- a/drivers/perf/arm_cspmu/arm_cspmu.c
> +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
> @@ -27,6 +27,7 @@
>  #include <linux/io-64-nonatomic-lo-hi.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
> +#include <linux/of.h>
>  #include <linux/perf_event.h>
>  #include <linux/platform_device.h>
> 
> @@ -114,7 +115,9 @@ static void arm_cspmu_set_ev_filter(struct
> arm_cspmu *cspmu,
> 
>  static struct acpi_apmt_node *arm_cspmu_apmt_node(struct device *dev)
>  {
> -       return *(struct acpi_apmt_node **)dev_get_platdata(dev);
> +       struct acpi_apmt_node **ptr = dev_get_platdata(dev);
> +
> +       return ptr ? *ptr : NULL;
>  }
> 
>  /*
> @@ -310,6 +313,10 @@ static const char *arm_cspmu_get_name(const
> struct arm_cspmu *cspmu)
> 
>         dev = cspmu->dev;
>         apmt_node = arm_cspmu_apmt_node(dev);
> +       if (!apmt_node)
> +               return devm_kasprintf(dev, GFP_KERNEL, PMUNAME "_%u",
> +                                     atomic_fetch_inc(&pmu_idx[0]));
> +
>         pmu_type = apmt_node->type;
> 
>         if (pmu_type >= ACPI_APMT_NODE_TYPE_COUNT) {
> @@ -425,7 +432,7 @@ static int arm_cspmu_init_impl_ops(struct
> arm_cspmu *cspmu)
>         };
> 
>         /* Firmware may override implementer/product ID from PMIIDR */
> -       if (apmt_node->impl_id)
> +       if (apmt_node && apmt_node->impl_id)
>                 cspmu->impl.pmiidr = apmt_node->impl_id;
> 
>         /* Find implementer specific attribute ops. */
> @@ -940,7 +947,14 @@ static struct arm_cspmu *arm_cspmu_alloc(struct
> platform_device *pdev)
>         platform_set_drvdata(pdev, cspmu);
> 
>         apmt_node = arm_cspmu_apmt_node(dev);
> -       cspmu->has_atomic_dword = apmt_node->flags &
> ACPI_APMT_FLAGS_ATOMIC;
> +       if (apmt_node) {
> +               cspmu->has_atomic_dword = apmt_node->flags &
> ACPI_APMT_FLAGS_ATOMIC;
> +       } else {
> +               u32 width = 0;
> +
> +               device_property_read_u32(dev, "reg-io-width", &width);
> +               cspmu->has_atomic_dword = (width == 8);
> +       }
> 
>         return cspmu;
>  }
> @@ -1131,11 +1145,6 @@ static int arm_cspmu_acpi_get_cpus(struct
> arm_cspmu *cspmu)
>                 }
>         }
> 
> -       if (cpumask_empty(&cspmu->associated_cpus)) {
> -               dev_dbg(cspmu->dev, "No cpu associated with the PMU\n");
> -               return -ENODEV;
> -       }
> -
>         return 0;
>  }
>  #else
> @@ -1145,9 +1154,36 @@ static int arm_cspmu_acpi_get_cpus(struct
> arm_cspmu *cspmu)
>  }
>  #endif
> 
> +static int arm_cspmu_of_get_cpus(struct arm_cspmu *cspmu)
> +{
> +       struct of_phandle_iterator it;
> +       int ret, cpu;
> +
> +       of_for_each_phandle(&it, ret, dev_of_node(cspmu->dev), "cpus", NULL,
> 0) {
> +               cpu = of_cpu_node_to_id(it.node);
> +               if (cpu < 0)
> +                       continue;
> +               cpumask_set_cpu(cpu, &cspmu->associated_cpus);
> +       }
> +       return ret == -ENOENT ? 0 : ret;
> +}
> +
>  static int arm_cspmu_get_cpus(struct arm_cspmu *cspmu)
>  {
> -       return arm_cspmu_acpi_get_cpus(cspmu);
> +       int ret = 0;
> +
> +       if (arm_cspmu_apmt_node(cspmu->dev))
> +               ret = arm_cspmu_acpi_get_cpus(cspmu);
> +       else if (device_property_present(cspmu->dev, "cpus"))
> +               ret = arm_cspmu_of_get_cpus(cspmu);
> +       else
> +               cpumask_copy(&cspmu->associated_cpus, cpu_possible_mask);
> +
> +       if (!ret && cpumask_empty(&cspmu->associated_cpus)) {
> +               dev_dbg(cspmu->dev, "No cpu associated with the PMU\n");
> +               ret = -ENODEV;
> +       }
> +       return ret;
>  }
> 
>  static int arm_cspmu_register_pmu(struct arm_cspmu *cspmu)
> @@ -1244,11 +1280,18 @@ static const struct platform_device_id
> arm_cspmu_id[] = {
>  };
>  MODULE_DEVICE_TABLE(platform, arm_cspmu_id);
> 
> +static const struct of_device_id arm_cspmu_of_match[] = {
> +       { .compatible = "arm,coresight-pmu" },
> +       {}
> +};
> +MODULE_DEVICE_TABLE(of, arm_cspmu_of_match);
> +
>  static struct platform_driver arm_cspmu_driver = {
>         .driver = {
> -                       .name = DRVNAME,
> -                       .suppress_bind_attrs = true,
> -               },
> +               .name = DRVNAME,
> +               .of_match_table = arm_cspmu_of_match,
> +               .suppress_bind_attrs = true,
> +       },
>         .probe = arm_cspmu_device_probe,
>         .remove = arm_cspmu_device_remove,
>         .id_table = arm_cspmu_id,
> --
> 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

Re: [PATCH v3 0/5] perf/arm_cspmu: Add devicetree support

[Will Deacon]

On Tue, 6 Feb 2024 10:27:53 +0000, Robin Murphy wrote:
> v2: https://lore.kernel.org/linux-arm-kernel/cover.1702571292.git.robin.murphy@arm.com/
> 
> Hi all,
> 
> Third time lucky... here's a hopefully-final spin to address the
> functional issues Besar pointed out, and give a little final polish to
> the binding (although I stand firm on teh point of a trivial example
> not being valuable :))
> 
> [...]

Applied to will (for-next/perf), thanks!

[1/5] perf/arm_cspmu: Simplify initialisation
      https://git.kernel.org/will/c/df2675ad7748
[2/5] perf/arm_cspmu: Simplify attribute groups
      https://git.kernel.org/will/c/7e6a3c3f8588
[3/5] perf/arm_cspmu: Simplify counter reset
      https://git.kernel.org/will/c/e7e8fa8e82af
[4/5] dt-bindings/perf: Add Arm CoreSight PMU
      https://git.kernel.org/will/c/7255cfb19941
[5/5] perf/arm_cspmu: Add devicetree support
      https://git.kernel.org/will/c/fd185a245155

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

Re: [PATCH v3 0/5] perf/arm_cspmu: Add devicetree support

[Will Deacon]

On Tue, 6 Feb 2024 10:27:53 +0000, Robin Murphy wrote:
> v2: https://lore.kernel.org/linux-arm-kernel/cover.1702571292.git.robin.murphy@arm.com/
> 
> Hi all,
> 
> Third time lucky... here's a hopefully-final spin to address the
> functional issues Besar pointed out, and give a little final polish to
> the binding (although I stand firm on teh point of a trivial example
> not being valuable :))
> 
> [...]

Applied to will (for-next/perf), thanks!

[1/5] perf/arm_cspmu: Simplify initialisation
      https://git.kernel.org/will/c/df2675ad7748
[2/5] perf/arm_cspmu: Simplify attribute groups
      https://git.kernel.org/will/c/7e6a3c3f8588
[3/5] perf/arm_cspmu: Simplify counter reset
      https://git.kernel.org/will/c/e7e8fa8e82af
[4/5] dt-bindings/perf: Add Arm CoreSight PMU
      https://git.kernel.org/will/c/7255cfb19941
[5/5] perf/arm_cspmu: Add devicetree support
      https://git.kernel.org/will/c/fd185a245155

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

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