[PATCH 0/2] perf/arm-ni: Optimize codes

[PATCH 0/2] perf/arm-ni: Optimize codes

[Shouping Wang]

Rename PMU device name, and display the topology info of clock domain.

Thanks,
Shouping.

Shouping Wang (2):
  perf/arm-ni: rename PMU device name
  perf: arm-ni: add topology debug info for the clock domain

 drivers/perf/arm-ni.c | 69 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 64 insertions(+), 5 deletions(-)

-- 
2.43.0

[PATCH 1/2] perf/arm-ni: rename PMU device name

[Shouping Wang]

The PMU device names are arm_ni_1_*,arm_ni_2_*, etc. 
The device names change based on the order of registration 
for multiple NI instances. By naming the PMU device using 
its address, the device name can be made independent of 
the registration order.

Signed-off-by: Shouping Wang <allen.wang@hj-micro.com>
---
 drivers/perf/arm-ni.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/perf/arm-ni.c b/drivers/perf/arm-ni.c
index 1615a0564031..32eabdbe877a 100644
--- a/drivers/perf/arm-ni.c
+++ b/drivers/perf/arm-ni.c
@@ -53,6 +53,7 @@
 
 #define NI_NUM_COUNTERS		8
 #define NI_CCNT_IDX		31
+#define NI_PMU_PA_SHIFT		12
 
 /* Event attributes */
 #define NI_CONFIG_TYPE		GENMASK_ULL(15, 0)
@@ -115,7 +116,6 @@ struct arm_ni {
 	struct device *dev;
 	void __iomem *base;
 	enum ni_part part;
-	int id;
 	int cpu;
 	int num_cds;
 	struct hlist_node cpuhp_node;
@@ -560,7 +560,7 @@ static int arm_ni_init_cd(struct arm_ni *ni, struct arm_ni_node *node, u64 res_s
 		.read = arm_ni_event_read,
 	};
 
-	name = devm_kasprintf(ni->dev, GFP_KERNEL, "arm_ni_%d_cd_%d", ni->id, cd->id);
+	name = devm_kasprintf(ni->dev, GFP_KERNEL, "arm_ni_%llx", res_start >> NI_PMU_PA_SHIFT);
 	if (!name)
 		return -ENOMEM;
 
@@ -623,7 +623,6 @@ static int arm_ni_probe(struct platform_device *pdev)
 	struct arm_ni *ni;
 	struct resource *res;
 	void __iomem *base;
-	static atomic_t id;
 	int ret, num_cds;
 	u32 reg, part;
 
@@ -674,7 +673,6 @@ static int arm_ni_probe(struct platform_device *pdev)
 	ni->base = base;
 	ni->num_cds = num_cds;
 	ni->part = part;
-	ni->id = atomic_fetch_inc(&id);
 	ni->cpu = cpumask_local_spread(0, dev_to_node(ni->dev));
 	platform_set_drvdata(pdev, ni);
 
-- 
2.43.0

[PATCH 2/2] perf: arm-ni: add topology debug info for the clock domain

[Shouping Wang]

When capturing PMU info with perf, it is necessary to know the nodeid of 
the interface. Here, we add topology debug info for the clock domain to 
display the nodeid of the interfaces.

$ cat /sys/kernel/debug/arm-ni/map_1f3c06
as shown below:
| HSNI #5 || PMNI #6 || PMU  #14 |

Signed-off-by: Shouping Wang <allen.wang@hj-micro.com>
---
 drivers/perf/arm-ni.c | 63 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 62 insertions(+), 1 deletion(-)

diff --git a/drivers/perf/arm-ni.c b/drivers/perf/arm-ni.c
index 32eabdbe877a..e0f0e18d0120 100644
--- a/drivers/perf/arm-ni.c
+++ b/drivers/perf/arm-ni.c
@@ -13,6 +13,7 @@
 #include <linux/perf_event.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
+#include <linux/debugfs.h>
 
 /* Common registers */
 #define NI_NODE_TYPE		0x000
@@ -110,6 +111,7 @@ struct arm_ni_cd {
 	struct arm_ni_unit *units;
 	struct perf_event *evcnt[NI_NUM_COUNTERS];
 	struct perf_event *ccnt;
+	struct dentry *debug;
 };
 
 struct arm_ni {
@@ -476,6 +478,59 @@ static irqreturn_t arm_ni_handle_irq(int irq, void *dev_id)
 	}
 }
 
