diff mbox

[2/2] ARM: OMAP5: Enable arch timer support

Message ID 1344856045-15134-3-git-send-email-santosh.shilimkar@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Santosh Shilimkar Aug. 13, 2012, 11:07 a.m. UTC
Enable Cortex A15 generic timer support for OMAP5 based SOCs.
The CPU local timers run on the free running real time counter clock.

Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 arch/arm/boot/dts/omap5.dtsi |    6 ++++++
 arch/arm/mach-omap2/Kconfig  |    1 +
 arch/arm/mach-omap2/timer.c  |    7 +++++++
 3 files changed, 14 insertions(+)

Comments

Santosh Shilimkar Sept. 10, 2012, 11:45 a.m. UTC | #1
Benoit,

On Mon, Aug 13, 2012 at 4:37 PM, Santosh Shilimkar
<santosh.shilimkar@ti.com> wrote:
> Enable Cortex A15 generic timer support for OMAP5 based SOCs.
> The CPU local timers run on the free running real time counter clock.
>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> ---
>  arch/arm/boot/dts/omap5.dtsi |    6 ++++++
>  arch/arm/mach-omap2/Kconfig  |    1 +
>  arch/arm/mach-omap2/timer.c  |    7 +++++++
>  3 files changed, 14 insertions(+)
>
Missed to copy you on this patch. Your comments/ack
on the DT part.

Regards
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Benoit Cousson Sept. 10, 2012, 12:47 p.m. UTC | #2
Hi Santosh,

On 08/13/2012 01:07 PM, Santosh Shilimkar wrote:
> Enable Cortex A15 generic timer support for OMAP5 based SOCs.
> The CPU local timers run on the free running real time counter clock.
> 
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> ---
>  arch/arm/boot/dts/omap5.dtsi |    6 ++++++
>  arch/arm/mach-omap2/Kconfig  |    1 +
>  arch/arm/mach-omap2/timer.c  |    7 +++++++
>  3 files changed, 14 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
> index 57e5270..9686056 100644
> --- a/arch/arm/boot/dts/omap5.dtsi
> +++ b/arch/arm/boot/dts/omap5.dtsi
> @@ -73,6 +73,12 @@
>  			      <0x48212000 0x1000>;
>  		};
>  
> +		arch-timer {

arch-timer is the ARM specific name, so I guess here it should be named
with the generic timer name.

> +			compatible = "arm,armv7-timer";
> +			interrupts = <1 14 0x304>;

Could you add some comment, because these hexa value are a little bit
hard to understand.

> +			clock-frequency = <6140000>;
> +		};
> +

That node does not even have a base address?
If this is located inside the MPU, it should not be in the OCP node.

Silly question: Don't we have one arch-timer per CPU?

Regards,
Benoit


>  		gpio1: gpio@4ae10000 {
>  			compatible = "ti,omap4-gpio";
>  			ti,hwmods = "gpio1";
> diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> index 2120f90..53fb77c 100644
> --- a/arch/arm/mach-omap2/Kconfig
> +++ b/arch/arm/mach-omap2/Kconfig
> @@ -73,6 +73,7 @@ config SOC_OMAP5
>  	select ARM_GIC
>  	select HAVE_SMP
>  	select SOC_HAS_REALTIME_COUNTER
> +	select ARM_ARCH_TIMER
>  
>  comment "OMAP Core Type"
>  	depends on ARCH_OMAP2
> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
> index 9b17e6c..f74dbb2 100644
> --- a/arch/arm/mach-omap2/timer.c
> +++ b/arch/arm/mach-omap2/timer.c
> @@ -41,6 +41,7 @@
>  #include <plat/dmtimer.h>
>  #include <asm/smp_twd.h>
>  #include <asm/sched_clock.h>
> +#include <asm/arch_timer.h>
>  #include "common.h"
>  #include <plat/omap_hwmod.h>
>  #include <plat/omap_device.h>
> @@ -480,9 +481,15 @@ OMAP_SYS_TIMER(4)
>  #ifdef CONFIG_SOC_OMAP5
>  static void __init omap5_timer_init(void)
>  {
> +	int err;
> +
>  	omap2_gp_clockevent_init(1, OMAP4_CLKEV_SOURCE);
>  	omap2_clocksource_init(2, OMAP4_MPU_SOURCE);
>  	realtime_counter_init();
> +
> +	err = arch_timer_of_register();
> +	if (err)
> +		pr_err("%s: arch_timer_register failed %d\n", __func__, err);
>  }
>  OMAP_SYS_TIMER(5)
>  #endif
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Santosh Shilimkar Sept. 10, 2012, 1:01 p.m. UTC | #3
On Mon, Sep 10, 2012 at 6:17 PM, Benoit Cousson <b-cousson@ti.com> wrote:
>
> Hi Santosh,
>
> On 08/13/2012 01:07 PM, Santosh Shilimkar wrote:
> > Enable Cortex A15 generic timer support for OMAP5 based SOCs.
> > The CPU local timers run on the free running real time counter clock.
> >
> > Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> > ---
> >  arch/arm/boot/dts/omap5.dtsi |    6 ++++++
> >  arch/arm/mach-omap2/Kconfig  |    1 +
> >  arch/arm/mach-omap2/timer.c  |    7 +++++++
> >  3 files changed, 14 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
> > index 57e5270..9686056 100644
> > --- a/arch/arm/boot/dts/omap5.dtsi
> > +++ b/arch/arm/boot/dts/omap5.dtsi
> > @@ -73,6 +73,12 @@
> >                             <0x48212000 0x1000>;
> >               };
> >
> > +             arch-timer {
>
> arch-timer is the ARM specific name, so I guess here it should be named
> with the generic timer name.
>
is "local_timer" name fine then?

> > +                     compatible = "arm,armv7-timer";
> > +                     interrupts = <1 14 0x304>;
>
> Could you add some comment, because these hexa value are a little bit
> hard to understand.
>
OK. Will add some comments.

> > +                     clock-frequency = <6140000>;
> > +             };
> > +
>
> That node does not even have a base address?
> If this is located inside the MPU, it should not be in the OCP node.
>
Its inside MPU and Cp15 control based. No OCP node.

> Silly question: Don't we have one arch-timer per CPU?
>
It is per CPU just like A9 TWD

Regards
santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Benoit Cousson Sept. 10, 2012, 1:14 p.m. UTC | #4
On 09/10/2012 03:01 PM, Shilimkar, Santosh wrote:
> On Mon, Sep 10, 2012 at 6:17 PM, Benoit Cousson <b-cousson@ti.com> wrote:
>>
>> Hi Santosh,
>>
>> On 08/13/2012 01:07 PM, Santosh Shilimkar wrote:
>>> Enable Cortex A15 generic timer support for OMAP5 based SOCs.
>>> The CPU local timers run on the free running real time counter clock.
>>>
>>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>>> ---
>>>  arch/arm/boot/dts/omap5.dtsi |    6 ++++++
>>>  arch/arm/mach-omap2/Kconfig  |    1 +
>>>  arch/arm/mach-omap2/timer.c  |    7 +++++++
>>>  3 files changed, 14 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
>>> index 57e5270..9686056 100644
>>> --- a/arch/arm/boot/dts/omap5.dtsi
>>> +++ b/arch/arm/boot/dts/omap5.dtsi
>>> @@ -73,6 +73,12 @@
>>>                             <0x48212000 0x1000>;
>>>               };
>>>
>>> +             arch-timer {
>>
>> arch-timer is the ARM specific name, so I guess here it should be named
>> with the generic timer name.
>>
> is "local_timer" name fine then?

No, *timer* is fine. The point here is to provide the generic name when
it exists. That name is supposed to be the general class of the device.

Potentially you can add a label to give an unique name, but since that
label will not be used elsewhere it is not even needed.

arch-timer: timer { ... }

> 
>>> +                     compatible = "arm,armv7-timer";
>>> +                     interrupts = <1 14 0x304>;
>>
>> Could you add some comment, because these hexa value are a little bit
>> hard to understand.
>>
> OK. Will add some comments.
> 
>>> +                     clock-frequency = <6140000>;
>>> +             };
>>> +
>>
>> That node does not even have a base address?
>> If this is located inside the MPU, it should not be in the OCP node.
>>
> Its inside MPU and Cp15 control based. No OCP node.

OK, so you must move it inside the CPU node.

>> Silly question: Don't we have one arch-timer per CPU?
>>
> It is per CPU just like A9 TWD

Shouldn't we have two nodes then?

