linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] ARM: dts: vf610: Add ARM Global Timer
@ 2014-09-11 12:06 Stefan Agner
  2014-09-11 12:06 ` [PATCH 2/2] ARM: vf610: Use ARM Global Timer as clocksource Stefan Agner
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Stefan Agner @ 2014-09-11 12:06 UTC (permalink / raw)
  To: linux-arm-kernel

Add Global Timer support which is part of the Snoop Control Unit
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..a03abf6 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>;
 			};
 
+			global_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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] ARM: vf610: Use ARM Global Timer as clocksource
  2014-09-11 12:06 [PATCH 1/2] ARM: dts: vf610: Add ARM Global Timer Stefan Agner
@ 2014-09-11 12:06 ` Stefan Agner
  2014-09-25  7:50   ` Shawn Guo
  2014-09-12  7:59 ` [PATCH 1/2] ARM: dts: vf610: Add ARM Global Timer Shawn Guo
  2014-09-23 15:54 ` Bill Pringlemeir
  2 siblings, 1 reply; 12+ messages in thread
From: Stefan Agner @ 2014-09-11 12:06 UTC (permalink / raw)
  To: linux-arm-kernel

Use ARM Global Timer as clocksource instead of the PIT timer. 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.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
Theoretically we could remove the PIT driver now. But we could also
enable both drivers, but is having two clock source useful for the
Kernel at all?

 arch/arm/mach-imx/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index 64161aa..adc77180 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -656,7 +656,8 @@ config SOC_VF610
 	bool "Vybrid Family VF610 support"
 	select ARM_GIC
 	select PINCTRL_VF610
-	select VF_PIT_TIMER
+	select ARM_GLOBAL_TIMER
+	select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
 	select PL310_ERRATA_769419 if CACHE_L2X0
 
 	help
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 1/2] ARM: dts: vf610: Add ARM Global Timer
  2014-09-11 12:06 [PATCH 1/2] ARM: dts: vf610: Add ARM Global Timer Stefan Agner
  2014-09-11 12:06 ` [PATCH 2/2] ARM: vf610: Use ARM Global Timer as clocksource Stefan Agner
@ 2014-09-12  7:59 ` Shawn Guo
  2014-09-19 14:32   ` Stefan Agner
  2014-09-23 15:54 ` Bill Pringlemeir
  2 siblings, 1 reply; 12+ messages in thread
From: Shawn Guo @ 2014-09-12  7:59 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Sep 11, 2014 at 02:06:14PM +0200, Stefan Agner wrote:
> Add Global Timer support which is part of the Snoop Control Unit
> 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..a03abf6 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>;
>  			};
>  
> +			global_timer: global-timer at 40002200 {

I think it's more idiomatic to use the generic name 'timer' for the
node.

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
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] ARM: dts: vf610: Add ARM Global Timer
  2014-09-12  7:59 ` [PATCH 1/2] ARM: dts: vf610: Add ARM Global Timer Shawn Guo
