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@xxxxxxxxxxxxxxx>
> Cc: Denis Ciocca <denis.ciocca@xxxxxx>
> Cc: Daniel Drake <drake@xxxxxxxxxxxx>
> Cc: Stephan Gerhold <stephan@xxxxxxxxxxx>
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
> 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
