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

[PATCH v4 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-v4

Based on v4.15-rc1

Changes since v3:
* Rebase
* Divide parent clock by 2 to give correct rate (again)                         * Take care not to overflow multiplication on 32 but platforms
* Use __ffs as FIELD_{GET,PREP} don't not work with non-constant masks
* Use correct mask in cpg_z_clk_recalc_rate()

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    | 143 +++++++++++++++++++++++++++++++++
 drivers/clk/renesas/rcar-gen3-cpg.h    |   2 +
 4 files changed, 149 insertions(+)

Results of testing this on the Salvator-X r8a7796 are as follows:

On Boot
-------

Z clock is half of pll0 and corresponds to CPU frequency.
Z2 clock is half pll2. This is as expected.

root@Debian:~# cd /sys/devices/system/cpu/cpu0/cpufreq
root@Debian:/sys/devices/system/cpu/cpu0/cpufreq# grep -E -w "pll0|z" /sys/kernel/debug/clk/clk_summary; grep -E -w "pll2|z2" /sys/kernel/debug/clk/clk_summary; grep . *cur*
       .pll0                              0            0  2999999880          0 0
          z                               0            0  1499999940          0 0
       .pll2                              0            0  2399999904          0 0
          z2                              0            0  1199999952          0 0
cpuinfo_cur_freq:1499999
scaling_cur_freq:1500000

CPU Freq scaled to 1000000 and then 500000
------------------------------------------

Z clock corresponds to new CPU frequency.
Z2 clock, PLL0 and PLL2 are unchanged.
This is also as expected.

root@Debian:/sys/devices/system/cpu/cpu0/cpufreq# echo 1000000 > scaling_max_freq
root@Debian:/sys/devices/system/cpu/cpu0/cpufreq# grep -E -w "pll0|z" /sys/kernel/debug/clk/clk_summary; grep -E -w "pll2|z2" /sys/kernel/debug/clk/clk_summary; grep . *cur*
 grep . *cur*lk_summary; grep -E -w "pll2|z2" /sys/kernel/debug/clk/clk_summary; 
       .pll0                              0            0  2999999880          0 0  
          z                               0            0  1031249959          0 0  
       .pll2                              0            0  2399999904          0 0  
          z2                              0            0  1199999952          0 0  
cpuinfo_cur_freq:1031249
scaling_cur_freq:1000000

root@Debian:/sys/devices/system/cpu/cpu0/cpufreq# echo 500000 > scaling_max_freq
root@Debian:/sys/devices/system/cpu/cpu0/cpufreq# grep -E -w "pll0|z" /sys/kernel/debug/clk/clk_summary; grep -E -w "pll2|z2" /sys/kernel/debug/clk/clk_summary; grep . *cur*
       .pll0                              0            0  2999999880          0 0  
          z                               0            0   468749981          0 0  
       .pll2                              0            0  2399999904          0 0  
          z2                              0            0  1199999952          0 0  
cpuinfo_cur_freq:468749
scaling_cur_freq:500000

[PATCH v4 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-v4

Based on v4.15-rc1

Changes since v3:
* Rebase
* Divide parent clock by 2 to give correct rate (again)                         * Take care not to overflow multiplication on 32 but platforms
* Use __ffs as FIELD_{GET,PREP} don't not work with non-constant masks
* Use correct mask in cpg_z_clk_recalc_rate()

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    | 143 +++++++++++++++++++++++++++++++++
 drivers/clk/renesas/rcar-gen3-cpg.h    |   2 +
 4 files changed, 149 insertions(+)

Results of testing this on the Salvator-X r8a7796 are as follows:

On Boot
-------

Z clock is half of pll0 and corresponds to CPU frequency.
Z2 clock is half pll2. This is as expected.

root at Debian:~# cd /sys/devices/system/cpu/cpu0/cpufreq
root at Debian:/sys/devices/system/cpu/cpu0/cpufreq# grep -E -w "pll0|z" /sys/kernel/debug/clk/clk_summary; grep -E -w "pll2|z2" /sys/kernel/debug/clk/clk_summary; grep . *cur*
       .pll0                              0            0  2999999880          0 0
          z                               0            0  1499999940          0 0
       .pll2                              0            0  2399999904          0 0
          z2                              0            0  1199999952          0 0
cpuinfo_cur_freq:1499999
scaling_cur_freq:1500000

CPU Freq scaled to 1000000 and then 500000
------------------------------------------

Z clock corresponds to new CPU frequency.
Z2 clock, PLL0 and PLL2 are unchanged.
This is also as expected.

root at Debian:/sys/devices/system/cpu/cpu0/cpufreq# echo 1000000 > scaling_max_freq
root at Debian:/sys/devices/system/cpu/cpu0/cpufreq# grep -E -w "pll0|z" /sys/kernel/debug/clk/clk_summary; grep -E -w "pll2|z2" /sys/kernel/debug/clk/clk_summary; grep . *cur*
 grep . *cur*lk_summary; grep -E -w "pll2|z2" /sys/kernel/debug/clk/clk_summary; 
       .pll0                              0            0  2999999880          0 0  
          z                               0            0  1031249959          0 0  
       .pll2                              0            0  2399999904          0 0  
          z2                              0            0  1199999952          0 0  
cpuinfo_cur_freq:1031249
scaling_cur_freq:1000000

root at Debian:/sys/devices/system/cpu/cpu0/cpufreq# echo 500000 > scaling_max_freq
root at Debian:/sys/devices/system/cpu/cpu0/cpufreq# grep -E -w "pll0|z" /sys/kernel/debug/clk/clk_summary; grep -E -w "pll2|z2" /sys/kernel/debug/clk/clk_summary; grep . *cur*
       .pll0                              0            0  2999999880          0 0  
          z                               0            0   468749981          0 0  
       .pll2                              0            0  2399999904          0 0  
          z2                              0            0  1199999952          0 0  
cpuinfo_cur_freq:468749
scaling_cur_freq:500000

[PATCH v4 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>
---

v4 [Simon Horman]
* Rebase
* Divide parent clock by 2 to give correct rate (again)
* Take care not to overflow multiplication on 32 but platforms

v3 [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 | 133 ++++++++++++++++++++++++++++++++++++
 drivers/clk/renesas/rcar-gen3-cpg.h |   1 +
 2 files changed, 134 insertions(+)

diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c b/drivers/clk/renesas/rcar-gen3-cpg.c
index 0904886f5501..b85918fa62c6 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>
@@ -62,6 +63,134 @@ static void cpg_simple_notifier_register(struct raw_notifier_head *notifiers,
 }
 
 /*
+ * 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));
+
+	/* Factor of 2 is for fixed divider */
+	return DIV_ROUND_CLOSEST_ULL((u64)parent_rate * mult, 32 * 2);
+}
+
+static long cpg_z_clk_round_rate(struct clk_hw *hw, unsigned long rate,
+				 unsigned long *parent_rate)
+{
+	/* Factor of 2 is for fixed divider */
+	unsigned long prate = *parent_rate / 2;
+	unsigned int mult;
+
+	mult = div_u64(rate * 32ULL, prate);
+	mult = clamp(mult, 1U, 32U);
+
+	return (u64)prate * 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;
+
+	/* Factor of 2 is for fixed divider */
+	mult = DIV_ROUND_CLOSEST_ULL(rate * 32ULL * 2, 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
  */
 #define CPG_SD_STP_HCK		BIT(9)
@@ -420,6 +549,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 2e4284399f53..c73d4d6fdc85 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.11.0

[PATCH v4 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>
---

v4 [Simon Horman]
* Rebase
* Divide parent clock by 2 to give correct rate (again)
* Take care not to overflow multiplication on 32 but platforms

v3 [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 | 133 ++++++++++++++++++++++++++++++++++++
 drivers/clk/renesas/rcar-gen3-cpg.h |   1 +
 2 files changed, 134 insertions(+)

diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c b/drivers/clk/renesas/rcar-gen3-cpg.c
index 0904886f5501..b85918fa62c6 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>
@@ -62,6 +63,134 @@ static void cpg_simple_notifier_register(struct raw_notifier_head *notifiers,
 }
 
 /*
+ * 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));
+
+	/* Factor of 2 is for fixed divider */
+	return DIV_ROUND_CLOSEST_ULL((u64)parent_rate * mult, 32 * 2);
+}
+
+static long cpg_z_clk_round_rate(struct clk_hw *hw, unsigned long rate,
+				 unsigned long *parent_rate)
+{
+	/* Factor of 2 is for fixed divider */
+	unsigned long prate = *parent_rate / 2;
+	unsigned int mult;
+
+	mult = div_u64(rate * 32ULL, prate);
+	mult = clamp(mult, 1U, 32U);
+
+	return (u64)prate * 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;
+
+	/* Factor of 2 is for fixed divider */
+	mult = DIV_ROUND_CLOSEST_ULL(rate * 32ULL * 2, 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
  */
 #define CPG_SD_STP_HCK		BIT(9)
@@ -420,6 +549,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 2e4284399f53..c73d4d6fdc85 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.11.0

[PATCH v4 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>
---
v4 [Simon Horman]
* Rebase
* Use __ffs as FIELD_{GET,PREP} don't not work with non-constant masks
* Use correct mask in cpg_z_clk_recalc_rate()

v3 [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 | 22 ++++++++++++++++------
 drivers/clk/renesas/rcar-gen3-cpg.h |  1 +
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c b/drivers/clk/renesas/rcar-gen3-cpg.c
index b85918fa62c6..a7d68cefda9c 100644
--- a/drivers/clk/renesas/rcar-gen3-cpg.c
+++ b/drivers/clk/renesas/rcar-gen3-cpg.c
@@ -63,7 +63,7 @@ static void cpg_simple_notifier_register(struct raw_notifier_head *notifiers,
 }
 
 /*
- * Z Clock
+ * Z Clock & Z2 Clock
  *
  * Traits of this clock:
  * prepare - clk_prepare only ensures that parents are prepared
@@ -75,11 +75,13 @@ static void cpg_simple_notifier_register(struct raw_notifier_head *notifiers,
 #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)
@@ -89,8 +91,10 @@ static unsigned long cpg_z_clk_recalc_rate(struct clk_hw *hw,
 {
 	struct cpg_z_clk *zclk = to_z_clk(hw);
 	unsigned int mult;
+	u32 val;
 
-	mult = 32 - FIELD_GET(CPG_FRQCRC_ZFC_MASK, clk_readl(zclk->reg));
+	val = clk_readl(zclk->reg) & zclk->mask;
+	mult = 32 - (val >> (__ffs(zclk->mask) - 1));
 
 	/* Factor of 2 is for fixed divider */
 	return DIV_ROUND_CLOSEST_ULL((u64)parent_rate * mult, 32 * 2);
@@ -124,8 +128,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) << (__ffs(zclk->mask) - 1)) & zclk->mask;
 	clk_writel(val, zclk->reg);
 
 	/*
@@ -163,7 +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)
+					      void __iomem *reg,
+					      unsigned long mask)
 {
 	struct clk_init_data init;
 	struct cpg_z_clk *zclk;
@@ -182,6 +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;
 
 	clk = clk_register(NULL, &zclk->hw);
 	if (IS_ERR(clk))
@@ -551,7 +557,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 c73d4d6fdc85..ea4f8fc3c4c9 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.11.0

[PATCH v4 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>
---
v4 [Simon Horman]
* Rebase
* Use __ffs as FIELD_{GET,PREP} don't not work with non-constant masks
* Use correct mask in cpg_z_clk_recalc_rate()

v3 [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 | 22 ++++++++++++++++------
 drivers/clk/renesas/rcar-gen3-cpg.h |  1 +
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c b/drivers/clk/renesas/rcar-gen3-cpg.c
index b85918fa62c6..a7d68cefda9c 100644
--- a/drivers/clk/renesas/rcar-gen3-cpg.c
+++ b/drivers/clk/renesas/rcar-gen3-cpg.c
@@ -63,7 +63,7 @@ static void cpg_simple_notifier_register(struct raw_notifier_head *notifiers,
 }
 
 /*
- * Z Clock
+ * Z Clock & Z2 Clock
  *
  * Traits of this clock:
  * prepare - clk_prepare only ensures that parents are prepared
@@ -75,11 +75,13 @@ static void cpg_simple_notifier_register(struct raw_notifier_head *notifiers,
 #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)
@@ -89,8 +91,10 @@ static unsigned long cpg_z_clk_recalc_rate(struct clk_hw *hw,
 {
 	struct cpg_z_clk *zclk = to_z_clk(hw);
 	unsigned int mult;
+	u32 val;
 
-	mult = 32 - FIELD_GET(CPG_FRQCRC_ZFC_MASK, clk_readl(zclk->reg));
+	val = clk_readl(zclk->reg) & zclk->mask;
+	mult = 32 - (val >> (__ffs(zclk->mask) - 1));
 
 	/* Factor of 2 is for fixed divider */
 	return DIV_ROUND_CLOSEST_ULL((u64)parent_rate * mult, 32 * 2);
@@ -124,8 +128,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) << (__ffs(zclk->mask) - 1)) & zclk->mask;
 	clk_writel(val, zclk->reg);
 
 	/*
@@ -163,7 +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)
+					      void __iomem *reg,
+					      unsigned long mask)
 {
 	struct clk_init_data init;
 	struct cpg_z_clk *zclk;
@@ -182,6 +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;
 
 	clk = clk_register(NULL, &zclk->hw);
 	if (IS_ERR(clk))
@@ -551,7 +557,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 c73d4d6fdc85..ea4f8fc3c4c9 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.11.0

[PATCH v4 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 b1d9f48eae9e..995a4c4fb01e 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.11.0

[PATCH v4 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 b1d9f48eae9e..995a4c4fb01e 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.11.0

[PATCH v4 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 995a4c4fb01e..775b0ceaa337 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.11.0

[PATCH v4 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 995a4c4fb01e..775b0ceaa337 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.11.0

[PATCH v4 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 b3767472088a..82b444ac66c6 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.11.0

[PATCH v4 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 b3767472088a..82b444ac66c6 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.11.0

[PATCH v4 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 82b444ac66c6..83a68e51e4ec 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.11.0

[PATCH v4 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 82b444ac66c6..83a68e51e4ec 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.11.0

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

[Geert Uytterhoeven]

Hi Simon,

On Wed, Jan 3, 2018 at 1:18 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>
> ---
> v4 [Simon Horman]
> * Rebase
> * Use __ffs as FIELD_{GET,PREP} don't not work with non-constant masks
> * Use correct mask in cpg_z_clk_recalc_rate()

Thanks for the update!

> --- a/drivers/clk/renesas/rcar-gen3-cpg.c
> +++ b/drivers/clk/renesas/rcar-gen3-cpg.c
> @@ -63,7 +63,7 @@ static void cpg_simple_notifier_register(struct raw_notifier_head *notifiers,
>  }
>
>  /*
> - * Z Clock
> + * Z Clock & Z2 Clock
>   *
>   * Traits of this clock:
>   * prepare - clk_prepare only ensures that parents are prepared
> @@ -75,11 +75,13 @@ static void cpg_simple_notifier_register(struct raw_notifier_head *notifiers,
>  #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)
> @@ -89,8 +91,10 @@ static unsigned long cpg_z_clk_recalc_rate(struct clk_hw *hw,
>  {
>         struct cpg_z_clk *zclk = to_z_clk(hw);
>         unsigned int mult;
> +       u32 val;
>
> -       mult = 32 - FIELD_GET(CPG_FRQCRC_ZFC_MASK, clk_readl(zclk->reg));
> +       val = clk_readl(zclk->reg) & zclk->mask;
> +       mult = 32 - (val >> (__ffs(zclk->mask) - 1));

Shouldn't that be

        mult = 32 - (val >> __ffs(zclk->mask));

(same below)?

__ffs() returns 0..31, so you will shift right by 7 (Z) or -1 (Z2)?

As the CPG/MSSR driver now has suspend/resume support, do we need
a notifier to restore the Z or Z2 registers? Or is that handled automatically
by cpufreq during system resume, for both the primary and the secondary
CPU cores?

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

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

[Geert Uytterhoeven]

Hi Simon,

On Wed, Jan 3, 2018 at 1:18 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>
> ---
> v4 [Simon Horman]
> * Rebase
> * Use __ffs as FIELD_{GET,PREP} don't not work with non-constant masks
> * Use correct mask in cpg_z_clk_recalc_rate()

Thanks for the update!

> --- a/drivers/clk/renesas/rcar-gen3-cpg.c
> +++ b/drivers/clk/renesas/rcar-gen3-cpg.c
> @@ -63,7 +63,7 @@ static void cpg_simple_notifier_register(struct raw_notifier_head *notifiers,
>  }
>
>  /*
> - * Z Clock
> + * Z Clock & Z2 Clock
>   *
>   * Traits of this clock:
>   * prepare - clk_prepare only ensures that parents are prepared
> @@ -75,11 +75,13 @@ static void cpg_simple_notifier_register(struct raw_notifier_head *notifiers,
>  #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)
> @@ -89,8 +91,10 @@ static unsigned long cpg_z_clk_recalc_rate(struct clk_hw *hw,
>  {
>         struct cpg_z_clk *zclk = to_z_clk(hw);
>         unsigned int mult;
> +       u32 val;
>
> -       mult = 32 - FIELD_GET(CPG_FRQCRC_ZFC_MASK, clk_readl(zclk->reg));
> +       val = clk_readl(zclk->reg) & zclk->mask;
> +       mult = 32 - (val >> (__ffs(zclk->mask) - 1));

Shouldn't that be

        mult = 32 - (val >> __ffs(zclk->mask));

(same below)?

__ffs() returns 0..31, so you will shift right by 7 (Z) or -1 (Z2)?

As the CPG/MSSR driver now has suspend/resume support, do we need
a notifier to restore the Z or Z2 registers? Or is that handled automatically
by cpufreq during system resume, for both the primary and the secondary
CPU cores?

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

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

[Simon Horman]

On Wed, Jan 03, 2018 at 01:47:08PM +0100, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Wed, Jan 3, 2018 at 1:18 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>
> > ---
> > v4 [Simon Horman]
> > * Rebase
> > * Use __ffs as FIELD_{GET,PREP} don't not work with non-constant masks
> > * Use correct mask in cpg_z_clk_recalc_rate()
> 
> Thanks for the update!
> 
> > --- a/drivers/clk/renesas/rcar-gen3-cpg.c
> > +++ b/drivers/clk/renesas/rcar-gen3-cpg.c
> > @@ -63,7 +63,7 @@ static void cpg_simple_notifier_register(struct raw_notifier_head *notifiers,
> >  }
> >
> >  /*
> > - * Z Clock
> > + * Z Clock & Z2 Clock
> >   *
> >   * Traits of this clock:
> >   * prepare - clk_prepare only ensures that parents are prepared
> > @@ -75,11 +75,13 @@ static void cpg_simple_notifier_register(struct raw_notifier_head *notifiers,
> >  #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)
> > @@ -89,8 +91,10 @@ static unsigned long cpg_z_clk_recalc_rate(struct clk_hw *hw,
> >  {
> >         struct cpg_z_clk *zclk = to_z_clk(hw);
> >         unsigned int mult;
> > +       u32 val;
> >
> > -       mult = 32 - FIELD_GET(CPG_FRQCRC_ZFC_MASK, clk_readl(zclk->reg));
> > +       val = clk_readl(zclk->reg) & zclk->mask;
> > +       mult = 32 - (val >> (__ffs(zclk->mask) - 1));
> 
> Shouldn't that be
> 
>         mult = 32 - (val >> __ffs(zclk->mask));
> 
> (same below)?
> 
> __ffs() returns 0..31, so you will shift right by 7 (Z) or -1 (Z2)?

Thanks, I'll look at fixing that.

> As the CPG/MSSR driver now has suspend/resume support, do we need
> a notifier to restore the Z or Z2 registers? Or is that handled automatically
> by cpufreq during system resume, for both the primary and the secondary
> CPU cores?

I am a bit unsure.

When using the A57 cores, which is the default case, the Z clk is queried
by CPUFreq on resume. It appears that on my system its already set to the
correct value but I assume if it was not then it would be reset. However,
this does not cover Z2 clk. So perhaps to be safe we need to register
notifiers and make sure they they play nicely with CPUFreq?

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

[Simon Horman]

On Wed, Jan 03, 2018 at 01:47:08PM +0100, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Wed, Jan 3, 2018 at 1:18 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>
> > ---
> > v4 [Simon Horman]
> > * Rebase
> > * Use __ffs as FIELD_{GET,PREP} don't not work with non-constant masks
> > * Use correct mask in cpg_z_clk_recalc_rate()
> 
> Thanks for the update!
> 
> > --- a/drivers/clk/renesas/rcar-gen3-cpg.c
> > +++ b/drivers/clk/renesas/rcar-gen3-cpg.c
> > @@ -63,7 +63,7 @@ static void cpg_simple_notifier_register(struct raw_notifier_head *notifiers,
> >  }
> >
> >  /*
> > - * Z Clock
> > + * Z Clock & Z2 Clock
> >   *
> >   * Traits of this clock:
> >   * prepare - clk_prepare only ensures that parents are prepared
> > @@ -75,11 +75,13 @@ static void cpg_simple_notifier_register(struct raw_notifier_head *notifiers,
> >  #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)
> > @@ -89,8 +91,10 @@ static unsigned long cpg_z_clk_recalc_rate(struct clk_hw *hw,
> >  {
> >         struct cpg_z_clk *zclk = to_z_clk(hw);
> >         unsigned int mult;
> > +       u32 val;
> >
> > -       mult = 32 - FIELD_GET(CPG_FRQCRC_ZFC_MASK, clk_readl(zclk->reg));
> > +       val = clk_readl(zclk->reg) & zclk->mask;
> > +       mult = 32 - (val >> (__ffs(zclk->mask) - 1));
> 
> Shouldn't that be
> 
>         mult = 32 - (val >> __ffs(zclk->mask));
> 
> (same below)?
> 
> __ffs() returns 0..31, so you will shift right by 7 (Z) or -1 (Z2)?

Thanks, I'll look at fixing that.

> As the CPG/MSSR driver now has suspend/resume support, do we need
> a notifier to restore the Z or Z2 registers? Or is that handled automatically
> by cpufreq during system resume, for both the primary and the secondary
> CPU cores?

I am a bit unsure.

When using the A57 cores, which is the default case, the Z clk is queried
by CPUFreq on resume. It appears that on my system its already set to the
correct value but I assume if it was not then it would be reset. However,
this does not cover Z2 clk. So perhaps to be safe we need to register
notifiers and make sure they they play nicely with CPUFreq?

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

[Geert Uytterhoeven]

Hi Simon,

On Fri, Jan 5, 2018 at 3:04 PM, Simon Horman <horms@verge.net.au> wrote:
> On Wed, Jan 03, 2018 at 01:47:08PM +0100, Geert Uytterhoeven wrote:
>> On Wed, Jan 3, 2018 at 1:18 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>

>> As the CPG/MSSR driver now has suspend/resume support, do we need
>> a notifier to restore the Z or Z2 registers? Or is that handled automatically
>> by cpufreq during system resume, for both the primary and the secondary
>> CPU cores?
>
> I am a bit unsure.
>
> When using the A57 cores, which is the default case, the Z clk is queried
> by CPUFreq on resume. It appears that on my system its already set to the
> correct value but I assume if it was not then it would be reset. However,
> this does not cover Z2 clk. So perhaps to be safe we need to register
> notifiers and make sure they they play nicely with CPUFreq?

Of course the CPU is special: unlike many other devices, it must be running
when the kernel is reentered upon system resume.
It may be running using a different frequency setting, though.
However, following "opp-suspend", the system will always suspend with the
Z clock running at 1.5GHz, which is the default?
So Z is probably OK.

It's more interesting to check what happens when the little cores are
enabled as well (unfortunately that requires different firmware).
1. Does cpufreq handle them correctly when they are onlined again during
   system resume?
2. What happens if you offline all big cores, and enter system suspend
   running with little cores only?
   Perhaps that's prevented by the "opp-suspend" property as well?

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

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

[Geert Uytterhoeven]

Hi Simon,

On Fri, Jan 5, 2018 at 3:04 PM, Simon Horman <horms@verge.net.au> wrote:
> On Wed, Jan 03, 2018 at 01:47:08PM +0100, Geert Uytterhoeven wrote:
>> On Wed, Jan 3, 2018 at 1:18 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>

>> As the CPG/MSSR driver now has suspend/resume support, do we need
>> a notifier to restore the Z or Z2 registers? Or is that handled automatically
>> by cpufreq during system resume, for both the primary and the secondary
>> CPU cores?
>
> I am a bit unsure.
>
> When using the A57 cores, which is the default case, the Z clk is queried
> by CPUFreq on resume. It appears that on my system its already set to the
> correct value but I assume if it was not then it would be reset. However,
> this does not cover Z2 clk. So perhaps to be safe we need to register
> notifiers and make sure they they play nicely with CPUFreq?

Of course the CPU is special: unlike many other devices, it must be running
when the kernel is reentered upon system resume.
It may be running using a different frequency setting, though.
However, following "opp-suspend", the system will always suspend with the
Z clock running at 1.5GHz, which is the default?
So Z is probably OK.

It's more interesting to check what happens when the little cores are
enabled as well (unfortunately that requires different firmware).
1. Does cpufreq handle them correctly when they are onlined again during
   system resume?
2. What happens if you offline all big cores, and enter system suspend
   running with little cores only?
   Perhaps that's prevented by the "opp-suspend" property as well?

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

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

[Simon Horman]

On Fri, Jan 05, 2018 at 03:35:13PM +0100, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Fri, Jan 5, 2018 at 3:04 PM, Simon Horman <horms@verge.net.au> wrote:
> > On Wed, Jan 03, 2018 at 01:47:08PM +0100, Geert Uytterhoeven wrote:
> >> On Wed, Jan 3, 2018 at 1:18 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>
> 
> >> As the CPG/MSSR driver now has suspend/resume support, do we need
> >> a notifier to restore the Z or Z2 registers? Or is that handled automatically
> >> by cpufreq during system resume, for both the primary and the secondary
> >> CPU cores?
> >
> > I am a bit unsure.
> >
> > When using the A57 cores, which is the default case, the Z clk is queried
> > by CPUFreq on resume. It appears that on my system its already set to the
> > correct value but I assume if it was not then it would be reset. However,
> > this does not cover Z2 clk. So perhaps to be safe we need to register
> > notifiers and make sure they they play nicely with CPUFreq?
> 
> Of course the CPU is special: unlike many other devices, it must be running
> when the kernel is reentered upon system resume.
> It may be running using a different frequency setting, though.
> However, following "opp-suspend", the system will always suspend with the
> Z clock running at 1.5GHz, which is the default?
> So Z is probably OK.
> 
> It's more interesting to check what happens when the little cores are
> enabled as well (unfortunately that requires different firmware).
> 1. Does cpufreq handle them correctly when they are onlined again during
>    system resume?
> 2. What happens if you offline all big cores, and enter system suspend
>    running with little cores only?
>    Perhaps that's prevented by the "opp-suspend" property as well?

Thanks for clarifying the problem.

I should be able to install updated firmware on:
i* r8a7796 (M3-W) ES1.0 / Salvator-X; and
* r8a7795 (H3) ES2.0 / Salvator-XS.
* But not easily r8a7795 (H3) / Salvator-X

Do you know what the minimal / desired firmware version is to exercise the
scenarios you describe.

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

[Simon Horman]

On Fri, Jan 05, 2018 at 03:35:13PM +0100, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Fri, Jan 5, 2018 at 3:04 PM, Simon Horman <horms@verge.net.au> wrote:
> > On Wed, Jan 03, 2018 at 01:47:08PM +0100, Geert Uytterhoeven wrote:
> >> On Wed, Jan 3, 2018 at 1:18 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>
> 
> >> As the CPG/MSSR driver now has suspend/resume support, do we need
> >> a notifier to restore the Z or Z2 registers? Or is that handled automatically
> >> by cpufreq during system resume, for both the primary and the secondary
> >> CPU cores?
> >
> > I am a bit unsure.
> >
> > When using the A57 cores, which is the default case, the Z clk is queried
> > by CPUFreq on resume. It appears that on my system its already set to the
> > correct value but I assume if it was not then it would be reset. However,
> > this does not cover Z2 clk. So perhaps to be safe we need to register
> > notifiers and make sure they they play nicely with CPUFreq?
> 
> Of course the CPU is special: unlike many other devices, it must be running
> when the kernel is reentered upon system resume.
> It may be running using a different frequency setting, though.
> However, following "opp-suspend", the system will always suspend with the
> Z clock running at 1.5GHz, which is the default?
> So Z is probably OK.
> 
> It's more interesting to check what happens when the little cores are
> enabled as well (unfortunately that requires different firmware).
> 1. Does cpufreq handle them correctly when they are onlined again during
>    system resume?
> 2. What happens if you offline all big cores, and enter system suspend
>    running with little cores only?
>    Perhaps that's prevented by the "opp-suspend" property as well?

Thanks for clarifying the problem.

I should be able to install updated firmware on:
i* r8a7796 (M3-W) ES1.0 / Salvator-X; and
* r8a7795 (H3) ES2.0 / Salvator-XS.
* But not easily r8a7795 (H3) / Salvator-X

Do you know what the minimal / desired firmware version is to exercise the
scenarios you describe.

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

[Geert Uytterhoeven]

Hi Simon,

On Mon, Jan 8, 2018 at 9:02 AM, Simon Horman <horms@verge.net.au> wrote:
> On Fri, Jan 05, 2018 at 03:35:13PM +0100, Geert Uytterhoeven wrote:
>> On Fri, Jan 5, 2018 at 3:04 PM, Simon Horman <horms@verge.net.au> wrote:
>> > On Wed, Jan 03, 2018 at 01:47:08PM +0100, Geert Uytterhoeven wrote:
>> >> On Wed, Jan 3, 2018 at 1:18 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>
>>
>> >> As the CPG/MSSR driver now has suspend/resume support, do we need
>> >> a notifier to restore the Z or Z2 registers? Or is that handled automatically
>> >> by cpufreq during system resume, for both the primary and the secondary
>> >> CPU cores?
>> >
>> > I am a bit unsure.
>> >
>> > When using the A57 cores, which is the default case, the Z clk is queried
>> > by CPUFreq on resume. It appears that on my system its already set to the
>> > correct value but I assume if it was not then it would be reset. However,
>> > this does not cover Z2 clk. So perhaps to be safe we need to register
>> > notifiers and make sure they they play nicely with CPUFreq?
>>
>> Of course the CPU is special: unlike many other devices, it must be running
>> when the kernel is reentered upon system resume.
>> It may be running using a different frequency setting, though.
>> However, following "opp-suspend", the system will always suspend with the
>> Z clock running at 1.5GHz, which is the default?
>> So Z is probably OK.
>>
>> It's more interesting to check what happens when the little cores are
>> enabled as well (unfortunately that requires different firmware).
>> 1. Does cpufreq handle them correctly when they are onlined again during
>>    system resume?
>> 2. What happens if you offline all big cores, and enter system suspend
>>    running with little cores only?
>>    Perhaps that's prevented by the "opp-suspend" property as well?
>
> Thanks for clarifying the problem.
>
> I should be able to install updated firmware on:
> i* r8a7796 (M3-W) ES1.0 / Salvator-X; and
> * r8a7795 (H3) ES2.0 / Salvator-XS.
> * But not easily r8a7795 (H3) / Salvator-X
>
> Do you know what the minimal / desired firmware version is to exercise the
> scenarios you describe.

There are binaries available for "v2.7.0 8core". Unfortunately I
believe it supports
your "not easily r8a7795 (H3) / Salvator-X" only...

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

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

[Geert Uytterhoeven]

Hi Simon,

On Mon, Jan 8, 2018 at 9:02 AM, Simon Horman <horms@verge.net.au> wrote:
> On Fri, Jan 05, 2018 at 03:35:13PM +0100, Geert Uytterhoeven wrote:
>> On Fri, Jan 5, 2018 at 3:04 PM, Simon Horman <horms@verge.net.au> wrote:
>> > On Wed, Jan 03, 2018 at 01:47:08PM +0100, Geert Uytterhoeven wrote:
>> >> On Wed, Jan 3, 2018 at 1:18 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>
>>
>> >> As the CPG/MSSR driver now has suspend/resume support, do we need
>> >> a notifier to restore the Z or Z2 registers? Or is that handled automatically
>> >> by cpufreq during system resume, for both the primary and the secondary
>> >> CPU cores?
>> >
>> > I am a bit unsure.
>> >
>> > When using the A57 cores, which is the default case, the Z clk is queried
>> > by CPUFreq on resume. It appears that on my system its already set to the
>> > correct value but I assume if it was not then it would be reset. However,
>> > this does not cover Z2 clk. So perhaps to be safe we need to register
>> > notifiers and make sure they they play nicely with CPUFreq?
>>
>> Of course the CPU is special: unlike many other devices, it must be running
>> when the kernel is reentered upon system resume.
>> It may be running using a different frequency setting, though.
>> However, following "opp-suspend", the system will always suspend with the
>> Z clock running at 1.5GHz, which is the default?
>> So Z is probably OK.
>>
>> It's more interesting to check what happens when the little cores are
>> enabled as well (unfortunately that requires different firmware).
>> 1. Does cpufreq handle them correctly when they are onlined again during
>>    system resume?
>> 2. What happens if you offline all big cores, and enter system suspend
>>    running with little cores only?
>>    Perhaps that's prevented by the "opp-suspend" property as well?
>
> Thanks for clarifying the problem.
>
> I should be able to install updated firmware on:
> i* r8a7796 (M3-W) ES1.0 / Salvator-X; and
> * r8a7795 (H3) ES2.0 / Salvator-XS.
> * But not easily r8a7795 (H3) / Salvator-X
>
> Do you know what the minimal / desired firmware version is to exercise the
> scenarios you describe.

There are binaries available for "v2.7.0 8core". Unfortunately I
believe it supports
your "not easily r8a7795 (H3) / Salvator-X" only...

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

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

[Simon Horman]

On Fri, Jan 05, 2018 at 03:35:13PM +0100, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Fri, Jan 5, 2018 at 3:04 PM, Simon Horman <horms@verge.net.au> wrote:
> > On Wed, Jan 03, 2018 at 01:47:08PM +0100, Geert Uytterhoeven wrote:
> >> On Wed, Jan 3, 2018 at 1:18 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>
> 
> >> As the CPG/MSSR driver now has suspend/resume support, do we need
> >> a notifier to restore the Z or Z2 registers? Or is that handled automatically
> >> by cpufreq during system resume, for both the primary and the secondary
> >> CPU cores?
> >
> > I am a bit unsure.
> >
> > When using the A57 cores, which is the default case, the Z clk is queried
> > by CPUFreq on resume. It appears that on my system its already set to the
> > correct value but I assume if it was not then it would be reset. However,
> > this does not cover Z2 clk. So perhaps to be safe we need to register
> > notifiers and make sure they they play nicely with CPUFreq?
> 
> Of course the CPU is special: unlike many other devices, it must be running
> when the kernel is reentered upon system resume.
> It may be running using a different frequency setting, though.
> However, following "opp-suspend", the system will always suspend with the
> Z clock running at 1.5GHz, which is the default?
> So Z is probably OK.
> 
> It's more interesting to check what happens when the little cores are
> enabled as well (unfortunately that requires different firmware).
> 1. Does cpufreq handle them correctly when they are onlined again during
>    system resume?

I tested this by updating the firmware on an H3 ES2.0 / Salvator-XS
using the instructions at
https://elinux.org/R-Car/Virtualization#Enabling_HYP_Support

I also fleshed out the opts table for the little CPUs using the following
from the 3.6.0 BSP:

diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
index 1485e6a8e112..671c2d7ed6d0 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
@@ -219,7 +219,17 @@
 		compatible = "operating-points-v2";
 		opp-shared;
 
-		opp-1200000000 {
+		opp@800000000 {
+			opp-hz = /bits/ 64 <800000000>;
+			opp-microvolt = <820000>;
+			clock-latency-ns = <300000>;
+		};
+		opp@1000000000 {
+			opp-hz = /bits/ 64 <1000000000>;
+			opp-microvolt = <820000>;
+			clock-latency-ns = <300000>;
+		};
+		opp@1200000000 {
 			opp-hz = /bits/ 64 <1200000000>;
 			opp-microvolt = <820000>;
 			clock-latency-ns = <300000>;

And fixed the out-by-one bug you pointed out elsewhere in this thread:

diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c b/drivers/clk/renesas/rcar-gen3-cpg.c
index a7d68cefda9c..0c8fe10d57fe 100644
--- a/drivers/clk/renesas/rcar-gen3-cpg.c
+++ b/drivers/clk/renesas/rcar-gen3-cpg.c
@@ -94,7 +94,7 @@ static unsigned long cpg_z_clk_recalc_rate(struct clk_hw *hw,
 	u32 val;
 
 	val = clk_readl(zclk->reg) & zclk->mask;
-	mult = 32 - (val >> (__ffs(zclk->mask) - 1));
+	mult = 32 - (val >> __ffs(zclk->mask));
 
 	/* Factor of 2 is for fixed divider */
 	return DIV_ROUND_CLOSEST_ULL((u64)parent_rate * mult, 32 * 2);
@@ -129,7 +129,7 @@ static int cpg_z_clk_set_rate(struct clk_hw *hw, unsigned long rate,
 		return -EBUSY;
 
 	val = clk_readl(zclk->reg) & ~zclk->mask;
-	val |= ((32 - mult) << (__ffs(zclk->mask) - 1)) & zclk->mask;
+	val |= ((32 - mult) << __ffs(zclk->mask)) & zclk->mask;
 	clk_writel(val, zclk->reg);
 
 	/*

With these changes in place CPUFreq seems to work for both BIG and little
CPUS. And the frequencies seem to be correct after S2RAM.

# cd /sys/devices/system/cpu/
# grep . cpu*/cpufreq/scaling_available_frequencies
cpu0/cpufreq/scaling_available_frequencies:500000 1000000 1500000 
...
cpu4/cpufreq/scaling_available_frequencies:800000 1000000 1200000 
...
# grep grep -E -w "pll[01]|z|z2" /sys/kernel/debug/clk/clk_summary
grep: pll[01]|z|z2: No such file or directory
# grep -E -w "pll[01]|z|z2" /sys/kernel/debug/clk/clk_summary
          z2                              0            0  1198080000          0 0  
       .pll1                              1            1  3194880000          0 0  
       .pll0                              0            0  2995200000          0 0  
          z                               0            0  1497600000          0 0  
# echo 500000 > cpu0/cpufreq/scaling_max_freq
# echo 800000 > cpu4/cpufreq/scaling_max_freq
# grep . cpu*/cpufreq/*cur*
cpu0/cpufreq/cpuinfo_cur_freq:468000
cpu0/cpufreq/scaling_cur_freq:500000
...
cpu4/cpufreq/cpuinfo_cur_freq:786240
cpu4/cpufreq/scaling_cur_freq:786240
...
# grep -E -w "pll[01]|z|z2" /sys/kernel/debug/clk/clk_summary
          z2                              0            0   786240000          0 0  
       .pll1                              1            1  3194880000          0 0  
       .pll0                              0            0  2995200000          0 0  
          z                               0            0   468000000          0 0  
# i2cset -f -y 7 0x30 0x20 0x0F
# echo mem > /sys/power/state
...
[  203.289585] Restarting tasks ... done.
[  203.299250] PM: suspend exit
# grep . cpu*/cpufreq/*cur*
cpu0/cpufreq/cpuinfo_cur_freq:468000
cpu0/cpufreq/scaling_cur_freq:500000
...
cpu4/cpufreq/cpuinfo_cur_freq:786240
cpu4/cpufreq/scaling_cur_freq:800000
...
# grep -E -w "pll[01]|z|z2" /sys/kernel/debug/clk/clk_summary
          z2                              0            0   786240000          0 0  
       .pll1                              1            1  3194880000          0 0  
       .pll0                              0            0  2995200000          0 0  
          z                               0            0   468000000          0 0  

> 2. What happens if you offline all big cores, and enter system suspend
>    running with little cores only?
>    Perhaps that's prevented by the "opp-suspend" property as well?

I am unable to take CPU0 offline.

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

[Simon Horman]

On Fri, Jan 05, 2018 at 03:35:13PM +0100, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Fri, Jan 5, 2018 at 3:04 PM, Simon Horman <horms@verge.net.au> wrote:
> > On Wed, Jan 03, 2018 at 01:47:08PM +0100, Geert Uytterhoeven wrote:
> >> On Wed, Jan 3, 2018 at 1:18 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>
> 
> >> As the CPG/MSSR driver now has suspend/resume support, do we need
> >> a notifier to restore the Z or Z2 registers? Or is that handled automatically
> >> by cpufreq during system resume, for both the primary and the secondary
> >> CPU cores?
> >
> > I am a bit unsure.
> >
> > When using the A57 cores, which is the default case, the Z clk is queried
> > by CPUFreq on resume. It appears that on my system its already set to the
> > correct value but I assume if it was not then it would be reset. However,
> > this does not cover Z2 clk. So perhaps to be safe we need to register
> > notifiers and make sure they they play nicely with CPUFreq?
> 
> Of course the CPU is special: unlike many other devices, it must be running
> when the kernel is reentered upon system resume.
> It may be running using a different frequency setting, though.
> However, following "opp-suspend", the system will always suspend with the
> Z clock running at 1.5GHz, which is the default?
> So Z is probably OK.
> 
> It's more interesting to check what happens when the little cores are
> enabled as well (unfortunately that requires different firmware).
> 1. Does cpufreq handle them correctly when they are onlined again during
>    system resume?

I tested this by updating the firmware on an H3 ES2.0 / Salvator-XS
using the instructions at
https://elinux.org/R-Car/Virtualization#Enabling_HYP_Support

I also fleshed out the opts table for the little CPUs using the following
from the 3.6.0 BSP:

diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
index 1485e6a8e112..671c2d7ed6d0 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
@@ -219,7 +219,17 @@
 		compatible = "operating-points-v2";
 		opp-shared;
 
-		opp-1200000000 {
+		opp at 800000000 {
+			opp-hz = /bits/ 64 <800000000>;
+			opp-microvolt = <820000>;
+			clock-latency-ns = <300000>;
+		};
+		opp at 1000000000 {
+			opp-hz = /bits/ 64 <1000000000>;
+			opp-microvolt = <820000>;
+			clock-latency-ns = <300000>;
+		};
+		opp at 1200000000 {
 			opp-hz = /bits/ 64 <1200000000>;
 			opp-microvolt = <820000>;
 			clock-latency-ns = <300000>;

And fixed the out-by-one bug you pointed out elsewhere in this thread:

diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c b/drivers/clk/renesas/rcar-gen3-cpg.c
index a7d68cefda9c..0c8fe10d57fe 100644
--- a/drivers/clk/renesas/rcar-gen3-cpg.c
+++ b/drivers/clk/renesas/rcar-gen3-cpg.c
@@ -94,7 +94,7 @@ static unsigned long cpg_z_clk_recalc_rate(struct clk_hw *hw,
 	u32 val;
 
 	val = clk_readl(zclk->reg) & zclk->mask;
-	mult = 32 - (val >> (__ffs(zclk->mask) - 1));
+	mult = 32 - (val >> __ffs(zclk->mask));
 
 	/* Factor of 2 is for fixed divider */
 	return DIV_ROUND_CLOSEST_ULL((u64)parent_rate * mult, 32 * 2);
@@ -129,7 +129,7 @@ static int cpg_z_clk_set_rate(struct clk_hw *hw, unsigned long rate,
 		return -EBUSY;
 
 	val = clk_readl(zclk->reg) & ~zclk->mask;
-	val |= ((32 - mult) << (__ffs(zclk->mask) - 1)) & zclk->mask;
+	val |= ((32 - mult) << __ffs(zclk->mask)) & zclk->mask;
 	clk_writel(val, zclk->reg);
 
 	/*

With these changes in place CPUFreq seems to work for both BIG and little
CPUS. And the frequencies seem to be correct after S2RAM.

# cd /sys/devices/system/cpu/
# grep . cpu*/cpufreq/scaling_available_frequencies
cpu0/cpufreq/scaling_available_frequencies:500000 1000000 1500000 
...
cpu4/cpufreq/scaling_available_frequencies:800000 1000000 1200000 
...
# grep grep -E -w "pll[01]|z|z2" /sys/kernel/debug/clk/clk_summary
grep: pll[01]|z|z2: No such file or directory
# grep -E -w "pll[01]|z|z2" /sys/kernel/debug/clk/clk_summary
          z2                              0            0  1198080000          0 0  
       .pll1                              1            1  3194880000          0 0  
       .pll0                              0            0  2995200000          0 0  
          z                               0            0  1497600000          0 0  
# echo 500000 > cpu0/cpufreq/scaling_max_freq
# echo 800000 > cpu4/cpufreq/scaling_max_freq
# grep . cpu*/cpufreq/*cur*
cpu0/cpufreq/cpuinfo_cur_freq:468000
cpu0/cpufreq/scaling_cur_freq:500000
...
cpu4/cpufreq/cpuinfo_cur_freq:786240
cpu4/cpufreq/scaling_cur_freq:786240
...
# grep -E -w "pll[01]|z|z2" /sys/kernel/debug/clk/clk_summary
          z2                              0            0   786240000          0 0  
       .pll1                              1            1  3194880000          0 0  
       .pll0                              0            0  2995200000          0 0  
          z                               0            0   468000000          0 0  
# i2cset -f -y 7 0x30 0x20 0x0F
# echo mem > /sys/power/state
...
[  203.289585] Restarting tasks ... done.
[  203.299250] PM: suspend exit
# grep . cpu*/cpufreq/*cur*
cpu0/cpufreq/cpuinfo_cur_freq:468000
cpu0/cpufreq/scaling_cur_freq:500000
...
cpu4/cpufreq/cpuinfo_cur_freq:786240
cpu4/cpufreq/scaling_cur_freq:800000
...
# grep -E -w "pll[01]|z|z2" /sys/kernel/debug/clk/clk_summary
          z2                              0            0   786240000          0 0  
       .pll1                              1            1  3194880000          0 0  
       .pll0                              0            0  2995200000          0 0  
          z                               0            0   468000000          0 0  

> 2. What happens if you offline all big cores, and enter system suspend
>    running with little cores only?
>    Perhaps that's prevented by the "opp-suspend" property as well?

I am unable to take CPU0 offline.

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

[Geert Uytterhoeven]

Hi Simon,

On Mon, Jan 29, 2018 at 3:24 PM, Simon Horman <horms@verge.net.au> wrote:
> On Fri, Jan 05, 2018 at 03:35:13PM +0100, Geert Uytterhoeven wrote:
>> On Fri, Jan 5, 2018 at 3:04 PM, Simon Horman <horms@verge.net.au> wrote:
>> > On Wed, Jan 03, 2018 at 01:47:08PM +0100, Geert Uytterhoeven wrote:
>> >> On Wed, Jan 3, 2018 at 1:18 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>
>>
>> >> As the CPG/MSSR driver now has suspend/resume support, do we need
>> >> a notifier to restore the Z or Z2 registers? Or is that handled automatically
>> >> by cpufreq during system resume, for both the primary and the secondary
>> >> CPU cores?
>> >
>> > I am a bit unsure.
>> >
>> > When using the A57 cores, which is the default case, the Z clk is queried
>> > by CPUFreq on resume. It appears that on my system its already set to the
>> > correct value but I assume if it was not then it would be reset. However,
>> > this does not cover Z2 clk. So perhaps to be safe we need to register
>> > notifiers and make sure they they play nicely with CPUFreq?
>>
>> Of course the CPU is special: unlike many other devices, it must be running
>> when the kernel is reentered upon system resume.
>> It may be running using a different frequency setting, though.
>> However, following "opp-suspend", the system will always suspend with the
>> Z clock running at 1.5GHz, which is the default?
>> So Z is probably OK.
>>
>> It's more interesting to check what happens when the little cores are
>> enabled as well (unfortunately that requires different firmware).
>> 1. Does cpufreq handle them correctly when they are onlined again during
>>    system resume?
>
> I tested this by updating the firmware on an H3 ES2.0 / Salvator-XS
> using the instructions at
> https://elinux.org/R-Car/Virtualization#Enabling_HYP_Support

> # grep -E -w "pll[01]|z|z2" /sys/kernel/debug/clk/clk_summary
>           z2                              0            0  1198080000          0 0
>        .pll1                              1            1  3194880000          0 0
>        .pll0                              0            0  2995200000          0 0
>           z                               0            0  1497600000          0 0

You know it's PLL2, not PLL1, you want to look at? ;-)

Thanks for checking, looks all good!

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

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

[Geert Uytterhoeven]

Hi Simon,

On Mon, Jan 29, 2018 at 3:24 PM, Simon Horman <horms@verge.net.au> wrote:
> On Fri, Jan 05, 2018 at 03:35:13PM +0100, Geert Uytterhoeven wrote:
>> On Fri, Jan 5, 2018 at 3:04 PM, Simon Horman <horms@verge.net.au> wrote:
>> > On Wed, Jan 03, 2018 at 01:47:08PM +0100, Geert Uytterhoeven wrote:
>> >> On Wed, Jan 3, 2018 at 1:18 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>
>>
>> >> As the CPG/MSSR driver now has suspend/resume support, do we need
>> >> a notifier to restore the Z or Z2 registers? Or is that handled automatically
>> >> by cpufreq during system resume, for both the primary and the secondary
>> >> CPU cores?
>> >
>> > I am a bit unsure.
>> >
>> > When using the A57 cores, which is the default case, the Z clk is queried
>> > by CPUFreq on resume. It appears that on my system its already set to the
>> > correct value but I assume if it was not then it would be reset. However,
>> > this does not cover Z2 clk. So perhaps to be safe we need to register
>> > notifiers and make sure they they play nicely with CPUFreq?
>>
>> Of course the CPU is special: unlike many other devices, it must be running
>> when the kernel is reentered upon system resume.
>> It may be running using a different frequency setting, though.
>> However, following "opp-suspend", the system will always suspend with the
>> Z clock running at 1.5GHz, which is the default?
>> So Z is probably OK.
>>
>> It's more interesting to check what happens when the little cores are
>> enabled as well (unfortunately that requires different firmware).
>> 1. Does cpufreq handle them correctly when they are onlined again during
>>    system resume?
>
> I tested this by updating the firmware on an H3 ES2.0 / Salvator-XS
> using the instructions at
> https://elinux.org/R-Car/Virtualization#Enabling_HYP_Support

> # grep -E -w "pll[01]|z|z2" /sys/kernel/debug/clk/clk_summary
>           z2                              0            0  1198080000          0 0
>        .pll1                              1            1  3194880000          0 0
>        .pll0                              0            0  2995200000          0 0
>           z                               0            0  1497600000          0 0

You know it's PLL2, not PLL1, you want to look at? ;-)

Thanks for checking, looks all good!

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

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

[Simon Horman]

On Mon, Jan 29, 2018 at 04:01:49PM +0100, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Mon, Jan 29, 2018 at 3:24 PM, Simon Horman <horms@verge.net.au> wrote:
> > On Fri, Jan 05, 2018 at 03:35:13PM +0100, Geert Uytterhoeven wrote:
> >> On Fri, Jan 5, 2018 at 3:04 PM, Simon Horman <horms@verge.net.au> wrote:
> >> > On Wed, Jan 03, 2018 at 01:47:08PM +0100, Geert Uytterhoeven wrote:
> >> >> On Wed, Jan 3, 2018 at 1:18 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>
> >>
> >> >> As the CPG/MSSR driver now has suspend/resume support, do we need
> >> >> a notifier to restore the Z or Z2 registers? Or is that handled automatically
> >> >> by cpufreq during system resume, for both the primary and the secondary
> >> >> CPU cores?
> >> >
> >> > I am a bit unsure.
> >> >
> >> > When using the A57 cores, which is the default case, the Z clk is queried
> >> > by CPUFreq on resume. It appears that on my system its already set to the
> >> > correct value but I assume if it was not then it would be reset. However,
> >> > this does not cover Z2 clk. So perhaps to be safe we need to register
> >> > notifiers and make sure they they play nicely with CPUFreq?
> >>
> >> Of course the CPU is special: unlike many other devices, it must be running
> >> when the kernel is reentered upon system resume.
> >> It may be running using a different frequency setting, though.
> >> However, following "opp-suspend", the system will always suspend with the
> >> Z clock running at 1.5GHz, which is the default?
> >> So Z is probably OK.
> >>
> >> It's more interesting to check what happens when the little cores are
> >> enabled as well (unfortunately that requires different firmware).
> >> 1. Does cpufreq handle them correctly when they are onlined again during
> >>    system resume?
> >
> > I tested this by updating the firmware on an H3 ES2.0 / Salvator-XS
> > using the instructions at
> > https://elinux.org/R-Car/Virtualization#Enabling_HYP_Support
> 
> > # grep -E -w "pll[01]|z|z2" /sys/kernel/debug/clk/clk_summary
> >           z2                              0            0  1198080000          0 0
> >        .pll1                              1            1  3194880000          0 0
> >        .pll0                              0            0  2995200000          0 0
> >           z                               0            0  1497600000          0 0
> 
> You know it's PLL2, not PLL1, you want to look at? ;-)

Sorry, I will double check but I had looked at PLL2 earlier:
I just messed things up when preparing things to post in email.

> Thanks for checking, looks all good!

Ok, so do you think we can merge this series with
the off-by-one problem fixed?

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

[Simon Horman]

On Mon, Jan 29, 2018 at 04:01:49PM +0100, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Mon, Jan 29, 2018 at 3:24 PM, Simon Horman <horms@verge.net.au> wrote:
> > On Fri, Jan 05, 2018 at 03:35:13PM +0100, Geert Uytterhoeven wrote:
> >> On Fri, Jan 5, 2018 at 3:04 PM, Simon Horman <horms@verge.net.au> wrote:
> >> > On Wed, Jan 03, 2018 at 01:47:08PM +0100, Geert Uytterhoeven wrote:
> >> >> On Wed, Jan 3, 2018 at 1:18 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>
> >>
> >> >> As the CPG/MSSR driver now has suspend/resume support, do we need
> >> >> a notifier to restore the Z or Z2 registers? Or is that handled automatically
> >> >> by cpufreq during system resume, for both the primary and the secondary
> >> >> CPU cores?
> >> >
> >> > I am a bit unsure.
> >> >
> >> > When using the A57 cores, which is the default case, the Z clk is queried
> >> > by CPUFreq on resume. It appears that on my system its already set to the
> >> > correct value but I assume if it was not then it would be reset. However,
> >> > this does not cover Z2 clk. So perhaps to be safe we need to register
> >> > notifiers and make sure they they play nicely with CPUFreq?
> >>
> >> Of course the CPU is special: unlike many other devices, it must be running
> >> when the kernel is reentered upon system resume.
> >> It may be running using a different frequency setting, though.
> >> However, following "opp-suspend", the system will always suspend with the
> >> Z clock running at 1.5GHz, which is the default?
> >> So Z is probably OK.
> >>
> >> It's more interesting to check what happens when the little cores are
> >> enabled as well (unfortunately that requires different firmware).
> >> 1. Does cpufreq handle them correctly when they are onlined again during
> >>    system resume?
> >
> > I tested this by updating the firmware on an H3 ES2.0 / Salvator-XS
> > using the instructions at
> > https://elinux.org/R-Car/Virtualization#Enabling_HYP_Support
> 
> > # grep -E -w "pll[01]|z|z2" /sys/kernel/debug/clk/clk_summary
> >           z2                              0            0  1198080000          0 0
> >        .pll1                              1            1  3194880000          0 0
> >        .pll0                              0            0  2995200000          0 0
> >           z                               0            0  1497600000          0 0
> 
> You know it's PLL2, not PLL1, you want to look at? ;-)

Sorry, I will double check but I had looked at PLL2 earlier:
I just messed things up when preparing things to post in email.

> Thanks for checking, looks all good!

Ok, so do you think we can merge this series with
the off-by-one problem fixed?

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

[Geert Uytterhoeven]

Hi Simon,

On Mon, Jan 29, 2018 at 4:19 PM, Simon Horman <horms@verge.net.au> wrote:
> On Mon, Jan 29, 2018 at 04:01:49PM +0100, Geert Uytterhoeven wrote:
>> On Mon, Jan 29, 2018 at 3:24 PM, Simon Horman <horms@verge.net.au> wrote:
>> > On Fri, Jan 05, 2018 at 03:35:13PM +0100, Geert Uytterhoeven wrote:
>> >> On Fri, Jan 5, 2018 at 3:04 PM, Simon Horman <horms@verge.net.au> wrote:
>> >> > On Wed, Jan 03, 2018 at 01:47:08PM +0100, Geert Uytterhoeven wrote:
>> >> >> On Wed, Jan 3, 2018 at 1:18 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>
>> >>
>> >> >> As the CPG/MSSR driver now has suspend/resume support, do we need
>> >> >> a notifier to restore the Z or Z2 registers? Or is that handled automatically
>> >> >> by cpufreq during system resume, for both the primary and the secondary
>> >> >> CPU cores?
>> >> >
>> >> > I am a bit unsure.
>> >> >
>> >> > When using the A57 cores, which is the default case, the Z clk is queried
>> >> > by CPUFreq on resume. It appears that on my system its already set to the
>> >> > correct value but I assume if it was not then it would be reset. However,
>> >> > this does not cover Z2 clk. So perhaps to be safe we need to register
>> >> > notifiers and make sure they they play nicely with CPUFreq?
>> >>
>> >> Of course the CPU is special: unlike many other devices, it must be running
>> >> when the kernel is reentered upon system resume.
>> >> It may be running using a different frequency setting, though.
>> >> However, following "opp-suspend", the system will always suspend with the
>> >> Z clock running at 1.5GHz, which is the default?
>> >> So Z is probably OK.
>> >>
>> >> It's more interesting to check what happens when the little cores are
>> >> enabled as well (unfortunately that requires different firmware).
>> >> 1. Does cpufreq handle them correctly when they are onlined again during
>> >>    system resume?
>> >
>> > I tested this by updating the firmware on an H3 ES2.0 / Salvator-XS
>> > using the instructions at
>> > https://elinux.org/R-Car/Virtualization#Enabling_HYP_Support
>>
>> > # grep -E -w "pll[01]|z|z2" /sys/kernel/debug/clk/clk_summary
>> >           z2                              0            0  1198080000          0 0
>> >        .pll1                              1            1  3194880000          0 0
>> >        .pll0                              0            0  2995200000          0 0
>> >           z                               0            0  1497600000          0 0
>>
>> You know it's PLL2, not PLL1, you want to look at? ;-)
>
> Sorry, I will double check but I had looked at PLL2 earlier:
> I just messed things up when preparing things to post in email.
>
>> Thanks for checking, looks all good!
>
> Ok, so do you think we can merge this series with
> the off-by-one problem fixed?

Yes we can. Thanks!

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

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

[Geert Uytterhoeven]

Hi Simon,

On Mon, Jan 29, 2018 at 4:19 PM, Simon Horman <horms@verge.net.au> wrote:
> On Mon, Jan 29, 2018 at 04:01:49PM +0100, Geert Uytterhoeven wrote:
>> On Mon, Jan 29, 2018 at 3:24 PM, Simon Horman <horms@verge.net.au> wrote:
>> > On Fri, Jan 05, 2018 at 03:35:13PM +0100, Geert Uytterhoeven wrote:
>> >> On Fri, Jan 5, 2018 at 3:04 PM, Simon Horman <horms@verge.net.au> wrote:
>> >> > On Wed, Jan 03, 2018 at 01:47:08PM +0100, Geert Uytterhoeven wrote:
>> >> >> On Wed, Jan 3, 2018 at 1:18 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>
>> >>
>> >> >> As the CPG/MSSR driver now has suspend/resume support, do we need
>> >> >> a notifier to restore the Z or Z2 registers? Or is that handled automatically
>> >> >> by cpufreq during system resume, for both the primary and the secondary
>> >> >> CPU cores?
>> >> >
>> >> > I am a bit unsure.
>> >> >
>> >> > When using the A57 cores, which is the default case, the Z clk is queried
>> >> > by CPUFreq on resume. It appears that on my system its already set to the
>> >> > correct value but I assume if it was not then it would be reset. However,
>> >> > this does not cover Z2 clk. So perhaps to be safe we need to register
>> >> > notifiers and make sure they they play nicely with CPUFreq?
>> >>
>> >> Of course the CPU is special: unlike many other devices, it must be running
>> >> when the kernel is reentered upon system resume.
>> >> It may be running using a different frequency setting, though.
>> >> However, following "opp-suspend", the system will always suspend with the
>> >> Z clock running at 1.5GHz, which is the default?
>> >> So Z is probably OK.
>> >>
>> >> It's more interesting to check what happens when the little cores are
>> >> enabled as well (unfortunately that requires different firmware).
>> >> 1. Does cpufreq handle them correctly when they are onlined again during
>> >>    system resume?
>> >
>> > I tested this by updating the firmware on an H3 ES2.0 / Salvator-XS
>> > using the instructions at
>> > https://elinux.org/R-Car/Virtualization#Enabling_HYP_Support
>>
>> > # grep -E -w "pll[01]|z|z2" /sys/kernel/debug/clk/clk_summary
>> >           z2                              0            0  1198080000          0 0
>> >        .pll1                              1            1  3194880000          0 0
>> >        .pll0                              0            0  2995200000          0 0
>> >           z                               0            0  1497600000          0 0
>>
>> You know it's PLL2, not PLL1, you want to look at? ;-)
>
> Sorry, I will double check but I had looked at PLL2 earlier:
> I just messed things up when preparing things to post in email.
>
>> Thanks for checking, looks all good!
>
> Ok, so do you think we can merge this series with
> the off-by-one problem fixed?

Yes we can. Thanks!

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

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

[Simon Horman]

On Mon, Jan 29, 2018 at 04:19:29PM +0100, Simon Horman wrote:
> On Mon, Jan 29, 2018 at 04:01:49PM +0100, Geert Uytterhoeven wrote:
> > Hi Simon,
> > 
> > On Mon, Jan 29, 2018 at 3:24 PM, Simon Horman <horms@verge.net.au> wrote:
> > > On Fri, Jan 05, 2018 at 03:35:13PM +0100, Geert Uytterhoeven wrote:
> > >> On Fri, Jan 5, 2018 at 3:04 PM, Simon Horman <horms@verge.net.au> wrote:
> > >> > On Wed, Jan 03, 2018 at 01:47:08PM +0100, Geert Uytterhoeven wrote:
> > >> >> On Wed, Jan 3, 2018 at 1:18 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>
> > >>
> > >> >> As the CPG/MSSR driver now has suspend/resume support, do we need
> > >> >> a notifier to restore the Z or Z2 registers? Or is that handled automatically
> > >> >> by cpufreq during system resume, for both the primary and the secondary
> > >> >> CPU cores?
> > >> >
> > >> > I am a bit unsure.
> > >> >
> > >> > When using the A57 cores, which is the default case, the Z clk is queried
> > >> > by CPUFreq on resume. It appears that on my system its already set to the
> > >> > correct value but I assume if it was not then it would be reset. However,
> > >> > this does not cover Z2 clk. So perhaps to be safe we need to register
> > >> > notifiers and make sure they they play nicely with CPUFreq?
> > >>
> > >> Of course the CPU is special: unlike many other devices, it must be running
> > >> when the kernel is reentered upon system resume.
> > >> It may be running using a different frequency setting, though.
> > >> However, following "opp-suspend", the system will always suspend with the
> > >> Z clock running at 1.5GHz, which is the default?
> > >> So Z is probably OK.
> > >>
> > >> It's more interesting to check what happens when the little cores are
> > >> enabled as well (unfortunately that requires different firmware).
> > >> 1. Does cpufreq handle them correctly when they are onlined again during
> > >>    system resume?
> > >
> > > I tested this by updating the firmware on an H3 ES2.0 / Salvator-XS
> > > using the instructions at
> > > https://elinux.org/R-Car/Virtualization#Enabling_HYP_Support
> > 
> > > # grep -E -w "pll[01]|z|z2" /sys/kernel/debug/clk/clk_summary
> > >           z2                              0            0  1198080000          0 0
> > >        .pll1                              1            1  3194880000          0 0
> > >        .pll0                              0            0  2995200000          0 0
> > >           z                               0            0  1497600000          0 0
> > 
> > You know it's PLL2, not PLL1, you want to look at? ;-)
> 
> Sorry, I will double check but I had looked at PLL2 earlier:
> I just messed things up when preparing things to post in email.

I checked, PLL2 is constant at 2396160000 across the test described
in my earlier email.

> > Thanks for checking, looks all good!
> 
> Ok, so do you think we can merge this series with
> the off-by-one problem fixed?
> 

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

[Simon Horman]

On Mon, Jan 29, 2018 at 04:19:29PM +0100, Simon Horman wrote:
> On Mon, Jan 29, 2018 at 04:01:49PM +0100, Geert Uytterhoeven wrote:
> > Hi Simon,
> > 
> > On Mon, Jan 29, 2018 at 3:24 PM, Simon Horman <horms@verge.net.au> wrote:
> > > On Fri, Jan 05, 2018 at 03:35:13PM +0100, Geert Uytterhoeven wrote:
> > >> On Fri, Jan 5, 2018 at 3:04 PM, Simon Horman <horms@verge.net.au> wrote:
> > >> > On Wed, Jan 03, 2018 at 01:47:08PM +0100, Geert Uytterhoeven wrote:
> > >> >> On Wed, Jan 3, 2018 at 1:18 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>
> > >>
> > >> >> As the CPG/MSSR driver now has suspend/resume support, do we need
> > >> >> a notifier to restore the Z or Z2 registers? Or is that handled automatically
> > >> >> by cpufreq during system resume, for both the primary and the secondary
> > >> >> CPU cores?
> > >> >
> > >> > I am a bit unsure.
> > >> >
> > >> > When using the A57 cores, which is the default case, the Z clk is queried
> > >> > by CPUFreq on resume. It appears that on my system its already set to the
> > >> > correct value but I assume if it was not then it would be reset. However,
> > >> > this does not cover Z2 clk. So perhaps to be safe we need to register
> > >> > notifiers and make sure they they play nicely with CPUFreq?
> > >>
> > >> Of course the CPU is special: unlike many other devices, it must be running
> > >> when the kernel is reentered upon system resume.
> > >> It may be running using a different frequency setting, though.
> > >> However, following "opp-suspend", the system will always suspend with the
> > >> Z clock running at 1.5GHz, which is the default?
> > >> So Z is probably OK.
> > >>
> > >> It's more interesting to check what happens when the little cores are
> > >> enabled as well (unfortunately that requires different firmware).
> > >> 1. Does cpufreq handle them correctly when they are onlined again during
> > >>    system resume?
> > >
> > > I tested this by updating the firmware on an H3 ES2.0 / Salvator-XS
> > > using the instructions at
> > > https://elinux.org/R-Car/Virtualization#Enabling_HYP_Support
> > 
> > > # grep -E -w "pll[01]|z|z2" /sys/kernel/debug/clk/clk_summary
> > >           z2                              0            0  1198080000          0 0
> > >        .pll1                              1            1  3194880000          0 0
> > >        .pll0                              0            0  2995200000          0 0
> > >           z                               0            0  1497600000          0 0
> > 
> > You know it's PLL2, not PLL1, you want to look at? ;-)
> 
> Sorry, I will double check but I had looked at PLL2 earlier:
> I just messed things up when preparing things to post in email.

I checked, PLL2 is constant at 2396160000 across the test described
in my earlier email.

> > Thanks for checking, looks all good!
> 
> Ok, so do you think we can merge this series with
> the off-by-one problem fixed?
> 

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

[Simon Horman]

On Mon, Jan 29, 2018 at 04:51:35PM +0100, Simon Horman wrote:
> On Mon, Jan 29, 2018 at 04:19:29PM +0100, Simon Horman wrote:
> > On Mon, Jan 29, 2018 at 04:01:49PM +0100, Geert Uytterhoeven wrote:
> > > Hi Simon,
> > > 
> > > On Mon, Jan 29, 2018 at 3:24 PM, Simon Horman <horms@verge.net.au> wrote:
> > > > On Fri, Jan 05, 2018 at 03:35:13PM +0100, Geert Uytterhoeven wrote:
> > > >> On Fri, Jan 5, 2018 at 3:04 PM, Simon Horman <horms@verge.net.au> wrote:
> > > >> > On Wed, Jan 03, 2018 at 01:47:08PM +0100, Geert Uytterhoeven wrote:
> > > >> >> On Wed, Jan 3, 2018 at 1:18 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>
> > > >>
> > > >> >> As the CPG/MSSR driver now has suspend/resume support, do we need
> > > >> >> a notifier to restore the Z or Z2 registers? Or is that handled automatically
> > > >> >> by cpufreq during system resume, for both the primary and the secondary
> > > >> >> CPU cores?
> > > >> >
> > > >> > I am a bit unsure.
> > > >> >
> > > >> > When using the A57 cores, which is the default case, the Z clk is queried
> > > >> > by CPUFreq on resume. It appears that on my system its already set to the
> > > >> > correct value but I assume if it was not then it would be reset. However,
> > > >> > this does not cover Z2 clk. So perhaps to be safe we need to register
> > > >> > notifiers and make sure they they play nicely with CPUFreq?
> > > >>
> > > >> Of course the CPU is special: unlike many other devices, it must be running
> > > >> when the kernel is reentered upon system resume.
> > > >> It may be running using a different frequency setting, though.
> > > >> However, following "opp-suspend", the system will always suspend with the
> > > >> Z clock running at 1.5GHz, which is the default?
> > > >> So Z is probably OK.
> > > >>
> > > >> It's more interesting to check what happens when the little cores are
> > > >> enabled as well (unfortunately that requires different firmware).
> > > >> 1. Does cpufreq handle them correctly when they are onlined again during
> > > >>    system resume?
> > > >
> > > > I tested this by updating the firmware on an H3 ES2.0 / Salvator-XS
> > > > using the instructions at
> > > > https://elinux.org/R-Car/Virtualization#Enabling_HYP_Support
> > > 
> > > > # grep -E -w "pll[01]|z|z2" /sys/kernel/debug/clk/clk_summary
> > > >           z2                              0            0  1198080000          0 0
> > > >        .pll1                              1            1  3194880000          0 0
> > > >        .pll0                              0            0  2995200000          0 0
> > > >           z                               0            0  1497600000          0 0
> > > 
> > > You know it's PLL2, not PLL1, you want to look at? ;-)
> > 
> > Sorry, I will double check but I had looked at PLL2 earlier:
> > I just messed things up when preparing things to post in email.
> 
> I checked, PLL2 is constant at 2396160000 across the test described
> in my earlier email.

I have now been able to reproduce an equivalent result on
an M3W ES1.0 / Salvator-X.

> > > Thanks for checking, looks all good!
> > 
> > Ok, so do you think we can merge this series with
> > the off-by-one problem fixed?

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

[Simon Horman]

On Mon, Jan 29, 2018 at 04:51:35PM +0100, Simon Horman wrote:
> On Mon, Jan 29, 2018 at 04:19:29PM +0100, Simon Horman wrote:
> > On Mon, Jan 29, 2018 at 04:01:49PM +0100, Geert Uytterhoeven wrote:
> > > Hi Simon,
> > > 
> > > On Mon, Jan 29, 2018 at 3:24 PM, Simon Horman <horms@verge.net.au> wrote:
> > > > On Fri, Jan 05, 2018 at 03:35:13PM +0100, Geert Uytterhoeven wrote:
> > > >> On Fri, Jan 5, 2018 at 3:04 PM, Simon Horman <horms@verge.net.au> wrote:
> > > >> > On Wed, Jan 03, 2018 at 01:47:08PM +0100, Geert Uytterhoeven wrote:
> > > >> >> On Wed, Jan 3, 2018 at 1:18 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>
> > > >>
> > > >> >> As the CPG/MSSR driver now has suspend/resume support, do we need
> > > >> >> a notifier to restore the Z or Z2 registers? Or is that handled automatically
> > > >> >> by cpufreq during system resume, for both the primary and the secondary
> > > >> >> CPU cores?
> > > >> >
> > > >> > I am a bit unsure.
> > > >> >
> > > >> > When using the A57 cores, which is the default case, the Z clk is queried
> > > >> > by CPUFreq on resume. It appears that on my system its already set to the
> > > >> > correct value but I assume if it was not then it would be reset. However,
> > > >> > this does not cover Z2 clk. So perhaps to be safe we need to register
> > > >> > notifiers and make sure they they play nicely with CPUFreq?
> > > >>
> > > >> Of course the CPU is special: unlike many other devices, it must be running
> > > >> when the kernel is reentered upon system resume.
> > > >> It may be running using a different frequency setting, though.
> > > >> However, following "opp-suspend", the system will always suspend with the
> > > >> Z clock running at 1.5GHz, which is the default?
> > > >> So Z is probably OK.
> > > >>
> > > >> It's more interesting to check what happens when the little cores are
> > > >> enabled as well (unfortunately that requires different firmware).
> > > >> 1. Does cpufreq handle them correctly when they are onlined again during
> > > >>    system resume?
> > > >
> > > > I tested this by updating the firmware on an H3 ES2.0 / Salvator-XS
> > > > using the instructions at
> > > > https://elinux.org/R-Car/Virtualization#Enabling_HYP_Support
> > > 
> > > > # grep -E -w "pll[01]|z|z2" /sys/kernel/debug/clk/clk_summary
> > > >           z2                              0            0  1198080000          0 0
> > > >        .pll1                              1            1  3194880000          0 0
> > > >        .pll0                              0            0  2995200000          0 0
> > > >           z                               0            0  1497600000          0 0
> > > 
> > > You know it's PLL2, not PLL1, you want to look at? ;-)
> > 
> > Sorry, I will double check but I had looked at PLL2 earlier:
> > I just messed things up when preparing things to post in email.
> 
> I checked, PLL2 is constant at 2396160000 across the test described
> in my earlier email.

I have now been able to reproduce an equivalent result on
an M3W ES1.0 / Salvator-X.

> > > Thanks for checking, looks all good!
> > 
> > Ok, so do you think we can merge this series with
> > the off-by-one problem fixed?

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

[Simon Horman]

On Mon, Jan 29, 2018 at 04:46:37PM +0100, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Mon, Jan 29, 2018 at 4:19 PM, Simon Horman <horms@verge.net.au> wrote:
> > On Mon, Jan 29, 2018 at 04:01:49PM +0100, Geert Uytterhoeven wrote:
> >> On Mon, Jan 29, 2018 at 3:24 PM, Simon Horman <horms@verge.net.au> wrote:
> >> > On Fri, Jan 05, 2018 at 03:35:13PM +0100, Geert Uytterhoeven wrote:
> >> >> On Fri, Jan 5, 2018 at 3:04 PM, Simon Horman <horms@verge.net.au> wrote:
> >> >> > On Wed, Jan 03, 2018 at 01:47:08PM +0100, Geert Uytterhoeven wrote:
> >> >> >> On Wed, Jan 3, 2018 at 1:18 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>
> >> >>
> >> >> >> As the CPG/MSSR driver now has suspend/resume support, do we need
> >> >> >> a notifier to restore the Z or Z2 registers? Or is that handled automatically
> >> >> >> by cpufreq during system resume, for both the primary and the secondary
> >> >> >> CPU cores?
> >> >> >
> >> >> > I am a bit unsure.
> >> >> >
> >> >> > When using the A57 cores, which is the default case, the Z clk is queried
> >> >> > by CPUFreq on resume. It appears that on my system its already set to the
> >> >> > correct value but I assume if it was not then it would be reset. However,
> >> >> > this does not cover Z2 clk. So perhaps to be safe we need to register
> >> >> > notifiers and make sure they they play nicely with CPUFreq?
> >> >>
> >> >> Of course the CPU is special: unlike many other devices, it must be running
> >> >> when the kernel is reentered upon system resume.
> >> >> It may be running using a different frequency setting, though.
> >> >> However, following "opp-suspend", the system will always suspend with the
> >> >> Z clock running at 1.5GHz, which is the default?
> >> >> So Z is probably OK.
> >> >>
> >> >> It's more interesting to check what happens when the little cores are
> >> >> enabled as well (unfortunately that requires different firmware).
> >> >> 1. Does cpufreq handle them correctly when they are onlined again during
> >> >>    system resume?
> >> >
> >> > I tested this by updating the firmware on an H3 ES2.0 / Salvator-XS
> >> > using the instructions at
> >> > https://elinux.org/R-Car/Virtualization#Enabling_HYP_Support
> >>
> >> > # grep -E -w "pll[01]|z|z2" /sys/kernel/debug/clk/clk_summary
> >> >           z2                              0            0  1198080000          0 0
> >> >        .pll1                              1            1  3194880000          0 0
> >> >        .pll0                              0            0  2995200000          0 0
> >> >           z                               0            0  1497600000          0 0
> >>
> >> You know it's PLL2, not PLL1, you want to look at? ;-)
> >
> > Sorry, I will double check but I had looked at PLL2 earlier:
> > I just messed things up when preparing things to post in email.
> >
> >> Thanks for checking, looks all good!
> >
> > Ok, so do you think we can merge this series with
> > the off-by-one problem fixed?
> 
> Yes we can. Thanks!

Excellent.

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

[Simon Horman]

On Mon, Jan 29, 2018 at 04:46:37PM +0100, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Mon, Jan 29, 2018 at 4:19 PM, Simon Horman <horms@verge.net.au> wrote:
> > On Mon, Jan 29, 2018 at 04:01:49PM +0100, Geert Uytterhoeven wrote:
> >> On Mon, Jan 29, 2018 at 3:24 PM, Simon Horman <horms@verge.net.au> wrote:
> >> > On Fri, Jan 05, 2018 at 03:35:13PM +0100, Geert Uytterhoeven wrote:
> >> >> On Fri, Jan 5, 2018 at 3:04 PM, Simon Horman <horms@verge.net.au> wrote:
> >> >> > On Wed, Jan 03, 2018 at 01:47:08PM +0100, Geert Uytterhoeven wrote:
> >> >> >> On Wed, Jan 3, 2018 at 1:18 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>
> >> >>
> >> >> >> As the CPG/MSSR driver now has suspend/resume support, do we need
> >> >> >> a notifier to restore the Z or Z2 registers? Or is that handled automatically
> >> >> >> by cpufreq during system resume, for both the primary and the secondary
> >> >> >> CPU cores?
> >> >> >
> >> >> > I am a bit unsure.
> >> >> >
> >> >> > When using the A57 cores, which is the default case, the Z clk is queried
> >> >> > by CPUFreq on resume. It appears that on my system its already set to the
> >> >> > correct value but I assume if it was not then it would be reset. However,
> >> >> > this does not cover Z2 clk. So perhaps to be safe we need to register
> >> >> > notifiers and make sure they they play nicely with CPUFreq?
> >> >>
> >> >> Of course the CPU is special: unlike many other devices, it must be running
> >> >> when the kernel is reentered upon system resume.
> >> >> It may be running using a different frequency setting, though.
> >> >> However, following "opp-suspend", the system will always suspend with the
> >> >> Z clock running at 1.5GHz, which is the default?
> >> >> So Z is probably OK.
> >> >>
> >> >> It's more interesting to check what happens when the little cores are
> >> >> enabled as well (unfortunately that requires different firmware).
> >> >> 1. Does cpufreq handle them correctly when they are onlined again during
> >> >>    system resume?
> >> >
> >> > I tested this by updating the firmware on an H3 ES2.0 / Salvator-XS
> >> > using the instructions at
> >> > https://elinux.org/R-Car/Virtualization#Enabling_HYP_Support
> >>
> >> > # grep -E -w "pll[01]|z|z2" /sys/kernel/debug/clk/clk_summary
> >> >           z2                              0            0  1198080000          0 0
> >> >        .pll1                              1            1  3194880000          0 0
> >> >        .pll0                              0            0  2995200000          0 0
> >> >           z                               0            0  1497600000          0 0
> >>
> >> You know it's PLL2, not PLL1, you want to look at? ;-)
> >
> > Sorry, I will double check but I had looked at PLL2 earlier:
> > I just messed things up when preparing things to post in email.
> >
> >> Thanks for checking, looks all good!
> >
> > Ok, so do you think we can merge this series with
> > the off-by-one problem fixed?
> 
> Yes we can. Thanks!

Excellent.