Re: [PATCH tty-next v3 4/6] serial: 8250: Specify console context for rs485_start/stop_tx

[Petr Mladek <pmladek@suse.com>]

On Fri 2024-10-25 13:03:26, John Ogness wrote:
> For RS485 mode, if SER_RS485_RX_DURING_TX is not available, the
> console write callback needs to enable/disable TX. It does this
> by calling the rs485_start/stop_tx() callbacks. However, these
> callbacks will disable/enable interrupts, which is a problem
> for console write, as it must be responsible for
> disabling/enabling interrupts.

It is not clear to me what exactly is the problem. Is the main
problem calling pm_runtime*() API because it uses extra locks
and can cause deadlocks? Or is it more complicated?

IMHO, it would deserve some explanation.

> Add an argument @in_con to the rs485_start/stop_tx() callbacks
> to specify if they are being called from console write. If so,
> the callbacks will not handle interrupt disabling/enabling.
> 
> For all call sites other than console write, there is no
> functional change.
> 
> Signed-off-by: John Ogness <john.ogness@linutronix.de>

It looks like the code does what the description says. And honestly,
I do not have any idea how to improve the naming. I would keep
it as is after reading John's answers in the thread.

IMHO, one thing which makes things comlicated is that
serial8250_em485_start_tx() and serial8250_em485_stop_tx()
are not completely reversible operations. Especially,
the change done by __serial8250_stop_rx_mask_dr() is
not reverted in serial8250_em485_stop_tx(). It makes
things look tricky. But I think that it is beyond the scope
of this patchset to do anything about it.

Just 2 my cents.

Best Regaards,
Petr