[PATCH v2 0/3] ARM: vf610: Fix/add clocksource

[PATCH v2 0/3] ARM: vf610: Fix/add clocksource

[Stefan Agner]

This patchset came out of need for a clocksource which supports shutdown.
In the first version I proposed to just replace the current clocksource
with the ARM Global Timer and even proposed to remove the PIT clocksource
driver. But Bill Pringlemeir brought up a reasonable point why we should
keep the PIT clocksource.

Since we need shutdown support for suspend/resume, I added another patch
which fixes shutdown functionality for the PIT driver. Now it's possible
to use suspend/resume functionality with each of the two clocksources.

Changes in v2:
- Added shutdown support for PIT clocksource (and hence clocksource
  maintainers as receipient).
- Create Kconfig entries which allow to choose between the two
  clocksources
- Clarified commit message
- Renamed device tree node to "timer"

Stefan Agner (3):
  clocksource: vf_pit_timer: Support shutdown mode
  ARM: dts: vf610: Add ARM Global Timer
  ARM: vf610: Add ARM Global Timer clocksource option

 arch/arm/boot/dts/vf610.dtsi       |  8 ++++++++
 arch/arm/mach-imx/Kconfig          | 19 ++++++++++++++++++-
 drivers/clocksource/vf_pit_timer.c |  5 +++++
 3 files changed, 31 insertions(+), 1 deletion(-)

-- 
2.1.0

[PATCH v2 1/3] clocksource: vf_pit_timer: Support shutdown mode

[Stefan Agner]

