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

[Crestez Dan Leonard]

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.

-- 
Regards,
Leonard