From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Fri, 31 Mar 2017 22:23:32 -0600 Subject: [U-Boot] [PATCH v2 2/6] clk: rk3399: fix off-by one during rate calculation in i2c/spi_set_rate In-Reply-To: <1490787091-21008-3-git-send-email-philipp.tomsich@theobroma-systems.com> References: <1490787091-21008-1-git-send-email-philipp.tomsich@theobroma-systems.com> <1490787091-21008-3-git-send-email-philipp.tomsich@theobroma-systems.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 29 March 2017 at 05:31, Philipp Tomsich wrote: > For the RK3399, i2c_set_rate (and by extension: our spi_set_rate, > which had been mindlessly following the template of the i2c_set_rate > implementation) miscalculates the rate returned due to a off-by-one > error resulting from the following sequence of events: > 1. calculates 'src_div := src_freq / target_freq' > 2. stores 'src_div - 1' into the register (the actual divider applied > in hardware is biased by adding 1) > 3. returns the result of the DIV_RATE(src_freq, src_div) macro, which > expects the (decremented) divider from the hardware-register and > implictly adds 1 (i.e. 'DIV_RATE(freq, div) := freq / (div + 1)') > > This can be observed with the SPI driver, which sets a rate of 99MHz > based on the GPLL frequency of 594MHz: the hardware generates a clock > of 99MHz (src_div is 6, the bitfield in the register correctly reads 5), > but reports a frequency of 84MHz (594 / 7) on return. > > To fix, we have two options: > * either we bias (i.e. "DIV_RATE(GPLL, src_div - 1)"), which doesn't > make for a particularily nice read > * we simply call the i2c/spi_get_rate function (introducing additional > overhead for the additional register-read), which reads the divider > from the register and then passes it through the DIV_RATE macro > > Given that this code is not time-critical, the more readable solution > (i.e. calling the appropriate get_rate function) is implemented in this > change. > > Signed-off-by: Philipp Tomsich > Tested-by: Klaus Goger > > --- > > Changes in v2: > - fixes an off-by-one for the RK3399 that cause the SPI module input > clock to be misstated as 84MHz (even though it was running at 99MHz) > > drivers/clk/rockchip/clk_rk3399.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Acked-by: Simon Glass