All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johan Hovold <johan@kernel.org>
To: Johan Hovold <johan@kernel.org>
Cc: Jonathan Olds <jontio@i4free.co.nz>,
	Michael Dreher <michael@5dot1.de>,
	linux-usb@vger.kernel.org
Subject: [PATCH] USB: serial: ch341: reimplement line-speed handling
Date: Fri,  1 Nov 2019 18:24:10 +0100	[thread overview]
Message-ID: <20191101172410.20419-1-johan@kernel.org> (raw)

The current ch341 divisor algorithm was known to give inaccurate results
for certain higher line speeds. Jonathan Olds <jontio@i4free.co.nz>
investigated this, determined the basic equations used to derive the
divisors and confirmed them experimentally [1].

The equations Jonathan used could be generalised further to:

	baud = 48000000 / (2^(12 - 3 * ps - fact) * div), where

		0 <= ps <= 3,
		0 <= fact <= 1,
		2 <= div <= 256 if fact = 0, or
		9 <= div <= 256 if fact = 1

which will also give better results for lower rates.

Notably the error is reduced for the following standard rates:

	1152000	(4.0% instead of 15% error)
	 921600	(0.16% instead of -7.5% error)
	 576000	(-0.80% instead of -5.6% error)
	    200	(0.16% instead of -0.69% error)
	    134	(-0.05% instead of -0.63% error)
	    110	(0.03% instead of -0.44% error)

but also for many non-standard ones.

The current algorithm also suffered from rounding issues (e.g. 2950000
was rounded to 2 Mbaud instead of 3 Mbaud resulting in a -32% instead of
1.7% error).

The new algorithm was inspired by the current vendor driver even if that
one only handles two higher rates that require fact=1 by hard coding the
corresponding divisors [2].

Michael Dreher <michael@5dot1.de> also did a similar generalisation of
Jonathan's work and has published his results with a very good summary
that provides further insights into how this device works [3].

[1] https://lkml.kernel.org/r/000001d51f34$bad6afd0$30840f70$@co.nz
[2] http://www.wch.cn/download/CH341SER_LINUX_ZIP.html
[3] https://github.com/nospam2000/ch341-baudrate-calculation

Reported-by: Jonathan Olds <jontio@i4free.co.nz>
Tested-by: Jonathan Olds <jontio@i4free.co.nz>
Cc: Michael Dreher <michael@5dot1.de>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/ch341.c | 97 +++++++++++++++++++++++++++++---------
 1 file changed, 75 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index 3bb1fff02bed..29f83ce1696e 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -48,12 +48,6 @@
 #define CH341_BIT_DCD 0x08
 #define CH341_BITS_MODEM_STAT 0x0f /* all bits */
 
-/*******************************/
-/* baudrate calculation factor */
-/*******************************/
-#define CH341_BAUDBASE_FACTOR 1532620800
-#define CH341_BAUDBASE_DIVMAX 3
-
 /* Break support - the information used to implement this was gleaned from
  * the Net/FreeBSD uchcom.c driver by Takanori Watanabe.  Domo arigato.
  */
@@ -144,37 +138,96 @@ static int ch341_control_in(struct usb_device *dev,
 	return 0;
 }
 
+#define CH341_CLKRATE		48000000
+#define CH341_CLK_DIV(ps, fact)	(1 << (12 - 3 * (ps) - (fact)))
+#define CH341_MIN_RATE(ps)	(CH341_CLKRATE / (CH341_CLK_DIV((ps), 1) * 512))
+
+static const speed_t ch341_min_rates[] = {
+	CH341_MIN_RATE(0),
+	CH341_MIN_RATE(1),
+	CH341_MIN_RATE(2),
+	CH341_MIN_RATE(3),
+};
+
+/*
+ * The device line speed is given by the following equation:
+ *
+ *	baud = 48000000 / (2^(12 - 3 * ps - fact) * div), where
+ *
+ *		0 <= ps <= 3,
+ *		0 <= fact <= 1,
+ *		2 <= div <= 256 if fact = 0, or
+ *		9 <= div <= 256 if fact = 1
+ */
+static int ch341_get_divisor(speed_t speed)
+{
+	unsigned int fact, div, clk_div;
+	int ps;
+
+	/*
+	 * Clamp to supported range, this makes the (ps < 0) and (div < 2)
+	 * sanity checks below redundant.
+	 */
+	speed = clamp(speed, 46U, 3000000U);
+
+	/*
+	 * Start with highest possible base clock (fact = 1) that will give a
+	 * divisor strictly less than 512.
+	 */
+	fact = 1;
+	for (ps = 3; ps >= 0; ps--) {
+		if (speed > ch341_min_rates[ps])
+			break;
+	}
+
+	if (ps < 0)
+		return -EINVAL;
+
+	/* Determine corresponding divisor, rounding down. */
+	clk_div = CH341_CLK_DIV(ps, fact);
+	div = CH341_CLKRATE / (clk_div * speed);
+
+	/* Halve base clock (fact = 0) if required. */
+	if (div < 9 || div > 255) {
+		div /= 2;
+		clk_div *= 2;
+		fact = 0;
+	}
+
+	if (div < 2)
+		return -EINVAL;
+
+	/*
+	 * Pick next divisor if resulting rate is closer to the requested one,
+	 * scale up to avoid rounding errors on low rates.
+	 */
+	if (16 * CH341_CLKRATE / (clk_div * div) - 16 * speed >=
+			16 * speed - 16 * CH341_CLKRATE / (clk_div * (div + 1)))
+		div++;
+
+	return (0x100 - div) << 8 | fact << 2 | ps;
+}
+
 static int ch341_set_baudrate_lcr(struct usb_device *dev,
 				  struct ch341_private *priv, u8 lcr)
 {
-	short a;
+	int val;
 	int r;
-	unsigned long factor;
-	short divisor;
 
 	if (!priv->baud_rate)
 		return -EINVAL;
-	factor = (CH341_BAUDBASE_FACTOR / priv->baud_rate);
-	divisor = CH341_BAUDBASE_DIVMAX;
-
-	while ((factor > 0xfff0) && divisor) {
-		factor >>= 3;
-		divisor--;
-	}
 
-	if (factor > 0xfff0)
+	val = ch341_get_divisor(priv->baud_rate);
+	if (val < 0)
 		return -EINVAL;
 
-	factor = 0x10000 - factor;
-	a = (factor & 0xff00) | divisor;
-
 	/*
 	 * CH341A buffers data until a full endpoint-size packet (32 bytes)
 	 * has been received unless bit 7 is set.
 	 */
-	a |= BIT(7);
+	val |= BIT(7);
 
-	r = ch341_control_out(dev, CH341_REQ_WRITE_REG, 0x1312, a);
+	r = ch341_control_out(dev, CH341_REQ_WRITE_REG, 0x1312, val);
 	if (r)
 		return r;
 
-- 
2.23.0


             reply	other threads:[~2019-11-01 17:24 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-01 17:24 Johan Hovold [this message]
2019-11-04 12:08 ` [PATCH] USB: serial: ch341: reimplement line-speed handling Johan Hovold

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191101172410.20419-1-johan@kernel.org \
    --to=johan@kernel.org \
    --cc=jontio@i4free.co.nz \
    --cc=linux-usb@vger.kernel.org \
    --cc=michael@5dot1.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.