USB: serial: ch341: reimplement line-speed handling
authorJohan Hovold <johan@kernel.org>
Fri, 1 Nov 2019 17:24:10 +0000 (18:24 +0100)
committerJohan Hovold <johan@kernel.org>
Mon, 4 Nov 2019 11:49:51 +0000 (12:49 +0100)
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:

baudrate = 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.
requesting 2950000 Bd resulted in a rate of 2 MBd instead of 3 MBd and
thus 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

index 3bb1fff..df582fe 100644 (file)
 #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:
+ *
+ *     baudrate = 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;