Hi Geert,
> Subject: Re: [PATCH] clk: renesas: r9a07g044: Fix 533MHz PLL2/3 clock
> multiplier and divider values
>
> 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.
The new values(for SDHI, SPI mult and M4) are matching with clock list
Document RZG2L_clock_list_r1.1.xlsx and HW manual(page 235/236)
Figure 7.2/7.3 Clock System Diagram.
Yes, we need to have some relaxation for clocks as mentioned
below.
Cheers,
Biju
> 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@linux-
> m68k.org
>
> 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
