diff mbox

[RFC,3/5] ARM: OMAP2+: timer: Add clocksource initialization and powerup support

Message ID 1394742919-32163-4-git-send-email-joelf@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Joel Fernandes March 13, 2014, 8:35 p.m. UTC
Introduce a generic omap timer initialization function that can
be used by all SoCs for which support is available in the clocksource
driver introduced in the series.

The function will also be responsible for calling clock initialization
required for everything else to work.

Signed-off-by: Joel Fernandes <joelf@ti.com>
---
 arch/arm/mach-omap2/common.h |    1 +
 arch/arm/mach-omap2/timer.c  |   28 ++++++++++++++++++++++++++++
 2 files changed, 29 insertions(+)

Comments

Rob Herring (Arm) March 13, 2014, 9:52 p.m. UTC | #1
On Thu, Mar 13, 2014 at 3:35 PM, Joel Fernandes <joelf@ti.com> wrote:
> Introduce a generic omap timer initialization function that can
> be used by all SoCs for which support is available in the clocksource
> driver introduced in the series.
>
> The function will also be responsible for calling clock initialization
> required for everything else to work.
>
> Signed-off-by: Joel Fernandes <joelf@ti.com>
> ---
>  arch/arm/mach-omap2/common.h |    1 +
>  arch/arm/mach-omap2/timer.c  |   28 ++++++++++++++++++++++++++++
>  2 files changed, 29 insertions(+)
>
> diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
> index a6aae30..e58d9a4 100644
> --- a/arch/arm/mach-omap2/common.h
> +++ b/arch/arm/mach-omap2/common.h
> @@ -92,6 +92,7 @@ extern void omap3_secure_sync32k_timer_init(void);
>  extern void omap3_gptimer_timer_init(void);
>  extern void omap4_local_timer_init(void);
>  extern void omap5_realtime_timer_init(void);
> +void omap_generic_timer_init(void);
>
>  void omap2420_init_early(void);
>  void omap2430_init_early(void);
> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
> index 74044aa..08c73a0 100644
> --- a/arch/arm/mach-omap2/timer.c
> +++ b/arch/arm/mach-omap2/timer.c
> @@ -324,6 +324,25 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
>         return r;
>  }
>
> +int __init omap_dmtimer_powerup(struct omap_dm_timer *timer,
> +                                       struct device_node *np) {

This function seems unrelated to the commit message.

> +       struct omap_hwmod *oh;
> +       const char *oh_name = NULL;
> +
> +       of_property_read_string_index(np, "ti,hwmods", 0, &oh_name);
> +       if (!oh_name)
> +               return -ENODEV;
> +
> +       oh = omap_hwmod_lookup(oh_name);
> +       if (!oh)
> +               return -ENODEV;
> +
> +       omap_hwmod_setup_one(oh_name);
> +
> +       omap_hwmod_enable(oh);
> +       return 0;
> +}
> +
>  static void __init omap2_gp_clockevent_init(int gptimer_id,
>                                                 const char *fck_source,
>                                                 const char *property)
> @@ -615,6 +634,15 @@ static OMAP_SYS_32K_TIMER_INIT(4, 1, "timer_32k_ck", "ti,timer-alwon",
>                                2, "sys_clkin_ck", NULL);
>  #endif
>
> +void omap_generic_timer_init(void)
> +{
> +       if (!of_have_populated_dt())
> +               BUG_ON("Generic timer init should only be used for DT boot\n");

I thought omap2 is always DT boot now.

> +
> +       omap_clk_init();

Can't you use CLK_OF_DECLARE and remove this? Then you can remove the
init_time function hook completely.

> +       clocksource_of_init();
> +}
> +
>  #ifdef CONFIG_ARCH_OMAP4
>  #ifdef CONFIG_HAVE_ARM_TWD
>  static DEFINE_TWD_LOCAL_TIMER(twd_local_timer, OMAP44XX_LOCAL_TWD_BASE, 29);
> --
> 1.7.9.5
>
Joel Fernandes March 13, 2014, 11:36 p.m. UTC | #2
On 03/13/2014 04:52 PM, Rob Herring wrote:
> On Thu, Mar 13, 2014 at 3:35 PM, Joel Fernandes <joelf@ti.com> wrote:
>> Introduce a generic omap timer initialization function that can
>> be used by all SoCs for which support is available in the clocksource
>> driver introduced in the series.
>>
>> The function will also be responsible for calling clock initialization
>> required for everything else to work.
>>
>> Signed-off-by: Joel Fernandes <joelf@ti.com>
>> ---
>>  arch/arm/mach-omap2/common.h |    1 +
>>  arch/arm/mach-omap2/timer.c  |   28 ++++++++++++++++++++++++++++
>>  2 files changed, 29 insertions(+)
>>
>> diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
>> index a6aae30..e58d9a4 100644
>> --- a/arch/arm/mach-omap2/common.h
>> +++ b/arch/arm/mach-omap2/common.h
>> @@ -92,6 +92,7 @@ extern void omap3_secure_sync32k_timer_init(void);
>>  extern void omap3_gptimer_timer_init(void);
>>  extern void omap4_local_timer_init(void);
>>  extern void omap5_realtime_timer_init(void);
>> +void omap_generic_timer_init(void);
>>
>>  void omap2420_init_early(void);
>>  void omap2430_init_early(void);
>> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
>> index 74044aa..08c73a0 100644
>> --- a/arch/arm/mach-omap2/timer.c
>> +++ b/arch/arm/mach-omap2/timer.c
>> @@ -324,6 +324,25 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
>>         return r;
>>  }
>>
>> +int __init omap_dmtimer_powerup(struct omap_dm_timer *timer,
>> +                                       struct device_node *np) {
> 
> This function seems unrelated to the commit message.

Ok, I'll add it in the message.

> 
>> +       struct omap_hwmod *oh;
>> +       const char *oh_name = NULL;
>> +
>> +       of_property_read_string_index(np, "ti,hwmods", 0, &oh_name);
>> +       if (!oh_name)
>> +               return -ENODEV;
>> +
>> +       oh = omap_hwmod_lookup(oh_name);
>> +       if (!oh)
>> +               return -ENODEV;
>> +
>> +       omap_hwmod_setup_one(oh_name);
>> +
>> +       omap_hwmod_enable(oh);
>> +       return 0;
>> +}
>> +
>>  static void __init omap2_gp_clockevent_init(int gptimer_id,
>>                                                 const char *fck_source,
>>                                                 const char *property)
>> @@ -615,6 +634,15 @@ static OMAP_SYS_32K_TIMER_INIT(4, 1, "timer_32k_ck", "ti,timer-alwon",
>>                                2, "sys_clkin_ck", NULL);
>>  #endif
>>
>> +void omap_generic_timer_init(void)
>> +{
>> +       if (!of_have_populated_dt())
>> +               BUG_ON("Generic timer init should only be used for DT boot\n");
> 
> I thought omap2 is always DT boot now.

That's right, sorry- I'll get rid of the check.

>> +
>> +       omap_clk_init();
> 
> Can't you use CLK_OF_DECLARE and remove this? Then you can remove the
> init_time function hook completely.

There is some dev-id conn-id -> DT node mapping done. I'll let Tero comment
more on this.

Thanks,
-Joel

> 
>> +       clocksource_of_init();
>> +}
>> +
>>  #ifdef CONFIG_ARCH_OMAP4
>>  #ifdef CONFIG_HAVE_ARM_TWD
>>  static DEFINE_TWD_LOCAL_TIMER(twd_local_timer, OMAP44XX_LOCAL_TWD_BASE, 29);
>> --
>> 1.7.9.5
>>
Tero Kristo March 14, 2014, 8:03 a.m. UTC | #3
On 03/14/2014 01:36 AM, Joel Fernandes wrote:
> On 03/13/2014 04:52 PM, Rob Herring wrote:
>> On Thu, Mar 13, 2014 at 3:35 PM, Joel Fernandes <joelf@ti.com> wrote:
>>> Introduce a generic omap timer initialization function that can
>>> be used by all SoCs for which support is available in the clocksource
>>> driver introduced in the series.
>>>
>>> The function will also be responsible for calling clock initialization
>>> required for everything else to work.
>>>
>>> Signed-off-by: Joel Fernandes <joelf@ti.com>
>>> ---
>>>   arch/arm/mach-omap2/common.h |    1 +
>>>   arch/arm/mach-omap2/timer.c  |   28 ++++++++++++++++++++++++++++
>>>   2 files changed, 29 insertions(+)
>>>
>>> diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
>>> index a6aae30..e58d9a4 100644
>>> --- a/arch/arm/mach-omap2/common.h
>>> +++ b/arch/arm/mach-omap2/common.h
>>> @@ -92,6 +92,7 @@ extern void omap3_secure_sync32k_timer_init(void);
>>>   extern void omap3_gptimer_timer_init(void);
>>>   extern void omap4_local_timer_init(void);
>>>   extern void omap5_realtime_timer_init(void);
>>> +void omap_generic_timer_init(void);
>>>
>>>   void omap2420_init_early(void);
>>>   void omap2430_init_early(void);
>>> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
>>> index 74044aa..08c73a0 100644
>>> --- a/arch/arm/mach-omap2/timer.c
>>> +++ b/arch/arm/mach-omap2/timer.c
>>> @@ -324,6 +324,25 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
>>>          return r;
>>>   }
>>>
>>> +int __init omap_dmtimer_powerup(struct omap_dm_timer *timer,
>>> +                                       struct device_node *np) {
>>
>> This function seems unrelated to the commit message.
>
> Ok, I'll add it in the message.
>
>>
>>> +       struct omap_hwmod *oh;
>>> +       const char *oh_name = NULL;
>>> +
>>> +       of_property_read_string_index(np, "ti,hwmods", 0, &oh_name);
>>> +       if (!oh_name)
>>> +               return -ENODEV;
>>> +
>>> +       oh = omap_hwmod_lookup(oh_name);
>>> +       if (!oh)
>>> +               return -ENODEV;
>>> +
>>> +       omap_hwmod_setup_one(oh_name);
>>> +
>>> +       omap_hwmod_enable(oh);
>>> +       return 0;
>>> +}
>>> +
>>>   static void __init omap2_gp_clockevent_init(int gptimer_id,
>>>                                                  const char *fck_source,
>>>                                                  const char *property)
>>> @@ -615,6 +634,15 @@ static OMAP_SYS_32K_TIMER_INIT(4, 1, "timer_32k_ck", "ti,timer-alwon",
>>>                                 2, "sys_clkin_ck", NULL);
>>>   #endif
>>>
>>> +void omap_generic_timer_init(void)
>>> +{
>>> +       if (!of_have_populated_dt())
>>> +               BUG_ON("Generic timer init should only be used for DT boot\n");
>>
>> I thought omap2 is always DT boot now.
>
> That's right, sorry- I'll get rid of the check.
>
>>> +
>>> +       omap_clk_init();
>>
>> Can't you use CLK_OF_DECLARE and remove this? Then you can remove the
>> init_time function hook completely.
>
> There is some dev-id conn-id -> DT node mapping done. I'll let Tero comment
> more on this.

It is actually more complicated than this, the reason we have 
omap_clk_init() is the static dependencies between the init order of 
various components in the system (slab, clocks, hwmod), and also because 
OMAP clks have pretty much different basic clock init compared to the 
generic DT clk init (memory mappings done differently etc.).... and, it 
was impossible to get the generic init modified to support OMAP clk init 
due to various sources of resistance so here we are. I hope we are able 
to merge the required stuff from omap_clk_init towards the generic clk 
init eventually and get rid of OMAP specific stuff.

-Tero

>
> Thanks,
> -Joel
>
>>
>>> +       clocksource_of_init();
>>> +}
>>> +
>>>   #ifdef CONFIG_ARCH_OMAP4
>>>   #ifdef CONFIG_HAVE_ARM_TWD
>>>   static DEFINE_TWD_LOCAL_TIMER(twd_local_timer, OMAP44XX_LOCAL_TWD_BASE, 29);
>>> --
>>> 1.7.9.5
>>>
>
Kevin Hilman March 14, 2014, 9:09 p.m. UTC | #4
Joel Fernandes <joelf@ti.com> writes:

