[PATCH 1/2] ARM: dts: Change dw-apb-timer-osc and dw-apb-timer-sp to just dw-apb-timer

[PATCH 1/2] ARM: dts: Change dw-apb-timer-osc and dw-apb-timer-sp to just dw-apb-timer

[dinguyen at altera.com]

From: Dinh Nguyen <dinguyen@altera.com>

"dw-apb-timer-osc" and "dw-apb-timer-sp" are the same implementation of the
DW APB timer, just fed by different clocks.

Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
CC: Rob Herring <rob.herring@calxeda.com>
Cc: Grant Likely <grant.likely@linaro.org>
CC: Arnd Bergmann <arnd@arndb.de>
Cc: Olof Johansson <olof@lixom.net>
CC: Jamie Iles <jamie@jamieiles.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Heiko Stuebner <heiko@sntech.de>
Cc: Pavel Machek <pavel@denx.de>
Cc: devicetree-discuss at lists.ozlabs.org
Cc: linux-arm-kernel at lists.infradead.org
---
 Documentation/devicetree/bindings/rtc/dw-apb.txt |   21 +++----------------
 arch/arm/boot/dts/rk3066a.dtsi                   |    6 +++---
 arch/arm/boot/dts/socfpga.dtsi                   |   24 +++++++++++-----------
 arch/arm/boot/dts/socfpga_cyclone5.dts           |    8 ++++----
 arch/arm/boot/dts/socfpga_vt.dts                 |    8 ++++----
 5 files changed, 26 insertions(+), 41 deletions(-)

diff --git a/Documentation/devicetree/bindings/rtc/dw-apb.txt b/Documentation/devicetree/bindings/rtc/dw-apb.txt
index eb2327b..0a1020e 100644
--- a/Documentation/devicetree/bindings/rtc/dw-apb.txt
+++ b/Documentation/devicetree/bindings/rtc/dw-apb.txt
@@ -1,7 +1,7 @@
 * Designware APB timer
 
 Required properties:
-- compatible: "snps,dw-apb-timer-sp" or "snps,dw-apb-timer-osc"
+- compatible: "snps,dw-apb-timer"
 - reg: physical base address of the controller and length of memory mapped
   region.
 - interrupts: IRQ line for the timer.
@@ -20,23 +20,8 @@ systems may use one.
 
 
 Example:
-
-		timer1: timer at ffc09000 {
-				compatible = "snps,dw-apb-timer-sp";
-				interrupts = <0 168 4>;
-				clock-frequency = <200000000>;
-				reg = <0xffc09000 0x1000>;
-			};
-
-		timer2: timer at ffd00000 {
-				compatible = "snps,dw-apb-timer-osc";
-				interrupts = <0 169 4>;
-				clock-frequency = <200000000>;
-				reg = <0xffd00000 0x1000>;
-			};
-
-		timer3: timer at ffe00000 {
-				compatible = "snps,dw-apb-timer-osc";
+		timer1: timer at ffe00000 {
+				compatible = "snps,dw-apb-timer";
 				interrupts = <0 170 4>;
 				reg = <0xffe00000 0x1000>;
 				clocks = <&timer_clk>, <&timer_pclk>;
diff --git a/arch/arm/boot/dts/rk3066a.dtsi b/arch/arm/boot/dts/rk3066a.dtsi
index 56bfac9..2dff468 100644
--- a/arch/arm/boot/dts/rk3066a.dtsi
+++ b/arch/arm/boot/dts/rk3066a.dtsi
@@ -71,7 +71,7 @@
 		};
 
 		timer at 20038000 {
-			compatible = "snps,dw-apb-timer-osc";
+			compatible = "snps,dw-apb-timer";
 			reg = <0x20038000 0x100>;
 			interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&clk_gates1 0>, <&clk_gates7 7>;
@@ -79,7 +79,7 @@
 		};
 
 		timer at 2003a000 {
-			compatible = "snps,dw-apb-timer-osc";
+			compatible = "snps,dw-apb-timer";
 			reg = <0x2003a000 0x100>;
 			interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&clk_gates1 1>, <&clk_gates7 8>;
@@ -87,7 +87,7 @@
 		};
 
 		timer at 2000e000 {
-			compatible = "snps,dw-apb-timer-osc";
+			compatible = "snps,dw-apb-timer";
 			reg = <0x2000e000 0x100>;
 			interrupts = <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&clk_gates1 2>, <&clk_gates7 9>;
diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
index 93ee655..4e8291b 100644
--- a/arch/arm/boot/dts/socfpga.dtsi
+++ b/arch/arm/boot/dts/socfpga.dtsi
@@ -26,10 +26,6 @@
 		ethernet1 = &gmac1;
 		serial0 = &uart0;
 		serial1 = &uart1;
-		timer0 = &timer0;
-		timer1 = &timer1;
-		timer2 = &timer2;
-		timer3 = &timer3;
 	};
 
 	cpus {
@@ -486,28 +482,32 @@
 			interrupts = <1 13 0xf04>;
 		};
 
-		timer0: timer0 at ffc08000 {
-			compatible = "snps,dw-apb-timer-sp";
+		timer0: timer at ffc08000 {
+			compatible = "snps,dw-apb-timer";
 			interrupts = <0 167 4>;
 			reg = <0xffc08000 0x1000>;
+			clocks = <&osc>;
 		};
 
-		timer1: timer1 at ffc09000 {
-			compatible = "snps,dw-apb-timer-sp";
+		timer1: timer at ffc09000 {
+			compatible = "snps,dw-apb-timer";
 			interrupts = <0 168 4>;
 			reg = <0xffc09000 0x1000>;
+			clocks = <&osc>;
 		};
 
-		timer2: timer2 at ffd00000 {
-			compatible = "snps,dw-apb-timer-osc";
+		timer2: timer at ffd00000 {
+			compatible = "snps,dw-apb-timer";
 			interrupts = <0 169 4>;
 			reg = <0xffd00000 0x1000>;
+			clocks = <&l4_sp_clk>;
 		};
 
-		timer3: timer3 at ffd01000 {
-			compatible = "snps,dw-apb-timer-osc";
+		timer3: timer at ffd01000 {
+			compatible = "snps,dw-apb-timer";
 			interrupts = <0 170 4>;
 			reg = <0xffd01000 0x1000>;
+			clocks = <&l4_sp_clk>;
 		};
 
 		uart0: serial0 at ffc02000 {
diff --git a/arch/arm/boot/dts/socfpga_cyclone5.dts b/arch/arm/boot/dts/socfpga_cyclone5.dts
index 102c4d8..54b8483 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5.dts
+++ b/arch/arm/boot/dts/socfpga_cyclone5.dts
@@ -67,19 +67,19 @@
 			};
 		};
 
-		timer0 at ffc08000 {
+		timer at ffc08000 {
 			clock-frequency = <100000000>;
 		};
 
-		timer1 at ffc09000 {
+		timer at ffc09000 {
 			clock-frequency = <100000000>;
 		};
 
-		timer2 at ffd00000 {
+		timer at ffd00000 {
 			clock-frequency = <25000000>;
 		};
 
-		timer3 at ffd01000 {
+		timer at ffd01000 {
 			clock-frequency = <25000000>;
 		};
 
diff --git a/arch/arm/boot/dts/socfpga_vt.dts b/arch/arm/boot/dts/socfpga_vt.dts
index d93deb0..6a1d8b7 100644
--- a/arch/arm/boot/dts/socfpga_vt.dts
+++ b/arch/arm/boot/dts/socfpga_vt.dts
@@ -58,19 +58,19 @@
 			};
 		};
 
-		timer0 at ffc08000 {
+		timer at ffc08000 {
 			clock-frequency = <7000000>;
 		};
 
-		timer1 at ffc09000 {
+		timer at ffc09000 {
 			clock-frequency = <7000000>;
 		};
 
-		timer2 at ffd00000 {
+		timer at ffd00000 {
 			clock-frequency = <7000000>;
 		};
 
-		timer3 at ffd01000 {
+		timer at ffd01000 {
 			clock-frequency = <7000000>;
 		};
 
-- 
1.7.9.5

[PATCH 2/2] clocksource: dw_apb_timer_of: Fix read_sched_clock

[dinguyen at altera.com]

From: Dinh Nguyen <dinguyen@altera.com>

The read_sched_clock should return the ~value because the clock is a
countdown implementation. read_sched_clock() should be the same as
__apbt_read_clocksource().

If a separate timer for the sched_clock exist, then read_sched_clock()
will return an incorrect value. The (sched_io_base + 0x4) needs to be in
the function for both cases.

Also, remove the use of "dw-apb-timer-sp" and "dw-apb-timer-osc" since
they are the same DW clock.

Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
Acked-by: Jamie Iles <jamie@jamieiles.com>
CC: Rob Herring <rob.herring@calxeda.com>
CC: Arnd Bergmann <arnd@arndb.de>
Cc: Olof Johansson <olof@lixom.net>
CC: Jamie Iles <jamie@jamieiles.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Pavel Machek <pavel@denx.de>
Cc: Heiko Stuebner <heiko@sntech.de>
Cc: linux-arm-kernel at lists.infradead.org
---
 drivers/clocksource/dw_apb_timer_of.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/clocksource/dw_apb_timer_of.c b/drivers/clocksource/dw_apb_timer_of.c
index 4cbae4f..8e00929 100644
--- a/drivers/clocksource/dw_apb_timer_of.c
+++ b/drivers/clocksource/dw_apb_timer_of.c
@@ -102,18 +102,17 @@ static void add_clocksource(struct device_node *source_timer)
 	 * timer is found. sched_io_base then points to the current_value
 	 * register of the clocksource timer.
 	 */
-	sched_io_base = iobase + 0x04;
+	sched_io_base = iobase;
 	sched_rate = rate;
 }
 
 static u32 read_sched_clock(void)
 {
-	return __raw_readl(sched_io_base);
+	return ~__raw_readl(sched_io_base + 0x4);
 }
 
 static const struct of_device_id sptimer_ids[] __initconst = {
 	{ .compatible = "picochip,pc3x2-rtc" },
-	{ .compatible = "snps,dw-apb-timer-sp" },
 	{ /* Sentinel */ },
 };
 
@@ -153,4 +152,4 @@ static void __init dw_apb_timer_init(struct device_node *timer)
 	num_called++;
 }
 CLOCKSOURCE_OF_DECLARE(pc3x2_timer, "picochip,pc3x2-timer", dw_apb_timer_init);
-CLOCKSOURCE_OF_DECLARE(apb_timer, "snps,dw-apb-timer-osc", dw_apb_timer_init);
+CLOCKSOURCE_OF_DECLARE(apb_timer, "snps,dw-apb-timer", dw_apb_timer_init);
-- 
1.7.9.5

[PATCH 2/2] clocksource: dw_apb_timer_of: Fix read_sched_clock

[Heiko Stübner]

Am Donnerstag, 25. Juli 2013, 05:43:17 schrieb dinguyen at altera.com:
> From: Dinh Nguyen <dinguyen@altera.com>
> 
> The read_sched_clock should return the ~value because the clock is a
> countdown implementation. read_sched_clock() should be the same as
> __apbt_read_clocksource().
> 
> If a separate timer for the sched_clock exist, then read_sched_clock()
> will return an incorrect value. The (sched_io_base + 0x4) needs to be in
> the function for both cases.
> 
> Also, remove the use of "dw-apb-timer-sp" and "dw-apb-timer-osc" since
> they are the same DW clock.
> 
> Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
> Acked-by: Jamie Iles <jamie@jamieiles.com>
> CC: Rob Herring <rob.herring@calxeda.com>
> CC: Arnd Bergmann <arnd@arndb.de>
> Cc: Olof Johansson <olof@lixom.net>
> CC: Jamie Iles <jamie@jamieiles.com>
> Cc: John Stultz <john.stultz@linaro.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Pavel Machek <pavel@denx.de>
> Cc: Heiko Stuebner <heiko@sntech.de>
> Cc: linux-arm-kernel at lists.infradead.org
> ---
>  drivers/clocksource/dw_apb_timer_of.c |    7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/clocksource/dw_apb_timer_of.c
> b/drivers/clocksource/dw_apb_timer_of.c index 4cbae4f..8e00929 100644
> --- a/drivers/clocksource/dw_apb_timer_of.c
> +++ b/drivers/clocksource/dw_apb_timer_of.c
> @@ -102,18 +102,17 @@ static void add_clocksource(struct device_node
> *source_timer) * timer is found. sched_io_base then points to the
> current_value * register of the clocksource timer.
>  	 */
> -	sched_io_base = iobase + 0x04;
> +	sched_io_base = iobase;
>  	sched_rate = rate;
>  }
> 
>  static u32 read_sched_clock(void)
>  {
> -	return __raw_readl(sched_io_base);
> +	return ~__raw_readl(sched_io_base + 0x4);
>  }
> 



>  static const struct of_device_id sptimer_ids[] __initconst = {
>  	{ .compatible = "picochip,pc3x2-rtc" },
> -	{ .compatible = "snps,dw-apb-timer-sp" },
>  	{ /* Sentinel */ },
>  };

I'm not 100% sure, but maybe the same explaination as below applies to this - 
with it better being part of the first patch.


> @@ -153,4 +152,4 @@ static void __init dw_apb_timer_init(struct device_node
> *timer) num_called++;
>  }
>  CLOCKSOURCE_OF_DECLARE(pc3x2_timer, "picochip,pc3x2-timer",
> dw_apb_timer_init); -CLOCKSOURCE_OF_DECLARE(apb_timer,
> "snps,dw-apb-timer-osc", dw_apb_timer_init);
> +CLOCKSOURCE_OF_DECLARE(apb_timer, "snps,dw-apb-timer",
> dw_apb_timer_init);

I think this hunk would better be part of the first patch, as you rename the 
devices in the dtsi files there and it has nothing to do with the sched_clock 
fix.

Changing the clocksource name here also breaks bisectability on the affected 
platforms because they wouldn't be able to add the clocksources when only the 
first patch is applied.


Heiko

[PATCH 2/2] clocksource: dw_apb_timer_of: Fix read_sched_clock

[Dinh Nguyen]

Hi Heiko,

On Thu, 2013-07-25 at 11:02 +0200, Heiko St?bner wrote:
> Am Donnerstag, 25. Juli 2013, 05:43:17 schrieb dinguyen at altera.com:
> > From: Dinh Nguyen <dinguyen@altera.com>
> > 
> > The read_sched_clock should return the ~value because the clock is a
> > countdown implementation. read_sched_clock() should be the same as
> > __apbt_read_clocksource().
> > 
> > If a separate timer for the sched_clock exist, then read_sched_clock()
> > will return an incorrect value. The (sched_io_base + 0x4) needs to be in
> > the function for both cases.
> > 
> > Also, remove the use of "dw-apb-timer-sp" and "dw-apb-timer-osc" since
> > they are the same DW clock.
> > 
> > Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
> > Acked-by: Jamie Iles <jamie@jamieiles.com>
> > CC: Rob Herring <rob.herring@calxeda.com>
> > CC: Arnd Bergmann <arnd@arndb.de>
> > Cc: Olof Johansson <olof@lixom.net>
> > CC: Jamie Iles <jamie@jamieiles.com>
> > Cc: John Stultz <john.stultz@linaro.org>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Linus Walleij <linus.walleij@linaro.org>
> > Cc: Pavel Machek <pavel@denx.de>
> > Cc: Heiko Stuebner <heiko@sntech.de>
> > Cc: linux-arm-kernel at lists.infradead.org
> > ---
> >  drivers/clocksource/dw_apb_timer_of.c |    7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/clocksource/dw_apb_timer_of.c
> > b/drivers/clocksource/dw_apb_timer_of.c index 4cbae4f..8e00929 100644
> > --- a/drivers/clocksource/dw_apb_timer_of.c
> > +++ b/drivers/clocksource/dw_apb_timer_of.c
> > @@ -102,18 +102,17 @@ static void add_clocksource(struct device_node
> > *source_timer) * timer is found. sched_io_base then points to the
> > current_value * register of the clocksource timer.
> >  	 */
> > -	sched_io_base = iobase + 0x04;
> > +	sched_io_base = iobase;
> >  	sched_rate = rate;
> >  }
> > 
> >  static u32 read_sched_clock(void)
> >  {
> > -	return __raw_readl(sched_io_base);
> > +	return ~__raw_readl(sched_io_base + 0x4);
> >  }
> > 
> 
> 
> 
> >  static const struct of_device_id sptimer_ids[] __initconst = {
> >  	{ .compatible = "picochip,pc3x2-rtc" },
> > -	{ .compatible = "snps,dw-apb-timer-sp" },
> >  	{ /* Sentinel */ },
> >  };
> 
> I'm not 100% sure, but maybe the same explaination as below applies to this - 
> with it better being part of the first patch.
> 
> 
> > @@ -153,4 +152,4 @@ static void __init dw_apb_timer_init(struct device_node
> > *timer) num_called++;
> >  }
> >  CLOCKSOURCE_OF_DECLARE(pc3x2_timer, "picochip,pc3x2-timer",
> > dw_apb_timer_init); -CLOCKSOURCE_OF_DECLARE(apb_timer,
> > "snps,dw-apb-timer-osc", dw_apb_timer_init);
> > +CLOCKSOURCE_OF_DECLARE(apb_timer, "snps,dw-apb-timer",
> > dw_apb_timer_init);
> 
> I think this hunk would better be part of the first patch, as you rename the 
> devices in the dtsi files there and it has nothing to do with the sched_clock 
> fix.
> 
> Changing the clocksource name here also breaks bisectability on the affected 
> platforms because they wouldn't be able to add the clocksources when only the 
> first patch is applied.

I agree with you, but I was under the impression that Arnd wanted to
keep DTS changes in a separate commit.

Dinh
> 
> 
> Heiko
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[PATCH 2/2] clocksource: dw_apb_timer_of: Fix read_sched_clock

[Pavel Machek]

Hi!

> The read_sched_clock should return the ~value because the clock is a
> countdown implementation. read_sched_clock() should be the same as
> __apbt_read_clocksource().

...for altera boards. But AFAICT:

> diff --git a/drivers/clocksource/dw_apb_timer_of.c b/drivers/clocksource/dw_apb_timer_of.c
> index 4cbae4f..8e00929 100644
> --- a/drivers/clocksource/dw_apb_timer_of.c
> +++ b/drivers/clocksource/dw_apb_timer_of.c
> @@ -102,18 +102,17 @@ static void add_clocksource(struct device_node *source_timer)
>  	 * timer is found. sched_io_base then points to the current_value
>  	 * register of the clocksource timer.
>  	 */
> -	sched_io_base = iobase + 0x04;
> +	sched_io_base = iobase;
>  	sched_rate = rate;
>  }
>  
>  static u32 read_sched_clock(void)
>  {
> -	return __raw_readl(sched_io_base);
> +	return ~__raw_readl(sched_io_base + 0x4);
>  }
>  
>  static const struct of_device_id sptimer_ids[] __initconst = {
>  	{ .compatible = "picochip,pc3x2-rtc" },
> -	{ .compatible = "snps,dw-apb-timer-sp" },
>  	{ /* Sentinel */ },
>  };
>

picochip,pc3x2-rtc is _not_ compatible with snps,dw-apb-timer, and
does count up. You correctly remove snps,dw-apb-timer-sp
compatibility, but AFAICT the ~value change will do something
interesting on picochip boards.

Or maybe I'm just confused about all the timers? Jamie acked this so
it should be fine...?

Thanks,
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH 1/2] ARM: dts: Change dw-apb-timer-osc and dw-apb-timer-sp to just dw-apb-timer

[Pavel Machek]

On Wed 2013-07-24 22:43:16, dinguyen at altera.com wrote:
> From: Dinh Nguyen <dinguyen@altera.com>
> 
> "dw-apb-timer-osc" and "dw-apb-timer-sp" are the same implementation of the
> DW APB timer, just fed by different clocks.
> 
> Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
> CC: Rob Herring <rob.herring@calxeda.com>
> Cc: Grant Likely <grant.likely@linaro.org>
> CC: Arnd Bergmann <arnd@arndb.de>
> Cc: Olof Johansson <olof@lixom.net>
> CC: Jamie Iles <jamie@jamieiles.com>
> Cc: John Stultz <john.stultz@linaro.org>
> Cc: Heiko Stuebner <heiko@sntech.de>

Looks good to me, thanks!
									Pavel

Reviewed-by: Pavel Machek <pavel@denx.de>
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH 1/2] ARM: dts: Change dw-apb-timer-osc and dw-apb-timer-sp to just dw-apb-timer

[Olof Johansson]

On Wed, Jul 24, 2013 at 10:43:16PM -0500, dinguyen at altera.com wrote:
> From: Dinh Nguyen <dinguyen@altera.com>
> 
> "dw-apb-timer-osc" and "dw-apb-timer-sp" are the same implementation of the
> DW APB timer, just fed by different clocks.
> 
> Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
> CC: Rob Herring <rob.herring@calxeda.com>
> Cc: Grant Likely <grant.likely@linaro.org>
> CC: Arnd Bergmann <arnd@arndb.de>
> Cc: Olof Johansson <olof@lixom.net>
> CC: Jamie Iles <jamie@jamieiles.com>
> Cc: John Stultz <john.stultz@linaro.org>
> Cc: Heiko Stuebner <heiko@sntech.de>
> Cc: Pavel Machek <pavel@denx.de>
> Cc: devicetree-discuss at lists.ozlabs.org
> Cc: linux-arm-kernel at lists.infradead.org
> ---
>  Documentation/devicetree/bindings/rtc/dw-apb.txt |   21 +++----------------
>  arch/arm/boot/dts/rk3066a.dtsi                   |    6 +++---
>  arch/arm/boot/dts/socfpga.dtsi                   |   24 +++++++++++-----------
>  arch/arm/boot/dts/socfpga_cyclone5.dts           |    8 ++++----
>  arch/arm/boot/dts/socfpga_vt.dts                 |    8 ++++----
>  5 files changed, 26 insertions(+), 41 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/rtc/dw-apb.txt b/Documentation/devicetree/bindings/rtc/dw-apb.txt
> index eb2327b..0a1020e 100644
> --- a/Documentation/devicetree/bindings/rtc/dw-apb.txt
> +++ b/Documentation/devicetree/bindings/rtc/dw-apb.txt
> @@ -1,7 +1,7 @@
>  * Designware APB timer
>  
>  Required properties:
> -- compatible: "snps,dw-apb-timer-sp" or "snps,dw-apb-timer-osc"
> +- compatible: "snps,dw-apb-timer"
>  - reg: physical base address of the controller and length of memory mapped
>    region.
>  - interrupts: IRQ line for the timer.

Here's a classic example of where we will eventually need to keep the ABI. You
can add a comment that the preferred compatible value is the new one, but the
old ones still need to be handled by the drivers, and shouldn't be removed.

That is, of course, if people are happy with the current binding and want to
keep it instead of revising it before declaring it locked in.


-Olof

[PATCH 1/2] ARM: dts: Change dw-apb-timer-osc and dw-apb-timer-sp to just dw-apb-timer

[Pavel Machek]

> > index eb2327b..0a1020e 100644
> > --- a/Documentation/devicetree/bindings/rtc/dw-apb.txt
> > +++ b/Documentation/devicetree/bindings/rtc/dw-apb.txt
> > @@ -1,7 +1,7 @@
> >  * Designware APB timer
> >  
> >  Required properties:
> > -- compatible: "snps,dw-apb-timer-sp" or "snps,dw-apb-timer-osc"
> > +- compatible: "snps,dw-apb-timer"
> >  - reg: physical base address of the controller and length of memory mapped
> >    region.
> >  - interrupts: IRQ line for the timer.
> 
> Here's a classic example of where we will eventually need to keep the ABI. You
> can add a comment that the preferred compatible value is the new one, but the
> old ones still need to be handled by the drivers, and shouldn't be removed.
> 
> That is, of course, if people are happy with the current binding and want to
> keep it instead of revising it before declaring it locked in.

The first -sp / -osc separation was bad idea from the start. It would
be better if we could get rid of it.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH 0/2] Fix potential merge conflict for dw_apb_timer_of

[dinguyen at altera.com]

From: Dinh Nguyen <dinguyen@altera.com>

Hi Arnd/Olof,

Because of the following patch series that is currently in arm-soc/for-next:

10021488997317d1121505a7ac659124c058efed clocksource: dw_apb_timer_of: use clocksource_of_init
1b4eca0f634be2a99f2baa6c29dfd183590ead3f clocksource: dw_apb_timer_of: select DW_APB_TIMER
a8b447f2bbbba737ff4478f498d7f83c75a9461b clocksource: dw_apb_timer_of: add clock-handling
a1198f83407ae3421f3f58355a0f296d5ea6249c clocksource: dw_apb_timer_of: enable the use the clocksource as sched clock

there will be a merge conflict with:

55a68c23e0a675b2b8ac2656fd6edbf98b78e4c6 dw_apb_timer_of.c: Remove parts that were picoxcell-specific

that is currently in John Stultz's tree fortglx/3.11/time.

The following 2 patches will eliminate the need for the patch in John
Stultz's tree. If there is to be merge of the 2 trees, then the
patch:

dw_apb_timer_of.c: Remove parts that were picoxcell-specific

can be removed from John's tree to avoid a merge-conflict.

Based on arm-soc/for-next:

PATCH[1/2] - Rename "dw-apb-timer-osc" and "dw-apb-timer-sp" bindings to just
"dw-apb-timer"
PATCH[2/2] - Fix user/system reporting by fixing read_sched_clock()

Thanks,
Dinh

Dinh Nguyen (2):
  ARM: dts: Change dw-apb-timer-osc and dw-apb-timer-sp to just
    dw-apb-timer
  clocksource: dw_apb_timer_of: Fix read_sched_clock

 Documentation/devicetree/bindings/rtc/dw-apb.txt |   21 +++------------------
 arch/arm/boot/dts/socfpga.dtsi                   |   12 ++++++++----
 drivers/clocksource/dw_apb_timer_of.c            |    7 +++----
 3 files changed, 14 insertions(+), 26 deletions(-)


CC: Arnd Bergmann <arnd@arndb.de>
Cc: Olof Johansson <olof@lixom.net>
CC: Jamie Iles <jamie@jamieiles.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Pavel Machek <pavel@denx.de>
Cc: devicetree-discuss at lists.ozlabs.org
Cc: linux-arm-kernel at lists.infradead.org

-- 
1.7.9.5

[PATCH 2/2] clocksource: dw_apb_timer_of: Fix read_sched_clock

[dinguyen at altera.com]

From: Dinh Nguyen <dinguyen@altera.com>

The read_sched_clock should return the ~value because the clock is a
countdown implementation. read_sched_clock() should be the same as
__apbt_read_clocksource().

If a separate timer for the sched_clock exist, then read_sched_clock()
will return an incorrect value. The (sched_io_base + 0x4) needs to be in
the function for both cases.

Also, remove the use of "dw-apb-timer-sp" and "dw-apb-timer-osc" since
they are the same DW clock.

Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
CC: Rob Herring <rob.herring@calxeda.com>
CC: Arnd Bergmann <arnd@arndb.de>
Cc: Olof Johansson <olof@lixom.net>
CC: Jamie Iles <jamie@jamieiles.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Pavel Machek <pavel@denx.de>
Cc: Heiko Stuebner <heiko@sntech.de>
Cc: linux-arm-kernel at lists.infradead.org
---
 drivers/clocksource/dw_apb_timer_of.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/clocksource/dw_apb_timer_of.c b/drivers/clocksource/dw_apb_timer_of.c
index cef5544..8dcbd4e 100644
--- a/drivers/clocksource/dw_apb_timer_of.c
+++ b/drivers/clocksource/dw_apb_timer_of.c
@@ -104,18 +104,17 @@ static void add_clocksource(struct device_node *source_timer)
 	 * timer is found. sched_io_base then points to the current_value
 	 * register of the clocksource timer.
 	 */
-	sched_io_base = iobase + 0x04;
+	sched_io_base = iobase;
 	sched_rate = rate;
 }
 
 static u32 read_sched_clock(void)
 {
-	return __raw_readl(sched_io_base);
+	return ~__raw_readl(sched_io_base + 0x4);
 }
 
 static const struct of_device_id sptimer_ids[] __initconst = {
 	{ .compatible = "picochip,pc3x2-rtc" },
-	{ .compatible = "snps,dw-apb-timer-sp" },
 	{ /* Sentinel */ },
 };
 
