Re: [RFC PATCH v1 3/3] iio:imu:mpu6050: enhance mounting matrix support

[Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

On 16/04/16 11:20, Jonathan Cameron wrote:
> On 16/04/16 11:18, Jonathan Cameron wrote:
>> On 11/04/16 22:44, Gregor Boirie wrote:
>>> On Mon, Apr 11, 2016 at 02:12:12PM -0500, Rob Herring wrote:
>>>> On Fri, Apr 08, 2016 at 04:12:05PM +0200, Gregor Boirie wrote:
>>>>> Add a new rotation matrix sysfs attribute compliant with IIO core
>>>>> mounting matrix API.
>>>>> Matrix is retrieved from "in_anglvel_mount_matrix" and
>>>>> "in_accel_mount_matrix" sysfs attributes. It is declared into mpu6050 DTS
>>>>> entry as a "mount-matrix" property.
>>>>
>>>> We have a common DT property, but can't have common sysfs interface?
>>> Agreed. Original driver was structured that way. In fact, a single matrix is
>>> enought for this sensor: AFAIK, reference frames are the same for gyro and
>>> accelero.
>>> I suppose that having a DTS and sysfs attribute per type of sample /
>>> "sub-device" ensure better userspace API consistency across drivers.
>>> Jonathan ?
>> If shared by all channels in device, could just be mount_matrix
>> (as per normal info_mask_shared_by_all elements on which we drop all reference
>> to the channel name).
>>
>> However, we do have to allow for the per channel type version (and at least in theory
>> the per channel entry)  There are are weird devices combining multiple parallel accelerometers
>> in a package.  Possible for some reason that these could be other than parallel in some future
>> device.
>> So we end up with a sort of pyramid with any of the following being possible (and needing
>> a sysfs entry if they occur)  Taking a few examples...
>>
>> mount_matrix
>> in_mount_matrix
>> out_mount_matrix
>> in_accel_mount_matrix
>> in_magn_mount_matrix
>> in_accel_x_mount_matrix
>> in_accel_y_mount_matrix
>> in_accel_x1_mount_matrix
>> in_accel_x2_mount_matrix 
>> etc where convention is to use the element highest up that covers the device in question.
>>
>> It gets awkward if you have say an additional general purpose ADC channel on the device
>> as then clearly the mount matrix has nothing to do with it, but convention says we still
>> have to drop down to say in_accel_mount_matrix rather than using the mount_matrix form
>> that implies that it applies to the in_voltage0 channel.  Common sense and ABIs never
>> combine that well ;)
Hmm. I should really read things properly before commenting.  This device has a temperature
channel to which the mount matrix clearly doesn't really apply so it should indeed take
the form you have it as already.

The ak8975 doesn't however, so you 'could' pick the mount_matrix form above if you like.
When two of these cover the same range of parameters we tend to be flexible as userspace
will see them as the same anyway...

So I'm happy to take this as is, if we have convinced Rob ;)
>>
>>>
>>>>
>>>> BTW, sysfs interfaces also have to be documented.
>>> Ok
>> Yes - update Documentation/sysfs/testing/sysfs-bus-iio with an appropriate entry please.
>> Btw - the above pyramid structure is why that document is huge (and often missing
>> entries that we all assumed were there for particular combinations of the above!)
> You have documented it in patch 1.  I'd forgotten about that.
> :)
>>>
>>>>
>>>>>
>>>>> Old interface is kept for backward userspace compatibility and may be
>>>>> retrieved from legacy platform_data mechanism only.
>>>>>
>>>>> Signed-off-by: Gregor Boirie <gregor.boirie-ITF29qwbsa/QT0dZR+AlfA@public.gmane.org>
>>>>> ---
>>>>>  .../devicetree/bindings/iio/imu/inv_mpu6050.txt    | 13 +++++
>>>>>  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c         | 56 ++++++++++++++++++++--
>>>>>  drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h          |  4 +-
>>>>>  include/linux/platform_data/invensense_mpu6050.h   |  5 +-
>>>>>  4 files changed, 72 insertions(+), 6 deletions(-)
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>