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; + + dev_warn_ratelimited(port->dev, + "%s (%d): invalid RTS setting, using RTS_AFTER_SEND instead\n", + port->name, port->line); + } + }
uart_sanitize_serial_rs485_delays(port, rs485);
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?
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
On Thu, 12 Oct 2023, Lino Sanfilippo wrote:
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?
8250_exar.c needs to fixed then? I was taking these as things one can "configure" even if when there's support only for a one of them there's not that much to configure. As there was neither in 8250_exar's code, I didn't add either flag.
But I suppose your interpretation of those flag makes more sense.
Hi Ilpo,
On 13.10.23 12:24, Ilpo Järvinen wrote:
On Thu, 12 Oct 2023, Lino Sanfilippo wrote:
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?
8250_exar.c needs to fixed then?
I was taking these as things one can
"configure" even if when there's support only for a one of them there's not that much to configure. As there was neither in 8250_exar's code, I didn't add either flag.
But I suppose your interpretation of those flag makes more sense.
IMHO this is consistent with what we have in uart_get_rs485_mode(). This function ensures that we have at least one RTS mode set (with default to RTS_ON_SEND). So concerning 8250_exar.c, I think it should be fixed (havent noticed the missing RTS mode though until you mentioned it). Would you like to provide a fix for this or shall I include one into the next version of this series?
BR, Lino
On Sat, 14 Oct 2023, Lino Sanfilippo wrote:
On 13.10.23 12:24, Ilpo Järvinen wrote:
On Thu, 12 Oct 2023, Lino Sanfilippo wrote:
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?
8250_exar.c needs to fixed then?
I was taking these as things one can
"configure" even if when there's support only for a one of them there's not that much to configure. As there was neither in 8250_exar's code, I didn't add either flag.
But I suppose your interpretation of those flag makes more sense.
IMHO this is consistent with what we have in uart_get_rs485_mode(). This function ensures that we have at least one RTS mode set (with default to RTS_ON_SEND). So concerning 8250_exar.c, I think it should be fixed (havent noticed the missing RTS mode though until you mentioned it). Would you like to provide a fix for this or shall I include one into the next version of this series?
Just create that fix yourself thank you and include it into your series, I'm busy with other stuff currently.
On 16.10.23 12:05, Ilpo Järvinen wrote:
On Sat, 14 Oct 2023, Lino Sanfilippo wrote:
On 13.10.23 12:24, Ilpo Järvinen wrote:
On Thu, 12 Oct 2023, Lino Sanfilippo wrote:
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?
8250_exar.c needs to fixed then?
I was taking these as things one can
"configure" even if when there's support only for a one of them there's not that much to configure. As there was neither in 8250_exar's code, I didn't add either flag.
But I suppose your interpretation of those flag makes more sense.
IMHO this is consistent with what we have in uart_get_rs485_mode(). This function ensures that we have at least one RTS mode set (with default to RTS_ON_SEND). So concerning 8250_exar.c, I think it should be fixed (havent noticed the missing RTS mode though until you mentioned it). Would you like to provide a fix for this or shall I include one into the next version of this series?
Just create that fix yourself thank you and include it into your series, I'm busy with other stuff currently.
Sure, will do.
BR, Lino
linux-stable-mirror@lists.linaro.org
-
Ilpo Järvinen -
Lino Sanfilippo -
Lino Sanfilippo