[PATCH v4 0/8] Fixes and cleanup for RS485

[PATCH v4 0/8] Fixes and cleanup for RS485

[Lino Sanfilippo]

From: Lino Sanfilippo <l.sanfilippo@kunbus.com>

The following series includes cleanup and fixes around RS485 in the serial
core and uart drivers:

Patch 1: ar933x: Fix check for RS485 support
Patch 2: Remove superfluous code in ar933x.
Patch 3: Set the rs485 termination GPIO in the serial core. This is needed
	 since if the gpio is only accessible in sleepable context. It also
	 is a further step to make the RS485 handling more generic.
Patch 4: Move sanitizing of RS485 delays into an own function. This is in 
	 preparation of patch 4.
Patch 5: Sanitize RS485 delays read from device tree.
Patch 6: Correct RS485 delays in binding documentation.
Patch 7: Remove redundant code in 8250_dwlib.
Patch 8: Remove redundant code in 8250-lpc18xx.

Changes in v4:
- fixed logical error found by 
- capitalize "uart" and "gpio" in commit messages

Changes in v3:
- remove obsolete patch (due to changes by Ilpo)
- corrected and rephrase commit messages (pointed out by Andy)
- remove superfluous check (pointed out by Andy)
- separate ar933x UART bugfix and cleanup into different patches
  (as suggested by Ilpo)
- put the ar933x fix at the beginning of the series (as suggested by Andy)

Changes in v2:
- print a warning if termination GPIO is specified in DT/ACPI but is not
  supported by driver 
- fixed commit message for devtree documentation (as suggested by Andy)
- fixed code comment
- added patch 7

Lino Sanfilippo (8):
  serial: ar933x: Fix check for RS485 support
  serial: ar933x: Remove superfluous code in ar933x_config_rs485()
  serial: core, 8250: set RS485 termination GPIO in serial core
  serial: core: move sanitizing of RS485 delays into own function
  serial: core: sanitize RS485 delays read from device tree
  dt_bindings: rs485: Correct delay values
  serial: 8250_dwlib: remove redundant sanity check for RS485 flags
  serial: 8250: lpc18xx: Remove redundant sanity check for RS485 flags

 .../devicetree/bindings/serial/rs485.yaml     |  4 +-
 drivers/tty/serial/8250/8250_dwlib.c          | 10 +---
 drivers/tty/serial/8250/8250_lpc18xx.c        |  6 +-
 drivers/tty/serial/8250/8250_port.c           |  3 -
 drivers/tty/serial/ar933x_uart.c              | 18 ++----
 drivers/tty/serial/serial_core.c              | 60 ++++++++++++-------
 6 files changed, 50 insertions(+), 51 deletions(-)


base-commit: 7e5b4322cde067e1d0f1bf8f490e93f664a7c843
-- 
2.25.1

[PATCH v4 0/8] Fixes and cleanup for RS485

[Lino Sanfilippo]

From: Lino Sanfilippo <l.sanfilippo@kunbus.com>

The following series includes cleanup and fixes around RS485 in the serial
core and uart drivers:

Patch 1: ar933x: Fix check for RS485 support
Patch 2: Remove superfluous code in ar933x.
Patch 3: Set the rs485 termination GPIO in the serial core. This is needed
	 since if the gpio is only accessible in sleepable context. It also
	 is a further step to make the RS485 handling more generic.
Patch 4: Move sanitizing of RS485 delays into an own function. This is in 
	 preparation of patch 4.
Patch 5: Sanitize RS485 delays read from device tree.
Patch 6: Correct RS485 delays in binding documentation.
Patch 7: Remove redundant code in 8250_dwlib.
Patch 8: Remove redundant code in 8250-lpc18xx.

Changes in v4:
- fixed logical error found by 
- capitalize "uart" and "gpio" in commit messages

Changes in v3:
- remove obsolete patch (due to changes by Ilpo)
- corrected and rephrase commit messages (pointed out by Andy)
- remove superfluous check (pointed out by Andy)
- separate ar933x UART bugfix and cleanup into different patches
  (as suggested by Ilpo)
- put the ar933x fix at the beginning of the series (as suggested by Andy)

Changes in v2:
- print a warning if termination GPIO is specified in DT/ACPI but is not
  supported by driver 
- fixed commit message for devtree documentation (as suggested by Andy)
- fixed code comment
- added patch 7

Lino Sanfilippo (8):
  serial: ar933x: Fix check for RS485 support
  serial: ar933x: Remove superfluous code in ar933x_config_rs485()
  serial: core, 8250: set RS485 termination GPIO in serial core
  serial: core: move sanitizing of RS485 delays into own function
  serial: core: sanitize RS485 delays read from device tree
  dt_bindings: rs485: Correct delay values
  serial: 8250_dwlib: remove redundant sanity check for RS485 flags
  serial: 8250: lpc18xx: Remove redundant sanity check for RS485 flags

 .../devicetree/bindings/serial/rs485.yaml     |  4 +-
 drivers/tty/serial/8250/8250_dwlib.c          | 10 +---
 drivers/tty/serial/8250/8250_lpc18xx.c        |  6 +-
 drivers/tty/serial/8250/8250_port.c           |  3 -
 drivers/tty/serial/ar933x_uart.c              | 18 ++----
 drivers/tty/serial/serial_core.c              | 60 ++++++++++++-------
 6 files changed, 50 insertions(+), 51 deletions(-)


