[PATCH v3 0/6] clk: renesas: r8a779[56]: Add Z and Z2 clock support

[PATCH v3 0/6] clk: renesas: r8a779[56]: Add Z and Z2 clock support

[Simon Horman]

Hi,

this patch-set adds Z and Z2 clock support.

These are dependencies for supporting CPUFreq. The remainder of that
work is being posted separately and can be found at:

https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git topic/rcar-gen3-cpufreq


A description of steps taken to lightly exercise the same feature for the
r88a7795 the above can be found at the link below. The results are the same
for the r8a7796 with the exception that it has two active CPU cores rather
than four.

http://elinux.org/Tests:R-CAR-GEN3-CPUFreq

Further testing on Salvator-X/M3-W ES1.0 is as follows:

# cd /sys/devices/system/cpu/cpu0/cpufreq
# grep . /sys/devices/system/cpu/cpu*/cpufreq/*_cur_freq /sys/kernel/debug/clk/z/clk_rate
/sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_cur_freq:468749
/sys/devices/system/cpu/cpu0/cpufreq/scaling_cur_freq:500000
/sys/devices/system/cpu/cpu1/cpufreq/cpuinfo_cur_freq:468749
/sys/devices/system/cpu/cpu1/cpufreq/scaling_cur_freq:500000
/sys/kernel/debug/clk/z/clk_rate:468749981
# echo performance > scaling_governor
# grep .  /sys/devices/system/cpu/cpu*/cpufreq/*_cur_freq /sys/kernel/debug/clk/z/clk_rate
/sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_cur_freq:1499999
/sys/devices/system/cpu/cpu0/cpufreq/scaling_cur_freq:1500000
/sys/devices/system/cpu/cpu1/cpufreq/cpuinfo_cur_freq:1499999
/sys/devices/system/cpu/cpu1/cpufreq/scaling_cur_freq:1500000
/sys/kernel/debug/clk/z/clk_rate:1499999940
# echo 1000000 > scaling_max_freq
# grep . /sys/devices/system/cpu/cpu*/cpufreq/*_cur_freq /sys/kernel/debug/clk/z/clk_rate
/sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_cur_freq:937499
/sys/devices/system/cpu/cpu0/cpufreq/scaling_cur_freq:1000000
/sys/devices/system/cpu/cpu1/cpufreq/cpuinfo_cur_freq:937499
/sys/devices/system/cpu/cpu1/cpufreq/scaling_cur_freq:1000000
/sys/kernel/debug/clk/z/clk_rate:937499963
# echo 500000 > scaling_max_freq 
# grep .  /sys/devices/system/cpu/cpu*/cpufreq/*_cur_freq /sys/kernel/debug/clk/z/clk_rate 
/sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_cur_freq:468749
/sys/devices/system/cpu/cpu0/cpufreq/scaling_cur_freq:500000
/sys/devices/system/cpu/cpu1/cpufreq/cpuinfo_cur_freq:468749
/sys/devices/system/cpu/cpu1/cpufreq/scaling_cur_freq:500000
/sys/kernel/debug/clk/z2/clk_rate:2399999904


Based on clk-renesas-for-v4.15-tag1

Changes since v2:
* Rebase
* Address review of v2 as noted in patch changelogs

Takeshi Kihara (6):
  clk: renesas: rcar-gen3: Add Z clock divider support
  clk: renesas: rcar-gen3: Add Z2 clock divider support
  clk: renesas: r8a7795: Add Z clock
  clk: renesas: r8a7795: Add Z2 clock
  clk: renesas: r8a7796: Add Z clock
  clk: renesas: r8a7796: Add Z2 clock

 drivers/clk/renesas/r8a7795-cpg-mssr.c |   2 +
 drivers/clk/renesas/r8a7796-cpg-mssr.c |   2 +
 drivers/clk/renesas/rcar-gen3-cpg.c    | 136 +++++++++++++++++++++++++++++++++
 drivers/clk/renesas/rcar-gen3-cpg.h    |   2 +
 4 files changed, 142 insertions(+)

-- 
2.1.4

[PATCH v3 1/6] clk: renesas: rcar-gen3: Add Z clock divider support

[Simon Horman]

From: Takeshi Kihara <takeshi.kihara.df@renesas.com>

This patch adds Z clock divider support for R-Car Gen3 SoC.

Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---

v2 [Simon Horman]
* Use DIV_ROUND_CLOSEST_ULL instead of open-coding the same behaviour
  using div_u64()
* Do not round rate to 100MHz in cpg_z_clk_recalc_rate
* Remove calculation for PLL post-divider, this is bogus.
  Instead do not round to closest in cpg_z_clk_round_rate()
* Drop check for !prate in cpg_z_clk_round_rate

v1 [Simon Horman]
* Divide parent clock by 2 to give correct rate
* Use GENMASK, FIELD_{GET,PREP}
* Correct whitespace
* Arrange local variables in reverse Christmas tree order

v0 [Takeshi Kihara]
---
 drivers/clk/renesas/rcar-gen3-cpg.c | 128 ++++++++++++++++++++++++++++++++++++
 drivers/clk/renesas/rcar-gen3-cpg.h |   1 +
 2 files changed, 129 insertions(+)

diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c b/drivers/clk/renesas/rcar-gen3-cpg.c
index 951105816547..d98272a6075f 100644
--- a/drivers/clk/renesas/rcar-gen3-cpg.c
+++ b/drivers/clk/renesas/rcar-gen3-cpg.c
@@ -13,6 +13,7 @@
  */
 
 #include <linux/bug.h>
+#include <linux/bitfield.h>
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/device.h>
@@ -29,6 +30,129 @@
 #define CPG_PLL2CR		0x002c
 #define CPG_PLL4CR		0x01f4
 
+/*
+ * Z Clock
+ *
+ * Traits of this clock:
+ * prepare - clk_prepare only ensures that parents are prepared
+ * enable - clk_enable only ensures that parents are enabled
+ * rate - rate is adjustable.  clk->rate = (parent->rate * mult / 32 ) / 2
+ * parent - fixed parent.  No clk_set_parent support
+ */
+#define CPG_FRQCRB			0x00000004
+#define CPG_FRQCRB_KICK			BIT(31)
+#define CPG_FRQCRC			0x000000e0
+#define CPG_FRQCRC_ZFC_MASK		GENMASK(12, 8)
+
+struct cpg_z_clk {
+	struct clk_hw hw;
+	void __iomem *reg;
+	void __iomem *kick_reg;
+};
+
+#define to_z_clk(_hw)	container_of(_hw, struct cpg_z_clk, hw)
+
+static unsigned long cpg_z_clk_recalc_rate(struct clk_hw *hw,
+					   unsigned long parent_rate)
+{
+	struct cpg_z_clk *zclk = to_z_clk(hw);
+	unsigned int mult;
+
+	mult = 32 - FIELD_GET(CPG_FRQCRC_ZFC_MASK, clk_readl(zclk->reg));
+	return DIV_ROUND_CLOSEST_ULL(parent_rate * mult, 32);
+}
+
+static long cpg_z_clk_round_rate(struct clk_hw *hw, unsigned long rate,
+				 unsigned long *parent_rate)
+{
+	unsigned long prate = *parent_rate;
+	unsigned int mult;
+
+	mult = div_u64((u64)rate * 32, prate);
+	mult = clamp(mult, 1U, 32U);
+
+	return *parent_rate * mult / 32;
+}
+
+static int cpg_z_clk_set_rate(struct clk_hw *hw, unsigned long rate,
+			      unsigned long parent_rate)
+{
+	struct cpg_z_clk *zclk = to_z_clk(hw);
+	unsigned int mult;
+	unsigned int i;
+	u32 val, kick;
+
+	mult = DIV_ROUND_CLOSEST_ULL(rate * 32ULL, parent_rate);
+	mult = clamp(mult, 1U, 32U);
+
+	if (clk_readl(zclk->kick_reg) & CPG_FRQCRB_KICK)
+		return -EBUSY;
+
+	val = clk_readl(zclk->reg) & ~CPG_FRQCRC_ZFC_MASK;
+	val |= FIELD_PREP(CPG_FRQCRC_ZFC_MASK, 32 - mult);
+	clk_writel(val, zclk->reg);
+
+	/*
+	 * Set KICK bit in FRQCRB to update hardware setting and wait for
+	 * clock change completion.
+	 */
+	kick = clk_readl(zclk->kick_reg);
+	kick |= CPG_FRQCRB_KICK;
+	clk_writel(kick, zclk->kick_reg);
+
+	/*
+	 * Note: There is no HW information about the worst case latency.
+	 *
+	 * Using experimental measurements, it seems that no more than
+	 * ~10 iterations are needed, independently of the CPU rate.
+	 * Since this value might be dependent of external xtal rate, pll1
+	 * rate or even the other emulation clocks rate, use 1000 as a
+	 * "super" safe value.
+	 */
+	for (i = 1000; i; i--) {
+		if (!(clk_readl(zclk->kick_reg) & CPG_FRQCRB_KICK))
+			return 0;
+
+		cpu_relax();
+	}
+
+	return -ETIMEDOUT;
+}
+
+static const struct clk_ops cpg_z_clk_ops = {
+	.recalc_rate = cpg_z_clk_recalc_rate,
+	.round_rate = cpg_z_clk_round_rate,
+	.set_rate = cpg_z_clk_set_rate,
+};
+
+static struct clk * __init cpg_z_clk_register(const char *name,
+					      const char *parent_name,
+					      void __iomem *reg)
+{
+	struct clk_init_data init;
+	struct cpg_z_clk *zclk;
+	struct clk *clk;
+
+	zclk = kzalloc(sizeof(*zclk), GFP_KERNEL);
+	if (!zclk)
+		return ERR_PTR(-ENOMEM);
+
+	init.name = name;
+	init.ops = &cpg_z_clk_ops;
+	init.flags = 0;
+	init.parent_names = &parent_name;
+	init.num_parents = 1;
+
+	zclk->reg = reg + CPG_FRQCRC;
+	zclk->kick_reg = reg + CPG_FRQCRB;
+	zclk->hw.init = &init;
+
+	clk = clk_register(NULL, &zclk->hw);
+	if (IS_ERR(clk))
+		kfree(zclk);
+
+	return clk;
+}
 
 /*
  * SDn Clock
@@ -373,6 +497,10 @@ struct clk * __init rcar_gen3_cpg_clk_register(struct device *dev,
 		mult = 1;
 		break;
 
+	case CLK_TYPE_GEN3_Z:
+		return cpg_z_clk_register(core->name, __clk_get_name(parent),
+					  base);
+
 	default:
 		return ERR_PTR(-EINVAL);
 	}
diff --git a/drivers/clk/renesas/rcar-gen3-cpg.h b/drivers/clk/renesas/rcar-gen3-cpg.h
index d756ef8b78eb..89da28f6f71b 100644
--- a/drivers/clk/renesas/rcar-gen3-cpg.h
+++ b/drivers/clk/renesas/rcar-gen3-cpg.h
@@ -21,6 +21,7 @@ enum rcar_gen3_clk_types {
 	CLK_TYPE_GEN3_SD,
 	CLK_TYPE_GEN3_R,
 	CLK_TYPE_GEN3_PE,
+	CLK_TYPE_GEN3_Z,
 };
 
 #define DEF_GEN3_SD(_name, _id, _parent, _offset)	\
-- 
2.1.4

[PATCH v3 1/6] clk: renesas: rcar-gen3: Add Z clock divider support

[Geert Uytterhoeven]

Hi Simon,

On Thu, Oct 5, 2017 at 3:23 PM, Simon Horman <horms+renesas@verge.net.au> wrote:
> From: Takeshi Kihara <takeshi.kihara.df@renesas.com>
>
> This patch adds Z clock divider support for R-Car Gen3 SoC.
>
> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> ---
>
> v2 [Simon Horman]
> * Use DIV_ROUND_CLOSEST_ULL instead of open-coding the same behaviour
>   using div_u64()
> * Do not round rate to 100MHz in cpg_z_clk_recalc_rate
> * Remove calculation for PLL post-divider, this is bogus.
>   Instead do not round to closest in cpg_z_clk_round_rate()
> * Drop check for !prate in cpg_z_clk_round_rate

Thanks for the updates!

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

> +static unsigned long cpg_z_clk_recalc_rate(struct clk_hw *hw,
> +                                          unsigned long parent_rate)
> +{
> +       struct cpg_z_clk *zclk = to_z_clk(hw);
> +       unsigned int mult;
> +
> +       mult = 32 - FIELD_GET(CPG_FRQCRC_ZFC_MASK, clk_readl(zclk->reg));
> +       return DIV_ROUND_CLOSEST_ULL(parent_rate * mult, 32);

While parent_rate is unsigned long and thus 64-bit on arm64, this will work
fine on R-Car Gen3.  However, if someone ever tries to reuse this on a
32-bit platform, the multiplication may overflow.
Hence I recommend to make this a 64-bit multiplication explicitly, i.e.

    return DIV_ROUND_CLOSEST_ULL((u64)parent_rate * mult, 32);

> +}
> +
> +static long cpg_z_clk_round_rate(struct clk_hw *hw, unsigned long rate,
> +                                unsigned long *parent_rate)
> +{
> +       unsigned long prate = *parent_rate;
> +       unsigned int mult;
> +
> +       mult = div_u64((u64)rate * 32, prate);

You can avoid the cast by making the constant 64-bit:

    mult = div_u64(rate * 32ULL, prate);

> +       mult = clamp(mult, 1U, 32U);
> +
> +       return *parent_rate * mult / 32;

Again, *parent_rate is 64-bit on arm64, but not on 32-bit platforms.

> +       /*
> +        * Note: There is no HW information about the worst case latency.
> +        *
> +        * Using experimental measurements, it seems that no more than
> +        * ~10 iterations are needed, independently of the CPU rate.
> +        * Since this value might be dependent of external xtal rate, pll1
> +        * rate or even the other emulation clocks rate, use 1000 as a

another emulated clock rate?
(Yeah, copied from Gen2 ;-)

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

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at 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

[PATCH v3 1/6] clk: renesas: rcar-gen3: Add Z clock divider support

[Simon Horman]

On Mon, Oct 09, 2017 at 10:02:34AM +0200, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Thu, Oct 5, 2017 at 3:23 PM, Simon Horman <horms+renesas@verge.net.au> wrote:
> > From: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> >
> > This patch adds Z clock divider support for R-Car Gen3 SoC.
> >
> > Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> > ---
> >
> > v2 [Simon Horman]
> > * Use DIV_ROUND_CLOSEST_ULL instead of open-coding the same behaviour
> >   using div_u64()
> > * Do not round rate to 100MHz in cpg_z_clk_recalc_rate
> > * Remove calculation for PLL post-divider, this is bogus.
> >   Instead do not round to closest in cpg_z_clk_round_rate()
> > * Drop check for !prate in cpg_z_clk_round_rate
> 
> Thanks for the updates!
> 
> > --- a/drivers/clk/renesas/rcar-gen3-cpg.c
> > +++ b/drivers/clk/renesas/rcar-gen3-cpg.c
> 
> > +static unsigned long cpg_z_clk_recalc_rate(struct clk_hw *hw,
> > +                                          unsigned long parent_rate)
> > +{
> > +       struct cpg_z_clk *zclk = to_z_clk(hw);
> > +       unsigned int mult;
> > +
> > +       mult = 32 - FIELD_GET(CPG_FRQCRC_ZFC_MASK, clk_readl(zclk->reg));
> > +       return DIV_ROUND_CLOSEST_ULL(parent_rate * mult, 32);
> 
> While parent_rate is unsigned long and thus 64-bit on arm64, this will work
> fine on R-Car Gen3.  However, if someone ever tries to reuse this on a
> 32-bit platform, the multiplication may overflow.
> Hence I recommend to make this a 64-bit multiplication explicitly, i.e.
> 
>     return DIV_ROUND_CLOSEST_ULL((u64)parent_rate * mult, 32);

Thanks, will do.

> > +}
> > +
> > +static long cpg_z_clk_round_rate(struct clk_hw *hw, unsigned long rate,
> > +                                unsigned long *parent_rate)
> > +{
> > +       unsigned long prate = *parent_rate;
> > +       unsigned int mult;
> > +
> > +       mult = div_u64((u64)rate * 32, prate);
> 
> You can avoid the cast by making the constant 64-bit:
> 
>     mult = div_u64(rate * 32ULL, prate);

Thanks, that looks a bit nicer.

> > +       mult = clamp(mult, 1U, 32U);
> > +
> > +       return *parent_rate * mult / 32;
> 
> Again, *parent_rate is 64-bit on arm64, but not on 32-bit platforms.
> 
> > +       /*
> > +        * Note: There is no HW information about the worst case latency.
> > +        *
> > +        * Using experimental measurements, it seems that no more than
> > +        * ~10 iterations are needed, independently of the CPU rate.
> > +        * Since this value might be dependent of external xtal rate, pll1
> > +        * rate or even the other emulation clocks rate, use 1000 as a
> 
> another emulated clock rate?
> (Yeah, copied from Gen2 ;-)

It seem so.

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

[PATCH v3 2/6] clk: renesas: rcar-gen3: Add Z2 clock divider support

[Simon Horman]

From: Takeshi Kihara <takeshi.kihara.df@renesas.com>

This patch adds Z2 clock divider support for R-Car Gen3 SoC.

Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
v2 [Simon Horman]
* Consolidate Z and Z2 clock ops
* Allow setting of Z2 clock

v1 [Simon Horman]
* Provide __cpg_z_clk_recalc_rate() helper
* Use GENMASK

v0 [Takeshi Kihara]
---
 drivers/clk/renesas/rcar-gen3-cpg.c | 18 +++++++++++++-----
 drivers/clk/renesas/rcar-gen3-cpg.h |  1 +
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c b/drivers/clk/renesas/rcar-gen3-cpg.c
index d98272a6075f..e794e7b9c40a 100644
--- a/drivers/clk/renesas/rcar-gen3-cpg.c
+++ b/drivers/clk/renesas/rcar-gen3-cpg.c
@@ -31,7 +31,7 @@
 #define CPG_PLL4CR		0x01f4
 
 /*
- * Z Clock
+ * Z Clock & Z2 Clock
  *
  * Traits of this clock:
  * prepare - clk_prepare only ensures that parents are prepared
@@ -43,11 +43,13 @@
 #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;
 	void __iomem *reg;
 	void __iomem *kick_reg;
+	unsigned long mask;
 };
 
 #define to_z_clk(_hw)	container_of(_hw, struct cpg_z_clk, hw)
@@ -88,8 +90,8 @@ static int cpg_z_clk_set_rate(struct clk_hw *hw, unsigned long rate,
 	if (clk_readl(zclk->kick_reg) & CPG_FRQCRB_KICK)
 		return -EBUSY;
 
-	val = clk_readl(zclk->reg) & ~CPG_FRQCRC_ZFC_MASK;
-	val |= FIELD_PREP(CPG_FRQCRC_ZFC_MASK, 32 - mult);
+	val = clk_readl(zclk->reg) & ~zclk->mask;
+	val |= ((32 - mult) << __bf_shf(zclk->mask)) & zclk->mask;
 	clk_writel(val, zclk->reg);
 
 	/*
@@ -127,7 +129,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)
+					      void __iomem *reg,
+					      unsigned long mask)
 {
 	struct clk_init_data init;
 	struct cpg_z_clk *zclk;
@@ -146,6 +149,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;
 
 	clk = clk_register(NULL, &zclk->hw);
 	if (IS_ERR(clk))
@@ -499,7 +503,11 @@ struct clk * __init rcar_gen3_cpg_clk_register(struct device *dev,
 
 	case CLK_TYPE_GEN3_Z:
 		return cpg_z_clk_register(core->name, __clk_get_name(parent),
-					  base);
+					  base, CPG_FRQCRC_ZFC_MASK);
+
+	case CLK_TYPE_GEN3_Z2:
+		return cpg_z_clk_register(core->name, __clk_get_name(parent),
+					  base, CPG_FRQCRC_Z2FC_MASK);
 
 	default:
 		return ERR_PTR(-EINVAL);
diff --git a/drivers/clk/renesas/rcar-gen3-cpg.h b/drivers/clk/renesas/rcar-gen3-cpg.h
index 89da28f6f71b..c03fd498c96b 100644
--- a/drivers/clk/renesas/rcar-gen3-cpg.h
+++ b/drivers/clk/renesas/rcar-gen3-cpg.h
@@ -22,6 +22,7 @@ enum rcar_gen3_clk_types {
 	CLK_TYPE_GEN3_R,
 	CLK_TYPE_GEN3_PE,
 	CLK_TYPE_GEN3_Z,
+	CLK_TYPE_GEN3_Z2,
 };
 
 #define DEF_GEN3_SD(_name, _id, _parent, _offset)	\
-- 
2.1.4

[PATCH v3 2/6] clk: renesas: rcar-gen3: Add Z2 clock divider support

[Geert Uytterhoeven]

Hi Simon,

On Thu, Oct 5, 2017 at 3:23 PM, Simon Horman <horms+renesas@verge.net.au> wrote:
> From: Takeshi Kihara <takeshi.kihara.df@renesas.com>
>
> This patch adds Z2 clock divider support for R-Car Gen3 SoC.
>
> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> ---
> v2 [Simon Horman]
> * Consolidate Z and Z2 clock ops
> * Allow setting of Z2 clock

Thanks for the update!

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

> @@ -88,8 +90,8 @@ static int cpg_z_clk_set_rate(struct clk_hw *hw, unsigned long rate,
>         if (clk_readl(zclk->kick_reg) & CPG_FRQCRB_KICK)
>                 return -EBUSY;
>
> -       val = clk_readl(zclk->reg) & ~CPG_FRQCRC_ZFC_MASK;
> -       val |= FIELD_PREP(CPG_FRQCRC_ZFC_MASK, 32 - mult);
> +       val = clk_readl(zclk->reg) & ~zclk->mask;
> +       val |= ((32 - mult) << __bf_shf(zclk->mask)) & zclk->mask;

Any special reason you're now open coding FIELD_PREP()?

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

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at 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

[PATCH v3 2/6] clk: renesas: rcar-gen3: Add Z2 clock divider support

[Geert Uytterhoeven]

Hi Simon,

On Mon, Oct 9, 2017 at 10:02 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Thu, Oct 5, 2017 at 3:23 PM, Simon Horman <horms+renesas@verge.net.au> wrote:
>> From: Takeshi Kihara <takeshi.kihara.df@renesas.com>
>>
>> This patch adds Z2 clock divider support for R-Car Gen3 SoC.
>>
>> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
>> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
>> ---
>> v2 [Simon Horman]
>> * Consolidate Z and Z2 clock ops
>> * Allow setting of Z2 clock
>
> Thanks for the update!
>
>> --- a/drivers/clk/renesas/rcar-gen3-cpg.c
>> +++ b/drivers/clk/renesas/rcar-gen3-cpg.c
>
>> @@ -88,8 +90,8 @@ static int cpg_z_clk_set_rate(struct clk_hw *hw, unsigned long rate,
>>         if (clk_readl(zclk->kick_reg) & CPG_FRQCRB_KICK)
>>                 return -EBUSY;
>>
>> -       val = clk_readl(zclk->reg) & ~CPG_FRQCRC_ZFC_MASK;
>> -       val |= FIELD_PREP(CPG_FRQCRC_ZFC_MASK, 32 - mult);
>> +       val = clk_readl(zclk->reg) & ~zclk->mask;
>> +       val |= ((32 - mult) << __bf_shf(zclk->mask)) & zclk->mask;
>
> Any special reason you're now open coding FIELD_PREP()?

Now I know: to circumvent the
BUILD_BUG_ON_MSG(!__builtin_constant_p(_mask), ...) in
__BF_FIELD_CHECK(), called from FIELD_PREP().

Given <linux/bitfield.h> is intended to work with constant masks
only, I think it's best to not use it, and replace __bf_shf() by __ffs().

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

Oops, looks like you forgot to replace CPG_FRQCRC_ZFC_MASK in
cpg_z_clk_recalc_rate() by zclk->mask.
Sorry for missing that.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at 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

[PATCH v3 2/6] clk: renesas: rcar-gen3: Add Z2 clock divider support

[Simon Horman]

On Tue, Oct 10, 2017 at 08:56:35AM +0200, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Mon, Oct 9, 2017 at 10:02 AM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> > On Thu, Oct 5, 2017 at 3:23 PM, Simon Horman <horms+renesas@verge.net.au> wrote:
> >> From: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> >>
> >> This patch adds Z2 clock divider support for R-Car Gen3 SoC.
> >>
> >> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> >> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> >> ---
> >> v2 [Simon Horman]
> >> * Consolidate Z and Z2 clock ops
> >> * Allow setting of Z2 clock
> >
> > Thanks for the update!
> >
> >> --- a/drivers/clk/renesas/rcar-gen3-cpg.c
> >> +++ b/drivers/clk/renesas/rcar-gen3-cpg.c
> >
> >> @@ -88,8 +90,8 @@ static int cpg_z_clk_set_rate(struct clk_hw *hw, unsigned long rate,
> >>         if (clk_readl(zclk->kick_reg) & CPG_FRQCRB_KICK)
> >>                 return -EBUSY;
> >>
> >> -       val = clk_readl(zclk->reg) & ~CPG_FRQCRC_ZFC_MASK;
> >> -       val |= FIELD_PREP(CPG_FRQCRC_ZFC_MASK, 32 - mult);
> >> +       val = clk_readl(zclk->reg) & ~zclk->mask;
> >> +       val |= ((32 - mult) << __bf_shf(zclk->mask)) & zclk->mask;
> >
> > Any special reason you're now open coding FIELD_PREP()?
> 
> Now I know: to circumvent the
> BUILD_BUG_ON_MSG(!__builtin_constant_p(_mask), ...) in
> __BF_FIELD_CHECK(), called from FIELD_PREP().

Yes, that is correct.
A comment is probably in order.

> Given <linux/bitfield.h> is intended to work with constant masks
> only, I think it's best to not use it, and replace __bf_shf() by __ffs().

I see there has been some discussion of this elsewhere.
Perhaps its best to let that cool down before I repost.

> 
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> Oops, looks like you forgot to replace CPG_FRQCRC_ZFC_MASK in
> cpg_z_clk_recalc_rate() by zclk->mask.
> Sorry for missing that.

Thanks, will fix.

[PATCH v3 3/6] clk: renesas: r8a7795: Add Z clock

[Simon Horman]

From: Takeshi Kihara <takeshi.kihara.df@renesas.com>

This patch adds Z clock for R8A7795 SoC.

Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/clk/renesas/r8a7795-cpg-mssr.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/renesas/r8a7795-cpg-mssr.c b/drivers/clk/renesas/r8a7795-cpg-mssr.c
index 762b2f8824f1..2a0546fd47dc 100644
--- a/drivers/clk/renesas/r8a7795-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c
@@ -74,6 +74,7 @@ static struct cpg_core_clk r8a7795_core_clks[] __initdata = {
 	DEF_FIXED(".sdsrc",     CLK_SDSRC,         CLK_PLL1_DIV2,  2, 1),
 
 	/* Core Clock Outputs */
