| Message ID | 1410377411-26656-4-git-send-email-bigeasy@linutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 09/10/2014 09:29 PM, Sebastian Andrzej Siewior wrote: > The serial8250_do_startup() function unconditionally clears the > interrupts and for that it reads from the RX-FIFO without checking if > there is a byte in the FIFO or not. This works fine on OMAP4+ HW like > AM335x or DRA7. > OMAP3630 ES1.1 (which means probably all OMAP3 and earlier) does not like > this: Hello, Sorry to wake up an old thread, but I'm affraid that this patch causes problems on Marvell 88f6282 (Kirkwood). When a caracter is received on the UART while the kernel is printing the boot messages, as soon as the kernel configures the UART for receiving (after root filesystem mount), it gets stuck printing the following message repeatedly: serial8250: too much work for irq29 Once stuck, the reception of another character allows the boot process to finish. From what I can gather, when we hit that, the UART_IIR_NO_INT is 0 (so the interrupt is raised), but the UART_LSR_DR bit is 0 as well so the UART_RX register is never read to clear the interrupt. We are using the second UART multiplexed on mpps 15 and 16. Reverting this particular patch fixes the issue. We are seing the problem on a 3.18 kernel. Regards,
Hi Nicolas, Thanks for the report. On 02/09/2015 08:34 AM, Nicolas Schichan wrote: > On 09/10/2014 09:29 PM, Sebastian Andrzej Siewior wrote: >> The serial8250_do_startup() function unconditionally clears the >> interrupts and for that it reads from the RX-FIFO without checking if >> there is a byte in the FIFO or not. This works fine on OMAP4+ HW like >> AM335x or DRA7. >> OMAP3630 ES1.1 (which means probably all OMAP3 and earlier) does not like >> this: > > Hello, > > Sorry to wake up an old thread, but I'm affraid that this patch causes > problems on Marvell 88f6282 (Kirkwood). > > When a caracter is received on the UART while the kernel is printing > the boot messages, as soon as the kernel configures the UART for > receiving (after root filesystem mount), it gets stuck printing the > following message repeatedly: > > serial8250: too much work for irq29 > > Once stuck, the reception of another character allows the boot process > to finish. > > From what I can gather, when we hit that, the UART_IIR_NO_INT is 0 (so the > interrupt is raised), but the UART_LSR_DR bit is 0 as well so the UART_RX > register is never read to clear the interrupt. The "too much work" message means serial8250_handle_irq() is returning 0, ie., not handled. Which in turn means IIR indicates no interrupt is pending (UART_IIR_NO_INT == 1). Can you log the register values for LSR and IIR at both patch locations in serial8250_do_startup()? (I can get you a debug patch, if necessary. Let me know) Regards, Peter Hurley > We are using the second UART multiplexed on mpps 15 and 16. > > Reverting this particular patch fixes the issue. > > We are seing the problem on a 3.18 kernel.
On 02/10/2015 12:34 AM, Peter Hurley wrote: > The "too much work" message means serial8250_handle_irq() is returning 0, > ie., not handled. Which in turn means IIR indicates no interrupt is pending > (UART_IIR_NO_INT == 1). The OMAP UART for instance have two possible TX-IRQ handling. The default is to fire the interrupt once there is room for at least one byte in the FIFO. I set the OMAP_UART_SCR_TX_EMPTY bit to get the same behavior as the 8250 driver expects (interrupt while the FIFO is empty). Without this bit set I've seen the message from time to time. Sebastian
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c index 3cf5c98013e4..547afde9fdda 100644 --- a/drivers/tty/serial/8250/8250_core.c +++ b/drivers/tty/serial/8250/8250_core.c @@ -2088,8 +2088,8 @@ int serial8250_do_startup(struct uart_port *port) /* * Clear the interrupt registers. */ - serial_port_in(port, UART_LSR); - serial_port_in(port, UART_RX); + if (serial_port_in(port, UART_LSR) & UART_LSR_DR) + serial_port_in(port, UART_RX); serial_port_in(port, UART_IIR); serial_port_in(port, UART_MSR); @@ -2250,8 +2250,8 @@ int serial8250_do_startup(struct uart_port *port) * saved flags to avoid getting false values from polling * routines or the previous session. */ - serial_port_in(port, UART_LSR); - serial_port_in(port, UART_RX); + if (serial_port_in(port, UART_LSR) & UART_LSR_DR) + serial_port_in(port, UART_RX); serial_port_in(port, UART_IIR); serial_port_in(port, UART_MSR); up->lsr_saved_flags = 0; @@ -2344,7 +2344,8 @@ void serial8250_do_shutdown(struct uart_port *port) * Read data port to reset things, and then unlink from * the IRQ chain. */ - serial_port_in(port, UART_RX); + if (serial_port_in(port, UART_LSR) & UART_LSR_DR) + serial_port_in(port, UART_RX); serial8250_rpm_put(up); del_timer_sync(&up->timer);