> On 28/09/16 21:06, Lorenzo Bianconi wrote:
>> Using realbits as i2c/spi read len, when that value is not byte aligned
>> (e.g 12 bits), lead to skip msb part of out data registers.
>> Fix this using storagebits as read length
>>
>> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@xxxxxx>
> Hi Lorenzo,
Hi Jonathan,
>
> This would ideally have had a fixes tag.
>
> I think it was
> Fixes: e7385de5 ("iio:st_sensors: align on storagebits boundaries")
> Doing that would also have lead to you to the logic that lead to
> this buggy change in the first place. Would also have shown
> you that there is another place that probably suffers from the
> same sort of issue.
>
Right. I missed the same issue in st_sensors_read_axis_data()
> Gregor can you take a look at this please.
>
> If I recall the issue that lead to the original patch it was
> that we were miss handling 24 bit values on pressure sensors and
> this was intended to pad them?
>
> I think the 'right' fix will be something along the lines of:
> unsigned int bytes_to_read = (channel->scan_type.realbits + 7) >> 3;
>
This should not be correct if you consider 8bit accel like LIS331DL.
Taking a look to lps22hb code I think storagebits should be 24 and not
32 for pressure channel. What do you think?
> Should give 2 bytes for a 12 bit and still the 3 bytes needed for a
> 24 bit read.
>
> Thanks,
>
Thanks,
Lorenzo
> Jonathan
>> ---
>> drivers/iio/common/st_sensors/st_sensors_buffer.c | 5 ++---
>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/common/st_sensors/st_sensors_buffer.c b/drivers/iio/common/st_sensors/st_sensors_buffer.c
>> index fe7775b..d01aa34 100644
>> --- a/drivers/iio/common/st_sensors/st_sensors_buffer.c
>> +++ b/drivers/iio/common/st_sensors/st_sensors_buffer.c
>> @@ -30,16 +30,15 @@ static int st_sensors_get_buffer_element(struct iio_dev *indio_dev, u8 *buf)
>>
>> for_each_set_bit(i, indio_dev->active_scan_mask, num_data_channels) {q
>> const struct iio_chan_spec *channel = &indio_dev->channels[i];
>> - unsigned int bytes_to_read = channel->scan_type.realbits >> 3;
>> unsigned int storage_bytes =
>> channel->scan_type.storagebits >> 3;
>>
>> buf = PTR_ALIGN(buf, storage_bytes);
>> if (sdata->tf->read_multiple_byte(&sdata->tb, sdata->dev,
>> channel->address,
>> - bytes_to_read, buf,
>> + storage_bytes, buf,
>> sdata->multiread_bit) <
>> - bytes_to_read)
>> + storage_bytes)
>> return -EIO;
>>
>> /* Advance the buffer pointer */
>>
>
--
UNIX is Sexy: who | grep -i blonde | talk; cd ~; wine; talk; touch;
unzip; touch; strip; gasp; finger; gasp; mount; fsck; more; yes; gasp;
umount; make clean; sleep
--
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
