serial: 8250_of: set UART_CAP_NOTEMT for rts-gpios RS485 direction

PATCH In review Nicolas Thibert 3 participants 4 messages Sep 7 – Sep 23 TTY layer and serial drivers

Share & tools

Nicolas Thibert

author

2026-09-07 17:30 UTC #0 lore raw

PATCH 1/1 serial: 8250_of: set UART_CAP_NOTEMT for rts-gpios RS485 direction

__stop_tx() (8250_port.c) only calls the RS485 rs485_stop_tx() hook (which de-asserts the direction GPIO/RTS line) once it has observed both UART_LSR_THRE and UART_LSR_TEMT for the last byte. If TEMT is never seen and the driver hasn't set UART_CAP_NOTEMT, the function returns without scheduling any retry -- the direction line is left asserted (driver enabled) forever, with nothing to un-stick it short of another kernel-visible LSR event.

of_platform_serial_setup()/of_platform_serial_probe() unconditionally wire up the generic em485 GPIO-RTS RS485 support (rs485_config/rs485_start_tx/rs485_stop_tx) for every port they register, but never set UART_CAP_NOTEMT, so any board using this driver whose 16550-compatible core doesn't reliably surface TEMT for its shift register hits the stuck-direction-GPIO case above.

Confirmed live on an ath79 QCA9531 board (SoC-internal ns16550a- compatible UART, RS485 transceiver DE/RE tied together on a GPIO via rts-gpios, linux,rs485-enabled-at-boot-time): the direction GPIO correctly asserts for the duration of a transmit, but never de-asserts afterwards -- confirmed by sampling the GPIO's debugfs state through and after a multi-hundred-byte write, on both the first transmit and repeated back-to-back transmits. Setting UART_CAP_NOTEMT, which makes __stop_tx() fall back to a frame-time- based timer instead of waiting indefinitely on TEMT, makes the direction GPIO reliably return low right after each transmit completes.

Scope the fix to ports that declare a GPIO-controlled direction line (rts-gpios), rather than setting it unconditionally for every port this driver registers: this is the class of hardware actually affected (RTS state has to be explicitly un-stuck by software, unlike a UART's native RTS pin), and it avoids adding the extra frame-time margin to ports relying on the native RTS pin, which this has not been observed to need.

Set it in of_platform_serial_probe(), after the existing "if (port8250.port.fifosize) port8250.capabilities = UART_CAP_FIFO;" assignment, rather than in of_platform_serial_setup(): that later plain assignment (not "|=") unconditionally overwrites port8250.capabilities on any port whose "fifo-size" DT property is set, silently discarding a capability bit set earlier in setup(). Not observed on the reporter's own board (it has no "fifo-size" property, so port.fifosize stays 0 and that assignment is skipped), but a real regression waiting to happen on any other of_platform_serial user that does set "fifo-size" -- which is a common, documented property for this binding.

Signed-off-by Nicolas Thibert <[email protected]>

Assisted-by LLM (Claude Sonnet 5, Anthropic)

---
 drivers/tty/serial/8250/8250_of.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

drivers/tty/serial/8250/8250_of.c +18 -0

@@ -233,6 +233,24 @@ static int of_platform_serial_probe(struct platform_device *ofdev)
 			&port8250.overrun_backoff_time_ms) != 0)
 		port8250.overrun_backoff_time_ms = 0;
 
+	/*
+	 * This generic driver never enables a dedicated line-status
+	 * interrupt on TEMT, so for ports whose RS485 direction is
+	 * controlled via a GPIO (rts-gpios) rather than the native RTS
+	 * pin, __stop_tx() (8250_port.c) can see THRE without TEMT on the
+	 * last byte and bail out without ever retrying -- leaving the
+	 * direction GPIO stuck asserted after the last byte sent, on
+	 * hardware whose shift register doesn't reliably surface TEMT.
+	 * UART_CAP_NOTEMT makes it fall back to a frame-time-based timer
+	 * instead of waiting on that interrupt. Scoped to rts-gpios users
+	 * only, to avoid changing timing for ports relying on the native
+	 * RTS pin, which this has not been observed to affect. Set here,
+	 * after the fifosize-based capabilities assignment above, so it
+	 * isn't clobbered by it.
+	 */
+	if (of_property_present(ofdev->dev.of_node, "rts-gpios"))
+		port8250.capabilities |= UART_CAP_NOTEMT;
+
 	ret = serial8250_register_8250_port(&port8250);
 	if (ret < 0)
 		goto err_dispose;

