On Thu, 2013-09-26 at 22:34 -0500, Wang Dongsheng-B40534 wrote:

quoted

-----Original Message-----
From: Wood Scott-B07421
Sent: Friday, September 27, 2013 5:37 AM
To: Wang Dongsheng-B40534
Cc: Bhushan Bharat-R65777; Wood Scott-B07421; linuxppc-
dev@lists.ozlabs.org
Subject: Re: [PATCH v4 4/4] powerpc/85xx: add sysfs for pw20 state and
altivec idle

On Thu, 2013-09-26 at 01:18 -0500, Wang Dongsheng-B40534 wrote:

quoted

quoted

-----Original Message-----
From: Bhushan Bharat-R65777
Sent: Thursday, September 26, 2013 12:23 PM
To: Wang Dongsheng-B40534; Wood Scott-B07421
Cc: linuxppc-dev@lists.ozlabs.org
Subject: RE: [PATCH v4 4/4] powerpc/85xx: add sysfs for pw20 state
and altivec idle

quoted

-----Original Message-----
From: Wang Dongsheng-B40534
Sent: Thursday, September 26, 2013 8:02 AM
To: Wood Scott-B07421
Cc: Bhushan Bharat-R65777; linuxppc-dev@lists.ozlabs.org
Subject: RE: [PATCH v4 4/4] powerpc/85xx: add sysfs for pw20 state
and altivec idle

quoted

-----Original Message-----
From: Wood Scott-B07421
Sent: Thursday, September 26, 2013 1:57 AM
To: Wang Dongsheng-B40534
Cc: Bhushan Bharat-R65777; Wood Scott-B07421; linuxppc-
dev@lists.ozlabs.org
Subject: Re: [PATCH v4 4/4] powerpc/85xx: add sysfs for pw20
state and altivec idle

On Wed, 2013-09-25 at 03:10 -0500, Wang Dongsheng-B40534 wrote:

quoted

quoted

-----Original Message-----
From: Bhushan Bharat-R65777
Sent: Wednesday, September 25, 2013 2:23 PM
To: Wang Dongsheng-B40534; Wood Scott-B07421
Cc: linuxppc-dev@lists.ozlabs.org; Wang Dongsheng-B40534
Subject: RE: [PATCH v4 4/4] powerpc/85xx: add sysfs for pw20
state and altivec idle

quoted

-----Original Message-----
From: Linuxppc-dev [mailto:linuxppc-dev-
bounces+bharat.bhushan=freescale.com@lists.ozlabs.org] On
bounces+Behalf Of Dongsheng
Wang
Sent: Tuesday, September 24, 2013 2:59 PM
To: Wood Scott-B07421
Cc: linuxppc-dev@lists.ozlabs.org; Wang Dongsheng-B40534
Subject: [PATCH v4 4/4] powerpc/85xx: add sysfs for pw20
state and altivec idle

From: Wang Dongsheng <redacted>

Add a sys interface to enable/diable pw20 state or altivec
idle, and control the wait entry time.

Enable/Disable interface:
0, disable. 1, enable.
/sys/devices/system/cpu/cpuX/pw20_state
/sys/devices/system/cpu/cpuX/altivec_idle

Set wait time interface:(Nanosecond)
/sys/devices/system/cpu/cpuX/pw20_wait_time
/sys/devices/system/cpu/cpuX/altivec_idle_wait_time
Example: Base on TBfreq is 41MHZ.
1~47(ns): TB[63]
48~95(ns): TB[62]
96~191(ns): TB[61]
192~383(ns): TB[62]
384~767(ns): TB[60]
...

Signed-off-by: Wang Dongsheng
[off-list ref]
---
*v4:
Move code from 85xx/common.c to kernel/sysfs.c.

Remove has_pw20_altivec_idle function.

Change wait "entry_bit" to wait time.

 arch/powerpc/kernel/sysfs.c | 291
++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 291 insertions(+)

diff --git a/arch/powerpc/kernel/sysfs.c

b/arch/powerpc/kernel/sysfs.c index 27a90b9..23fece6
100644

--- a/arch/powerpc/kernel/sysfs.c
+++ b/arch/powerpc/kernel/sysfs.c

@@ -85,6 +85,279 @@ __setup("smt-snooze-delay=",

setup_smt_snooze_delay);

 #endif /* CONFIG_PPC64 */

+#ifdef CONFIG_FSL_SOC
+#define MAX_BIT		63
+
+static u64 pw20_wt;
+static u64 altivec_idle_wt;
+
+static unsigned int get_idle_ticks_bit(u64 ns) {
+	u64 cycle;
+
+	cycle = div_u64(ns, 1000 / tb_ticks_per_usec);

When tb_ticks_per_usec  > 1000 (timebase frequency > 1GHz)
then this will always be ns, which is not correct, no?

Actually it'll be a divide by zero in that case.

tb_ticks_per_usec = ppc_tb_freq / 1000000; Means TB freq should be
more than 1MHZ.

if ppc_tb_freq less than 1000000, the tb_ticks_per_usec will be a
divide by zero.
If this condition is established, I think kernel cannot work as a

normal.

quoted

So I think we need to believe that the variable is not zero.

We do believe it is non-zero but greater than 1000 :)

quoted

And I think TB freq
should not less than 1MHZ on PPC platform, because if TB freq less
than 1MHZ, the precision time will become very poor and system
response time will be slower.

Not sure what you are describing here related to divide by zero we
are mentioning.
You are talking about if tb_ticks_per_usec is ZERO and we are
talking about if (1000/tb_ticks_per_usec) will be zero.

BTW, div_u64() handle the case where divider is zero.

How?  When I checked yesterday it looked like div_u64() mapped to a
hardware division on 64-bit targets.  In any case it's not good to rely
on such behavior.

quoted

cycle = div_u64(ns, 1000 / tb_ticks_per_usec); For this, I think we
were discussing the two issues:

1. Scott talking about, when the tb_ticks_per_usec is a zero.

No I wasn't.  I was talking about when tb_ticks_per_usec > 1000, and thus
"1000 / tb_ticks_per_usec" is zero.  You said that the result would be ns
in that case.

quoted

2. You are talking about 1000/tb_ticks_per_usec is a zero.
This situation is about TB freq > 1GHZ.

I will fix this issue. Like I said before, "If timebase frequency >
1GHz, this should be "tb_ticks_per_usec / 1000" and to get

tb_ticks_per_nsec.

quoted

This should be changed to "cycle = ns * tb_ticks_per_nsec;""

#define TB_FREQ_1GHZ	1000

If (tb_ticks_per_usec > TB_FREQ_1GHZ)
	cycle = ns * (tb_ticks_per_usec / 1000); else
	cycle = div_u64(ns, 1000 / tb_ticks_per_usec);

how about this? :)

I suggested an alternative to satisfy your complaint that it's hard to
test one of those if/else branches.

Plus, your version will be quite inaccurate if (e.g.) tb_ticks_per_usec
is 501, or 1999.

cycle = div_u64(ns * tb_ticks_per_usec, 1000); It's look good.
But why we need:
	if (ns >= 10000)
		cycle = ((ns + 500) / 1000) * tb_ticks_per_usec;
?

I think "cycle = div_u64(ns * tb_ticks_per_usec, 1000)" is good enough. :)

It's to deal with overflow if a very large value of ns is provided
(and/or tb_ticks_per_usec is larger than we expect).

-Scott