[RFC PATCH v1 0/3] iio sensor mounting matrix support

[RFC PATCH v1 0/3] iio sensor mounting matrix support

[Gregor Boirie]

This RFC patch series follows up on Rob Herring and Jonathan Cameron comments
about ak8975 magnetometer mounting matrix support. Thread starts here:
http://www.spinics.net/lists/linux-iio/msg23505.html

As a recall, we want to expose a rotation matrix to indicate userspace the chip
placement with respect to the overall hardware system. This allows an
application to adjust coordinates sampled from a sensor chip when its position
deviates from the main hardware system.
Rob mentionned that the interface could be appropriate for other sensors such as
gyro, accelero, etc... This would prevent from "ending up with a bunch of
similar yet different interfaces".

Requirements:
1. delegate computation to userspace ;
2. floating point arithmetics support ;
3. per sample type matrix ;
4. remain compliant with legacy interface (mpu6050).

Point 1. above allows application to perform arbitrary transformations in
addition to sensor alignment handling.
Point 2. is required for flexible chip positioning.
Point 3. comes from the fact that chips, such as ADIS16407, may implement
different axis direction references for each measurement space.
See http://www.analog.com/media/en/technical-documentation/data-sheets/ADIS16407.pdf

Firs patch attached raise several questions for which I'd like to hear your
suggestions.

>From point 3. above, we should end up with something like the following for a
a magneto + gyro + accelero chip :
iio:deviceX/in_anglvel_mount_matrix, applicable to all 3 gyro channels
iio:deviceX/in_accel_mount_matrix, applicable to all 3 accelero channels
iio:deviceX/in_magn_mount_matrix, applicable to all 3 magneto channels

Do we need to ensure proper naming consistency for sysfs attributes ? How ?
Reuse iio_chan_type_name_spec array ?

Should we limit matrix sysfs attribute instantiation to a subset of sensor
types ? Which ones ? Or should we leave this to driver writers choice ?

Last patch relates to mpu6050 mounting matrix evolutions : legacy attribute was
extracted from platform_data. For sake of simplicity and as platform data
mechanism is becoming more and more "obsolete", I implemented new mounting
matrix API support for device-tree nodes only. Good enought ?

Best regards,
gregor.

Gregor Boirie (3):
  iio:core: mounting matrix support
  iio:ak8975: add mounting matrix support
  iio:imu:mpu6050: enhance mounting matrix support

 Documentation/ABI/testing/sysfs-bus-iio            | 51 ++++++++++++++
 .../devicetree/bindings/iio/imu/inv_mpu6050.txt    | 13 ++++
 .../bindings/iio/magnetometer/ak8975.txt           | 10 +++
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c         | 56 +++++++++++++--
 drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h          |  4 +-
 drivers/iio/industrialio-core.c                    | 81 ++++++++++++++++++++++
 drivers/iio/magnetometer/ak8975.c                  | 40 ++++++++++-
 include/linux/iio/magnetometer/ak8975.h            | 16 +++++
 include/linux/iio/sysfs.h                          | 25 +++++++
 include/linux/platform_data/invensense_mpu6050.h   |  5 +-
 10 files changed, 293 insertions(+), 8 deletions(-)
 create mode 100644 include/linux/iio/magnetometer/ak8975.h

-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH v1 1/3] iio:core: mounting matrix support

[Gregor Boirie]

Expose a rotation matrix to indicate userspace the chip placement with
respect to the overall hardware system. This is needed to adjust
coordinates sampled from a sensor chip when its position deviates from the
main hardware system.

Final coordinates computation is delegated to userspace since:
* computation may involve floating point arithmetics ;
* it allows an application to combine adjustments with arbitrary
  transformations.

This 3 dimentional space rotation matrix is expressed as 3x3 array of
strings to support floating point numbers. It may be retrieved from a
"in_<type>_mount_matrix" sysfs attribute file. It is declared into a
device / driver specific DTS property or platform data.

Signed-off-by: Gregor Boirie <gregor.boirie-ITF29qwbsa/QT0dZR+AlfA@public.gmane.org>
---
 Documentation/ABI/testing/sysfs-bus-iio | 51 +++++++++++++++++++++
 drivers/iio/industrialio-core.c         | 81 +++++++++++++++++++++++++++++++++
 include/linux/iio/sysfs.h               | 25 ++++++++++
 3 files changed, 157 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index f155eff..f2badda 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -1512,3 +1512,54 @@ Contact:	linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
 Description:
 		Raw (unscaled no offset etc.) pH reading of a substance as a negative
 		base-10 logarithm of hydrodium ions in a litre of water.
