lkml.org 
[lkml]   [2023]   [Oct]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH v3 3/6] serial: core: fix sanitizing check for RTS settings
    From
    Hi,

    On 12.10.23 15:10, Ilpo Järvinen wrote:

    >
    >
    > On Wed, 11 Oct 2023, Lino Sanfilippo wrote:
    >
    >> Among other things uart_sanitize_serial_rs485() tests the sanity of the RTS
    >> settings in a RS485 configuration that has been passed by userspace.
    >> If RTS-on-send and RTS-after-send are both set or unset the configuration
    >> is adjusted and RTS-after-send is disabled and RTS-on-send enabled.
    >>
    >> This however makes only sense if both RTS modes are actually supported by
    >> the driver.
    >>
    >> With commit be2e2cb1d281 ("serial: Sanitize rs485_struct") the code does
    >> take the driver support into account but only checks if one of both RTS
    >> modes are supported. This may lead to the errorneous result of RTS-on-send
    >> being set even if only RTS-after-send is supported.
    >>
    >> Fix this by changing the implemented logic: First clear all unsupported
    >> flags in the RS485 configuration, then adjust an invalid RTS setting by
    >> taking into account which RTS mode is supported.
    >>
    >> Cc: stable@vger.kernel.org
    >> Fixes: be2e2cb1d281 ("serial: Sanitize rs485_struct")
    >> Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
    >> ---
    >> drivers/tty/serial/serial_core.c | 28 ++++++++++++++++++----------
    >> 1 file changed, 18 insertions(+), 10 deletions(-)
    >>
    >> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
    >> index 697c36dc7ec8..f4feebf8200f 100644
    >> --- a/drivers/tty/serial/serial_core.c
    >> +++ b/drivers/tty/serial/serial_core.c
    >> @@ -1370,19 +1370,27 @@ static void uart_sanitize_serial_rs485(struct uart_port *port, struct serial_rs4
    >> return;
    >> }
    >>
    >> + rs485->flags &= supported_flags;
    >> +
    >> /* Pick sane settings if the user hasn't */
    >> - if ((supported_flags & (SER_RS485_RTS_ON_SEND|SER_RS485_RTS_AFTER_SEND)) &&
    >> - !(rs485->flags & SER_RS485_RTS_ON_SEND) ==
    >> + if (!(rs485->flags & SER_RS485_RTS_ON_SEND) ==
    >> !(rs485->flags & SER_RS485_RTS_AFTER_SEND)) {
    >> - dev_warn_ratelimited(port->dev,
    >> - "%s (%d): invalid RTS setting, using RTS_ON_SEND instead\n",
    >> - port->name, port->line);
    >> - rs485->flags |= SER_RS485_RTS_ON_SEND;
    >> - rs485->flags &= ~SER_RS485_RTS_AFTER_SEND;
    >> - supported_flags |= SER_RS485_RTS_ON_SEND|SER_RS485_RTS_AFTER_SEND;
    >> - }
    >> + if (supported_flags & SER_RS485_RTS_ON_SEND) {
    >> + rs485->flags |= SER_RS485_RTS_ON_SEND;
    >> + rs485->flags &= ~SER_RS485_RTS_AFTER_SEND;
    >>
    >> - rs485->flags &= supported_flags;
    >> + dev_warn_ratelimited(port->dev,
    >> + "%s (%d): invalid RTS setting, using RTS_ON_SEND instead\n",
    >> + port->name, port->line);
    >> + } else {
    >> + rs485->flags |= SER_RS485_RTS_AFTER_SEND;
    >> + rs485->flags &= ~SER_RS485_RTS_ON_SEND;
    >
    > So if neither of the flags is supported, what will happen? You might want
    > add if after that else?
    >

    I would consider this a bug in the driver, as at least one of both modes
    has to be supported. If the driver does not have at least one of both flags
    set in rs485_supported.flags we could print a warning though. Would you prefer that?

    Regards,
    Lino

    \
     
     \ /
      Last update: 2023-10-12 23:02    [W:6.805 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site