[PATCH 0/4] Make Armada 375 use the reference clock when possible

[PATCH 0/4] Make Armada 375 use the reference clock when possible

[Ezequiel Garcia]

This series adds support for the 25 MHz reference clock available on
Armada 375 SoC to use on the timer and watchdog drivers. It is
similar to the one present in Armada XP SoC.

Given we initially had access to only a very early SoC revision (A375 Z0)
and due to a hardware issue, the timer and watchdog support was originally
submitted to use the core clock.

Now that the A0 SoC revision is out, we can fix this and use the reference
clock. The reason for this change is that the core clock is subject to the
SSCG, so boards where SSCG is enabled exhibit a very large timer drift.

To prevent any compatibility issues when booting with an older devicetree,
this series provides proper fall backs in each case.

The series applies on v3.18-rc1. As usual, any feedback is well received!

Ezequiel Garcia (4):
  clocksource: armada-370-xp: Add missing clock enable
  watchdog: orion: Use the reference clock on Armada 375 SoC
  clocksource: armada-370-xp: Use the reference clock on A375 SoC
  ARM: dts: Enable the reference clock for timer and watchdog on Armada
    375 SoC

 .../bindings/timer/marvell,armada-370-xp-timer.txt |  9 +++--
 arch/arm/boot/dts/armada-375.dtsi                  | 12 +++++--
 drivers/clocksource/time-armada-370-xp.c           | 30 ++++++++++++++++
 drivers/watchdog/orion_wdt.c                       | 40 +++++++++++++++++++++-
 4 files changed, 86 insertions(+), 5 deletions(-)

-- 
2.1.0

[PATCH 1/4] clocksource: armada-370-xp: Add missing clock enable

[Ezequiel Garcia]

This commit makes sure the timer clock is prepared and enabled
before retrieving its rate.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/clocksource/time-armada-370-xp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clocksource/time-armada-370-xp.c b/drivers/clocksource/time-armada-370-xp.c
index 0451e62..d8555f9 100644
--- a/drivers/clocksource/time-armada-370-xp.c
+++ b/drivers/clocksource/time-armada-370-xp.c
@@ -293,6 +293,7 @@ static void __init armada_xp_timer_init(struct device_node *np)
 
 	/* The 25Mhz fixed clock is mandatory, and must always be available */
 	BUG_ON(IS_ERR(clk));
+	clk_prepare_enable(clk);
 	timer_clk = clk_get_rate(clk);
 
 	armada_370_xp_timer_common_init(np);
@@ -305,6 +306,7 @@ static void __init armada_370_timer_init(struct device_node *np)
 	struct clk *clk = of_clk_get(np, 0);
 
 	BUG_ON(IS_ERR(clk));
+	clk_prepare_enable(clk);
 	timer_clk = clk_get_rate(clk) / TIMER_DIVIDER;
 	timer25Mhz = false;
 
-- 
2.1.0

[PATCH 1/4] clocksource: armada-370-xp: Add missing clock enable

[Thomas Petazzoni]

Dear Ezequiel Garcia,

On Wed, 22 Oct 2014 10:34:41 -0300, Ezequiel Garcia wrote:
> This commit makes sure the timer clock is prepared and enabled
> before retrieving its rate.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
>  drivers/clocksource/time-armada-370-xp.c | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH 1/4] clocksource: armada-370-xp: Add missing clock enable

[Gregory CLEMENT]

Hi Ezequiel,

On 22/10/2014 15:34, Ezequiel Garcia wrote:
> This commit makes sure the timer clock is prepared and enabled
> before retrieving its rate.

Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com>


Thanks,

Gregory

> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
>  drivers/clocksource/time-armada-370-xp.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/clocksource/time-armada-370-xp.c b/drivers/clocksource/time-armada-370-xp.c
> index 0451e62..d8555f9 100644
> --- a/drivers/clocksource/time-armada-370-xp.c
> +++ b/drivers/clocksource/time-armada-370-xp.c
> @@ -293,6 +293,7 @@ static void __init armada_xp_timer_init(struct device_node *np)
>  
>  	/* The 25Mhz fixed clock is mandatory, and must always be available */
>  	BUG_ON(IS_ERR(clk));
> +	clk_prepare_enable(clk);
>  	timer_clk = clk_get_rate(clk);
>  
>  	armada_370_xp_timer_common_init(np);
> @@ -305,6 +306,7 @@ static void __init armada_370_timer_init(struct device_node *np)
>  	struct clk *clk = of_clk_get(np, 0);
>  
>  	BUG_ON(IS_ERR(clk));
> +	clk_prepare_enable(clk);
>  	timer_clk = clk_get_rate(clk) / TIMER_DIVIDER;
>  	timer25Mhz = false;
>  
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 2/4] watchdog: orion: Use the reference clock on Armada 375 SoC

[Ezequiel Garcia]

The 25 MHz reference clock has better stability so its use is preferred over the
core clock. Changes the Armada 375 clock initialization to use this reference
clock. To ensure the driver is compatible with an old devicetree, also provide
a fallback path which will silently return to the previous behavior.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 drivers/watchdog/orion_wdt.c | 40 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 00d0741..6452fa2 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -114,6 +114,44 @@ static int armada370_wdt_clock_init(struct platform_device *pdev,
 	return 0;
 }
 
