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

From: Jonathan Cameron <jic23@kernel.org>
To: Nuno Sa via B4 Relay <devnull+nuno.sa.analog.com@kernel.org>
Cc: <nuno.sa@analog.com>,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-iio@vger.kernel.org,
	Olivier MOYSAN <olivier.moysan@foss.st.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	Frank Rowand <frowand.list@gmail.com>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Michael Hennerich <Michael.Hennerich@analog.com>
Subject: Re: [PATCH 05/12] iio: adc: ad9467: don't ignore error codes
Date: Mon, 4 Dec 2023 15:19:10 +0000	[thread overview]
Message-ID: <20231204151910.6ab1728f@jic23-huawei> (raw)
In-Reply-To: <20231121-dev-iio-backend-v1-5-6a3d542eba35@analog.com>

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;
> 