Hi Geert,
Thank you for the review.
On Thu, Oct 22, 2020 at 3:09 PM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
>
> Hi Prabhakar,
>
> On Fri, Oct 16, 2020 at 2:17 PM Lad Prabhakar
> <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> wrote:
> > Describe the RPCSRC internal clock and the RPC[D2] clocks derived from it,
> > as well as the RPC-IF module clock, in the RZ/G2E (R8A774C0) CPG/MSSR
> > driver.
> >
> > Inspired by commit 94e3935b5756 ("clk: renesas: r8a77980: Add RPC clocks").
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> > Reviewed-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
>
> Thanks for your patch!
>
> > --- a/drivers/clk/renesas/r8a774c0-cpg-mssr.c
> > +++ b/drivers/clk/renesas/r8a774c0-cpg-mssr.c
> > @@ -73,6 +74,12 @@ static const struct cpg_core_clk r8a774c0_core_clks[] __initconst = {
> > DEF_FIXED(".s2", CLK_S2, CLK_PLL1, 4, 1),
> > DEF_FIXED(".s3", CLK_S3, CLK_PLL1, 6, 1),
> > DEF_FIXED(".sdsrc", CLK_SDSRC, CLK_PLL1, 2, 1),
> > + DEF_BASE(".rpcsrc", CLK_RPCSRC, CLK_TYPE_GEN3_RPCSRC, CLK_PLL1),
> > +
> > + DEF_BASE("rpc", R8A774C0_CLK_RPC, CLK_TYPE_GEN3_RPC,
> > + CLK_RPCSRC),
> > + DEF_BASE("rpcd2", R8A774C0_CLK_RPCD2, CLK_TYPE_GEN3_RPCD2,
> > + R8A774C0_CLK_RPC),
> >
> > DEF_DIV6_RO(".r", CLK_RINT, CLK_EXTAL, CPG_RCKCR, 32),
> >
>
> > @@ -275,6 +283,10 @@ static int __init r8a774c0_cpg_mssr_init(struct device *dev)
> > return rcar_gen3_cpg_init(cpg_pll_config, 0, cpg_mode);
> > }
> >
> > +static const struct clk_div_table cpg_rpcsrc_div_table[] = {
> > + { 0, 5 }, { 1, 3 }, { 2, 8 }, {3, 2}, {0, 0},
> > +};
>
> The above models RPCSRC as a clock generated by dividing PLL1 by either
> 5, 3, 8, or 2. This does not match the hardware user's manual, which
> states that RPCSRC is either PLL1 divided by 5 or 3, or PLL0 divided by
> 8 or 2.
>
Oops I completely missed that.
But as per the manual (R-Car manual Rev.2.20) which I am referring to
5, 3 and 2 are sourced from PLL1 and 5/8 (ie D3/E3) are sourced from
PLL0.
> I think you need a new clock type (CLK_TYPE_GEN3E_RPCSRC, as it applies
> to RZ/G2E, and R-Car E3?), which registers a composite clock consisting
> of a mux and divider. This is a bit similar to the RPC/RPCD2 clocks,
> which are composite clocks consisting of a divider and a gate.
>
atm rcar_gen3_cpg_clk_register() only supports single parent, so if I
am getting it right you mean I need to add two separate entries for
RPSRC one with PLL0 and one with PLL1 ?
> Note that R-Car D3 is similar, except that PLL0 is divided by 5 or 2, which
> means yet another clock type (and div_table).
>
I'm a bit confused here for D3 PLL0 is divided by 5 (n=5) ?
Cheers,
Prabhakar
> 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