In order to avoid waking up the system in a low power mode, the
clocksource should not generate interrupts anymore. Disable the PIT
timer interrupt when changing into the CLOCK_EVT_MODE_SHUTDOWN mode.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/clocksource/vf_pit_timer.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/clocksource/vf_pit_timer.c b/drivers/clocksource/vf_pit_timer.c
index a918bc4..eb354b4 100644
--- a/drivers/clocksource/vf_pit_timer.c
+++ b/drivers/clocksource/vf_pit_timer.c
@@ -89,10 +89,15 @@ static int pit_set_next_event(unsigned long delta,
 static void pit_set_mode(enum clock_event_mode mode,
 				struct clock_event_device *evt)
 {
+
 	switch (mode) {
 	case CLOCK_EVT_MODE_PERIODIC:
 		pit_set_next_event(cycle_per_jiffy, evt);
 		break;
+	case CLOCK_EVT_MODE_SHUTDOWN:
+	case CLOCK_EVT_MODE_UNUSED:
+		pit_timer_disable();
+		break;
 	default:
 		break;
 	}
-- 
2.1.0

[PATCH v2 1/3] clocksource: vf_pit_timer: Support shutdown mode

[Sergei Shtylyov]

Hello.

On 9/24/2014 8:20 PM, Stefan Agner wrote:

> In order to avoid waking up the system in a low power mode, the
> clocksource should not generate interrupts anymore. Disable the PIT
> timer interrupt when changing into the CLOCK_EVT_MODE_SHUTDOWN mode.

> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>   drivers/clocksource/vf_pit_timer.c | 5 +++++
>   1 file changed, 5 insertions(+)

> diff --git a/drivers/clocksource/vf_pit_timer.c b/drivers/clocksource/vf_pit_timer.c
> index a918bc4..eb354b4 100644
> --- a/drivers/clocksource/vf_pit_timer.c
> +++ b/drivers/clocksource/vf_pit_timer.c
> @@ -89,10 +89,15 @@ static int pit_set_next_event(unsigned long delta,
>   static void pit_set_mode(enum clock_event_mode mode,
>   				struct clock_event_device *evt)
>   {
> +

    scripts/checkpatch.pl should protest about this empty line, it's not 
needed at all.

WBR, Sergei

[PATCH v2 2/3] ARM: dts: vf610: Add ARM Global Timer

[Stefan Agner]

Add Global Timer support which is part of the private peripherals
of the Cortex-A5 processor. This Global Timer is compatible with the
Cortex-A9 implementation. It's a 64-bit timer and is clocked by the
peripheral clock, which is typically 133 or 166MHz on Vybrid.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 arch/arm/boot/dts/vf610.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/vf610.dtsi b/arch/arm/boot/dts/vf610.dtsi
index 4d2ec32..3232b9a 100644
--- a/arch/arm/boot/dts/vf610.dtsi
+++ b/arch/arm/boot/dts/vf610.dtsi
@@ -11,6 +11,7 @@
 #include "vf610-pinfunc.h"
 #include <dt-bindings/clock/vf610-clock.h>
 #include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
 
 / {
 	aliases {
@@ -83,6 +84,13 @@
 				      <0x40002100 0x100>;
 			};
 
+			timer: global-timer at 40002200 {
+				compatible = "arm,cortex-a9-global-timer";
+				reg = <0x40002200 0x20>;
+				interrupts = <GIC_PPI 11 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clks VF610_CLK_PLATFORM_BUS>;
+			};
+
 			L2: l2-cache at 40006000 {
 				compatible = "arm,pl310-cache";
 				reg = <0x40006000 0x1000>;
-- 
2.1.0

[PATCH v2 2/3] ARM: dts: vf610: Add ARM Global Timer

[Shawn Guo]

On Wed, Sep 24, 2014 at 06:20:09PM +0200, Stefan Agner wrote:
> Add Global Timer support which is part of the private peripherals
> of the Cortex-A5 processor. This Global Timer is compatible with the
> Cortex-A9 implementation. It's a 64-bit timer and is clocked by the
> peripheral clock, which is typically 133 or 166MHz on Vybrid.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  arch/arm/boot/dts/vf610.dtsi | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/vf610.dtsi b/arch/arm/boot/dts/vf610.dtsi
> index 4d2ec32..3232b9a 100644
> --- a/arch/arm/boot/dts/vf610.dtsi
> +++ b/arch/arm/boot/dts/vf610.dtsi
> @@ -11,6 +11,7 @@
>  #include "vf610-pinfunc.h"
>  #include <dt-bindings/clock/vf610-clock.h>
>  #include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>  
>  / {
>  	aliases {
> @@ -83,6 +84,13 @@
>  				      <0x40002100 0x100>;
>  			};
>  
> +			timer: global-timer at 40002200 {

The renaming to "timer" should be applied on node name, not the label
name.  I change it as below and applied the patch.

	global_timer: timer at 40002200

Shawn

> +				compatible = "arm,cortex-a9-global-timer";
> +				reg = <0x40002200 0x20>;
> +				interrupts = <GIC_PPI 11 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clks VF610_CLK_PLATFORM_BUS>;
> +			};
> +
>  			L2: l2-cache at 40006000 {
>  				compatible = "arm,pl310-cache";
>  				reg = <0x40006000 0x1000>;
> -- 
> 2.1.0
> 

[PATCH v2 3/3] ARM: vf610: Add ARM Global Timer clocksource option

[Stefan Agner]

Add the ARM Global Timer as clocksource/scheduler clock option and
use it as default scheduler clock. This leaves the PIT timer for
other users e.g. the secondary Cortex-M4 core. Also, the Global Timer
has double the precission (running at pheripheral clock compared to
IPG clock) and a 64-bit incrementing counter register. We still keep
the PIT timer as an secondary option in case the ARM Global Timer is
not available.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 arch/arm/mach-imx/Kconfig | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index 11b2957..9482f58 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -633,12 +633,29 @@ config SOC_VF610
 	bool "Vybrid Family VF610 support"
 	select ARM_GIC
 	select PINCTRL_VF610
-	select VF_PIT_TIMER
 	select PL310_ERRATA_769419 if CACHE_L2X0
 
 	help
 	  This enable support for Freescale Vybrid VF610 processor.
 
+choice
+	prompt "Clocksource for scheduler clock"
+	depends on SOC_VF610
+	default ARM_GLOBAL_TIMER
+
+	config ARM_GLOBAL_TIMER
+		bool "Use ARM Global Timer"
+		select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
+		help
+		  Use the ARM Global Timer as clocksource
+
+	config VF_PIT_TIMER
+		bool "Use PIT timer"
+		help
+		  Use SoC Periodic Interrupt Timer (PIT) as clocksource
+
+endchoice
+
 endif
 
 source "arch/arm/mach-imx/devices/Kconfig"
-- 
2.1.0

[PATCH v2 3/3] ARM: vf610: Add ARM Global Timer clocksource option

[Shawn Guo]

On Wed, Sep 24, 2014 at 06:20:10PM +0200, Stefan Agner wrote:
> Add the ARM Global Timer as clocksource/scheduler clock option and
> use it as default scheduler clock. This leaves the PIT timer for
> other users e.g. the secondary Cortex-M4 core. Also, the Global Timer
> has double the precission (running at pheripheral clock compared to
> IPG clock) and a 64-bit incrementing counter register. We still keep
> the PIT timer as an secondary option in case the ARM Global Timer is
> not available.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>

Applied, thanks.

[PATCH v2 3/3] ARM: vf610: Add ARM Global Timer clocksource option

[Shawn Guo]

On Wed, Sep 24, 2014 at 06:20:10PM +0200, Stefan Agner wrote:
> Add the ARM Global Timer as clocksource/scheduler clock option and
> use it as default scheduler clock. This leaves the PIT timer for
> other users e.g. the secondary Cortex-M4 core. Also, the Global Timer
> has double the precission (running at pheripheral clock compared to
> IPG clock) and a 64-bit incrementing counter register. We still keep
> the PIT timer as an secondary option in case the ARM Global Timer is
> not available.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  arch/arm/mach-imx/Kconfig | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
> index 11b2957..9482f58 100644
> --- a/arch/arm/mach-imx/Kconfig
> +++ b/arch/arm/mach-imx/Kconfig
> @@ -633,12 +633,29 @@ config SOC_VF610
>  	bool "Vybrid Family VF610 support"
>  	select ARM_GIC
>  	select PINCTRL_VF610
> -	select VF_PIT_TIMER
>  	select PL310_ERRATA_769419 if CACHE_L2X0
>  
>  	help
>  	  This enable support for Freescale Vybrid VF610 processor.
>  
> +choice
> +	prompt "Clocksource for scheduler clock"
> +	depends on SOC_VF610
> +	default ARM_GLOBAL_TIMER
> +
> +	config ARM_GLOBAL_TIMER
> +		bool "Use ARM Global Timer"
> +		select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
> +		help
> +		  Use the ARM Global Timer as clocksource
> +
> +	config VF_PIT_TIMER
> +		bool "Use PIT timer"
> +		help
> +		  Use SoC Periodic Interrupt Timer (PIT) as clocksource
> +
> +endchoice
> +

As per the report from 0-DAY kernel build test (thanks), the change
causes the following warning on a particular config.

warning: (ARCH_ROCKCHIP && ARCH_BCM_5301X) selects CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK which has unmet direct dependencies (ARM_GLOBAL_TIMER)

It looks the redefinition of ARM_GLOBAL_TIMER is the cause for that
warning.  I think it's more safer to use new config symbols for choosing
the clocksource device for VF610.  I propose the following change to fix
the warning.  If there is no objection, I will squash it into the
original commit soon.

diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index 9482f5837b7e..e26b0fdd72db 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -641,16 +641,18 @@ config SOC_VF610
 choice
        prompt "Clocksource for scheduler clock"
        depends on SOC_VF610
-       default ARM_GLOBAL_TIMER
+       default VF_USE_ARM_GLOBAL_TIMER
 
-       config ARM_GLOBAL_TIMER
+       config VF_USE_ARM_GLOBAL_TIMER
                bool "Use ARM Global Timer"
+               select ARM_GLOBAL_TIMER
                select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
                help
                  Use the ARM Global Timer as clocksource
 
-       config VF_PIT_TIMER
+       config VF_USE_PIT_TIMER
                bool "Use PIT timer"
+               select VF_PIT_TIMER
                help
                  Use SoC Periodic Interrupt Timer (PIT) as clocksource
 

[PATCH v2 3/3] ARM: vf610: Add ARM Global Timer clocksource option

[Stefan Agner]

Am 2014-09-29 13:51, schrieb Shawn Guo:
> On Wed, Sep 24, 2014 at 06:20:10PM +0200, Stefan Agner wrote:
>> Add the ARM Global Timer as clocksource/scheduler clock option and
>> use it as default scheduler clock. This leaves the PIT timer for
>> other users e.g. the secondary Cortex-M4 core. Also, the Global Timer
>> has double the precission (running at pheripheral clock compared to
>> IPG clock) and a 64-bit incrementing counter register. We still keep
>> the PIT timer as an secondary option in case the ARM Global Timer is
>> not available.
>>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>>  arch/arm/mach-imx/Kconfig | 19 ++++++++++++++++++-
>>  1 file changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
>> index 11b2957..9482f58 100644
>> --- a/arch/arm/mach-imx/Kconfig
>> +++ b/arch/arm/mach-imx/Kconfig
>> @@ -633,12 +633,29 @@ config SOC_VF610
>>  	bool "Vybrid Family VF610 support"
>>  	select ARM_GIC
>>  	select PINCTRL_VF610
>> -	select VF_PIT_TIMER
>>  	select PL310_ERRATA_769419 if CACHE_L2X0
>>
>>  	help
>>  	  This enable support for Freescale Vybrid VF610 processor.
>>
>> +choice
>> +	prompt "Clocksource for scheduler clock"
>> +	depends on SOC_VF610
>> +	default ARM_GLOBAL_TIMER
>> +
>> +	config ARM_GLOBAL_TIMER
>> +		bool "Use ARM Global Timer"
>> +		select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
>> +		help
>> +		  Use the ARM Global Timer as clocksource
>> +
>> +	config VF_PIT_TIMER
>> +		bool "Use PIT timer"
>> +		help
>> +		  Use SoC Periodic Interrupt Timer (PIT) as clocksource
>> +
>> +endchoice
>> +
> 
> As per the report from 0-DAY kernel build test (thanks), the change
> causes the following warning on a particular config.
> 
> warning: (ARCH_ROCKCHIP && ARCH_BCM_5301X) selects
> CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK which has unmet direct
> dependencies (ARM_GLOBAL_TIMER)
> 
> It looks the redefinition of ARM_GLOBAL_TIMER is the cause for that
> warning.  I think it's more safer to use new config symbols for choosing
> the clocksource device for VF610.  I propose the following change to fix
> the warning.  If there is no objection, I will squash it into the
> original commit soon.
> 
> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
> index 9482f5837b7e..e26b0fdd72db 100644
> --- a/arch/arm/mach-imx/Kconfig
> +++ b/arch/arm/mach-imx/Kconfig
> @@ -641,16 +641,18 @@ config SOC_VF610
>  choice
>         prompt "Clocksource for scheduler clock"
>         depends on SOC_VF610
> -       default ARM_GLOBAL_TIMER
> +       default VF_USE_ARM_GLOBAL_TIMER
>  
> -       config ARM_GLOBAL_TIMER
> +       config VF_USE_ARM_GLOBAL_TIMER
>                 bool "Use ARM Global Timer"
> +               select ARM_GLOBAL_TIMER
>                 select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
>                 help
>                   Use the ARM Global Timer as clocksource
>  
> -       config VF_PIT_TIMER
> +       config VF_USE_PIT_TIMER
>                 bool "Use PIT timer"
> +               select VF_PIT_TIMER
>                 help
>                   Use SoC Periodic Interrupt Timer (PIT) as clocksource

I thought that Kconfig actually allowed to define a config symbol twice.
But maybe this is not a good idea with dependencies (depends on
SOC_VF610 on the choice level...?).

Thx Shawn, sorry about that.

Acked-by: Stefan Agner <stefan@agner.ch>

[PATCH v2 0/3] ARM: vf610: Fix/add clocksource

[Bill Pringlemeir]

On 24 Sep 2014, stefan at agner.ch wrote:

> This patchset came out of need for a clocksource which supports
> shutdown.  In the first version I proposed to just replace the current
> clocksource with the ARM Global Timer and even proposed to remove the
> PIT clocksource driver. But Bill Pringlemeir brought up a reasonable
> point why we should keep the PIT clocksource.
>
> Since we need shutdown support for suspend/resume, I added another
> patch which fixes shutdown functionality for the PIT driver. Now it's
> possible to use suspend/resume functionality with each of the two
> clocksources.
>
> Changes in v2:
> - Added shutdown support for PIT clocksource (and hence clocksource
> maintainers as receipient).
> - Create Kconfig entries which allow to choose between the two
> clocksources
> - Clarified commit message
> - Renamed device tree node to "timer"
>
> Stefan Agner (3):
> clocksource: vf_pit_timer: Support shutdown mode
> ARM: dts: vf610: Add ARM Global Timer
> ARM: vf610: Add ARM Global Timer clocksource option
>
> arch/arm/boot/dts/vf610.dtsi       |  8 ++++++++
> arch/arm/mach-imx/Kconfig          | 19 ++++++++++++++++++-
> drivers/clocksource/vf_pit_timer.c |  5 +++++
> 3 files changed, 31 insertions(+), 1 deletion(-)

Acked-by: Bill Pringlemeir <bpringlemeir@nbsps.com>

[PATCH v2 0/3] ARM: vf610: Fix/add clocksource

[Daniel Lezcano]

On 09/24/2014 06:20 PM, Stefan Agner wrote:
> This patchset came out of need for a clocksource which supports shutdown.
> In the first version I proposed to just replace the current clocksource
> with the ARM Global Timer and even proposed to remove the PIT clocksource
> driver. But Bill Pringlemeir brought up a reasonable point why we should
> keep the PIT clocksource.
>
> Since we need shutdown support for suspend/resume, I added another patch
> which fixes shutdown functionality for the PIT driver. Now it's possible
> to use suspend/resume functionality with each of the two clocksources.
>
> Changes in v2:
> - Added shutdown support for PIT clocksource (and hence clocksource
>    maintainers as receipient).
> - Create Kconfig entries which allow to choose between the two
>    clocksources
> - Clarified commit message
> - Renamed device tree node to "timer"
>

Hi Stefan,

I will take the patch 1/3 and let the other patches to be acked-by or 
picked in another tree.

Thanks
   -- Daniel

> Stefan Agner (3):
>    clocksource: vf_pit_timer: Support shutdown mode
>    ARM: dts: vf610: Add ARM Global Timer
>    ARM: vf610: Add ARM Global Timer clocksource option
>
>   arch/arm/boot/dts/vf610.dtsi       |  8 ++++++++
>   arch/arm/mach-imx/Kconfig          | 19 ++++++++++++++++++-
>   drivers/clocksource/vf_pit_timer.c |  5 +++++
>   3 files changed, 31 insertions(+), 1 deletion(-)
>


-- 
  <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[PATCH v2 0/3] ARM: vf610: Fix/add clocksource

[Stefan Agner]

Am 2014-09-26 09:01, schrieb Daniel Lezcano:
> On 09/24/2014 06:20 PM, Stefan Agner wrote:
>> This patchset came out of need for a clocksource which supports shutdown.
>> In the first version I proposed to just replace the current clocksource
>> with the ARM Global Timer and even proposed to remove the PIT clocksource
>> driver. But Bill Pringlemeir brought up a reasonable point why we should
>> keep the PIT clocksource.
>>
>> Since we need shutdown support for suspend/resume, I added another patch
>> which fixes shutdown functionality for the PIT driver. Now it's possible
>> to use suspend/resume functionality with each of the two clocksources.
>>
>> Changes in v2:
>> - Added shutdown support for PIT clocksource (and hence clocksource
>>    maintainers as receipient).
>> - Create Kconfig entries which allow to choose between the two
>>    clocksources
>> - Clarified commit message
>> - Renamed device tree node to "timer"
>>
> 
> Hi Stefan,
> 
> I will take the patch 1/3 and let the other patches to be acked-by or
> picked in another tree.
> 
> Thanks
>   -- Daniel

Hi Daniel, 

Great thx. If not already to late, can you drop that superfluous empty
line mentioned by Sergei?

--
Stefan

> 
>> Stefan Agner (3):
>>    clocksource: vf_pit_timer: Support shutdown mode
>>    ARM: dts: vf610: Add ARM Global Timer
>>    ARM: vf610: Add ARM Global Timer clocksource option
>>
>>   arch/arm/boot/dts/vf610.dtsi       |  8 ++++++++
>>   arch/arm/mach-imx/Kconfig          | 19 ++++++++++++++++++-
>>   drivers/clocksource/vf_pit_timer.c |  5 +++++
>>   3 files changed, 31 insertions(+), 1 deletion(-)
>>