Re: [patch 2.6.28-rc3] regulator: add REGULATOR_MODE_OFF

[David Brownell <david-b@pacbell.net>]

On Wednesday 12 November 2008, Mark Brown wrote:
> On Mon, Nov 10, 2008 at 08:56:19PM -0800, David Brownell wrote:
> > On Monday 10 November 2008, Mark Brown wrote:
>
> > > regulator_disable() needn't imply that the regulator will actually be
> > > off -
> 
> > Would you say that also for regulator_ops.disable() though?
> 
> If we ignore regulator_force_disable() (which isn't used yet) for the
> moment I'd actually say that in your case with non-software enables it's
> reasonable for it to still be powered by some other part of the system.

I'll read that as a "yes"...

... but I don't know what you mean by "non-software enables".  Maybe you
mean "non-Linux"?  One of the other inputs would normally be controlled
by software -- just not from Linux.  Maybe it'd help if you think about
a system structured like:

  - one ARM running Linux for UI, Internet access, etc
  - another one running some realtime cell modem firmware
  - both coordinating through a PMIC

So the regulator's hardware ENABLE signal is is fed by an OR gate from
both CPUs.  Similarly with a signal that's high to put the regulator
in "normal" mode (else it goes into low-current "standby" when enabled).

Those two signals (enable, normal-mode) would be under software control.
Or hardware ... the key point is that some inputs aren't from Linux.


> Mainly because it's proably easier to just ignore the other enables than
> it is to explain to the rest of the system that there are others it
> can't know about.

My recently posted v2 of this patch solves this easily enough.

It suffices to have one method report the actual hardware state
(if that includes OFF as well as NORMAL, STANDBY, etc) ... that
seems to be what you mean by calling one value "primary".  Others
can report what Linux has requested.

And having regulator_ops.get_mode() be the call to report that
hardware state is straightforward.  Especially once you consider
that the actual state *will* be what Linux requested, except in
cases like:  (a) shared regulators, like the scenario above, for
which I suspect twl4030 is the first case in Linux; (b) hardware
fault handling, like overcurrent/overtemp shutdown; and maybe
(c) "smart" regulators that switch modes automatically.

That seems to be a minimal-disruption change.

 
> > > > 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:
> 
> > > ...this sounds like the same thing done in hardware.
> 
> > Seems to me more like a three input OR gate.  No counters.  ;)
> > At least, if you ignore the additional arbitration between
> > ACTIVE, STANDBY, and OFF modes.  (Highest one wins...)
> 
> Logically that's what the regulator API is doing, it's just that
> currently it doesn't support reconfiguration of shared regulators.

Not entirely true.  In this case the issue is exposing regulator
output state ... the regulator_ops suffice for inputs, which would
combine with other inputs to determine the actual regulator state
that's reported using by regulator_ops.get_mode().


> If 
> you remember the pre-submission versions you looked at they had support
> for computing the most restrictive voltage constraint from multiple
> clients, this sounds like the same thing done in hardware for the
> regulator mode.

Yes ... the current "Dynamic Regulator Mode Switching" (DRMS) code
does some of that:  the point of having different client handles
is that those handles can encapsulate different load constraints.
And then when the load changes, the requested mode can change.


> The view of the regulator API is that the operating configuration of the
> regulator (including not only the mode but also the voltage or current)
> is done separately to enabling or disabling it.

OK, but configuration inputs to the regulator drivers are
a different issue from how to expose regulator state.

It seems we've almost converged on the result of get_mode()
being that regulator state output, which is a function of more
than just the inputs from Linux.


> The expectation when writing this was that anything 
> software controlled would be fully software controlled.

The problem is that the current code *also* ignores the fact
that hardware status is a function of more inputs than just
those from Linux.  Like thermal shutdown from overcurrent.
The configuration inputs might be fine, but the output of
a regulator necessarily incorporates other inputs.

The top reason to display system state in sysfs is to support
troubleshooting ... and hiding the actual system state makes
that needlessly difficult.

- Dave