Linux-IIO Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v5 1/6] iio: imu: st_lsm6dsx: move interrupt thread to core
@ 2019-09-09  9:45 Sean Nyekjaer
  2019-09-09  9:45 ` [PATCH v5 2/6] iio: imu: st_lsm6dsx: add motion events Sean Nyekjaer
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Sean Nyekjaer @ 2019-09-09  9:45 UTC (permalink / raw)
  To: linux-iio, jic23, lorenzo.bianconi83
  Cc: Sean Nyekjaer, denis.ciocca, mario.tesi, armando.visconti,
	martin, Lorenzo Bianconi

This prepares the interrupt to be used for other stuff than
fifo reading + event readings.

Signed-off-by: Sean Nyekjaer <sean@geanix.com>
Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    | 78 +---------------
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c  | 88 +++++++++++++++++++
 2 files changed, 89 insertions(+), 77 deletions(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
index b0f3da1976e4..ef579650fd52 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
@@ -30,8 +30,6 @@
  * Denis Ciocca <denis.ciocca@st.com>
  */
 #include <linux/module.h>
-#include <linux/interrupt.h>
-#include <linux/irq.h>
 #include <linux/iio/kfifo_buf.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/buffer.h>
@@ -42,10 +40,6 @@
 
 #include "st_lsm6dsx.h"
 
-#define ST_LSM6DSX_REG_HLACTIVE_ADDR		0x12
-#define ST_LSM6DSX_REG_HLACTIVE_MASK		BIT(5)
-#define ST_LSM6DSX_REG_PP_OD_ADDR		0x12
-#define ST_LSM6DSX_REG_PP_OD_MASK		BIT(4)
 #define ST_LSM6DSX_REG_FIFO_MODE_ADDR		0x0a
 #define ST_LSM6DSX_FIFO_MODE_MASK		GENMASK(2, 0)
 #define ST_LSM6DSX_FIFO_ODR_MASK		GENMASK(6, 3)
@@ -654,25 +648,6 @@ int st_lsm6dsx_update_fifo(struct st_lsm6dsx_sensor *sensor, bool enable)
 	return err;
 }
 
-static irqreturn_t st_lsm6dsx_handler_irq(int irq, void *private)
-{
-	struct st_lsm6dsx_hw *hw = private;
-
-	return hw->sip > 0 ? IRQ_WAKE_THREAD : IRQ_NONE;
-}
-
-static irqreturn_t st_lsm6dsx_handler_thread(int irq, void *private)
-{
-	struct st_lsm6dsx_hw *hw = private;
-	int count;
-
-	mutex_lock(&hw->fifo_lock);
-	count = hw->settings->fifo_ops.read_fifo(hw);
-	mutex_unlock(&hw->fifo_lock);
-
-	return count ? IRQ_HANDLED : IRQ_NONE;
-}
-
 static int st_lsm6dsx_buffer_preenable(struct iio_dev *iio_dev)
 {
 	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
@@ -702,59 +677,8 @@ static const struct iio_buffer_setup_ops st_lsm6dsx_buffer_ops = {
 
 int st_lsm6dsx_fifo_setup(struct st_lsm6dsx_hw *hw)
 {
-	struct device_node *np = hw->dev->of_node;
-	struct st_sensors_platform_data *pdata;
 	struct iio_buffer *buffer;
-	unsigned long irq_type;
-	bool irq_active_low;
-	int i, err;
-
-	irq_type = irqd_get_trigger_type(irq_get_irq_data(hw->irq));
-
-	switch (irq_type) {
-	case IRQF_TRIGGER_HIGH:
-	case IRQF_TRIGGER_RISING:
-		irq_active_low = false;
-		break;
-	case IRQF_TRIGGER_LOW:
-	case IRQF_TRIGGER_FALLING:
-		irq_active_low = true;
-		break;
-	default:
-		dev_info(hw->dev, "mode %lx unsupported\n", irq_type);
-		return -EINVAL;
-	}
-
-	err = regmap_update_bits(hw->regmap, ST_LSM6DSX_REG_HLACTIVE_ADDR,
-				 ST_LSM6DSX_REG_HLACTIVE_MASK,
-				 FIELD_PREP(ST_LSM6DSX_REG_HLACTIVE_MASK,
-					    irq_active_low));
-	if (err < 0)
-		return err;
-
-	pdata = (struct st_sensors_platform_data *)hw->dev->platform_data;
-	if ((np && of_property_read_bool(np, "drive-open-drain")) ||
-	    (pdata && pdata->open_drain)) {
-		err = regmap_update_bits(hw->regmap, ST_LSM6DSX_REG_PP_OD_ADDR,
-					 ST_LSM6DSX_REG_PP_OD_MASK,
-					 FIELD_PREP(ST_LSM6DSX_REG_PP_OD_MASK,
-						    1));
-		if (err < 0)
-			return err;
-
-		irq_type |= IRQF_SHARED;
-	}
-
-	err = devm_request_threaded_irq(hw->dev, hw->irq,
-					st_lsm6dsx_handler_irq,
-					st_lsm6dsx_handler_thread,
-					irq_type | IRQF_ONESHOT,
-					"lsm6dsx", hw);
-	if (err) {
-		dev_err(hw->dev, "failed to request trigger irq %d\n",
-			hw->irq);
-		return err;
-	}
+	int i;
 
 	for (i = 0; i < ST_LSM6DSX_ID_MAX; i++) {
 		if (!hw->iio_devs[i])
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index 2d3495560136..d0bcbbfb6297 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -50,6 +50,8 @@
 #include <linux/delay.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
 #include <linux/pm.h>
 #include <linux/regmap.h>
 #include <linux/bitfield.h>
@@ -65,6 +67,11 @@
 #define ST_LSM6DSX_REG_BDU_ADDR			0x12
 #define ST_LSM6DSX_REG_BDU_MASK			BIT(6)
 
+#define ST_LSM6DSX_REG_HLACTIVE_ADDR		0x12
+#define ST_LSM6DSX_REG_HLACTIVE_MASK		BIT(5)
+#define ST_LSM6DSX_REG_PP_OD_ADDR		0x12
+#define ST_LSM6DSX_REG_PP_OD_MASK		BIT(4)
+
 static const struct iio_chan_spec st_lsm6dsx_acc_channels[] = {
 	ST_LSM6DSX_CHANNEL(IIO_ACCEL, 0x28, IIO_MOD_X, 0),
 	ST_LSM6DSX_CHANNEL(IIO_ACCEL, 0x2a, IIO_MOD_Y, 1),
@@ -1466,6 +1473,83 @@ static struct iio_dev *st_lsm6dsx_alloc_iiodev(struct st_lsm6dsx_hw *hw,
 	return iio_dev;
 }
 
+static irqreturn_t st_lsm6dsx_handler_irq(int irq, void *private)
+{
+	struct st_lsm6dsx_hw *hw = private;
+
+	return hw->sip > 0 ? IRQ_WAKE_THREAD : IRQ_NONE;
+}
+
+static irqreturn_t st_lsm6dsx_handler_thread(int irq, void *private)
+{
+	struct st_lsm6dsx_hw *hw = private;
+	int count;
+
+	mutex_lock(&hw->fifo_lock);
+	count = hw->settings->fifo_ops.read_fifo(hw);
+	mutex_unlock(&hw->fifo_lock);
+
+	return count ? IRQ_HANDLED : IRQ_NONE;
+}
+
+static int st_lsm6dsx_irq_setup(struct st_lsm6dsx_hw *hw)
+{
+	struct st_sensors_platform_data *pdata;
+	struct device_node *np = hw->dev->of_node;
+	unsigned long irq_type;
+	bool irq_active_low;
+	int err;
+
+	irq_type = irqd_get_trigger_type(irq_get_irq_data(hw->irq));
+
+	switch (irq_type) {
+	case IRQF_TRIGGER_HIGH:
+	case IRQF_TRIGGER_RISING:
+		irq_active_low = false;
+		break;
+	case IRQF_TRIGGER_LOW:
+	case IRQF_TRIGGER_FALLING:
+		irq_active_low = true;
+		break;
+	default:
+		dev_info(hw->dev, "mode %lx unsupported\n", irq_type);
+		return -EINVAL;
+	}
+
+	err = regmap_update_bits(hw->regmap, ST_LSM6DSX_REG_HLACTIVE_ADDR,
+				 ST_LSM6DSX_REG_HLACTIVE_MASK,
+				 FIELD_PREP(ST_LSM6DSX_REG_HLACTIVE_MASK,
+					    irq_active_low));
+	if (err < 0)
+		return err;
+
+	pdata = (struct st_sensors_platform_data *)hw->dev->platform_data;
+	if ((np && of_property_read_bool(np, "drive-open-drain")) ||
+	    (pdata && pdata->open_drain)) {
+		err = regmap_update_bits(hw->regmap, ST_LSM6DSX_REG_PP_OD_ADDR,
+					 ST_LSM6DSX_REG_PP_OD_MASK,
+					 FIELD_PREP(ST_LSM6DSX_REG_PP_OD_MASK,
+						    1));
+		if (err < 0)
+			return err;
+
+		irq_type |= IRQF_SHARED;
+	}
+
+	err = devm_request_threaded_irq(hw->dev, hw->irq,
+					st_lsm6dsx_handler_irq,
+					st_lsm6dsx_handler_thread,
+					irq_type | IRQF_ONESHOT,
+					"lsm6dsx", hw);
+	if (err) {
+		dev_err(hw->dev, "failed to request trigger irq %d\n",
+			hw->irq);
+		return err;
+	}
+
+	return 0;
+}
+
 int st_lsm6dsx_probe(struct device *dev, int irq, int hw_id,
 		     struct regmap *regmap)
 {
@@ -1514,6 +1598,10 @@ int st_lsm6dsx_probe(struct device *dev, int irq, int hw_id,
 	}
 
 	if (hw->irq > 0) {
+		err = st_lsm6dsx_irq_setup(hw);
+		if (err < 0)
+			return err;
+
 		err = st_lsm6dsx_fifo_setup(hw);
 		if (err < 0)
 			return err;
-- 
2.23.0


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

* [PATCH v5 2/6] iio: imu: st_lsm6dsx: add motion events
  2019-09-09  9:45 [PATCH v5 1/6] iio: imu: st_lsm6dsx: move interrupt thread to core Sean Nyekjaer
@ 2019-09-09  9:45 ` Sean Nyekjaer
  2019-09-09 10:21   ` Lorenzo Bianconi
  2019-09-09 11:13   ` Sean Nyekjaer
  2019-09-09  9:45 ` [PATCH v5 3/6] iio: imu: st_lsm6dsx: add wakeup-source option Sean Nyekjaer
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 12+ messages in thread
From: Sean Nyekjaer @ 2019-09-09  9:45 UTC (permalink / raw)
  To: linux-iio, jic23, lorenzo.bianconi83
  Cc: Sean Nyekjaer, denis.ciocca, mario.tesi, armando.visconti, martin

Add event channels that controls the creation of motion events.
Tested on ISM330DLC

Signed-off-by: Sean Nyekjaer <sean@geanix.com>
---
Changes since v4:
 * Added check for event support
 * Added registers for more devices that support this event

 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  41 ++++
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 214 ++++++++++++++++++-
 2 files changed, 251 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
index 5e3cd96b0059..d04473861fba 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
@@ -12,6 +12,7 @@
 #define ST_LSM6DSX_H
 
 #include <linux/device.h>
+#include <linux/iio/iio.h>
 
 #define ST_LSM6DS3_DEV_NAME	"lsm6ds3"
 #define ST_LSM6DS3H_DEV_NAME	"lsm6ds3h"
@@ -54,6 +55,26 @@ enum st_lsm6dsx_hw_id {
 					 * ST_LSM6DSX_TAGGED_SAMPLE_SIZE)
 #define ST_LSM6DSX_SHIFT_VAL(val, mask)	(((val) << __ffs(mask)) & (mask))
 
+#define ST_LSM6DSX_CHANNEL_ACC(chan_type, addr, mod, scan_idx)		\
+{									\
+	.type = chan_type,						\
+	.address = addr,						\
+	.modified = 1,							\
+	.channel2 = mod,						\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),		\
+	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
+	.scan_index = scan_idx,						\
+	.scan_type = {							\
+		.sign = 's',						\
+		.realbits = 16,						\
+		.storagebits = 16,					\
+		.endianness = IIO_LE,					\
+	},								\
+	.event_spec = &st_lsm6dsx_event,				\
+	.num_event_specs = 1,						\
+}
+
 #define ST_LSM6DSX_CHANNEL(chan_type, addr, mod, scan_idx)		\
 {									\
 	.type = chan_type,						\
@@ -162,6 +183,11 @@ struct st_lsm6dsx_shub_settings {
 	u8 batch_en;
 };
 
+struct st_lsm6dsx_event_settings {
+	struct st_lsm6dsx_reg enable_reg;
+	struct st_lsm6dsx_reg wakeup_reg;
+};
+
 enum st_lsm6dsx_ext_sensor_id {
 	ST_LSM6DSX_ID_MAGN,
 };
@@ -223,6 +249,9 @@ struct st_lsm6dsx_settings {
 	u8 wai;
 	u8 int1_addr;
 	u8 int2_addr;
+	u8 int1_func_addr;
+	u8 int2_func_addr;
+	u8 int_func_mask;
 	u8 reset_addr;
 	u16 max_fifo_size;
 	struct {
@@ -240,6 +269,7 @@ struct st_lsm6dsx_settings {
 	struct st_lsm6dsx_fifo_ops fifo_ops;
 	struct st_lsm6dsx_hw_ts_settings ts_settings;
 	struct st_lsm6dsx_shub_settings shub_settings;
+	struct st_lsm6dsx_event_settings event_settings;
 };
 
 enum st_lsm6dsx_sensor_id {
@@ -320,6 +350,10 @@ struct st_lsm6dsx_hw {
 	u8 ts_sip;
 	u8 sip;
 
+	u8 event_threshold;
+	bool enable_event;
+	struct st_lsm6dsx_reg irq_routing;
+
 	u8 *buff;
 
 	struct iio_dev *iio_devs[ST_LSM6DSX_ID_MAX];
@@ -327,6 +361,13 @@ struct st_lsm6dsx_hw {
 	const struct st_lsm6dsx_settings *settings;
 };
 
+static const struct iio_event_spec st_lsm6dsx_event = {
+	.type = IIO_EV_TYPE_THRESH,
+	.dir = IIO_EV_DIR_EITHER,
+	.mask_separate = BIT(IIO_EV_INFO_VALUE) |
+			 BIT(IIO_EV_INFO_ENABLE)
+};
+
 static const unsigned long st_lsm6dsx_available_scan_masks[] = {0x7, 0x0};
 extern const struct dev_pm_ops st_lsm6dsx_pm_ops;
 
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index d0bcbbfb6297..fb83076d8f17 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -73,9 +73,9 @@
 #define ST_LSM6DSX_REG_PP_OD_MASK		BIT(4)
 
 static const struct iio_chan_spec st_lsm6dsx_acc_channels[] = {
-	ST_LSM6DSX_CHANNEL(IIO_ACCEL, 0x28, IIO_MOD_X, 0),
-	ST_LSM6DSX_CHANNEL(IIO_ACCEL, 0x2a, IIO_MOD_Y, 1),
-	ST_LSM6DSX_CHANNEL(IIO_ACCEL, 0x2c, IIO_MOD_Z, 2),
+	ST_LSM6DSX_CHANNEL_ACC(IIO_ACCEL, 0x28, IIO_MOD_X, 0),
+	ST_LSM6DSX_CHANNEL_ACC(IIO_ACCEL, 0x2a, IIO_MOD_Y, 1),
+	ST_LSM6DSX_CHANNEL_ACC(IIO_ACCEL, 0x2c, IIO_MOD_Z, 2),
 	IIO_CHAN_SOFT_TIMESTAMP(3),
 };
 
@@ -168,6 +168,9 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 		.wai = 0x69,
 		.int1_addr = 0x0d,
 		.int2_addr = 0x0e,
+		.int1_func_addr = 0x5e,
+		.int2_func_addr = 0x5f,
+		.int_func_mask = BIT(5),
 		.reset_addr = 0x12,
 		.max_fifo_size = 1365,
 		.id = {
@@ -275,11 +278,25 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.mask = GENMASK(5, 3),
 			},
 		},
+		.event_settings = {
+			.wakeup_reg = {
+				.addr = 0x5B,
+				.mask = GENMASK(5, 0),
+			},
+			.wakeup_src_reg = 0x1b,
+			.wakeup_src_status_mask = BIT(3),
+			.wakeup_src_z_mask = BIT(0),
+			.wakeup_src_y_mask = BIT(1),
+			.wakeup_src_x_mask = BIT(2),
+		},
 	},
 	{
 		.wai = 0x69,
 		.int1_addr = 0x0d,
 		.int2_addr = 0x0e,
+		.int1_func_addr = 0x5e,
+		.int2_func_addr = 0x5f,
+		.int_func_mask = BIT(5),
 		.reset_addr = 0x12,
 		.max_fifo_size = 682,
 		.id = {
@@ -387,11 +404,25 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.mask = GENMASK(5, 3),
 			},
 		},
+		.event_settings = {
+			.wakeup_reg = {
+				.addr = 0x5B,
+				.mask = GENMASK(5, 0),
+			},
+			.wakeup_src_reg = 0x1b,
+			.wakeup_src_status_mask = BIT(3),
+			.wakeup_src_z_mask = BIT(0),
+			.wakeup_src_y_mask = BIT(1),
+			.wakeup_src_x_mask = BIT(2),
+		},
 	},
 	{
 		.wai = 0x6a,
 		.int1_addr = 0x0d,
 		.int2_addr = 0x0e,
+		.int1_func_addr = 0x5e,
+		.int2_func_addr = 0x5f,
+		.int_func_mask = BIT(5),
 		.reset_addr = 0x12,
 		.max_fifo_size = 682,
 		.id = {
@@ -508,6 +539,16 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.mask = GENMASK(5, 3),
 			},
 		},
+		.event_settings = {
+			.enable_reg = {
+				.addr = 0x58,
+				.mask = BIT(7),
+			},
+			.wakeup_reg = {
+				.addr = 0x5B,
+				.mask = GENMASK(5, 0),
+			},
+		},
 	},
 	{
 		.wai = 0x6c,
@@ -646,6 +687,9 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 		.wai = 0x6b,
 		.int1_addr = 0x0d,
 		.int2_addr = 0x0e,
+		.int1_func_addr = 0x5e,
+		.int2_func_addr = 0x5f,
+		.int_func_mask = BIT(5),
 		.reset_addr = 0x12,
 		.max_fifo_size = 512,
 		.id = {
@@ -745,11 +789,29 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.mask = GENMASK(7, 6),
 			},
 		},
+		.event_settings = {
+			.enable_reg = {
+				.addr = 0x58,
+				.mask = BIT(7),
+			},
+			.wakeup_reg = {
+				.addr = 0x5B,
+				.mask = GENMASK(5, 0),
+			},
+			.wakeup_src_reg = 0x1b,
+			.wakeup_src_status_mask = BIT(3),
+			.wakeup_src_z_mask = BIT(0),
+			.wakeup_src_y_mask = BIT(1),
+			.wakeup_src_x_mask = BIT(2),
+		},
 	},
 	{
 		.wai = 0x6b,
 		.int1_addr = 0x0d,
 		.int2_addr = 0x0e,
+		.int1_func_addr = 0x5e,
+		.int2_func_addr = 0x5f,
+		.int_func_mask = BIT(5),
 		.reset_addr = 0x12,
 		.max_fifo_size = 512,
 		.id = {
@@ -877,6 +939,21 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 			.slv0_addr = 0x15,
 			.dw_slv0_addr = 0x21,
 			.batch_en = BIT(3),
+		},
+		.event_settings = {
+			.enable_reg = {
+				.addr = 0x58,
+				.mask = BIT(7),
+			},
+			.wakeup_reg = {
+				.addr = 0x5B,
+				.mask = GENMASK(5, 0),
+			},
+			.wakeup_src_reg = 0x1b,
+			.wakeup_src_status_mask = BIT(3),
+			.wakeup_src_z_mask = BIT(0),
+			.wakeup_src_y_mask = BIT(1),
+			.wakeup_src_x_mask = BIT(2),
 		}
 	},
 };
@@ -1083,7 +1160,8 @@ static int st_lsm6dsx_read_oneshot(struct st_lsm6dsx_sensor *sensor,
 	if (err < 0)
 		return err;
 
-	st_lsm6dsx_sensor_set_enable(sensor, false);
+	if (!hw->enable_event)
+		st_lsm6dsx_sensor_set_enable(sensor, false);
 
 	*val = (s16)le16_to_cpu(data);
 
@@ -1156,6 +1234,126 @@ static int st_lsm6dsx_write_raw(struct iio_dev *iio_dev,
 	return err;
 }
 
+int st_lsm6dsx_event_setup(struct st_lsm6dsx_hw *hw, int state)
+{
+	int err;
+	u8 enable = 0;
+
+	if (!hw->settings->int1_func_addr)
+		return -ENOTSUPP;
+
+	enable = state ? hw->settings->event_settings.enable_reg.mask : 0;
+
+	err = regmap_update_bits(hw->regmap,
+				 hw->settings->event_settings.enable_reg.addr,
+				 hw->settings->event_settings.enable_reg.mask,
+				 enable);
+	if (err < 0)
+		return err;
+
+	enable = state ? hw->irq_routing.mask : 0;
+
+	/* Enable wakeup interrupt */
+	return regmap_update_bits(hw->regmap, hw->irq_routing.addr,
+				  hw->irq_routing.mask,
+				  enable);
+}
+
+static int st_lsm6dsx_read_event(struct iio_dev *iio_dev,
+				   const struct iio_chan_spec *chan,
+				   enum iio_event_type type,
+				   enum iio_event_direction dir,
+				   enum iio_event_info info,
+				   int *val, int *val2)
+{
+	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
+	struct st_lsm6dsx_hw *hw = sensor->hw;
+
+	if (type != IIO_EV_TYPE_THRESH)
+		return -EINVAL;
+
+	*val2 = 0;
+	*val = hw->event_threshold;
+
+	return IIO_VAL_INT;
+}
+
+static int st_lsm6dsx_write_event(struct iio_dev *iio_dev,
+				    const struct iio_chan_spec *chan,
+				    enum iio_event_type type,
+				    enum iio_event_direction dir,
+				    enum iio_event_info info,
+				    int val, int val2)
+{
+	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
+	struct st_lsm6dsx_hw *hw = sensor->hw;
+	int err;
+
+	if (type != IIO_EV_TYPE_THRESH)
+		return -EINVAL;
+
+	if (val < 0 || val > 31)
+		return -EINVAL;
+
+	err = regmap_update_bits(hw->regmap,
+				 hw->settings->event_settings.wakeup_reg.addr,
+				 hw->settings->event_settings.wakeup_reg.mask,
+				 val);
+	if (err)
+		return -EINVAL;
+
+	hw->event_threshold = val;
+
+	return 0;
+}
+
+static int st_lsm6dsx_read_event_config(struct iio_dev *iio_dev,
+					  const struct iio_chan_spec *chan,
+					  enum iio_event_type type,
+					  enum iio_event_direction dir)
+{
+	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
+	struct st_lsm6dsx_hw *hw = sensor->hw;
+
+	if (type != IIO_EV_TYPE_THRESH)
+		return -EINVAL;
+
+	return hw->enable_event;
+}
+
+static int st_lsm6dsx_write_event_config(struct iio_dev *iio_dev,
+					   const struct iio_chan_spec *chan,
+					   enum iio_event_type type,
+					   enum iio_event_direction dir,
+					   int state)
+{
+	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
+	struct st_lsm6dsx_hw *hw = sensor->hw;
+	int err = 0;
+
+	if (type != IIO_EV_TYPE_THRESH)
+		return -EINVAL;
+
+	if (hw->fifo_mode != ST_LSM6DSX_FIFO_BYPASS)
+		return -EBUSY;
+
+	/* do not enable events if they are already enabled */
+	if (state && hw->enable_event)
+		return 0;
+
+	err = st_lsm6dsx_event_setup(hw, state);
+	if (err < 0)
+		return err;
+
+	err = st_lsm6dsx_sensor_set_enable(sensor, state);
+	if (err < 0)
+		return err;
+
+	hw->enable_event = state;
+
+	return 0;
+}
+
 int st_lsm6dsx_set_watermark(struct iio_dev *iio_dev, unsigned int val)
 {
 	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
@@ -1240,6 +1438,10 @@ static const struct iio_info st_lsm6dsx_acc_info = {
 	.attrs = &st_lsm6dsx_acc_attribute_group,
 	.read_raw = st_lsm6dsx_read_raw,
 	.write_raw = st_lsm6dsx_write_raw,
+	.read_event_value = st_lsm6dsx_read_event,
+	.write_event_value = st_lsm6dsx_write_event,
+	.read_event_config = st_lsm6dsx_read_event_config,
+	.write_event_config = st_lsm6dsx_write_event_config,
 	.hwfifo_set_watermark = st_lsm6dsx_set_watermark,
 };
 
@@ -1285,9 +1487,13 @@ static int st_lsm6dsx_get_drdy_reg(struct st_lsm6dsx_hw *hw, u8 *drdy_reg)
 	switch (drdy_pin) {
 	case 1:
 		*drdy_reg = hw->settings->int1_addr;
+		hw->irq_routing.addr = hw->settings->int1_func_addr;
+		hw->irq_routing.mask = hw->settings->int_func_mask;
 		break;
 	case 2:
 		*drdy_reg = hw->settings->int2_addr;
+		hw->irq_routing.addr = hw->settings->int2_func_addr;
+		hw->irq_routing.mask = hw->settings->int_func_mask;
 		break;
 	default:
 		dev_err(hw->dev, "unsupported data ready pin\n");
-- 
2.23.0


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

* [PATCH v5 3/6] iio: imu: st_lsm6dsx: add wakeup-source option
  2019-09-09  9:45 [PATCH v5 1/6] iio: imu: st_lsm6dsx: move interrupt thread to core Sean Nyekjaer
  2019-09-09  9:45 ` [PATCH v5 2/6] iio: imu: st_lsm6dsx: add motion events Sean Nyekjaer
@ 2019-09-09  9:45 ` Sean Nyekjaer
  2019-09-09  9:45 ` [PATCH v5 4/6] iio: imu: st_lsm6dsx: always enter interrupt thread Sean Nyekjaer
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Sean Nyekjaer @ 2019-09-09  9:45 UTC (permalink / raw)
  To: linux-iio, jic23, lorenzo.bianconi83
  Cc: Sean Nyekjaer, denis.ciocca, mario.tesi, armando.visconti, martin

This add ways for the SoC to wake from accelerometer wake events.

In the suspend function we skip disabling the sensor if wakeup-source
and events are activated.

Signed-off-by: Sean Nyekjaer <sean@geanix.com>
---
Changes since v4:
 * More devices supports wakeup

 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index fb83076d8f17..68df2a8d628b 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -1822,6 +1822,9 @@ int st_lsm6dsx_probe(struct device *dev, int irq, int hw_id,
 			return err;
 	}
 
+	if (dev->of_node && of_property_read_bool(dev->of_node, "wakeup-source"))
+		device_init_wakeup(dev, true);
+
 	return 0;
 }
 EXPORT_SYMBOL(st_lsm6dsx_probe);
@@ -1840,6 +1843,12 @@ static int __maybe_unused st_lsm6dsx_suspend(struct device *dev)
 		if (!(hw->enable_mask & BIT(sensor->id)))
 			continue;
 
+		if (device_may_wakeup(dev)) {
+			/* Enable wake from IRQ */
+			enable_irq_wake(hw->irq);
+			continue;
+		}
+
 		if (sensor->id == ST_LSM6DSX_ID_EXT0 ||
 		    sensor->id == ST_LSM6DSX_ID_EXT1 ||
 		    sensor->id == ST_LSM6DSX_ID_EXT2)
@@ -1872,6 +1881,11 @@ static int __maybe_unused st_lsm6dsx_resume(struct device *dev)
 		if (!(hw->suspend_mask & BIT(sensor->id)))
 			continue;
 
+		if (device_may_wakeup(dev)) {
+			disable_irq_wake(hw->irq);
+			continue;
+		}
+
 		if (sensor->id == ST_LSM6DSX_ID_EXT0 ||
 		    sensor->id == ST_LSM6DSX_ID_EXT1 ||
 		    sensor->id == ST_LSM6DSX_ID_EXT2)
-- 
2.23.0


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

* [PATCH v5 4/6] iio: imu: st_lsm6dsx: always enter interrupt thread
  2019-09-09  9:45 [PATCH v5 1/6] iio: imu: st_lsm6dsx: move interrupt thread to core Sean Nyekjaer
  2019-09-09  9:45 ` [PATCH v5 2/6] iio: imu: st_lsm6dsx: add motion events Sean Nyekjaer
  2019-09-09  9:45 ` [PATCH v5 3/6] iio: imu: st_lsm6dsx: add wakeup-source option Sean Nyekjaer
@ 2019-09-09  9:45 ` Sean Nyekjaer
  2019-09-09  9:45 ` [PATCH v5 5/6] iio: imu: st_lsm6dsx: add motion report function and call from interrupt Sean Nyekjaer
  2019-09-09  9:45 ` [PATCH v5 6/6] iio: imu: st_lsm6dsx: prohibit the use of events and buffered reads simultaneously Sean Nyekjaer
  4 siblings, 0 replies; 12+ messages in thread
From: Sean Nyekjaer @ 2019-09-09  9:45 UTC (permalink / raw)
  To: linux-iio, jic23, lorenzo.bianconi83
  Cc: Sean Nyekjaer, denis.ciocca, mario.tesi, armando.visconti, martin

The interrupt source can come from multiple sources,
fifo and wake interrupts.
Enter interrupt thread to check which interrupt that has fired.

Signed-off-by: Sean Nyekjaer <sean@geanix.com>
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index 68df2a8d628b..7ab31206104e 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -1681,9 +1681,7 @@ static struct iio_dev *st_lsm6dsx_alloc_iiodev(struct st_lsm6dsx_hw *hw,
 
 static irqreturn_t st_lsm6dsx_handler_irq(int irq, void *private)
 {
-	struct st_lsm6dsx_hw *hw = private;
-
-	return hw->sip > 0 ? IRQ_WAKE_THREAD : IRQ_NONE;
+	return IRQ_WAKE_THREAD;
 }
 
 static irqreturn_t st_lsm6dsx_handler_thread(int irq, void *private)
-- 
2.23.0


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

* [PATCH v5 5/6] iio: imu: st_lsm6dsx: add motion report function and call from interrupt
  2019-09-09  9:45 [PATCH v5 1/6] iio: imu: st_lsm6dsx: move interrupt thread to core Sean Nyekjaer
                   ` (2 preceding siblings ...)
  2019-09-09  9:45 ` [PATCH v5 4/6] iio: imu: st_lsm6dsx: always enter interrupt thread Sean Nyekjaer
@ 2019-09-09  9:45 ` Sean Nyekjaer
  2019-09-09  9:45 ` [PATCH v5 6/6] iio: imu: st_lsm6dsx: prohibit the use of events and buffered reads simultaneously Sean Nyekjaer
  4 siblings, 0 replies; 12+ messages in thread
From: Sean Nyekjaer @ 2019-09-09  9:45 UTC (permalink / raw)
  To: linux-iio, jic23, lorenzo.bianconi83
  Cc: Sean Nyekjaer, denis.ciocca, mario.tesi, armando.visconti, martin

Report iio motion events to iio subsystem

Signed-off-by: Sean Nyekjaer <sean@geanix.com>
---
Changes since v4:
 * Updated bitmask as pr Jonathans comments

 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  5 ++
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 49 ++++++++++++++++++++
 2 files changed, 54 insertions(+)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
index d04473861fba..015b837f366f 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
@@ -186,6 +186,11 @@ struct st_lsm6dsx_shub_settings {
 struct st_lsm6dsx_event_settings {
 	struct st_lsm6dsx_reg enable_reg;
 	struct st_lsm6dsx_reg wakeup_reg;
+	u8 wakeup_src_reg;
+	u8 wakeup_src_status_mask;
+	u8 wakeup_src_z_mask;
+	u8 wakeup_src_y_mask;
+	u8 wakeup_src_x_mask;
 };
 
 enum st_lsm6dsx_ext_sensor_id {
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index 7ab31206104e..00ba14d15c13 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -48,6 +48,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/delay.h>
+#include <linux/iio/events.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 #include <linux/interrupt.h>
@@ -548,6 +549,11 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.addr = 0x5B,
 				.mask = GENMASK(5, 0),
 			},
+			.wakeup_src_reg = 0x1b,
+			.wakeup_src_status_mask = BIT(3),
+			.wakeup_src_z_mask = BIT(0),
+			.wakeup_src_y_mask = BIT(1),
+			.wakeup_src_x_mask = BIT(2),
 		},
 	},
 	{
@@ -1679,6 +1685,38 @@ static struct iio_dev *st_lsm6dsx_alloc_iiodev(struct st_lsm6dsx_hw *hw,
 	return iio_dev;
 }
 
+void st_lsm6dsx_report_motion_event(struct st_lsm6dsx_hw *hw, int data)
+{
+	s64 timestamp = iio_get_time_ns(hw->iio_devs[ST_LSM6DSX_ID_ACC]);
+
+	if (data & hw->settings->event_settings.wakeup_src_z_mask)
+		iio_push_event(hw->iio_devs[ST_LSM6DSX_ID_ACC],
+			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
+						  0,
+						  IIO_MOD_Z,
+						  IIO_EV_TYPE_THRESH,
+						  IIO_EV_DIR_EITHER),
+						  timestamp);
+
+	if (data & hw->settings->event_settings.wakeup_src_x_mask)
+		iio_push_event(hw->iio_devs[ST_LSM6DSX_ID_ACC],
+			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
+						  0,
+						  IIO_MOD_Y,
+						  IIO_EV_TYPE_THRESH,
+						  IIO_EV_DIR_EITHER),
+						  timestamp);
+
+	if (data & hw->settings->event_settings.wakeup_src_x_mask)
+		iio_push_event(hw->iio_devs[ST_LSM6DSX_ID_ACC],
+			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
+						  0,
+						  IIO_MOD_X,
+						  IIO_EV_TYPE_THRESH,
+						  IIO_EV_DIR_EITHER),
+						  timestamp);
+}
+
 static irqreturn_t st_lsm6dsx_handler_irq(int irq, void *private)
 {
 	return IRQ_WAKE_THREAD;
@@ -1688,6 +1726,17 @@ static irqreturn_t st_lsm6dsx_handler_thread(int irq, void *private)
 {
 	struct st_lsm6dsx_hw *hw = private;
 	int count;
+	int data, err;
+
+	if (hw->enable_event) {
+		err = regmap_read(hw->regmap,
+				  hw->settings->event_settings.wakeup_src_reg, &data);
+		if (err < 0)
+			return IRQ_NONE;
+
+		if (data & hw->settings->event_settings.wakeup_src_status_mask)
+			st_lsm6dsx_report_motion_event(hw, data);
+	}
 
 	mutex_lock(&hw->fifo_lock);
 	count = hw->settings->fifo_ops.read_fifo(hw);
-- 
2.23.0


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

* [PATCH v5 6/6] iio: imu: st_lsm6dsx: prohibit the use of events and buffered reads simultaneously
  2019-09-09  9:45 [PATCH v5 1/6] iio: imu: st_lsm6dsx: move interrupt thread to core Sean Nyekjaer
                   ` (3 preceding siblings ...)
  2019-09-09  9:45 ` [PATCH v5 5/6] iio: imu: st_lsm6dsx: add motion report function and call from interrupt Sean Nyekjaer
@ 2019-09-09  9:45 ` Sean Nyekjaer
  2019-09-09  9:56   ` [PATCH v5.1 " Sean Nyekjaer
  4 siblings, 1 reply; 12+ messages in thread
From: Sean Nyekjaer @ 2019-09-09  9:45 UTC (permalink / raw)
  To: linux-iio, jic23, lorenzo.bianconi83
  Cc: Sean Nyekjaer, denis.ciocca, mario.tesi, armando.visconti, martin

When events and buffered reads is enabled simultaneously, and the first
event accours the interrupt pin stays high.

This can be reverted when we find a solution to allow events and
buffered reads simultaneously.

Signed-off-by: Sean Nyekjaer <sean@geanix.com>
---
Changes since v4:
 * Use fifo configuration mutex to prevent a race in hw->enable_event
   check.

 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 3 +++
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c   | 4 ++++
 2 files changed, 7 insertions(+)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
index ef579650fd52..f9f27a42e49a 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
@@ -603,6 +603,9 @@ int st_lsm6dsx_update_fifo(struct st_lsm6dsx_sensor *sensor, bool enable)
 
 	mutex_lock(&hw->conf_lock);
 
+	if (hw->enable_event)
+		return -EBUSY;
+
 	if (hw->fifo_mode != ST_LSM6DSX_FIFO_BYPASS) {
 		err = st_lsm6dsx_flush_fifo(hw);
 		if (err < 0)
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index 00ba14d15c13..b4df15a8558e 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -1340,6 +1340,8 @@ static int st_lsm6dsx_write_event_config(struct iio_dev *iio_dev,
 	if (type != IIO_EV_TYPE_THRESH)
 		return -EINVAL;
 
+	mutex_unlock(&hw->conf_lock);
+
 	if (hw->fifo_mode != ST_LSM6DSX_FIFO_BYPASS)
 		return -EBUSY;
 
@@ -1351,6 +1353,8 @@ static int st_lsm6dsx_write_event_config(struct iio_dev *iio_dev,
 	if (err < 0)
 		return err;
 
+	mutex_unlock(&hw->conf_lock);
+
 	err = st_lsm6dsx_sensor_set_enable(sensor, state);
 	if (err < 0)
 		return err;
-- 
2.23.0


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

* [PATCH v5.1 6/6] iio: imu: st_lsm6dsx: prohibit the use of events and buffered reads simultaneously
  2019-09-09  9:45 ` [PATCH v5 6/6] iio: imu: st_lsm6dsx: prohibit the use of events and buffered reads simultaneously Sean Nyekjaer
@ 2019-09-09  9:56   ` " Sean Nyekjaer
  2019-09-09 10:40     ` Lorenzo Bianconi
  0 siblings, 1 reply; 12+ messages in thread
From: Sean Nyekjaer @ 2019-09-09  9:56 UTC (permalink / raw)
  To: linux-iio, jic23, lorenzo.bianconi83
  Cc: Sean Nyekjaer, denis.ciocca, mario.tesi, armando.visconti, martin

When events and buffered reads is enabled simultaneously, and the first
event accours the interrupt pin stays high.

This can be reverted when we find a solution to allow events and
buffered reads simultaneously.

Signed-off-by: Sean Nyekjaer <sean@geanix.com>
---
Changes since v4:
 * Use fifo configuration mutex to prevent a race in hw->enable_event
   check.

Changes since v5:
 * Updated do not return without unlocking mutexes

 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c |  5 +++++
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c   | 13 ++++++++++---
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
index ef579650fd52..b87a1872bc60 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
@@ -603,6 +603,11 @@ int st_lsm6dsx_update_fifo(struct st_lsm6dsx_sensor *sensor, bool enable)
 
 	mutex_lock(&hw->conf_lock);
 
+	if (hw->enable_event) {
+		err = -EBUSY;
+		goto out;
+	}
+
 	if (hw->fifo_mode != ST_LSM6DSX_FIFO_BYPASS) {
 		err = st_lsm6dsx_flush_fifo(hw);
 		if (err < 0)
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index 00ba14d15c13..a13d0c154b82 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -1340,8 +1340,12 @@ static int st_lsm6dsx_write_event_config(struct iio_dev *iio_dev,
 	if (type != IIO_EV_TYPE_THRESH)
 		return -EINVAL;
 
-	if (hw->fifo_mode != ST_LSM6DSX_FIFO_BYPASS)
-		return -EBUSY;
+	mutex_unlock(&hw->conf_lock);
+
+	if (hw->fifo_mode != ST_LSM6DSX_FIFO_BYPASS) {
+		err = -EBUSY;
+		goto out;
+	}
 
 	/* do not enable events if they are already enabled */
 	if (state && hw->enable_event)
@@ -1357,7 +1361,10 @@ static int st_lsm6dsx_write_event_config(struct iio_dev *iio_dev,
 
 	hw->enable_event = state;
 
-	return 0;
+out:
+	mutex_unlock(&hw->conf_lock);
+
+	return err;
 }
 
 int st_lsm6dsx_set_watermark(struct iio_dev *iio_dev, unsigned int val)
-- 
2.23.0


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

* Re: [PATCH v5 2/6] iio: imu: st_lsm6dsx: add motion events
  2019-09-09  9:45 ` [PATCH v5 2/6] iio: imu: st_lsm6dsx: add motion events Sean Nyekjaer
@ 2019-09-09 10:21   ` Lorenzo Bianconi
  2019-09-09 10:26     ` Sean Nyekjaer
  2019-09-09 11:13   ` Sean Nyekjaer
  1 sibling, 1 reply; 12+ messages in thread
From: Lorenzo Bianconi @ 2019-09-09 10:21 UTC (permalink / raw)
  To: Sean Nyekjaer
  Cc: linux-iio, jic23, lorenzo.bianconi83, denis.ciocca, mario.tesi,
	armando.visconti, martin

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

> Add event channels that controls the creation of motion events.
> Tested on ISM330DLC
> 
> Signed-off-by: Sean Nyekjaer <sean@geanix.com>
> ---
> Changes since v4:
>  * Added check for event support
>  * Added registers for more devices that support this event
> 
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  41 ++++
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 214 ++++++++++++++++++-
>  2 files changed, 251 insertions(+), 4 deletions(-)
> 

still someting wrong here:

drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c:955:5: error: ‘struct st_lsm6dsx_event_settings’ has no member named ‘wakeup_src_y_mask’
  955 |    .wakeup_src_y_mask = BIT(1),
      |     ^~~~~~~~~~~~~~~~~
In file included from ./include/linux/bitops.h:5,
                 from ./include/linux/kernel.h:12,
                 from drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c:48:
./include/linux/bits.h:8:19: warning: excess elements in struct initializer
    8 | #define BIT(nr)   (UL(1) << (nr))
      |                   ^
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c:955:25: note: in expansion of macro ‘BIT’
  955 |    .wakeup_src_y_mask = BIT(1),
      |                         ^~~
./include/linux/bits.h:8:19: note: (near initialization for ‘st_lsm6dsx_sensor_settings[6].event_settings’)
    8 | #define BIT(nr)   (UL(1) << (nr))
      |                   ^
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c:955:25: note: in expansion of macro ‘BIT’
  955 |    .wakeup_src_y_mask = BIT(1),
      |                         ^~~
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c:956:5: error: ‘struct st_lsm6dsx_event_settings’ has no member named ‘wakeup_src_x_mask’
  956 |    .wakeup_src_x_mask = BIT(2),
      |     ^~~~~~~~~~~~~~~~~
In file included from ./include/linux/bitops.h:5,
                 from ./include/linux/kernel.h:12,
                 from drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c:48:


You defined wakeup_src_{w,y,z}_mask in patch 5/6 and you are using them here. Please do not do so since you are breaking 'git bisect'

Regards,
Lorenzo


> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> index 5e3cd96b0059..d04473861fba 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> @@ -12,6 +12,7 @@
>  #define ST_LSM6DSX_H
>  
>  #include <linux/device.h>
> +#include <linux/iio/iio.h>
>  
>  #define ST_LSM6DS3_DEV_NAME	"lsm6ds3"
>  #define ST_LSM6DS3H_DEV_NAME	"lsm6ds3h"
> @@ -54,6 +55,26 @@ enum st_lsm6dsx_hw_id {
>  					 * ST_LSM6DSX_TAGGED_SAMPLE_SIZE)
>  #define ST_LSM6DSX_SHIFT_VAL(val, mask)	(((val) << __ffs(mask)) & (mask))
>  
> +#define ST_LSM6DSX_CHANNEL_ACC(chan_type, addr, mod, scan_idx)		\
> +{									\
> +	.type = chan_type,						\
> +	.address = addr,						\
> +	.modified = 1,							\
> +	.channel2 = mod,						\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),		\
> +	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
> +	.scan_index = scan_idx,						\
> +	.scan_type = {							\
> +		.sign = 's',						\
> +		.realbits = 16,						\
> +		.storagebits = 16,					\
> +		.endianness = IIO_LE,					\
> +	},								\
> +	.event_spec = &st_lsm6dsx_event,				\
> +	.num_event_specs = 1,						\
> +}
> +
>  #define ST_LSM6DSX_CHANNEL(chan_type, addr, mod, scan_idx)		\
>  {									\
>  	.type = chan_type,						\
> @@ -162,6 +183,11 @@ struct st_lsm6dsx_shub_settings {
>  	u8 batch_en;
>  };
>  
> +struct st_lsm6dsx_event_settings {
> +	struct st_lsm6dsx_reg enable_reg;
> +	struct st_lsm6dsx_reg wakeup_reg;
> +};
> +
>  enum st_lsm6dsx_ext_sensor_id {
>  	ST_LSM6DSX_ID_MAGN,
>  };
> @@ -223,6 +249,9 @@ struct st_lsm6dsx_settings {
>  	u8 wai;
>  	u8 int1_addr;
>  	u8 int2_addr;
> +	u8 int1_func_addr;
> +	u8 int2_func_addr;
> +	u8 int_func_mask;
>  	u8 reset_addr;
>  	u16 max_fifo_size;
>  	struct {
> @@ -240,6 +269,7 @@ struct st_lsm6dsx_settings {
>  	struct st_lsm6dsx_fifo_ops fifo_ops;
>  	struct st_lsm6dsx_hw_ts_settings ts_settings;
>  	struct st_lsm6dsx_shub_settings shub_settings;
> +	struct st_lsm6dsx_event_settings event_settings;
>  };
>  
>  enum st_lsm6dsx_sensor_id {
> @@ -320,6 +350,10 @@ struct st_lsm6dsx_hw {
>  	u8 ts_sip;
>  	u8 sip;
>  
> +	u8 event_threshold;
> +	bool enable_event;
> +	struct st_lsm6dsx_reg irq_routing;
> +
>  	u8 *buff;
>  
>  	struct iio_dev *iio_devs[ST_LSM6DSX_ID_MAX];
> @@ -327,6 +361,13 @@ struct st_lsm6dsx_hw {
>  	const struct st_lsm6dsx_settings *settings;
>  };
>  
> +static const struct iio_event_spec st_lsm6dsx_event = {
> +	.type = IIO_EV_TYPE_THRESH,
> +	.dir = IIO_EV_DIR_EITHER,
> +	.mask_separate = BIT(IIO_EV_INFO_VALUE) |
> +			 BIT(IIO_EV_INFO_ENABLE)
> +};
> +
>  static const unsigned long st_lsm6dsx_available_scan_masks[] = {0x7, 0x0};
>  extern const struct dev_pm_ops st_lsm6dsx_pm_ops;
>  
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index d0bcbbfb6297..fb83076d8f17 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -73,9 +73,9 @@
>  #define ST_LSM6DSX_REG_PP_OD_MASK		BIT(4)
>  
>  static const struct iio_chan_spec st_lsm6dsx_acc_channels[] = {
> -	ST_LSM6DSX_CHANNEL(IIO_ACCEL, 0x28, IIO_MOD_X, 0),
> -	ST_LSM6DSX_CHANNEL(IIO_ACCEL, 0x2a, IIO_MOD_Y, 1),
> -	ST_LSM6DSX_CHANNEL(IIO_ACCEL, 0x2c, IIO_MOD_Z, 2),
> +	ST_LSM6DSX_CHANNEL_ACC(IIO_ACCEL, 0x28, IIO_MOD_X, 0),
> +	ST_LSM6DSX_CHANNEL_ACC(IIO_ACCEL, 0x2a, IIO_MOD_Y, 1),
> +	ST_LSM6DSX_CHANNEL_ACC(IIO_ACCEL, 0x2c, IIO_MOD_Z, 2),
>  	IIO_CHAN_SOFT_TIMESTAMP(3),
>  };
>  
> @@ -168,6 +168,9 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  		.wai = 0x69,
>  		.int1_addr = 0x0d,
>  		.int2_addr = 0x0e,
> +		.int1_func_addr = 0x5e,
> +		.int2_func_addr = 0x5f,
> +		.int_func_mask = BIT(5),
>  		.reset_addr = 0x12,
>  		.max_fifo_size = 1365,
>  		.id = {
> @@ -275,11 +278,25 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  				.mask = GENMASK(5, 3),
>  			},
>  		},
> +		.event_settings = {
> +			.wakeup_reg = {
> +				.addr = 0x5B,
> +				.mask = GENMASK(5, 0),
> +			},
> +			.wakeup_src_reg = 0x1b,
> +			.wakeup_src_status_mask = BIT(3),
> +			.wakeup_src_z_mask = BIT(0),
> +			.wakeup_src_y_mask = BIT(1),
> +			.wakeup_src_x_mask = BIT(2),
> +		},
>  	},
>  	{
>  		.wai = 0x69,
>  		.int1_addr = 0x0d,
>  		.int2_addr = 0x0e,
> +		.int1_func_addr = 0x5e,
> +		.int2_func_addr = 0x5f,
> +		.int_func_mask = BIT(5),
>  		.reset_addr = 0x12,
>  		.max_fifo_size = 682,
>  		.id = {
> @@ -387,11 +404,25 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  				.mask = GENMASK(5, 3),
>  			},
>  		},
> +		.event_settings = {
> +			.wakeup_reg = {
> +				.addr = 0x5B,
> +				.mask = GENMASK(5, 0),
> +			},
> +			.wakeup_src_reg = 0x1b,
> +			.wakeup_src_status_mask = BIT(3),
> +			.wakeup_src_z_mask = BIT(0),
> +			.wakeup_src_y_mask = BIT(1),
> +			.wakeup_src_x_mask = BIT(2),
> +		},
>  	},
>  	{
>  		.wai = 0x6a,
>  		.int1_addr = 0x0d,
>  		.int2_addr = 0x0e,
> +		.int1_func_addr = 0x5e,
> +		.int2_func_addr = 0x5f,
> +		.int_func_mask = BIT(5),
>  		.reset_addr = 0x12,
>  		.max_fifo_size = 682,
>  		.id = {
> @@ -508,6 +539,16 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  				.mask = GENMASK(5, 3),
>  			},
>  		},
> +		.event_settings = {
> +			.enable_reg = {
> +				.addr = 0x58,
> +				.mask = BIT(7),
> +			},
> +			.wakeup_reg = {
> +				.addr = 0x5B,
> +				.mask = GENMASK(5, 0),
> +			},
> +		},
>  	},
>  	{
>  		.wai = 0x6c,
> @@ -646,6 +687,9 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  		.wai = 0x6b,
>  		.int1_addr = 0x0d,
>  		.int2_addr = 0x0e,
> +		.int1_func_addr = 0x5e,
> +		.int2_func_addr = 0x5f,
> +		.int_func_mask = BIT(5),
>  		.reset_addr = 0x12,
>  		.max_fifo_size = 512,
>  		.id = {
> @@ -745,11 +789,29 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  				.mask = GENMASK(7, 6),
>  			},
>  		},
> +		.event_settings = {
> +			.enable_reg = {
> +				.addr = 0x58,
> +				.mask = BIT(7),
> +			},
> +			.wakeup_reg = {
> +				.addr = 0x5B,
> +				.mask = GENMASK(5, 0),
> +			},
> +			.wakeup_src_reg = 0x1b,
> +			.wakeup_src_status_mask = BIT(3),
> +			.wakeup_src_z_mask = BIT(0),
> +			.wakeup_src_y_mask = BIT(1),
> +			.wakeup_src_x_mask = BIT(2),
> +		},
>  	},
>  	{
>  		.wai = 0x6b,
>  		.int1_addr = 0x0d,
>  		.int2_addr = 0x0e,
> +		.int1_func_addr = 0x5e,
> +		.int2_func_addr = 0x5f,
> +		.int_func_mask = BIT(5),
>  		.reset_addr = 0x12,
>  		.max_fifo_size = 512,
>  		.id = {
> @@ -877,6 +939,21 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  			.slv0_addr = 0x15,
>  			.dw_slv0_addr = 0x21,
>  			.batch_en = BIT(3),
> +		},
> +		.event_settings = {
> +			.enable_reg = {
> +				.addr = 0x58,
> +				.mask = BIT(7),
> +			},
> +			.wakeup_reg = {
> +				.addr = 0x5B,
> +				.mask = GENMASK(5, 0),
> +			},
> +			.wakeup_src_reg = 0x1b,
> +			.wakeup_src_status_mask = BIT(3),
> +			.wakeup_src_z_mask = BIT(0),
> +			.wakeup_src_y_mask = BIT(1),
> +			.wakeup_src_x_mask = BIT(2),
>  		}
>  	},
>  };
> @@ -1083,7 +1160,8 @@ static int st_lsm6dsx_read_oneshot(struct st_lsm6dsx_sensor *sensor,
>  	if (err < 0)
>  		return err;
>  
> -	st_lsm6dsx_sensor_set_enable(sensor, false);
> +	if (!hw->enable_event)
> +		st_lsm6dsx_sensor_set_enable(sensor, false);
>  
>  	*val = (s16)le16_to_cpu(data);
>  
> @@ -1156,6 +1234,126 @@ static int st_lsm6dsx_write_raw(struct iio_dev *iio_dev,
>  	return err;
>  }
>  
> +int st_lsm6dsx_event_setup(struct st_lsm6dsx_hw *hw, int state)
> +{
> +	int err;
> +	u8 enable = 0;

useless initialization

> +
> +	if (!hw->settings->int1_func_addr)
> +		return -ENOTSUPP;
> +
> +	enable = state ? hw->settings->event_settings.enable_reg.mask : 0;
> +
> +	err = regmap_update_bits(hw->regmap,
> +				 hw->settings->event_settings.enable_reg.addr,
> +				 hw->settings->event_settings.enable_reg.mask,
> +				 enable);
> +	if (err < 0)
> +		return err;
> +
> +	enable = state ? hw->irq_routing.mask : 0;
> +
> +	/* Enable wakeup interrupt */
> +	return regmap_update_bits(hw->regmap, hw->irq_routing.addr,
> +				  hw->irq_routing.mask,
> +				  enable);
> +}
> +
> +static int st_lsm6dsx_read_event(struct iio_dev *iio_dev,
> +				   const struct iio_chan_spec *chan,
> +				   enum iio_event_type type,
> +				   enum iio_event_direction dir,
> +				   enum iio_event_info info,
> +				   int *val, int *val2)
> +{
> +	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
> +	struct st_lsm6dsx_hw *hw = sensor->hw;
> +
> +	if (type != IIO_EV_TYPE_THRESH)
> +		return -EINVAL;
> +
> +	*val2 = 0;
> +	*val = hw->event_threshold;
> +
> +	return IIO_VAL_INT;
> +}
> +
> +static int st_lsm6dsx_write_event(struct iio_dev *iio_dev,
> +				    const struct iio_chan_spec *chan,
> +				    enum iio_event_type type,
> +				    enum iio_event_direction dir,
> +				    enum iio_event_info info,
> +				    int val, int val2)
> +{
> +	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
> +	struct st_lsm6dsx_hw *hw = sensor->hw;
> +	int err;
> +
> +	if (type != IIO_EV_TYPE_THRESH)
> +		return -EINVAL;
> +
> +	if (val < 0 || val > 31)
> +		return -EINVAL;
> +
> +	err = regmap_update_bits(hw->regmap,
> +				 hw->settings->event_settings.wakeup_reg.addr,
> +				 hw->settings->event_settings.wakeup_reg.mask,
> +				 val);
> +	if (err)
> +		return -EINVAL;
> +
> +	hw->event_threshold = val;
> +
> +	return 0;
> +}
> +
> +static int st_lsm6dsx_read_event_config(struct iio_dev *iio_dev,
> +					  const struct iio_chan_spec *chan,
> +					  enum iio_event_type type,
> +					  enum iio_event_direction dir)
> +{
> +	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
> +	struct st_lsm6dsx_hw *hw = sensor->hw;
> +
> +	if (type != IIO_EV_TYPE_THRESH)
> +		return -EINVAL;
> +
> +	return hw->enable_event;
> +}
> +
> +static int st_lsm6dsx_write_event_config(struct iio_dev *iio_dev,
> +					   const struct iio_chan_spec *chan,
> +					   enum iio_event_type type,
> +					   enum iio_event_direction dir,
> +					   int state)
> +{
> +	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
> +	struct st_lsm6dsx_hw *hw = sensor->hw;
> +	int err = 0;
> +
> +	if (type != IIO_EV_TYPE_THRESH)
> +		return -EINVAL;
> +
> +	if (hw->fifo_mode != ST_LSM6DSX_FIFO_BYPASS)
> +		return -EBUSY;
> +
> +	/* do not enable events if they are already enabled */
> +	if (state && hw->enable_event)
> +		return 0;
> +
> +	err = st_lsm6dsx_event_setup(hw, state);
> +	if (err < 0)
> +		return err;
> +
> +	err = st_lsm6dsx_sensor_set_enable(sensor, state);
> +	if (err < 0)
> +		return err;
> +
> +	hw->enable_event = state;
> +
> +	return 0;
> +}
> +
>  int st_lsm6dsx_set_watermark(struct iio_dev *iio_dev, unsigned int val)
>  {
>  	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
> @@ -1240,6 +1438,10 @@ static const struct iio_info st_lsm6dsx_acc_info = {
>  	.attrs = &st_lsm6dsx_acc_attribute_group,
>  	.read_raw = st_lsm6dsx_read_raw,
>  	.write_raw = st_lsm6dsx_write_raw,
> +	.read_event_value = st_lsm6dsx_read_event,
> +	.write_event_value = st_lsm6dsx_write_event,
> +	.read_event_config = st_lsm6dsx_read_event_config,
> +	.write_event_config = st_lsm6dsx_write_event_config,
>  	.hwfifo_set_watermark = st_lsm6dsx_set_watermark,
>  };
>  
> @@ -1285,9 +1487,13 @@ static int st_lsm6dsx_get_drdy_reg(struct st_lsm6dsx_hw *hw, u8 *drdy_reg)
>  	switch (drdy_pin) {
>  	case 1:
>  		*drdy_reg = hw->settings->int1_addr;
> +		hw->irq_routing.addr = hw->settings->int1_func_addr;
> +		hw->irq_routing.mask = hw->settings->int_func_mask;
>  		break;
>  	case 2:
>  		*drdy_reg = hw->settings->int2_addr;
> +		hw->irq_routing.addr = hw->settings->int2_func_addr;
> +		hw->irq_routing.mask = hw->settings->int_func_mask;
>  		break;
>  	default:
>  		dev_err(hw->dev, "unsupported data ready pin\n");
> -- 
> 2.23.0
> 

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

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

* Re: [PATCH v5 2/6] iio: imu: st_lsm6dsx: add motion events
  2019-09-09 10:21   ` Lorenzo Bianconi
@ 2019-09-09 10:26     ` Sean Nyekjaer
  0 siblings, 0 replies; 12+ messages in thread
From: Sean Nyekjaer @ 2019-09-09 10:26 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: linux-iio, jic23, lorenzo.bianconi83, denis.ciocca, mario.tesi,
	armando.visconti, martin



On 09/09/2019 12.21, Lorenzo Bianconi wrote:
> You defined wakeup_src_{w,y,z}_mask in patch 5/6 and you are using them here. Please do not do so since you are breaking 'git bisect'
> 
> Regards,
> Lorenzo

My bad, will update in v6 :)

Will move the device sensor definitions to 5/6, so we avoid
"warning: variable ‘**’ set but not used"

/Sean

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

* Re: [PATCH v5.1 6/6] iio: imu: st_lsm6dsx: prohibit the use of events and buffered reads simultaneously
  2019-09-09  9:56   ` [PATCH v5.1 " Sean Nyekjaer
@ 2019-09-09 10:40     ` Lorenzo Bianconi
  2019-09-09 11:07       ` Sean Nyekjaer
  0 siblings, 1 reply; 12+ messages in thread
From: Lorenzo Bianconi @ 2019-09-09 10:40 UTC (permalink / raw)
  To: Sean Nyekjaer
  Cc: linux-iio, jic23, lorenzo.bianconi83, denis.ciocca, mario.tesi,
	armando.visconti, martin

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

> When events and buffered reads is enabled simultaneously, and the first
> event accours the interrupt pin stays high.
> 
> This can be reverted when we find a solution to allow events and
> buffered reads simultaneously.
> 
> Signed-off-by: Sean Nyekjaer <sean@geanix.com>
> ---
> Changes since v4:
>  * Use fifo configuration mutex to prevent a race in hw->enable_event
>    check.
> 
> Changes since v5:
>  * Updated do not return without unlocking mutexes
> 
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c |  5 +++++
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c   | 13 ++++++++++---
>  2 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> index ef579650fd52..b87a1872bc60 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> @@ -603,6 +603,11 @@ int st_lsm6dsx_update_fifo(struct st_lsm6dsx_sensor *sensor, bool enable)
>  
>  	mutex_lock(&hw->conf_lock);
>  
> +	if (hw->enable_event) {
> +		err = -EBUSY;
> +		goto out;
> +	}
> +
>  	if (hw->fifo_mode != ST_LSM6DSX_FIFO_BYPASS) {
>  		err = st_lsm6dsx_flush_fifo(hw);
>  		if (err < 0)
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index 00ba14d15c13..a13d0c154b82 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -1340,8 +1340,12 @@ static int st_lsm6dsx_write_event_config(struct iio_dev *iio_dev,
>  	if (type != IIO_EV_TYPE_THRESH)
>  		return -EINVAL;
>  
> -	if (hw->fifo_mode != ST_LSM6DSX_FIFO_BYPASS)
> -		return -EBUSY;
> +	mutex_unlock(&hw->conf_lock);

I _really_ do not think you are testing these patches since there are plenty of
errors here:

1- mutex_unlock instead of mutex_lock
2- returning without releasing the lock

Please put more attention next time and test you patches

Lorenzo

> +
> +	if (hw->fifo_mode != ST_LSM6DSX_FIFO_BYPASS) {
> +		err = -EBUSY;
> +		goto out;
> +	}
>  
>  	/* do not enable events if they are already enabled */
>  	if (state && hw->enable_event)
> @@ -1357,7 +1361,10 @@ static int st_lsm6dsx_write_event_config(struct iio_dev *iio_dev,
>  
>  	hw->enable_event = state;
>  
> -	return 0;
> +out:
> +	mutex_unlock(&hw->conf_lock);
> +
> +	return err;
>  }
>  
>  int st_lsm6dsx_set_watermark(struct iio_dev *iio_dev, unsigned int val)
> -- 
> 2.23.0
> 

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

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

* Re: [PATCH v5.1 6/6] iio: imu: st_lsm6dsx: prohibit the use of events and buffered reads simultaneously
  2019-09-09 10:40     ` Lorenzo Bianconi
@ 2019-09-09 11:07       ` Sean Nyekjaer
  0 siblings, 0 replies; 12+ messages in thread
From: Sean Nyekjaer @ 2019-09-09 11:07 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: linux-iio, jic23, lorenzo.bianconi83, denis.ciocca, mario.tesi,
	armando.visconti, martin



On 09/09/2019 12.40, Lorenzo Bianconi wrote:
> I_really_  do not think you are testing these patches since there are plenty of
> errors here:
> 
> 1- mutex_unlock instead of mutex_lock

Again, my bad :)

The error show quite quickly:
WARNING: bad unlock balance detected!

> 2- returning without releasing the lock
> 
> Please put more attention next time and test you patches

Will do...
Thanks for the review

/Sean

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

* Re: [PATCH v5 2/6] iio: imu: st_lsm6dsx: add motion events
  2019-09-09  9:45 ` [PATCH v5 2/6] iio: imu: st_lsm6dsx: add motion events Sean Nyekjaer
  2019-09-09 10:21   ` Lorenzo Bianconi
@ 2019-09-09 11:13   ` Sean Nyekjaer
  1 sibling, 0 replies; 12+ messages in thread
From: Sean Nyekjaer @ 2019-09-09 11:13 UTC (permalink / raw)
  To: linux-iio, jic23, lorenzo.bianconi83
  Cc: denis.ciocca, mario.tesi, armando.visconti, martin



On 09/09/2019 11.45, Sean Nyekjaer wrote:
> +int st_lsm6dsx_event_setup(struct st_lsm6dsx_hw *hw, int state)
> +{
> +	int err;
> +	u8 enable = 0;
> +
> +	if (!hw->settings->int1_func_addr)
> +		return -ENOTSUPP;
> +
> +	enable = state ? hw->settings->event_settings.enable_reg.mask : 0;
> +
> +	err = regmap_update_bits(hw->regmap,
> +				 hw->settings->event_settings.enable_reg.addr,
> +				 hw->settings->event_settings.enable_reg.mask,
> +				 enable);
> +	if (err < 0)
> +		return err;
> +
> +	enable = state ? hw->irq_routing.mask : 0;
> +
> +	/* Enable wakeup interrupt */
> +	return regmap_update_bits(hw->regmap, hw->irq_routing.addr,
> +				  hw->irq_routing.mask,
> +				  enable);
> +}
> +
> +static int st_lsm6dsx_read_event(struct iio_dev *iio_dev,
> +				   const struct iio_chan_spec *chan,
> +				   enum iio_event_type type,
> +				   enum iio_event_direction dir,
> +				   enum iio_event_info info,
> +				   int *val, int *val2)
> +{
> +	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
> +	struct st_lsm6dsx_hw *hw = sensor->hw;
> +
> +	if (type != IIO_EV_TYPE_THRESH)
> +		return -EINVAL;
> +
> +	*val2 = 0;
> +	*val = hw->event_threshold;
> +
> +	return IIO_VAL_INT;
> +}

For a device that not support wake events the user will get a:
root@board:/sys/bus/iio/devices/iio:device2# echo 1 > 
events/in_accel_x_thresh_either_en
-sh: echo: write error: Unknown error 524

/Sean

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

end of thread, back to index

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-09  9:45 [PATCH v5 1/6] iio: imu: st_lsm6dsx: move interrupt thread to core Sean Nyekjaer
2019-09-09  9:45 ` [PATCH v5 2/6] iio: imu: st_lsm6dsx: add motion events Sean Nyekjaer
2019-09-09 10:21   ` Lorenzo Bianconi
2019-09-09 10:26     ` Sean Nyekjaer
2019-09-09 11:13   ` Sean Nyekjaer
2019-09-09  9:45 ` [PATCH v5 3/6] iio: imu: st_lsm6dsx: add wakeup-source option Sean Nyekjaer
2019-09-09  9:45 ` [PATCH v5 4/6] iio: imu: st_lsm6dsx: always enter interrupt thread Sean Nyekjaer
2019-09-09  9:45 ` [PATCH v5 5/6] iio: imu: st_lsm6dsx: add motion report function and call from interrupt Sean Nyekjaer
2019-09-09  9:45 ` [PATCH v5 6/6] iio: imu: st_lsm6dsx: prohibit the use of events and buffered reads simultaneously Sean Nyekjaer
2019-09-09  9:56   ` [PATCH v5.1 " Sean Nyekjaer
2019-09-09 10:40     ` Lorenzo Bianconi
2019-09-09 11:07       ` Sean Nyekjaer

Linux-IIO Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-iio/0 linux-iio/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-iio linux-iio/ https://lore.kernel.org/linux-iio \
		linux-iio@vger.kernel.org linux-iio@archiver.kernel.org
	public-inbox-index linux-iio


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-iio


AGPL code for this site: git clone https://public-inbox.org/ public-inbox