Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-12-14 Thread Russ Dill
On Thu, Nov 8, 2012 at 9:08 AM, Hiremath, Vaibhav  wrote:
> No we do not have 32k_counter block in AM335x.
>
> If you are referring to 32Khz clock availability alone, then yes, we need to
> get persistent clock and we use RTC 32Khz clock source for it.
> But please note that this is not a 32k_counter block which OMAP family of
> devices has.
>
> The way AM335x works is, we have timers (0-7), timer0 being secure timer.
> We use timer1 and timer2 for clockevent and clocksource and they are driven
> by 26MHz OSC clock currently. So when you go into Low power state, OSC is 
> turned off and you need persistent clock for time keeping, right?
>
> And only persistent clock available is RTC 32khz crystal clock, being RTC is
> in wakeup domain.

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


Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-12-10 Thread Paul Walmsley
On Sun, 11 Nov 2012, Igor Grinberg wrote:

> Yep, but the /800 do not get you the 32768...
> and that makes the timer suck.
> Of course this can be dealt with in the clock subsystem
> (I remember Paul said that he will look into that), but it will take time.

It should be possible to implement this now that the CCF conversion is 
done.  Not sure if I'll be able to get to it soon, though.

Is there anyone out there at TI who is focused on AM3517/3505 upstream 
work?


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


Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-13 Thread Igor Grinberg
On 11/13/12 18:13, Jon Hunter wrote:
> 
> On 11/13/2012 03:14 AM, Igor Grinberg wrote:
>> On 11/12/12 21:15, Jon Hunter wrote:
>>>
>>> On 11/11/2012 05:28 AM, Igor Grinberg wrote:


 On 11/08/12 21:16, Jon Hunter wrote:
>
> On 11/08/2012 12:59 PM, Hiremath, Vaibhav wrote:
>> On Fri, Nov 09, 2012 at 00:24:23, Hunter, Jon wrote:
>>>
>>> On 11/08/2012 01:59 AM, Igor Grinberg wrote:
>>>
>>> [snip]
>>>
 There is no reliable way to determine which source should be used in 
 runtime
 for boards that do not have the 32k oscillator wired.
>>>
>>> So thinking about this some more and given that we are moving away from
>>> board files, if a board does not provide a 32kHz clock source, then this
>>> should be reflected in the device-tree source file for that board.
>>> Hence, at boot time we should be able to determine if a 32kHz clock
>>> source can be used.
>>>
>>
>> Let me feed some more thoughts here :)
>>
>> The way it is being detected currently is based on timer idle status bit.
>> I am worried that, this is the only option we have.
>
> Why not use device-tree to indicate the presence of a 32k clock source?
> This seems like a board level configuration and so device-tree seems to
> be the perfect place for this IMO.

 Well, that is what my commit message says...
>>>
>>> Sorry, but that was not clear to me from whats in the commit message.
>>
>> From the commit message:
>> "1) Timer structures and initialization functions are named by the platform
>>name and the clock source in use. The decision which timer is
>>used is done statically from the machine_desc structure. In the
>>future it should come from DT."
>>
>> The last sentence has it.
> 
> Right, but it does not go into the details. It would be good to have
> added a comment to the effect of "some boards do not have a 32k clock
> source and in the future this should be handled by device-tree".

Ok.

> 
>> The transition to DT is not immediate and we can't (still) neglect
>> the non-DT setups.
> 
> Absolutely, but I am trying to understand if there are boards being
> "neglected". I see now that your CM-T3517 would be. This was not clear
> from your patch as even the CM-T3517 board was being configured to the
> use the sync32k timer. So from looking at your patch I did not see any
> neglected boards, however, I understand your motivation to add all these
> init functions so that boards could be customised easily.

I did not changed the CM-T3517, because I believe it should be done
in a separate patch.

> 
>>> Should we be doing this now instead of adding all these static timer
>>> init functions?
>>
>> I don't see this as "adding ...", I see this as expanding the setup
>> which was previously hidden by the CONFIG_OMAP_32K_TIMER option.
>>
>>>
>>> Are there any boards today (supported in the kernel that is), that don't
>>> support a 32k?
>>
>> Yes, starting from revision 1.2, CM-T3517 does not have the 32k.
> 
> Thanks, this is the exact information I was looking for. You should put
> this in your commit message to highlight the fact that there are boards
> that don't have a 32k clock source.
> 
> I am familiar with the OMAP devices, but less familiar with these AM
> derivatives (as I don't work with these) and so it is good to put these
> specifics in the commit message.

Ok.


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


Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-13 Thread Jon Hunter

On 11/13/2012 03:14 AM, Igor Grinberg wrote:
> On 11/12/12 21:15, Jon Hunter wrote:
>>
>> On 11/11/2012 05:28 AM, Igor Grinberg wrote:
>>>
>>>
>>> On 11/08/12 21:16, Jon Hunter wrote:

 On 11/08/2012 12:59 PM, Hiremath, Vaibhav wrote:
> On Fri, Nov 09, 2012 at 00:24:23, Hunter, Jon wrote:
>>
>> On 11/08/2012 01:59 AM, Igor Grinberg wrote:
>>
>> [snip]
>>
>>> There is no reliable way to determine which source should be used in 
>>> runtime
>>> for boards that do not have the 32k oscillator wired.
>>
>> So thinking about this some more and given that we are moving away from
>> board files, if a board does not provide a 32kHz clock source, then this
>> should be reflected in the device-tree source file for that board.
>> Hence, at boot time we should be able to determine if a 32kHz clock
>> source can be used.
>>
>
> Let me feed some more thoughts here :)
>
> The way it is being detected currently is based on timer idle status bit.
> I am worried that, this is the only option we have.

 Why not use device-tree to indicate the presence of a 32k clock source?
 This seems like a board level configuration and so device-tree seems to
 be the perfect place for this IMO.
>>>
>>> Well, that is what my commit message says...
>>
>> Sorry, but that was not clear to me from whats in the commit message.
> 
> From the commit message:
> "1) Timer structures and initialization functions are named by the platform
>name and the clock source in use. The decision which timer is
>used is done statically from the machine_desc structure. In the
>future it should come from DT."
> 
> The last sentence has it.

Right, but it does not go into the details. It would be good to have
added a comment to the effect of "some boards do not have a 32k clock
source and in the future this should be handled by device-tree".

> The transition to DT is not immediate and we can't (still) neglect
> the non-DT setups.

Absolutely, but I am trying to understand if there are boards being
"neglected". I see now that your CM-T3517 would be. This was not clear
from your patch as even the CM-T3517 board was being configured to the
use the sync32k timer. So from looking at your patch I did not see any
neglected boards, however, I understand your motivation to add all these
init functions so that boards could be customised easily.

>> Should we be doing this now instead of adding all these static timer
>> init functions?
> 
> I don't see this as "adding ...", I see this as expanding the setup
> which was previously hidden by the CONFIG_OMAP_32K_TIMER option.
> 
>>
>> Are there any boards today (supported in the kernel that is), that don't
>> support a 32k?
> 
> Yes, starting from revision 1.2, CM-T3517 does not have the 32k.

Thanks, this is the exact information I was looking for. You should put
this in your commit message to highlight the fact that there are boards
that don't have a 32k clock source.

I am familiar with the OMAP devices, but less familiar with these AM
derivatives (as I don't work with these) and so it is good to put these
specifics in the commit message.

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


Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-13 Thread Igor Grinberg
On 11/12/12 21:15, Jon Hunter wrote:
> 
> On 11/11/2012 05:28 AM, Igor Grinberg wrote:
>>
>>
>> On 11/08/12 21:16, Jon Hunter wrote:
>>>
>>> On 11/08/2012 12:59 PM, Hiremath, Vaibhav wrote:
 On Fri, Nov 09, 2012 at 00:24:23, Hunter, Jon wrote:
>
> On 11/08/2012 01:59 AM, Igor Grinberg wrote:
>
> [snip]
>
>> There is no reliable way to determine which source should be used in 
>> runtime
>> for boards that do not have the 32k oscillator wired.
>
> So thinking about this some more and given that we are moving away from
> board files, if a board does not provide a 32kHz clock source, then this
> should be reflected in the device-tree source file for that board.
> Hence, at boot time we should be able to determine if a 32kHz clock
> source can be used.
>

 Let me feed some more thoughts here :)

 The way it is being detected currently is based on timer idle status bit.
 I am worried that, this is the only option we have.
>>>
>>> Why not use device-tree to indicate the presence of a 32k clock source?
>>> This seems like a board level configuration and so device-tree seems to
>>> be the perfect place for this IMO.
>>
>> Well, that is what my commit message says...
> 
> Sorry, but that was not clear to me from whats in the commit message.

>From the commit message:
"1) Timer structures and initialization functions are named by the platform
   name and the clock source in use. The decision which timer is
   used is done statically from the machine_desc structure. In the
   future it should come from DT."

The last sentence has it.
The transition to DT is not immediate and we can't (still) neglect
the non-DT setups.

> 
> Should we be doing this now instead of adding all these static timer
> init functions?

I don't see this as "adding ...", I see this as expanding the setup
which was previously hidden by the CONFIG_OMAP_32K_TIMER option.

> 
> Are there any boards today (supported in the kernel that is), that don't
> support a 32k?

Yes, starting from revision 1.2, CM-T3517 does not have the 32k.

[...]

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


Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-13 Thread Igor Grinberg
On 11/12/12 21:05, Jon Hunter wrote:
> 
> On 11/11/2012 03:16 AM, Igor Grinberg wrote:
>> On 11/08/12 18:20, Tony Lindgren wrote:
>>> * Igor Grinberg  [121107 23:15]:
 On 11/07/12 19:33, Tony Lindgren wrote:
>
> I think this should be the default for the timers as that counter
> does not stop during deeper idle states.

 Well, it is the default as you can see from the patch.
 The problem is that for boards that for some reason do not have
 the 32k wired and rely on MPU/GP timer source, the default will not work
 and currently there is no way for board to specify which timer source
 it can use.
>>>
>>> Yes. I was just wondering if we can avoid patching all the board
>>> files by doing it the other way around by introducing a new
>>> omap_gp_timer rather than renaming all the existing ones?
>>
>> Is the renaming that bad? One line per machine_desc structure?
>> Of course we can skip the renaming, but then it will be less consistent
>> and will not reflect the actual timer source used.
>> I tried to make it flexible as much as possible and self explanatory.
>> So above are my considerations, but at this point in time I don't really
>> care if we rename them or just add a new one, but we have to get rid of
>> the ugly fall back.
> 
> I am not sure if you guys disagree, but does it make sense to start
> thinking about this with regard to device-tree? With device-tree all the
> boards files will become obsolete and so we need to be able to handle
> this during boot time and not compile time.

This is understood completely, but I personally think that we should not
neglect the "still not converted to DT boards", because the conversion
takes time and involves much more effort. Also taking into account that
there are subsystems that are still not converted to DT.

Also, removing the CONFIG_OMAP_32K_TIMER _does_ get us closer to handle
the timers init during boot time.

> 
>>>
 We have discussed this in San Diego (remember?) and you actually proposed
 this way as a solution. Well, may be I took it a bit further than you
 thought, but this is because the board code cannot know which timer source
 should be used at runtime and the fall back described below, does not work.
>>>
>>> Yes thanks I agree we should get rid of that Kconfig option for sure. 
>>>
>> --- a/arch/arm/mach-omap2/board-2430sdp.c
>> +++ b/arch/arm/mach-omap2/board-2430sdp.c
>> @@ -284,6 +284,6 @@ MACHINE_START(OMAP_2430SDP, "OMAP2430 sdp2430 board")
>>  .handle_irq = omap2_intc_handle_irq,
>>  .init_machine   = omap_2430sdp_init,
>>  .init_late  = omap2430_init_late,
>> -.timer  = &omap2_timer,
>> +.timer  = &omap2_sync32k_timer,
>>  .restart= omap_prcm_restart,
>>  MACHINE_END
>> --- a/arch/arm/mach-omap2/board-3430sdp.c
>> +++ b/arch/arm/mach-omap2/board-3430sdp.c
>> @@ -596,6 +596,6 @@ MACHINE_START(OMAP_3430SDP, "OMAP3430 3430SDP board")
>>  .handle_irq = omap3_intc_handle_irq,
>>  .init_machine   = omap_3430sdp_init,
>>  .init_late  = omap3430_init_late,
>> -.timer  = &omap3_timer,
>> +.timer  = &omap3_sync32k_timer,
>>  .restart= omap_prcm_restart,
>>  MACHINE_END
> ...
>
> Can't we assume that the default timer is omap[234]_sync32k_timer to
> avoid renaming the timer entries in all the board files?

 Hmmm...
 How will this work with the macros defining the sys_timer structure?
 I would also not want to hide the exact timer used under the default name.
>>>
>>> Can't you just add a new sys_timer (or a new macro) for GP only setups? 
>>
>> Of course I can... but I tried to create a flexible generic code, so
>> no meter how a board will be wired, you just need to specify which timer 
>> source
>> it uses and be done with it.
> 
> If you are concerned about how a board is wired up (if the 32k is
> present), then I think that that is best handled via device-tree and we
> should query device-tree on boot to see what our options are.
> 
> What do you guys think?

Yes, but again, you can't neglect the non-DT case... at least
until you have everything converted to DT.


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


Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-12 Thread Tony Lindgren
* Igor Grinberg  [12 01:18]:
> On 11/08/12 18:20, Tony Lindgren wrote:
> > 
> > I guess what I'm after is just to avoid renaming the existing
> > timers in the board-*.c files and only rename the ones that
> > need gp timer only.
> 
> This means: get rid of the 32k to gptimer fall back.
> I've got your point, will cook something shortly.

Thanks that sounds reasonable to me.

Regards,

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


Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-12 Thread Jon Hunter

On 11/11/2012 05:28 AM, Igor Grinberg wrote:
> 
> 
> On 11/08/12 21:16, Jon Hunter wrote:
>>
>> On 11/08/2012 12:59 PM, Hiremath, Vaibhav wrote:
>>> On Fri, Nov 09, 2012 at 00:24:23, Hunter, Jon wrote:

 On 11/08/2012 01:59 AM, Igor Grinberg wrote:

 [snip]

> There is no reliable way to determine which source should be used in 
> runtime
> for boards that do not have the 32k oscillator wired.

 So thinking about this some more and given that we are moving away from
 board files, if a board does not provide a 32kHz clock source, then this
 should be reflected in the device-tree source file for that board.
 Hence, at boot time we should be able to determine if a 32kHz clock
 source can be used.

