diff mbox series

[v3,2/5] clk: renesas: rcar-gen3: Parameterise Z and Z2 clock offset

Message ID 20190131094021.3092-3-horms+renesas@verge.net.au (mailing list archive)
State Awaiting Upstream, archived
Headers show
Series clk: renesas: r8a77990: Add Z2 clock | expand

Commit Message

Simon Horman Jan. 31, 2019, 9:40 a.m. UTC
Parameterise the offset of control bits within the FRQCRC register
for Z and Z2 clocks.

This is in preparation for supporting the Z2 clock on the R-Car E3
(r8a77990) SoC which uses a different offset for control bits to
other, already, supported SoCs.

This mechanism should be extendable to other clocks, such as ZG,
f.e. by adding the number of control bits as a parameter to
cpg_z_clk_register().

As suggested by Geert Uytterhoeven.

Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
v3: New patch
---
 drivers/clk/renesas/r8a774a1-cpg-mssr.c |  4 ++--
 drivers/clk/renesas/r8a7795-cpg-mssr.c  |  4 ++--
 drivers/clk/renesas/r8a7796-cpg-mssr.c  |  4 ++--
 drivers/clk/renesas/r8a77965-cpg-mssr.c |  2 +-
 drivers/clk/renesas/rcar-gen3-cpg.c     | 15 ++++-----------
 drivers/clk/renesas/rcar-gen3-cpg.h     |  4 ++--
 6 files changed, 13 insertions(+), 20 deletions(-)

Comments

Geert Uytterhoeven Feb. 5, 2019, 9:28 a.m. UTC | #1
On Thu, Jan 31, 2019 at 10:40 AM Simon Horman
<horms+renesas@verge.net.au> wrote:
> Parameterise the offset of control bits within the FRQCRC register
> for Z and Z2 clocks.
>
> This is in preparation for supporting the Z2 clock on the R-Car E3
> (r8a77990) SoC which uses a different offset for control bits to
> other, already, supported SoCs.
>
> This mechanism should be extendable to other clocks, such as ZG,
> f.e. by adding the number of control bits as a parameter to
> cpg_z_clk_register().
>
> As suggested by Geert Uytterhoeven.
>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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 Feb. 5, 2019, 10:48 a.m. UTC | #2
Hi Simon,

On Thu, Jan 31, 2019 at 10:40 AM Simon Horman
<horms+renesas@verge.net.au> wrote:
> Parameterise the offset of control bits within the FRQCRC register
> for Z and Z2 clocks.
>
> This is in preparation for supporting the Z2 clock on the R-Car E3
> (r8a77990) SoC which uses a different offset for control bits to
> other, already, supported SoCs.
>
> This mechanism should be extendable to other clocks, such as ZG,
> f.e. by adding the number of control bits as a parameter to
> cpg_z_clk_register().
>
> As suggested by Geert Uytterhoeven.
>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>

> --- a/drivers/clk/renesas/rcar-gen3-cpg.c
> +++ b/drivers/clk/renesas/rcar-gen3-cpg.c

> @@ -568,14 +566,9 @@ struct clk * __init rcar_gen3_cpg_clk_register(struct device *dev,
>                 break;
>
>         case CLK_TYPE_GEN3_Z:
> -               return cpg_z_clk_register(core->name, __clk_get_name(parent),
> -                                         base, CPG_FRQCRC_ZFC_MASK,
> -                                         core->div);
> -
>         case CLK_TYPE_GEN3_Z2:
>                 return cpg_z_clk_register(core->name, __clk_get_name(parent),
> -                                         base, CPG_FRQCRC_Z2FC_MASK,
> -                                         core->div);
> +                                         base, core->div, core->offset);

CLK_TYPE_GEN3_Z and CLK_TYPE_GEN3_Z2 are now the same type.
Perhaps they can be merged completely, and be absorbed into the
DEF_GEN3_Z() macro?
Or not, depending on how ZG support will be added...

Gr{oetje,eeting}s,

                        Geert
