Hi Biju,
CC Wolfram (SDHI)
On Tue, Sep 13, 2022 at 10:11 AM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > Subject: Re: [PATCH] clk: renesas: r9a07g044: Fix 533MHz PLL2/3 clock
> > multiplier and divider values
> > On Tue, Sep 13, 2022 at 9:44 AM Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > wrote:
> > > As per the HW manual (Rev.1.10 Apr, 2022) clock rate for 533MHz PLL2
> > > and
> > > PLL3 clocks should be 533 MHz, but with current multiplier and divider
> > > values this resulted to 533.333333 MHz.
> > >
> > > This patch updates the multiplier and divider values for 533 MHz PLL2
> > > and
> > > PLL3 clocks so that we get the exact (533 MHz) values.
> >
> > Does this matter? Is there anything that doesn't work (well) because of
> > this?
>
> Yes, SDHI performance gone bad as it selects 400Mhz clock instead of 533Mhz.
> Similar case for RZ/G2UL, which I am testing it now.
>
> Previously:-
> 533333333->src clk0
> 400000000->src clk1
> 266666666->src clk2
>
> Now:-
> 533000000->src clk0
> 400000000->src clk1
> 266500000->src clk2
>
> If I am correct, with wrong values, it ended
> up in 533333332(parent rate= 133333333 *4) and requested rate 533333333
> and it selected best rate as 400000000.
IC, that is annoying.
However, I don't think the right fix is to change the dividers to
values that do not match the hardware.
Due to the (in)accuracy of clock crystals, the least significant
digits in the above clock rates are not significant anyway.
Perhaps the "if (freq > (new_clock << i))" check in
renesas_sdhi_clk_update() can be slightly relaxed, so it allows
e.g. a 0.1% (or 1/1024th?) higher clock rate than requested?
> > > - DEF_FIXED(".pll2_533", CLK_PLL2_533, CLK_PLL2, 1, 3),
> > > + DEF_FIXED(".pll2_533", CLK_PLL2_533, CLK_PLL2, 533, 1600),
> >
> > I highly doubt the actual hardware is not using a by-3 divider....
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