@ 2014-09-19 14:32   ` Stefan Agner
  0 siblings, 0 replies; 12+ messages in thread
From: Stefan Agner @ 2014-09-19 14:32 UTC (permalink / raw)
  To: linux-arm-kernel

Am 2014-09-12 09:59, schrieb Shawn Guo:
> On Thu, Sep 11, 2014 at 02:06:14PM +0200, Stefan Agner wrote:
>> Add Global Timer support which is part of the Snoop Control Unit
>> 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..a03abf6 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>;
>>  			};
>>
>> +			global_timer: global-timer at 40002200 {
> 
> I think it's more idiomatic to use the generic name 'timer' for the
> node.

Agree to that. I guess you can change that when you apply it, in case
there are no other objections to this patchset..?

> 
> 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
>>


--
Stefan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] ARM: dts: vf610: Add ARM Global Timer
  2014-09-11 12:06 [PATCH 1/2] ARM: dts: vf610: Add ARM Global Timer Stefan Agner
  2014-09-11 12:06 ` [PATCH 2/2] ARM: vf610: Use ARM Global Timer as clocksource Stefan Agner
  2014-09-12  7:59 ` [PATCH 1/2] ARM: dts: vf610: Add ARM Global Timer Shawn Guo
@ 2014-09-23 15:54 ` Bill Pringlemeir
  2014-09-24  7:55   ` Stefan Agner
  2 siblings, 1 reply; 12+ messages in thread
From: Bill Pringlemeir @ 2014-09-23 15:54 UTC (permalink / raw)
  To: linux-arm-kernel

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

> Add Global Timer support which is part of the Snoop Control Unit
> 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(+)

As per the GPC and SRC series, adding these peripherals to the
'vf610.dtsi' may make some configuration no longer boot.  I have an
Cortex-A5 MQX in the secure world and it uses the Global timer for the
OS tick.  Maybe that is just my problem and I need to have several
trees.  However, It would be nice if the system timer choice was made in
a '.config' and the machine DT and not the generic Vybrid one.

Also, the timer is listed in the same bank as the snoop control unit,
but it is part of several banks of registers,

 0x40002000 AIPS slot 2, CA5-SCU+GIC CPU Interface registers 1

 0x000-0x054 SCU 
 0x100-0x1fc GIC local registers
 0x200-0x218 Global Timer
 0x600-0x634 Local timer/watchdog timer

I think that the 'reg' mapping will be limited to 4k MMU pages and so we
will have a bunch of aliases.  At least the GIC registers are already
mapped.  Is there some way in the DT to provide several sets of
registers under one mapping and then use the different offsets in the
driver/device instance?

Fwiw,
Bill Pringlemeir.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] ARM: dts: vf610: Add ARM Global Timer
  2014-09-23 15:54 ` Bill Pringlemeir
@ 2014-09-24  7:55   ` Stefan Agner
  2014-09-24 15:26     ` Bill Pringlemeir
  0 siblings, 1 reply; 12+ messages in thread
From: Stefan Agner @ 2014-09-24  7:55 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Bill,

Am 2014-09-23 17:54, schrieb Bill Pringlemeir:
> On 11 Sep 2014, stefan at agner.ch wrote:
> 
>> Add Global Timer support which is part of the Snoop Control Unit
>> 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(+)
> 
> As per the GPC and SRC series, adding these peripherals to the
> 'vf610.dtsi' may make some configuration no longer boot.  I have an
> Cortex-A5 MQX in the secure world and it uses the Global timer for the
> OS tick.  Maybe that is just my problem and I need to have several
> trees.  However, It would be nice if the system timer choice was made in
> a '.config' and the machine DT and not the generic Vybrid one.

So MQX is running "beneath" Linux and steals CPU from it? And this works
with an unmodified Linux kernel? Did not know that this is possible and
implemented for Vybrid.

I just thought that it would be nice to have the PIT timer free for the
M4, and hence use the Cortex-A5 private ARM Global Timer.

But if there are reasons to not use the Global Timer but opt for the PIT
timer, I guess we could create a configuration here.


> Also, the timer is listed in the same bank as the snoop control unit,
> but it is part of several banks of registers,
> 
>  0x40002000 AIPS slot 2, CA5-SCU+GIC CPU Interface registers 1
> 
>  0x000-0x054 SCU 
>  0x100-0x1fc GIC local registers
>  0x200-0x218 Global Timer
>  0x600-0x634 Local timer/watchdog timer
> 
> I think that the 'reg' mapping will be limited to 4k MMU pages and so we
> will have a bunch of aliases.  At least the GIC registers are already
> mapped.  Is there some way in the DT to provide several sets of
> registers under one mapping and then use the different offsets in the
> driver/device instance?

You can define multiple address/size tuples in one reg property, as its
done for the GIC:

intc: interrupt-controller at 40002000 {
	compatible = "arm,cortex-a9-gic";
	#interrupt-cells = <3>;
	interrupt-controller;
	reg = <0x40003000 0x1000>,
	      <0x40002100 0x100>;
};

