add support for ASM330LHHXG1

PATCH Superseded Lorenzo Bianconi 2 participants 8 messages Jan 24, 2024 – Jan 28, 2024

Subsystems (3)

Device tree bindings IIO ST LSM6DSx IMU IIO driver

lore

Share & tools

lore

Lorenzo Bianconi

author maintainer

2024-01-24 10:52 UTC #0 lore raw

cover add support for ASM330LHHXG1

Add support for ASM330LHHXG1 IMU automotive mems sensor
https://www.st.com/resource/en/datasheet/asm330lhhxg1.pdf

Lorenzo Bianconi (2):
iio: imu: st_lsm6dsx: add support to ASM330LHHXG1
dt-bindings: iio: imu: st_lsm6dsx: add asm330lhhxg1

 .../bindings/iio/imu/st,lsm6dsx.yaml          |  3 ++
 drivers/iio/imu/st_lsm6dsx/Kconfig            |  4 +-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h       | 48 ++++++++++---------
 .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    |  2 +-
 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, 46 insertions(+), 27 deletions(-)

Signature · 1 line

2.43.0

Lorenzo Bianconi

author maintainer

2024-01-24 10:52 UTC #1 lore raw

PATCH 1/2 iio: imu: st_lsm6dsx: add support to ASM330LHHXG1

Add support to STM ASM330LHHXG1 (accelerometer and gyroscope) Mems sensor.
The ASM330LHHXG1 sensor can use ASM330LHHX as fallback device since it implements all the ASM330LHHXG1 features currently implemented in st_lsm6dsx.

Link https://www.st.com/resource/en/datasheet/asm330lhhxg1.pdf

Signed-off-by Lorenzo Bianconi <[email protected]>

---
 drivers/iio/imu/st_lsm6dsx/Kconfig            |  4 +-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h       | 48 ++++++++++---------
 .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    |  2 +-
 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, 43 insertions(+), 27 deletions(-)

drivers/iio/imu/st_lsm6dsx/Kconfig +2 -2

index 5865a295a4df..645039edd606 100644
@@ -14,8 +14,8 @@ config IIO_ST_LSM6DSX
 	  sensor. Supported devices: lsm6ds3, lsm6ds3h, lsm6dsl, lsm6dsm,
 	  ism330dlc, lsm6dso, lsm6dsox, asm330lhh, asm330lhhx, lsm6dsr,
 	  lsm6ds3tr-c, ism330dhcx, lsm6dsrx, lsm6ds0, lsm6dsop, lsm6dstx,
-	  lsm6dsv, lsm6dsv16x, lsm6dso16is, ism330is, asm330lhb, lsm6dst
-	  and the accelerometer/gyroscope of lsm9ds1.
+	  lsm6dsv, lsm6dsv16x, lsm6dso16is, ism330is, asm330lhb, asm330lhhxg1,
+	  lsm6dst and the accelerometer/gyroscope of lsm9ds1.
 
 	  To compile this driver as a module, choose M here: the module
 	  will be called st_lsm6dsx.

drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h +25 ~~-23~~

60 lines not on this page · raw message

drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c +1 -1

index 066fe561c5e8..5d9539822ec6 100644
@@ -15,7 +15,7 @@
  * value of the decimation factor and ODR set for each FIFO data set.
  *
  * LSM6DSO/LSM6DSOX/ASM330LHH/ASM330LHHX/LSM6DSR/LSM6DSRX/ISM330DHCX/
- * LSM6DST/LSM6DSOP/LSM6DSTX/LSM6DSV/ASM330LHB:
+ * LSM6DST/LSM6DSOP/LSM6DSTX/LSM6DSV/ASM330LHB/ASM330LHHXG1:
  * 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).

drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +5 -1

index b6e6b1df8a61..27ecf2a5d0bc 100644
@@ -27,7 +27,7 @@
  *   - FIFO size: 4KB
  *
  * - LSM6DSO/LSM6DSOX/ASM330LHH/ASM330LHHX/LSM6DSR/ISM330DHCX/LSM6DST/LSM6DSOP/
- *   LSM6DSTX/LSM6DSO16IS/ISM330IS:
+ *   LSM6DSTX/LSM6DSO16IS/ISM330IS/ASM330LHHXG1:
  *   - Accelerometer/Gyroscope supported ODR [Hz]: 12.5, 26, 52, 104, 208, 416,
  *     833
  *   - Accelerometer supported full-scale [g]: +-2/+-4/+-8/+-16

@@ -820,6 +820,10 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.hw_id = ST_ASM330LHHX_ID,
 				.name = ST_ASM330LHHX_DEV_NAME,
 				.wai = 0x6b,