+
+What:           /sys/bus/iio/devices/iio:deviceX/in_magn_mount_matrix
+What:           /sys/bus/iio/devices/iio:deviceX/in_anglvel_mount_matrix
+What:           /sys/bus/iio/devices/iio:deviceX/in_accel_mount_matrix
+KernelVersion:  4.6
+Contact:        linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description:
+		Mounting matrix for IIO sensors. This is a rotation matrix which
+		informs userspace about sensor chip's placement relative to the
+		main hardware it is mounted on.
+		Main hardware placement is defined according to the local
+		reference frame related to the physical quantity the sensor
+		measures.
+		Given that the rotation matrix is defined in a board specific
+		way (platform data and / or device-tree), the main hardware
+		reference frame definition is left to the implementor's choice
+		(see below for a magnetometer example).
+		Applications should apply this rotation matrix to samples so
+		that when main hardware reference frame is aligned onto local
+		reference frame, then sensor chip reference frame is also
+		perfectly aligned with it.
+		Matrix is a 3x3 unitary matrix and typically looks like
+		[0, 1, 0; 1, 0, 0; 0, 0, -1]. Identity matrix
+		[1, 0, 0; 0, 1, 0; 0, 0, 1] means sensor chip and main hardware
+		are perfectly aligned with each other.
+
+		For example, a mounting matrix for a magnetometer sensor informs
+		userspace about sensor chip's ORIENTATION relative to the main
+		hardware.
+		More specifically, main hardware orientation is defined with
+		respect to the LOCAL EARTH GEOMAGNETIC REFERENCE FRAME where :
+		* Y is in the ground plane and positive towards magnetic North ;
+		* X is in the ground plane, perpendicular to the North axis and
+		  positive towards the East ;
+		* Z is perpendicular to the ground plane and positive upwards.
+
+		An implementor might consider that for a hand-held device, a
+		'natural' orientation would be 'front facing camera at the top'.
+		The main hardware reference frame could then be described as :
+		* Y is in the plane of the screen and is positive towards the
+		  top of the screen ;
+		* X is in the plane of the screen, perpendicular to Y axis, and
+		  positive towards the right hand side of the screen ;
+		* Z is perpendicular to the screen plane and positive out of the
+		  screen.
+		Another example for a quadrotor UAV might be :
+		* Y is in the plane of the propellers and positive towards the
+		  front-view camera;
+		* X is in the plane of the propellers, perpendicular to Y axis,
+		  and positive towards the starboard side of the UAV ;
+		* Z is perpendicular to propellers plane and positive upwards.
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 190a593..f9337e8 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -177,6 +177,87 @@ ssize_t iio_read_const_attr(struct device *dev,
 }
 EXPORT_SYMBOL(iio_read_const_attr);
 
