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; [email protected]; linuxppc- > > [email protected] > > 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: [email protected]; [email protected] > > > > 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 <[email protected]> > > > > > > > > > > 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. > > > > 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. -Scott _______________________________________________ Linuxppc-dev mailing list [email protected] https://lists.ozlabs.org/listinfo/linuxppc-dev
