Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER
On Thu, Nov 8, 2012 at 9:08 AM, Hiremath, Vaibhav hvaib...@ti.com 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
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
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 grinb...@compulab.co.il [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
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
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
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
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
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
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
On 11/11/2012 03:16 AM, Igor Grinberg wrote: On 11/08/12 18:20, Tony Lindgren wrote: * Igor Grinberg grinb...@compulab.co.il [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
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
* Igor Grinberg grinb...@compulab.co.il [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
On 11/08/12 18:20, Tony Lindgren wrote: * Igor Grinberg grinb...@compulab.co.il [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
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 grinb...@compulab.co.il
Cc: Jon Hunter jon-hun...@ti.com
Cc: Santosh Shilimkar santosh.shilim...@ti.com
Cc: Vaibhav Hiremath hvaib...@ti.com
[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_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
Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER
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
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
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
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
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
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 grinb...@compulab.co.il
Cc: Jon Hunter jon-hun...@ti.com
Cc: Santosh Shilimkar santosh.shilim...@ti.com
Cc: Vaibhav Hiremath hvaib...@ti.com
[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_SOURCEfunc_32k_ck
#define OMAP3_32K_SOURCEomap_32k_fck
#define OMAP4_32K_SOURCEsys_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_ALWONti,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_clocksource_init(int
gptimer_id,
gptimer_id, clksrc.rate);
}
-static void __init omap2_clocksource_init(int
Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER
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 grinb...@compulab.co.il
Cc: Jon Hunter jon-hun...@ti.com
Cc: Santosh Shilimkar santosh.shilim...@ti.com
Cc: Vaibhav Hiremath hvaib...@ti.com
[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 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.
Thanks.
{
int res;
@@ -466,23 +447,10 @@
Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER
* Igor Grinberg grinb...@compulab.co.il [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
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 grinb...@compulab.co.il
Cc: Jon Hunter jon-hun...@ti.com
Cc: Santosh Shilimkar santosh.shilim...@ti.com
Cc: Vaibhav Hiremath hvaib...@ti.com
[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_SECUREti,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_SECUREti,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 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
RE: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER
On Thu, Nov 08, 2012 at 21:50:05, Tony Lindgren wrote: * Igor Grinberg grinb...@compulab.co.il [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
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 grinb...@compulab.co.il
Cc: Jon Hunter jon-hun...@ti.com
Cc: Santosh Shilimkar santosh.shilim...@ti.com
Cc: Vaibhav Hiremath hvaib...@ti.com
[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_SECUREti,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_SECUREti,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 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:
RE: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER
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 grinb...@compulab.co.il
Cc: Jon Hunter jon-hun...@ti.com
Cc: Santosh Shilimkar santosh.shilim...@ti.com
Cc: Vaibhav Hiremath hvaib...@ti.com
[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,
+
Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER
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
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
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
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
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
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
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
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
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
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
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
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
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
[PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER
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 grinb...@compulab.co.il Cc: Jon Hunter jon-hun...@ti.com Cc: Santosh Shilimkar santosh.shilim...@ti.com Cc: Vaibhav Hiremath hvaib...@ti.com --- 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. arch/arm/mach-omap2/board-2430sdp.c|2 +- arch/arm/mach-omap2/board-3430sdp.c|2 +- arch/arm/mach-omap2/board-3630sdp.c|2 +- arch/arm/mach-omap2/board-4430sdp.c|2 +- arch/arm/mach-omap2/board-am3517crane.c|2 +- arch/arm/mach-omap2/board-am3517evm.c |2 +- arch/arm/mach-omap2/board-apollon.c|2 +- arch/arm/mach-omap2/board-cm-t35.c | 18 ++-- arch/arm/mach-omap2/board-cm-t3517.c |2 +- arch/arm/mach-omap2/board-devkit8000.c |2 +- arch/arm/mach-omap2/board-generic.c| 14 ++-- arch/arm/mach-omap2/board-h4.c |2 +- arch/arm/mach-omap2/board-igep0020.c |4 +- arch/arm/mach-omap2/board-ldp.c|2 +- arch/arm/mach-omap2/board-n8x0.c |6 +- arch/arm/mach-omap2/board-omap3beagle.c|2 +- arch/arm/mach-omap2/board-omap3encore.c|2 +- arch/arm/mach-omap2/board-omap3evm.c |2 +- arch/arm/mach-omap2/board-omap3logic.c |4 +- arch/arm/mach-omap2/board-omap3pandora.c |2 +- arch/arm/mach-omap2/board-omap3stalker.c |2 +- arch/arm/mach-omap2/board-omap3touchbook.c |2 +- arch/arm/mach-omap2/board-omap4panda.c |2 +- arch/arm/mach-omap2/board-omap4pcm049.c|2 +- arch/arm/mach-omap2/board-overo.c |2 +- arch/arm/mach-omap2/board-rm680.c |4 +- arch/arm/mach-omap2/board-rx51.c |2 +- arch/arm/mach-omap2/board-ti8168evm.c |4 +- arch/arm/mach-omap2/board-zoom.c |4 +- arch/arm/mach-omap2/common.h | 17 ++- arch/arm/mach-omap2/timer.c| 162 ++-- arch/arm/plat-omap/Kconfig |5 + 32 files changed, 147 insertions(+), 137 deletions(-) diff --git a/arch/arm/mach-omap2/board-2430sdp.c b/arch/arm/mach-omap2/board-2430sdp.c index d1c0162..90c1584 100644 --- 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 diff --git a/arch/arm/mach-omap2/board-3430sdp.c b/arch/arm/mach-omap2/board-3430sdp.c index 79fd904..e14b355 100644 --- 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 diff --git a/arch/arm/mach-omap2/board-3630sdp.c b/arch/arm/mach-omap2/board-3630sdp.c index 81871b1..030d292 100644 --- a/arch/arm/mach-omap2/board-3630sdp.c +++ b/arch/arm/mach-omap2/board-3630sdp.c @@ -211,6 +211,6 @@ MACHINE_START(OMAP_3630SDP, OMAP 3630SDP board) .handle_irq = omap3_intc_handle_irq, .init_machine = omap_sdp_init, .init_late = omap3630_init_late, - .timer = omap3_timer, + .timer = omap3_sync32k_timer, .restart= omap_prcm_restart, MACHINE_END diff --git
Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER
* Igor Grinberg grinb...@compulab.co.il [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 grinb...@compulab.co.il Cc: Jon Hunter jon-hun...@ti.com Cc: Santosh Shilimkar santosh.shilim...@ti.com Cc: Vaibhav Hiremath hvaib...@ti.com --- 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
Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER
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 grinb...@compulab.co.il
Cc: Jon Hunter jon-hun...@ti.com
Cc: Santosh Shilimkar santosh.shilim...@ti.com
Cc: Vaibhav Hiremath hvaib...@ti.com
[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_SECUREti,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_SECUREti,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 (omap2_sync32k_clocksource_init())
- /*
Re: [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER
On 11/07/12 19:33, Tony Lindgren wrote: * Igor Grinberg grinb...@compulab.co.il [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 grinb...@compulab.co.il Cc: Jon Hunter jon-hun...@ti.com Cc: Santosh Shilimkar santosh.shilim...@ti.com Cc: Vaibhav Hiremath hvaib...@ti.com --- 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