> On 03/13/2014 04:52 PM, Rob Herring wrote:
>> On Thu, Mar 13, 2014 at 3:35 PM, Joel Fernandes <joelf@ti.com> wrote:
>>> Introduce a generic omap timer initialization function that can
>>> be used by all SoCs for which support is available in the clocksource
>>> driver introduced in the series.
>>>
>>> The function will also be responsible for calling clock initialization
>>> required for everything else to work.
>>>
>>> Signed-off-by: Joel Fernandes <joelf@ti.com>
>>> ---
>>>  arch/arm/mach-omap2/common.h |    1 +
>>>  arch/arm/mach-omap2/timer.c  |   28 ++++++++++++++++++++++++++++
>>>  2 files changed, 29 insertions(+)
>>>
>>> diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
>>> index a6aae30..e58d9a4 100644
>>> --- a/arch/arm/mach-omap2/common.h
>>> +++ b/arch/arm/mach-omap2/common.h
>>> @@ -92,6 +92,7 @@ extern void omap3_secure_sync32k_timer_init(void);
>>>  extern void omap3_gptimer_timer_init(void);
>>>  extern void omap4_local_timer_init(void);
>>>  extern void omap5_realtime_timer_init(void);
>>> +void omap_generic_timer_init(void);
>>>
>>>  void omap2420_init_early(void);
>>>  void omap2430_init_early(void);
>>> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
>>> index 74044aa..08c73a0 100644
>>> --- a/arch/arm/mach-omap2/timer.c
>>> +++ b/arch/arm/mach-omap2/timer.c
>>> @@ -324,6 +324,25 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
>>>         return r;
>>>  }
>>>
>>> +int __init omap_dmtimer_powerup(struct omap_dm_timer *timer,
>>> +                                       struct device_node *np) {
>> 
>> This function seems unrelated to the commit message.
>
> Ok, I'll add it in the message.
>
>> 
>>> +       struct omap_hwmod *oh;
>>> +       const char *oh_name = NULL;
>>> +
>>> +       of_property_read_string_index(np, "ti,hwmods", 0, &oh_name);
>>> +       if (!oh_name)
>>> +               return -ENODEV;
>>> +
>>> +       oh = omap_hwmod_lookup(oh_name);
>>> +       if (!oh)
>>> +               return -ENODEV;
>>> +
>>> +       omap_hwmod_setup_one(oh_name);
>>> +
>>> +       omap_hwmod_enable(oh);
>>> +       return 0;
>>> +}
>>> +
>>>  static void __init omap2_gp_clockevent_init(int gptimer_id,
>>>                                                 const char *fck_source,
>>>                                                 const char *property)
>>> @@ -615,6 +634,15 @@ static OMAP_SYS_32K_TIMER_INIT(4, 1, "timer_32k_ck", "ti,timer-alwon",
>>>                                2, "sys_clkin_ck", NULL);
>>>  #endif
>>>
>>> +void omap_generic_timer_init(void)
>>> +{
>>> +       if (!of_have_populated_dt())
>>> +               BUG_ON("Generic timer init should only be used for DT boot\n");
>> 
>> I thought omap2 is always DT boot now.
>
> That's right, sorry- I'll get rid of the check.

Actually, mainline still supports legacy boot and has board files for
OMAP3 platforms, and we shouldn't break legacy boot on purpose IMO.

Kevin
Rob Herring (Arm) March 14, 2014, 9:16 p.m. UTC | #5
On Fri, Mar 14, 2014 at 4:09 PM, Kevin Hilman <khilman@linaro.org> wrote:
> Joel Fernandes <joelf@ti.com> writes:
>
>> On 03/13/2014 04:52 PM, Rob Herring wrote:
>>> On Thu, Mar 13, 2014 at 3:35 PM, Joel Fernandes <joelf@ti.com> wrote:
>>>> Introduce a generic omap timer initialization function that can
>>>> be used by all SoCs for which support is available in the clocksource
>>>> driver introduced in the series.
>>>>
>>>> The function will also be responsible for calling clock initialization
>>>> required for everything else to work.
>>>>
>>>> Signed-off-by: Joel Fernandes <joelf@ti.com>

[snip]

>>>> +void omap_generic_timer_init(void)
>>>> +{
>>>> +       if (!of_have_populated_dt())
>>>> +               BUG_ON("Generic timer init should only be used for DT boot\n");
>>>
>>> I thought omap2 is always DT boot now.
>>
>> That's right, sorry- I'll get rid of the check.
>
> Actually, mainline still supports legacy boot and has board files for
> OMAP3 platforms, and we shouldn't break legacy boot on purpose IMO.

It is not breaking anything. You'd have to call this function
mistakenly from a legacy machine desc or other non-DT boot path. It's
a question of how paranoid you are adding checks. IMHO, the BUG_ON is
quite pointless here. You are not likely to progress much further
anyway, and you won't see the BUG print because the serial port is not
up yet.

Rob
diff mbox

Patch

diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
index a6aae30..e58d9a4 100644
--- a/arch/arm/mach-omap2/common.h
+++ b/arch/arm/mach-omap2/common.h
@@ -92,6 +92,7 @@  extern void omap3_secure_sync32k_timer_init(void);
 extern void omap3_gptimer_timer_init(void);
 extern void omap4_local_timer_init(void);
 extern void omap5_realtime_timer_init(void);
+void omap_generic_timer_init(void);
 
 void omap2420_init_early(void);
 void omap2430_init_early(void);
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 74044aa..08c73a0 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -324,6 +324,25 @@  static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
 	return r;
 }
 
+int __init omap_dmtimer_powerup(struct omap_dm_timer *timer,
+					struct device_node *np) {
+	struct omap_hwmod *oh;
+	const char *oh_name = NULL;
+
+	of_property_read_string_index(np, "ti,hwmods", 0, &oh_name);
+	if (!oh_name)
+		return -ENODEV;
+
+	oh = omap_hwmod_lookup(oh_name);
+	if (!oh)
+		return -ENODEV;
+
+	omap_hwmod_setup_one(oh_name);
+
+	omap_hwmod_enable(oh);
+	return 0;
+}
+
 static void __init omap2_gp_clockevent_init(int gptimer_id,
 						const char *fck_source,
 						const char *property)
@@ -615,6 +634,15 @@  static OMAP_SYS_32K_TIMER_INIT(4, 1, "timer_32k_ck", "ti,timer-alwon",
 			       2, "sys_clkin_ck", NULL);
 #endif
 
+void omap_generic_timer_init(void)
+{
+	if (!of_have_populated_dt())
+		BUG_ON("Generic timer init should only be used for DT boot\n");
+
+	omap_clk_init();
+	clocksource_of_init();
+}
+
 #ifdef CONFIG_ARCH_OMAP4
 #ifdef CONFIG_HAVE_ARM_TWD
 static DEFINE_TWD_LOCAL_TIMER(twd_local_timer, OMAP44XX_LOCAL_TWD_BASE, 29);