>>>
>>> Let me feed some more thoughts here :)
>>>
>>> The way it is being detected currently is based on timer idle status bit.
>>> I am worried that, this is the only option we have.
>>
>> Why not use device-tree to indicate the presence of a 32k clock source?
>> This seems like a board level configuration and so device-tree seems to
>> be the perfect place for this IMO.
> 
> Well, that is what my commit message says...

Sorry, but that was not clear to me from whats in the commit message.

Should we be doing this now instead of adding all these static timer
init functions?

Are there any boards today (supported in the kernel that is), that don't
support a 32k?

If not, then this becomes simpler for the non-DT case and given that
boards such as the AM335x will only support DT boot then we can figure
out how to detect the presence of the 32k for DT only.

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


Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-12 Thread Jon Hunter

On 11/11/2012 03:16 AM, Igor Grinberg wrote:
> On 11/08/12 18:20, Tony Lindgren wrote:
>> * Igor Grinberg  [121107 23:15]:
>>> On 11/07/12 19:33, Tony Lindgren wrote:

 I think this should be the default for the timers as that counter
 does not stop during deeper idle states.
>>>
>>> Well, it is the default as you can see from the patch.
>>> The problem is that for boards that for some reason do not have
>>> the 32k wired and rely on MPU/GP timer source, the default will not work
>>> and currently there is no way for board to specify which timer source
>>> it can use.
>>
>> Yes. I was just wondering if we can avoid patching all the board
>> files by doing it the other way around by introducing a new
>> omap_gp_timer rather than renaming all the existing ones?
> 
> Is the renaming that bad? One line per machine_desc structure?
> Of course we can skip the renaming, but then it will be less consistent
> and will not reflect the actual timer source used.
> I tried to make it flexible as much as possible and self explanatory.
> So above are my considerations, but at this point in time I don't really
> care if we rename them or just add a new one, but we have to get rid of
> the ugly fall back.

I am not sure if you guys disagree, but does it make sense to start
thinking about this with regard to device-tree? With device-tree all the
boards files will become obsolete and so we need to be able to handle
this during boot time and not compile time.

>>
>>> We have discussed this in San Diego (remember?) and you actually proposed
>>> this way as a solution. Well, may be I took it a bit further than you
>>> thought, but this is because the board code cannot know which timer source
>>> should be used at runtime and the fall back described below, does not work.
>>
>> Yes thanks I agree we should get rid of that Kconfig option for sure. 
>>
> --- a/arch/arm/mach-omap2/board-2430sdp.c
> +++ b/arch/arm/mach-omap2/board-2430sdp.c
> @@ -284,6 +284,6 @@ MACHINE_START(OMAP_2430SDP, "OMAP2430 sdp2430 board")
>   .handle_irq = omap2_intc_handle_irq,
>   .init_machine   = omap_2430sdp_init,
>   .init_late  = omap2430_init_late,
> - .timer  = &omap2_timer,
> + .timer  = &omap2_sync32k_timer,
>   .restart= omap_prcm_restart,
>  MACHINE_END
> --- a/arch/arm/mach-omap2/board-3430sdp.c
> +++ b/arch/arm/mach-omap2/board-3430sdp.c
> @@ -596,6 +596,6 @@ MACHINE_START(OMAP_3430SDP, "OMAP3430 3430SDP board")
>   .handle_irq = omap3_intc_handle_irq,
>   .init_machine   = omap_3430sdp_init,
>   .init_late  = omap3430_init_late,
> - .timer  = &omap3_timer,
> + .timer  = &omap3_sync32k_timer,
>   .restart= omap_prcm_restart,
>  MACHINE_END
 ...

 Can't we assume that the default timer is omap[234]_sync32k_timer to
 avoid renaming the timer entries in all the board files?
>>>
>>> Hmmm...
>>> How will this work with the macros defining the sys_timer structure?
>>> I would also not want to hide the exact timer used under the default name.
>>
>> Can't you just add a new sys_timer (or a new macro) for GP only setups? 
> 
> Of course I can... but I tried to create a flexible generic code, so
> no meter how a board will be wired, you just need to specify which timer 
> source
> it uses and be done with it.

If you are concerned about how a board is wired up (if the 32k is
present), then I think that that is best handled via device-tree and we
should query device-tree on boot to see what our options are.

What do you guys think?

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


Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-12 Thread Benoit Cousson
Hi Vaibhav,

On 11/12/2012 11:38 AM, Hiremath, Vaibhav wrote:
> On Fri, Nov 09, 2012 at 00:46:28, Hunter, Jon wrote:
>>
>> On 11/08/2012 12:59 PM, Hiremath, Vaibhav wrote:
>>> On Fri, Nov 09, 2012 at 00:24:23, Hunter, Jon wrote:

 On 11/08/2012 01:59 AM, Igor Grinberg wrote:

 [snip]

> There is no reliable way to determine which source should be used in 
> runtime
> for boards that do not have the 32k oscillator wired.

 So thinking about this some more and given that we are moving away from
 board files, if a board does not provide a 32kHz clock source, then this
 should be reflected in the device-tree source file for that board.
 Hence, at boot time we should be able to determine if a 32kHz clock
 source can be used.

>>>
>>> Let me feed some more thoughts here :)
>>>
>>> The way it is being detected currently is based on timer idle status bit.
>>> I am worried that, this is the only option we have.
>>
>> Why not use device-tree to indicate the presence of a 32k clock source?
>> This seems like a board level configuration and so device-tree seems to
>> be the perfect place for this IMO.
>>
> 
> I think I agree with you, but for this to happen in clean way, its time to 
> start populating clock-nodes in DT, don't you think? Something like,
> 
> 
> clocks {
>   rtc_clk: clk@X {
>   compatible = "crystal-32k, per-32k, xyz";
>   clock-frequency = <32768>;
>   };
>   ...
> };
> 
> Timer {
> 
>   ref-clock = <&rtc_clk>;
> };
> 
> What do you think?

That's indeed the proper way to do it, since this is a pure board level
parameter and we do have the binding to express that.
We just have to add that in the DTS:-)

Regards,
Benoit



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


RE: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-12 Thread Hiremath, Vaibhav
On Mon, Nov 12, 2012 at 12:54:59, Igor Grinberg wrote:
> On 11/12/12 08:38, Hiremath, Vaibhav wrote:
> > On Sun, Nov 11, 2012 at 17:05:07, Igor Grinberg wrote:
> >>
> >>
> >> On 11/08/12 20:34, Jon Hunter wrote:
> >>>
> >>> On 11/08/2012 12:17 PM, Paul Walmsley wrote:
>  On Thu, 8 Nov 2012, Jon Hunter wrote:
> 
> > On 11/08/2012 11:58 AM, Paul Walmsley wrote:
> >> On Thu, 8 Nov 2012, Jon Hunter wrote:
> >>
> >>> Igor was mentioning a h/w scenario where the 32kHz source is not
> >>> present. However, I am not sure which devices support this and is
> >>> applicable too.
> >>
> >> Pretty sure Igor is referring to the AM3517/3505.  This is very poorly 
> >> documented, but can be observed in the AM3517 TRM Rev B (SPRUGR0B) 
> >> Figure 
> >> 4-23 "PRM Clock Generator" and the AM3517 DM Rev C (SPRS550C) Section 
> >> 4 
> >> "Clock Specifications".
> >
> > But AFAICT, even in that h/w configuration the internal 32k
> > oscillator will be used
> 
>  Just to clarify, there's no internal 32k oscillator used on the 
>  3517/3505; 
>  just a divider from the HF clock.
> >>>
> >>> Ah yes I see that now!
> >>>
> > and so the gptimer will still have a 32k clock source.
> 
>  That's a good question and you might want to check with Igor on that one,
>  the AM3517 TRM conflicts with the DM as to whether it's available to the 
>  GPTIMER or not :-(
> >>>
> >>> Well the external 32k and internal divided down version go through the
> >>> same mux and so that seems to imply either they are both available to
> >>> the gptimer or neither is.
> >>
> >> Yep, but the /800 do not get you the 32768...
> >> and that makes the timer suck.
> >> Of course this can be dealt with in the clock subsystem
> >> (I remember Paul said that he will look into that), but it will take time.
> >>
> >> Also, what about having the sys_clk instead of 32k for higher precision?
> >> Is that possible already (without my patch)?
> >>
> > 
> > Yes, it is possible. You can choose it through bootargs.
> 
> Is the kernel command line the only way for doing this?
> I personally dislike it, because it brings multiple maintenance problems.
> This must be possible at least through DT.
> 

Its standard interface, so carries all the applicable pros and cons.
I am not quite sure about runtime change of clocksoyrce, its pros and cons.

Thanks,
Vaibhav

> 
> -- 
> Regards,
> Igor.
> 

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


RE: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-12 Thread Hiremath, Vaibhav
On Fri, Nov 09, 2012 at 00:46:28, Hunter, Jon wrote:
> 
> On 11/08/2012 12:59 PM, Hiremath, Vaibhav wrote:
> > On Fri, Nov 09, 2012 at 00:24:23, Hunter, Jon wrote:
> >>
> >> On 11/08/2012 01:59 AM, Igor Grinberg wrote:
> >>
> >> [snip]
> >>
> >>> There is no reliable way to determine which source should be used in 
> >>> runtime
> >>> for boards that do not have the 32k oscillator wired.
> >>
> >> So thinking about this some more and given that we are moving away from
> >> board files, if a board does not provide a 32kHz clock source, then this
> >> should be reflected in the device-tree source file for that board.
> >> Hence, at boot time we should be able to determine if a 32kHz clock
> >> source can be used.
> >>
> > 
> > Let me feed some more thoughts here :)
> > 
> > The way it is being detected currently is based on timer idle status bit.
> > I am worried that, this is the only option we have.
> 
> Why not use device-tree to indicate the presence of a 32k clock source?
> This seems like a board level configuration and so device-tree seems to
> be the perfect place for this IMO.
> 

I think I agree with you, but for this to happen in clean way, its time to 
start populating clock-nodes in DT, don't you think? Something like,


clocks {
rtc_clk: clk@X {
compatible = "crystal-32k, per-32k, xyz";
clock-frequency = <32768>;
};
...
};

Timer {

ref-clock = <&rtc_clk>;
};

What do you think?

Thanks,
Vaibhav
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-11 Thread Igor Grinberg
On 11/12/12 08:38, Hiremath, Vaibhav wrote:
> On Sun, Nov 11, 2012 at 17:05:07, Igor Grinberg wrote:
>>
>>
>> On 11/08/12 20:34, Jon Hunter wrote:
>>>
>>> On 11/08/2012 12:17 PM, Paul Walmsley wrote:
 On Thu, 8 Nov 2012, Jon Hunter wrote:

> On 11/08/2012 11:58 AM, Paul Walmsley wrote:
>> On Thu, 8 Nov 2012, Jon Hunter wrote:
>>
>>> Igor was mentioning a h/w scenario where the 32kHz source is not
>>> present. However, I am not sure which devices support this and is
>>> applicable too.
>>
>> Pretty sure Igor is referring to the AM3517/3505.  This is very poorly 
>> documented, but can be observed in the AM3517 TRM Rev B (SPRUGR0B) 
>> Figure 
>> 4-23 "PRM Clock Generator" and the AM3517 DM Rev C (SPRS550C) Section 4 
>> "Clock Specifications".
>
> But AFAICT, even in that h/w configuration the internal 32k
> oscillator will be used

 Just to clarify, there's no internal 32k oscillator used on the 3517/3505; 
 just a divider from the HF clock.
