[PATCH V2 0/4] clk: mvebu: Improve clock drift

[PATCH V2 0/4] clk: mvebu: Improve clock drift

[Gregory CLEMENT]

Hi Mike, Jason, Andrew and Sebastian,

Few users reported a timer drift on the Armada 370 based board such as
the mirabox or the Netgear ReadyNAS 102. This is the second series
with few improvements after the review of the 1st version.

The reason is that when the SSCG (Spread Spectrum Clock Generator) is
enabled, it shifts the frequency of the clock. The percentage is no
more than 1% but when the clock is used for a timer it leads to a
clock drift.

This series allows to correct the affected clock when the SSCG is
enabled. This drift can happen on all the mvebu SoC on the cpu clock
block (ie cpu, ddr and l2 cache). Currently the only notable effect is
for the Armada 370 because this SoC use the l2cache clock as source
for the timer. That's why even if the series allow any of the mvebu
SoC to benefit to this correction, Armada 370 is the only user of it.

The first 2 patches should go through the clk subsystem, whereas the
third one should go to the arm-soc through the mvebu tree.

The last one is just to fix a typo I found while I was reading the clk
code.

Thanks,

Gregory

Changelog:

v1 -> v2:
- made the function a370_is_sscg_enabled() a static one
- reword the comment to make clear that the formula in datasheet is
  erroneous
- added a fix_sscg_deviation() callback in order to be able to deal
  with the Dove case which is different from the other mvebu SoCs


Gregory CLEMENT (4):
  clk: mvebu: Fix clk frequency value if SSCG is enabled
  clk: mvebu: armada-370: Fix timer drift caused by the SSCG deviation
  ARM: mvebu: add SSCG to Armada 370 Device Tree
  clk: mvebu: armada-375: Fix the description of the SAR in the comment

 arch/arm/boot/dts/armada-370.dtsi |  4 ++
 drivers/clk/mvebu/armada-370.c    |  8 ++++
 drivers/clk/mvebu/armada-375.c    |  4 +-
 drivers/clk/mvebu/common.c        | 82 +++++++++++++++++++++++++++++++++++++++
 drivers/clk/mvebu/common.h        |  7 ++++
 5 files changed, 103 insertions(+), 2 deletions(-)

-- 
1.9.1

[PATCH V2 1/4] clk: mvebu: Fix clk frequency value if SSCG is enabled

[Gregory CLEMENT]

When the SSCG (Spread Spectrum Clock Generator) is enabled, it shifts
the frequency of the clock. The percentage is no more than 1% but when
the clock is used for a timer it leads to a clock drift.

This patch allows to correct the affected clock when the SSCG is
enabled. The check is done in an new optional function related to each
SoC: is_sscg_enabled(). The fix is done with the other new optional
function related to each SoC: fix_sscg_deviation. If one these
functions are not present then no correction is done on the clock
frequency.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 drivers/clk/mvebu/common.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/clk/mvebu/common.h |  7 ++++
 2 files changed, 89 insertions(+)

diff --git a/drivers/clk/mvebu/common.c b/drivers/clk/mvebu/common.c
index 25ceccf939ad..354bbadb3bce 100644
--- a/drivers/clk/mvebu/common.c
+++ b/drivers/clk/mvebu/common.c
@@ -26,8 +26,85 @@
  * Core Clocks
  */
 
+#define SSCG_CONF_MODE(reg)	(((reg) >> 16) & 0x3)
+#define SSCG_SPREAD_DOWN	0x0
+#define SSCG_SPREAD_UP		0x1
+#define SSCG_SPREAD_CENTRAL	0x2
+#define SSCG_CONF_LOW(reg)	(((reg) >> 8) & 0xFF)
+#define SSCG_CONF_HIGH(reg)	((reg) & 0xFF)
+
 static struct clk_onecell_data clk_data;
 
