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

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

Signed-off-by: Sean Nyekjaer <sean@geanix.com>
---
Changes since v1:
 * none
Changes since v2:
 * none

 .../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..8e0da5eb4283 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;
+}
+
+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 err;
+}
+
 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] 16+ messages in thread

* [PATCH v3 2/6] iio: imu: st_lsm6dsx: add motion events
  2019-09-04  9:17 [PATCH v3 1/6] iio: imu: st_lsm6dsx: move interrupt thread to core Sean Nyekjaer
@ 2019-09-04  9:17 ` Sean Nyekjaer
  2019-09-04 17:24   ` Lorenzo Bianconi
  2019-09-04  9:17 ` [PATCH v3 3/6] iio: imu: st_lsm6dsx: add wakeup-source option Sean Nyekjaer
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Sean Nyekjaer @ 2019-09-04  9:17 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.

Signed-off-by: Sean Nyekjaer <sean@geanix.com>
---
Changes since v1:
 * added handling of LSM6
 * added CHANNEL info with events for ACC
 * removed st_lsm6dsx_set_event_threshold function
 * added check of event type to event channels

Changes since v2:
 * added devices specific registers to st_lsm6dsx_sensor_settings
 * allow to write threshold when the events are disabled

How do we use the existing macro's when I want to expand them with
events for the accelerometer only?

 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  43 +++++
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 156 +++++++++++++++++--
 2 files changed, 190 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
index 5e3cd96b0059..10f3191e8695 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) |			\
+			      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,13 @@ struct st_lsm6dsx_shub_settings {
 	u8 batch_en;
 };
 
+struct st_lsm6dsx_event_settings {
+	u8 enable_addr;
+	u8 enable_mask;
+	u8 wakeup_addr;
+	u8 wakeup_thres_mask;
+};
+
 enum st_lsm6dsx_ext_sensor_id {
 	ST_LSM6DSX_ID_MAGN,
 };
@@ -223,6 +251,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 +271,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 +352,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 +363,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 8e0da5eb4283..d4d6ba934b6d 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),
 };
 
@@ -392,6 +392,9 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 		.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 +511,12 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.mask = GENMASK(5, 3),
 			},
 		},
+		.event_settings = {
+			.enable_addr = 0x58,
+			.enable_mask = BIT(7),
+			.wakeup_addr = 0x5B,
+			.wakeup_thres_mask = GENMASK(5, 0),
+		},
 	},
 	{
 		.wai = 0x6c,
@@ -1072,18 +1081,21 @@ static int st_lsm6dsx_read_oneshot(struct st_lsm6dsx_sensor *sensor,
 	int err, delay;
 	__le16 data;
 
-	err = st_lsm6dsx_sensor_set_enable(sensor, true);
-	if (err < 0)
-		return err;
+	if (!hw->enable_event) {
+		err = st_lsm6dsx_sensor_set_enable(sensor, true);
+		if (err < 0)
+			return err;
 
-	delay = 1000000 / sensor->odr;
-	usleep_range(delay, 2 * delay);
+		delay = 1000000 / sensor->odr;
+		usleep_range(delay, 2 * delay);
+	}
 
 	err = st_lsm6dsx_read_locked(hw, addr, &data, sizeof(data));
 	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 +1168,124 @@ 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 (state)
+		enable = hw->settings->event_settings.enable_mask;
+
+	err = regmap_update_bits(hw->regmap,
+				 hw->settings->event_settings.enable_addr,
+				 hw->settings->event_settings.enable_mask,
+				 enable);
+	if (err < 0)
+		return err;
+
+	enable = 0;
+	if (state)
+		enable = hw->irq_routing.mask;
+
+	/* Enable wakeup interrupt */
+	err = regmap_update_bits(hw->regmap, hw->irq_routing.addr,
+				 hw->irq_routing.mask,
+				 enable);
+
+	return err;
+}
+
+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_addr,
+				 hw->settings->event_settings.wakeup_thres_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 (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 +1370,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 +1419,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] 16+ messages in thread

* [PATCH v3 3/6] iio: imu: st_lsm6dsx: add wakeup-source option
  2019-09-04  9:17 [PATCH v3 1/6] iio: imu: st_lsm6dsx: move interrupt thread to core Sean Nyekjaer
  2019-09-04  9:17 ` [PATCH v3 2/6] iio: imu: st_lsm6dsx: add motion events Sean Nyekjaer
@ 2019-09-04  9:17 ` Sean Nyekjaer
  2019-09-04 17:27   ` Lorenzo Bianconi
  2019-09-04  9:17 ` [PATCH v3 4/6] iio: imu: st_lsm6dsx: always enter interrupt thread Sean Nyekjaer
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Sean Nyekjaer @ 2019-09-04  9:17 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 v1:
 * none, as the call to st_lsm6dsx_flush_fifo will put the fifo in
   bypass mode.

Changes since v2:
 * Removed unnecessary brackets
 * Move enable_irq to after the enable mask

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

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index d4d6ba934b6d..bb72800bf99b 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -1754,6 +1754,10 @@ int st_lsm6dsx_probe(struct device *dev, int irq, int hw_id,
 			return err;
 	}
 
+	if (dev->of_node)
+		if (of_property_read_bool(dev->of_node, "wakeup-source"))
+			device_init_wakeup(dev, true);
+
 	return 0;
 }
 EXPORT_SYMBOL(st_lsm6dsx_probe);
@@ -1772,6 +1776,12 @@ static int __maybe_unused st_lsm6dsx_suspend(struct device *dev)
 		if (!(hw->enable_mask & BIT(sensor->id)))
 			continue;
 
+		if (device_may_wakeup(dev) && i == ST_LSM6DSX_ID_ACC) {
+			/* 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)
@@ -1804,6 +1814,11 @@ static int __maybe_unused st_lsm6dsx_resume(struct device *dev)
 		if (!(hw->suspend_mask & BIT(sensor->id)))
 			continue;
 
+		if (device_may_wakeup(dev) && i == ST_LSM6DSX_ID_ACC) {
+			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] 16+ messages in thread

* [PATCH v3 4/6] iio: imu: st_lsm6dsx: always enter interrupt thread
  2019-09-04  9:17 [PATCH v3 1/6] iio: imu: st_lsm6dsx: move interrupt thread to core Sean Nyekjaer
  2019-09-04  9:17 ` [PATCH v3 2/6] iio: imu: st_lsm6dsx: add motion events Sean Nyekjaer
  2019-09-04  9:17 ` [PATCH v3 3/6] iio: imu: st_lsm6dsx: add wakeup-source option Sean Nyekjaer
@ 2019-09-04  9:17 ` Sean Nyekjaer
  2019-09-05  6:34   ` Lorenzo Bianconi
  2019-09-04  9:17 ` [PATCH v3 5/6] iio: imu: st_lsm6dsx: add motion report function and call from interrupt Sean Nyekjaer
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Sean Nyekjaer @ 2019-09-04  9:17 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>
---
Changes since v1:
 * None

Changes since v2:
 * None

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

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index bb72800bf99b..513506caa460 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -1613,19 +1613,19 @@ 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)
 {
 	struct st_lsm6dsx_hw *hw = private;
-	int count;
+	int count = 0;
 
-	mutex_lock(&hw->fifo_lock);
-	count = hw->settings->fifo_ops.read_fifo(hw);
-	mutex_unlock(&hw->fifo_lock);
+	if (hw->sip > 0) {
+		mutex_lock(&hw->fifo_lock);
+		count = st_lsm6dsx_read_fifo(hw);
+		mutex_unlock(&hw->fifo_lock);
+	}
 
 	return count ? IRQ_HANDLED : IRQ_NONE;
 }
-- 
2.23.0


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

* [PATCH v3 5/6] iio: imu: st_lsm6dsx: add motion report function and call from interrupt
  2019-09-04  9:17 [PATCH v3 1/6] iio: imu: st_lsm6dsx: move interrupt thread to core Sean Nyekjaer
                   ` (2 preceding siblings ...)
  2019-09-04  9:17 ` [PATCH v3 4/6] iio: imu: st_lsm6dsx: always enter interrupt thread Sean Nyekjaer
@ 2019-09-04  9:17 ` Sean Nyekjaer
  2019-09-05  6:39   ` Lorenzo Bianconi
  2019-09-04  9:17 ` [PATCH v3 6/6] iio: imu: st_lsm6dsx: prohibit the use of events and buffered reads simultaneously Sean Nyekjaer
  2019-09-04 17:11 ` [PATCH v3 1/6] iio: imu: st_lsm6dsx: move interrupt thread to core Lorenzo Bianconi
  5 siblings, 1 reply; 16+ messages in thread
From: Sean Nyekjaer @ 2019-09-04  9:17 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 v1:
 * none

Changes since v2:
 * none

Should we include these new defines in device settings?

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

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index 513506caa460..2114c3c78888 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>
@@ -72,6 +73,12 @@
 #define ST_LSM6DSX_REG_PP_OD_ADDR		0x12
 #define ST_LSM6DSX_REG_PP_OD_MASK		BIT(4)
 
+#define ST_LSM6DSX_REG_WAKE_UP_SRC_ADDR		0x1B
+#define ST_LSM6DSX_REG_WAKE_UP_SRC_Z_WU_MASK	BIT(0)
+#define ST_LSM6DSX_REG_WAKE_UP_SRC_Y_WU_MASK	BIT(1)
+#define ST_LSM6DSX_REG_WAKE_UP_SRC_X_WU_MASK	BIT(2)
+#define ST_LSM6DSX_REG_WAKE_UP_SRC_WU_MASK	BIT(4)
+
 static const struct iio_chan_spec st_lsm6dsx_acc_channels[] = {
 	ST_LSM6DSX_CHANNEL_ACC(IIO_ACCEL, 0x28, IIO_MOD_X, 0),
 	ST_LSM6DSX_CHANNEL_ACC(IIO_ACCEL, 0x2a, IIO_MOD_Y, 1),
@@ -1611,6 +1618,40 @@ static struct iio_dev *st_lsm6dsx_alloc_iiodev(struct st_lsm6dsx_hw *hw,
 	return iio_dev;
 }
 
+int 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 & ST_LSM6DSX_REG_WAKE_UP_SRC_Z_WU_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 & ST_LSM6DSX_REG_WAKE_UP_SRC_Y_WU_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 & ST_LSM6DSX_REG_WAKE_UP_SRC_X_WU_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);
+
+	return 0;
+}
+
 static irqreturn_t st_lsm6dsx_handler_irq(int irq, void *private)
 {
 	return IRQ_WAKE_THREAD;
@@ -1620,7 +1661,19 @@ static irqreturn_t st_lsm6dsx_handler_thread(int irq, void *private)
 {
 	struct st_lsm6dsx_hw *hw = private;
 	int count = 0;
+	int data, err;
+
+	if (hw->enable_event) {
+		err = regmap_read(hw->regmap,
+				  ST_LSM6DSX_REG_WAKE_UP_SRC_ADDR, &data);
+		if (err < 0)
+			goto try_fifo;
+
+		if (data & ST_LSM6DSX_REG_WAKE_UP_SRC_WU_MASK)
+			st_lsm6dsx_report_motion_event(hw, data);
+	}
 
+try_fifo:
 	if (hw->sip > 0) {
 		mutex_lock(&hw->fifo_lock);
 		count = st_lsm6dsx_read_fifo(hw);
-- 
2.23.0


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

* [PATCH v3 6/6] iio: imu: st_lsm6dsx: prohibit the use of events and buffered reads simultaneously
  2019-09-04  9:17 [PATCH v3 1/6] iio: imu: st_lsm6dsx: move interrupt thread to core Sean Nyekjaer
                   ` (3 preceding siblings ...)
  2019-09-04  9:17 ` [PATCH v3 5/6] iio: imu: st_lsm6dsx: add motion report function and call from interrupt Sean Nyekjaer
@ 2019-09-04  9:17 ` Sean Nyekjaer
  2019-09-04 17:11 ` [PATCH v3 1/6] iio: imu: st_lsm6dsx: move interrupt thread to core Lorenzo Bianconi
  5 siblings, 0 replies; 16+ messages in thread
From: Sean Nyekjaer @ 2019-09-04  9:17 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>
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 3 +++
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c   | 3 +++
 2 files changed, 6 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..94e8884a1db1 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
@@ -601,6 +601,9 @@ int st_lsm6dsx_update_fifo(struct st_lsm6dsx_sensor *sensor, bool enable)
 	struct st_lsm6dsx_hw *hw = sensor->hw;
 	int err;
 
+	if (hw->enable_event)
+		return -EBUSY;
+
 	mutex_lock(&hw->conf_lock);
 
 	if (hw->fifo_mode != ST_LSM6DSX_FIFO_BYPASS) {
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index 2114c3c78888..7d0850df0a5e 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -1277,6 +1277,9 @@ 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;
+
 	if (state && hw->enable_event)
 		return 0;
 
-- 
2.23.0


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

* Re: [PATCH v3 1/6] iio: imu: st_lsm6dsx: move interrupt thread to core
  2019-09-04  9:17 [PATCH v3 1/6] iio: imu: st_lsm6dsx: move interrupt thread to core Sean Nyekjaer
                   ` (4 preceding siblings ...)
  2019-09-04  9:17 ` [PATCH v3 6/6] iio: imu: st_lsm6dsx: prohibit the use of events and buffered reads simultaneously Sean Nyekjaer
@ 2019-09-04 17:11 ` Lorenzo Bianconi
  5 siblings, 0 replies; 16+ messages in thread
From: Lorenzo Bianconi @ 2019-09-04 17:11 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: 3776 bytes --]

> This prepares the interrupt to be used for other stuff than
> fifo reading + event readings.
> 
> Signed-off-by: Sean Nyekjaer <sean@geanix.com>
> ---
> Changes since v1:
>  * none
> Changes since v2:
>  * none
> 
>  .../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(-)

Hi Sean,

just a couple of nitpick inline (they can be addressed even in a follow up
patch)

Regards,
Lorenzo

> 
> 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"

[...]

> +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;
> +}
> +
> +int st_lsm6dsx_irq_setup(struct st_lsm6dsx_hw *hw)

it can be static

> +{
> +	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 err;

return 0 here or remove the previous one

> +}
> +
>  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
> 

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

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

* Re: [PATCH v3 2/6] iio: imu: st_lsm6dsx: add motion events
  2019-09-04  9:17 ` [PATCH v3 2/6] iio: imu: st_lsm6dsx: add motion events Sean Nyekjaer
@ 2019-09-04 17:24   ` Lorenzo Bianconi
  2019-09-05  5:29     ` Sean Nyekjaer
  0 siblings, 1 reply; 16+ messages in thread
From: Lorenzo Bianconi @ 2019-09-04 17:24 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: 10816 bytes --]

> Add event channels that controls the creation of motion events.
> 
> Signed-off-by: Sean Nyekjaer <sean@geanix.com>
> ---
> Changes since v1:
>  * added handling of LSM6
>  * added CHANNEL info with events for ACC
>  * removed st_lsm6dsx_set_event_threshold function
>  * added check of event type to event channels
> 
> Changes since v2:
>  * added devices specific registers to st_lsm6dsx_sensor_settings
>  * allow to write threshold when the events are disabled
> 
> How do we use the existing macro's when I want to expand them with
> events for the accelerometer only?
> 
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  43 +++++
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 156 +++++++++++++++++--
>  2 files changed, 190 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> index 5e3cd96b0059..10f3191e8695 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) |			\
> +			      BIT(IIO_CHAN_INFO_SCALE),			\
> +	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),	\


This patch is based on a old driver version:

commit 48cceecfa7625e700ed9c57392bd17923c4832c0
iio: imu: st_lsm6dsx: make IIO_CHAN_INFO_SCALE shared by type

> +	.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)		\

I guess you can rename ST_LSM6DSX_CHANNEL in ST_LSM6DSX_CHANNEL_GYRO now

>  {									\
>  	.type = chan_type,						\
> @@ -162,6 +183,13 @@ struct st_lsm6dsx_shub_settings {
>  	u8 batch_en;
>  };
>  
> +struct st_lsm6dsx_event_settings {
> +	u8 enable_addr;
> +	u8 enable_mask;
> +	u8 wakeup_addr;
> +	u8 wakeup_thres_mask;

you can rely on struct st_lsm6dsx_reg for them

> +};
> +
>  enum st_lsm6dsx_ext_sensor_id {
>  	ST_LSM6DSX_ID_MAGN,
>  };
> @@ -223,6 +251,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 +271,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 +352,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 +363,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 8e0da5eb4283..d4d6ba934b6d 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),
>  };
>  
> @@ -392,6 +392,9 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  		.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 +511,12 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  				.mask = GENMASK(5, 3),
>  			},
>  		},
> +		.event_settings = {
> +			.enable_addr = 0x58,
> +			.enable_mask = BIT(7),
> +			.wakeup_addr = 0x5B,
> +			.wakeup_thres_mask = GENMASK(5, 0),
> +		},
>  	},
>  	{
>  		.wai = 0x6c,
> @@ -1072,18 +1081,21 @@ static int st_lsm6dsx_read_oneshot(struct st_lsm6dsx_sensor *sensor,
>  	int err, delay;
>  	__le16 data;
>  
> -	err = st_lsm6dsx_sensor_set_enable(sensor, true);
> -	if (err < 0)
> -		return err;
> +	if (!hw->enable_event) {
> +		err = st_lsm6dsx_sensor_set_enable(sensor, true);
> +		if (err < 0)
> +			return err;
>  
> -	delay = 1000000 / sensor->odr;
> -	usleep_range(delay, 2 * delay);
> +		delay = 1000000 / sensor->odr;
> +		usleep_range(delay, 2 * delay);
> +	}
>  
>  	err = st_lsm6dsx_read_locked(hw, addr, &data, sizeof(data));
>  	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 +1168,124 @@ 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 (state)
> +		enable = hw->settings->event_settings.enable_mask;

enable = state ? hw->settings->event_settings.enable_mask : 0
> +
> +	err = regmap_update_bits(hw->regmap,
> +				 hw->settings->event_settings.enable_addr,
> +				 hw->settings->event_settings.enable_mask,
> +				 enable);
> +	if (err < 0)
> +		return err;
> +
> +	enable = 0;
> +	if (state)
> +		enable = hw->irq_routing.mask;

this is pretty ugly, why not doing:

enable = state ? hw->irq_routing.mask : 0

> +
> +	/* Enable wakeup interrupt */
> +	err = regmap_update_bits(hw->regmap, hw->irq_routing.addr,
> +				 hw->irq_routing.mask,
> +				 enable);
> +
> +	return err;
> +}
> +
> +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_addr,
> +				 hw->settings->event_settings.wakeup_thres_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 (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 +1370,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 +1419,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] 16+ messages in thread

* Re: [PATCH v3 3/6] iio: imu: st_lsm6dsx: add wakeup-source option
  2019-09-04  9:17 ` [PATCH v3 3/6] iio: imu: st_lsm6dsx: add wakeup-source option Sean Nyekjaer
@ 2019-09-04 17:27   ` Lorenzo Bianconi
  0 siblings, 0 replies; 16+ messages in thread
From: Lorenzo Bianconi @ 2019-09-04 17:27 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: 2237 bytes --]

On Sep 04, Sean Nyekjaer wrote:
> 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 v1:
>  * none, as the call to st_lsm6dsx_flush_fifo will put the fifo in
>    bypass mode.
> 
> Changes since v2:
>  * Removed unnecessary brackets
>  * Move enable_irq to after the enable mask
> 
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index d4d6ba934b6d..bb72800bf99b 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -1754,6 +1754,10 @@ int st_lsm6dsx_probe(struct device *dev, int irq, int hw_id,
>  			return err;
>  	}
>  
> +	if (dev->of_node)
> +		if (of_property_read_bool(dev->of_node, "wakeup-source"))
> +			device_init_wakeup(dev, true);

if (dev->of_node &&
    of_property_read_bool(dev->of_node, "wakeup-source"))
	device_init_wakeup(dev, true);

I guess it is more readable in this way, isn't it?

> +
>  	return 0;
>  }
>  EXPORT_SYMBOL(st_lsm6dsx_probe);
> @@ -1772,6 +1776,12 @@ static int __maybe_unused st_lsm6dsx_suspend(struct device *dev)
>  		if (!(hw->enable_mask & BIT(sensor->id)))
>  			continue;
>  
> +		if (device_may_wakeup(dev) && i == ST_LSM6DSX_ID_ACC) {
> +			/* 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)
> @@ -1804,6 +1814,11 @@ static int __maybe_unused st_lsm6dsx_resume(struct device *dev)
>  		if (!(hw->suspend_mask & BIT(sensor->id)))
>  			continue;
>  
> +		if (device_may_wakeup(dev) && i == ST_LSM6DSX_ID_ACC) {
> +			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
> 

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

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

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



On 04/09/2019 19.24, Lorenzo Bianconi wrote:

[..]

>> +	.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)		\
> 
> I guess you can rename ST_LSM6DSX_CHANNEL in ST_LSM6DSX_CHANNEL_GYRO now
> 

We still have the channels for the magnetometer, which are similar to 
the gyro channels :-)

>>   {									\
>>   	.type = chan_type,						\
>> @@ -162,6 +183,13 @@ struct st_lsm6dsx_shub_settings {
>>   	u8 batch_en;
>>   };
>>   

/Sean

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

* Re: [PATCH v3 4/6] iio: imu: st_lsm6dsx: always enter interrupt thread
  2019-09-04  9:17 ` [PATCH v3 4/6] iio: imu: st_lsm6dsx: always enter interrupt thread Sean Nyekjaer
@ 2019-09-05  6:34   ` Lorenzo Bianconi
  2019-09-05  6:37     ` Sean Nyekjaer
  0 siblings, 1 reply; 16+ messages in thread
From: Lorenzo Bianconi @ 2019-09-05  6:34 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: 1713 bytes --]

> 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>
> ---
> Changes since v1:
>  * None
> 
> Changes since v2:
>  * None
> 
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index bb72800bf99b..513506caa460 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -1613,19 +1613,19 @@ 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)
>  {
>  	struct st_lsm6dsx_hw *hw = private;
> -	int count;
> +	int count = 0;
>  
> -	mutex_lock(&hw->fifo_lock);
> -	count = hw->settings->fifo_ops.read_fifo(hw);
> -	mutex_unlock(&hw->fifo_lock);
> +	if (hw->sip > 0) {
> +		mutex_lock(&hw->fifo_lock);
> +		count = st_lsm6dsx_read_fifo(hw);
> +		mutex_unlock(&hw->fifo_lock);

This chunck is unnecessary since read_fifo() will check number of available
samples in the FIFO and moreover it is obviously broken since you are running
st_lsm6dsx_read_fifo directly instead of using fifo_ops pointer.

> +	}
>  
>  	return count ? IRQ_HANDLED : IRQ_NONE;
>  }
> -- 
> 2.23.0
> 

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

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

* Re: [PATCH v3 4/6] iio: imu: st_lsm6dsx: always enter interrupt thread
  2019-09-05  6:34   ` Lorenzo Bianconi
@ 2019-09-05  6:37     ` Sean Nyekjaer
  2019-09-05  6:47       ` Lorenzo Bianconi
  0 siblings, 1 reply; 16+ messages in thread
From: Sean Nyekjaer @ 2019-09-05  6:37 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: linux-iio, jic23, lorenzo.bianconi83, denis.ciocca, mario.tesi,
	armando.visconti, martin



On 05/09/2019 08.34, Lorenzo Bianconi wrote:
> This chunck is unnecessary since read_fifo() will check number of available
> samples in the FIFO and moreover it is obviously broken since you are running
> st_lsm6dsx_read_fifo directly instead of using fifo_ops pointer.

Thanks for the review :-)

I have missed that, I have spend a lot of time rebasing "iio: imu: 
st_lsm6dsx: move interrupt thread to core".
It could be nice if that patch could be applied in the near future :-)

/Sean

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

* Re: [PATCH v3 5/6] iio: imu: st_lsm6dsx: add motion report function and call from interrupt
  2019-09-04  9:17 ` [PATCH v3 5/6] iio: imu: st_lsm6dsx: add motion report function and call from interrupt Sean Nyekjaer
@ 2019-09-05  6:39   ` Lorenzo Bianconi
  2019-09-05  7:01     ` Sean Nyekjaer
  0 siblings, 1 reply; 16+ messages in thread
From: Lorenzo Bianconi @ 2019-09-05  6:39 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: 3394 bytes --]

> Report iio motion events to iio subsystem
> 
> Signed-off-by: Sean Nyekjaer <sean@geanix.com>
> ---
> Changes since v1:
>  * none
> 
> Changes since v2:
>  * none
> 
> Should we include these new defines in device settings?

nope if are the same for all available chips..have you double checked?

> 
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 53 ++++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index 513506caa460..2114c3c78888 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>
> @@ -72,6 +73,12 @@
>  #define ST_LSM6DSX_REG_PP_OD_ADDR		0x12
>  #define ST_LSM6DSX_REG_PP_OD_MASK		BIT(4)
>  
> +#define ST_LSM6DSX_REG_WAKE_UP_SRC_ADDR		0x1B
> +#define ST_LSM6DSX_REG_WAKE_UP_SRC_Z_WU_MASK	BIT(0)
> +#define ST_LSM6DSX_REG_WAKE_UP_SRC_Y_WU_MASK	BIT(1)
> +#define ST_LSM6DSX_REG_WAKE_UP_SRC_X_WU_MASK	BIT(2)
> +#define ST_LSM6DSX_REG_WAKE_UP_SRC_WU_MASK	BIT(4)
> +
>  static const struct iio_chan_spec st_lsm6dsx_acc_channels[] = {
>  	ST_LSM6DSX_CHANNEL_ACC(IIO_ACCEL, 0x28, IIO_MOD_X, 0),
>  	ST_LSM6DSX_CHANNEL_ACC(IIO_ACCEL, 0x2a, IIO_MOD_Y, 1),
> @@ -1611,6 +1618,40 @@ static struct iio_dev *st_lsm6dsx_alloc_iiodev(struct st_lsm6dsx_hw *hw,
>  	return iio_dev;
>  }
>  
> +int st_lsm6dsx_report_motion_event(struct st_lsm6dsx_hw *hw, int data)

void here

> +{
> +	s64 timestamp = iio_get_time_ns(hw->iio_devs[ST_LSM6DSX_ID_ACC]);
> +
> +	if (data & ST_LSM6DSX_REG_WAKE_UP_SRC_Z_WU_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 & ST_LSM6DSX_REG_WAKE_UP_SRC_Y_WU_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 & ST_LSM6DSX_REG_WAKE_UP_SRC_X_WU_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);
> +
> +	return 0;
> +}
> +
>  static irqreturn_t st_lsm6dsx_handler_irq(int irq, void *private)
>  {
>  	return IRQ_WAKE_THREAD;
> @@ -1620,7 +1661,19 @@ static irqreturn_t st_lsm6dsx_handler_thread(int irq, void *private)
>  {
>  	struct st_lsm6dsx_hw *hw = private;
>  	int count = 0;
> +	int data, err;
> +
> +	if (hw->enable_event) {
> +		err = regmap_read(hw->regmap,
> +				  ST_LSM6DSX_REG_WAKE_UP_SRC_ADDR, &data);
> +		if (err < 0)
> +			goto try_fifo;
> +
> +		if (data & ST_LSM6DSX_REG_WAKE_UP_SRC_WU_MASK)
> +			st_lsm6dsx_report_motion_event(hw, data);
> +	}
>  
> +try_fifo:
>  	if (hw->sip > 0) {
>  		mutex_lock(&hw->fifo_lock);
>  		count = st_lsm6dsx_read_fifo(hw);
> -- 
> 2.23.0
> 

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

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

* Re: [PATCH v3 4/6] iio: imu: st_lsm6dsx: always enter interrupt thread
  2019-09-05  6:37     ` Sean Nyekjaer
@ 2019-09-05  6:47       ` Lorenzo Bianconi
  0 siblings, 0 replies; 16+ messages in thread
From: Lorenzo Bianconi @ 2019-09-05  6:47 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: 736 bytes --]

> 
> 
> On 05/09/2019 08.34, Lorenzo Bianconi wrote:
> > This chunck is unnecessary since read_fifo() will check number of available
> > samples in the FIFO and moreover it is obviously broken since you are running
> > st_lsm6dsx_read_fifo directly instead of using fifo_ops pointer.
> 
> Thanks for the review :-)
> 
> I have missed that, I have spend a lot of time rebasing "iio: imu:
> st_lsm6dsx: move interrupt thread to core".
> It could be nice if that patch could be applied in the near future :-)
> 
> /Sean

I think Jonathan will prefer the patch to be part of the series where it is
actually needed (so up to him). I am fine even if you repost the single patch
adding requested changes

Regards,
Lorenzo

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

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

* Re: [PATCH v3 5/6] iio: imu: st_lsm6dsx: add motion report function and call from interrupt
  2019-09-05  6:39   ` Lorenzo Bianconi
@ 2019-09-05  7:01     ` Sean Nyekjaer
  2019-09-05  7:07       ` Lorenzo Bianconi
  0 siblings, 1 reply; 16+ messages in thread
From: Sean Nyekjaer @ 2019-09-05  7:01 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: linux-iio, jic23, lorenzo.bianconi83, denis.ciocca, mario.tesi,
	armando.visconti, martin



On 05/09/2019 08.39, Lorenzo Bianconi wrote:
>> Should we include these new defines in device settings?
> nope if are the same for all available chips..have you double checked?
> 

lsm6ds3: yes
lsm6ds3h: yes
lsm6dsl: yes
lsm6dsm: yes
ism330dlc: yes :-)
lsm6dso: yes
asm330lhh: yes
lsm6dsox: yes
lsm6dsr: yes
lsm6ds3tr-c: yes
ism330dhcx: yes
lsm9ds1_imu: no it have a very different reg layout

I would a lot more confidence in patch/work if we only enable these 
events for the devices i actually can test... (The ISM330DLC)

/Sean

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

* Re: [PATCH v3 5/6] iio: imu: st_lsm6dsx: add motion report function and call from interrupt
  2019-09-05  7:01     ` Sean Nyekjaer
@ 2019-09-05  7:07       ` Lorenzo Bianconi
  0 siblings, 0 replies; 16+ messages in thread
From: Lorenzo Bianconi @ 2019-09-05  7:07 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: 700 bytes --]

> 
> 
> On 05/09/2019 08.39, Lorenzo Bianconi wrote:
> > > Should we include these new defines in device settings?
> > nope if are the same for all available chips..have you double checked?
> > 
> 
> lsm6ds3: yes
> lsm6ds3h: yes
> lsm6dsl: yes
> lsm6dsm: yes
> ism330dlc: yes :-)
> lsm6dso: yes
> asm330lhh: yes
> lsm6dsox: yes
> lsm6dsr: yes
> lsm6ds3tr-c: yes
> ism330dhcx: yes
> lsm9ds1_imu: no it have a very different reg layout
> 
> I would a lot more confidence in patch/work if we only enable these events
> for the devices i actually can test... (The ISM330DLC)

in this case I would prefer to have the register in device settings

Regards,
Lorenzo

> 
> /Sean

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

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

end of thread, back to index

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-04  9:17 [PATCH v3 1/6] iio: imu: st_lsm6dsx: move interrupt thread to core Sean Nyekjaer
2019-09-04  9:17 ` [PATCH v3 2/6] iio: imu: st_lsm6dsx: add motion events Sean Nyekjaer
2019-09-04 17:24   ` Lorenzo Bianconi
2019-09-05  5:29     ` Sean Nyekjaer
2019-09-04  9:17 ` [PATCH v3 3/6] iio: imu: st_lsm6dsx: add wakeup-source option Sean Nyekjaer
2019-09-04 17:27   ` Lorenzo Bianconi
2019-09-04  9:17 ` [PATCH v3 4/6] iio: imu: st_lsm6dsx: always enter interrupt thread Sean Nyekjaer
2019-09-05  6:34   ` Lorenzo Bianconi
2019-09-05  6:37     ` Sean Nyekjaer
2019-09-05  6:47       ` Lorenzo Bianconi
2019-09-04  9:17 ` [PATCH v3 5/6] iio: imu: st_lsm6dsx: add motion report function and call from interrupt Sean Nyekjaer
2019-09-05  6:39   ` Lorenzo Bianconi
2019-09-05  7:01     ` Sean Nyekjaer
2019-09-05  7:07       ` Lorenzo Bianconi
2019-09-04  9:17 ` [PATCH v3 6/6] iio: imu: st_lsm6dsx: prohibit the use of events and buffered reads simultaneously Sean Nyekjaer
2019-09-04 17:11 ` [PATCH v3 1/6] iio: imu: st_lsm6dsx: move interrupt thread to core Lorenzo Bianconi

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