Linux-IIO Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v4 1/6] iio: imu: st_lsm6dsx: move interrupt thread to core
@ 2019-09-06 12:17 Sean Nyekjaer
  2019-09-06 12:17 ` [PATCH v4 2/6] iio: imu: st_lsm6dsx: add motion events Sean Nyekjaer
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Sean Nyekjaer @ 2019-09-06 12: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>
---
 .../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] 14+ messages in thread

* [PATCH v4 2/6] iio: imu: st_lsm6dsx: add motion events
  2019-09-06 12:17 [PATCH v4 1/6] iio: imu: st_lsm6dsx: move interrupt thread to core Sean Nyekjaer
@ 2019-09-06 12:17 ` Sean Nyekjaer
  2019-09-06 14:02   ` Lorenzo Bianconi
  2019-09-06 12:17 ` [PATCH v4 3/6] iio: imu: st_lsm6dsx: add wakeup-source option Sean Nyekjaer
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Sean Nyekjaer @ 2019-09-06 12: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 v3:
 * based channel struct on newer driver
 * use st_lsm6dsx_reg for relevant values

 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  41 +++++
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 157 +++++++++++++++++--
 2 files changed, 189 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..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..7761ee5b1ec7 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,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,
@@ -1072,18 +1085,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 +1172,121 @@ 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;
+
+	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 */
+	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_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 (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 +1371,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 +1420,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] 14+ messages in thread

* [PATCH v4 3/6] iio: imu: st_lsm6dsx: add wakeup-source option
  2019-09-06 12:17 [PATCH v4 1/6] iio: imu: st_lsm6dsx: move interrupt thread to core Sean Nyekjaer
  2019-09-06 12:17 ` [PATCH v4 2/6] iio: imu: st_lsm6dsx: add motion events Sean Nyekjaer
@ 2019-09-06 12:17 ` Sean Nyekjaer
  2019-09-06 12:17 ` [PATCH v4 4/6] iio: imu: st_lsm6dsx: always enter interrupt thread Sean Nyekjaer
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Sean Nyekjaer @ 2019-09-06 12: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>
---
 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 7761ee5b1ec7..2dda0f90a816 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -1755,6 +1755,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);
@@ -1773,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)
@@ -1805,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] 14+ messages in thread

* [PATCH v4 4/6] iio: imu: st_lsm6dsx: always enter interrupt thread
  2019-09-06 12:17 [PATCH v4 1/6] iio: imu: st_lsm6dsx: move interrupt thread to core Sean Nyekjaer
  2019-09-06 12:17 ` [PATCH v4 2/6] iio: imu: st_lsm6dsx: add motion events Sean Nyekjaer
  2019-09-06 12:17 ` [PATCH v4 3/6] iio: imu: st_lsm6dsx: add wakeup-source option Sean Nyekjaer
@ 2019-09-06 12:17 ` Sean Nyekjaer
  2019-09-06 12:17 ` [PATCH v4 5/6] iio: imu: st_lsm6dsx: add motion report function and call from interrupt Sean Nyekjaer
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Sean Nyekjaer @ 2019-09-06 12: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 v3:
 * Removed unnecessary chunk in handler_irq

 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 2dda0f90a816..ce467308eadd 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -1614,9 +1614,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] 14+ messages in thread

* [PATCH v4 5/6] iio: imu: st_lsm6dsx: add motion report function and call from interrupt
  2019-09-06 12:17 [PATCH v4 1/6] iio: imu: st_lsm6dsx: move interrupt thread to core Sean Nyekjaer
                   ` (2 preceding siblings ...)
  2019-09-06 12:17 ` [PATCH v4 4/6] iio: imu: st_lsm6dsx: always enter interrupt thread Sean Nyekjaer
@ 2019-09-06 12:17 ` Sean Nyekjaer
  2019-09-06 14:04   ` Lorenzo Bianconi
  2019-09-07 11:08   ` Jonathan Cameron
  2019-09-06 12:17 ` [PATCH v4 6/6] iio: imu: st_lsm6dsx: prohibit the use of events and buffered reads simultaneously Sean Nyekjaer
  2019-09-06 13:01 ` [PATCH v4 1/6] iio: imu: st_lsm6dsx: move interrupt thread to core Lorenzo Bianconi
  5 siblings, 2 replies; 14+ messages in thread
From: Sean Nyekjaer @ 2019-09-06 12: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 v3:
 * added wakeup_src_reg and masks to device settings

 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  5 ++
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 51 ++++++++++++++++++++
 2 files changed, 56 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 ce467308eadd..470821b54933 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>
@@ -520,6 +521,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(4),
+			.wakeup_src_z_mask = BIT(0),
+			.wakeup_src_y_mask = BIT(1),
+			.wakeup_src_x_mask = BIT(2),
 		},
 	},
 	{
@@ -1612,6 +1618,40 @@ 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);
+
+	return;
+}
+
 static irqreturn_t st_lsm6dsx_handler_irq(int irq, void *private)
 {
 	return IRQ_WAKE_THREAD;
@@ -1621,6 +1661,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] 14+ messages in thread

* [PATCH v4 6/6] iio: imu: st_lsm6dsx: prohibit the use of events and buffered reads simultaneously
  2019-09-06 12:17 [PATCH v4 1/6] iio: imu: st_lsm6dsx: move interrupt thread to core Sean Nyekjaer
                   ` (3 preceding siblings ...)
  2019-09-06 12:17 ` [PATCH v4 5/6] iio: imu: st_lsm6dsx: add motion report function and call from interrupt Sean Nyekjaer
@ 2019-09-06 12:17 ` Sean Nyekjaer
  2019-09-07 10:54   ` Jonathan Cameron
  2019-09-06 13:01 ` [PATCH v4 1/6] iio: imu: st_lsm6dsx: move interrupt thread to core Lorenzo Bianconi
  5 siblings, 1 reply; 14+ messages in thread
From: Sean Nyekjaer @ 2019-09-06 12: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 470821b54933..fdc44ff9601b 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] 14+ messages in thread

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

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

Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>

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

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

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

* Re: [PATCH v4 2/6] iio: imu: st_lsm6dsx: add motion events
  2019-09-06 12:17 ` [PATCH v4 2/6] iio: imu: st_lsm6dsx: add motion events Sean Nyekjaer
@ 2019-09-06 14:02   ` Lorenzo Bianconi
  2019-09-06 14:13     ` Lorenzo Bianconi
  0 siblings, 1 reply; 14+ messages in thread
From: Lorenzo Bianconi @ 2019-09-06 14:02 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: 7076 bytes --]

> Add event channels that controls the creation of motion events.
> 
> Signed-off-by: Sean Nyekjaer <sean@geanix.com>
> ---
> Changes since v3:
>  * based channel struct on newer driver
>  * use st_lsm6dsx_reg for relevant values
> 
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  41 +++++
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 157 +++++++++++++++++--
>  2 files changed, 189 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..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))
>  

[...]

> @@ -508,6 +511,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,
> @@ -1072,18 +1085,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) {

I think we do not need this since it is ok if the acc is already enable right?
Just check to not disable it

> +		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 +1172,121 @@ 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;
> +
> +	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 */
> +	err = regmap_update_bits(hw->regmap, hw->irq_routing.addr,
> +				 hw->irq_routing.mask,
> +				 enable);

return 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_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 (state && hw->enable_event)
> +		return 0;

This does not make a lot of sense to me since you just check to not enable it
if it has been already done. Moreover you need to check if the particular
sensor supports this event (AFAIU just one sensor does right?)

> +
> +	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 +1371,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 +1420,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] 14+ messages in thread

