Re: [PATCH v2 6/7] iio: inv_mpu6050: Reformat sample for active scan mask

[Jonathan Cameron]

On 31 May 2016 09:56:51 BST, Crestez Dan Leonard <leonard.crestez@xxxxxxxxx> wrote:
>On 05/31/2016 12:42 AM, Jonathan Cameron wrote:
>> On 30 May 2016 14:44:41 BST, Crestez Dan Leonard
><leonard.crestez@xxxxxxxxx> wrote:
>>> On 05/29/2016 06:47 PM, Jonathan Cameron wrote:
>>>> On 18/05/16 16:00, Crestez Dan Leonard wrote:
>>>>> Right now it is possible to only enable some of the x/y/z
>channels,
>>> for
>>>>> example you can enable accel_z without x or y but if you actually
>do
>>>>> that what you get is actually only the x channel.
>>>>>
>>>>> Fix this by reformatting the hardware sample to only include the
>>>>> requested channels.
>>>> As it stands here there is no benefit in doing this over using the
>>> core
>>>> demux. In fact it's considerably less efficient (fair enough that
>you
>>>> are keeping it simple in the first instance).
>>>> The patch description should make that clear.
>>>
>>> Why is it less efficient? All it really does is a bunch of memcpy.
>> 
>> Not doing agglomeration of neighbouring copies (iirc) not git either
>set of code to
>> hand!
>
>You're right about that. But the total data rate is still very low.
>
>>>> I'd definitely like to see simple extension of that option to
>handle
>>>> a callback to get the nearest scanmask that is possible (as an
>>> alternative
>>>> to the static scan_masks_available list.)
>>>>
>>>> This only gets interesting if we are dealing with the unaligned
>case
>>> and for
>>>> these parts that only kicks in I think if the slave devices have
>say
>>> 3 bytes in
>>>> their data type.  
>>>
>>> But I want to deal with the unaligned case because it's better than
>>> introducing odd validations on slave channels. If I added an
>extension
>>> to get the nearest scanmask I would have to remove it in PATCH 7.
>> Hmm I must have misread that. Though you were only supporting 16 bit
>channels
>>  for aux sensors.
>
>That was in a previous version, I dropped that limitation now.
>
>> Then for now can we give this a slightly less generic name. I am not
>happy
>>  enough that we want this in the core 'yet'. 
>> Easy to rename later if it makes sense.
>
>Ok, I will rename these functions to start with inv_mpu_* instead of
>iio_*.
>
>In theory it would be interesting to refactor the iio demuxing code to
>support this but then this patch serios would grow even more
>complicated.

Agreed. Let's leave that for a separate effort!

I was vaguely wondering about letting the core demux input be unaligned but
 enforce the output keeping current alignment.  Fiddly even then however.

J

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.