All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Glass <sjg@chromium.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v2 2/6] clk: rk3399: fix off-by one during rate calculation in i2c/spi_set_rate
Date: Fri, 31 Mar 2017 22:23:32 -0600	[thread overview]
Message-ID: <CAPnjgZ3_3MPkD4xka1Y9aqzTudFpuR83PxhKAracyYx_bmF5dg@mail.gmail.com> (raw)
In-Reply-To: <1490787091-21008-3-git-send-email-philipp.tomsich@theobroma-systems.com>

On 29 March 2017 at 05:31, Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> 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 <philipp.tomsich@theobroma-systems.com>
> Tested-by: Klaus Goger <klaus.goger@theobroma-systems.com>
>
> ---
>
> 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 <sjg@chromium.org>

  reply	other threads:[~2017-04-01  4:23 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-29 11:31 [U-Boot] [PATCH v2 0/6] rockchip: spl: rk3399: prepare to have SPI config per-board Philipp Tomsich
2017-03-29 11:31 ` [U-Boot] [PATCH v2 1/6] rockchip: clk: rk3399: add clock support for SCLK_SPI1 and SCLK_SPI5 Philipp Tomsich
2017-03-29 11:31 ` [U-Boot] [PATCH v2 2/6] clk: rk3399: fix off-by one during rate calculation in i2c/spi_set_rate Philipp Tomsich
2017-04-01  4:23   ` Simon Glass [this message]
2017-03-29 11:31 ` [U-Boot] [PATCH v2 3/6] rockchip: pinctrl: rk3399: add support for the SPI5 controller Philipp Tomsich
2017-04-01  4:23   ` Simon Glass
2017-03-29 11:31 ` [U-Boot] [PATCH v2 4/6] rockchip: spi: enable support for the rk_spi driver for the RK3399 Philipp Tomsich
2017-04-01  4:23   ` Simon Glass
2017-03-29 11:31 ` [U-Boot] [PATCH v2 5/6] rockchip: spi: rk3399: move CONFIG_SPI and CONFIG_SPI_FLASH to defconfig Philipp Tomsich
2017-04-01  4:23   ` Simon Glass
2017-04-05  1:05     ` Simon Glass
2017-03-29 11:31 ` [U-Boot] [PATCH v2 6/6] rockchip: spl: rk3399: enable SPL_SPI_LOAD if SPI is enabled for SPL Philipp Tomsich
2017-04-01  4:23   ` Simon Glass

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=CAPnjgZ3_3MPkD4xka1Y9aqzTudFpuR83PxhKAracyYx_bmF5dg@mail.gmail.com \
    --to=sjg@chromium.org \
    --cc=u-boot@lists.denx.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.