Re: [PATCH 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Wed, Apr 11, 2012 at 6:30 AM, Ming Lei tom.leim...@gmail.com wrote: On Tue, Apr 10, 2012 at 5:51 PM, Shilimkar, Santosh santosh.shilim...@ti.com wrote: On Tue, Apr 10, 2012 at 2:59 PM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Tue, Apr 10, 2012 at 02:27:36PM +0530, Santosh Shilimkar wrote: On Tuesday 10 April 2012 02:14 PM, Russell King - ARM Linux wrote: On Mon, Apr 09, 2012 at 03:18:22PM -0500, Jon Hunter wrote: True, but we would always want to use the 32k timer if CONFIG_PM is specified. So what I am saying is that if a device has a 32ksync timer and CONFIG_PM is defined, we always want to use the 32ksync timer and a gptimer should never be used. Why? What if you want to have PM enabled, and you also want to use the kernels high resolution timers, or you want more accurate timing than the 30.5us tick interval of the 32k timer? You might have missed the earlier comments on the thread. High resolution GP timer(sysclk) will stop in deeper power states and hence it can't be used with PM enabled usecases. Which means folk should be given the choice at boot time between running with the deeper power states and having a higher resolution timing source, rather than denying them the higher resolution timing source when PM is enabled. Good point. My point is such facilities is already part of the kernel and there is no need to add a new one. The last proposal was allowing user to choose gptimer as a clocksource and then you already have facility to disable C-state now so, all should work in general 'nohlt' in boot cmd should work to prevent CPU from entering deep C-state, which looks enough to make gptimer working well if system suspend isn't considered. That's another good option Looks like we get all the needed flexibility with the reworked patch from Vaibhav to choose the command like option for high res. timer source. Regards Santosh -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Mon, Apr 09, 2012 at 03:18:22PM -0500, Jon Hunter wrote: True, but we would always want to use the 32k timer if CONFIG_PM is specified. So what I am saying is that if a device has a 32ksync timer and CONFIG_PM is defined, we always want to use the 32ksync timer and a gptimer should never be used. Why? What if you want to have PM enabled, and you also want to use the kernels high resolution timers, or you want more accurate timing than the 30.5us tick interval of the 32k timer? -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Tuesday 10 April 2012 02:14 PM, Russell King - ARM Linux wrote: On Mon, Apr 09, 2012 at 03:18:22PM -0500, Jon Hunter wrote: True, but we would always want to use the 32k timer if CONFIG_PM is specified. So what I am saying is that if a device has a 32ksync timer and CONFIG_PM is defined, we always want to use the 32ksync timer and a gptimer should never be used. Why? What if you want to have PM enabled, and you also want to use the kernels high resolution timers, or you want more accurate timing than the 30.5us tick interval of the 32k timer? You might have missed the earlier comments on the thread. High resolution GP timer(sysclk) will stop in deeper power states and hence it can't be used with PM enabled usecases. Regards Santosh -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Tue, Apr 10, 2012 at 02:27:36PM +0530, Santosh Shilimkar wrote: On Tuesday 10 April 2012 02:14 PM, Russell King - ARM Linux wrote: On Mon, Apr 09, 2012 at 03:18:22PM -0500, Jon Hunter wrote: True, but we would always want to use the 32k timer if CONFIG_PM is specified. So what I am saying is that if a device has a 32ksync timer and CONFIG_PM is defined, we always want to use the 32ksync timer and a gptimer should never be used. Why? What if you want to have PM enabled, and you also want to use the kernels high resolution timers, or you want more accurate timing than the 30.5us tick interval of the 32k timer? You might have missed the earlier comments on the thread. High resolution GP timer(sysclk) will stop in deeper power states and hence it can't be used with PM enabled usecases. Which means folk should be given the choice at boot time between running with the deeper power states and having a higher resolution timing source, rather than denying them the higher resolution timing source when PM is enabled. -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Tue, Apr 10, 2012 at 2:59 PM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Tue, Apr 10, 2012 at 02:27:36PM +0530, Santosh Shilimkar wrote: On Tuesday 10 April 2012 02:14 PM, Russell King - ARM Linux wrote: On Mon, Apr 09, 2012 at 03:18:22PM -0500, Jon Hunter wrote: True, but we would always want to use the 32k timer if CONFIG_PM is specified. So what I am saying is that if a device has a 32ksync timer and CONFIG_PM is defined, we always want to use the 32ksync timer and a gptimer should never be used. Why? What if you want to have PM enabled, and you also want to use the kernels high resolution timers, or you want more accurate timing than the 30.5us tick interval of the 32k timer? You might have missed the earlier comments on the thread. High resolution GP timer(sysclk) will stop in deeper power states and hence it can't be used with PM enabled usecases. Which means folk should be given the choice at boot time between running with the deeper power states and having a higher resolution timing source, rather than denying them the higher resolution timing source when PM is enabled. Good point. My point is such facilities is already part of the kernel and there is no need to add a new one. The last proposal was allowing user to choose gptimer as a clocksource and then you already have facility to disable C-state now so, all should work in general Regards Santosh -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On 04/10/2012 04:51 AM, Shilimkar, Santosh wrote: On Tue, Apr 10, 2012 at 2:59 PM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Tue, Apr 10, 2012 at 02:27:36PM +0530, Santosh Shilimkar wrote: On Tuesday 10 April 2012 02:14 PM, Russell King - ARM Linux wrote: On Mon, Apr 09, 2012 at 03:18:22PM -0500, Jon Hunter wrote: True, but we would always want to use the 32k timer if CONFIG_PM is specified. So what I am saying is that if a device has a 32ksync timer and CONFIG_PM is defined, we always want to use the 32ksync timer and a gptimer should never be used. Why? What if you want to have PM enabled, and you also want to use the kernels high resolution timers, or you want more accurate timing than the 30.5us tick interval of the 32k timer? You might have missed the earlier comments on the thread. High resolution GP timer(sysclk) will stop in deeper power states and hence it can't be used with PM enabled usecases. Which means folk should be given the choice at boot time between running with the deeper power states and having a higher resolution timing source, rather than denying them the higher resolution timing source when PM is enabled. Good point. My point is such facilities is already part of the kernel and there is no need to add a new one. The last proposal was allowing user to choose gptimer as a clocksource and then you already have facility to disable C-state now so, all should work in general Actually, thinking about this some more, you do not even need to disable c-states. By using a gptimer1 and configuring it to use the SYSCLK as it source, as long as the gptimer1 is not disabled, it will prevent SYSCLK from ever turning off. Probably for good measure in this case we should also disable auto clock gating of SYSCLK via the PRM_CLKSRC_CTRL (OMAP2/3) or PRM_CLKREQCTRL (OMAP4). Sounds like a good approach. 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Tue, Apr 10, 2012 at 5:51 PM, Shilimkar, Santosh santosh.shilim...@ti.com wrote: On Tue, Apr 10, 2012 at 2:59 PM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Tue, Apr 10, 2012 at 02:27:36PM +0530, Santosh Shilimkar wrote: On Tuesday 10 April 2012 02:14 PM, Russell King - ARM Linux wrote: On Mon, Apr 09, 2012 at 03:18:22PM -0500, Jon Hunter wrote: True, but we would always want to use the 32k timer if CONFIG_PM is specified. So what I am saying is that if a device has a 32ksync timer and CONFIG_PM is defined, we always want to use the 32ksync timer and a gptimer should never be used. Why? What if you want to have PM enabled, and you also want to use the kernels high resolution timers, or you want more accurate timing than the 30.5us tick interval of the 32k timer? You might have missed the earlier comments on the thread. High resolution GP timer(sysclk) will stop in deeper power states and hence it can't be used with PM enabled usecases. Which means folk should be given the choice at boot time between running with the deeper power states and having a higher resolution timing source, rather than denying them the higher resolution timing source when PM is enabled. Good point. My point is such facilities is already part of the kernel and there is no need to add a new one. The last proposal was allowing user to choose gptimer as a clocksource and then you already have facility to disable C-state now so, all should work in general 'nohlt' in boot cmd should work to prevent CPU from entering deep C-state, which looks enough to make gptimer working well if system suspend isn't considered. Thanks, -- Ming Lei -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Fri, Apr 06, 2012 at 23:34:52, Tony Lindgren wrote:
* Hiremath, Vaibhav hvaib...@ti.com [120405 22:25]:
On Fri, Apr 06, 2012 at 03:03:01, Hilman, Kevin wrote:
What we need is only one-time selection at boot based on presence (or
not) of various timers. IOW, we still only ever need to call
setup_sched_clock() once based on which HW timers are available.
Why not just delay the setup_sched_clock() until the clocksource is
decided?
I think that's we're already doing for omap1 as 15xx does not
have the 32 KiHz timer and the CONFIG_OMAP_32K_TIMER is no longer
conflicting with the MPU timer.
No we are not delaying anything here, we still are calling
setup_sched_clock() function in timer-init() callback.
The call sequence for omap1 is,
struct sys_timer omap1_timer = {
.init = omap1_timer_init,
};
static void __init omap1_timer_init(void)
{
if (!omap_32k_timer_usable())
omap_mpu_timer_init();
}
omap_32k_timer_usable() is just return for omap730 omap15xx and fallback to
omap_mpu_timer_init(). For all other platforms, 32ksync timer init call
happens.
In both the cases, setup_sched_clock() is called.
I liked Santosh's idea in using command line argument clocksource= and
make decision based on this. I have implemented it and tried it on both
OMAP3EVM and beaglebone and it works great.
I have introduced something like this in mach-omap2/timer.c,
static int __init omap2_override_clocksource(char* str)
{
if (!str)
return 0;
/*
* For OMAP architecture, we only have two options
*- sync_32k (default)
*- gp timer
*/
if (!strcmp(str, gp timer))
use_gptimer_clksrc = true;
return 0;
}
early_param(clocksource, omap2_override_clocksource);
Sure a cmdline override is nice to have for user selection.
But we should also by default do the right thing based on what the
board wants in .timer entry.
I did not understand what exactly you are trying to point here.
I think we are doing exactly what board needs in .timer.
It solves all issues what we have been trying address.
I'm a bit confused.. Can you briefly summarize again what all
issues you're having? Just want to select a different clocksource
for beaglebone? If you don't have the 32 KiHz then that can't
be selected naturally?
Let me summarize it here again,
Currently, the timer code is using config option CONFIG_OMAP_32K_TIMER,
to choose between 32ksync counter and gptimer; it is compile time option.
If user wants to use gptimer for HR ticks, he must build the kernel without
CONFIG_OMAP_32K_TIMER option.
AM335x family of devices doesn't have 32ksync_counter available, only option
here is to use gptimer for kernel clocksource and clockevents.
So in order to support, multi-omap build including devices like AM335x, we
must get rid of this option CONFIG_OMAP_32K_TIMER, atleast from clocksource
registration perspective.
So that means, we need to have some mechanism to handle or detect available
clocksource runtime. Options would be,
- Use HWMOD to detect availability of 32ksync_counter, else fallback
to gptimer. [This was my original patch]
But this restricts the use of gptimer completely on omap architecture,
where we have 32ksync counter module.
- So the next solution is to still keep compile time option, so that user
will get to use gptimer atleast just changing the kernel config option.
But, still, this is going to be kernel rebuild.
- Next option came up was, register both the timers and override using
kernel parameter.
This will work only if, both the timers run at same rate/frequency; since
we have one more layer here setup_sched_clock(), which actually can be
called only once.
- Next option suggested by Santosh, is to use kernel parameter as in parse
it early, to make decision on if user wants to override default
clocksource (32ksync)
This is something will work for us and solves all above issues.
I have validated it on OMAP3EVM and it seems to be working for me without
any issues.
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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Sat, Apr 07, 2012 at 02:48:47, Hilman, Kevin wrote:
Hiremath, Vaibhav hvaib...@ti.com writes:
[...]
I liked Santosh's idea in using command line argument clocksource= and
make decision based on this. I have implemented it and tried it on both
OMAP3EVM and beaglebone and it works great.
I have introduced something like this in mach-omap2/timer.c,
static int __init omap2_override_clocksource(char* str)
{
if (!str)
return 0;
/*
* For OMAP architecture, we only have two options
*- sync_32k (default)
*- gp timer
*/
if (!strcmp(str, gp timer))
use_gptimer_clksrc = true;
return 0;
}
early_param(clocksource, omap2_override_clocksource);
How does this interact with the existing clocksource cmdline parameter
already in kernel/time/clocksource.c? (c.f. boot_override_clocksource())
Not really, it doesn't interact with existing clocksource parameter
Already present in kernel/time/clocksource.c.
Both works independently...
IMO, this duplicates that functionality but less elegantly.
What should happen is to let clocksource selection happen normally
(based on presence or lack of HW, or cmdline override.) Once that has
happened, you can then setup_sched_clock() with parameters from querying
the clocksource itself.
After Russell's response, it is clear that we should not change the
clocksource dynamically, its not useful. So I do not see any benefits
following that feature (as of now).
We just need to make sure that, we detect our clocksource and call
setup_sched_clock() with appropriate rating.
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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
Hi Vaibhav, On 04/09/2012 01:19 AM, Hiremath, Vaibhav wrote: [...] Let me summarize it here again, Currently, the timer code is using config option CONFIG_OMAP_32K_TIMER, to choose between 32ksync counter and gptimer; it is compile time option. If user wants to use gptimer for HR ticks, he must build the kernel without CONFIG_OMAP_32K_TIMER option. AM335x family of devices doesn't have 32ksync_counter available, only option here is to use gptimer for kernel clocksource and clockevents. So in order to support, multi-omap build including devices like AM335x, we must get rid of this option CONFIG_OMAP_32K_TIMER, atleast from clocksource registration perspective. So that means, we need to have some mechanism to handle or detect available clocksource runtime. Options would be, - Use HWMOD to detect availability of 32ksync_counter, else fallback to gptimer. [This was my original patch] But this restricts the use of gptimer completely on omap architecture, where we have 32ksync counter module. True, but we would always want to use the 32k timer if CONFIG_PM is specified. So what I am saying is that if a device has a 32ksync timer and CONFIG_PM is defined, we always want to use the 32ksync timer and a gptimer should never be used. So we should/must restrict the use of a gptimer is CONFIG_PM is enabled for devices that have the 32ksync timer. - So the next solution is to still keep compile time option, so that user will get to use gptimer atleast just changing the kernel config option. But, still, this is going to be kernel rebuild. - Next option came up was, register both the timers and override using kernel parameter. This will work only if, both the timers run at same rate/frequency; since we have one more layer here setup_sched_clock(), which actually can be called only once. - Next option suggested by Santosh, is to use kernel parameter as in parse it early, to make decision on if user wants to override default clocksource (32ksync) This is something will work for us and solves all above issues. What happens if PM is enabled? Can you still override the default? I don't think this should be allowed for devices with a 32ksync timer. 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Tue, Apr 10, 2012 at 01:48:22, Hunter, Jon wrote: Hi Vaibhav, On 04/09/2012 01:19 AM, Hiremath, Vaibhav wrote: [...] Let me summarize it here again, Currently, the timer code is using config option CONFIG_OMAP_32K_TIMER, to choose between 32ksync counter and gptimer; it is compile time option. If user wants to use gptimer for HR ticks, he must build the kernel without CONFIG_OMAP_32K_TIMER option. AM335x family of devices doesn't have 32ksync_counter available, only option here is to use gptimer for kernel clocksource and clockevents. So in order to support, multi-omap build including devices like AM335x, we must get rid of this option CONFIG_OMAP_32K_TIMER, atleast from clocksource registration perspective. So that means, we need to have some mechanism to handle or detect available clocksource runtime. Options would be, - Use HWMOD to detect availability of 32ksync_counter, else fallback to gptimer. [This was my original patch] But this restricts the use of gptimer completely on omap architecture, where we have 32ksync counter module. True, but we would always want to use the 32k timer if CONFIG_PM is specified. So what I am saying is that if a device has a 32ksync timer and CONFIG_PM is defined, we always want to use the 32ksync timer and a gptimer should never be used. So we should/must restrict the use of a gptimer is CONFIG_PM is enabled for devices that have the 32ksync timer. - So the next solution is to still keep compile time option, so that user will get to use gptimer atleast just changing the kernel config option. But, still, this is going to be kernel rebuild. - Next option came up was, register both the timers and override using kernel parameter. This will work only if, both the timers run at same rate/frequency; since we have one more layer here setup_sched_clock(), which actually can be called only once. - Next option suggested by Santosh, is to use kernel parameter as in parse it early, to make decision on if user wants to override default clocksource (32ksync) This is something will work for us and solves all above issues. What happens if PM is enabled? Can you still override the default? I don't think this should be allowed for devices with a 32ksync timer. If user is overriding it explicitly, that itself means he knows what he is doing here. I too much to ask for...and unnecessary thing or expectation from SW. 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
* Hiremath, Vaibhav hvaib...@ti.com [120405 22:25]:
On Fri, Apr 06, 2012 at 03:03:01, Hilman, Kevin wrote:
What we need is only one-time selection at boot based on presence (or
not) of various timers. IOW, we still only ever need to call
setup_sched_clock() once based on which HW timers are available.
Why not just delay the setup_sched_clock() until the clocksource is
decided?
I think that's we're already doing for omap1 as 15xx does not
have the 32 KiHz timer and the CONFIG_OMAP_32K_TIMER is no longer
conflicting with the MPU timer.
I liked Santosh's idea in using command line argument clocksource= and
make decision based on this. I have implemented it and tried it on both
OMAP3EVM and beaglebone and it works great.
I have introduced something like this in mach-omap2/timer.c,
static int __init omap2_override_clocksource(char* str)
{
if (!str)
return 0;
/*
* For OMAP architecture, we only have two options
*- sync_32k (default)
*- gp timer
*/
if (!strcmp(str, gp timer))
use_gptimer_clksrc = true;
return 0;
}
early_param(clocksource, omap2_override_clocksource);
Sure a cmdline override is nice to have for user selection.
But we should also by default do the right thing based on what the
board wants in .timer entry.
It solves all issues what we have been trying address.
I'm a bit confused.. Can you briefly summarize again what all
issues you're having? Just want to select a different clocksource
for beaglebone? If you don't have the 32 KiHz then that can't
be selected naturally?
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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
Hiremath, Vaibhav hvaib...@ti.com writes:
[...]
I liked Santosh's idea in using command line argument clocksource= and
make decision based on this. I have implemented it and tried it on both
OMAP3EVM and beaglebone and it works great.
I have introduced something like this in mach-omap2/timer.c,
static int __init omap2_override_clocksource(char* str)
{
if (!str)
return 0;
/*
* For OMAP architecture, we only have two options
*- sync_32k (default)
*- gp timer
*/
if (!strcmp(str, gp timer))
use_gptimer_clksrc = true;
return 0;
}
early_param(clocksource, omap2_override_clocksource);
How does this interact with the existing clocksource cmdline parameter
already in kernel/time/clocksource.c? (c.f. boot_override_clocksource())
IMO, this duplicates that functionality but less elegantly.
What should happen is to let clocksource selection happen normally
(based on presence or lack of HW, or cmdline override.) Once that has
happened, you can then setup_sched_clock() with parameters from querying
the clocksource itself.
Kevin
--
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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Wed, Apr 04, 2012 at 16:09:51, Hiremath, Vaibhav wrote:
On Wed, Apr 04, 2012 at 14:34:09, Shilimkar, Santosh wrote:
On Tue, Apr 3, 2012 at 9:05 PM, Hiremath, Vaibhav hvaib...@ti.com wrote:
On Tue, Apr 03, 2012 at 00:05:38, Hilman, Kevin wrote:
Shilimkar, Santosh santosh.shilim...@ti.com writes:
[...]
I don't personally like to add features which hardly anybody use and
fundamentally broken with full kernel.
Let's keep sane defaults, but not make it unreasonable to tweak eaither.
I suggest what has already been mentioned.
Register both timers, but have the sync timer have a higher rating. On
AMxxx where there is no sync timer, GPtimer will be used.
This is another good option, I can change the rating of both the timers.
With below description and given understanding/discussion/usability of
both
the timers, we can reverse the rating,
Btw, if we are going with this path, then there won't any need of
commandline
option since clock-source can be switched from user space as well.
Yes, both the options will be open now and its upto user how he want to
choose it (either bootargs or sysfs).
Santosh, Kevin and others (may be Russell can comment/conform),
Unfortunately, I am still struggling to get final patch out of this
discussion,
Although now we concluded on changing the rates for 32ksync (rating = 300)
and gptmier (rating = 250), and we thought kernel will choose better
clocksource automatically, in default case it would be 32ksync_counter, and
that is what we want; and user can use bootargs to override the clocksource
to gptimer.
But we all missed on setup_sched_clock(), which is dependent on clock
Frequency, and based on that whole sched_clock() function works.
In case of 32ksync_counter:
--
cd.mult - 40, cd.shift - 17
sched_clock: 32 bits at 32kHz, resolution 30517ns, wraps every 131071999ms
In case of gptimer:
--
cd.mult - 2581110154, cd.shift - 26
sched_clock: 32 bits at 26MHz, resolution 38ns, wraps every 165191ms
Also, in addition to this,
- We do not have any API available in sched_clock code to update the
mult/shift factors.
- You supposed to not call setup_sched_clock() multiple times, it dumps
whole stack with,
WARN_ON(read_sched_clock != jiffy_sched_clock_read);
- In case, user pass clocksource=gp timer in bootargs, still initially
Kernel is going to use 32ksync_counter as a clocksource and the late in
the boot sequence it will switch to gptimer.
[0.637512] Switching to clocksource gp timer
There seems to be limitation for ARM architecture, it is restricted by
sched_clock implementation present in arch/arm/kernel/sched_clock.c.
Natively, clocksource framework does support change in rate/frequency for
registered timer, using,
- __clocksource_updatefreq_scale()
- __clocksource_updatefreq_hz()
- __clocksource_updatefreq_khz()
May be I am missing something here, so far this is what my understanding is,
I am still digging through the code to understand how best we can handle
this. And also, I wouldn't want to again create our own sched_clock
interface.
May be Russell can help me to conform my understanding here.
Coming back to our actual problem of registering 2 clocksources, with all
above things, I am slowly moving towards keeping CONFIG_OMAP_32K_TIMER
config option and use compile time option for this (original approach) along
with hwmod detection, so that it can be reused for devices like AM33xx.
Code implementation will be,
#ifdef CONFIG_OMAP_32K_TIMER
oh = omap_hwmod_lookup(oh_name);
if (oh oh-slaves_cnt != 0) {
u32 pbase;
unsigned long size;
struct resource mem_rsrc;
res = omap_hwmod_get_resource_byname(oh, IORESOURCE_MEM,
NULL, mem_rsrc);
if (!res) {
pbase = mem_rsrc.start + 0x10;
size = mem_rsrc.end - mem_rsrc.start;
res = omap_init_clocksource_32k(pbase, size);
if (!res)
return;
}
}
#endif
/* Fall back to gp-timer code */
res = omap_dm_timer_init_one(clksrc, gptimer_id, fck_source);
BUG_ON(res);
__omap_dm_timer_load_start(clksrc,
OMAP_TIMER_CTRL_ST | OMAP_TIMER_CTRL_AR, 0, 1);
setup_sched_clock(dmtimer_read_sched_clock, 32, clksrc.rate);
if (clocksource_register_hz(clocksource_gpt, clksrc.rate))
pr_err(Could not register clocksource %s\n,
clocksource_gpt.name);
else
pr_info(OMAP clocksource: GPTIMER%d at %lu Hz\n,
gptimer_id, clksrc.rate);
Thanks,
Vaibhav
--
To unsubscribe from this list: send
Re: [PATCH 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Thu, Apr 05, 2012 at 09:36:00AM +, Hiremath, Vaibhav wrote: There seems to be limitation for ARM architecture, it is restricted by sched_clock implementation present in arch/arm/kernel/sched_clock.c. Natively, clocksource framework does support change in rate/frequency for registered timer, using, Any kind of switching of timing sources introduces loss of time issues by the mere fact that you can't instantly know the counter values of both timing sources at precisely the same instant - because CPUs can only do one thing at a time. So any kind of repeated dynamic switching _will_ result in a gradual loss of time - which will be indeterminant as it depends on the frequency of the switching and the relative delta between the two. To put it another way - it is not possible to maintain high precision and accuracy while dynamically switching your timing sources. I'm not about to lift the restriction that there's only one source for sched_clock() just for OMAP. That'd require a lot of additional code - it's not just about adjusting the multiplier and shift, you also have to correct the returned ns value as well, which means synchronizing against two counters. (And as I've pointed out above, that's impossible to do accurately.) -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Thu, Apr 05, 2012 at 15:22:21, Russell King - ARM Linux wrote: On Thu, Apr 05, 2012 at 09:36:00AM +, Hiremath, Vaibhav wrote: There seems to be limitation for ARM architecture, it is restricted by sched_clock implementation present in arch/arm/kernel/sched_clock.c. Natively, clocksource framework does support change in rate/frequency for registered timer, using, Any kind of switching of timing sources introduces loss of time issues by the mere fact that you can't instantly know the counter values of both timing sources at precisely the same instant - because CPUs can only do one thing at a time. So any kind of repeated dynamic switching _will_ result in a gradual loss of time - which will be indeterminant as it depends on the frequency of the switching and the relative delta between the two. To put it another way - it is not possible to maintain high precision and accuracy while dynamically switching your timing sources. I'm not about to lift the restriction that there's only one source for sched_clock() just for OMAP. That'd require a lot of additional code - it's not just about adjusting the multiplier and shift, you also have to correct the returned ns value as well, which means synchronizing against two counters. (And as I've pointed out above, that's impossible to do accurately.) Thanks a ton Russell for confirming on this, I understand, we also have to adjust ns value and such confirmation is what exactly I was looking for. So this means, we have to use compile time option, as existing implementation in arch/arm/mach-omap2/timer.c. Thanks again, I will repost patches shortly with the code changes (mentioned in my last email) 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Thursday 05 April 2012 04:01 PM, Hiremath, Vaibhav wrote: On Thu, Apr 05, 2012 at 15:22:21, Russell King - ARM Linux wrote: On Thu, Apr 05, 2012 at 09:36:00AM +, Hiremath, Vaibhav wrote: There seems to be limitation for ARM architecture, it is restricted by sched_clock implementation present in arch/arm/kernel/sched_clock.c. Natively, clocksource framework does support change in rate/frequency for registered timer, using, Any kind of switching of timing sources introduces loss of time issues by the mere fact that you can't instantly know the counter values of both timing sources at precisely the same instant - because CPUs can only do one thing at a time. So any kind of repeated dynamic switching _will_ result in a gradual loss of time - which will be indeterminant as it depends on the frequency of the switching and the relative delta between the two. To put it another way - it is not possible to maintain high precision and accuracy while dynamically switching your timing sources. I'm not about to lift the restriction that there's only one source for sched_clock() just for OMAP. That'd require a lot of additional code - it's not just about adjusting the multiplier and shift, you also have to correct the returned ns value as well, which means synchronizing against two counters. (And as I've pointed out above, that's impossible to do accurately.) Thanks a ton Russell for confirming on this, I understand, we also have to adjust ns value and such confirmation is what exactly I was looking for. So this means, we have to use compile time option, as existing implementation in arch/arm/mach-omap2/timer.c. Thanks again, I will repost patches shortly with the code changes (mentioned in my last email) I suggest you wait for Kevin and Tony to look at it. Am still going back to what I proposed initially. Why not the conditional way as shown in the patch [1] I proposed or your initial patch with some updates? Something like this. if(commandline.clksource == gpt) omap2_gptimer_clocksource_init(gptimer_id, fck_source); else if (omap_init_clocksource_32k()) omap2_gptimer_clocksource_init(gptimer_id, fck_source); This won't need compile time option and gives you all everybody wants. 1. Ability to use gpt as a clock-source for perf like stuff 2. Hardware like AMXX where 32K synctimer doesn't exist which means omap_init_clocksource_32k() will fail and use gptimer. 3. For OMAP, it will continue to use 32K sync timer. What am I missing Vaibhav? Regards Santosh -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
Hiremath, Vaibhav hvaib...@ti.com writes: On Thu, Apr 05, 2012 at 15:22:21, Russell King - ARM Linux wrote: On Thu, Apr 05, 2012 at 09:36:00AM +, Hiremath, Vaibhav wrote: There seems to be limitation for ARM architecture, it is restricted by sched_clock implementation present in arch/arm/kernel/sched_clock.c. Natively, clocksource framework does support change in rate/frequency for registered timer, using, Any kind of switching of timing sources introduces loss of time issues by the mere fact that you can't instantly know the counter values of both timing sources at precisely the same instant - because CPUs can only do one thing at a time. So any kind of repeated dynamic switching _will_ result in a gradual loss of time - which will be indeterminant as it depends on the frequency of the switching and the relative delta between the two. To put it another way - it is not possible to maintain high precision and accuracy while dynamically switching your timing sources. I'm not about to lift the restriction that there's only one source for sched_clock() just for OMAP. That'd require a lot of additional code - it's not just about adjusting the multiplier and shift, you also have to correct the returned ns value as well, which means synchronizing against two counters. (And as I've pointed out above, that's impossible to do accurately.) Thanks a ton Russell for confirming on this, I understand, we also have to adjust ns value and such confirmation is what exactly I was looking for. So this means, we have to use compile time option, as existing implementation in arch/arm/mach-omap2/timer.c. Not exactly. A compile time option isn't (yet) the only option. Russell points out the various problems with dynamic switching of clocksources, which is true. However, we're not trying to dynamically switch clocksources. What we need is only one-time selection at boot based on presence (or not) of various timers. IOW, we still only ever need to call setup_sched_clock() once based on which HW timers are available. Why not just delay the setup_sched_clock() until the clocksource is decided? Kevin -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Fri, Apr 06, 2012 at 03:03:01, Hilman, Kevin wrote:
Hiremath, Vaibhav hvaib...@ti.com writes:
On Thu, Apr 05, 2012 at 15:22:21, Russell King - ARM Linux wrote:
On Thu, Apr 05, 2012 at 09:36:00AM +, Hiremath, Vaibhav wrote:
There seems to be limitation for ARM architecture, it is restricted by
sched_clock implementation present in arch/arm/kernel/sched_clock.c.
Natively, clocksource framework does support change in rate/frequency for
registered timer, using,
Any kind of switching of timing sources introduces loss of time issues
by the mere fact that you can't instantly know the counter values of
both timing sources at precisely the same instant - because CPUs can
only do one thing at a time. So any kind of repeated dynamic switching
_will_ result in a gradual loss of time - which will be indeterminant
as it depends on the frequency of the switching and the relative delta
between the two.
To put it another way - it is not possible to maintain high precision
and accuracy while dynamically switching your timing sources.
I'm not about to lift the restriction that there's only one source for
sched_clock() just for OMAP. That'd require a lot of additional code -
it's not just about adjusting the multiplier and shift, you also have to
correct the returned ns value as well, which means synchronizing against
two counters. (And as I've pointed out above, that's impossible to do
accurately.)
Thanks a ton Russell for confirming on this,
I understand, we also have to adjust ns value and such confirmation is what
exactly I was looking for.
So this means, we have to use compile time option, as existing
implementation in arch/arm/mach-omap2/timer.c.
Not exactly. A compile time option isn't (yet) the only option.
Russell points out the various problems with dynamic switching of
clocksources, which is true. However, we're not trying to dynamically
switch clocksources.
What we need is only one-time selection at boot based on presence (or
not) of various timers. IOW, we still only ever need to call
setup_sched_clock() once based on which HW timers are available.
Why not just delay the setup_sched_clock() until the clocksource is
decided?
Kevin,
I liked Santosh's idea in using command line argument clocksource= and
make decision based on this. I have implemented it and tried it on both
OMAP3EVM and beaglebone and it works great.
I have introduced something like this in mach-omap2/timer.c,
static int __init omap2_override_clocksource(char* str)
{
if (!str)
return 0;
/*
* For OMAP architecture, we only have two options
*- sync_32k (default)
*- gp timer
*/
if (!strcmp(str, gp timer))
use_gptimer_clksrc = true;
return 0;
}
early_param(clocksource, omap2_override_clocksource);
It solves all issues what we have been trying address.
Thanks,
Vaibhav
Kevin
--
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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Tue, Apr 3, 2012 at 9:05 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: On Tue, Apr 03, 2012 at 00:05:38, Hilman, Kevin wrote: Shilimkar, Santosh santosh.shilim...@ti.com writes: [...] I don't personally like to add features which hardly anybody use and fundamentally broken with full kernel. Let's keep sane defaults, but not make it unreasonable to tweak eaither. I suggest what has already been mentioned. Register both timers, but have the sync timer have a higher rating. On AMxxx where there is no sync timer, GPtimer will be used. This is another good option, I can change the rating of both the timers. With below description and given understanding/discussion/usability of both the timers, we can reverse the rating, Btw, if we are going with this path, then there won't any need of commandline option since clock-source can be switched from user space as well. Regards santosh -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Wed, Apr 04, 2012 at 14:34:09, Shilimkar, Santosh wrote: On Tue, Apr 3, 2012 at 9:05 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: On Tue, Apr 03, 2012 at 00:05:38, Hilman, Kevin wrote: Shilimkar, Santosh santosh.shilim...@ti.com writes: [...] I don't personally like to add features which hardly anybody use and fundamentally broken with full kernel. Let's keep sane defaults, but not make it unreasonable to tweak eaither. I suggest what has already been mentioned. Register both timers, but have the sync timer have a higher rating. On AMxxx where there is no sync timer, GPtimer will be used. This is another good option, I can change the rating of both the timers. With below description and given understanding/discussion/usability of both the timers, we can reverse the rating, Btw, if we are going with this path, then there won't any need of commandline option since clock-source can be switched from user space as well. Yes, both the options will be open now and its upto user how he want to choose it (either bootargs or sysfs). 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Tue, Apr 03, 2012 at 00:05:38, Hilman, Kevin wrote: Shilimkar, Santosh santosh.shilim...@ti.com writes: [...] I don't personally like to add features which hardly anybody use and fundamentally broken with full kernel. Let's keep sane defaults, but not make it unreasonable to tweak eaither. I suggest what has already been mentioned. Register both timers, but have the sync timer have a higher rating. On AMxxx where there is no sync timer, GPtimer will be used. This is another good option, I can change the rating of both the timers. With below description and given understanding/discussion/usability of both the timers, we can reverse the rating, 32K-sync counter = 300 GPTIMER = 250 Considering the fact that, 32k-sync counter is more usable than gptimer on OMAP platform. @include/linux/clocksource.h * @rating: rating value for selection (higher is better) * To avoid rating inflation the following * list should give you a guide as to how * to assign your clocksource a rating * 1-99: Unfit for real use * Only available for bootup and testing purposes. * 100-199: Base level usability. * Functional for real use, but not desired. * 200-299: Good. * A correct and usable clocksource. * 300-399: Desired. * A reasonably fast and accurate clocksource. * 400-499: Perfect * The ideal clocksource. A must-use where * available. 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Tue, Apr 3, 2012 at 12:05 AM, Kevin Hilman khil...@ti.com wrote: Shilimkar, Santosh santosh.shilim...@ti.com writes: [...] I don't personally like to add features which hardly anybody use and fundamentally broken with full kernel. Let's keep sane defaults, but not make it unreasonable to tweak eaither. Exactly. Thanks for echoing the concern. I suggest what has already been mentioned. Register both timers, but have the sync timer have a higher rating. On AMxxx where there is no sync timer, GPtimer will be used. Technically it's a hack just from clock precision point of view but I don't mind this. For those who want to use GPtimer, they can boot using clocksource= to override the default. Sounds good to me. Santosh is right, GPtimer will not work on a PM enabled kernel, but there are lots of ways to use the cmdline to get a non-working kernel, so that's OK by me. Let's just ensure that the boot-defaults are sane. Absolutely. Regards Santosh -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Sat, Mar 31, 2012 at 7:00 AM, Ming Lei tom.leim...@gmail.com wrote: On Fri, Mar 30, 2012 at 5:20 PM, Shilimkar, Santosh santosh.shilim...@ti.com After Ming Lie's comment, the point that I came to my mind was, certainly there will be resolution difference between these two clocksources, if gptimer2 is sourced from sys_ck (26Mhz). GPTIMER2 with sysclock is not an option. GPTIMER is not in wakeup domain and when sysclock is cut, it stops. If neither CONFIG_PM or CONFIG_CPUIDLE is set, GPTIMER2 with sysclock should be an option. So it's a PM disable kernel and you will recompile it. As I commented before, high resolution timer is very useful for trace, debug or high frequency perf sample. Suppose you want to trace the running time of one function, 32K counter can only give you a resolution of ~35us, which is too coarse to work well. Since you need to recompile the kernel, you can very much tweak the clocksource to use GPTIMER with sysl clock. Support for that is still in place. Regards Santosh -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Sat, Mar 31, 2012 at 2:30 PM, Shilimkar, Santosh Since you need to recompile the kernel, you can very much tweak the clocksource to use GPTIMER with sysl clock. Support for that is still in place. With current kernel, running 'make menuconfig' can do it, but after applying Hiremath's patch[1], I have to edit the source code manually to get it, so looks this kind of tweaking is not friendly enough, :-( [1], http://marc.info/?l=linux-arm-kernelm=133311647410324w=2 Thanks, -- Ming Lei -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Sat, Mar 31, 2012 at 2:09 PM, Ming Lei tom.leim...@gmail.com wrote: On Sat, Mar 31, 2012 at 2:30 PM, Shilimkar, Santosh Since you need to recompile the kernel, you can very much tweak the clocksource to use GPTIMER with sysl clock. Support for that is still in place. With current kernel, running 'make menuconfig' can do it, but after applying Hiremath's patch[1], I have to edit the source code manually to get it, so looks this kind of tweaking is not friendly enough, :-( It's not friendly but doable. But above can be supported I guess. Since you are talking about doing menuconfig and rebuilding kernel so what can be done is when one disable CPUIDLE and PM, one can disabled 32K source as well. And then 32K clocksource init should fail and fallback on gptimer clock source. Vaibhav, Can you update your patch to support above. The patch which I did was doing exactly that but was not using hwmod. The problem with your patch is synctimer hwmod lookup on OMAP will never fail if the hwmod entry is supported. So you might better use failure case of omap_init_clocksource_32k() for fall back on gptimer source as I tried in my patch. That should support the usage mentioned by Ming Lie Regards Santosh -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Sun, Apr 1, 2012 at 3:10 AM, Shilimkar, Santosh santosh.shilim...@ti.com wrote: On Sat, Mar 31, 2012 at 2:09 PM, Ming Lei tom.leim...@gmail.com wrote: On Sat, Mar 31, 2012 at 2:30 PM, Shilimkar, Santosh Since you need to recompile the kernel, you can very much tweak the clocksource to use GPTIMER with sysl clock. Support for that is still in place. With current kernel, running 'make menuconfig' can do it, but after applying Hiremath's patch[1], I have to edit the source code manually to get it, so looks this kind of tweaking is not friendly enough, :-( It's not friendly but doable. But above can be supported I guess. Since you are talking about doing menuconfig and rebuilding kernel so what can be done is when one disable CPUIDLE and PM, one can disabled 32K source as well. And then 32K clocksource init should fail and fallback on gptimer clock source. OK, it should work, but looks OMAP_32K_TIMER option has to be kept for the usage, which may be a bit divergent to the purpose of the patch set. So how about introducing a kernel parameter to decide if bypassing 32K source and using gptimer source directly, and let which depend on PM? Vaibhav, Can you update your patch to support above. The patch which I did was doing exactly that but was not using hwmod. The problem with your patch is synctimer hwmod lookup on OMAP will never fail if the hwmod entry is supported. So you might better use failure case of omap_init_clocksource_32k() for fall back on gptimer source as I tried in my patch. That should support You patch still doesn't work for the usage because you removed 32K option. Thanks, -- Ming Lei -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Sun, Apr 1, 2012 at 7:09 AM, Ming Lei tom.leim...@gmail.com wrote: On Sun, Apr 1, 2012 at 3:10 AM, Shilimkar, Santosh santosh.shilim...@ti.com wrote: On Sat, Mar 31, 2012 at 2:09 PM, Ming Lei tom.leim...@gmail.com wrote: On Sat, Mar 31, 2012 at 2:30 PM, Shilimkar, Santosh Since you need to recompile the kernel, you can very much tweak the clocksource to use GPTIMER with sysl clock. Support for that is still in place. With current kernel, running 'make menuconfig' can do it, but after applying Hiremath's patch[1], I have to edit the source code manually to get it, so looks this kind of tweaking is not friendly enough, :-( It's not friendly but doable. But above can be supported I guess. Since you are talking about doing menuconfig and rebuilding kernel so what can be done is when one disable CPUIDLE and PM, one can disabled 32K source as well. And then 32K clocksource init should fail and fallback on gptimer clock source. OK, it should work, but looks OMAP_32K_TIMER option has to be kept for the usage, which may be a bit divergent to the purpose of the patch set. So how about introducing a kernel parameter to decide if bypassing 32K source and using gptimer source directly, and let which depend on PM? We are just doing circles on this patch set. Till the dynamic clock-soucre switching supported for low power entry exit, the high precision clocksoucre selection is already broken and very much limited for use I don't personally like to add features which hardly anybody use and fundamentally broken with full kernel. I let Tony take final call on what's appropriate. Regards Santosh -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Wed, Mar 28, 2012 at 20:19:07, Shilimkar, Santosh wrote:
On Wed, Mar 28, 2012 at 8:07 PM, Hiremath, Vaibhav hvaib...@ti.com wrote:
On Wed, Mar 28, 2012 at 19:50:02, Shilimkar, Santosh wrote:
On Wed, Mar 28, 2012 at 7:46 PM, Hiremath, Vaibhav hvaib...@ti.com wrote:
On Wed, Mar 21, 2012 at 19:30:01, Shilimkar, Santosh wrote:
On Wed, Mar 21, 2012 at 5:12 PM, Hiremath, Vaibhav hvaib...@ti.com
wrote:
On Mon, Mar 19, 2012 at 17:45:32, Shilimkar, Santosh wrote:
On Monday 19 March 2012 05:14 PM, Ming Lei wrote:
On Mon, Mar 19, 2012 at 7:11 PM, Hiremath, Vaibhav
hvaib...@ti.com wrote:
[...]
Btw, if you need PM, how are you going to use GPTIMER
as a clocksource. Note sys-clock is generally stopped in
low power states. So that leaves you option with using
gptimer with 32K clock and in that case, GPTIMER
is not a better clock-source compare to 32K sync timer
and so shouldn't be the rating.
AM33xx has GPTIMER1 in wakeup domain, so that we are already using as a
Clocksource, without any issues.
GPTIMER1 is in wakeup domain on OMAP too but that doesn't
solve the issue I am talking. Once the sysclock is stopped, GPTIMER
can't tick anymore even if it is in wakeup domain.
The only way it will work is using always running 32KHz clock as
the clock input to GPT1. And then the end result is the accuracy
of GPTIMER = sync 32K timer. So they are of same rating.
Ohh ok, sorry I should have clarified it in my last response itself.
np.
AM33xx architecture is bit different than OMAP family, and gmtimer1 is
sourced from RTC32K clock, which is in wakeup domain.
This means we have RTC32K clock always running across suspend/resume.
0 - SEL1: Select CLK_M_OSC clock
1 - SEL2: Select CLK_32KHZ clock
2 - SEL3: Select TCLKIN clock
3 - SEL4: Select CLK_RC32K clock
4 - SEL5: Selects the CLK_32768 from 32KHz Crystal Osc
We use value 4 here, for RTC32K (always on clock).
I hope this clarifies what I am trying to say here.
I thought so and now if you look at last part of my comment.
Rating of 32K based synctimer and 32K based GPTIMER
should be same because of the precision. That's the main
point why I was saying that GPTIMER is not a better
clock-source( with 32k clock src) than sync timer when
both are enabled together.
Santosh,
In case of OMAP, we are using timer 2 for clocksource
OMAP_SYS_TIMER_INIT(2, 1, OMAP2_CLKEV_SOURCE, 2, OMAP2_MPU_SOURCE)
OMAP_SYS_TIMER_INIT(3, 1, OMAP3_CLKEV_SOURCE, 2, OMAP3_MPU_SOURCE)
And timer2 as clocksource is never used, since we have CONFIG_OMAP_32K_TIMER
defined in our defconfig.
Also, after looking at the code, I came across another problem,
setup_sched_clock(). But this can be easily fixed, if we source both the timers
with same clock (here 32k_fck).
Let me propose this,
How about, if I keep timer2 source always set to 32k_fclk for omap2,3,4?
And also, as mentioned by Santosh, I will register both the clocks as
clocksource with the same rating.
By default, kernel is going to use 32k_counter as a clocksource, and through
Command prompt user can override it without any issues.
Just to make sure that, whatever I am trying to propse here works and don't get
into unknown issue; I changed my code for this, and it is working for me
without any issues.
Also, this way I can completely get rid of option CONFIG_OMAP_32K_TIMER_HZ.
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 6b12372..ded78b7 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -45,6 +45,7 @@
#include plat/omap_hwmod.h
#include plat/omap_device.h
#include plat/omap-pm.h
+#include plat/clock.h
#include powerdomain.h
@@ -57,18 +58,18 @@
#define OMAP3_32K_SOURCE omap_32k_fck
#define OMAP4_32K_SOURCE sys_32k_ck
-#ifdef CONFIG_OMAP_32K_TIMER
+//#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
-#else
+/*#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
#endif
-
+*/
/* MAX_GPTIMER_ID: number of GPTIMERs on the chip */
#define MAX_GPTIMER_ID 12
@@ -244,6 +245,7 @@ static void __init omap2_gp_clockevent_init(int gptimer_id,
/* Clocksource code */
static struct omap_dm_timer clksrc;
+static bool is_dmtimer_clocksource;
/*
* clocksource
@@ -253,20 +255,38 @@ static cycle_t clocksource_read_cycles(struct clocksource
*cs)
return (cycle_t)__omap_dm_timer_read_counter(clksrc, 1);
}
+static int clocksource_enable(struct clocksource *cs)
+{
+ __omap_dm_timer_load_start(clksrc,
+ OMAP_TIMER_CTRL_ST | OMAP_TIMER_CTRL_AR,
+ omap_32k_read_sched_clock(), 1);
+
Re: [PATCH 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Fri, Mar 30, 2012 at 12:04 PM, Hiremath, Vaibhav hvaib...@ti.com wrote:
On Wed, Mar 28, 2012 at 20:19:07, Shilimkar, Santosh wrote:
On Wed, Mar 28, 2012 at 8:07 PM, Hiremath, Vaibhav hvaib...@ti.com wrote:
On Wed, Mar 28, 2012 at 19:50:02, Shilimkar, Santosh wrote:
On Wed, Mar 28, 2012 at 7:46 PM, Hiremath, Vaibhav hvaib...@ti.com
wrote:
On Wed, Mar 21, 2012 at 19:30:01, Shilimkar, Santosh wrote:
On Wed, Mar 21, 2012 at 5:12 PM, Hiremath, Vaibhav hvaib...@ti.com
wrote:
On Mon, Mar 19, 2012 at 17:45:32, Shilimkar, Santosh wrote:
On Monday 19 March 2012 05:14 PM, Ming Lei wrote:
On Mon, Mar 19, 2012 at 7:11 PM, Hiremath, Vaibhav
hvaib...@ti.com wrote:
[...]
I thought so and now if you look at last part of my comment.
Rating of 32K based synctimer and 32K based GPTIMER
should be same because of the precision. That's the main
point why I was saying that GPTIMER is not a better
clock-source( with 32k clock src) than sync timer when
both are enabled together.
Santosh,
In case of OMAP, we are using timer 2 for clocksource
OMAP_SYS_TIMER_INIT(2, 1, OMAP2_CLKEV_SOURCE, 2, OMAP2_MPU_SOURCE)
OMAP_SYS_TIMER_INIT(3, 1, OMAP3_CLKEV_SOURCE, 2, OMAP3_MPU_SOURCE)
And timer2 as clocksource is never used, since we have CONFIG_OMAP_32K_TIMER
defined in our defconfig.
Also, after looking at the code, I came across another problem,
setup_sched_clock(). But this can be easily fixed, if we source both the
timers with same clock (here 32k_fck).
Let me propose this,
How about, if I keep timer2 source always set to 32k_fclk for omap2,3,4?
And also, as mentioned by Santosh, I will register both the clocks as
clocksource with the same rating.
By default, kernel is going to use 32k_counter as a clocksource, and through
Command prompt user can override it without any issues.
Just to make sure that, whatever I am trying to propse here works and don't
get into unknown issue; I changed my code for this, and it is working for me
without any issues.
Let's not make this more complicated. I guess below simple patch should sort
out the issue. I briefly tested it on OMAP4 and it works. let me know
if it helps AMxxx machines.
Regards
Santosh
From 0a9283e26174d76ff2753ac88521b61a26b24e8f Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar santosh.shilim...@ti.com
Date: Fri, 30 Mar 2012 12:43:29 +0530
Subject: [PATCH] ARM: OMAP: Make OMAP clocksource source selection runtime.
Current OMAP code support couple of clocksource options based on compilation
flag. The 32KHz based sync timer and a gptimer based clock source which can
run on 32KHz or system clock (e.g 38.4 MHz). So there can be 3 options.
1. 32KHz synctimer
2. Sysclock based(e.g 38.4 MHz) gptimer
3. 32KHz based gptimer.
The optional gptimer based clocksource was added so that it can give the
high precision than synctimer so expected usage was 2 and not 3. Unfortunatlly
option 2, clocksource doesn't meet the requirement of free-running clock
as per clocksource need. It stops in low power states when sysclock is cut.
That makes gptimer based clocksource option useless for OMAP2/3/4 devices
with sysclock as a clock input. Option 3 will still work but it is no better'
than 32K synctimer based clocksource.
So ideally we can kill the gptimer based clocksource option but there are some
OMAP based derivative SoCs like AM which doesn't have synictimer hardware IP
and need to fallback on 32KHz based gptimer clocksource.
Considering above, make synctimer and gptimer clocksource runtime
selectable so that both OMAP and AM continue to use the same code.
Not-yet-signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
---
arch/arm/mach-omap2/timer.c | 25 -
1 files changed, 8 insertions(+), 17 deletions(-)
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index c512bac..3878e59 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -234,21 +234,6 @@ static void __init omap2_gp_clockevent_init(int gptimer_id,
}
/* Clocksource code */
-
-#ifdef CONFIG_OMAP_32K_TIMER
-/*
- * When 32k-timer is enabled, don't use GPTimer for clocksource
- * instead, just leave default clocksource which uses the 32k
- * sync counter. See clocksource setup in plat-omap/counter_32k.c
- */
-
-static void __init omap2_gp_clocksource_init(int unused, const char *dummy)
-{
- omap_init_clocksource_32k();
-}
-
-#else
-
static struct omap_dm_timer clksrc;
/*
@@ -276,7 +261,7 @@ static u32 notrace dmtimer_read_sched_clock(void)
}
/* Setup free-running counter for clocksource */
-static void __init omap2_gp_clocksource_init(int gptimer_id,
+static void __init omap2_gptimer_clocksource_init(int gptimer_id,
const char *fck_source)
{
int res;
@@ -295,7 +280,13 @@ static void __init omap2_gp_clocksource_init(int
gptimer_id,
pr_err(Could not register clocksource %s\n,
RE: [PATCH 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Fri, Mar 30, 2012 at 13:11:35, Shilimkar, Santosh wrote:
On Fri, Mar 30, 2012 at 12:04 PM, Hiremath, Vaibhav hvaib...@ti.com wrote:
On Wed, Mar 28, 2012 at 20:19:07, Shilimkar, Santosh wrote:
On Wed, Mar 28, 2012 at 8:07 PM, Hiremath, Vaibhav hvaib...@ti.com wrote:
On Wed, Mar 28, 2012 at 19:50:02, Shilimkar, Santosh wrote:
On Wed, Mar 28, 2012 at 7:46 PM, Hiremath, Vaibhav hvaib...@ti.com
wrote:
On Wed, Mar 21, 2012 at 19:30:01, Shilimkar, Santosh wrote:
On Wed, Mar 21, 2012 at 5:12 PM, Hiremath, Vaibhav hvaib...@ti.com
wrote:
On Mon, Mar 19, 2012 at 17:45:32, Shilimkar, Santosh wrote:
On Monday 19 March 2012 05:14 PM, Ming Lei wrote:
On Mon, Mar 19, 2012 at 7:11 PM, Hiremath, Vaibhav
hvaib...@ti.com wrote:
[...]
I thought so and now if you look at last part of my comment.
Rating of 32K based synctimer and 32K based GPTIMER
should be same because of the precision. That's the main
point why I was saying that GPTIMER is not a better
clock-source( with 32k clock src) than sync timer when
both are enabled together.
Santosh,
In case of OMAP, we are using timer 2 for clocksource
OMAP_SYS_TIMER_INIT(2, 1, OMAP2_CLKEV_SOURCE, 2, OMAP2_MPU_SOURCE)
OMAP_SYS_TIMER_INIT(3, 1, OMAP3_CLKEV_SOURCE, 2, OMAP3_MPU_SOURCE)
And timer2 as clocksource is never used, since we have CONFIG_OMAP_32K_TIMER
defined in our defconfig.
Also, after looking at the code, I came across another problem,
setup_sched_clock(). But this can be easily fixed, if we source both the
timers with same clock (here 32k_fck).
Let me propose this,
How about, if I keep timer2 source always set to 32k_fclk for omap2,3,4?
And also, as mentioned by Santosh, I will register both the clocks as
clocksource with the same rating.
By default, kernel is going to use 32k_counter as a clocksource, and through
Command prompt user can override it without any issues.
Just to make sure that, whatever I am trying to propse here works and don't
get into unknown issue; I changed my code for this, and it is working for
me without any issues.
Let's not make this more complicated. I guess below simple patch should sort
out the issue. I briefly tested it on OMAP4 and it works. let me know
if it helps AMxxx machines.
Regards
Santosh
From 0a9283e26174d76ff2753ac88521b61a26b24e8f Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar santosh.shilim...@ti.com
Date: Fri, 30 Mar 2012 12:43:29 +0530
Subject: [PATCH] ARM: OMAP: Make OMAP clocksource source selection runtime.
Current OMAP code support couple of clocksource options based on compilation
flag. The 32KHz based sync timer and a gptimer based clock source which can
run on 32KHz or system clock (e.g 38.4 MHz). So there can be 3 options.
1. 32KHz synctimer
2. Sysclock based(e.g 38.4 MHz) gptimer
3. 32KHz based gptimer.
The optional gptimer based clocksource was added so that it can give the
high precision than synctimer so expected usage was 2 and not 3. Unfortunatlly
option 2, clocksource doesn't meet the requirement of free-running clock
as per clocksource need. It stops in low power states when sysclock is cut.
That makes gptimer based clocksource option useless for OMAP2/3/4 devices
with sysclock as a clock input. Option 3 will still work but it is no better'
than 32K synctimer based clocksource.
So ideally we can kill the gptimer based clocksource option but there are some
OMAP based derivative SoCs like AM which doesn't have synictimer hardware
IP
and need to fallback on 32KHz based gptimer clocksource.
Considering above, make synctimer and gptimer clocksource runtime
selectable so that both OMAP and AM continue to use the same code.
Not-yet-signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
---
arch/arm/mach-omap2/timer.c | 25 -
1 files changed, 8 insertions(+), 17 deletions(-)
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index c512bac..3878e59 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -234,21 +234,6 @@ static void __init omap2_gp_clockevent_init(int
gptimer_id,
}
/* Clocksource code */
-
-#ifdef CONFIG_OMAP_32K_TIMER
-/*
- * When 32k-timer is enabled, don't use GPTimer for clocksource
- * instead, just leave default clocksource which uses the 32k
- * sync counter. See clocksource setup in plat-omap/counter_32k.c
- */
-
-static void __init omap2_gp_clocksource_init(int unused, const char *dummy)
-{
- omap_init_clocksource_32k();
-}
-
-#else
-
static struct omap_dm_timer clksrc;
/*
@@ -276,7 +261,7 @@ static u32 notrace dmtimer_read_sched_clock(void)
}
/* Setup free-running counter for clocksource */
-static void __init omap2_gp_clocksource_init(int gptimer_id,
+static void __init omap2_gptimer_clocksource_init(int gptimer_id,
const char
Re: [PATCH 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Friday 30 March 2012 02:02 PM, Hiremath, Vaibhav wrote:
On Fri, Mar 30, 2012 at 13:11:35, Shilimkar, Santosh wrote:
On Fri, Mar 30, 2012 at 12:04 PM, Hiremath, Vaibhav hvaib...@ti.com wrote:
On Wed, Mar 28, 2012 at 20:19:07, Shilimkar, Santosh wrote:
On Wed, Mar 28, 2012 at 8:07 PM, Hiremath, Vaibhav hvaib...@ti.com wrote:
On Wed, Mar 28, 2012 at 19:50:02, Shilimkar, Santosh wrote:
On Wed, Mar 28, 2012 at 7:46 PM, Hiremath, Vaibhav hvaib...@ti.com
wrote:
On Wed, Mar 21, 2012 at 19:30:01, Shilimkar, Santosh wrote:
On Wed, Mar 21, 2012 at 5:12 PM, Hiremath, Vaibhav hvaib...@ti.com
wrote:
On Mon, Mar 19, 2012 at 17:45:32, Shilimkar, Santosh wrote:
On Monday 19 March 2012 05:14 PM, Ming Lei wrote:
On Mon, Mar 19, 2012 at 7:11 PM, Hiremath, Vaibhav
hvaib...@ti.com wrote:
[...]
I thought so and now if you look at last part of my comment.
Rating of 32K based synctimer and 32K based GPTIMER
should be same because of the precision. That's the main
point why I was saying that GPTIMER is not a better
clock-source( with 32k clock src) than sync timer when
both are enabled together.
Santosh,
In case of OMAP, we are using timer 2 for clocksource
OMAP_SYS_TIMER_INIT(2, 1, OMAP2_CLKEV_SOURCE, 2, OMAP2_MPU_SOURCE)
OMAP_SYS_TIMER_INIT(3, 1, OMAP3_CLKEV_SOURCE, 2, OMAP3_MPU_SOURCE)
And timer2 as clocksource is never used, since we have CONFIG_OMAP_32K_TIMER
defined in our defconfig.
Also, after looking at the code, I came across another problem,
setup_sched_clock(). But this can be easily fixed, if we source both the
timers with same clock (here 32k_fck).
Let me propose this,
How about, if I keep timer2 source always set to 32k_fclk for omap2,3,4?
And also, as mentioned by Santosh, I will register both the clocks as
clocksource with the same rating.
By default, kernel is going to use 32k_counter as a clocksource, and through
Command prompt user can override it without any issues.
Just to make sure that, whatever I am trying to propse here works and don't
get into unknown issue; I changed my code for this, and it is working for
me without any issues.
Let's not make this more complicated. I guess below simple patch should sort
out the issue. I briefly tested it on OMAP4 and it works. let me know
if it helps AMxxx machines.
Regards
Santosh
From 0a9283e26174d76ff2753ac88521b61a26b24e8f Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar santosh.shilim...@ti.com
Date: Fri, 30 Mar 2012 12:43:29 +0530
Subject: [PATCH] ARM: OMAP: Make OMAP clocksource source selection runtime.
Current OMAP code support couple of clocksource options based on compilation
flag. The 32KHz based sync timer and a gptimer based clock source which can
run on 32KHz or system clock (e.g 38.4 MHz). So there can be 3 options.
1. 32KHz synctimer
2. Sysclock based(e.g 38.4 MHz) gptimer
3. 32KHz based gptimer.
The optional gptimer based clocksource was added so that it can give the
high precision than synctimer so expected usage was 2 and not 3.
Unfortunatlly
option 2, clocksource doesn't meet the requirement of free-running clock
as per clocksource need. It stops in low power states when sysclock is cut.
That makes gptimer based clocksource option useless for OMAP2/3/4 devices
with sysclock as a clock input. Option 3 will still work but it is no better'
than 32K synctimer based clocksource.
So ideally we can kill the gptimer based clocksource option but there are
some
OMAP based derivative SoCs like AM which doesn't have synictimer
hardware IP
and need to fallback on 32KHz based gptimer clocksource.
Considering above, make synctimer and gptimer clocksource runtime
selectable so that both OMAP and AM continue to use the same code.
Not-yet-signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
---
arch/arm/mach-omap2/timer.c | 25 -
1 files changed, 8 insertions(+), 17 deletions(-)
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index c512bac..3878e59 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -234,21 +234,6 @@ static void __init omap2_gp_clockevent_init(int
gptimer_id,
}
/* Clocksource code */
-
-#ifdef CONFIG_OMAP_32K_TIMER
-/*
- * When 32k-timer is enabled, don't use GPTimer for clocksource
- * instead, just leave default clocksource which uses the 32k
- * sync counter. See clocksource setup in plat-omap/counter_32k.c
- */
-
-static void __init omap2_gp_clocksource_init(int unused, const char *dummy)
-{
-omap_init_clocksource_32k();
-}
-
-#else
-
static struct omap_dm_timer clksrc;
/*
@@ -276,7 +261,7 @@ static u32 notrace dmtimer_read_sched_clock(void)
}
/* Setup free-running counter for clocksource */
-static void __init omap2_gp_clocksource_init(int gptimer_id,
+static void __init omap2_gptimer_clocksource_init(int gptimer_id,
const char *fck_source)
{
int res;
RE: [PATCH 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Fri, Mar 30, 2012 at 14:08:20, Shilimkar, Santosh wrote:
On Friday 30 March 2012 02:02 PM, Hiremath, Vaibhav wrote:
On Fri, Mar 30, 2012 at 13:11:35, Shilimkar, Santosh wrote:
On Fri, Mar 30, 2012 at 12:04 PM, Hiremath, Vaibhav hvaib...@ti.com
wrote:
On Wed, Mar 28, 2012 at 20:19:07, Shilimkar, Santosh wrote:
On Wed, Mar 28, 2012 at 8:07 PM, Hiremath, Vaibhav hvaib...@ti.com
wrote:
On Wed, Mar 28, 2012 at 19:50:02, Shilimkar, Santosh wrote:
On Wed, Mar 28, 2012 at 7:46 PM, Hiremath, Vaibhav hvaib...@ti.com
wrote:
On Wed, Mar 21, 2012 at 19:30:01, Shilimkar, Santosh wrote:
On Wed, Mar 21, 2012 at 5:12 PM, Hiremath, Vaibhav hvaib...@ti.com
wrote:
On Mon, Mar 19, 2012 at 17:45:32, Shilimkar, Santosh wrote:
On Monday 19 March 2012 05:14 PM, Ming Lei wrote:
On Mon, Mar 19, 2012 at 7:11 PM, Hiremath, Vaibhav
hvaib...@ti.com wrote:
[...]
I thought so and now if you look at last part of my comment.
Rating of 32K based synctimer and 32K based GPTIMER
should be same because of the precision. That's the main
point why I was saying that GPTIMER is not a better
clock-source( with 32k clock src) than sync timer when
both are enabled together.
Santosh,
In case of OMAP, we are using timer 2 for clocksource
OMAP_SYS_TIMER_INIT(2, 1, OMAP2_CLKEV_SOURCE, 2, OMAP2_MPU_SOURCE)
OMAP_SYS_TIMER_INIT(3, 1, OMAP3_CLKEV_SOURCE, 2, OMAP3_MPU_SOURCE)
And timer2 as clocksource is never used, since we have
CONFIG_OMAP_32K_TIMER
defined in our defconfig.
Also, after looking at the code, I came across another problem,
setup_sched_clock(). But this can be easily fixed, if we source both the
timers with same clock (here 32k_fck).
Let me propose this,
How about, if I keep timer2 source always set to 32k_fclk for omap2,3,4?
And also, as mentioned by Santosh, I will register both the clocks as
clocksource with the same rating.
By default, kernel is going to use 32k_counter as a clocksource, and
through
Command prompt user can override it without any issues.
Just to make sure that, whatever I am trying to propse here works and
don't get into unknown issue; I changed my code for this, and it is
working for me without any issues.
Let's not make this more complicated. I guess below simple patch should
sort
out the issue. I briefly tested it on OMAP4 and it works. let me know
if it helps AMxxx machines.
Regards
Santosh
From 0a9283e26174d76ff2753ac88521b61a26b24e8f Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar santosh.shilim...@ti.com
Date: Fri, 30 Mar 2012 12:43:29 +0530
Subject: [PATCH] ARM: OMAP: Make OMAP clocksource source selection runtime.
Current OMAP code support couple of clocksource options based on
compilation
flag. The 32KHz based sync timer and a gptimer based clock source which can
run on 32KHz or system clock (e.g 38.4 MHz). So there can be 3 options.
1. 32KHz synctimer
2. Sysclock based(e.g 38.4 MHz) gptimer
3. 32KHz based gptimer.
The optional gptimer based clocksource was added so that it can give the
high precision than synctimer so expected usage was 2 and not 3.
Unfortunatlly
option 2, clocksource doesn't meet the requirement of free-running clock
as per clocksource need. It stops in low power states when sysclock is
cut.
That makes gptimer based clocksource option useless for OMAP2/3/4 devices
with sysclock as a clock input. Option 3 will still work but it is no
better'
than 32K synctimer based clocksource.
So ideally we can kill the gptimer based clocksource option but there are
some
OMAP based derivative SoCs like AM which doesn't have synictimer
hardware IP
and need to fallback on 32KHz based gptimer clocksource.
Considering above, make synctimer and gptimer clocksource runtime
selectable so that both OMAP and AM continue to use the same code.
Not-yet-signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
---
arch/arm/mach-omap2/timer.c | 25 -
1 files changed, 8 insertions(+), 17 deletions(-)
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index c512bac..3878e59 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -234,21 +234,6 @@ static void __init omap2_gp_clockevent_init(int
gptimer_id,
}
/* Clocksource code */
-
-#ifdef CONFIG_OMAP_32K_TIMER
-/*
- * When 32k-timer is enabled, don't use GPTimer for clocksource
- * instead, just leave default clocksource which uses the 32k
- * sync counter. See clocksource setup in plat-omap/counter_32k.c
- */
-
-static void __init omap2_gp_clocksource_init(int unused, const char
*dummy)
-{
- omap_init_clocksource_32k();
-}
-
-#else
-
static struct omap_dm_timer clksrc;
/*
@@ -276,7 +261,7 @@ static u32 notrace dmtimer_read_sched_clock(void)
}
/* Setup free-running counter for clocksource */
Re: [PATCH 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Fri, Mar 30, 2012 at 2:42 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: On Fri, Mar 30, 2012 at 14:08:20, Shilimkar, Santosh wrote: On Friday 30 March 2012 02:02 PM, Hiremath, Vaibhav wrote: On Fri, Mar 30, 2012 at 13:11:35, Shilimkar, Santosh wrote: [] With this patch, will you be able to choose gptimer as a clocksource using bootparameter (or sysfs) for given kernel uImage? Why do you want that ? Look at changelog. The gptimer based clocksource is useless for OMAP and for AM devices synctimer is not available. The answer is simply NO...as the registration of gptimer is based on failure from omap_init_clocksource_32k(). And this is nothing different than my original patch, my patch exactly does same thing. I ight have missed your original patch. If that patch is similar then no problems. The requirement after 'ming Lie' response on my patch was, there will be usecases where we might need to use gptimer for clocksource and with the patch it is not possible, since you will only register 32k_counter here. I think Ming Lie might have expected that gptimer clocksource might be better which is not the case. So in order to allow user to choose between 32K and gptimer, you must register both and make 32k as a default thing. As described in the commit log, its not needed at all. Let's not add a feature which is just useless because the gptimer based clock source has no advantage against the syntimer. I completely agree with you, and that is my understanding too. Thanks !! After Ming Lie's comment, the point that I came to my mind was, certainly there will be resolution difference between these two clocksources, if gptimer2 is sourced from sys_ck (26Mhz). GPTIMER2 with sysclock is not an option. GPTIMER is not in wakeup domain and when sysclock is cut, it stops. I am quite not sure, whether will there be any practical usecase where you change the kernel clocksource for high resolution dynamically through sysfs or something. May be notbut still it is possible. Even if there is a usecase, there no option with full PM. In that case my original patch still holds good here. I would still request you to review the same and give your acked-by or tested-by. I just looked at that. It looks fine to me. Can you repost that patch addressing Kevin and Tony's comments. Also update the change log as describe in the patch i posted. Once that done, will ack it. Regards Santosh -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Fri, Mar 30, 2012 at 14:50:02, Shilimkar, Santosh wrote: On Fri, Mar 30, 2012 at 2:42 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: On Fri, Mar 30, 2012 at 14:08:20, Shilimkar, Santosh wrote: On Friday 30 March 2012 02:02 PM, Hiremath, Vaibhav wrote: On Fri, Mar 30, 2012 at 13:11:35, Shilimkar, Santosh wrote: [] With this patch, will you be able to choose gptimer as a clocksource using bootparameter (or sysfs) for given kernel uImage? Why do you want that ? Look at changelog. The gptimer based clocksource is useless for OMAP and for AM devices synctimer is not available. The answer is simply NO...as the registration of gptimer is based on failure from omap_init_clocksource_32k(). And this is nothing different than my original patch, my patch exactly does same thing. I ight have missed your original patch. If that patch is similar then no problems. The requirement after 'ming Lie' response on my patch was, there will be usecases where we might need to use gptimer for clocksource and with the patch it is not possible, since you will only register 32k_counter here. I think Ming Lie might have expected that gptimer clocksource might be better which is not the case. So in order to allow user to choose between 32K and gptimer, you must register both and make 32k as a default thing. As described in the commit log, its not needed at all. Let's not add a feature which is just useless because the gptimer based clock source has no advantage against the syntimer. I completely agree with you, and that is my understanding too. Thanks !! After Ming Lie's comment, the point that I came to my mind was, certainly there will be resolution difference between these two clocksources, if gptimer2 is sourced from sys_ck (26Mhz). GPTIMER2 with sysclock is not an option. GPTIMER is not in wakeup domain and when sysclock is cut, it stops. I am quite not sure, whether will there be any practical usecase where you change the kernel clocksource for high resolution dynamically through sysfs or something. May be notbut still it is possible. Even if there is a usecase, there no option with full PM. What if before suspending the system, you switch back to 32k_counter everytime, and in resume you again switch to gp_timer? Please consider this as just a technical discussion, as I am myself quite not sure whether we have such use-case available. In that case my original patch still holds good here. I would still request you to review the same and give your acked-by or tested-by. I just looked at that. It looks fine to me. Can you repost that patch addressing Kevin and Tony's comments. Also update the change log as describe in the patch i posted. Once that done, will ack it. Thanks for the review and discussion, I will submit revised version shortly. 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Fri, Mar 30, 2012 at 2:58 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: On Fri, Mar 30, 2012 at 14:50:02, Shilimkar, Santosh wrote: On Fri, Mar 30, 2012 at 2:42 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: On Fri, Mar 30, 2012 at 14:08:20, Shilimkar, Santosh wrote: On Friday 30 March 2012 02:02 PM, Hiremath, Vaibhav wrote: On Fri, Mar 30, 2012 at 13:11:35, Shilimkar, Santosh wrote: [] With this patch, will you be able to choose gptimer as a clocksource using bootparameter (or sysfs) for given kernel uImage? Why do you want that ? Look at changelog. The gptimer based clocksource is useless for OMAP and for AM devices synctimer is not available. The answer is simply NO...as the registration of gptimer is based on failure from omap_init_clocksource_32k(). And this is nothing different than my original patch, my patch exactly does same thing. I ight have missed your original patch. If that patch is similar then no problems. The requirement after 'ming Lie' response on my patch was, there will be usecases where we might need to use gptimer for clocksource and with the patch it is not possible, since you will only register 32k_counter here. I think Ming Lie might have expected that gptimer clocksource might be better which is not the case. So in order to allow user to choose between 32K and gptimer, you must register both and make 32k as a default thing. As described in the commit log, its not needed at all. Let's not add a feature which is just useless because the gptimer based clock source has no advantage against the syntimer. I completely agree with you, and that is my understanding too. Thanks !! After Ming Lie's comment, the point that I came to my mind was, certainly there will be resolution difference between these two clocksources, if gptimer2 is sourced from sys_ck (26Mhz). GPTIMER2 with sysclock is not an option. GPTIMER is not in wakeup domain and when sysclock is cut, it stops. I am quite not sure, whether will there be any practical usecase where you change the kernel clocksource for high resolution dynamically through sysfs or something. May be notbut still it is possible. Even if there is a usecase, there no option with full PM. What if before suspending the system, you switch back to 32k_counter everytime, and in resume you again switch to gp_timer? This has been discussed at length. Dynamic switching between clock-sources affects the NTP time corrections. Go through [1] when you have time, since its a long thread. If and when that feature works, we can update the clocksource code. Regards Santosh [1] https://lkml.org/lkml/2011/6/2/167 -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Fri, Mar 30, 2012 at 15:12:19, Shilimkar, Santosh wrote: On Fri, Mar 30, 2012 at 2:58 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: On Fri, Mar 30, 2012 at 14:50:02, Shilimkar, Santosh wrote: On Fri, Mar 30, 2012 at 2:42 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: On Fri, Mar 30, 2012 at 14:08:20, Shilimkar, Santosh wrote: On Friday 30 March 2012 02:02 PM, Hiremath, Vaibhav wrote: On Fri, Mar 30, 2012 at 13:11:35, Shilimkar, Santosh wrote: ... Santosh others, Do you think, it makes sense to clean up the CONFIG_OMAP_32K_TIMER usage in mach-omap2/timer.c file now? Specifically below code-snippet - #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 #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 #endif And lets make 32k_fclk as default clocksource for gptimer, if it is being used as clocksource. Atleast with this I can clean whole omap2/3/4 families, and I can take omap1 thing later . (as I have to understand/read about omap1 first). If you agree, I can include this cleanup also in my next version of patch- series. What's your opinion here? 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Friday 30 March 2012 04:59 PM, Hiremath, Vaibhav wrote: On Fri, Mar 30, 2012 at 15:12:19, Shilimkar, Santosh wrote: On Fri, Mar 30, 2012 at 2:58 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: On Fri, Mar 30, 2012 at 14:50:02, Shilimkar, Santosh wrote: On Fri, Mar 30, 2012 at 2:42 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: On Fri, Mar 30, 2012 at 14:08:20, Shilimkar, Santosh wrote: On Friday 30 March 2012 02:02 PM, Hiremath, Vaibhav wrote: On Fri, Mar 30, 2012 at 13:11:35, Shilimkar, Santosh wrote: ... Santosh others, Do you think, it makes sense to clean up the CONFIG_OMAP_32K_TIMER usage in mach-omap2/timer.c file now? Specifically below code-snippet - #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 #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 #endif And lets make 32k_fclk as default clocksource for gptimer, if it is being used as clocksource. Atleast with this I can clean whole omap2/3/4 families, and I can take omap1 thing later . (as I have to understand/read about omap1 first). If you agree, I can include this cleanup also in my next version of patch- series. What's your opinion here? I am ok with this clean up. But I let Tony take decision on it. Regards Santosh -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Fri, Mar 30, 2012 at 5:20 PM, Shilimkar, Santosh santosh.shilim...@ti.com After Ming Lie's comment, the point that I came to my mind was, certainly there will be resolution difference between these two clocksources, if gptimer2 is sourced from sys_ck (26Mhz). GPTIMER2 with sysclock is not an option. GPTIMER is not in wakeup domain and when sysclock is cut, it stops. If neither CONFIG_PM or CONFIG_CPUIDLE is set, GPTIMER2 with sysclock should be an option. As I commented before, high resolution timer is very useful for trace, debug or high frequency perf sample. Suppose you want to trace the running time of one function, 32K counter can only give you a resolution of ~35us, which is too coarse to work well. Thanks, -- Ming Lei -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Wed, Mar 21, 2012 at 19:30:01, Shilimkar, Santosh wrote: On Wed, Mar 21, 2012 at 5:12 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: On Mon, Mar 19, 2012 at 17:45:32, Shilimkar, Santosh wrote: On Monday 19 March 2012 05:14 PM, Ming Lei wrote: On Mon, Mar 19, 2012 at 7:11 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: I think you made very good point here. With the above patch, we are almost missing the capability of registering dmtimer as a clocksource for OMAP. It will always use 32k-counter, and never fall back to dmtimer. Then the only options we have here is, 1) Register both the timers, 32k-counter and dmtimer for clocksource; let Kernel pick up best rating clocksource out of these two. In case of OMAP1/2/3/4, kernel will use dmtimer, since it has better Rating. User can choose the 32k-counter clocksource via bootargs. Impact: without bootargs for clocksource selection, kernel will choose dmtimer, impacting loss of time during suspend/resume. This is the right option. The problem is gptimer clocksource doesn't work across power transitions and hence it is broken. Even for the perf, with PM enabled, dmtimer can't be used or it needs to be used with 32KHz clock which makes it no better than sync timer. So here keeping 32K sync timer is default clocksource makes sense since it is the only working and viable option. So what can be done is register both 32K and gptimer together but make gptimer clocksource registration depends on PM enabled. This should solve all the needs I guess. I am not sure this will work on all platforms, for example, AM33XX, where We do not have 32ksync timer available, but we need PM support. Also, I personally think, we should not again use compile time option here. Why not ? If 32ksync timer is not available, don't register it. Then in AM case you just have one clock-source. In case of AM33xx, what about kernel without PM support? As clocksource registration would be dependent on PM option, it won't work. I am against the idea of making gptimer as the default and asking user to choose sync32k from command-line. I understand your concern here. Btw, if you need PM, how are you going to use GPTIMER as a clocksource. Note sys-clock is generally stopped in low power states. So that leaves you option with using gptimer with 32K clock and in that case, GPTIMER is not a better clock-source compare to 32K sync timer and so shouldn't be the rating. AM33xx has GPTIMER1 in wakeup domain, so that we are already using as a Clocksource, without any issues. Thanks, Vaibhav Regards Santosh -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Wed, Mar 28, 2012 at 7:46 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: On Wed, Mar 21, 2012 at 19:30:01, Shilimkar, Santosh wrote: On Wed, Mar 21, 2012 at 5:12 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: On Mon, Mar 19, 2012 at 17:45:32, Shilimkar, Santosh wrote: On Monday 19 March 2012 05:14 PM, Ming Lei wrote: On Mon, Mar 19, 2012 at 7:11 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: [...] Btw, if you need PM, how are you going to use GPTIMER as a clocksource. Note sys-clock is generally stopped in low power states. So that leaves you option with using gptimer with 32K clock and in that case, GPTIMER is not a better clock-source compare to 32K sync timer and so shouldn't be the rating. AM33xx has GPTIMER1 in wakeup domain, so that we are already using as a Clocksource, without any issues. GPTIMER1 is in wakeup domain on OMAP too but that doesn't solve the issue I am talking. Once the sysclock is stopped, GPTIMER can't tick anymore even if it is in wakeup domain. The only way it will work is using always running 32KHz clock as the clock input to GPT1. And then the end result is the accuracy of GPTIMER = sync 32K timer. So they are of same rating. Hope it is clear now. Regards Santosh -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Wed, Mar 28, 2012 at 19:50:02, Shilimkar, Santosh wrote: On Wed, Mar 28, 2012 at 7:46 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: On Wed, Mar 21, 2012 at 19:30:01, Shilimkar, Santosh wrote: On Wed, Mar 21, 2012 at 5:12 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: On Mon, Mar 19, 2012 at 17:45:32, Shilimkar, Santosh wrote: On Monday 19 March 2012 05:14 PM, Ming Lei wrote: On Mon, Mar 19, 2012 at 7:11 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: [...] Btw, if you need PM, how are you going to use GPTIMER as a clocksource. Note sys-clock is generally stopped in low power states. So that leaves you option with using gptimer with 32K clock and in that case, GPTIMER is not a better clock-source compare to 32K sync timer and so shouldn't be the rating. AM33xx has GPTIMER1 in wakeup domain, so that we are already using as a Clocksource, without any issues. GPTIMER1 is in wakeup domain on OMAP too but that doesn't solve the issue I am talking. Once the sysclock is stopped, GPTIMER can't tick anymore even if it is in wakeup domain. The only way it will work is using always running 32KHz clock as the clock input to GPT1. And then the end result is the accuracy of GPTIMER = sync 32K timer. So they are of same rating. Ohh ok, sorry I should have clarified it in my last response itself. AM33xx architecture is bit different than OMAP family, and gmtimer1 is sourced from RTC32K clock, which is in wakeup domain. This means we have RTC32K clock always running across suspend/resume. 0 - SEL1: Select CLK_M_OSC clock 1 - SEL2: Select CLK_32KHZ clock 2 - SEL3: Select TCLKIN clock 3 - SEL4: Select CLK_RC32K clock 4 - SEL5: Selects the CLK_32768 from 32KHz Crystal Osc We use value 4 here, for RTC32K (always on clock). I hope this clarifies what I am trying to say here. Thanks, Vaibhav Hope it is clear now. Regards Santosh -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Wed, Mar 28, 2012 at 8:07 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: On Wed, Mar 28, 2012 at 19:50:02, Shilimkar, Santosh wrote: On Wed, Mar 28, 2012 at 7:46 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: On Wed, Mar 21, 2012 at 19:30:01, Shilimkar, Santosh wrote: On Wed, Mar 21, 2012 at 5:12 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: On Mon, Mar 19, 2012 at 17:45:32, Shilimkar, Santosh wrote: On Monday 19 March 2012 05:14 PM, Ming Lei wrote: On Mon, Mar 19, 2012 at 7:11 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: [...] Btw, if you need PM, how are you going to use GPTIMER as a clocksource. Note sys-clock is generally stopped in low power states. So that leaves you option with using gptimer with 32K clock and in that case, GPTIMER is not a better clock-source compare to 32K sync timer and so shouldn't be the rating. AM33xx has GPTIMER1 in wakeup domain, so that we are already using as a Clocksource, without any issues. GPTIMER1 is in wakeup domain on OMAP too but that doesn't solve the issue I am talking. Once the sysclock is stopped, GPTIMER can't tick anymore even if it is in wakeup domain. The only way it will work is using always running 32KHz clock as the clock input to GPT1. And then the end result is the accuracy of GPTIMER = sync 32K timer. So they are of same rating. Ohh ok, sorry I should have clarified it in my last response itself. np. AM33xx architecture is bit different than OMAP family, and gmtimer1 is sourced from RTC32K clock, which is in wakeup domain. This means we have RTC32K clock always running across suspend/resume. 0 - SEL1: Select CLK_M_OSC clock 1 - SEL2: Select CLK_32KHZ clock 2 - SEL3: Select TCLKIN clock 3 - SEL4: Select CLK_RC32K clock 4 - SEL5: Selects the CLK_32768 from 32KHz Crystal Osc We use value 4 here, for RTC32K (always on clock). I hope this clarifies what I am trying to say here. I thought so and now if you look at last part of my comment. Rating of 32K based synctimer and 32K based GPTIMER should be same because of the precision. That's the main point why I was saying that GPTIMER is not a better clock-source( with 32k clock src) than sync timer when both are enabled together. Regards Santosh -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Wed, Mar 21, 2012 at 7:29 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: On Mon, Mar 19, 2012 at 17:14:30, Ming Lei wrote: On Mon, Mar 19, 2012 at 7:11 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: I think you made very good point here. With the above patch, we are almost missing the capability of registering dmtimer as a clocksource for OMAP. It will always use 32k-counter, and never fall back to dmtimer. Then the only options we have here is, 1) Register both the timers, 32k-counter and dmtimer for clocksource; let Kernel pick up best rating clocksource out of these two. In case of OMAP1/2/3/4, kernel will use dmtimer, since it has better Rating. User can choose the 32k-counter clocksource via bootargs. Impact: without bootargs for clocksource selection, kernel will choose dmtimer, impacting loss of time during suspend/resume. 2) Let the current code be as is, means, the clocksource registration will Happened based on #ifdef CONFIG_OMAP_32K_TIMER and this option selection will be Controlled by Kconfig rules. How about the 3rd option? 3), take the way in your patch 1) at default, but will switch to register dmtimer directly and bypass 32k-counter if user need it via kernel parameter. As far as I can think of, the situations required for dmtimer are high-frequency perf sample and high precision trace points, so looks it is OK to take 32k-counter at default. But if you register both the timers (dmtimer 32ksync), then initially kernel will only pick up dmtimer, as this has better rating. And late in Looks not so, I found that 32ksync is always selected as the default clocksource if both are registered. the boot sequence clocksource switch will happen, base on kernel parameter (clocksource=). So logically dmtimer will be always used as a default here. Not so at least on my Pandaboard. Thanks, -- Ming Lei -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Mon, Mar 19, 2012 at 17:14:30, Ming Lei wrote: On Mon, Mar 19, 2012 at 7:11 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: I think you made very good point here. With the above patch, we are almost missing the capability of registering dmtimer as a clocksource for OMAP. It will always use 32k-counter, and never fall back to dmtimer. Then the only options we have here is, 1) Register both the timers, 32k-counter and dmtimer for clocksource; let Kernel pick up best rating clocksource out of these two. In case of OMAP1/2/3/4, kernel will use dmtimer, since it has better Rating. User can choose the 32k-counter clocksource via bootargs. Impact: without bootargs for clocksource selection, kernel will choose dmtimer, impacting loss of time during suspend/resume. 2) Let the current code be as is, means, the clocksource registration will Happened based on #ifdef CONFIG_OMAP_32K_TIMER and this option selection will be Controlled by Kconfig rules. How about the 3rd option? 3), take the way in your patch 1) at default, but will switch to register dmtimer directly and bypass 32k-counter if user need it via kernel parameter. As far as I can think of, the situations required for dmtimer are high-frequency perf sample and high precision trace points, so looks it is OK to take 32k-counter at default. But if you register both the timers (dmtimer 32ksync), then initially kernel will only pick up dmtimer, as this has better rating. And late in the boot sequence clocksource switch will happen, base on kernel parameter (clocksource=). So logically dmtimer will be always used as a default here. Thanks, Vaibhav Thanks, -- Ming Lei -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Mon, Mar 19, 2012 at 17:45:32, Shilimkar, Santosh wrote:
On Monday 19 March 2012 05:14 PM, Ming Lei wrote:
On Mon, Mar 19, 2012 at 7:11 PM, Hiremath, Vaibhav hvaib...@ti.com wrote:
I think you made very good point here. With the above patch, we are almost
missing the capability of registering dmtimer as a clocksource for OMAP.
It will always use 32k-counter, and never fall back to dmtimer.
Then the only options we have here is,
1) Register both the timers, 32k-counter and dmtimer for clocksource; let
Kernel pick up best rating clocksource out of these two.
In case of OMAP1/2/3/4, kernel will use dmtimer, since it has better
Rating. User can choose the 32k-counter clocksource via bootargs.
Impact: without bootargs for clocksource selection, kernel will choose
dmtimer, impacting loss of time during suspend/resume.
This is the right option. The problem is gptimer clocksource
doesn't work across power transitions and hence it is broken.
Even for the perf, with PM enabled, dmtimer can't be used or it needs
to be used with 32KHz clock which makes it no better than sync timer.
So here keeping 32K sync timer is default clocksource makes sense since
it is the only working and viable option.
So what can be done is register both 32K and gptimer together but make
gptimer clocksource registration depends on PM enabled.
This should solve all the needs I guess.
I am not sure this will work on all platforms, for example, AM33XX, where
We do not have 32ksync timer available, but we need PM support. Also, I
personally think, we should not again use compile time option here.
So the only option I have here is, register both the clocksources, let dmtimer
be a default clocksource for the kernel (since it has better rating),
And based on kernel parameter user can change the clocksource, specially
for PM use-cases.
Implementation point of view, I just need to do something like,
static void __init omap2_gp_clocksource_init(int gptimer_id,
const char *fck_source)
{
int res;
res = omap_init_clocksource_32k();
if (!res)
pr_err(failed to register 32ksync counter as a clocksource\n);
/*
* Continue with dmtimer registration as well, irrespective of
* whether 32ksync timer registration succeeded or not.
*/
}
2) Let the current code be as is, means, the clocksource registration will
Happened based on #ifdef CONFIG_OMAP_32K_TIMER and this option
selection will be Controlled by Kconfig rules.
We should get rid off CONFIG_OMAP_32K_TIMER.
Agreed, I will take this activity once I close on this.
Thanks,
Vaibhav
Regards
Santosh
--
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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Wed, Mar 21, 2012 at 5:12 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: On Mon, Mar 19, 2012 at 17:45:32, Shilimkar, Santosh wrote: On Monday 19 March 2012 05:14 PM, Ming Lei wrote: On Mon, Mar 19, 2012 at 7:11 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: I think you made very good point here. With the above patch, we are almost missing the capability of registering dmtimer as a clocksource for OMAP. It will always use 32k-counter, and never fall back to dmtimer. Then the only options we have here is, 1) Register both the timers, 32k-counter and dmtimer for clocksource; let Kernel pick up best rating clocksource out of these two. In case of OMAP1/2/3/4, kernel will use dmtimer, since it has better Rating. User can choose the 32k-counter clocksource via bootargs. Impact: without bootargs for clocksource selection, kernel will choose dmtimer, impacting loss of time during suspend/resume. This is the right option. The problem is gptimer clocksource doesn't work across power transitions and hence it is broken. Even for the perf, with PM enabled, dmtimer can't be used or it needs to be used with 32KHz clock which makes it no better than sync timer. So here keeping 32K sync timer is default clocksource makes sense since it is the only working and viable option. So what can be done is register both 32K and gptimer together but make gptimer clocksource registration depends on PM enabled. This should solve all the needs I guess. I am not sure this will work on all platforms, for example, AM33XX, where We do not have 32ksync timer available, but we need PM support. Also, I personally think, we should not again use compile time option here. Why not ? If 32ksync timer is not available, don't register it. Then in AM case you just have one clock-source. I am against the idea of making gptimer as the default and asking user to choose sync32k from command-line. Btw, if you need PM, how are you going to use GPTIMER as a clocksource. Note sys-clock is generally stopped in low power states. So that leaves you option with using gptimer with 32K clock and in that case, GPTIMER is not a better clock-source compare to 32K sync timer and so shouldn't be the rating. Regards Santosh -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Tue, Mar 13, 2012 at 17:07:16, Ming Lei wrote: Hi, On Tue, Jan 24, 2012 at 7:38 AM, Kevin Hilman khil...@ti.com wrote: Vaibhav Hiremath hvaib...@ti.com writes: OMAP device has 32k-sync timer which is currently used as a clocksource in the kernel (omap2plus_defconfig). The current implementation uses compile time selection between gp-timer and 32k-sync timer, which breaks multi-omap build for the devices like AM33xx, where 32k-sync timer is not available. So use hwmod database lookup mechanism, through which at run-time we can identify availability of 32k-sync timer on the device, else fall back to gp-timer. With the runtime detection fallback, I suggest also removing the Kconfig choice between the two as well. IMO, under some situations, gp timer clocksource has high precision and is very useful, so hope the Kconfig choice can be kept, or using kernel parameter way, and the patch need to be changed to support the selection. I think you made very good point here. With the above patch, we are almost missing the capability of registering dmtimer as a clocksource for OMAP. It will always use 32k-counter, and never fall back to dmtimer. Then the only options we have here is, 1) Register both the timers, 32k-counter and dmtimer for clocksource; let Kernel pick up best rating clocksource out of these two. In case of OMAP1/2/3/4, kernel will use dmtimer, since it has better Rating. User can choose the 32k-counter clocksource via bootargs. Impact: without bootargs for clocksource selection, kernel will choose dmtimer, impacting loss of time during suspend/resume. 2) Let the current code be as is, means, the clocksource registration will Happened based on #ifdef CONFIG_OMAP_32K_TIMER and this option selection will be Controlled by Kconfig rules. Based on this conclusion, 1st patch of this series needs some changes. But Irrespective of this, we still need 2 patches from this series, [PATCH 2/3] ARM: OMAP2+: hwmod data: Add 32k-sync timer data to hwmod database [PATCH 3/3] ARM: OMAP2/3: Add idle_st bits for ST_32KSYNC timer to prcm-common header Thanks, Vaibhav thanks, -- Ming Lei -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Mon, Mar 19, 2012 at 7:11 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: I think you made very good point here. With the above patch, we are almost missing the capability of registering dmtimer as a clocksource for OMAP. It will always use 32k-counter, and never fall back to dmtimer. Then the only options we have here is, 1) Register both the timers, 32k-counter and dmtimer for clocksource; let Kernel pick up best rating clocksource out of these two. In case of OMAP1/2/3/4, kernel will use dmtimer, since it has better Rating. User can choose the 32k-counter clocksource via bootargs. Impact: without bootargs for clocksource selection, kernel will choose dmtimer, impacting loss of time during suspend/resume. 2) Let the current code be as is, means, the clocksource registration will Happened based on #ifdef CONFIG_OMAP_32K_TIMER and this option selection will be Controlled by Kconfig rules. How about the 3rd option? 3), take the way in your patch 1) at default, but will switch to register dmtimer directly and bypass 32k-counter if user need it via kernel parameter. As far as I can think of, the situations required for dmtimer are high-frequency perf sample and high precision trace points, so looks it is OK to take 32k-counter at default. Thanks, -- Ming Lei -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Monday 19 March 2012 05:14 PM, Ming Lei wrote: On Mon, Mar 19, 2012 at 7:11 PM, Hiremath, Vaibhav hvaib...@ti.com wrote: I think you made very good point here. With the above patch, we are almost missing the capability of registering dmtimer as a clocksource for OMAP. It will always use 32k-counter, and never fall back to dmtimer. Then the only options we have here is, 1) Register both the timers, 32k-counter and dmtimer for clocksource; let Kernel pick up best rating clocksource out of these two. In case of OMAP1/2/3/4, kernel will use dmtimer, since it has better Rating. User can choose the 32k-counter clocksource via bootargs. Impact: without bootargs for clocksource selection, kernel will choose dmtimer, impacting loss of time during suspend/resume. This is the right option. The problem is gptimer clocksource doesn't work across power transitions and hence it is broken. Even for the perf, with PM enabled, dmtimer can't be used or it needs to be used with 32KHz clock which makes it no better than sync timer. So here keeping 32K sync timer is default clocksource makes sense since it is the only working and viable option. So what can be done is register both 32K and gptimer together but make gptimer clocksource registration depends on PM enabled. This should solve all the needs I guess. 2) Let the current code be as is, means, the clocksource registration will Happened based on #ifdef CONFIG_OMAP_32K_TIMER and this option selection will be Controlled by Kconfig rules. We should get rid off CONFIG_OMAP_32K_TIMER. Regards Santosh -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
Hi, On Tue, Jan 24, 2012 at 7:38 AM, Kevin Hilman khil...@ti.com wrote: Vaibhav Hiremath hvaib...@ti.com writes: OMAP device has 32k-sync timer which is currently used as a clocksource in the kernel (omap2plus_defconfig). The current implementation uses compile time selection between gp-timer and 32k-sync timer, which breaks multi-omap build for the devices like AM33xx, where 32k-sync timer is not available. So use hwmod database lookup mechanism, through which at run-time we can identify availability of 32k-sync timer on the device, else fall back to gp-timer. With the runtime detection fallback, I suggest also removing the Kconfig choice between the two as well. IMO, under some situations, gp timer clocksource has high precision and is very useful, so hope the Kconfig choice can be kept, or using kernel parameter way, and the patch need to be changed to support the selection. thanks, -- Ming Lei -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Fri, Mar 09, 2012 at 05:58:00PM +, Hiremath, Vaibhav wrote:
On Tue, Mar 06, 2012 at 04:25:30, Tony Lindgren wrote:
Hi,
* Vaibhav Hiremath hvaib...@ti.com [120119 06:01]:
OMAP device has 32k-sync timer which is currently used as a
clocksource in the kernel (omap2plus_defconfig).
The current implementation uses compile time selection between
gp-timer and 32k-sync timer, which breaks multi-omap build for
the devices like AM33xx, where 32k-sync timer is not available.
So use hwmod database lookup mechanism, through which at run-time
we can identify availability of 32k-sync timer on the device,
else fall back to gp-timer.
...
--- a/arch/arm/plat-omap/counter_32k.c
+++ b/arch/arm/plat-omap/counter_32k.c
@@ -69,52 +69,55 @@ void read_persistent_clock(struct timespec *ts)
int __init omap_init_clocksource_32k(void)
{
- static char err[] __initdata = KERN_ERR
- %s: can't register clocksource!\n;
-
- if (cpu_is_omap16xx() || cpu_class_is_omap2()) {
- u32 pbase;
- unsigned long size = SZ_4K;
- void __iomem *base;
- struct clk *sync_32k_ick;
-
- if (cpu_is_omap16xx()) {
- pbase = OMAP16XX_TIMER_32K_SYNCHRONIZED;
- size = SZ_1K;
- } else if (cpu_is_omap2420())
- pbase = OMAP2420_32KSYNCT_BASE + 0x10;
- else if (cpu_is_omap2430())
- pbase = OMAP2430_32KSYNCT_BASE + 0x10;
- else if (cpu_is_omap34xx())
- pbase = OMAP3430_32KSYNCT_BASE + 0x10;
- else if (cpu_is_omap44xx())
- pbase = OMAP4430_32KSYNCT_BASE + 0x10;
- else
+ u32 pbase;
+ unsigned long size = SZ_4K;
+ void __iomem *base;
+ struct clk *sync_32k_ick;
+
+ if (cpu_is_omap16xx()) {
+ pbase = OMAP16XX_TIMER_32K_SYNCHRONIZED;
+ size = SZ_1K;
+ } else if (cpu_class_is_omap2()) {
+ struct omap_hwmod *oh;
+ const char *oh_name = counter_32k;
+
+ oh = omap_hwmod_lookup(oh_name);
+ if (!oh || oh-slaves_cnt == 0) {
+ pr_err(Could not lookup %s hwmod\n, oh_name);
return -ENODEV;
+ }
+ pbase = oh-slaves[0]-addr-pa_start + 0x10;
+ } else {
+ return -ENODEV;
+ }
How about have separate omap1 and omap2+ init functions that
call a common function and passes the pbase as a parameter?
That way we can get rid of the cpu_is_omap tests here.
Tony,
In the morning, I replied very soon, without much thinking...
Just now I started working on the patch, I was just reviewing the code,
and I felt that, it is unnecessary to split the code between omap1 and
omap2+.
The reason is,
Currently Only OMAP16xx base-address is hardcoded with
cpu_is_omap16xx() macro, For all other omap family of devices the
complete information is fetched from HWDMO api's/data.
In that case, why don't you create the platform_device by hand on
arch/arm/mach-omap1/devices.c and move the omap2+ (which is based on
hwmod) to arch/arm/mach-omap2/devices.c ?
--
balbi
signature.asc
Description: Digital signature
RE: [PATCH 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Mon, Mar 12, 2012 at 15:09:24, Balbi, Felipe wrote:
On Fri, Mar 09, 2012 at 05:58:00PM +, Hiremath, Vaibhav wrote:
On Tue, Mar 06, 2012 at 04:25:30, Tony Lindgren wrote:
Hi,
* Vaibhav Hiremath hvaib...@ti.com [120119 06:01]:
OMAP device has 32k-sync timer which is currently used as a
clocksource in the kernel (omap2plus_defconfig).
The current implementation uses compile time selection between
gp-timer and 32k-sync timer, which breaks multi-omap build for
the devices like AM33xx, where 32k-sync timer is not available.
So use hwmod database lookup mechanism, through which at run-time
we can identify availability of 32k-sync timer on the device,
else fall back to gp-timer.
...
--- a/arch/arm/plat-omap/counter_32k.c
+++ b/arch/arm/plat-omap/counter_32k.c
@@ -69,52 +69,55 @@ void read_persistent_clock(struct timespec *ts)
int __init omap_init_clocksource_32k(void)
{
- static char err[] __initdata = KERN_ERR
- %s: can't register clocksource!\n;
-
- if (cpu_is_omap16xx() || cpu_class_is_omap2()) {
- u32 pbase;
- unsigned long size = SZ_4K;
- void __iomem *base;
- struct clk *sync_32k_ick;
-
- if (cpu_is_omap16xx()) {
- pbase = OMAP16XX_TIMER_32K_SYNCHRONIZED;
- size = SZ_1K;
- } else if (cpu_is_omap2420())
- pbase = OMAP2420_32KSYNCT_BASE + 0x10;
- else if (cpu_is_omap2430())
- pbase = OMAP2430_32KSYNCT_BASE + 0x10;
- else if (cpu_is_omap34xx())
- pbase = OMAP3430_32KSYNCT_BASE + 0x10;
- else if (cpu_is_omap44xx())
- pbase = OMAP4430_32KSYNCT_BASE + 0x10;
- else
+ u32 pbase;
+ unsigned long size = SZ_4K;
+ void __iomem *base;
+ struct clk *sync_32k_ick;
+
+ if (cpu_is_omap16xx()) {
+ pbase = OMAP16XX_TIMER_32K_SYNCHRONIZED;
+ size = SZ_1K;
+ } else if (cpu_class_is_omap2()) {
+ struct omap_hwmod *oh;
+ const char *oh_name = counter_32k;
+
+ oh = omap_hwmod_lookup(oh_name);
+ if (!oh || oh-slaves_cnt == 0) {
+ pr_err(Could not lookup %s hwmod\n, oh_name);
return -ENODEV;
+ }
+ pbase = oh-slaves[0]-addr-pa_start + 0x10;
+ } else {
+ return -ENODEV;
+ }
How about have separate omap1 and omap2+ init functions that
call a common function and passes the pbase as a parameter?
That way we can get rid of the cpu_is_omap tests here.
Tony,
In the morning, I replied very soon, without much thinking...
Just now I started working on the patch, I was just reviewing the code,
and I felt that, it is unnecessary to split the code between omap1 and
omap2+.
The reason is,
Currently Only OMAP16xx base-address is hardcoded with
cpu_is_omap16xx() macro, For all other omap family of devices the
complete information is fetched from HWDMO api's/data.
In that case, why don't you create the platform_device by hand on
arch/arm/mach-omap1/devices.c and move the omap2+ (which is based on
hwmod) to arch/arm/mach-omap2/devices.c ?
Balbi,
32k_counter code is not a platform_device/driver, we don't build the
any device here. We only need hwmod data to fetch the basic information like,
base-address, size, etc...
And I am note sure whether we really intend to make it
platform_device/driver thing.
Thanks,
Vaibhav
--
balbi
--
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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
Hi,
On Mon, Mar 12, 2012 at 09:48:20AM +, Hiremath, Vaibhav wrote:
On Mon, Mar 12, 2012 at 15:09:24, Balbi, Felipe wrote:
On Fri, Mar 09, 2012 at 05:58:00PM +, Hiremath, Vaibhav wrote:
On Tue, Mar 06, 2012 at 04:25:30, Tony Lindgren wrote:
Hi,
* Vaibhav Hiremath hvaib...@ti.com [120119 06:01]:
OMAP device has 32k-sync timer which is currently used as a
clocksource in the kernel (omap2plus_defconfig).
The current implementation uses compile time selection between
gp-timer and 32k-sync timer, which breaks multi-omap build for
the devices like AM33xx, where 32k-sync timer is not available.
So use hwmod database lookup mechanism, through which at run-time
we can identify availability of 32k-sync timer on the device,
else fall back to gp-timer.
...
--- a/arch/arm/plat-omap/counter_32k.c
+++ b/arch/arm/plat-omap/counter_32k.c
@@ -69,52 +69,55 @@ void read_persistent_clock(struct timespec *ts)
int __init omap_init_clocksource_32k(void)
{
- static char err[] __initdata = KERN_ERR
- %s: can't register clocksource!\n;
-
- if (cpu_is_omap16xx() || cpu_class_is_omap2()) {
- u32 pbase;
- unsigned long size = SZ_4K;
- void __iomem *base;
- struct clk *sync_32k_ick;
-
- if (cpu_is_omap16xx()) {
- pbase = OMAP16XX_TIMER_32K_SYNCHRONIZED;
- size = SZ_1K;
- } else if (cpu_is_omap2420())
- pbase = OMAP2420_32KSYNCT_BASE + 0x10;
- else if (cpu_is_omap2430())
- pbase = OMAP2430_32KSYNCT_BASE + 0x10;
- else if (cpu_is_omap34xx())
- pbase = OMAP3430_32KSYNCT_BASE + 0x10;
- else if (cpu_is_omap44xx())
- pbase = OMAP4430_32KSYNCT_BASE + 0x10;
- else
+ u32 pbase;
+ unsigned long size = SZ_4K;
+ void __iomem *base;
+ struct clk *sync_32k_ick;
+
+ if (cpu_is_omap16xx()) {
+ pbase = OMAP16XX_TIMER_32K_SYNCHRONIZED;
+ size = SZ_1K;
+ } else if (cpu_class_is_omap2()) {
+ struct omap_hwmod *oh;
+ const char *oh_name = counter_32k;
+
+ oh = omap_hwmod_lookup(oh_name);
+ if (!oh || oh-slaves_cnt == 0) {
+ pr_err(Could not lookup %s hwmod\n, oh_name);
return -ENODEV;
+ }
+ pbase = oh-slaves[0]-addr-pa_start + 0x10;
+ } else {
+ return -ENODEV;
+ }
How about have separate omap1 and omap2+ init functions that
call a common function and passes the pbase as a parameter?
That way we can get rid of the cpu_is_omap tests here.
Tony,
In the morning, I replied very soon, without much thinking...
Just now I started working on the patch, I was just reviewing the code,
and I felt that, it is unnecessary to split the code between omap1 and
omap2+.
The reason is,
Currently Only OMAP16xx base-address is hardcoded with
cpu_is_omap16xx() macro, For all other omap family of devices the
complete information is fetched from HWDMO api's/data.
In that case, why don't you create the platform_device by hand on
arch/arm/mach-omap1/devices.c and move the omap2+ (which is based on
hwmod) to arch/arm/mach-omap2/devices.c ?
Balbi,
32k_counter code is not a platform_device/driver, we don't build the
any device here. We only need hwmod data to fetch the basic information like,
base-address, size, etc...
And I am note sure whether we really intend to make it
platform_device/driver thing.
that's kinda weird... anyway, you can also pass base as argument to
omap_init_clocksource_32k()... dunno.
--
balbi
signature.asc
Description: Digital signature
RE: [PATCH 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Mon, Mar 12, 2012 at 15:47:58, Balbi, Felipe wrote:
Hi,
On Mon, Mar 12, 2012 at 09:48:20AM +, Hiremath, Vaibhav wrote:
On Mon, Mar 12, 2012 at 15:09:24, Balbi, Felipe wrote:
On Fri, Mar 09, 2012 at 05:58:00PM +, Hiremath, Vaibhav wrote:
On Tue, Mar 06, 2012 at 04:25:30, Tony Lindgren wrote:
Hi,
* Vaibhav Hiremath hvaib...@ti.com [120119 06:01]:
OMAP device has 32k-sync timer which is currently used as a
clocksource in the kernel (omap2plus_defconfig).
The current implementation uses compile time selection between
gp-timer and 32k-sync timer, which breaks multi-omap build for
the devices like AM33xx, where 32k-sync timer is not available.
So use hwmod database lookup mechanism, through which at run-time
we can identify availability of 32k-sync timer on the device,
else fall back to gp-timer.
...
--- a/arch/arm/plat-omap/counter_32k.c
+++ b/arch/arm/plat-omap/counter_32k.c
@@ -69,52 +69,55 @@ void read_persistent_clock(struct timespec *ts)
int __init omap_init_clocksource_32k(void)
{
- static char err[] __initdata = KERN_ERR
- %s: can't register clocksource!\n;
-
- if (cpu_is_omap16xx() || cpu_class_is_omap2()) {
- u32 pbase;
- unsigned long size = SZ_4K;
- void __iomem *base;
- struct clk *sync_32k_ick;
-
- if (cpu_is_omap16xx()) {
- pbase = OMAP16XX_TIMER_32K_SYNCHRONIZED;
- size = SZ_1K;
- } else if (cpu_is_omap2420())
- pbase = OMAP2420_32KSYNCT_BASE + 0x10;
- else if (cpu_is_omap2430())
- pbase = OMAP2430_32KSYNCT_BASE + 0x10;
- else if (cpu_is_omap34xx())
- pbase = OMAP3430_32KSYNCT_BASE + 0x10;
- else if (cpu_is_omap44xx())
- pbase = OMAP4430_32KSYNCT_BASE + 0x10;
- else
+ u32 pbase;
+ unsigned long size = SZ_4K;
+ void __iomem *base;
+ struct clk *sync_32k_ick;
+
+ if (cpu_is_omap16xx()) {
+ pbase = OMAP16XX_TIMER_32K_SYNCHRONIZED;
+ size = SZ_1K;
+ } else if (cpu_class_is_omap2()) {
+ struct omap_hwmod *oh;
+ const char *oh_name = counter_32k;
+
+ oh = omap_hwmod_lookup(oh_name);
+ if (!oh || oh-slaves_cnt == 0) {
+ pr_err(Could not lookup %s hwmod\n, oh_name);
return -ENODEV;
+ }
+ pbase = oh-slaves[0]-addr-pa_start + 0x10;
+ } else {
+ return -ENODEV;
+ }
How about have separate omap1 and omap2+ init functions that
call a common function and passes the pbase as a parameter?
That way we can get rid of the cpu_is_omap tests here.
Tony,
In the morning, I replied very soon, without much thinking...
Just now I started working on the patch, I was just reviewing the code,
and I felt that, it is unnecessary to split the code between omap1 and
omap2+.
The reason is,
Currently Only OMAP16xx base-address is hardcoded with
cpu_is_omap16xx() macro, For all other omap family of devices the
complete information is fetched from HWDMO api's/data.
In that case, why don't you create the platform_device by hand on
arch/arm/mach-omap1/devices.c and move the omap2+ (which is based on
hwmod) to arch/arm/mach-omap2/devices.c ?
Balbi,
32k_counter code is not a platform_device/driver, we don't build the
any device here. We only need hwmod data to fetch the basic information
like,
base-address, size, etc...
And I am note sure whether we really intend to make it
platform_device/driver thing.
that's kinda weird...
Balbi,
Its not that weird, I think its because, this code is used as system
clocksource.
anyway, you can also pass base as argument to
omap_init_clocksource_32k()... dunno.
That's the whole point, just for one platform (omap16xx), we are changing the
code. Rest all other platforms are using hwmod data, which is I believe
is clean interface as of now.
To be more specific, we have to choose between something like -
arch/arm/mach-omap1/timer32k.c
bool __init omap_32k_timer_init(void) {
omap_init_clocksource_32k(OMAP16XX_TIMER_32K_SYNCHRONIZED, SZ_1K);
...
}
arch/arm/mach-omap2/timer.c
static void __init omap2_gp_clocksource_init(int gptimer_id,
const char *fck_source)
{
/* Parse the hwmod data here*/
Base = oh-slaves[0]-addr-pa_start + 0x10;
Size = oh-slaves[0]-addr-pa_end - oh-slaves[0]-addr-pa_start;
res
RE: [PATCH 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Tue, Mar 06, 2012 at 04:25:30, Tony Lindgren wrote:
Hi,
* Vaibhav Hiremath hvaib...@ti.com [120119 06:01]:
OMAP device has 32k-sync timer which is currently used as a
clocksource in the kernel (omap2plus_defconfig).
The current implementation uses compile time selection between
gp-timer and 32k-sync timer, which breaks multi-omap build for
the devices like AM33xx, where 32k-sync timer is not available.
So use hwmod database lookup mechanism, through which at run-time
we can identify availability of 32k-sync timer on the device,
else fall back to gp-timer.
...
--- a/arch/arm/plat-omap/counter_32k.c
+++ b/arch/arm/plat-omap/counter_32k.c
@@ -69,52 +69,55 @@ void read_persistent_clock(struct timespec *ts)
int __init omap_init_clocksource_32k(void)
{
- static char err[] __initdata = KERN_ERR
- %s: can't register clocksource!\n;
-
- if (cpu_is_omap16xx() || cpu_class_is_omap2()) {
- u32 pbase;
- unsigned long size = SZ_4K;
- void __iomem *base;
- struct clk *sync_32k_ick;
-
- if (cpu_is_omap16xx()) {
- pbase = OMAP16XX_TIMER_32K_SYNCHRONIZED;
- size = SZ_1K;
- } else if (cpu_is_omap2420())
- pbase = OMAP2420_32KSYNCT_BASE + 0x10;
- else if (cpu_is_omap2430())
- pbase = OMAP2430_32KSYNCT_BASE + 0x10;
- else if (cpu_is_omap34xx())
- pbase = OMAP3430_32KSYNCT_BASE + 0x10;
- else if (cpu_is_omap44xx())
- pbase = OMAP4430_32KSYNCT_BASE + 0x10;
- else
+ u32 pbase;
+ unsigned long size = SZ_4K;
+ void __iomem *base;
+ struct clk *sync_32k_ick;
+
+ if (cpu_is_omap16xx()) {
+ pbase = OMAP16XX_TIMER_32K_SYNCHRONIZED;
+ size = SZ_1K;
+ } else if (cpu_class_is_omap2()) {
+ struct omap_hwmod *oh;
+ const char *oh_name = counter_32k;
+
+ oh = omap_hwmod_lookup(oh_name);
+ if (!oh || oh-slaves_cnt == 0) {
+ pr_err(Could not lookup %s hwmod\n, oh_name);
return -ENODEV;
+ }
+ pbase = oh-slaves[0]-addr-pa_start + 0x10;
+ } else {
+ return -ENODEV;
+ }
How about have separate omap1 and omap2+ init functions that
call a common function and passes the pbase as a parameter?
That way we can get rid of the cpu_is_omap tests here.
Tony,
In the morning, I replied very soon, without much thinking...
Just now I started working on the patch, I was just reviewing the code,
and I felt that, it is unnecessary to split the code between omap1 and
omap2+.
The reason is,
Currently Only OMAP16xx base-address is hardcoded with cpu_is_omap16xx()
macro, For all other omap family of devices the complete information is fetched
from HWDMO api's/data.
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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Tue, Mar 06, 2012 at 04:25:30, Tony Lindgren wrote:
Hi,
* Vaibhav Hiremath hvaib...@ti.com [120119 06:01]:
OMAP device has 32k-sync timer which is currently used as a
clocksource in the kernel (omap2plus_defconfig).
The current implementation uses compile time selection between
gp-timer and 32k-sync timer, which breaks multi-omap build for
the devices like AM33xx, where 32k-sync timer is not available.
So use hwmod database lookup mechanism, through which at run-time
we can identify availability of 32k-sync timer on the device,
else fall back to gp-timer.
...
--- a/arch/arm/plat-omap/counter_32k.c
+++ b/arch/arm/plat-omap/counter_32k.c
@@ -69,52 +69,55 @@ void read_persistent_clock(struct timespec *ts)
int __init omap_init_clocksource_32k(void)
{
- static char err[] __initdata = KERN_ERR
- %s: can't register clocksource!\n;
-
- if (cpu_is_omap16xx() || cpu_class_is_omap2()) {
- u32 pbase;
- unsigned long size = SZ_4K;
- void __iomem *base;
- struct clk *sync_32k_ick;
-
- if (cpu_is_omap16xx()) {
- pbase = OMAP16XX_TIMER_32K_SYNCHRONIZED;
- size = SZ_1K;
- } else if (cpu_is_omap2420())
- pbase = OMAP2420_32KSYNCT_BASE + 0x10;
- else if (cpu_is_omap2430())
- pbase = OMAP2430_32KSYNCT_BASE + 0x10;
- else if (cpu_is_omap34xx())
- pbase = OMAP3430_32KSYNCT_BASE + 0x10;
- else if (cpu_is_omap44xx())
- pbase = OMAP4430_32KSYNCT_BASE + 0x10;
- else
+ u32 pbase;
+ unsigned long size = SZ_4K;
+ void __iomem *base;
+ struct clk *sync_32k_ick;
+
+ if (cpu_is_omap16xx()) {
+ pbase = OMAP16XX_TIMER_32K_SYNCHRONIZED;
+ size = SZ_1K;
+ } else if (cpu_class_is_omap2()) {
+ struct omap_hwmod *oh;
+ const char *oh_name = counter_32k;
+
+ oh = omap_hwmod_lookup(oh_name);
+ if (!oh || oh-slaves_cnt == 0) {
+ pr_err(Could not lookup %s hwmod\n, oh_name);
return -ENODEV;
+ }
+ pbase = oh-slaves[0]-addr-pa_start + 0x10;
+ } else {
+ return -ENODEV;
+ }
How about have separate omap1 and omap2+ init functions that
call a common function and passes the pbase as a parameter?
That way we can get rid of the cpu_is_omap tests here.
Ok, let me clean this code accordingly and submit the next version.
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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
Hi,
* Vaibhav Hiremath hvaib...@ti.com [120119 06:01]:
OMAP device has 32k-sync timer which is currently used as a
clocksource in the kernel (omap2plus_defconfig).
The current implementation uses compile time selection between
gp-timer and 32k-sync timer, which breaks multi-omap build for
the devices like AM33xx, where 32k-sync timer is not available.
So use hwmod database lookup mechanism, through which at run-time
we can identify availability of 32k-sync timer on the device,
else fall back to gp-timer.
...
--- a/arch/arm/plat-omap/counter_32k.c
+++ b/arch/arm/plat-omap/counter_32k.c
@@ -69,52 +69,55 @@ void read_persistent_clock(struct timespec *ts)
int __init omap_init_clocksource_32k(void)
{
- static char err[] __initdata = KERN_ERR
- %s: can't register clocksource!\n;
-
- if (cpu_is_omap16xx() || cpu_class_is_omap2()) {
- u32 pbase;
- unsigned long size = SZ_4K;
- void __iomem *base;
- struct clk *sync_32k_ick;
-
- if (cpu_is_omap16xx()) {
- pbase = OMAP16XX_TIMER_32K_SYNCHRONIZED;
- size = SZ_1K;
- } else if (cpu_is_omap2420())
- pbase = OMAP2420_32KSYNCT_BASE + 0x10;
- else if (cpu_is_omap2430())
- pbase = OMAP2430_32KSYNCT_BASE + 0x10;
- else if (cpu_is_omap34xx())
- pbase = OMAP3430_32KSYNCT_BASE + 0x10;
- else if (cpu_is_omap44xx())
- pbase = OMAP4430_32KSYNCT_BASE + 0x10;
- else
+ u32 pbase;
+ unsigned long size = SZ_4K;
+ void __iomem *base;
+ struct clk *sync_32k_ick;
+
+ if (cpu_is_omap16xx()) {
+ pbase = OMAP16XX_TIMER_32K_SYNCHRONIZED;
+ size = SZ_1K;
+ } else if (cpu_class_is_omap2()) {
+ struct omap_hwmod *oh;
+ const char *oh_name = counter_32k;
+
+ oh = omap_hwmod_lookup(oh_name);
+ if (!oh || oh-slaves_cnt == 0) {
+ pr_err(Could not lookup %s hwmod\n, oh_name);
return -ENODEV;
+ }
+ pbase = oh-slaves[0]-addr-pa_start + 0x10;
+ } else {
+ return -ENODEV;
+ }
How about have separate omap1 and omap2+ init functions that
call a common function and passes the pbase as a parameter?
That way we can get rid of the cpu_is_omap tests here.
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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Tue, Jan 24, 2012 at 23:17:56, Hilman, Kevin wrote: Hiremath, Vaibhav hvaib...@ti.com writes: On Tue, Jan 24, 2012 at 05:08:50, Hilman, Kevin wrote: Vaibhav Hiremath hvaib...@ti.com writes: OMAP device has 32k-sync timer which is currently used as a clocksource in the kernel (omap2plus_defconfig). The current implementation uses compile time selection between gp-timer and 32k-sync timer, which breaks multi-omap build for the devices like AM33xx, where 32k-sync timer is not available. So use hwmod database lookup mechanism, through which at run-time we can identify availability of 32k-sync timer on the device, else fall back to gp-timer. With the runtime detection fallback, I suggest also removing the Kconfig choice between the two as well. Kevin, Actually I thought of cleaning whole 32KTIMER related code, but then realized that, it would be major cleanup, since it is being used in lot of places. So let's not block this patch-series for this further cleanup. I will work on cleaning up the complete 32KTIMER macro and submit the patches. OK, Tony can make the call. Tony, Can this also get merged? Thanks, Vaibhav Kevin -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
On Tue, Jan 24, 2012 at 05:08:50, Hilman, Kevin wrote: Vaibhav Hiremath hvaib...@ti.com writes: OMAP device has 32k-sync timer which is currently used as a clocksource in the kernel (omap2plus_defconfig). The current implementation uses compile time selection between gp-timer and 32k-sync timer, which breaks multi-omap build for the devices like AM33xx, where 32k-sync timer is not available. So use hwmod database lookup mechanism, through which at run-time we can identify availability of 32k-sync timer on the device, else fall back to gp-timer. With the runtime detection fallback, I suggest also removing the Kconfig choice between the two as well. Kevin, Actually I thought of cleaning whole 32KTIMER related code, but then realized that, it would be major cleanup, since it is being used in lot of places. So let's not block this patch-series for this further cleanup. I will work on cleaning up the complete 32KTIMER macro and submit the patches. Simple grep command gives - === psplinux060:/datalocal/omap-kernelgrep -r OMAP_32K_TIMER * arch/arm/mach-omap1/Makefile:obj-$(CONFIG_OMAP_32K_TIMER) += timer32k.o arch/arm/mach-omap1/time.c:#ifdef CONFIG_OMAP_32K_TIMER arch/arm/mach-omap1/pm.c:#ifdef CONFIG_OMAP_32K_TIMER arch/arm/mach-omap1/pm.c:#ifdef CONFIG_OMAP_32K_TIMER arch/arm/mach-omap1/pm.c:#ifdef CONFIG_OMAP_32K_TIMER arch/arm/mach-omap1/timer32k.c:#define OMAP_32K_TIMER_TICK_PERIOD ((OMAP_32K_TICKS_PER_SEC / HZ) - 1) arch/arm/mach-omap1/timer32k.c: omap_32k_timer_start(OMAP_32K_TIMER_TICK_PERIOD); arch/arm/mach-omap2/timer.c:#ifdef CONFIG_OMAP_32K_TIMER arch/arm/configs/omap1_defconfig:CONFIG_OMAP_32K_TIMER=y arch/arm/plat-omap/include/plat/timex.h:#ifdef CONFIG_OMAP_32K_TIMER arch/arm/plat-omap/include/plat/timex.h:#define CLOCK_TICK_RATE (CONFIG_OMAP_32K_TIMER_HZ) arch/arm/plat-omap/include/plat/param.h:#ifdef CONFIG_OMAP_32K_TIMER_HZ arch/arm/plat-omap/include/plat/param.h:#define HZ CONFIG_OMAP_32K_TIMER_HZ arch/arm/plat-omap/Kconfig:config OMAP_32K_TIMER arch/arm/plat-omap/Kconfig:config OMAP_32K_TIMER_HZ arch/arm/plat-omap/Kconfig: depends on OMAP_32K_TIMER arch/arm/Kconfig: default OMAP_32K_TIMER_HZ if ARCH_OMAP OMAP_32K_TIMER Thanks, Vaibhav Kevin -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
Hiremath, Vaibhav hvaib...@ti.com writes: On Tue, Jan 24, 2012 at 05:08:50, Hilman, Kevin wrote: Vaibhav Hiremath hvaib...@ti.com writes: OMAP device has 32k-sync timer which is currently used as a clocksource in the kernel (omap2plus_defconfig). The current implementation uses compile time selection between gp-timer and 32k-sync timer, which breaks multi-omap build for the devices like AM33xx, where 32k-sync timer is not available. So use hwmod database lookup mechanism, through which at run-time we can identify availability of 32k-sync timer on the device, else fall back to gp-timer. With the runtime detection fallback, I suggest also removing the Kconfig choice between the two as well. Kevin, Actually I thought of cleaning whole 32KTIMER related code, but then realized that, it would be major cleanup, since it is being used in lot of places. So let's not block this patch-series for this further cleanup. I will work on cleaning up the complete 32KTIMER macro and submit the patches. OK, Tony can make the call. Kevin -- 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 1/3] ARM: OMAP2+: 32k-counter: Use hwmod lookup to check presence of 32k timer
Vaibhav Hiremath hvaib...@ti.com writes: OMAP device has 32k-sync timer which is currently used as a clocksource in the kernel (omap2plus_defconfig). The current implementation uses compile time selection between gp-timer and 32k-sync timer, which breaks multi-omap build for the devices like AM33xx, where 32k-sync timer is not available. So use hwmod database lookup mechanism, through which at run-time we can identify availability of 32k-sync timer on the device, else fall back to gp-timer. With the runtime detection fallback, I suggest also removing the Kconfig choice between the two as well. Kevin -- 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
