From: Andy Shevchenko Date: Wed, 31 Aug 2016 16:46:55 +0000 (+0300) Subject: serial: 8250_port: fix runtime PM use in __do_stop_tx_rs485() X-Git-Url: http://drtracing.org/?a=commitdiff_plain;h=b3965767d86cf4534dfe1affbde0453d3224ed7f;p=deliverable%2Flinux.git serial: 8250_port: fix runtime PM use in __do_stop_tx_rs485() There are calls to serial8250_rpm_{get|put}() in __do_stop_tx_rs485() that are certainly placed in a wrong location. I dunno how it had been tested with runtime PM enabled because it is obvious "sleep in atomic context" error. Besides that serial8250_rpm_get() is called immediately after an IO just happened. It implies that the device is already powered on, see implementation of serial8250_em485_rts_after_send() and serial8250_clear_fifos() for the details. There is no bug have been seen due to, as I can guess, use of auto suspend mode when scheduled transaction to suspend is invoked quite lately than it's needed for a few writes to the port. It might be possible to trigger a warning if stop_tx_timer fires when device is suspended. Refactor the code to use runtime PM only in case of timer function. Fixes: 0c66940d584d ("tty/serial/8250: fix RS485 half-duplex RX") Cc: "Matwey V. Kornilov" Tested-by: Yegor Yefremov Signed-off-by: Andy Shevchenko Signed-off-by: Greg Kroah-Hartman --- diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c index 1bd7ea0aff03..a8ecc253ef66 100644 --- a/drivers/tty/serial/8250/8250_port.c +++ b/drivers/tty/serial/8250/8250_port.c @@ -1414,12 +1414,8 @@ static void __do_stop_tx_rs485(struct uart_8250_port *p) if (!(p->port.rs485.flags & SER_RS485_RX_DURING_TX)) { serial8250_clear_fifos(p); - serial8250_rpm_get(p); - p->ier |= UART_IER_RLSI | UART_IER_RDI; serial_port_out(&p->port, UART_IER, p->ier); - - serial8250_rpm_put(p); } } @@ -1429,6 +1425,7 @@ static void serial8250_em485_handle_stop_tx(unsigned long arg) struct uart_8250_em485 *em485 = p->em485; unsigned long flags; + serial8250_rpm_get(p); spin_lock_irqsave(&p->port.lock, flags); if (em485 && em485->active_timer == &em485->stop_tx_timer) { @@ -1436,6 +1433,7 @@ static void serial8250_em485_handle_stop_tx(unsigned long arg) em485->active_timer = NULL; } spin_unlock_irqrestore(&p->port.lock, flags); + serial8250_rpm_put(p); } static void __stop_tx_rs485(struct uart_8250_port *p) @@ -1475,7 +1473,7 @@ static inline void __stop_tx(struct uart_8250_port *p) unsigned char lsr = serial_in(p, UART_LSR); /* * To provide required timeing and allow FIFO transfer, - * __stop_tx_rs485 must be called only when both FIFO and + * __stop_tx_rs485() must be called only when both FIFO and * shift register are empty. It is for device driver to enable * interrupt on TEMT. */ @@ -1484,9 +1482,10 @@ static inline void __stop_tx(struct uart_8250_port *p) del_timer(&em485->start_tx_timer); em485->active_timer = NULL; + + __stop_tx_rs485(p); } __do_stop_tx(p); - __stop_tx_rs485(p); } static void serial8250_stop_tx(struct uart_port *port)