+static int armada375_wdt_clock_init(struct platform_device *pdev,
+				    struct orion_watchdog *dev)
+{
+	int ret;
+
+	dev->clk = of_clk_get_by_name(pdev->dev.of_node, "fixed");
+	if (!IS_ERR(dev->clk)) {
+
+		ret = clk_prepare_enable(dev->clk);
+		if (ret) {
+			clk_put(dev->clk);
+			return ret;
+		}
+
+		atomic_io_modify(dev->reg + TIMER_CTRL,
+				WDT_AXP_FIXED_ENABLE_BIT,
+				WDT_AXP_FIXED_ENABLE_BIT);
+		dev->clk_rate = clk_get_rate(dev->clk);
+		return 0;
+	}
+
+	/* Mandatory fallback for proper devicetree backward compatibility */
+	dev->clk = clk_get(&pdev->dev, NULL);
+	if (IS_ERR(dev->clk))
+		return PTR_ERR(dev->clk);
+	ret = clk_prepare_enable(dev->clk);
+	if (ret) {
+		clk_put(dev->clk);
+		return ret;
+	}
+
+	atomic_io_modify(dev->reg + TIMER_CTRL,
+			WDT_A370_RATIO_MASK(WDT_A370_RATIO_SHIFT),
+			WDT_A370_RATIO_MASK(WDT_A370_RATIO_SHIFT));
+	dev->clk_rate = clk_get_rate(dev->clk) / WDT_A370_RATIO;
+	return 0;
+}
+
 static int armadaxp_wdt_clock_init(struct platform_device *pdev,
 				   struct orion_watchdog *dev)
 {
@@ -394,7 +432,7 @@ static const struct orion_watchdog_data armada375_data = {
 	.rstout_mask_bit = BIT(10),
 	.wdt_enable_bit = BIT(8),
 	.wdt_counter_offset = 0x34,
-	.clock_init = armada370_wdt_clock_init,
+	.clock_init = armada375_wdt_clock_init,
 	.enabled = armada375_enabled,
 	.start = armada375_start,
 	.stop = armada375_stop,
-- 
2.1.0

[PATCH 2/4] watchdog: orion: Use the reference clock on Armada 375 SoC

[Thomas Petazzoni]

Dear Ezequiel Garcia,

On Wed, 22 Oct 2014 10:34:42 -0300, Ezequiel Garcia wrote:
> The 25 MHz reference clock has better stability so its use is preferred over the
> core clock. Changes the Armada 375 clock initialization to use this reference
> clock. To ensure the driver is compatible with an old devicetree, also provide
> a fallback path which will silently return to the previous behavior.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
>  drivers/watchdog/orion_wdt.c | 40 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 39 insertions(+), 1 deletion(-)

The Armada 375 case should be similar to the Armada XP case, so can't
we use the existing armadaxp_wdt_clock_init() function ?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH 2/4] watchdog: orion: Use the reference clock on Armada 375 SoC

[Andrew Lunn]

On Wed, Oct 22, 2014 at 10:34:42AM -0300, Ezequiel Garcia wrote:
> The 25 MHz reference clock has better stability so its use is preferred over the
> core clock. Changes the Armada 375 clock initialization to use this reference
> clock. To ensure the driver is compatible with an old devicetree, also provide
> a fallback path which will silently return to the previous behavior.

Hi Ezequiel

There is now quite a lot of code in orion_wdt.c which is not relevant
to Orion5x and Kirkwood. Would it be possible to put some of it inside
a #ifdef MACH_MVEBU_V7?

  Andrew

> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
>  drivers/watchdog/orion_wdt.c | 40 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
> index 00d0741..6452fa2 100644
> --- a/drivers/watchdog/orion_wdt.c
> +++ b/drivers/watchdog/orion_wdt.c
> @@ -114,6 +114,44 @@ static int armada370_wdt_clock_init(struct platform_device *pdev,
>  	return 0;
>  }
>  
> +static int armada375_wdt_clock_init(struct platform_device *pdev,
> +				    struct orion_watchdog *dev)
> +{
> +	int ret;
> +
> +	dev->clk = of_clk_get_by_name(pdev->dev.of_node, "fixed");
> +	if (!IS_ERR(dev->clk)) {
> +
> +		ret = clk_prepare_enable(dev->clk);
> +		if (ret) {
> +			clk_put(dev->clk);
> +			return ret;
> +		}
> +
> +		atomic_io_modify(dev->reg + TIMER_CTRL,
> +				WDT_AXP_FIXED_ENABLE_BIT,
> +				WDT_AXP_FIXED_ENABLE_BIT);
> +		dev->clk_rate = clk_get_rate(dev->clk);
> +		return 0;
> +	}
> +
> +	/* Mandatory fallback for proper devicetree backward compatibility */
> +	dev->clk = clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(dev->clk))
> +		return PTR_ERR(dev->clk);
> +	ret = clk_prepare_enable(dev->clk);
> +	if (ret) {
> +		clk_put(dev->clk);
> +		return ret;
> +	}
> +
> +	atomic_io_modify(dev->reg + TIMER_CTRL,
> +			WDT_A370_RATIO_MASK(WDT_A370_RATIO_SHIFT),
> +			WDT_A370_RATIO_MASK(WDT_A370_RATIO_SHIFT));
> +	dev->clk_rate = clk_get_rate(dev->clk) / WDT_A370_RATIO;
> +	return 0;
> +}
> +
>  static int armadaxp_wdt_clock_init(struct platform_device *pdev,
>  				   struct orion_watchdog *dev)
>  {
> @@ -394,7 +432,7 @@ static const struct orion_watchdog_data armada375_data = {
>  	.rstout_mask_bit = BIT(10),
>  	.wdt_enable_bit = BIT(8),
>  	.wdt_counter_offset = 0x34,
> -	.clock_init = armada370_wdt_clock_init,
> +	.clock_init = armada375_wdt_clock_init,
>  	.enabled = armada375_enabled,
>  	.start = armada375_start,
>  	.stop = armada375_stop,
> -- 
> 2.1.0
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/4] watchdog: orion: Use the reference clock on Armada 375 SoC

[Ezequiel Garcia]

On 10/22/2014 11:02 AM, Andrew Lunn wrote:
> On Wed, Oct 22, 2014 at 10:34:42AM -0300, Ezequiel Garcia wrote:
>> The 25 MHz reference clock has better stability so its use is preferred over the
>> core clock. Changes the Armada 375 clock initialization to use this reference
>> clock. To ensure the driver is compatible with an old devicetree, also provide
>> a fallback path which will silently return to the previous behavior.
> 
> Hi Ezequiel
> 
> There is now quite a lot of code in orion_wdt.c which is not relevant
> to Orion5x and Kirkwood. Would it be possible to put some of it inside
> a #ifdef MACH_MVEBU_V7?
> 

Hum.. I found ifdefs scary, so I tend to avoid them if at all possible.
Just did a quick hack enclosing all the armada-xxx stuff around #if 0
and here's the result:

$ ./scripts/bloat-o-meter ~/linux/.builds/mvebu_v7/drivers/watchdog/orion_wdt.o ~/linux/.builds/orion5x/drivers/watchdog/orion_wdt.o
add/remove: 0/0 grow/shrink: 1/4 up/down: 12/-80 (-68)
function                                     old     new   delta
orion_wdt_probe                              732     744     +12
orion_wdt_get_timeleft                        44      40      -4
orion_enabled                                 68      60      -8
orion_start                                  120      88     -32
orion_wdt_ping                                80      44     -36

To be honest, I don't think it's worth the ugliness.
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[PATCH 2/4] watchdog: orion: Use the reference clock on Armada 375 SoC

[Guenter Roeck]

On Wed, Oct 22, 2014 at 07:41:27PM -0300, Ezequiel Garcia wrote:
> On 10/22/2014 11:02 AM, Andrew Lunn wrote:
> > On Wed, Oct 22, 2014 at 10:34:42AM -0300, Ezequiel Garcia wrote:
> >> The 25 MHz reference clock has better stability so its use is preferred over the
> >> core clock. Changes the Armada 375 clock initialization to use this reference
> >> clock. To ensure the driver is compatible with an old devicetree, also provide
> >> a fallback path which will silently return to the previous behavior.
> > 
> > Hi Ezequiel
> > 
> > There is now quite a lot of code in orion_wdt.c which is not relevant
> > to Orion5x and Kirkwood. Would it be possible to put some of it inside
> > a #ifdef MACH_MVEBU_V7?
> > 
> 
> Hum.. I found ifdefs scary, so I tend to avoid them if at all possible.
> Just did a quick hack enclosing all the armada-xxx stuff around #if 0
> and here's the result:
> 
> $ ./scripts/bloat-o-meter ~/linux/.builds/mvebu_v7/drivers/watchdog/orion_wdt.o ~/linux/.builds/orion5x/drivers/watchdog/orion_wdt.o
> add/remove: 0/0 grow/shrink: 1/4 up/down: 12/-80 (-68)
> function                                     old     new   delta
> orion_wdt_probe                              732     744     +12
> orion_wdt_get_timeleft                        44      40      -4
> orion_enabled                                 68      60      -8
> orion_start                                  120      88     -32
> orion_wdt_ping                                80      44     -36
> 
> To be honest, I don't think it's worth the ugliness.

I agree, especially since each #ifdef means that some code may not be compiled
and errors are easier to creep into the code.

Guenter

[PATCH 2/4] watchdog: orion: Use the reference clock on Armada 375 SoC

[Andrew Lunn]

On Wed, Oct 22, 2014 at 07:41:27PM -0300, Ezequiel Garcia wrote:
> On 10/22/2014 11:02 AM, Andrew Lunn wrote:
> > On Wed, Oct 22, 2014 at 10:34:42AM -0300, Ezequiel Garcia wrote:
> >> The 25 MHz reference clock has better stability so its use is preferred over the
> >> core clock. Changes the Armada 375 clock initialization to use this reference
> >> clock. To ensure the driver is compatible with an old devicetree, also provide
> >> a fallback path which will silently return to the previous behavior.
> > 
> > Hi Ezequiel
> > 
> > There is now quite a lot of code in orion_wdt.c which is not relevant
> > to Orion5x and Kirkwood. Would it be possible to put some of it inside
> > a #ifdef MACH_MVEBU_V7?
> > 
> 
> Hum.. I found ifdefs scary, so I tend to avoid them if at all possible.
> Just did a quick hack enclosing all the armada-xxx stuff around #if 0
> and here's the result:
> 
> $ ./scripts/bloat-o-meter ~/linux/.builds/mvebu_v7/drivers/watchdog/orion_wdt.o ~/linux/.builds/orion5x/drivers/watchdog/orion_wdt.o
> add/remove: 0/0 grow/shrink: 1/4 up/down: 12/-80 (-68)
> function                                     old     new   delta
> orion_wdt_probe                              732     744     +12
> orion_wdt_get_timeleft                        44      40      -4
> orion_enabled                                 68      60      -8
> orion_start                                  120      88     -32
> orion_wdt_ping                                80      44     -36

Hi Ezequiel

I did a similar test:

size drivers/watchdog/orion_wdt.o-*
   text	   data	    bss	    dec	    hex	filename
   4428	    100       1    4529    11b1	drivers/watchdog/orion_wdt.o-full-fat
   2324	    100	      1    2425	    979	drivers/watchdog/orion_wdt.o-skimmed

So for v5 kirkwood/orion5x, there is about 90% overhead from the v7
code, in the text section. This is for -rc1 code, before adding any
more v7 code in this patchset.

I don't think the #ifdef's look that scary.

  Andrew

[PATCH 2/4] watchdog: orion: Use the reference clock on Armada 375 SoC

[Ezequiel Garcia]

On 10/22/2014 08:59 PM, Andrew Lunn wrote:
> On Wed, Oct 22, 2014 at 07:41:27PM -0300, Ezequiel Garcia wrote:
>> On 10/22/2014 11:02 AM, Andrew Lunn wrote:
>>> On Wed, Oct 22, 2014 at 10:34:42AM -0300, Ezequiel Garcia wrote:
>>>> The 25 MHz reference clock has better stability so its use is preferred over the
>>>> core clock. Changes the Armada 375 clock initialization to use this reference
>>>> clock. To ensure the driver is compatible with an old devicetree, also provide
>>>> a fallback path which will silently return to the previous behavior.
>>>
>>> Hi Ezequiel
>>>
>>> There is now quite a lot of code in orion_wdt.c which is not relevant
>>> to Orion5x and Kirkwood. Would it be possible to put some of it inside
>>> a #ifdef MACH_MVEBU_V7?
>>>
>>
>> Hum.. I found ifdefs scary, so I tend to avoid them if at all possible.
>> Just did a quick hack enclosing all the armada-xxx stuff around #if 0
>> and here's the result:
>>
>> $ ./scripts/bloat-o-meter ~/linux/.builds/mvebu_v7/drivers/watchdog/orion_wdt.o ~/linux/.builds/orion5x/drivers/watchdog/orion_wdt.o
>> add/remove: 0/0 grow/shrink: 1/4 up/down: 12/-80 (-68)
>> function                                     old     new   delta
>> orion_wdt_probe                              732     744     +12
>> orion_wdt_get_timeleft                        44      40      -4
>> orion_enabled                                 68      60      -8
>> orion_start                                  120      88     -32
>> orion_wdt_ping                                80      44     -36
> 
> Hi Ezequiel
> 
> I did a similar test:
> 
> size drivers/watchdog/orion_wdt.o-*
>    text	   data	    bss	    dec	    hex	filename
>    4428	    100       1    4529    11b1	drivers/watchdog/orion_wdt.o-full-fat
>    2324	    100	      1    2425	    979	drivers/watchdog/orion_wdt.o-skimmed
> 
> So for v5 kirkwood/orion5x, there is about 90% overhead from the v7
> code, in the text section. This is for -rc1 code, before adding any
> more v7 code in this patchset.
> 
> I don't think the #ifdef's look that scary.
> 

I understand it's a lot of overhead when you think of the relative
driver's size, but still it's just a 2 KiB saving.

Don't get me wrong, I'm all for reducing bloat, but I don't want to
author a patch that ifdefs all around the place to save a couple kilobytes.
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[PATCH 3/4] clocksource: armada-370-xp: Use the reference clock on A375 SoC

[Ezequiel Garcia]

The 25 MHz reference clock has better stability so its use is preferred over the
core clock.

This commit takes advantage of the already introduced Armada 375 devicetree
compatible string and adds a new timer initialization. If available, the timer
will use the reference clock (named as 'fixed'). Otherwise, it falls back to the
previous behavior.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 .../bindings/timer/marvell,armada-370-xp-timer.txt |  9 +++++--
 drivers/clocksource/time-armada-370-xp.c           | 28 ++++++++++++++++++++++
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/timer/marvell,armada-370-xp-timer.txt b/Documentation/devicetree/bindings/timer/marvell,armada-370-xp-timer.txt
index f455182..5ef42bf 100644
--- a/Documentation/devicetree/bindings/timer/marvell,armada-370-xp-timer.txt
+++ b/Documentation/devicetree/bindings/timer/marvell,armada-370-xp-timer.txt
@@ -2,8 +2,10 @@ Marvell Armada 370 and Armada XP Timers
 ---------------------------------------
 
 Required properties:
-- compatible: Should be either "marvell,armada-370-timer" or
-  "marvell,armada-xp-timer" as appropriate.
+- compatible: Should be one of the following
+              "marvell,armada-370-timer",
+	      "marvell,armada-375-timer",
+	      "marvell,armada-xp-timer".
 - interrupts: Should contain the list of Global Timer interrupts and
   then local timer interrupts
 - reg: Should contain location and length for timers register. First
@@ -13,6 +15,9 @@ Required properties:
 Clocks required for compatible = "marvell,armada-370-timer":
 - clocks : Must contain a single entry describing the clock input
 
+Clocks required for compatible = "marvell,armada-375-timer":
+- clocks : Must contain a "fixed" name clock entry
+
 Clocks required for compatible = "marvell,armada-xp-timer":
 - clocks : Must contain an entry for each entry in clock-names.
 - clock-names : Must include the following entries:
diff --git a/drivers/clocksource/time-armada-370-xp.c b/drivers/clocksource/time-armada-370-xp.c
index d8555f9..3a0704b 100644
--- a/drivers/clocksource/time-armada-370-xp.c
+++ b/drivers/clocksource/time-armada-370-xp.c
@@ -301,6 +301,34 @@ static void __init armada_xp_timer_init(struct device_node *np)
 CLOCKSOURCE_OF_DECLARE(armada_xp, "marvell,armada-xp-timer",
 		       armada_xp_timer_init);
 
+static void __init armada_375_timer_init(struct device_node *np)
+{
+	struct clk *clk;
+
+	clk = of_clk_get_by_name(np, "fixed");
+	if (!IS_ERR(clk)) {
+		clk_prepare_enable(clk);
+		timer_clk = clk_get_rate(clk);
+	} else {
+
+		/*
+		 * This fallback is required in order to retain proper
+		 * devicetree backwards compatibility.
+		 */
+		clk = of_clk_get(np, 0);
+
+		/* Must have at least a clock */
+		BUG_ON(IS_ERR(clk));
+		clk_prepare_enable(clk);
+		timer_clk = clk_get_rate(clk) / TIMER_DIVIDER;
+		timer25Mhz = false;
+	}
+
+	armada_370_xp_timer_common_init(np);
+}
+CLOCKSOURCE_OF_DECLARE(armada_375, "marvell,armada-375-timer",
+		       armada_375_timer_init);
+
 static void __init armada_370_timer_init(struct device_node *np)
 {
 	struct clk *clk = of_clk_get(np, 0);
-- 
2.1.0

[PATCH 3/4] clocksource: armada-370-xp: Use the reference clock on A375 SoC

[Thomas Petazzoni]

Dear Ezequiel Garcia,

On Wed, 22 Oct 2014 10:34:43 -0300, Ezequiel Garcia wrote:

> +static void __init armada_375_timer_init(struct device_node *np)
> +{
> +	struct clk *clk;
> +
> +	clk = of_clk_get_by_name(np, "fixed");
> +	if (!IS_ERR(clk)) {
> +		clk_prepare_enable(clk);
> +		timer_clk = clk_get_rate(clk);
> +	} else {
> +
> +		/*
> +		 * This fallback is required in order to retain proper
> +		 * devicetree backwards compatibility.
> +		 */
> +		clk = of_clk_get(np, 0);
> +
> +		/* Must have at least a clock */
> +		BUG_ON(IS_ERR(clk));
> +		clk_prepare_enable(clk);
> +		timer_clk = clk_get_rate(clk) / TIMER_DIVIDER;
> +		timer25Mhz = false;
> +	}

Maybe we could re-use a bit more existing code, don't know if it makes
things clearer though:

	clk = of_clk_get_by_name(np, "fixed");
	if (!IS_ERR(clk))
		armada_xp_timer_init(np);
	else
		armada_370_timer_init(np);

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH 3/4] clocksource: armada-370-xp: Use the reference clock on A375 SoC

[Ezequiel Garcia]

On 10/22/2014 10:54 AM, Thomas Petazzoni wrote:
> Dear Ezequiel Garcia,
> 
> On Wed, 22 Oct 2014 10:34:43 -0300, Ezequiel Garcia wrote:
> 
>> +static void __init armada_375_timer_init(struct device_node *np)
>> +{
>> +	struct clk *clk;
>> +
>> +	clk = of_clk_get_by_name(np, "fixed");
>> +	if (!IS_ERR(clk)) {
>> +		clk_prepare_enable(clk);
>> +		timer_clk = clk_get_rate(clk);
>> +	} else {
>> +
>> +		/*
>> +		 * This fallback is required in order to retain proper
>> +		 * devicetree backwards compatibility.
>> +		 */
>> +		clk = of_clk_get(np, 0);
>> +
>> +		/* Must have at least a clock */
>> +		BUG_ON(IS_ERR(clk));
>> +		clk_prepare_enable(clk);
>> +		timer_clk = clk_get_rate(clk) / TIMER_DIVIDER;
>> +		timer25Mhz = false;
>> +	}
> 
> Maybe we could re-use a bit more existing code, don't know if it makes
> things clearer though:
> 
> 	clk = of_clk_get_by_name(np, "fixed");
> 	if (!IS_ERR(clk))
> 		armada_xp_timer_init(np);
> 	else
> 		armada_370_timer_init(np);
> 

Yeah, I thought about this option, but chosen to implement it explicitly.

The reason is that I wanted to make sure we had a sane backward
compatibility fallback, which is specifically crafted for A375.

By re-using the 370 and XP, we are exposed to someone changing the
armada_xp_timer_init() thinking it will only affect AXP, which might
break things for A375.
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[PATCH 4/4] ARM: dts: Enable the reference clock for timer and watchdog on Armada 375 SoC

[Ezequiel Garcia]

Now that the timer and watchdog drivers support the Armada 375 usage of
the reference clock, we can enable it in the devicetree.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/boot/dts/armada-375.dtsi | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/armada-375.dtsi b/arch/arm/boot/dts/armada-375.dtsi
index de65714..4d89145 100644
--- a/arch/arm/boot/dts/armada-375.dtsi
+++ b/arch/arm/boot/dts/armada-375.dtsi
@@ -36,6 +36,12 @@
 			#clock-cells = <0>;
 			clock-frequency = <2000000000>;
 		};
+		/* 25 MHz reference crystal */
+		refclk: oscillator {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <25000000>;
+		};
 	};
 
 	cpus {
@@ -366,13 +372,15 @@
 						      <&gic  GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>,
 						      <&mpic 5>,
 						      <&mpic 6>;
-				clocks = <&coreclk 0>;
+				clocks = <&refclk>;
+				clock-names = "fixed";
 			};
 
 			watchdog at 20300 {
 				compatible = "marvell,armada-375-wdt";
 				reg = <0x20300 0x34>, <0x20704 0x4>, <0x18254 0x4>;
-				clocks = <&coreclk 0>;
+				clocks = <&refclk>;
+				clock-names = "fixed";
 			};
 
 			cpurst at 20800 {
-- 
2.1.0

[PATCH 4/4] ARM: dts: Enable the reference clock for timer and watchdog on Armada 375 SoC

[Thomas Petazzoni]

Dear Ezequiel Garcia,

On Wed, 22 Oct 2014 10:34:44 -0300, Ezequiel Garcia wrote:

> -				clocks = <&coreclk 0>;
> +				clocks = <&refclk>;
> +				clock-names = "fixed";

Why not do like we do on Armada XP, and actually represent the
hardware, by showing that it takes two clocks as input:

                        timer at 20300 {
                                compatible = "marvell,armada-xp-timer";
                                clocks = <&coreclk 2>, <&refclk>;
                                clock-names = "nbclk", "fixed";
                        };



>  			watchdog at 20300 {
>  				compatible = "marvell,armada-375-wdt";
>  				reg = <0x20300 0x34>, <0x20704 0x4>, <0x18254 0x4>;
> -				clocks = <&coreclk 0>;
> +				clocks = <&refclk>;
> +				clock-names = "fixed";
>  			};

Ditto.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH 4/4] ARM: dts: Enable the reference clock for timer and watchdog on Armada 375 SoC

[Ezequiel Garcia]

On 10/22/2014 10:55 AM, Thomas Petazzoni wrote:
> Dear Ezequiel Garcia,
> 
> On Wed, 22 Oct 2014 10:34:44 -0300, Ezequiel Garcia wrote:
> 
>> -				clocks = <&coreclk 0>;
>> +				clocks = <&refclk>;
>> +				clock-names = "fixed";
> 
> Why not do like we do on Armada XP, and actually represent the
> hardware, by showing that it takes two clocks as input:
> 

Yes, I guess that would work.
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[PATCH 0/4] Make Armada 375 use the reference clock when possible

[Andrew Lunn]

On Wed, Oct 22, 2014 at 10:34:40AM -0300, Ezequiel Garcia wrote:
> This series adds support for the 25 MHz reference clock available on
> Armada 375 SoC to use on the timer and watchdog drivers. It is
> similar to the one present in Armada XP SoC.
> 
> Given we initially had access to only a very early SoC revision (A375 Z0)
> and due to a hardware issue, the timer and watchdog support was originally
> submitted to use the core clock.

Hi Ezequiel

What happens with Z0 when used with these patches? Do they continue to
work? Do these hardware issues cause a problem?

> Now that the A0 SoC revision is out, we can fix this and use the reference
> clock. The reason for this change is that the core clock is subject to the
> SSCG, so boards where SSCG is enabled exhibit a very large timer drift.

It would of been nice to say what SSCG is. I had to go look it up.
However, since this is just in the cover note and not in the patches
itself, it is not important.

	Andrew

[PATCH 0/4] Make Armada 375 use the reference clock when possible

[Thomas Petazzoni]

Dear Ezequiel Garcia,

On Wed, 22 Oct 2014 10:34:40 -0300, Ezequiel Garcia wrote:
> This series adds support for the 25 MHz reference clock available on
> Armada 375 SoC to use on the timer and watchdog drivers. It is
> similar to the one present in Armada XP SoC.
> 
> Given we initially had access to only a very early SoC revision (A375 Z0)

Actually: s/Z0/Z1/.

> and due to a hardware issue, the timer and watchdog support was originally
> submitted to use the core clock.
> 
> Now that the A0 SoC revision is out, we can fix this and use the reference
> clock. The reason for this change is that the core clock is subject to the
> SSCG, so boards where SSCG is enabled exhibit a very large timer drift.
> 
> To prevent any compatibility issues when booting with an older devicetree,
> this series provides proper fall backs in each case.

You don't clearly state whether your patch series keep compatibility
with the 375 Z1 or not. And in fact, it doesn't keep compatibility with
Z1. I'm fine with that, but then it means we should officially declare
the Z1 support in mainline as dead, and get rid of the workarounds that
applied only to 375 Z1.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH 0/4] Make Armada 375 use the reference clock when possible

