diff mbox series

[1/4] clk: renesas: rcar-gen3: Parameterise Z and Z2 clock register offset

Message ID 20190228135246.31714-2-horms+renesas@verge.net.au (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show
Series clk: renesas: rcar-gen3: Add ZG support for E3, D3 and RZ/G2E | expand

Commit Message

Simon Horman Feb. 28, 2019, 1:52 p.m. UTC
Parameterise the offset of the control register for
for Z and Z2 clocks.

This is in preparation for supporting the ZG clock on the R-Car E3
(r8a77990), D3 (r8a7795) and RZ/G2E (r8a774c0) SoCs which uses a different
control register to existing support for Z and Z2 clocks.

Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
Tested for regressions on Ebisu by using CPUFreq to alter Z and Z2 clock rates
---
 drivers/clk/renesas/r8a774a1-cpg-mssr.c |  6 ++++--
 drivers/clk/renesas/r8a774c0-cpg-mssr.c |  3 ++-
 drivers/clk/renesas/r8a7795-cpg-mssr.c  |  6 ++++--
 drivers/clk/renesas/r8a7796-cpg-mssr.c  |  6 ++++--
 drivers/clk/renesas/r8a77965-cpg-mssr.c |  3 ++-
 drivers/clk/renesas/r8a77990-cpg-mssr.c |  3 ++-
 drivers/clk/renesas/rcar-gen3-cpg.c     | 10 +++++-----
 drivers/clk/renesas/rcar-gen3-cpg.h     | 13 +++++++++++--
 8 files changed, 34 insertions(+), 16 deletions(-)

Comments

Geert Uytterhoeven March 1, 2019, 12:38 p.m. UTC | #1
Hi Simon,

Thanks for your patch!

On Thu, Feb 28, 2019 at 2:53 PM Simon Horman <horms+renesas@verge.net.au> wrote:
> Parameterise the offset of the control register for
> for Z and Z2 clocks.

double "for"

> This is in preparation for supporting the ZG clock on the R-Car E3
> (r8a77990), D3 (r8a7795) and RZ/G2E (r8a774c0) SoCs which uses a different

D3 (r8a77995) ... use

> control register to existing support for Z and Z2 clocks.
>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>

> --- a/drivers/clk/renesas/r8a774a1-cpg-mssr.c
> +++ b/drivers/clk/renesas/r8a774a1-cpg-mssr.c
> @@ -71,8 +71,10 @@ 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, 8),
> -       DEF_GEN3_Z("z2",        R8A774A1_CLK_Z2,    CLK_TYPE_GEN3_Z,  CLK_PLL2, 2, 0),

Given "[PATCH v5 0/6] clk: renesas: r8a77990, r8a774c0: Add Z2 clock"
still used CLK_TYPE_GEN3_Z2, this patch does not apply.
Have you forgotten to send out v6?

> +       DEF_GEN3_Z("z",         R8A774A1_CLK_Z,     CLK_TYPE_GEN3_Z,
> +                  CLK_PLL0, 2, CPG_FRQCRC, 8),
> +       DEF_GEN3_Z("z2",        R8A774A1_CLK_Z2,    CLK_TYPE_GEN3_Z,
> +                  CLK_PLL2, 2, CPG_FRQCRC, 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),

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

