[PATCH 1/4] ARM: perf: don't warn about missing interrupt-affinity property for PPIs

[PATCH 1/4] ARM: perf: don't warn about missing interrupt-affinity property for PPIs

[Will Deacon]

PPIs are affine by nature, so the interrupt-affinity property is not
used and therefore we shouldn't print a warning in its absence.

Reported-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/kernel/perf_event_cpu.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm/kernel/perf_event_cpu.c b/arch/arm/kernel/perf_event_cpu.c
index 91c7ba182dcd..becf7ad6eddc 100644
--- a/arch/arm/kernel/perf_event_cpu.c
+++ b/arch/arm/kernel/perf_event_cpu.c
@@ -303,12 +303,17 @@ static int probe_current_pmu(struct arm_pmu *pmu)
 
 static int of_pmu_irq_cfg(struct platform_device *pdev)
 {
-	int i;
+	int i, irq;
 	int *irqs = kcalloc(pdev->num_resources, sizeof(*irqs), GFP_KERNEL);
 
 	if (!irqs)
 		return -ENOMEM;
 
+	/* Don't bother with PPIs; they're already affine */
+	irq = platform_get_irq(pdev, 0);
+	if (irq >= 0 && irq_is_percpu(irq))
+		return 0;
+
 	for (i = 0; i < pdev->num_resources; ++i) {
 		struct device_node *dn;
 		int cpu;
-- 
2.1.4

[PATCH 2/4] ARM perf: Fix the pmu node name in warning message

[Will Deacon]

With commit 9fd85eb502a7 ("ARM: pmu: add support for interrupt-affinity
property"), we print a warning when we find a PMU SPI with a missing
missing interrupt-affinity property in a pmu node. Unfortunately, we
pass the wrong (NULL) device node to of_node_full_name, resulting in
unhelpful messages such as:

 hw perfevents: Failed to parse <no-node>/interrupt-affinity[0]

This patch fixes the name to that of the pmu node.

Fixes: 9fd85eb502a7 (ARM: pmu: add support for interrupt-affinity property)
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/kernel/perf_event_cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/kernel/perf_event_cpu.c b/arch/arm/kernel/perf_event_cpu.c
index becf7ad6eddc..213919ba326f 100644
--- a/arch/arm/kernel/perf_event_cpu.c
+++ b/arch/arm/kernel/perf_event_cpu.c
@@ -322,7 +322,7 @@ static int of_pmu_irq_cfg(struct platform_device *pdev)
 				      i);
 		if (!dn) {
 			pr_warn("Failed to parse %s/interrupt-affinity[%d]\n",
-				of_node_full_name(dn), i);
+				of_node_full_name(pdev->dev.of_node), i);
 			break;
 		}
 
-- 
2.1.4

[PATCH 3/4] arm64: perf: don't warn about missing interrupt-affinity property for PPIs

[Will Deacon]

PPIs are affine by nature, so the interrupt-affinity property is not
used and therefore we shouldn't print a warning in its absence.

Reported-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/kernel/perf_event.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index 195991dadc37..2a9cbcb61126 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -1310,7 +1310,7 @@ static const struct of_device_id armpmu_of_device_ids[] = {
 
 static int armpmu_device_probe(struct platform_device *pdev)
 {
-	int i, *irqs;
+	int i, irq, *irqs;
 
 	if (!cpu_pmu)
 		return -ENODEV;
@@ -1319,6 +1319,11 @@ static int armpmu_device_probe(struct platform_device *pdev)
 	if (!irqs)
 		return -ENOMEM;
 
+	/* Don't bother with PPIs; they're already affine */
+	irq = platform_get_irq(pdev, 0);
+	if (irq >= 0 && irq_is_percpu(irq))
+		return 0;
+
 	for (i = 0; i < pdev->num_resources; ++i) {
 		struct device_node *dn;
 		int cpu;
-- 
2.1.4

[PATCH 4/4] arm64: perf: Fix the pmu node name in warning message

[Will Deacon]

From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>

With commit d5efd9cc9cf2 ("arm64: pmu: add support for interrupt-affinity
property"), we print a warning when we find a PMU SPI with a missing
missing interrupt-affinity property in a pmu node. Unfortunately, we
pass the wrong (NULL) device node to of_node_full_name, resulting in
unhelpful messages such as:

 hw perfevents: Failed to parse <no-node>/interrupt-affinity[0]

This patch fixes the name to that of the pmu node.

Fixes: d5efd9cc9cf2 (arm64: pmu: add support for interrupt-affinity property)
Acked-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/kernel/perf_event.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index 2a9cbcb61126..23f25acf43a9 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -1332,7 +1332,7 @@ static int armpmu_device_probe(struct platform_device *pdev)
 				      i);
 		if (!dn) {
 			pr_warn("Failed to parse %s/interrupt-affinity[%d]\n",
-				of_node_full_name(dn), i);
+				of_node_full_name(pdev->dev.of_node), i);
 			break;
 		}
 
-- 
2.1.4

[PATCH 2/4] ARM perf: Fix the pmu node name in warning message

[Mark Rutland]

On Thu, Apr 23, 2015 at 02:50:31PM +0100, Will Deacon wrote:
> With commit 9fd85eb502a7 ("ARM: pmu: add support for interrupt-affinity
> property"), we print a warning when we find a PMU SPI with a missing
> missing interrupt-affinity property in a pmu node. Unfortunately, we
> pass the wrong (NULL) device node to of_node_full_name, resulting in
> unhelpful messages such as:
> 
>  hw perfevents: Failed to parse <no-node>/interrupt-affinity[0]
> 
> This patch fixes the name to that of the pmu node.
> 
> Fixes: 9fd85eb502a7 (ARM: pmu: add support for interrupt-affinity property)
> Signed-off-by: Will Deacon <will.deacon@arm.com>

Acked-by: Mark Rutland <mark.rutland@arm.com>

> ---
>  arch/arm/kernel/perf_event_cpu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/kernel/perf_event_cpu.c b/arch/arm/kernel/perf_event_cpu.c
> index becf7ad6eddc..213919ba326f 100644
> --- a/arch/arm/kernel/perf_event_cpu.c
> +++ b/arch/arm/kernel/perf_event_cpu.c
> @@ -322,7 +322,7 @@ static int of_pmu_irq_cfg(struct platform_device *pdev)
>  				      i);
>  		if (!dn) {
>  			pr_warn("Failed to parse %s/interrupt-affinity[%d]\n",
> -				of_node_full_name(dn), i);
> +				of_node_full_name(pdev->dev.of_node), i);
>  			break;
>  		}
>  
> -- 
> 2.1.4
> 

[PATCH 1/4] ARM: perf: don't warn about missing interrupt-affinity property for PPIs

[Mark Rutland]

On Thu, Apr 23, 2015 at 02:50:30PM +0100, Will Deacon wrote:
> PPIs are affine by nature, so the interrupt-affinity property is not
> used and therefore we shouldn't print a warning in its absence.
> 
> Reported-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm/kernel/perf_event_cpu.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/kernel/perf_event_cpu.c b/arch/arm/kernel/perf_event_cpu.c
> index 91c7ba182dcd..becf7ad6eddc 100644
> --- a/arch/arm/kernel/perf_event_cpu.c
> +++ b/arch/arm/kernel/perf_event_cpu.c
> @@ -303,12 +303,17 @@ static int probe_current_pmu(struct arm_pmu *pmu)
>  
>  static int of_pmu_irq_cfg(struct platform_device *pdev)
>  {
> -	int i;
> +	int i, irq;
>  	int *irqs = kcalloc(pdev->num_resources, sizeof(*irqs), GFP_KERNEL);
>  
>  	if (!irqs)
>  		return -ENOMEM;
>  
> +	/* Don't bother with PPIs; they're already affine */
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq >= 0 && irq_is_percpu(irq))
> +		return 0;

Is 0 still a valid IRQ on any ARM platforms?

Otherwise, looks fine to me.

Acked-by: Mark Rutland <mark.rutland@arm.com>

Mark

[PATCH 1/4] ARM: perf: don't warn about missing interrupt-affinity property for PPIs

[Will Deacon]

On Thu, Apr 23, 2015 at 03:01:00PM +0100, Mark Rutland wrote:
> On Thu, Apr 23, 2015 at 02:50:30PM +0100, Will Deacon wrote:
> > PPIs are affine by nature, so the interrupt-affinity property is not
> > used and therefore we shouldn't print a warning in its absence.
> > 
> > Reported-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > ---
> >  arch/arm/kernel/perf_event_cpu.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm/kernel/perf_event_cpu.c b/arch/arm/kernel/perf_event_cpu.c
> > index 91c7ba182dcd..becf7ad6eddc 100644
> > --- a/arch/arm/kernel/perf_event_cpu.c
> > +++ b/arch/arm/kernel/perf_event_cpu.c
> > @@ -303,12 +303,17 @@ static int probe_current_pmu(struct arm_pmu *pmu)
> >  
> >  static int of_pmu_irq_cfg(struct platform_device *pdev)
> >  {
> > -	int i;
> > +	int i, irq;
> >  	int *irqs = kcalloc(pdev->num_resources, sizeof(*irqs), GFP_KERNEL);
> >  
> >  	if (!irqs)
> >  		return -ENOMEM;
> >  
> > +	/* Don't bother with PPIs; they're already affine */
> > +	irq = platform_get_irq(pdev, 0);
> > +	if (irq >= 0 && irq_is_percpu(irq))
> > +		return 0;
> 
> Is 0 still a valid IRQ on any ARM platforms?

I just went for consistency with the cpu_pmu_{request,free}_irq paths.

Will

[PATCH 1/4] ARM: perf: don't warn about missing interrupt-affinity property for PPIs

[Mark Rutland]

On Thu, Apr 23, 2015 at 03:02:24PM +0100, Will Deacon wrote:
> On Thu, Apr 23, 2015 at 03:01:00PM +0100, Mark Rutland wrote:
> > On Thu, Apr 23, 2015 at 02:50:30PM +0100, Will Deacon wrote:
> > > PPIs are affine by nature, so the interrupt-affinity property is not
> > > used and therefore we shouldn't print a warning in its absence.
> > > 
> > > Reported-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > > ---
> > >  arch/arm/kernel/perf_event_cpu.c | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/arm/kernel/perf_event_cpu.c b/arch/arm/kernel/perf_event_cpu.c
> > > index 91c7ba182dcd..becf7ad6eddc 100644
> > > --- a/arch/arm/kernel/perf_event_cpu.c
> > > +++ b/arch/arm/kernel/perf_event_cpu.c
> > > @@ -303,12 +303,17 @@ static int probe_current_pmu(struct arm_pmu *pmu)
> > >  
> > >  static int of_pmu_irq_cfg(struct platform_device *pdev)
> > >  {
> > > -	int i;
> > > +	int i, irq;
> > >  	int *irqs = kcalloc(pdev->num_resources, sizeof(*irqs), GFP_KERNEL);
> > >  
> > >  	if (!irqs)
> > >  		return -ENOMEM;
> > >  
> > > +	/* Don't bother with PPIs; they're already affine */
> > > +	irq = platform_get_irq(pdev, 0);
> > > +	if (irq >= 0 && irq_is_percpu(irq))
> > > +		return 0;
> > 
> > Is 0 still a valid IRQ on any ARM platforms?
> 
> I just went for consistency with the cpu_pmu_{request,free}_irq paths.

Ah, ok. That makes sense.

I guess that also applies to the arm64 part, so feel free to add my ack
to both.

Mark.

[PATCH 1/4] ARM: perf: don't warn about missing interrupt-affinity property for PPIs

[Maxime Ripard]

On Thu, Apr 23, 2015 at 02:50:30PM +0100, Will Deacon wrote:
> PPIs are affine by nature, so the interrupt-affinity property is not
> used and therefore we shouldn't print a warning in its absence.
> 
> Reported-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>

For this one and the arm64 one,

Reviewed-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150423/2a8fb712/attachment.sig>

[PATCH 2/4] ARM perf: Fix the pmu node name in warning message

[Sudeep Holla]

On 23/04/15 14:50, Will Deacon wrote:
> With commit 9fd85eb502a7 ("ARM: pmu: add support for interrupt-affinity
> property"), we print a warning when we find a PMU SPI with a missing
> missing interrupt-affinity property in a pmu node. Unfortunately, we
> pass the wrong (NULL) device node to of_node_full_name, resulting in
> unhelpful messages such as:
>
>   hw perfevents: Failed to parse <no-node>/interrupt-affinity[0]
>
> This patch fixes the name to that of the pmu node.
>
> Fixes: 9fd85eb502a7 (ARM: pmu: add support for interrupt-affinity property)
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>   arch/arm/kernel/perf_event_cpu.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/kernel/perf_event_cpu.c b/arch/arm/kernel/perf_event_cpu.c
> index becf7ad6eddc..213919ba326f 100644
> --- a/arch/arm/kernel/perf_event_cpu.c
> +++ b/arch/arm/kernel/perf_event_cpu.c
> @@ -322,7 +322,7 @@ static int of_pmu_irq_cfg(struct platform_device *pdev)
>   				      i);
>   		if (!dn) {
>   			pr_warn("Failed to parse %s/interrupt-affinity[%d]\n",
> -				of_node_full_name(dn), i);
> +				of_node_full_name(pdev->dev.of_node), i);
>   			break;

With old DT we will see this message and one might think perf is broken.
But since the code still assumes SPIs are listed in order of *logical*
CPU number and continues to work, does it make sense to update the
warning accordingly ?

Regards,
Sudeep

[PATCH 2/4] ARM perf: Fix the pmu node name in warning message

[Will Deacon]

On Fri, May 01, 2015 at 02:59:30PM +0100, Sudeep Holla wrote:
> On 23/04/15 14:50, Will Deacon wrote:
> > With commit 9fd85eb502a7 ("ARM: pmu: add support for interrupt-affinity
> > property"), we print a warning when we find a PMU SPI with a missing
> > missing interrupt-affinity property in a pmu node. Unfortunately, we
> > pass the wrong (NULL) device node to of_node_full_name, resulting in
> > unhelpful messages such as:
> >
> >   hw perfevents: Failed to parse <no-node>/interrupt-affinity[0]
> >
> > This patch fixes the name to that of the pmu node.
> >
> > Fixes: 9fd85eb502a7 (ARM: pmu: add support for interrupt-affinity property)
> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > ---
> >   arch/arm/kernel/perf_event_cpu.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/kernel/perf_event_cpu.c b/arch/arm/kernel/perf_event_cpu.c
> > index becf7ad6eddc..213919ba326f 100644
> > --- a/arch/arm/kernel/perf_event_cpu.c
> > +++ b/arch/arm/kernel/perf_event_cpu.c
> > @@ -322,7 +322,7 @@ static int of_pmu_irq_cfg(struct platform_device *pdev)
> >   				      i);
> >   		if (!dn) {
> >   			pr_warn("Failed to parse %s/interrupt-affinity[%d]\n",
> > -				of_node_full_name(dn), i);
> > +				of_node_full_name(pdev->dev.of_node), i);
> >   			break;
> 
> With old DT we will see this message and one might think perf is broken.
> But since the code still assumes SPIs are listed in order of *logical*
> CPU number and continues to work, does it make sense to update the
> warning accordingly ?

Dunno; it doesn't work on Juno, for example. I've also already sent the
arm64 pull, so it would need to be a separate patch if you wanted to
print something different and we'd need a way to detect the cases where
the routing is actually wrong.

Will

[PATCH 2/4] ARM perf: Fix the pmu node name in warning message

[Mark Rutland]

On Fri, May 01, 2015 at 02:59:30PM +0100, Sudeep Holla wrote:
> 
> 
> On 23/04/15 14:50, Will Deacon wrote:
> > With commit 9fd85eb502a7 ("ARM: pmu: add support for interrupt-affinity
> > property"), we print a warning when we find a PMU SPI with a missing
> > missing interrupt-affinity property in a pmu node. Unfortunately, we
> > pass the wrong (NULL) device node to of_node_full_name, resulting in
> > unhelpful messages such as:
> >
> >   hw perfevents: Failed to parse <no-node>/interrupt-affinity[0]
> >
> > This patch fixes the name to that of the pmu node.
> >
> > Fixes: 9fd85eb502a7 (ARM: pmu: add support for interrupt-affinity property)
> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > ---
> >   arch/arm/kernel/perf_event_cpu.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/kernel/perf_event_cpu.c b/arch/arm/kernel/perf_event_cpu.c
> > index becf7ad6eddc..213919ba326f 100644
> > --- a/arch/arm/kernel/perf_event_cpu.c
> > +++ b/arch/arm/kernel/perf_event_cpu.c
> > @@ -322,7 +322,7 @@ static int of_pmu_irq_cfg(struct platform_device *pdev)
> >   				      i);
> >   		if (!dn) {
> >   			pr_warn("Failed to parse %s/interrupt-affinity[%d]\n",
> > -				of_node_full_name(dn), i);
> > +				of_node_full_name(pdev->dev.of_node), i);
> >   			break;
> 
> With old DT we will see this message and one might think perf is broken.
> But since the code still assumes SPIs are listed in order of *logical*
> CPU number and continues to work, does it make sense to update the
> warning accordingly ?

The issue is that while it may work in that configuration, it can easily
be made to not work. So even if things happen to align, we should warn
when we don't have explicit information regarding the affinity.

Do you have a suggestion for a better error message to cover that?

Thanks
Mark.

[PATCH 2/4] ARM perf: Fix the pmu node name in warning message

[Sudeep Holla]

On 01/05/15 15:07, Mark Rutland wrote:
> On Fri, May 01, 2015 at 02:59:30PM +0100, Sudeep Holla wrote:
>>
>>
>> On 23/04/15 14:50, Will Deacon wrote:
>>> With commit 9fd85eb502a7 ("ARM: pmu: add support for interrupt-affinity
>>> property"), we print a warning when we find a PMU SPI with a missing
>>> missing interrupt-affinity property in a pmu node. Unfortunately, we
>>> pass the wrong (NULL) device node to of_node_full_name, resulting in
>>> unhelpful messages such as:
>>>
>>>    hw perfevents: Failed to parse <no-node>/interrupt-affinity[0]
>>>
>>> This patch fixes the name to that of the pmu node.
>>>
>>> Fixes: 9fd85eb502a7 (ARM: pmu: add support for interrupt-affinity property)
>>> Signed-off-by: Will Deacon <will.deacon@arm.com>
>>> ---
>>>    arch/arm/kernel/perf_event_cpu.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/kernel/perf_event_cpu.c b/arch/arm/kernel/perf_event_cpu.c
>>> index becf7ad6eddc..213919ba326f 100644
>>> --- a/arch/arm/kernel/perf_event_cpu.c
>>> +++ b/arch/arm/kernel/perf_event_cpu.c
>>> @@ -322,7 +322,7 @@ static int of_pmu_irq_cfg(struct platform_device *pdev)
>>>    				      i);
>>>    		if (!dn) {
>>>    			pr_warn("Failed to parse %s/interrupt-affinity[%d]\n",
>>> -				of_node_full_name(dn), i);
>>> +				of_node_full_name(pdev->dev.of_node), i);
>>>    			break;
>>
>> With old DT we will see this message and one might think perf is broken.
>> But since the code still assumes SPIs are listed in order of *logical*
>> CPU number and continues to work, does it make sense to update the
>> warning accordingly ?
>
> The issue is that while it may work in that configuration, it can easily
> be made to not work. So even if things happen to align, we should warn
> when we don't have explicit information regarding the affinity.
>
> Do you have a suggestion for a better error message to cover that?
>

Yes I thought it could be reworded but since that might make people to
ignore the warning and fail to fix DT. Also as Will suggested in the
other mail we need a way to detect correctness of routing to give more
appropriate message. So it's better to leave it as is.

Regards,
Sudeep