* Re: [PATCH v4 5/6] iio: imu: st_lsm6dsx: add motion report function and call from interrupt
  2019-09-06 12:17 ` [PATCH v4 5/6] iio: imu: st_lsm6dsx: add motion report function and call from interrupt Sean Nyekjaer
@ 2019-09-06 14:04   ` Lorenzo Bianconi
  2019-09-07 11:08   ` Jonathan Cameron
  1 sibling, 0 replies; 14+ messages in thread
From: Lorenzo Bianconi @ 2019-09-06 14:04 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: 3749 bytes --]

> Report iio motion events to iio subsystem
> 
> Signed-off-by: Sean Nyekjaer <sean@geanix.com>
> ---
> Changes since v3:
>  * added wakeup_src_reg and masks to device settings
> 
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  5 ++
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 51 ++++++++++++++++++++
>  2 files changed, 56 insertions(+)
> 

nitpick inline

> 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 ce467308eadd..470821b54933 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>
> @@ -520,6 +521,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(4),
> +			.wakeup_src_z_mask = BIT(0),
> +			.wakeup_src_y_mask = BIT(1),
> +			.wakeup_src_x_mask = BIT(2),
>  		},
>  	},
>  	{
> @@ -1612,6 +1618,40 @@ 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);
> +
> +	return;

useless

> +}
> +
>  static irqreturn_t st_lsm6dsx_handler_irq(int irq, void *private)
>  {
>  	return IRQ_WAKE_THREAD;
> @@ -1621,6 +1661,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
> 

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

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

* Re: [PATCH v4 2/6] iio: imu: st_lsm6dsx: add motion events
  2019-09-06 14:02   ` Lorenzo Bianconi
@ 2019-09-06 14:13     ` Lorenzo Bianconi
  2019-09-09  6:41       ` Sean Nyekjaer
  0 siblings, 1 reply; 14+ messages in thread
From: Lorenzo Bianconi @ 2019-09-06 14:13 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: 7702 bytes --]

On Sep 06, Lorenzo Bianconi wrote:
> > Add event channels that controls the creation of motion events.
> > 
> > Signed-off-by: Sean Nyekjaer <sean@geanix.com>
> > ---
> > Changes since v3:
> >  * based channel struct on newer driver
> >  * use st_lsm6dsx_reg for relevant values
> > 
> >  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  41 +++++
> >  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 157 +++++++++++++++++--
> >  2 files changed, 189 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..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))
> >  
> 
> [...]
> 
> > @@ -508,6 +511,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,
> > @@ -1072,18 +1085,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) {
> 
> I think we do not need this since it is ok if the acc is already enable right?
> Just check to not disable it
> 
> > +		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 +1172,121 @@ 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;

here you do something like:

	if (!hw->settings->event_settings.enable_reg.addr)
		return -ENOSUPP;

> > +
> > +	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 */
> > +	err = regmap_update_bits(hw->regmap, hw->irq_routing.addr,
> > +				 hw->irq_routing.mask,
> > +				 enable);
> 
> return 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_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 (state && hw->enable_event)
> > +		return 0;
> 
> This does not make a lot of sense to me since you just check to not enable it
> if it has been already done. Moreover you need to check if the particular
> sensor supports this event (AFAIU just one sensor does right?)
> 
> > +
> > +	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 +1371,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 +1420,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] 14+ messages in thread

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

On Fri,  6 Sep 2019 14:17:16 +0200
Sean Nyekjaer <sean@geanix.com> wrote:

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

This strikes me as racey.  What stops you getting past the check and before
you get to the next line of code, an event is enabled? 

More than likely I'm missing a reason that can't happen, but a comment
here to explain why will make it immediately obvious!

>  	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 470821b54933..fdc44ff9601b 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;

Again, looks superficially racey.

> +
>  	if (state && hw->enable_event)
>  		return 0;
>  


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

* Re: [PATCH v4 5/6] iio: imu: st_lsm6dsx: add motion report function and call from interrupt
  2019-09-06 12:17 ` [PATCH v4 5/6] iio: imu: st_lsm6dsx: add motion report function and call from interrupt Sean Nyekjaer
  2019-09-06 14:04   ` Lorenzo Bianconi
@ 2019-09-07 11:08   ` Jonathan Cameron
  2019-09-09  6:21     ` Sean Nyekjaer
  1 sibling, 1 reply; 14+ messages in thread
From: Jonathan Cameron @ 2019-09-07 11:08 UTC (permalink / raw)
  To: Sean Nyekjaer
  Cc: linux-iio, lorenzo.bianconi83, denis.ciocca, mario.tesi,
	armando.visconti, martin

On Fri,  6 Sep 2019 14:17:15 +0200
Sean Nyekjaer <sean@geanix.com> wrote:

> Report iio motion events to iio subsystem
> 
> Signed-off-by: Sean Nyekjaer <sean@geanix.com>

I'm confused by a disagreement between the bit mask you have here
and what I'm understanding from the datasheets..

> ---
> Changes since v3:
>  * added wakeup_src_reg and masks to device settings
> 
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  5 ++
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 51 ++++++++++++++++++++
>  2 files changed, 56 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 ce467308eadd..470821b54933 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>
> @@ -520,6 +521,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(4),

Bit 4?  I just checked the LSM6DS3 data sheet and the
Wakeup Event Detection status is bit 3.
Bit 4 is the sleep event status...
Also checked the LSM6DSL.

> +			.wakeup_src_z_mask = BIT(0),
> +			.wakeup_src_y_mask = BIT(1),
> +			.wakeup_src_x_mask = BIT(2),
>  		},
>  	},
>  	{
> @@ -1612,6 +1618,40 @@ 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);
> +
> +	return;
> +}
> +
>  static irqreturn_t st_lsm6dsx_handler_irq(int irq, void *private)
>  {
>  	return IRQ_WAKE_THREAD;
> @@ -1621,6 +1661,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);


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

* Re: [PATCH v4 5/6] iio: imu: st_lsm6dsx: add motion report function and call from interrupt
  2019-09-07 11:08   ` Jonathan Cameron
@ 2019-09-09  6:21     ` Sean Nyekjaer
  0 siblings, 0 replies; 14+ messages in thread
From: Sean Nyekjaer @ 2019-09-09  6:21 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-iio, lorenzo.bianconi83, denis.ciocca, mario.tesi,
	armando.visconti, martin



On 07/09/2019 13.08, Jonathan Cameron wrote:
>>   			},
>> +			.wakeup_src_reg = 0x1b,
>> +			.wakeup_src_status_mask = BIT(4),
> Bit 4?  I just checked the LSM6DS3 data sheet and the
> Wakeup Event Detection status is bit 3.
> Bit 4 is the sleep event status...
> Also checked the LSM6DSL.
> 
Hi Jonathan

You are right :-)
Can't quite understand why it worked with bit 4, it still works by 
checking the correct BIT(3)

/Sean

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

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



On 06/09/2019 16.13, Lorenzo Bianconi wrote:
>>> +int st_lsm6dsx_event_setup(struct st_lsm6dsx_hw *hw, int state)
>>> +{
>>> +	int err;
>>> +	u8 enable = 0;
> here you do something like:
> 
> 	if (!hw->settings->event_settings.enable_reg.addr)
> 		return -ENOSUPP;
> 

Here we need to do something else the LSM6DS3 doesn't feature this 
enable register.
My best guess it's enough to enable the routing of the interrupt in the 
MD register

/Sean

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

end of thread, back to index

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-06 12:17 [PATCH v4 1/6] iio: imu: st_lsm6dsx: move interrupt thread to core Sean Nyekjaer
2019-09-06 12:17 ` [PATCH v4 2/6] iio: imu: st_lsm6dsx: add motion events Sean Nyekjaer
2019-09-06 14:02   ` Lorenzo Bianconi
2019-09-06 14:13     ` Lorenzo Bianconi
2019-09-09  6:41       ` Sean Nyekjaer
2019-09-06 12:17 ` [PATCH v4 3/6] iio: imu: st_lsm6dsx: add wakeup-source option Sean Nyekjaer
2019-09-06 12:17 ` [PATCH v4 4/6] iio: imu: st_lsm6dsx: always enter interrupt thread Sean Nyekjaer
2019-09-06 12:17 ` [PATCH v4 5/6] iio: imu: st_lsm6dsx: add motion report function and call from interrupt Sean Nyekjaer
2019-09-06 14:04   ` Lorenzo Bianconi
2019-09-07 11:08   ` Jonathan Cameron
2019-09-09  6:21     ` Sean Nyekjaer
2019-09-06 12:17 ` [PATCH v4 6/6] iio: imu: st_lsm6dsx: prohibit the use of events and buffered reads simultaneously Sean Nyekjaer
2019-09-07 10:54   ` Jonathan Cameron
2019-09-06 13:01 ` [PATCH v4 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