The device tree should describe the hardware, and when it comes to GIC
and Global Timer it quite accurate: For the Global Timer, I only map
those 0x20 registers (actually its one 32-bit register too many, but I
don't think this is used for anything). The GIC mapping is only mapping
0x100-0x1ff of the snoop contorl unit bank.

Both, the GIC driver as well as the ARM Global Timer use of_iomap on
those registers. It apparently works.

--
Stefan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] ARM: dts: vf610: Add ARM Global Timer
  2014-09-24  7:55   ` Stefan Agner
@ 2014-09-24 15:26     ` Bill Pringlemeir
  0 siblings, 0 replies; 12+ messages in thread
From: Bill Pringlemeir @ 2014-09-24 15:26 UTC (permalink / raw)
  To: linux-arm-kernel

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

> Am 2014-09-23 17:54, schrieb Bill Pringlemeir:
>> On 11 Sep 2014, stefan at agner.ch wrote:
>>
>>> Add Global Timer support which is part of the Snoop Control Unit
>>> 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(+)
>>
>> As per the GPC and SRC series, adding these peripherals to the
>> 'vf610.dtsi' may make some configuration no longer boot.  I have an
>> Cortex-A5 MQX in the secure world and it uses the Global timer for
>> the OS tick.  Maybe that is just my problem and I need to have
>> several trees.  However, It would be nice if the system timer choice
>> was made in a '.config' and the machine DT and not the generic Vybrid
>> one.

> So MQX is running "beneath" Linux and steals CPU from it? And this
> works with an unmodified Linux kernel? Did not know that this is
> possible and implemented for Vybrid.

Yes, this was working well.  The only patch is,

diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index 076172b..4c560e7 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -1514,6 +1514,10 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
 
        data = of_match_node(l2x0_ids, np)->data;
 
+       /* Call monitor to turn on L2 cache. */
+       if(of_property_read_bool(np,"trustzone"))
+               asm(" .arch_extension sec\n smc #5\n");
+
        if (of_device_is_compatible(np, "arm,pl310-cache") &&
            of_property_read_bool(np, "arm,io-coherent"))
                data = &of_l2c310_coherent_data;

Ie, the 'normal' world Linux doesn't have permission to access the L2.
I think this may not be an issue if you don't use a 'bzImage'.  It seems
the compressed loader expect cache disabled, but then enables it.  Also,
the 'cache-l2x0.c' will see it exists and then try to enable it, if not
already enabled.  So, the compress boot needs it disabled, but the
'cache-l2x0.c' needs it enabled.

MQX needs more significant modifications.  Especially, the DMA support
needs to be disabled.   Probably the same for M4-MQX/A5-Linux.

> I just thought that it would be nice to have the PIT timer free for
> the M4, and hence use the Cortex-A5 private ARM Global Timer.

> But if there are reasons to not use the Global Timer but opt for the
> PIT timer, I guess we could create a configuration here.

Other use cases are the M4 is 'secure' and the A5 runs in normal world
without any secure TZ A5 accesses.  People may wish to do this to 'lock
down' an A5 Linux; Ie, restrict bus peripherals.  Here, whatever boot
loader could give permission for the Normal world to use the ARM global
timer.  I just think it should be an option as opposed to a default.

There is a stronger case to make this the default for the A5.  Only a
trustzone version would care about this on the Vybrid.  The M4 can not
access the global timer.

>> Also, the timer is listed in the same bank as the snoop control unit,
>> but it is part of several banks of registers,
>>
>> 0x40002000 AIPS slot 2, CA5-SCU+GIC CPU Interface registers 1
>>
>> 0x000-0x054 SCU 
>> 0x100-0x1fc GIC local registers
>> 0x200-0x218 Global Timer
>> 0x600-0x634 Local timer/watchdog timer
>>
>> I think that the 'reg' mapping will be limited to 4k MMU pages and so
>> we will have a bunch of aliases.  At least the GIC registers are
>> already mapped.  Is there some way in the DT to provide several sets
>> of registers under one mapping and then use the different offsets in
>> the driver/device instance?
>
> You can define multiple address/size tuples in one reg property, as
> its done for the GIC:
>
> intc: interrupt-controller at 40002000 {
> 	compatible = "arm,cortex-a9-gic";
> 	#interrupt-cells = <3>;
> 	interrupt-controller;
> 	reg = <0x40003000 0x1000>,
> 	      <0x40002100 0x100>;
> };