base-commit: 7e5b4322cde067e1d0f1bf8f490e93f664a7c843
-- 
2.25.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 1/8] serial: ar933x: Fix check for RS485 support

[Lino Sanfilippo]

From: Lino Sanfilippo <l.sanfilippo@kunbus.com>

RS485 is not possible without an RTS GPIO regardless of whether RS485 is
enabled at boot time or not. So correct the concerning check in the probe()
function.

Fixes: e849145e1fdd ("serial: ar933x: Fill in rs485_supported")
Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
---
 drivers/tty/serial/ar933x_uart.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/ar933x_uart.c b/drivers/tty/serial/ar933x_uart.c
index f931ecbc0bc0..f7b4638d69e5 100644
--- a/drivers/tty/serial/ar933x_uart.c
+++ b/drivers/tty/serial/ar933x_uart.c
@@ -798,11 +798,12 @@ static int ar933x_uart_probe(struct platform_device *pdev)
 
 	up->rts_gpiod = mctrl_gpio_to_gpiod(up->gpios, UART_GPIO_RTS);
 
-	if ((port->rs485.flags & SER_RS485_ENABLED) &&
-	    !up->rts_gpiod) {
-		dev_err(&pdev->dev, "lacking rts-gpio, disabling RS485\n");
-		port->rs485.flags &= ~SER_RS485_ENABLED;
+	if (!up->rts_gpiod) {
 		port->rs485_supported = ar933x_no_rs485;
+		if (port->rs485.flags & SER_RS485_ENABLED) {
+			dev_err(&pdev->dev, "lacking rts-gpio, disabling RS485\n");
+			port->rs485.flags &= ~SER_RS485_ENABLED;
+		}
 	}
 
 #ifdef CONFIG_SERIAL_AR933X_CONSOLE
-- 
2.25.1

[PATCH v4 1/8] serial: ar933x: Fix check for RS485 support

[Lino Sanfilippo]

From: Lino Sanfilippo <l.sanfilippo@kunbus.com>

RS485 is not possible without an RTS GPIO regardless of whether RS485 is
enabled at boot time or not. So correct the concerning check in the probe()
function.

Fixes: e849145e1fdd ("serial: ar933x: Fill in rs485_supported")
Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
---
 drivers/tty/serial/ar933x_uart.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/ar933x_uart.c b/drivers/tty/serial/ar933x_uart.c
index f931ecbc0bc0..f7b4638d69e5 100644
--- a/drivers/tty/serial/ar933x_uart.c
+++ b/drivers/tty/serial/ar933x_uart.c
@@ -798,11 +798,12 @@ static int ar933x_uart_probe(struct platform_device *pdev)
 
 	up->rts_gpiod = mctrl_gpio_to_gpiod(up->gpios, UART_GPIO_RTS);
 
-	if ((port->rs485.flags & SER_RS485_ENABLED) &&
-	    !up->rts_gpiod) {
-		dev_err(&pdev->dev, "lacking rts-gpio, disabling RS485\n");
-		port->rs485.flags &= ~SER_RS485_ENABLED;
+	if (!up->rts_gpiod) {
 		port->rs485_supported = ar933x_no_rs485;
+		if (port->rs485.flags & SER_RS485_ENABLED) {
+			dev_err(&pdev->dev, "lacking rts-gpio, disabling RS485\n");
+			port->rs485.flags &= ~SER_RS485_ENABLED;
+		}
 	}
 
 #ifdef CONFIG_SERIAL_AR933X_CONSOLE
-- 
2.25.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 2/8] serial: ar933x: Remove superfluous code in ar933x_config_rs485()

[Lino Sanfilippo]

From: Lino Sanfilippo <l.sanfilippo@kunbus.com>

In ar933x_config_rs485() the check for the RTS GPIO is not needed since in
case the GPIO is not available at driver init ar933x_no_rs485 is assigned
to port->rs485_supported and this function is never called. So remove the
check.

Also in uart_set_rs485_config() the serial core already assigns the passed
serial_rs485 struct to the uart port. So remove the assignment in the
drivers rs485_config() function to avoid redundancy.

Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
---
 drivers/tty/serial/ar933x_uart.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/tty/serial/ar933x_uart.c b/drivers/tty/serial/ar933x_uart.c
index f7b4638d69e5..32caeac12985 100644
--- a/drivers/tty/serial/ar933x_uart.c
+++ b/drivers/tty/serial/ar933x_uart.c
@@ -583,15 +583,6 @@ static const struct uart_ops ar933x_uart_ops = {
 static int ar933x_config_rs485(struct uart_port *port, struct ktermios *termios,
 				struct serial_rs485 *rs485conf)
 {
-	struct ar933x_uart_port *up =
-		container_of(port, struct ar933x_uart_port, port);
-
-	if ((rs485conf->flags & SER_RS485_ENABLED) &&
-	    !up->rts_gpiod) {
-		dev_err(port->dev, "RS485 needs rts-gpio\n");
-		return 1;
-	}
-	port->rs485 = *rs485conf;
 	return 0;
 }
 
-- 
2.25.1

[PATCH v4 2/8] serial: ar933x: Remove superfluous code in ar933x_config_rs485()

[Lino Sanfilippo]

From: Lino Sanfilippo <l.sanfilippo@kunbus.com>

In ar933x_config_rs485() the check for the RTS GPIO is not needed since in
case the GPIO is not available at driver init ar933x_no_rs485 is assigned
to port->rs485_supported and this function is never called. So remove the
check.

Also in uart_set_rs485_config() the serial core already assigns the passed
serial_rs485 struct to the uart port. So remove the assignment in the
drivers rs485_config() function to avoid redundancy.

Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
---
 drivers/tty/serial/ar933x_uart.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/tty/serial/ar933x_uart.c b/drivers/tty/serial/ar933x_uart.c
index f7b4638d69e5..32caeac12985 100644
--- a/drivers/tty/serial/ar933x_uart.c
+++ b/drivers/tty/serial/ar933x_uart.c
@@ -583,15 +583,6 @@ static const struct uart_ops ar933x_uart_ops = {
 static int ar933x_config_rs485(struct uart_port *port, struct ktermios *termios,
 				struct serial_rs485 *rs485conf)
 {
-	struct ar933x_uart_port *up =
-		container_of(port, struct ar933x_uart_port, port);
-
-	if ((rs485conf->flags & SER_RS485_ENABLED) &&
-	    !up->rts_gpiod) {
-		dev_err(port->dev, "RS485 needs rts-gpio\n");
-		return 1;
-	}
-	port->rs485 = *rs485conf;
 	return 0;
 }
 
-- 
2.25.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 3/8] serial: core, 8250: set RS485 termination GPIO in serial core

[Lino Sanfilippo]

From: Lino Sanfilippo <l.sanfilippo@kunbus.com>

In serial8250_em485_config() the termination GPIO is set with the uart_port
spinlock held. This is an issue if setting the GPIO line can sleep (e.g.
since the concerning GPIO expander is connected via SPI or I2C).

Fix this by setting the termination line outside of the uart_port spinlock
in the serial core and using gpiod_set_value_cansleep() which instead of
gpiod_set_value() allows it to sleep.

Beside fixing the termination GPIO line setting for the 8250 driver this
change also makes setting the termination GPIO generic for all UART
drivers.

Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
---
 drivers/tty/serial/8250/8250_port.c |  3 ---
 drivers/tty/serial/serial_core.c    | 12 ++++++++++++
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index ed2a606f2da7..72252d956f17 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -676,9 +676,6 @@ int serial8250_em485_config(struct uart_port *port, struct ktermios *termios,
 		rs485->flags &= ~SER_RS485_RTS_AFTER_SEND;
 	}
 
-	gpiod_set_value(port->rs485_term_gpio,
-			rs485->flags & SER_RS485_TERMINATE_BUS);
-
 	/*
 	 * Both serial8250_em485_init() and serial8250_em485_destroy()
 	 * are idempotent.
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 1db44cde76f6..047ec51dbd41 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1358,12 +1358,23 @@ static void uart_sanitize_serial_rs485(struct uart_port *port, struct serial_rs4
 	memset(rs485->padding1, 0, sizeof(rs485->padding1));
 }
 
+static void uart_set_rs485_termination(struct uart_port *port,
+				       const struct serial_rs485 *rs485)
+{
+	if (!(rs485->flags & SER_RS485_ENABLED))
+		return;
+
+	gpiod_set_value_cansleep(port->rs485_term_gpio,
+				 !!(rs485->flags & SER_RS485_TERMINATE_BUS));
+}
+
 int uart_rs485_config(struct uart_port *port)
 {
 	struct serial_rs485 *rs485 = &port->rs485;
 	int ret;
 
 	uart_sanitize_serial_rs485(port, rs485);
+	uart_set_rs485_termination(port, rs485);
 
 	ret = port->rs485_config(port, NULL, rs485);
 	if (ret)
@@ -1406,6 +1417,7 @@ static int uart_set_rs485_config(struct tty_struct *tty, struct uart_port *port,
 	if (ret)
 		return ret;
 	uart_sanitize_serial_rs485(port, &rs485);
+	uart_set_rs485_termination(port, &rs485);
 
 	spin_lock_irqsave(&port->lock, flags);
 	ret = port->rs485_config(port, &tty->termios, &rs485);
-- 
2.25.1

[PATCH v4 3/8] serial: core, 8250: set RS485 termination GPIO in serial core

[Lino Sanfilippo]

From: Lino Sanfilippo <l.sanfilippo@kunbus.com>

In serial8250_em485_config() the termination GPIO is set with the uart_port
spinlock held. This is an issue if setting the GPIO line can sleep (e.g.
since the concerning GPIO expander is connected via SPI or I2C).

Fix this by setting the termination line outside of the uart_port spinlock
in the serial core and using gpiod_set_value_cansleep() which instead of
gpiod_set_value() allows it to sleep.

Beside fixing the termination GPIO line setting for the 8250 driver this
change also makes setting the termination GPIO generic for all UART
drivers.

Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
---
 drivers/tty/serial/8250/8250_port.c |  3 ---
 drivers/tty/serial/serial_core.c    | 12 ++++++++++++
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index ed2a606f2da7..72252d956f17 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -676,9 +676,6 @@ int serial8250_em485_config(struct uart_port *port, struct ktermios *termios,
 		rs485->flags &= ~SER_RS485_RTS_AFTER_SEND;
 	}
 
-	gpiod_set_value(port->rs485_term_gpio,
-			rs485->flags & SER_RS485_TERMINATE_BUS);
-
 	/*
 	 * Both serial8250_em485_init() and serial8250_em485_destroy()
 	 * are idempotent.
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 1db44cde76f6..047ec51dbd41 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1358,12 +1358,23 @@ static void uart_sanitize_serial_rs485(struct uart_port *port, struct serial_rs4
 	memset(rs485->padding1, 0, sizeof(rs485->padding1));
 }
 
+static void uart_set_rs485_termination(struct uart_port *port,
+				       const struct serial_rs485 *rs485)
+{
+	if (!(rs485->flags & SER_RS485_ENABLED))
+		return;
+
+	gpiod_set_value_cansleep(port->rs485_term_gpio,
+				 !!(rs485->flags & SER_RS485_TERMINATE_BUS));
+}
+
 int uart_rs485_config(struct uart_port *port)
 {
 	struct serial_rs485 *rs485 = &port->rs485;
 	int ret;
 
 	uart_sanitize_serial_rs485(port, rs485);
+	uart_set_rs485_termination(port, rs485);
 
 	ret = port->rs485_config(port, NULL, rs485);
 	if (ret)
@@ -1406,6 +1417,7 @@ static int uart_set_rs485_config(struct tty_struct *tty, struct uart_port *port,
 	if (ret)
 		return ret;
 	uart_sanitize_serial_rs485(port, &rs485);
+	uart_set_rs485_termination(port, &rs485);
 
 	spin_lock_irqsave(&port->lock, flags);
 	ret = port->rs485_config(port, &tty->termios, &rs485);
-- 
2.25.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 4/8] serial: core: move sanitizing of RS485 delays into own function

[Lino Sanfilippo]

From: Lino Sanfilippo <l.sanfilippo@kunbus.com>

Move the sanitizing of RS485 delays out of uart_sanitize_serial_rs485()
into the new function uart_sanitize_serial_rs485_delays().

Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/tty/serial/serial_core.c | 46 ++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 20 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 047ec51dbd41..3158f05a328c 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1302,27 +1302,9 @@ static int uart_check_rs485_flags(struct uart_port *port, struct serial_rs485 *r
 	return 0;
 }
 
-static void uart_sanitize_serial_rs485(struct uart_port *port, struct serial_rs485 *rs485)
+static void uart_sanitize_serial_rs485_delays(struct uart_port *port,
+					      struct serial_rs485 *rs485)
 {
-	u32 supported_flags = port->rs485_supported.flags;
-
-	if (!(rs485->flags & SER_RS485_ENABLED)) {
-		memset(rs485, 0, sizeof(*rs485));
-		return;
-	}
-
-	/* 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) ==
-	    !(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 (!port->rs485_supported.delay_rts_before_send) {
 		if (rs485->delay_rts_before_send) {
 			dev_warn_ratelimited(port->dev,
@@ -1350,9 +1332,33 @@ static void uart_sanitize_serial_rs485(struct uart_port *port, struct serial_rs4
 			"%s (%d): RTS delay after sending clamped to %u ms\n",
 			port->name, port->line, rs485->delay_rts_after_send);
 	}
+}
+
+static void uart_sanitize_serial_rs485(struct uart_port *port, struct serial_rs485 *rs485)
+{
+	u32 supported_flags = port->rs485_supported.flags;
+
+	if (!(rs485->flags & SER_RS485_ENABLED)) {
+		memset(rs485, 0, sizeof(*rs485));
+		return;
+	}
+
+	/* 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) ==
+	    !(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;
+	}
 
 	rs485->flags &= supported_flags;
 
+	uart_sanitize_serial_rs485_delays(port, rs485);
+
 	/* Return clean padding area to userspace */
 	memset(rs485->padding0, 0, sizeof(rs485->padding0));
 	memset(rs485->padding1, 0, sizeof(rs485->padding1));
-- 
2.25.1

[PATCH v4 4/8] serial: core: move sanitizing of RS485 delays into own function

[Lino Sanfilippo]

From: Lino Sanfilippo <l.sanfilippo@kunbus.com>

Move the sanitizing of RS485 delays out of uart_sanitize_serial_rs485()
into the new function uart_sanitize_serial_rs485_delays().

Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/tty/serial/serial_core.c | 46 ++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 20 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 047ec51dbd41..3158f05a328c 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1302,27 +1302,9 @@ static int uart_check_rs485_flags(struct uart_port *port, struct serial_rs485 *r
 	return 0;
 }
 
-static void uart_sanitize_serial_rs485(struct uart_port *port, struct serial_rs485 *rs485)
+static void uart_sanitize_serial_rs485_delays(struct uart_port *port,
+					      struct serial_rs485 *rs485)
 {
-	u32 supported_flags = port->rs485_supported.flags;
-
-	if (!(rs485->flags & SER_RS485_ENABLED)) {
-		memset(rs485, 0, sizeof(*rs485));
-		return;
-	}
-
-	/* 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) ==
-	    !(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 (!port->rs485_supported.delay_rts_before_send) {
 		if (rs485->delay_rts_before_send) {
 			dev_warn_ratelimited(port->dev,
@@ -1350,9 +1332,33 @@ static void uart_sanitize_serial_rs485(struct uart_port *port, struct serial_rs4
 			"%s (%d): RTS delay after sending clamped to %u ms\n",
 			port->name, port->line, rs485->delay_rts_after_send);
 	}
+}
+
+static void uart_sanitize_serial_rs485(struct uart_port *port, struct serial_rs485 *rs485)
+{
+	u32 supported_flags = port->rs485_supported.flags;
+
+	if (!(rs485->flags & SER_RS485_ENABLED)) {
+		memset(rs485, 0, sizeof(*rs485));
+		return;
+	}
+
+	/* 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) ==
+	    !(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;
+	}
 
 	rs485->flags &= supported_flags;
 
+	uart_sanitize_serial_rs485_delays(port, rs485);
+
 	/* Return clean padding area to userspace */
 	memset(rs485->padding0, 0, sizeof(rs485->padding0));
 	memset(rs485->padding1, 0, sizeof(rs485->padding1));
-- 
2.25.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 5/8] serial: core: sanitize RS485 delays read from device tree

[Lino Sanfilippo]

From: Lino Sanfilippo <l.sanfilippo@kunbus.com>

Currently the RTS delays set via device tree are not clamped to a maximum
value although the device tree bindings documentation for RS485 claims that
only a maximum of 1000 msecs is allowed.

So clamp the values to avoid arbitrary high delay settings. However clamp
the values to 100 instead of 1000 msecs to be consistent which the maximum
that is allowed when setting the delays from userspace via the UART ioctl
TIOCSRS485.

Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
---
 drivers/tty/serial/serial_core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 3158f05a328c..ac198d0d4c80 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -3395,6 +3395,8 @@ int uart_get_rs485_mode(struct uart_port *port)
 		rs485conf->delay_rts_after_send = 0;
 	}
 
+	uart_sanitize_serial_rs485_delays(port, rs485conf);
+
 	/*
 	 * Clear full-duplex and enabled flags, set RTS polarity to active high
 	 * to get to a defined state with the following properties:
-- 
2.25.1

[PATCH v4 5/8] serial: core: sanitize RS485 delays read from device tree

[Lino Sanfilippo]

From: Lino Sanfilippo <l.sanfilippo@kunbus.com>

Currently the RTS delays set via device tree are not clamped to a maximum
value although the device tree bindings documentation for RS485 claims that
only a maximum of 1000 msecs is allowed.

So clamp the values to avoid arbitrary high delay settings. However clamp
the values to 100 instead of 1000 msecs to be consistent which the maximum
that is allowed when setting the delays from userspace via the UART ioctl
TIOCSRS485.

Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
---
 drivers/tty/serial/serial_core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 3158f05a328c..ac198d0d4c80 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -3395,6 +3395,8 @@ int uart_get_rs485_mode(struct uart_port *port)
 		rs485conf->delay_rts_after_send = 0;
 	}
 
+	uart_sanitize_serial_rs485_delays(port, rs485conf);
+
 	/*
 	 * Clear full-duplex and enabled flags, set RTS polarity to active high
 	 * to get to a defined state with the following properties:
-- 
2.25.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 6/8] dt_bindings: rs485: Correct delay values

[Lino Sanfilippo]

From: Lino Sanfilippo <l.sanfilippo@kunbus.com>

Currently the documentation claims that a maximum of 1000 msecs is allowed
for RTS delays. However nothing actually checks the values read from device
tree/ACPI and so it is possible to set much higher values.

There is already a maximum of 100 ms enforced for RTS delays that are set
via the UART TIOCSRS485 ioctl. To be consistent with that use the same
limit for DT/ACPI values.

Although this change is visible to userspace the risk of breaking anything
when reducing the max delays from 1000 to 100 ms should be very low, since
100 ms is already a very high maximum for delays that are usually rather in
the usecs range.

Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/serial/rs485.yaml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/serial/rs485.yaml b/Documentation/devicetree/bindings/serial/rs485.yaml
index f2c9c9fe6aa7..90a1bab40f05 100644
--- a/Documentation/devicetree/bindings/serial/rs485.yaml
+++ b/Documentation/devicetree/bindings/serial/rs485.yaml
@@ -22,12 +22,12 @@ properties:
         - description: Delay between rts signal and beginning of data sent in
             milliseconds. It corresponds to the delay before sending data.
           default: 0
-          maximum: 1000
+          maximum: 100
         - description: Delay between end of data sent and rts signal in milliseconds.
             It corresponds to the delay after sending data and actual release
             of the line.
           default: 0
-          maximum: 1000
+          maximum: 100
 
   rs485-rts-active-low:
     description: drive RTS low when sending (default is high).
-- 
2.25.1

[PATCH v4 6/8] dt_bindings: rs485: Correct delay values

[Lino Sanfilippo]

From: Lino Sanfilippo <l.sanfilippo@kunbus.com>

Currently the documentation claims that a maximum of 1000 msecs is allowed
for RTS delays. However nothing actually checks the values read from device
tree/ACPI and so it is possible to set much higher values.

There is already a maximum of 100 ms enforced for RTS delays that are set
via the UART TIOCSRS485 ioctl. To be consistent with that use the same
limit for DT/ACPI values.

Although this change is visible to userspace the risk of breaking anything
when reducing the max delays from 1000 to 100 ms should be very low, since
100 ms is already a very high maximum for delays that are usually rather in
the usecs range.

Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/serial/rs485.yaml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/serial/rs485.yaml b/Documentation/devicetree/bindings/serial/rs485.yaml
index f2c9c9fe6aa7..90a1bab40f05 100644
--- a/Documentation/devicetree/bindings/serial/rs485.yaml
+++ b/Documentation/devicetree/bindings/serial/rs485.yaml
@@ -22,12 +22,12 @@ properties:
         - description: Delay between rts signal and beginning of data sent in
             milliseconds. It corresponds to the delay before sending data.
           default: 0
-          maximum: 1000
+          maximum: 100
         - description: Delay between end of data sent and rts signal in milliseconds.
             It corresponds to the delay after sending data and actual release
             of the line.
           default: 0
-          maximum: 1000
+          maximum: 100
 
   rs485-rts-active-low:
     description: drive RTS low when sending (default is high).
-- 
2.25.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 7/8] serial: 8250_dwlib: remove redundant sanity check for RS485 flags

[Lino Sanfilippo]

From: Lino Sanfilippo <l.sanfilippo@kunbus.com>

Before the drivers rs485_config() function is called the serial core
already ensures that only one of both options RTS on send or RTS after send
is set. So remove the concerning sanity check in the driver function to
avoid redundancy.

Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/tty/serial/8250/8250_dwlib.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dwlib.c b/drivers/tty/serial/8250/8250_dwlib.c
index 2c3b1468bd88..dbe4d44f60d4 100644
--- a/drivers/tty/serial/8250/8250_dwlib.c
+++ b/drivers/tty/serial/8250/8250_dwlib.c
@@ -187,16 +187,10 @@ static int dw8250_rs485_config(struct uart_port *p, struct ktermios *termios,
 	if (rs485->flags & SER_RS485_ENABLED) {
 		tcr |= DW_UART_TCR_RS485_EN;
 
-		if (rs485->flags & SER_RS485_RX_DURING_TX) {
+		if (rs485->flags & SER_RS485_RX_DURING_TX)
 			tcr |= DW_UART_TCR_XFER_MODE_DE_DURING_RE;
-		} else {
-			/* HW does not support same DE level for tx and rx */
-			if (!(rs485->flags & SER_RS485_RTS_ON_SEND) ==
-			    !(rs485->flags & SER_RS485_RTS_AFTER_SEND))
-				return -EINVAL;
-
+		else
 			tcr |= DW_UART_TCR_XFER_MODE_DE_OR_RE;
-		}
 		dw8250_writel_ext(p, DW_UART_DE_EN, 1);
 		dw8250_writel_ext(p, DW_UART_RE_EN, 1);
 	} else {
-- 
2.25.1

[PATCH v4 7/8] serial: 8250_dwlib: remove redundant sanity check for RS485 flags

[Lino Sanfilippo]

From: Lino Sanfilippo <l.sanfilippo@kunbus.com>

Before the drivers rs485_config() function is called the serial core
already ensures that only one of both options RTS on send or RTS after send
is set. So remove the concerning sanity check in the driver function to
avoid redundancy.

Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/tty/serial/8250/8250_dwlib.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dwlib.c b/drivers/tty/serial/8250/8250_dwlib.c
index 2c3b1468bd88..dbe4d44f60d4 100644
--- a/drivers/tty/serial/8250/8250_dwlib.c
+++ b/drivers/tty/serial/8250/8250_dwlib.c
@@ -187,16 +187,10 @@ static int dw8250_rs485_config(struct uart_port *p, struct ktermios *termios,
 	if (rs485->flags & SER_RS485_ENABLED) {
 		tcr |= DW_UART_TCR_RS485_EN;
 
-		if (rs485->flags & SER_RS485_RX_DURING_TX) {
+		if (rs485->flags & SER_RS485_RX_DURING_TX)
 			tcr |= DW_UART_TCR_XFER_MODE_DE_DURING_RE;
-		} else {
-			/* HW does not support same DE level for tx and rx */
-			if (!(rs485->flags & SER_RS485_RTS_ON_SEND) ==
-			    !(rs485->flags & SER_RS485_RTS_AFTER_SEND))
-				return -EINVAL;
-
+		else
 			tcr |= DW_UART_TCR_XFER_MODE_DE_OR_RE;
-		}
 		dw8250_writel_ext(p, DW_UART_DE_EN, 1);
 		dw8250_writel_ext(p, DW_UART_RE_EN, 1);
 	} else {
-- 
2.25.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 8/8] serial: 8250: lpc18xx: Remove redundant sanity check for RS485 flags

[Lino Sanfilippo]

From: Lino Sanfilippo <l.sanfilippo@kunbus.com>

Before the drivers rs485_config() function is called the serial core
already ensures that only one of both options RTS on send or RTS after send
is set. So remove the concerning sanity check in the driver function to
avoid redundancy.

Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/tty/serial/8250/8250_lpc18xx.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_lpc18xx.c b/drivers/tty/serial/8250/8250_lpc18xx.c
index d6ca0d47e9d5..6dc85aaba5d0 100644
--- a/drivers/tty/serial/8250/8250_lpc18xx.c
+++ b/drivers/tty/serial/8250/8250_lpc18xx.c
@@ -44,12 +44,8 @@ static int lpc18xx_rs485_config(struct uart_port *port, struct ktermios *termios
 		rs485_ctrl_reg |= LPC18XX_UART_RS485CTRL_NMMEN |
 				  LPC18XX_UART_RS485CTRL_DCTRL;
 
-		if (rs485->flags & SER_RS485_RTS_ON_SEND) {
+		if (rs485->flags & SER_RS485_RTS_ON_SEND)
 			rs485_ctrl_reg |= LPC18XX_UART_RS485CTRL_OINV;
-			rs485->flags &= ~SER_RS485_RTS_AFTER_SEND;
-		} else {
-			rs485->flags |= SER_RS485_RTS_AFTER_SEND;
-		}
 	}
 
 	if (rs485->delay_rts_after_send) {
-- 
2.25.1

[PATCH v4 8/8] serial: 8250: lpc18xx: Remove redundant sanity check for RS485 flags

[Lino Sanfilippo]

From: Lino Sanfilippo <l.sanfilippo@kunbus.com>

Before the drivers rs485_config() function is called the serial core
already ensures that only one of both options RTS on send or RTS after send
is set. So remove the concerning sanity check in the driver function to
avoid redundancy.

Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/tty/serial/8250/8250_lpc18xx.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_lpc18xx.c b/drivers/tty/serial/8250/8250_lpc18xx.c
index d6ca0d47e9d5..6dc85aaba5d0 100644
--- a/drivers/tty/serial/8250/8250_lpc18xx.c
+++ b/drivers/tty/serial/8250/8250_lpc18xx.c
@@ -44,12 +44,8 @@ static int lpc18xx_rs485_config(struct uart_port *port, struct ktermios *termios
 		rs485_ctrl_reg |= LPC18XX_UART_RS485CTRL_NMMEN |
 				  LPC18XX_UART_RS485CTRL_DCTRL;
 
-		if (rs485->flags & SER_RS485_RTS_ON_SEND) {
+		if (rs485->flags & SER_RS485_RTS_ON_SEND)
 			rs485_ctrl_reg |= LPC18XX_UART_RS485CTRL_OINV;
-			rs485->flags &= ~SER_RS485_RTS_AFTER_SEND;
-		} else {
-			rs485->flags |= SER_RS485_RTS_AFTER_SEND;
-		}
 	}
 
 	if (rs485->delay_rts_after_send) {
-- 
2.25.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 0/8] Fixes and cleanup for RS485

[Andy Shevchenko]

On Sun, Jul 10, 2022 at 06:44:34PM +0200, Lino Sanfilippo wrote:
> From: Lino Sanfilippo <l.sanfilippo@kunbus.com>
> 
> The following series includes cleanup and fixes around RS485 in the serial
> core and uart drivers:
> 
> Patch 1: ar933x: Fix check for RS485 support
> Patch 2: Remove superfluous code in ar933x.
> Patch 3: Set the rs485 termination GPIO in the serial core. This is needed
> 	 since if the gpio is only accessible in sleepable context. It also
> 	 is a further step to make the RS485 handling more generic.
> Patch 4: Move sanitizing of RS485 delays into an own function. This is in 
> 	 preparation of patch 4.
> Patch 5: Sanitize RS485 delays read from device tree.
> Patch 6: Correct RS485 delays in binding documentation.
> Patch 7: Remove redundant code in 8250_dwlib.
> Patch 8: Remove redundant code in 8250-lpc18xx.

> Changes in v4:
> - fixed logical error found by 
> - capitalize "uart" and "gpio" in commit messages

Please, avoid sending sequential version of the series more often than once per
a few days, recommended interval is one week.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 0/8] Fixes and cleanup for RS485

[Andy Shevchenko]

On Sun, Jul 10, 2022 at 06:44:34PM +0200, Lino Sanfilippo wrote:
> From: Lino Sanfilippo <l.sanfilippo@kunbus.com>
> 
> The following series includes cleanup and fixes around RS485 in the serial
> core and uart drivers:
> 
> Patch 1: ar933x: Fix check for RS485 support
> Patch 2: Remove superfluous code in ar933x.
> Patch 3: Set the rs485 termination GPIO in the serial core. This is needed
> 	 since if the gpio is only accessible in sleepable context. It also
> 	 is a further step to make the RS485 handling more generic.
> Patch 4: Move sanitizing of RS485 delays into an own function. This is in 
> 	 preparation of patch 4.
> Patch 5: Sanitize RS485 delays read from device tree.
> Patch 6: Correct RS485 delays in binding documentation.
> Patch 7: Remove redundant code in 8250_dwlib.
> Patch 8: Remove redundant code in 8250-lpc18xx.

> Changes in v4:
> - fixed logical error found by 
> - capitalize "uart" and "gpio" in commit messages

Please, avoid sending sequential version of the series more often than once per
a few days, recommended interval is one week.

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v4 0/8] Fixes and cleanup for RS485

[Lino Sanfilippo]

Hi,

On 10.07.22 20:43, Andy Shevchenko wrote:
> On Sun, Jul 10, 2022 at 06:44:34PM +0200, Lino Sanfilippo wrote:
>> From: Lino Sanfilippo <l.sanfilippo@kunbus.com>
>>
>> The following series includes cleanup and fixes around RS485 in the serial
>> core and uart drivers:
>>
>> Patch 1: ar933x: Fix check for RS485 support
>> Patch 2: Remove superfluous code in ar933x.
>> Patch 3: Set the rs485 termination GPIO in the serial core. This is needed
>> 	 since if the gpio is only accessible in sleepable context. It also
>> 	 is a further step to make the RS485 handling more generic.
>> Patch 4: Move sanitizing of RS485 delays into an own function. This is in
>> 	 preparation of patch 4.
>> Patch 5: Sanitize RS485 delays read from device tree.
>> Patch 6: Correct RS485 delays in binding documentation.
>> Patch 7: Remove redundant code in 8250_dwlib.
>> Patch 8: Remove redundant code in 8250-lpc18xx.
>
>> Changes in v4:
>> - fixed logical error found by
>> - capitalize "uart" and "gpio" in commit messages
>
> Please, avoid sending sequential version of the series more often than once per
> a few days, recommended interval is one week.
>

sorry, this was due to the error found by the kernel test robot. I was not sure
if to wait, resend with the same version number or send the next version.
I guess waiting a few days would have been the best option. Will do so next
time.

Regards,
Lino

Re: [PATCH v4 0/8] Fixes and cleanup for RS485

[Lino Sanfilippo]

Hi,

On 10.07.22 20:43, Andy Shevchenko wrote:
> On Sun, Jul 10, 2022 at 06:44:34PM +0200, Lino Sanfilippo wrote:
>> From: Lino Sanfilippo <l.sanfilippo@kunbus.com>
>>
>> The following series includes cleanup and fixes around RS485 in the serial
>> core and uart drivers:
>>
>> Patch 1: ar933x: Fix check for RS485 support
>> Patch 2: Remove superfluous code in ar933x.
>> Patch 3: Set the rs485 termination GPIO in the serial core. This is needed
>> 	 since if the gpio is only accessible in sleepable context. It also
>> 	 is a further step to make the RS485 handling more generic.
>> Patch 4: Move sanitizing of RS485 delays into an own function. This is in
>> 	 preparation of patch 4.
>> Patch 5: Sanitize RS485 delays read from device tree.
>> Patch 6: Correct RS485 delays in binding documentation.
>> Patch 7: Remove redundant code in 8250_dwlib.
>> Patch 8: Remove redundant code in 8250-lpc18xx.
>
>> Changes in v4:
>> - fixed logical error found by
>> - capitalize "uart" and "gpio" in commit messages
>
> Please, avoid sending sequential version of the series more often than once per
> a few days, recommended interval is one week.
>

sorry, this was due to the error found by the kernel test robot. I was not sure
if to wait, resend with the same version number or send the next version.
I guess waiting a few days would have been the best option. Will do so next
time.

Regards,
Lino

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel