On 26/06/16 15:04, Jonathan Cameron wrote:
> On 26/06/16 15:01, Jonathan Cameron wrote:
>> 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...
> Pah, I decided the fixup was obvious and have applied this to the
> togreg branch of iio.git. Usual pushed out as testing etc etc.
Sorry I've backed this out as the obvious fix doesn't then give
the right result to apply the later patches.
Please fixup the series and resend.
Jonathan
>
> Jonathan
>>> ---
>>> 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
>>
>
> --
> 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
>
--
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
