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

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

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

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.

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

From this function definition:

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.

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

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

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

* drivers never look into the regulator pointer, and only
  pass it into regulator APIs which can cope with the NULL
  value when CONFIG_REGULATOR is disabled.

That would be similar to what we have for clocks. Which part of
my interpretation is wrong?

See above.