From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from db9outboundpool.messaging.microsoft.com (mail-db9lp0251.outbound.messaging.microsoft.com [213.199.154.251]) (using TLSv1 with cipher AES128-SHA (128/128 bits)) (Client CN "mail.global.frontbridge.com", Issuer "MSIT Machine Auth CA 2" (not verified)) by ozlabs.org (Postfix) with ESMTPS id 177C72C00DE for ; Tue, 5 Nov 2013 10:48:36 +1100 (EST) Received: from mail147-db9 (localhost [127.0.0.1]) by mail147-db9-R.bigfish.com (Postfix) with ESMTP id 988EB260157 for ; Mon, 4 Nov 2013 23:48:30 +0000 (UTC) Received: from DB9EHSMHS028.bigfish.com (unknown [10.174.16.253]) by mail147-db9.bigfish.com (Postfix) with ESMTP id 9BAA61C00A3 for ; Mon, 4 Nov 2013 23:47:46 +0000 (UTC) Message-ID: <1383608853.25829.38.camel@snotra.buserror.net> Subject: Re: [PATCH v5 4/4] powerpc/85xx: add sysfs for pw20 state and altivec idle From: Scott Wood To: Wang Dongsheng-B40534 Date: Mon, 4 Nov 2013 17:47:33 -0600 In-Reply-To: References: <1381828871-17110-1-git-send-email-dongsheng.wang@freescale.com> <1381828871-17110-4-git-send-email-dongsheng.wang@freescale.com> <6A3DF150A5B70D4F9B66A25E3F7C888D071C08E4@039-SN2MPN1-013.039d.mgd.msft.net> <6A3DF150A5B70D4F9B66A25E3F7C888D071C3279@039-SN2MPN1-013.039d.mgd.msft.net> <1382028703.7979.782.camel@snotra.buserror.net> <1382124202.7979.886.camel@snotra.buserror.net> Content-Type: text/plain; charset="UTF-8" MIME-Version: 1.0 Cc: Wood Scott-B07421 , "linuxppc-dev@lists.ozlabs.org" , Bhushan Bharat-R65777 List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Sun, 2013-10-20 at 22:27 -0500, Wang Dongsheng-B40534 wrote: > > > -----Original Message----- > > From: Wood Scott-B07421 > > Sent: Saturday, October 19, 2013 3:23 AM > > To: Wang Dongsheng-B40534 > > Cc: Wood Scott-B07421; Bhushan Bharat-R65777; 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 21:36 -0500, Wang Dongsheng-B40534 wrote: > > > > > > > -----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: > > > > > > > > > > > -----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 > > > > > > > > > > > > > > > > > > > > > > > > > -----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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -----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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -----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 > > > > > > > > > Subject: [PATCH v5 4/4] powerpc/85xx: add sysfs for pw20 > > > > > > > > > state and > > > > > > > > altivec idle > > > > > > > > > > > > > > > > > > +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); > > > > > > > > > + 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, > > > > > > > > > > > > > > > > > + 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. > > > if (!pw20_wt) { > smp_call_function_single(cpu, do_show_pwrmgtcr0, &value, 1); > value = (value & PWRMGTCR0_PW20_ENT) >> > PWRMGTCR0_PW20_ENT_SHIFT; > > tb_cycle = (1 << (MAX_BIT - value)) * 2; > time = tb_cycle * (1000 / tb_ticks_per_usec) - 1; > } else { > time = pw20_wt; > } > > As we have discussed before we need a variable to save To save the users to set wait-entry-time value. > > See the code, if user have set a value, and the value will be set in pw20_wt. > When the user read it back, the code will do "time = pw20_wt" branch. > > When the user not set the wait-entry-time value and read this sys interface, the code will do the following branch. Oh, so it's not that you "need" this, you already have it. -Scott