[PATCH V2 1/2] iio: meter: ade7754: add error handling in _reset and _stop_device

[PATCH V2 1/2] iio: meter: ade7754: add error handling in _reset and _stop_device

[Devendra Naga]

This patch adds the error handling for the value returned from
ade7754_spi_read_reg_8. With this patch, the following randconfig
warnings get fixed automatically.

drivers/staging/iio/meter/ade7754.c:222:6: warning: ‘val’ may be used
uninitialized in this function [-Wmaybe-uninitialized]
drivers/staging/iio/meter/ade7754.c:368:6: warning: ‘val’ may be used
uninitialized in this function [-Wmaybe-uninitialized]

Signed-off-by: Devendra Naga <devendra.aaru@gmail.com>
---

 Hello, 

 Please see if this is the error message you wanted.

 Build tested on next-20141231 with randconfig sent to iio ml,
 and with allmodconfig.

 drivers/staging/iio/meter/ade7754.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/meter/ade7754.c b/drivers/staging/iio/meter/ade7754.c
index 81f6731..746b188 100644
--- a/drivers/staging/iio/meter/ade7754.c
+++ b/drivers/staging/iio/meter/ade7754.c
@@ -216,9 +216,13 @@ error_ret:
 
 static int ade7754_reset(struct device *dev)
 {
+	int ret;
 	u8 val;
 
-	ade7754_spi_read_reg_8(dev, ADE7754_OPMODE, &val);
+	ret = ade7754_spi_read_reg_8(dev, ADE7754_OPMODE, &val);
+	if (ret < 0)
+		return ret;
+
 	val |= 1 << 6; /* Software Chip Reset */
 	return ade7754_spi_write_reg_8(dev, ADE7754_OPMODE, val);
 }
@@ -362,9 +366,16 @@ error_ret:
 /* Power down the device */
 static int ade7754_stop_device(struct device *dev)
 {
+	int ret;
 	u8 val;
 
-	ade7754_spi_read_reg_8(dev, ADE7754_OPMODE, &val);
+	ret = ade7754_spi_read_reg_8(dev, ADE7754_OPMODE, &val);
+	if (ret < 0) {
+		dev_err(dev, "unable to power down the device, error: %d",
+			ret);
+		return ret;
+	}
+
 	val |= 7 << 3;  /* ADE7754 powered down */
 	return ade7754_spi_write_reg_8(dev, ADE7754_OPMODE, val);
 }
-- 
1.9.3

[PATCH V2 2/2] iio: meter: ade7759: add error handling in _reset and _stop_device

[Devendra Naga]

This patch adds the error handling for the value returned from
ade7759_spi_read_reg_16. With this patch, the following randconfig
warnings get fixed automatically.

drivers/staging/iio/meter/ade7759.c:224:6: warning: ‘val’ may be
used uninitialized in this function [-Wmaybe-uninitialized]
drivers/staging/iio/meter/ade7759.c:309:6: warning: ‘val’ may be
used uninitialized in this function [-Wmaybe-uninitialized]

Signed-off-by: Devendra Naga <devendra.aaru@gmail.com>
---
 Hello, 

 Please see if this is the error message you wanted.

 Build tested on next-20141231 with randconfig sent to iio ml,
 and with allmodconfig.

 drivers/staging/iio/meter/ade7759.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/meter/ade7759.c b/drivers/staging/iio/meter/ade7759.c
index 7d21743..86b5145 100644
--- a/drivers/staging/iio/meter/ade7759.c
+++ b/drivers/staging/iio/meter/ade7759.c
@@ -218,9 +218,12 @@ static int ade7759_reset(struct device *dev)
 	int ret;
 	u16 val;
 
