diff mbox series

[1/2] irqchip/armada-370-xp: Do not touch Performance Counter Overflow on A375, A38x, A39x

Message ID 20220425113706.29310-1-pali@kernel.org (mailing list archive)
State New, archived
Headers show
Series [1/2] irqchip/armada-370-xp: Do not touch Performance Counter Overflow on A375, A38x, A39x | expand

Commit Message

Pali Rohár April 25, 2022, 11:37 a.m. UTC
Register ARMADA_370_XP_INT_FABRIC_MASK_OFFS is Armada 370 and XP specific
and on new Armada platforms it has different meaning. It does not configure
Performance Counter Overflow interrupt masking. So do not touch this
register on non-A370/XP platforms (A375, A38x and A39x).

Signed-off-by: Pali Rohár <pali@kernel.org>
Cc: stable@vger.kernel.org
---
 drivers/irqchip/irq-armada-370-xp.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Comments

Andrew Lunn April 29, 2022, 12:23 p.m. UTC | #1
On Mon, Apr 25, 2022 at 01:37:05PM +0200, Pali Rohár wrote:
> Register ARMADA_370_XP_INT_FABRIC_MASK_OFFS is Armada 370 and XP specific
> and on new Armada platforms it has different meaning. It does not configure
> Performance Counter Overflow interrupt masking. So do not touch this
> register on non-A370/XP platforms (A375, A38x and A39x).

Hi Pali

Do the Armada 375, 38x and 39x have an over flow interrupt? I assume
not.

Does this need a fixes tag? Should it be back ported in stable?

     Andrew
Pali Rohár April 29, 2022, 1:05 p.m. UTC | #2
On Friday 29 April 2022 14:23:08 Andrew Lunn wrote:
> On Mon, Apr 25, 2022 at 01:37:05PM +0200, Pali Rohár wrote:
> > Register ARMADA_370_XP_INT_FABRIC_MASK_OFFS is Armada 370 and XP specific
> > and on new Armada platforms it has different meaning. It does not configure
> > Performance Counter Overflow interrupt masking. So do not touch this
> > register on non-A370/XP platforms (A375, A38x and A39x).
> 
> Hi Pali
> 
> Do the Armada 375, 38x and 39x have an over flow interrupt? I assume
> not.

Hello! According to documentation there is something named performance
counter interrupt, but it is in different register... and this register
is not per-cpu.

> Does this need a fixes tag? Should it be back ported in stable?

git blame show that this functionality appeared in commit 28da06dfd9e4
("irqchip: armada-370-xp: Enable the PMU interrupts").
Andrew Lunn April 29, 2022, 10:23 p.m. UTC | #3
On Fri, Apr 29, 2022 at 03:05:24PM +0200, Pali Rohár wrote:
> On Friday 29 April 2022 14:23:08 Andrew Lunn wrote:
> > On Mon, Apr 25, 2022 at 01:37:05PM +0200, Pali Rohár wrote:
> > > Register ARMADA_370_XP_INT_FABRIC_MASK_OFFS is Armada 370 and XP specific
> > > and on new Armada platforms it has different meaning. It does not configure
> > > Performance Counter Overflow interrupt masking. So do not touch this
> > > register on non-A370/XP platforms (A375, A38x and A39x).
> > 
> > Hi Pali
> > 
> > Do the Armada 375, 38x and 39x have an over flow interrupt? I assume
> > not.
> 
> Hello! According to documentation there is something named performance
> counter interrupt, but it is in different register... and this register
> is not per-cpu.

O.K, not something which can be quickly added. 

> > Does this need a fixes tag? Should it be back ported in stable?
> 
> git blame show that this functionality appeared in commit 28da06dfd9e4
> ("irqchip: armada-370-xp: Enable the PMU interrupts").

It is more a question of:

 o It must fix a real bug that bothers people (not a, “This could be a
   problem…” type thing).

From https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html

Have you seen bad things happen because of this?

     Andrew
Pali Rohár April 29, 2022, 10:31 p.m. UTC | #4
On Saturday 30 April 2022 00:23:34 Andrew Lunn wrote:
> On Fri, Apr 29, 2022 at 03:05:24PM +0200, Pali Rohár wrote:
> > On Friday 29 April 2022 14:23:08 Andrew Lunn wrote:
> > > On Mon, Apr 25, 2022 at 01:37:05PM +0200, Pali Rohár wrote:
> > > > Register ARMADA_370_XP_INT_FABRIC_MASK_OFFS is Armada 370 and XP specific
> > > > and on new Armada platforms it has different meaning. It does not configure
> > > > Performance Counter Overflow interrupt masking. So do not touch this
> > > > register on non-A370/XP platforms (A375, A38x and A39x).
> > > 
> > > Hi Pali
> > > 
> > > Do the Armada 375, 38x and 39x have an over flow interrupt? I assume
> > > not.
> > 
> > Hello! According to documentation there is something named performance
> > counter interrupt, but it is in different register... and this register
> > is not per-cpu.
> 
> O.K, not something which can be quickly added. 
> 
> > > Does this need a fixes tag? Should it be back ported in stable?
> > 
> > git blame show that this functionality appeared in commit 28da06dfd9e4
> > ("irqchip: armada-370-xp: Enable the PMU interrupts").
> 
> It is more a question of:
> 
>  o It must fix a real bug that bothers people (not a, “This could be a
>    problem…” type thing).
> 
> From https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> 
> Have you seen bad things happen because of this?
> 
>      Andrew

On those new Armada SoCs that register is used for unmasking interrupts
from another hierarchy. Until you start using those new interrupts there
probably could not be any issue. But in any case unmasking unwanted
interrupt is not a wise idea...

I spotted this because I started using those extended interrupts from
another hierarchy. I'm planning to send a patch which properly export
them via DTS. Consumer would be pci-mvebu.c driver as in this extended
hierarchy are PCIe AER and PME interrupts.

Anyway, Fixes tag is probably better in this situation than stable tag.
So would it be fine to replace it by?

Fixes: 28da06dfd9e4 ("irqchip: armada-370-xp: Enable the PMU interrupts")
Andrew Lunn April 29, 2022, 10:39 p.m. UTC | #5
> On those new Armada SoCs that register is used for unmasking interrupts
> from another hierarchy. Until you start using those new interrupts there
> probably could not be any issue. But in any case unmasking unwanted
> interrupt is not a wise idea...

O.K. Please add the Fixes tag.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew
diff mbox series

Patch

diff --git a/drivers/irqchip/irq-armada-370-xp.c b/drivers/irqchip/irq-armada-370-xp.c
index 5b8d571c041d..1120084cba09 100644
--- a/drivers/irqchip/irq-armada-370-xp.c
+++ b/drivers/irqchip/irq-armada-370-xp.c
@@ -308,7 +308,16 @@  static inline int armada_370_xp_msi_init(struct device_node *node,
 
 static void armada_xp_mpic_perf_init(void)
 {
-	unsigned long cpuid = cpu_logical_map(smp_processor_id());
+	unsigned long cpuid;
+
+	/*
+	 * This Performance Counter Overflow interrupt is specific for
+	 * Armada 370 and XP. It is not available on Armada 375, 38x and 39x.
+	 */
+	if (!of_machine_is_compatible("marvell,armada-370-xp"))
+		return;
+
+	cpuid = cpu_logical_map(smp_processor_id());
 
 	/* Enable Performance Counter Overflow interrupts */
 	writel(ARMADA_370_XP_INT_CAUSE_PERF(cpuid),