The GPIO interrupt controller on the espressobin board only supports edge 
interrupts.
If one enables the use of hardware interrupts in the device tree for the 
88E6341, it is
possible to miss an edge.  When this happens, the INTn pin on the Marvell 
switch is
stuck low and no further interrupts occur.

I found after adding debug statements to mv88e6xxx_g1_irq_thread_work() that 
there is
a race in handling device interrupts (e.g. PHY link interrupts).  Some 
interrupts are
directly cleared by reading the Global 1 status register.  However, the device 
interrupt
flag, for example, is not cleared until all the unmasked SERDES and PHY ports 
are serviced.
This is done by reading the relevant SERDES and PHY status register.

The code only services interrupts whose status bit is set at the time of 
reading its status
register.  If an interrupt event occurs after its status is read and before all 
interrupts
are serviced, then this event will not be serviced and the INTn output pin will 
remain low.

This is not a problem with polling or level interrupts since the handler will 
be called
again to process the event.  However, it's a big problem when using level 
interrupts.

The fix presented here is to add a loop around the code servicing switch 
interrupts.  If
any pending interrupts remain after the current set has been handled, we loop 
and process
the new set.  If there are no pending interrupts after servicing, we are sure 
that INTn has
gone high and we will get an edge when a new event occurs.

Tested on espressobin board.

Signed-off-by:  John David Anglin <dave.ang...@bell.net>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 8dca2c949e73..12fd7ce3f1ff 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -261,6 +261,7 @@ static irqreturn_t mv88e6xxx_g1_irq_thread_work(struct 
mv88e6xxx_chip *chip)
        unsigned int sub_irq;
        unsigned int n;
        u16 reg;
+       u16 ctl1;
        int err;

        mutex_lock(&chip->reg_lock);
@@ -270,13 +271,28 @@ static irqreturn_t mv88e6xxx_g1_irq_thread_work(struct 
mv88e6xxx_chip *chip)
        if (err)
                goto out;

-       for (n = 0; n < chip->g1_irq.nirqs; ++n) {
-               if (reg & (1 << n)) {
-                       sub_irq = irq_find_mapping(chip->g1_irq.domain, n);
-                       handle_nested_irq(sub_irq);
-                       ++nhandled;
+       do {
+               for (n = 0; n < chip->g1_irq.nirqs; ++n) {
+                       if (reg & (1 << n)) {
+                               sub_irq = irq_find_mapping(chip->g1_irq.domain,
+                                                          n);
+                               handle_nested_irq(sub_irq);
+                               ++nhandled;
+                       }
                }
-       }
+
+               mutex_lock(&chip->reg_lock);
+               err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_CTL1, &ctl1);
+               if (err)
+                       goto unlock;
+               err = mv88e6xxx_g1_read(chip, MV88E6XXX_G1_STS, &reg);
+unlock:
+               mutex_unlock(&chip->reg_lock);
+               if (err)
+                       goto out;
+               ctl1 &= GENMASK(chip->g1_irq.nirqs, 0);
+       } while (reg & ctl1);
+
 out:
        return (nhandled > 0 ? IRQ_HANDLED : IRQ_NONE);
 }
-- 
2.11.0

Reply via email to