> The device tree should describe the hardware, and when it comes to GIC
> and Global Timer it quite accurate: For the Global Timer, I only map
> those 0x20 registers (actually its one 32-bit register too many, but I
> don't think this is used for anything). The GIC mapping is only
> mapping 0x100-0x1ff of the snoop contorl unit bank.

> Both, the GIC driver as well as the ARM Global Timer use of_iomap on
> those registers. It apparently works.

Yes, it works.  I just mean that we will have the same memory mapped
twice.  of_iomap() will be limited to 4k regions by the ARM hardware.
Both the 'interrupt-controller' and the 'global-timer' will have virtual
mappings to the same 4k physical page.

There seems to be no way for two drivers to share the same virtual
mapping; maybe Linux will reference count this and just provide an
offset to an existing mapping.  Sorry, I think that side tracked me.

Mainly the commit message is a little wrong,

> Add Global Timer support which is part of the Snoop Control Unit
> of the Cortex-A5 processor.

Maybe,

> Add Global Timer support which is part of the private peripherals
> of the Cortex-A5 processor.

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] ARM: vf610: Use ARM Global Timer as clocksource
  2014-09-11 12:06 ` [PATCH 2/2] ARM: vf610: Use ARM Global Timer as clocksource Stefan Agner
@ 2014-09-25  7:50   ` Shawn Guo
  2014-09-25  8:25     ` Stefan Agner
  0 siblings, 1 reply; 12+ messages in thread
From: Shawn Guo @ 2014-09-25  7:50 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Sep 11, 2014 at 02:06:15PM +0200, Stefan Agner wrote:
> Use ARM Global Timer as clocksource instead of the PIT timer. 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.

I just think of one thing.  Will this change cause a problem of the low
power idle support in case we want to power down ARM core in there?

Shawn

> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
> Theoretically we could remove the PIT driver now. But we could also
> enable both drivers, but is having two clock source useful for the
> Kernel at all?
> 
>  arch/arm/mach-imx/Kconfig | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
> index 64161aa..adc77180 100644
> --- a/arch/arm/mach-imx/Kconfig
> +++ b/arch/arm/mach-imx/Kconfig
> @@ -656,7 +656,8 @@ config SOC_VF610
>  	bool "Vybrid Family VF610 support"
>  	select ARM_GIC
>  	select PINCTRL_VF610
> -	select VF_PIT_TIMER
> +	select ARM_GLOBAL_TIMER
> +	select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
>  	select PL310_ERRATA_769419 if CACHE_L2X0
>  
>  	help
> -- 
> 2.1.0
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/2] ARM: vf610: Use ARM Global Timer as clocksource
  2014-09-25  7:50   ` Shawn Guo
@ 2014-09-25  8:25     ` Stefan Agner
  2014-09-25 15:10       ` Shawn Guo
  2014-09-25 22:12       ` Bill Pringlemeir
  0 siblings, 2 replies; 12+ messages in thread
From: Stefan Agner @ 2014-09-25  8:25 UTC (permalink / raw)
  To: linux-arm-kernel

Am 2014-09-25 09:50, schrieb Shawn Guo:
> On Thu, Sep 11, 2014 at 02:06:15PM +0200, Stefan Agner wrote:
>> Use ARM Global Timer as clocksource instead of the PIT timer. 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.
> 
> I just think of one thing.  Will this change cause a problem of the low
> power idle support in case we want to power down ARM core in there?
> 

I'm not sure what really happend to the Global Timer when we power down
the ARM core. We use a clocksoure of different power domain now, so it
might make a difference in low power modes, but I think it will improve
things: The PIT timer's clock currently have been clock gated even in
STOP mode, which does not power down the ARM core. And it would be shut
down completely in LP-Mode 1-3 which since PIT is part of the big power
domain 1. 

But AFAIK, its not required that the clocksource is running while in low
power modes. The time should just not jump, and if the timers registers
are lost during suspend, a proper suspend/resume support need to be
implemented. 


> 
>>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>> Theoretically we could remove the PIT driver now. But we could also
>> enable both drivers, but is having two clock source useful for the
>> Kernel at all?
>>
>>  arch/arm/mach-imx/Kconfig | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
>> index 64161aa..adc77180 100644
>> --- a/arch/arm/mach-imx/Kconfig
>> +++ b/arch/arm/mach-imx/Kconfig
>> @@ -656,7 +656,8 @@ config SOC_VF610
>>  	bool "Vybrid Family VF610 support"
>>  	select ARM_GIC
>>  	select PINCTRL_VF610
>> -	select VF_PIT_TIMER
>> +	select ARM_GLOBAL_TIMER
>> +	select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
>>  	select PL310_ERRATA_769419 if CACHE_L2X0
>>
>>  	help
>> --
>> 2.1.0
>>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/2] ARM: vf610: Use ARM Global Timer as clocksource
  2014-09-25  8:25     ` Stefan Agner
