lkml.org 
[lkml]   [2016]   [Jun]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] iio: as3935: improve error reporting in as3935_event_work
From
Date
On 31/05/16 15:53, Andrew F. Davis wrote:
> On 05/30/2016 09:52 AM, Arnd Bergmann wrote:
>> gcc warns about a potentially uninitialized variable use
>> in as3935_event_work:
>>
>> drivers/iio/proximity/as3935.c: In function ‘as3935_event_work’:
>> drivers/iio/proximity/as3935.c:231:6: error: ‘val’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>
>> This case specifically happens when spi_w8r8() fails with a
>> negative return code. We check all other users of this function
>> except this one.
>>
>> As the error is rather unlikely to happen after the device
>> has already been initialized, this just adds a dev_warn().
>> Another warning already existst in the same function, but is
>
> ^^ typo
>
>> missing a trailing '\n' character, so I'm fixing that too.
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> ---
>> drivers/iio/proximity/as3935.c | 10 ++++++++--
>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iio/proximity/as3935.c b/drivers/iio/proximity/as3935.c
>> index f4d29d5dbd5f..b49e3ab5730a 100644
>> --- a/drivers/iio/proximity/as3935.c
>> +++ b/drivers/iio/proximity/as3935.c
>> @@ -224,10 +224,16 @@ static void as3935_event_work(struct work_struct *work)
>> {
>> struct as3935_state *st;
>> int val;
>> + int ret;
>>
>> st = container_of(work, struct as3935_state, work.work);
>>
>> - as3935_read(st, AS3935_INT, &val);
>> + ret = as3935_read(st, AS3935_INT, &val);
>> + if (ret) {
>> + dev_warn(&st->spi->dev, "read error\n");
>
> Maybe I'm misunderstanding the commit message, why does this error not
> use dev_err()? A read error here would be rather serious, it might even
> be worth it to return a code and fail through the caller too.
They are unusual and typically result in momentary corruption. Hmm.
As this is in a work function, there is no easy way of actually
passing the error upstream.. dev_err is a little brutal so perhaps
this is the best option...
>
>> + return;
>> + }
>> +
>> val &= AS3935_INT_MASK;
>>
>> switch (val) {
>> @@ -235,7 +241,7 @@ static void as3935_event_work(struct work_struct *work)
>> iio_trigger_poll(st->trig);
>> break;
>> case AS3935_NOISE_INT:
>> - dev_warn(&st->spi->dev, "noise level is too high");
>> + dev_warn(&st->spi->dev, "noise level is too high\n");
>> break;
>> }
>> }
>>
> --
> 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
>

\
 
 \ /
  Last update: 2016-06-11 19:01    [W:0.086 / U:0.084 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site