[PATCH v4] serial: 8250: always disable IRQ during THRE test

[Alban Bedel]

From: Peng Zhang <zhangpeng.00@xxxxxxxxxxxxx>

commit 039d4926379b ("serial: 8250: Toggle IER bits on only after irq
has been set up") moved IRQ setup before the THRE test, in combination
with commit 205d300aea75 ("serial: 8250: change lock order in
serial8250_do_startup()") the interrupt handler can run during the
test and race with its IIR reads. This can produce wrong THRE test
results and cause spurious registration of the
serial8250_backup_timeout timer. Unconditionally disable the IRQ for
the short duration of the test and re-enable it afterwards to avoid
the race.

Cc: stable@xxxxxxxxxxxxxxx
Fixes: 039d4926379b ("serial: 8250: Toggle IER bits on only after irq has been set up")
Depends-on: 205d300aea75 ("serial: 8250: change lock order in serial8250_do_startup()")
Signed-off-by: Peng Zhang <zhangpeng.00@xxxxxxxxxxxxx>
Reviewed-by: Muchun Song <songmuchun@xxxxxxxxxxxxx>
Signed-off-by: Alban Bedel <alban.bedel@xxxxxxxxxx>
Tested-by: Maximilian Lueer <maximilian.lueer@xxxxxxxxxx>
---
Changelog:

v2: Replaced disable_irq_nosync() with disable_irq() to prevent interrupts
    that are currently being handled
v3: Added changelog
v4: Updated commit message to mention the addtional relation to commit
    205d300aea75
---

Commit 039d4926379b ("serial: 8250: Toggle IER bits on only after irq
has been set up"), in combination with 205d300aea75 ("serial: 8250:
change lock order in serial8250_do_startup()"), introduced a bug which
showed in some of our devices after updating them to kernel
5.15.169. These devices have an I2C touch screen which is behind an
UART to I2C bridge. This bug lead to an ever growing latency on the
bus, delaying the touch events by up to several seconds.

Looking for other solutions than reverting commit 039d4926379b I found
Peng Zhang's patch, backported it to 5.15 and could confirm that it
solve the high delay issue.

As this a quiet sever regression I'm taking the liberty to re-submit
Peng Zhang's patch in the hope it will be considered for inclusion. I
added the changelog requested by greg k-h's patch email bot after the
v2 submission, as well as the mention of commit 205d300aea75 requested
by Jiri Slaby after v3.

Also please note that commit 039d4926379b was backported as far back as
5.10, so quiet a few stable kernels are affected. This patch doesn't
apply as-is on older kernels, I can provide a patch for 5.15 if desired.

Alban Bedel
---
 drivers/tty/serial/8250/8250_port.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index cc94af2d578a6..a743964c9d227 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2147,8 +2147,7 @@ static void serial8250_THRE_test(struct uart_port *port)
 	if (up->port.flags & UPF_NO_THRE_TEST)
 		return;
 
-	if (port->irqflags & IRQF_SHARED)
-		disable_irq_nosync(port->irq);
+	disable_irq(port->irq);
 
 	/*
 	 * Test for UARTs that do not reassert THRE when the transmitter is idle and the interrupt
@@ -2170,8 +2169,7 @@ static void serial8250_THRE_test(struct uart_port *port)
 		serial_port_out(port, UART_IER, 0);
 	}
 
-	if (port->irqflags & IRQF_SHARED)
-		enable_irq(port->irq);
+	enable_irq(port->irq);
 
 	/*
 	 * If the interrupt is not reasserted, or we otherwise don't trust the iir, setup a timer to
-- 
2.39.5