diff mbox series

[4/4] clk: renesas: r8a774c0: Add RPC clocks

Message ID 20201016121709.8447-5-prabhakar.mahadev-lad.rj@bp.renesas.com
State Accepted
Delegated to: Geert Uytterhoeven
Headers show
Series Renesas RZ/G2x enable RPC clocks | expand

Commit Message

Lad Prabhakar Oct. 16, 2020, 12:17 p.m. UTC
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>
---
 drivers/clk/renesas/r8a774c0-cpg-mssr.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Geert Uytterhoeven Oct. 22, 2020, 2:09 p.m. UTC | #1
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
Lad, Prabhakar Oct. 23, 2020, 11:05 a.m. UTC | #2
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
Geert Uytterhoeven Oct. 23, 2020, 12:42 p.m. UTC | #3
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 mbox series

Patch

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),