rockchip: spi: rewrite rkspi_set_clk for a more conservative baudrate setting
authorPhilipp Tomsich <philipp.tomsich@theobroma-systems.com>
Thu, 20 Apr 2017 20:05:52 +0000 (22:05 +0200)
committerSimon Glass <sjg@chromium.org>
Wed, 10 May 2017 19:37:21 +0000 (13:37 -0600)
The baudrate in rkspi was calculated by using an integer division
(which implicitly discarded any fractional result), then rounding to
an even number and finally clamping to 0xfffe using a bitwise AND
operator.  This introduced two issues:
1) for very small baudrates (overflowing the 0xfffe range), the
   bitwise-AND generates rather random-looking (wildly varying)
   actual output bitrates
2) for higher baudrates, the calculation tends to 'err towards a
   higher baudrate' with the actual error increasing as the dividers
   become very small. E.g., with a 99MHz input clock, a request
   for a 20MBit baudrate (99/20 = 4.95), a 24.75 MBit would be use
   (which amounts to a 23.75% error)... for a 34 MBit request this
   would be an actual outbout of 49.5 Mbit (i.e. a 45% error).

This change rewrites the divider selection (i.e. baudrate calculation)
by making sure that
a) for the normal case: the largest representable baudrate below the
   requested rate will be chosen;
b) for the denormal case (i.e. when the divider can no longer be
   represented), the lowest representable baudrate is chosen.

Even though the denormal case (b) may be of little concern in real
world applications (even with a 198MHz input clock, this will only
happen at below approx. 3kHz/3kBit), our board-verification team kept
complaining.

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Tested-by: Klaus Goger <klaus.goger@theobroma-systems.com>
drivers/spi/rk_spi.c

index 0c627d963d8d49fe9c4d7e9a1f5f1537a72008c7..050eacb27ccd57a76deaaaa80c18657ecc7e4bb5 100644 (file)
@@ -79,12 +79,31 @@ static void rkspi_enable_chip(struct rockchip_spi *regs, bool enable)
 
 static void rkspi_set_clk(struct rockchip_spi_priv *priv, uint speed)
 {
-       uint clk_div;
+       /*
+        * We should try not to exceed the speed requested by the caller:
+        * when selecting a divider, we need to make sure we round up.
+        */
+       uint clk_div = DIV_ROUND_UP(priv->input_rate, speed);
+
+       /* The baudrate register (BAUDR) is defined as a 32bit register where
+        * the upper 16bit are reserved and having 'Fsclk_out' in the lower
+        * 16bits with 'Fsclk_out' defined as follows:
+        *
+        *   Fsclk_out = Fspi_clk/ SCKDV
+        *   Where SCKDV is any even value between 2 and 65534.
+        */
+       if (clk_div > 0xfffe) {
+               clk_div = 0xfffe;
+               debug("%s: can't divide down to %d hz (actual will be %d hz)\n",
+                     __func__, speed, priv->input_rate / clk_div);
+       }
+
+       /* Round up to the next even 16bit number */
+       clk_div = (clk_div + 1) & 0xfffe;
 
-       clk_div = clk_get_divisor(priv->input_rate, speed);
        debug("spi speed %u, div %u\n", speed, clk_div);
 
-       writel(clk_div, &priv->regs->baudr);
+       clrsetbits_le32(&priv->regs->baudr, 0xffff, clk_div);
        priv->last_speed_hz = speed;
 }