Re: [patch 2.6.29-rc3-git 1/2] regulator: twl4030 regulators

[Mark Brown <broonie@sirena.org.uk>]

On Sun, Feb 08, 2009 at 04:04:29PM -0800, David Brownell wrote:
> On Sunday 08 February 2009, Mark Brown wrote:

> > If we're going to do this I think it'd be better to push it into the
> > core and let the regulators pass in a set of constraints so that the
> > behaviour will be consistent between drivers.

> Maybe, but there is no such mechanism right yet.
> When it's created, then this could switch over.

Since you appear to be writing the code already... :)

> > I'd also expect to have the registration fail or at least issue a
> > warning if the code kicks in since that indicates that the board
> > constraints have been set up incorrectly.

> A warning might make sense in some cases ... that's
> something I would expect to see from the regulator
> core, though.

That's what I'm suggesting should happen - that things like this should
be implemented in the core so that the behaviour of the API remains
consistent for users moving between platforms and everyone benefits from
new features.

>                Example, I see no "max < min" checks
> triggering registration errors.

Not a bad idea, though.  The core currently doesn't do much checking of
the constraints but that's as much because nobody spent the time on it
as anything else.  To the extent it's a deliberate design decision it's
because the constraints and consumer lists are where the information
about what's possible on a given system comes from and so the checking
that can be done by software is relatively minor.

> > There's a reasonable chance 
> > that the fixed up constraints will still need to be changed for the
> > board to be configured properly and things may end up being driven out
> > of spec, potentially causing damage.

> I don't see that happening ... all that code does is
> tighten existing constraints to match what the hardware
> can do.  The result is just a subset of the range the
> board already said was OK.  If no valid subset exists,

The trouble is that this code should only do anything if the board code
provided a configuration that can't be supported by the hardware, which
is a bit of a red flag.  I'd expect it'd end up catching things like the
user typing extra digits by mistake (this has definitely happened when
writing consumer drivers).

Your current patch does also set constraints for the voltages if they
weren't there previously (though this shouldn't matter since voltage
setting shouldn't be enabled without voltage value constraints).

> I can easily imagine having things partially set up, and
> not really caring whether, on a particular board, those
> initial constraints are really the most specific ones
> applicable.  One component tolerates a range of 1V8..3V6
> maybe, but on any given board it can be hooked up to any
> supply that's even partially in-range.

The pattern you're describing is very much that for consumer and
regulator drivers.  They should work with as wide a set of constraints
as possible to allow them to be used on different systems with different
capabilities and needs - allowing this is essential for the API to be
usable since so many chips are flexible about the supplies they can use.

It's different for the machine constraints since they are by definition
specific to a given system.  While it's expected that users (especially
those sensitive to power consumption) may want to optimise the
configuration of their system during development to get the best
performance out of it there is also an expectation that users will be
making decisions based on knowledge of the hardware design.