+			}, {
+				.hw_id = ST_ASM330LHHXG1_ID,
+				.name = ST_ASM330LHHXG1_DEV_NAME,
+				.wai = 0x6b,
 			}, {
 				.hw_id = ST_LSM6DSTX_ID,
 				.name = ST_LSM6DSTX_DEV_NAME,

drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c +5 -0

index 911444ec57c0..cddf41cc0ca9 100644
@@ -134,6 +134,10 @@ static const struct of_device_id st_lsm6dsx_i2c_of_match[] = {
 		.compatible = "st,asm330lhb",
 		.data = (void *)ST_ASM330LHB_ID,
 	},
+	{
+		.compatible = "st,asm330lhhxg1",
+		.data = (void *)ST_ASM330LHHXG1_ID,
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, st_lsm6dsx_i2c_of_match);

@@ -168,6 +172,7 @@ static const struct i2c_device_id st_lsm6dsx_i2c_id_table[] = {
 	{ ST_LSM6DSO16IS_DEV_NAME, ST_LSM6DSO16IS_ID },
 	{ ST_ISM330IS_DEV_NAME, ST_ISM330IS_ID },
 	{ ST_ASM330LHB_DEV_NAME, ST_ASM330LHB_ID },
+	{ ST_ASM330LHHXG1_DEV_NAME, ST_ASM330LHHXG1_ID },
 	{},
 };
 MODULE_DEVICE_TABLE(i2c, st_lsm6dsx_i2c_id_table);

drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c +5 -0

index f56c170c41a9..c122c8831365 100644
@@ -129,6 +129,10 @@ static const struct of_device_id st_lsm6dsx_spi_of_match[] = {
 		.compatible = "st,asm330lhb",
 		.data = (void *)ST_ASM330LHB_ID,
 	},
+	{
+		.compatible = "st,asm330lhhxg1",
+		.data = (void *)ST_ASM330LHHXG1_ID,
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, st_lsm6dsx_spi_of_match);

@@ -157,6 +161,7 @@ static const struct spi_device_id st_lsm6dsx_spi_id_table[] = {
 	{ ST_LSM6DSO16IS_DEV_NAME, ST_LSM6DSO16IS_ID },
 	{ ST_ISM330IS_DEV_NAME, ST_ISM330IS_ID },
 	{ ST_ASM330LHB_DEV_NAME, ST_ASM330LHB_ID },
+	{ ST_ASM330LHHXG1_DEV_NAME, ST_ASM330LHHXG1_ID },
 	{},
 };
 MODULE_DEVICE_TABLE(spi, st_lsm6dsx_spi_id_table);

Signature · 1 line

2.43.0

Jonathan Cameron

maintainer

2024-01-27 15:00 UTC #2 lore raw

Re: [PATCH 1/2] iio: imu: st_lsm6dsx: add support to ASM330LHHXG1

Quoted 7 lines · On Wed, 24 Jan 2024 11:52:33 +0100 Lorenzo Bianconi <[email protected]> wrote:

Add support to STM ASM330LHHXG1 (accelerometer and gyroscope) Mems sensor.
The ASM330LHHXG1 sensor can use ASM330LHHX as fallback device since it implements all the ASM330LHHXG1 features currently implemented in st_lsm6dsx.
Link https://www.st.com/resource/en/datasheet/asm330lhhxg1.pdf

Datasheet:

Quoted 1 line

Signed-off-by Lorenzo Bianconi <[email protected]>

Hi Lorenzo.

A few comments inline. Mostly about make it less noisy to add additional devices in future.

Quoted 14 lines

---
 drivers/iio/imu/st_lsm6dsx/Kconfig            |  4 +-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h       | 48 ++++++++++---------
 .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    |  2 +-
 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, 43 insertions(+), 27 deletions(-)

drivers/iio/imu/st_lsm6dsx/Kconfig +0 -0

index 5865a295a4df..645039edd606 100644

drivers/iio/imu/st_lsm6dsx/Kconfig config IIO_ST_LSM6DSX · line 18

Quoted 7 lines

drivers/iio/imu/st_lsm6dsx/Kconfig +2 -2

@@ -14,8 +14,8 @@ config IIO_ST_LSM6DSX
 	  sensor. Supported devices: lsm6ds3, lsm6ds3h, lsm6dsl, lsm6dsm,
 	  ism330dlc, lsm6dso, lsm6dsox, asm330lhh, asm330lhhx, lsm6dsr,
 	  lsm6ds3tr-c, ism330dhcx, lsm6dsrx, lsm6ds0, lsm6dsop, lsm6dstx,
-	  lsm6dsv, lsm6dsv16x, lsm6dso16is, ism330is, asm330lhb, lsm6dst
-	  and the accelerometer/gyroscope of lsm9ds1.
+	  lsm6dsv, lsm6dsv16x, lsm6dso16is, ism330is, asm330lhb, asm330lhhxg1,
+	  lsm6dst and the accelerometer/gyroscope of lsm9ds1.

I think it is worth a precursor to reformat this into a one entry per line list so that changes become less noisy (and easier to deal with merge conflicts) There are some examples of lists in drivers/hid/Kconfig and they look find in menuconfig etc.

If I were not requesting other changes I'd have suggested this as a follow up patch but given you are going to be respinning again, it would be neater as a precursor!

Quoted 48 lines

drivers/iio/imu/st_lsm6dsx/Kconfig +0 -0

 	  To compile this driver as a module, choose M here: the module
 	  will be called st_lsm6dsx.

drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h +14 ~~-23~~

index c19237717e81..78d12d3c2759 100644
@@ -15,29 +15,30 @@
 #include <linux/iio/iio.h>
 #include <linux/regulator/consumer.h>
 
-#define ST_LSM6DS3_DEV_NAME	"lsm6ds3"
-#define ST_LSM6DS3H_DEV_NAME	"lsm6ds3h"
-#define ST_LSM6DSL_DEV_NAME	"lsm6dsl"
-#define ST_LSM6DSM_DEV_NAME	"lsm6dsm"
-#define ST_ISM330DLC_DEV_NAME	"ism330dlc"
-#define ST_LSM6DSO_DEV_NAME	"lsm6dso"
-#define ST_ASM330LHH_DEV_NAME	"asm330lhh"
-#define ST_LSM6DSOX_DEV_NAME	"lsm6dsox"
-#define ST_LSM6DSR_DEV_NAME	"lsm6dsr"
-#define ST_LSM6DS3TRC_DEV_NAME	"lsm6ds3tr-c"
-#define ST_ISM330DHCX_DEV_NAME	"ism330dhcx"
-#define ST_LSM9DS1_DEV_NAME	"lsm9ds1-imu"
-#define ST_LSM6DS0_DEV_NAME	"lsm6ds0"
-#define ST_LSM6DSRX_DEV_NAME	"lsm6dsrx"
-#define ST_LSM6DST_DEV_NAME	"lsm6dst"
-#define ST_LSM6DSOP_DEV_NAME	"lsm6dsop"
-#define ST_ASM330LHHX_DEV_NAME	"asm330lhhx"
-#define ST_LSM6DSTX_DEV_NAME	"lsm6dstx"
-#define ST_LSM6DSV_DEV_NAME	"lsm6dsv"
-#define ST_LSM6DSV16X_DEV_NAME	"lsm6dsv16x"
-#define ST_LSM6DSO16IS_DEV_NAME	"lsm6dso16is"
-#define ST_ISM330IS_DEV_NAME	"ism330is"
-#define ST_ASM330LHB_DEV_NAME	"asm330lhb"
+#define ST_LSM6DS3_DEV_NAME		"lsm6ds3"
+#define ST_LSM6DS3H_DEV_NAME		"lsm6ds3h"
+#define ST_LSM6DSL_DEV_NAME		"lsm6dsl"
+#define ST_LSM6DSM_DEV_NAME		"lsm6dsm"
+#define ST_ISM330DLC_DEV_NAME		"ism330dlc"
+#define ST_LSM6DSO_DEV_NAME		"lsm6dso"
+#define ST_ASM330LHH_DEV_NAME		"asm330lhh"
+#define ST_LSM6DSOX_DEV_NAME		"lsm6dsox"
+#define ST_LSM6DSR_DEV_NAME		"lsm6dsr"
+#define ST_LSM6DS3TRC_DEV_NAME		"lsm6ds3tr-c"
+#define ST_ISM330DHCX_DEV_NAME		"ism330dhcx"
+#define ST_LSM9DS1_DEV_NAME		"lsm9ds1-imu"
+#define ST_LSM6DS0_DEV_NAME		"lsm6ds0"
+#define ST_LSM6DSRX_DEV_NAME		"lsm6dsrx"

drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h line 41

Quoted 10 lines

drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h +10 -0

+#define ST_LSM6DST_DEV_NAME		"lsm6dst"
+#define ST_LSM6DSOP_DEV_NAME		"lsm6dsop"
+#define ST_ASM330LHHX_DEV_NAME		"asm330lhhx"
+#define ST_LSM6DSTX_DEV_NAME		"lsm6dstx"
+#define ST_LSM6DSV_DEV_NAME		"lsm6dsv"
+#define ST_LSM6DSV16X_DEV_NAME		"lsm6dsv16x"
+#define ST_LSM6DSO16IS_DEV_NAME		"lsm6dso16is"
+#define ST_ISM330IS_DEV_NAME		"ism330is"
+#define ST_ASM330LHB_DEV_NAME		"asm330lhb"
+#define ST_ASM330LHHXG1_DEV_NAME	"asm330lhhxg1"

Too much noise. I don't care if they are all aligned the same. If you really want to do this, break it out to a precursor patch. I'd just have one entry with a different indent :)

