Re: [PATCH tty-linus] serial: Reduce spinlocked portion of uart_rs485_config()

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 1256 bytes --]

On Fri, 22 Sep 2023, Lukas Wunner wrote:

> On Fri, Sep 22, 2023 at 06:02:28AM +0200, Lukas Wunner wrote:
> > On Thu, Sep 21, 2023 at 06:01:41PM +0300, Ilpo Järvinen wrote:
> > > On Thu, 21 Sep 2023, Lukas Wunner wrote:
> > > > --- a/drivers/tty/serial/serial_core.c
> > > > +++ b/drivers/tty/serial/serial_core.c
> > > > @@ -1404,12 +1404,18 @@ static void uart_set_rs485_termination(struct uart_port *port,
> > > >  static int uart_rs485_config(struct uart_port *port)
> > > >  {
> > > >  	struct serial_rs485 *rs485 = &port->rs485;
> > > > +	unsigned long flags;
> > > >  	int ret;
> > > >  
> > > > +	if (!(rs485->flags & SER_RS485_ENABLED))
> > > > +		return 0;
> > > > +
> > > >  	uart_sanitize_serial_rs485(port, rs485);
> > > 
> > > There's a subtle change in behavior here, uart_sanitize_serial_rs485() 
> > > memset()s rs485 if RS485 is not enabled but the early return above does 
> > > not.
> > 
> > The two callers of uart_rs485_config() only call it if
> > (!(uport->rs485.flags & SER_RS485_ENABLED)).
> 
> I meant to say "if (uport->rs485.flags & SER_RS485_ENABLED)"
> (i.e. without negation).
> 
> Otherwise my point still stands. :)

Okay yeah, it's fine. I did not take the behavior under the old code into 
account well enough.

-- 
 i.