From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752817AbcFKQcK (ORCPT ); Sat, 11 Jun 2016 12:32:10 -0400 Received: from saturn.retrosnub.co.uk ([178.18.118.26]:52312 "EHLO saturn.retrosnub.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752113AbcFKQcH (ORCPT ); Sat, 11 Jun 2016 12:32:07 -0400 Subject: Re: [PATCH] iio: as3935: improve error reporting in as3935_event_work To: "Andrew F. Davis" , Arnd Bergmann References: <1464619938-988956-1-git-send-email-arnd@arndb.de> <574DA582.50408@ti.com> Cc: Hartmut Knaack , Lars-Peter Clausen , Peter Meerwald-Stadler , Mark Brown , Javier Martinez Canillas , linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org From: Jonathan Cameron Message-ID: <75c2c65a-f1ac-8ad4-7f04-319939a1866a@kernel.org> Date: Sat, 11 Jun 2016 17:32:04 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.0 MIME-Version: 1.0 In-Reply-To: <574DA582.50408@ti.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >> --- >> 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 >