| Message ID | 20210517233322.383043-4-linus.walleij@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/5,v2] iio: st_sensors: Create extended attr macro | expand |
Hi, On Tue, May 18, 2021 at 01:33:21AM +0200, Linus Walleij wrote: > Add support to read and present the mounting matrix on ST magnetometers. > > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Cc: Denis Ciocca <denis.ciocca@st.com> > Cc: Daniel Drake <drake@endlessm.com> > Cc: Stephan Gerhold <stephan@gerhold.net> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > ChangeLog v1->v2: > - New patch because why not. Thanks for patching the other ST drivers as well! That reminds me about something I was thinking about when I was making the changes a week ago. :) > --- > drivers/iio/magnetometer/st_magn_core.c | 64 ++++++++++++++++++------- > 1 file changed, 46 insertions(+), 18 deletions(-) > > diff --git a/drivers/iio/magnetometer/st_magn_core.c b/drivers/iio/magnetometer/st_magn_core.c > index 71faebd07feb..fa587975cb85 100644 > --- a/drivers/iio/magnetometer/st_magn_core.c > +++ b/drivers/iio/magnetometer/st_magn_core.c > @@ -53,51 +53,74 @@ > #define ST_MAGN_3_OUT_Y_L_ADDR 0x6a > #define ST_MAGN_3_OUT_Z_L_ADDR 0x6c > > +static const struct iio_mount_matrix * > +st_magn_get_mount_matrix(const struct iio_dev *indio_dev, > + const struct iio_chan_spec *chan) > +{ > + struct st_sensor_data *mdata = iio_priv(indio_dev); > + > + return &mdata->mount_matrix; > +} > + > +static const struct iio_chan_spec_ext_info st_magn_mount_matrix_ext_info[] = { > + IIO_MOUNT_MATRIX(IIO_SHARED_BY_ALL, st_magn_get_mount_matrix), > + { }, > +}; > + I'm not sure if this is worth it (or even a particularly good idea), but we could share this function and struct in st_sensors_core.c since it's exactly the same for st_accel/magn/gyro AFAICT. (It just operates on the common struct st_sensor_data). This could be done with a macro like ST_SENSORS_LSM_CHANNELS_MOUNT_MATRIX(...) (maybe a bit long and clumsy) instead of _EXT(...) and pointing to a struct iio_chan_spec_ext_info somewhere in st_sensors_core.c. The disadvantage however is that st_accel/magn/gyro couldn't add other (sensor-specific) ext_info later. Not sure if that is realistic though. I wasn't entirely sure myself that's why I went with the _EXT(...) instead (especially since I only wanted to patch st_accel). Just wanted to mention it :) Stephan
On Tue, May 18, 2021 at 01:33:21AM +0200, Linus Walleij wrote: > Add support to read and present the mounting matrix on ST magnetometers. ... > +static const struct iio_chan_spec_ext_info st_magn_mount_matrix_ext_info[] = { > + IIO_MOUNT_MATRIX(IIO_SHARED_BY_ALL, st_magn_get_mount_matrix), > + { }, No need to have comma in terminator line. > +};
On Tue, 18 May 2021 11:01:37 +0200 Stephan Gerhold <stephan@gerhold.net> wrote: > Hi, > > On Tue, May 18, 2021 at 01:33:21AM +0200, Linus Walleij wrote: > > Add support to read and present the mounting matrix on ST magnetometers. > > > > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Cc: Denis Ciocca <denis.ciocca@st.com> > > Cc: Daniel Drake <drake@endlessm.com> > > Cc: Stephan Gerhold <stephan@gerhold.net> > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > > --- > > ChangeLog v1->v2: > > - New patch because why not. > > Thanks for patching the other ST drivers as well! > That reminds me about something I was thinking about when I was making > the changes a week ago. :) > > > --- > > drivers/iio/magnetometer/st_magn_core.c | 64 ++++++++++++++++++------- > > 1 file changed, 46 insertions(+), 18 deletions(-) > > > > diff --git a/drivers/iio/magnetometer/st_magn_core.c b/drivers/iio/magnetometer/st_magn_core.c > > index 71faebd07feb..fa587975cb85 100644 > > --- a/drivers/iio/magnetometer/st_magn_core.c > > +++ b/drivers/iio/magnetometer/st_magn_core.c > > @@ -53,51 +53,74 @@ > > #define ST_MAGN_3_OUT_Y_L_ADDR 0x6a > > #define ST_MAGN_3_OUT_Z_L_ADDR 0x6c > > > > +static const struct iio_mount_matrix * > > +st_magn_get_mount_matrix(const struct iio_dev *indio_dev, > > + const struct iio_chan_spec *chan) > > +{ > > + struct st_sensor_data *mdata = iio_priv(indio_dev); > > + > > + return &mdata->mount_matrix; > > +} > > + > > +static const struct iio_chan_spec_ext_info st_magn_mount_matrix_ext_info[] = { > > + IIO_MOUNT_MATRIX(IIO_SHARED_BY_ALL, st_magn_get_mount_matrix), > > + { }, > > +}; > > + > > I'm not sure if this is worth it (or even a particularly good idea), > but we could share this function and struct in st_sensors_core.c > since it's exactly the same for st_accel/magn/gyro AFAICT. > (It just operates on the common struct st_sensor_data). > > This could be done with a macro like ST_SENSORS_LSM_CHANNELS_MOUNT_MATRIX(...) > (maybe a bit long and clumsy) instead of _EXT(...) and pointing to > a struct iio_chan_spec_ext_info somewhere in st_sensors_core.c. > > The disadvantage however is that st_accel/magn/gyro couldn't add other > (sensor-specific) ext_info later. Not sure if that is realistic though. My gut feeling is it's not worth the effort for such a small bit of repetition, but I don't feel that strongly about it so will take whatever comes in v3 ;) Jonathan > > I wasn't entirely sure myself that's why I went with the _EXT(...) > instead (especially since I only wanted to patch st_accel). > Just wanted to mention it :) > > Stephan
diff --git a/drivers/iio/magnetometer/st_magn_core.c b/drivers/iio/magnetometer/st_magn_core.c index 71faebd07feb..fa587975cb85 100644 --- a/drivers/iio/magnetometer/st_magn_core.c +++ b/drivers/iio/magnetometer/st_magn_core.c @@ -53,51 +53,74 @@ #define ST_MAGN_3_OUT_Y_L_ADDR 0x6a #define ST_MAGN_3_OUT_Z_L_ADDR 0x6c +static const struct iio_mount_matrix * +st_magn_get_mount_matrix(const struct iio_dev *indio_dev, + const struct iio_chan_spec *chan) +{ + struct st_sensor_data *mdata = iio_priv(indio_dev); + + return &mdata->mount_matrix; +} + +static const struct iio_chan_spec_ext_info st_magn_mount_matrix_ext_info[] = { + IIO_MOUNT_MATRIX(IIO_SHARED_BY_ALL, st_magn_get_mount_matrix), + { }, +}; + static const struct iio_chan_spec st_magn_16bit_channels[] = { - ST_SENSORS_LSM_CHANNELS(IIO_MAGN, + ST_SENSORS_LSM_CHANNELS_EXT(IIO_MAGN, BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), ST_SENSORS_SCAN_X, 1, IIO_MOD_X, 's', IIO_BE, 16, 16, - ST_MAGN_DEFAULT_OUT_X_H_ADDR), - ST_SENSORS_LSM_CHANNELS(IIO_MAGN, + ST_MAGN_DEFAULT_OUT_X_H_ADDR, + st_magn_mount_matrix_ext_info), + ST_SENSORS_LSM_CHANNELS_EXT(IIO_MAGN, BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), ST_SENSORS_SCAN_Y, 1, IIO_MOD_Y, 's', IIO_BE, 16, 16, - ST_MAGN_DEFAULT_OUT_Y_H_ADDR), - ST_SENSORS_LSM_CHANNELS(IIO_MAGN, + ST_MAGN_DEFAULT_OUT_Y_H_ADDR, + st_magn_mount_matrix_ext_info), + ST_SENSORS_LSM_CHANNELS_EXT(IIO_MAGN, BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), ST_SENSORS_SCAN_Z, 1, IIO_MOD_Z, 's', IIO_BE, 16, 16, - ST_MAGN_DEFAULT_OUT_Z_H_ADDR), + ST_MAGN_DEFAULT_OUT_Z_H_ADDR, + st_magn_mount_matrix_ext_info), IIO_CHAN_SOFT_TIMESTAMP(3) }; static const struct iio_chan_spec st_magn_2_16bit_channels[] = { - ST_SENSORS_LSM_CHANNELS(IIO_MAGN, + ST_SENSORS_LSM_CHANNELS_EXT(IIO_MAGN, BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), ST_SENSORS_SCAN_X, 1, IIO_MOD_X, 's', IIO_LE, 16, 16, - ST_MAGN_2_OUT_X_L_ADDR), - ST_SENSORS_LSM_CHANNELS(IIO_MAGN, + ST_MAGN_2_OUT_X_L_ADDR, + st_magn_mount_matrix_ext_info), + ST_SENSORS_LSM_CHANNELS_EXT(IIO_MAGN, BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), ST_SENSORS_SCAN_Y, 1, IIO_MOD_Y, 's', IIO_LE, 16, 16, - ST_MAGN_2_OUT_Y_L_ADDR), - ST_SENSORS_LSM_CHANNELS(IIO_MAGN, + ST_MAGN_2_OUT_Y_L_ADDR, + st_magn_mount_matrix_ext_info), + ST_SENSORS_LSM_CHANNELS_EXT(IIO_MAGN, BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), ST_SENSORS_SCAN_Z, 1, IIO_MOD_Z, 's', IIO_LE, 16, 16, - ST_MAGN_2_OUT_Z_L_ADDR), + ST_MAGN_2_OUT_Z_L_ADDR, + st_magn_mount_matrix_ext_info), IIO_CHAN_SOFT_TIMESTAMP(3) }; static const struct iio_chan_spec st_magn_3_16bit_channels[] = { - ST_SENSORS_LSM_CHANNELS(IIO_MAGN, + ST_SENSORS_LSM_CHANNELS_EXT(IIO_MAGN, BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), ST_SENSORS_SCAN_X, 1, IIO_MOD_X, 's', IIO_LE, 16, 16, - ST_MAGN_3_OUT_X_L_ADDR), - ST_SENSORS_LSM_CHANNELS(IIO_MAGN, + ST_MAGN_3_OUT_X_L_ADDR, + st_magn_mount_matrix_ext_info), + ST_SENSORS_LSM_CHANNELS_EXT(IIO_MAGN, BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), ST_SENSORS_SCAN_Y, 1, IIO_MOD_Y, 's', IIO_LE, 16, 16, - ST_MAGN_3_OUT_Y_L_ADDR), - ST_SENSORS_LSM_CHANNELS(IIO_MAGN, + ST_MAGN_3_OUT_Y_L_ADDR, + st_magn_mount_matrix_ext_info), + ST_SENSORS_LSM_CHANNELS_EXT(IIO_MAGN, BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), ST_SENSORS_SCAN_Z, 1, IIO_MOD_Z, 's', IIO_LE, 16, 16, - ST_MAGN_3_OUT_Z_L_ADDR), + ST_MAGN_3_OUT_Z_L_ADDR, + st_magn_mount_matrix_ext_info), IIO_CHAN_SOFT_TIMESTAMP(3) }; @@ -507,6 +530,11 @@ int st_magn_common_probe(struct iio_dev *indio_dev) indio_dev->channels = mdata->sensor_settings->ch; indio_dev->num_channels = ST_SENSORS_NUMBER_ALL_CHANNELS; + err = iio_read_mount_matrix(mdata->dev, "mount-matrix", + &mdata->mount_matrix); + if (err) + goto st_magn_power_off; + mdata->current_fullscale = &mdata->sensor_settings->fs.fs_avl[0]; mdata->odr = mdata->sensor_settings->odr.odr_avl[0].hz;
Add support to read and present the mounting matrix on ST magnetometers. Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Cc: Denis Ciocca <denis.ciocca@st.com> Cc: Daniel Drake <drake@endlessm.com> Cc: Stephan Gerhold <stephan@gerhold.net> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- ChangeLog v1->v2: - New patch because why not. --- drivers/iio/magnetometer/st_magn_core.c | 64 ++++++++++++++++++------- 1 file changed, 46 insertions(+), 18 deletions(-)