Re: [patch 2.6.30-rc3] regulator: regression fix

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

On Tuesday 28 April 2009, Mark Brown wrote:
> On Mon, Apr 27, 2009 at 07:59:40PM -0700, David Brownell wrote:
> 
> > By removing the "else", it breaks the handling of fixed-voltage
> > regulators ... turning a non-error/non-warning situation into
> > a complete init failure, which can then prevent system startup.
> 
> The change you're making isn't relevant to what I suspect the actual
> problem is (you didn't specify, I may be wrong here).

Restoring the "else" fixed the logic flaw ...


> For fixed voltage regulators either the user will have specified a
> voltage constraint (in which case we'll fall into your else case since
> cmin ought to be non-zero) or they won't (in which case it's the default
> constraint code you added will fill it in).  The problem I think you're
> seeing is that the code you added to fill in a default constraint for
> fixed voltage regulators uses INT_MIN as the minimum contraint value.
> This is a negative value and so fails the correctness check further
> down.

That was my conclusion too.  I forget all the relevant history,
except that you disliked the notion that boards be able to accept
whatever constraint the regulator allows ... so that some of the
logic there is left over.


> > You might want to provide a different patch, but ignoring
> > this regression doesn't seem practical...
> 
> The code that was being fixed was only even in -next for a relatively
> brief period of time.

This is the first time I've seen the "fix" though.  Recall that
the code in question has been in use for several months now, while
waiting to wend its way into mainline.  It might be useful to CC
a few more folk on such "fix" patches.


> >  		/* else require explicit machine-level constraints */
> > -		if (cmin <= 0 || cmax <= 0 || cmax < cmin) {
> > +		else if (cmin <= 0 || cmax <= 0 || cmax < cmin) {
> 
> Yeah, a different patch I think.  I'll send one shortly.
> 
>