Simon Horman Feb. 5, 2019, 2:35 p.m. UTC | #3
On Tue, Feb 05, 2019 at 11:48:06AM +0100, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Thu, Jan 31, 2019 at 10:40 AM Simon Horman
> <horms+renesas@verge.net.au> wrote:
> > Parameterise the offset of control bits within the FRQCRC register
> > for Z and Z2 clocks.
> >
> > This is in preparation for supporting the Z2 clock on the R-Car E3
> > (r8a77990) SoC which uses a different offset for control bits to
> > other, already, supported SoCs.
> >
> > This mechanism should be extendable to other clocks, such as ZG,
> > f.e. by adding the number of control bits as a parameter to
> > cpg_z_clk_register().
> >
> > As suggested by Geert Uytterhoeven.
> >
> > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> 
> > --- a/drivers/clk/renesas/rcar-gen3-cpg.c
> > +++ b/drivers/clk/renesas/rcar-gen3-cpg.c
> 
> > @@ -568,14 +566,9 @@ struct clk * __init rcar_gen3_cpg_clk_register(struct device *dev,
> >                 break;
> >
> >         case CLK_TYPE_GEN3_Z:
> > -               return cpg_z_clk_register(core->name, __clk_get_name(parent),
> > -                                         base, CPG_FRQCRC_ZFC_MASK,
> > -                                         core->div);
> > -
> >         case CLK_TYPE_GEN3_Z2:
> >                 return cpg_z_clk_register(core->name, __clk_get_name(parent),
> > -                                         base, CPG_FRQCRC_Z2FC_MASK,
> > -                                         core->div);
> > +                                         base, core->div, core->offset);
> 
> CLK_TYPE_GEN3_Z and CLK_TYPE_GEN3_Z2 are now the same type.
> Perhaps they can be merged completely, and be absorbed into the
> DEF_GEN3_Z() macro?
> Or not, depending on how ZG support will be added...

Strange, I did have them merged locally and I think that is the right thing
to do, but some how this version got posted. I think that if they
subsequently need to be re-split then so be it. But lets not jump
to conclusions.

I'll plan on posting v4 unless you object.
Geert Uytterhoeven Feb. 5, 2019, 2:55 p.m. UTC | #4
Hi Simon,

On Tue, Feb 5, 2019 at 3:36 PM Simon Horman <horms@verge.net.au> wrote:
> On Tue, Feb 05, 2019 at 11:48:06AM +0100, Geert Uytterhoeven wrote:
> > On Thu, Jan 31, 2019 at 10:40 AM Simon Horman
> > <horms+renesas@verge.net.au> wrote:
> > > Parameterise the offset of control bits within the FRQCRC register
> > > for Z and Z2 clocks.
> > >
> > > This is in preparation for supporting the Z2 clock on the R-Car E3
> > > (r8a77990) SoC which uses a different offset for control bits to
> > > other, already, supported SoCs.
> > >
> > > This mechanism should be extendable to other clocks, such as ZG,
> > > f.e. by adding the number of control bits as a parameter to
> > > cpg_z_clk_register().
> > >
> > > As suggested by Geert Uytterhoeven.
> > >
> > > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> >
> > > --- a/drivers/clk/renesas/rcar-gen3-cpg.c
> > > +++ b/drivers/clk/renesas/rcar-gen3-cpg.c
> >
> > > @@ -568,14 +566,9 @@ struct clk * __init rcar_gen3_cpg_clk_register(struct device *dev,
> > >                 break;
> > >
> > >         case CLK_TYPE_GEN3_Z:
> > > -               return cpg_z_clk_register(core->name, __clk_get_name(parent),
> > > -                                         base, CPG_FRQCRC_ZFC_MASK,
> > > -                                         core->div);
> > > -
> > >         case CLK_TYPE_GEN3_Z2:
> > >                 return cpg_z_clk_register(core->name, __clk_get_name(parent),
> > > -                                         base, CPG_FRQCRC_Z2FC_MASK,
> > > -                                         core->div);
> > > +                                         base, core->div, core->offset);
> >
> > CLK_TYPE_GEN3_Z and CLK_TYPE_GEN3_Z2 are now the same type.
> > Perhaps they can be merged completely, and be absorbed into the
> > DEF_GEN3_Z() macro?
> > Or not, depending on how ZG support will be added...
>
> Strange, I did have them merged locally and I think that is the right thing
> to do, but some how this version got posted. I think that if they
> subsequently need to be re-split then so be it. But lets not jump
> to conclusions.
>
> I'll plan on posting v4 unless you object.

