Messages in this thread | | | Date | Thu, 8 Feb 2024 09:27:57 +0100 | Subject | Re: [PATCH v3] serial: port: Don't suspend if the port is still busy | From | Jiri Slaby <> |
| |
Hi,
On 08. 02. 24, 8:52, Yicong Yang wrote: > From: Yicong Yang <yangyicong@hisilicon.com> > > We accidently met the issue that the bash prompt is not shown after the > previous command done and until the next input if there's only one CPU > (In our issue other CPUs are isolated by isolcpus=). Further analysis > shows it's because the port entering runtime suspend even if there's > still pending chars in the buffer and the pending chars will only be > processed in next device resuming. We are using amba-pl011 and the > problematic flow is like below:
.. > --- a/drivers/tty/serial/serial_port.c > +++ b/drivers/tty/serial/serial_port.c > @@ -19,8 +19,13 @@ > /* Only considers pending TX for now. Caller must take care of locking */ > static int __serial_port_busy(struct uart_port *port) > { > - return !uart_tx_stopped(port) && > - uart_circ_chars_pending(&port->state->xmit); > + if (uart_tx_stopped(port)) > + return 0; > + > + if (uart_circ_chars_pending(&port->state->xmit)) > + return -EBUSY;
Why do you do this change at all? If anything, __serial_port_busy() should be made to return a bool and not to return an error. Look how it is named -- returning EBUSY is sort of unexpected in my eyes. And if this needed to be done, it should have been in a separate patch anyway.
And then:
> @@ -46,8 +51,33 @@ static int serial_port_runtime_resume(struct device *dev) > return 0; > } > > +static int serial_port_runtime_suspend(struct device *dev) > +{ > + struct serial_port_device *port_dev = to_serial_base_port_device(dev); > + struct uart_port *port; > + unsigned long flags; > + int ret;
bool busy;
> + > + port = port_dev->port; > + > + if (port->flags & UPF_DEAD) > + return 0; > + > + uart_port_lock_irqsave(port, &flags); > + ret = __serial_port_busy(port); > + if (ret)
busy = ... if (busy)
> + port->ops->start_tx(port); > + uart_port_unlock_irqrestore(port, flags); > + > + if (ret)
if (busy)
> + pm_runtime_mark_last_busy(dev); > + > + return ret;
return busy ? -EBUSY : 0;
> +}
thanks, -- js suse labs
| |