Quoted 16 lines

drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h +1 -0

 enum st_lsm6dsx_hw_id {
 	ST_LSM6DS3_ID = 1,

@@ -63,6 +64,7 @@ enum st_lsm6dsx_hw_id {
 	ST_LSM6DSO16IS_ID,
 	ST_ISM330IS_ID,
 	ST_ASM330LHB_ID,
+	ST_ASM330LHHXG1_ID,
 	ST_LSM6DSX_MAX_ID,
 };

drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c +0 -0

index 066fe561c5e8..5d9539822ec6 100644

drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c line 18

Quoted 5 lines

drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c +1 -1

@@ -15,7 +15,7 @@
  * value of the decimation factor and ODR set for each FIFO data set.
  *
  * LSM6DSO/LSM6DSOX/ASM330LHH/ASM330LHHX/LSM6DSR/LSM6DSRX/ISM330DHCX/
- * LSM6DST/LSM6DSOP/LSM6DSTX/LSM6DSV/ASM330LHB:
+ * LSM6DST/LSM6DSOP/LSM6DSTX/LSM6DSV/ASM330LHB/ASM330LHHXG1:

A list here as well would help.

drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c line 30

Quoted 13 lines

drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c +0 -0

  * 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).

drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +1 -1

index b6e6b1df8a61..27ecf2a5d0bc 100644
@@ -27,7 +27,7 @@
  *   - FIFO size: 4KB
  *
  * - LSM6DSO/LSM6DSOX/ASM330LHH/ASM330LHHX/LSM6DSR/ISM330DHCX/LSM6DST/LSM6DSOP/
