Re: [PATCH] net/cadence/macb: clear interrupts simply and correctly
From: Sören Brinkmann <hidden>
Date: 2014-06-12 15:48:17
Also in:
lkml
Hi Jongsung, On Thu, 2014-06-12 at 05:50PM +0900, Jongsung Kim wrote:
The "Rx used bit read" interrupt is enabled but not cleared for some systems with the ISR (Interrupt Status Register) configured as clear- on-write.
Does this interrupt need to be enabled? There is nothing checking that bit and handling this IRQ in the handler, AFAICT. And you solve this by simply clearing the bit. So, I wonder whether not enabling this IRQ in the first place would solve things too.
quoted hunk ↗ jump to hunk
This interrupt may be asserted when the CPU does not handle Rx-complete interrupts for a long time. (e.g., if the CPU is stopped by debugger) Once asserted, it'll not be cleared, and the CPU will loop infinitly in the interrupt handler. This patch forces to use a dedicated function for reading the ISR, and the function clears it if clear-on-write. So the ISR is always cleared after read, regardless of clear-on-write configuration. Reported-by: Hayun Hwang <redacted> Signed-off-by: Youngkyu Choi <redacted> Signed-off-by: Jongsung Kim <redacted> Tested-by: Hayun Hwang <redacted> --- drivers/net/ethernet/cadence/macb.c | 37 ++++++++++++++-------------------- 1 files changed, 15 insertions(+), 22 deletions(-)diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c index e9daa07..21cc022 100644 --- a/drivers/net/ethernet/cadence/macb.c +++ b/drivers/net/ethernet/cadence/macb.c@@ -98,6 +98,16 @@ static void *macb_rx_buffer(struct macb *bp, unsigned int index) return bp->rx_buffers + bp->rx_buffer_size * macb_rx_ring_wrap(index); } +static u32 macb_read_isr(struct macb *bp) +{ + u32 status = macb_readl(bp, ISR); + + if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE) + macb_writel(bp, ISR, status); + + return status; +} + void macb_set_hwaddr(struct macb *bp) { u32 bottom;@@ -552,9 +562,6 @@ static void macb_tx_interrupt(struct macb *bp) status = macb_readl(bp, TSR); macb_writel(bp, TSR, status); - if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE) - macb_writel(bp, ISR, MACB_BIT(TCOMP)); - netdev_vdbg(bp->dev, "macb_tx_interrupt status = 0x%03lx\n", (unsigned long)status);@@ -883,13 +890,10 @@ static int macb_poll(struct napi_struct *napi, int budget) /* Packets received while interrupts were disabled */ status = macb_readl(bp, RSR);
This is now clearing all IRQ flags which is probably not what we want here. This is handling RX only. We still want the non-RX interrupts to go to the actual interrupt service routing. Sören