+	DEF_BASE("z",           R8A7795_CLK_Z,     CLK_TYPE_GEN3_Z, CLK_PLL0),
 	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),
-- 
2.1.4

[PATCH v3 4/6] clk: renesas: r8a7795: Add Z2 clock

[Simon Horman]

From: Takeshi Kihara <takeshi.kihara.df@renesas.com>

This patch adds Z2 clock for r8a7795 SoC.

Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/clk/renesas/r8a7795-cpg-mssr.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/renesas/r8a7795-cpg-mssr.c b/drivers/clk/renesas/r8a7795-cpg-mssr.c
index 2a0546fd47dc..1c931d8ec0cc 100644
--- a/drivers/clk/renesas/r8a7795-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c
@@ -75,6 +75,7 @@ static struct cpg_core_clk r8a7795_core_clks[] __initdata = {
 
 	/* Core Clock Outputs */
 	DEF_BASE("z",           R8A7795_CLK_Z,     CLK_TYPE_GEN3_Z, CLK_PLL0),
+	DEF_BASE("z2",          R8A7795_CLK_Z2,    CLK_TYPE_GEN3_Z2, CLK_PLL2),
 	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),
-- 
2.1.4

[PATCH v3 5/6] clk: renesas: r8a7796: Add Z clock

[Simon Horman]

