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 ;) > >> >>> >>> 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@xxxxxxxxxx> >>>> --- >>>> .../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@xxxxxxxxxxxxxxx >> 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@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
