Re: [PATCH] serial: core: Fix missing shutdown and startup for serial base port

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Thu, Apr 11, 2024 at 08:58:45AM +0300, Tony Lindgren wrote:
> We are seeing start_tx being called after port shutdown as noted by Jiri.
> This happens because we are missing the startup and shutdown related
> functions for the serial base port.
> 
> Let's fix the issue by adding startup and shutdown functions for the
> serial base port to block tx flushing for the serial base port when the
> port is not in use.

I'm going to test this later today, meanwhile some comments below.

...

> +out_base_port_startup:
> +	uport = uart_port_check(state);
> +	if (!uport)
> +		return -EIO;
> +
> +	serial_base_port_startup(uport);

So, we call this even on uninitialised TTY. Is it okay?

> +	return 0;
>  }


...

>  	if (tty)
>  		set_bit(TTY_IO_ERROR, &tty->flags);

> +	if (uport)
> +		serial_base_port_shutdown(uport);

Why not to call it after the below check to be reverse-symmetrical with startup?

>  	if (tty_port_initialized(port)) {
>  		tty_port_set_initialized(port, false);
>  

...

>  	/* Flush any pending TX for the port */
>  	uart_port_lock_irqsave(port, &flags);
> +	if (!port_dev->tx_enabled)
> +		goto unlock;

Can't this be integrated into...

>  	if (__serial_port_busy(port))

...this call?

>  		port->ops->start_tx(port);
> +
> +unlock:
>  	uart_port_unlock_irqrestore(port, flags);
>  

...

>  	uart_port_lock_irqsave(port, &flags);
> +	if (!port_dev->tx_enabled) {
> +		uart_port_unlock_irqrestore(port, flags);
> +		return 0;
> +	}
> +
>  	busy = __serial_port_busy(port);
>  	if (busy)
>  		port->ops->start_tx(port);

Ditto.

-- 
With Best Regards,
Andy Shevchenko