[email protected]

AI review

2026-09-07 17:36 UTC #1 lore raw

Sashiko has reviewed this patch and found no issues. It looks great!

Quoted 15 lines

__stop_tx() (8250_port.c) only calls the RS485 rs485_stop_tx() hook (which de-asserts the direction GPIO/RTS line) once it has observed both UART_LSR_THRE and UART_LSR_TEMT for the last byte. If TEMT is never seen and the driver hasn't set UART_CAP_NOTEMT, the function returns without scheduling any retry -- the direction line is left asserted (driver enabled) forever, with nothing to un-stick it short of another kernel-visible LSR event.
of_platform_serial_setup()/of_platform_serial_probe() unconditionally wire up the generic em485 GPIO-RTS RS485 support (rs485_config/rs485_start_tx/rs485_stop_tx) for every port they register, but never set UART_CAP_NOTEMT, so any board using this driver whose 16550-compatible core doesn't reliably surface TEMT for its shift register hits the stuck-direction-GPIO case above.

Quoted 6 lines

[ ... ]
regression waiting to happen on any other of_platform_serial user that does set "fifo-size" -- which is a common, documented property for this binding.
Signed-off-by Nicolas Thibert <[email protected]>

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Greg KH

maintainer

2026-09-23 12:20 UTC #2 lore raw

In reply to Nicolas Thibert

Quoted 35 lines · Nicolas Thibert

__stop_tx() (8250_port.c) only calls the RS485 rs485_stop_tx() hook (which de-asserts the direction GPIO/RTS line) once it has observed both UART_LSR_THRE and UART_LSR_TEMT for the last byte. If TEMT is never seen and the driver hasn't set UART_CAP_NOTEMT, the function returns without scheduling any retry -- the direction line is left asserted (driver enabled) forever, with nothing to un-stick it short of another kernel-visible LSR event.
of_platform_serial_setup()/of_platform_serial_probe() unconditionally wire up the generic em485 GPIO-RTS RS485 support (rs485_config/rs485_start_tx/rs485_stop_tx) for every port they register, but never set UART_CAP_NOTEMT, so any board using this driver whose 16550-compatible core doesn't reliably surface TEMT for its shift register hits the stuck-direction-GPIO case above.
Confirmed live on an ath79 QCA9531 board (SoC-internal ns16550a- compatible UART, RS485 transceiver DE/RE tied together on a GPIO via rts-gpios, linux,rs485-enabled-at-boot-time): the direction GPIO correctly asserts for the duration of a transmit, but never de-asserts afterwards -- confirmed by sampling the GPIO's debugfs state through and after a multi-hundred-byte write, on both the first transmit and repeated back-to-back transmits. Setting UART_CAP_NOTEMT, which makes __stop_tx() fall back to a frame-time- based timer instead of waiting indefinitely on TEMT, makes the direction GPIO reliably return low right after each transmit completes.
Scope the fix to ports that declare a GPIO-controlled direction line (rts-gpios), rather than setting it unconditionally for every port this driver registers: this is the class of hardware actually affected (RTS state has to be explicitly un-stuck by software, unlike a UART's native RTS pin), and it avoids adding the extra frame-time margin to ports relying on the native RTS pin, which this has not been observed to need.

Quoted 11 lines

Set it in of_platform_serial_probe(), after the existing "if (port8250.port.fifosize) port8250.capabilities = UART_CAP_FIFO;" assignment, rather than in of_platform_serial_setup(): that later plain assignment (not "|=") unconditionally overwrites port8250.capabilities on any port whose "fifo-size" DT property is set, silently discarding a capability bit set earlier in setup(). Not observed on the reporter's own board (it has no "fifo-size" property, so port.fifosize stays 0 and that assignment is skipped), but a real regression waiting to happen on any other of_platform_serial user that does set "fifo-size" -- which is a common, documented property for this binding.

Please rewrite this to be in english and says things properly.

Quoted 3 lines

Signed-off-by Nicolas Thibert <[email protected]>
Assisted-by LLM (Claude Sonnet 5, Anthropic)

signed-off-by goes last.

Quoted 15 lines

---
 drivers/tty/serial/8250/8250_of.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

drivers/tty/serial/8250/8250_of.c +5 -0

@@ -233,6 +233,24 @@ static int of_platform_serial_probe(struct platform_device *ofdev)
 			&port8250.overrun_backoff_time_ms) != 0)
 		port8250.overrun_backoff_time_ms = 0;
 
+	/*
+	 * This generic driver never enables a dedicated line-status
+	 * interrupt on TEMT, so for ports whose RS485 direction is
+	 * controlled via a GPIO (rts-gpios) rather than the native RTS
+	 * pin, __stop_tx() (8250_port.c) can see THRE without TEMT on the

drivers/tty/serial/8250/8250_of.c static int of_platform_serial_probe(struct platform_device *ofdev) · line 250

Quoted 10 lines

drivers/tty/serial/8250/8250_of.c +10 -0

+	 * last byte and bail out without ever retrying -- leaving the
+	 * direction GPIO stuck asserted after the last byte sent, on
+	 * hardware whose shift register doesn't reliably surface TEMT.
+	 * UART_CAP_NOTEMT makes it fall back to a frame-time-based timer
+	 * instead of waiting on that interrupt. Scoped to rts-gpios users
+	 * only, to avoid changing timing for ports relying on the native
+	 * RTS pin, which this has not been observed to affect. Set here,
+	 * after the fifosize-based capabilities assignment above, so it
+	 * isn't clobbered by it.
+	 */

Is this comment really needed?

drivers/tty/serial/8250/8250_of.c static int of_platform_serial_probe(struct platform_device *ofdev) · line 251

Quoted 1 line

drivers/tty/serial/8250/8250_of.c +1 -0
+	if (of_property_present(ofdev->dev.of_node, "rts-gpios"))

What will change from this now on existing systems?

How was this tested?

And why was this sent twice?

thanks,

greg k-h

Nicolas Thibert

author

2026-09-23 13:42 UTC #3 lore raw

Hello,

Comment was just to understand the modification done. I think the double submission is because I first try in standard mail but then with git-email
I tested it on my QCA4531 (QCA953x base) board with an RS485 converter (CA-IS2092A). The converter use a standard GPIO to drive the bus between input and output. When declaring such device in dts, the GPIO was not used remaining in a steady state and preventing from receiving RS485 answers
Mod neede for such a dts :

&uart {
    pinctrl-names = "default";
    rts-gpios = <&gpio 3 GPIO_ACTIVE_HIGH>;
    linux,rs485-enabled-at-boot-time;
    status = "okay";
};

Regards

Nicolas

Quoted 97 lines · Greg KH

