[PATCH 0/2] Support clock domains without a PMU instance

[PATCH 0/2] Support clock domains without a PMU instance

[Baisheng Gao]

In a system, it is possible that there isn't a PMU instance in some
clock domains. The original driver will crash because of the pmusela
pointer being NULL in this condition. This series fix it.

Baisheng Gao (2):
  perf/arm-ni: Don't crash in probing clock domains without a PMU
    instance
  dt-bindings/perf: Drop irqs for clock domains without a PMU instance

 Documentation/devicetree/bindings/perf/arm,ni.yaml | 3 ++-
 drivers/perf/arm-ni.c                              | 8 +++++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

Signed-off-by: Baisheng Gao <baisheng.gao@unisoc.com>
-- 
2.34.1

[PATCH 1/2] perf/arm-ni: Don't crash in probing clock domains without a PMU instance

[Baisheng Gao]

The NULL pmusela pointer implies that current clock domain doesn't have
a PMU instance. Return 0 for probing the next clock domain. Otherwise a
kernel crash will happen.

Signed-off-by: Baisheng Gao <baisheng.gao@unisoc.com>
---
 drivers/perf/arm-ni.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/perf/arm-ni.c b/drivers/perf/arm-ni.c
index 66858c65215d..53b656983da1 100644
--- a/drivers/perf/arm-ni.c
+++ b/drivers/perf/arm-ni.c
@@ -526,6 +526,7 @@ static int arm_ni_init_cd(struct arm_ni *ni, struct arm_ni_node *node, u64 res_s
 {
 	struct arm_ni_cd *cd = ni->cds + node->id;
 	const char *name;
+	static atomic_t id;
 
 	cd->id = node->id;
 	cd->num_units = node->num_components;
@@ -562,6 +563,11 @@ static int arm_ni_init_cd(struct arm_ni *ni, struct arm_ni_node *node, u64 res_s
 		case NI_TMNI:
 		case NI_CMNI:
 			unit->pmusela = arm_ni_get_pmusel(ni, unit_base);
+			if (!unit->pmusela) {
+				dev_info(ni->dev, "No have PMU %d\n", cd->id);
+				devm_kfree(ni->dev, cd->units);
+				return 0;
+			}
 			writel_relaxed(1, unit->pmusela);
 			if (readl_relaxed(unit->pmusela) != 1)
 				dev_info(ni->dev, "No access to node 0x%04x%04x\n", unit->id, unit->type);
@@ -591,7 +597,7 @@ static int arm_ni_init_cd(struct arm_ni *ni, struct arm_ni_node *node, u64 res_s
 	writel_relaxed(U32_MAX, cd->pmu_base + NI_PMCNTENCLR);
 	writel_relaxed(U32_MAX, cd->pmu_base + NI_PMOVSCLR);
 
-	cd->irq = platform_get_irq(to_platform_device(ni->dev), cd->id);
+	cd->irq = platform_get_irq(to_platform_device(ni->dev), atomic_fetch_inc(&id));
 	if (cd->irq < 0)
 		return cd->irq;
 
-- 
2.34.1

[PATCH 2/2] dt-bindings/perf: Drop irqs for clock domains without a PMU instance

[Baisheng Gao]

No need to specify the interrupts for the clock domains without a
PMU instance.

Signed-off-by: Baisheng Gao <baisheng.gao@unisoc.com>
---
 Documentation/devicetree/bindings/perf/arm,ni.yaml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/perf/arm,ni.yaml b/Documentation/devicetree/bindings/perf/arm,ni.yaml
index d66fffa256d5..40a5b8929ef2 100644
--- a/Documentation/devicetree/bindings/perf/arm,ni.yaml
+++ b/Documentation/devicetree/bindings/perf/arm,ni.yaml
@@ -20,7 +20,8 @@ properties:
   interrupts:
     minItems: 1
     maxItems: 32
-    description: Overflow interrupts, one per clock domain, in order of domain ID
+    description: Overflow interrupts, one per clock domain which has a PMU
+      instance, in order of domain ID.
 
 required:
   - compatible
-- 
2.34.1

Re: [PATCH 1/2] perf/arm-ni: Don't crash in probing clock domains without a PMU instance

[Robin Murphy]

On 2026-01-26 3:30 am, Baisheng Gao wrote:
> The NULL pmusela pointer implies that current clock domain doesn't have
> a PMU instance. Return 0 for probing the next clock domain. Otherwise a
> kernel crash will happen.

Sorry, this doesn't add up with the diff below. All of the documentation 
says that the PMU is in integral part of the clock domain, and I can 
find no mention of any configuration parameter allowing it to be 
omitted. It is possible for the PMU registers to be inaccessible because 
Non-Secure access has not been enabled, but we account for that already.

> Signed-off-by: Baisheng Gao <baisheng.gao@unisoc.com>
> ---
>   drivers/perf/arm-ni.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/perf/arm-ni.c b/drivers/perf/arm-ni.c
> index 66858c65215d..53b656983da1 100644
> --- a/drivers/perf/arm-ni.c
> +++ b/drivers/perf/arm-ni.c
> @@ -526,6 +526,7 @@ static int arm_ni_init_cd(struct arm_ni *ni, struct arm_ni_node *node, u64 res_s
>   {
>   	struct arm_ni_cd *cd = ni->cds + node->id;
>   	const char *name;
> +	static atomic_t id;
>   
>   	cd->id = node->id;
>   	cd->num_units = node->num_components;
> @@ -562,6 +563,11 @@ static int arm_ni_init_cd(struct arm_ni *ni, struct arm_ni_node *node, u64 res_s
>   		case NI_TMNI:
>   		case NI_CMNI:
>   			unit->pmusela = arm_ni_get_pmusel(ni, unit_base);
> +			if (!unit->pmusela) {

...However this is not about the PMU node anyway; this would represent 
the FCU at an interface node being missing. Again, it's possible for 
access to the FCU itself to be restricted, per the test below, but the 
subfeature ID registers should always be readable, and per the "Should 
be impossible" comment in arm_ni_get_pmusel(), the nodes that can 
generate PMU events should always include an FCU.

Could you please clarify some more details of what the exact situation 
is that you're trying to deal with here?

> +				dev_info(ni->dev, "No have PMU %d\n", cd->id);
> +				devm_kfree(ni->dev, cd->units);
> +				return 0;
> +			}
>   			writel_relaxed(1, unit->pmusela);
>   			if (readl_relaxed(unit->pmusela) != 1)
>   				dev_info(ni->dev, "No access to node 0x%04x%04x\n", unit->id, unit->type);
> @@ -591,7 +597,7 @@ static int arm_ni_init_cd(struct arm_ni *ni, struct arm_ni_node *node, u64 res_s
>   	writel_relaxed(U32_MAX, cd->pmu_base + NI_PMCNTENCLR);
>   	writel_relaxed(U32_MAX, cd->pmu_base + NI_PMOVSCLR);
>   
> -	cd->irq = platform_get_irq(to_platform_device(ni->dev), cd->id);
> +	cd->irq = platform_get_irq(to_platform_device(ni->dev), atomic_fetch_inc(&id));

This is clearly wrong. Disregarding how badly it would go with multiple 
NI instances, even within a single instance I don';t think there's any 
obvious guarantee of a stable order. The firmware bindings are already 
defined, and that definition is not "the order in which a particular 
version of the Linux driver happens to parse things".

Thanks,
Robin.

>   	if (cd->irq < 0)
>   		return cd->irq;
>   

Re: [PATCH 2/2] dt-bindings/perf: Drop irqs for clock domains without a PMU instance

[Robin Murphy]

On 2026-01-26 3:30 am, Baisheng Gao wrote:
> No need to specify the interrupts for the clock domains without a
> PMU instance.

Yes there is a need, because it's what the binding has already defined 
and systems are already implementing, so breaking compatibility at this 
point more than a year after its introduction is not really acceptable. 
And although there's no strict requirement for the DT and ACPI bindings 
to be equivalent, in this case they currently are, and it doesn't seem 
like you've accounted for ACPI here either.

Fact is, the Arm NI-700, NI-710AE, NoC S3 and SI L1 designs do all 
define <CLKNAME>_nPMUINTERRUPT outputs for each <CLKNAME> domain, and 
the intent of the binding was always to describe the hardware. If it's 
the case that one or more of those interrupts are not wired up at all 
(and presumably the corresponding PMU is never exposed to Non-Secure, 
since it's unlikely to be useful), then at worst it's reasonable to use 
dummy entries to pad the array.

If on the other hand you really have got something that is mangled to 
the point of not being compatible with the stock Arm designs, then it 
most likely warrants its own binding.

Thanks,
Robin.

> Signed-off-by: Baisheng Gao <baisheng.gao@unisoc.com>
> ---
>   Documentation/devicetree/bindings/perf/arm,ni.yaml | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/perf/arm,ni.yaml b/Documentation/devicetree/bindings/perf/arm,ni.yaml
> index d66fffa256d5..40a5b8929ef2 100644
> --- a/Documentation/devicetree/bindings/perf/arm,ni.yaml
> +++ b/Documentation/devicetree/bindings/perf/arm,ni.yaml
> @@ -20,7 +20,8 @@ properties:
>     interrupts:
>       minItems: 1
>       maxItems: 32
> -    description: Overflow interrupts, one per clock domain, in order of domain ID
> +    description: Overflow interrupts, one per clock domain which has a PMU
> +      instance, in order of domain ID.
>   
>   required:
>     - compatible