Hi Prabhakar, On Fri, Oct 23, 2020 at 1:06 PM Lad, Prabhakar <prabhakar.csengg@xxxxxxxxx> wrote: > On Thu, Oct 22, 2020 at 3:09 PM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote: > > 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. True, I misread the clocksource of the last entry. > > 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 ? You can encode a second parent in the high halfword, cfr. what CLK_TYPE_GEN3_MDSEL and CLK_TYPE_GEN3_RCKSEL. Note to myself: convert cpg_core_clk to a union, to make encoding and decoding easier. > > 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) ? Sorry, same misreading as above. Have a nice weekend! 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