[Andrew Lunn]

> You don't clearly state whether your patch series keep compatibility
> with the 375 Z1 or not. And in fact, it doesn't keep compatibility with
> Z1.

Hi Thomas

That answers my question. Thanks

> I'm fine with that, but then it means we should officially declare
> the Z1 support in mainline as dead, and get rid of the workarounds that
> applied only to 375 Z1.

A big ACK.

How bad is it if you run these patches on a Z1? Is it worth adding a
revision check and calling BUG()?

	 Andrew

[PATCH 0/4] Make Armada 375 use the reference clock when possible

[Thomas Petazzoni]

Dear Andrew Lunn,

On Wed, 22 Oct 2014 16:08:47 +0200, Andrew Lunn wrote:

> How bad is it if you run these patches on a Z1? Is it worth adding a
> revision check and calling BUG()?

To be tested, I don't have the answer off-hand.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH 0/4] Make Armada 375 use the reference clock when possible

[Ezequiel Garcia]

On 10/22/2014 11:25 AM, Thomas Petazzoni wrote:
> Dear Andrew Lunn,
> 
> On Wed, 22 Oct 2014 16:08:47 +0200, Andrew Lunn wrote:
> 
>> How bad is it if you run these patches on a Z1? Is it worth adding a
>> revision check and calling BUG()?
> 
> To be tested, I don't have the answer off-hand.
> 

