| From e26d3526c3a8435d040aa787623c7ee51029fcbc Mon Sep 17 00:00:00 2001 |
| From: John David Anglin <dave.anglin@bell.net> |
| Date: Mon, 11 Feb 2019 13:40:21 -0500 |
| Subject: dsa: mv88e6xxx: Ensure all pending interrupts are handled prior to |
| exit |
| |
| [ Upstream commit 7c0db24cc431e2196d98a5d5ddaa9088e2fcbfe5 ] |
| |
| 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. |
| |
| Fixes: dc30c35be720 ("net: dsa: mv88e6xxx: Implement interrupt support.") |
| Signed-off-by: John David Anglin <dave.anglin@bell.net> |
| Tested-by: Andrew Lunn <andrew@lunn.ch> |
| Signed-off-by: David S. Miller <davem@davemloft.net> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| 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 258918d8a4165..9f697a5b8e3df 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, ®); |
| +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.19.1 |
| |