@ 2014-09-25 15:10       ` Shawn Guo
  2014-09-25 22:12       ` Bill Pringlemeir
  1 sibling, 0 replies; 12+ messages in thread
From: Shawn Guo @ 2014-09-25 15:10 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Sep 25, 2014 at 10:25:13AM +0200, Stefan Agner wrote:
> Am 2014-09-25 09:50, schrieb Shawn Guo:
> > On Thu, Sep 11, 2014 at 02:06:15PM +0200, Stefan Agner wrote:
> >> Use ARM Global Timer as clocksource instead of the PIT timer. 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.
> > 
> > I just think of one thing.  Will this change cause a problem of the low
> > power idle support in case we want to power down ARM core in there?
> > 
> 
> I'm not sure what really happend to the Global Timer when we power down
> the ARM core. We use a clocksoure of different power domain now, so it
> might make a difference in low power modes, but I think it will improve
> things: The PIT timer's clock currently have been clock gated even in
> STOP mode, which does not power down the ARM core. And it would be shut
> down completely in LP-Mode 1-3 which since PIT is part of the big power
> domain 1. 
> 
> But AFAIK, its not required that the clocksource is running while in low
> power modes. The time should just not jump, and if the timers registers
> are lost during suspend, a proper suspend/resume support need to be
> implemented. 

Sorry, I should be more specific in the first place.  What I'm concerned
is more about clockevent than clocksource.  If some day we have a
cpuidle driver for vf610, which powers off ARM core in a deep C-state,
the clockevent device will be gone as long as system enters the C-state,
and no timer interrupt can wake up the core from idle state.

Shawn

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/2] ARM: vf610: Use ARM Global Timer as clocksource
  2014-09-25  8:25     ` Stefan Agner
  2014-09-25 15:10       ` Shawn Guo
@ 2014-09-25 22:12       ` Bill Pringlemeir
  2014-09-26  0:48         ` Shawn Guo
  1 sibling, 1 reply; 12+ messages in thread
From: Bill Pringlemeir @ 2014-09-25 22:12 UTC (permalink / raw)
  To: linux-arm-kernel

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

> Am 2014-09-25 09:50, schrieb Shawn Guo:
>> On Thu, Sep 11, 2014 at 02:06:15PM +0200, Stefan Agner wrote:
>>> Use ARM Global Timer as clocksource instead of the PIT timer. 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.

>> I just think of one thing.  Will this change cause a problem of the
>> low power idle support in case we want to power down ARM core in
>> there?

> I'm not sure what really happend to the Global Timer when we power
> down the ARM core. We use a clocksoure of different power domain now,
> so it might make a difference in low power modes, but I think it will
> improve things: The PIT timer's clock currently have been clock gated
> even in STOP mode, which does not power down the ARM core. And it
> would be shut down completely in LP-Mode 1-3 which since PIT is part
> of the big power domain 1.

> But AFAIK, its not required that the clocksource is running while in
> low power modes. The time should just not jump, and if the timers
> registers are lost during suspend, a proper suspend/resume support
> need to be implemented.

