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

[John Ogness <john.ogness@linutronix.de>]

On 2024-10-30, Jiri Slaby <jirislaby@kernel.org> wrote:
>> -void serial8250_em485_start_tx(struct uart_8250_port *up)
>> +void serial8250_em485_start_tx(struct uart_8250_port *up, bool in_con)
>>   {
>>   	unsigned char mcr = serial8250_in_MCR(up);
>>   
>> -	if (!(up->port.rs485.flags & SER_RS485_RX_DURING_TX))
>> -		serial8250_stop_rx(&up->port);
>> +	if (!(up->port.rs485.flags & SER_RS485_RX_DURING_TX)) {
>> +		/*
>> +		 * In console context, caller handles interrupt disabling. So
>> +		 * only LSR_DR masking is needed.
>> +		 */
>> +		if (in_con)
>> +			__serial8250_stop_rx_mask_dr(&up->port);
>> +		else
>> +			serial8250_stop_rx(&up->port);
>
> Would it make sense to propagate in_con into serial8250_stop_rx() and do 
> the logic there? That would effectively eliminate patch 2/6.

I considered this, however:

1. The whole idea of stopping RX in order to do TX is an RS485
issue. Modifying the general ->stop_rx() callback for this purpose is
kind of out of place.

2. The ->stop_rx() callback is a general uart_ops callback. Changing its
prototype would literally affect all serial drivers. OTOH the
->rs485_start_tx() callback is specific to the 8250 driver. (It seems
each driver has implemented their own method for handling the RS485
hacks.)

So I would prefer to keep the necessary RS485 changes 8250-specific for
now.

John