Re: regulator_get_optional() no longer returning NULL?

[Mark Brown]

On Wed, Feb 01, 2017 at 02:23:31PM -0800, Dmitry Torokhov wrote:

> It appears that [devm_]regulator_get_optional() and
> [devm_]gpiod_get_optional() behave irritatingly differently. While the
> latter returns NULL for non-existing GPIOs, the former started returning
> -ENODEV instead of NULL, starting with commit below.

> Why did we do that? It is much more convenient to write:

As the commit description for that commit says it's fixing a bug with us
not returning the error code we get from regulator_dev_lookup().

> I.e. it is nice to treat *all* codes returned by
> devm_regulator_get_optional() as fatal and NULL as special instead of
> vetting by hand (and having chance that list of vetted codes will bit
> rot).

There's no real risk of bitrot I can see here, the only thing something
using optional regulator is looking for is that the regulator does not
exist and we have the specific error code -ENODEV for that.

> Can we please revert this patch?

That won't do what you want so I don't know why you're asking for it -
the default value of regulator is ERR_PTR(-EPROBE_DEFER) so if we skip
assigning the actual return code you'll get that and not NULL.  I don't
recall that behaviour ever working, it's not what's documented so it
doesn't look like something broke here.  If you wanted to implement it
you'd need to add new code to do it (eg, squash down -ENODEV in
_get_optional()).

However I am a bit dubious about doing that as I'm not thrilled with
adding in more things for people to check.  Even if you check for NULL
with an optional regulator you'd still need to also go and check that
you didn't get an error code like -EPROBE_DEFER, you'd only get a NULL
in cases where the regulator does not exist.  Given that I'm not sure
that it's actually simplifying things.

Attachment: signature.asc
Description: PGP signature