diff mbox series

[05/12] iio: adc: ad9467: don't ignore error codes

Message ID 20231121-dev-iio-backend-v1-5-6a3d542eba35@analog.com (mailing list archive)
State Changes Requested
Headers show
Series iio: add new backend framework | expand

Commit Message

Nuno Sa via B4 Relay Nov. 21, 2023, 10:20 a.m. UTC 
From: Nuno Sa <nuno.sa@analog.com>

Make sure functions that return errors are not ignored.

Fixes: ad6797120238 ("iio: adc: ad9467: add support AD9467 ADC")
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/iio/adc/ad9467.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

Comments

David Lechner Nov. 30, 2023, 9:44 p.m. UTC | #1 
On Tue, Nov 21, 2023 at 4:17 AM Nuno Sa via B4 Relay
<devnull+nuno.sa.analog.com@kernel.org> wrote:
>
> From: Nuno Sa <nuno.sa@analog.com>
>
> Make sure functions that return errors are not ignored.
>
> Fixes: ad6797120238 ("iio: adc: ad9467: add support AD9467 ADC")
> Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> ---
>  drivers/iio/adc/ad9467.c | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
> index 368ea57be117..04474dbfa631 100644
> --- a/drivers/iio/adc/ad9467.c
> +++ b/drivers/iio/adc/ad9467.c
> @@ -6,6 +6,7 @@
>   */
>
>  #include <linux/module.h>
> +#include <linux/mutex.h>

This looks like an unrelated change (should probably be in a separate commit).

>  #include <linux/device.h>
>  #include <linux/kernel.h>
>  #include <linux/slab.h>
Nuno Sá Dec. 1, 2023, 8:47 a.m. UTC | #2 
On Thu, 2023-11-30 at 15:44 -0600, David Lechner wrote:
> On Tue, Nov 21, 2023 at 4:17 AM Nuno Sa via B4 Relay
> <devnull+nuno.sa.analog.com@kernel.org> wrote:
> > 
> > From: Nuno Sa <nuno.sa@analog.com>
> > 
> > Make sure functions that return errors are not ignored.
> > 
> > Fixes: ad6797120238 ("iio: adc: ad9467: add support AD9467 ADC")
> > Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> > ---
> >  drivers/iio/adc/ad9467.c | 25 ++++++++++++++++---------
> >  1 file changed, 16 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
> > index 368ea57be117..04474dbfa631 100644
> > --- a/drivers/iio/adc/ad9467.c
> > +++ b/drivers/iio/adc/ad9467.c
> > @@ -6,6 +6,7 @@
> >   */
> > 
> >  #include <linux/module.h>
> > +#include <linux/mutex.h>
> 
> This looks like an unrelated change (should probably be in a separate commit).

Indeed it is... 

> 
> >  #include <linux/device.h>
> >  #include <linux/kernel.h>
> >  #include <linux/slab.h>
Jonathan Cameron Dec. 4, 2023, 3:19 p.m. UTC | #3 
On Tue, 21 Nov 2023 11:20:18 +0100
Nuno Sa via B4 Relay <devnull+nuno.sa.analog.com@kernel.org> wrote:

> From: Nuno Sa <nuno.sa@analog.com>
> 
> Make sure functions that return errors are not ignored.
> 
> Fixes: ad6797120238 ("iio: adc: ad9467: add support AD9467 ADC")
> Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> ---
>  drivers/iio/adc/ad9467.c | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
> index 368ea57be117..04474dbfa631 100644
> --- a/drivers/iio/adc/ad9467.c
> +++ b/drivers/iio/adc/ad9467.c
> @@ -6,6 +6,7 @@
>   */
>  
>  #include <linux/module.h>
> +#include <linux/mutex.h>
David noted this one...

