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

On 03/20/2013 01:45:03 AM, Wang Dongsheng-B40534 wrote:

=20
=20

quoted

-----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; =20

linuxppc-dev@lists.ozlabs.org;

quoted

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:

quoted

quoted

quoted

+static void convert_ticks_to_time(struct timer_group_priv =20

*priv,

quoted

quoted

quoted

quoted

+		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, =20

&rem_us);

quoted

quoted

quoted

quoted

+		tmp_sec =3D (u64)time->tv_sec * =20

(u64)priv->timerfreq;

quoted

quoted

quoted

quoted

+		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

quoted

quoted

/ div);
+	tmp_sec =3D div_u64((u64)time->tv_sec * =20

(u64)priv->timerfreq,

quoted

quoted

quoted

quoted

div);
+
+	time->tv_usec =3D (__kernel_suseconds_t)
+		div_u64((ticks - tmp_sec) * 1000000, =20

priv->timerfreq /

quoted

quoted

quoted

quoted

div);
+
+	return;

Why don't you just adjust the clock frequency up front for

CLKDIV_64,

quoted

rather than introduce alternate (and untested!) code paths

throughout the

quoted

driver?

No, It cannot be integrated. The div cannot be removed.
Because if do priv->timerfreq /=3D div, that will affect the =20

accuracy.

quoted

quoted

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 =20

clock

quoted

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 =20
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. =20
This is just conversion ticks.
These calculated ticks will be set to the hardware.
=20

quoted

How you round is a different question.  You should probably be =20

rounding

quoted

up always, based on the final clock frequency -- though it's =20

unlikely to

quoted

matter much given the high precision of the timer relative to the =20

input

quoted

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 =20
more
accurate, more close to real time.
If echo 40 seconds may be difference is not obvious. But echo =20
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 =20
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...

-Scott=