| Message ID | 8593125b207045797adb9406aa2d3d2f43c30153.1586170271.git.lorenzo@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] iio: imu: st_lsm6dsx: drop huge include in sensor-hub driver | expand |
On Mon, Apr 6, 2020 at 1:52 PM Lorenzo Bianconi <lorenzo@kernel.org> wrote: > > st_sensors.h contains common stm sensor definitions but in > st_lsm6dsx_shub driver it is used just to introduce the default Still doesn't fully clear why only this part of the st_lsm6dsx is not okay with the header. You need to explain that common/st_sensors is for separate ST sensor drivers, while LSM6DSx is a driver for certain IMU which *does not* use ST common infrastructure. > wai address for LIS3MDL sensor. > Drop this largely unconnected include and introduce the default wai > address in st_lsm6dsx_ext_dev_settings register map > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > --- > Changes since v1: > - improve commit message > --- > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c > index 280925dd8edb..947ca3a7dcaf 100644 > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c > @@ -28,7 +28,6 @@ > #include <linux/iio/sysfs.h> > #include <linux/bitfield.h> > > -#include <linux/iio/common/st_sensors.h> > #include "st_lsm6dsx.h" > > #define ST_LSM6DSX_SLV_ADDR(n, base) ((base) + (n) * 3) > @@ -93,7 +92,7 @@ static const struct st_lsm6dsx_ext_dev_settings st_lsm6dsx_ext_dev_table[] = { > { > .i2c_addr = { 0x1e }, > .wai = { > - .addr = ST_SENSORS_DEFAULT_WAI_ADDRESS, > + .addr = 0x0f, > .val = 0x3d, > }, > .id = ST_LSM6DSX_ID_MAGN, > -- > 2.25.1 >
> On Mon, Apr 6, 2020 at 1:52 PM Lorenzo Bianconi <lorenzo@kernel.org> wrote: > > > > st_sensors.h contains common stm sensor definitions but in > > > st_lsm6dsx_shub driver it is used just to introduce the default > > Still doesn't fully clear why only this part of the st_lsm6dsx is not > okay with the header. > You need to explain that common/st_sensors is for separate ST sensor > drivers, while LSM6DSx is a driver for certain IMU which *does not* > use ST common infrastructure. I guess it is pretty simple and evident: we did not use it at all before the LISM3DL commit and at moment we need it just for ST_SENSORS_DEFAULT_WAI_ADDRESS definition. It is better to hard code the value directly. > > > wai address for LIS3MDL sensor. > > Drop this largely unconnected include and introduce the default wai > > address in st_lsm6dsx_ext_dev_settings register map > > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > > --- > > Changes since v1: > > - improve commit message > > --- > > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c > > index 280925dd8edb..947ca3a7dcaf 100644 > > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c > > @@ -28,7 +28,6 @@ > > #include <linux/iio/sysfs.h> > > #include <linux/bitfield.h> > > > > -#include <linux/iio/common/st_sensors.h> > > #include "st_lsm6dsx.h" > > > > #define ST_LSM6DSX_SLV_ADDR(n, base) ((base) + (n) * 3) > > @@ -93,7 +92,7 @@ static const struct st_lsm6dsx_ext_dev_settings st_lsm6dsx_ext_dev_table[] = { > > { > > .i2c_addr = { 0x1e }, > > .wai = { > > - .addr = ST_SENSORS_DEFAULT_WAI_ADDRESS, > > + .addr = 0x0f, > > .val = 0x3d, > > }, > > .id = ST_LSM6DSX_ID_MAGN, > > -- > > 2.25.1 > > > > > -- > With Best Regards, > Andy Shevchenko
On Mon, Apr 6, 2020 at 2:20 PM Lorenzo Bianconi <lorenzo@kernel.org> wrote: > > On Mon, Apr 6, 2020 at 1:52 PM Lorenzo Bianconi <lorenzo@kernel.org> wrote: > > > > > > st_sensors.h contains common stm sensor definitions but in > > > > > st_lsm6dsx_shub driver it is used just to introduce the default > > > > Still doesn't fully clear why only this part of the st_lsm6dsx is not > > okay with the header. > > You need to explain that common/st_sensors is for separate ST sensor > > drivers, while LSM6DSx is a driver for certain IMU which *does not* > > use ST common infrastructure. > > I guess it is pretty simple and evident: we did not use it at all before > the LISM3DL commit and at moment we need it just for ST_SENSORS_DEFAULT_WAI_ADDRESS > definition. It is better to hard code the value directly. Yes, my point is that for a reader (*) of this commit message is not fully clear why on a sudden we did that. (*) the reader may not know full history. > > > wai address for LIS3MDL sensor. > > > Drop this largely unconnected include and introduce the default wai > > > address in st_lsm6dsx_ext_dev_settings register map
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c index 280925dd8edb..947ca3a7dcaf 100644 --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c @@ -28,7 +28,6 @@ #include <linux/iio/sysfs.h> #include <linux/bitfield.h> -#include <linux/iio/common/st_sensors.h> #include "st_lsm6dsx.h" #define ST_LSM6DSX_SLV_ADDR(n, base) ((base) + (n) * 3) @@ -93,7 +92,7 @@ static const struct st_lsm6dsx_ext_dev_settings st_lsm6dsx_ext_dev_table[] = { { .i2c_addr = { 0x1e }, .wai = { - .addr = ST_SENSORS_DEFAULT_WAI_ADDRESS, + .addr = 0x0f, .val = 0x3d, }, .id = ST_LSM6DSX_ID_MAGN,