@@ -155,4 +154,4 @@ static void __init dw_apb_timer_init(struct device_node *timer)
 	num_called++;
 }
 CLOCKSOURCE_OF_DECLARE(pc3x2_timer, "picochip,pc3x2-timer", dw_apb_timer_init);
-CLOCKSOURCE_OF_DECLARE(apb_timer, "snps,dw-apb-timer-osc", dw_apb_timer_init);
+CLOCKSOURCE_OF_DECLARE(apb_timer, "snps,dw-apb-timer", dw_apb_timer_init);
-- 
1.7.9.5

[PATCH 2/2] clocksource: dw_apb_timer_of: Fix read_sched_clock

[Jamie Iles]

Hi Dinh,

On Mon, Jun 17, 2013 at 07:08:49PM -0500, dinguyen at altera.com wrote:
> From: Dinh Nguyen <dinguyen@altera.com>
> 
> The read_sched_clock should return the ~value because the clock is a
> countdown implementation. read_sched_clock() should be the same as
> __apbt_read_clocksource().
> 
> If a separate timer for the sched_clock exist, then read_sched_clock()
> will return an incorrect value. The (sched_io_base + 0x4) needs to be in
> the function for both cases.

Actually the old behaviour is correct for picoxcell as we're using the 
DesignWare RTC rather than the counter block and the RTC does count 
upwards, but your change will work as we'll start using the regular 
timers anyway.

Looks good to me though.

Acked-by: Jamie Iles <jamie@jamieiles.com>

for the series.

Jamie