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

[Jonathan Cameron <jic23@kernel.org>]

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
>