[PATCH 1/2] iio: drop const typing from iio_mount_matrix rotation

[PATCH 1/2] iio: drop const typing from iio_mount_matrix rotation

[Daniel Drake]

In order to conform with orientation data from the firmware, we
need to dynamically construct the mount matrix in the ST accelerometer
driver.

Drop the const type from the mount matrix data to make this possible.

Signed-off-by: Daniel Drake <drake@endlessm.com>
---
 include/linux/iio/iio.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index a74cb177dc6f..d3094ffeb9da 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -125,7 +125,7 @@ ssize_t iio_enum_write(struct iio_dev *indio_dev,
  *            main hardware
  */
 struct iio_mount_matrix {
-	const char *rotation[9];
+	char *rotation[9];
 };
 
 ssize_t iio_show_mount_matrix(struct iio_dev *indio_dev, uintptr_t priv,
-- 
2.19.1

[PATCH 2/2] iio: st_accel: use ACPI orientation data

[Daniel Drake]

Platform-specific ST accelerometer mount matrix information can be
provided by returning a package of 6 integers from the ACPI _ONT
method. This has been seen on Acer products such as Veriton Z4860G,
Z6860G and A890, which include a ST SMO8840 sensor. We have also
confirmed experimentally that the Windows driver uses such information.

The _ONT data format was explained by a ST vendor contact. However,
strangely enough, the _ONT transformations must be applied after first
applying another mount matrix which we determined experimentally. ST
have not commented on why this is the case, but we imagine that perhaps
earlier devices (before _ONT was introduced) required this translation
and hence it became 'standard.'

Interpret the _ONT data and export the equivalent mount matrix to
userspace.

If no _ONT data is present, no mount matrix is exported.

Signed-off-by: Daniel Drake <drake@endlessm.com>
---
 drivers/iio/accel/st_accel_core.c     | 171 +++++++++++++++++++++++++-
 include/linux/iio/common/st_sensors.h |   1 +
 2 files changed, 171 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
index 3e6fd5a8ac5b..54cc1a0c6a1d 100644
--- a/drivers/iio/accel/st_accel_core.c
+++ b/drivers/iio/accel/st_accel_core.c
@@ -11,6 +11,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/slab.h>
+#include <linux/acpi.h>
 #include <linux/errno.h>
 #include <linux/types.h>
 #include <linux/mutex.h>
@@ -917,12 +918,167 @@ static const struct iio_trigger_ops st_accel_trigger_ops = {
 #define ST_ACCEL_TRIGGER_OPS NULL
 #endif
 
+static const struct iio_mount_matrix *
+get_mount_matrix(const struct iio_dev *indio_dev,
+		 const struct iio_chan_spec *chan)
+{
+	struct st_sensor_data *adata = iio_priv(indio_dev);
+
+	return adata->mount_matrix;
+}
+
+static const struct iio_chan_spec_ext_info mount_matrix_ext_info[] = {
+	IIO_MOUNT_MATRIX(IIO_SHARED_BY_ALL, get_mount_matrix),
+	{ },
+};
+
+/* Read ST-specific _ONT orientation data from ACPI and generate an
+ * appropriate mount matrix.
+ */
+static int apply_acpi_orientation(struct iio_dev *indio_dev,
+				  struct iio_chan_spec *channels)
+{
+#ifdef CONFIG_ACPI
+	struct st_sensor_data *adata = iio_priv(indio_dev);
+	struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
+	struct acpi_device *adev;
+	union acpi_object *ont;
+	union acpi_object *elements;
+	acpi_status status;
+	int ret = -EINVAL;
+	unsigned int val;
+	int i, j;
+	int final_ont[3][3] = { 0 };
+
+	/* For some reason, ST's _ONT translation does not apply directly
+	 * to the data read from the sensor. Another translation must be
+	 * performed first, as described by the matrix below. Perhaps
+	 * ST required this specific translation for the first product
+	 * where the device was mounted?
+	 */
+	const int default_ont[3][3] = {
+		{  0,  1,  0 },
+		{ -1,  0,  0 },
+		{  0,  0, -1 },
+	};
+
+
+	adev = ACPI_COMPANION(adata->dev);
+	if (!adev)
+		return 0;
+
+	/* Read _ONT data, which should be a package of 6 integers. */
+	status = acpi_evaluate_object(adev->handle, "_ONT", NULL, &buffer);
+	if (status == AE_NOT_FOUND) {
+		return 0;
+	} else if (ACPI_FAILURE(status)) {
+		dev_warn(&indio_dev->dev, "failed to execute _ONT: %d\n",
+			 status);
+		return status;
+	}
+
+	ont = buffer.pointer;
+	if (ont->type != ACPI_TYPE_PACKAGE || ont->package.count != 6)
+		goto out;
+
+	/* The first 3 integers provide axis order information.
+	 * e.g. 0 1 2 would indicate normal X,Y,Z ordering.
+	 * e.g. 1 0 2 indicates that data arrives in order Y,X,Z.
+	 */
+	elements = ont->package.elements;
+	for (i = 0; i < 3; i++) {
+		if (elements[i].type != ACPI_TYPE_INTEGER)
+			goto out;
+
+		val = elements[i].integer.value;
+		if (val < 0 || val > 2)
+			goto out;
+
+		/* Avoiding full matrix multiplication, we simply reorder the
+		 * columns in the default_ont matrix according to the
+		 * ordering provided by _ONT.
+		 */
+		final_ont[0][i] = default_ont[0][val];
+		final_ont[1][i] = default_ont[1][val];
+		final_ont[2][i] = default_ont[2][val];
+	}
+
+	/* The final 3 integers provide sign flip information.
+	 * 0 means no change, 1 means flip.
+	 * e.g. 0 0 1 means that Z data should be sign-flipped.
+	 * This is applied after the axis reordering from above.
+	 */
+	elements += 3;
+	for (i = 0; i < 3; i++) {
+		if (elements[i].type != ACPI_TYPE_INTEGER)
+			goto out;
+
+		val = elements[i].integer.value;
+		if (val != 0 && val != 1)
+			goto out;
+		if (!val)
+			continue;
+
+		/* Flip the values in the indicated column */
+		final_ont[0][i] *= -1;
+		final_ont[1][i] *= -1;
+		final_ont[2][i] *= -1;
+	}
+
+	/* Convert our integer matrix to a string-based iio_mount_matrix */
+	adata->mount_matrix = devm_kmalloc(&indio_dev->dev,
+					   sizeof(*adata->mount_matrix),
+					   GFP_KERNEL);
+	if (!adata->mount_matrix) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	for (i = 0; i < 3; i++) {
+		for (j = 0; j < 3; j++) {
+			int matrix_val = final_ont[i][j];
+			char *str_value;
+
+			switch (matrix_val) {
+			case -1:
+				str_value = "-1";
+				break;
+			case 0:
+				str_value = "0";
+				break;
+			case 1:
+				str_value = "1";
+				break;
+			default:
+				goto out;
+			}
+			adata->mount_matrix->rotation[i * 3 + j] = str_value;
+		}
+	}
+
+	/* Expose the mount matrix via ext_info */
+	for (i = 0; i < indio_dev->num_channels; i++)
+		channels[i].ext_info = mount_matrix_ext_info;
+
+	ret = 0;
+	dev_info(&indio_dev->dev, "computed mount matrix from ACPI\n");
+
+out:
+	kfree(buffer.pointer);
+	return ret;
+#else /* !CONFIG_ACPI */
+	return 0;
+#endif
+}
+
 int st_accel_common_probe(struct iio_dev *indio_dev)
 {
 	struct st_sensor_data *adata = iio_priv(indio_dev);
 	struct st_sensors_platform_data *pdata =
 		(struct st_sensors_platform_data *)adata->dev->platform_data;
 	int irq = adata->get_irq_data_ready(indio_dev);
+	struct iio_chan_spec *channels;
+	size_t channels_size;
 	int err;
 
 	indio_dev->modes = INDIO_DIRECT_MODE;
@@ -941,9 +1097,22 @@ int st_accel_common_probe(struct iio_dev *indio_dev)
 
 	adata->num_data_channels = ST_ACCEL_NUMBER_DATA_CHANNELS;
 	adata->multiread_bit = adata->sensor_settings->multi_read_bit;
-	indio_dev->channels = adata->sensor_settings->ch;
 	indio_dev->num_channels = ST_SENSORS_NUMBER_ALL_CHANNELS;
 
+	channels_size = indio_dev->num_channels * sizeof(struct iio_chan_spec);
+	channels = devm_kmemdup(&indio_dev->dev,
+				adata->sensor_settings->ch,
+				channels_size, GFP_KERNEL);
+	if (!channels) {
+		err = -ENOMEM;
+		goto st_accel_power_off;
+	}
+
+	if (apply_acpi_orientation(indio_dev, channels))
+		dev_warn(&indio_dev->dev,
+			 "failed to apply ACPI orientation data: %d\n", err);
+
+	indio_dev->channels = channels;
 	adata->current_fullscale = (struct st_sensor_fullscale_avl *)
 					&adata->sensor_settings->fs.fs_avl[0];
 	adata->odr = adata->sensor_settings->odr.odr_avl[0].hz;
diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
index f9bd6e8ab138..fe15694128ac 100644
--- a/include/linux/iio/common/st_sensors.h
+++ b/include/linux/iio/common/st_sensors.h
@@ -260,6 +260,7 @@ struct st_sensor_settings {
 struct st_sensor_data {
 	struct device *dev;
 	struct iio_trigger *trig;
+	struct iio_mount_matrix *mount_matrix;
 	struct st_sensor_settings *sensor_settings;
 	struct st_sensor_fullscale_avl *current_fullscale;
 	struct regulator *vdd;
-- 
2.19.1

Re: [PATCH 2/2] iio: st_accel: use ACPI orientation data

[Jonathan Cameron]

On Thu, 20 Dec 2018 14:59:33 +0800
Daniel Drake <drake@endlessm.com> wrote:

> Platform-specific ST accelerometer mount matrix information can be
> provided by returning a package of 6 integers from the ACPI _ONT
> method. This has been seen on Acer products such as Veriton Z4860G,
> Z6860G and A890, which include a ST SMO8840 sensor. We have also
> confirmed experimentally that the Windows driver uses such information.
> 
> The _ONT data format was explained by a ST vendor contact. However,
> strangely enough, the _ONT transformations must be applied after first
> applying another mount matrix which we determined experimentally. ST
> have not commented on why this is the case, but we imagine that perhaps
> earlier devices (before _ONT was introduced) required this translation
> and hence it became 'standard.'
> 
> Interpret the _ONT data and export the equivalent mount matrix to
> userspace.
> 
> If no _ONT data is present, no mount matrix is exported.
> 
> Signed-off-by: Daniel Drake <drake@endlessm.com>
A trivial point.  Please stick to the local (and most common in kernel)
multiline comment syntax of
/*
 * asdfsdfs
 * more...
 */

Otherwise looks sensible.  However, I'd really like Dennis or someone
else at ST to take a look before I take this.  We are early in the cycle
(well it hasn't quite started yet) so plenty of time.  Give me a
bump in a few weeks if nothing is happening...

Thanks,

Jonathan


> ---
>  drivers/iio/accel/st_accel_core.c     | 171 +++++++++++++++++++++++++-
>  include/linux/iio/common/st_sensors.h |   1 +
>  2 files changed, 171 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
> index 3e6fd5a8ac5b..54cc1a0c6a1d 100644
> --- a/drivers/iio/accel/st_accel_core.c
> +++ b/drivers/iio/accel/st_accel_core.c
> @@ -11,6 +11,7 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/slab.h>
> +#include <linux/acpi.h>
>  #include <linux/errno.h>
>  #include <linux/types.h>
>  #include <linux/mutex.h>
> @@ -917,12 +918,167 @@ static const struct iio_trigger_ops st_accel_trigger_ops = {
>  #define ST_ACCEL_TRIGGER_OPS NULL
>  #endif
>  
> +static const struct iio_mount_matrix *
> +get_mount_matrix(const struct iio_dev *indio_dev,
> +		 const struct iio_chan_spec *chan)
> +{
> +	struct st_sensor_data *adata = iio_priv(indio_dev);
> +
> +	return adata->mount_matrix;
> +}
> +
> +static const struct iio_chan_spec_ext_info mount_matrix_ext_info[] = {
> +	IIO_MOUNT_MATRIX(IIO_SHARED_BY_ALL, get_mount_matrix),
> +	{ },
> +};
> +
> +/* Read ST-specific _ONT orientation data from ACPI and generate an
> + * appropriate mount matrix.
> + */
> +static int apply_acpi_orientation(struct iio_dev *indio_dev,
> +				  struct iio_chan_spec *channels)
> +{
> +#ifdef CONFIG_ACPI
> +	struct st_sensor_data *adata = iio_priv(indio_dev);
> +	struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
> +	struct acpi_device *adev;
> +	union acpi_object *ont;
> +	union acpi_object *elements;
> +	acpi_status status;
> +	int ret = -EINVAL;
> +	unsigned int val;
> +	int i, j;
> +	int final_ont[3][3] = { 0 };
> +
> +	/* For some reason, ST's _ONT translation does not apply directly
> +	 * to the data read from the sensor. Another translation must be
> +	 * performed first, as described by the matrix below. Perhaps
> +	 * ST required this specific translation for the first product
> +	 * where the device was mounted?
> +	 */
> +	const int default_ont[3][3] = {
> +		{  0,  1,  0 },
> +		{ -1,  0,  0 },
> +		{  0,  0, -1 },
> +	};
> +
> +
> +	adev = ACPI_COMPANION(adata->dev);
> +	if (!adev)
> +		return 0;
> +
> +	/* Read _ONT data, which should be a package of 6 integers. */
> +	status = acpi_evaluate_object(adev->handle, "_ONT", NULL, &buffer);
> +	if (status == AE_NOT_FOUND) {
> +		return 0;
> +	} else if (ACPI_FAILURE(status)) {
> +		dev_warn(&indio_dev->dev, "failed to execute _ONT: %d\n",
> +			 status);
> +		return status;
> +	}
> +
> +	ont = buffer.pointer;
> +	if (ont->type != ACPI_TYPE_PACKAGE || ont->package.count != 6)
> +		goto out;
> +
> +	/* The first 3 integers provide axis order information.
> +	 * e.g. 0 1 2 would indicate normal X,Y,Z ordering.
> +	 * e.g. 1 0 2 indicates that data arrives in order Y,X,Z.
> +	 */
> +	elements = ont->package.elements;
> +	for (i = 0; i < 3; i++) {
> +		if (elements[i].type != ACPI_TYPE_INTEGER)
> +			goto out;
> +
> +		val = elements[i].integer.value;
> +		if (val < 0 || val > 2)
> +			goto out;
> +
> +		/* Avoiding full matrix multiplication, we simply reorder the
> +		 * columns in the default_ont matrix according to the
> +		 * ordering provided by _ONT.
> +		 */
> +		final_ont[0][i] = default_ont[0][val];
> +		final_ont[1][i] = default_ont[1][val];
> +		final_ont[2][i] = default_ont[2][val];
> +	}
> +
> +	/* The final 3 integers provide sign flip information.
> +	 * 0 means no change, 1 means flip.
> +	 * e.g. 0 0 1 means that Z data should be sign-flipped.
> +	 * This is applied after the axis reordering from above.
> +	 */
> +	elements += 3;
> +	for (i = 0; i < 3; i++) {
> +		if (elements[i].type != ACPI_TYPE_INTEGER)
> +			goto out;
> +
> +		val = elements[i].integer.value;
> +		if (val != 0 && val != 1)
> +			goto out;
> +		if (!val)
> +			continue;
> +
> +		/* Flip the values in the indicated column */
> +		final_ont[0][i] *= -1;
> +		final_ont[1][i] *= -1;
> +		final_ont[2][i] *= -1;
> +	}
> +
> +	/* Convert our integer matrix to a string-based iio_mount_matrix */
> +	adata->mount_matrix = devm_kmalloc(&indio_dev->dev,
> +					   sizeof(*adata->mount_matrix),
> +					   GFP_KERNEL);
> +	if (!adata->mount_matrix) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	for (i = 0; i < 3; i++) {
> +		for (j = 0; j < 3; j++) {
> +			int matrix_val = final_ont[i][j];
> +			char *str_value;
> +
> +			switch (matrix_val) {
> +			case -1:
> +				str_value = "-1";
> +				break;
> +			case 0:
> +				str_value = "0";
> +				break;
> +			case 1:
> +				str_value = "1";
> +				break;
> +			default:
> +				goto out;
> +			}
> +			adata->mount_matrix->rotation[i * 3 + j] = str_value;
> +		}
> +	}
> +
> +	/* Expose the mount matrix via ext_info */
> +	for (i = 0; i < indio_dev->num_channels; i++)
> +		channels[i].ext_info = mount_matrix_ext_info;
> +
> +	ret = 0;
> +	dev_info(&indio_dev->dev, "computed mount matrix from ACPI\n");
> +
> +out:
> +	kfree(buffer.pointer);
> +	return ret;
> +#else /* !CONFIG_ACPI */
> +	return 0;
> +#endif
> +}
> +
>  int st_accel_common_probe(struct iio_dev *indio_dev)
>  {
>  	struct st_sensor_data *adata = iio_priv(indio_dev);
>  	struct st_sensors_platform_data *pdata =
>  		(struct st_sensors_platform_data *)adata->dev->platform_data;
>  	int irq = adata->get_irq_data_ready(indio_dev);
> +	struct iio_chan_spec *channels;
> +	size_t channels_size;
>  	int err;
>  
>  	indio_dev->modes = INDIO_DIRECT_MODE;
> @@ -941,9 +1097,22 @@ int st_accel_common_probe(struct iio_dev *indio_dev)
>  
>  	adata->num_data_channels = ST_ACCEL_NUMBER_DATA_CHANNELS;
>  	adata->multiread_bit = adata->sensor_settings->multi_read_bit;
> -	indio_dev->channels = adata->sensor_settings->ch;
>  	indio_dev->num_channels = ST_SENSORS_NUMBER_ALL_CHANNELS;
>  
> +	channels_size = indio_dev->num_channels * sizeof(struct iio_chan_spec);
> +	channels = devm_kmemdup(&indio_dev->dev,
> +				adata->sensor_settings->ch,
> +				channels_size, GFP_KERNEL);
> +	if (!channels) {
> +		err = -ENOMEM;
> +		goto st_accel_power_off;
> +	}
> +
> +	if (apply_acpi_orientation(indio_dev, channels))
> +		dev_warn(&indio_dev->dev,
> +			 "failed to apply ACPI orientation data: %d\n", err);
> +
> +	indio_dev->channels = channels;
>  	adata->current_fullscale = (struct st_sensor_fullscale_avl *)
>  					&adata->sensor_settings->fs.fs_avl[0];
>  	adata->odr = adata->sensor_settings->odr.odr_avl[0].hz;
> diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
> index f9bd6e8ab138..fe15694128ac 100644
> --- a/include/linux/iio/common/st_sensors.h
> +++ b/include/linux/iio/common/st_sensors.h
> @@ -260,6 +260,7 @@ struct st_sensor_settings {
>  struct st_sensor_data {
>  	struct device *dev;
>  	struct iio_trigger *trig;
> +	struct iio_mount_matrix *mount_matrix;
>  	struct st_sensor_settings *sensor_settings;
>  	struct st_sensor_fullscale_avl *current_fullscale;
>  	struct regulator *vdd;

Re: [PATCH 2/2] iio: st_accel: use ACPI orientation data

[Jonathan Cameron]

On Sat, 22 Dec 2018 18:09:14 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> On Thu, 20 Dec 2018 14:59:33 +0800
> Daniel Drake <drake@endlessm.com> wrote:
> 
> > Platform-specific ST accelerometer mount matrix information can be
> > provided by returning a package of 6 integers from the ACPI _ONT
> > method. This has been seen on Acer products such as Veriton Z4860G,
> > Z6860G and A890, which include a ST SMO8840 sensor. We have also
> > confirmed experimentally that the Windows driver uses such information.
> > 
> > The _ONT data format was explained by a ST vendor contact. However,
> > strangely enough, the _ONT transformations must be applied after first
> > applying another mount matrix which we determined experimentally. ST
> > have not commented on why this is the case, but we imagine that perhaps
> > earlier devices (before _ONT was introduced) required this translation
> > and hence it became 'standard.'
> > 
> > Interpret the _ONT data and export the equivalent mount matrix to
> > userspace.
> > 
> > If no _ONT data is present, no mount matrix is exported.
> > 
> > Signed-off-by: Daniel Drake <drake@endlessm.com>  
> A trivial point.  Please stick to the local (and most common in kernel)
> multiline comment syntax of
> /*
>  * asdfsdfs
>  * more...
>  */
> 
> Otherwise looks sensible.  However, I'd really like Dennis or someone
> else at ST to take a look before I take this.  We are early in the cycle
> (well it hasn't quite started yet) so plenty of time.  Give me a
> bump in a few weeks if nothing is happening...

Note  I'm still lookign for Denis or someone else to come back on this one!

Thanks,

Jonathan
> 
> Thanks,
> 
> Jonathan
> 
> 
> > ---
> >  drivers/iio/accel/st_accel_core.c     | 171 +++++++++++++++++++++++++-
> >  include/linux/iio/common/st_sensors.h |   1 +
> >  2 files changed, 171 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
> > index 3e6fd5a8ac5b..54cc1a0c6a1d 100644
> > --- a/drivers/iio/accel/st_accel_core.c
> > +++ b/drivers/iio/accel/st_accel_core.c
> > @@ -11,6 +11,7 @@
> >  #include <linux/kernel.h>
> >  #include <linux/module.h>
> >  #include <linux/slab.h>
> > +#include <linux/acpi.h>
> >  #include <linux/errno.h>
> >  #include <linux/types.h>
> >  #include <linux/mutex.h>
> > @@ -917,12 +918,167 @@ static const struct iio_trigger_ops st_accel_trigger_ops = {
> >  #define ST_ACCEL_TRIGGER_OPS NULL
> >  #endif
> >  
> > +static const struct iio_mount_matrix *
> > +get_mount_matrix(const struct iio_dev *indio_dev,
> > +		 const struct iio_chan_spec *chan)
> > +{
> > +	struct st_sensor_data *adata = iio_priv(indio_dev);
> > +
> > +	return adata->mount_matrix;
> > +}
> > +
> > +static const struct iio_chan_spec_ext_info mount_matrix_ext_info[] = {
> > +	IIO_MOUNT_MATRIX(IIO_SHARED_BY_ALL, get_mount_matrix),
> > +	{ },
> > +};
> > +
> > +/* Read ST-specific _ONT orientation data from ACPI and generate an
> > + * appropriate mount matrix.
> > + */
> > +static int apply_acpi_orientation(struct iio_dev *indio_dev,
> > +				  struct iio_chan_spec *channels)
> > +{
> > +#ifdef CONFIG_ACPI
> > +	struct st_sensor_data *adata = iio_priv(indio_dev);
> > +	struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
> > +	struct acpi_device *adev;
> > +	union acpi_object *ont;
> > +	union acpi_object *elements;
> > +	acpi_status status;
> > +	int ret = -EINVAL;
> > +	unsigned int val;
> > +	int i, j;
> > +	int final_ont[3][3] = { 0 };
> > +
> > +	/* For some reason, ST's _ONT translation does not apply directly
> > +	 * to the data read from the sensor. Another translation must be
> > +	 * performed first, as described by the matrix below. Perhaps
> > +	 * ST required this specific translation for the first product
> > +	 * where the device was mounted?
> > +	 */
> > +	const int default_ont[3][3] = {
> > +		{  0,  1,  0 },
> > +		{ -1,  0,  0 },
> > +		{  0,  0, -1 },
> > +	};
> > +
> > +
> > +	adev = ACPI_COMPANION(adata->dev);
> > +	if (!adev)
> > +		return 0;
> > +
> > +	/* Read _ONT data, which should be a package of 6 integers. */
> > +	status = acpi_evaluate_object(adev->handle, "_ONT", NULL, &buffer);
> > +	if (status == AE_NOT_FOUND) {
> > +		return 0;
> > +	} else if (ACPI_FAILURE(status)) {
> > +		dev_warn(&indio_dev->dev, "failed to execute _ONT: %d\n",
> > +			 status);
> > +		return status;
> > +	}
> > +
> > +	ont = buffer.pointer;
> > +	if (ont->type != ACPI_TYPE_PACKAGE || ont->package.count != 6)
> > +		goto out;
> > +
> > +	/* The first 3 integers provide axis order information.
> > +	 * e.g. 0 1 2 would indicate normal X,Y,Z ordering.
> > +	 * e.g. 1 0 2 indicates that data arrives in order Y,X,Z.
> > +	 */
> > +	elements = ont->package.elements;
> > +	for (i = 0; i < 3; i++) {
> > +		if (elements[i].type != ACPI_TYPE_INTEGER)
> > +			goto out;
> > +
> > +		val = elements[i].integer.value;
> > +		if (val < 0 || val > 2)
> > +			goto out;
> > +
> > +		/* Avoiding full matrix multiplication, we simply reorder the
> > +		 * columns in the default_ont matrix according to the
> > +		 * ordering provided by _ONT.
> > +		 */
> > +		final_ont[0][i] = default_ont[0][val];
> > +		final_ont[1][i] = default_ont[1][val];
> > +		final_ont[2][i] = default_ont[2][val];
> > +	}
> > +
> > +	/* The final 3 integers provide sign flip information.
> > +	 * 0 means no change, 1 means flip.
> > +	 * e.g. 0 0 1 means that Z data should be sign-flipped.
> > +	 * This is applied after the axis reordering from above.
> > +	 */
> > +	elements += 3;
> > +	for (i = 0; i < 3; i++) {
> > +		if (elements[i].type != ACPI_TYPE_INTEGER)
> > +			goto out;
> > +
> > +		val = elements[i].integer.value;
> > +		if (val != 0 && val != 1)
> > +			goto out;
> > +		if (!val)
> > +			continue;
> > +
> > +		/* Flip the values in the indicated column */
> > +		final_ont[0][i] *= -1;
> > +		final_ont[1][i] *= -1;
> > +		final_ont[2][i] *= -1;
> > +	}
> > +
> > +	/* Convert our integer matrix to a string-based iio_mount_matrix */
> > +	adata->mount_matrix = devm_kmalloc(&indio_dev->dev,
> > +					   sizeof(*adata->mount_matrix),
> > +					   GFP_KERNEL);
> > +	if (!adata->mount_matrix) {
> > +		ret = -ENOMEM;
> > +		goto out;
> > +	}
> > +
> > +	for (i = 0; i < 3; i++) {
> > +		for (j = 0; j < 3; j++) {
> > +			int matrix_val = final_ont[i][j];
> > +			char *str_value;
> > +
> > +			switch (matrix_val) {
> > +			case -1:
> > +				str_value = "-1";
> > +				break;
> > +			case 0:
> > +				str_value = "0";
> > +				break;
> > +			case 1:
> > +				str_value = "1";
> > +				break;
> > +			default:
> > +				goto out;
> > +			}
> > +			adata->mount_matrix->rotation[i * 3 + j] = str_value;
> > +		}
> > +	}
> > +
> > +	/* Expose the mount matrix via ext_info */
> > +	for (i = 0; i < indio_dev->num_channels; i++)
> > +		channels[i].ext_info = mount_matrix_ext_info;
> > +
> > +	ret = 0;
> > +	dev_info(&indio_dev->dev, "computed mount matrix from ACPI\n");
> > +
> > +out:
> > +	kfree(buffer.pointer);
> > +	return ret;
> > +#else /* !CONFIG_ACPI */
> > +	return 0;
> > +#endif
> > +}
> > +
> >  int st_accel_common_probe(struct iio_dev *indio_dev)
> >  {
> >  	struct st_sensor_data *adata = iio_priv(indio_dev);
> >  	struct st_sensors_platform_data *pdata =
> >  		(struct st_sensors_platform_data *)adata->dev->platform_data;
> >  	int irq = adata->get_irq_data_ready(indio_dev);
> > +	struct iio_chan_spec *channels;
> > +	size_t channels_size;
> >  	int err;
> >  
> >  	indio_dev->modes = INDIO_DIRECT_MODE;
> > @@ -941,9 +1097,22 @@ int st_accel_common_probe(struct iio_dev *indio_dev)
> >  
> >  	adata->num_data_channels = ST_ACCEL_NUMBER_DATA_CHANNELS;
> >  	adata->multiread_bit = adata->sensor_settings->multi_read_bit;
> > -	indio_dev->channels = adata->sensor_settings->ch;
> >  	indio_dev->num_channels = ST_SENSORS_NUMBER_ALL_CHANNELS;
> >  
> > +	channels_size = indio_dev->num_channels * sizeof(struct iio_chan_spec);
> > +	channels = devm_kmemdup(&indio_dev->dev,
> > +				adata->sensor_settings->ch,
> > +				channels_size, GFP_KERNEL);
> > +	if (!channels) {
> > +		err = -ENOMEM;
> > +		goto st_accel_power_off;
> > +	}
> > +
> > +	if (apply_acpi_orientation(indio_dev, channels))
> > +		dev_warn(&indio_dev->dev,
> > +			 "failed to apply ACPI orientation data: %d\n", err);
> > +
> > +	indio_dev->channels = channels;
> >  	adata->current_fullscale = (struct st_sensor_fullscale_avl *)
> >  					&adata->sensor_settings->fs.fs_avl[0];
> >  	adata->odr = adata->sensor_settings->odr.odr_avl[0].hz;
> > diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
> > index f9bd6e8ab138..fe15694128ac 100644
> > --- a/include/linux/iio/common/st_sensors.h
> > +++ b/include/linux/iio/common/st_sensors.h
> > @@ -260,6 +260,7 @@ struct st_sensor_settings {
> >  struct st_sensor_data {
> >  	struct device *dev;
> >  	struct iio_trigger *trig;
> > +	struct iio_mount_matrix *mount_matrix;
> >  	struct st_sensor_settings *sensor_settings;
> >  	struct st_sensor_fullscale_avl *current_fullscale;
> >  	struct regulator *vdd;  
> 

RE: [PATCH 2/2] iio: st_accel: use ACPI orientation data

[Denis CIOCCA]

Hi Jonathan, Daniel,

I am not familiar with ACPI and I am not sure why axis orientation is not following the standard there.
What I wonder is if this is related to SMO8840, Acer or in general to ST accelerometers integration. Your patch seems to be fine technically but are you able to confirm it will works even outside the 3 parts tested? The other accel supported by ACPI probing SMO8A90 is affected also?

Denis



-----Original Message-----
From: Jonathan Cameron <jic23@kernel.org> 
Sent: Saturday, January 12, 2019 11:13 AM
To: Daniel Drake <drake@endlessm.com>
Cc: knaack.h@gmx.de; lars@metafoo.de; pmeerw@pmeerw.net; linux-iio@vger.kernel.org; linux@endlessm.com; lorenzo.bianconi83@gmail.com; Denis CIOCCA <denis.ciocca@st.com>; hadess@hadess.net; hdegoede@redhat.com
Subject: Re: [PATCH 2/2] iio: st_accel: use ACPI orientation data

On Sat, 22 Dec 2018 18:09:14 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> On Thu, 20 Dec 2018 14:59:33 +0800
> Daniel Drake <drake@endlessm.com> wrote:
> 
> > Platform-specific ST accelerometer mount matrix information can be 
> > provided by returning a package of 6 integers from the ACPI _ONT 
> > method. This has been seen on Acer products such as Veriton Z4860G, 
> > Z6860G and A890, which include a ST SMO8840 sensor. We have also 
> > confirmed experimentally that the Windows driver uses such information.
> > 
> > The _ONT data format was explained by a ST vendor contact. However, 
> > strangely enough, the _ONT transformations must be applied after 
> > first applying another mount matrix which we determined 
> > experimentally. ST have not commented on why this is the case, but 
> > we imagine that perhaps earlier devices (before _ONT was introduced) 
> > required this translation and hence it became 'standard.'
> > 
> > Interpret the _ONT data and export the equivalent mount matrix to 
> > userspace.
> > 
> > If no _ONT data is present, no mount matrix is exported.
> > 
> > Signed-off-by: Daniel Drake <drake@endlessm.com>
> A trivial point.  Please stick to the local (and most common in 
> kernel) multiline comment syntax of
> /*
>  * asdfsdfs
>  * more...
>  */
> 
> Otherwise looks sensible.  However, I'd really like Dennis or someone 
> else at ST to take a look before I take this.  We are early in the 
> cycle (well it hasn't quite started yet) so plenty of time.  Give me a 
> bump in a few weeks if nothing is happening...

Note  I'm still lookign for Denis or someone else to come back on this one!

Thanks,

Jonathan
> 
> Thanks,
> 
> Jonathan
> 
> 
> > ---
> >  drivers/iio/accel/st_accel_core.c     | 171 +++++++++++++++++++++++++-
> >  include/linux/iio/common/st_sensors.h |   1 +
> >  2 files changed, 171 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iio/accel/st_accel_core.c 
> > b/drivers/iio/accel/st_accel_core.c
> > index 3e6fd5a8ac5b..54cc1a0c6a1d 100644
> > --- a/drivers/iio/accel/st_accel_core.c
> > +++ b/drivers/iio/accel/st_accel_core.c
> > @@ -11,6 +11,7 @@
> >  #include <linux/kernel.h>
> >  #include <linux/module.h>
> >  #include <linux/slab.h>
> > +#include <linux/acpi.h>
> >  #include <linux/errno.h>
> >  #include <linux/types.h>
> >  #include <linux/mutex.h>
> > @@ -917,12 +918,167 @@ static const struct iio_trigger_ops 
> > st_accel_trigger_ops = {  #define ST_ACCEL_TRIGGER_OPS NULL  #endif
> >  
> > +static const struct iio_mount_matrix * get_mount_matrix(const 
> > +struct iio_dev *indio_dev,
> > +		 const struct iio_chan_spec *chan) {
> > +	struct st_sensor_data *adata = iio_priv(indio_dev);
> > +
> > +	return adata->mount_matrix;
> > +}
> > +
> > +static const struct iio_chan_spec_ext_info mount_matrix_ext_info[] = {
> > +	IIO_MOUNT_MATRIX(IIO_SHARED_BY_ALL, get_mount_matrix),
> > +	{ },
> > +};
> > +
> > +/* Read ST-specific _ONT orientation data from ACPI and generate an
> > + * appropriate mount matrix.
> > + */
> > +static int apply_acpi_orientation(struct iio_dev *indio_dev,
> > +				  struct iio_chan_spec *channels) { #ifdef CONFIG_ACPI
> > +	struct st_sensor_data *adata = iio_priv(indio_dev);
> > +	struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
> > +	struct acpi_device *adev;
> > +	union acpi_object *ont;
> > +	union acpi_object *elements;
> > +	acpi_status status;
> > +	int ret = -EINVAL;
> > +	unsigned int val;
> > +	int i, j;
> > +	int final_ont[3][3] = { 0 };
> > +
> > +	/* For some reason, ST's _ONT translation does not apply directly
> > +	 * to the data read from the sensor. Another translation must be
> > +	 * performed first, as described by the matrix below. Perhaps
> > +	 * ST required this specific translation for the first product
> > +	 * where the device was mounted?
> > +	 */
> > +	const int default_ont[3][3] = {
> > +		{  0,  1,  0 },
> > +		{ -1,  0,  0 },
> > +		{  0,  0, -1 },
> > +	};
> > +
> > +
> > +	adev = ACPI_COMPANION(adata->dev);
> > +	if (!adev)
> > +		return 0;
> > +
> > +	/* Read _ONT data, which should be a package of 6 integers. */
> > +	status = acpi_evaluate_object(adev->handle, "_ONT", NULL, &buffer);
> > +	if (status == AE_NOT_FOUND) {
> > +		return 0;
> > +	} else if (ACPI_FAILURE(status)) {
> > +		dev_warn(&indio_dev->dev, "failed to execute _ONT: %d\n",
> > +			 status);
> > +		return status;
> > +	}
> > +
> > +	ont = buffer.pointer;
> > +	if (ont->type != ACPI_TYPE_PACKAGE || ont->package.count != 6)
> > +		goto out;
> > +
> > +	/* The first 3 integers provide axis order information.
> > +	 * e.g. 0 1 2 would indicate normal X,Y,Z ordering.
> > +	 * e.g. 1 0 2 indicates that data arrives in order Y,X,Z.
> > +	 */
> > +	elements = ont->package.elements;
> > +	for (i = 0; i < 3; i++) {
> > +		if (elements[i].type != ACPI_TYPE_INTEGER)
> > +			goto out;
> > +
> > +		val = elements[i].integer.value;
> > +		if (val < 0 || val > 2)
> > +			goto out;
> > +
> > +		/* Avoiding full matrix multiplication, we simply reorder the
> > +		 * columns in the default_ont matrix according to the
> > +		 * ordering provided by _ONT.
> > +		 */
> > +		final_ont[0][i] = default_ont[0][val];
> > +		final_ont[1][i] = default_ont[1][val];
> > +		final_ont[2][i] = default_ont[2][val];
> > +	}
> > +
> > +	/* The final 3 integers provide sign flip information.
> > +	 * 0 means no change, 1 means flip.
> > +	 * e.g. 0 0 1 means that Z data should be sign-flipped.
> > +	 * This is applied after the axis reordering from above.
> > +	 */
> > +	elements += 3;
> > +	for (i = 0; i < 3; i++) {
> > +		if (elements[i].type != ACPI_TYPE_INTEGER)
> > +			goto out;
> > +
> > +		val = elements[i].integer.value;
> > +		if (val != 0 && val != 1)
> > +			goto out;
> > +		if (!val)
> > +			continue;
> > +
> > +		/* Flip the values in the indicated column */
> > +		final_ont[0][i] *= -1;
> > +		final_ont[1][i] *= -1;
> > +		final_ont[2][i] *= -1;
> > +	}
> > +
> > +	/* Convert our integer matrix to a string-based iio_mount_matrix */
> > +	adata->mount_matrix = devm_kmalloc(&indio_dev->dev,
> > +					   sizeof(*adata->mount_matrix),
> > +					   GFP_KERNEL);
> > +	if (!adata->mount_matrix) {
> > +		ret = -ENOMEM;
> > +		goto out;
> > +	}
> > +
> > +	for (i = 0; i < 3; i++) {
> > +		for (j = 0; j < 3; j++) {
> > +			int matrix_val = final_ont[i][j];
> > +			char *str_value;
> > +
> > +			switch (matrix_val) {
> > +			case -1:
> > +				str_value = "-1";
> > +				break;
> > +			case 0:
> > +				str_value = "0";
> > +				break;
> > +			case 1:
> > +				str_value = "1";
> > +				break;
> > +			default:
> > +				goto out;
> > +			}
> > +			adata->mount_matrix->rotation[i * 3 + j] = str_value;
> > +		}
> > +	}
> > +
> > +	/* Expose the mount matrix via ext_info */
> > +	for (i = 0; i < indio_dev->num_channels; i++)
> > +		channels[i].ext_info = mount_matrix_ext_info;
> > +
> > +	ret = 0;
> > +	dev_info(&indio_dev->dev, "computed mount matrix from ACPI\n");
> > +
> > +out:
> > +	kfree(buffer.pointer);
> > +	return ret;
> > +#else /* !CONFIG_ACPI */
> > +	return 0;
> > +#endif
> > +}
> > +
> >  int st_accel_common_probe(struct iio_dev *indio_dev)  {
> >  	struct st_sensor_data *adata = iio_priv(indio_dev);
> >  	struct st_sensors_platform_data *pdata =
> >  		(struct st_sensors_platform_data *)adata->dev->platform_data;
> >  	int irq = adata->get_irq_data_ready(indio_dev);
> > +	struct iio_chan_spec *channels;
> > +	size_t channels_size;
> >  	int err;
> >  
> >  	indio_dev->modes = INDIO_DIRECT_MODE; @@ -941,9 +1097,22 @@ int 
> > st_accel_common_probe(struct iio_dev *indio_dev)
> >  
> >  	adata->num_data_channels = ST_ACCEL_NUMBER_DATA_CHANNELS;
> >  	adata->multiread_bit = adata->sensor_settings->multi_read_bit;
> > -	indio_dev->channels = adata->sensor_settings->ch;
> >  	indio_dev->num_channels = ST_SENSORS_NUMBER_ALL_CHANNELS;
> >  
> > +	channels_size = indio_dev->num_channels * sizeof(struct iio_chan_spec);
> > +	channels = devm_kmemdup(&indio_dev->dev,
> > +				adata->sensor_settings->ch,
> > +				channels_size, GFP_KERNEL);
> > +	if (!channels) {
> > +		err = -ENOMEM;
> > +		goto st_accel_power_off;
> > +	}
> > +
> > +	if (apply_acpi_orientation(indio_dev, channels))
> > +		dev_warn(&indio_dev->dev,
> > +			 "failed to apply ACPI orientation data: %d\n", err);
> > +
> > +	indio_dev->channels = channels;
> >  	adata->current_fullscale = (struct st_sensor_fullscale_avl *)
> >  					&adata->sensor_settings->fs.fs_avl[0];
> >  	adata->odr = adata->sensor_settings->odr.odr_avl[0].hz;
> > diff --git a/include/linux/iio/common/st_sensors.h 
> > b/include/linux/iio/common/st_sensors.h
> > index f9bd6e8ab138..fe15694128ac 100644
> > --- a/include/linux/iio/common/st_sensors.h
> > +++ b/include/linux/iio/common/st_sensors.h
> > @@ -260,6 +260,7 @@ struct st_sensor_settings {  struct 
> > st_sensor_data {
> >  	struct device *dev;
> >  	struct iio_trigger *trig;
> > +	struct iio_mount_matrix *mount_matrix;
> >  	struct st_sensor_settings *sensor_settings;
> >  	struct st_sensor_fullscale_avl *current_fullscale;
> >  	struct regulator *vdd;
> 

Re: [PATCH 2/2] iio: st_accel: use ACPI orientation data

[Daniel Drake]

Hi Denis,

On Tue, Jan 15, 2019 at 8:18 AM Denis CIOCCA <denis.ciocca@st.com> wrote:
> I am not familiar with ACPI and I am not sure why axis orientation is not following the standard there.
> What I wonder is if this is related to SMO8840, Acer or in general to ST accelerometers integration. Your patch seems to be fine technically but are you able to confirm it will works even outside the 3 parts tested? The other accel supported by ACPI probing SMO8A90 is affected also?

The _ONT data format was explained to us by a vendor support contact
at ST. (however, the fact that the _ONT transformation must be applied
after first applying another translation was determined experimentally
by me).

We have seen this _ONT data present in ACPI tables from multiple
product vendors, on SMO8820, SMO8821 and SMO8840 devices, so I believe
there is no doubt that it is a ST-specific thing.

The specific transformation behaviour in my patch has only been tested
on products with SMO8840 though, since those are the only ones we have
on hand.

Daniel

Re: [PATCH 1/2] iio: drop const typing from iio_mount_matrix rotation

[Jonathan Cameron]

On Thu, 20 Dec 2018 14:59:32 +0800
Daniel Drake <drake@endlessm.com> wrote:

> In order to conform with orientation data from the firmware, we
> need to dynamically construct the mount matrix in the ST accelerometer
> driver.
> 
> Drop the const type from the mount matrix data to make this possible.
> 
> Signed-off-by: Daniel Drake <drake@endlessm.com>
Not quite that simple...

drivers/iio/industrialio-core.c:551:21: error: passing argument 3 of ‘of_property_read_string_array’ from incompatible pointer type [-Werror=incompatible-pointer-types]
     propname, matrix->rotation,
               ~~~~~~^~~~~~~~~~
In file included from ./include/linux/iio/iio.h:16,
                 from drivers/iio/industrialio-core.c:29:
./include/linux/of.h:1107:42: note: expected ‘const char **’ but argument is of type ‘char **’
       const char *propname, const char **out_strs,
                             ~~~~~~~~~~~~~^~~~~~~~
cc1: some warnings being treated as errors

Also, I didn't think that much about it until now, but I'm not sure
why you need this change in the first place. It is an array of
pointers to constant characters.  Given we allocate said array
before setting this variable to it and don't modify it via this
pointer, I believe we are fine with out this.

Compiler certainly isn't complaining about patch 2 without this
one.  Am I missing something?

Jonathan


> ---
>  include/linux/iio/iio.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index a74cb177dc6f..d3094ffeb9da 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -125,7 +125,7 @@ ssize_t iio_enum_write(struct iio_dev *indio_dev,
>   *            main hardware
>   */
>  struct iio_mount_matrix {
> -	const char *rotation[9];
> +	char *rotation[9];
>  };
>  
>  ssize_t iio_show_mount_matrix(struct iio_dev *indio_dev, uintptr_t priv,

Re: [PATCH 2/2] iio: st_accel: use ACPI orientation data

[Jonathan Cameron]

On Tue, 15 Jan 2019 11:09:51 +0800
Daniel Drake <drake@endlessm.com> wrote:

> Hi Denis,
> 
> On Tue, Jan 15, 2019 at 8:18 AM Denis CIOCCA <denis.ciocca@st.com> wrote:
> > I am not familiar with ACPI and I am not sure why axis orientation is not following the standard there.
> > What I wonder is if this is related to SMO8840, Acer or in general to ST accelerometers integration. Your patch seems to be fine technically but are you able to confirm it will works even outside the 3 parts tested? The other accel supported by ACPI probing SMO8A90 is affected also?  
> 
> The _ONT data format was explained to us by a vendor support contact
> at ST. (however, the fact that the _ONT transformation must be applied
> after first applying another translation was determined experimentally
> by me).
> 
> We have seen this _ONT data present in ACPI tables from multiple
> product vendors, on SMO8820, SMO8821 and SMO8840 devices, so I believe
> there is no doubt that it is a ST-specific thing.
> 
> The specific transformation behaviour in my patch has only been tested
> on products with SMO8840 though, since those are the only ones we have
> on hand.
> 
> Daniel
I suspect we aren't going to come to a firm conclusion on this any time
soon.  As it only effects devices that do provide _ONT, it is fairly
unlikely to cause a regression.  Ideally we would know if it worked on
the other devices where this is known to exist but such is life.

If people agree that I'm right in patch 1 not being necessary and
we don't have any objections to merging this I'll do so, probably next
weekend now.

Thanks,

Jonathan

Re: [PATCH 1/2] iio: drop const typing from iio_mount_matrix rotation

[Daniel Drake]

On Sun, Jan 20, 2019 at 12:49 AM Jonathan Cameron <jic23@kernel.org> wrote:
> Also, I didn't think that much about it until now, but I'm not sure
> why you need this change in the first place. It is an array of
> pointers to constant characters.  Given we allocate said array
> before setting this variable to it and don't modify it via this
> pointer, I believe we are fine with out this.
>
> Compiler certainly isn't complaining about patch 2 without this
> one.  Am I missing something?

Hmm, I definitely had a need for it, but I can't reproduce any
compiler errors without it now, as you say.

I think I changed approach somewhere along the way, and this patch is
no longer necessary.

Thanks
Daniel

Re: [PATCH 2/2] iio: st_accel: use ACPI orientation data

[Jonathan Cameron]

On Sat, 19 Jan 2019 16:50:15 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> On Tue, 15 Jan 2019 11:09:51 +0800
> Daniel Drake <drake@endlessm.com> wrote:
> 
> > Hi Denis,
> > 
> > On Tue, Jan 15, 2019 at 8:18 AM Denis CIOCCA <denis.ciocca@st.com> wrote:  
> > > I am not familiar with ACPI and I am not sure why axis orientation is not following the standard there.
> > > What I wonder is if this is related to SMO8840, Acer or in general to ST accelerometers integration. Your patch seems to be fine technically but are you able to confirm it will works even outside the 3 parts tested? The other accel supported by ACPI probing SMO8A90 is affected also?    
> > 
> > The _ONT data format was explained to us by a vendor support contact
> > at ST. (however, the fact that the _ONT transformation must be applied
> > after first applying another translation was determined experimentally
> > by me).
> > 
> > We have seen this _ONT data present in ACPI tables from multiple
> > product vendors, on SMO8820, SMO8821 and SMO8840 devices, so I believe
> > there is no doubt that it is a ST-specific thing.
> > 
> > The specific transformation behaviour in my patch has only been tested
> > on products with SMO8840 though, since those are the only ones we have
> > on hand.
> > 
> > Daniel  
> I suspect we aren't going to come to a firm conclusion on this any time
> soon.  As it only effects devices that do provide _ONT, it is fairly
> unlikely to cause a regression.  Ideally we would know if it worked on
> the other devices where this is known to exist but such is life.
> 
> If people agree that I'm right in patch 1 not being necessary and
> we don't have any objections to merging this I'll do so, probably next
> weekend now.
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

Thanks,

Jonathan

> 
> Thanks,
> 
> Jonathan
> 
>