On Sun, 2019-11-03 at 10:41 +0000, Jonathan Cameron wrote: > On Fri, 1 Nov 2019 11:35:03 +0200 > Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx> wrote: > > > This was found only after the whole thing with the inline functions, > > but > > the compiler actually found something. The value of the `bias` (in > > adis16480_get_calibbias()) should only be set if the read operation was > > successful. > > > > Fixes: 2f3abe6cbb6c9 ("iio:imu: Add support for the ADIS16480 and > > similar IMUs") > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx> > > Hmm. It's not really a fix as such unless there is an in kernel consumer > that is using this and not checking the return value. I thought about > dropping > the fixes tag, but it is avoiding confusing the compiler so I suppose we > might > as well leave it marked as a fix. I will add a note that this doesn't > cause > any known real problems and so probably isn't stable material. Thanks. No idea if it's worth having a Fixes tag or not. I thought about adding it, since it seemed like a-bit-of-a-fix, and wasn't sure, so I added. I guess after upstreaming things a bit, I started wondering about these tags a bit more. Alex > > Applied. > > Thanks, > > Jonathan > > > --- > > drivers/iio/imu/adis16480.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c > > index b9e2695bcfad..c0e7e768be41 100644 > > --- a/drivers/iio/imu/adis16480.c > > +++ b/drivers/iio/imu/adis16480.c > > @@ -451,12 +451,14 @@ static int adis16480_get_calibbias(struct iio_dev > > *indio_dev, > > case IIO_MAGN: > > case IIO_PRESSURE: > > ret = adis_read_reg_16(&st->adis, reg, &val16); > > - *bias = sign_extend32(val16, 15); > > + if (ret == 0) > > + *bias = sign_extend32(val16, 15); > > break; > > case IIO_ANGL_VEL: > > case IIO_ACCEL: > > ret = adis_read_reg_32(&st->adis, reg, &val32); > > - *bias = sign_extend32(val32, 31); > > + if (ret == 0) > > + *bias = sign_extend32(val32, 31); > > break; > > default: > > ret = -EINVAL;