diff mbox series

[10/13] irqchip/armada-370-xp: Fix reenabling last per-CPU interrupt

Message ID 20240715105156.18388-11-kabel@kernel.org (mailing list archive)
State New, archived
Headers show
Series armada-370-xp irqchip updates round 5 | expand

Commit Message

Marek Behún July 15, 2024, 10:51 a.m. UTC
The number of per-CPU interrupts is 29 (0 to 28). This is described by
the constant MPIC_MAX_PER_CPU_IRQS, set to 28 (the maximum per-CPU
interrupt).

Commit 0fa4ce746d1d ("irqchip/armada-370-xp: Re-enable per-CPU
interrupts at resume time") used the constant incorrectly in the
for-loop, it used the operator < instead of <=, causing it to iterate
only the first 28 interrupts (0 to 27), ignoring the last, 28th,
per-CPU interrupt.

To avoid this kind of confusions, fix this issue by renaming the constant
to MPIC_PER_CPU_IRQS_NR and set it to 29, the number of per-CPU IRQs.
Update its use in mpic_is_percpu_irq() accordingly.

Fixes: 0fa4ce746d1d ("irqchip/armada-370-xp: Re-enable per-CPU interrupts at resume time")
Signed-off-by: Marek Behún <kabel@kernel.org>
---
 drivers/irqchip/irq-armada-370-xp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Thomas Gleixner July 28, 2024, 9:47 p.m. UTC | #1
On Mon, Jul 15 2024 at 12:51, Marek Behún wrote:
> The number of per-CPU interrupts is 29 (0 to 28). This is described by
> the constant MPIC_MAX_PER_CPU_IRQS, set to 28 (the maximum per-CPU
> interrupt).
>
> Commit 0fa4ce746d1d ("irqchip/armada-370-xp: Re-enable per-CPU
> interrupts at resume time") used the constant incorrectly in the
> for-loop, it used the operator < instead of <=, causing it to iterate
> only the first 28 interrupts (0 to 27), ignoring the last, 28th,
> per-CPU interrupt.
>
> To avoid this kind of confusions, fix this issue by renaming the constant
> to MPIC_PER_CPU_IRQS_NR and set it to 29, the number of per-CPU IRQs.
> Update its use in mpic_is_percpu_irq() accordingly.
>
> Fixes: 0fa4ce746d1d ("irqchip/armada-370-xp: Re-enable per-CPU interrupts at resume time")
> Signed-off-by: Marek Behún <kabel@kernel.org>

Please don't hide fixes in the middle of a refactoring series. Split
them out and make sure that they can be applied w/o prerequisites so
they can be easily backported.

Thanks,

        tglx
Marek Behún July 29, 2024, 1:28 p.m. UTC | #2
On Sun, Jul 28, 2024 at 11:47:30PM +0200, Thomas Gleixner wrote:
> On Mon, Jul 15 2024 at 12:51, Marek Behún wrote:
> > The number of per-CPU interrupts is 29 (0 to 28). This is described by
> > the constant MPIC_MAX_PER_CPU_IRQS, set to 28 (the maximum per-CPU
> > interrupt).
> >
> > Commit 0fa4ce746d1d ("irqchip/armada-370-xp: Re-enable per-CPU
> > interrupts at resume time") used the constant incorrectly in the
> > for-loop, it used the operator < instead of <=, causing it to iterate
> > only the first 28 interrupts (0 to 27), ignoring the last, 28th,
> > per-CPU interrupt.
> >
> > To avoid this kind of confusions, fix this issue by renaming the constant
> > to MPIC_PER_CPU_IRQS_NR and set it to 29, the number of per-CPU IRQs.
> > Update its use in mpic_is_percpu_irq() accordingly.
> >
> > Fixes: 0fa4ce746d1d ("irqchip/armada-370-xp: Re-enable per-CPU interrupts at resume time")
> > Signed-off-by: Marek Behún <kabel@kernel.org>
> 
> Please don't hide fixes in the middle of a refactoring series. Split
> them out and make sure that they can be applied w/o prerequisites so
> they can be easily backported.

Hi Thomas,

but now that you applied my previous refactors to irq/core, even if I
rebase the patch on top of those, it won't apply to stable kernels.

I can either:
- ignore this issue and post the patch alone as a fixes patch
- rebase on top of v6.11-rc1 and send you updated version of patches
  you already applied to irq/core
- drop the Fixes tag (the 29th per-CPU interrupt is not used in any
  real device-tree)
Thomas Gleixner July 29, 2024, 1:36 p.m. UTC | #3
On Mon, Jul 29 2024 at 15:28, Marek Behún wrote:
> On Sun, Jul 28, 2024 at 11:47:30PM +0200, Thomas Gleixner wrote:
>> On Mon, Jul 15 2024 at 12:51, Marek Behún wrote:
>> > The number of per-CPU interrupts is 29 (0 to 28). This is described by
>> > the constant MPIC_MAX_PER_CPU_IRQS, set to 28 (the maximum per-CPU
>> > interrupt).
>> >
>> > Commit 0fa4ce746d1d ("irqchip/armada-370-xp: Re-enable per-CPU
>> > interrupts at resume time") used the constant incorrectly in the
>> > for-loop, it used the operator < instead of <=, causing it to iterate
>> > only the first 28 interrupts (0 to 27), ignoring the last, 28th,
>> > per-CPU interrupt.
>> >
>> > To avoid this kind of confusions, fix this issue by renaming the constant
>> > to MPIC_PER_CPU_IRQS_NR and set it to 29, the number of per-CPU IRQs.
>> > Update its use in mpic_is_percpu_irq() accordingly.
>> >
>> > Fixes: 0fa4ce746d1d ("irqchip/armada-370-xp: Re-enable per-CPU interrupts at resume time")
>> > Signed-off-by: Marek Behún <kabel@kernel.org>
>> 
>> Please don't hide fixes in the middle of a refactoring series. Split
>> them out and make sure that they can be applied w/o prerequisites so
>> they can be easily backported.
>
> Hi Thomas,
>
> but now that you applied my previous refactors to irq/core, even if I
> rebase the patch on top of those, it won't apply to stable kernels.
>
> I can either:
> - ignore this issue and post the patch alone as a fixes patch
> - rebase on top of v6.11-rc1 and send you updated version of patches
>   you already applied to irq/core
> - drop the Fixes tag (the 29th per-CPU interrupt is not used in any
>   real device-tree)

Drop the fixes tag then.
diff mbox series

Patch

diff --git a/drivers/irqchip/irq-armada-370-xp.c b/drivers/irqchip/irq-armada-370-xp.c
index e8daa967e5fc..78d9c7699972 100644
--- a/drivers/irqchip/irq-armada-370-xp.c
+++ b/drivers/irqchip/irq-armada-370-xp.c
@@ -133,7 +133,7 @@ 
 #define MPIC_INT_FABRIC_MASK			0x54
 #define MPIC_INT_CAUSE_PERF(cpu)		BIT(cpu)
 
-#define MPIC_MAX_PER_CPU_IRQS			28
+#define MPIC_PER_CPU_IRQS_NR			29
 
 /* IPI and MSI interrupt definitions for IPI platforms */
 #define IPI_DOORBELL_NR				8
@@ -195,7 +195,7 @@  static inline bool mpic_is_ipi_available(struct mpic *mpic)
 
 static inline bool mpic_is_percpu_irq(irq_hw_number_t hwirq)
 {
-	return hwirq <= MPIC_MAX_PER_CPU_IRQS;
+	return hwirq < MPIC_PER_CPU_IRQS_NR;
 }
 
 /*
@@ -546,7 +546,7 @@  static void mpic_smp_cpu_init(struct mpic *mpic)
 static void mpic_reenable_percpu(struct mpic *mpic)
 {
 	/* Re-enable per-CPU interrupts that were enabled before suspend */
-	for (irq_hw_number_t i = 0; i < MPIC_MAX_PER_CPU_IRQS; i++) {
+	for (irq_hw_number_t i = 0; i < MPIC_PER_CPU_IRQS_NR; i++) {
 		unsigned int virq = irq_linear_revmap(mpic->domain, i);
 		struct irq_data *d;