On 24/10/16 16:22, Arnd Bergmann wrote: > The ad5933_i2c_read function returns an error code to indicate > whether it could read data or not. However ad5933_work() ignores > this return code and just accesses the data unconditionally, > which gets detected by gcc as a possible bug: > > drivers/staging/iio/impedance-analyzer/ad5933.c: In function 'ad5933_work': > drivers/staging/iio/impedance-analyzer/ad5933.c:649:16: warning: 'status' may be used uninitialized in this function [-Wmaybe-uninitialized] > > This adds minimal error handling so we only evaluate the > data if it was correctly read. > > Link: https://patchwork.kernel.org/patch/8110281/ > Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx> Looks good to me. Lars? > --- > v2: handle failure for both affected reads instead of just the one > we get a warning for > v2: do not retry on failure to avoid looping in case of permanent > errors > --- > drivers/staging/iio/impedance-analyzer/ad5933.c | 17 ++++++++++------- > 1 file changed, 10 insertions(+), 7 deletions(-) > > diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c b/drivers/staging/iio/impedance-analyzer/ad5933.c > index 5eecf1cb1028..3892a7470410 100644 > --- a/drivers/staging/iio/impedance-analyzer/ad5933.c > +++ b/drivers/staging/iio/impedance-analyzer/ad5933.c > @@ -655,6 +655,7 @@ static void ad5933_work(struct work_struct *work) > __be16 buf[2]; > int val[2]; > unsigned char status; > + int ret; > > mutex_lock(&indio_dev->mlock); > if (st->state == AD5933_CTRL_INIT_START_FREQ) { > @@ -662,19 +663,22 @@ static void ad5933_work(struct work_struct *work) > ad5933_cmd(st, AD5933_CTRL_START_SWEEP); > st->state = AD5933_CTRL_START_SWEEP; > schedule_delayed_work(&st->work, st->poll_time_jiffies); > - mutex_unlock(&indio_dev->mlock); > - return; > + goto out; > } > > - ad5933_i2c_read(st->client, AD5933_REG_STATUS, 1, &status); > + ret = ad5933_i2c_read(st->client, AD5933_REG_STATUS, 1, &status); > + if (ret) > + goto out; > > if (status & AD5933_STAT_DATA_VALID) { > int scan_count = bitmap_weight(indio_dev->active_scan_mask, > indio_dev->masklength); > - ad5933_i2c_read(st->client, > + ret = ad5933_i2c_read(st->client, > test_bit(1, indio_dev->active_scan_mask) ? > AD5933_REG_REAL_DATA : AD5933_REG_IMAG_DATA, > scan_count * 2, (u8 *)buf); > + if (ret) > + goto out; > > if (scan_count == 2) { > val[0] = be16_to_cpu(buf[0]); > @@ -686,8 +690,7 @@ static void ad5933_work(struct work_struct *work) > } else { > /* no data available - try again later */ > schedule_delayed_work(&st->work, st->poll_time_jiffies); > - mutex_unlock(&indio_dev->mlock); > - return; > + goto out; > } > > if (status & AD5933_STAT_SWEEP_DONE) { > @@ -700,7 +703,7 @@ static void ad5933_work(struct work_struct *work) > ad5933_cmd(st, AD5933_CTRL_INC_FREQ); > schedule_delayed_work(&st->work, st->poll_time_jiffies); > } > - > +out: > mutex_unlock(&indio_dev->mlock); > } > > -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
