Re: [PATCH 6/8] [POWERPC] sysdev,qe_lib: implement FSL GTM support

[Anton Vorontsov <avorontsov@ru.mvista.com>]

On Thu, Apr 17, 2008 at 09:19:03AM -0500, Scott Wood wrote:
> On Thu, Apr 17, 2008 at 04:52:35PM +0400, Anton Vorontsov wrote:
> > Heh. Scott, think about it. You have single 16bit timer with variable
> > frequency. To use it, you'd better know what exactly precision you need.
> 
> Why?  I know the timeout I need.
> 
> > Then you limited to u16 for the interval for this chosen precision.
> > 
> > Yes, you can implement this:
> > 
> > #define MAX_PRESCALER (256 * 256 * 16)
> > 
> > int gtm_reset_weird_behaving_utimer16(struct gtm_timer *tmr,
> > 				      unsigned long long usec,
> > 				      bool free_run)
> > {
> > 	int freq = 1000000;
> > 	int min_hz2 = (tmr->gtm->freq / MAX_PRESCALER) << 1;
> > 
> > 	while (!(freq & 1) && !(usec & 1) && freq >= min_hz2) {
> > 		freq >>= 1;
> > 		usec >>= 1;
> > 	}
> > 
> > 	if (usec > 0xffff)
> > 		return -EINVAL;
> > 
> > 	return gtm_reset_ref_timer16(tmr, freq, (u16)usec, free_run);
> > }
> 
> Try something like this:
> 
> int gtm_reset_sane_behaving_timer(struct gtm_timer *tmr,
>                                   u64 usec, bool free_run)
> {
> 	int freq = 1000000;
> 	int min_hz2 = (tmr->gtm->freq / MAX_PRESCALER) << 1;
> 
> 	while (usec > 0xffff && freq >= min_hz2) {

This isn't a timer with usec precision! This is a timer that silently
crops precision as it wants to. Ahh, I see you dropped "u" prefix.

Well. I'm not going to use it anyway, so just give it some name you
prefer and I'll wrap it into the patch. Preferably, drop a line here with
kerneldoc for it, so I'll not have to document its drawbacks. :-)

> 		freq >>= 1;
> 		usec >>= 1;
> 	}
> 
> 	if (usec > 0xffff)
> 		return -EINVAL;
> 
> 	return gtm_reset_ref_timer16(tmr, freq, usec, free_run);
> }
> 
> It could be made faster using cntlzw.

No need to cntlzw, there is fls() already. Though, here you'll need
two because of u64.

Btw, I hope you aware that single GTM timer running at 166MHz will give you
6 minutes of sleep, maximum. With cascaded timer you'll get much better
result of 310 days. Is that possible to use cascaded timer as a wakeup
event on 8313? If so, I'd suggest you to implement cascading firstly.

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2