- *   LSM6DSTX/LSM6DSO16IS/ISM330IS:
+ *   LSM6DSTX/LSM6DSO16IS/ISM330IS/ASM330LHHXG1:

Another place where reformatting as a list would make adding new entries less noisy
+ whilst doing that, please fix the ordering to be alphabetical.

Quoted 60 lines

drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +4 -0

  *   - Accelerometer/Gyroscope supported ODR [Hz]: 12.5, 26, 52, 104, 208, 416,
  *     833
  *   - Accelerometer supported full-scale [g]: +-2/+-4/+-8/+-16

@@ -820,6 +820,10 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.hw_id = ST_ASM330LHHX_ID,
 				.name = ST_ASM330LHHX_DEV_NAME,
 				.wai = 0x6b,
+			}, {
+				.hw_id = ST_ASM330LHHXG1_ID,
+				.name = ST_ASM330LHHXG1_DEV_NAME,
+				.wai = 0x6b,
 			}, {
 				.hw_id = ST_LSM6DSTX_ID,
 				.name = ST_LSM6DSTX_DEV_NAME,

drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c +5 -0

index 911444ec57c0..cddf41cc0ca9 100644
@@ -134,6 +134,10 @@ static const struct of_device_id st_lsm6dsx_i2c_of_match[] = {
 		.compatible = "st,asm330lhb",
 		.data = (void *)ST_ASM330LHB_ID,
 	},
+	{
+		.compatible = "st,asm330lhhxg1",
+		.data = (void *)ST_ASM330LHHXG1_ID,
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, st_lsm6dsx_i2c_of_match);

@@ -168,6 +172,7 @@ static const struct i2c_device_id st_lsm6dsx_i2c_id_table[] = {
 	{ ST_LSM6DSO16IS_DEV_NAME, ST_LSM6DSO16IS_ID },
 	{ ST_ISM330IS_DEV_NAME, ST_ISM330IS_ID },
 	{ ST_ASM330LHB_DEV_NAME, ST_ASM330LHB_ID },
+	{ ST_ASM330LHHXG1_DEV_NAME, ST_ASM330LHHXG1_ID },
 	{},
 };
 MODULE_DEVICE_TABLE(i2c, st_lsm6dsx_i2c_id_table);

drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c +5 -0

index f56c170c41a9..c122c8831365 100644
@@ -129,6 +129,10 @@ static const struct of_device_id st_lsm6dsx_spi_of_match[] = {
 		.compatible = "st,asm330lhb",
 		.data = (void *)ST_ASM330LHB_ID,
 	},
+	{
+		.compatible = "st,asm330lhhxg1",
+		.data = (void *)ST_ASM330LHHXG1_ID,
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, st_lsm6dsx_spi_of_match);

@@ -157,6 +161,7 @@ static const struct spi_device_id st_lsm6dsx_spi_id_table[] = {
 	{ ST_LSM6DSO16IS_DEV_NAME, ST_LSM6DSO16IS_ID },
 	{ ST_ISM330IS_DEV_NAME, ST_ISM330IS_ID },
 	{ ST_ASM330LHB_DEV_NAME, ST_ASM330LHB_ID },
+	{ ST_ASM330LHHXG1_DEV_NAME, ST_ASM330LHHXG1_ID },
 	{},
 };
 MODULE_DEVICE_TABLE(spi, st_lsm6dsx_spi_id_table);

Lorenzo Bianconi

author maintainer

2024-01-28 10:38 UTC #3 lore raw

Re: [PATCH 1/2] iio: imu: st_lsm6dsx: add support to ASM330LHHXG1

Quoted 45 lines

Quoted 7 lines · On Wed, 24 Jan 2024 11:52:33 +0100 Lorenzo Bianconi <[email protected]> wrote:
Add support to STM ASM330LHHXG1 (accelerometer and gyroscope) Mems sensor.
The ASM330LHHXG1 sensor can use ASM330LHHX as fallback device since it implements all the ASM330LHHXG1 features currently implemented in st_lsm6dsx.
Link https://www.st.com/resource/en/datasheet/asm330lhhxg1.pdf
Datasheet:
Quoted 1 line
Signed-off-by Lorenzo Bianconi <[email protected]>
Hi Lorenzo.
A few comments inline. Mostly about make it less noisy to add additional devices in future.
Quoted 14 lines
---
 drivers/iio/imu/st_lsm6dsx/Kconfig            |  4 +-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h       | 48 ++++++++++---------
 .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    |  2 +-
 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, 43 insertions(+), 27 deletions(-)
drivers/iio/imu/st_lsm6dsx/Kconfig +0 -0
index 5865a295a4df..645039edd606 100644
drivers/iio/imu/st_lsm6dsx/Kconfig config IIO_ST_LSM6DSX · line 18
Quoted 7 lines
drivers/iio/imu/st_lsm6dsx/Kconfig +2 -2
@@ -14,8 +14,8 @@ config IIO_ST_LSM6DSX
 	  sensor. Supported devices: lsm6ds3, lsm6ds3h, lsm6dsl, lsm6dsm,
 	  ism330dlc, lsm6dso, lsm6dsox, asm330lhh, asm330lhhx, lsm6dsr,
 	  lsm6ds3tr-c, ism330dhcx, lsm6dsrx, lsm6ds0, lsm6dsop, lsm6dstx,
-	  lsm6dsv, lsm6dsv16x, lsm6dso16is, ism330is, asm330lhb, lsm6dst
-	  and the accelerometer/gyroscope of lsm9ds1.
+	  lsm6dsv, lsm6dsv16x, lsm6dso16is, ism330is, asm330lhb, asm330lhhxg1,
+	  lsm6dst and the accelerometer/gyroscope of lsm9ds1.
I think it is worth a precursor to reformat this into a one entry per line list so that changes become less noisy (and easier to deal with merge conflicts) There are some examples of lists in drivers/hid/Kconfig and they look find in menuconfig etc.

Quoted 3 lines

If I were not requesting other changes I'd have suggested this as a follow up patch but given you are going to be respinning again, it would be neater as a precursor!

ack, I will fix it in v2.

Quoted 56 lines

drivers/iio/imu/st_lsm6dsx/Kconfig +0 -0

 	  To compile this driver as a module, choose M here: the module
 	  will be called st_lsm6dsx.

drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h +20 ~~-23~~

index c19237717e81..78d12d3c2759 100644
@@ -15,29 +15,30 @@
 #include <linux/iio/iio.h>
 #include <linux/regulator/consumer.h>
 
-#define ST_LSM6DS3_DEV_NAME	"lsm6ds3"
-#define ST_LSM6DS3H_DEV_NAME	"lsm6ds3h"
-#define ST_LSM6DSL_DEV_NAME	"lsm6dsl"
-#define ST_LSM6DSM_DEV_NAME	"lsm6dsm"
-#define ST_ISM330DLC_DEV_NAME	"ism330dlc"
-#define ST_LSM6DSO_DEV_NAME	"lsm6dso"
-#define ST_ASM330LHH_DEV_NAME	"asm330lhh"
-#define ST_LSM6DSOX_DEV_NAME	"lsm6dsox"
-#define ST_LSM6DSR_DEV_NAME	"lsm6dsr"
-#define ST_LSM6DS3TRC_DEV_NAME	"lsm6ds3tr-c"
-#define ST_ISM330DHCX_DEV_NAME	"ism330dhcx"
-#define ST_LSM9DS1_DEV_NAME	"lsm9ds1-imu"
-#define ST_LSM6DS0_DEV_NAME	"lsm6ds0"
-#define ST_LSM6DSRX_DEV_NAME	"lsm6dsrx"
-#define ST_LSM6DST_DEV_NAME	"lsm6dst"
-#define ST_LSM6DSOP_DEV_NAME	"lsm6dsop"
-#define ST_ASM330LHHX_DEV_NAME	"asm330lhhx"
-#define ST_LSM6DSTX_DEV_NAME	"lsm6dstx"
-#define ST_LSM6DSV_DEV_NAME	"lsm6dsv"
-#define ST_LSM6DSV16X_DEV_NAME	"lsm6dsv16x"
-#define ST_LSM6DSO16IS_DEV_NAME	"lsm6dso16is"
-#define ST_ISM330IS_DEV_NAME	"ism330is"
-#define ST_ASM330LHB_DEV_NAME	"asm330lhb"
+#define ST_LSM6DS3_DEV_NAME		"lsm6ds3"
+#define ST_LSM6DS3H_DEV_NAME		"lsm6ds3h"
+#define ST_LSM6DSL_DEV_NAME		"lsm6dsl"
+#define ST_LSM6DSM_DEV_NAME		"lsm6dsm"
+#define ST_ISM330DLC_DEV_NAME		"ism330dlc"
+#define ST_LSM6DSO_DEV_NAME		"lsm6dso"
+#define ST_ASM330LHH_DEV_NAME		"asm330lhh"
+#define ST_LSM6DSOX_DEV_NAME		"lsm6dsox"
+#define ST_LSM6DSR_DEV_NAME		"lsm6dsr"
+#define ST_LSM6DS3TRC_DEV_NAME		"lsm6ds3tr-c"
+#define ST_ISM330DHCX_DEV_NAME		"ism330dhcx"
+#define ST_LSM9DS1_DEV_NAME		"lsm9ds1-imu"
+#define ST_LSM6DS0_DEV_NAME		"lsm6ds0"
+#define ST_LSM6DSRX_DEV_NAME		"lsm6dsrx"
+#define ST_LSM6DST_DEV_NAME		"lsm6dst"
+#define ST_LSM6DSOP_DEV_NAME		"lsm6dsop"
+#define ST_ASM330LHHX_DEV_NAME		"asm330lhhx"
+#define ST_LSM6DSTX_DEV_NAME		"lsm6dstx"
+#define ST_LSM6DSV_DEV_NAME		"lsm6dsv"
+#define ST_LSM6DSV16X_DEV_NAME		"lsm6dsv16x"

Quoted 8 lines

drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h line 41
Quoted 4 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h +4 -0
+#define ST_LSM6DSO16IS_DEV_NAME		"lsm6dso16is"
+#define ST_ISM330IS_DEV_NAME		"ism330is"
+#define ST_ASM330LHB_DEV_NAME		"asm330lhb"
+#define ST_ASM330LHHXG1_DEV_NAME	"asm330lhhxg1"
Too much noise. I don't care if they are all aligned the same. If you really want to do this, break it out to a precursor patch. I'd just have one entry with a different indent :)

ack, I will fix it in v2.

Quoted 13 lines

drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h +1 -0

 enum st_lsm6dsx_hw_id {
 	ST_LSM6DS3_ID = 1,

@@ -63,6 +64,7 @@ enum st_lsm6dsx_hw_id {
 	ST_LSM6DSO16IS_ID,
 	ST_ISM330IS_ID,
 	ST_ASM330LHB_ID,
+	ST_ASM330LHHXG1_ID,
 	ST_LSM6DSX_MAX_ID,
 };

Quoted 12 lines

drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c line 18

Quoted 10 lines

drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c +1 -1

index 066fe561c5e8..5d9539822ec6 100644
@@ -15,7 +15,7 @@
  * value of the decimation factor and ODR set for each FIFO data set.
  *
  * LSM6DSO/LSM6DSOX/ASM330LHH/ASM330LHHX/LSM6DSR/LSM6DSRX/ISM330DHCX/
- * LSM6DST/LSM6DSOP/LSM6DSTX/LSM6DSV/ASM330LHB:
+ * LSM6DST/LSM6DSOP/LSM6DSTX/LSM6DSV/ASM330LHB/ASM330LHHXG1:

A list here as well would help.

ack, I will fix it in v2.

Quoted 17 lines

Quoted 9 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c +0 -0
  * 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).
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +0 -0
index b6e6b1df8a61..27ecf2a5d0bc 100644
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c line 30
Quoted 5 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +1 -1
@@ -27,7 +27,7 @@
  *   - FIFO size: 4KB
  *
  * - LSM6DSO/LSM6DSOX/ASM330LHH/ASM330LHHX/LSM6DSR/ISM330DHCX/LSM6DST/LSM6DSOP/