Right, I'll take a look.
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[PATCH 0/4] Make Armada 375 use the reference clock when possible

[Ezequiel Garcia]

On 10/22/2014 11:08 AM, Andrew Lunn wrote:
>> You don't clearly state whether your patch series keep compatibility
>> with the 375 Z1 or not. And in fact, it doesn't keep compatibility with
>> Z1.
> 
> Hi Thomas
> 
> That answers my question. Thanks
> 
>> I'm fine with that, but then it means we should officially declare
>> the Z1 support in mainline as dead, and get rid of the workarounds that
>> applied only to 375 Z1.
> 
> A big ACK.
> 
> How bad is it if you run these patches on a Z1? Is it worth adding a
> revision check and calling BUG()?
> 

So, I just tried this on a Z1. Here's the result:

sleep 3 -> sleeps for 6 seconds

Configuring the watchdog for 3 seconds and:

echo "barf" > /dev/watchdog -> resets after 6 seconds

So using the 25 MHz reference clock is broken on Z1, but the point at
least boots; it will "just" keep track of time wrongly.
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[PATCH 0/4] Make Armada 375 use the reference clock when possible

[Ezequiel Garcia]

On 10/22/2014 10:56 AM, Thomas Petazzoni wrote:
> Dear Ezequiel Garcia,
> 
> On Wed, 22 Oct 2014 10:34:40 -0300, Ezequiel Garcia wrote:
>> This series adds support for the 25 MHz reference clock available on
>> Armada 375 SoC to use on the timer and watchdog drivers. It is
>> similar to the one present in Armada XP SoC.
>>
>> Given we initially had access to only a very early SoC revision (A375 Z0)
> 
> Actually: s/Z0/Z1/.
> 
>> and due to a hardware issue, the timer and watchdog support was originally
>> submitted to use the core clock.
>>
>> Now that the A0 SoC revision is out, we can fix this and use the reference
>> clock. The reason for this change is that the core clock is subject to the
>> SSCG, so boards where SSCG is enabled exhibit a very large timer drift.
>>
>> To prevent any compatibility issues when booting with an older devicetree,
>> this series provides proper fall backs in each case.
> 
> You don't clearly state whether your patch series keep compatibility
> with the 375 Z1 or not. And in fact, it doesn't keep compatibility with
> Z1. I'm fine with that, but then it means we should officially declare
> the Z1 support in mainline as dead, and get rid of the workarounds that
> applied only to 375 Z1.
> 

