On Thu, 12 Jan 2023 12:32:11 +0100
Lorenzo Bianconi <lorenzo@xxxxxxxxxx> wrote:
> > Hello Lorenzo,
> >
> > On Thu, Jan 12, 2023 at 10:51:03AM +0100, Lorenzo Bianconi wrote:
> > > Date: Thu, 12 Jan 2023 10:51:03 +0100
> > > From: Lorenzo Bianconi <lorenzo@xxxxxxxxxx>
> > > To: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> > > Cc: Philippe De Muyter <phdm@xxxxxxx>, linux-iio@xxxxxxxxxxxxxxx
> > > Subject: Re: sysfs mount_matrix for st_lsm6dsx gyro
> > >
> > > > 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.
> > >
> > > I am not sure if accel and gyro can be mounted with a different orientation.
> > > Do you think we should have a per-sensor mount_matrix?
> >
> > My chip is a 'ism330dlc'. It's one chip containing an accel and a gyro,
> > so the mount_matrix should be the same for the accel and the gyro.
> > Unfortunately the accel and the gyro are presented in /sysfs as two
> > separate devices, and only the accel one has a 'mount_matrix' entry.
> > So a user looking for any gyro will find the gyro entry, but without
> > 'mount_matrix'.
> >
> > I have followed Jonathan's proposal and added this simple patch in
> > the definition of the ST_LSM6DSX_CHANNEL macro :
> >
> > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > index d80ba2e688ed..9d18145d5041 100644
> > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > @@ -96,6 +96,7 @@ enum st_lsm6dsx_hw_id {
> > .storagebits = 16, \
> > .endianness = IIO_LE, \
> > }, \
> > + .ext_info = st_lsm6dsx_accel_ext_info, \
> > }
>
> I am fine with this approach, probably having a per-sensor mount_matrix is
> unnecessary. Can you please just rename st_lsm6dsx_accel_ext_info in
> st_lsm6dsx_ext_info?
>
> Regards,
> Lorenzo
>
> >
> > struct st_lsm6dsx_reg {
> >
> > Another fix would be to create only one chip entry in sysfs, but that's
> > above my knowledge of this driver.
We can't make that change. It would be ABI breakage - userspace code would
stop working.
Jonathan
> >
> > Best regards
> >
> > Philippe
>
