Messages in this thread | | | From | David Brownell <> | Subject | Re: [patch 2.6.28-rc3] regulator: add REGULATOR_MODE_OFF | Date | Mon, 10 Nov 2008 07:43:34 -0800 |
| |
On Monday 10 November 2008, Liam Girdwood wrote: > On Sun, 2008-11-09 at 15:31 -0800, David Brownell wrote: > > From: David Brownell <dbrownell@users.sourceforge.net> > > > > The regulator framework needs to expose an OFF mode for regulators > > with a single state machine. Example: TWL4030 regulators each > > have a status register exposing the current mode, which will be > > either ACTIVE, STANDBY, or OFF. But regulator_ops.get_mode() > > currently has no way to report that third (OFF) mode. > > OFF is currently not a regulator operating mode but is a regulator > operating state (e.g. state is either ON or OFF).
The regulator itself supports exactly three states/modes.
You seem to imply that the programming interface should be exposing four -- {ACTIVE, STANDBY } x { ON, OFF } -- which doesn't reflect how the hardware works.
See below; the key conceptual problem in this interface is probably the assumption that the Linux CPU isn't sharing control over the regulator. So regulator_disable() can't imply REGULATOR_MODE_OFF ... another CPU may need to keep it in some other state.
> The modes define the > ON (supplying power) operating modes supported by a regulator. > I should probably add some more docs/comments here......
Seems to me more like this is a "fix the interface" case instead of a "document the problem" one. It's not that the implication was unclear ... but that it won't work.
> I assume the TWL4030's ACTIVE and STANDBY modes supply power and > probably all share the same register/bits with OFF (thus making > it more tightly coupled in the hardware).
It's *very* tightly coupled to the hardware. The regulator state (active/standby/off) is determined by a vote between three hardware request mechanisms ... the CPU running Linux only gets one vote. Have a look at the docs[1], if you dare.
So for example when any of the three requestors asks for the regulator to go ACTIVE it will do so. This means you can have cases like:
- One CPU (running Linux, say) asks to disable() the regulator * implemented by clearing that CPU's bit in a mask * is_enabled() tests that bit and says "no, not enabled" - Another CPU needs it active * request might be coupled to the nSLEEP2 signal * thus get_mode() will say it's ACTIVE
So you see why enable/disable is orthogonal to MODE_OFF.
It's true that it won't be OFF unless the Linux CPU is not requesting it ("disabled" its request) ... but the converse is false, because of the non-Linux requestor(s).
> The other two patches are fine. Would you be able to resend the first > without the OFF mode patch changes.
I could, but I'd rather get the interface problem resolved first. At this point, adding MODE_OFF is the only viable option on the table...
- Dave
[1] http://focus.ti.com/docs/prod/folders/print/tps65950.html
"TPS65950" is a mouthful, so it's easier to say TWL5030 (equivalent part) or TWL4030 (predecessor part, which is in more developers' hands).
The most relevant section of the doc seem to be chapter 5, pp. 221-390 ... yes, some Linux-capable SOCs are smaller and simpler chips; and no, I've not read it all either. You'd want the TRM, 9+ MBytes, for programming info.
| |