Skip to contentSkip to conversation

iio: meter: ade7754: add error handling in _reset and _stop_device

PATCH Applied Devendra Naga 3 participants 8 messages Jan 2, 2015 – Feb 1, 2015

Subsystems (4)
Staging IIO Analog Devices IIO drivers Staging - industrial io
lore
Share & tools
lore

Devendra Naga

author
#0 lore raw

PATCH v2 1/2 iio: meter: ade7754: add error handling in _reset and _stop_device

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 <[email protected]>

---

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(-)
drivers/staging/iio/meter/ade7754.c +13 -2
index 81f6731..746b188 100644
@@ -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);
 }
Signature · 1 line
1.9.3
author
#1 lore raw

PATCH v2 2/2 iio: meter: ade7759: add error handling in _reset and _stop_device

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 <[email protected]>

---
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(-)
drivers/staging/iio/meter/ade7759.c +12 -2
index 7d21743..86b5145 100644
@@ -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);
Signature · 1 line
1.9.3
maintainer Applied
#2 lore raw

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

Quoted 10 lines · 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 <[email protected]>

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

Quoted 50 lines

---
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(-)
drivers/staging/iio/meter/ade7759.c +12 -2
index 7d21743..86b5145 100644
@@ -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);
maintainer
#3 lore raw
In reply to Devendra Naga
Quoted 10 lines · 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 <[email protected]>

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

Quoted 50 lines

---

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(-)
drivers/staging/iio/meter/ade7754.c +13 -2
index 81f6731..746b188 100644
@@ -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);
 }
author
#4 lore raw
Quoted 10 lines · Jonathan Cameron
Quoted 9 lines · 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]

Quoted 8 lines
Quoted 1 line
Signed-off-by Devendra Naga <[email protected]>

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

Quoted 1 line

---

Thanks for the review Jonathan.

maintainer Applied
#5 lore raw
Quoted 15 lines · devendra.aaru
Quoted 14 lines · Jonathan Cameron
Quoted 10 lines · 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 <[email protected]>

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.

Quoted 6 lines
Quoted 4 lines

Thanks,

Jonathan

Quoted 1 line

---

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

Quoted 5 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#6 lore raw
In reply to Devendra Naga
Quoted 27 lines · 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 <[email protected]>

---

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(-)
drivers/staging/iio/meter/ade7754.c +0 -0
index 81f6731..746b188 100644

drivers/staging/iio/meter/ade7754.c error_ret: · line 223

Quoted 9 lines
drivers/staging/iio/meter/ade7754.c +3 -1
@@ -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.

Quoted 30 lines
drivers/staging/iio/meter/ade7754.c +10 -2
+               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
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

maintainer
#7 lore raw
Quoted 34 lines · sanjeev sharma
Quoted 33 lines · 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 <[email protected]>

---

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(-)
drivers/staging/iio/meter/ade7754.c +1 -0
index 81f6731..746b188 100644
@@ -216,9 +216,13 @@ error_ret:
 
 static int ade7754_reset(struct device *dev)
 {
+       int ret;
        u8 val;
Quoted 6 lines

drivers/staging/iio/meter/ade7754.c error_ret: · line 223

Quoted 3 lines
drivers/staging/iio/meter/ade7754.c +2 -1
-       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.

Quoted 35 lines
Quoted 26 lines
drivers/staging/iio/meter/ade7754.c +10 -2
+               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

Quoted 4 lines

_______________________________________________
devel mailing list
[email protected]
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html