From: Takeshi Kihara <takeshi.kihara.df@renesas.com>

This patch adds Z clock for R8A7796 SoC.

Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/clk/renesas/r8a7796-cpg-mssr.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/renesas/r8a7796-cpg-mssr.c b/drivers/clk/renesas/r8a7796-cpg-mssr.c
index e5e7fb212288..e8d5e75afc65 100644
--- a/drivers/clk/renesas/r8a7796-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a7796-cpg-mssr.c
@@ -74,6 +74,7 @@ static const struct cpg_core_clk r8a7796_core_clks[] __initconst = {
 	DEF_FIXED(".sdsrc",     CLK_SDSRC,         CLK_PLL1_DIV2,  2, 1),
 
 	/* Core Clock Outputs */
+	DEF_BASE("z",           R8A7796_CLK_Z,     CLK_TYPE_GEN3_Z, CLK_PLL0),
 	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),
-- 
2.1.4

[PATCH v3 6/6] clk: renesas: r8a7796: Add Z2 clock

[Simon Horman]

From: Takeshi Kihara <takeshi.kihara.df@renesas.com>

This patch adds Z2 clock for R8A7796 SoC.

Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/clk/renesas/r8a7796-cpg-mssr.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/renesas/r8a7796-cpg-mssr.c b/drivers/clk/renesas/r8a7796-cpg-mssr.c
index e8d5e75afc65..48731741b59e 100644
--- a/drivers/clk/renesas/r8a7796-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a7796-cpg-mssr.c
@@ -75,6 +75,7 @@ static const struct cpg_core_clk r8a7796_core_clks[] __initconst = {
 
 	/* Core Clock Outputs */
 	DEF_BASE("z",           R8A7796_CLK_Z,     CLK_TYPE_GEN3_Z, CLK_PLL0),
+	DEF_BASE("z2",          R8A7796_CLK_Z2,    CLK_TYPE_GEN3_Z2, CLK_PLL2),
 	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),
