| Message ID | 20211101071822.522178-2-gwendal@chromium.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | iio: Use scan_type shift and realbits when processing raw data | expand |
On Mon, 1 Nov 2021 00:18:19 -0700 Gwendal Grignou <gwendal@chromium.org> wrote: > When user space application read iio buffer though libiio, data is > converted (see iio_channel_convert()) using the _type sysfs parameter. > In particular, scan_type.shift and scan_type.realbits are used to shift > and tell on how many bits signed elements are encoded on. > > When reading elements directly using the raw sysfs attributes, the same > rules for shifting and signing should apply. > > Use channel definition as root of trust and replace constant with > them for the simple cases. > > Signed-off-by: Gwendal Grignou <gwendal@chromium.org> Hmm. I'd have been tempted to do this as a long series, at least partly so it would let me pick up the ones I'm happy with + also we may create some history that needs backporting at some stage and that's a mess if we have code touching lots of drivers in one patch. Ludovic, Eugen, This change raised a question about the current code in at91-sama5d2_adc.c > --- > drivers/iio/accel/bma220_spi.c | 3 ++- > drivers/iio/accel/kxcjk-1013.c | 3 ++- > drivers/iio/accel/mma7455_core.c | 3 ++- > drivers/iio/accel/sca3000.c | 5 +++-- > drivers/iio/accel/stk8312.c | 2 +- > drivers/iio/accel/stk8ba50.c | 3 ++- > drivers/iio/adc/ad7266.c | 3 ++- > drivers/iio/adc/at91-sama5d2_adc.c | 3 ++- > drivers/iio/adc/ti-adc12138.c | 3 ++- > drivers/iio/magnetometer/mag3110.c | 6 ++++-- > 10 files changed, 22 insertions(+), 12 deletions(-) > > diff --git a/drivers/iio/accel/bma220_spi.c b/drivers/iio/accel/bma220_spi.c > index bc4c626e454d3..812d6749b24a7 100644 > --- a/drivers/iio/accel/bma220_spi.c > +++ b/drivers/iio/accel/bma220_spi.c > @@ -125,7 +125,8 @@ static int bma220_read_raw(struct iio_dev *indio_dev, > ret = bma220_read_reg(data->spi_device, chan->address); > if (ret < 0) > return -EINVAL; > - *val = sign_extend32(ret >> BMA220_DATA_SHIFT, 5); > + *val = sign_extend32(ret >> chan->scan_type.shift, > + chan->scan_type.realbits - 1); The BMA220_DATA_SHIFT define is now only used as the value for .shift, so could you move the value inline to there and get rid of the define. That will be match what is done for all the other parts of scan_type. > return IIO_VAL_INT; > case IIO_CHAN_INFO_SCALE: > ret = bma220_read_reg(data->spi_device, BMA220_REG_RANGE); > diff --git a/drivers/iio/accel/sca3000.c b/drivers/iio/accel/sca3000.c > index c6b75308148aa..938eb6bda73b3 100644 > --- a/drivers/iio/accel/sca3000.c > +++ b/drivers/iio/accel/sca3000.c > @@ -730,8 +730,9 @@ static int sca3000_read_raw(struct iio_dev *indio_dev, > mutex_unlock(&st->lock); > return ret; > } > - *val = (be16_to_cpup((__be16 *)st->rx) >> 3) & 0x1FFF; > - *val = sign_extend32(*val, 12); > + *val = (be16_to_cpup((__be16 *)st->rx) >> > + chan->scan_type.shift) & 0x1FFF; While here, GENMASK(12, 0) for the mask might be a nice to have.. > + *val = sign_extend32(*val, chan->scan_type.realbits - 1); > } else { > /* get the temperature when available */ > ret = sca3000_read_data_short(st, The code following this is exciting as well... and would benefit from being rewritten as be16_to_cpup() with a shift and mask but that's a job for a different patch, or you could add the scan_type to the temperature channel and add it to this series using that... It's unsigned unlike the above, so it probably doesn't make sense to have a single path. > diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c > index 4c922ef634f8e..92a57cf10fba4 100644 > --- a/drivers/iio/adc/at91-sama5d2_adc.c > +++ b/drivers/iio/adc/at91-sama5d2_adc.c > @@ -1586,7 +1586,8 @@ static int at91_adc_read_info_raw(struct iio_dev *indio_dev, > *val = st->conversion_value; > ret = at91_adc_adjust_val_osr(st, val); > if (chan->scan_type.sign == 's') > - *val = sign_extend32(*val, 11); > + *val = sign_extend32(*val, > + chan->scan_type.realbits - 1); > st->conversion_done = false; Hmm. This is exciting. I'm not sure if the current code is correct. There is a comment that says it's a voltage channel if we are in this path a few lines earlier, yet the only signed voltage channel is from the macro AT91_SAMA5D2_CHAN_DIFF() which sets realbits = 14, not 12. > } > The other changes all looked good to me. Jonathan
On Wed, Nov 3, 2021 at 11:37 AM Jonathan Cameron <jic23@kernel.org> wrote: > > On Mon, 1 Nov 2021 00:18:19 -0700 > Gwendal Grignou <gwendal@chromium.org> wrote: > > > When user space application read iio buffer though libiio, data is > > converted (see iio_channel_convert()) using the _type sysfs parameter. > > In particular, scan_type.shift and scan_type.realbits are used to shift > > and tell on how many bits signed elements are encoded on. > > > > When reading elements directly using the raw sysfs attributes, the same > > rules for shifting and signing should apply. > > > > Use channel definition as root of trust and replace constant with > > them for the simple cases. > > > > Signed-off-by: Gwendal Grignou <gwendal@chromium.org> > > Hmm. I'd have been tempted to do this as a long series, at least > partly so it would let me pick up the ones I'm happy with + also > we may create some history that needs backporting at some stage > and that's a mess if we have code touching lots of drivers in one patch. I will split this patch, one per driver so that you can pick only the safe changes only. There are many more drivers that would benefit from using scan_type, but they require more restructuring. > > Ludovic, Eugen, This change raised a question about the current > code in at91-sama5d2_adc.c > > > --- > > drivers/iio/accel/bma220_spi.c | 3 ++- > > drivers/iio/accel/kxcjk-1013.c | 3 ++- > > drivers/iio/accel/mma7455_core.c | 3 ++- > > drivers/iio/accel/sca3000.c | 5 +++-- > > drivers/iio/accel/stk8312.c | 2 +- > > drivers/iio/accel/stk8ba50.c | 3 ++- > > drivers/iio/adc/ad7266.c | 3 ++- > > drivers/iio/adc/at91-sama5d2_adc.c | 3 ++- > > drivers/iio/adc/ti-adc12138.c | 3 ++- > > drivers/iio/magnetometer/mag3110.c | 6 ++++-- > > 10 files changed, 22 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/iio/accel/bma220_spi.c b/drivers/iio/accel/bma220_spi.c > > index bc4c626e454d3..812d6749b24a7 100644 > > --- a/drivers/iio/accel/bma220_spi.c > > +++ b/drivers/iio/accel/bma220_spi.c > > @@ -125,7 +125,8 @@ static int bma220_read_raw(struct iio_dev *indio_dev, > > ret = bma220_read_reg(data->spi_device, chan->address); > > if (ret < 0) > > return -EINVAL; > > - *val = sign_extend32(ret >> BMA220_DATA_SHIFT, 5); > > + *val = sign_extend32(ret >> chan->scan_type.shift, > > + chan->scan_type.realbits - 1); > > The BMA220_DATA_SHIFT define is now only used as the value for .shift, so > could you move the value inline to there and get rid of the define. > > That will be match what is done for all the other parts of scan_type. Done. > > > return IIO_VAL_INT; > > case IIO_CHAN_INFO_SCALE: > > ret = bma220_read_reg(data->spi_device, BMA220_REG_RANGE); > > > > diff --git a/drivers/iio/accel/sca3000.c b/drivers/iio/accel/sca3000.c > > index c6b75308148aa..938eb6bda73b3 100644 > > --- a/drivers/iio/accel/sca3000.c > > +++ b/drivers/iio/accel/sca3000.c > > @@ -730,8 +730,9 @@ static int sca3000_read_raw(struct iio_dev *indio_dev, > > mutex_unlock(&st->lock); > > return ret; > > } > > - *val = (be16_to_cpup((__be16 *)st->rx) >> 3) & 0x1FFF; > > - *val = sign_extend32(*val, 12); > > + *val = (be16_to_cpup((__be16 *)st->rx) >> > > + chan->scan_type.shift) & 0x1FFF; > > While here, GENMASK(12, 0) for the mask might be a nice to have.. Actually, it may not be needed at all: we push a 16bit field 3 bits to the left, so 13 bits remain anyway. > > > + *val = sign_extend32(*val, chan->scan_type.realbits - 1); > > } else { > > /* get the temperature when available */ > > ret = sca3000_read_data_short(st, > The code following this is exciting as well... and would benefit from > being rewritten as be16_to_cpup() with a shift and mask but that's a job for > a different patch, or you could add the scan_type to the temperature channel and > add it to this series using that... It's unsigned unlike the above, so > it probably doesn't make sense to have a single path. Done > > > > > > diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c > > index 4c922ef634f8e..92a57cf10fba4 100644 > > --- a/drivers/iio/adc/at91-sama5d2_adc.c > > +++ b/drivers/iio/adc/at91-sama5d2_adc.c > > @@ -1586,7 +1586,8 @@ static int at91_adc_read_info_raw(struct iio_dev *indio_dev, > > *val = st->conversion_value; > > ret = at91_adc_adjust_val_osr(st, val); > > if (chan->scan_type.sign == 's') > > - *val = sign_extend32(*val, 11); > > + *val = sign_extend32(*val, > > + chan->scan_type.realbits - 1); > > st->conversion_done = false; > Hmm. This is exciting. I'm not sure if the current code is correct. > There is a comment that says it's a voltage channel if we are in this path > a few lines earlier, yet the only signed voltage channel is from the > macro AT91_SAMA5D2_CHAN_DIFF() which sets realbits = 14, not 12. From https://ww1.microchip.com/downloads/en/DeviceDoc/DS60001476B.pdf, the ADC is natively 12 bits, but can be configured to oversample to reach 14 bits resolution. |realbit| may actually be a function of IIO_CHAN_INFO_OVERSAMPLING_RATIO [aka "oversampling_ratio"] value. > > > > } > > > > The other changes all looked good to me. Thanks, Gwendal. > > Jonathan >
On 11/4/21 10:24 AM, Gwendal Grignou wrote: > On Wed, Nov 3, 2021 at 11:37 AM Jonathan Cameron <jic23@kernel.org> wrote: >> >> On Mon, 1 Nov 2021 00:18:19 -0700 >> Gwendal Grignou <gwendal@chromium.org> wrote: >> >>> When user space application read iio buffer though libiio, data is >>> converted (see iio_channel_convert()) using the _type sysfs parameter. >>> In particular, scan_type.shift and scan_type.realbits are used to shift >>> and tell on how many bits signed elements are encoded on. >>> >>> When reading elements directly using the raw sysfs attributes, the same >>> rules for shifting and signing should apply. >>> >>> Use channel definition as root of trust and replace constant with >>> them for the simple cases. >>> >>> Signed-off-by: Gwendal Grignou <gwendal@chromium.org> >> >> Hmm. I'd have been tempted to do this as a long series, at least >> partly so it would let me pick up the ones I'm happy with + also >> we may create some history that needs backporting at some stage >> and that's a mess if we have code touching lots of drivers in one patch. > I will split this patch, one per driver so that you can pick only the > safe changes only. > There are many more drivers that would benefit from using scan_type, > but they require more restructuring. > >> >> Ludovic, Eugen, This change raised a question about the current >> code in at91-sama5d2_adc.c >> >>> --- >>> drivers/iio/accel/bma220_spi.c | 3 ++- >>> drivers/iio/accel/kxcjk-1013.c | 3 ++- >>> drivers/iio/accel/mma7455_core.c | 3 ++- >>> drivers/iio/accel/sca3000.c | 5 +++-- >>> drivers/iio/accel/stk8312.c | 2 +- >>> drivers/iio/accel/stk8ba50.c | 3 ++- >>> drivers/iio/adc/ad7266.c | 3 ++- >>> drivers/iio/adc/at91-sama5d2_adc.c | 3 ++- >>> drivers/iio/adc/ti-adc12138.c | 3 ++- >>> drivers/iio/magnetometer/mag3110.c | 6 ++++-- >>> 10 files changed, 22 insertions(+), 12 deletions(-) >>> >>> diff --git a/drivers/iio/accel/bma220_spi.c b/drivers/iio/accel/bma220_spi.c >>> index bc4c626e454d3..812d6749b24a7 100644 >>> --- a/drivers/iio/accel/bma220_spi.c >>> +++ b/drivers/iio/accel/bma220_spi.c >>> @@ -125,7 +125,8 @@ static int bma220_read_raw(struct iio_dev *indio_dev, >>> ret = bma220_read_reg(data->spi_device, chan->address); >>> if (ret < 0) >>> return -EINVAL; >>> - *val = sign_extend32(ret >> BMA220_DATA_SHIFT, 5); >>> + *val = sign_extend32(ret >> chan->scan_type.shift, >>> + chan->scan_type.realbits - 1); >> >> The BMA220_DATA_SHIFT define is now only used as the value for .shift, so >> could you move the value inline to there and get rid of the define. >> >> That will be match what is done for all the other parts of scan_type. > Done. >> >>> return IIO_VAL_INT; >>> case IIO_CHAN_INFO_SCALE: >>> ret = bma220_read_reg(data->spi_device, BMA220_REG_RANGE); >> >> >>> diff --git a/drivers/iio/accel/sca3000.c b/drivers/iio/accel/sca3000.c >>> index c6b75308148aa..938eb6bda73b3 100644 >>> --- a/drivers/iio/accel/sca3000.c >>> +++ b/drivers/iio/accel/sca3000.c >>> @@ -730,8 +730,9 @@ static int sca3000_read_raw(struct iio_dev *indio_dev, >>> mutex_unlock(&st->lock); >>> return ret; >>> } >>> - *val = (be16_to_cpup((__be16 *)st->rx) >> 3) & 0x1FFF; >>> - *val = sign_extend32(*val, 12); >>> + *val = (be16_to_cpup((__be16 *)st->rx) >> >>> + chan->scan_type.shift) & 0x1FFF; >> >> While here, GENMASK(12, 0) for the mask might be a nice to have.. > Actually, it may not be needed at all: we push a 16bit field 3 bits to > the left, so 13 bits remain anyway. >> >>> + *val = sign_extend32(*val, chan->scan_type.realbits - 1); >>> } else { >>> /* get the temperature when available */ >>> ret = sca3000_read_data_short(st, >> The code following this is exciting as well... and would benefit from >> being rewritten as be16_to_cpup() with a shift and mask but that's a job for >> a different patch, or you could add the scan_type to the temperature channel and >> add it to this series using that... It's unsigned unlike the above, so >> it probably doesn't make sense to have a single path. > Done >> >> >> >> >>> diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c >>> index 4c922ef634f8e..92a57cf10fba4 100644 >>> --- a/drivers/iio/adc/at91-sama5d2_adc.c >>> +++ b/drivers/iio/adc/at91-sama5d2_adc.c >>> @@ -1586,7 +1586,8 @@ static int at91_adc_read_info_raw(struct iio_dev *indio_dev, >>> *val = st->conversion_value; >>> ret = at91_adc_adjust_val_osr(st, val); >>> if (chan->scan_type.sign == 's') >>> - *val = sign_extend32(*val, 11); >>> + *val = sign_extend32(*val, >>> + chan->scan_type.realbits - 1); >>> st->conversion_done = false; >> Hmm. This is exciting. I'm not sure if the current code is correct. >> There is a comment that says it's a voltage channel if we are in this path >> a few lines earlier, yet the only signed voltage channel is from the >> macro AT91_SAMA5D2_CHAN_DIFF() which sets realbits = 14, not 12. > From https://ww1.microchip.com/downloads/en/DeviceDoc/DS60001476B.pdf, > the ADC is natively 12 bits, but can be configured to oversample to > reach 14 bits resolution. > |realbit| may actually be a function of > IIO_CHAN_INFO_OVERSAMPLING_RATIO [aka "oversampling_ratio"] value. >> Hi, Ever since the oversampling was implemented, the realbits was changed to 14. This means that if oversampling is enabled, the data will have 14 bits of precision. If oversampling is disabled, it will have 12 bits of useful data, but it will be shifted to the left by 2 bits, so in the end the result will be 14 bits, but with LSB 2 bits always zero. ( this is what at91_adc_adjust_val_osr does ) So I think that maybe a fix for this commit should be done first : 6794e23fa3fe ("iio: adc: at91-sama5d2_adc: add support for oversampling resolution") which does not change the '11' value to '13' and then you can apply your commit that starts using the 'realbits' field. So in fact, another bit was used to obtain the sign.. I wonder if someone uses these differential channels since I haven't heard of any problem reported. And during my tests it looked to be fine, but maybe it was a bit coincidence. If I get my hands on a signal generator I will try to make a differential GND to a sinus waveform and plot the results to see if it works as expected Nice catch and thanks, Eugen >> >>> } >>> >> >> The other changes all looked good to me. > Thanks, > Gwendal. >> >> Jonathan >>
diff --git a/drivers/iio/accel/bma220_spi.c b/drivers/iio/accel/bma220_spi.c index bc4c626e454d3..812d6749b24a7 100644 --- a/drivers/iio/accel/bma220_spi.c +++ b/drivers/iio/accel/bma220_spi.c @@ -125,7 +125,8 @@ static int bma220_read_raw(struct iio_dev *indio_dev, ret = bma220_read_reg(data->spi_device, chan->address); if (ret < 0) return -EINVAL; - *val = sign_extend32(ret >> BMA220_DATA_SHIFT, 5); + *val = sign_extend32(ret >> chan->scan_type.shift, + chan->scan_type.realbits - 1); return IIO_VAL_INT; case IIO_CHAN_INFO_SCALE: ret = bma220_read_reg(data->spi_device, BMA220_REG_RANGE); diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c index a51fdd3c9b5b5..88cf0c276893a 100644 --- a/drivers/iio/accel/kxcjk-1013.c +++ b/drivers/iio/accel/kxcjk-1013.c @@ -927,7 +927,8 @@ static int kxcjk1013_read_raw(struct iio_dev *indio_dev, mutex_unlock(&data->mutex); return ret; } - *val = sign_extend32(ret >> 4, 11); + *val = sign_extend32(ret >> chan->scan_type.shift, + chan->scan_type.realbits - 1); ret = kxcjk1013_set_power_state(data, false); } mutex_unlock(&data->mutex); diff --git a/drivers/iio/accel/mma7455_core.c b/drivers/iio/accel/mma7455_core.c index 777c6c384b09e..e6739ba74edfa 100644 --- a/drivers/iio/accel/mma7455_core.c +++ b/drivers/iio/accel/mma7455_core.c @@ -134,7 +134,8 @@ static int mma7455_read_raw(struct iio_dev *indio_dev, if (ret) return ret; - *val = sign_extend32(le16_to_cpu(data), 9); + *val = sign_extend32(le16_to_cpu(data), + chan->scan_type.realbits - 1); return IIO_VAL_INT; diff --git a/drivers/iio/accel/sca3000.c b/drivers/iio/accel/sca3000.c index c6b75308148aa..938eb6bda73b3 100644 --- a/drivers/iio/accel/sca3000.c +++ b/drivers/iio/accel/sca3000.c @@ -730,8 +730,9 @@ static int sca3000_read_raw(struct iio_dev *indio_dev, mutex_unlock(&st->lock); return ret; } - *val = (be16_to_cpup((__be16 *)st->rx) >> 3) & 0x1FFF; - *val = sign_extend32(*val, 12); + *val = (be16_to_cpup((__be16 *)st->rx) >> + chan->scan_type.shift) & 0x1FFF; + *val = sign_extend32(*val, chan->scan_type.realbits - 1); } else { /* get the temperature when available */ ret = sca3000_read_data_short(st, diff --git a/drivers/iio/accel/stk8312.c b/drivers/iio/accel/stk8312.c index 43c621d0f11e4..de0cdf8c1f94c 100644 --- a/drivers/iio/accel/stk8312.c +++ b/drivers/iio/accel/stk8312.c @@ -355,7 +355,7 @@ static int stk8312_read_raw(struct iio_dev *indio_dev, mutex_unlock(&data->lock); return ret; } - *val = sign_extend32(ret, 7); + *val = sign_extend32(ret, chan->scan_type.realbits - 1); ret = stk8312_set_mode(data, data->mode & (~STK8312_MODE_ACTIVE)); mutex_unlock(&data->lock); diff --git a/drivers/iio/accel/stk8ba50.c b/drivers/iio/accel/stk8ba50.c index e137a34b5c9a9..517c57ed9e949 100644 --- a/drivers/iio/accel/stk8ba50.c +++ b/drivers/iio/accel/stk8ba50.c @@ -227,7 +227,8 @@ static int stk8ba50_read_raw(struct iio_dev *indio_dev, mutex_unlock(&data->lock); return -EINVAL; } - *val = sign_extend32(ret >> STK8BA50_DATA_SHIFT, 9); + *val = sign_extend32(ret >> chan->scan_type.shift, + chan->scan_type.realbits - 1); stk8ba50_set_power(data, STK8BA50_MODE_SUSPEND); mutex_unlock(&data->lock); return IIO_VAL_INT; diff --git a/drivers/iio/adc/ad7266.c b/drivers/iio/adc/ad7266.c index a8ec3efd659ed..1d345d66742d8 100644 --- a/drivers/iio/adc/ad7266.c +++ b/drivers/iio/adc/ad7266.c @@ -159,7 +159,8 @@ static int ad7266_read_raw(struct iio_dev *indio_dev, *val = (*val >> 2) & 0xfff; if (chan->scan_type.sign == 's') - *val = sign_extend32(*val, 11); + *val = sign_extend32(*val, + chan->scan_type.realbits - 1); return IIO_VAL_INT; case IIO_CHAN_INFO_SCALE: diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c index 4c922ef634f8e..92a57cf10fba4 100644 --- a/drivers/iio/adc/at91-sama5d2_adc.c +++ b/drivers/iio/adc/at91-sama5d2_adc.c @@ -1586,7 +1586,8 @@ static int at91_adc_read_info_raw(struct iio_dev *indio_dev, *val = st->conversion_value; ret = at91_adc_adjust_val_osr(st, val); if (chan->scan_type.sign == 's') - *val = sign_extend32(*val, 11); + *val = sign_extend32(*val, + chan->scan_type.realbits - 1); st->conversion_done = false; } diff --git a/drivers/iio/adc/ti-adc12138.c b/drivers/iio/adc/ti-adc12138.c index fcd5d39dd03ea..5b5d452105393 100644 --- a/drivers/iio/adc/ti-adc12138.c +++ b/drivers/iio/adc/ti-adc12138.c @@ -239,7 +239,8 @@ static int adc12138_read_raw(struct iio_dev *iio, if (ret) return ret; - *value = sign_extend32(be16_to_cpu(data) >> 3, 12); + *value = sign_extend32(be16_to_cpu(data) >> channel->scan_type.shift, + channel->scan_type.realbits - 1); return IIO_VAL_INT; case IIO_CHAN_INFO_SCALE: diff --git a/drivers/iio/magnetometer/mag3110.c b/drivers/iio/magnetometer/mag3110.c index c96415a1aeadd..17c62d806218d 100644 --- a/drivers/iio/magnetometer/mag3110.c +++ b/drivers/iio/magnetometer/mag3110.c @@ -291,7 +291,8 @@ static int mag3110_read_raw(struct iio_dev *indio_dev, if (ret < 0) goto release; *val = sign_extend32( - be16_to_cpu(buffer[chan->scan_index]), 15); + be16_to_cpu(buffer[chan->scan_index]), + chan->scan_type.realbits - 1); ret = IIO_VAL_INT; break; case IIO_TEMP: /* in 1 C / LSB */ @@ -306,7 +307,8 @@ static int mag3110_read_raw(struct iio_dev *indio_dev, mutex_unlock(&data->lock); if (ret < 0) goto release; - *val = sign_extend32(ret, 7); + *val = sign_extend32(ret, + chan->scan_type.realbits - 1); ret = IIO_VAL_INT; break; default:
When user space application read iio buffer though libiio, data is converted (see iio_channel_convert()) using the _type sysfs parameter. In particular, scan_type.shift and scan_type.realbits are used to shift and tell on how many bits signed elements are encoded on. When reading elements directly using the raw sysfs attributes, the same rules for shifting and signing should apply. Use channel definition as root of trust and replace constant with them for the simple cases. Signed-off-by: Gwendal Grignou <gwendal@chromium.org> --- drivers/iio/accel/bma220_spi.c | 3 ++- drivers/iio/accel/kxcjk-1013.c | 3 ++- drivers/iio/accel/mma7455_core.c | 3 ++- drivers/iio/accel/sca3000.c | 5 +++-- drivers/iio/accel/stk8312.c | 2 +- drivers/iio/accel/stk8ba50.c | 3 ++- drivers/iio/adc/ad7266.c | 3 ++- drivers/iio/adc/at91-sama5d2_adc.c | 3 ++- drivers/iio/adc/ti-adc12138.c | 3 ++- drivers/iio/magnetometer/mag3110.c | 6 ++++-- 10 files changed, 22 insertions(+), 12 deletions(-)