+/*
+ * This function can be used by the Kirkwood, the Armada 370, the
+ * Armada XP and the Armada 375 SoC. The name of the function was
+ * chosen following the dt convention: using the first known SoC
+ * compatible with it.
+ */
+u32 kirkwood_fix_sscg_deviation(struct device_node *np, u32 system_clk)
+{
+	struct device_node *sscg_np = NULL;
+	void __iomem *sscg_map;
+	u32 sscg_reg;
+	s32 low_bound, high_bound;
+	u64 freq_swing_half;
+
+	sscg_np = of_find_node_by_name(np, "sscg");
+	if (sscg_np == NULL) {
+		pr_err("cannot get SSCG register node\n");
+		return system_clk;
+	}
+
+	sscg_map = of_iomap(sscg_np, 0);
+	if (sscg_map == NULL) {
+		pr_err("cannot map SSCG register\n");
+		goto out;
+	}
+
+	sscg_reg = readl(sscg_map);
+	high_bound = SSCG_CONF_HIGH(sscg_reg);
+	low_bound = SSCG_CONF_LOW(sscg_reg);
+
+	if ((high_bound - low_bound) <= 0)
+		goto out;
+	/*
+	 * From Marvell engineer we got the following formula (when
+	 * this code was written, the datasheet was erroneous)
+	 * Spread percentage = 1/96 * (H - L) / H
+	 * H = SSCG_High_Boundary
+	 * L = SSCG_Low_Boundary
+	 *
+	 * As the deviation is half of spread then it lead to the
+	 * following formula in the code.
+	 *
+	 * To avoid an overflow and not lose any significant digit in
+	 * the same time we have to use a 64 bit integer.
+	 */
+
+	freq_swing_half = (((u64)high_bound - (u64)low_bound)
+			* (u64)system_clk);
+	do_div(freq_swing_half, (2 * 96 * high_bound));
+
+	switch (SSCG_CONF_MODE(sscg_reg)) {
+	case SSCG_SPREAD_DOWN:
+		system_clk -= freq_swing_half;
+		break;
+	case SSCG_SPREAD_UP:
+		system_clk += freq_swing_half;
+		break;
+	case SSCG_SPREAD_CENTRAL:
+	default:
+		break;
+	}
+
+	iounmap(sscg_map);
+
+out:
+	of_node_put(sscg_np);
+
+	return system_clk;
+}
+
 void __init mvebu_coreclk_setup(struct device_node *np,
 				const struct coreclk_soc_desc *desc)
 {
@@ -62,6 +139,11 @@ void __init mvebu_coreclk_setup(struct device_node *np,
 	of_property_read_string_index(np, "clock-output-names", 1,
 				      &cpuclk_name);
 	rate = desc->get_cpu_freq(base);
+
+	if (desc->is_sscg_enabled && desc->fix_sscg_deviation
+		&& desc->is_sscg_enabled(base))
+		rate = desc->fix_sscg_deviation(np, rate);
+
 	clk_data.clks[1] = clk_register_fixed_rate(NULL, cpuclk_name, NULL,
 						   CLK_IS_ROOT, rate);
 	WARN_ON(IS_ERR(clk_data.clks[1]));
diff --git a/drivers/clk/mvebu/common.h b/drivers/clk/mvebu/common.h
index f968b4d9df92..59efaa850bde 100644
--- a/drivers/clk/mvebu/common.h
+++ b/drivers/clk/mvebu/common.h
@@ -28,6 +28,8 @@ struct coreclk_soc_desc {
 	u32 (*get_tclk_freq)(void __iomem *sar);
 	u32 (*get_cpu_freq)(void __iomem *sar);
 	void (*get_clk_ratio)(void __iomem *sar, int id, int *mult, int *div);
+	bool (*is_sscg_enabled)(void __iomem *sar);
+	u32 (*fix_sscg_deviation)(struct device_node *np, u32 system_clk);
 	const struct coreclk_ratio *ratios;
 	int num_ratios;
 };
@@ -45,4 +47,9 @@ void __init mvebu_coreclk_setup(struct device_node *np,
 void __init mvebu_clk_gating_setup(struct device_node *np,
 				   const struct clk_gating_soc_desc *desc);
 
+/*
+ * This function is shared among the Kirkwood, Armada 370, Armada XP
+ * and Armada 375 SoC
+ */
+u32 kirkwood_fix_sscg_deviation(struct device_node *np, u32 system_clk);
 #endif
-- 
1.9.1

[PATCH V2 2/4] clk: mvebu: armada-370: Fix timer drift caused by the SSCG deviation

[Gregory CLEMENT]

This commit activates the SSCG deviation correction for the Armada
370. It uses the optional function introduced by the commit "clk:
mvebu: Fix clk frequency value if SSCG is enabled".

Without this fix the deviation measured on a Mirabox was of a few
second each hour, whereas with this fix it was reduced at around
50ppm (around 4s per day).

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 drivers/clk/mvebu/armada-370.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/clk/mvebu/armada-370.c b/drivers/clk/mvebu/armada-370.c
index bef198a83863..756f0f39d6a3 100644
--- a/drivers/clk/mvebu/armada-370.c
+++ b/drivers/clk/mvebu/armada-370.c
@@ -23,6 +23,7 @@
  */
 
 #define SARL				0	/* Low part [0:31] */
+#define	 SARL_A370_SSCG_ENABLE		BIT(10)
 #define	 SARL_A370_PCLK_FREQ_OPT	11
 #define	 SARL_A370_PCLK_FREQ_OPT_MASK	0xF
 #define	 SARL_A370_FAB_FREQ_OPT		15
@@ -133,10 +134,17 @@ static void __init a370_get_clk_ratio(
 	}
 }
 
+static bool a370_is_sscg_enabled(void __iomem *sar)
+{
+	return !(readl(sar) & SARL_A370_SSCG_ENABLE);
+}
+
 static const struct coreclk_soc_desc a370_coreclks = {
 	.get_tclk_freq = a370_get_tclk_freq,
 	.get_cpu_freq = a370_get_cpu_freq,
 	.get_clk_ratio = a370_get_clk_ratio,
+	.is_sscg_enabled = a370_is_sscg_enabled,
+	.fix_sscg_deviation = kirkwood_fix_sscg_deviation,
 	.ratios = a370_coreclk_ratios,
 	.num_ratios = ARRAY_SIZE(a370_coreclk_ratios),
 };
-- 
1.9.1

[PATCH V2 3/4] ARM: mvebu: add SSCG to Armada 370 Device Tree

[Gregory CLEMENT]

The Armada 370 SoC has a Spread Spectrum Clock Generator. This commit
adds the description of this generator to the Device Tree describing
this SoC.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 arch/arm/boot/dts/armada-370.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/armada-370.dtsi b/arch/arm/boot/dts/armada-370.dtsi
index 21b588b6f6bd..dfbbb709f4fb 100644
--- a/arch/arm/boot/dts/armada-370.dtsi
+++ b/arch/arm/boot/dts/armada-370.dtsi
@@ -206,6 +206,10 @@
 				status = "okay";
 			};
 
+			sscg at 18330 {
+				reg = <0x18330 0x4>;
+			};
+
 			interrupt-controller at 20000 {
 				reg = <0x20a00 0x1d0>, <0x21870 0x58>;
 			};
-- 
1.9.1

[PATCH V2 4/4] clk: mvebu: armada-375: Fix the description of the SAR in the comment

[Gregory CLEMENT]

For dealing with the code we use the SAR1 and not the SAR0. The code
was correct, and now the comments too.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 drivers/clk/mvebu/armada-375.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/mvebu/armada-375.c b/drivers/clk/mvebu/armada-375.c
index c991a4d95e10..c7af2242b796 100644
--- a/drivers/clk/mvebu/armada-375.c
+++ b/drivers/clk/mvebu/armada-375.c
@@ -27,14 +27,14 @@
  * all modified at the same time, and not separately as for the Armada
  * 370 or the Armada XP SoCs.
  *
- * SAR0[21:17]   : CPU frequency    DDR frequency   L2 frequency
+ * SAR1[21:17]   : CPU frequency    DDR frequency   L2 frequency
  *		 6   =  400 MHz	    400 MHz	    200 MHz
  *		 15  =  600 MHz	    600 MHz	    300 MHz
  *		 21  =  800 MHz	    534 MHz	    400 MHz
  *		 25  = 1000 MHz	    500 MHz	    500 MHz
  *		 others reserved.
  *
- * SAR0[22]   : TCLK frequency
+ * SAR1[22]   : TCLK frequency
  *		 0 = 166 MHz
  *		 1 = 200 MHz
  */
-- 
1.9.1

[PATCH V2 0/4] clk: mvebu: Improve clock drift

[Leigh Brown]

On 2014-09-02 09:15, Gregory CLEMENT wrote:
> Few users reported a timer drift on the Armada 370 based board such as
> the mirabox or the Netgear ReadyNAS 102. This is the second series
> with few improvements after the review of the 1st version.
> 
> The reason is that when the SSCG (Spread Spectrum Clock Generator) is
> enabled, it shifts the frequency of the clock. The percentage is no
> more than 1% but when the clock is used for a timer it leads to a
> clock drift.
> 
> This series allows to correct the affected clock when the SSCG is
> enabled. This drift can happen on all the mvebu SoC on the cpu clock
> block (ie cpu, ddr and l2 cache). Currently the only notable effect is
> for the Armada 370 because this SoC use the l2cache clock as source
> for the timer. That's why even if the series allow any of the mvebu
> SoC to benefit to this correction, Armada 370 is the only user of it.
> 
> The first 2 patches should go through the clk subsystem, whereas the
> third one should go to the arm-soc through the mvebu tree.
> 
> The last one is just to fix a typo I found while I was reading the clk
> code.

This is working superbly for me on my Mirabox, and ntpd is now 
completely stable.

Tested-by: Leigh Brown <leigh@solinno.co.uk>

[PATCH V2 0/4] clk: mvebu: Improve clock drift

[Mike Turquette]

Quoting Leigh Brown (2014-09-03 07:40:56)
> On 2014-09-02 09:15, Gregory CLEMENT wrote:
> > Few users reported a timer drift on the Armada 370 based board such as
> > the mirabox or the Netgear ReadyNAS 102. This is the second series
> > with few improvements after the review of the 1st version.
> > 
> > The reason is that when the SSCG (Spread Spectrum Clock Generator) is
> > enabled, it shifts the frequency of the clock. The percentage is no
> > more than 1% but when the clock is used for a timer it leads to a
> > clock drift.
> > 
> > This series allows to correct the affected clock when the SSCG is
> > enabled. This drift can happen on all the mvebu SoC on the cpu clock
> > block (ie cpu, ddr and l2 cache). Currently the only notable effect is
> > for the Armada 370 because this SoC use the l2cache clock as source
> > for the timer. That's why even if the series allow any of the mvebu
> > SoC to benefit to this correction, Armada 370 is the only user of it.
> > 
> > The first 2 patches should go through the clk subsystem, whereas the
> > third one should go to the arm-soc through the mvebu tree.
> > 
> > The last one is just to fix a typo I found while I was reading the clk
> > code.
> 
> This is working superbly for me on my Mirabox, and ntpd is now 
> completely stable.
> 
> Tested-by: Leigh Brown <leigh@solinno.co.uk>
> 

Clock patches look good to me.

Regards,
Mike

[PATCH V2 0/4] clk: mvebu: Improve clock drift

[Jason Cooper]

On Wed, Sep 03, 2014 at 10:03:39AM -0700, Mike Turquette wrote:
> Quoting Leigh Brown (2014-09-03 07:40:56)
> > On 2014-09-02 09:15, Gregory CLEMENT wrote:
> > > Few users reported a timer drift on the Armada 370 based board such as
> > > the mirabox or the Netgear ReadyNAS 102. This is the second series
> > > with few improvements after the review of the 1st version.
> > > 
> > > The reason is that when the SSCG (Spread Spectrum Clock Generator) is
> > > enabled, it shifts the frequency of the clock. The percentage is no
> > > more than 1% but when the clock is used for a timer it leads to a
> > > clock drift.
> > > 
> > > This series allows to correct the affected clock when the SSCG is
> > > enabled. This drift can happen on all the mvebu SoC on the cpu clock
> > > block (ie cpu, ddr and l2 cache). Currently the only notable effect is
> > > for the Armada 370 because this SoC use the l2cache clock as source
> > > for the timer. That's why even if the series allow any of the mvebu
> > > SoC to benefit to this correction, Armada 370 is the only user of it.
> > > 
> > > The first 2 patches should go through the clk subsystem, whereas the
> > > third one should go to the arm-soc through the mvebu tree.
> > > 
> > > The last one is just to fix a typo I found while I was reading the clk
> > > code.
> > 
> > This is working superbly for me on my Mirabox, and ntpd is now 
> > completely stable.
> > 
> > Tested-by: Leigh Brown <leigh@solinno.co.uk>
> > 
> 
> Clock patches look good to me.

Ok, I'll pull it together into a clock branch and so forth.

thx,

Jason.

[PATCH V2 0/4] clk: mvebu: Improve clock drift

[Jason Cooper]

On Tue, Sep 02, 2014 at 10:15:15AM +0200, Gregory CLEMENT wrote:
> Hi Mike, Jason, Andrew and Sebastian,
> 
> Few users reported a timer drift on the Armada 370 based board such as
> the mirabox or the Netgear ReadyNAS 102. This is the second series
> with few improvements after the review of the 1st version.
> 
> The reason is that when the SSCG (Spread Spectrum Clock Generator) is
> enabled, it shifts the frequency of the clock. The percentage is no
> more than 1% but when the clock is used for a timer it leads to a
> clock drift.
> 
> This series allows to correct the affected clock when the SSCG is
> enabled. This drift can happen on all the mvebu SoC on the cpu clock
> block (ie cpu, ddr and l2 cache). Currently the only notable effect is
> for the Armada 370 because this SoC use the l2cache clock as source
> for the timer. That's why even if the series allow any of the mvebu
> SoC to benefit to this correction, Armada 370 is the only user of it.
> 
> The first 2 patches should go through the clk subsystem, whereas the
> third one should go to the arm-soc through the mvebu tree.
> 
> The last one is just to fix a typo I found while I was reading the clk
> code.
> 
> Thanks,
> 
> Gregory
> 
> Changelog:
> 
> v1 -> v2:
> - made the function a370_is_sscg_enabled() a static one
> - reword the comment to make clear that the formula in datasheet is
>   erroneous
> - added a fix_sscg_deviation() callback in order to be able to deal
>   with the Dove case which is different from the other mvebu SoCs
> 
> 
> Gregory CLEMENT (4):
>   clk: mvebu: Fix clk frequency value if SSCG is enabled
>   clk: mvebu: armada-370: Fix timer drift caused by the SSCG deviation
>   ARM: mvebu: add SSCG to Armada 370 Device Tree
>   clk: mvebu: armada-375: Fix the description of the SAR in the comment
> 
>  arch/arm/boot/dts/armada-370.dtsi |  4 ++
>  drivers/clk/mvebu/armada-370.c    |  8 ++++
>  drivers/clk/mvebu/armada-375.c    |  4 +-
>  drivers/clk/mvebu/common.c        | 82 +++++++++++++++++++++++++++++++++++++++
>  drivers/clk/mvebu/common.h        |  7 ++++
>  5 files changed, 103 insertions(+), 2 deletions(-)

Patches 1, 2, and 4 applied to mvebu/drivers-clk with Leigh's Tested-by.
#3 applied to mvebu/dt with same Tested-by.

thx,

Jason.