The global timer is based off the 133/166Mhz BUSCLK.  I think the PIT is
also and I don't think you can clock the PIT with the 32Khz clock?  The
ARM MPCore-A5 has this to say,

   2.4.2 Power domains
   In addition, the SCU provides two more power domains:

   ? One for the SCU control logic and peripherals, such as the interrupt
     controller and timer/watchdog units, V SCU.

   The separate SCU power domains can remain active even when all the
   cores are powered down.

So, it seems that the global timer could remain powered.  However, we
need the GPC to have the 'Global timer' as a wakeup source.  I think the
PIT can wake the CPU via the PDB.

The Global timer may keep counting even when the Core is in some stop
modes as long as the BUSCLK is on.  However, I don't think it can wake
the CPU and it definitely can not run from the 32K clock which would be
the lowest power mode.  I think only the LPTMR and the FTM support this
clock.  The PIT seems to be as fragile as the ARM global timer, except
for wakeups.  If the BUSCLK changes due to frequency scaling, then both
will be affected.

The ARM global timer seems like it is more efficient to access than the
PIT which runs through the AXI bus.  For this reason, I thought it
should be the default.  I guess benchmarks would be nice?  At least
Stefan's patches give this opportunity.

Fwiw,
Bill Pringlemeir.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/2] ARM: vf610: Use ARM Global Timer as clocksource
  2014-09-25 22:12       ` Bill Pringlemeir
@ 2014-09-26  0:48         ` Shawn Guo
  0 siblings, 0 replies; 12+ messages in thread
From: Shawn Guo @ 2014-09-26  0:48 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Sep 25, 2014 at 06:12:24PM -0400, Bill Pringlemeir wrote:
> The global timer is based off the 133/166Mhz BUSCLK.  I think the PIT is
> also and I don't think you can clock the PIT with the 32Khz clock?  The
> ARM MPCore-A5 has this to say,
> 
>    2.4.2 Power domains
>    In addition, the SCU provides two more power domains:
> 
>    ? One for the SCU control logic and peripherals, such as the interrupt
>      controller and timer/watchdog units, V SCU.
> 
>    The separate SCU power domains can remain active even when all the
>    cores are powered down.
> 
> So, it seems that the global timer could remain powered.  However, we
> need the GPC to have the 'Global timer' as a wakeup source.  I think the
> PIT can wake the CPU via the PDB.
> 
> The Global timer may keep counting even when the Core is in some stop
> modes as long as the BUSCLK is on.  However, I don't think it can wake
> the CPU and it definitely can not run from the 32K clock which would be
> the lowest power mode.  I think only the LPTMR and the FTM support this
> clock.  The PIT seems to be as fragile as the ARM global timer, except
> for wakeups.  If the BUSCLK changes due to frequency scaling, then both
> will be affected.
> 
> The ARM global timer seems like it is more efficient to access than the
> PIT which runs through the AXI bus.  For this reason, I thought it
> should be the default.  I guess benchmarks would be nice?  At least
> Stefan's patches give this opportunity.

So it sounds like the global timer will keep counting even when ARM core
is powered down, and you're saying that ARM global timer doesn't do
anything worse than PIT and should be the default.  In that case, I'm
fine with the patch and will apply them shortly.

Shawn

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2014-09-26  0:48 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-11 12:06 [PATCH 1/2] ARM: dts: vf610: Add ARM Global Timer Stefan Agner
2014-09-11 12:06 ` [PATCH 2/2] ARM: vf610: Use ARM Global Timer as clocksource Stefan Agner
2014-09-25  7:50   ` Shawn Guo
2014-09-25  8:25     ` Stefan Agner
2014-09-25 15:10       ` Shawn Guo
2014-09-25 22:12       ` Bill Pringlemeir
2014-09-26  0:48         ` Shawn Guo
2014-09-12  7:59 ` [PATCH 1/2] ARM: dts: vf610: Add ARM Global Timer Shawn Guo
2014-09-19 14:32   ` Stefan Agner
2014-09-23 15:54 ` Bill Pringlemeir
2014-09-24  7:55   ` Stefan Agner
2014-09-24 15:26     ` Bill Pringlemeir

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).