+static struct dentry *arm_ni_cd_debugfs;
+
+#ifdef CONFIG_DEBUG_FS
+static const char *arm_ni_node_type(enum ni_node_type type)
+{
+	switch (type) {
+	case NI_ASNI:
+		return "| ASNI ";
+	case NI_AMNI:
+		return "| AMNI ";
+	case NI_PMU:
+		return "| PMU  ";
+	case NI_HSNI:
+		return "| HSNI ";
+	case NI_HMNI:
+		return "| HMNI ";
+	case NI_PMNI:
+		return "| PMNI ";
+	default:
+		return "| ???? ";
+	}
+}
+
+static int arm_ni_cd_map_show(struct seq_file *s, void *data)
+{
+	struct arm_ni_cd *cd = s->private;
+
+	cd_for_each_unit(cd, unit) {
+		seq_printf(s, "%s#%-2d |", arm_ni_node_type(unit->type), unit->id);
+	}
+	seq_puts(s, "\n");
+
+	return 0;
+}
+
+DEFINE_SHOW_ATTRIBUTE(arm_ni_cd_map);
+
+static void arm_ni_cd_debugfs_init(struct arm_ni_cd *cd, u64 res_start)
+{
+	const char *name = "map";
+
+	if (res_start > 0)
+		name = devm_kasprintf(cd_to_ni(cd)->dev, GFP_KERNEL, "map_%llx",
+				res_start >> NI_PMU_PA_SHIFT);
+	if (!name)
+		return;
+
+	cd->debug = debugfs_create_file(name, 0444, arm_ni_cd_debugfs, cd, &arm_ni_cd_map_fops);
+}
+#else
+static void arm_ni_cd_debugfs_init(struct arm_ni_cd *cd, u64 res_start) {}
+#endif
+
 static int arm_ni_init_cd(struct arm_ni *ni, struct arm_ni_node *node, u64 res_start)
 {
 	struct arm_ni_cd *cd = ni->cds + node->id;
@@ -563,6 +618,7 @@ static int arm_ni_init_cd(struct arm_ni *ni, struct arm_ni_node *node, u64 res_s
 	name = devm_kasprintf(ni->dev, GFP_KERNEL, "arm_ni_%llx", res_start >> NI_PMU_PA_SHIFT);
 	if (!name)
 		return -ENOMEM;
+	arm_ni_cd_debugfs_init(cd, res_start);
 
 	return perf_pmu_register(&cd->pmu, name, -1);
 }
@@ -575,6 +631,7 @@ static void arm_ni_remove(struct platform_device *pdev)
 		writel_relaxed(0, cd->pmu_base + NI_PMCR);
 		writel_relaxed(U32_MAX, cd->pmu_base + NI_PMINTENCLR);
 		perf_pmu_unregister(&cd->pmu);
+		debugfs_remove(cd->debug);
 	}
 	cpuhp_state_remove_instance_nocalls(arm_ni_hp_state, &ni->cpuhp_node);
 }
@@ -787,9 +844,12 @@ static int __init arm_ni_init(void)
 
 	arm_ni_hp_state = ret;
 
+	arm_ni_cd_debugfs = debugfs_create_dir("arm-ni", NULL);
 	ret = platform_driver_register(&arm_ni_driver);
-	if (ret)
+	if (ret) {
 		cpuhp_remove_multi_state(arm_ni_hp_state);
+		debugfs_remove(arm_ni_cd_debugfs);
+	}
 	return ret;
 }
 
