Hi Geert,
Thanks for the feedback.
> Subject: Re: [PATCH 2/2] drivers: clk: renesas: r9a07g044-cpg: Add SDHI
> clock and reset entries
>
> Hi Biju,
>
> On Wed, Aug 4, 2021 at 8:08 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> wrote:
> > Add SDHI{0,1} mux, clock and reset entries to CPG driver.
> >
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
>
> Thanks for your patch!
>
> > --- a/drivers/clk/renesas/r9a07g044-cpg.c
> > +++ b/drivers/clk/renesas/r9a07g044-cpg.c
>
> > @@ -77,6 +85,11 @@ static const struct cpg_core_clk
> r9a07g044_core_clks[] __initconst = {
> > DEF_FIXED(".pll6_2", CLK_PLL6_2, CLK_PLL6, 1, 1),
> >
> > DEF_FIXED(".pll2_div2", CLK_PLL2_DIV2, CLK_PLL2, 1, 2),
> > + DEF_FIXED(".clk800fix_c", CLK_800FIX_C, CLK_PLL2, 1, 2),
> > + DEF_FIXED(".clk533fix_c", CLK_533FIX_C, CLK_PLL2, 2, 6),
>
> "2, 6" can be simplified to "1, 3".
OK, Will change it to 1,3.
>
> > + DEF_FIXED(".div_pll2_div8", CLK_DIV_PLL2_DIV8, CLK_800FIX_C, 1,
> 2),
> > + DEF_FIXED(".div_pll2_div12", CLK_DIV_PLL2_DIV12, CLK_533FIX_C,
> > + 1, 2),
>
> I just love the confusing clock naming in the User's Manual!
> DIV_PLL2_DIV8 runs at PLL2 / 4, and DIV_PLL2_DIV12 runs at PLL2 / 6 :-(
>
There is an update on latest HW manual(Rev1.00, sep,2021)
As per this, it is just 400 MHZ and 266 MHz. I will send new patch based on this.
> > +
> > DEF_FIXED(".pll2_div16", CLK_PLL2_DIV16, CLK_PLL2, 1, 16),
> > DEF_FIXED(".pll2_div20", CLK_PLL2_DIV20, CLK_PLL2, 1, 20),
> >
> > @@ -103,6 +116,12 @@ static const struct cpg_core_clk
> r9a07g044_core_clks[] __initconst = {
> > DEF_FIXED("ZT", R9A07G044_CLK_ZT, CLK_PLL3_DIV2_4_2, 1, 1),
> > DEF_MUX("HP", R9A07G044_CLK_HP, SEL_PLL6_2,
> > sel_pll6_2, ARRAY_SIZE(sel_pll6_2), 0,
> > CLK_MUX_HIWORD_MASK),
> > + DEF_SD_MUX("SD0", R9A07G044_CLK_SD0, SEL_SDHI0,
> > + sel_shdi, ARRAY_SIZE(sel_shdi), 0, 0),
> > + DEF_SD_MUX("SD1", R9A07G044_CLK_SD1, SEL_SDHI1,
> > + sel_shdi, ARRAY_SIZE(sel_shdi), 0, 0),
>
> Looks like both .flag and .mux_flags are unneeded?
OK. Will remove it.
Regards,
Biju
>
> > + DEF_FIXED("SD0_DIV4", CLK_SD0_DIV4, R9A07G044_CLK_SD0, 1, 4),
> > + DEF_FIXED("SD1_DIV4", CLK_SD1_DIV4, R9A07G044_CLK_SD1, 1, 4),
> > };
> >
> > static struct rzg2l_mod_clk r9a07g044_mod_clks[] = {
>
> The rest looks good to me.
>
> 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
