Re: [PATCH] serial: 8250_fintek: Check SER_RS485_RTS_* only with RS485

[Ricardo Ribalda Delgado <ribalda@kernel.org>]

On Fri, May 13, 2022 at 3:47 PM Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> SER_RS485_RTS_ON_SEND and SER_RS485_RTS_AFTER_SEND relate to behavior
> within RS485 operation. The driver checks if they have the same value
> which is not possible to realize with the hardware. The check is taken
> regardless of SER_RS485_ENABLED flag and -EINVAL is returned when the
> check fails, which creates problems.
>
> This check makes it unnecessarily complicated to turn RS485 mode off as
> simple zeroed serial_rs485 struct will trigger that equal values check.
> In addition, the driver itself memsets its rs485 structure to zero when
> RS485 is disabled but if userspace would try to make an TIOCSRS485
> ioctl() call with the very same struct, it would end up failing with
> -EINVAL which doesn't make much sense.
>
> Resolve the problem by moving the check inside SER_RS485_ENABLED block.
>
> Fixes: 7ecc77011c6f ("serial: 8250_fintek: Return -EINVAL on invalid configuration")
> Cc: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
>
> ---
>  drivers/tty/serial/8250/8250_fintek.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_fintek.c b/drivers/tty/serial/8250/8250_fintek.c
> index 251f0018ae8c..dba5950b8d0e 100644
> --- a/drivers/tty/serial/8250/8250_fintek.c
> +++ b/drivers/tty/serial/8250/8250_fintek.c
> @@ -200,12 +200,12 @@ static int fintek_8250_rs485_config(struct uart_port *port,
>         if (!pdata)
>                 return -EINVAL;
>
> -       /* Hardware do not support same RTS level on send and receive */
> -       if (!(rs485->flags & SER_RS485_RTS_ON_SEND) ==
> -                       !(rs485->flags & SER_RS485_RTS_AFTER_SEND))
> -               return -EINVAL;
>
>         if (rs485->flags & SER_RS485_ENABLED) {
> +               /* Hardware do not support same RTS level on send and receive */
> +               if (!(rs485->flags & SER_RS485_RTS_ON_SEND) ==
> +                   !(rs485->flags & SER_RS485_RTS_AFTER_SEND))
> +                       return -EINVAL;
>                 memset(rs485->padding, 0, sizeof(rs485->padding));
>                 config |= RS485_URA;
>         } else {
>
> --
> tg: (4419da5d5d4b..) fix/8250-fintek-rts-check (depends on: tty-next)