2020-01-04 13:22:45

by Martin Kepplinger

[permalink] [raw]
Subject: [PATCH v2] iio: imu: st_lsm6dsx: add mount matrix support

Allow to read the mount-matrix device tree property and provide the
mount_matrix file for userspace to read.

Signed-off-by: Martin Kepplinger <[email protected]>
---

tested using the lsm9ds1 on the librem5-devkit (and userspace tools like
iio-sensor-proxy) where this will be needed.

thanks,

martin

revision history
----------------
v2: additions and simplifications according to Lorenzo's review. thanks.


drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 19 +++++++++++++++++++
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 2 +-
2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
index a763ff46f596..7076fc8c4c3b 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
@@ -76,6 +76,7 @@ enum st_lsm6dsx_hw_id {
.endianness = IIO_LE, \
}, \
.event_spec = &st_lsm6dsx_event, \
+ .ext_info = st_lsm6dsx_accel_ext_info, \
.num_event_specs = 1, \
}

@@ -380,6 +381,7 @@ struct st_lsm6dsx_sensor {
* @enable_event: enabled event bitmask.
* @iio_devs: Pointers to acc/gyro iio_dev instances.
* @settings: Pointer to the specific sensor settings in use.
+ * @orientation: sensor chip orientation relative to main hardware.
*/
struct st_lsm6dsx_hw {
struct device *dev;
@@ -406,6 +408,8 @@ struct st_lsm6dsx_hw {
struct iio_dev *iio_devs[ST_LSM6DSX_ID_MAX];

const struct st_lsm6dsx_settings *settings;
+
+ struct iio_mount_matrix orientation;
};

static __maybe_unused const struct iio_event_spec st_lsm6dsx_event = {
@@ -479,4 +483,19 @@ st_lsm6dsx_write_locked(struct st_lsm6dsx_hw *hw, unsigned int addr,
return err;
}

+static const inline struct iio_mount_matrix *
+st_lsm6dsx_get_mount_matrix(const struct iio_dev *iio_dev,
+ const struct iio_chan_spec *chan)
+{
+ struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
+ struct st_lsm6dsx_hw *hw = sensor->hw;
+
+ return &hw->orientation;
+}
+
+static const struct iio_chan_spec_ext_info st_lsm6dsx_accel_ext_info[] = {
+ IIO_MOUNT_MATRIX(IIO_SHARED_BY_ALL, st_lsm6dsx_get_mount_matrix),
+ { }
+};
+
#endif /* ST_LSM6DSX_H */
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index 0c64e35c7599..27e157f8a031 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -2325,7 +2325,7 @@ int st_lsm6dsx_probe(struct device *dev, int irq, int hw_id,
(pdata && pdata->wakeup_source))
device_init_wakeup(dev, true);

- return 0;
+ return iio_read_mount_matrix(hw->dev, "mount-matrix", &hw->orientation);
}
EXPORT_SYMBOL(st_lsm6dsx_probe);

--
2.20.1


2020-01-11 12:16:41

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2] iio: imu: st_lsm6dsx: add mount matrix support

On Sat, 4 Jan 2020 14:20:52 +0100
Martin Kepplinger <[email protected]> wrote:

> Allow to read the mount-matrix device tree property and provide the
> mount_matrix file for userspace to read.
>
> Signed-off-by: Martin Kepplinger <[email protected]>
> ---
>
> tested using the lsm9ds1 on the librem5-devkit (and userspace tools like
> iio-sensor-proxy) where this will be needed.
>
> thanks,
>
> martin
>
> revision history
> ----------------
> v2: additions and simplifications according to Lorenzo's review. thanks.
>
>
> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 19 +++++++++++++++++++
> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 2 +-
> 2 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> index a763ff46f596..7076fc8c4c3b 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> @@ -76,6 +76,7 @@ enum st_lsm6dsx_hw_id {
> .endianness = IIO_LE, \
> }, \
> .event_spec = &st_lsm6dsx_event, \
> + .ext_info = st_lsm6dsx_accel_ext_info, \
> .num_event_specs = 1, \
> }
>
> @@ -380,6 +381,7 @@ struct st_lsm6dsx_sensor {
> * @enable_event: enabled event bitmask.
> * @iio_devs: Pointers to acc/gyro iio_dev instances.
> * @settings: Pointer to the specific sensor settings in use.
> + * @orientation: sensor chip orientation relative to main hardware.
> */
> struct st_lsm6dsx_hw {
> struct device *dev;
> @@ -406,6 +408,8 @@ struct st_lsm6dsx_hw {
> struct iio_dev *iio_devs[ST_LSM6DSX_ID_MAX];
>
> const struct st_lsm6dsx_settings *settings;
> +
> + struct iio_mount_matrix orientation;
> };
>
> static __maybe_unused const struct iio_event_spec st_lsm6dsx_event = {
> @@ -479,4 +483,19 @@ st_lsm6dsx_write_locked(struct st_lsm6dsx_hw *hw, unsigned int addr,
> return err;
> }
>
> +static const inline struct iio_mount_matrix *
> +st_lsm6dsx_get_mount_matrix(const struct iio_dev *iio_dev,
> + const struct iio_chan_spec *chan)
> +{
> + struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
> + struct st_lsm6dsx_hw *hw = sensor->hw;
> +
> + return &hw->orientation;
> +}
> +
> +static const struct iio_chan_spec_ext_info st_lsm6dsx_accel_ext_info[] = {
> + IIO_MOUNT_MATRIX(IIO_SHARED_BY_ALL, st_lsm6dsx_get_mount_matrix),
> + { }
> +};
> +
> #endif /* ST_LSM6DSX_H */
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index 0c64e35c7599..27e157f8a031 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -2325,7 +2325,7 @@ int st_lsm6dsx_probe(struct device *dev, int irq, int hw_id,
> (pdata && pdata->wakeup_source))
> device_init_wakeup(dev, true);
>
> - return 0;
> + return iio_read_mount_matrix(hw->dev, "mount-matrix", &hw->orientation);

Race condition. iio_device_register has already been called by this point.
Hence userspace interfaces are exposed. Userspace can read the mount matrix
before it is initialized. This needs to be earlier in probe.

Jonathan

> }
> EXPORT_SYMBOL(st_lsm6dsx_probe);
>

2020-01-11 12:19:20

by Martin Kepplinger

[permalink] [raw]
Subject: Re: [PATCH v2] iio: imu: st_lsm6dsx: add mount matrix support



Am 11. Jänner 2020 13:15:18 MEZ schrieb Jonathan Cameron <[email protected]>:
>On Sat, 4 Jan 2020 14:20:52 +0100
>Martin Kepplinger <[email protected]> wrote:
>
>> Allow to read the mount-matrix device tree property and provide the
>> mount_matrix file for userspace to read.
>>
>> Signed-off-by: Martin Kepplinger <[email protected]>
>> ---
>>
>> tested using the lsm9ds1 on the librem5-devkit (and userspace tools
>like
>> iio-sensor-proxy) where this will be needed.
>>
>> thanks,
>>
>> martin
>>
>> revision history
>> ----------------
>> v2: additions and simplifications according to Lorenzo's review.
>thanks.
>>
>>
>> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 19
>+++++++++++++++++++
>> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 2 +-
>> 2 files changed, 20 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
>b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
>> index a763ff46f596..7076fc8c4c3b 100644
>> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
>> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
>> @@ -76,6 +76,7 @@ enum st_lsm6dsx_hw_id {
>> .endianness = IIO_LE, \
>> }, \
>> .event_spec = &st_lsm6dsx_event, \
>> + .ext_info = st_lsm6dsx_accel_ext_info, \
>> .num_event_specs = 1, \
>> }
>>
>> @@ -380,6 +381,7 @@ struct st_lsm6dsx_sensor {
>> * @enable_event: enabled event bitmask.
>> * @iio_devs: Pointers to acc/gyro iio_dev instances.
>> * @settings: Pointer to the specific sensor settings in use.
>> + * @orientation: sensor chip orientation relative to main hardware.
>> */
>> struct st_lsm6dsx_hw {
>> struct device *dev;
>> @@ -406,6 +408,8 @@ struct st_lsm6dsx_hw {
>> struct iio_dev *iio_devs[ST_LSM6DSX_ID_MAX];
>>
>> const struct st_lsm6dsx_settings *settings;
>> +
>> + struct iio_mount_matrix orientation;
>> };
>>
>> static __maybe_unused const struct iio_event_spec st_lsm6dsx_event =
>{
>> @@ -479,4 +483,19 @@ st_lsm6dsx_write_locked(struct st_lsm6dsx_hw
>*hw, unsigned int addr,
>> return err;
>> }
>>
>> +static const inline struct iio_mount_matrix *
>> +st_lsm6dsx_get_mount_matrix(const struct iio_dev *iio_dev,
>> + const struct iio_chan_spec *chan)
>> +{
>> + struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
>> + struct st_lsm6dsx_hw *hw = sensor->hw;
>> +
>> + return &hw->orientation;
>> +}
>> +
>> +static const struct iio_chan_spec_ext_info
>st_lsm6dsx_accel_ext_info[] = {
>> + IIO_MOUNT_MATRIX(IIO_SHARED_BY_ALL, st_lsm6dsx_get_mount_matrix),
>> + { }
>> +};
>> +
>> #endif /* ST_LSM6DSX_H */
>> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
>b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
>> index 0c64e35c7599..27e157f8a031 100644
>> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
>> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
>> @@ -2325,7 +2325,7 @@ int st_lsm6dsx_probe(struct device *dev, int
>irq, int hw_id,
>> (pdata && pdata->wakeup_source))
>> device_init_wakeup(dev, true);
>>
>> - return 0;
>> + return iio_read_mount_matrix(hw->dev, "mount-matrix",
>&hw->orientation);
>
>Race condition. iio_device_register has already been called by this
>point.
>Hence userspace interfaces are exposed. Userspace can read the mount
>matrix
>before it is initialized. This needs to be earlier in probe.
>
>Jonathan

ah thanks! I somehow knew this looks wrong but I was lazy. resending soon.


>
>> }
>> EXPORT_SYMBOL(st_lsm6dsx_probe);
>>

--
Sent from mobile.