Re: [PATCH] PM / OPP: Initialize regulator pointer to an error value

On Tuesday 16 February 2016 13:11:08 Mark Brown wrote:

On Tue, Feb 16, 2016 at 10:10:44AM +0100, Arnd Bergmann wrote:

quoted

On Tuesday 16 February 2016 01:56:16 Mark Brown wrote:

quoted

quoted

No, NULL is explicitly not something you can substitute in,
essentially all the users are just not bothering to implement error
checking and we don't want to encourage that.  The set of use cases
where we legitimately have optional supplies is very small, much smaller
than clocks, because it makes the electrical engineering a lot harder.

quoted

I must have misinterpreted the idea behind that API as well then.

quoted

From this function definition:

quoted

static inline struct regulator *__must_check regulator_get(struct device *dev,
        const char *id)
{       
        /* Nothing except the stubbed out regulator API should be
         * looking at the value except to check if it is an error
         * value. Drivers are free to handle NULL specifically by
         * skipping all regulator API calls, but they don't have to.
         * Drivers which don't, should make sure they properly handle
         * corner cases of the API, such as regulator_get_voltage()
         * returning 0.
         */             
        return NULL;
}

This is the stubbed regulator API which is only ever used with the stub
regulator API, it uses NULL to give a non-error pointer it can return to
well written callers so they don't know they are running with the stubs.
We are explicitly using NULL because callers should treat it as a valid
regulator.

Right, that is what I understood.

quoted

my reading was that the expected behavior in any driver was:

quoted

* call regulator_get()
* if IS_ERR(), fail device probe function, never use invalid
  pointer other than PTR_ERR()
* if NULL, and regulator is required, fail probe so we never
  use the regulator

No, drivers should never look at the value of the pointer other than to
check it for error.  If there is a problem of any kind an error will be
returned.

quoted

* if NULL, and regulators are optional, continue with the NULL
  value.

No, we always return an error pointer if we fail to get a regulator.
The difference with optional regulators is in how we handle the
situation where we have full constraints and a regulator is not mapped
in, normally we assume there must be one with no software control but we
need to work around buggy bindings as the device would be non-functional
without power.

Sorry, I should not have said "optional" here, which has a specific
meaning in the API. I meant a driver that can work with either
CONFIG_REGULATOR enabled or disabled (which is something slightly
different).

I guess a driver needing to know whether regulators are built-in
should check 'if (IS_ENABLED(CONFIG_REGULATOR))' rather than
checking the return code for NULL.

	Arnd