-- 
2.1.4

[PATCH v3 0/6] clk: renesas: r8a779[56]: Add Z and Z2 clock support

[Geert Uytterhoeven]

Hi Simon,

On Thu, Oct 5, 2017 at 3:23 PM, Simon Horman <horms+renesas@verge.net.au> wrote:
> this patch-set adds Z and Z2 clock support.
>
> These are dependencies for supporting CPUFreq. The remainder of that
> work is being posted separately and can be found at:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git topic/rcar-gen3-cpufreq
>
>
> A description of steps taken to lightly exercise the same feature for the
> r88a7795 the above can be found at the link below. The results are the same
> for the r8a7796 with the exception that it has two active CPU cores rather
> than four.
>
> http://elinux.org/Tests:R-CAR-GEN3-CPUFreq

Thanks for your patches!

I see the following anomalies on Salvator-X (R-Car H3 ES1.0 and M3-W ES1.0)
and Salvator-XS (R-Car H3 ES2.0):

1. dmesg:

    cpufreq: cpufreq_online: CPU0: Running at unlisted freq: 2999999 KHz
    cpu cpu0: dev_pm_opp_set_rate: failed to find current OPP for freq
2999999880 (-34)
    cpufreq: cpufreq_online: CPU0: Unlisted initial frequency changed
to: 1500000 KHz

2. Z2 clock frequency:

    grep -wE "(z|z2)" /sys/kernel/debug/clk/clk_summary
          z2                              0            0  2399999904
       0 0
          z                               0            0  1499999940
       0 0

So Linux is off by a factor of two for the initial Z and Z2 clock frequencies.
Which means it's probably off by a factor of two all the time, and thus running
the CA57 CPU cores at 750 MHz instead of 1.5 GHz?

<looking@the PMU cycle counter>

Yes, it's only running at 750 MHz.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at 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