Re: [PATCH] m68k: coldfire: Prevent spurious interrupts when masking IMR

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jean-Michel,

On Tue, 4 Feb 2025 at 19:38, Jean-Michel Hautbois
<jeanmichel.hautbois@xxxxxxxxxx> wrote:
The ColdFire interrupt controller can generate spurious interrupts if an
interrupt source is masked in the IMR while the CPU interrupt priority
mask (SR[I]) is set lower than the interrupt level.

The reference manual states:

To avoid this situation for interrupts sources with levels 1-6, first
write a higher level interrupt mask to the status register, before
setting the mask in the IMR or the module’s interrupt mask register.
After the mask is set, return the interrupt mask in the status register
to its previous value.

It can be tested like this:
- Prepare a iperf3 server on the coldfire target (iperf3 -s -D)
- Start a high priority cyclictest:
    cyclictest --secaligned -m -p 99 -i 2500 -q
- Start iperf3 -c $COLDFIRE_IP -t 0

After a few seconds the dmesg may display:
[   84.784301] irq 24, desc: dbc502da, depth: 1, count: 0, unhandled: 0
[   84.784455] ->handle_irq():  0ba0aca3, handle_bad_irq+0x0/0x1e0
[   84.784610] ->irq_data.chip(): c6779d4f, 0x41652544
[   84.784719] ->action(): 00000000
[   84.784770] unexpected IRQ trap at vector 18

With this patch, I never saw it in a few hours testing.

Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@xxxxxxxxxx>

Thanks for your patch!

--- a/arch/m68k/coldfire/intc-simr.c
+++ b/arch/m68k/coldfire/intc-simr.c
@@ -58,6 +58,14 @@ static inline unsigned int irq2ebit(unsigned int irq)

 #endif

+static inline void intc_irq_setlevel(unsigned long level)
+{
+       asm volatile ("move.w %0,%%sr"
+                     : /* no outputs */
+                     : "d" (0x2000 | ((level) << 8))
+                     : "memory");
+}
+
 /*
  *     There maybe one, two or three interrupt control units, each has 64
  *     interrupts. If there is no second or third unit then MCFINTC1_* or
@@ -67,13 +75,17 @@ static inline unsigned int irq2ebit(unsigned int irq)
 static void intc_irq_mask(struct irq_data *d)
 {
        unsigned int irq = d->irq - MCFINT_VECBASE;
+       unsigned long flags = arch_local_save_flags();

+       intc_irq_setlevel(7);

Can't all of the above just be replaced by

    unsigned long flags = arch_local_irq_save();

        if (MCFINTC2_SIMR && (irq > 127))
                __raw_writeb(irq - 128, MCFINTC2_SIMR);
        else if (MCFINTC1_SIMR && (irq > 63))
                __raw_writeb(irq - 64, MCFINTC1_SIMR);
        else
                __raw_writeb(irq, MCFINTC0_SIMR);
+
+       arch_local_irq_restore(flags);
 }

 static void intc_irq_unmask(struct irq_data *d)


Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds





[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux