linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] iio: imu: st_lsm6sdx: move some register definitions to sensor_settings struct
@ 2019-07-15 13:19 Martin Kepplinger
  2019-07-15 13:19 ` [PATCH 2/3] iio: imu: st_lsm6dsx: add support for accel/gyro unit of lsm9sd1 Martin Kepplinger
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Martin Kepplinger @ 2019-07-15 13:19 UTC (permalink / raw)
  To: lorenzo.bianconi83, jic23, knaack.h, lars, pmeerw
  Cc: linux-iio, devicetree, linux-kernel, Martin Kepplinger

Move some register definitions to the per-device array of struct
st_lsm6dsx_sensor_settings in order to simplify adding new sensor
devices to the driver.

Also, remove completely unused register definitions.

Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  6 ++++
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 31 ++++++++++++++------
 2 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
index c14bf533b66b..f072ac14f213 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
@@ -196,6 +196,9 @@ struct st_lsm6dsx_ext_dev_settings {
 /**
  * struct st_lsm6dsx_settings - ST IMU sensor settings
  * @wai: Sensor WhoAmI default value.
+ * @reg_int1_addr: Control Register address for INT1
+ * @reg_int2_addr: Control Register address for INT2
+ * @reg_reset_addr: register address for reset/reboot
  * @max_fifo_size: Sensor max fifo length in FIFO words.
  * @id: List of hw id/device name supported by the driver configuration.
  * @decimator: List of decimator register info (addr + mask).
@@ -206,6 +209,9 @@ struct st_lsm6dsx_ext_dev_settings {
  */
 struct st_lsm6dsx_settings {
 	u8 wai;
+	u8 reg_int1_addr;
+	u8 reg_int2_addr;
+	u8 reg_reset_addr;
 	u16 max_fifo_size;
 	struct {
 		enum st_lsm6dsx_hw_id hw_id;
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index a6702a74570e..7a4fe70a8f20 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -49,17 +49,12 @@
 
 #include "st_lsm6dsx.h"
 
-#define ST_LSM6DSX_REG_INT1_ADDR		0x0d
-#define ST_LSM6DSX_REG_INT2_ADDR		0x0e
 #define ST_LSM6DSX_REG_FIFO_FTH_IRQ_MASK	BIT(3)
 #define ST_LSM6DSX_REG_WHOAMI_ADDR		0x0f
-#define ST_LSM6DSX_REG_RESET_ADDR		0x12
 #define ST_LSM6DSX_REG_RESET_MASK		BIT(0)
 #define ST_LSM6DSX_REG_BOOT_MASK		BIT(7)
 #define ST_LSM6DSX_REG_BDU_ADDR			0x12
 #define ST_LSM6DSX_REG_BDU_MASK			BIT(6)
-#define ST_LSM6DSX_REG_INT2_ON_INT1_ADDR	0x13
-#define ST_LSM6DSX_REG_INT2_ON_INT1_MASK	BIT(5)
 
 #define ST_LSM6DSX_REG_ACC_OUT_X_L_ADDR		0x28
 #define ST_LSM6DSX_REG_ACC_OUT_Y_L_ADDR		0x2a
@@ -122,6 +117,9 @@ static const struct st_lsm6dsx_fs_table_entry st_lsm6dsx_fs_table[] = {
 static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 	{
 		.wai = 0x69,
+		.reg_int1_addr = 0x0d,
+		.reg_int2_addr = 0x0e,
+		.reg_reset_addr = 0x12,
 		.max_fifo_size = 1365,
 		.id = {
 			{
@@ -172,6 +170,9 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 	},
 	{
 		.wai = 0x69,
+		.reg_int1_addr = 0x0d,
+		.reg_int2_addr = 0x0e,
+		.reg_reset_addr = 0x12,
 		.max_fifo_size = 682,
 		.id = {
 			{
@@ -222,6 +223,9 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 	},
 	{
 		.wai = 0x6a,
+		.reg_int1_addr = 0x0d,
+		.reg_int2_addr = 0x0e,
+		.reg_reset_addr = 0x12,
 		.max_fifo_size = 682,
 		.id = {
 			{
@@ -278,6 +282,9 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 	},
 	{
 		.wai = 0x6c,
+		.reg_int1_addr = 0x0d,
+		.reg_int2_addr = 0x0e,
+		.reg_reset_addr = 0x12,
 		.max_fifo_size = 512,
 		.id = {
 			{
@@ -349,6 +356,9 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 	},
 	{
 		.wai = 0x6b,
+		.reg_int1_addr = 0x0d,
+		.reg_int2_addr = 0x0e,
+		.reg_reset_addr = 0x12,
 		.max_fifo_size = 512,
 		.id = {
 			{
@@ -391,6 +401,9 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 	},
 	{
 		.wai = 0x6b,
+		.reg_int1_addr = 0x0d,
+		.reg_int2_addr = 0x0e,
+		.reg_reset_addr = 0x12,
 		.max_fifo_size = 512,
 		.id = {
 			{
@@ -873,10 +886,10 @@ static int st_lsm6dsx_get_drdy_reg(struct st_lsm6dsx_hw *hw, u8 *drdy_reg)
 
 	switch (drdy_pin) {
 	case 1:
-		*drdy_reg = ST_LSM6DSX_REG_INT1_ADDR;
+		*drdy_reg = hw->settings->reg_int1_addr;
 		break;
 	case 2:
-		*drdy_reg = ST_LSM6DSX_REG_INT2_ADDR;
+		*drdy_reg = hw->settings->reg_int2_addr;
 		break;
 	default:
 		dev_err(hw->dev, "unsupported data ready pin\n");
@@ -976,7 +989,7 @@ static int st_lsm6dsx_init_device(struct st_lsm6dsx_hw *hw)
 	int err;
 
 	/* device sw reset */
-	err = regmap_update_bits(hw->regmap, ST_LSM6DSX_REG_RESET_ADDR,
+	err = regmap_update_bits(hw->regmap, hw->settings->reg_reset_addr,
 				 ST_LSM6DSX_REG_RESET_MASK,
 				 FIELD_PREP(ST_LSM6DSX_REG_RESET_MASK, 1));
 	if (err < 0)
@@ -985,7 +998,7 @@ static int st_lsm6dsx_init_device(struct st_lsm6dsx_hw *hw)
 	msleep(50);
 
 	/* reload trimming parameter */
-	err = regmap_update_bits(hw->regmap, ST_LSM6DSX_REG_RESET_ADDR,
+	err = regmap_update_bits(hw->regmap, hw->settings->reg_reset_addr,
 				 ST_LSM6DSX_REG_BOOT_MASK,
 				 FIELD_PREP(ST_LSM6DSX_REG_BOOT_MASK, 1));
 	if (err < 0)
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/3] iio: imu: st_lsm6dsx: add support for accel/gyro unit of lsm9sd1
  2019-07-15 13:19 [PATCH 1/3] iio: imu: st_lsm6sdx: move some register definitions to sensor_settings struct Martin Kepplinger
@ 2019-07-15 13:19 ` Martin Kepplinger
  2019-07-15 21:49   ` Lorenzo Bianconi
  2019-07-15 13:19 ` [PATCH 3/3] dt-bindings: iio: imu: st_lsm6dsx: add lsm9ds1 device bindings Martin Kepplinger
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Martin Kepplinger @ 2019-07-15 13:19 UTC (permalink / raw)
  To: lorenzo.bianconi83, jic23, knaack.h, lars, pmeerw
  Cc: linux-iio, devicetree, linux-kernel, Martin Kepplinger

The LSM9DS1's accelerometer / gyroscope unit and it's magnetometer (separately
supported in iio/magnetometer/st_magn*) are located on a separate i2c addresses
on the bus.

For the datasheet, see https://www.st.com/resource/en/datasheet/lsm9ds1.pdf

Treat it just like the LSM6* devices and, despite it's name, hook it up
to the st_lsm6dsx driver, using it's basic functionality.

Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
---

What do you think about an addition like this? How confusing is it to support
an LSM9 module by the lsm6 driver, despite it's name? It requires almost no
code, so why not think about it, right?

Oh, I'm not 100% convinced by my new "if" in probe(), but even that is
not too confusing I guess.

thanks,

                           martin

p.s.: todos:
* hook up the fifo / buffer / trigger functionality,
* (off-topic a bit) move the (currently strange) gyro-only support
  for lsm9ds0 to this driver as well.



 drivers/iio/imu/st_lsm6dsx/Kconfig           |   3 +-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |   4 +
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 105 ++++++++++++++++++-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c  |   5 +
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c  |   5 +
 5 files changed, 117 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/Kconfig b/drivers/iio/imu/st_lsm6dsx/Kconfig
index 002a423eae52..0b5a568e4c16 100644
--- a/drivers/iio/imu/st_lsm6dsx/Kconfig
+++ b/drivers/iio/imu/st_lsm6dsx/Kconfig
@@ -10,7 +10,8 @@ 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
+	  ism330dlc, lsm6dso, lsm6dsox, asm330lhh, lsm6dsr and the
+	  accelerometer/gyroscope of lsm9ds1.
 
 	  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 f072ac14f213..8af9641260fa 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
@@ -22,6 +22,7 @@
 #define ST_ASM330LHH_DEV_NAME	"asm330lhh"
 #define ST_LSM6DSOX_DEV_NAME	"lsm6dsox"
 #define ST_LSM6DSR_DEV_NAME	"lsm6dsr"
+#define ST_LSM9DS1_DEV_NAME	"lsm9ds1"
 
 enum st_lsm6dsx_hw_id {
 	ST_LSM6DS3_ID,
@@ -33,6 +34,7 @@ enum st_lsm6dsx_hw_id {
 	ST_ASM330LHH_ID,
 	ST_LSM6DSOX_ID,
 	ST_LSM6DSR_ID,
+	ST_LSM9DS1_ID,
 	ST_LSM6DSX_MAX_ID,
 };
 
@@ -230,6 +232,8 @@ enum st_lsm6dsx_sensor_id {
 	ST_LSM6DSX_ID_EXT0,
 	ST_LSM6DSX_ID_EXT1,
 	ST_LSM6DSX_ID_EXT2,
+	ST_LSM9DSX_ID_GYRO,
+	ST_LSM9DSX_ID_ACC,
 	ST_LSM6DSX_ID_MAX,
 };
 
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index 7a4fe70a8f20..6acfe63073de 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -10,6 +10,8 @@
  * +-125/+-245/+-500/+-1000/+-2000 dps
  * LSM6DSx series has an integrated First-In-First-Out (FIFO) buffer
  * allowing dynamic batching of sensor data.
+ * LSM9DSx series is similar but includes an additional magnetometer, handled
+ * by a different driver.
  *
  * Supported sensors:
  * - LSM6DS3:
@@ -30,6 +32,13 @@
  *   - Gyroscope supported full-scale [dps]: +-125/+-245/+-500/+-1000/+-2000
  *   - FIFO size: 3KB
  *
+ * - LSM9DS1:
+ *   - Accelerometer supported ODR [Hz]: 10, 50, 119, 238, 476, 952
+ *   - Accelerometer supported full-scale [g]: +-2/+-4/+-8/+-16
+ *   - Gyroscope supported ODR [Hz]: 15, 60, 119, 238, 476, 952
+ *   - Gyroscope supported full-scale [dps]: +-245/+-500/+-2000
+ *   - FIFO size: 32
+ *
  * Copyright 2016 STMicroelectronics Inc.
  *
  * Lorenzo Bianconi <lorenzo.bianconi@st.com>
@@ -64,6 +73,10 @@
 #define ST_LSM6DSX_REG_GYRO_OUT_Y_L_ADDR	0x24
 #define ST_LSM6DSX_REG_GYRO_OUT_Z_L_ADDR	0x26
 
+#define ST_LSM9DSX_REG_GYRO_OUT_X_L_ADDR	0x18
+#define ST_LSM9DSX_REG_GYRO_OUT_Y_L_ADDR	0x1a
+#define ST_LSM9DSX_REG_GYRO_OUT_Z_L_ADDR	0x1c
+
 static const struct st_lsm6dsx_odr_table_entry st_lsm6dsx_odr_table[] = {
 	[ST_LSM6DSX_ID_ACC] = {
 		.reg = {
@@ -88,6 +101,30 @@ static const struct st_lsm6dsx_odr_table_entry st_lsm6dsx_odr_table[] = {
 		.odr_avl[3] = { 104, 0x04 },
 		.odr_avl[4] = { 208, 0x05 },
 		.odr_avl[5] = { 416, 0x06 },
+	},
+	[ST_LSM9DSX_ID_ACC] = {
+		.reg = {
+			.addr = 0x20,
+			.mask = GENMASK(7, 5),
+		},
+		.odr_avl[0] = {  10, 0x01 },
+		.odr_avl[1] = {  50, 0x02 },
+		.odr_avl[2] = { 119, 0x03 },
+		.odr_avl[3] = { 238, 0x04 },
+		.odr_avl[4] = { 476, 0x05 },
+		.odr_avl[5] = { 952, 0x06 },
+	},
+	[ST_LSM9DSX_ID_GYRO] = {
+		.reg = {
+			.addr = 0x10,
+			.mask = GENMASK(7, 5),
+		},
+		.odr_avl[0] = {  15, 0x01 },
+		.odr_avl[1] = {  60, 0x02 },
+		.odr_avl[2] = { 119, 0x03 },
+		.odr_avl[3] = { 238, 0x04 },
+		.odr_avl[4] = { 476, 0x05 },
+		.odr_avl[5] = { 952, 0x06 },
 	}
 };
 
@@ -111,10 +148,43 @@ static const struct st_lsm6dsx_fs_table_entry st_lsm6dsx_fs_table[] = {
 		.fs_avl[1] = { IIO_DEGREE_TO_RAD(17500), 0x1 },
 		.fs_avl[2] = { IIO_DEGREE_TO_RAD(35000), 0x2 },
 		.fs_avl[3] = { IIO_DEGREE_TO_RAD(70000), 0x3 },
+	},
+	[ST_LSM9DSX_ID_ACC] = {
+		.reg = {
+			.addr = 0x20,
+			.mask = GENMASK(4, 3),
+		},
+		.fs_avl[0] = {  599, 0x0 },
+		.fs_avl[1] = { 1197, 0x2 },
+		.fs_avl[2] = { 2394, 0x3 },
+		.fs_avl[3] = { 4788, 0x1 },
+	},
+	[ST_LSM9DSX_ID_GYRO] = {
+		.reg = {
+			.addr = 0x10,
+			.mask = GENMASK(4, 3),
+		},
+		.fs_avl[0] = { IIO_DEGREE_TO_RAD(245), 0x0 },
+		.fs_avl[1] = { IIO_DEGREE_TO_RAD(500), 0x1 },
+		.fs_avl[2] = { IIO_DEGREE_TO_RAD(0), 0x2 },
+		.fs_avl[3] = { IIO_DEGREE_TO_RAD(2000), 0x3 },
 	}
 };
 
 static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
+	{
+		.wai = 0x68,
+		.reg_int1_addr = 0x0c,
+		.reg_int2_addr = 0x0d,
+		.reg_reset_addr = 0x22,
+		.max_fifo_size = 32,
+		.id = {
+			{
+				.hw_id = ST_LSM9DS1_ID,
+				.name = ST_LSM9DS1_DEV_NAME,
+			},
+		},
+	},
 	{
 		.wai = 0x69,
 		.reg_int1_addr = 0x0d,
@@ -492,6 +562,16 @@ static const struct iio_chan_spec st_lsm6dsx_gyro_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(3),
 };
 
+static const struct iio_chan_spec st_lsm9dsx_gyro_channels[] = {
+	ST_LSM6DSX_CHANNEL(IIO_ANGL_VEL, ST_LSM9DSX_REG_GYRO_OUT_X_L_ADDR,
+			   IIO_MOD_X, 0),
+	ST_LSM6DSX_CHANNEL(IIO_ANGL_VEL, ST_LSM9DSX_REG_GYRO_OUT_Y_L_ADDR,
+			   IIO_MOD_Y, 1),
+	ST_LSM6DSX_CHANNEL(IIO_ANGL_VEL, ST_LSM9DSX_REG_GYRO_OUT_Z_L_ADDR,
+			   IIO_MOD_Z, 2),
+	IIO_CHAN_SOFT_TIMESTAMP(3),
+};
+
 int st_lsm6dsx_set_page(struct st_lsm6dsx_hw *hw, bool enable)
 {
 	const struct st_lsm6dsx_shub_settings *hub_settings;
@@ -1056,6 +1136,7 @@ static struct iio_dev *st_lsm6dsx_alloc_iiodev(struct st_lsm6dsx_hw *hw,
 
 	switch (id) {
 	case ST_LSM6DSX_ID_ACC:
+	case ST_LSM9DSX_ID_ACC:
 		iio_dev->channels = st_lsm6dsx_acc_channels;
 		iio_dev->num_channels = ARRAY_SIZE(st_lsm6dsx_acc_channels);
 		iio_dev->info = &st_lsm6dsx_acc_info;
@@ -1068,6 +1149,14 @@ static struct iio_dev *st_lsm6dsx_alloc_iiodev(struct st_lsm6dsx_hw *hw,
 		iio_dev->num_channels = ARRAY_SIZE(st_lsm6dsx_gyro_channels);
 		iio_dev->info = &st_lsm6dsx_gyro_info;
 
+		scnprintf(sensor->name, sizeof(sensor->name), "%s_gyro",
+			  name);
+		break;
+	case ST_LSM9DSX_ID_GYRO:
+		iio_dev->channels = st_lsm9dsx_gyro_channels;
+		iio_dev->num_channels = ARRAY_SIZE(st_lsm9dsx_gyro_channels);
+		iio_dev->info = &st_lsm6dsx_gyro_info;
+
 		scnprintf(sensor->name, sizeof(sensor->name), "%s_gyro",
 			  name);
 		break;
@@ -1109,10 +1198,18 @@ int st_lsm6dsx_probe(struct device *dev, int irq, int hw_id,
 	if (err < 0)
 		return err;
 
-	for (i = 0; i < ST_LSM6DSX_ID_EXT0; i++) {
-		hw->iio_devs[i] = st_lsm6dsx_alloc_iiodev(hw, i, name);
-		if (!hw->iio_devs[i])
-			return -ENOMEM;
+	if (hw_id == ST_LSM9DS1_ID) {
+		for (i = ST_LSM9DSX_ID_GYRO; i <= ST_LSM9DSX_ID_ACC; i++) {
+			hw->iio_devs[i] = st_lsm6dsx_alloc_iiodev(hw, i, name);
+			if (!hw->iio_devs[i])
+				return -ENOMEM;
+		}
+	} else {
+		for (i = 0; i < ST_LSM6DSX_ID_EXT0; i++) {
+			hw->iio_devs[i] = st_lsm6dsx_alloc_iiodev(hw, i, name);
+			if (!hw->iio_devs[i])
+				return -ENOMEM;
+		}
 	}
 
 	err = st_lsm6dsx_init_device(hw);
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c
index b3211e0ac07b..a684e7db1299 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c
@@ -75,6 +75,10 @@ static const struct of_device_id st_lsm6dsx_i2c_of_match[] = {
 		.compatible = "st,lsm6dsr",
 		.data = (void *)ST_LSM6DSR_ID,
 	},
+	{
+		.compatible = "st,lsm9ds1",
+		.data = (void *)ST_LSM9DS1_ID,
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, st_lsm6dsx_i2c_of_match);
@@ -89,6 +93,7 @@ static const struct i2c_device_id st_lsm6dsx_i2c_id_table[] = {
 	{ ST_ASM330LHH_DEV_NAME, ST_ASM330LHH_ID },
 	{ ST_LSM6DSOX_DEV_NAME, ST_LSM6DSOX_ID },
 	{ ST_LSM6DSR_DEV_NAME, ST_LSM6DSR_ID },
+	{ ST_LSM9DS1_DEV_NAME, ST_LSM9DS1_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 c9d3c4711018..709769177e91 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c
@@ -75,6 +75,10 @@ static const struct of_device_id st_lsm6dsx_spi_of_match[] = {
 		.compatible = "st,lsm6dsr",
 		.data = (void *)ST_LSM6DSR_ID,
 	},
+	{
+		.compatible = "st,lsm9ds1",
+		.data = (void *)ST_LSM9DS1_ID,
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, st_lsm6dsx_spi_of_match);
@@ -89,6 +93,7 @@ static const struct spi_device_id st_lsm6dsx_spi_id_table[] = {
 	{ ST_ASM330LHH_DEV_NAME, ST_ASM330LHH_ID },
 	{ ST_LSM6DSOX_DEV_NAME, ST_LSM6DSOX_ID },
 	{ ST_LSM6DSR_DEV_NAME, ST_LSM6DSR_ID },
+	{ ST_LSM9DS1_DEV_NAME, ST_LSM9DS1_ID },
 	{},
 };
 MODULE_DEVICE_TABLE(spi, st_lsm6dsx_spi_id_table);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/3] dt-bindings: iio: imu: st_lsm6dsx: add lsm9ds1 device bindings
  2019-07-15 13:19 [PATCH 1/3] iio: imu: st_lsm6sdx: move some register definitions to sensor_settings struct Martin Kepplinger
  2019-07-15 13:19 ` [PATCH 2/3] iio: imu: st_lsm6dsx: add support for accel/gyro unit of lsm9sd1 Martin Kepplinger
@ 2019-07-15 13:19 ` Martin Kepplinger
  2019-07-15 21:10 ` [PATCH 1/3] iio: imu: st_lsm6sdx: move some register definitions to sensor_settings struct Lorenzo Bianconi
  2019-07-25  5:40 ` Martin Kepplinger
  3 siblings, 0 replies; 8+ messages in thread
From: Martin Kepplinger @ 2019-07-15 13:19 UTC (permalink / raw)
  To: lorenzo.bianconi83, jic23, knaack.h, lars, pmeerw
  Cc: linux-iio, devicetree, linux-kernel, Martin Kepplinger

Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
---
 Documentation/devicetree/bindings/iio/imu/st_lsm6dsx.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/iio/imu/st_lsm6dsx.txt b/Documentation/devicetree/bindings/iio/imu/st_lsm6dsx.txt
index efec9ece034a..c5ac1730ad22 100644
--- a/Documentation/devicetree/bindings/iio/imu/st_lsm6dsx.txt
+++ b/Documentation/devicetree/bindings/iio/imu/st_lsm6dsx.txt
@@ -11,6 +11,7 @@ Required properties:
   "st,asm330lhh"
   "st,lsm6dsox"
   "st,lsm6dsr"
+  "st,lsm9ds1"
 - reg: i2c address of the sensor / spi cs line
 
 Optional properties:
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] iio: imu: st_lsm6sdx: move some register definitions to sensor_settings struct
  2019-07-15 13:19 [PATCH 1/3] iio: imu: st_lsm6sdx: move some register definitions to sensor_settings struct Martin Kepplinger
  2019-07-15 13:19 ` [PATCH 2/3] iio: imu: st_lsm6dsx: add support for accel/gyro unit of lsm9sd1 Martin Kepplinger
  2019-07-15 13:19 ` [PATCH 3/3] dt-bindings: iio: imu: st_lsm6dsx: add lsm9ds1 device bindings Martin Kepplinger
@ 2019-07-15 21:10 ` Lorenzo Bianconi
  2019-07-16  6:11   ` Martin Kepplinger
  2019-07-25  5:40 ` Martin Kepplinger
  3 siblings, 1 reply; 8+ messages in thread
From: Lorenzo Bianconi @ 2019-07-15 21:10 UTC (permalink / raw)
  To: Martin Kepplinger
  Cc: lorenzo.bianconi83, jic23, knaack.h, lars, pmeerw, linux-iio, devicetree

[-- Attachment #1: Type: text/plain, Size: 5565 bytes --]

> Move some register definitions to the per-device array of struct
> st_lsm6dsx_sensor_settings in order to simplify adding new sensor
> devices to the driver.
> 
> Also, remove completely unused register definitions.
> 

Hi Martin,

just few comments inline

Regards,
Lorenzo

> Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
> ---
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  6 ++++
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 31 ++++++++++++++------
>  2 files changed, 28 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> index c14bf533b66b..f072ac14f213 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> @@ -196,6 +196,9 @@ struct st_lsm6dsx_ext_dev_settings {
>  /**
>   * struct st_lsm6dsx_settings - ST IMU sensor settings
>   * @wai: Sensor WhoAmI default value.
> + * @reg_int1_addr: Control Register address for INT1
> + * @reg_int2_addr: Control Register address for INT2
> + * @reg_reset_addr: register address for reset/reboot
>   * @max_fifo_size: Sensor max fifo length in FIFO words.
>   * @id: List of hw id/device name supported by the driver configuration.
>   * @decimator: List of decimator register info (addr + mask).
> @@ -206,6 +209,9 @@ struct st_lsm6dsx_ext_dev_settings {
>   */
>  struct st_lsm6dsx_settings {
>  	u8 wai;
> +	u8 reg_int1_addr;
> +	u8 reg_int2_addr;
> +	u8 reg_reset_addr;

could you please rename them in int1_addr, int2_addr and reset_addr in order to
be inline with other st_lsm6dsx_settings fields?

>  	u16 max_fifo_size;
>  	struct {
>  		enum st_lsm6dsx_hw_id hw_id;
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index a6702a74570e..7a4fe70a8f20 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -49,17 +49,12 @@
>  
>  #include "st_lsm6dsx.h"
>  
> -#define ST_LSM6DSX_REG_INT1_ADDR		0x0d
> -#define ST_LSM6DSX_REG_INT2_ADDR		0x0e
>  #define ST_LSM6DSX_REG_FIFO_FTH_IRQ_MASK	BIT(3)
>  #define ST_LSM6DSX_REG_WHOAMI_ADDR		0x0f
> -#define ST_LSM6DSX_REG_RESET_ADDR		0x12
>  #define ST_LSM6DSX_REG_RESET_MASK		BIT(0)
>  #define ST_LSM6DSX_REG_BOOT_MASK		BIT(7)
>  #define ST_LSM6DSX_REG_BDU_ADDR			0x12
>  #define ST_LSM6DSX_REG_BDU_MASK			BIT(6)
> -#define ST_LSM6DSX_REG_INT2_ON_INT1_ADDR	0x13
> -#define ST_LSM6DSX_REG_INT2_ON_INT1_MASK	BIT(5)
>  
>  #define ST_LSM6DSX_REG_ACC_OUT_X_L_ADDR		0x28
>  #define ST_LSM6DSX_REG_ACC_OUT_Y_L_ADDR		0x2a
> @@ -122,6 +117,9 @@ static const struct st_lsm6dsx_fs_table_entry st_lsm6dsx_fs_table[] = {
>  static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  	{
>  		.wai = 0x69,
> +		.reg_int1_addr = 0x0d,
> +		.reg_int2_addr = 0x0e,
> +		.reg_reset_addr = 0x12,
>  		.max_fifo_size = 1365,
>  		.id = {
>  			{
> @@ -172,6 +170,9 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  	},
>  	{
>  		.wai = 0x69,
> +		.reg_int1_addr = 0x0d,
> +		.reg_int2_addr = 0x0e,
> +		.reg_reset_addr = 0x12,
>  		.max_fifo_size = 682,
>  		.id = {
>  			{
> @@ -222,6 +223,9 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  	},
>  	{
>  		.wai = 0x6a,
> +		.reg_int1_addr = 0x0d,
> +		.reg_int2_addr = 0x0e,
> +		.reg_reset_addr = 0x12,
>  		.max_fifo_size = 682,
>  		.id = {
>  			{
> @@ -278,6 +282,9 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  	},
>  	{
>  		.wai = 0x6c,
> +		.reg_int1_addr = 0x0d,
> +		.reg_int2_addr = 0x0e,
> +		.reg_reset_addr = 0x12,
>  		.max_fifo_size = 512,
>  		.id = {
>  			{
> @@ -349,6 +356,9 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  	},
>  	{
>  		.wai = 0x6b,
> +		.reg_int1_addr = 0x0d,
> +		.reg_int2_addr = 0x0e,
> +		.reg_reset_addr = 0x12,
>  		.max_fifo_size = 512,
>  		.id = {
>  			{
> @@ -391,6 +401,9 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  	},
>  	{
>  		.wai = 0x6b,
> +		.reg_int1_addr = 0x0d,
> +		.reg_int2_addr = 0x0e,
> +		.reg_reset_addr = 0x12,
>  		.max_fifo_size = 512,
>  		.id = {
>  			{
> @@ -873,10 +886,10 @@ static int st_lsm6dsx_get_drdy_reg(struct st_lsm6dsx_hw *hw, u8 *drdy_reg)
>  
>  	switch (drdy_pin) {
>  	case 1:
> -		*drdy_reg = ST_LSM6DSX_REG_INT1_ADDR;
> +		*drdy_reg = hw->settings->reg_int1_addr;
>  		break;
>  	case 2:
> -		*drdy_reg = ST_LSM6DSX_REG_INT2_ADDR;
> +		*drdy_reg = hw->settings->reg_int2_addr;
>  		break;
>  	default:
>  		dev_err(hw->dev, "unsupported data ready pin\n");
> @@ -976,7 +989,7 @@ static int st_lsm6dsx_init_device(struct st_lsm6dsx_hw *hw)
>  	int err;
>  
>  	/* device sw reset */
> -	err = regmap_update_bits(hw->regmap, ST_LSM6DSX_REG_RESET_ADDR,
> +	err = regmap_update_bits(hw->regmap, hw->settings->reg_reset_addr,
>  				 ST_LSM6DSX_REG_RESET_MASK,
>  				 FIELD_PREP(ST_LSM6DSX_REG_RESET_MASK, 1));
>  	if (err < 0)
> @@ -985,7 +998,7 @@ static int st_lsm6dsx_init_device(struct st_lsm6dsx_hw *hw)
>  	msleep(50);
>  
>  	/* reload trimming parameter */
> -	err = regmap_update_bits(hw->regmap, ST_LSM6DSX_REG_RESET_ADDR,
> +	err = regmap_update_bits(hw->regmap, hw->settings->reg_reset_addr,
>  				 ST_LSM6DSX_REG_BOOT_MASK,
>  				 FIELD_PREP(ST_LSM6DSX_REG_BOOT_MASK, 1));
>  	if (err < 0)
> -- 
> 2.20.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] iio: imu: st_lsm6dsx: add support for accel/gyro unit of lsm9sd1
  2019-07-15 13:19 ` [PATCH 2/3] iio: imu: st_lsm6dsx: add support for accel/gyro unit of lsm9sd1 Martin Kepplinger
@ 2019-07-15 21:49   ` Lorenzo Bianconi
  2019-07-16  6:10     ` Martin Kepplinger
  0 siblings, 1 reply; 8+ messages in thread
From: Lorenzo Bianconi @ 2019-07-15 21:49 UTC (permalink / raw)
  To: Martin Kepplinger
  Cc: lorenzo.bianconi83, jic23, knaack.h, lars, pmeerw, linux-iio,
	devicetree, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 11329 bytes --]

> The LSM9DS1's accelerometer / gyroscope unit and it's magnetometer (separately
> supported in iio/magnetometer/st_magn*) are located on a separate i2c addresses
> on the bus.
> 
> For the datasheet, see https://www.st.com/resource/en/datasheet/lsm9ds1.pdf
> 
> Treat it just like the LSM6* devices and, despite it's name, hook it up
> to the st_lsm6dsx driver, using it's basic functionality.
> 
> Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
> ---
> 
> What do you think about an addition like this? How confusing is it to support
> an LSM9 module by the lsm6 driver, despite it's name? It requires almost no
> code, so why not think about it, right?

I am fine with (it was on my ToDo list, so thanks for working on this).
I have just posted the following series that will help you adding support for
LSM9DS1
https://patchwork.kernel.org/cover/11045061/
I think you just need to take care of gyro channels allocating iio devices (you
probably need to pass hw_id to st_lsm6dsx_alloc_iiodev())

> 
> Oh, I'm not 100% convinced by my new "if" in probe(), but even that is
> not too confusing I guess.
> 
> thanks,
> 
>                            martin
> 
> p.s.: todos:
> * hook up the fifo / buffer / trigger functionality,
> * (off-topic a bit) move the (currently strange) gyro-only support
>   for lsm9ds0 to this driver as well.

Regarding FIFO I guess it is enough to set decimator factor to 1 for both accel
and gyro.

Regards,
Lorenzo

> 
> 
> 
>  drivers/iio/imu/st_lsm6dsx/Kconfig           |   3 +-
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |   4 +
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 105 ++++++++++++++++++-
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c  |   5 +
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c  |   5 +
>  5 files changed, 117 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/Kconfig b/drivers/iio/imu/st_lsm6dsx/Kconfig
> index 002a423eae52..0b5a568e4c16 100644
> --- a/drivers/iio/imu/st_lsm6dsx/Kconfig
> +++ b/drivers/iio/imu/st_lsm6dsx/Kconfig
> @@ -10,7 +10,8 @@ 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
> +	  ism330dlc, lsm6dso, lsm6dsox, asm330lhh, lsm6dsr and the
> +	  accelerometer/gyroscope of lsm9ds1.
>  
>  	  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 f072ac14f213..8af9641260fa 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> @@ -22,6 +22,7 @@
>  #define ST_ASM330LHH_DEV_NAME	"asm330lhh"
>  #define ST_LSM6DSOX_DEV_NAME	"lsm6dsox"
>  #define ST_LSM6DSR_DEV_NAME	"lsm6dsr"
> +#define ST_LSM9DS1_DEV_NAME	"lsm9ds1"
>  
>  enum st_lsm6dsx_hw_id {
>  	ST_LSM6DS3_ID,
> @@ -33,6 +34,7 @@ enum st_lsm6dsx_hw_id {
>  	ST_ASM330LHH_ID,
>  	ST_LSM6DSOX_ID,
>  	ST_LSM6DSR_ID,
> +	ST_LSM9DS1_ID,
>  	ST_LSM6DSX_MAX_ID,
>  };
>  
> @@ -230,6 +232,8 @@ enum st_lsm6dsx_sensor_id {
>  	ST_LSM6DSX_ID_EXT0,
>  	ST_LSM6DSX_ID_EXT1,
>  	ST_LSM6DSX_ID_EXT2,
> +	ST_LSM9DSX_ID_GYRO,
> +	ST_LSM9DSX_ID_ACC,
>  	ST_LSM6DSX_ID_MAX,
>  };
>  
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index 7a4fe70a8f20..6acfe63073de 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -10,6 +10,8 @@
>   * +-125/+-245/+-500/+-1000/+-2000 dps
>   * LSM6DSx series has an integrated First-In-First-Out (FIFO) buffer
>   * allowing dynamic batching of sensor data.
> + * LSM9DSx series is similar but includes an additional magnetometer, handled
> + * by a different driver.
>   *
>   * Supported sensors:
>   * - LSM6DS3:
> @@ -30,6 +32,13 @@
>   *   - Gyroscope supported full-scale [dps]: +-125/+-245/+-500/+-1000/+-2000
>   *   - FIFO size: 3KB
>   *
> + * - LSM9DS1:
> + *   - Accelerometer supported ODR [Hz]: 10, 50, 119, 238, 476, 952
> + *   - Accelerometer supported full-scale [g]: +-2/+-4/+-8/+-16
> + *   - Gyroscope supported ODR [Hz]: 15, 60, 119, 238, 476, 952
> + *   - Gyroscope supported full-scale [dps]: +-245/+-500/+-2000
> + *   - FIFO size: 32
> + *
>   * Copyright 2016 STMicroelectronics Inc.
>   *
>   * Lorenzo Bianconi <lorenzo.bianconi@st.com>
> @@ -64,6 +73,10 @@
>  #define ST_LSM6DSX_REG_GYRO_OUT_Y_L_ADDR	0x24
>  #define ST_LSM6DSX_REG_GYRO_OUT_Z_L_ADDR	0x26
>  
> +#define ST_LSM9DSX_REG_GYRO_OUT_X_L_ADDR	0x18
> +#define ST_LSM9DSX_REG_GYRO_OUT_Y_L_ADDR	0x1a
> +#define ST_LSM9DSX_REG_GYRO_OUT_Z_L_ADDR	0x1c
> +
>  static const struct st_lsm6dsx_odr_table_entry st_lsm6dsx_odr_table[] = {
>  	[ST_LSM6DSX_ID_ACC] = {
>  		.reg = {
> @@ -88,6 +101,30 @@ static const struct st_lsm6dsx_odr_table_entry st_lsm6dsx_odr_table[] = {
>  		.odr_avl[3] = { 104, 0x04 },
>  		.odr_avl[4] = { 208, 0x05 },
>  		.odr_avl[5] = { 416, 0x06 },
> +	},
> +	[ST_LSM9DSX_ID_ACC] = {
> +		.reg = {
> +			.addr = 0x20,
> +			.mask = GENMASK(7, 5),
> +		},
> +		.odr_avl[0] = {  10, 0x01 },
> +		.odr_avl[1] = {  50, 0x02 },
> +		.odr_avl[2] = { 119, 0x03 },
> +		.odr_avl[3] = { 238, 0x04 },
> +		.odr_avl[4] = { 476, 0x05 },
> +		.odr_avl[5] = { 952, 0x06 },
> +	},
> +	[ST_LSM9DSX_ID_GYRO] = {
> +		.reg = {
> +			.addr = 0x10,
> +			.mask = GENMASK(7, 5),
> +		},
> +		.odr_avl[0] = {  15, 0x01 },
> +		.odr_avl[1] = {  60, 0x02 },
> +		.odr_avl[2] = { 119, 0x03 },
> +		.odr_avl[3] = { 238, 0x04 },
> +		.odr_avl[4] = { 476, 0x05 },
> +		.odr_avl[5] = { 952, 0x06 },
>  	}
>  };
>  
> @@ -111,10 +148,43 @@ static const struct st_lsm6dsx_fs_table_entry st_lsm6dsx_fs_table[] = {
>  		.fs_avl[1] = { IIO_DEGREE_TO_RAD(17500), 0x1 },
>  		.fs_avl[2] = { IIO_DEGREE_TO_RAD(35000), 0x2 },
>  		.fs_avl[3] = { IIO_DEGREE_TO_RAD(70000), 0x3 },
> +	},
> +	[ST_LSM9DSX_ID_ACC] = {
> +		.reg = {
> +			.addr = 0x20,
> +			.mask = GENMASK(4, 3),
> +		},
> +		.fs_avl[0] = {  599, 0x0 },
> +		.fs_avl[1] = { 1197, 0x2 },
> +		.fs_avl[2] = { 2394, 0x3 },
> +		.fs_avl[3] = { 4788, 0x1 },
> +	},
> +	[ST_LSM9DSX_ID_GYRO] = {
> +		.reg = {
> +			.addr = 0x10,
> +			.mask = GENMASK(4, 3),
> +		},
> +		.fs_avl[0] = { IIO_DEGREE_TO_RAD(245), 0x0 },
> +		.fs_avl[1] = { IIO_DEGREE_TO_RAD(500), 0x1 },
> +		.fs_avl[2] = { IIO_DEGREE_TO_RAD(0), 0x2 },
> +		.fs_avl[3] = { IIO_DEGREE_TO_RAD(2000), 0x3 },
>  	}
>  };
>  
>  static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
> +	{
> +		.wai = 0x68,
> +		.reg_int1_addr = 0x0c,
> +		.reg_int2_addr = 0x0d,
> +		.reg_reset_addr = 0x22,
> +		.max_fifo_size = 32,
> +		.id = {
> +			{
> +				.hw_id = ST_LSM9DS1_ID,
> +				.name = ST_LSM9DS1_DEV_NAME,
> +			},
> +		},
> +	},
>  	{
>  		.wai = 0x69,
>  		.reg_int1_addr = 0x0d,
> @@ -492,6 +562,16 @@ static const struct iio_chan_spec st_lsm6dsx_gyro_channels[] = {
>  	IIO_CHAN_SOFT_TIMESTAMP(3),
>  };
>  
> +static const struct iio_chan_spec st_lsm9dsx_gyro_channels[] = {
> +	ST_LSM6DSX_CHANNEL(IIO_ANGL_VEL, ST_LSM9DSX_REG_GYRO_OUT_X_L_ADDR,
> +			   IIO_MOD_X, 0),
> +	ST_LSM6DSX_CHANNEL(IIO_ANGL_VEL, ST_LSM9DSX_REG_GYRO_OUT_Y_L_ADDR,
> +			   IIO_MOD_Y, 1),
> +	ST_LSM6DSX_CHANNEL(IIO_ANGL_VEL, ST_LSM9DSX_REG_GYRO_OUT_Z_L_ADDR,
> +			   IIO_MOD_Z, 2),
> +	IIO_CHAN_SOFT_TIMESTAMP(3),
> +};
> +
>  int st_lsm6dsx_set_page(struct st_lsm6dsx_hw *hw, bool enable)
>  {
>  	const struct st_lsm6dsx_shub_settings *hub_settings;
> @@ -1056,6 +1136,7 @@ static struct iio_dev *st_lsm6dsx_alloc_iiodev(struct st_lsm6dsx_hw *hw,
>  
>  	switch (id) {
>  	case ST_LSM6DSX_ID_ACC:
> +	case ST_LSM9DSX_ID_ACC:
>  		iio_dev->channels = st_lsm6dsx_acc_channels;
>  		iio_dev->num_channels = ARRAY_SIZE(st_lsm6dsx_acc_channels);
>  		iio_dev->info = &st_lsm6dsx_acc_info;
> @@ -1068,6 +1149,14 @@ static struct iio_dev *st_lsm6dsx_alloc_iiodev(struct st_lsm6dsx_hw *hw,
>  		iio_dev->num_channels = ARRAY_SIZE(st_lsm6dsx_gyro_channels);
>  		iio_dev->info = &st_lsm6dsx_gyro_info;
>  
> +		scnprintf(sensor->name, sizeof(sensor->name), "%s_gyro",
> +			  name);
> +		break;
> +	case ST_LSM9DSX_ID_GYRO:
> +		iio_dev->channels = st_lsm9dsx_gyro_channels;
> +		iio_dev->num_channels = ARRAY_SIZE(st_lsm9dsx_gyro_channels);
> +		iio_dev->info = &st_lsm6dsx_gyro_info;
> +
>  		scnprintf(sensor->name, sizeof(sensor->name), "%s_gyro",
>  			  name);
>  		break;
> @@ -1109,10 +1198,18 @@ int st_lsm6dsx_probe(struct device *dev, int irq, int hw_id,
>  	if (err < 0)
>  		return err;
>  
> -	for (i = 0; i < ST_LSM6DSX_ID_EXT0; i++) {
> -		hw->iio_devs[i] = st_lsm6dsx_alloc_iiodev(hw, i, name);
> -		if (!hw->iio_devs[i])
> -			return -ENOMEM;
> +	if (hw_id == ST_LSM9DS1_ID) {
> +		for (i = ST_LSM9DSX_ID_GYRO; i <= ST_LSM9DSX_ID_ACC; i++) {
> +			hw->iio_devs[i] = st_lsm6dsx_alloc_iiodev(hw, i, name);
> +			if (!hw->iio_devs[i])
> +				return -ENOMEM;
> +		}
> +	} else {
> +		for (i = 0; i < ST_LSM6DSX_ID_EXT0; i++) {
> +			hw->iio_devs[i] = st_lsm6dsx_alloc_iiodev(hw, i, name);
> +			if (!hw->iio_devs[i])
> +				return -ENOMEM;
> +		}
>  	}
>  
>  	err = st_lsm6dsx_init_device(hw);
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c
> index b3211e0ac07b..a684e7db1299 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c
> @@ -75,6 +75,10 @@ static const struct of_device_id st_lsm6dsx_i2c_of_match[] = {
>  		.compatible = "st,lsm6dsr",
>  		.data = (void *)ST_LSM6DSR_ID,
>  	},
> +	{
> +		.compatible = "st,lsm9ds1",
> +		.data = (void *)ST_LSM9DS1_ID,
> +	},
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, st_lsm6dsx_i2c_of_match);
> @@ -89,6 +93,7 @@ static const struct i2c_device_id st_lsm6dsx_i2c_id_table[] = {
>  	{ ST_ASM330LHH_DEV_NAME, ST_ASM330LHH_ID },
>  	{ ST_LSM6DSOX_DEV_NAME, ST_LSM6DSOX_ID },
>  	{ ST_LSM6DSR_DEV_NAME, ST_LSM6DSR_ID },
> +	{ ST_LSM9DS1_DEV_NAME, ST_LSM9DS1_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 c9d3c4711018..709769177e91 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c
> @@ -75,6 +75,10 @@ static const struct of_device_id st_lsm6dsx_spi_of_match[] = {
>  		.compatible = "st,lsm6dsr",
>  		.data = (void *)ST_LSM6DSR_ID,
>  	},
> +	{
> +		.compatible = "st,lsm9ds1",
> +		.data = (void *)ST_LSM9DS1_ID,
> +	},
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, st_lsm6dsx_spi_of_match);
> @@ -89,6 +93,7 @@ static const struct spi_device_id st_lsm6dsx_spi_id_table[] = {
>  	{ ST_ASM330LHH_DEV_NAME, ST_ASM330LHH_ID },
>  	{ ST_LSM6DSOX_DEV_NAME, ST_LSM6DSOX_ID },
>  	{ ST_LSM6DSR_DEV_NAME, ST_LSM6DSR_ID },
> +	{ ST_LSM9DS1_DEV_NAME, ST_LSM9DS1_ID },
>  	{},
>  };
>  MODULE_DEVICE_TABLE(spi, st_lsm6dsx_spi_id_table);
> -- 
> 2.20.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] iio: imu: st_lsm6dsx: add support for accel/gyro unit of lsm9sd1
  2019-07-15 21:49   ` Lorenzo Bianconi
@ 2019-07-16  6:10     ` Martin Kepplinger
  0 siblings, 0 replies; 8+ messages in thread
From: Martin Kepplinger @ 2019-07-16  6:10 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: lorenzo.bianconi83, jic23, knaack.h, lars, pmeerw, linux-iio,
	devicetree, linux-kernel

On 15.07.19 23:49, Lorenzo Bianconi wrote:
>> The LSM9DS1's accelerometer / gyroscope unit and it's magnetometer (separately
>> supported in iio/magnetometer/st_magn*) are located on a separate i2c addresses
>> on the bus.
>>
>> For the datasheet, see https://www.st.com/resource/en/datasheet/lsm9ds1.pdf
>>
>> Treat it just like the LSM6* devices and, despite it's name, hook it up
>> to the st_lsm6dsx driver, using it's basic functionality.
>>
>> Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
>> ---
>>
>> What do you think about an addition like this? How confusing is it to support
>> an LSM9 module by the lsm6 driver, despite it's name? It requires almost no
>> code, so why not think about it, right?
> 
> I am fine with (it was on my ToDo list, so thanks for working on this).
> I have just posted the following series that will help you adding support for
> LSM9DS1
> https://patchwork.kernel.org/cover/11045061/
> I think you just need to take care of gyro channels allocating iio devices (you
> probably need to pass hw_id to st_lsm6dsx_alloc_iiodev())

yes. I'll look over allocation once more.

> 
>>
>> Oh, I'm not 100% convinced by my new "if" in probe(), but even that is
>> not too confusing I guess.
>>
>> thanks,
>>
>>                            martin
>>
>> p.s.: todos:
>> * hook up the fifo / buffer / trigger functionality,
>> * (off-topic a bit) move the (currently strange) gyro-only support
>>   for lsm9ds0 to this driver as well.
> 
> Regarding FIFO I guess it is enough to set decimator factor to 1 for both accel
> and gyro.
> 
> Regards,
> Lorenzo
> 
>>
>>
>>
>>  drivers/iio/imu/st_lsm6dsx/Kconfig           |   3 +-
>>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |   4 +
>>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 105 ++++++++++++++++++-
>>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c  |   5 +
>>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c  |   5 +
>>  5 files changed, 117 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/iio/imu/st_lsm6dsx/Kconfig b/drivers/iio/imu/st_lsm6dsx/Kconfig
>> index 002a423eae52..0b5a568e4c16 100644
>> --- a/drivers/iio/imu/st_lsm6dsx/Kconfig
>> +++ b/drivers/iio/imu/st_lsm6dsx/Kconfig
>> @@ -10,7 +10,8 @@ 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
>> +	  ism330dlc, lsm6dso, lsm6dsox, asm330lhh, lsm6dsr and the
>> +	  accelerometer/gyroscope of lsm9ds1.
>>  
>>  	  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 f072ac14f213..8af9641260fa 100644
>> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
>> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
>> @@ -22,6 +22,7 @@
>>  #define ST_ASM330LHH_DEV_NAME	"asm330lhh"
>>  #define ST_LSM6DSOX_DEV_NAME	"lsm6dsox"
>>  #define ST_LSM6DSR_DEV_NAME	"lsm6dsr"
>> +#define ST_LSM9DS1_DEV_NAME	"lsm9ds1"
>>  
>>  enum st_lsm6dsx_hw_id {
>>  	ST_LSM6DS3_ID,
>> @@ -33,6 +34,7 @@ enum st_lsm6dsx_hw_id {
>>  	ST_ASM330LHH_ID,
>>  	ST_LSM6DSOX_ID,
>>  	ST_LSM6DSR_ID,
>> +	ST_LSM9DS1_ID,
>>  	ST_LSM6DSX_MAX_ID,
>>  };
>>  
>> @@ -230,6 +232,8 @@ enum st_lsm6dsx_sensor_id {
>>  	ST_LSM6DSX_ID_EXT0,
>>  	ST_LSM6DSX_ID_EXT1,
>>  	ST_LSM6DSX_ID_EXT2,
>> +	ST_LSM9DSX_ID_GYRO,
>> +	ST_LSM9DSX_ID_ACC,
>>  	ST_LSM6DSX_ID_MAX,
>>  };
>>  
>> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
>> index 7a4fe70a8f20..6acfe63073de 100644
>> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
>> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
>> @@ -10,6 +10,8 @@
>>   * +-125/+-245/+-500/+-1000/+-2000 dps
>>   * LSM6DSx series has an integrated First-In-First-Out (FIFO) buffer
>>   * allowing dynamic batching of sensor data.
>> + * LSM9DSx series is similar but includes an additional magnetometer, handled
>> + * by a different driver.
>>   *
>>   * Supported sensors:
>>   * - LSM6DS3:
>> @@ -30,6 +32,13 @@
>>   *   - Gyroscope supported full-scale [dps]: +-125/+-245/+-500/+-1000/+-2000
>>   *   - FIFO size: 3KB
>>   *
>> + * - LSM9DS1:
>> + *   - Accelerometer supported ODR [Hz]: 10, 50, 119, 238, 476, 952
>> + *   - Accelerometer supported full-scale [g]: +-2/+-4/+-8/+-16
>> + *   - Gyroscope supported ODR [Hz]: 15, 60, 119, 238, 476, 952
>> + *   - Gyroscope supported full-scale [dps]: +-245/+-500/+-2000
>> + *   - FIFO size: 32
>> + *
>>   * Copyright 2016 STMicroelectronics Inc.
>>   *
>>   * Lorenzo Bianconi <lorenzo.bianconi@st.com>
>> @@ -64,6 +73,10 @@
>>  #define ST_LSM6DSX_REG_GYRO_OUT_Y_L_ADDR	0x24
>>  #define ST_LSM6DSX_REG_GYRO_OUT_Z_L_ADDR	0x26
>>  
>> +#define ST_LSM9DSX_REG_GYRO_OUT_X_L_ADDR	0x18
>> +#define ST_LSM9DSX_REG_GYRO_OUT_Y_L_ADDR	0x1a
>> +#define ST_LSM9DSX_REG_GYRO_OUT_Z_L_ADDR	0x1c
>> +
>>  static const struct st_lsm6dsx_odr_table_entry st_lsm6dsx_odr_table[] = {
>>  	[ST_LSM6DSX_ID_ACC] = {
>>  		.reg = {
>> @@ -88,6 +101,30 @@ static const struct st_lsm6dsx_odr_table_entry st_lsm6dsx_odr_table[] = {
>>  		.odr_avl[3] = { 104, 0x04 },
>>  		.odr_avl[4] = { 208, 0x05 },
>>  		.odr_avl[5] = { 416, 0x06 },
>> +	},
>> +	[ST_LSM9DSX_ID_ACC] = {
>> +		.reg = {
>> +			.addr = 0x20,
>> +			.mask = GENMASK(7, 5),
>> +		},
>> +		.odr_avl[0] = {  10, 0x01 },
>> +		.odr_avl[1] = {  50, 0x02 },
>> +		.odr_avl[2] = { 119, 0x03 },
>> +		.odr_avl[3] = { 238, 0x04 },
>> +		.odr_avl[4] = { 476, 0x05 },
>> +		.odr_avl[5] = { 952, 0x06 },
>> +	},
>> +	[ST_LSM9DSX_ID_GYRO] = {
>> +		.reg = {
>> +			.addr = 0x10,
>> +			.mask = GENMASK(7, 5),
>> +		},
>> +		.odr_avl[0] = {  15, 0x01 },
>> +		.odr_avl[1] = {  60, 0x02 },
>> +		.odr_avl[2] = { 119, 0x03 },
>> +		.odr_avl[3] = { 238, 0x04 },
>> +		.odr_avl[4] = { 476, 0x05 },
>> +		.odr_avl[5] = { 952, 0x06 },
>>  	}
>>  };
>>  
>> @@ -111,10 +148,43 @@ static const struct st_lsm6dsx_fs_table_entry st_lsm6dsx_fs_table[] = {
>>  		.fs_avl[1] = { IIO_DEGREE_TO_RAD(17500), 0x1 },
>>  		.fs_avl[2] = { IIO_DEGREE_TO_RAD(35000), 0x2 },
>>  		.fs_avl[3] = { IIO_DEGREE_TO_RAD(70000), 0x3 },
>> +	},
>> +	[ST_LSM9DSX_ID_ACC] = {
>> +		.reg = {
>> +			.addr = 0x20,
>> +			.mask = GENMASK(4, 3),
>> +		},
>> +		.fs_avl[0] = {  599, 0x0 },
>> +		.fs_avl[1] = { 1197, 0x2 },
>> +		.fs_avl[2] = { 2394, 0x3 },
>> +		.fs_avl[3] = { 4788, 0x1 },
>> +	},
>> +	[ST_LSM9DSX_ID_GYRO] = {
>> +		.reg = {
>> +			.addr = 0x10,
>> +			.mask = GENMASK(4, 3),
>> +		},
>> +		.fs_avl[0] = { IIO_DEGREE_TO_RAD(245), 0x0 },
>> +		.fs_avl[1] = { IIO_DEGREE_TO_RAD(500), 0x1 },
>> +		.fs_avl[2] = { IIO_DEGREE_TO_RAD(0), 0x2 },
>> +		.fs_avl[3] = { IIO_DEGREE_TO_RAD(2000), 0x3 },
>>  	}
>>  };
>>  
>>  static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>> +	{
>> +		.wai = 0x68,
>> +		.reg_int1_addr = 0x0c,
>> +		.reg_int2_addr = 0x0d,
>> +		.reg_reset_addr = 0x22,
>> +		.max_fifo_size = 32,
>> +		.id = {
>> +			{
>> +				.hw_id = ST_LSM9DS1_ID,
>> +				.name = ST_LSM9DS1_DEV_NAME,
>> +			},
>> +		},
>> +	},
>>  	{
>>  		.wai = 0x69,
>>  		.reg_int1_addr = 0x0d,
>> @@ -492,6 +562,16 @@ static const struct iio_chan_spec st_lsm6dsx_gyro_channels[] = {
>>  	IIO_CHAN_SOFT_TIMESTAMP(3),
>>  };
>>  
>> +static const struct iio_chan_spec st_lsm9dsx_gyro_channels[] = {
>> +	ST_LSM6DSX_CHANNEL(IIO_ANGL_VEL, ST_LSM9DSX_REG_GYRO_OUT_X_L_ADDR,
>> +			   IIO_MOD_X, 0),
>> +	ST_LSM6DSX_CHANNEL(IIO_ANGL_VEL, ST_LSM9DSX_REG_GYRO_OUT_Y_L_ADDR,
>> +			   IIO_MOD_Y, 1),
>> +	ST_LSM6DSX_CHANNEL(IIO_ANGL_VEL, ST_LSM9DSX_REG_GYRO_OUT_Z_L_ADDR,
>> +			   IIO_MOD_Z, 2),
>> +	IIO_CHAN_SOFT_TIMESTAMP(3),
>> +};
>> +
>>  int st_lsm6dsx_set_page(struct st_lsm6dsx_hw *hw, bool enable)
>>  {
>>  	const struct st_lsm6dsx_shub_settings *hub_settings;
>> @@ -1056,6 +1136,7 @@ static struct iio_dev *st_lsm6dsx_alloc_iiodev(struct st_lsm6dsx_hw *hw,
>>  
>>  	switch (id) {
>>  	case ST_LSM6DSX_ID_ACC:
>> +	case ST_LSM9DSX_ID_ACC:
>>  		iio_dev->channels = st_lsm6dsx_acc_channels;
>>  		iio_dev->num_channels = ARRAY_SIZE(st_lsm6dsx_acc_channels);
>>  		iio_dev->info = &st_lsm6dsx_acc_info;
>> @@ -1068,6 +1149,14 @@ static struct iio_dev *st_lsm6dsx_alloc_iiodev(struct st_lsm6dsx_hw *hw,
>>  		iio_dev->num_channels = ARRAY_SIZE(st_lsm6dsx_gyro_channels);
>>  		iio_dev->info = &st_lsm6dsx_gyro_info;
>>  
>> +		scnprintf(sensor->name, sizeof(sensor->name), "%s_gyro",
>> +			  name);
>> +		break;
>> +	case ST_LSM9DSX_ID_GYRO:
>> +		iio_dev->channels = st_lsm9dsx_gyro_channels;
>> +		iio_dev->num_channels = ARRAY_SIZE(st_lsm9dsx_gyro_channels);
>> +		iio_dev->info = &st_lsm6dsx_gyro_info;
>> +
>>  		scnprintf(sensor->name, sizeof(sensor->name), "%s_gyro",
>>  			  name);
>>  		break;
>> @@ -1109,10 +1198,18 @@ int st_lsm6dsx_probe(struct device *dev, int irq, int hw_id,
>>  	if (err < 0)
>>  		return err;
>>  
>> -	for (i = 0; i < ST_LSM6DSX_ID_EXT0; i++) {
>> -		hw->iio_devs[i] = st_lsm6dsx_alloc_iiodev(hw, i, name);
>> -		if (!hw->iio_devs[i])
>> -			return -ENOMEM;
>> +	if (hw_id == ST_LSM9DS1_ID) {
>> +		for (i = ST_LSM9DSX_ID_GYRO; i <= ST_LSM9DSX_ID_ACC; i++) {
>> +			hw->iio_devs[i] = st_lsm6dsx_alloc_iiodev(hw, i, name);
>> +			if (!hw->iio_devs[i])
>> +				return -ENOMEM;
>> +		}
>> +	} else {
>> +		for (i = 0; i < ST_LSM6DSX_ID_EXT0; i++) {
>> +			hw->iio_devs[i] = st_lsm6dsx_alloc_iiodev(hw, i, name);
>> +			if (!hw->iio_devs[i])
>> +				return -ENOMEM;
>> +		}
>>  	}
>>  
>>  	err = st_lsm6dsx_init_device(hw);
>> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c
>> index b3211e0ac07b..a684e7db1299 100644
>> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c
>> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c
>> @@ -75,6 +75,10 @@ static const struct of_device_id st_lsm6dsx_i2c_of_match[] = {
>>  		.compatible = "st,lsm6dsr",
>>  		.data = (void *)ST_LSM6DSR_ID,
>>  	},
>> +	{
>> +		.compatible = "st,lsm9ds1",
>> +		.data = (void *)ST_LSM9DS1_ID,
>> +	},
>>  	{},
>>  };
>>  MODULE_DEVICE_TABLE(of, st_lsm6dsx_i2c_of_match);
>> @@ -89,6 +93,7 @@ static const struct i2c_device_id st_lsm6dsx_i2c_id_table[] = {
>>  	{ ST_ASM330LHH_DEV_NAME, ST_ASM330LHH_ID },
>>  	{ ST_LSM6DSOX_DEV_NAME, ST_LSM6DSOX_ID },
>>  	{ ST_LSM6DSR_DEV_NAME, ST_LSM6DSR_ID },
>> +	{ ST_LSM9DS1_DEV_NAME, ST_LSM9DS1_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 c9d3c4711018..709769177e91 100644
>> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c
>> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c
>> @@ -75,6 +75,10 @@ static const struct of_device_id st_lsm6dsx_spi_of_match[] = {
>>  		.compatible = "st,lsm6dsr",
>>  		.data = (void *)ST_LSM6DSR_ID,
>>  	},
>> +	{
>> +		.compatible = "st,lsm9ds1",
>> +		.data = (void *)ST_LSM9DS1_ID,
>> +	},
>>  	{},
>>  };
>>  MODULE_DEVICE_TABLE(of, st_lsm6dsx_spi_of_match);
>> @@ -89,6 +93,7 @@ static const struct spi_device_id st_lsm6dsx_spi_id_table[] = {
>>  	{ ST_ASM330LHH_DEV_NAME, ST_ASM330LHH_ID },
>>  	{ ST_LSM6DSOX_DEV_NAME, ST_LSM6DSOX_ID },
>>  	{ ST_LSM6DSR_DEV_NAME, ST_LSM6DSR_ID },
>> +	{ ST_LSM9DS1_DEV_NAME, ST_LSM9DS1_ID },
>>  	{},
>>  };
>>  MODULE_DEVICE_TABLE(spi, st_lsm6dsx_spi_id_table);
>> -- 
>> 2.20.1
>>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] iio: imu: st_lsm6sdx: move some register definitions to sensor_settings struct
  2019-07-15 21:10 ` [PATCH 1/3] iio: imu: st_lsm6sdx: move some register definitions to sensor_settings struct Lorenzo Bianconi
@ 2019-07-16  6:11   ` Martin Kepplinger
  0 siblings, 0 replies; 8+ messages in thread
From: Martin Kepplinger @ 2019-07-16  6:11 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: lorenzo.bianconi83, jic23, knaack.h, lars, pmeerw, linux-iio, devicetree

On 15.07.19 23:10, Lorenzo Bianconi wrote:
>> Move some register definitions to the per-device array of struct
>> st_lsm6dsx_sensor_settings in order to simplify adding new sensor
>> devices to the driver.
>>
>> Also, remove completely unused register definitions.
>>
> 
> Hi Martin,
> 
> just few comments inline
> 
> Regards,
> Lorenzo
> 
>> Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
>> ---
>>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  6 ++++
>>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 31 ++++++++++++++------
>>  2 files changed, 28 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
>> index c14bf533b66b..f072ac14f213 100644
>> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
>> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
>> @@ -196,6 +196,9 @@ struct st_lsm6dsx_ext_dev_settings {
>>  /**
>>   * struct st_lsm6dsx_settings - ST IMU sensor settings
>>   * @wai: Sensor WhoAmI default value.
>> + * @reg_int1_addr: Control Register address for INT1
>> + * @reg_int2_addr: Control Register address for INT2
>> + * @reg_reset_addr: register address for reset/reboot
>>   * @max_fifo_size: Sensor max fifo length in FIFO words.
>>   * @id: List of hw id/device name supported by the driver configuration.
>>   * @decimator: List of decimator register info (addr + mask).
>> @@ -206,6 +209,9 @@ struct st_lsm6dsx_ext_dev_settings {
>>   */
>>  struct st_lsm6dsx_settings {
>>  	u8 wai;
>> +	u8 reg_int1_addr;
>> +	u8 reg_int2_addr;
>> +	u8 reg_reset_addr;
> 
> could you please rename them in int1_addr, int2_addr and reset_addr in order to
> be inline with other st_lsm6dsx_settings fields?
> 

sure. thanks for reviewing!

                             martin


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] iio: imu: st_lsm6sdx: move some register definitions to sensor_settings struct
  2019-07-15 13:19 [PATCH 1/3] iio: imu: st_lsm6sdx: move some register definitions to sensor_settings struct Martin Kepplinger
                   ` (2 preceding siblings ...)
  2019-07-15 21:10 ` [PATCH 1/3] iio: imu: st_lsm6sdx: move some register definitions to sensor_settings struct Lorenzo Bianconi
@ 2019-07-25  5:40 ` Martin Kepplinger
  3 siblings, 0 replies; 8+ messages in thread
From: Martin Kepplinger @ 2019-07-25  5:40 UTC (permalink / raw)
  To: Martin Kepplinger, lorenzo.bianconi83, jic23, knaack.h, lars, pmeerw
  Cc: linux-iio, devicetree, linux-kernel

On 15.07.19 15:19, Martin Kepplinger wrote:
> Move some register definitions to the per-device array of struct
> st_lsm6dsx_sensor_settings in order to simplify adding new sensor
> devices to the driver.
> 
> Also, remove completely unused register definitions.
> 
> Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
> ---
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  6 ++++
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 31 ++++++++++++++------
>  2 files changed, 28 insertions(+), 9 deletions(-)
> 

this series has been resent (and rebased) to be more readable:
https://lore.kernel.org/linux-iio/20190725053132.9589-1-martin.kepplinger@puri.sm/

thanks,
                             martin

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-07-25  5:49 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-15 13:19 [PATCH 1/3] iio: imu: st_lsm6sdx: move some register definitions to sensor_settings struct Martin Kepplinger
2019-07-15 13:19 ` [PATCH 2/3] iio: imu: st_lsm6dsx: add support for accel/gyro unit of lsm9sd1 Martin Kepplinger
2019-07-15 21:49   ` Lorenzo Bianconi
2019-07-16  6:10     ` Martin Kepplinger
2019-07-15 13:19 ` [PATCH 3/3] dt-bindings: iio: imu: st_lsm6dsx: add lsm9ds1 device bindings Martin Kepplinger
2019-07-15 21:10 ` [PATCH 1/3] iio: imu: st_lsm6sdx: move some register definitions to sensor_settings struct Lorenzo Bianconi
2019-07-16  6:11   ` Martin Kepplinger
2019-07-25  5:40 ` Martin Kepplinger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).