[PATCH v3 4/4] powerpc/85xx: add sysfs for pw20 state and altivec idle

[Wang Dongsheng-B40534]

> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Friday, September 13, 2013 2:07 AM
> To: Wang Dongsheng-B40534
> Cc: Wood Scott-B07421; galak at kernel.crashing.org; linuxppc-
> dev at 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: galak at kernel.crashing.org; linuxppc-dev at 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.wang at 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.

> > > 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. :)