DW UART EM485 broken because autoconfig resets UART_CAP_NOTEMT

[Filip Štědronský <r.lkml@xxxxxxxxxx>]

Hi,

it seems that the EM485 mode on Designware UART controller is broken.
At the start of transmission, the RTS line is correctly asserted, but
it never gets deasserted.

Checked this with a logic analyzer. See:

    https://regnarg.cz/tmp/em485_bad.sr (PulseView dump)
    https://regnarg.cz/tmp/em485_bad.png

The test system is FriendlyElec NanoPi Neo-LTS (Allwinner H3).
For reference, the test userspace program I used:

    https://regnarg.cz/tmp/485.c

The mechanism seems to be this:

- The DW UART driver sets UART_CAP_NOTEMT when creating the port
  (8250_dwlib.c:dw8250_setup_port) to indicate that the controller
  does not generate interrupt on emptying the shift register.

- This should be then used in 8250_port.c:__stop_tx to use a timer
  instead of an interrupt to trigger DE deassertion.

- However, the port also goes through the 8250 autoconfig mechanism.
  For my controller, dw8250_setup_port does _not_ set UPF_FIXED_TYPE,
  so it tries to autodetect port type. As part of this, the
  up->capabilities field is reset, dropping the UART_CAP_NOTEMT
  (8250_port.c:autoconfig).
    * On this particular controller, the Component Version Register
      (DW_UART_UCV) returns zero, so the dw8250_setup_port function
      returns after this block:

          reg = dw8250_readl_ext(p, DW_UART_UCV);
              return;

- Without UART_CAP_NOTEMT, __stop_tx does not set up a timer and instead
  leaves deasserting DE to an interrupt that never comes.

If I hotfix autoconfig to preserve UART_CAP_NOTEMT, the
EM485 functionality seems to work correctly:


diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 893bc493f662..1c2d24074722 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1140,6 +1140,7 @@ static void autoconfig(struct uart_8250_port *up)
 	struct uart_port *port = &up->port;
 	unsigned long flags;
 	unsigned int old_capabilities;
+	unsigned int preserved_capabilities;
 
 	if (!port->iobase && !port->mapbase && !port->membase)
 		return;
@@ -1155,7 +1156,8 @@ static void autoconfig(struct uart_8250_port *up)
 	 */
 	uart_port_lock_irqsave(port, &flags);
 
-	up->capabilities = 0;
+	preserved_capabilities = up->capabilities & UART_CAP_NOTEMT;
+	up->capabilities = preserved_capabilities;
 	up->bugs = 0;
 
 	if (!(port->flags & UPF_BUGGY_UART)) {
@@ -1266,7 +1268,7 @@ static void autoconfig(struct uart_8250_port *up)
 
 	port->fifosize = uart_config[up->port.type].fifo_size;
 	old_capabilities = up->capabilities;
-	up->capabilities = uart_config[port->type].flags;
+	up->capabilities = uart_config[port->type].flags | preserved_capabilities;
 	up->tx_loadsz = uart_config[port->type].tx_loadsz;
 
 	if (port->type == PORT_UNKNOWN)

And the result:

    https://regnarg.cz/tmp/em485_good.sr (PulseView dump)
    https://regnarg.cz/tmp/em485_good.png

But I am not quite sure what the 'proper' fix is, as I am missing a lot
of context regarding the relationship between DW UART, 8250 and the
various port types.

Filip Štědronský