+/**
+ * iio_show_mount_matrix() - sysfs helper for showing iio device mounting matrix
+ *                           attribute
+ * @matrix:	mounting matrix to show
+ * @buf:	char buffer to be filled with matrix elements
+ */
+ssize_t iio_show_mount_matrix(const struct iio_mount_matrix *matrix,
+			      char *buf)
+{
+	return sprintf(buf, "%s, %s, %s; %s, %s, %s; %s, %s, %s\n",
+		       matrix->rotation[0], matrix->rotation[1],
+		       matrix->rotation[2], matrix->rotation[3],
+		       matrix->rotation[4], matrix->rotation[5],
+		       matrix->rotation[6], matrix->rotation[7],
+		       matrix->rotation[8]);
+}
+EXPORT_SYMBOL(iio_show_mount_matrix);
+
+static const struct iio_mount_matrix iio_mount_idmatrix = {
+	.rotation = {
+		"1", "0", "0",
+		"0", "1", "0",
+		"0", "0", "1"
+	}
+};
+
+static int iio_setup_mount_idmatrix(const struct device *dev,
+				    struct iio_mount_matrix *matrix)
+{
+	*matrix = iio_mount_idmatrix;
+	dev_info(dev, "mounting matrix not found: using identity...\n");
+	return 0;
+}
+
+/**
+ * of_iio_read_mount_matrix() - retrieve iio device mounting matrix from
+ *                              device-tree "mount-matrix" property
+ * @dev:	device the mounting matrix property is assigned to
+ * @propname:	device specific mounting matrix property name
+ * @matrix:	where to store retrieved matrix
+ *
+ * If device is assigned no mounting matrix property, a default 3x3 identity
+ * matrix will be filled in.
+ *
+ * Return: 0 if success, or a negative error code on failure.
+ */
+#ifdef CONFIG_OF
+int of_iio_read_mount_matrix(const struct device *dev,
+			     const char *propname,
+			     struct iio_mount_matrix *matrix)
+{
+	if (dev->of_node) {
+		int err = of_property_read_string_array(dev->of_node,
+				propname, matrix->rotation,
+				ARRAY_SIZE(iio_mount_idmatrix.rotation));
+
+		if (err == ARRAY_SIZE(iio_mount_idmatrix.rotation))
+			return 0;
+
+		if (err >= 0)
+			/* Invalid number of matrix entries. */
+			return -EINVAL;
+
+		if (err != -EINVAL)
+			/* Invalid matrix declaration format. */
+			return err;
+	}
+
+	/* Matrix was not declared at all: fallback to identity. */
+	return iio_setup_mount_idmatrix(dev, matrix);
+}
+#else
+int of_iio_read_mount_matrix(const struct device *dev,
+			     const char *propname,
+			     struct iio_mount_matrix *matrix)
+{
+	return iio_setup_mount_idmatrix(dev, matrix);
+}
+#endif
+EXPORT_SYMBOL(of_iio_read_mount_matrix);
+
 static int __init iio_init(void)
 {
 	int ret;
diff --git a/include/linux/iio/sysfs.h b/include/linux/iio/sysfs.h
index 9cd8f74..39ad842 100644
--- a/include/linux/iio/sysfs.h
+++ b/include/linux/iio/sysfs.h
@@ -125,4 +125,29 @@ struct iio_const_attr {
 #define IIO_CONST_ATTR_TEMP_SCALE(_string)		\
 	IIO_CONST_ATTR(in_temp_scale, _string)
 
+/**
+ * struct iio_mount_matrix - iio mounting matrix
+ * @rotation: 3 dimensional space rotation matrix defining sensor alignment with
+ *            main hardware
+ */
+struct iio_mount_matrix {
+	const char *rotation[9];
+};
+
+int of_iio_read_mount_matrix(const struct device *dev,
+			     const char *propname,
+			     struct iio_mount_matrix *matrix);
+
+ssize_t iio_show_mount_matrix(const struct iio_mount_matrix *matrix,
+			      char *buf);
+
+/**
+ * IIO_DEV_ATTR_MOUNT_MATRIX - iio mounting matrix attribute
+ * @_show: output method for the attribute
+ * @_addr: optional associated register address to distinguish between
+ *         sensor chip's sub-devices
+ **/
+#define IIO_DEV_ATTR_MOUNT_MATRIX(_name, _show, _addr) \
+	IIO_DEVICE_ATTR(_name, S_IRUGO, _show, NULL, _addr)
+
 #endif /* _INDUSTRIAL_IO_SYSFS_H_ */
-- 
2.1.4

[RFC PATCH v1 2/3] iio:ak8975: add mounting matrix support

[Gregor Boirie]

Expose a rotation matrix to indicate userspace the chip orientation with
respect to the overall hardware system.

Signed-off-by: Gregor Boirie <gregor.boirie-ITF29qwbsa/QT0dZR+AlfA@public.gmane.org>
---
 .../bindings/iio/magnetometer/ak8975.txt           | 10 ++++++
 drivers/iio/magnetometer/ak8975.c                  | 40 ++++++++++++++++++++--
 include/linux/iio/magnetometer/ak8975.h            | 16 +++++++++
 3 files changed, 64 insertions(+), 2 deletions(-)
 create mode 100644 include/linux/iio/magnetometer/ak8975.h

diff --git a/Documentation/devicetree/bindings/iio/magnetometer/ak8975.txt b/Documentation/devicetree/bindings/iio/magnetometer/ak8975.txt
index 34a3206..e1e7dd32 100644
--- a/Documentation/devicetree/bindings/iio/magnetometer/ak8975.txt
+++ b/Documentation/devicetree/bindings/iio/magnetometer/ak8975.txt
@@ -9,6 +9,7 @@ Optional properties:
 
   - gpios : should be device tree identifier of the magnetometer DRDY pin
   - vdd-supply: an optional regulator that needs to be on to provide VDD
+  - mount-matrix: an optional 3x3 mounting rotation matrix
 
 Example:
 
@@ -17,4 +18,13 @@ ak8975@0c {
         reg = <0x0c>;
         gpios = <&gpj0 7 0>;
         vdd-supply = <&ldo_3v3_gnss>;
+        mount-matrix = "-0.984807753012208",  /* x0 */
+                       "0",                   /* y0 */
+                       "-0.173648177666930",  /* z0 */
+                       "0",                   /* x1 */
+                       "-1",                  /* y1 */
+                       "0",                   /* z1 */
+                       "-0.173648177666930",  /* x2 */
+                       "0",                   /* y2 */
+                       "0.984807753012208";   /* z2 */
 };
diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index 48d127a..ec3f27a 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -36,6 +36,9 @@
 
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
+
+#include <linux/iio/magnetometer/ak8975.h>
+
 /*
  * Register definitions, as well as various shifts and masks to get at the
  * individual fields of the registers.
@@ -370,6 +373,7 @@ struct ak8975_data {
 	wait_queue_head_t	data_ready_queue;
 	unsigned long		flags;
 	u8			cntl_cache;
+	struct iio_mount_matrix orientation;
 	struct regulator	*vdd;
 };
 
@@ -714,6 +718,27 @@ static int ak8975_read_raw(struct iio_dev *indio_dev,
 	return -EINVAL;
 }
 
+static ssize_t ak8975_show_orientation(struct device *dev,
+				       struct device_attribute *attr,
+				       char *buf)
+{
+	const struct ak8975_data *data = iio_priv(dev_to_iio_dev(dev));
+
+	return iio_show_mount_matrix(&data->orientation, buf);
+}
+
+static IIO_DEV_ATTR_MOUNT_MATRIX(in_magn_mount_matrix, ak8975_show_orientation,
+				 -1);
+
+static struct attribute *ak8975_attrs[] = {
+	&iio_dev_attr_in_magn_mount_matrix.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group ak8975_attrs_group = {
+	.attrs = ak8975_attrs,
+};
+
 #define AK8975_CHANNEL(axis, index)					\
 	{								\
 		.type = IIO_MAGN,					\
@@ -730,6 +755,7 @@ static const struct iio_chan_spec ak8975_channels[] = {
 
 static const struct iio_info ak8975_info = {
 	.read_raw = &ak8975_read_raw,
+	.attrs = &ak8975_attrs_group,
 	.driver_module = THIS_MODULE,
 };
 
@@ -765,10 +791,11 @@ static int ak8975_probe(struct i2c_client *client,
 	int err;
 	const char *name = NULL;
 	enum asahi_compass_chipset chipset;
+	const struct ak8975_platform_data *pdata = client->dev.platform_data;
 
 	/* Grab and set up the supplied GPIO. */
-	if (client->dev.platform_data)
-		eoc_gpio = *(int *)(client->dev.platform_data);
+	if (pdata)
+		eoc_gpio = pdata->eoc_gpio;
 	else if (client->dev.of_node)
 		eoc_gpio = of_get_gpio(client->dev.of_node, 0);
 	else
@@ -802,6 +829,15 @@ static int ak8975_probe(struct i2c_client *client,
 	data->eoc_gpio = eoc_gpio;
 	data->eoc_irq = 0;
 
+	if (!pdata) {
+		err = of_iio_read_mount_matrix(&client->dev,
+					       "mount-matrix",
+					       &data->orientation);
+		if (err)
+			return err;
+	} else
+		data->orientation = pdata->orientation;
+
 	/* id will be NULL when enumerated via ACPI */
 	if (id) {
 		chipset = (enum asahi_compass_chipset)(id->driver_data);
diff --git a/include/linux/iio/magnetometer/ak8975.h b/include/linux/iio/magnetometer/ak8975.h
new file mode 100644
index 0000000..52947d7
--- /dev/null
+++ b/include/linux/iio/magnetometer/ak8975.h
@@ -0,0 +1,16 @@
+#ifndef __IIO_MAGNETOMETER_AK8975_H__
+#define __IIO_MAGNETOMETER_AK8975_H__
+
+#include <linux/iio/sysfs.h>
+
+/**
+ * struct ak8975_platform_data - AK8975 magnetometer driver platform data
+ * @eoc_gpio:    data ready event gpio
+ * @orientation: mounting matrix relative to main hardware
+ */
+struct ak8975_platform_data {
+	int                     eoc_gpio;
+	struct iio_mount_matrix orientation;
+};
+
+#endif
-- 
2.1.4

Re: [RFC PATCH v1 2/3] iio:ak8975: add mounting matrix support

[Rob Herring]

On Fri, Apr 08, 2016 at 04:12:04PM +0200, Gregor Boirie wrote:
> Expose a rotation matrix to indicate userspace the chip orientation with
> respect to the overall hardware system.
> 
> Signed-off-by: Gregor Boirie <gregor.boirie-ITF29qwbsa/QT0dZR+AlfA@public.gmane.org>
> ---
>  .../bindings/iio/magnetometer/ak8975.txt           | 10 ++++++

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

>  drivers/iio/magnetometer/ak8975.c                  | 40 ++++++++++++++++++++--
>  include/linux/iio/magnetometer/ak8975.h            | 16 +++++++++
>  3 files changed, 64 insertions(+), 2 deletions(-)
>  create mode 100644 include/linux/iio/magnetometer/ak8975.h

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

[Gregor Boirie]

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.

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(-)

diff --git a/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt b/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
index e4d8f1c..a9fc11e 100644
--- a/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
+++ b/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
@@ -8,10 +8,23 @@ Required properties:
  - interrupt-parent : should be the phandle for the interrupt controller
  - interrupts : interrupt mapping for GPIO IRQ
 
+Optional properties:
+ - mount-matrix: an optional 3x3 mounting rotation matrix
+
+
 Example:
 	mpu6050@68 {
 		compatible = "invensense,mpu6050";
 		reg = <0x68>;
 		interrupt-parent = <&gpio1>;
 		interrupts = <18 1>;
+		mount-matrix = "-0.984807753012208",  /* x0 */
+		               "0",                   /* y0 */
+		               "-0.173648177666930",  /* z0 */
+		               "0",                   /* x1 */
+		               "-1",                  /* y1 */
+		               "0",                   /* z1 */
+		               "-0.173648177666930",  /* x2 */
+		               "0",                   /* y2 */
+		               "0.984807753012208";   /* z2 */
 	};
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index d192953..deb93bc 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -599,7 +599,11 @@ inv_fifo_rate_show(struct device *dev, struct device_attribute *attr,
 
 /**
  * inv_attr_show() - calling this function will show current
- *                    parameters.
+ *                   parameters.
+ *
+ * Deprecated in favor of IIO mounting matrix API.
+ *
+ * See inv_show_mount_matrix()
  */
 static ssize_t inv_attr_show(struct device *dev, struct device_attribute *attr,
 			     char *buf)
@@ -625,6 +629,30 @@ static ssize_t inv_attr_show(struct device *dev, struct device_attribute *attr,
 }
 
 /**
+ * inv_show_mount_matrix() - calling this function will show current sensor's
+ *                           orientation.
+ */
+static ssize_t inv_show_mount_matrix(struct device *dev,
+				     struct device_attribute *attr,
+				     char *buf)
+{
+	struct inv_mpu6050_state *st = iio_priv(dev_to_iio_dev(dev));
+	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
+
+	switch (this_attr->address) {
+	/*
+	 * In MPU6050, the two matrix are the same because gyro and accel
+	 * are integrated in one chip
+	 */
+	case ATTR_GYRO_MATRIX:
+	case ATTR_ACCL_MATRIX:
+		return iio_show_mount_matrix(&st->orientation, buf);
+	default:
+		return -EINVAL;
+	}
+}
+
+/**
  * inv_mpu6050_validate_trigger() - validate_trigger callback for invensense
  *                                  MPU6050 device.
  * @indio_dev: The IIO device
@@ -692,14 +720,23 @@ static IIO_CONST_ATTR(in_accel_scale_available,
 					  "0.000598 0.001196 0.002392 0.004785");
 static IIO_DEV_ATTR_SAMP_FREQ(S_IRUGO | S_IWUSR, inv_fifo_rate_show,
 	inv_mpu6050_fifo_rate_store);
+
+/* Deprecated: kept for userspace backward compatibility. */
 static IIO_DEVICE_ATTR(in_gyro_matrix, S_IRUGO, inv_attr_show, NULL,
 	ATTR_GYRO_MATRIX);
 static IIO_DEVICE_ATTR(in_accel_matrix, S_IRUGO, inv_attr_show, NULL,
 	ATTR_ACCL_MATRIX);
+/* Replacement for old matrix attribute interface. */
+static IIO_DEV_ATTR_MOUNT_MATRIX(in_anglvel_mount_matrix, inv_show_mount_matrix,
+	ATTR_GYRO_MATRIX);
+static IIO_DEV_ATTR_MOUNT_MATRIX(in_accel_mount_matrix, inv_show_mount_matrix,
+	ATTR_ACCL_MATRIX);
 
 static struct attribute *inv_attributes[] = {
-	&iio_dev_attr_in_gyro_matrix.dev_attr.attr,
-	&iio_dev_attr_in_accel_matrix.dev_attr.attr,
+	&iio_dev_attr_in_gyro_matrix.dev_attr.attr, /* deprecated */
+	&iio_dev_attr_in_accel_matrix.dev_attr.attr,/* deprecated */
+	&iio_dev_attr_in_anglvel_mount_matrix.dev_attr.attr,
+	&iio_dev_attr_in_accel_mount_matrix.dev_attr.attr,
 	&iio_dev_attr_sampling_frequency.dev_attr.attr,
 	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
 	&iio_const_attr_in_accel_scale_available.dev_attr.attr,
@@ -779,9 +816,20 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
 	st->powerup_count = 0;
 	st->irq = irq;
 	st->map = regmap;
+
 	pdata = dev_get_platdata(dev);
-	if (pdata)
+	if (!pdata) {
+		result = of_iio_read_mount_matrix(dev, "mount-matrix",
+						  &st->orientation);
+		if (result) {
+			dev_err(dev, "Failed to retrieve mounting matrix %d\n",
+				result);
+			return result;
+		}
+	} else {
 		st->plat_data = *pdata;
+	}
+
 	/* power is turned on inside check chip type*/
 	result = inv_check_and_setup_chip(st);
 	if (result)
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
index e302a49..52d60cd 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
@@ -114,7 +114,8 @@ struct inv_mpu6050_hw {
  *  @hw:		Other hardware-specific information.
  *  @chip_type:		chip type.
  *  @time_stamp_lock:	spin lock to time stamp.
- *  @plat_data:		platform data.
+ *  @plat_data:		platform data (deprecated in favor of @orientation).
+ *  @orientation:	sensor chip orientation relative to main hardware.
  *  @timestamps:        kfifo queue to store time stamp.
  *  @map		regmap pointer.
  *  @irq		interrupt number.
@@ -131,6 +132,7 @@ struct inv_mpu6050_state {
 	struct i2c_client *mux_client;
 	unsigned int powerup_count;
 	struct inv_mpu6050_platform_data plat_data;
+	struct iio_mount_matrix orientation;
 	DECLARE_KFIFO(timestamps, long long, TIMESTAMP_FIFO_SIZE);
 	struct regmap *map;
 	int irq;
diff --git a/include/linux/platform_data/invensense_mpu6050.h b/include/linux/platform_data/invensense_mpu6050.h
index ad3aa7b..554b598 100644
--- a/include/linux/platform_data/invensense_mpu6050.h
+++ b/include/linux/platform_data/invensense_mpu6050.h
@@ -16,13 +16,16 @@
 
 /**
  * struct inv_mpu6050_platform_data - Platform data for the mpu driver
- * @orientation:	Orientation matrix of the chip
+ * @orientation:	Orientation matrix of the chip (deprecated in favor of
+ *			mounting matrix retrieved from device-tree)
  *
  * Contains platform specific information on how to configure the MPU6050 to
  * work on this platform.  The orientation matricies are 3x3 rotation matricies
  * that are applied to the data to rotate from the mounting orientation to the
  * platform orientation.  The values must be one of 0, 1, or -1 and each row and
  * column should have exactly 1 non-zero value.
+ *
+ * Deprecated in favor of mounting matrix retrieved from device-tree.
  */
 struct inv_mpu6050_platform_data {
 	__s8 orientation[9];
-- 
2.1.4

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

[Rob Herring]

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?

BTW, sysfs interfaces also have to be documented.

> 
> 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(-)

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

[Gregor Boirie]

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 ?

> 
> BTW, sysfs interfaces also have to be documented.
Ok

> 
> > 
> > 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(-)

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

[Jonathan Cameron]

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!)
> 
>>
>>>
>>> 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
> 

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

[Jonathan Cameron]

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-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
> 

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

[Jonathan Cameron]

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
> 

Re: [RFC PATCH v1 0/3] iio sensor mounting matrix support

[Jonathan Cameron]

On 08/04/16 15:12, Gregor Boirie wrote:
> This RFC patch series follows up on Rob Herring and Jonathan Cameron comments
> about ak8975 magnetometer mounting matrix support. Thread starts here:
> http://www.spinics.net/lists/linux-iio/msg23505.html
> 
> As a recall, we want to expose a rotation matrix to indicate userspace the chip
> placement with respect to the overall hardware system. This allows an
> application to adjust coordinates sampled from a sensor chip when its position
> deviates from the main hardware system.
> Rob mentionned that the interface could be appropriate for other sensors such as
> gyro, accelero, etc... This would prevent from "ending up with a bunch of
> similar yet different interfaces".
> 
> Requirements:
> 1. delegate computation to userspace ;
> 2. floating point arithmetics support ;
> 3. per sample type matrix ;
> 4. remain compliant with legacy interface (mpu6050).
> 
> Point 1. above allows application to perform arbitrary transformations in
> addition to sensor alignment handling.
> Point 2. is required for flexible chip positioning.
> Point 3. comes from the fact that chips, such as ADIS16407, may implement
> different axis direction references for each measurement space.
> See http://www.analog.com/media/en/technical-documentation/data-sheets/ADIS16407.pdf
> 
> Firs patch attached raise several questions for which I'd like to hear your
> suggestions.
> 
> From point 3. above, we should end up with something like the following for a
> a magneto + gyro + accelero chip :
> iio:deviceX/in_anglvel_mount_matrix, applicable to all 3 gyro channels
> iio:deviceX/in_accel_mount_matrix, applicable to all 3 accelero channels
> iio:deviceX/in_magn_mount_matrix, applicable to all 3 magneto channels
> 
> Do we need to ensure proper naming consistency for sysfs attributes ? How ?
How about doing this through the ext_info element of iio_chan_spec?  That is
intended to handle this exact case.   Also, if in a convoluted way, it does
allow access to the mount matrix from client drivers within the kernel.  I
can conceive that the equivalent matrix might want to be exposed through an
input bridge driver at some point for example.  I'm slowly trying to kill off
usage of device_attributes like you add here precisely because they aren't
accessible via our inkernel interfaces.

That interface allows you to create arbitary form sysfs attributes, but using
the same concepts of shared_by_type etc as we have for info_mask elements.
> Reuse iio_chan_type_name_spec array ?
With the ext_info stuff you get this for free.
> 
> Should we limit matrix sysfs attribute instantiation to a subset of sensor
> types ? Which ones ? Or should we leave this to driver writers choice ?
Leave it up to the writer I think.  Chances are we'll end up with something
we haven't thought of in the future if we start limiting the coverage.
We can rely on documentation to make people think about it before using the
interface.
> 
> Last patch relates to mpu6050 mounting matrix evolutions : legacy attribute was
> extracted from platform_data. For sake of simplicity and as platform data
> mechanism is becoming more and more "obsolete", I implemented new mounting
> matrix API support for device-tree nodes only. Good enought ?
Good enough for now.  If anyone needs platform_data support then they can
add it.  No need for it to be your problem!

Other than the above suggestion of using ext_info the patch set looks good
to me.

Jonathan
> 
> Best regards,
> gregor.
> 
> Gregor Boirie (3):
>   iio:core: mounting matrix support
>   iio:ak8975: add mounting matrix support
>   iio:imu:mpu6050: enhance mounting matrix support
> 
>  Documentation/ABI/testing/sysfs-bus-iio            | 51 ++++++++++++++
>  .../devicetree/bindings/iio/imu/inv_mpu6050.txt    | 13 ++++
>  .../bindings/iio/magnetometer/ak8975.txt           | 10 +++
>  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c         | 56 +++++++++++++--
>  drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h          |  4 +-
>  drivers/iio/industrialio-core.c                    | 81 ++++++++++++++++++++++
>  drivers/iio/magnetometer/ak8975.c                  | 40 ++++++++++-
>  include/linux/iio/magnetometer/ak8975.h            | 16 +++++
>  include/linux/iio/sysfs.h                          | 25 +++++++
>  include/linux/platform_data/invensense_mpu6050.h   |  5 +-
>  10 files changed, 293 insertions(+), 8 deletions(-)
>  create mode 100644 include/linux/iio/magnetometer/ak8975.h
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html