On Thu, 2013-10-17 at 21:36 -0500, Wang Dongsheng-B40534 wrote:

quoted

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

On Thu, 2013-10-17 at 00:51 -0500, Wang Dongsheng-B40534 wrote:

quoted

quoted

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

quoted

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

quoted

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

quoted

-----Original Message-----
From: Wang Dongsheng-B40534
Sent: Tuesday, October 15, 2013 2:51 PM
To: Wood Scott-B07421
Cc: Bhushan Bharat-R65777; linuxppc-dev@lists.ozlabs.org; Wang

Dongsheng-B40534

quoted

Subject: [PATCH v5 4/4] powerpc/85xx: add sysfs for pw20 state
and

altivec idle

quoted

+static ssize_t show_pw20_wait_time(struct device *dev,
+				struct device_attribute *attr, char *buf) {
+	u32 value;
+	u64 tb_cycle;
+	s64 time;
+
+	unsigned int cpu = dev->id;
+
+	if (!pw20_wt) {
+		smp_call_function_single(cpu, do_show_pwrmgtcr0, &value,

1);

quoted

quoted

quoted

+		value = (value & PWRMGTCR0_PW20_ENT) >>
+					PWRMGTCR0_PW20_ENT_SHIFT;
+
+		tb_cycle = (1 << (MAX_BIT - value)) * 2;

Is value = 0 and value = 1 legal? These will make tb_cycle = 0,

quoted

+		time = div_u64(tb_cycle * 1000, tb_ticks_per_usec) - 1;

And time = -1;

Please look at the end of the function, :)

"return sprintf(buf, "%llu\n", time > 0 ? time : 0);"

I know you return 0 if value = 0/1, my question was that, is this
correct as per specification?

Ahh, also for "value" upto 7 you will return 0, no?

If value = 0, MAX_BIT - value = 63
tb_cycle = 0xffffffff_ffffffff,

Actually, tb_cycle will be undefined because you shifted a 32-bit value
(1) by more than 31 bits.  s/1/1ULL/

Actually, we have been discussing this situation that could not have happened.
See !pw20_wt branch, this branch is read default wait bit.
The default wait bit is 50, the time is about 1ms.
The default wait bit cannot less than 50, means the wait entry time cannot greater than 1ms.
We have already begun benchmark test, and we got a preliminary results.
55, 56, 57bit looks good, but we need more benchmark to get the default bit.

What does the default have to do with it?  The user could have set a
different value, and then read it back.

Plus, how much time corresponds to bit 50 depends on the actual timebase
frequency which could vary.

-Scott