> @@ -166,7 +165,7 @@ 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,
> +                                             void __iomem *base,
>                                               unsigned int div,
>                                               unsigned int offset)
>  {
> @@ -184,10 +183,11 @@ static struct clk * __init cpg_z_clk_register(const char *name,
>         init.parent_names = &parent_name;
>         init.num_parents = 1;
>
> -       zclk->reg = reg + CPG_FRQCRC;
> -       zclk->kick_reg = reg + CPG_FRQCRB;
> +       zclk->reg = base + GEN3_Z_REG_OFFSET(offset);
> +       zclk->kick_reg = base + CPG_FRQCRB;
>         zclk->hw.init = &init;
> -       zclk->mask = GENMASK(offset + 4, offset);
> +       zclk->mask = GENMASK(GEN3_Z_BIT_OFFSET(offset) + 4,
> +                            GEN3_Z_BIT_OFFSET(offset));

I think the code would be easier to read if you would move the splitting
of offset to the caller, and thus pass separate reg_offset and bit_offset
parameters.

>         zclk->fixed_div = div; /* PLLVCO x 1/div x SYS-CPU divider */
>
>         clk = clk_register(NULL, &zclk->hw);
> diff --git a/drivers/clk/renesas/rcar-gen3-cpg.h b/drivers/clk/renesas/rcar-gen3-cpg.h
> index a0535341b5da..02bf3785263c 100644
> --- a/drivers/clk/renesas/rcar-gen3-cpg.h
> +++ b/drivers/clk/renesas/rcar-gen3-cpg.h
> @@ -48,8 +48,16 @@ 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, _offset)  \
> -       DEF_BASE(_name, _id, _type, _parent, .div = _div, .offset = _offset)
> +#define DEF_GEN3_Z_OFFSET(_reg_offset, _bit_offset) \
> +       ((_reg_offset) << 16 | (_bit_offset) )
> +
> +#define GEN3_Z_REG_OFFSET(_offset) ((_offset) >> 16)
> +
> +#define GEN3_Z_BIT_OFFSET(_offset) ((_offset) & 0xffff)

Do you think these 3 (un)marshalling macros add much value, given they're
used in a single place only?

> +
> +#define DEF_GEN3_Z(_name, _id, _type, _parent, _div, _reg_offset, _bit_offset)\
> +       DEF_BASE(_name, _id, _type, _parent, .div = _div, \
> +                .offset = DEF_GEN3_Z_OFFSET(_reg_offset, _bit_offset))
>
>  struct rcar_gen3_cpg_pll_config {
>         u8 extal_div;

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
Simon Horman March 4, 2019, 9:44 a.m. UTC | #2
On Fri, Mar 01, 2019 at 01:38:43PM +0100, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> Thanks for your patch!
> 
> On Thu, Feb 28, 2019 at 2:53 PM Simon Horman <horms+renesas@verge.net.au> wrote:
> > Parameterise the offset of the control register for
> > for Z and Z2 clocks.
> 
> double "for"
> 
> > This is in preparation for supporting the ZG clock on the R-Car E3
> > (r8a77990), D3 (r8a7795) and RZ/G2E (r8a774c0) SoCs which uses a different
> 
> D3 (r8a77995) ... use
> 
> > control register to existing support for Z and Z2 clocks.
> >
> > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> 
> > --- a/drivers/clk/renesas/r8a774a1-cpg-mssr.c
> > +++ b/drivers/clk/renesas/r8a774a1-cpg-mssr.c
> > @@ -71,8 +71,10 @@ 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, 8),
> > -       DEF_GEN3_Z("z2",        R8A774A1_CLK_Z2,    CLK_TYPE_GEN3_Z,  CLK_PLL2, 2, 0),
> 
> Given "[PATCH v5 0/6] clk: renesas: r8a77990, r8a774c0: Add Z2 clock"
> still used CLK_TYPE_GEN3_Z2, this patch does not apply.
> Have you forgotten to send out v6?

There was supposed to be a consolidation patch at the beginning
of this series. Sorry for omitting it.

> 
> > +       DEF_GEN3_Z("z",         R8A774A1_CLK_Z,     CLK_TYPE_GEN3_Z,
> > +                  CLK_PLL0, 2, CPG_FRQCRC, 8),
> > +       DEF_GEN3_Z("z2",        R8A774A1_CLK_Z2,    CLK_TYPE_GEN3_Z,
> > +                  CLK_PLL2, 2, CPG_FRQCRC, 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),
> 
> > --- a/drivers/clk/renesas/rcar-gen3-cpg.c
> > +++ b/drivers/clk/renesas/rcar-gen3-cpg.c
> 
> > @@ -166,7 +165,7 @@ 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,
> > +                                             void __iomem *base,
> >                                               unsigned int div,
> >                                               unsigned int offset)
> >  {
> > @@ -184,10 +183,11 @@ static struct clk * __init cpg_z_clk_register(const char *name,
> >         init.parent_names = &parent_name;
> >         init.num_parents = 1;
> >
> > -       zclk->reg = reg + CPG_FRQCRC;
> > -       zclk->kick_reg = reg + CPG_FRQCRB;
> > +       zclk->reg = base + GEN3_Z_REG_OFFSET(offset);
> > +       zclk->kick_reg = base + CPG_FRQCRB;
> >         zclk->hw.init = &init;
> > -       zclk->mask = GENMASK(offset + 4, offset);
> > +       zclk->mask = GENMASK(GEN3_Z_BIT_OFFSET(offset) + 4,
> > +                            GEN3_Z_BIT_OFFSET(offset));
> 
> I think the code would be easier to read if you would move the splitting
> of offset to the caller, and thus pass separate reg_offset and bit_offset
> parameters.

Sure, that sounds reasonable.

> 
> >         zclk->fixed_div = div; /* PLLVCO x 1/div x SYS-CPU divider */
> >
> >         clk = clk_register(NULL, &zclk->hw);
> > diff --git a/drivers/clk/renesas/rcar-gen3-cpg.h b/drivers/clk/renesas/rcar-gen3-cpg.h
> > index a0535341b5da..02bf3785263c 100644
> > --- a/drivers/clk/renesas/rcar-gen3-cpg.h
> > +++ b/drivers/clk/renesas/rcar-gen3-cpg.h
> > @@ -48,8 +48,16 @@ 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, _offset)  \
> > -       DEF_BASE(_name, _id, _type, _parent, .div = _div, .offset = _offset)
> > +#define DEF_GEN3_Z_OFFSET(_reg_offset, _bit_offset) \
> > +       ((_reg_offset) << 16 | (_bit_offset) )
> > +
> > +#define GEN3_Z_REG_OFFSET(_offset) ((_offset) >> 16)
> > +
> > +#define GEN3_Z_BIT_OFFSET(_offset) ((_offset) & 0xffff)
> 
> Do you think these 3 (un)marshalling macros add much value, given they're
> used in a single place only?

Yes, they make things very clear to my mind.
But I'm open to an alternative if you have one in mind.

> 
> > +
> > +#define DEF_GEN3_Z(_name, _id, _type, _parent, _div, _reg_offset, _bit_offset)\
> > +       DEF_BASE(_name, _id, _type, _parent, .div = _div, \
> > +                .offset = DEF_GEN3_Z_OFFSET(_reg_offset, _bit_offset))
> >
> >  struct rcar_gen3_cpg_pll_config {
> >         u8 extal_div;
> 
> 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
>
diff mbox series

Patch

diff --git a/drivers/clk/renesas/r8a774a1-cpg-mssr.c b/drivers/clk/renesas/r8a774a1-cpg-mssr.c
index d93eb4da152b..9ac61fae993a 100644
--- a/drivers/clk/renesas/r8a774a1-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a774a1-cpg-mssr.c
@@ -71,8 +71,10 @@  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, 8),
-	DEF_GEN3_Z("z2",	R8A774A1_CLK_Z2,    CLK_TYPE_GEN3_Z,  CLK_PLL2, 2, 0),
+	DEF_GEN3_Z("z",		R8A774A1_CLK_Z,     CLK_TYPE_GEN3_Z,
+		   CLK_PLL0, 2, CPG_FRQCRC, 8),
+	DEF_GEN3_Z("z2",	R8A774A1_CLK_Z2,    CLK_TYPE_GEN3_Z,
+		   CLK_PLL2, 2, CPG_FRQCRC, 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/r8a774c0-cpg-mssr.c b/drivers/clk/renesas/r8a774c0-cpg-mssr.c
index 5ba575242eee..d9130723d6c8 100644
--- a/drivers/clk/renesas/r8a774c0-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a774c0-cpg-mssr.c
@@ -79,7 +79,8 @@  static const struct cpg_core_clk r8a774c0_core_clks[] __initconst = {
 	/* Core Clock Outputs */
 	DEF_FIXED("za2",       R8A774C0_CLK_ZA2,   CLK_PLL0D24,    1, 1),
 	DEF_FIXED("za8",       R8A774C0_CLK_ZA8,   CLK_PLL0D8,     1, 1),
-	DEF_GEN3_Z("z2",       R8A774C0_CLK_Z2,    CLK_TYPE_GEN3_Z, CLK_PLL0, 4, 8),
+	DEF_GEN3_Z("z2",       R8A774C0_CLK_Z2,    CLK_TYPE_GEN3_Z,
+		   CLK_PLL0, 4, CPG_FRQCRC, 8),
 	DEF_FIXED("ztr",       R8A774C0_CLK_ZTR,   CLK_PLL1,       6, 1),
 	DEF_FIXED("zt",        R8A774C0_CLK_ZT,    CLK_PLL1,       4, 1),
 	DEF_FIXED("zx",        R8A774C0_CLK_ZX,    CLK_PLL1,       3, 1),
diff --git a/drivers/clk/renesas/r8a7795-cpg-mssr.c b/drivers/clk/renesas/r8a7795-cpg-mssr.c
index 8287816523c3..935085161b47 100644
--- a/drivers/clk/renesas/r8a7795-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c
@@ -74,8 +74,10 @@  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, 8),
-	DEF_GEN3_Z("z2",        R8A7795_CLK_Z2,    CLK_TYPE_GEN3_Z,  CLK_PLL2, 2, 0),
+	DEF_GEN3_Z("z",         R8A7795_CLK_Z,     CLK_TYPE_GEN3_Z,  CLK_PLL0,
+		   2, CPG_FRQCRC, 8),
+	DEF_GEN3_Z("z2",        R8A7795_CLK_Z2,    CLK_TYPE_GEN3_Z,  CLK_PLL2,
+		   2, CPG_FRQCRC, 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 5cde1bff8923..ca7fcec5bce2 100644
--- a/drivers/clk/renesas/r8a7796-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a7796-cpg-mssr.c
@@ -74,8 +74,10 @@  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, 8),
-	DEF_GEN3_Z("z2",        R8A7796_CLK_Z2,    CLK_TYPE_GEN3_Z,  CLK_PLL2, 2, 0),
+	DEF_GEN3_Z("z",         R8A7796_CLK_Z,     CLK_TYPE_GEN3_Z,  CLK_PLL0,
+		   2, CPG_FRQCRC, 8),
+	DEF_GEN3_Z("z2",        R8A7796_CLK_Z2,    CLK_TYPE_GEN3_Z,  CLK_PLL2,
+		   2, CPG_FRQCRC, 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 fefa26a1a797..fb5de677f828 100644
--- a/drivers/clk/renesas/r8a77965-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a77965-cpg-mssr.c
@@ -71,7 +71,8 @@  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, 8),
+	DEF_GEN3_Z("z",		R8A77965_CLK_Z,		CLK_TYPE_GEN3_Z,
+		   CLK_PLL0, 2, CPG_FRQCRC, 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/r8a77990-cpg-mssr.c b/drivers/clk/renesas/r8a77990-cpg-mssr.c
index 99f602cb30a5..0e475dcb68b9 100644
--- a/drivers/clk/renesas/r8a77990-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a77990-cpg-mssr.c
@@ -81,7 +81,8 @@  static const struct cpg_core_clk r8a77990_core_clks[] __initconst = {
 	/* Core Clock Outputs */
 	DEF_FIXED("za2",       R8A77990_CLK_ZA2,   CLK_PLL0D24,    1, 1),
 	DEF_FIXED("za8",       R8A77990_CLK_ZA8,   CLK_PLL0D8,     1, 1),
-	DEF_GEN3_Z("z2",       R8A77990_CLK_Z2,    CLK_TYPE_GEN3_Z, CLK_PLL0, 4, 8),
+	DEF_GEN3_Z("z2",       R8A77990_CLK_Z2,    CLK_TYPE_GEN3_Z, CLK_PLL0,
+		   4, CPG_FRQCRC, 8),
 	DEF_FIXED("ztr",       R8A77990_CLK_ZTR,   CLK_PLL1,       6, 1),
 	DEF_FIXED("zt",        R8A77990_CLK_ZT,    CLK_PLL1,       4, 1),
 	DEF_FIXED("zx",        R8A77990_CLK_ZX,    CLK_PLL1,       3, 1),
diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c b/drivers/clk/renesas/rcar-gen3-cpg.c
index a40ad6d03ece..14a82c51682e 100644
--- a/drivers/clk/renesas/rcar-gen3-cpg.c
+++ b/drivers/clk/renesas/rcar-gen3-cpg.c
@@ -72,7 +72,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
 
 struct cpg_z_clk {
 	struct clk_hw hw;
@@ -166,7 +165,7 @@  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,
+					      void __iomem *base,
 					      unsigned int div,
 					      unsigned int offset)
 {
@@ -184,10 +183,11 @@  static struct clk * __init cpg_z_clk_register(const char *name,
 	init.parent_names = &parent_name;
 	init.num_parents = 1;
 
-	zclk->reg = reg + CPG_FRQCRC;
-	zclk->kick_reg = reg + CPG_FRQCRB;
+	zclk->reg = base + GEN3_Z_REG_OFFSET(offset);
+	zclk->kick_reg = base + CPG_FRQCRB;
 	zclk->hw.init = &init;
-	zclk->mask = GENMASK(offset + 4, offset);
+	zclk->mask = GENMASK(GEN3_Z_BIT_OFFSET(offset) + 4,
+			     GEN3_Z_BIT_OFFSET(offset));
 	zclk->fixed_div = div; /* PLLVCO x 1/div x SYS-CPU divider */
 
 	clk = clk_register(NULL, &zclk->hw);
diff --git a/drivers/clk/renesas/rcar-gen3-cpg.h b/drivers/clk/renesas/rcar-gen3-cpg.h
index a0535341b5da..02bf3785263c 100644
--- a/drivers/clk/renesas/rcar-gen3-cpg.h
+++ b/drivers/clk/renesas/rcar-gen3-cpg.h
@@ -48,8 +48,16 @@  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, _offset)	\
-	DEF_BASE(_name, _id, _type, _parent, .div = _div, .offset = _offset)
+#define DEF_GEN3_Z_OFFSET(_reg_offset, _bit_offset) \
+	((_reg_offset) << 16 | (_bit_offset) )
+
+#define GEN3_Z_REG_OFFSET(_offset) ((_offset) >> 16)
+
+#define GEN3_Z_BIT_OFFSET(_offset) ((_offset) & 0xffff)
+
+#define DEF_GEN3_Z(_name, _id, _type, _parent, _div, _reg_offset, _bit_offset)\
+	DEF_BASE(_name, _id, _type, _parent, .div = _div, \
+		 .offset = DEF_GEN3_Z_OFFSET(_reg_offset, _bit_offset))
 
 struct rcar_gen3_cpg_pll_config {
 	u8 extal_div;
@@ -60,6 +68,7 @@  struct rcar_gen3_cpg_pll_config {
 	u8 osc_prediv;
 };
 
+#define CPG_FRQCRC	0x0e0
 #define CPG_RCKCR	0x240
 
 struct clk *rcar_gen3_cpg_clk_register(struct device *dev,