- *   LSM6DSTX/LSM6DSO16IS/ISM330IS:
+ *   LSM6DSTX/LSM6DSO16IS/ISM330IS/ASM330LHHXG1:
Another place where reformatting as a list would make adding new entries less noisy
+ whilst doing that, please fix the ordering to be alphabetical.

ack, I will fix it in v2.

Regards,
Lorenzo

Quoted 62 lines

Quoted 61 lines

drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +4 -0

  *   - Accelerometer/Gyroscope supported ODR [Hz]: 12.5, 26, 52, 104, 208, 416,
  *     833
  *   - Accelerometer supported full-scale [g]: +-2/+-4/+-8/+-16

@@ -820,6 +820,10 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.hw_id = ST_ASM330LHHX_ID,
 				.name = ST_ASM330LHHX_DEV_NAME,
 				.wai = 0x6b,
+			}, {
+				.hw_id = ST_ASM330LHHXG1_ID,
+				.name = ST_ASM330LHHXG1_DEV_NAME,
+				.wai = 0x6b,
 			}, {
 				.hw_id = ST_LSM6DSTX_ID,
 				.name = ST_LSM6DSTX_DEV_NAME,

drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c +5 -0

index 911444ec57c0..cddf41cc0ca9 100644
@@ -134,6 +134,10 @@ static const struct of_device_id st_lsm6dsx_i2c_of_match[] = {
 		.compatible = "st,asm330lhb",
 		.data = (void *)ST_ASM330LHB_ID,
 	},
+	{
+		.compatible = "st,asm330lhhxg1",
+		.data = (void *)ST_ASM330LHHXG1_ID,
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, st_lsm6dsx_i2c_of_match);

@@ -168,6 +172,7 @@ static const struct i2c_device_id st_lsm6dsx_i2c_id_table[] = {
 	{ ST_LSM6DSO16IS_DEV_NAME, ST_LSM6DSO16IS_ID },
 	{ ST_ISM330IS_DEV_NAME, ST_ISM330IS_ID },
 	{ ST_ASM330LHB_DEV_NAME, ST_ASM330LHB_ID },
+	{ ST_ASM330LHHXG1_DEV_NAME, ST_ASM330LHHXG1_ID },
 	{},
 };
 MODULE_DEVICE_TABLE(i2c, st_lsm6dsx_i2c_id_table);

drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c +5 -0

index f56c170c41a9..c122c8831365 100644
@@ -129,6 +129,10 @@ static const struct of_device_id st_lsm6dsx_spi_of_match[] = {
 		.compatible = "st,asm330lhb",
 		.data = (void *)ST_ASM330LHB_ID,
 	},
+	{
+		.compatible = "st,asm330lhhxg1",
+		.data = (void *)ST_ASM330LHHXG1_ID,
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, st_lsm6dsx_spi_of_match);

@@ -157,6 +161,7 @@ static const struct spi_device_id st_lsm6dsx_spi_id_table[] = {
 	{ ST_LSM6DSO16IS_DEV_NAME, ST_LSM6DSO16IS_ID },
 	{ ST_ISM330IS_DEV_NAME, ST_ISM330IS_ID },
 	{ ST_ASM330LHB_DEV_NAME, ST_ASM330LHB_ID },
+	{ ST_ASM330LHHXG1_DEV_NAME, ST_ASM330LHHXG1_ID },
 	{},
 };
 MODULE_DEVICE_TABLE(spi, st_lsm6dsx_spi_id_table);

Lorenzo Bianconi

author maintainer

2024-01-24 10:52 UTC #4 lore raw

PATCH 2/2 dt-bindings: iio: imu: st_lsm6dsx: add asm330lhhxg1

In reply to Lorenzo Bianconi

Add device bindings for asm330lhhxg1 IMU sensor.
Use asm330lhhx as fallback device for asm330lhhxg1 since it implements all the features currently supported by asm330lhhxg1.

Signed-off-by Lorenzo Bianconi <[email protected]>

---
 Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml | 3 +++
 1 file changed, 3 insertions(+)

Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml +3 -0

index 28b667a9cb76..7ba3de66a2e1 100644
@@ -49,6 +49,9 @@ properties:
       - items:
           - const: st,asm330lhb
           - const: st,asm330lhh
+      - items:
+          - const: st,asm330lhhxg1
+          - const: st,asm330lhhx
 
   reg:
     maxItems: 1

Signature · 1 line

2.43.0

Jonathan Cameron

maintainer

2024-01-27 15:04 UTC #5 lore raw

Re: [PATCH 2/2] dt-bindings: iio: imu: st_lsm6dsx: add asm330lhhxg1

Quoted 3 lines · On Wed, 24 Jan 2024 11:52:34 +0100 Lorenzo Bianconi <[email protected]> wrote:

Add device bindings for asm330lhhxg1 IMU sensor.
Use asm330lhhx as fallback device for asm330lhhxg1 since it implements all the features currently supported by asm330lhhxg1.

Can we clarify if the hardware of the asm330lhhxg1 supports
1) Exactly the same features as the asm330lhhx
(Valid use for a fallback compatible, because we might find
bugs etc in future).
2) More features, but the asm330lhhx features set is a subset. (Most valid use for a fallback compatible)
3) Different features from the asm330llhhx but driver only supports a set both support? (this is a problem)

