On 24/06/16 10:11, Linus Walleij wrote: > IS_ERR_OR_NULL() should never be used with regulators because > a NULL pointer may be a perfectly valid dummy regulator > > We should always succeed to fetch and enable a regulator, but > it may be a dummy. That is fine, so bail out for any real > errors or probe deferrals > > Include the error code in the warning print so we know what > kind of problem we're dealing with (for example it is nice to > see if it is a probe deferral). > > As we will bail out of probe if the regulator is erroneous, > just issue regulator_disable() on the poweroff path: it will > succeed. > > Cc: Mark Brown <broonie@xxxxxxxxxx> > Cc: Lars-Peter Clausen Lars-Peter Clausen <lars@xxxxxxxxxx> > Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx> Umm. Linus - something went wrong here... See below... > --- > ChangeLog v1->v2: > - No changes > --- > drivers/iio/magnetometer/ak8975.c | 22 ++-------------------- > 1 file changed, 2 insertions(+), 20 deletions(-) > > diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c > index e0e43f497f40..05e9d5041e7a 100644 > --- a/drivers/iio/magnetometer/ak8975.c > +++ b/drivers/iio/magnetometer/ak8975.c > @@ -390,10 +390,8 @@ static int ak8975_power_on(struct i2c_client *client) > int ret; > > data->vdd = devm_regulator_get(&client->dev, "vdd"); > - if (IS_ERR_OR_NULL(data->vdd)) { > + if (IS_ERR(data->vdd)) { > ret = PTR_ERR(data->vdd); > - if (ret == -ENODEV) > - ret = 0; > } else { > ret = regulator_enable(data->vdd); > } > @@ -402,21 +400,6 @@ static int ak8975_power_on(struct i2c_client *client) > "Failed to enable specified Vdd supply\n"); > return ret; > } Where did the next block come from? You introduce one of these in the following patch, but there isn't one currently in the driver. > - > - data->vid = devm_regulator_get(&client->dev, "vid"); > - if (IS_ERR_OR_NULL(data->vid)) { > - ret = PTR_ERR(data->vdd); > - if (ret == -ENODEV) > - ret = 0; > - } else { > - ret = regulator_enable(data->vdd); > - } > - if (ret) { > - dev_warn(&client->dev, > - "Failed to enable specified Vid supply\n"); > - return ret; > - } > - return 0; > } > > /* Disable attached power regulator if any. */ > @@ -425,8 +408,7 @@ static void ak8975_power_off(const struct i2c_client *client) > const struct iio_dev *indio_dev = i2c_get_clientdata(client); > const struct ak8975_data *data = iio_priv(indio_dev); > > - if (!IS_ERR_OR_NULL(data->vdd)) > - regulator_disable(data->vdd); > + regulator_disable(data->vdd); > } > > /* > -- 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