On Fri, 2 Mar 2018 19:04:49 +0530
Shreeya Patel <shreeya.patel23...@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 makes the code
> semantically more clear.
>
> Signed-off-by: Shreeya Patel <shreeya.patel23...@gmail.com>
Changes are fine, but 2 changes to different things should be
in 2 patches. Please split. I'd probably have taken this anyway if
it hadn't been queued behind the requested changes to the previous 2
patches anyway.
Good work on this series though so looking forward to v3.
Note please mark the set as [PATCH v3] to make it clear it
is the 3rd version of some of this.
> ---
> 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 7363fd0..5ab44a4 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; /* 0.30518 mV */
> - else
> + break;
> + case 1:
> *val2 = 610500; /* 0.6105 mV */
> + break;
> + default:
> + return -EINVAL;
> + }
> return IIO_VAL_INT_PLUS_MICRO;
> case IIO_TEMP:
> *val = -470;
> @@ -197,9 +203,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;
