diff mbox series

USB: serial: ch341: fix character loss at high transfer rates

Message ID 20210724152739.18726-1-w@1wt.eu (mailing list archive)
State Accepted
Commit 3c18e9baee0ef97510dcda78c82285f52626764b
Headers show
Series USB: serial: ch341: fix character loss at high transfer rates | expand

Commit Message

Willy Tarreau July 24, 2021, 3:27 p.m. UTC
The chip supports high transfer rates, but with the small default buffers
(64 bytes read), some entire blocks are regularly lost. This typically
happens at 1.5 Mbps (which is the default speed on Rockchip devices) when
used as a console to access U-Boot where the output of the "help" command
misses many lines and where "printenv" mangles the environment.

The FTDI driver doesn't suffer at all from this. One difference is that
it uses 512 bytes rx buffers and 256 bytes tx buffers. Adopting these
values completely resolved the issue, even the output of "dmesg" is
reliable. I preferred to leave the Tx value unchanged as it is not
involved in this issue, while a change could increase the risk of
triggering the same issue with other devices having too small buffers.

I verified that it backports well (and works) at least to 5.4. It's of
low importance enough to be dropped where it doesn't trivially apply
anymore.

Cc: stable@vger.kernel.org
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
 drivers/usb/serial/ch341.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Johan Hovold July 29, 2021, 11:51 a.m. UTC | #1
On Sat, Jul 24, 2021 at 05:27:39PM +0200, Willy Tarreau wrote:
> The chip supports high transfer rates, but with the small default buffers
> (64 bytes read), some entire blocks are regularly lost. This typically
> happens at 1.5 Mbps (which is the default speed on Rockchip devices) when
> used as a console to access U-Boot where the output of the "help" command
> misses many lines and where "printenv" mangles the environment.
> 
> The FTDI driver doesn't suffer at all from this. One difference is that
> it uses 512 bytes rx buffers and 256 bytes tx buffers. Adopting these
> values completely resolved the issue, even the output of "dmesg" is
> reliable. I preferred to leave the Tx value unchanged as it is not
> involved in this issue, while a change could increase the risk of
> triggering the same issue with other devices having too small buffers.

Since these device do not support automatic flow control this is indeed
the best we can to do here (otherwise I'd probably prefer framing it
more as an optimisation than a fix).

> I verified that it backports well (and works) at least to 5.4. It's of
> low importance enough to be dropped where it doesn't trivially apply
> anymore.

This should be fine to backport to all stable trees.

> Cc: stable@vger.kernel.org
> Signed-off-by: Willy Tarreau <w@1wt.eu>

Now applied for 5.14, thanks.

Johan
diff mbox series

Patch

diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index 2db917eab799..8a521b5ea769 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -851,6 +851,7 @@  static struct usb_serial_driver ch341_device = {
 		.owner	= THIS_MODULE,
 		.name	= "ch341-uart",
 	},
+	.bulk_in_size      = 512,
 	.id_table          = id_table,
 	.num_ports         = 1,
 	.open              = ch341_open,