Re: regulator: adding constraints to regulator_desc?

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

On Mon, Sep 14, 2009 at 12:41:01PM +0200, Wolfram Sang wrote:
> the constraints on the power-domain-level. As we also have constraints on the
> regulator-level, all the regulator drivers do sanity checks in their
> set_voltage()-functions. These checks differ and it is not always clear to me
> if additional checks other drivers have were forgotten or intentionally omitted
> for a specific driver. I also wondered about code-duplication and if it was
> maybe worthwhile to simply add 'min' and 'max' members to struct regulator_desc
> and let the core do sanity checks (like it does for the constraints on the
> power-domain-level). Has this been considered already?

The checks at the regulator level should be checking more than can be
specified in the constraints - they should also be checking that the
regulator can actually deliver the requested voltage.  It is possible
that a request could be within the constraints but in between two steps
that the regulator can deliver and therefore not supportable.  Drivers
that don't do the additional checks should really be doing them.

These checks could be factored out if we change the API for setting
voltage to work in terms of voltage selectors and force the
implementation of list_voltage() but it's never seemed worth the hassle
yet since we've got cross-tree merge issues caused by the fact that half
the regulator drivers end up getting merged via the MFD tree.  Those
could be dealt with by supporting both methods for a release cycle.  I
might have a look again today, I'm going to be looking at some regulator
drivers anyway.

> One thing which also raised my attention was the beginning of
> regulator_check_voltage(). It starts with

> 	BUG_ON(*min_uV > *max_uV);

> Is it really necessary to halt the kernel? Wouldn't a big warning and -EINVAL
> do like at the end of the function?

That should also be OK, yes.  The risk is that once you start loosing
the plot on regulator stuff the system will often die anyway due to
power problems but WARN_ON and an error would cover the diagnostics just
as well.