>>>
>>> Ah yes I see that now!
>>>
> and so the gptimer will still have a 32k clock source.

 That's a good question and you might want to check with Igor on that one,
 the AM3517 TRM conflicts with the DM as to whether it's available to the 
 GPTIMER or not :-(
>>>
>>> Well the external 32k and internal divided down version go through the
>>> same mux and so that seems to imply either they are both available to
>>> the gptimer or neither is.
>>
>> Yep, but the /800 do not get you the 32768...
>> and that makes the timer suck.
>> Of course this can be dealt with in the clock subsystem
>> (I remember Paul said that he will look into that), but it will take time.
>>
>> Also, what about having the sys_clk instead of 32k for higher precision?
>> Is that possible already (without my patch)?
>>
> 
> Yes, it is possible. You can choose it through bootargs.

Is the kernel command line the only way for doing this?
I personally dislike it, because it brings multiple maintenance problems.
This must be possible at least through DT.


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


RE: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-11 Thread Hiremath, Vaibhav
On Fri, Nov 09, 2012 at 06:25:49, Hunter, Jon wrote:
> 
> On 11/08/2012 12:06 PM, Hiremath, Vaibhav wrote:
> > On Thu, Nov 08, 2012 at 23:28:53, Hunter, Jon wrote:
> >>
> >> On 11/08/2012 11:47 AM, Hiremath, Vaibhav wrote:
> >>> On Thu, Nov 08, 2012 at 23:09:57, Hunter, Jon wrote:
> >>
> >> [snip]
> >>
>  I think you are missing the point here. For OMAP devices we have two
>  main external clock sources which are the 32kHz clock and the sys_clk
>  (can be a range of frequencies from 12-38.4MHz for OMAP4). The point is
>  for OMAP these clock sources are always present and AFAIK there is no
>  h/w configuration that allows you not to have the 32kHz clock source.
>  PRCM needs it and I think for OMAP1 the reset logic needs it (if memory
>  serves).
> 
>  Igor was mentioning a h/w scenario where the 32kHz source is not
>  present. However, I am not sure which devices support this and is
>  applicable too.
> 
>  So we are not discussing the 32k-sync-timer here. We are discussing
>  whether there are any device configurations where a 32k clock source
>  would not be available for the gptimer.
> 
> >>>
> >>> Exactly that is the point I am trying to make here,
> >>>
> >>> In case of AM33xx, it is certainly possible to build the system without 
> >>> this 32Khz clock. 
> >>>
> >>> Interesting point here is, this 32KHz clock is external clock coming from 
> >>> crystal connected to in-built RTC module.
> >>
> >> Thanks. Looking at the AM3358 data manual it states ...
> >>
> >> "The OSC1 oscillator provides a 32.768-kHz reference clock to the
> >> real-time clock (RTC) and is connected to the RTC_XTALIN and RTC_XTALOUT
> >> terminals. OSC1 is disabled by default after power is applied. This
> >> clock input is optional and may not be required if the RTC is configured
> >> to receive a clock from the internal 32k RC oscillator (CLK_RC32K) or
> >> peripheral PLL (CLK_32KHZ) which receives a reference clock from the
> >> OSC0 input."
> >>
> >> So what is clear to me that an external 32k clock source is optional.
> >> However, it still appears that you will always have an internal 32k
> >> source and so regardless of the h/w configuration, a gptimer will always
> >> have an 32k clock available on the AM335x devices. Is that correct?
> >>
> > 
> > Internal RC oscillator is not accurate at all, so not guaranteed to give 
> > accurate 32.768Hz clock. The oscillation band is from 16Khz to 64Khz.
> > 
> > So it may not be useful as a system clocks, right?
> 
> By the way, according to the AM3358 data manual (paragraph above), even
> if there is no external 32k clock source and if you don't use the
> internal 32k oscillator, you can still generate a 32k clock from the PER
> PLL (CLK_32KHZ). So therefore, there should always be a 32k clock source
> available for the gptimer. Is that correct?

Yes that's correct, but it is from PER domain, so duging suspend time, it 
will be disabled. This can not be treated as a persistent clock.


> 
> At the end of the day, I am just trying to understand if any OMAP/AM
> device supports a h/w configuration where there is no 32k clock source
> available for the gptimer.
> 

I know and completely understand. We need to come up with solution which 
works on all platforms.

Thanks,
Vaibhav
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-11 Thread Hiremath, Vaibhav
On Sun, Nov 11, 2012 at 17:05:07, Igor Grinberg wrote:
> 
> 
> On 11/08/12 20:34, Jon Hunter wrote:
> > 
> > On 11/08/2012 12:17 PM, Paul Walmsley wrote:
> >> On Thu, 8 Nov 2012, Jon Hunter wrote:
> >>
> >>> On 11/08/2012 11:58 AM, Paul Walmsley wrote:
>  On Thu, 8 Nov 2012, Jon Hunter wrote:
> 
> > Igor was mentioning a h/w scenario where the 32kHz source is not
> > present. However, I am not sure which devices support this and is
> > applicable too.
> 
>  Pretty sure Igor is referring to the AM3517/3505.  This is very poorly 
>  documented, but can be observed in the AM3517 TRM Rev B (SPRUGR0B) 
>  Figure 
>  4-23 "PRM Clock Generator" and the AM3517 DM Rev C (SPRS550C) Section 4 
>  "Clock Specifications".
> >>>
> >>> But AFAICT, even in that h/w configuration the internal 32k
> >>> oscillator will be used
> >>
> >> Just to clarify, there's no internal 32k oscillator used on the 3517/3505; 
> >> just a divider from the HF clock.
> > 
> > Ah yes I see that now!
> > 
> >>> and so the gptimer will still have a 32k clock source.
> >>
> >> That's a good question and you might want to check with Igor on that one,
> >> the AM3517 TRM conflicts with the DM as to whether it's available to the 
> >> GPTIMER or not :-(
> > 
> > Well the external 32k and internal divided down version go through the
> > same mux and so that seems to imply either they are both available to
> > the gptimer or neither is.
> 
> Yep, but the /800 do not get you the 32768...
> and that makes the timer suck.
> Of course this can be dealt with in the clock subsystem
> (I remember Paul said that he will look into that), but it will take time.
> 
> Also, what about having the sys_clk instead of 32k for higher precision?
> Is that possible already (without my patch)?
> 

Yes, it is possible. You can choose it through bootargs.

Thanks,
Vaibhav

> 
> -- 
> Regards,
> Igor.
> 

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


Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-11 Thread Igor Grinberg


On 11/08/12 20:34, Jon Hunter wrote:
> 
> On 11/08/2012 12:17 PM, Paul Walmsley wrote:
>> On Thu, 8 Nov 2012, Jon Hunter wrote:
>>
>>> On 11/08/2012 11:58 AM, Paul Walmsley wrote:
 On Thu, 8 Nov 2012, Jon Hunter wrote:

> Igor was mentioning a h/w scenario where the 32kHz source is not
> present. However, I am not sure which devices support this and is
> applicable too.

 Pretty sure Igor is referring to the AM3517/3505.  This is very poorly 
 documented, but can be observed in the AM3517 TRM Rev B (SPRUGR0B) Figure 
 4-23 "PRM Clock Generator" and the AM3517 DM Rev C (SPRS550C) Section 4 
 "Clock Specifications".
>>>
>>> But AFAICT, even in that h/w configuration the internal 32k
>>> oscillator will be used
>>
>> Just to clarify, there's no internal 32k oscillator used on the 3517/3505; 
>> just a divider from the HF clock.
> 
> Ah yes I see that now!
> 
>>> and so the gptimer will still have a 32k clock source.
>>
>> That's a good question and you might want to check with Igor on that one,
>> the AM3517 TRM conflicts with the DM as to whether it's available to the 
>> GPTIMER or not :-(
> 
> Well the external 32k and internal divided down version go through the
> same mux and so that seems to imply either they are both available to
> the gptimer or neither is.

Yep, but the /800 do not get you the 32768...
and that makes the timer suck.
Of course this can be dealt with in the clock subsystem
(I remember Paul said that he will look into that), but it will take time.

Also, what about having the sys_clk instead of 32k for higher precision?
Is that possible already (without my patch)?


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


Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-11 Thread Igor Grinberg


On 11/08/12 21:16, Jon Hunter wrote:
> 
> On 11/08/2012 12:59 PM, Hiremath, Vaibhav wrote:
>> On Fri, Nov 09, 2012 at 00:24:23, Hunter, Jon wrote:
>>>
>>> On 11/08/2012 01:59 AM, Igor Grinberg wrote:
>>>
>>> [snip]
>>>
 There is no reliable way to determine which source should be used in 
 runtime
 for boards that do not have the 32k oscillator wired.
>>>
>>> So thinking about this some more and given that we are moving away from
>>> board files, if a board does not provide a 32kHz clock source, then this
>>> should be reflected in the device-tree source file for that board.
>>> Hence, at boot time we should be able to determine if a 32kHz clock
>>> source can be used.
>>>
>>
>> Let me feed some more thoughts here :)
>>
>> The way it is being detected currently is based on timer idle status bit.
>> I am worried that, this is the only option we have.
> 
> Why not use device-tree to indicate the presence of a 32k clock source?
> This seems like a board level configuration and so device-tree seems to
> be the perfect place for this IMO.

Well, that is what my commit message says...


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


Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-11 Thread Igor Grinberg
On 11/08/12 18:16, Jon Hunter wrote:
> 
> On 11/08/2012 01:59 AM, Igor Grinberg wrote:
>> On 11/07/12 23:36, Jon Hunter wrote:
>>> Hi Igor,
>>>
>>> On 11/07/2012 08:42 AM, Igor Grinberg wrote:
 CONFIG_OMAP_32K_TIMER is kind of standing on the single zImage way.
 Make OMAP2+ timer code independant from the CONFIG_OMAP_32K_TIMER
 setting.
 To remove the dependancy, several conversions/additions had to be done:
 1) Timer structures and initialization functions are named by the platform
name and the clock source in use. The decision which timer is
used is done statically from the machine_desc structure. In the
future it should come from DT.
 2) Settings under the CONFIG_OMAP_32K_TIMER option are expanded into
separate timer structures along with the timer init functions.
This removes the CONFIG_OMAP_32K_TIMER on OMAP2+ timer code.
 3) Since we have all the timers defined inside machine_desc structure
and we no longer need the fallback to gp_timer clock source in case
32k_timer clock source is unavailable (namely on AM33xx), we no
longer need the #ifdef around __omap2_sync32k_clocksource_init()
function. Remove the #ifdef CONFIG_OMAP_32K_TIMER around the
__omap2_sync32k_clocksource_init() function.

 Signed-off-by: Igor Grinberg 
 Cc: Jon Hunter 
 Cc: Santosh Shilimkar 
 Cc: Vaibhav Hiremath 
>>>
>>> [snip]
>>>
 diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
 index 684d2fc..a4ad7a0 100644
 --- a/arch/arm/mach-omap2/timer.c
 +++ b/arch/arm/mach-omap2/timer.c
 @@ -63,20 +63,8 @@
  #define OMAP2_32K_SOURCE  "func_32k_ck"
  #define OMAP3_32K_SOURCE  "omap_32k_fck"
  #define OMAP4_32K_SOURCE  "sys_32k_ck"
 -
 -#ifdef CONFIG_OMAP_32K_TIMER
 -#define OMAP2_CLKEV_SOURCEOMAP2_32K_SOURCE
 -#define OMAP3_CLKEV_SOURCEOMAP3_32K_SOURCE
 -#define OMAP4_CLKEV_SOURCEOMAP4_32K_SOURCE
 -#define OMAP3_SECURE_TIMER12
  #define TIMER_PROP_SECURE "ti,timer-secure"
 -#else
 -#define OMAP2_CLKEV_SOURCEOMAP2_MPU_SOURCE
 -#define OMAP3_CLKEV_SOURCEOMAP3_MPU_SOURCE
 -#define OMAP4_CLKEV_SOURCEOMAP4_MPU_SOURCE
 -#define OMAP3_SECURE_TIMER1
 -#define TIMER_PROP_SECURE "ti,timer-alwon"
 -#endif
 +#define TIMER_PROP_ALWON  "ti,timer-alwon"
>>>
>>> Nit-pick, can we drop the TIMER_PROP_SECURE/ALWON and use the
>>> "ti,timer-secure" and "ti,timer-alwon" directly?
>>>
>>> Initially, I also defined TIMER_PROP_ALWON and Rob Herring's feedback
>>> was to drop this and use the property string directly to remove any
>>> abstraction.
>>
>> Well, I don't understand what do you mean by "any abstraction".
>> The purpose of defining those two was to eliminate multiple occurrences
>> of the string in the code, so for example if someone decides to change the
>> property string for some currently unknown reason - it will be easy and 
>> small.
>> Defines are a common practice for that, no?
>> If you still think it should be inlined, I will do.
> 
> I understand your point, but right now I don't anticipate that we will
> have many options here and so I think that we should drop these.
> 
  #define REALTIME_COUNTER_BASE 0x48243200
  #define INCREMENTER_NUMERATOR_OFFSET  0x10
 @@ -216,7 +204,7 @@ void __init omap_dmtimer_init(void)
  
/* If we are a secure device, remove any secure timer nodes */
if ((omap_type() != OMAP2_DEVICE_TYPE_GP)) {
 -  np = omap_get_timer_dt(omap_timer_match, "ti,timer-secure");
 +  np = omap_get_timer_dt(omap_timer_match, TIMER_PROP_SECURE);
if (np)
of_node_put(np);
}
 @@ -378,9 +366,8 @@ static u32 notrace dmtimer_read_sched_clock(void)
return 0;
  }
  
 -#ifdef CONFIG_OMAP_32K_TIMER
  /* Setup free-running counter for clocksource */
 -static int __init omap2_sync32k_clocksource_init(void)
 +static int __init __omap2_sync32k_clocksource_init(void)
>>>
>>> Not sure I follow why you renamed this function here ...
>>
>> I didn't want to add unused arguments to this function, so I've made a
>> wrapper below to have both the sync32k and the gp functions have the same
>> signature (argument list) and be called from a single macro.
>> Anyway, see below.
> 
> Ok.
> 
>>>
  {
int ret;
struct device_node *np = NULL;
 @@ -439,15 +426,9 @@ static int __init omap2_sync32k_clocksource_init(void)
  
return ret;
  }
 -#else
 -static inline int omap2_sync32k_clocksource_init(void)
 -{
 -  return -ENODEV;
 -}
 -#endif
  
 -static void __init omap2_gptimer_clocksource_init(int gptimer_id,
 -  const char *fck_source)
 +static void __init omap2_gp_cloc

Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-11 Thread Igor Grinberg
On 11/08/12 18:20, Tony Lindgren wrote:
> * Igor Grinberg  [121107 23:15]:
>> On 11/07/12 19:33, Tony Lindgren wrote:
>>>
>>> I think this should be the default for the timers as that counter
>>> does not stop during deeper idle states.
>>
>> Well, it is the default as you can see from the patch.
>> The problem is that for boards that for some reason do not have
>> the 32k wired and rely on MPU/GP timer source, the default will not work
>> and currently there is no way for board to specify which timer source
>> it can use.
> 
> Yes. I was just wondering if we can avoid patching all the board
> files by doing it the other way around by introducing a new
> omap_gp_timer rather than renaming all the existing ones?

Is the renaming that bad? One line per machine_desc structure?
Of course we can skip the renaming, but then it will be less consistent
and will not reflect the actual timer source used.
I tried to make it flexible as much as possible and self explanatory.
So above are my considerations, but at this point in time I don't really
care if we rename them or just add a new one, but we have to get rid of
the ugly fall back.

> 
>> We have discussed this in San Diego (remember?) and you actually proposed
>> this way as a solution. Well, may be I took it a bit further than you
>> thought, but this is because the board code cannot know which timer source
>> should be used at runtime and the fall back described below, does not work.
> 
> Yes thanks I agree we should get rid of that Kconfig option for sure. 
> 
 --- a/arch/arm/mach-omap2/board-2430sdp.c
 +++ b/arch/arm/mach-omap2/board-2430sdp.c
 @@ -284,6 +284,6 @@ MACHINE_START(OMAP_2430SDP, "OMAP2430 sdp2430 board")
.handle_irq = omap2_intc_handle_irq,
.init_machine   = omap_2430sdp_init,
.init_late  = omap2430_init_late,
 -  .timer  = &omap2_timer,
 +  .timer  = &omap2_sync32k_timer,
.restart= omap_prcm_restart,
  MACHINE_END
 --- a/arch/arm/mach-omap2/board-3430sdp.c
 +++ b/arch/arm/mach-omap2/board-3430sdp.c
 @@ -596,6 +596,6 @@ MACHINE_START(OMAP_3430SDP, "OMAP3430 3430SDP board")
.handle_irq = omap3_intc_handle_irq,
.init_machine   = omap_3430sdp_init,
.init_late  = omap3430_init_late,
 -  .timer  = &omap3_timer,
 +  .timer  = &omap3_sync32k_timer,
.restart= omap_prcm_restart,
  MACHINE_END
