Hi Biju, On Mon, Jul 26, 2021 at 1:43 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote: > > Subject: Re: [PATCH net-next 03/18] drivers: clk: renesas: r9a07g044-cpg: > > Add ethernet clock sources > > On Thu, Jul 22, 2021 at 4:14 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> > > wrote: > > > Ethernet reference clock can be sourced from PLL5_2 or PLL6_2. Add > > > support for ethernet source clock selection using SEL_PLL_6_2 mux. > > > > > > 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 > > > @@ -35,8 +35,10 @@ enum clk_ids { > > > CLK_PLL3_DIV4, > > > CLK_PLL4, > > > CLK_PLL5, > > > + CLK_PLL5_2, > > > > Why do you need this? We already have CLK_PLL5_DIV2? > > As per clock list HP =250 MHz. > > With PLL5_DIV2 the frequency, what we get is 3000 x 1/2 = 1500 MHz, which is incorrect > > There is some issue with HW manual. It is not clear. I have requested to fix those issue. > > Actually PLL5 generates 3 clk outputs (As per page 403 of manual) > > FOUTPOSTDIV= 3000 > FOUT3 = 500 > FOUT1PH0 = 1500 > > To match the clock list document and Figure 8.3 Clock system diagram for HP clock. > This clock has to be FOUT3. > > FOUT3(second PLL5 clock source)* 1/2 =250 MHz(HP clock). > > On the HW manual, Divider name for PLL5 is mentioned as PLL_5_2_DIV12 and for PLL6 as PLL6_2_DIV2. > > > > > > CLK_PLL5_DIV2, > > > CLK_PLL6, > > > + CLK_PLL6_DIV2, > > > CLK_P1_DIV2, > > > > > > /* Module Clocks */ > > > @@ -53,6 +55,9 @@ static const struct clk_div_table dtable_1_32[] = { > > > {0, 0}, > > > }; > > > > > > +/* Mux clock tables */ > > > +static const char * const sel_pll6_2[] = { ".pll6_2_div2", > > > +".pll5_2_div2" }; > > > > ".pll6_div2", ".pll5_div2" > > Basically PLL6 generates 2 clock outputs. That is the reason it is termed as PLL6_2 for second clock > And pll5 generates 3 clock outputs and pll5_2(second clock source) is used for HP. > > To match the register definition, I will change this to ".pll6_2_div2" and ".pll5_2_div12". Is it ok? OK, thanks for the explanation! 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