On 12/15/19 10:09 PM, Lars Möllendorf wrote:
> -----Ursprüngliche Nachricht-----
>> Von: Lars Möllendorf <lars.moellendorf@xxxxxxxxxx>
>> Gesendet: Freitag 13 Dezember 2019 14:58
>> An: Jonathan Cameron <jic23@xxxxxxxxxx>; Hartmut Knaack <knaack.h@xxxxxx>; Lars-Peter Clausen <lars@xxxxxxxxxx>; Peter Meerwald-Stadler <pmeerw@xxxxxxxxxx>; linux-iio@xxxxxxxxxxxxxxx
>> CC: Lars Möllendorf <lars.moellendorf@xxxxxxxxxx>
>> Betreff: [PATCH v3] iio: buffer: align the size of scan bytes to size of the largest element
>>
>> Previous versions of `iio_compute_scan_bytes` only aligned each element
>> to its own length (i.e. its own natural alignment). Because multiple
>> consecutive sets of scan elements are buffered this does not work in
>> case the computed scan bytes do not align with the natural alignment of
>> the first scan element in the set.
>>
>> This commit fixes this by aligning the scan bytes to the natural
>> alignment of the largest scan element in the set.
>
>
>
> After re-reading my commit message, I come to the conclusion that it really is sufficient to align the scan bytes to the natural alignment of the *first* element. This would save us the `max()` comparisons for each bit. At the moment I am not at my workstation, but I could submit a v4 next Friday.
>
I thought so too in the beginning, but as Jonathan pointed out, it does
not work for all cases. Lets say you have u16,u16,u32,u16. If all
channels are enabled the size is aligned to the first element, but the
u32 would not be aligned in the second dataset.
>
>
>> Fixes: 959d2952d124 ("staging:iio: make iio_sw_buffer_preenable much more
>> general.")
>> Signed-off-by: Lars Möllendorf <lars.moellendorf@xxxxxxxxxx>
>> ---
>> v3:
>> - Fix the problem description in the commit message
>> - Add "Fixes" tag
>>
>> v2:
>> - Fix subject of patch which marked it the first in a set of three.
>> - Add a description of the problem in the commit message
>>
>> ---
>> drivers/iio/industrialio-buffer.c | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
>> index 5d05c38c4ba9..2f037cd59d53 100644
>> --- a/drivers/iio/industrialio-buffer.c
>> +++ b/drivers/iio/industrialio-buffer.c
>> @@ -546,7 +546,7 @@ static int iio_compute_scan_bytes(struct iio_dev *indio_dev,
>> const unsigned long *mask, bool timestamp)
>> {
>> unsigned bytes = 0;
>> - int length, i;
>> + int length, i, largest = 0;
>>
>> /* How much space will the demuxed element take? */
>> for_each_set_bit(i, mask,
>> @@ -554,13 +554,17 @@ static int iio_compute_scan_bytes(struct iio_dev *indio_dev,
>> length = iio_storage_bytes_for_si(indio_dev, i);
>> bytes = ALIGN(bytes, length);
>> bytes += length;
>> + largest = max(largest, length);
>> }
>>
>> if (timestamp) {
>> length = iio_storage_bytes_for_timestamp(indio_dev);
>> bytes = ALIGN(bytes, length);
>> bytes += length;
>> + largest = max(largest, length);
>> }
>> +
>> + bytes = ALIGN(bytes, largest);
>> return bytes;
>> }
>>
>> --
>> 2.23.0
>>