OK, eagerly awaiting an even more improved version!

Gr{oetje,eeting}s,

                        Geert
Simon Horman Feb. 5, 2019, 3:14 p.m. UTC | #5
On Tue, Feb 05, 2019 at 03:55:08PM +0100, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Tue, Feb 5, 2019 at 3:36 PM Simon Horman <horms@verge.net.au> wrote:
> > On Tue, Feb 05, 2019 at 11:48:06AM +0100, Geert Uytterhoeven wrote:
> > > On Thu, Jan 31, 2019 at 10:40 AM Simon Horman
> > > <horms+renesas@verge.net.au> wrote:
> > > > Parameterise the offset of control bits within the FRQCRC register
> > > > for Z and Z2 clocks.
> > > >
> > > > This is in preparation for supporting the Z2 clock on the R-Car E3
> > > > (r8a77990) SoC which uses a different offset for control bits to
> > > > other, already, supported SoCs.
> > > >
> > > > This mechanism should be extendable to other clocks, such as ZG,
> > > > f.e. by adding the number of control bits as a parameter to
> > > > cpg_z_clk_register().
> > > >
> > > > As suggested by Geert Uytterhoeven.
> > > >
> > > > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> > >
> > > > --- a/drivers/clk/renesas/rcar-gen3-cpg.c
> > > > +++ b/drivers/clk/renesas/rcar-gen3-cpg.c
> > >
> > > > @@ -568,14 +566,9 @@ struct clk * __init rcar_gen3_cpg_clk_register(struct device *dev,
> > > >                 break;
> > > >
> > > >         case CLK_TYPE_GEN3_Z:
> > > > -               return cpg_z_clk_register(core->name, __clk_get_name(parent),
> > > > -                                         base, CPG_FRQCRC_ZFC_MASK,
> > > > -                                         core->div);
> > > > -
> > > >         case CLK_TYPE_GEN3_Z2:
> > > >                 return cpg_z_clk_register(core->name, __clk_get_name(parent),
> > > > -                                         base, CPG_FRQCRC_Z2FC_MASK,
> > > > -                                         core->div);
> > > > +                                         base, core->div, core->offset);
> > >
> > > CLK_TYPE_GEN3_Z and CLK_TYPE_GEN3_Z2 are now the same type.
> > > Perhaps they can be merged completely, and be absorbed into the
> > > DEF_GEN3_Z() macro?
> > > Or not, depending on how ZG support will be added...
> >
> > Strange, I did have them merged locally and I think that is the right thing
> > to do, but some how this version got posted. I think that if they
> > subsequently need to be re-split then so be it. But lets not jump
> > to conclusions.
> >
> > I'll plan on posting v4 unless you object.
> 
> OK, eagerly awaiting an even more improved version!

Sorry, I was confused (as is often the case).

I think that removing the duplicate code, as I have above,
is the right thing to do at this point as I do entirely
expect the CLK_TYPE_GEN3_Z and CLK_TYPE_GEN3_Z2 cases
to diverge again when we add ZG support.
diff mbox series

Patch