I 'think' it is 2 from your text, but I'm not 100% sure I've understood correctly.

Quoted 20 lines

Signed-off-by Lorenzo Bianconi <[email protected]>

---
 Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml | 3 +++
 1 file changed, 3 insertions(+)

Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml +3 -0

index 28b667a9cb76..7ba3de66a2e1 100644
@@ -49,6 +49,9 @@ properties:
       - items:
           - const: st,asm330lhb
           - const: st,asm330lhh
+      - items:
+          - const: st,asm330lhhxg1
+          - const: st,asm330lhhx
 
   reg:
     maxItems: 1

Lorenzo Bianconi

author maintainer

2024-01-28 10:35 UTC #6 lore raw

Re: [PATCH 2/2] dt-bindings: iio: imu: st_lsm6dsx: add asm330lhhxg1

Quoted 7 lines

Quoted 3 lines · On Wed, 24 Jan 2024 11:52:34 +0100 Lorenzo Bianconi <[email protected]> wrote:
Add device bindings for asm330lhhxg1 IMU sensor.
Use asm330lhhx as fallback device for asm330lhhxg1 since it implements all the features currently supported by asm330lhhxg1.

Quoted 6 lines

Can we clarify if the hardware of the asm330lhhxg1 supports
1) Exactly the same features as the asm330lhhx (Valid use for a fallback compatible, because we might find bugs etc in future).
2) More features, but the asm330lhhx features set is a subset. (Most valid use for a fallback compatible)

