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

On 03/27/2013 09:29:26 PM, Wang Dongsheng-B40534 wrote:

=20
=20

quoted

-----Original Message-----
From: Wood Scott-B07421
Sent: Thursday, March 28, 2013 1:12 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/26/2013 10:23:38 PM, Wang Dongsheng-B40534 wrote:

quoted

quoted

-----Original Message-----
From: Wood Scott-B07421
Sent: Wednesday, March 27, 2013 1:32 AM
To: Wang Dongsheng-B40534
Cc: Wood Scott-B07421; Gala Kumar-B11780;

linuxppc-dev@lists.ozlabs.org;

quoted

Li Yang-R58472
Subject: Re: [PATCH 2/3] powerpc/mpic: add global timer support

On 03/25/2013 10:29:58 PM, Wang Dongsheng-B40534 wrote:

quoted

quoted

-----Original Message-----
From: Wood Scott-B07421
Sent: Saturday, March 23, 2013 6:30 AM
To: Wang Dongsheng-B40534
Cc: Wood Scott-B07421; Gala Kumar-B11780;

linuxppc-dev@lists.ozlabs.org;

quoted

Li Yang-R58472
Subject: Re: [PATCH 2/3] powerpc/mpic: add global timer =20

support

quoted

quoted

quoted

quoted

quoted

On 03/22/2013 01:14:51 AM, Wang Dongsheng-B40534 wrote:

quoted

quoted

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

quoted

Li Yang-R58472
Subject: Re: [PATCH 2/3] powerpc/mpic: add global timer

support

quoted

quoted

quoted

quoted

quoted

BTW, the input clock frequency has been similarly =20

scaled,

quoted

quoted

yet

quoted

quoted

you

quoted

quoted

don't

quoted

try to scrounge up that information to get further

precision...

quoted

quoted

quoted

quoted

quoted

Let's go back patch, do you think the code is repeated?
I will remove "if (!(priv->flags & FSL_GLOBAL_TIMER))" =20

branch,

quoted

quoted

quoted

quoted

there

quoted

quoted

will be no redundant code.

I'd rather that branch be kept and the more complicated =20

branch

quoted

quoted

quoted

quoted

deleted,

quoted

and priv->timerfreq frequency be adjusted on initialization =20

to

quoted

quoted

quoted

quoted

account

quoted

for the scaler.

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;

        time->tv_sec =3D (__kernel_time_t)div_u64(ticks,
priv->timerfreq);
        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);

quoted

quoted

        return;
}

timer_group_get_freq() {
	...
	if (priv->flags & FSL_GLOBAL_TIMER) {
		div =3D (1 << (MPIC_TIMER_TCR_CLKDIV_64 >> 8)) * =20

8;

quoted

quoted

quoted

quoted

		priv->timerfreq /=3D div;
	}
	...
}
Do you want to do that?

	if (priv->flags & FSL_GLOBAL_TIMER)
		priv->timerfreq /=3D 64;

...but otherwise yes.

Ok, I would like do this.

if (priv->flags & FSL_GLOBAL_TIMER) {
	div =3D (1 << (MPIC_TIMER_TCR_CLKDIV_64 >> 8)) * 8;
	priv->timerfreq /=3D div;

Why?  What do you get out of that obfuscation?

Change MPIC_TIMER_TCR_CLKDIV_64 to MPIC_TIMER_TCR_CLKDIV

OK, that would at least provide the ability to adjust the clock divider =20
in one place rather than two -- though I don't know why we'd ever need =20
to change the divider.

Because macro is friendly, and other functions also used the macro.

Using the macro rather than hardcoding register bit encodings is =20
friendly.  The calculation to turn the bit encoding into an actual =20
divider value is not particularly friendly.

-Scott=