Regards,
Benoit


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Santosh Shilimkar Sept. 10, 2012, 1:39 p.m. UTC | #5
On Mon, Sep 10, 2012 at 6:44 PM, Benoit Cousson <b-cousson@ti.com> wrote:
>
> On 09/10/2012 03:01 PM, Shilimkar, Santosh wrote:
> > On Mon, Sep 10, 2012 at 6:17 PM, Benoit Cousson <b-cousson@ti.com>
> > wrote:
> >>
> >> Hi Santosh,
> >>
> >> On 08/13/2012 01:07 PM, Santosh Shilimkar wrote:
> >>> Enable Cortex A15 generic timer support for OMAP5 based SOCs.
> >>> The CPU local timers run on the free running real time counter clock.
> >>>
> >>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> >>> ---
> >>>  arch/arm/boot/dts/omap5.dtsi |    6 ++++++
> >>>  arch/arm/mach-omap2/Kconfig  |    1 +
> >>>  arch/arm/mach-omap2/timer.c  |    7 +++++++
> >>>  3 files changed, 14 insertions(+)
> >>>
> >>> diff --git a/arch/arm/boot/dts/omap5.dtsi
> >>> b/arch/arm/boot/dts/omap5.dtsi
> >>> index 57e5270..9686056 100644
> >>> --- a/arch/arm/boot/dts/omap5.dtsi
> >>> +++ b/arch/arm/boot/dts/omap5.dtsi
> >>> @@ -73,6 +73,12 @@
> >>>                             <0x48212000 0x1000>;
> >>>               };
> >>>
> >>> +             arch-timer {
> >>
> >> arch-timer is the ARM specific name, so I guess here it should be named
> >> with the generic timer name.
> >>
> > is "local_timer" name fine then?
>
> No, *timer* is fine. The point here is to provide the generic name when
> it exists. That name is supposed to be the general class of the device.
>
> Potentially you can add a label to give an unique name, but since that
> label will not be used elsewhere it is not even needed.
>
> arch-timer: timer { ... }
>
Ok. Will use this.

> >
> >>> +                     compatible = "arm,armv7-timer";
> >>> +                     interrupts = <1 14 0x304>;
> >>
> >> Could you add some comment, because these hexa value are a little bit
> >> hard to understand.
> >>
> > OK. Will add some comments.
> >
> >>> +                     clock-frequency = <6140000>;
> >>> +             };
> >>> +
> >>
> >> That node does not even have a base address?
> >> If this is located inside the MPU, it should not be in the OCP node.
> >>
> > Its inside MPU and Cp15 control based. No OCP node.
>
> OK, so you must move it inside the CPU node.
>
OK. Will do.

> >> Silly question: Don't we have one arch-timer per CPU?
> >>
> > It is per CPU just like A9 TWD
>
> Shouldn't we have two nodes then?
>
I need to check this but arch timer DT node should be same
as the twd DT node. May be one node with reference to
each CPU node should do but am not too sure about the DT
nodes and how all that work.

Regards
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
index 57e5270..9686056 100644
--- a/arch/arm/boot/dts/omap5.dtsi
+++ b/arch/arm/boot/dts/omap5.dtsi
@@ -73,6 +73,12 @@ 
 			      <0x48212000 0x1000>;
 		};
 
+		arch-timer {
+			compatible = "arm,armv7-timer";
+			interrupts = <1 14 0x304>;
+			clock-frequency = <6140000>;
+		};
+
 		gpio1: gpio@4ae10000 {
 			compatible = "ti,omap4-gpio";
 			ti,hwmods = "gpio1";
diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
index 2120f90..53fb77c 100644
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -73,6 +73,7 @@  config SOC_OMAP5
 	select ARM_GIC
 	select HAVE_SMP
 	select SOC_HAS_REALTIME_COUNTER
+	select ARM_ARCH_TIMER
 
 comment "OMAP Core Type"
 	depends on ARCH_OMAP2
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 9b17e6c..f74dbb2 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -41,6 +41,7 @@ 
 #include <plat/dmtimer.h>
 #include <asm/smp_twd.h>
 #include <asm/sched_clock.h>
+#include <asm/arch_timer.h>
 #include "common.h"
 #include <plat/omap_hwmod.h>
 #include <plat/omap_device.h>
@@ -480,9 +481,15 @@  OMAP_SYS_TIMER(4)
 #ifdef CONFIG_SOC_OMAP5
 static void __init omap5_timer_init(void)
 {
+	int err;
+
 	omap2_gp_clockevent_init(1, OMAP4_CLKEV_SOURCE);
 	omap2_clocksource_init(2, OMAP4_MPU_SOURCE);
 	realtime_counter_init();
+
+	err = arch_timer_of_register();
+	if (err)
+		pr_err("%s: arch_timer_register failed %d\n", __func__, err);
 }
 OMAP_SYS_TIMER(5)
 #endif