| Message ID | 20161024205649.32430-1-robh@kernel.org |
|---|---|
| State | New |
| Headers | show |
Hi Rob, On Mon, Oct 24, 2016 at 10:56 PM, Rob Herring <robh@kernel.org> wrote: > Commit 761ed4a94582 ('tty: serial_core: convert uart_close to use > tty_port_close') started setting the ttyport console flag for serial > drivers. This is causing crashes, hangs, or garbage output on several > platforms because the serial shutdown is skipped and IRQs are left > enabled. No, for me it hangs because uart_tty_port_shutdown() is _not_ skipped when the UART is used as a serial console. Writing to the serial port while it's shut down hangs. > Partially revert commit 761ed4a94582 and drop reporting UART tty_ports > as a console leaving the console handling to the serial_core as it was > before. > > Fixes: 761ed4a94582ab29 ("tty: serial_core: convert uart_close to use tty_port_close") > Reported-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > Reported-by: Mike Galbraith <umgwanakikbuti@gmail.com> > Reported-by: Mugunthan V N <mugunthanvnm@ti.com> > Cc: Peter Hurley <peter@hurleysoftware.com> > Cc: Geert Uytterhoeven <geert+renesas@glider.be> > Cc: Alan Cox <alan@linux.intel.com> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Jiri Slaby <jslaby@suse.com> > Cc: linux-serial@vger.kernel.org > Signed-off-by: Rob Herring <robh@kernel.org> > --- > Geert's fix seems to be inadequate for some systems as Mike reported[1]. > This fix works in Mike's case. > > Geert, Mugunthan, Can you all test this is enough to fix your issues. This doesn't work for me, as port->console is still false. It should be true when used as a serial console, to make sure tty_port_shutdown() skips an active serial console. Mike: I see you are using a PC, while I'm using an ARM board (with DT). Are you using a serial console? If yes, what's the value of port->console before and after the call to uart_console() that Rob's patch below removes? Thanks! > [1] https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1254753.html > > drivers/tty/serial/serial_core.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c > index 6e4f63627479..664c99aeeca5 100644 > --- a/drivers/tty/serial/serial_core.c > +++ b/drivers/tty/serial/serial_core.c > @@ -2746,8 +2746,6 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport) > uport->cons = drv->cons; > uport->minor = drv->tty_driver->minor_start + uport->line; > > - port->console = uart_console(uport); > - > /* > * If this port is a console, then the spinlock is already > * initialised. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Oct 25, 2016 at 7:55 AM, Mike Galbraith <umgwanakikbuti@gmail.com> wrote: > On Tue, 2016-10-25 at 12:40 +0200, Geert Uytterhoeven wrote: > >> Mike: I see you are using a PC, while I'm using an ARM board (with DT). >> Are you using a serial console? If yes, what's the value of port->console >> before and after the call to uart_console() that Rob's patch below removes? > > Well, it's a bit larger than the average PC, it's a 64 core HP DL980, > and my console is its virtual serial port. > > [ 15.217487] Serial: 8250/16550 driver, 32 ports, IRQ sharing disabled > [ 15.228212] MIKE port:ffff880276400000 port->console:0 > [ 15.240889] MIKE port:ffff880276400000 port->console:0 > [ 15.269606] 00:03: ttyS0 at I/O 0x3f8 (irq = 4, base_baud = 115200) is a 16550A > [ 15.282538] MIKE port:ffff8802764001a8 port->console:0 > [ 15.293195] MIKE port:ffff8802764001a8 port->console:1 > [ 15.322806] serial8250: ttyS1 at I/O 0x2f8 (irq = 3, base_baud = 115200) is a 16550A > (30 more calls, all 0 pre 0 post) > > That's with the below from Jiri Slaby applied to quell the reboot time > explosion. This alone is enough to fix things for you? Rob -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2016-10-25 at 08:25 -0500, Rob Herring wrote: > On Tue, Oct 25, 2016 at 7:55 AM, Mike Galbraith > <umgwanakikbuti@gmail.com> wrote: > > On Tue, 2016-10-25 at 12:40 +0200, Geert Uytterhoeven wrote: > > > > > Mike: I see you are using a PC, while I'm using an ARM board (with DT). > > > Are you using a serial console? If yes, what's the value of port->console > > > before and after the call to uart_console() that Rob's patch below removes? > > > > Well, it's a bit larger than the average PC, it's a 64 core HP DL980, > > and my console is its virtual serial port. > > > > [ 15.217487] Serial: 8250/16550 driver, 32 ports, IRQ sharing disabled > > [ 15.228212] MIKE port:ffff880276400000 port->console:0 > > [ 15.240889] MIKE port:ffff880276400000 port->console:0 > > [ 15.269606] 00:03: ttyS0 at I/O 0x3f8 (irq = 4, base_baud = 115200) is a 16550A > > [ 15.282538] MIKE port:ffff8802764001a8 port->console:0 > > [ 15.293195] MIKE port:ffff8802764001a8 port->console:1 > > [ 15.322806] serial8250: ttyS1 at I/O 0x2f8 (irq = 3, base_baud = 115200) is a 16550A > > (30 more calls, all 0 pre 0 post) > > > > That's with the below from Jiri Slaby applied to quell the reboot time > > explosion. > > This alone is enough to fix things for you? Yup. -Mike -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index 6e4f63627479..664c99aeeca5 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -2746,8 +2746,6 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport) uport->cons = drv->cons; uport->minor = drv->tty_driver->minor_start + uport->line; - port->console = uart_console(uport); - /* * If this port is a console, then the spinlock is already * initialised.
Commit 761ed4a94582 ('tty: serial_core: convert uart_close to use tty_port_close') started setting the ttyport console flag for serial drivers. This is causing crashes, hangs, or garbage output on several platforms because the serial shutdown is skipped and IRQs are left enabled. Partially revert commit 761ed4a94582 and drop reporting UART tty_ports as a console leaving the console handling to the serial_core as it was before. Fixes: 761ed4a94582ab29 ("tty: serial_core: convert uart_close to use tty_port_close") Reported-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> Reported-by: Mike Galbraith <umgwanakikbuti@gmail.com> Reported-by: Mugunthan V N <mugunthanvnm@ti.com> Cc: Peter Hurley <peter@hurleysoftware.com> Cc: Geert Uytterhoeven <geert+renesas@glider.be> Cc: Alan Cox <alan@linux.intel.com> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Jiri Slaby <jslaby@suse.com> Cc: linux-serial@vger.kernel.org Signed-off-by: Rob Herring <robh@kernel.org> --- Geert's fix seems to be inadequate for some systems as Mike reported[1]. This fix works in Mike's case. Geert, Mugunthan, Can you all test this is enough to fix your issues. Rob [1] https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1254753.html drivers/tty/serial/serial_core.c | 2 -- 1 file changed, 2 deletions(-) -- 2.10.1 -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html