How many people have Z1 boards? And is someone actually using one for
something? Do we have any other reason to support Z1?

FWIW, a v3.18-rc1 kernel built with mvebu_v7_defconfig, silently stalls
in the middle of the boot on Z1. Had to remove lots of compile time
options to make it boot.

Before I start digging into this, maybe we can discuss your suggestion
to drop it. If nobody is booting this often enough, maybe nobody cares
about this, and it makes sense to drop the support?
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[PATCH 0/4] Make Armada 375 use the reference clock when possible

[Thomas Petazzoni]

Dear Ezequiel Garcia,

On Thu, 23 Oct 2014 09:16:33 -0300, Ezequiel Garcia wrote:

> > You don't clearly state whether your patch series keep compatibility
> > with the 375 Z1 or not. And in fact, it doesn't keep compatibility with
> > Z1. I'm fine with that, but then it means we should officially declare
> > the Z1 support in mainline as dead, and get rid of the workarounds that
> > applied only to 375 Z1.
> 
> How many people have Z1 boards? And is someone actually using one for
> something? Do we have any other reason to support Z1?
> 
> FWIW, a v3.18-rc1 kernel built with mvebu_v7_defconfig, silently stalls
> in the middle of the boot on Z1. Had to remove lots of compile time
> options to make it boot.
> 
> Before I start digging into this, maybe we can discuss your suggestion
> to drop it. If nobody is booting this often enough, maybe nobody cares
> about this, and it makes sense to drop the support?

