> On Wed, 11 Jan 2023 13:09:40 +0100
> Philippe De Muyter <phdm@xxxxxxx> wrote:
>
> > Hello Lorenzo and list,
> >
> > I do not find a "*mount_matrix" entry in sysfs for a 'ism330dlc_gyro'
> > iio device.
> > Is that normal ?
> > Is a fix available ?
>
> Looks like the channel definition for the gyro does not include an
> appropriate ext_info entry unlike the accelerometer channels which
> have one with mount_matrix support.
>
> From a quick glance looks like a simple fix. Add that entry.
something like (per-sensor mount_matrix):
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
index 5b6f195748fc..5e0c184e1055 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
@@ -113,6 +113,8 @@ enum st_lsm6dsx_hw_id {
.storagebits = 16, \
.endianness = IIO_LE, \
}, \
+ .ext_info = st_lsm6dsx_gyro_ext_info, \
+ .num_event_specs = 1, \
}
struct st_lsm6dsx_reg {
@@ -356,6 +358,7 @@ enum st_lsm6dsx_fifo_mode {
* @decimator: Sensor decimation factor.
* @sip: Number of samples in a given pattern.
* @ts_ref: Sensor timestamp reference for hw one.
+ * @orientation: sensor chip orientation relative to main hardware.
* @ext_info: Sensor settings if it is connected to i2c controller
*/
struct st_lsm6dsx_sensor {
@@ -371,6 +374,8 @@ struct st_lsm6dsx_sensor {
u8 sip;
s64 ts_ref;
+ struct iio_mount_matrix orientation;
+
struct {
const struct st_lsm6dsx_ext_dev_settings *settings;
u32 slv_odr;
@@ -398,7 +403,6 @@ 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.
* @scan: Temporary buffers used to align data before iio_push_to_buffers()
*/
struct st_lsm6dsx_hw {
@@ -427,7 +431,6 @@ struct st_lsm6dsx_hw {
const struct st_lsm6dsx_settings *settings;
- struct iio_mount_matrix orientation;
/* Ensure natural alignment of buffer elements */
struct {
__le16 channels[3];
@@ -511,9 +514,8 @@ 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;
+ return &sensor->orientation;
}
static inline int
@@ -533,4 +535,10 @@ struct iio_chan_spec_ext_info __maybe_unused st_lsm6dsx_accel_ext_info[] = {
{ }
};
+static const
+struct iio_chan_spec_ext_info __maybe_unused st_lsm6dsx_gyro_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 3f6060c64f32..15c7184a1895 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -2359,6 +2359,9 @@ static struct iio_dev *st_lsm6dsx_alloc_iiodev(struct st_lsm6dsx_hw *hw,
sensor->gain = hw->settings->fs_table[id].fs_avl[0].gain;
sensor->watermark = 1;
+ if (iio_read_mount_matrix(hw->dev, &sensor->orientation))
+ return NULL;
+
switch (id) {
case ST_LSM6DSX_ID_ACC:
iio_dev->info = &st_lsm6dsx_acc_info;
@@ -2676,10 +2679,6 @@ int st_lsm6dsx_probe(struct device *dev, int irq, int hw_id,
return err;
}
- err = iio_read_mount_matrix(hw->dev, &hw->orientation);
- if (err)
- return err;
-
for (i = 0; i < ST_LSM6DSX_ID_MAX; i++) {
if (!hw->iio_devs[i])
continue;
> >
> > Some more info :
> >
> > I have backported drivers/iio/imu/st_lsm6dsx to linux-4.9 in order
> > to drive a ism330dlc imu on a custom board. The chip is correctly
> > detected and two devices are created in /sys/bus/iio/devices/
> >
> > the first one (where name is 'ism330dlc_gyro') has the following entries :
> >
> > me@proto4:~$ ls /sys/bus/iio/devices/iio\:device1/
> > buffer in_anglvel_x_raw sampling_frequency
> > current_timestamp_clock in_anglvel_y_raw sampling_frequency_available
> > dev in_anglvel_z_raw scan_elements
> > in_anglvel_scale name subsystem
> > in_anglvel_scale_available power uevent
> > me@proto4:~$
> >
> > the second one (where name is 'ism330dlc_accel') has those entries :
> >
> > me@proto4:~$ ls /sys/bus/iio/devices/iio\:device2
> > buffer in_accel_x_raw sampling_frequency
> > current_timestamp_clock in_accel_y_raw sampling_frequency_available
> > dev in_accel_z_raw scan_elements
> > events mount_matrix subsystem
> > in_accel_scale name uevent
> > in_accel_scale_available power
> > me@proto4:~$
> >
> > The 'mount_matrix' entry is only present in the 'ism330dlc_accel' device
> > but not in the 'ism330dlc_gyro' device.
> >
> > On a similar board, but with mpu9250 imu, I get only one iio:deviceX
> > entry but with two *mount_matrix entries :
> >
> > in_accel_mount_matrix
> > in_anglvel_mount_matrix
> >
> > In both cases, I would have expected only one 'iio:deviceX' entry with
> > only one 'mount_matrix' entry.
>
> There are multiple devices because the driver predates the addition
> of multiple buffer support to IIO and IIRC is capable of producing data
> at different sampling rates for the accelerometer and the gyros.
> Hence when it was implemented the only choice was to register multiple
> devices in order to get the multiple buffers. It's ABI now so we can't
> fix it in an old driver unfortunately. We'd do this differently today..
>
> The double mount_matrix for the mpu9250 is a little odd and I can't
> immediately spot why that one is happening.
>
>
> >
> > Best regards
> >
> > Philippe
> >
>
Attachment:
signature.asc
Description: PGP signature
