2021-05-12 19:54:12

by Alexandru Ardelean

[permalink] [raw]
Subject: [PATCH v3 04/12] iio: adc: ad7192: handle zero Avdd regulator value

This change fixes a corner-case, where for a zero regulator value, the
driver would exit early, initializing the driver only partially.
The driver would be in an unknown state.

If the regulator value is zero, then the internal reference will be zero
(though that value will be zero for anything less than 1 millivolt).

Fixes: ab0afa65bbc7 ("staging: iio: adc: ad7192: fail probe on get_voltage")
Cc: Alexandru Tachici <[email protected]>
Signed-off-by: Alexandru Ardelean <[email protected]>
---
drivers/iio/adc/ad7192.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
index d3be67aa0522..9da394ad3868 100644
--- a/drivers/iio/adc/ad7192.c
+++ b/drivers/iio/adc/ad7192.c
@@ -951,7 +951,7 @@ static int ad7192_probe(struct spi_device *spi)

voltage_uv = regulator_get_voltage(st->avdd);

- if (voltage_uv > 0) {
+ if (voltage_uv >= 0) {
st->int_vref_mv = voltage_uv / 1000;
} else {
ret = voltage_uv;
--
2.31.1


2021-05-13 07:45:22

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: [PATCH v3 04/12] iio: adc: ad7192: handle zero Avdd regulator value

On 5/12/21 7:49 PM, Alexandru Ardelean wrote:
> This change fixes a corner-case, where for a zero regulator value, the
> driver would exit early, initializing the driver only partially.
> The driver would be in an unknown state.
>
> If the regulator value is zero, then the internal reference will be zero
> (though that value will be zero for anything less than 1 millivolt).
>
> Fixes: ab0afa65bbc7 ("staging: iio: adc: ad7192: fail probe on get_voltage")
> Cc: Alexandru Tachici <[email protected]>
> Signed-off-by: Alexandru Ardelean <[email protected]>
> ---
> drivers/iio/adc/ad7192.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
> index d3be67aa0522..9da394ad3868 100644
> --- a/drivers/iio/adc/ad7192.c
> +++ b/drivers/iio/adc/ad7192.c
> @@ -951,7 +951,7 @@ static int ad7192_probe(struct spi_device *spi)
>
> voltage_uv = regulator_get_voltage(st->avdd);
>
> - if (voltage_uv > 0) {
> + if (voltage_uv >= 0) {
> st->int_vref_mv = voltage_uv / 1000;
> } else {
> ret = voltage_uv;

More idiomatic would be to check for voltage_uv < 0.

    if (voltage_uv < 0) {
        ret = voltage_uv;
        dev_err(&spi->dev, "Device tree error, reference voltage
undefined\n");
        goto error_disable_avdd;
    }

    st->int_vref_mv = voltage_uv / 1000;


2021-05-13 11:16:37

by Alexandru Ardelean

[permalink] [raw]
Subject: Re: [PATCH v3 04/12] iio: adc: ad7192: handle zero Avdd regulator value

On Thu, May 13, 2021 at 10:43 AM Lars-Peter Clausen <[email protected]> wrote:
>
> On 5/12/21 7:49 PM, Alexandru Ardelean wrote:
> > This change fixes a corner-case, where for a zero regulator value, the
> > driver would exit early, initializing the driver only partially.
> > The driver would be in an unknown state.
> >
> > If the regulator value is zero, then the internal reference will be zero
> > (though that value will be zero for anything less than 1 millivolt).
> >
> > Fixes: ab0afa65bbc7 ("staging: iio: adc: ad7192: fail probe on get_voltage")
> > Cc: Alexandru Tachici <[email protected]>
> > Signed-off-by: Alexandru Ardelean <[email protected]>
> > ---
> > drivers/iio/adc/ad7192.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
> > index d3be67aa0522..9da394ad3868 100644
> > --- a/drivers/iio/adc/ad7192.c
> > +++ b/drivers/iio/adc/ad7192.c
> > @@ -951,7 +951,7 @@ static int ad7192_probe(struct spi_device *spi)
> >
> > voltage_uv = regulator_get_voltage(st->avdd);
> >
> > - if (voltage_uv > 0) {
> > + if (voltage_uv >= 0) {
> > st->int_vref_mv = voltage_uv / 1000;
> > } else {
> > ret = voltage_uv;
>
> More idiomatic would be to check for voltage_uv < 0.

sure;
makes sense
will re-spin

>
> if (voltage_uv < 0) {
> ret = voltage_uv;
> dev_err(&spi->dev, "Device tree error, reference voltage
> undefined\n");
> goto error_disable_avdd;
> }
>
> st->int_vref_mv = voltage_uv / 1000;
>