* [PATCH v2 1/2] iio: fetch and enable regulators unconditionally
@ 2016-09-05 9:14 Crt Mori
2016-09-05 9:14 ` [PATCH v2 2/2] iio: devm_regulator_get_optional never returns NULL Crt Mori
2016-09-05 20:09 ` [PATCH v2 1/2] iio: fetch and enable regulators unconditionally Jonathan Cameron
0 siblings, 2 replies; 6+ messages in thread
From: Crt Mori @ 2016-09-05 9:14 UTC (permalink / raw)
To: linux-iio, Jonathan Cameron
Cc: Daniel Baluta, Gregor Boirie, Matt Ranostay, Sean Nyekjaer,
Markus Pargmann, Peter Meerwald-Stadler, Hartmut Knaack,
Lars-Peter Clausen, Linus Walleij, Crt Mori
This patch is inspired by a comment of Jonathan Cameron on patch of
Linus Walleij commit aeb55fff3891834e07a3144159a7298a19696af8 ("iio: st_sensors: fetch and enable regulators unconditionally").
The explanation for this change is same as in that patch:
"Supplies are *not* optional (optional means that the supply is
optional in the electrical sense, not the software sense) so we need to
get the and enable them at all times.
If the device tree or board file does not define suitable regulators for
the component, it will be substituted by a dummy regulator, or, if
regulators are disabled altogether, by stubs. There is no need to use the
IS_ERR_OR_NULL() check that is considered harmful.
Reported-by: Linus Wallerij <linus.walleij@linaro.org>
Suggested-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Crt Mori <cmo@melexis.com>
---
drivers/iio/pressure/ms5611_core.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/pressure/ms5611_core.c b/drivers/iio/pressure/ms5611_core.c
index feb41f8..a74ed1f 100644
--- a/drivers/iio/pressure/ms5611_core.c
+++ b/drivers/iio/pressure/ms5611_core.c
@@ -416,8 +416,7 @@ static int ms5611_init(struct iio_dev *indio_dev)
return 0;
err_regulator_disable:
- if (!IS_ERR_OR_NULL(st->vdd))
- regulator_disable(st->vdd);
+ regulator_disable(st->vdd);
return ret;
}
@@ -425,8 +424,7 @@ static void ms5611_fini(const struct iio_dev *indio_dev)
{
const struct ms5611_state *st = iio_priv(indio_dev);
- if (!IS_ERR_OR_NULL(st->vdd))
- regulator_disable(st->vdd);
+ regulator_disable(st->vdd);
}
int ms5611_probe(struct iio_dev *indio_dev, struct device *dev,
--
2.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] iio: devm_regulator_get_optional never returns NULL
2016-09-05 9:14 [PATCH v2 1/2] iio: fetch and enable regulators unconditionally Crt Mori
@ 2016-09-05 9:14 ` Crt Mori
2016-09-05 9:55 ` Linus Walleij
2016-09-05 10:33 ` Lars-Peter Clausen
2016-09-05 20:09 ` [PATCH v2 1/2] iio: fetch and enable regulators unconditionally Jonathan Cameron
1 sibling, 2 replies; 6+ messages in thread
From: Crt Mori @ 2016-09-05 9:14 UTC (permalink / raw)
To: linux-iio, Jonathan Cameron
Cc: Daniel Baluta, Gregor Boirie, Matt Ranostay, Sean Nyekjaer,
Markus Pargmann, Peter Meerwald-Stadler, Hartmut Knaack,
Lars-Peter Clausen, Linus Walleij, Crt Mori
This patch is inspired by a comment of Jonathan Cameron on patch of
Linus Walleij commit aeb55fff3891834e07a3144159a7298a19696af8 ("iio:
st_sensors: fetch and enable regulators unconditionally"). Because
changes made in this patch are actually reference generators they should
be using devm_regulator_get_optional, but if they do not explicitly set
the reference to NULL they should not be using IS_ERR_OR_NULL, but
simple IS_ERR check.
Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Crt Mori <cmo@melexis.com>
---
drivers/iio/adc/ad7266.c | 4 ++--
drivers/iio/adc/ti-ads8688.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/adc/ad7266.c b/drivers/iio/adc/ad7266.c
index c0f6a98..b8d5cfd 100644
--- a/drivers/iio/adc/ad7266.c
+++ b/drivers/iio/adc/ad7266.c
@@ -481,7 +481,7 @@ error_free_gpios:
if (!st->fixed_addr)
gpio_free_array(st->gpios, ARRAY_SIZE(st->gpios));
error_disable_reg:
- if (!IS_ERR_OR_NULL(st->reg))
+ if (!IS_ERR(st->reg))
regulator_disable(st->reg);
return ret;
@@ -496,7 +496,7 @@ static int ad7266_remove(struct spi_device *spi)
iio_triggered_buffer_cleanup(indio_dev);
if (!st->fixed_addr)
gpio_free_array(st->gpios, ARRAY_SIZE(st->gpios));
- if (!IS_ERR_OR_NULL(st->reg))
+ if (!IS_ERR(st->reg))
regulator_disable(st->reg);
return 0;
diff --git a/drivers/iio/adc/ti-ads8688.c b/drivers/iio/adc/ti-ads8688.c
index c400439..4a16349 100644
--- a/drivers/iio/adc/ti-ads8688.c
+++ b/drivers/iio/adc/ti-ads8688.c
@@ -438,7 +438,7 @@ static int ads8688_probe(struct spi_device *spi)
return 0;
error_out:
- if (!IS_ERR_OR_NULL(st->reg))
+ if (!IS_ERR(st->reg))
regulator_disable(st->reg);
return ret;
@@ -451,7 +451,7 @@ static int ads8688_remove(struct spi_device *spi)
iio_device_unregister(indio_dev);
- if (!IS_ERR_OR_NULL(st->reg))
+ if (!IS_ERR(st->reg))
regulator_disable(st->reg);
return 0;
--
2.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] iio: devm_regulator_get_optional never returns NULL
2016-09-05 9:14 ` [PATCH v2 2/2] iio: devm_regulator_get_optional never returns NULL Crt Mori
@ 2016-09-05 9:55 ` Linus Walleij
2016-09-05 10:33 ` Lars-Peter Clausen
1 sibling, 0 replies; 6+ messages in thread
From: Linus Walleij @ 2016-09-05 9:55 UTC (permalink / raw)
To: Crt Mori
Cc: linux-iio, Jonathan Cameron, Daniel Baluta, Gregor Boirie,
Matt Ranostay, Sean Nyekjaer, Markus Pargmann,
Peter Meerwald-Stadler, Hartmut Knaack, Lars-Peter Clausen
On Mon, Sep 5, 2016 at 11:14 AM, Crt Mori <cmo@melexis.com> wrote:
> This patch is inspired by a comment of Jonathan Cameron on patch of
> Linus Walleij commit aeb55fff3891834e07a3144159a7298a19696af8 ("iio:
> st_sensors: fetch and enable regulators unconditionally"). Because
> changes made in this patch are actually reference generators they should
> be using devm_regulator_get_optional, but if they do not explicitly set
> the reference to NULL they should not be using IS_ERR_OR_NULL, but
> simple IS_ERR check.
>
> Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Crt Mori <cmo@melexis.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] iio: devm_regulator_get_optional never returns NULL
2016-09-05 9:14 ` [PATCH v2 2/2] iio: devm_regulator_get_optional never returns NULL Crt Mori
2016-09-05 9:55 ` Linus Walleij
@ 2016-09-05 10:33 ` Lars-Peter Clausen
2016-09-05 20:10 ` Jonathan Cameron
1 sibling, 1 reply; 6+ messages in thread
From: Lars-Peter Clausen @ 2016-09-05 10:33 UTC (permalink / raw)
To: Crt Mori, linux-iio, Jonathan Cameron
Cc: Daniel Baluta, Gregor Boirie, Matt Ranostay, Sean Nyekjaer,
Markus Pargmann, Peter Meerwald-Stadler, Hartmut Knaack,
Linus Walleij
On 09/05/2016 11:14 AM, Crt Mori wrote:
> This patch is inspired by a comment of Jonathan Cameron on patch of
> Linus Walleij commit aeb55fff3891834e07a3144159a7298a19696af8 ("iio:
> st_sensors: fetch and enable regulators unconditionally"). Because
> changes made in this patch are actually reference generators they should
> be using devm_regulator_get_optional, but if they do not explicitly set
> the reference to NULL they should not be using IS_ERR_OR_NULL, but
> simple IS_ERR check.
>
> Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Crt Mori <cmo@melexis.com>
Looks good, thanks.
Acked-by: Lars-Peter Clausen <lars@metafoo.de>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] iio: fetch and enable regulators unconditionally
2016-09-05 9:14 [PATCH v2 1/2] iio: fetch and enable regulators unconditionally Crt Mori
2016-09-05 9:14 ` [PATCH v2 2/2] iio: devm_regulator_get_optional never returns NULL Crt Mori
@ 2016-09-05 20:09 ` Jonathan Cameron
1 sibling, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2016-09-05 20:09 UTC (permalink / raw)
To: Crt Mori, linux-iio
Cc: Daniel Baluta, Gregor Boirie, Matt Ranostay, Sean Nyekjaer,
Markus Pargmann, Peter Meerwald-Stadler, Hartmut Knaack,
Lars-Peter Clausen, Linus Walleij
On 05/09/16 10:14, Crt Mori wrote:
> This patch is inspired by a comment of Jonathan Cameron on patch of
> Linus Walleij commit aeb55fff3891834e07a3144159a7298a19696af8 ("iio: st_sensors: fetch and enable regulators unconditionally").
>
> The explanation for this change is same as in that patch:
> "Supplies are *not* optional (optional means that the supply is
> optional in the electrical sense, not the software sense) so we need to
> get the and enable them at all times.
>
> If the device tree or board file does not define suitable regulators for
> the component, it will be substituted by a dummy regulator, or, if
> regulators are disabled altogether, by stubs. There is no need to use the
> IS_ERR_OR_NULL() check that is considered harmful.
>
> Reported-by: Linus Wallerij <linus.walleij@linaro.org>
> Suggested-by: Jonathan Cameron <jic23@kernel.org>
> Signed-off-by: Crt Mori <cmo@melexis.com>
Applied to the togreg branch of iio.git. Initially pushed out as testing
for the autobuilders to do their magic.
Thanks,
Jonathan
> ---
> drivers/iio/pressure/ms5611_core.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/pressure/ms5611_core.c b/drivers/iio/pressure/ms5611_core.c
> index feb41f8..a74ed1f 100644
> --- a/drivers/iio/pressure/ms5611_core.c
> +++ b/drivers/iio/pressure/ms5611_core.c
> @@ -416,8 +416,7 @@ static int ms5611_init(struct iio_dev *indio_dev)
> return 0;
>
> err_regulator_disable:
> - if (!IS_ERR_OR_NULL(st->vdd))
> - regulator_disable(st->vdd);
> + regulator_disable(st->vdd);
> return ret;
> }
>
> @@ -425,8 +424,7 @@ static void ms5611_fini(const struct iio_dev *indio_dev)
> {
> const struct ms5611_state *st = iio_priv(indio_dev);
>
> - if (!IS_ERR_OR_NULL(st->vdd))
> - regulator_disable(st->vdd);
> + regulator_disable(st->vdd);
> }
>
> int ms5611_probe(struct iio_dev *indio_dev, struct device *dev,
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] iio: devm_regulator_get_optional never returns NULL
2016-09-05 10:33 ` Lars-Peter Clausen
@ 2016-09-05 20:10 ` Jonathan Cameron
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2016-09-05 20:10 UTC (permalink / raw)
To: Lars-Peter Clausen, Crt Mori, linux-iio
Cc: Daniel Baluta, Gregor Boirie, Matt Ranostay, Sean Nyekjaer,
Markus Pargmann, Peter Meerwald-Stadler, Hartmut Knaack,
Linus Walleij
On 05/09/16 11:33, Lars-Peter Clausen wrote:
> On 09/05/2016 11:14 AM, Crt Mori wrote:
>> This patch is inspired by a comment of Jonathan Cameron on patch of
>> Linus Walleij commit aeb55fff3891834e07a3144159a7298a19696af8 ("iio:
>> st_sensors: fetch and enable regulators unconditionally"). Because
>> changes made in this patch are actually reference generators they should
>> be using devm_regulator_get_optional, but if they do not explicitly set
>> the reference to NULL they should not be using IS_ERR_OR_NULL, but
>> simple IS_ERR check.
>>
>> Suggested-by: Lars-Peter Clausen <lars@metafoo.de>
>> Signed-off-by: Crt Mori <cmo@melexis.com>
>
> Looks good, thanks.
>
> Acked-by: Lars-Peter Clausen <lars@metafoo.de>
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.
Thanks,
Jonathan
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-09-05 20:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-05 9:14 [PATCH v2 1/2] iio: fetch and enable regulators unconditionally Crt Mori
2016-09-05 9:14 ` [PATCH v2 2/2] iio: devm_regulator_get_optional never returns NULL Crt Mori
2016-09-05 9:55 ` Linus Walleij
2016-09-05 10:33 ` Lars-Peter Clausen
2016-09-05 20:10 ` Jonathan Cameron
2016-09-05 20:09 ` [PATCH v2 1/2] iio: fetch and enable regulators unconditionally Jonathan Cameron
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.