Messages in this thread | | | Date | Fri, 21 Mar 2014 09:06:18 -0400 | From | Peter Hurley <> | Subject | Re: [PATCH resend] serial_core: Fix pm imbalance on unbind |
| |
On 03/21/2014 05:08 AM, Geert Uytterhoeven wrote: > From: Geert Uytterhoeven <geert+renesas@linux-m68k.org> > > When a serial port is closed, uart_close() takes care of shutting down the > hardware, and powering it down. > > When a serial port is unbound while in use, uart_close() bypasses all of > this, as this is supposed to be done through uart_hangup() (invoked via > tty_vhangup() in uart_remove_one_port()). > > However, uart_hangup() does not set the hardware's power state, leaving it > powered up. This may also lead to unbounded nesting counts in clock and > power management, depending on their internal implementation. > > Make sure to power down the port in uart_hangup(), except when the port is > used as a serial console. For serial consoles, this must be postponed until > after their deregistration in uart_remove_one_port() (symmetry with > registration in uart_configure_port(), invoked from uart_add_one_port()). > > After this, the module clock used by the sh-sci driver is disabled on > unbind while the serial port is in use. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org> > --- > drivers/tty/serial/serial_core.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c > index 2cf5649a6dc0..56dda84f82a5 100644 > --- a/drivers/tty/serial/serial_core.c > +++ b/drivers/tty/serial/serial_core.c > @@ -1452,6 +1452,8 @@ static void uart_hangup(struct tty_struct *tty) > clear_bit(ASYNCB_NORMAL_ACTIVE, &port->flags); > spin_unlock_irqrestore(&port->lock, flags); > tty_port_tty_set(port, NULL); > + if (!uart_console(state->uart_port)) > + uart_change_pm(state, UART_PM_STATE_OFF);
Ok.
> wake_up_interruptible(&port->open_wait); > wake_up_interruptible(&port->delta_msr_wait); > } > @@ -2681,10 +2683,12 @@ int uart_remove_one_port(struct uart_driver *drv, struct uart_port *uport) > } > > /* > - * If the port is used as a console, unregister it > + * If the port is used as a console, unregister it, and power it down > */ > - if (uart_console(uport)) > + if (uart_console(uport)) { > unregister_console(uport->cons); > + uart_change_pm(state, UART_PM_STATE_OFF);
Won't this power off the port while tty consoles may still be open? I think the right thing here is to unregister_console then set uport->cons = NULL
[uport->cons is properly reassigned when/if a port is re-added via uart_add_one_port()).]
Then, uart_close() will power off the port when all ttys using the port have been closed.
> + } > > /* > * Free the port IO and memory resources, if any. >
| |