Marvell has always said they are not interested in having Z1 supported
in mainline, except as a first step to start getting the 375 support in
mainline. I have been waiting for everyone of us to have access to 375
A0 platforms, which is now the case. So I believe we can probably get
rid of the 375 Z1 support entirely.

It saddens me to know that a shiny development board in the office will
no longer be useful for anything else but its own physical beauty, but
I believe that's a normal retirement strategy for a very early
development platform :-)

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH 0/4] Make Armada 375 use the reference clock when possible

[Ezequiel Garcia]

On 10/23/2014 09:26 AM, Thomas Petazzoni wrote:
> Dear Ezequiel Garcia,
> 
> On Thu, 23 Oct 2014 09:16:33 -0300, Ezequiel Garcia wrote:
> 
>>> You don't clearly state whether your patch series keep compatibility
>>> with the 375 Z1 or not. And in fact, it doesn't keep compatibility with
>>> Z1. I'm fine with that, but then it means we should officially declare
>>> the Z1 support in mainline as dead, and get rid of the workarounds that
>>> applied only to 375 Z1.
>>
>> How many people have Z1 boards? And is someone actually using one for
>> something? Do we have any other reason to support Z1?
>>
>> FWIW, a v3.18-rc1 kernel built with mvebu_v7_defconfig, silently stalls
>> in the middle of the boot on Z1. Had to remove lots of compile time
>> options to make it boot.
>>
>> Before I start digging into this, maybe we can discuss your suggestion
>> to drop it. If nobody is booting this often enough, maybe nobody cares
>> about this, and it makes sense to drop the support?
> 
> Marvell has always said they are not interested in having Z1 supported
> in mainline, except as a first step to start getting the 375 support in
> mainline. I have been waiting for everyone of us to have access to 375
> A0 platforms, which is now the case. So I believe we can probably get
> rid of the 375 Z1 support entirely.
> 
> It saddens me to know that a shiny development board in the office will
> no longer be useful for anything else but its own physical beauty, but
> I believe that's a normal retirement strategy for a very early
> development platform :-)
> 

