[PATCH 1/2] iio: meter: ade7754: fix build warnings with make randconfig

[PATCH 1/2] iio: meter: ade7754: fix build warnings with make randconfig

[Devendra Naga]

fixes

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]

the fix here is to check the return value of ade7754_spi_read_reg_8.

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

 hapens on next-20141215 with make randconfig. compile tested only on x86_64.

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

diff --git a/drivers/staging/iio/meter/ade7754.c b/drivers/staging/iio/meter/ade7754.c
index 81f6731..9e71575 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,13 @@ 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)
+		return ret;
+
 	val |= 7 << 3;  /* ADE7754 powered down */
 	return ade7754_spi_write_reg_8(dev, ADE7754_OPMODE, val);
 }
-- 
2.1.0

[PATCH 2/2] iio: meter: ade7759: fix build warnings with make randconfig

[Devendra Naga]

fixes

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]

the fix is to check the return value of ade7759_spi_read_reg_16.

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

 hapens on next-20141215 with make randconfig. compile tested only on x86_64.

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

diff --git a/drivers/staging/iio/meter/ade7759.c b/drivers/staging/iio/meter/ade7759.c
index 7d21743..aa10042 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,15 @@ 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)
+		return ret;
+
 	val |= 1 << 4;  /* AD converters can be turned off */
 
 	return ade7759_spi_write_reg_16(dev, ADE7759_MODE, val);
-- 
2.1.0

Re: [PATCH 1/2] iio: meter: ade7754: fix build warnings with make randconfig

[Hartmut Knaack]

Devendra Naga schrieb am 15.12.2014 um 12:46:
> fixes
> 
> 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]
> 
> the fix here is to check the return value of ade7754_spi_read_reg_8.
Hi,
I would recommend to use a different topic name, which represents a bit
better what your patch actually does (something like add error handling
in _reset and _stop_device).
Your attempt is basically right, but in _stop_device I would at least
recommend to add an error message, given that it is only called from
_remove, which does no error handling. So, at least it becomes clear
that something went wrong and the device didn't get powered down.
Since your second patch is very similar to this one, the same applies
there, too.
Thanks,

Hartmut
> 
> Signed-off-by: Devendra Naga <devendra.aaru@gmail.com>
> ---
> 
>  hapens on next-20141215 with make randconfig. compile tested only on x86_64.
> 
>  drivers/staging/iio/meter/ade7754.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/iio/meter/ade7754.c b/drivers/staging/iio/meter/ade7754.c
> index 81f6731..9e71575 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,13 @@ 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)
> +		return ret;
> +
>  	val |= 7 << 3;  /* ADE7754 powered down */
>  	return ade7754_spi_write_reg_8(dev, ADE7754_OPMODE, val);
>  }
> 

Re: [PATCH 1/2] iio: meter: ade7754: fix build warnings with make randconfig

[devendra.aaru]

[-- Attachment #1: Type: text/plain, Size: 1288 bytes --]

Hello,

Sorry for sending the email in the holidays...

On Thu Dec 18 2014 at 4:06:45 PM Hartmut Knaack <knaack.h@gmx.de> wrote:

> Devendra Naga schrieb am 15.12.2014 um 12:46:
> > fixes
> >
> > 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]
> >
> > the fix here is to check the return value of ade7754_spi_read_reg_8.
> Hi,
> I would recommend to use a different topic name, which represents a bit
> better what your patch actually does (something like add error handling
> in _reset and _stop_device).


Okay. Got you.


>

Your attempt is basically right, but in _stop_device I would at least
> recommend to add an error message, given that it is only called from
> _remove, which does no error handling. So, at least it becomes clear
> that something went wrong and the device didn't get powered down.
> Since your second patch is very similar to this one, the same applies
> there, too.
> Thanks,
>
>
Okay. Will add the error message in _stop_device.

I will update both of the patches with your comments.

-- Devendra

[-- Attachment #2: Type: text/html, Size: 1903 bytes --]