> -----Original Message----- > From: Wood Scott-B07421 > Sent: Tuesday, September 17, 2013 5:09 AM > To: Wang Dongsheng-B40534 > Cc: Wood Scott-B07421; ga...@kernel.crashing.org; linuxppc- > d...@lists.ozlabs.org > Subject: Re: [PATCH v3 4/4] powerpc/85xx: add sysfs for pw20 state and > altivec idle > > On Thu, 2013-09-12 at 21:53 -0500, Wang Dongsheng-B40534 wrote: > > > > > -----Original Message----- > > > From: Wood Scott-B07421 > > > Sent: Friday, September 13, 2013 2:07 AM > > > To: Wang Dongsheng-B40534 > > > Cc: Wood Scott-B07421; ga...@kernel.crashing.org; linuxppc- > > > d...@lists.ozlabs.org > > > Subject: Re: [PATCH v3 4/4] powerpc/85xx: add sysfs for pw20 state > > > and altivec idle > > > > > > On Wed, 2013-09-11 at 22:48 -0500, Wang Dongsheng-B40534 wrote: > > > > > > > > > -----Original Message----- > > > > > From: Wood Scott-B07421 > > > > > Sent: Thursday, September 12, 2013 7:04 AM > > > > > To: Wang Dongsheng-B40534 > > > > > Cc: ga...@kernel.crashing.org; linuxppc-dev@lists.ozlabs.org > > > > > Subject: Re: [PATCH v3 4/4] powerpc/85xx: add sysfs for pw20 > > > > > state and altivec idle > > > > > > > > > > On Wed, 2013-09-11 at 13:56 +0800, Dongsheng Wang wrote: > > > > > > From: Wang Dongsheng <dongsheng.w...@freescale.com> > > > > > > > > > > > > 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 entry bit interface: > > > > > > bit value range 0~63, 0 bit is Mintime, 63 bit is Maxtime. > > > > > > /sys/devices/system/cpu/cpuX/pw20_wait_entry_bit > > > > > > /sys/devices/system/cpu/cpuX/altivec_idle_wait_entry_bit > > > > > > > > > > I'm no fan of the way powerpc does bit numbering, but don't flip > > > > > it around here -- you'll just cause confusion. > > > > > > > > > OK. 0 bit is maxtime, 63 bit is mintime. > > > > > > > > > Better yet, this interface should take real time units rather > > > > > than a timebase bit. > > > > > > > > > I think the real time is not suitable, because timebase bit does > > > > not correspond with real time. > > > > > > It's a bit sloppy due to how the hardware works, but you could > > > convert it like you did in earlier patches. Semantically it should > > > probably be the minimum time to wait before entering the low power > state. > > > > > But there has a problem, we can't convert bit to the real time when > user read this sysfs. > > Like: > > echo 1000(us) > /sys/*/pw20_wait_entry_bit, after convert we get bit is > 49. > > cat /sys/*/pw20_wait_entry_bit, after convert the time is 1598(us). > > > > The read out of the time is not real time. Unless we define a variable > to save the real time. > > It's not the end of the world if the value is different when read back. > It just gets rounded up when you write it. > Ok, make a variable to save it.
> > > > > Also, you disable the power saving mode if the maximum interval > > > > > is selected, > > > > It's not disable the pw20 state or altivec idle, just max-delay > > > > entry > > > time. > > > > > > No, the code checks for zero to set or clear the enabling bit (e.g. > > > PW20_WAIT). > > > > > There has pw20_state/altivec_idle sys interface to control > > "enable/disable", There is only to control wait bit. Did you mean > remove "pw20_state/altivec_idle" > > sys interface, and reuse "pw20_wait_entry_bit/altivec_idle*" sys > interface? > > When echo zero into "pw20_wait_entry_bit" we just to disable pw20 > > state, I think that is reasonable. :) > > Sorry, I misread the patch and didn't realize these were separate > interfaces. Keep the "pw20_state/altivec_idle" interfaces. -dongsheng _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev