Messages in this thread | | | Date | Mon, 14 Sep 2009 11:57:15 +0100 | From | Mark Brown <> | Subject | Re: regulator: adding constraints to regulator_desc? |
| |
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.
| |