From 9fc354e246fe466ad634617e12d092d6053503ed Mon Sep 17 00:00:00 2001 From: Philipp Tomsich Date: Thu, 20 Apr 2017 22:05:52 +0200 Subject: [PATCH] rockchip: spi: rewrite rkspi_set_clk for a more conservative baudrate setting 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 Tested-by: Klaus Goger --- drivers/spi/rk_spi.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/drivers/spi/rk_spi.c b/drivers/spi/rk_spi.c index 0c627d963d..050eacb27c 100644 --- a/drivers/spi/rk_spi.c +++ b/drivers/spi/rk_spi.c @@ -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; } -- 2.39.5