>  #include <linux/device.h>
>  #include <linux/kernel.h>
>  #include <linux/slab.h>
> @@ -160,11 +161,12 @@ static int ad9467_reg_access(struct adi_axi_adc_conv *conv, unsigned int reg,
>  	struct spi_device *spi = st->spi;
>  	int ret;
>  
> -	if (readval == NULL) {
> +	if (!readval) {

Nothing wrong with tidying this up if the !readval syntax is more common
in the driver, but it doesn't have anything to do with the fix, so not in this
patch.

>  		ret = ad9467_spi_write(spi, reg, writeval);
> -		ad9467_spi_write(spi, AN877_ADC_REG_TRANSFER,
> -				 AN877_ADC_TRANSFER_SYNC);
> -		return ret;
> +		if (ret)
> +			return ret;
> +		return ad9467_spi_write(spi, AN877_ADC_REG_TRANSFER,
> +					AN877_ADC_TRANSFER_SYNC);
>  	}
>  
>  	ret = ad9467_spi_read(spi, reg);
> @@ -274,6 +276,8 @@ static int ad9467_get_scale(struct adi_axi_adc_conv *conv, int *val, int *val2)
>  	unsigned int i, vref_val;
unsigned and you check it for < 0 ..

>  
>  	vref_val = ad9467_spi_read(st->spi, AN877_ADC_REG_VREF);
> +	if (vref_val < 0)
> +		return vref_val;

int ret = ...

	vref_val = ret & info1->vref_mask; 
if not an error.


>  
>  	vref_val &= info1->vref_mask;
>  
> @@ -296,6 +300,7 @@ static int ad9467_set_scale(struct adi_axi_adc_conv *conv, int val, int val2)
>  	struct ad9467_state *st = adi_axi_adc_conv_priv(conv);
>  	unsigned int scale_val[2];
>  	unsigned int i;
> +	int ret;
>  
>  	if (val != 0)
>  		return -EINVAL;
> @@ -305,11 +310,13 @@ static int ad9467_set_scale(struct adi_axi_adc_conv *conv, int val, int val2)
>  		if (scale_val[0] != val || scale_val[1] != val2)
>  			continue;
>  
> -		ad9467_spi_write(st->spi, AN877_ADC_REG_VREF,
> -				 info->scale_table[i][1]);
> -		ad9467_spi_write(st->spi, AN877_ADC_REG_TRANSFER,
> -				 AN877_ADC_TRANSFER_SYNC);
> -		return 0;
> +		ret = ad9467_spi_write(st->spi, AN877_ADC_REG_VREF,
> +				       info->scale_table[i][1]);
> +		if (ret < 0)
> +			return ret;
> +
> +		return ad9467_spi_write(st->spi, AN877_ADC_REG_TRANSFER,
> +					AN877_ADC_TRANSFER_SYNC);
>  	}
>  
>  	return -EINVAL;
>
diff mbox series

Patch

diff --git a/drivers/iio/adc/ad9467.c b/drivers/iio/adc/ad9467.c
index 368ea57be117..04474dbfa631 100644
--- a/drivers/iio/adc/ad9467.c
+++ b/drivers/iio/adc/ad9467.c
@@ -6,6 +6,7 @@ 
  */
 
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/device.h>
 #include <linux/kernel.h>
 #include <linux/slab.h>
@@ -160,11 +161,12 @@  static int ad9467_reg_access(struct adi_axi_adc_conv *conv, unsigned int reg,
 	struct spi_device *spi = st->spi;
 	int ret;
 
-	if (readval == NULL) {
+	if (!readval) {
 		ret = ad9467_spi_write(spi, reg, writeval);
-		ad9467_spi_write(spi, AN877_ADC_REG_TRANSFER,
-				 AN877_ADC_TRANSFER_SYNC);
-		return ret;
+		if (ret)
+			return ret;
+		return ad9467_spi_write(spi, AN877_ADC_REG_TRANSFER,
+					AN877_ADC_TRANSFER_SYNC);
 	}
 
 	ret = ad9467_spi_read(spi, reg);
@@ -274,6 +276,8 @@  static int ad9467_get_scale(struct adi_axi_adc_conv *conv, int *val, int *val2)
 	unsigned int i, vref_val;
 
 	vref_val = ad9467_spi_read(st->spi, AN877_ADC_REG_VREF);
+	if (vref_val < 0)
+		return vref_val;
 
 	vref_val &= info1->vref_mask;
 
@@ -296,6 +300,7 @@  static int ad9467_set_scale(struct adi_axi_adc_conv *conv, int val, int val2)
 	struct ad9467_state *st = adi_axi_adc_conv_priv(conv);
 	unsigned int scale_val[2];
 	unsigned int i;
+	int ret;
 
 	if (val != 0)
 		return -EINVAL;
@@ -305,11 +310,13 @@  static int ad9467_set_scale(struct adi_axi_adc_conv *conv, int val, int val2)
 		if (scale_val[0] != val || scale_val[1] != val2)
 			continue;
 
-		ad9467_spi_write(st->spi, AN877_ADC_REG_VREF,
-				 info->scale_table[i][1]);
-		ad9467_spi_write(st->spi, AN877_ADC_REG_TRANSFER,
-				 AN877_ADC_TRANSFER_SYNC);
-		return 0;
+		ret = ad9467_spi_write(st->spi, AN877_ADC_REG_VREF,
+				       info->scale_table[i][1]);
+		if (ret < 0)
+			return ret;
+
+		return ad9467_spi_write(st->spi, AN877_ADC_REG_TRANSFER,
+					AN877_ADC_TRANSFER_SYNC);
 	}
 
 	return -EINVAL;