Quoted 35 lines · Nicolas Thibert
__stop_tx() (8250_port.c) only calls the RS485 rs485_stop_tx() hook (which de-asserts the direction GPIO/RTS line) once it has observed both UART_LSR_THRE and UART_LSR_TEMT for the last byte. If TEMT is never seen and the driver hasn't set UART_CAP_NOTEMT, the function returns without scheduling any retry -- the direction line is left asserted (driver enabled) forever, with nothing to un-stick it short of another kernel-visible LSR event.
of_platform_serial_setup()/of_platform_serial_probe() unconditionally wire up the generic em485 GPIO-RTS RS485 support (rs485_config/rs485_start_tx/rs485_stop_tx) for every port they register, but never set UART_CAP_NOTEMT, so any board using this driver whose 16550-compatible core doesn't reliably surface TEMT for its shift register hits the stuck-direction-GPIO case above.
Confirmed live on an ath79 QCA9531 board (SoC-internal ns16550a- compatible UART, RS485 transceiver DE/RE tied together on a GPIO via rts-gpios, linux,rs485-enabled-at-boot-time): the direction GPIO correctly asserts for the duration of a transmit, but never de-asserts afterwards -- confirmed by sampling the GPIO's debugfs state through and after a multi-hundred-byte write, on both the first transmit and repeated back-to-back transmits. Setting UART_CAP_NOTEMT, which makes __stop_tx() fall back to a frame-time- based timer instead of waiting indefinitely on TEMT, makes the direction GPIO reliably return low right after each transmit completes.
Scope the fix to ports that declare a GPIO-controlled direction line (rts-gpios), rather than setting it unconditionally for every port this driver registers: this is the class of hardware actually affected (RTS state has to be explicitly un-stuck by software, unlike a UART's native RTS pin), and it avoids adding the extra frame-time margin to ports relying on the native RTS pin, which this has not been observed to need.
Quoted 11 lines
Set it in of_platform_serial_probe(), after the existing "if (port8250.port.fifosize) port8250.capabilities = UART_CAP_FIFO;" assignment, rather than in of_platform_serial_setup(): that later plain assignment (not "|=") unconditionally overwrites port8250.capabilities on any port whose "fifo-size" DT property is set, silently discarding a capability bit set earlier in setup(). Not observed on the reporter's own board (it has no "fifo-size" property, so port.fifosize stays 0 and that assignment is skipped), but a real regression waiting to happen on any other of_platform_serial user that does set "fifo-size" -- which is a common, documented property for this binding.
Please rewrite this to be in english and says things properly.
Quoted 3 lines
Signed-off-by Nicolas Thibert <[email protected]>
Assisted-by LLM (Claude Sonnet 5, Anthropic)
signed-off-by goes last.
Quoted 15 lines
---
 drivers/tty/serial/8250/8250_of.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
drivers/tty/serial/8250/8250_of.c +5 -0
@@ -233,6 +233,24 @@ static int of_platform_serial_probe(struct platform_device *ofdev)
                      &port8250.overrun_backoff_time_ms) != 0)
              port8250.overrun_backoff_time_ms = 0;
 
+     /*
+      * This generic driver never enables a dedicated line-status
+      * interrupt on TEMT, so for ports whose RS485 direction is
+      * controlled via a GPIO (rts-gpios) rather than the native RTS
+      * pin, __stop_tx() (8250_port.c) can see THRE without TEMT on the
drivers/tty/serial/8250/8250_of.c static int of_platform_serial_probe(struct platform_device *ofdev) · line 250
Quoted 10 lines
drivers/tty/serial/8250/8250_of.c +10 -0
+      * last byte and bail out without ever retrying -- leaving the
+      * direction GPIO stuck asserted after the last byte sent, on
+      * hardware whose shift register doesn't reliably surface TEMT.
+      * UART_CAP_NOTEMT makes it fall back to a frame-time-based timer
+      * instead of waiting on that interrupt. Scoped to rts-gpios users
+      * only, to avoid changing timing for ports relying on the native
+      * RTS pin, which this has not been observed to affect. Set here,
+      * after the fifosize-based capabilities assignment above, so it
+      * isn't clobbered by it.
+      */
Is this comment really needed?
drivers/tty/serial/8250/8250_of.c static int of_platform_serial_probe(struct platform_device *ofdev) · line 251
Quoted 1 line
drivers/tty/serial/8250/8250_of.c +1 -0
+     if (of_property_present(ofdev->dev.of_node, "rts-gpios"))
What will change from this now on existing systems?
How was this tested?
And why was this sent twice?
thanks,
greg k-h