lkml.org 
[lkml]   [2024]   [Feb]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v3] serial: port: Don't suspend if the port is still busy
From
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


\
 
 \ /
  Last update: 2024-05-27 14:54    [W:0.076 / U:0.440 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site