-	ade7759_spi_read_reg_16(dev,
+	ret = ade7759_spi_read_reg_16(dev,
 			ADE7759_MODE,
 			&val);
+	if (ret < 0)
+		return ret;
+
 	val |= 1 << 6; /* Software Chip Reset */
 	ret = ade7759_spi_write_reg_16(dev,
 			ADE7759_MODE,
@@ -301,11 +304,18 @@ error_ret:
 /* Power down the device */
 static int ade7759_stop_device(struct device *dev)
 {
+	int ret;
 	u16 val;
 
-	ade7759_spi_read_reg_16(dev,
+	ret = ade7759_spi_read_reg_16(dev,
 			ADE7759_MODE,
 			&val);
+	if (ret < 0) {
+		dev_err(dev, "unable to power down the device, error: %d\n",
+			ret);
+		return ret;
+	}
+
 	val |= 1 << 4;  /* AD converters can be turned off */
 
 	return ade7759_spi_write_reg_16(dev, ADE7759_MODE, val);
-- 
1.9.3

Re: [PATCH V2 2/2] iio: meter: ade7759: add error handling in _reset and _stop_device

[Jonathan Cameron]

On 02/01/15 09:02, Devendra Naga wrote:
> This patch adds the error handling for the value returned from
> ade7759_spi_read_reg_16. With this patch, the following randconfig
> warnings get fixed automatically.
> 
> drivers/staging/iio/meter/ade7759.c:224:6: warning: ‘val’ may be
> used uninitialized in this function [-Wmaybe-uninitialized]
> drivers/staging/iio/meter/ade7759.c:309:6: warning: ‘val’ may be
> used uninitialized in this function [-Wmaybe-uninitialized]
> 
> Signed-off-by: Devendra Naga <devendra.aaru@gmail.com>
applied to the togreg branch of iio.git - initially pushed out
as testing to let  the autobuilders play with it.

Again, this is now 3.21 material...
> ---
>  Hello, 
> 
>  Please see if this is the error message you wanted.
> 
>  Build tested on next-20141231 with randconfig sent to iio ml,
>  and with allmodconfig.
> 
>  drivers/staging/iio/meter/ade7759.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/iio/meter/ade7759.c b/drivers/staging/iio/meter/ade7759.c
> index 7d21743..86b5145 100644
> --- a/drivers/staging/iio/meter/ade7759.c
> +++ b/drivers/staging/iio/meter/ade7759.c
> @@ -218,9 +218,12 @@ static int ade7759_reset(struct device *dev)
>  	int ret;
>  	u16 val;
>  
> -	ade7759_spi_read_reg_16(dev,
> +	ret = ade7759_spi_read_reg_16(dev,
>  			ADE7759_MODE,
>  			&val);
> +	if (ret < 0)
> +		return ret;
> +
>  	val |= 1 << 6; /* Software Chip Reset */
>  	ret = ade7759_spi_write_reg_16(dev,
>  			ADE7759_MODE,
> @@ -301,11 +304,18 @@ error_ret:
>  /* Power down the device */
>  static int ade7759_stop_device(struct device *dev)
>  {
> +	int ret;
>  	u16 val;
>  
> -	ade7759_spi_read_reg_16(dev,
> +	ret = ade7759_spi_read_reg_16(dev,
>  			ADE7759_MODE,
>  			&val);
> +	if (ret < 0) {
> +		dev_err(dev, "unable to power down the device, error: %d\n",
> +			ret);
> +		return ret;
> +	}
> +
>  	val |= 1 << 4;  /* AD converters can be turned off */
>  
>  	return ade7759_spi_write_reg_16(dev, ADE7759_MODE, val);
> 

Re: [PATCH V2 1/2] iio: meter: ade7754: add error handling in _reset and _stop_device

[Jonathan Cameron]

On 02/01/15 09:02, Devendra Naga wrote:
> This patch adds the error handling for the value returned from
> ade7754_spi_read_reg_8. With this patch, the following randconfig
> warnings get fixed automatically.
> 
> drivers/staging/iio/meter/ade7754.c:222:6: warning: ‘val’ may be used
> uninitialized in this function [-Wmaybe-uninitialized]
> drivers/staging/iio/meter/ade7754.c:368:6: warning: ‘val’ may be used
> uninitialized in this function [-Wmaybe-uninitialized]
> 
> Signed-off-by: Devendra Naga <devendra.aaru@gmail.com>
Looks good to me.  A Hartmut reviewed the original patch I'll leave it
a few days to see if he wants to take a look at the update.

Thanks,

Jonathan
> ---
> 
>  Hello, 
> 
>  Please see if this is the error message you wanted.
> 
>  Build tested on next-20141231 with randconfig sent to iio ml,
>  and with allmodconfig.
> 
>  drivers/staging/iio/meter/ade7754.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/iio/meter/ade7754.c b/drivers/staging/iio/meter/ade7754.c
> index 81f6731..746b188 100644
> --- a/drivers/staging/iio/meter/ade7754.c
> +++ b/drivers/staging/iio/meter/ade7754.c
> @@ -216,9 +216,13 @@ error_ret:
>  
>  static int ade7754_reset(struct device *dev)
>  {
> +	int ret;
>  	u8 val;
>  
> -	ade7754_spi_read_reg_8(dev, ADE7754_OPMODE, &val);
> +	ret = ade7754_spi_read_reg_8(dev, ADE7754_OPMODE, &val);
> +	if (ret < 0)
> +		return ret;
> +
>  	val |= 1 << 6; /* Software Chip Reset */
>  	return ade7754_spi_write_reg_8(dev, ADE7754_OPMODE, val);
>  }
> @@ -362,9 +366,16 @@ error_ret:
>  /* Power down the device */
>  static int ade7754_stop_device(struct device *dev)
>  {
> +	int ret;
>  	u8 val;
>  
> -	ade7754_spi_read_reg_8(dev, ADE7754_OPMODE, &val);
> +	ret = ade7754_spi_read_reg_8(dev, ADE7754_OPMODE, &val);
> +	if (ret < 0) {
> +		dev_err(dev, "unable to power down the device, error: %d",
> +			ret);
> +		return ret;
> +	}
> +
>  	val |= 7 << 3;  /* ADE7754 powered down */
>  	return ade7754_spi_write_reg_8(dev, ADE7754_OPMODE, val);
>  }
> 

Re: [PATCH V2 1/2] iio: meter: ade7754: add error handling in _reset and _stop_device

[devendra.aaru]

On Sun, Jan 4, 2015 at 6:06 AM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 02/01/15 09:02, Devendra Naga wrote:
>> This patch adds the error handling for the value returned from
>> ade7754_spi_read_reg_8. With this patch, the following randconfig
>> warnings get fixed automatically.
>>
>> drivers/staging/iio/meter/ade7754.c:222:6: warning: ‘val’ may be used
>> uninitialized in this function [-Wmaybe-uninitialized]
>> drivers/staging/iio/meter/ade7754.c:368:6: warning: ‘val’ may be used
>> uninitialized in this function [-Wmaybe-uninitialized]
>>
>> Signed-off-by: Devendra Naga <devendra.aaru@gmail.com>
> Looks good to me.  A Hartmut reviewed the original patch I'll leave it
> a few days to see if he wants to take a look at the update.
>
> Thanks,
>
> Jonathan
>> ---

Thanks for the review Jonathan.

Re: [PATCH V2 1/2] iio: meter: ade7754: add error handling in _reset and _stop_device

[Jonathan Cameron]

On 05/01/15 06:12, devendra.aaru wrote:
> On Sun, Jan 4, 2015 at 6:06 AM, Jonathan Cameron <jic23@kernel.org> wrote:
>> On 02/01/15 09:02, Devendra Naga wrote:
>>> This patch adds the error handling for the value returned from
>>> ade7754_spi_read_reg_8. With this patch, the following randconfig
>>> warnings get fixed automatically.
>>>
>>> drivers/staging/iio/meter/ade7754.c:222:6: warning: ‘val’ may be used
>>> uninitialized in this function [-Wmaybe-uninitialized]
>>> drivers/staging/iio/meter/ade7754.c:368:6: warning: ‘val’ may be used
>>> uninitialized in this function [-Wmaybe-uninitialized]
>>>
>>> Signed-off-by: Devendra Naga <devendra.aaru@gmail.com>
>> Looks good to me.  A Hartmut reviewed the original patch I'll leave it
>> a few days to see if he wants to take a look at the update.
>>
>> Thanks,
>>
>> Jonathan
>>> ---
> 
> Thanks for the review Jonathan.
Applied to the togreg branch of iio.git.  These have missed the
merge window for this cycle (sorry, I forgot about them!)

Anyhow, will get queued up for after the window closes.

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

Re: [PATCH V2 1/2] iio: meter: ade7754: add error handling in _reset and _stop_device

[sanjeev sharma]

On Fri, Jan 2, 2015 at 2:32 PM, Devendra Naga <devendra.aaru@gmail.com> wrote:
> This patch adds the error handling for the value returned from
> ade7754_spi_read_reg_8. With this patch, the following randconfig
> warnings get fixed automatically.
>
> drivers/staging/iio/meter/ade7754.c:222:6: warning: ‘val’ may be used
> uninitialized in this function [-Wmaybe-uninitialized]
> drivers/staging/iio/meter/ade7754.c:368:6: warning: ‘val’ may be used
> uninitialized in this function [-Wmaybe-uninitialized]
>
> Signed-off-by: Devendra Naga <devendra.aaru@gmail.com>
> ---
>
>  Hello,
>
>  Please see if this is the error message you wanted.
>
>  Build tested on next-20141231 with randconfig sent to iio ml,
>  and with allmodconfig.
>
>  drivers/staging/iio/meter/ade7754.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/iio/meter/ade7754.c b/drivers/staging/iio/meter/ade7754.c
> index 81f6731..746b188 100644
> --- a/drivers/staging/iio/meter/ade7754.c
> +++ b/drivers/staging/iio/meter/ade7754.c
> @@ -216,9 +216,13 @@ error_ret:
>
>  static int ade7754_reset(struct device *dev)
>  {
> +       int ret;
>         u8 val;
>
> -       ade7754_spi_read_reg_8(dev, ADE7754_OPMODE, &val);
> +       ret = ade7754_spi_read_reg_8(dev, ADE7754_OPMODE, &val);
> +       if (ret < 0)

IMO,we should have consistent change throughout and here also we
should use dev_err in failure scenario.
> +               return ret;
> +
>         val |= 1 << 6; /* Software Chip Reset */
>         return ade7754_spi_write_reg_8(dev, ADE7754_OPMODE, val);
>  }
> @@ -362,9 +366,16 @@ error_ret:
>  /* Power down the device */
>  static int ade7754_stop_device(struct device *dev)
>  {
> +       int ret;
>         u8 val;
>
> -       ade7754_spi_read_reg_8(dev, ADE7754_OPMODE, &val);
> +       ret = ade7754_spi_read_reg_8(dev, ADE7754_OPMODE, &val);
> +       if (ret < 0) {
> +               dev_err(dev, "unable to power down the device, error: %d",
> +                       ret);
> +               return ret;
> +       }
> +
>         val |= 7 << 3;  /* ADE7754 powered down */
>         return ade7754_spi_write_reg_8(dev, ADE7754_OPMODE, val);
>  }
> --
> 1.9.3
>
> _______________________________________________
> devel mailing list
> devel@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH V2 1/2] iio: meter: ade7754: add error handling in _reset and _stop_device

[Jonathan Cameron]

On 05/01/15 05:57, sanjeev sharma wrote:
> On Fri, Jan 2, 2015 at 2:32 PM, Devendra Naga <devendra.aaru@gmail.com> wrote:
>> This patch adds the error handling for the value returned from
>> ade7754_spi_read_reg_8. With this patch, the following randconfig
>> warnings get fixed automatically.
>>
>> drivers/staging/iio/meter/ade7754.c:222:6: warning: ‘val’ may be used
>> uninitialized in this function [-Wmaybe-uninitialized]
>> drivers/staging/iio/meter/ade7754.c:368:6: warning: ‘val’ may be used
>> uninitialized in this function [-Wmaybe-uninitialized]
>>
>> Signed-off-by: Devendra Naga <devendra.aaru@gmail.com>
>> ---
>>
>>  Hello,
>>
>>  Please see if this is the error message you wanted.
>>
>>  Build tested on next-20141231 with randconfig sent to iio ml,
>>  and with allmodconfig.
>>
>>  drivers/staging/iio/meter/ade7754.c | 15 +++++++++++++--
>>  1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/iio/meter/ade7754.c b/drivers/staging/iio/meter/ade7754.c
>> index 81f6731..746b188 100644
>> --- a/drivers/staging/iio/meter/ade7754.c
>> +++ b/drivers/staging/iio/meter/ade7754.c
>> @@ -216,9 +216,13 @@ error_ret:
>>
>>  static int ade7754_reset(struct device *dev)
>>  {
>> +       int ret;
>>         u8 val;
>>
>> -       ade7754_spi_read_reg_8(dev, ADE7754_OPMODE, &val);
>> +       ret = ade7754_spi_read_reg_8(dev, ADE7754_OPMODE, &val);
>> +       if (ret < 0)
> 
> IMO,we should have consistent change throughout and here also we
> should use dev_err in failure scenario.
That would perhaps be an additional improvement, but the patch as it
stands is still a good thing so applied.  Feel free to follow up
with additional debugging information.

>> +               return ret;
>> +
>>         val |= 1 << 6; /* Software Chip Reset */
>>         return ade7754_spi_write_reg_8(dev, ADE7754_OPMODE, val);
>>  }
>> @@ -362,9 +366,16 @@ error_ret:
>>  /* Power down the device */
>>  static int ade7754_stop_device(struct device *dev)
>>  {
>> +       int ret;
>>         u8 val;
>>
>> -       ade7754_spi_read_reg_8(dev, ADE7754_OPMODE, &val);
>> +       ret = ade7754_spi_read_reg_8(dev, ADE7754_OPMODE, &val);
>> +       if (ret < 0) {
>> +               dev_err(dev, "unable to power down the device, error: %d",
>> +                       ret);
>> +               return ret;
>> +       }
>> +
>>         val |= 7 << 3;  /* ADE7754 powered down */
>>         return ade7754_spi_write_reg_8(dev, ADE7754_OPMODE, val);
>>  }
>> --
>> 1.9.3
>>
>> _______________________________________________
>> devel mailing list
>> devel@linuxdriverproject.org
>> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
> --
> 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
>