> -----Original Message----- > From: Wood Scott-B07421 > Sent: Thursday, March 21, 2013 7:00 AM > To: Wang Dongsheng-B40534 > Cc: Wood Scott-B07421; Gala Kumar-B11780; linuxppc-dev@lists.ozlabs.org; > Li Yang-R58472 > Subject: Re: [PATCH 2/3] powerpc/mpic: add global timer support > > On 03/20/2013 01:45:03 AM, Wang Dongsheng-B40534 wrote: > > > > > > > -----Original Message----- > > > From: Wood Scott-B07421 > > > Sent: Wednesday, March 20, 2013 6:59 AM > > > To: Wang Dongsheng-B40534 > > > Cc: Wood Scott-B07421; Gala Kumar-B11780; > > linuxppc-dev@lists.ozlabs.org; > > > Li Yang-R58472 > > > Subject: Re: [PATCH 2/3] powerpc/mpic: add global timer support > > > > > > On 03/19/2013 02:55:58 AM, Wang Dongsheng-B40534 wrote: > > > > > > +static void convert_ticks_to_time(struct timer_group_priv > > *priv, > > > > > > + const u64 ticks, struct timeval *time) { > > > > > > + u64 tmp_sec; > > > > > > + u32 rem_us; > > > > > > + u32 div; > > > > > > + > > > > > > + if (!(priv->flags & FSL_GLOBAL_TIMER)) { > > > > > > + time->tv_sec = (__kernel_time_t) > > > > > > + div_u64_rem(ticks, priv->timerfreq, > > &rem_us); > > > > > > + tmp_sec = (u64)time->tv_sec * > > (u64)priv->timerfreq; > > > > > > + time->tv_usec = (__kernel_suseconds_t) > > > > > > + div_u64((ticks - tmp_sec) * 1000000, > > > > > > priv->timerfreq); > > > > > > + > > > > > > + return; > > > > > > + } > > > > > > + > > > > > > + div = (1 << (MPIC_TIMER_TCR_CLKDIV_64 >> 8)) * 8; > > > > > > + > > > > > > + time->tv_sec = (__kernel_time_t)div_u64(ticks, priv- > > > >timerfreq > > > > > > / div); > > > > > > + tmp_sec = div_u64((u64)time->tv_sec * > > (u64)priv->timerfreq, > > > > > > div); > > > > > > + > > > > > > + time->tv_usec = (__kernel_suseconds_t) > > > > > > + div_u64((ticks - tmp_sec) * 1000000, > > priv->timerfreq / > > > > > > div); > > > > > > + > > > > > > + return; > > > > > > > > > > Why don't you just adjust the clock frequency up front for > > > > CLKDIV_64, > > > > > rather than introduce alternate (and untested!) code paths > > > > throughout the > > > > > driver? > > > > > > > > > No, It cannot be integrated. The div cannot be removed. > > > > Because if do priv->timerfreq /= div, that will affect the > > accuracy. > > > > > > > > Like: > > > > 3 * 5 / 2 = 7; > > > > 3 / 2 * 5 = 5; > > > > > > I don't follow -- a change in the clock speed is a change in the > > clock > > > speed, no matter how you accomplish it. > > > > > This is not change hardware clock frequency. > > Citation needed. It looks like a change in the timer frequency to me: > > Clock ratio. Specifies the ratio of the timer frequency to the MPIC > input clock (platform clock/2) . The > following clock ratios are supported: > 00 > 01 > 10 > 11 > Default. Divide by 8 > Divide by 16 > Divide by 32 > Divide by 64 > > The end result is that the counter in the timer register changes only > 1/64 as often as the input clock. There's nothing special about that, > compared to having an input clock that is 1/64 the speed. > > > The mpic timer hardware clock is not be changed after initialization. > > This is just conversion ticks. > > These calculated ticks will be set to the hardware. > > > > > How you round is a different question. You should probably be > > rounding > > > up always, based on the final clock frequency -- though it's > > unlikely to > > > matter much given the high precision of the timer relative to the > > input > > > granularity. > > > > > Each ticks are based on the mpic timer hardware clock frequency. > > The conversion and calculation are in order to make the tick value is > > more > > accurate, more close to real time. > > If echo 40 seconds may be difference is not obvious. But echo > > 315360000(10 years) > > difference is obvious. > > So basically you're taking advantage of the fact that you have what > appears to be a more precise value of the frequency than is expressible > in integer Hz -- but I think that's false precision; odds are the > frequency is not accurate to 1 Hz to begin with. Even if it is, I doubt > it's worth worrying about. The error as a percentage will still be very > small with an input frequency of many MHz. Does an error of a few > minutes really matter if you're delaying for 10 years? That's > acceptable > clock drift for something not synced to network time. The main thing is > to ensure that you round up, not down, so that software doesn't see an > early wakeup as measured by its own timers. > > BTW, the input clock frequency has been similarly scaled, yet you don't > try to scrounge up that information to get further precision... > Let's go back patch, do you think the code is repeated? I will remove "if (!(priv->flags & FSL_GLOBAL_TIMER))" branch, there will be no redundant code.
_______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev