* [PATCH 0/2] add support for ASM330LHHX @ 2022-04-02 10:09 Lorenzo Bianconi 2022-04-02 10:09 ` [PATCH 1/2] iio: imu: st_lsm6dsx: add support to ASM330LHHX Lorenzo Bianconi 2022-04-02 10:09 ` [PATCH 2/2] dt-bindings: iio: imu: st_lsm6dsx: add asm330lhhx device bindings Lorenzo Bianconi 0 siblings, 2 replies; 11+ messages in thread From: Lorenzo Bianconi @ 2022-04-02 10:09 UTC (permalink / raw) To: jic23; +Cc: linux-iio, devicetree, lorenzo.bianconi, robh Add support for ASM330LHHX IMU mems sensor https://www.st.com/resource/en/datasheet/asm330lhhx.pdf Lorenzo Bianconi (2): iio: imu: st_lsm6dsx: add support to ASM330LHHX dt-bindings: iio: imu: st_lsm6dsx: add asm330lhhx device bindings Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml | 1 + drivers/iio/imu/st_lsm6dsx/Kconfig | 6 +++--- drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 2 ++ drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 3 ++- drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 6 +++++- drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c | 5 +++++ drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c | 5 +++++ 7 files changed, 23 insertions(+), 5 deletions(-) -- 2.35.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] iio: imu: st_lsm6dsx: add support to ASM330LHHX 2022-04-02 10:09 [PATCH 0/2] add support for ASM330LHHX Lorenzo Bianconi @ 2022-04-02 10:09 ` Lorenzo Bianconi 2022-04-02 16:22 ` Jonathan Cameron 2022-04-02 10:09 ` [PATCH 2/2] dt-bindings: iio: imu: st_lsm6dsx: add asm330lhhx device bindings Lorenzo Bianconi 1 sibling, 1 reply; 11+ messages in thread From: Lorenzo Bianconi @ 2022-04-02 10:09 UTC (permalink / raw) To: jic23; +Cc: linux-iio, devicetree, lorenzo.bianconi, robh Add support to STM ASM330LHHX (acc + gyro) Mems sensor https://www.st.com/resource/en/datasheet/asm330lhhx.pdf Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> --- drivers/iio/imu/st_lsm6dsx/Kconfig | 6 +++--- drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 2 ++ drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 3 ++- drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 6 +++++- drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c | 5 +++++ drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c | 5 +++++ 6 files changed, 22 insertions(+), 5 deletions(-) diff --git a/drivers/iio/imu/st_lsm6dsx/Kconfig b/drivers/iio/imu/st_lsm6dsx/Kconfig index 85860217aaf3..fefd0b939100 100644 --- a/drivers/iio/imu/st_lsm6dsx/Kconfig +++ b/drivers/iio/imu/st_lsm6dsx/Kconfig @@ -11,9 +11,9 @@ config IIO_ST_LSM6DSX help Say yes here to build support for STMicroelectronics LSM6DSx imu sensor. Supported devices: lsm6ds3, lsm6ds3h, lsm6dsl, lsm6dsm, - ism330dlc, lsm6dso, lsm6dsox, asm330lhh, lsm6dsr, lsm6ds3tr-c, - ism330dhcx, lsm6dsrx, lsm6ds0, lsm6dsop, the accelerometer/gyroscope - of lsm9ds1 and lsm6dst. + ism330dlc, lsm6dso, lsm6dsox, asm330lhh, asm330lhhx, lsm6dsr, + lsm6ds3tr-c, ism330dhcx, lsm6dsrx, lsm6ds0, lsm6dsop, + the accelerometer/gyroscope of lsm9ds1 and lsm6dst. To compile this driver as a module, choose M here: the module will be called st_lsm6dsx. diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h index 6ac4eac36458..a86dd29a4738 100644 --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h @@ -31,6 +31,7 @@ #define ST_LSM6DSRX_DEV_NAME "lsm6dsrx" #define ST_LSM6DST_DEV_NAME "lsm6dst" #define ST_LSM6DSOP_DEV_NAME "lsm6dsop" +#define ST_ASM330LHHX_DEV_NAME "asm330lhhx" enum st_lsm6dsx_hw_id { ST_LSM6DS3_ID, @@ -49,6 +50,7 @@ enum st_lsm6dsx_hw_id { ST_LSM6DSRX_ID, ST_LSM6DST_ID, ST_LSM6DSOP_ID, + ST_ASM330LHHX_ID, ST_LSM6DSX_MAX_ID, }; diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c index 16730a780964..38bb5f645ebd 100644 --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c @@ -14,7 +14,8 @@ * (e.g. Gx, Gy, Gz, Ax, Ay, Az), then data are repeated depending on the * value of the decimation factor and ODR set for each FIFO data set. * - * LSM6DSO/LSM6DSOX/ASM330LHH/LSM6DSR/LSM6DSRX/ISM330DHCX/LSM6DST/LSM6DSOP: + * LSM6DSO/LSM6DSOX/ASM330LHH/ASM330LHHX/LSM6DSR/LSM6DSRX/ISM330DHCX/ + * LSM6DST/LSM6DSOP: * The FIFO buffer can be configured to store data from gyroscope and * accelerometer. Each sample is queued with a tag (1B) indicating data * source (gyroscope, accelerometer, hw timer). diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c index b1d8d5a66f01..910397716833 100644 --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c @@ -26,7 +26,7 @@ * - Gyroscope supported full-scale [dps]: +-125/+-245/+-500/+-1000/+-2000 * - FIFO size: 4KB * - * - LSM6DSO/LSM6DSOX/ASM330LHH/LSM6DSR/ISM330DHCX/LSM6DST/LSM6DSOP: + * - LSM6DSO/LSM6DSOX/ASM330LHH/ASM330LHHX/LSM6DSR/ISM330DHCX/LSM6DST/LSM6DSOP: * - Accelerometer/Gyroscope supported ODR [Hz]: 12.5, 26, 52, 104, 208, 416, * 833 * - Accelerometer supported full-scale [g]: +-2/+-4/+-8/+-16 @@ -786,6 +786,10 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = { .hw_id = ST_LSM6DST_ID, .name = ST_LSM6DST_DEV_NAME, .wai = 0x6d, + }, { + .hw_id = ST_ASM330LHHX_ID, + .name = ST_ASM330LHHX_DEV_NAME, + .wai = 0x6b, }, }, .channels = { diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c index 8b4fc2c15622..715fbdc8190e 100644 --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c @@ -101,6 +101,10 @@ static const struct of_device_id st_lsm6dsx_i2c_of_match[] = { .compatible = "st,lsm6dsop", .data = (void *)ST_LSM6DSOP_ID, }, + { + .compatible = "st,asm330lhhx", + .data = (void *)ST_ASM330LHHX_ID, + }, {}, }; MODULE_DEVICE_TABLE(of, st_lsm6dsx_i2c_of_match); @@ -122,6 +126,7 @@ static const struct i2c_device_id st_lsm6dsx_i2c_id_table[] = { { ST_LSM6DSRX_DEV_NAME, ST_LSM6DSRX_ID }, { ST_LSM6DST_DEV_NAME, ST_LSM6DST_ID }, { ST_LSM6DSOP_DEV_NAME, ST_LSM6DSOP_ID }, + { ST_ASM330LHHX_DEV_NAME, ST_ASM330LHHX_ID }, {}, }; MODULE_DEVICE_TABLE(i2c, st_lsm6dsx_i2c_id_table); diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c index e80110b6b280..f5767cf76c1d 100644 --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c @@ -101,6 +101,10 @@ static const struct of_device_id st_lsm6dsx_spi_of_match[] = { .compatible = "st,lsm6dsop", .data = (void *)ST_LSM6DSOP_ID, }, + { + .compatible = "st,asm330lhhx", + .data = (void *)ST_ASM330LHHX_ID, + }, {}, }; MODULE_DEVICE_TABLE(of, st_lsm6dsx_spi_of_match); @@ -122,6 +126,7 @@ static const struct spi_device_id st_lsm6dsx_spi_id_table[] = { { ST_LSM6DSRX_DEV_NAME, ST_LSM6DSRX_ID }, { ST_LSM6DST_DEV_NAME, ST_LSM6DST_ID }, { ST_LSM6DSOP_DEV_NAME, ST_LSM6DSOP_ID }, + { ST_ASM330LHHX_DEV_NAME, ST_ASM330LHHX_ID }, {}, }; MODULE_DEVICE_TABLE(spi, st_lsm6dsx_spi_id_table); -- 2.35.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] iio: imu: st_lsm6dsx: add support to ASM330LHHX 2022-04-02 10:09 ` [PATCH 1/2] iio: imu: st_lsm6dsx: add support to ASM330LHHX Lorenzo Bianconi @ 2022-04-02 16:22 ` Jonathan Cameron 2022-04-03 11:15 ` Lorenzo Bianconi 0 siblings, 1 reply; 11+ messages in thread From: Jonathan Cameron @ 2022-04-02 16:22 UTC (permalink / raw) To: Lorenzo Bianconi; +Cc: linux-iio, devicetree, lorenzo.bianconi, robh On Sat, 2 Apr 2022 12:09:29 +0200 Lorenzo Bianconi <lorenzo@kernel.org> wrote: > Add support to STM ASM330LHHX (acc + gyro) Mems sensor > https://www.st.com/resource/en/datasheet/asm330lhhx.pdf Use a Datasheet tag as below. Perhaps mention it's an automotive rated sensor and that it's compatible otherwise with xxx existing part. Quick glance at the datasheet suggests this part has a sensor hub... Should it be in a the block with parameters for that as you currently have it in the one for no sensorhub I think. Or is it using an incompatible sensor hub interface? > Datasheet: https://www.st.com/resource/en/datasheet/asm330lhhx.pdf > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > --- > drivers/iio/imu/st_lsm6dsx/Kconfig | 6 +++--- > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 2 ++ > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 3 ++- > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 6 +++++- > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c | 5 +++++ > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c | 5 +++++ > 6 files changed, 22 insertions(+), 5 deletions(-) > > diff --git a/drivers/iio/imu/st_lsm6dsx/Kconfig b/drivers/iio/imu/st_lsm6dsx/Kconfig > index 85860217aaf3..fefd0b939100 100644 > --- a/drivers/iio/imu/st_lsm6dsx/Kconfig > +++ b/drivers/iio/imu/st_lsm6dsx/Kconfig > @@ -11,9 +11,9 @@ config IIO_ST_LSM6DSX > help > Say yes here to build support for STMicroelectronics LSM6DSx imu > sensor. Supported devices: lsm6ds3, lsm6ds3h, lsm6dsl, lsm6dsm, > - ism330dlc, lsm6dso, lsm6dsox, asm330lhh, lsm6dsr, lsm6ds3tr-c, > - ism330dhcx, lsm6dsrx, lsm6ds0, lsm6dsop, the accelerometer/gyroscope > - of lsm9ds1 and lsm6dst. > + ism330dlc, lsm6dso, lsm6dsox, asm330lhh, asm330lhhx, lsm6dsr, > + lsm6ds3tr-c, ism330dhcx, lsm6dsrx, lsm6ds0, lsm6dsop, > + the accelerometer/gyroscope of lsm9ds1 and lsm6dst. > > To compile this driver as a module, choose M here: the module > will be called st_lsm6dsx. > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h > index 6ac4eac36458..a86dd29a4738 100644 > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h > @@ -31,6 +31,7 @@ > #define ST_LSM6DSRX_DEV_NAME "lsm6dsrx" > #define ST_LSM6DST_DEV_NAME "lsm6dst" > #define ST_LSM6DSOP_DEV_NAME "lsm6dsop" > +#define ST_ASM330LHHX_DEV_NAME "asm330lhhx" > > enum st_lsm6dsx_hw_id { > ST_LSM6DS3_ID, > @@ -49,6 +50,7 @@ enum st_lsm6dsx_hw_id { > ST_LSM6DSRX_ID, > ST_LSM6DST_ID, > ST_LSM6DSOP_ID, > + ST_ASM330LHHX_ID, > ST_LSM6DSX_MAX_ID, > }; > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c > index 16730a780964..38bb5f645ebd 100644 > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c > @@ -14,7 +14,8 @@ > * (e.g. Gx, Gy, Gz, Ax, Ay, Az), then data are repeated depending on the > * value of the decimation factor and ODR set for each FIFO data set. > * > - * LSM6DSO/LSM6DSOX/ASM330LHH/LSM6DSR/LSM6DSRX/ISM330DHCX/LSM6DST/LSM6DSOP: > + * LSM6DSO/LSM6DSOX/ASM330LHH/ASM330LHHX/LSM6DSR/LSM6DSRX/ISM330DHCX/ > + * LSM6DST/LSM6DSOP: > * The FIFO buffer can be configured to store data from gyroscope and > * accelerometer. Each sample is queued with a tag (1B) indicating data > * source (gyroscope, accelerometer, hw timer). > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c > index b1d8d5a66f01..910397716833 100644 > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c > @@ -26,7 +26,7 @@ > * - Gyroscope supported full-scale [dps]: +-125/+-245/+-500/+-1000/+-2000 > * - FIFO size: 4KB > * > - * - LSM6DSO/LSM6DSOX/ASM330LHH/LSM6DSR/ISM330DHCX/LSM6DST/LSM6DSOP: > + * - LSM6DSO/LSM6DSOX/ASM330LHH/ASM330LHHX/LSM6DSR/ISM330DHCX/LSM6DST/LSM6DSOP: > * - Accelerometer/Gyroscope supported ODR [Hz]: 12.5, 26, 52, 104, 208, 416, > * 833 > * - Accelerometer supported full-scale [g]: +-2/+-4/+-8/+-16 > @@ -786,6 +786,10 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = { > .hw_id = ST_LSM6DST_ID, > .name = ST_LSM6DST_DEV_NAME, > .wai = 0x6d, > + }, { > + .hw_id = ST_ASM330LHHX_ID, > + .name = ST_ASM330LHHX_DEV_NAME, > + .wai = 0x6b, Probably nicer to put it next to he other 0x6b entries. > }, > }, > .channels = { > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c > index 8b4fc2c15622..715fbdc8190e 100644 > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c > @@ -101,6 +101,10 @@ static const struct of_device_id st_lsm6dsx_i2c_of_match[] = { > .compatible = "st,lsm6dsop", > .data = (void *)ST_LSM6DSOP_ID, > }, > + { > + .compatible = "st,asm330lhhx", > + .data = (void *)ST_ASM330LHHX_ID, > + }, > {}, > }; > MODULE_DEVICE_TABLE(of, st_lsm6dsx_i2c_of_match); > @@ -122,6 +126,7 @@ static const struct i2c_device_id st_lsm6dsx_i2c_id_table[] = { > { ST_LSM6DSRX_DEV_NAME, ST_LSM6DSRX_ID }, > { ST_LSM6DST_DEV_NAME, ST_LSM6DST_ID }, > { ST_LSM6DSOP_DEV_NAME, ST_LSM6DSOP_ID }, > + { ST_ASM330LHHX_DEV_NAME, ST_ASM330LHHX_ID }, > {}, > }; > MODULE_DEVICE_TABLE(i2c, st_lsm6dsx_i2c_id_table); > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c > index e80110b6b280..f5767cf76c1d 100644 > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c > @@ -101,6 +101,10 @@ static const struct of_device_id st_lsm6dsx_spi_of_match[] = { > .compatible = "st,lsm6dsop", > .data = (void *)ST_LSM6DSOP_ID, > }, > + { > + .compatible = "st,asm330lhhx", > + .data = (void *)ST_ASM330LHHX_ID, > + }, > {}, > }; > MODULE_DEVICE_TABLE(of, st_lsm6dsx_spi_of_match); > @@ -122,6 +126,7 @@ static const struct spi_device_id st_lsm6dsx_spi_id_table[] = { > { ST_LSM6DSRX_DEV_NAME, ST_LSM6DSRX_ID }, > { ST_LSM6DST_DEV_NAME, ST_LSM6DST_ID }, > { ST_LSM6DSOP_DEV_NAME, ST_LSM6DSOP_ID }, > + { ST_ASM330LHHX_DEV_NAME, ST_ASM330LHHX_ID }, > {}, > }; > MODULE_DEVICE_TABLE(spi, st_lsm6dsx_spi_id_table); ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] iio: imu: st_lsm6dsx: add support to ASM330LHHX 2022-04-02 16:22 ` Jonathan Cameron @ 2022-04-03 11:15 ` Lorenzo Bianconi 0 siblings, 0 replies; 11+ messages in thread From: Lorenzo Bianconi @ 2022-04-03 11:15 UTC (permalink / raw) To: Jonathan Cameron; +Cc: linux-iio, devicetree, lorenzo.bianconi, robh [-- Attachment #1: Type: text/plain, Size: 6931 bytes --] > On Sat, 2 Apr 2022 12:09:29 +0200 > Lorenzo Bianconi <lorenzo@kernel.org> wrote: > > > Add support to STM ASM330LHHX (acc + gyro) Mems sensor > > https://www.st.com/resource/en/datasheet/asm330lhhx.pdf > Use a Datasheet tag as below. > Perhaps mention it's an automotive rated sensor and that > it's compatible otherwise with xxx existing part. ack, will do > > Quick glance at the datasheet suggests this part has a sensor hub... > Should it be in a the block with parameters for that as you > currently have it in the one for no sensorhub I think. > > Or is it using an incompatible sensor hub interface? asm330lhhx supports sensor hub with the same configuration of LSM6DSR or LSM6DSO. Regards, Lorenzo > > > > Datasheet: https://www.st.com/resource/en/datasheet/asm330lhhx.pdf > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > > --- > > drivers/iio/imu/st_lsm6dsx/Kconfig | 6 +++--- > > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 2 ++ > > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 3 ++- > > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 6 +++++- > > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c | 5 +++++ > > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c | 5 +++++ > > 6 files changed, 22 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/iio/imu/st_lsm6dsx/Kconfig b/drivers/iio/imu/st_lsm6dsx/Kconfig > > index 85860217aaf3..fefd0b939100 100644 > > --- a/drivers/iio/imu/st_lsm6dsx/Kconfig > > +++ b/drivers/iio/imu/st_lsm6dsx/Kconfig > > @@ -11,9 +11,9 @@ config IIO_ST_LSM6DSX > > help > > Say yes here to build support for STMicroelectronics LSM6DSx imu > > sensor. Supported devices: lsm6ds3, lsm6ds3h, lsm6dsl, lsm6dsm, > > - ism330dlc, lsm6dso, lsm6dsox, asm330lhh, lsm6dsr, lsm6ds3tr-c, > > - ism330dhcx, lsm6dsrx, lsm6ds0, lsm6dsop, the accelerometer/gyroscope > > - of lsm9ds1 and lsm6dst. > > + ism330dlc, lsm6dso, lsm6dsox, asm330lhh, asm330lhhx, lsm6dsr, > > + lsm6ds3tr-c, ism330dhcx, lsm6dsrx, lsm6ds0, lsm6dsop, > > + the accelerometer/gyroscope of lsm9ds1 and lsm6dst. > > > > To compile this driver as a module, choose M here: the module > > will be called st_lsm6dsx. > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h > > index 6ac4eac36458..a86dd29a4738 100644 > > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h > > @@ -31,6 +31,7 @@ > > #define ST_LSM6DSRX_DEV_NAME "lsm6dsrx" > > #define ST_LSM6DST_DEV_NAME "lsm6dst" > > #define ST_LSM6DSOP_DEV_NAME "lsm6dsop" > > +#define ST_ASM330LHHX_DEV_NAME "asm330lhhx" > > > > enum st_lsm6dsx_hw_id { > > ST_LSM6DS3_ID, > > @@ -49,6 +50,7 @@ enum st_lsm6dsx_hw_id { > > ST_LSM6DSRX_ID, > > ST_LSM6DST_ID, > > ST_LSM6DSOP_ID, > > + ST_ASM330LHHX_ID, > > ST_LSM6DSX_MAX_ID, > > }; > > > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c > > index 16730a780964..38bb5f645ebd 100644 > > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c > > @@ -14,7 +14,8 @@ > > * (e.g. Gx, Gy, Gz, Ax, Ay, Az), then data are repeated depending on the > > * value of the decimation factor and ODR set for each FIFO data set. > > * > > - * LSM6DSO/LSM6DSOX/ASM330LHH/LSM6DSR/LSM6DSRX/ISM330DHCX/LSM6DST/LSM6DSOP: > > + * LSM6DSO/LSM6DSOX/ASM330LHH/ASM330LHHX/LSM6DSR/LSM6DSRX/ISM330DHCX/ > > + * LSM6DST/LSM6DSOP: > > * The FIFO buffer can be configured to store data from gyroscope and > > * accelerometer. Each sample is queued with a tag (1B) indicating data > > * source (gyroscope, accelerometer, hw timer). > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c > > index b1d8d5a66f01..910397716833 100644 > > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c > > @@ -26,7 +26,7 @@ > > * - Gyroscope supported full-scale [dps]: +-125/+-245/+-500/+-1000/+-2000 > > * - FIFO size: 4KB > > * > > - * - LSM6DSO/LSM6DSOX/ASM330LHH/LSM6DSR/ISM330DHCX/LSM6DST/LSM6DSOP: > > + * - LSM6DSO/LSM6DSOX/ASM330LHH/ASM330LHHX/LSM6DSR/ISM330DHCX/LSM6DST/LSM6DSOP: > > * - Accelerometer/Gyroscope supported ODR [Hz]: 12.5, 26, 52, 104, 208, 416, > > * 833 > > * - Accelerometer supported full-scale [g]: +-2/+-4/+-8/+-16 > > @@ -786,6 +786,10 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = { > > .hw_id = ST_LSM6DST_ID, > > .name = ST_LSM6DST_DEV_NAME, > > .wai = 0x6d, > > + }, { > > + .hw_id = ST_ASM330LHHX_ID, > > + .name = ST_ASM330LHHX_DEV_NAME, > > + .wai = 0x6b, > > Probably nicer to put it next to he other 0x6b entries. > > > }, > > }, > > .channels = { > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c > > index 8b4fc2c15622..715fbdc8190e 100644 > > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c > > @@ -101,6 +101,10 @@ static const struct of_device_id st_lsm6dsx_i2c_of_match[] = { > > .compatible = "st,lsm6dsop", > > .data = (void *)ST_LSM6DSOP_ID, > > }, > > + { > > + .compatible = "st,asm330lhhx", > > + .data = (void *)ST_ASM330LHHX_ID, > > + }, > > {}, > > }; > > MODULE_DEVICE_TABLE(of, st_lsm6dsx_i2c_of_match); > > @@ -122,6 +126,7 @@ static const struct i2c_device_id st_lsm6dsx_i2c_id_table[] = { > > { ST_LSM6DSRX_DEV_NAME, ST_LSM6DSRX_ID }, > > { ST_LSM6DST_DEV_NAME, ST_LSM6DST_ID }, > > { ST_LSM6DSOP_DEV_NAME, ST_LSM6DSOP_ID }, > > + { ST_ASM330LHHX_DEV_NAME, ST_ASM330LHHX_ID }, > > {}, > > }; > > MODULE_DEVICE_TABLE(i2c, st_lsm6dsx_i2c_id_table); > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c > > index e80110b6b280..f5767cf76c1d 100644 > > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c > > @@ -101,6 +101,10 @@ static const struct of_device_id st_lsm6dsx_spi_of_match[] = { > > .compatible = "st,lsm6dsop", > > .data = (void *)ST_LSM6DSOP_ID, > > }, > > + { > > + .compatible = "st,asm330lhhx", > > + .data = (void *)ST_ASM330LHHX_ID, > > + }, > > {}, > > }; > > MODULE_DEVICE_TABLE(of, st_lsm6dsx_spi_of_match); > > @@ -122,6 +126,7 @@ static const struct spi_device_id st_lsm6dsx_spi_id_table[] = { > > { ST_LSM6DSRX_DEV_NAME, ST_LSM6DSRX_ID }, > > { ST_LSM6DST_DEV_NAME, ST_LSM6DST_ID }, > > { ST_LSM6DSOP_DEV_NAME, ST_LSM6DSOP_ID }, > > + { ST_ASM330LHHX_DEV_NAME, ST_ASM330LHHX_ID }, > > {}, > > }; > > MODULE_DEVICE_TABLE(spi, st_lsm6dsx_spi_id_table); > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] dt-bindings: iio: imu: st_lsm6dsx: add asm330lhhx device bindings 2022-04-02 10:09 [PATCH 0/2] add support for ASM330LHHX Lorenzo Bianconi 2022-04-02 10:09 ` [PATCH 1/2] iio: imu: st_lsm6dsx: add support to ASM330LHHX Lorenzo Bianconi @ 2022-04-02 10:09 ` Lorenzo Bianconi 2022-04-02 16:17 ` Jonathan Cameron 1 sibling, 1 reply; 11+ messages in thread From: Lorenzo Bianconi @ 2022-04-02 10:09 UTC (permalink / raw) To: jic23; +Cc: linux-iio, devicetree, lorenzo.bianconi, robh Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> --- Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml index 0750f700a143..23637c420d20 100644 --- a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml +++ b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml @@ -31,6 +31,7 @@ properties: - st,lsm6dsrx - st,lsm6dst - st,lsm6dsop + - st,asm330lhhx reg: maxItems: 1 -- 2.35.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] dt-bindings: iio: imu: st_lsm6dsx: add asm330lhhx device bindings 2022-04-02 10:09 ` [PATCH 2/2] dt-bindings: iio: imu: st_lsm6dsx: add asm330lhhx device bindings Lorenzo Bianconi @ 2022-04-02 16:17 ` Jonathan Cameron 2022-04-03 14:56 ` Lorenzo Bianconi 0 siblings, 1 reply; 11+ messages in thread From: Jonathan Cameron @ 2022-04-02 16:17 UTC (permalink / raw) To: Lorenzo Bianconi, linux-iio; +Cc: devicetree, lorenzo.bianconi, robh On Sat, 2 Apr 2022 12:09:30 +0200 Lorenzo Bianconi <lorenzo@kernel.org> wrote: > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> Hi Lorenzo, This runs in to the same feedback that was recently had for https://lore.kernel.org/all/?q=Add+support+for+ICM-20608-D but in a more extreme sense as this one presents the same whoami value as for other sensors already supported. Things are made more fun by the fact that sensors with the same WAI seem to have different features (presence or not of a sensor hub - is there any documented way to detect that?). As such, we should really be listing this as compatible with one of the parts that is already supported such as the LSM6DSR. For that we'll need a slightly more complex binding and it would have the side effect that if the match was on that compatible we would list the name as whatever that part is. I'm not sure that really matters a great deal, but it could in theory create a userspace ABI change if we later needed to add explicit support for the part due to some real differences not indicated by the WAI value. An extension is whether we should relax the need to match on WAI if the part is considered compatible. I guess that depends on just how compatible we think they are. So I see several steps to this process. 1) Add fallback compatibles for existing entries to first one with same WAI and same feature set. 2) Add fallback compatibles beyond that to first part introduced with particular characteristics. For this we'd also want to have the driver relax its handling to just warn if the WAI isn't listed for any of the parts that share a particular set of characteristic (so you'll have to loop over the local array again to check): https://elixir.bootlin.com/linux/latest/source/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c#L1197 Same argument applies as for the mpu6050 that, whilst we should modify that code to cope, it's not a prerequisit for adding the compatible fallback to the binding. Personally I'd like it to be the first patch in the series that modifies the binding though. Note it'll be easy to add the fallbacks for this new part as no mainline trees presumably use it. To 'fix' the rest we'll have to find and update any DTs in mainline. Note this won't stop us needing to add compatibles to newer kernels (at very least to the dt-binding, but probably also the driver), but it should help a newer DT 'work' with an old kernel. Jonathan > --- > Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > index 0750f700a143..23637c420d20 100644 > --- a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > +++ b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > @@ -31,6 +31,7 @@ properties: > - st,lsm6dsrx > - st,lsm6dst > - st,lsm6dsop > + - st,asm330lhhx > > reg: > maxItems: 1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] dt-bindings: iio: imu: st_lsm6dsx: add asm330lhhx device bindings 2022-04-02 16:17 ` Jonathan Cameron @ 2022-04-03 14:56 ` Lorenzo Bianconi 2022-04-04 9:22 ` Jonathan Cameron 0 siblings, 1 reply; 11+ messages in thread From: Lorenzo Bianconi @ 2022-04-03 14:56 UTC (permalink / raw) To: Jonathan Cameron; +Cc: linux-iio, devicetree, lorenzo.bianconi, robh [-- Attachment #1: Type: text/plain, Size: 4619 bytes --] > On Sat, 2 Apr 2022 12:09:30 +0200 > Lorenzo Bianconi <lorenzo@kernel.org> wrote: > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > Hi Lorenzo, > > This runs in to the same feedback that was recently had for > https://lore.kernel.org/all/?q=Add+support+for+ICM-20608-D > but in a more extreme sense as this one presents the same whoami value > as for other sensors already supported. Things are made more > fun by the fact that sensors with the same WAI seem to have different > features (presence or not of a sensor hub - is there any documented > way to detect that?). Hi Jonathan, if we consider only the features implemented in st_lsm6dsx, asm330lhhx will be 1:1 compatible with lsm6dsr or lsm6dso, so we can just use one of bindings in this section to support it (the only side effect is it will be listed as "lsm6dsr" or "lsm6dso", but I guess it is ok). Agree? The only difference between asm330lhhx and asm330lhh is the former supports sensor-hub while the latter does not declare it (even if the use the same whoami). AFAIK there is no way to autodetect if the sensor supports sensor-hub and we can just try to discover slave devices connected. This can have some downside as described in the commit: commit 35619155d044830357f06f1d2c8188c4530b4d7a Author: Lorenzo Bianconi <lorenzo@kernel.org> Date: Sat Nov 13 16:23:14 2021 +0100 iio: imu: st_lsm6dsx: add dts property to disable sensor-hub I would like to merge the sections in st_lsm6dsx_settings struct for lsm6dsr, lsm6dso.. and lsm6dsop, asm330lhh since the only difference is sensor-hub support. I guess we can have 2 option here to avoid any sensor-hub corner cases: - provide the "st,disable-sensor-hub" in dts to disable sensor-hub for asm330lhh, lsm6dsop (need user changes) - add a bool variable st_lsm6dsx_settings[].id[] in order to specify if the chip supports sensor-hub. Which one do you prefer? Regards, Lorenzo > > As such, we should really be listing this as compatible with one > of the parts that is already supported such as the > LSM6DSR. > > For that we'll need a slightly more complex binding and it would > have the side effect that if the match was on that compatible we > would list the name as whatever that part is. > > I'm not sure that really matters a great deal, but it could in theory > create a userspace ABI change if we later needed to add explicit support > for the part due to some real differences not indicated by the WAI value. > > An extension is whether we should relax the need to match on WAI if > the part is considered compatible. I guess that depends on just how > compatible we think they are. > > So I see several steps to this process. > > 1) Add fallback compatibles for existing entries to first one with same WAI and > same feature set. > 2) Add fallback compatibles beyond that to first part introduced with particular > characteristics. For this we'd also want to have the driver relax its > handling to just warn if the WAI isn't listed for any of the parts that > share a particular set of characteristic (so you'll have to loop over the local > array again to check): > https://elixir.bootlin.com/linux/latest/source/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c#L1197 > Same argument applies as for the mpu6050 that, whilst we should modify that code to > cope, it's not a prerequisit for adding the compatible fallback to the binding. > Personally I'd like it to be the first patch in the series that modifies the > binding though. Note it'll be easy to add the fallbacks for this new part as > no mainline trees presumably use it. To 'fix' the rest we'll have to find > and update any DTs in mainline. > > Note this won't stop us needing to add compatibles to newer kernels (at very > least to the dt-binding, but probably also the driver), but it should help a newer > DT 'work' with an old kernel. > > Jonathan > > > > --- > > Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > > index 0750f700a143..23637c420d20 100644 > > --- a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > > +++ b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > > @@ -31,6 +31,7 @@ properties: > > - st,lsm6dsrx > > - st,lsm6dst > > - st,lsm6dsop > > + - st,asm330lhhx > > > > reg: > > maxItems: 1 > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] dt-bindings: iio: imu: st_lsm6dsx: add asm330lhhx device bindings 2022-04-03 14:56 ` Lorenzo Bianconi @ 2022-04-04 9:22 ` Jonathan Cameron 2022-04-04 9:33 ` Lorenzo Bianconi 0 siblings, 1 reply; 11+ messages in thread From: Jonathan Cameron @ 2022-04-04 9:22 UTC (permalink / raw) To: Lorenzo Bianconi Cc: Jonathan Cameron, linux-iio, devicetree, lorenzo.bianconi, robh On Sun, 3 Apr 2022 16:56:51 +0200 Lorenzo Bianconi <lorenzo@kernel.org> wrote: > > On Sat, 2 Apr 2022 12:09:30 +0200 > > Lorenzo Bianconi <lorenzo@kernel.org> wrote: > > > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > > Hi Lorenzo, > > > > This runs in to the same feedback that was recently had for > > https://lore.kernel.org/all/?q=Add+support+for+ICM-20608-D > > but in a more extreme sense as this one presents the same whoami value > > as for other sensors already supported. Things are made more > > fun by the fact that sensors with the same WAI seem to have different > > features (presence or not of a sensor hub - is there any documented > > way to detect that?). > > Hi Jonathan, > > if we consider only the features implemented in st_lsm6dsx, asm330lhhx > will be 1:1 compatible with lsm6dsr or lsm6dso, so we can just use one > of bindings in this section to support it (the only side effect is it > will be listed as "lsm6dsr" or "lsm6dso", but I guess it is ok). Agree? If the part has more features than the base compatible (or a different WAI) then we can definitely have a backup compatible for it (hence making that subset of features work on an old kernel). We still want to introduce the new compatible so that we get the name right etc going forwards and are in a good position to add the extra features if we ever get around to it. > > The only difference between asm330lhhx and asm330lhh is the former supports > sensor-hub while the latter does not declare it (even if the use the same > whoami). > AFAIK there is no way to autodetect if the sensor supports sensor-hub and > we can just try to discover slave devices connected. This can have some > downside as described in the commit: Ah thanks. I'd forgotten this. > > commit 35619155d044830357f06f1d2c8188c4530b4d7a > Author: Lorenzo Bianconi <lorenzo@kernel.org> > Date: Sat Nov 13 16:23:14 2021 +0100 > > iio: imu: st_lsm6dsx: add dts property to disable sensor-hub > > I would like to merge the sections in st_lsm6dsx_settings struct for > lsm6dsr, lsm6dso.. and lsm6dsop, asm330lhh since the only difference is > sensor-hub support. I guess we can have 2 option here to avoid any > sensor-hub corner cases: > - provide the "st,disable-sensor-hub" in dts to disable sensor-hub for > asm330lhh, lsm6dsop (need user changes) > - add a bool variable st_lsm6dsx_settings[].id[] in order to specify if the > chip supports sensor-hub. > > Which one do you prefer? > > Regards, > Lorenzo > > > > > As such, we should really be listing this as compatible with one > > of the parts that is already supported such as the > > LSM6DSR. > > > > For that we'll need a slightly more complex binding and it would > > have the side effect that if the match was on that compatible we > > would list the name as whatever that part is. > > > > I'm not sure that really matters a great deal, but it could in theory > > create a userspace ABI change if we later needed to add explicit support > > for the part due to some real differences not indicated by the WAI value. > > > > An extension is whether we should relax the need to match on WAI if > > the part is considered compatible. I guess that depends on just how > > compatible we think they are. > > > > So I see several steps to this process. > > > > 1) Add fallback compatibles for existing entries to first one with same WAI and > > same feature set. > > 2) Add fallback compatibles beyond that to first part introduced with particular > > characteristics. For this we'd also want to have the driver relax its > > handling to just warn if the WAI isn't listed for any of the parts that > > share a particular set of characteristic (so you'll have to loop over the local > > array again to check): > > https://elixir.bootlin.com/linux/latest/source/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c#L1197 > > Same argument applies as for the mpu6050 that, whilst we should modify that code to > > cope, it's not a prerequisit for adding the compatible fallback to the binding. > > Personally I'd like it to be the first patch in the series that modifies the > > binding though. Note it'll be easy to add the fallbacks for this new part as > > no mainline trees presumably use it. To 'fix' the rest we'll have to find > > and update any DTs in mainline. > > > > Note this won't stop us needing to add compatibles to newer kernels (at very > > least to the dt-binding, but probably also the driver), but it should help a newer > > DT 'work' with an old kernel. > > > > Jonathan > > > > > > > --- > > > Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > > > index 0750f700a143..23637c420d20 100644 > > > --- a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > > > +++ b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > > > @@ -31,6 +31,7 @@ properties: > > > - st,lsm6dsrx > > > - st,lsm6dst > > > - st,lsm6dsop > > > + - st,asm330lhhx > > > > > > reg: > > > maxItems: 1 > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] dt-bindings: iio: imu: st_lsm6dsx: add asm330lhhx device bindings 2022-04-04 9:22 ` Jonathan Cameron @ 2022-04-04 9:33 ` Lorenzo Bianconi 2022-04-04 16:17 ` Jonathan Cameron 0 siblings, 1 reply; 11+ messages in thread From: Lorenzo Bianconi @ 2022-04-04 9:33 UTC (permalink / raw) To: Jonathan Cameron Cc: Lorenzo Bianconi, Jonathan Cameron, linux-iio, devicetree, robh [-- Attachment #1: Type: text/plain, Size: 5986 bytes --] On Apr 04, Jonathan Cameron wrote: > On Sun, 3 Apr 2022 16:56:51 +0200 > Lorenzo Bianconi <lorenzo@kernel.org> wrote: > > > > On Sat, 2 Apr 2022 12:09:30 +0200 > > > Lorenzo Bianconi <lorenzo@kernel.org> wrote: > > > > > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > > > Hi Lorenzo, > > > > > > This runs in to the same feedback that was recently had for > > > https://lore.kernel.org/all/?q=Add+support+for+ICM-20608-D > > > but in a more extreme sense as this one presents the same whoami value > > > as for other sensors already supported. Things are made more > > > fun by the fact that sensors with the same WAI seem to have different > > > features (presence or not of a sensor hub - is there any documented > > > way to detect that?). > > > > Hi Jonathan, > > > > if we consider only the features implemented in st_lsm6dsx, asm330lhhx > > will be 1:1 compatible with lsm6dsr or lsm6dso, so we can just use one > > of bindings in this section to support it (the only side effect is it > > will be listed as "lsm6dsr" or "lsm6dso", but I guess it is ok). Agree? > > If the part has more features than the base compatible (or a different WAI) > then we can definitely have a backup compatible for it (hence making that > subset of features work on an old kernel). We still want to introduce > the new compatible so that we get the name right etc going forwards and > are in a good position to add the extra features if we ever get around to it. ack. I did not completely get what you mean here with "backup compatible". Do you mean: - use "st,lsm6dsr" for asm330lhhx on older kernels and add "st,asm330lhhx" on new ones. Do you have any pointer on how to document it? or - add a "wildcard" compatible string for this kind of devices. Do you have any pointers? Regards, Lorenzo > > > > > > The only difference between asm330lhhx and asm330lhh is the former supports > > sensor-hub while the latter does not declare it (even if the use the same > > whoami). > > AFAIK there is no way to autodetect if the sensor supports sensor-hub and > > we can just try to discover slave devices connected. This can have some > > downside as described in the commit: > > Ah thanks. I'd forgotten this. > > > > > commit 35619155d044830357f06f1d2c8188c4530b4d7a > > Author: Lorenzo Bianconi <lorenzo@kernel.org> > > Date: Sat Nov 13 16:23:14 2021 +0100 > > > > iio: imu: st_lsm6dsx: add dts property to disable sensor-hub > > > > I would like to merge the sections in st_lsm6dsx_settings struct for > > lsm6dsr, lsm6dso.. and lsm6dsop, asm330lhh since the only difference is > > sensor-hub support. I guess we can have 2 option here to avoid any > > sensor-hub corner cases: > > - provide the "st,disable-sensor-hub" in dts to disable sensor-hub for > > asm330lhh, lsm6dsop (need user changes) > > - add a bool variable st_lsm6dsx_settings[].id[] in order to specify if the > > chip supports sensor-hub. > > > > Which one do you prefer? > > > > Regards, > > Lorenzo > > > > > > > > As such, we should really be listing this as compatible with one > > > of the parts that is already supported such as the > > > LSM6DSR. > > > > > > For that we'll need a slightly more complex binding and it would > > > have the side effect that if the match was on that compatible we > > > would list the name as whatever that part is. > > > > > > I'm not sure that really matters a great deal, but it could in theory > > > create a userspace ABI change if we later needed to add explicit support > > > for the part due to some real differences not indicated by the WAI value. > > > > > > An extension is whether we should relax the need to match on WAI if > > > the part is considered compatible. I guess that depends on just how > > > compatible we think they are. > > > > > > So I see several steps to this process. > > > > > > 1) Add fallback compatibles for existing entries to first one with same WAI and > > > same feature set. > > > 2) Add fallback compatibles beyond that to first part introduced with particular > > > characteristics. For this we'd also want to have the driver relax its > > > handling to just warn if the WAI isn't listed for any of the parts that > > > share a particular set of characteristic (so you'll have to loop over the local > > > array again to check): > > > https://elixir.bootlin.com/linux/latest/source/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c#L1197 > > > Same argument applies as for the mpu6050 that, whilst we should modify that code to > > > cope, it's not a prerequisit for adding the compatible fallback to the binding. > > > Personally I'd like it to be the first patch in the series that modifies the > > > binding though. Note it'll be easy to add the fallbacks for this new part as > > > no mainline trees presumably use it. To 'fix' the rest we'll have to find > > > and update any DTs in mainline. > > > > > > Note this won't stop us needing to add compatibles to newer kernels (at very > > > least to the dt-binding, but probably also the driver), but it should help a newer > > > DT 'work' with an old kernel. > > > > > > Jonathan > > > > > > > > > > --- > > > > Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml | 1 + > > > > 1 file changed, 1 insertion(+) > > > > > > > > diff --git a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > > > > index 0750f700a143..23637c420d20 100644 > > > > --- a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > > > > +++ b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > > > > @@ -31,6 +31,7 @@ properties: > > > > - st,lsm6dsrx > > > > - st,lsm6dst > > > > - st,lsm6dsop > > > > + - st,asm330lhhx > > > > > > > > reg: > > > > maxItems: 1 > > > > > > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] dt-bindings: iio: imu: st_lsm6dsx: add asm330lhhx device bindings 2022-04-04 9:33 ` Lorenzo Bianconi @ 2022-04-04 16:17 ` Jonathan Cameron 2022-04-04 19:18 ` Lorenzo Bianconi 0 siblings, 1 reply; 11+ messages in thread From: Jonathan Cameron @ 2022-04-04 16:17 UTC (permalink / raw) To: Lorenzo Bianconi Cc: Lorenzo Bianconi, Jonathan Cameron, linux-iio, devicetree, robh On Mon, 4 Apr 2022 11:33:17 +0200 Lorenzo Bianconi <lorenzo.bianconi@redhat.com> wrote: > On Apr 04, Jonathan Cameron wrote: > > On Sun, 3 Apr 2022 16:56:51 +0200 > > Lorenzo Bianconi <lorenzo@kernel.org> wrote: > > > > > > On Sat, 2 Apr 2022 12:09:30 +0200 > > > > Lorenzo Bianconi <lorenzo@kernel.org> wrote: > > > > > > > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > > > > Hi Lorenzo, > > > > > > > > This runs in to the same feedback that was recently had for > > > > https://lore.kernel.org/all/?q=Add+support+for+ICM-20608-D > > > > but in a more extreme sense as this one presents the same whoami value > > > > as for other sensors already supported. Things are made more > > > > fun by the fact that sensors with the same WAI seem to have different > > > > features (presence or not of a sensor hub - is there any documented > > > > way to detect that?). > > > > > > Hi Jonathan, > > > > > > if we consider only the features implemented in st_lsm6dsx, asm330lhhx > > > will be 1:1 compatible with lsm6dsr or lsm6dso, so we can just use one > > > of bindings in this section to support it (the only side effect is it > > > will be listed as "lsm6dsr" or "lsm6dso", but I guess it is ok). Agree? > > > > If the part has more features than the base compatible (or a different WAI) > > then we can definitely have a backup compatible for it (hence making that > > subset of features work on an old kernel). We still want to introduce > > the new compatible so that we get the name right etc going forwards and > > are in a good position to add the extra features if we ever get around to it. > > ack. I did not completely get what you mean here with "backup compatible". > Do you mean: > - use "st,lsm6dsr" for asm330lhhx on older kernels and add "st,asm330lhhx" on > new ones. Do you have any pointer on how to document it? Take a look at the mpu6050 patches. properties: compatible: - enum: - - invensense,iam20680 - - invensense,icm20608 - - invensense,icm20609 - - invensense,icm20689 - - invensense,icm20602 - - invensense,icm20690 - - invensense,mpu6000 - - invensense,mpu6050 - - invensense,mpu6500 - - invensense,mpu6515 - - invensense,mpu6880 - - invensense,mpu9150 - - invensense,mpu9250 - - invensense,mpu9255 + oneOf: + - enum: + - invensense,iam20680 + - invensense,icm20608 + - invensense,icm20609 + - invensense,icm20689 + - invensense,icm20602 + - invensense,icm20690 + - invensense,mpu6000 + - invensense,mpu6050 + - invensense,mpu6500 + - invensense,mpu6515 + - invensense,mpu6880 + - invensense,mpu9150 + - invensense,mpu9250 + - invensense,mpu9255 + - items: + - const: invensense,icm20608d + - const: invensense,icm20608 Which ends up expecting compatible = "invensense,icm20608d", "invensense,icm20608" and will try matching on the first. If that fails it will try with the second value. > > or > > - add a "wildcard" compatible string for this kind of devices. Do you have any > pointers? > > Regards, > Lorenzo > > > > > > > > > > > The only difference between asm330lhhx and asm330lhh is the former supports > > > sensor-hub while the latter does not declare it (even if the use the same > > > whoami). > > > AFAIK there is no way to autodetect if the sensor supports sensor-hub and > > > we can just try to discover slave devices connected. This can have some > > > downside as described in the commit: > > > > Ah thanks. I'd forgotten this. > > > > > > > > commit 35619155d044830357f06f1d2c8188c4530b4d7a > > > Author: Lorenzo Bianconi <lorenzo@kernel.org> > > > Date: Sat Nov 13 16:23:14 2021 +0100 > > > > > > iio: imu: st_lsm6dsx: add dts property to disable sensor-hub > > > > > > I would like to merge the sections in st_lsm6dsx_settings struct for > > > lsm6dsr, lsm6dso.. and lsm6dsop, asm330lhh since the only difference is > > > sensor-hub support. I guess we can have 2 option here to avoid any > > > sensor-hub corner cases: > > > - provide the "st,disable-sensor-hub" in dts to disable sensor-hub for > > > asm330lhh, lsm6dsop (need user changes) > > > - add a bool variable st_lsm6dsx_settings[].id[] in order to specify if the > > > chip supports sensor-hub. > > > > > > Which one do you prefer? > > > > > > Regards, > > > Lorenzo > > > > > > > > > > > As such, we should really be listing this as compatible with one > > > > of the parts that is already supported such as the > > > > LSM6DSR. > > > > > > > > For that we'll need a slightly more complex binding and it would > > > > have the side effect that if the match was on that compatible we > > > > would list the name as whatever that part is. > > > > > > > > I'm not sure that really matters a great deal, but it could in theory > > > > create a userspace ABI change if we later needed to add explicit support > > > > for the part due to some real differences not indicated by the WAI value. > > > > > > > > An extension is whether we should relax the need to match on WAI if > > > > the part is considered compatible. I guess that depends on just how > > > > compatible we think they are. > > > > > > > > So I see several steps to this process. > > > > > > > > 1) Add fallback compatibles for existing entries to first one with same WAI and > > > > same feature set. > > > > 2) Add fallback compatibles beyond that to first part introduced with particular > > > > characteristics. For this we'd also want to have the driver relax its > > > > handling to just warn if the WAI isn't listed for any of the parts that > > > > share a particular set of characteristic (so you'll have to loop over the local > > > > array again to check): > > > > https://elixir.bootlin.com/linux/latest/source/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c#L1197 > > > > Same argument applies as for the mpu6050 that, whilst we should modify that code to > > > > cope, it's not a prerequisit for adding the compatible fallback to the binding. > > > > Personally I'd like it to be the first patch in the series that modifies the > > > > binding though. Note it'll be easy to add the fallbacks for this new part as > > > > no mainline trees presumably use it. To 'fix' the rest we'll have to find > > > > and update any DTs in mainline. > > > > > > > > Note this won't stop us needing to add compatibles to newer kernels (at very > > > > least to the dt-binding, but probably also the driver), but it should help a newer > > > > DT 'work' with an old kernel. > > > > > > > > Jonathan > > > > > > > > > > > > > --- > > > > > Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml | 1 + > > > > > 1 file changed, 1 insertion(+) > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > > > > > index 0750f700a143..23637c420d20 100644 > > > > > --- a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > > > > > +++ b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > > > > > @@ -31,6 +31,7 @@ properties: > > > > > - st,lsm6dsrx > > > > > - st,lsm6dst > > > > > - st,lsm6dsop > > > > > + - st,asm330lhhx > > > > > > > > > > reg: > > > > > maxItems: 1 > > > > > > > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] dt-bindings: iio: imu: st_lsm6dsx: add asm330lhhx device bindings 2022-04-04 16:17 ` Jonathan Cameron @ 2022-04-04 19:18 ` Lorenzo Bianconi 0 siblings, 0 replies; 11+ messages in thread From: Lorenzo Bianconi @ 2022-04-04 19:18 UTC (permalink / raw) To: Jonathan Cameron Cc: Lorenzo Bianconi, Jonathan Cameron, linux-iio, devicetree, robh [-- Attachment #1: Type: text/plain, Size: 8083 bytes --] > On Mon, 4 Apr 2022 11:33:17 +0200 > Lorenzo Bianconi <lorenzo.bianconi@redhat.com> wrote: > > > On Apr 04, Jonathan Cameron wrote: > > > On Sun, 3 Apr 2022 16:56:51 +0200 > > > Lorenzo Bianconi <lorenzo@kernel.org> wrote: > > > > > > > > On Sat, 2 Apr 2022 12:09:30 +0200 > > > > > Lorenzo Bianconi <lorenzo@kernel.org> wrote: > > > > > > > > > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > > > > > Hi Lorenzo, > > > > > > > > > > This runs in to the same feedback that was recently had for > > > > > https://lore.kernel.org/all/?q=Add+support+for+ICM-20608-D > > > > > but in a more extreme sense as this one presents the same whoami value > > > > > as for other sensors already supported. Things are made more > > > > > fun by the fact that sensors with the same WAI seem to have different > > > > > features (presence or not of a sensor hub - is there any documented > > > > > way to detect that?). > > > > > > > > Hi Jonathan, > > > > > > > > if we consider only the features implemented in st_lsm6dsx, asm330lhhx > > > > will be 1:1 compatible with lsm6dsr or lsm6dso, so we can just use one > > > > of bindings in this section to support it (the only side effect is it > > > > will be listed as "lsm6dsr" or "lsm6dso", but I guess it is ok). Agree? > > > > > > If the part has more features than the base compatible (or a different WAI) > > > then we can definitely have a backup compatible for it (hence making that > > > subset of features work on an old kernel). We still want to introduce > > > the new compatible so that we get the name right etc going forwards and > > > are in a good position to add the extra features if we ever get around to it. > > > > ack. I did not completely get what you mean here with "backup compatible". > > Do you mean: > > - use "st,lsm6dsr" for asm330lhhx on older kernels and add "st,asm330lhhx" on > > new ones. Do you have any pointer on how to document it? > Take a look at the mpu6050 patches. > > > properties: > compatible: > - enum: > - - invensense,iam20680 > - - invensense,icm20608 > - - invensense,icm20609 > - - invensense,icm20689 > - - invensense,icm20602 > - - invensense,icm20690 > - - invensense,mpu6000 > - - invensense,mpu6050 > - - invensense,mpu6500 > - - invensense,mpu6515 > - - invensense,mpu6880 > - - invensense,mpu9150 > - - invensense,mpu9250 > - - invensense,mpu9255 > + oneOf: > + - enum: > + - invensense,iam20680 > + - invensense,icm20608 > + - invensense,icm20609 > + - invensense,icm20689 > + - invensense,icm20602 > + - invensense,icm20690 > + - invensense,mpu6000 > + - invensense,mpu6050 > + - invensense,mpu6500 > + - invensense,mpu6515 > + - invensense,mpu6880 > + - invensense,mpu9150 > + - invensense,mpu9250 > + - invensense,mpu9255 > + - items: > + - const: invensense,icm20608d > + - const: invensense,icm20608 > > Which ends up expecting > > compatible = "invensense,icm20608d", "invensense,icm20608" > and will try matching on the first. If that fails it will try > with the second value. ack, thx for pointing this out. I will fix it in v2. Regards, Lorenzo > > > > > or > > > > - add a "wildcard" compatible string for this kind of devices. Do you have any > > pointers? > > > > Regards, > > Lorenzo > > > > > > > > > > > > > > > > The only difference between asm330lhhx and asm330lhh is the former supports > > > > sensor-hub while the latter does not declare it (even if the use the same > > > > whoami). > > > > AFAIK there is no way to autodetect if the sensor supports sensor-hub and > > > > we can just try to discover slave devices connected. This can have some > > > > downside as described in the commit: > > > > > > Ah thanks. I'd forgotten this. > > > > > > > > > > > commit 35619155d044830357f06f1d2c8188c4530b4d7a > > > > Author: Lorenzo Bianconi <lorenzo@kernel.org> > > > > Date: Sat Nov 13 16:23:14 2021 +0100 > > > > > > > > iio: imu: st_lsm6dsx: add dts property to disable sensor-hub > > > > > > > > I would like to merge the sections in st_lsm6dsx_settings struct for > > > > lsm6dsr, lsm6dso.. and lsm6dsop, asm330lhh since the only difference is > > > > sensor-hub support. I guess we can have 2 option here to avoid any > > > > sensor-hub corner cases: > > > > - provide the "st,disable-sensor-hub" in dts to disable sensor-hub for > > > > asm330lhh, lsm6dsop (need user changes) > > > > - add a bool variable st_lsm6dsx_settings[].id[] in order to specify if the > > > > chip supports sensor-hub. > > > > > > > > Which one do you prefer? > > > > > > > > Regards, > > > > Lorenzo > > > > > > > > > > > > > > As such, we should really be listing this as compatible with one > > > > > of the parts that is already supported such as the > > > > > LSM6DSR. > > > > > > > > > > For that we'll need a slightly more complex binding and it would > > > > > have the side effect that if the match was on that compatible we > > > > > would list the name as whatever that part is. > > > > > > > > > > I'm not sure that really matters a great deal, but it could in theory > > > > > create a userspace ABI change if we later needed to add explicit support > > > > > for the part due to some real differences not indicated by the WAI value. > > > > > > > > > > An extension is whether we should relax the need to match on WAI if > > > > > the part is considered compatible. I guess that depends on just how > > > > > compatible we think they are. > > > > > > > > > > So I see several steps to this process. > > > > > > > > > > 1) Add fallback compatibles for existing entries to first one with same WAI and > > > > > same feature set. > > > > > 2) Add fallback compatibles beyond that to first part introduced with particular > > > > > characteristics. For this we'd also want to have the driver relax its > > > > > handling to just warn if the WAI isn't listed for any of the parts that > > > > > share a particular set of characteristic (so you'll have to loop over the local > > > > > array again to check): > > > > > https://elixir.bootlin.com/linux/latest/source/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c#L1197 > > > > > Same argument applies as for the mpu6050 that, whilst we should modify that code to > > > > > cope, it's not a prerequisit for adding the compatible fallback to the binding. > > > > > Personally I'd like it to be the first patch in the series that modifies the > > > > > binding though. Note it'll be easy to add the fallbacks for this new part as > > > > > no mainline trees presumably use it. To 'fix' the rest we'll have to find > > > > > and update any DTs in mainline. > > > > > > > > > > Note this won't stop us needing to add compatibles to newer kernels (at very > > > > > least to the dt-binding, but probably also the driver), but it should help a newer > > > > > DT 'work' with an old kernel. > > > > > > > > > > Jonathan > > > > > > > > > > > > > > > > --- > > > > > > Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml | 1 + > > > > > > 1 file changed, 1 insertion(+) > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > > > > > > index 0750f700a143..23637c420d20 100644 > > > > > > --- a/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > > > > > > +++ b/Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml > > > > > > @@ -31,6 +31,7 @@ properties: > > > > > > - st,lsm6dsrx > > > > > > - st,lsm6dst > > > > > > - st,lsm6dsop > > > > > > + - st,asm330lhhx > > > > > > > > > > > > reg: > > > > > > maxItems: 1 > > > > > > > > > > > > > > > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-04-04 21:18 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-04-02 10:09 [PATCH 0/2] add support for ASM330LHHX Lorenzo Bianconi 2022-04-02 10:09 ` [PATCH 1/2] iio: imu: st_lsm6dsx: add support to ASM330LHHX Lorenzo Bianconi 2022-04-02 16:22 ` Jonathan Cameron 2022-04-03 11:15 ` Lorenzo Bianconi 2022-04-02 10:09 ` [PATCH 2/2] dt-bindings: iio: imu: st_lsm6dsx: add asm330lhhx device bindings Lorenzo Bianconi 2022-04-02 16:17 ` Jonathan Cameron 2022-04-03 14:56 ` Lorenzo Bianconi 2022-04-04 9:22 ` Jonathan Cameron 2022-04-04 9:33 ` Lorenzo Bianconi 2022-04-04 16:17 ` Jonathan Cameron 2022-04-04 19:18 ` Lorenzo Bianconi
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.