All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Brownell <david-b@pacbell.net>
To: Mark Brown <broonie@sirena.org.uk>
Cc: Liam Girdwood <lrg@slimlogic.co.uk>,
	lkml <linux-kernel@vger.kernel.org>,
	OMAP <linux-omap@vger.kernel.org>
Subject: Re: [patch 2.6.29-rc3-git 1/2] regulator: twl4030 regulators
Date: Mon, 9 Feb 2009 16:24:01 -0800	[thread overview]
Message-ID: <200902091624.01802.david-b@pacbell.net> (raw)
In-Reply-To: <20090209172714.GA17637@sirena.org.uk>

On Monday 09 February 2009, Mark Brown wrote:
> 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... :)

Not for the regulator core, though.  Switching from
a simplistic constraint engine to one that starts to
handle a more realistic "union of constraints" model
would be doable, but is a bit more off-topic work than
I'd want to sign up for just now.


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

Or as I put it earlier:  the current constraint model is
a bit simplistic.

Such things can *EASILY* be over-done; starting under-done
isn't a bad model.  Better to collect a few real examples of
how/why "under-done" needs to be improved, than overdesign
from the start.  (And I've seen some power "constraint"
frameworks that seemed way overdone.)

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

The model you're working with doesn't do a good job of
component-izing things ... "board" may be "board stack",
where notions like "the" hardware are fluid.

And in particular, "can't be supported" was NOT an issue
in the scenarios I gave.  The same mainboard could easily
support two regulators with different selections of output
voltages.  The problem was the model where the constraints
from the mainboard would be specific to one regulator,
instead of just to the mainboard.


> Your current patch does also set constraints for the voltages if they
> weren't there previously

I was thinking that boards which don't need constraints
shouldn't need to create any ... they could just pass on
the capabilities of the underlying regulator.


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

Only when that "system" is componentized that way.
Not all are.

Modular systems can have plug-in regulator boards;
constraints on a mainboard would necessarily overlap
with supported regulator boards ... but the regulators
themselves would naturally have different constraints.

One way to view this:  what you're calling "regulator"
constraints are really coming from the "machine".

Those few lines of code that seem to bother you are
only recognizing that they are, in fact, two very
different entities.

Without changing the regulator core, the only way
to handle contstraints which come from the actual
regulators is to force the machine constraints
to be in-range with respect to whatever regulator
driver ends up using them.

- Dave


  reply	other threads:[~2009-02-10  0:24 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-02-08 18:37 [patch 2.6.29-rc3-git 1/2] regulator: twl4030 regulators David Brownell
2009-02-08 23:29 ` Mark Brown
2009-02-09  0:04   ` David Brownell
2009-02-09  0:04     ` David Brownell
2009-02-09 17:27     ` Mark Brown
2009-02-10  0:24       ` David Brownell [this message]
2009-02-10 22:48         ` Mark Brown
2009-02-23 20:45           ` David Brownell
2009-02-23 20:52             ` [patch/rfc 2.6.29-rc6 1/2] regulator: enumerate voltages David Brownell
2009-02-24 22:23               ` Mark Brown
2009-02-25  0:17                 ` David Brownell
2009-02-25 15:17                   ` Mark Brown
2009-02-25 22:12                     ` David Brownell
2009-02-25 23:01                       ` Mark Brown
2009-02-25 23:47                         ` David Brownell
2009-02-25 23:47                           ` David Brownell
2009-02-26 11:05                           ` Mark Brown
2009-02-26  1:02                     ` David Brownell
2009-02-26  1:02                       ` David Brownell
2009-02-26 10:46                       ` Mark Brown
2009-02-26 18:56                         ` David Brownell
2009-02-26 19:05                           ` Mark Brown
2009-02-26 19:38                             ` David Brownell
2009-02-26 20:02                               ` Liam Girdwood
2009-02-26 20:59                                 ` David Brownell
2009-02-26 20:59                                   ` David Brownell
2009-02-26 19:48               ` [patch 2.6.29-rc6 1/2] regulator: enumerate voltages (v2) David Brownell
2009-02-26 20:20                 ` Mark Brown
2009-02-26 21:12                   ` David Brownell
2009-02-26 21:48                     ` [patch 2.6.29-rc6+misc] MMC: regulator utilities David Brownell
2009-03-02 20:59                       ` Pierre Ossman
2009-03-02 21:27                         ` David Brownell
2009-03-02 21:40                           ` Pierre Ossman
2009-03-02 22:00                             ` David Brownell
2009-03-04  3:18                               ` David Brownell
2009-03-08 13:59                                 ` Pierre Ossman
2009-03-08 20:34                                   ` David Brownell
2009-03-08 21:49                                     ` Pierre Ossman
2009-03-09 11:52                                       ` Liam Girdwood
2009-03-11 11:30                                         ` David Brownell
2009-03-11 14:34                                           ` Liam Girdwood
2009-02-26 20:53                 ` [patch 2.6.29-rc6 1/2] regulator: enumerate voltages (v2) Liam Girdwood
2009-02-26 21:28                   ` David Brownell
2009-02-26 21:58                     ` Liam Girdwood
2009-02-27  0:10                       ` David Brownell
2009-02-23 20:54             ` [patch/rfc 2.6.29-rc6 2/2] regulator: twl4030 voltage enumeration David Brownell
2009-02-26 19:50               ` [patch/rfc 2.6.29-rc6 2/2] regulator: twl4030 voltage enumeration (v2) David Brownell
2009-02-26 20:25                 ` Mark Brown
2009-02-26 22:16                 ` Liam Girdwood
2009-02-27  0:02                   ` David Brownell
2009-02-27  0:02                     ` David Brownell
2009-02-27 12:32                     ` Liam Girdwood
2009-02-27 20:39                       ` David Brownell
2009-02-27 21:26                         ` Liam Girdwood
2009-03-03 22:59                       ` David Brownell
2009-03-04 11:47                         ` Liam Girdwood
2009-02-23 22:04             ` [patch 2.6.29-rc3-git 1/2] regulator: twl4030 regulators Mark Brown
2009-02-23 22:43               ` David Brownell
2009-02-24  0:55                 ` Mark Brown
2009-02-24  2:03                   ` David Brownell
2009-02-24 12:41                     ` Mark Brown
2009-02-24  2:22                   ` David Brownell
2009-02-24  2:22                     ` David Brownell
2009-02-24  7:25                     ` David Brownell
2009-02-24  7:25                       ` David Brownell
2009-02-26 22:15 ` Liam Girdwood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200902091624.01802.david-b@pacbell.net \
    --to=david-b@pacbell.net \
    --cc=broonie@sirena.org.uk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=lrg@slimlogic.co.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.