>>> ...
>>>
>>> Can't we assume that the default timer is omap[234]_sync32k_timer to
>>> avoid renaming the timer entries in all the board files?
>>
>> Hmmm...
>> How will this work with the macros defining the sys_timer structure?
>> I would also not want to hide the exact timer used under the default name.
> 
> Can't you just add a new sys_timer (or a new macro) for GP only setups? 

Of course I can... but I tried to create a flexible generic code, so
no meter how a board will be wired, you just need to specify which timer source
it uses and be done with it.

>  
>>> Then we just need a new timer entries for the hardware that does
>>> not have the sycn32k_timer available?
>>
>> Well, I tried to make it small patch just for the hardware that needs it,
>> but I always found some corner case where, IMHO, this does not work/look 
>> good.
> 
> Can you explain a bit further?

Yes, OMAP4 has its own "local" timer function, OMAP5 - the real time timer
function, we have that fall back from 32k to gptimer for AM33xx and we also
have the use_gptimer_clksrc parameter.
All the above call the same timer source init functions at the end.

> 
> I guess what I'm after is just to avoid renaming the existing
> timers in the board-*.c files and only rename the ones that
> need gp timer only.

This means: get rid of the 32k to gptimer fall back.
I've got your point, will cook something shortly.


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


Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-08 Thread Jon Hunter

On 11/08/2012 12:06 PM, Hiremath, Vaibhav wrote:
> On Thu, Nov 08, 2012 at 23:28:53, Hunter, Jon wrote:
>>
>> On 11/08/2012 11:47 AM, Hiremath, Vaibhav wrote:
>>> On Thu, Nov 08, 2012 at 23:09:57, Hunter, Jon wrote:
>>
>> [snip]
>>
 I think you are missing the point here. For OMAP devices we have two
 main external clock sources which are the 32kHz clock and the sys_clk
 (can be a range of frequencies from 12-38.4MHz for OMAP4). The point is
 for OMAP these clock sources are always present and AFAIK there is no
 h/w configuration that allows you not to have the 32kHz clock source.
 PRCM needs it and I think for OMAP1 the reset logic needs it (if memory
 serves).

 Igor was mentioning a h/w scenario where the 32kHz source is not
 present. However, I am not sure which devices support this and is
 applicable too.

 So we are not discussing the 32k-sync-timer here. We are discussing
 whether there are any device configurations where a 32k clock source
 would not be available for the gptimer.

>>>
>>> Exactly that is the point I am trying to make here,
>>>
>>> In case of AM33xx, it is certainly possible to build the system without 
>>> this 32Khz clock. 
>>>
>>> Interesting point here is, this 32KHz clock is external clock coming from 
>>> crystal connected to in-built RTC module.
>>
>> Thanks. Looking at the AM3358 data manual it states ...
>>
>> "The OSC1 oscillator provides a 32.768-kHz reference clock to the
>> real-time clock (RTC) and is connected to the RTC_XTALIN and RTC_XTALOUT
>> terminals. OSC1 is disabled by default after power is applied. This
>> clock input is optional and may not be required if the RTC is configured
>> to receive a clock from the internal 32k RC oscillator (CLK_RC32K) or
>> peripheral PLL (CLK_32KHZ) which receives a reference clock from the
>> OSC0 input."
>>
>> So what is clear to me that an external 32k clock source is optional.
>> However, it still appears that you will always have an internal 32k
>> source and so regardless of the h/w configuration, a gptimer will always
>> have an 32k clock available on the AM335x devices. Is that correct?
>>
> 
> Internal RC oscillator is not accurate at all, so not guaranteed to give 
> accurate 32.768Hz clock. The oscillation band is from 16Khz to 64Khz.
> 
> So it may not be useful as a system clocks, right?

By the way, according to the AM3358 data manual (paragraph above), even
if there is no external 32k clock source and if you don't use the
internal 32k oscillator, you can still generate a 32k clock from the PER
PLL (CLK_32KHZ). So therefore, there should always be a 32k clock source
available for the gptimer. Is that correct?

At the end of the day, I am just trying to understand if any OMAP/AM
device supports a h/w configuration where there is no 32k clock source
available for the gptimer.

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


Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-08 Thread Jon Hunter

On 11/08/2012 12:59 PM, Hiremath, Vaibhav wrote:
> On Fri, Nov 09, 2012 at 00:24:23, Hunter, Jon wrote:
>>
>> On 11/08/2012 01:59 AM, Igor Grinberg wrote:
>>
>> [snip]
>>
>>> There is no reliable way to determine which source should be used in runtime
>>> for boards that do not have the 32k oscillator wired.
>>
>> So thinking about this some more and given that we are moving away from
>> board files, if a board does not provide a 32kHz clock source, then this
>> should be reflected in the device-tree source file for that board.
>> Hence, at boot time we should be able to determine if a 32kHz clock
>> source can be used.
>>
> 
> Let me feed some more thoughts here :)
> 
> The way it is being detected currently is based on timer idle status bit.
> I am worried that, this is the only option we have.

Why not use device-tree to indicate the presence of a 32k clock source?
This seems like a board level configuration and so device-tree seems to
be the perfect place for this IMO.

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


RE: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-08 Thread Hiremath, Vaibhav
On Fri, Nov 09, 2012 at 00:24:23, Hunter, Jon wrote:
> 
> On 11/08/2012 01:59 AM, Igor Grinberg wrote:
> 
> [snip]
> 
> > There is no reliable way to determine which source should be used in runtime
> > for boards that do not have the 32k oscillator wired.
> 
> So thinking about this some more and given that we are moving away from
> board files, if a board does not provide a 32kHz clock source, then this
> should be reflected in the device-tree source file for that board.
> Hence, at boot time we should be able to determine if a 32kHz clock
> source can be used.
> 

Let me feed some more thoughts here :)

The way it is being detected currently is based on timer idle status bit.
I am worried that, this is the only option we have.

You can also refer to the implementation, so that it will help us to 
conclude on this -

http://arago-project.org/git/projects/?p=linux-am33x.git;a=commitdiff;h=58abec6ac010f4f8818caa4a52d16c4802e14acc

Note that this is based on v3.2 kernel (+ additional patches), you should 
look the implementation of function omap_dm_timer_switch_src().

Thanks,
Vaibhav
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-08 Thread Jon Hunter

On 11/08/2012 01:59 AM, Igor Grinberg wrote:

[snip]

> There is no reliable way to determine which source should be used in runtime
> for boards that do not have the 32k oscillator wired.

So thinking about this some more and given that we are moving away from
board files, if a board does not provide a 32kHz clock source, then this
should be reflected in the device-tree source file for that board.
Hence, at boot time we should be able to determine if a 32kHz clock
source can be used.

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


Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-08 Thread Jon Hunter

On 11/08/2012 12:28 PM, Hiremath, Vaibhav wrote:
> On Thu, Nov 08, 2012 at 23:43:31, Hunter, Jon wrote:
>>
>> On 11/08/2012 12:06 PM, Hiremath, Vaibhav wrote:
>>> On Thu, Nov 08, 2012 at 23:28:53, Hunter, Jon wrote:

 On 11/08/2012 11:47 AM, Hiremath, Vaibhav wrote:
> On Thu, Nov 08, 2012 at 23:09:57, Hunter, Jon wrote:

 [snip]

>> I think you are missing the point here. For OMAP devices we have two
>> main external clock sources which are the 32kHz clock and the sys_clk
>> (can be a range of frequencies from 12-38.4MHz for OMAP4). The point is
>> for OMAP these clock sources are always present and AFAIK there is no
>> h/w configuration that allows you not to have the 32kHz clock source.
>> PRCM needs it and I think for OMAP1 the reset logic needs it (if memory
>> serves).
>>
>> Igor was mentioning a h/w scenario where the 32kHz source is not
>> present. However, I am not sure which devices support this and is
>> applicable too.
>>
>> So we are not discussing the 32k-sync-timer here. We are discussing
>> whether there are any device configurations where a 32k clock source
>> would not be available for the gptimer.
>>
>
> Exactly that is the point I am trying to make here,
>
> In case of AM33xx, it is certainly possible to build the system without 
> this 32Khz clock. 
>
> Interesting point here is, this 32KHz clock is external clock coming from 
> crystal connected to in-built RTC module.

 Thanks. Looking at the AM3358 data manual it states ...

 "The OSC1 oscillator provides a 32.768-kHz reference clock to the
 real-time clock (RTC) and is connected to the RTC_XTALIN and RTC_XTALOUT
 terminals. OSC1 is disabled by default after power is applied. This
 clock input is optional and may not be required if the RTC is configured
 to receive a clock from the internal 32k RC oscillator (CLK_RC32K) or
 peripheral PLL (CLK_32KHZ) which receives a reference clock from the
 OSC0 input."

 So what is clear to me that an external 32k clock source is optional.
 However, it still appears that you will always have an internal 32k
 source and so regardless of the h/w configuration, a gptimer will always
 have an 32k clock available on the AM335x devices. Is that correct?

>>>
>>> Internal RC oscillator is not accurate at all, so not guaranteed to give 
>>> accurate 32.768Hz clock. The oscillation band is from 16Khz to 64Khz.
>>>
>>> So it may not be useful as a system clocks, right?
>>
>> So I admit I am not familiar with the details on the AM stuff side
>> so much.
>>
>> So maybe I should ask this question ...
>>
>> Do you support a configuration where there is no external 32k clock AND
>> the internal oscillator and hence, RTC are not used?
>>
> 
> Why not, you could give-up on persistent time across suspend/resume and use 
> OSC clock as a input clock to timer.
> 
> And the timer code, which we have we have added fallback mechanism for this,
> First make sure that timer is properly set for RTC32K, if yes, then use it, 
> and if no, then fall back to OSC clock.

You mean sync-32k and not rtc32k right? We are just detecting the
presence of the sync-32k counter and if so use it, otherwise use a gptimer.

>> I would have expected that you would always want the RTC running, but if
>> you are saying that you don't require an external 32k and the internal
>> 32k is not accurate, then I assume that having the RTC available is not
>> mandatory either.
>>
> 
> Yes, it is not mandatory, considering fact that, you will not have 
> persistent time and other low-power modes. 
> Actually it depends on board/EVM use-case, right? 

Yes absolutely, just trying to understand if that is a valid use-case or
not. So for AM33xx the external 32k is not mandatory. Thanks.

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


Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-08 Thread Jon Hunter

On 11/08/2012 12:17 PM, Paul Walmsley wrote:
> On Thu, 8 Nov 2012, Jon Hunter wrote:
> 
>> On 11/08/2012 11:58 AM, Paul Walmsley wrote:
>>> On Thu, 8 Nov 2012, Jon Hunter wrote:
>>>
 Igor was mentioning a h/w scenario where the 32kHz source is not
 present. However, I am not sure which devices support this and is
 applicable too.
>>>
>>> Pretty sure Igor is referring to the AM3517/3505.  This is very poorly 
>>> documented, but can be observed in the AM3517 TRM Rev B (SPRUGR0B) Figure 
>>> 4-23 "PRM Clock Generator" and the AM3517 DM Rev C (SPRS550C) Section 4 
>>> "Clock Specifications".
>>
>> But AFAICT, even in that h/w configuration the internal 32k
>> oscillator will be used
> 
> Just to clarify, there's no internal 32k oscillator used on the 3517/3505; 
> just a divider from the HF clock.

Ah yes I see that now!

>> and so the gptimer will still have a 32k clock source.
> 
> That's a good question and you might want to check with Igor on that one,
> the AM3517 TRM conflicts with the DM as to whether it's available to the 
> GPTIMER or not :-(

Well the external 32k and internal divided down version go through the
same mux and so that seems to imply either they are both available to
the gptimer or neither is.

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


RE: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-08 Thread Hiremath, Vaibhav
On Thu, Nov 08, 2012 at 23:43:31, Hunter, Jon wrote:
> 
> On 11/08/2012 12:06 PM, Hiremath, Vaibhav wrote:
> > On Thu, Nov 08, 2012 at 23:28:53, Hunter, Jon wrote:
> >>
> >> On 11/08/2012 11:47 AM, Hiremath, Vaibhav wrote:
> >>> On Thu, Nov 08, 2012 at 23:09:57, Hunter, Jon wrote:
> >>
> >> [snip]
> >>
>  I think you are missing the point here. For OMAP devices we have two
>  main external clock sources which are the 32kHz clock and the sys_clk
>  (can be a range of frequencies from 12-38.4MHz for OMAP4). The point is
>  for OMAP these clock sources are always present and AFAIK there is no
>  h/w configuration that allows you not to have the 32kHz clock source.
>  PRCM needs it and I think for OMAP1 the reset logic needs it (if memory
>  serves).
> 
>  Igor was mentioning a h/w scenario where the 32kHz source is not
>  present. However, I am not sure which devices support this and is
>  applicable too.
> 
>  So we are not discussing the 32k-sync-timer here. We are discussing
>  whether there are any device configurations where a 32k clock source
>  would not be available for the gptimer.
> 
> >>>
> >>> Exactly that is the point I am trying to make here,
> >>>
> >>> In case of AM33xx, it is certainly possible to build the system without 
> >>> this 32Khz clock. 
> >>>
> >>> Interesting point here is, this 32KHz clock is external clock coming from 
> >>> crystal connected to in-built RTC module.
> >>
> >> Thanks. Looking at the AM3358 data manual it states ...
> >>
> >> "The OSC1 oscillator provides a 32.768-kHz reference clock to the
> >> real-time clock (RTC) and is connected to the RTC_XTALIN and RTC_XTALOUT
> >> terminals. OSC1 is disabled by default after power is applied. This
> >> clock input is optional and may not be required if the RTC is configured
> >> to receive a clock from the internal 32k RC oscillator (CLK_RC32K) or
> >> peripheral PLL (CLK_32KHZ) which receives a reference clock from the
> >> OSC0 input."
> >>
> >> So what is clear to me that an external 32k clock source is optional.
> >> However, it still appears that you will always have an internal 32k
> >> source and so regardless of the h/w configuration, a gptimer will always
> >> have an 32k clock available on the AM335x devices. Is that correct?
> >>
> > 
> > Internal RC oscillator is not accurate at all, so not guaranteed to give 
> > accurate 32.768Hz clock. The oscillation band is from 16Khz to 64Khz.
> > 
> > So it may not be useful as a system clocks, right?
> 
> So I admit I am not familiar with the details on the AM stuff side
> so much.
> 
> So maybe I should ask this question ...
> 
> Do you support a configuration where there is no external 32k clock AND
> the internal oscillator and hence, RTC are not used?
> 

Why not, you could give-up on persistent time across suspend/resume and use 
OSC clock as a input clock to timer.

And the timer code, which we have we have added fallback mechanism for this,
First make sure that timer is properly set for RTC32K, if yes, then use it, 
and if no, then fall back to OSC clock.


> I would have expected that you would always want the RTC running, but if
> you are saying that you don't require an external 32k and the internal
> 32k is not accurate, then I assume that having the RTC available is not
> mandatory either.
> 

Yes, it is not mandatory, considering fact that, you will not have 
persistent time and other low-power modes. 
Actually it depends on board/EVM use-case, right? 

Thanks,
Vaibhav

> Jon
> 

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


Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-08 Thread Paul Walmsley
On Thu, 8 Nov 2012, Jon Hunter wrote:

> On 11/08/2012 11:58 AM, Paul Walmsley wrote:
> > On Thu, 8 Nov 2012, Jon Hunter wrote:
> > 
> >> Igor was mentioning a h/w scenario where the 32kHz source is not
> >> present. However, I am not sure which devices support this and is
> >> applicable too.
> > 
> > Pretty sure Igor is referring to the AM3517/3505.  This is very poorly 
> > documented, but can be observed in the AM3517 TRM Rev B (SPRUGR0B) Figure 
> > 4-23 "PRM Clock Generator" and the AM3517 DM Rev C (SPRS550C) Section 4 
> > "Clock Specifications".
> 
> But AFAICT, even in that h/w configuration the internal 32k
> oscillator will be used

Just to clarify, there's no internal 32k oscillator used on the 3517/3505; 
just a divider from the HF clock.

> and so the gptimer will still have a 32k clock source.

That's a good question and you might want to check with Igor on that one,
the AM3517 TRM conflicts with the DM as to whether it's available to the 
GPTIMER or not :-(


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


Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-08 Thread Jon Hunter

On 11/08/2012 11:47 AM, Hiremath, Vaibhav wrote:
> On Thu, Nov 08, 2012 at 23:09:57, Hunter, Jon wrote:

[snip]

>> I think you are missing the point here. For OMAP devices we have two
>> main external clock sources which are the 32kHz clock and the sys_clk
>> (can be a range of frequencies from 12-38.4MHz for OMAP4). The point is
>> for OMAP these clock sources are always present and AFAIK there is no
>> h/w configuration that allows you not to have the 32kHz clock source.
>> PRCM needs it and I think for OMAP1 the reset logic needs it (if memory
>> serves).
>>
>> Igor was mentioning a h/w scenario where the 32kHz source is not
>> present. However, I am not sure which devices support this and is
>> applicable too.
>>
>> So we are not discussing the 32k-sync-timer here. We are discussing
>> whether there are any device configurations where a 32k clock source
>> would not be available for the gptimer.
>>
> 
> Exactly that is the point I am trying to make here,
> 
> In case of AM33xx, it is certainly possible to build the system without 
> this 32Khz clock. 
> 
> Interesting point here is, this 32KHz clock is external clock coming from 
> crystal connected to in-built RTC module.

Thanks. Looking at the AM3358 data manual it states ...

"The OSC1 oscillator provides a 32.768-kHz reference clock to the
real-time clock (RTC) and is connected to the RTC_XTALIN and RTC_XTALOUT
terminals. OSC1 is disabled by default after power is applied. This
clock input is optional and may not be required if the RTC is configured
to receive a clock from the internal 32k RC oscillator (CLK_RC32K) or
peripheral PLL (CLK_32KHZ) which receives a reference clock from the
OSC0 input."

So what is clear to me that an external 32k clock source is optional.
However, it still appears that you will always have an internal 32k
source and so regardless of the h/w configuration, a gptimer will always
have an 32k clock available on the AM335x devices. Is that correct?

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


Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-08 Thread Jon Hunter

On 11/08/2012 12:06 PM, Hiremath, Vaibhav wrote:
> On Thu, Nov 08, 2012 at 23:28:53, Hunter, Jon wrote:
>>
>> On 11/08/2012 11:47 AM, Hiremath, Vaibhav wrote:
>>> On Thu, Nov 08, 2012 at 23:09:57, Hunter, Jon wrote:
>>
>> [snip]
>>
 I think you are missing the point here. For OMAP devices we have two
 main external clock sources which are the 32kHz clock and the sys_clk
 (can be a range of frequencies from 12-38.4MHz for OMAP4). The point is
 for OMAP these clock sources are always present and AFAIK there is no
 h/w configuration that allows you not to have the 32kHz clock source.
 PRCM needs it and I think for OMAP1 the reset logic needs it (if memory
 serves).

 Igor was mentioning a h/w scenario where the 32kHz source is not
 present. However, I am not sure which devices support this and is
 applicable too.

 So we are not discussing the 32k-sync-timer here. We are discussing
 whether there are any device configurations where a 32k clock source
 would not be available for the gptimer.

>>>
>>> Exactly that is the point I am trying to make here,
>>>
>>> In case of AM33xx, it is certainly possible to build the system without 
>>> this 32Khz clock. 
>>>
>>> Interesting point here is, this 32KHz clock is external clock coming from 
>>> crystal connected to in-built RTC module.
>>
>> Thanks. Looking at the AM3358 data manual it states ...
>>
>> "The OSC1 oscillator provides a 32.768-kHz reference clock to the
>> real-time clock (RTC) and is connected to the RTC_XTALIN and RTC_XTALOUT
>> terminals. OSC1 is disabled by default after power is applied. This
>> clock input is optional and may not be required if the RTC is configured
>> to receive a clock from the internal 32k RC oscillator (CLK_RC32K) or
>> peripheral PLL (CLK_32KHZ) which receives a reference clock from the
>> OSC0 input."
>>
>> So what is clear to me that an external 32k clock source is optional.
>> However, it still appears that you will always have an internal 32k
>> source and so regardless of the h/w configuration, a gptimer will always
>> have an 32k clock available on the AM335x devices. Is that correct?
>>
> 
> Internal RC oscillator is not accurate at all, so not guaranteed to give 
> accurate 32.768Hz clock. The oscillation band is from 16Khz to 64Khz.
> 
> So it may not be useful as a system clocks, right?

So I admit I am not familiar with the details on the AM stuff side
so much.

So maybe I should ask this question ...

Do you support a configuration where there is no external 32k clock AND
the internal oscillator and hence, RTC are not used?

I would have expected that you would always want the RTC running, but if
you are saying that you don't require an external 32k and the internal
32k is not accurate, then I assume that having the RTC available is not
mandatory either.

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


Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-08 Thread Jon Hunter

On 11/08/2012 11:58 AM, Paul Walmsley wrote:
> On Thu, 8 Nov 2012, Jon Hunter wrote:
> 
>> Igor was mentioning a h/w scenario where the 32kHz source is not
>> present. However, I am not sure which devices support this and is
>> applicable too.
> 
> Pretty sure Igor is referring to the AM3517/3505.  This is very poorly 
> documented, but can be observed in the AM3517 TRM Rev B (SPRUGR0B) Figure 
> 4-23 "PRM Clock Generator" and the AM3517 DM Rev C (SPRS550C) Section 4 
> "Clock Specifications".

Thanks Paul. But AFAICT, even in that h/w configuration the internal 32k
oscillator will be used and so the gptimer will still have a 32k clock
source.

So I still don't see a use-case where the gptimer would not have a 32k
source available. Admittedly, I could be missing one somewhere or I am
just plain old wrong ...

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


RE: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-08 Thread Hiremath, Vaibhav
On Thu, Nov 08, 2012 at 23:28:53, Hunter, Jon wrote:
> 
> On 11/08/2012 11:47 AM, Hiremath, Vaibhav wrote:
> > On Thu, Nov 08, 2012 at 23:09:57, Hunter, Jon wrote:
> 
> [snip]
> 
> >> I think you are missing the point here. For OMAP devices we have two
> >> main external clock sources which are the 32kHz clock and the sys_clk
> >> (can be a range of frequencies from 12-38.4MHz for OMAP4). The point is
> >> for OMAP these clock sources are always present and AFAIK there is no
> >> h/w configuration that allows you not to have the 32kHz clock source.
> >> PRCM needs it and I think for OMAP1 the reset logic needs it (if memory
> >> serves).
> >>
> >> Igor was mentioning a h/w scenario where the 32kHz source is not
> >> present. However, I am not sure which devices support this and is
> >> applicable too.
> >>
> >> So we are not discussing the 32k-sync-timer here. We are discussing
> >> whether there are any device configurations where a 32k clock source
> >> would not be available for the gptimer.
> >>
> > 
> > Exactly that is the point I am trying to make here,
> > 
> > In case of AM33xx, it is certainly possible to build the system without 
> > this 32Khz clock. 
> > 
> > Interesting point here is, this 32KHz clock is external clock coming from 
> > crystal connected to in-built RTC module.
> 
> Thanks. Looking at the AM3358 data manual it states ...
> 
> "The OSC1 oscillator provides a 32.768-kHz reference clock to the
> real-time clock (RTC) and is connected to the RTC_XTALIN and RTC_XTALOUT
> terminals. OSC1 is disabled by default after power is applied. This
> clock input is optional and may not be required if the RTC is configured
> to receive a clock from the internal 32k RC oscillator (CLK_RC32K) or
> peripheral PLL (CLK_32KHZ) which receives a reference clock from the
> OSC0 input."
> 
> So what is clear to me that an external 32k clock source is optional.
> However, it still appears that you will always have an internal 32k
> source and so regardless of the h/w configuration, a gptimer will always
> have an 32k clock available on the AM335x devices. Is that correct?
> 

Internal RC oscillator is not accurate at all, so not guaranteed to give 
accurate 32.768Hz clock. The oscillation band is from 16Khz to 64Khz.

So it may not be useful as a system clocks, right?

Thanks,
Vaibhav

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


Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-08 Thread Paul Walmsley
On Thu, 8 Nov 2012, Jon Hunter wrote:

> Igor was mentioning a h/w scenario where the 32kHz source is not
> present. However, I am not sure which devices support this and is
> applicable too.

Pretty sure Igor is referring to the AM3517/3505.  This is very poorly 
documented, but can be observed in the AM3517 TRM Rev B (SPRUGR0B) Figure 
4-23 "PRM Clock Generator" and the AM3517 DM Rev C (SPRS550C) Section 4 
"Clock Specifications".


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


RE: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-08 Thread Hiremath, Vaibhav
On Thu, Nov 08, 2012 at 23:09:57, Hunter, Jon wrote:
> 
> On 11/08/2012 11:08 AM, Hiremath, Vaibhav wrote:
> > On Thu, Nov 08, 2012 at 21:46:53, Hunter, Jon wrote:
> >>
> >> On 11/08/2012 01:59 AM, Igor Grinberg wrote:
> >>> On 11/07/12 23:36, Jon Hunter wrote:
>  Hi Igor,
> 
>  On 11/07/2012 08:42 AM, Igor Grinberg wrote:
> > CONFIG_OMAP_32K_TIMER is kind of standing on the single zImage way.
> > Make OMAP2+ timer code independant from the CONFIG_OMAP_32K_TIMER
> > setting.
> > To remove the dependancy, several conversions/additions had to be done:
> > 1) Timer structures and initialization functions are named by the 
> > platform
> >name and the clock source in use. The decision which timer is
> >used is done statically from the machine_desc structure. In the
> >future it should come from DT.
> > 2) Settings under the CONFIG_OMAP_32K_TIMER option are expanded into
> >separate timer structures along with the timer init functions.
> >This removes the CONFIG_OMAP_32K_TIMER on OMAP2+ timer code.
> > 3) Since we have all the timers defined inside machine_desc structure
> >and we no longer need the fallback to gp_timer clock source in case
> >32k_timer clock source is unavailable (namely on AM33xx), we no
> >longer need the #ifdef around __omap2_sync32k_clocksource_init()
> >function. Remove the #ifdef CONFIG_OMAP_32K_TIMER around the
> >__omap2_sync32k_clocksource_init() function.
> >
> > Signed-off-by: Igor Grinberg 
> > Cc: Jon Hunter 
> > Cc: Santosh Shilimkar 
> > Cc: Vaibhav Hiremath 
> 
>  [snip]
> 
> > diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
> > index 684d2fc..a4ad7a0 100644
> > --- a/arch/arm/mach-omap2/timer.c
> > +++ b/arch/arm/mach-omap2/timer.c
> > @@ -63,20 +63,8 @@
> >  #define OMAP2_32K_SOURCE   "func_32k_ck"
> >  #define OMAP3_32K_SOURCE   "omap_32k_fck"
> >  #define OMAP4_32K_SOURCE   "sys_32k_ck"
> > -
> > -#ifdef CONFIG_OMAP_32K_TIMER
> > -#define OMAP2_CLKEV_SOURCE OMAP2_32K_SOURCE
> > -#define OMAP3_CLKEV_SOURCE OMAP3_32K_SOURCE
> > -#define OMAP4_CLKEV_SOURCE OMAP4_32K_SOURCE
> > -#define OMAP3_SECURE_TIMER 12
> >  #define TIMER_PROP_SECURE  "ti,timer-secure"
> > -#else
> > -#define OMAP2_CLKEV_SOURCE OMAP2_MPU_SOURCE
> > -#define OMAP3_CLKEV_SOURCE OMAP3_MPU_SOURCE
> > -#define OMAP4_CLKEV_SOURCE OMAP4_MPU_SOURCE
> > -#define OMAP3_SECURE_TIMER 1
> > -#define TIMER_PROP_SECURE  "ti,timer-alwon"
> > -#endif
> > +#define TIMER_PROP_ALWON   "ti,timer-alwon"
> 
>  Nit-pick, can we drop the TIMER_PROP_SECURE/ALWON and use the
>  "ti,timer-secure" and "ti,timer-alwon" directly?
> 
>  Initially, I also defined TIMER_PROP_ALWON and Rob Herring's feedback
>  was to drop this and use the property string directly to remove any
>  abstraction.
> >>>
> >>> Well, I don't understand what do you mean by "any abstraction".
> >>> The purpose of defining those two was to eliminate multiple occurrences
> >>> of the string in the code, so for example if someone decides to change the
> >>> property string for some currently unknown reason - it will be easy and 
> >>> small.
> >>> Defines are a common practice for that, no?
> >>> If you still think it should be inlined, I will do.
> >>
> >> I understand your point, but right now I don't anticipate that we will
> >> have many options here and so I think that we should drop these.
> >>
> >  #define REALTIME_COUNTER_BASE  0x48243200
> >  #define INCREMENTER_NUMERATOR_OFFSET   0x10
> > @@ -216,7 +204,7 @@ void __init omap_dmtimer_init(void)
> >  
> > /* If we are a secure device, remove any secure timer nodes */
> > if ((omap_type() != OMAP2_DEVICE_TYPE_GP)) {
> > -   np = omap_get_timer_dt(omap_timer_match, 
> > "ti,timer-secure");
> > +   np = omap_get_timer_dt(omap_timer_match, 
> > TIMER_PROP_SECURE);
> > if (np)
> > of_node_put(np);
> > }
> > @@ -378,9 +366,8 @@ static u32 notrace dmtimer_read_sched_clock(void)
> > return 0;
> >  }
> >  
> > -#ifdef CONFIG_OMAP_32K_TIMER
> >  /* Setup free-running counter for clocksource */
> > -static int __init omap2_sync32k_clocksource_init(void)
> > +static int __init __omap2_sync32k_clocksource_init(void)
> 
>  Not sure I follow why you renamed this function here ...
> >>>
> >>> I didn't want to add unused arguments to this function, so I've made a
> >>> wrapper below to have both the sync32k and the gp functions have the same
> >>> signature (argument list) and be called from a single macro.
> >>> Anyway, see below.
> >>
> >> Ok.
> >>
> >>

Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-08 Thread Jon Hunter

