Re: [PATCH 5/5] dt-bindings: iio: Add adis16475 documentation

[nunojsa <noname.nuno@gmail.com>]

On 07.03.20 12:33, Jonathan Cameron wrote:
> On Thu, 5 Mar 2020 13:04:27 +0000
> "Sa, Nuno" <Nuno.Sa@analog.com> wrote:
> 
>> On Thu, 2020-03-05 at 13:43 +0100, Lars-Peter Clausen wrote:
>>> On 3/5/20 1:27 PM, Sa, Nuno wrote:  
>>>>> In my opinion there is should not be a difference in the
>>>>> userspace
>>>>> interface for chips that do support 32-bit burst and those that
>>>>> don't.
>>>>> For devices that don't support 32-bit burst it should be emulated
>>>>> by
>>>>> reading the LSB bits registers manually.  
>>>> Hmm. In terms of interface I think there is no difference. We
>>>> always
>>>> report 32bits channels (for accel and gyro). However, what we do
>>>> right
>>>> know is just to set the LSB to 0 if burst32 is not supported. So,
>>>> we
>>>> can be just ignoring the LSB bits if they are being used...  
>>>
>>> What I meant was that somebody might still want to get the full 32-
>>> bit 
>>> values in buffered mode, even if the device does not support burst32.  
>>
>> They are. Just that the LSB part is always set to 0 :). And that, in my
>> opinion, is wrong. As you say, we should do the manual readings if
>> there are any bits on the LSB registers...
>>
>> - Nuno Sá
>>> In 
>>> that case you can first do a 16-bit burst read to get the MSBs and
>>> then 
>>> do manual reads of all the LSB registers and then put both into the
>>> buffer.
>>> - Lars
>>>   
>>
> Thanks Lars and Nuno, I'd not grasped exactly what this was.
> 
> Hmm.  Not allowing for variable bit widths has bitten us a few times in the
> past.  Howwever, it's a really nasty thing to try and add to the core now
> unfortunately.
> 
> In some cases we've just padded the smaller bitwidth buffer but that
> is costly to actually do.  You get fast reads from the hardware then loose
> at least some of the benefit respacing the data.
> 
> Still it is definitely a policy decision so not DT.  It's ugly but if
> we want to support it and can't do it at runtime, perhaps a module parameter
> is the best option?
>

So, we can decide this at runtime. As Lars pointed out, the LSB bits are not
used by default (decimation and FIR filters disabled). However, applications can
change this by changing the sampling frequency (affects the decimation filter)
and the low_pass_filter_3db_freq (affects the FIR filter). If one of these filters
is used, then the LSB bits are meaningful and makes sense to use burst32. For parts
that do not support burst32, we should manually read the data.

I started to prepare the version 2 of this series and Im starting to have mixed
feelings. For now, I can see 3 ways of handling this:

1) If we assume that changing from burst32 to burst mode can occur at any
time, we need some special handling. We need to realloc the buffer used
on the spi transfer and readjust the spi xfer length. I'm not a big fan of the
realloc part...

2) Alternatively, we could introduce a `burst_max_len` in the library that could be
used in devices with different burst modes with different sizes. Max len would just
hold the maximum burst len (as the name implies) and would be used to allocate
the buffer to use on the spi tranfer. On the spi xfer we would then use the real
burst length. With this we would not need to care about reallocs...

3) More conservative, we would not allow changing burst modes if buffering is
ongoing... Changing a filter setting that would lead to burst mode change when
buffering would return -EPERM...

Either way, I will probably just send the v2 patch with 1) and then everyone can have
a better look on how it looks and we can discuss improvements/other mechanism in the
v2 thread.

- Nuno Sá
> Thanks,
> 
> Jonathan
> 
>