Re: [PATCH 2/3] powerpc/mpic: add global timer support

[Scott Wood <scottwood@freescale.com>]

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 =3D (__kernel_time_t)
> > > +			div_u64_rem(ticks, priv->timerfreq, &rem_us);
> > > +		tmp_sec =3D (u64)time->tv_sec * (u64)priv->timerfreq;
> > > +		time->tv_usec =3D (__kernel_suseconds_t)
> > > +			div_u64((ticks - tmp_sec) * 1000000,
> > > priv->timerfreq);
> > > +
> > > +		return;
> > > +	}
> > > +
> > > +	div =3D (1 << (MPIC_TIMER_TCR_CLKDIV_64 >> 8)) * 8;
> > > +
> > > +	time->tv_sec =3D (__kernel_time_t)div_u64(ticks, priv->timerfreq
> > > / div);
> > > +	tmp_sec =3D div_u64((u64)time->tv_sec * (u64)priv->timerfreq,
> > > div);
> > > +
> > > +	time->tv_usec =3D (__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 =20
> CLKDIV_64,
> > rather than introduce alternate (and untested!) code paths =20
> throughout the
> > driver?
> >
> No, It cannot be integrated. The div cannot be removed.
> Because if do priv->timerfreq /=3D div, that will affect the accuracy.
>=20
> Like:
> 3 * 5 / 2 =3D 7;
> 3 / 2 * 5 =3D 5;

I don't follow -- a change in the clock speed is a change in the clock =20
speed, no matter how you accomplish it.

How you round is a different question.  You should probably be rounding =20
up always, based on the final clock frequency -- though it's unlikely =20
to matter much given the high precision of the timer relative to the =20
input granularity.

> BTW
> if (!(priv->flags & FSL_GLOBAL_TIMER)) {
> 	time->tv_sec =3D (__kernel_time_t)
> 		div_u64_rem(ticks, priv->timerfreq, &rem_us);
> 	tmp_sec =3D (u64)time->tv_sec * (u64)priv->timerfreq;
> 	time->tv_usec =3D (__kernel_suseconds_t)
> 		div_u64((ticks - tmp_sec) * 1000000, priv->timerfreq);
>=20
> 	return;
> }
> This branch I has been tested.
>=20
> Test methods:
> 1. Get timerfreq and set timerfreq.
>    timerfreq /=3D 64;(Clock ratio is divide by 64)
>=20
> 2. Clear FSL_GLOBAL_TIMER flag.

Even if it was tested once, it's unlikely to continue to be tested =20
without a user.

-Scott=