Re: [PATCH v1 3/3] serial: 8250_dw: Remove long delay in dw8250_tx_wait_empty()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

While decreasing or removing the delay may work at 4 Mbps, this will break behaviour at lower baud rates.

When adding the delay, I tested down to 9600 baud (the default on many user-facing devices). The combination of 1000 retries of 10 µs was enough to allow the "resize" command (busybox) to function correctly. Without the delay, characters were going missing in the middle of a command sequence, resulting in a messed up terminal window on the other end of the serial cable (minicom). Smaller delays do work for higher baud rates, but support for 9600 and other common low baud rates should be maintained.

A couple of alternatives to removing the delay:

 * Moving from 1,000 retries to 10,000 retries of 1 µs each

 * Having the delay occur only below a certain baud rate


Cheers,
Joshua Scott
________________________________________
From: linux-serial-owner@xxxxxxxxxxxxxxx <linux-serial-owner@xxxxxxxxxxxxxxx> on behalf of Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Sent: Friday, 2 March 2018 12:41 a.m.
To: Greg Kroah-Hartman; linux-serial@xxxxxxxxxxxxxxx; Jiri Slaby
Cc: Andy Shevchenko; Joshua Scott
Subject: [PATCH v1 3/3] serial: 8250_dw: Remove long delay in dw8250_tx_wait_empty()

10 microseconds is a time when 4 characters at speed of 4 Mbps are able
to go to the peer. 4 characters is also a condition to generate Rx timeout
interrupt on the receiver's side. This is not good and might decrease
a throughput and increase a load on the CPU on the remote side.

Remove long delay completely. There is no delay in dw8250_check_lsr()
either, assuming we are fine w/o it on lower speeds.

Cc: Joshua Scott <joshua.scott@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
---
 drivers/tty/serial/8250/8250_dw.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 4ad67707a2cd..da337bf453b6 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -130,7 +130,6 @@ static void dw8250_tx_wait_empty(struct uart_port *p)

                if (lsr & UART_LSR_TEMT)
                        break;
-               udelay (10);
        }
 }

--
2.16.1

--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux