Re: [PATCH 1/2] iio: bmi160: Support hardware fifo

[Jonathan Cameron <jic23@kernel.org>]

On 09/11/16 14:16, Marcin Niestroj wrote:
> Hi,
> Thanks for review, below are my comments.
> 
> On 03.11.2016 13:09, Peter Meerwald-Stadler wrote:
>>
>>> This patch was developed primarily based on bmc150_accel hardware fifo
>>> implementation.
>>
>> parts of the patch are cleanup and bugfixing; should be separate?
>>
>> more comments below
>>
>>> IRQ handler was added, which for now is responsible only for handling
>>> watermark interrupts. The BMI160 chip has two interrupt outputs. By
>>> default INT is considered to be connected. If INT2 is used instead, the
>>> interrupt-names device-tree property can be used to specify that.
>>>
>>> Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
>>> ---
<snip>
>>> +    /* Handle demux */
>>> +    timestamp = last_timestamp - (last_samples * data->fifo_sample_period);
>>> +    buffer_iter = buffer;
>>> +    for (i = 0; i < fifo_samples; i++) {
>>> +        u8 tmp_buf[indio_dev->scan_bytes];
>>
>> non-constant array size, is this allowed these days?
> 
> I am not sure if this is permitted in kernel. [1] says that ISO C99 adds arrays of variable length.
> 
> The reason I have used it is that iio_push_to_buffers_with_timestamp uses indio_dev->scan_bytes to write timestamp to last element in array.
> One of the possiblities to get rid of non-constant array size is to determine maximum size. Is that what I should do?
Yes.
> 
> [1] https://gcc.gnu.org/onlinedocs/gcc/Variable-Length.html
> 
>>
<snip>