I'm afraid I have news for you. Even now, the board is almost unusable,
as the mvpp2 network driver only supports A0 (Z1 support was never even
planned).

So you have a NAS development board without network... sounds pretty
useless to me :)
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

[PATCH 0/4] Make Armada 375 use the reference clock when possible

[Thomas Petazzoni]

Dear Ezequiel Garcia,

On Thu, 23 Oct 2014 09:43:29 -0300, Ezequiel Garcia wrote:

> > Marvell has always said they are not interested in having Z1 supported
> > in mainline, except as a first step to start getting the 375 support in
> > mainline. I have been waiting for everyone of us to have access to 375
> > A0 platforms, which is now the case. So I believe we can probably get
> > rid of the 375 Z1 support entirely.
> > 
> > It saddens me to know that a shiny development board in the office will
> > no longer be useful for anything else but its own physical beauty, but
> > I believe that's a normal retirement strategy for a very early
> > development platform :-)
> > 
> 
> I'm afraid I have news for you. Even now, the board is almost unusable,
> as the mvpp2 network driver only supports A0 (Z1 support was never even
> planned).
> 
> So you have a NAS development board without network... sounds pretty
> useless to me :)

Who needs network these days? :-)

Well, joke aside, I'm fine with seeing the Armada 375 Z1 support go
away.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH 0/4] Make Armada 375 use the reference clock when possible

