Skip to content

Commit f4e6d51

Browse files
andy-shevgregkh
authored andcommitted
serial: 8250: Check UPF_IRQ_SHARED in advance
commit 7febbcb upstream. The commit 54e53b2 ("tty: serial: 8250: pass IRQ shared flag to UART ports") nicely explained the problem: ---8<---8<--- On some systems IRQ lines between multiple UARTs might be shared. If so, the irqflags have to be configured accordingly. The reason is: The 8250 port startup code performs IRQ tests *before* the IRQ handler for that particular port is registered. This is performed in serial8250_do_startup(). This function checks whether IRQF_SHARED is configured and only then disables the IRQ line while testing. This test is performed upon each open() of the UART device. Imagine two UARTs share the same IRQ line: On is already opened and the IRQ is active. When the second UART is opened, the IRQ line has to be disabled while performing IRQ tests. Otherwise an IRQ might handler might be invoked, but the IRQ itself cannot be handled, because the corresponding handler isn't registered, yet. That's because the 8250 code uses a chain-handler and invokes the corresponding port's IRQ handling routines himself. Unfortunately this IRQF_SHARED flag isn't configured for UARTs probed via device tree even if the IRQs are shared. This way, the actual and shared IRQ line isn't disabled while performing tests and the kernel correctly detects a spurious IRQ. So, adding this flag to the DT probe solves the issue. Note: The UPF_SHARE_IRQ flag is configured unconditionally. Therefore, the IRQF_SHARED flag can be set unconditionally as well. Example stack trace by performing `echo 1 > /dev/ttyS2` on a non-patched system: |irq 85: nobody cared (try booting with the "irqpoll" option) | [...] |handlers: |[<ffff0000080fc628>] irq_default_primary_handler threaded [<ffff00000855fbb8>] serial8250_interrupt |Disabling IRQ #85 ---8<---8<--- But unfortunately didn't fix the root cause. Let's try again here by moving IRQ flag assignment from serial_link_irq_chain() to serial8250_do_startup(). This should fix the similar issue reported for 8250_pnp case. Since this change we don't need to have custom solutions in 8250_aspeed_vuart and 8250_of drivers, thus, drop them. Fixes: 1c2f049 ("serial: 8250: add IRQ trigger support") Reported-by: Li RongQing <[email protected]> Cc: Kurt Kanzenbach <[email protected]> Cc: Vikram Pandita <[email protected]> Signed-off-by: Andy Shevchenko <[email protected]> Cc: stable <[email protected]> Acked-by: Kurt Kanzenbach <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent f28ec25 commit f4e6d51

File tree

4 files changed

+6
-5
lines changed

4 files changed

+6
-5
lines changed

drivers/tty/serial/8250/8250_aspeed_vuart.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,6 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
375375
port.port.line = rc;
376376

377377
port.port.irq = irq_of_parse_and_map(np, 0);
378-
port.port.irqflags = IRQF_SHARED;
379378
port.port.handle_irq = aspeed_vuart_handle_irq;
380379
port.port.iotype = UPIO_MEM;
381380
port.port.type = PORT_16550A;

drivers/tty/serial/8250/8250_core.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ static int serial_link_irq_chain(struct uart_8250_port *up)
177177
struct hlist_head *h;
178178
struct hlist_node *n;
179179
struct irq_info *i;
180-
int ret, irq_flags = up->port.flags & UPF_SHARE_IRQ ? IRQF_SHARED : 0;
180+
int ret;
181181

182182
mutex_lock(&hash_mutex);
183183

@@ -212,9 +212,8 @@ static int serial_link_irq_chain(struct uart_8250_port *up)
212212
INIT_LIST_HEAD(&up->list);
213213
i->head = &up->list;
214214
spin_unlock_irq(&i->lock);
215-
irq_flags |= up->port.irqflags;
216215
ret = request_irq(up->port.irq, serial8250_interrupt,
217-
irq_flags, up->port.name, i);
216+
up->port.irqflags, up->port.name, i);
218217
if (ret < 0)
219218
serial_do_unlink(i, up);
220219
}

drivers/tty/serial/8250/8250_of.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,6 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
171171

172172
port->type = type;
173173
port->uartclk = clk;
174-
port->irqflags |= IRQF_SHARED;
175174

176175
if (of_property_read_bool(np, "no-loopback-test"))
177176
port->flags |= UPF_SKIP_TEST;

drivers/tty/serial/8250/8250_port.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2253,6 +2253,10 @@ int serial8250_do_startup(struct uart_port *port)
22532253
}
22542254
}
22552255

2256+
/* Check if we need to have shared IRQs */
2257+
if (port->irq && (up->port.flags & UPF_SHARE_IRQ))
2258+
up->port.irqflags |= IRQF_SHARED;
2259+
22562260
if (port->irq && !(up->port.flags & UPF_NO_THRE_TEST)) {
22572261
unsigned char iir1;
22582262
/*

0 commit comments

Comments
 (0)