diff --git a/drivers/clk/renesas/r8a774a1-cpg-mssr.c b/drivers/clk/renesas/r8a774a1-cpg-mssr.c
index 103253bee055..8f6a182bea27 100644
--- a/drivers/clk/renesas/r8a774a1-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a774a1-cpg-mssr.c
@@ -71,8 +71,8 @@  static const struct cpg_core_clk r8a774a1_core_clks[] __initconst = {
 	DEF_GEN3_OSC(".r",      CLK_RINT,          CLK_EXTAL,      32),
 
 	/* Core Clock Outputs */
-	DEF_GEN3_Z("z",		R8A774A1_CLK_Z,     CLK_TYPE_GEN3_Z, CLK_PLL0, 2),
-	DEF_GEN3_Z("z2",	R8A774A1_CLK_Z2,    CLK_TYPE_GEN3_Z2, CLK_PLL2, 2),
+	DEF_GEN3_Z("z",		R8A774A1_CLK_Z,     CLK_TYPE_GEN3_Z,  CLK_PLL0, 2, 8),
+	DEF_GEN3_Z("z2",	R8A774A1_CLK_Z2,    CLK_TYPE_GEN3_Z2, CLK_PLL2, 2, 0),
 	DEF_FIXED("ztr",        R8A774A1_CLK_ZTR,   CLK_PLL1_DIV2,  6, 1),
 	DEF_FIXED("ztrd2",      R8A774A1_CLK_ZTRD2, CLK_PLL1_DIV2, 12, 1),
 	DEF_FIXED("zt",         R8A774A1_CLK_ZT,    CLK_PLL1_DIV2,  4, 1),
diff --git a/drivers/clk/renesas/r8a7795-cpg-mssr.c b/drivers/clk/renesas/r8a7795-cpg-mssr.c
index d4cf1c91533e..d09c0abb032d 100644
--- a/drivers/clk/renesas/r8a7795-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c
@@ -74,8 +74,8 @@  static struct cpg_core_clk r8a7795_core_clks[] __initdata = {
 	DEF_GEN3_OSC(".r",      CLK_RINT,          CLK_EXTAL,      32),
 
 	/* Core Clock Outputs */
-	DEF_GEN3_Z("z",         R8A7795_CLK_Z,	   CLK_TYPE_GEN3_Z,  CLK_PLL0, 2),
-	DEF_GEN3_Z("z2",        R8A7795_CLK_Z2,    CLK_TYPE_GEN3_Z2, CLK_PLL2, 2),
+	DEF_GEN3_Z("z",         R8A7795_CLK_Z,     CLK_TYPE_GEN3_Z,  CLK_PLL0, 2, 8),
+	DEF_GEN3_Z("z2",        R8A7795_CLK_Z2,    CLK_TYPE_GEN3_Z2, CLK_PLL2, 2, 0),
 	DEF_FIXED("ztr",        R8A7795_CLK_ZTR,   CLK_PLL1_DIV2,  6, 1),
 	DEF_FIXED("ztrd2",      R8A7795_CLK_ZTRD2, CLK_PLL1_DIV2, 12, 1),
 	DEF_FIXED("zt",         R8A7795_CLK_ZT,    CLK_PLL1_DIV2,  4, 1),
diff --git a/drivers/clk/renesas/r8a7796-cpg-mssr.c b/drivers/clk/renesas/r8a7796-cpg-mssr.c
index 77254f2b4519..7efd0311dcbd 100644
--- a/drivers/clk/renesas/r8a7796-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a7796-cpg-mssr.c
@@ -74,8 +74,8 @@  static const struct cpg_core_clk r8a7796_core_clks[] __initconst = {
 	DEF_GEN3_OSC(".r",      CLK_RINT,          CLK_EXTAL,      32),
 
 	/* Core Clock Outputs */
-	DEF_GEN3_Z("z",         R8A7796_CLK_Z,     CLK_TYPE_GEN3_Z,  CLK_PLL0, 2),
-	DEF_GEN3_Z("z2",        R8A7796_CLK_Z2,    CLK_TYPE_GEN3_Z2, CLK_PLL2, 2),
+	DEF_GEN3_Z("z",         R8A7796_CLK_Z,     CLK_TYPE_GEN3_Z,  CLK_PLL0, 2, 8),
+	DEF_GEN3_Z("z2",        R8A7796_CLK_Z2,    CLK_TYPE_GEN3_Z2, CLK_PLL2, 2, 0),
 	DEF_FIXED("ztr",        R8A7796_CLK_ZTR,   CLK_PLL1_DIV2,  6, 1),
 	DEF_FIXED("ztrd2",      R8A7796_CLK_ZTRD2, CLK_PLL1_DIV2, 12, 1),
 	DEF_FIXED("zt",         R8A7796_CLK_ZT,    CLK_PLL1_DIV2,  4, 1),
diff --git a/drivers/clk/renesas/r8a77965-cpg-mssr.c b/drivers/clk/renesas/r8a77965-cpg-mssr.c
index f8f73558c1ec..fefa26a1a797 100644
--- a/drivers/clk/renesas/r8a77965-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a77965-cpg-mssr.c
@@ -71,7 +71,7 @@  static const struct cpg_core_clk r8a77965_core_clks[] __initconst = {
 	DEF_GEN3_OSC(".r",	CLK_RINT,		CLK_EXTAL,	32),
 
 	/* Core Clock Outputs */
-	DEF_GEN3_Z("z",		R8A77965_CLK_Z,		CLK_TYPE_GEN3_Z,  CLK_PLL0, 2),
+	DEF_GEN3_Z("z",		R8A77965_CLK_Z,		CLK_TYPE_GEN3_Z,  CLK_PLL0, 2, 8),
 	DEF_FIXED("ztr",	R8A77965_CLK_ZTR,	CLK_PLL1_DIV2,	6, 1),
 	DEF_FIXED("ztrd2",	R8A77965_CLK_ZTRD2,	CLK_PLL1_DIV2,	12, 1),
 	DEF_FIXED("zt",		R8A77965_CLK_ZT,	CLK_PLL1_DIV2,	4, 1),
diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c b/drivers/clk/renesas/rcar-gen3-cpg.c
index 5923028064a5..6b146c2cf6a3 100644
--- a/drivers/clk/renesas/rcar-gen3-cpg.c
+++ b/drivers/clk/renesas/rcar-gen3-cpg.c
@@ -73,8 +73,6 @@  static void cpg_simple_notifier_register(struct raw_notifier_head *notifiers,
 #define CPG_FRQCRB			0x00000004
 #define CPG_FRQCRB_KICK			BIT(31)
 #define CPG_FRQCRC			0x000000e0
-#define CPG_FRQCRC_ZFC_MASK		GENMASK(12, 8)
-#define CPG_FRQCRC_Z2FC_MASK		GENMASK(4, 0)
 
 struct cpg_z_clk {
 	struct clk_hw hw;
@@ -169,8 +167,8 @@  static const struct clk_ops cpg_z_clk_ops = {
 static struct clk * __init cpg_z_clk_register(const char *name,
 					      const char *parent_name,
 					      void __iomem *reg,
-					      unsigned long mask,
-					      unsigned int div)
+					      unsigned int div,
+					      unsigned int offset)
 {
 	struct clk_init_data init;
 	struct cpg_z_clk *zclk;
@@ -189,7 +187,7 @@  static struct clk * __init cpg_z_clk_register(const char *name,
 	zclk->reg = reg + CPG_FRQCRC;
 	zclk->kick_reg = reg + CPG_FRQCRB;
 	zclk->hw.init = &init;
-	zclk->mask = mask;
+	zclk->mask = GENMASK(offset + 4, offset);
 	zclk->fixed_div = div; /* PLLVCO x 1/div x SYS-CPU divider */
 
 	clk = clk_register(NULL, &zclk->hw);
@@ -568,14 +566,9 @@  struct clk * __init rcar_gen3_cpg_clk_register(struct device *dev,
 		break;
 
 	case CLK_TYPE_GEN3_Z:
-		return cpg_z_clk_register(core->name, __clk_get_name(parent),
-					  base, CPG_FRQCRC_ZFC_MASK,
-					  core->div);
-
 	case CLK_TYPE_GEN3_Z2:
 		return cpg_z_clk_register(core->name, __clk_get_name(parent),
-					  base, CPG_FRQCRC_Z2FC_MASK,
-					  core->div);
+					  base, core->div, core->offset);
 
 	case CLK_TYPE_GEN3_OSC:
 		/*
diff --git a/drivers/clk/renesas/rcar-gen3-cpg.h b/drivers/clk/renesas/rcar-gen3-cpg.h
index 60038e245e8b..13f1f7e6fb34 100644
--- a/drivers/clk/renesas/rcar-gen3-cpg.h
+++ b/drivers/clk/renesas/rcar-gen3-cpg.h
@@ -49,8 +49,8 @@  enum rcar_gen3_clk_types {
 	DEF_BASE(_name, _id, CLK_TYPE_GEN3_RCKSEL,	\
 		 (_parent0) << 16 | (_parent1),	.div = (_div0) << 16 | (_div1))
 
-#define DEF_GEN3_Z(_name, _id, _type, _parent, _div)	\
-	DEF_BASE(_name, _id, _type, _parent, .div = _div)
+#define DEF_GEN3_Z(_name, _id, _type, _parent, _div, _offset)	\
+	DEF_BASE(_name, _id, _type, _parent, .div = _div, .offset = _offset)
 
 struct rcar_gen3_cpg_pll_config {
 	u8 extal_div;