From mboxrd@z Thu Jan 1 00:00:00 1970 From: xuwei5@hisilicon.com (Wei Xu) Date: Tue, 30 Jan 2018 09:28:58 +0000 Subject: [RFC PATCH] tty: pl011: Avoid stuck-off spurious interrupts In-Reply-To: <5A6F5EDB.20405@hisilicon.com> References: <1517242181-20652-1-git-send-email-Dave.Martin@arm.com> <5A6F5EDB.20405@hisilicon.com> Message-ID: <5A703ADA.404@hisilicon.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Dave, On 2018/1/29 17:50, Wei Xu wrote: > Hi Dave, > > On 2018/1/29 16:09, Dave Martin wrote: >> Commit 9b96fbacda34 ("serial: PL011: clear pending interrupts") >> clears the RX and receive timeout interrupts on pl011 startup, to >> avoid a screaming-interrupt scenario that can occur when the >> firmware or bootloader leaves these interrupts spuriously >> asserted. >> >> This has been noted as an issue when running Linux on qemu [1]. >> >> Unfortunately, the above fix seems to lead to potential >> misbehaviour if the RX FIFO interrupt is asserted _non_ spuriously >> on driver startup, if the RX FIFO is also already full to the >> trigger level. >> >> Clearing the RX FIFO interrupt does not change the FIFO fill level. >> In this scenario, because the interrupt is now clear and because >> the FIFO is already full to the trigger level, no new assertion of >> the RX FIFO interrupt can occur unless the FIFO is drained back >> below the trigger level. This never occurs because the pl011 >> driver is waiting for an RX FIFO interrupt to tell it that there is >> something to read, and does not read the FIFO at all until that >> interrupt occurs. >> >> Thus, simply clearing "spurious" interrupts on startup may be >> misguided, since there is no way to be sure that the interrupts are >> truly spurious, and things can go wrong if they are not. >> >> This patch attempts to handle (suspected) spurious interrupts more >> robustly, by allowing the interrupt(s) to fire but quenching the >> scream. >> >> pl011_int() runs and attempts to drain the FIFO anyway just as if >> the interrupts were real. If the FIFO is already empty, great. To >> avoid a screaming spurious interrupt, the RX FIFO and timeout >> interrupts are now explicitly cleared in between committing to >> drain the RX FIFO and actually draining it. We do not have to >> worry about lost interrupts here, because we are effectively in >> polled mode inside pl011_int() until the RX FIFO becomes empty: >> >> * A new char received before the RX FIFO is fully drained will be >> drained out synchronously by pl011_int() along with the other >> chars already pending. A new char received after the RX FIFO >> is drained will result in correct RX FIFO interrupt assertion, >> because emptying the RX FIFO guarantees that the RX FIFO / >> timeout interrupt state machines are back in a sane state. >> >> * A new RX timeout before the RX FIFO is fully drained is no >> problem, because pl011_int() has already committed to emptying >> the FIFO at this point, guaranteeing that no stray chars will >> be left behind. A new RX timeout after the RX FIFO is fully >> drained will result in correct interrupt assertion. >> >> This patch does not attempt to address the case where the RX FIFO >> fills faster than it can be drained: that is a pathological >> condition that is beyond the scope of the driver to work around. >> Users cannot expect this to work unless they enable hardware flow >> control. >> >> [1] [Qemu-devel] [Qemu-arm] [PATCH] pl011: do not put into fifo >> before enabled the interruption >> https://lists.gnu.org/archive/html/qemu-devel/2018-01/msg06446.html >> >> Reported-by: Wei Xu >> Cc: Russell King >> Cc: Linus Walleij >> Cc: Peter Maydell >> Fixes: 9b96fbacda34 ("serial: PL011: clear pending interrupts") >> Signed-off-by: Dave Martin >> --- >> drivers/tty/serial/amba-pl011.c | 6 +----- >> 1 file changed, 1 insertion(+), 5 deletions(-) >> > > After commented the 1549 line in the amba-pl011.c[1] and applied this patch, > the console is not hanged any more. Sorry, it should be line 1678[1] for the 4.15-rc9. > > The UART011_ICR is cleared in the pl011_hwinit that will clear the > RX interruption as well. > Is it OK to remove 1549 as well? > Thanks! > > [1]:http://elixir.free-electrons.com/linux/v4.4/source/drivers/tty/serial/amba-pl011.c#L1549 [1]: http://elixir.free-electrons.com/linux/v4.15-rc9/source/drivers/tty/serial/amba-pl011.c#L1678 Best Regards, Wei > > Best Regards, > Wei >