Hi Jonathan,

afaik asm330lhhx supports a subset of asm330lhhxg1 features (adding st folks in cc for some clarification here) but they are not implemented in st_lsm6dsx.

Regards,
Lorenzo

Quoted 26 lines

3) Different features from the asm330llhhx but driver only supports a set both support? (this is a problem)
I 'think' it is 2 from your text, but I'm not 100% sure I've understood correctly.
Quoted 20 lines
Signed-off-by Lorenzo Bianconi <[email protected]>
---
 Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml | 3 +++
 1 file changed, 3 insertions(+)
Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml +3 -0
index 28b667a9cb76..7ba3de66a2e1 100644
@@ -49,6 +49,9 @@ properties:
       - items:
           - const: st,asm330lhb
           - const: st,asm330lhh
+      - items:
+          - const: st,asm330lhhxg1
+          - const: st,asm330lhhx
 
   reg:
     maxItems: 1

Jonathan Cameron

maintainer

2024-01-28 13:39 UTC #7 lore raw

Re: [PATCH 2/2] dt-bindings: iio: imu: st_lsm6dsx: add asm330lhhxg1

Quoted 14 lines · On Sun, 28 Jan 2024 11:35:23 +0100 Lorenzo Bianconi <[email protected]> wrote:

Quoted 7 lines
Quoted 3 lines · On Wed, 24 Jan 2024 11:52:34 +0100 Lorenzo Bianconi <[email protected]> wrote:
Add device bindings for asm330lhhxg1 IMU sensor.
Use asm330lhhx as fallback device for asm330lhhxg1 since it implements all the features currently supported by asm330lhhxg1.
Quoted 6 lines
Can we clarify if the hardware of the asm330lhhxg1 supports
1) Exactly the same features as the asm330lhhx (Valid use for a fallback compatible, because we might find bugs etc in future).
2) More features, but the asm330lhhx features set is a subset. (Most valid use for a fallback compatible)

Quoted 4 lines

Hi Jonathan,
afaik asm330lhhx supports a subset of asm330lhhxg1 features (adding st folks in cc for some clarification here) but they are not implemented in st_lsm6dsx.

Great. State that a little more clearly in the patch description for v2 please.

Thanks,

Jonathan

Quoted 30 lines

Regards,
Lorenzo
Quoted 26 lines
3) Different features from the asm330llhhx but driver only supports a set both support? (this is a problem)
I 'think' it is 2 from your text, but I'm not 100% sure I've understood correctly.
Quoted 20 lines
Signed-off-by Lorenzo Bianconi <[email protected]>
---
 Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml | 3 +++
 1 file changed, 3 insertions(+)
Documentation/devicetree/bindings/iio/imu/st,lsm6dsx.yaml +3 -0
index 28b667a9cb76..7ba3de66a2e1 100644
@@ -49,6 +49,9 @@ properties:
       - items:
           - const: st,asm330lhb
           - const: st,asm330lhh
+      - items:
+          - const: st,asm330lhhxg1
+          - const: st,asm330lhhx
 
   reg:
     maxItems: 1  