On 03/15/2016 10:29 AM, Gregor Boirie wrote:
> Hi all,
>
> After Jonathan's ms5611 review, I digged a bit deeper into regulator API.
> I must confess it looks rather confusing to me, especially with regard to
> optional regulator support.
>
> Most often, IIO drivers implement the following scheme at probing time to
> enable an optional regulator:
> </code>
> /* At probing time */
> void mydriver_power_enable(struct iio_dev *indio_dev)
> {
> struct mydriver_priv *priv = iio_priv(indio_dev);
>
> /* Regulators not mandatory, but if requested we should enable them. */
> priv->vdd = devm_regulator_get_optional(indio_dev->dev.parent, "vdd");
> if (!IS_ERR(priv->vdd)) {
> int err = regulator_enable(priv->vdd);
> if (err != 0)
> dev_warn(&indio_dev->dev,
> "Failed to enable specified Vdd supply\n");
> }
>
> /* Keep going */
> ...
> }
>
> /* At removal time */
> void mydriver_power_disable(struct iio_dev *indio_dev)
> {
> struct mydriver_priv *priv = iio_priv(indio_dev);
>
> ...
> if (!IS_ERR(priv->vdd))
> regulator_disable(priv->vdd);
> ...
> }
> <code/>
>
> A problem may arise with this piece of code: when devm_regulator_get_optional
> returns -EPROBE_DEFER, driver will go on initializing with no regulator instead
> of deferring their probing operations at a later time as it should (to wait
> regulator initialization completion).
> Although related to real troubles, other error codes are also ignored (EPERM,
> EBUSY, ENOMEM...), which is even worse.
> From my understanding, optional regulator usage should be enforced when present
> and ignored if not there. The only error code which may be ignored should be
> ENODEV when the regulator is optional.
> This is clearly not implemented this way in the above example.
>
> You can see such code in:
> drivers/adc/max1363.c
> drivers/common/st_sensors/st_sensors_core.c
> drivers/dac/ad5686.c
>
> The same seems to happen when using the alternate devm_regulator_get for
> optional
> usage purposes as in:
> drivers/iio/amplifiers/ad8366.c
> drivers/iio/dac/ad5624r_spi.c
> and others...
>
> So here is my simple question: how should I implement optional regulator
> support ?
The only error code that should be ignored is ENODEV. Everything else should
be propagated further up the stack.
- Lars
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
