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

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

[Ilpo Järvinen]

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

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>

---
 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)

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

[Ricardo Ribalda Delgado]

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)