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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <84plngljn2.fsf@jogness.linutronix.de>
Date: Thu, 31 Oct 2024 10:19:53 +0106
From: John Ogness <john.ogness@...utronix.de>
To: Jiri Slaby <jirislaby@...nel.org>, Greg Kroah-Hartman
 <gregkh@...uxfoundation.org>
Cc: Petr Mladek <pmladek@...e.com>, Sergey Senozhatsky
 <senozhatsky@...omium.org>, Steven Rostedt <rostedt@...dmis.org>, Thomas
 Gleixner <tglx@...utronix.de>, Esben Haabendal <esben@...nix.com>,
 linux-serial@...r.kernel.org, linux-kernel@...r.kernel.org, Florian
 Fainelli <florian.fainelli@...adcom.com>, Broadcom internal kernel review
 list <bcm-kernel-feedback-list@...adcom.com>, Ray Jui <rjui@...adcom.com>,
 Scott Branden <sbranden@...adcom.com>, Andy Shevchenko
 <andriy.shevchenko@...ux.intel.com>, "Paul E. McKenney"
 <paulmck@...nel.org>, Arnd Bergmann <arnd@...db.de>, Stefan Wahren
 <wahrenst@....net>, Uwe Kleine-König
 <u.kleine-koenig@...libre.com>, Kevin
 Hilman <khilman@...libre.com>, Markus Schneider-Pargmann
 <msp@...libre.com>, Ronald Wahl <ronald.wahl@...itan.com>, Udit Kumar
 <u-kumar1@...com>, Griffin Kroah-Hartman <griffin@...ah.com>, Rengarajan S
 <rengarajan.s@...rochip.com>, Lino Sanfilippo <l.sanfilippo@...bus.com>,
 Serge Semin <fancer.lancer@...il.com>,
 linux-rpi-kernel@...ts.infradead.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH tty-next v3 4/6] serial: 8250: Specify console context
 for rs485_start/stop_tx

On 2024-10-30, Jiri Slaby <jirislaby@...nel.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