Message ID | 20201016121709.8447-5-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive) |
---|---|
State | Superseded, archived |
Headers | show |
Series | Renesas RZ/G2x enable RPC clocks | expand |
Hi Prabhakar, On Fri, Oct 16, 2020 at 2:17 PM Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> 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@bp.renesas.com> > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> 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. 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. 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). Gr{oetje,eeting}s, Geert
Hi Geert, Thank you for the review. On Thu, Oct 22, 2020 at 3:09 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Prabhakar, > > On Fri, Oct 16, 2020 at 2:17 PM Lad Prabhakar > <prabhakar.mahadev-lad.rj@bp.renesas.com> 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@bp.renesas.com> > > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> > > 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@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
Hi Prabhakar, On Fri, Oct 23, 2020 at 1:06 PM Lad, Prabhakar <prabhakar.csengg@gmail.com> wrote: > On Thu, Oct 22, 2020 at 3:09 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Fri, Oct 16, 2020 at 2:17 PM Lad Prabhakar > > <prabhakar.mahadev-lad.rj@bp.renesas.com> 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@bp.renesas.com> > > > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > 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
diff --git a/drivers/clk/renesas/r8a774c0-cpg-mssr.c b/drivers/clk/renesas/r8a774c0-cpg-mssr.c index 9fc9fa9e531a..1615b31c32ee 100644 --- a/drivers/clk/renesas/r8a774c0-cpg-mssr.c +++ b/drivers/clk/renesas/r8a774c0-cpg-mssr.c @@ -44,6 +44,7 @@ enum clk_ids { CLK_S2, CLK_S3, CLK_SDSRC, + CLK_RPCSRC, CLK_RINT, CLK_OCO, @@ -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), @@ -199,6 +206,7 @@ static const struct mssr_mod_clk r8a774c0_mod_clks[] __initconst = { DEF_MOD("can-fd", 914, R8A774C0_CLK_S3D2), DEF_MOD("can-if1", 915, R8A774C0_CLK_S3D4), DEF_MOD("can-if0", 916, R8A774C0_CLK_S3D4), + DEF_MOD("rpc-if", 917, R8A774C0_CLK_RPCD2), DEF_MOD("i2c6", 918, R8A774C0_CLK_S3D2), DEF_MOD("i2c5", 919, R8A774C0_CLK_S3D2), DEF_MOD("i2c-dvfs", 926, R8A774C0_CLK_CP), @@ -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}, +}; + const struct cpg_mssr_info r8a774c0_cpg_mssr_info __initconst = { /* Core Clocks */ .core_clks = r8a774c0_core_clks, @@ -287,6 +299,8 @@ const struct cpg_mssr_info r8a774c0_cpg_mssr_info __initconst = { .num_mod_clks = ARRAY_SIZE(r8a774c0_mod_clks), .num_hw_mod_clks = 12 * 32, + .cpg_rpcsrc_div_table = cpg_rpcsrc_div_table, + /* Critical Module Clocks */ .crit_mod_clks = r8a774c0_crit_mod_clks, .num_crit_mod_clks = ARRAY_SIZE(r8a774c0_crit_mod_clks),