On 11/08/2012 11:08 AM, Hiremath, Vaibhav wrote:
> On Thu, Nov 08, 2012 at 21:46:53, Hunter, Jon wrote:
>>
>> On 11/08/2012 01:59 AM, Igor Grinberg wrote:
>>> On 11/07/12 23:36, Jon Hunter wrote:
 Hi Igor,

 On 11/07/2012 08:42 AM, Igor Grinberg wrote:
> CONFIG_OMAP_32K_TIMER is kind of standing on the single zImage way.
> Make OMAP2+ timer code independant from the CONFIG_OMAP_32K_TIMER
> setting.
> To remove the dependancy, several conversions/additions had to be done:
> 1) Timer structures and initialization functions are named by the platform
>name and the clock source in use. The decision which timer is
>used is done statically from the machine_desc structure. In the
>future it should come from DT.
> 2) Settings under the CONFIG_OMAP_32K_TIMER option are expanded into
>separate timer structures along with the timer init functions.
>This removes the CONFIG_OMAP_32K_TIMER on OMAP2+ timer code.
> 3) Since we have all the timers defined inside machine_desc structure
>and we no longer need the fallback to gp_timer clock source in case
>32k_timer clock source is unavailable (namely on AM33xx), we no
>longer need the #ifdef around __omap2_sync32k_clocksource_init()
>function. Remove the #ifdef CONFIG_OMAP_32K_TIMER around the
>__omap2_sync32k_clocksource_init() function.
>
> Signed-off-by: Igor Grinberg 
> Cc: Jon Hunter 
> Cc: Santosh Shilimkar 
> Cc: Vaibhav Hiremath 

 [snip]

> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
> index 684d2fc..a4ad7a0 100644
> --- a/arch/arm/mach-omap2/timer.c
> +++ b/arch/arm/mach-omap2/timer.c
> @@ -63,20 +63,8 @@
>  #define OMAP2_32K_SOURCE "func_32k_ck"
>  #define OMAP3_32K_SOURCE "omap_32k_fck"
>  #define OMAP4_32K_SOURCE "sys_32k_ck"
> -
> -#ifdef CONFIG_OMAP_32K_TIMER
> -#define OMAP2_CLKEV_SOURCE   OMAP2_32K_SOURCE
> -#define OMAP3_CLKEV_SOURCE   OMAP3_32K_SOURCE
> -#define OMAP4_CLKEV_SOURCE   OMAP4_32K_SOURCE
> -#define OMAP3_SECURE_TIMER   12
>  #define TIMER_PROP_SECURE"ti,timer-secure"
> -#else
> -#define OMAP2_CLKEV_SOURCE   OMAP2_MPU_SOURCE
> -#define OMAP3_CLKEV_SOURCE   OMAP3_MPU_SOURCE
> -#define OMAP4_CLKEV_SOURCE   OMAP4_MPU_SOURCE
> -#define OMAP3_SECURE_TIMER   1
> -#define TIMER_PROP_SECURE"ti,timer-alwon"
> -#endif
> +#define TIMER_PROP_ALWON "ti,timer-alwon"

 Nit-pick, can we drop the TIMER_PROP_SECURE/ALWON and use the
 "ti,timer-secure" and "ti,timer-alwon" directly?

 Initially, I also defined TIMER_PROP_ALWON and Rob Herring's feedback
 was to drop this and use the property string directly to remove any
 abstraction.
>>>
>>> Well, I don't understand what do you mean by "any abstraction".
>>> The purpose of defining those two was to eliminate multiple occurrences
>>> of the string in the code, so for example if someone decides to change the
>>> property string for some currently unknown reason - it will be easy and 
>>> small.
>>> Defines are a common practice for that, no?
>>> If you still think it should be inlined, I will do.
>>
>> I understand your point, but right now I don't anticipate that we will
>> have many options here and so I think that we should drop these.
>>
>  #define REALTIME_COUNTER_BASE0x48243200
>  #define INCREMENTER_NUMERATOR_OFFSET 0x10
> @@ -216,7 +204,7 @@ void __init omap_dmtimer_init(void)
>  
>   /* If we are a secure device, remove any secure timer nodes */
>   if ((omap_type() != OMAP2_DEVICE_TYPE_GP)) {
> - np = omap_get_timer_dt(omap_timer_match, "ti,timer-secure");
> + np = omap_get_timer_dt(omap_timer_match, TIMER_PROP_SECURE);
>   if (np)
>   of_node_put(np);
>   }
> @@ -378,9 +366,8 @@ static u32 notrace dmtimer_read_sched_clock(void)
>   return 0;
>  }
>  
> -#ifdef CONFIG_OMAP_32K_TIMER
>  /* Setup free-running counter for clocksource */
> -static int __init omap2_sync32k_clocksource_init(void)
> +static int __init __omap2_sync32k_clocksource_init(void)

 Not sure I follow why you renamed this function here ...
>>>
>>> I didn't want to add unused arguments to this function, so I've made a
>>> wrapper below to have both the sync32k and the gp functions have the same
>>> signature (argument list) and be called from a single macro.
>>> Anyway, see below.
>>
>> Ok.
>>

>  {
>   int ret;
>   struct device_node *np = NULL;
> @@ -439,15 +426,9 @@ static int __init 
> omap2_sync32k_clocksource_init(void)
>  
>   return ret;
>  }
> -#else
> -static inline int omap2_sync32k_clocksource_init(void)
> -{
> - return -ENODEV;
> -}
> -#endif
>  
> -

RE: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-08 Thread Hiremath, Vaibhav
On Thu, Nov 08, 2012 at 21:50:05, Tony Lindgren wrote:
> * Igor Grinberg  [121107 23:15]:
> > On 11/07/12 19:33, Tony Lindgren wrote:
> > > 
> > > I think this should be the default for the timers as that counter
> > > does not stop during deeper idle states.
> > 
> > Well, it is the default as you can see from the patch.
> > The problem is that for boards that for some reason do not have
> > the 32k wired and rely on MPU/GP timer source, the default will not work
> > and currently there is no way for board to specify which timer source
> > it can use.
> 
> Yes. I was just wondering if we can avoid patching all the board
> files by doing it the other way around by introducing a new
> omap_gp_timer rather than renaming all the existing ones?
> 
> > We have discussed this in San Diego (remember?) and you actually proposed
> > this way as a solution. Well, may be I took it a bit further than you
> > thought, but this is because the board code cannot know which timer source
> > should be used at runtime and the fall back described below, does not work.
> 
> Yes thanks I agree we should get rid of that Kconfig option for sure. 
> 
> > >> --- a/arch/arm/mach-omap2/board-2430sdp.c
> > >> +++ b/arch/arm/mach-omap2/board-2430sdp.c
> > >> @@ -284,6 +284,6 @@ MACHINE_START(OMAP_2430SDP, "OMAP2430 sdp2430 board")
> > >>  .handle_irq = omap2_intc_handle_irq,
> > >>  .init_machine   = omap_2430sdp_init,
> > >>  .init_late  = omap2430_init_late,
> > >> -.timer  = &omap2_timer,
> > >> +.timer  = &omap2_sync32k_timer,
> > >>  .restart= omap_prcm_restart,
> > >>  MACHINE_END
> > >> --- a/arch/arm/mach-omap2/board-3430sdp.c
> > >> +++ b/arch/arm/mach-omap2/board-3430sdp.c
> > >> @@ -596,6 +596,6 @@ MACHINE_START(OMAP_3430SDP, "OMAP3430 3430SDP board")
> > >>  .handle_irq = omap3_intc_handle_irq,
> > >>  .init_machine   = omap_3430sdp_init,
> > >>  .init_late  = omap3430_init_late,
> > >> -.timer  = &omap3_timer,
> > >> +.timer  = &omap3_sync32k_timer,
> > >>  .restart= omap_prcm_restart,
> > >>  MACHINE_END
> > > ...
> > > 
> > > Can't we assume that the default timer is omap[234]_sync32k_timer to
> > > avoid renaming the timer entries in all the board files?
> > 
> > Hmmm...
> > How will this work with the macros defining the sys_timer structure?
> > I would also not want to hide the exact timer used under the default name.
> 
> Can't you just add a new sys_timer (or a new macro) for GP only setups? 
>  
> > > Then we just need a new timer entries for the hardware that does
> > > not have the sycn32k_timer available?
> > 
> > Well, I tried to make it small patch just for the hardware that needs it,
> > but I always found some corner case where, IMHO, this does not work/look 
> > good.
> 
> Can you explain a bit further?
> 
> I guess what I'm after is just to avoid renaming the existing
> timers in the board-*.c files and only rename the ones that
> need gp timer only.
> 

That would be AM33xx family :)

Thanks,
Vaibhav

> Regards,
> 
> Tony
> 

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


