On Wed, 12 Apr 2023, Johan Hovold wrote:
> The RS485 multipoint addressing support for some reason added a new
> ADDRB termios cflag which is (only!) updated from one of the RS485
> ioctls.
>
> Make sure to take the termios rw semaphore for the right ioctl (i.e.
> set, not get).
>
> Fixes: ae50bb275283 ("serial: take termios_rwsem for ->rs485_config() & pass termios as param")
> Cc: stable@xxxxxxxxxxxxxxx # 6.0
> Cc: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
> Signed-off-by: Johan Hovold <johan@xxxxxxxxxx>
> ---
>
> I did not have time to review the multipoint addressing patches at the
> time and only skimmed the archives now, but I can't seem to find any
> motivation for why a precious termios bit was seemingly wasted on ADDRB
> when it is only updated from the RS485 ioctls.
>
> I hope it wasn't done just to simplify the implementation of
> tty_get_frame_size()? Or was it a left-over from the RFC which
> apparently actually used termios to enable this feature?
No. I made it intentionally. It felt natural place for storing it because
ADDRB does impact the wire format and cflag is where other wire-format
impacting bits are also stored.
> Should we consider dropping the Linux-specific ADDRB bit again?
>
> Johan
>
>
> drivers/tty/serial/serial_core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 2bd32c8ece39..728cb72be066 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -1552,7 +1552,7 @@ uart_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg)
> goto out;
>
> /* rs485_config requires more locking than others */
> - if (cmd == TIOCGRS485)
> + if (cmd == TIOCSRS485)
> down_write(&tty->termios_rwsem);
>
> mutex_lock(&port->mutex);
> @@ -1595,7 +1595,7 @@ uart_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg)
> }
> out_up:
> mutex_unlock(&port->mutex);
> - if (cmd == TIOCGRS485)
> + if (cmd == TIOCSRS485)
> up_write(&tty->termios_rwsem);
> out:
> return ret;
>
Indeed, the caps are so blinding.
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
--
i.