@@ -797,6 +857,7 @@ static void __exit arm_ni_exit(void)
 {
 	platform_driver_unregister(&arm_ni_driver);
 	cpuhp_remove_multi_state(arm_ni_hp_state);
+	debugfs_remove(arm_ni_cd_debugfs);
 }
 
 module_init(arm_ni_init);
-- 
2.43.0

Re: [PATCH 1/2] perf/arm-ni: rename PMU device name

[Will Deacon]

On Fri, Nov 07, 2025 at 04:43:18PM +0800, Shouping Wang wrote:
> The PMU device names are arm_ni_1_*,arm_ni_2_*, etc. 
> The device names change based on the order of registration 
> for multiple NI instances. By naming the PMU device using 
> its address, the device name can be made independent of 
> the registration order.
> 
> Signed-off-by: Shouping Wang <allen.wang@hj-micro.com>
> ---
>  drivers/perf/arm-ni.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/perf/arm-ni.c b/drivers/perf/arm-ni.c
> index 1615a0564031..32eabdbe877a 100644
> --- a/drivers/perf/arm-ni.c
> +++ b/drivers/perf/arm-ni.c
> @@ -53,6 +53,7 @@
>  
>  #define NI_NUM_COUNTERS		8
>  #define NI_CCNT_IDX		31
> +#define NI_PMU_PA_SHIFT		12
>  
>  /* Event attributes */
>  #define NI_CONFIG_TYPE		GENMASK_ULL(15, 0)
> @@ -115,7 +116,6 @@ struct arm_ni {
>  	struct device *dev;
>  	void __iomem *base;
>  	enum ni_part part;
> -	int id;
>  	int cpu;
>  	int num_cds;
>  	struct hlist_node cpuhp_node;
> @@ -560,7 +560,7 @@ static int arm_ni_init_cd(struct arm_ni *ni, struct arm_ni_node *node, u64 res_s
>  		.read = arm_ni_event_read,
>  	};
>  
> -	name = devm_kasprintf(ni->dev, GFP_KERNEL, "arm_ni_%d_cd_%d", ni->id, cd->id);
> +	name = devm_kasprintf(ni->dev, GFP_KERNEL, "arm_ni_%llx", res_start >> NI_PMU_PA_SHIFT);

Doesn't this have the potential to break userspace (e.g. scripts) that
expect the current naming to be stable?

Either way, I'll need Robin's acks for these changes.

Will

Re: [PATCH 1/2] perf/arm-ni: rename PMU device name

[Robin Murphy]

On 2025-11-24 3:47 pm, Will Deacon wrote:
> On Fri, Nov 07, 2025 at 04:43:18PM +0800, Shouping Wang wrote:
>> The PMU device names are arm_ni_1_*,arm_ni_2_*, etc.
>> The device names change based on the order of registration
>> for multiple NI instances. By naming the PMU device using
>> its address, the device name can be made independent of
>> the registration order.
>>
>> Signed-off-by: Shouping Wang <allen.wang@hj-micro.com>
>> ---
>>   drivers/perf/arm-ni.c | 6 ++----
>>   1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/perf/arm-ni.c b/drivers/perf/arm-ni.c
>> index 1615a0564031..32eabdbe877a 100644
>> --- a/drivers/perf/arm-ni.c
>> +++ b/drivers/perf/arm-ni.c
>> @@ -53,6 +53,7 @@
>>   
>>   #define NI_NUM_COUNTERS		8
>>   #define NI_CCNT_IDX		31
>> +#define NI_PMU_PA_SHIFT		12
>>   
>>   /* Event attributes */
>>   #define NI_CONFIG_TYPE		GENMASK_ULL(15, 0)
>> @@ -115,7 +116,6 @@ struct arm_ni {
>>   	struct device *dev;
>>   	void __iomem *base;
>>   	enum ni_part part;
>> -	int id;
>>   	int cpu;
>>   	int num_cds;
>>   	struct hlist_node cpuhp_node;
>> @@ -560,7 +560,7 @@ static int arm_ni_init_cd(struct arm_ni *ni, struct arm_ni_node *node, u64 res_s
>>   		.read = arm_ni_event_read,
>>   	};
>>   
>> -	name = devm_kasprintf(ni->dev, GFP_KERNEL, "arm_ni_%d_cd_%d", ni->id, cd->id);
>> +	name = devm_kasprintf(ni->dev, GFP_KERNEL, "arm_ni_%llx", res_start >> NI_PMU_PA_SHIFT);
> 
> Doesn't this have the potential to break userspace (e.g. scripts) that
> expect the current naming to be stable?

Certainly on platforms where only arm_ni_0 ever exists. For 
multi-instance cases it's always been documented that this ID is 
arbitrary, and users should look at the platform device that is the 
sysfs parent of the PMU device(s) in order to disambiguate - from there 
they already have the freedom to use whatever information they like, 
including MMIO resources, but also interrupt numbers, ACPI _UIDs or 
whatever. So although changing one arbitrary value to a different 
arbitrary value is in theory something well-behaved users should be OK 
with, in practice changing it from a small decimal integer to a massive 
hex number may well still break parsing expectations.

Thanks,
Robin.

> 
> Either way, I'll need Robin's acks for these changes.
> 
> Will

Re: [PATCH 2/2] perf: arm-ni: add topology debug info for the clock domain

[Robin Murphy]

On 2025-11-07 8:43 am, Shouping Wang wrote:
> When capturing PMU info with perf, it is necessary to know the nodeid of
> the interface. Here, we add topology debug info for the clock domain to
> display the nodeid of the interfaces.
> 
> $ cat /sys/kernel/debug/arm-ni/map_1f3c06
> as shown below:
> | HSNI #5 || PMNI #6 || PMU  #14 |

This doesn't really provide any useful driver debugging facility though, 
it's just repeating the exact same information that's already perfectly 
visible through the perf interface.

Debugfs is not meant for presenting information to end users. If they 
really don't know the NI topology, it's trivial to query what interfaces 
the driver knows about by simply opening an event for every possible ID 
of the relevant type and seeing which are considered valid - at most 
that's 128*5 perf_event_open syscalls, which can be done in a matter of 
milliseconds. A user only needs to do that once per platform, then 
they're free to use that data to generate numbers that are still largely 
meaningless without knowledge of what those interfaces represent in the 
context of the system outside the NI :)

Thanks,
Robin.

> 
> Signed-off-by: Shouping Wang <allen.wang@hj-micro.com>
> ---
>   drivers/perf/arm-ni.c | 63 ++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 62 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/perf/arm-ni.c b/drivers/perf/arm-ni.c
> index 32eabdbe877a..e0f0e18d0120 100644
> --- a/drivers/perf/arm-ni.c
> +++ b/drivers/perf/arm-ni.c
> @@ -13,6 +13,7 @@
>   #include <linux/perf_event.h>
>   #include <linux/platform_device.h>
>   #include <linux/slab.h>
> +#include <linux/debugfs.h>
>   
>   /* Common registers */
>   #define NI_NODE_TYPE		0x000
> @@ -110,6 +111,7 @@ struct arm_ni_cd {
>   	struct arm_ni_unit *units;
>   	struct perf_event *evcnt[NI_NUM_COUNTERS];
>   	struct perf_event *ccnt;
> +	struct dentry *debug;
>   };
>   
>   struct arm_ni {
> @@ -476,6 +478,59 @@ static irqreturn_t arm_ni_handle_irq(int irq, void *dev_id)
>   	}
>   }
>   
> +static struct dentry *arm_ni_cd_debugfs;
> +
> +#ifdef CONFIG_DEBUG_FS
> +static const char *arm_ni_node_type(enum ni_node_type type)
> +{
> +	switch (type) {
> +	case NI_ASNI:
> +		return "| ASNI ";
> +	case NI_AMNI:
> +		return "| AMNI ";
> +	case NI_PMU:
> +		return "| PMU  ";
> +	case NI_HSNI:
> +		return "| HSNI ";
> +	case NI_HMNI:
> +		return "| HMNI ";
> +	case NI_PMNI:
> +		return "| PMNI ";
> +	default:
> +		return "| ???? ";
> +	}
> +}
> +
> +static int arm_ni_cd_map_show(struct seq_file *s, void *data)
> +{
> +	struct arm_ni_cd *cd = s->private;
> +
> +	cd_for_each_unit(cd, unit) {
> +		seq_printf(s, "%s#%-2d |", arm_ni_node_type(unit->type), unit->id);
> +	}
> +	seq_puts(s, "\n");
> +
> +	return 0;
> +}
> +
> +DEFINE_SHOW_ATTRIBUTE(arm_ni_cd_map);
> +
> +static void arm_ni_cd_debugfs_init(struct arm_ni_cd *cd, u64 res_start)
> +{
> +	const char *name = "map";
> +
> +	if (res_start > 0)
> +		name = devm_kasprintf(cd_to_ni(cd)->dev, GFP_KERNEL, "map_%llx",
> +				res_start >> NI_PMU_PA_SHIFT);
> +	if (!name)
> +		return;
> +
> +	cd->debug = debugfs_create_file(name, 0444, arm_ni_cd_debugfs, cd, &arm_ni_cd_map_fops);
> +}
> +#else
> +static void arm_ni_cd_debugfs_init(struct arm_ni_cd *cd, u64 res_start) {}
> +#endif
> +
>   static int arm_ni_init_cd(struct arm_ni *ni, struct arm_ni_node *node, u64 res_start)
>   {
>   	struct arm_ni_cd *cd = ni->cds + node->id;
> @@ -563,6 +618,7 @@ static int arm_ni_init_cd(struct arm_ni *ni, struct arm_ni_node *node, u64 res_s
>   	name = devm_kasprintf(ni->dev, GFP_KERNEL, "arm_ni_%llx", res_start >> NI_PMU_PA_SHIFT);
>   	if (!name)
>   		return -ENOMEM;
> +	arm_ni_cd_debugfs_init(cd, res_start);
>   
>   	return perf_pmu_register(&cd->pmu, name, -1);
>   }
> @@ -575,6 +631,7 @@ static void arm_ni_remove(struct platform_device *pdev)
>   		writel_relaxed(0, cd->pmu_base + NI_PMCR);
>   		writel_relaxed(U32_MAX, cd->pmu_base + NI_PMINTENCLR);
>   		perf_pmu_unregister(&cd->pmu);
> +		debugfs_remove(cd->debug);
>   	}
>   	cpuhp_state_remove_instance_nocalls(arm_ni_hp_state, &ni->cpuhp_node);
>   }
> @@ -787,9 +844,12 @@ static int __init arm_ni_init(void)
>   
>   	arm_ni_hp_state = ret;
>   
> +	arm_ni_cd_debugfs = debugfs_create_dir("arm-ni", NULL);
>   	ret = platform_driver_register(&arm_ni_driver);
> -	if (ret)
> +	if (ret) {
>   		cpuhp_remove_multi_state(arm_ni_hp_state);
> +		debugfs_remove(arm_ni_cd_debugfs);
> +	}
>   	return ret;
>   }
>   
> @@ -797,6 +857,7 @@ static void __exit arm_ni_exit(void)
>   {
>   	platform_driver_unregister(&arm_ni_driver);
>   	cpuhp_remove_multi_state(arm_ni_hp_state);
> +	debugfs_remove(arm_ni_cd_debugfs);
>   }
>   
>   module_init(arm_ni_init);