[Gregory CLEMENT]

Hi Ezequiel,

On 22/10/2014 15:34, Ezequiel Garcia wrote:
> This series adds support for the 25 MHz reference clock available on
> Armada 375 SoC to use on the timer and watchdog drivers. It is
> similar to the one present in Armada XP SoC.

I agree with Thomas P. comments: could you see if it was possible to reuse the same
function and dt bindings that the ones used for Armada XP ?
I am not aware of any difference between Armada XP and Armada 375 for this IP.


Thanks,

Gregory


> 
> Given we initially had access to only a very early SoC revision (A375 Z0)
> and due to a hardware issue, the timer and watchdog support was originally
> submitted to use the core clock.
> 
> Now that the A0 SoC revision is out, we can fix this and use the reference
> clock. The reason for this change is that the core clock is subject to the
> SSCG, so boards where SSCG is enabled exhibit a very large timer drift.
> 
> To prevent any compatibility issues when booting with an older devicetree,
> this series provides proper fall backs in each case.
> 
> The series applies on v3.18-rc1. As usual, any feedback is well received!
> 
> Ezequiel Garcia (4):
>   clocksource: armada-370-xp: Add missing clock enable
>   watchdog: orion: Use the reference clock on Armada 375 SoC
>   clocksource: armada-370-xp: Use the reference clock on A375 SoC
>   ARM: dts: Enable the reference clock for timer and watchdog on Armada
>     375 SoC
> 
>  .../bindings/timer/marvell,armada-370-xp-timer.txt |  9 +++--
>  arch/arm/boot/dts/armada-375.dtsi                  | 12 +++++--
>  drivers/clocksource/time-armada-370-xp.c           | 30 ++++++++++++++++
>  drivers/watchdog/orion_wdt.c                       | 40 +++++++++++++++++++++-
>  4 files changed, 86 insertions(+), 5 deletions(-)
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 0/4] Make Armada 375 use the reference clock when possible

[Thomas Petazzoni]

Dear Gregory CLEMENT,

On Wed, 22 Oct 2014 16:43:02 +0200, Gregory CLEMENT wrote:

> On 22/10/2014 15:34, Ezequiel Garcia wrote:
> > This series adds support for the 25 MHz reference clock available on
> > Armada 375 SoC to use on the timer and watchdog drivers. It is
> > similar to the one present in Armada XP SoC.
> 
> I agree with Thomas P. comments: could you see if it was possible to reuse the same
> function and dt bindings that the ones used for Armada XP ?
> I am not aware of any difference between Armada XP and Armada 375 for this IP.

Well, there's one difference: on Armada XP we don't need to support the
"old" Device Tree, which didn't had the fixed 25 Mhz clock input
described.

So either we decide that it was a mistake due to an early version of
the 375 and we skip backward DT compatibility. Or we need in some way a
different logic than Armada XP, because Armada XP doesn't have this
backward compatibility requirement.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH 0/4] Make Armada 375 use the reference clock when possible

[Ezequiel Garcia]

On 10/22/2014 11:49 AM, Thomas Petazzoni wrote:
> Dear Gregory CLEMENT,
> 
> On Wed, 22 Oct 2014 16:43:02 +0200, Gregory CLEMENT wrote:
> 
>> On 22/10/2014 15:34, Ezequiel Garcia wrote:
>>> This series adds support for the 25 MHz reference clock available on
>>> Armada 375 SoC to use on the timer and watchdog drivers. It is
>>> similar to the one present in Armada XP SoC.
>>
>> I agree with Thomas P. comments: could you see if it was possible to reuse the same
>> function and dt bindings that the ones used for Armada XP ?
>> I am not aware of any difference between Armada XP and Armada 375 for this IP.
> 
> Well, there's one difference: on Armada XP we don't need to support the
> "old" Device Tree, which didn't had the fixed 25 Mhz clock input
> described.
> 

Indeed.

> So either we decide that it was a mistake due to an early version of
> the 375 and we skip backward DT compatibility. Or we need in some way a
> different logic than Armada XP, because Armada XP doesn't have this
> backward compatibility requirement.
> 

Backward DT compatibility is a must. For instance, patches can be merged
through different routes (which has already happened) and you can break
things badly.

It doesn't worth the risk, for something that can be solved by just
implementing things carefully.
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com