| Message ID | 317f97b7a3b53bf3ae1457d7c38f78befc4b99db.1518883035.git.shreeya.patel23498@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, 17 Feb 2018 21:51:35 +0530 Shreeya Patel <shreeya.patel23498@gmail.com> wrote: > Use sign_extend32 function instead of manually coding it. Also, adjust a > switch block to explicitly match channels and return -EINVAL as default > case which improves code readability. Two items, two patches please. The -EINVAL is a bit of an odd one. We know it can never happen and in reality we only add it to suppress the static checker warnings that may otherwise ensue. I would definitely not say that it helps readability. The change to the switch statement makes it clear there are only two channels so is semantically clearer to my mind and it is that which I argued improved readability. Jonathan > > Signed-off-by: Shreeya Patel <shreeya.patel23498@gmail.com> > --- > drivers/staging/iio/accel/adis16209.c | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/drivers/staging/iio/accel/adis16209.c b/drivers/staging/iio/accel/adis16209.c > index 6101212..f7c228b 100644 > --- a/drivers/staging/iio/accel/adis16209.c > +++ b/drivers/staging/iio/accel/adis16209.c > @@ -150,10 +150,16 @@ static int adis16209_read_raw(struct iio_dev *indio_dev, > switch (chan->type) { > case IIO_VOLTAGE: > *val = 0; > - if (chan->channel == 0) > + switch (chan->channel) { > + case 0: > *val2 = 305180; > - else > + break; > + case 1: > *val2 = 610500; > + break; > + default: > + return -EINVAL; > + } > return IIO_VAL_INT_PLUS_MICRO; > case IIO_TEMP: > *val = -470; > @@ -187,9 +193,8 @@ static int adis16209_read_raw(struct iio_dev *indio_dev, > ret = adis_read_reg_16(st, addr, &val16); > if (ret) > return ret; > - val16 &= (1 << bits) - 1; > - val16 = (s16)(val16 << (16 - bits)) >> (16 - bits); > - *val = val16; > + > + *val = sign_extend32(val16, bits - 1); > return IIO_VAL_INT; > } > return -EINVAL; -- 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
diff --git a/drivers/staging/iio/accel/adis16209.c b/drivers/staging/iio/accel/adis16209.c index 6101212..f7c228b 100644 --- a/drivers/staging/iio/accel/adis16209.c +++ b/drivers/staging/iio/accel/adis16209.c @@ -150,10 +150,16 @@ static int adis16209_read_raw(struct iio_dev *indio_dev, switch (chan->type) { case IIO_VOLTAGE: *val = 0; - if (chan->channel == 0) + switch (chan->channel) { + case 0: *val2 = 305180; - else + break; + case 1: *val2 = 610500; + break; + default: + return -EINVAL; + } return IIO_VAL_INT_PLUS_MICRO; case IIO_TEMP: *val = -470; @@ -187,9 +193,8 @@ static int adis16209_read_raw(struct iio_dev *indio_dev, ret = adis_read_reg_16(st, addr, &val16); if (ret) return ret; - val16 &= (1 << bits) - 1; - val16 = (s16)(val16 << (16 - bits)) >> (16 - bits); - *val = val16; + + *val = sign_extend32(val16, bits - 1); return IIO_VAL_INT; } return -EINVAL;
Use sign_extend32 function instead of manually coding it. Also, adjust a switch block to explicitly match channels and return -EINVAL as default case which improves code readability. Signed-off-by: Shreeya Patel <shreeya.patel23498@gmail.com> --- drivers/staging/iio/accel/adis16209.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)