RE: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-08 Thread Hiremath, Vaibhav
On Thu, Nov 08, 2012 at 21:46:53, Hunter, Jon wrote:
> 
> On 11/08/2012 01:59 AM, Igor Grinberg wrote:
> > On 11/07/12 23:36, Jon Hunter wrote:
> >> Hi Igor,
> >>
> >> On 11/07/2012 08:42 AM, Igor Grinberg wrote:
> >>> CONFIG_OMAP_32K_TIMER is kind of standing on the single zImage way.
> >>> Make OMAP2+ timer code independant from the CONFIG_OMAP_32K_TIMER
> >>> setting.
> >>> To remove the dependancy, several conversions/additions had to be done:
> >>> 1) Timer structures and initialization functions are named by the platform
> >>>name and the clock source in use. The decision which timer is
> >>>used is done statically from the machine_desc structure. In the
> >>>future it should come from DT.
> >>> 2) Settings under the CONFIG_OMAP_32K_TIMER option are expanded into
> >>>separate timer structures along with the timer init functions.
> >>>This removes the CONFIG_OMAP_32K_TIMER on OMAP2+ timer code.
> >>> 3) Since we have all the timers defined inside machine_desc structure
> >>>and we no longer need the fallback to gp_timer clock source in case
> >>>32k_timer clock source is unavailable (namely on AM33xx), we no
> >>>longer need the #ifdef around __omap2_sync32k_clocksource_init()
> >>>function. Remove the #ifdef CONFIG_OMAP_32K_TIMER around the
> >>>__omap2_sync32k_clocksource_init() function.
> >>>
> >>> Signed-off-by: Igor Grinberg 
> >>> Cc: Jon Hunter 
> >>> Cc: Santosh Shilimkar 
> >>> Cc: Vaibhav Hiremath 
> >>
> >> [snip]
> >>
> >>> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
> >>> index 684d2fc..a4ad7a0 100644
> >>> --- a/arch/arm/mach-omap2/timer.c
> >>> +++ b/arch/arm/mach-omap2/timer.c
> >>> @@ -63,20 +63,8 @@
> >>>  #define OMAP2_32K_SOURCE "func_32k_ck"
> >>>  #define OMAP3_32K_SOURCE "omap_32k_fck"
> >>>  #define OMAP4_32K_SOURCE "sys_32k_ck"
> >>> -
> >>> -#ifdef CONFIG_OMAP_32K_TIMER
> >>> -#define OMAP2_CLKEV_SOURCE   OMAP2_32K_SOURCE
> >>> -#define OMAP3_CLKEV_SOURCE   OMAP3_32K_SOURCE
> >>> -#define OMAP4_CLKEV_SOURCE   OMAP4_32K_SOURCE
> >>> -#define OMAP3_SECURE_TIMER   12
> >>>  #define TIMER_PROP_SECURE"ti,timer-secure"
> >>> -#else
> >>> -#define OMAP2_CLKEV_SOURCE   OMAP2_MPU_SOURCE
> >>> -#define OMAP3_CLKEV_SOURCE   OMAP3_MPU_SOURCE
> >>> -#define OMAP4_CLKEV_SOURCE   OMAP4_MPU_SOURCE
> >>> -#define OMAP3_SECURE_TIMER   1
> >>> -#define TIMER_PROP_SECURE"ti,timer-alwon"
> >>> -#endif
> >>> +#define TIMER_PROP_ALWON "ti,timer-alwon"
> >>
> >> Nit-pick, can we drop the TIMER_PROP_SECURE/ALWON and use the
> >> "ti,timer-secure" and "ti,timer-alwon" directly?
> >>
> >> Initially, I also defined TIMER_PROP_ALWON and Rob Herring's feedback
> >> was to drop this and use the property string directly to remove any
> >> abstraction.
> > 
> > Well, I don't understand what do you mean by "any abstraction".
> > The purpose of defining those two was to eliminate multiple occurrences
> > of the string in the code, so for example if someone decides to change the
> > property string for some currently unknown reason - it will be easy and 
> > small.
> > Defines are a common practice for that, no?
> > If you still think it should be inlined, I will do.
> 
> I understand your point, but right now I don't anticipate that we will
> have many options here and so I think that we should drop these.
> 
> >>>  #define REALTIME_COUNTER_BASE0x48243200
> >>>  #define INCREMENTER_NUMERATOR_OFFSET 0x10
> >>> @@ -216,7 +204,7 @@ void __init omap_dmtimer_init(void)
> >>>  
> >>>   /* If we are a secure device, remove any secure timer nodes */
> >>>   if ((omap_type() != OMAP2_DEVICE_TYPE_GP)) {
> >>> - np = omap_get_timer_dt(omap_timer_match, "ti,timer-secure");
> >>> + np = omap_get_timer_dt(omap_timer_match, TIMER_PROP_SECURE);
> >>>   if (np)
> >>>   of_node_put(np);
> >>>   }
> >>> @@ -378,9 +366,8 @@ static u32 notrace dmtimer_read_sched_clock(void)
> >>>   return 0;
> >>>  }
> >>>  
> >>> -#ifdef CONFIG_OMAP_32K_TIMER
> >>>  /* Setup free-running counter for clocksource */
> >>> -static int __init omap2_sync32k_clocksource_init(void)
> >>> +static int __init __omap2_sync32k_clocksource_init(void)
> >>
> >> Not sure I follow why you renamed this function here ...
> > 
> > I didn't want to add unused arguments to this function, so I've made a
> > wrapper below to have both the sync32k and the gp functions have the same
> > signature (argument list) and be called from a single macro.
> > Anyway, see below.
> 
> Ok.
> 
> >>
> >>>  {
> >>>   int ret;
> >>>   struct device_node *np = NULL;
> >>> @@ -439,15 +426,9 @@ static int __init 
> >>> omap2_sync32k_clocksource_init(void)
> >>>  
> >>>   return ret;
> >>>  }
> >>> -#else
> >>> -static inline int omap2_sync32k_clocksource_init(void)
> >>> -{
> >>> - return -ENODEV;
> >>> -}
> >>> -#endif
> >>>  
> >>> -static void __init omap2_gptimer_clocksource_init(int 

Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-08 Thread Tony Lindgren
* Igor Grinberg  [121107 23:15]:
> On 11/07/12 19:33, Tony Lindgren wrote:
> > 
> > I think this should be the default for the timers as that counter
> > does not stop during deeper idle states.
> 
> Well, it is the default as you can see from the patch.
> The problem is that for boards that for some reason do not have
> the 32k wired and rely on MPU/GP timer source, the default will not work
> and currently there is no way for board to specify which timer source
> it can use.

Yes. I was just wondering if we can avoid patching all the board
files by doing it the other way around by introducing a new
omap_gp_timer rather than renaming all the existing ones?

> We have discussed this in San Diego (remember?) and you actually proposed
> this way as a solution. Well, may be I took it a bit further than you
> thought, but this is because the board code cannot know which timer source
> should be used at runtime and the fall back described below, does not work.

Yes thanks I agree we should get rid of that Kconfig option for sure. 

> >> --- a/arch/arm/mach-omap2/board-2430sdp.c
> >> +++ b/arch/arm/mach-omap2/board-2430sdp.c
> >> @@ -284,6 +284,6 @@ MACHINE_START(OMAP_2430SDP, "OMAP2430 sdp2430 board")
> >>.handle_irq = omap2_intc_handle_irq,
> >>.init_machine   = omap_2430sdp_init,
> >>.init_late  = omap2430_init_late,
> >> -  .timer  = &omap2_timer,
> >> +  .timer  = &omap2_sync32k_timer,
> >>.restart= omap_prcm_restart,
> >>  MACHINE_END
> >> --- a/arch/arm/mach-omap2/board-3430sdp.c
> >> +++ b/arch/arm/mach-omap2/board-3430sdp.c
> >> @@ -596,6 +596,6 @@ MACHINE_START(OMAP_3430SDP, "OMAP3430 3430SDP board")
> >>.handle_irq = omap3_intc_handle_irq,
> >>.init_machine   = omap_3430sdp_init,
> >>.init_late  = omap3430_init_late,
> >> -  .timer  = &omap3_timer,
> >> +  .timer  = &omap3_sync32k_timer,
> >>.restart= omap_prcm_restart,
> >>  MACHINE_END
> > ...
> > 
> > Can't we assume that the default timer is omap[234]_sync32k_timer to
> > avoid renaming the timer entries in all the board files?
> 
> Hmmm...
> How will this work with the macros defining the sys_timer structure?
> I would also not want to hide the exact timer used under the default name.

Can't you just add a new sys_timer (or a new macro) for GP only setups? 
 
> > Then we just need a new timer entries for the hardware that does
> > not have the sycn32k_timer available?
> 
> Well, I tried to make it small patch just for the hardware that needs it,
> but I always found some corner case where, IMHO, this does not work/look good.

Can you explain a bit further?

I guess what I'm after is just to avoid renaming the existing
timers in the board-*.c files and only rename the ones that
need gp timer only.

Regards,

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


Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-08 Thread Jon Hunter

On 11/08/2012 01:59 AM, Igor Grinberg wrote:
> On 11/07/12 23:36, Jon Hunter wrote:
>> Hi Igor,
>>
>> On 11/07/2012 08:42 AM, Igor Grinberg wrote:
>>> CONFIG_OMAP_32K_TIMER is kind of standing on the single zImage way.
>>> Make OMAP2+ timer code independant from the CONFIG_OMAP_32K_TIMER
>>> setting.
>>> To remove the dependancy, several conversions/additions had to be done:
>>> 1) Timer structures and initialization functions are named by the platform
>>>name and the clock source in use. The decision which timer is
>>>used is done statically from the machine_desc structure. In the
>>>future it should come from DT.
>>> 2) Settings under the CONFIG_OMAP_32K_TIMER option are expanded into
>>>separate timer structures along with the timer init functions.
>>>This removes the CONFIG_OMAP_32K_TIMER on OMAP2+ timer code.
>>> 3) Since we have all the timers defined inside machine_desc structure
>>>and we no longer need the fallback to gp_timer clock source in case
>>>32k_timer clock source is unavailable (namely on AM33xx), we no
>>>longer need the #ifdef around __omap2_sync32k_clocksource_init()
>>>function. Remove the #ifdef CONFIG_OMAP_32K_TIMER around the
>>>__omap2_sync32k_clocksource_init() function.
>>>
>>> Signed-off-by: Igor Grinberg 
>>> Cc: Jon Hunter 
>>> Cc: Santosh Shilimkar 
>>> Cc: Vaibhav Hiremath 
>>
>> [snip]
>>
>>> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
>>> index 684d2fc..a4ad7a0 100644
>>> --- a/arch/arm/mach-omap2/timer.c
>>> +++ b/arch/arm/mach-omap2/timer.c
>>> @@ -63,20 +63,8 @@
>>>  #define OMAP2_32K_SOURCE   "func_32k_ck"
>>>  #define OMAP3_32K_SOURCE   "omap_32k_fck"
>>>  #define OMAP4_32K_SOURCE   "sys_32k_ck"
>>> -
>>> -#ifdef CONFIG_OMAP_32K_TIMER
>>> -#define OMAP2_CLKEV_SOURCE OMAP2_32K_SOURCE
>>> -#define OMAP3_CLKEV_SOURCE OMAP3_32K_SOURCE
>>> -#define OMAP4_CLKEV_SOURCE OMAP4_32K_SOURCE
>>> -#define OMAP3_SECURE_TIMER 12
>>>  #define TIMER_PROP_SECURE  "ti,timer-secure"
>>> -#else
>>> -#define OMAP2_CLKEV_SOURCE OMAP2_MPU_SOURCE
>>> -#define OMAP3_CLKEV_SOURCE OMAP3_MPU_SOURCE
>>> -#define OMAP4_CLKEV_SOURCE OMAP4_MPU_SOURCE
>>> -#define OMAP3_SECURE_TIMER 1
>>> -#define TIMER_PROP_SECURE  "ti,timer-alwon"
>>> -#endif
>>> +#define TIMER_PROP_ALWON   "ti,timer-alwon"
>>
>> Nit-pick, can we drop the TIMER_PROP_SECURE/ALWON and use the
>> "ti,timer-secure" and "ti,timer-alwon" directly?
>>
>> Initially, I also defined TIMER_PROP_ALWON and Rob Herring's feedback
>> was to drop this and use the property string directly to remove any
>> abstraction.
> 
> Well, I don't understand what do you mean by "any abstraction".
> The purpose of defining those two was to eliminate multiple occurrences
> of the string in the code, so for example if someone decides to change the
> property string for some currently unknown reason - it will be easy and small.
> Defines are a common practice for that, no?
> If you still think it should be inlined, I will do.

I understand your point, but right now I don't anticipate that we will
have many options here and so I think that we should drop these.

>>>  #define REALTIME_COUNTER_BASE  0x48243200
>>>  #define INCREMENTER_NUMERATOR_OFFSET   0x10
>>> @@ -216,7 +204,7 @@ void __init omap_dmtimer_init(void)
>>>  
>>> /* If we are a secure device, remove any secure timer nodes */
>>> if ((omap_type() != OMAP2_DEVICE_TYPE_GP)) {
>>> -   np = omap_get_timer_dt(omap_timer_match, "ti,timer-secure");
>>> +   np = omap_get_timer_dt(omap_timer_match, TIMER_PROP_SECURE);
>>> if (np)
>>> of_node_put(np);
>>> }
>>> @@ -378,9 +366,8 @@ static u32 notrace dmtimer_read_sched_clock(void)
>>> return 0;
>>>  }
>>>  
>>> -#ifdef CONFIG_OMAP_32K_TIMER
>>>  /* Setup free-running counter for clocksource */
>>> -static int __init omap2_sync32k_clocksource_init(void)
>>> +static int __init __omap2_sync32k_clocksource_init(void)
>>
>> Not sure I follow why you renamed this function here ...
> 
> I didn't want to add unused arguments to this function, so I've made a
> wrapper below to have both the sync32k and the gp functions have the same
> signature (argument list) and be called from a single macro.
> Anyway, see below.

Ok.

>>
>>>  {
>>> int ret;
>>> struct device_node *np = NULL;
>>> @@ -439,15 +426,9 @@ static int __init omap2_sync32k_clocksource_init(void)
>>>  
>>> return ret;
>>>  }
>>> -#else
>>> -static inline int omap2_sync32k_clocksource_init(void)
>>> -{
>>> -   return -ENODEV;
>>> -}
>>> -#endif
>>>  
>>> -static void __init omap2_gptimer_clocksource_init(int gptimer_id,
>>> -   const char *fck_source)
>>> +static void __init omap2_gp_clocksource_init(int gptimer_id,
>>> +const char *fck_source)
>>
>> Nit, I personally prefer keeping gptimer, because gp just means
>> "general-purpose" and does not 

Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-08 Thread Igor Grinberg
On 11/07/12 23:36, Jon Hunter wrote:
> Hi Igor,
> 
> On 11/07/2012 08:42 AM, Igor Grinberg wrote:
>> CONFIG_OMAP_32K_TIMER is kind of standing on the single zImage way.
>> Make OMAP2+ timer code independant from the CONFIG_OMAP_32K_TIMER
>> setting.
>> To remove the dependancy, several conversions/additions had to be done:
>> 1) Timer structures and initialization functions are named by the platform
>>name and the clock source in use. The decision which timer is
>>used is done statically from the machine_desc structure. In the
>>future it should come from DT.
>> 2) Settings under the CONFIG_OMAP_32K_TIMER option are expanded into
>>separate timer structures along with the timer init functions.
>>This removes the CONFIG_OMAP_32K_TIMER on OMAP2+ timer code.
>> 3) Since we have all the timers defined inside machine_desc structure
>>and we no longer need the fallback to gp_timer clock source in case
>>32k_timer clock source is unavailable (namely on AM33xx), we no
>>longer need the #ifdef around __omap2_sync32k_clocksource_init()
>>function. Remove the #ifdef CONFIG_OMAP_32K_TIMER around the
>>__omap2_sync32k_clocksource_init() function.
>>
>> Signed-off-by: Igor Grinberg 
>> Cc: Jon Hunter 
>> Cc: Santosh Shilimkar 
>> Cc: Vaibhav Hiremath 
> 
> [snip]
> 
>> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
>> index 684d2fc..a4ad7a0 100644
>> --- a/arch/arm/mach-omap2/timer.c
>> +++ b/arch/arm/mach-omap2/timer.c
>> @@ -63,20 +63,8 @@
>>  #define OMAP2_32K_SOURCE"func_32k_ck"
>>  #define OMAP3_32K_SOURCE"omap_32k_fck"
>>  #define OMAP4_32K_SOURCE"sys_32k_ck"
>> -
>> -#ifdef CONFIG_OMAP_32K_TIMER
>> -#define OMAP2_CLKEV_SOURCE  OMAP2_32K_SOURCE
>> -#define OMAP3_CLKEV_SOURCE  OMAP3_32K_SOURCE
>> -#define OMAP4_CLKEV_SOURCE  OMAP4_32K_SOURCE
>> -#define OMAP3_SECURE_TIMER  12
>>  #define TIMER_PROP_SECURE   "ti,timer-secure"
>> -#else
>> -#define OMAP2_CLKEV_SOURCE  OMAP2_MPU_SOURCE
>> -#define OMAP3_CLKEV_SOURCE  OMAP3_MPU_SOURCE
>> -#define OMAP4_CLKEV_SOURCE  OMAP4_MPU_SOURCE
>> -#define OMAP3_SECURE_TIMER  1
>> -#define TIMER_PROP_SECURE   "ti,timer-alwon"
>> -#endif
>> +#define TIMER_PROP_ALWON"ti,timer-alwon"
> 
> Nit-pick, can we drop the TIMER_PROP_SECURE/ALWON and use the
> "ti,timer-secure" and "ti,timer-alwon" directly?
> 
> Initially, I also defined TIMER_PROP_ALWON and Rob Herring's feedback
> was to drop this and use the property string directly to remove any
> abstraction.

Well, I don't understand what do you mean by "any abstraction".
The purpose of defining those two was to eliminate multiple occurrences
of the string in the code, so for example if someone decides to change the
property string for some currently unknown reason - it will be easy and small.
Defines are a common practice for that, no?
If you still think it should be inlined, I will do.

