Re: ch341 garbage read with 5.5.x kernel

[Johan Hovold <johan@kernel.org>]

On Wed, Feb 05, 2020 at 08:53:55AM +0100, jakub nantl wrote:
> On 05. 02. 20 8:43, Johan Hovold wrote:
> > There were some fixes to the baudrate handling that went into 5.5 that
> > are likely related to this.
> >
> > These changes provide more accurate output rates, but I have since
> > received one report that it may inadvertently have made the device more
> > sensitive to errors in the input rate. In that case, the reporter
> > switched to a baudrate that matches his actual rate which was 117647
> > rather than 115200 (i.e. 2.1% error) and that addressed the problem.
> >
> > Which baudrate are you using here?
> 
> my baudrate is 115200, how can I get "my" actual rate ?

You can always use a scope or logic analyser, but you can derive it
theoretically if you know what oscillator frequency the arduino is
using. You should be able to find more details in the datasheet for the
MCU in question.

For example, if you look at section 19.11 in the ATmega238p datasheet
you see that with a 16 MHz clock you may end up with a -3.5% or 2.1%
error at 115200. The latter is likely the 117647 rate mentioned above.

That said, I have a hypothesis for how we may get the best of both
worlds here.

Can you try the below patch which restores a component included in the
first version of the new algorithm, but which I ultimately deemed
redundant?

Johan


From 0b833ae9dde819ffb41c8d16d3591484d1eab04c Mon Sep 17 00:00:00 2001
From: Johan Hovold <johan@kernel.org>
Date: Wed, 5 Feb 2020 09:33:15 +0100
Subject: [PATCH] USB: serial: ch341: prefer lower divisors

Although it was assumed to not make a difference, not using the factor 2
prescaler appears to make the receiver more susceptible for errors in
the input rate. Specifically, there are reports of problems with devices
that cannot generate a 115200 rate with a smaller error than 2.1% (e.g.
117647 bps).

So whenever possible, enable the factor 2 prescaler and halve the
divisor in order to use settings closer to that of the previous
algorithm.

Fixes: 35714565089e ("USB: serial: ch341: reimplement line-speed handling")
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/ch341.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index df582fe855f0..86686e60238a 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -205,6 +205,12 @@ static int ch341_get_divisor(speed_t speed)
 			16 * speed - 16 * CH341_CLKRATE / (clk_div * (div + 1)))
 		div++;
 
+	/* Prefer lower base clock (fact = 0) if even divisor. */
+	if (fact == 1 && div % 2 == 0) {
+		div >>= 1;
+		fact = 0;
+	}
+
 	return (0x100 - div) << 8 | fact << 2 | ps;
 }
 
-- 
2.24.1