diff mbox series

[v4,04/12] iio: adc: ad7192: handle regulator voltage error first

Message ID 20210513120752.90074-5-aardelean@deviqon.com (mailing list archive)
State Accepted
Headers show
Series ad_sigma_delta: convert all drivers to device-managed | expand

Commit Message

Alexandru Ardelean May 13, 2021, 12:07 p.m. UTC
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.

This change reworks the code to check regulator_voltage() return value
for negative (error) first, and return early. This is the more common
idiom.

Also, this change is removing the 'voltage_uv' variable and using the 'ret'
value directly. The only place where 'voltage_uv' is being used is to
compute the internal reference voltage, and the type of this variable is
'int' (same are for 'ret'). Using only 'ret' avoids having to assign it on
the error path.

Fixes: ab0afa65bbc7 ("staging: iio: adc: ad7192: fail probe on get_voltage")
Cc: Alexandru Tachici <alexandru.tachici@analog.com>
Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
 drivers/iio/adc/ad7192.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

Comments

Jonathan Cameron May 13, 2021, 3:38 p.m. UTC | #1
On Thu, 13 May 2021 15:07:44 +0300
Alexandru Ardelean <aardelean@deviqon.com> 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.
> 
> This change reworks the code to check regulator_voltage() return value
> for negative (error) first, and return early. This is the more common
> idiom.
> 
> Also, this change is removing the 'voltage_uv' variable and using the 'ret'
> value directly. The only place where 'voltage_uv' is being used is to
> compute the internal reference voltage, and the type of this variable is
> 'int' (same are for 'ret'). Using only 'ret' avoids having to assign it on
> the error path.
> 
> Fixes: ab0afa65bbc7 ("staging: iio: adc: ad7192: fail probe on get_voltage")
> Cc: Alexandru Tachici <alexandru.tachici@analog.com>
> Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>

I've applied the first 4 patches to the fixes-togreg branch of iio.git and
cc'd stable.

Added note to the first 3 about the sign-off chain as its a bit unusual
for it to include loops ;)  I always debate if I should sign-off again
when this happens, but decide against.

Jonathan

> ---
>  drivers/iio/adc/ad7192.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
> index d3be67aa0522..1141cc13a124 100644
> --- a/drivers/iio/adc/ad7192.c
> +++ b/drivers/iio/adc/ad7192.c
> @@ -912,7 +912,7 @@ static int ad7192_probe(struct spi_device *spi)
>  {
>  	struct ad7192_state *st;
>  	struct iio_dev *indio_dev;
> -	int ret, voltage_uv = 0;
> +	int ret;
>  
>  	if (!spi->irq) {
>  		dev_err(&spi->dev, "no IRQ?\n");
> @@ -949,15 +949,12 @@ static int ad7192_probe(struct spi_device *spi)
>  		goto error_disable_avdd;
>  	}
>  
> -	voltage_uv = regulator_get_voltage(st->avdd);
> -
> -	if (voltage_uv > 0) {
> -		st->int_vref_mv = voltage_uv / 1000;
> -	} else {
> -		ret = voltage_uv;
> +	ret = regulator_get_voltage(st->avdd);
> +	if (ret < 0) {
>  		dev_err(&spi->dev, "Device tree error, reference voltage undefined\n");
>  		goto error_disable_avdd;
>  	}
> +	st->int_vref_mv = ret / 1000;
>  
>  	spi_set_drvdata(spi, indio_dev);
>  	st->chip_info = of_device_get_match_data(&spi->dev);
diff mbox series

Patch

diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
index d3be67aa0522..1141cc13a124 100644
--- a/drivers/iio/adc/ad7192.c
+++ b/drivers/iio/adc/ad7192.c
@@ -912,7 +912,7 @@  static int ad7192_probe(struct spi_device *spi)
 {
 	struct ad7192_state *st;
 	struct iio_dev *indio_dev;
-	int ret, voltage_uv = 0;
+	int ret;
 
 	if (!spi->irq) {
 		dev_err(&spi->dev, "no IRQ?\n");
@@ -949,15 +949,12 @@  static int ad7192_probe(struct spi_device *spi)
 		goto error_disable_avdd;
 	}
 
-	voltage_uv = regulator_get_voltage(st->avdd);
-
-	if (voltage_uv > 0) {
-		st->int_vref_mv = voltage_uv / 1000;
-	} else {
-		ret = voltage_uv;
+	ret = regulator_get_voltage(st->avdd);
+	if (ret < 0) {
 		dev_err(&spi->dev, "Device tree error, reference voltage undefined\n");
 		goto error_disable_avdd;
 	}
+	st->int_vref_mv = ret / 1000;
 
 	spi_set_drvdata(spi, indio_dev);
 	st->chip_info = of_device_get_match_data(&spi->dev);