> 
>>  #define REALTIME_COUNTER_BASE   0x48243200
>>  #define INCREMENTER_NUMERATOR_OFFSET0x10
>> @@ -216,7 +204,7 @@ void __init omap_dmtimer_init(void)
>>  
>>  /* If we are a secure device, remove any secure timer nodes */
>>  if ((omap_type() != OMAP2_DEVICE_TYPE_GP)) {
>> -np = omap_get_timer_dt(omap_timer_match, "ti,timer-secure");
>> +np = omap_get_timer_dt(omap_timer_match, TIMER_PROP_SECURE);
>>  if (np)
>>  of_node_put(np);
>>  }
>> @@ -378,9 +366,8 @@ static u32 notrace dmtimer_read_sched_clock(void)
>>  return 0;
>>  }
>>  
>> -#ifdef CONFIG_OMAP_32K_TIMER
>>  /* Setup free-running counter for clocksource */
>> -static int __init omap2_sync32k_clocksource_init(void)
>> +static int __init __omap2_sync32k_clocksource_init(void)
> 
> Not sure I follow why you renamed this function here ...

I didn't want to add unused arguments to this function, so I've made a
wrapper below to have both the sync32k and the gp functions have the same
signature (argument list) and be called from a single macro.
Anyway, see below.

> 
>>  {
>>  int ret;
>>  struct device_node *np = NULL;
>> @@ -439,15 +426,9 @@ static int __init omap2_sync32k_clocksource_init(void)
>>  
>>  return ret;
>>  }
>> -#else
>> -static inline int omap2_sync32k_clocksource_init(void)
>> -{
>> -return -ENODEV;
>> -}
>> -#endif
>>  
>> -static void __init omap2_gptimer_clocksource_init(int gptimer_id,
>> -const char *fck_source)
>> +static void __init omap2_gp_clocksource_init(int gptimer_id,
>> + const char *fck_source)
> 
> Nit, I personally prefer keeping gptimer, because gp just means
> "general-purpose" and does not imply a timer per-se.

I've made this change, so we will not get something like:
omapx_gptimer_timer_init(), but this really does not meter to me,
so no problem will do for v2.

> 
>>  {
>>  int res;
>>  
>> @@ -466,23 +447,10 @@ static void __init omap2_gptimer_clockso

Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-07 Thread Igor Grinberg
On 11/07/12 19:33, Tony Lindgren wrote:
> * Igor Grinberg  [121107 06:44]:
>> CONFIG_OMAP_32K_TIMER is kind of standing on the single zImage way.
>> Make OMAP2+ timer code independant from the CONFIG_OMAP_32K_TIMER
>> setting.
>> To remove the dependancy, several conversions/additions had to be done:
>> 1) Timer structures and initialization functions are named by the platform
>>name and the clock source in use. The decision which timer is
>>used is done statically from the machine_desc structure. In the
>>future it should come from DT.
>> 2) Settings under the CONFIG_OMAP_32K_TIMER option are expanded into
>>separate timer structures along with the timer init functions.
>>This removes the CONFIG_OMAP_32K_TIMER on OMAP2+ timer code.
> 
> I think this should be the default for the timers as that counter
> does not stop during deeper idle states.

Well, it is the default as you can see from the patch.
The problem is that for boards that for some reason do not have
the 32k wired and rely on MPU/GP timer source, the default will not work
and currently there is no way for board to specify which timer source
it can use.
We have discussed this in San Diego (remember?) and you actually proposed
this way as a solution. Well, may be I took it a bit further than you
thought, but this is because the board code cannot know which timer source
should be used at runtime and the fall back described below, does not work.

> 
>> 3) Since we have all the timers defined inside machine_desc structure
>>and we no longer need the fallback to gp_timer clock source in case
>>32k_timer clock source is unavailable (namely on AM33xx), we no
>>longer need the #ifdef around __omap2_sync32k_clocksource_init()
>>function. Remove the #ifdef CONFIG_OMAP_32K_TIMER around the
>>__omap2_sync32k_clocksource_init() function.
>>
>> Signed-off-by: Igor Grinberg 
>> Cc: Jon Hunter 
>> Cc: Santosh Shilimkar 
>> Cc: Vaibhav Hiremath 
>> ---
>> Finally I'm sending this out...
>> I've lost following Tony's branches and deciding which one to base on,
>> so I used linux-omap/master as a base for the patch.
>> Tony, tell me if you want it based on some other branch.
>> This has been compile tested on omap1|2plus_defconfig only.
> 
> Yes sorry it's been a bit crazy with branches to get this
> header clean up done.. If it applies to master it should
> be easy to apply on others.

No problem ;-)

> 
>> --- a/arch/arm/mach-omap2/board-2430sdp.c
>> +++ b/arch/arm/mach-omap2/board-2430sdp.c
>> @@ -284,6 +284,6 @@ MACHINE_START(OMAP_2430SDP, "OMAP2430 sdp2430 board")
>>  .handle_irq = omap2_intc_handle_irq,
>>  .init_machine   = omap_2430sdp_init,
>>  .init_late  = omap2430_init_late,
>> -.timer  = &omap2_timer,
>> +.timer  = &omap2_sync32k_timer,
>>  .restart= omap_prcm_restart,
>>  MACHINE_END
>> --- a/arch/arm/mach-omap2/board-3430sdp.c
>> +++ b/arch/arm/mach-omap2/board-3430sdp.c
>> @@ -596,6 +596,6 @@ MACHINE_START(OMAP_3430SDP, "OMAP3430 3430SDP board")
>>  .handle_irq = omap3_intc_handle_irq,
>>  .init_machine   = omap_3430sdp_init,
>>  .init_late  = omap3430_init_late,
>> -.timer  = &omap3_timer,
>> +.timer  = &omap3_sync32k_timer,
>>  .restart= omap_prcm_restart,
>>  MACHINE_END
> ...
> 
> Can't we assume that the default timer is omap[234]_sync32k_timer to
> avoid renaming the timer entries in all the board files?

Hmmm...
How will this work with the macros defining the sys_timer structure?
I would also not want to hide the exact timer used under the default name.

> 
> Then we just need a new timer entries for the hardware that does
> not have the sycn32k_timer available?

Well, I tried to make it small patch just for the hardware that needs it,
but I always found some corner case where, IMHO, this does not work/look good.


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


Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-07 Thread Jon Hunter
Hi Igor,

On 11/07/2012 08:42 AM, Igor Grinberg wrote:
> CONFIG_OMAP_32K_TIMER is kind of standing on the single zImage way.
> Make OMAP2+ timer code independant from the CONFIG_OMAP_32K_TIMER
> setting.
> To remove the dependancy, several conversions/additions had to be done:
> 1) Timer structures and initialization functions are named by the platform
>name and the clock source in use. The decision which timer is
>used is done statically from the machine_desc structure. In the
>future it should come from DT.
> 2) Settings under the CONFIG_OMAP_32K_TIMER option are expanded into
>separate timer structures along with the timer init functions.
>This removes the CONFIG_OMAP_32K_TIMER on OMAP2+ timer code.
> 3) Since we have all the timers defined inside machine_desc structure
>and we no longer need the fallback to gp_timer clock source in case
>32k_timer clock source is unavailable (namely on AM33xx), we no
>longer need the #ifdef around __omap2_sync32k_clocksource_init()
>function. Remove the #ifdef CONFIG_OMAP_32K_TIMER around the
>__omap2_sync32k_clocksource_init() function.
> 
> Signed-off-by: Igor Grinberg 
> Cc: Jon Hunter 
> Cc: Santosh Shilimkar 
> Cc: Vaibhav Hiremath 

[snip]

> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
> index 684d2fc..a4ad7a0 100644
> --- a/arch/arm/mach-omap2/timer.c
> +++ b/arch/arm/mach-omap2/timer.c
> @@ -63,20 +63,8 @@
>  #define OMAP2_32K_SOURCE "func_32k_ck"
>  #define OMAP3_32K_SOURCE "omap_32k_fck"
>  #define OMAP4_32K_SOURCE "sys_32k_ck"
> -
> -#ifdef CONFIG_OMAP_32K_TIMER
> -#define OMAP2_CLKEV_SOURCE   OMAP2_32K_SOURCE
> -#define OMAP3_CLKEV_SOURCE   OMAP3_32K_SOURCE
> -#define OMAP4_CLKEV_SOURCE   OMAP4_32K_SOURCE
> -#define OMAP3_SECURE_TIMER   12
>  #define TIMER_PROP_SECURE"ti,timer-secure"
> -#else
> -#define OMAP2_CLKEV_SOURCE   OMAP2_MPU_SOURCE
> -#define OMAP3_CLKEV_SOURCE   OMAP3_MPU_SOURCE
> -#define OMAP4_CLKEV_SOURCE   OMAP4_MPU_SOURCE
> -#define OMAP3_SECURE_TIMER   1
> -#define TIMER_PROP_SECURE"ti,timer-alwon"
> -#endif
> +#define TIMER_PROP_ALWON "ti,timer-alwon"

Nit-pick, can we drop the TIMER_PROP_SECURE/ALWON and use the
"ti,timer-secure" and "ti,timer-alwon" directly?

Initially, I also defined TIMER_PROP_ALWON and Rob Herring's feedback
was to drop this and use the property string directly to remove any
abstraction.

>  #define REALTIME_COUNTER_BASE0x48243200
>  #define INCREMENTER_NUMERATOR_OFFSET 0x10
> @@ -216,7 +204,7 @@ void __init omap_dmtimer_init(void)
>  
>   /* If we are a secure device, remove any secure timer nodes */
>   if ((omap_type() != OMAP2_DEVICE_TYPE_GP)) {
> - np = omap_get_timer_dt(omap_timer_match, "ti,timer-secure");
> + np = omap_get_timer_dt(omap_timer_match, TIMER_PROP_SECURE);
>   if (np)
>   of_node_put(np);
>   }
> @@ -378,9 +366,8 @@ static u32 notrace dmtimer_read_sched_clock(void)
>   return 0;
>  }
>  
> -#ifdef CONFIG_OMAP_32K_TIMER
>  /* Setup free-running counter for clocksource */
> -static int __init omap2_sync32k_clocksource_init(void)
> +static int __init __omap2_sync32k_clocksource_init(void)

Not sure I follow why you renamed this function here ...

>  {
>   int ret;
>   struct device_node *np = NULL;
> @@ -439,15 +426,9 @@ static int __init omap2_sync32k_clocksource_init(void)
>  
>   return ret;
>  }
> -#else
> -static inline int omap2_sync32k_clocksource_init(void)
> -{
> - return -ENODEV;
> -}
> -#endif
>  
> -static void __init omap2_gptimer_clocksource_init(int gptimer_id,
> - const char *fck_source)
> +static void __init omap2_gp_clocksource_init(int gptimer_id,
> +  const char *fck_source)

Nit, I personally prefer keeping gptimer, because gp just means
"general-purpose" and does not imply a timer per-se.

>  {
>   int res;
>  
> @@ -466,23 +447,10 @@ static void __init omap2_gptimer_clocksource_init(int 
> gptimer_id,
>   gptimer_id, clksrc.rate);
>  }
>  
> -static void __init omap2_clocksource_init(int gptimer_id,
> - const char *fck_source)
> +static void __init omap2_sync32k_clocksource_init(int gptimer_id,
> +   const char *fck_source)
>  {
> - /*
> -  * First give preference to kernel parameter configuration
> -  * by user (clocksource="gp_timer").
> -  *
> -  * In case of missing kernel parameter for clocksource,
> -  * first check for availability for 32k-sync timer, in case
> -  * of failure in finding 32k_counter module or registering
> -  * it as clocksource, execution will fallback to gp-timer.
> -  */
> - if (use_gptimer_clksrc == true)
> - omap2_gptimer_clocksource_init(gptimer_id, fck_source);
> - else if (om

Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER

2012-11-07 Thread Tony Lindgren
* Igor Grinberg  [121107 06:44]:
> CONFIG_OMAP_32K_TIMER is kind of standing on the single zImage way.
> Make OMAP2+ timer code independant from the CONFIG_OMAP_32K_TIMER
> setting.
> To remove the dependancy, several conversions/additions had to be done:
> 1) Timer structures and initialization functions are named by the platform
>name and the clock source in use. The decision which timer is
>used is done statically from the machine_desc structure. In the
>future it should come from DT.
> 2) Settings under the CONFIG_OMAP_32K_TIMER option are expanded into
>separate timer structures along with the timer init functions.
>This removes the CONFIG_OMAP_32K_TIMER on OMAP2+ timer code.

I think this should be the default for the timers as that counter
does not stop during deeper idle states.

> 3) Since we have all the timers defined inside machine_desc structure
>and we no longer need the fallback to gp_timer clock source in case
>32k_timer clock source is unavailable (namely on AM33xx), we no
>longer need the #ifdef around __omap2_sync32k_clocksource_init()
>function. Remove the #ifdef CONFIG_OMAP_32K_TIMER around the
>__omap2_sync32k_clocksource_init() function.
> 
> Signed-off-by: Igor Grinberg 
> Cc: Jon Hunter 
> Cc: Santosh Shilimkar 
> Cc: Vaibhav Hiremath 
> ---
> Finally I'm sending this out...
> I've lost following Tony's branches and deciding which one to base on,
> so I used linux-omap/master as a base for the patch.
> Tony, tell me if you want it based on some other branch.
> This has been compile tested on omap1|2plus_defconfig only.

Yes sorry it's been a bit crazy with branches to get this
header clean up done.. If it applies to master it should
be easy to apply on others.

> --- a/arch/arm/mach-omap2/board-2430sdp.c
> +++ b/arch/arm/mach-omap2/board-2430sdp.c
> @@ -284,6 +284,6 @@ MACHINE_START(OMAP_2430SDP, "OMAP2430 sdp2430 board")
>   .handle_irq = omap2_intc_handle_irq,
>   .init_machine   = omap_2430sdp_init,
>   .init_late  = omap2430_init_late,
> - .timer  = &omap2_timer,
> + .timer  = &omap2_sync32k_timer,
>   .restart= omap_prcm_restart,
>  MACHINE_END
> --- a/arch/arm/mach-omap2/board-3430sdp.c
> +++ b/arch/arm/mach-omap2/board-3430sdp.c
> @@ -596,6 +596,6 @@ MACHINE_START(OMAP_3430SDP, "OMAP3430 3430SDP board")
>   .handle_irq = omap3_intc_handle_irq,
>   .init_machine   = omap_3430sdp_init,
>   .init_late  = omap3430_init_late,
> - .timer  = &omap3_timer,
> + .timer  = &omap3_sync32k_timer,
>   .restart= omap_prcm_restart,
>  MACHINE_END
...

Can't we assume that the default timer is omap[234]_sync32k_timer to
avoid renaming the timer entries in all the board files?

Then we just need a new timer entries for the hardware that does
not have the sycn32k_timer available?

Regards,

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