Hi Biju,
On Mon, Sep 26, 2022 at 7:10 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> Due to clk rounding errors on RZ/G2L platforms, it selects a clock source
> with a lower clock rate compared to a higher one.
> For eg: The rounding error (533333333 Hz / 4 * 4 = 533333332 Hz < 5333333
> 33 Hz) selects a clk source of 400 MHz instead of 533.333333 MHz.
>
> This patch fixes this issue by adding a margin of (1/1024) higher to
> the clock rate.
>
> Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> Tested-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
> ---
> v3->v4:
> * Added Tested-by tag from Wolfram.
> * Updated commit description and code comment with real example.
Thanks for the update!
Unfortunately this patch causes a change in the clock frequencies
used on R-Car M2-W:
-clk_summary: sd0 97500000
+clk_summary: sd0 32500000
-clk_summary: sdhi0 97500000
+clk_summary: sdhi0 32500000
-clk_summary: sd3 12786885
+clk_summary: sd3 12187500
-clk_summary: sdhi3 12786885
+clk_summary: sdhi3 12187500
-clk_summary: sd2 12786885
+clk_summary: sd2 12187500
-clk_summary: sdhi2 12786885
+clk_summary: sdhi2 12187500
> --- a/drivers/mmc/host/renesas_sdhi_core.c
> +++ b/drivers/mmc/host/renesas_sdhi_core.c
> @@ -153,10 +154,17 @@ static unsigned int renesas_sdhi_clk_update(struct tmio_mmc_host *host,
> * greater than, new_clock. As we can divide by 1 << i for
> * any i in [0, 9] we want the input clock to be as close as
> * possible, but no greater than, new_clock << i.
> + *
> + * Add an upper limit of 1/1024 rate higher to the clock rate to fix
> + * clk rate jumping to lower rate due to rounding error (eg: RZ/G2L has
> + * 3 clk sources 533.333333 MHz, 400 MHz and 266.666666 MHz. The request
> + * for 533.333333 MHz will selects a slower 400 MHz due to rounding
> + * error (533333333 Hz / 4 * 4 = 533333332 Hz < 533333333 Hz)).
> */
> + new_upper_limit = (new_clock << i) + ((new_clock << i) >> 10);
Mea culpa: while new_clock is a constant inside the loop, i is not!
So it should be moved back inside the loop below.
With that change, R-Car M2-W is happy again, and I noticed no
regression on R-Car H3 ES2.0.
> for (i = min(9, ilog2(UINT_MAX / new_clock)); i >= 0; i--) {
> freq = clk_round_rate(ref_clk, new_clock << i);
> - if (freq > (new_clock << i)) {
> + if (freq > new_upper_limit) {
> /* Too fast; look for a slightly slower option */
> freq = clk_round_rate(ref_clk, (new_clock << i) / 4 * 3);
> if (freq > (new_clock << i))
^^^^^^^^^^^^^^^^
Probably this should become new_upper_limit too, for consistency?
It doesn't seem to matter in my testing, though.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
