[PATCH v7 1/2] iio:imu: inv_mpu6050: support more interrupt types

[PATCH v7 1/2] iio:imu: inv_mpu6050: support more interrupt types

[Martin Kelly]

Currently, we support only rising edge interrupts, and in fact we assume
that the interrupt we're given is rising edge (and things won't work if
it's not). However, the device supports rising edge, falling edge, level
low, and level high interrupts.

Empirically, on my system, switching to level interrupts has fixed a
problem I had with significant (~40%) interrupt loss with edge
interrupts. This issue is likely related to the SoC I'm using (Allwinner
H3), but being able to switch the interrupt type is still a very useful
workaround.

I tested this with each interrupt type and verified correct behavior in
a logic analyzer.

Add support for these interrupt types while also eliminating the error
case of the device tree and driver using different interrupt types.

Signed-off-by: Martin Kelly <mkelly@xevo.com>
---
v7:
- Actually flush the FIFO when we fail to ACK the interrupt, and end the session
  when we get a spurious interrupt.
v6:
- Use -EINVAL if devicetree interrupt configuration is missing.
- Use u8 for irq_mask.
- Stop mixing register and bit defines, and group them separately.
- If we fail to ACK the interrupt, flush the FIFO.
v5:
- Check interrupt status in all cases rather than only in the level triggered
   case, and warn if we get spurious interrupts.
- Rename INV_MPU6050_LEVEL_TRIGGERED to INV_MPU6050_LATCH_INT_EN to match
   datasheet naming.
- Write st->irq_mask prior to device power off to make sure it is fully set.
- Write st->irq_mask instead of 0 in inv_mpu6050_deselect_bypass.
- Make irq_type local instead of part of the driver state, as we use it only at
   probe time and never again.
- Remove the comment about bus lockups, as I have determined them to be
   unrelated.
- Add missing documentation for irq_type and irq_mask.
v4:
- Moved the ACK inside the mutex.
v3:
- Sent version 2 too quickly. Now that the ACK is moved to the top of the
   function, the "goto out" logic is unnecessary, so we can clean it up.
v2:
- Changed to ACK level interrupts at the start of the bottom half thread instead
   of at the end of it. Without this, the sample timestamps get distorted because
   the time to handle the bottom half thread delays future interrupts. With this
   change, the timestamps appear evenly distributed at the right frequency.

 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c    | 33 ++++++++++++++++++++++++++-
 drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c     |  5 ++--
 drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h     | 15 ++++++++++--
 drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c    | 15 ++++++++++++
 drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c |  4 ++--
 5 files changed, 64 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index 7d64be353403..c2cec7508451 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -24,6 +24,7 @@
 #include <linux/spinlock.h>
 #include <linux/iio/iio.h>
 #include <linux/acpi.h>
+#include <linux/platform_device.h>
 #include "inv_mpu_iio.h"
 
 /*
@@ -52,6 +53,7 @@ static const struct inv_mpu6050_reg_map reg_set_6500 = {
 	.raw_accl               = INV_MPU6050_REG_RAW_ACCEL,
 	.temperature            = INV_MPU6050_REG_TEMPERATURE,
 	.int_enable             = INV_MPU6050_REG_INT_ENABLE,
+	.int_status             = INV_MPU6050_REG_INT_STATUS,
 	.pwr_mgmt_1             = INV_MPU6050_REG_PWR_MGMT_1,
 	.pwr_mgmt_2             = INV_MPU6050_REG_PWR_MGMT_2,
 	.int_pin_cfg		= INV_MPU6050_REG_INT_PIN_CFG,
@@ -278,6 +280,10 @@ static int inv_mpu6050_init_config(struct iio_dev *indio_dev)
 	if (result)
 		return result;
 
+	result = regmap_write(st->map, st->reg->int_pin_cfg, st->irq_mask);
+	if (result)
+		return result;
+
 	memcpy(&st->chip_config, hw_info[st->chip_type].config,
 	       sizeof(struct inv_mpu6050_chip_config));
 	result = inv_mpu6050_set_power_itg(st, false);
@@ -882,6 +888,8 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
 	struct inv_mpu6050_platform_data *pdata;
 	struct device *dev = regmap_get_device(regmap);
 	int result;
+	struct irq_data *desc;
+	int irq_type;
 
 	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
 	if (!indio_dev)
@@ -913,6 +921,29 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
 		st->plat_data = *pdata;
 	}
 
+	desc = irq_get_irq_data(irq);
+	if (!desc) {
+		dev_err(dev, "Could not find IRQ %d\n", irq);
+		return -EINVAL;
+	}
+
+	irq_type = irqd_get_trigger_type(desc);
+	if (irq_type == IRQF_TRIGGER_RISING)
+		st->irq_mask = INV_MPU6050_ACTIVE_HIGH;
+	else if (irq_type == IRQF_TRIGGER_FALLING)
+		st->irq_mask = INV_MPU6050_ACTIVE_LOW;
+	else if (irq_type == IRQF_TRIGGER_HIGH)
+		st->irq_mask = INV_MPU6050_ACTIVE_HIGH |
+			INV_MPU6050_LATCH_INT_EN;
+	else if (irq_type == IRQF_TRIGGER_LOW)
+		st->irq_mask = INV_MPU6050_ACTIVE_LOW |
+			INV_MPU6050_LATCH_INT_EN;
+	else {
+		dev_err(dev, "Invalid interrupt type 0x%x specified\n",
+			irq_type);
+		return -EINVAL;
+	}
+
 	/* power is turned on inside check chip type*/
 	result = inv_check_and_setup_chip(st);
 	if (result)
@@ -948,7 +979,7 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
 		dev_err(dev, "configure buffer fail %d\n", result);
 		return result;
 	}
-	result = inv_mpu6050_probe_trigger(indio_dev);
+	result = inv_mpu6050_probe_trigger(indio_dev, irq_type);
 	if (result) {
 		dev_err(dev, "trigger probe fail %d\n", result);
 		goto out_unreg_ring;
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
index fcd7a92b6cf8..1b02d2b69174 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
@@ -44,8 +44,7 @@ static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id)
 	if (!ret) {
 		st->powerup_count++;
 		ret = regmap_write(st->map, st->reg->int_pin_cfg,
-				   INV_MPU6050_INT_PIN_CFG |
-				   INV_MPU6050_BIT_BYPASS_EN);
+			   st->irq_mask | INV_MPU6050_BIT_BYPASS_EN);
 	}
 write_error:
 	mutex_unlock(&st->lock);
@@ -60,7 +59,7 @@ static int inv_mpu6050_deselect_bypass(struct i2c_mux_core *muxc, u32 chan_id)
 
 	mutex_lock(&st->lock);
 	/* It doesn't really mattter, if any of the calls fails */
-	regmap_write(st->map, st->reg->int_pin_cfg, INV_MPU6050_INT_PIN_CFG);
+	regmap_write(st->map, st->reg->int_pin_cfg, st->irq_mask);
 	st->powerup_count--;
 	if (!st->powerup_count)
 		regmap_write(st->map, st->reg->pwr_mgmt_1,
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
index 065794162d65..eaa9158ac753 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
@@ -40,6 +40,7 @@
  *  @raw_accl:		Address of first accel register.
  *  @temperature:	temperature register
  *  @int_enable:	Interrupt enable register.
+ *  @int_status:	Interrupt status register.
  *  @pwr_mgmt_1:	Controls chip's power state and clock source.
  *  @pwr_mgmt_2:	Controls power state of individual sensors.
  *  @int_pin_cfg;	Controls interrupt pin configuration.
@@ -60,6 +61,7 @@ struct inv_mpu6050_reg_map {
 	u8 raw_accl;
 	u8 temperature;
 	u8 int_enable;
+	u8 int_status;
 	u8 pwr_mgmt_1;
 	u8 pwr_mgmt_2;
 	u8 int_pin_cfg;
@@ -125,6 +127,7 @@ struct inv_mpu6050_hw {
  *  @timestamps:        kfifo queue to store time stamp.
  *  @map		regmap pointer.
  *  @irq		interrupt number.
+ *  @irq_mask		the int_pin_cfg mask to configure interrupt type.
  */
 struct inv_mpu6050_state {
 #define TIMESTAMP_FIFO_SIZE 16
@@ -143,6 +146,7 @@ struct inv_mpu6050_state {
 	DECLARE_KFIFO(timestamps, long long, TIMESTAMP_FIFO_SIZE);
 	struct regmap *map;
 	int irq;
+	u8 irq_mask;
 };
 
 /*register and associated bit definition*/
@@ -166,6 +170,9 @@ struct inv_mpu6050_state {
 #define INV_MPU6050_REG_TEMPERATURE         0x41
 #define INV_MPU6050_REG_RAW_GYRO            0x43
 
+#define INV_MPU6050_REG_INT_STATUS          0x3A
+#define INV_MPU6050_BIT_RAW_DATA_RDY_INT    0x01
+
 #define INV_MPU6050_REG_USER_CTRL           0x6A
 #define INV_MPU6050_BIT_FIFO_RST            0x04
 #define INV_MPU6050_BIT_DMP_RST             0x08
@@ -215,8 +222,12 @@ struct inv_mpu6050_state {
 #define INV_MPU6050_OUTPUT_DATA_SIZE         24
 
 #define INV_MPU6050_REG_INT_PIN_CFG	0x37
+#define INV_MPU6050_ACTIVE_HIGH		0x00
+#define INV_MPU6050_ACTIVE_LOW		0x80
+/* enable level triggering */
+#define INV_MPU6050_LATCH_INT_EN	0x20
 #define INV_MPU6050_BIT_BYPASS_EN	0x2
-#define INV_MPU6050_INT_PIN_CFG		0
+
 
 /* init parameters */
 #define INV_MPU6050_INIT_FIFO_RATE           50
@@ -287,7 +298,7 @@ enum inv_mpu6050_clock_sel_e {
 
 irqreturn_t inv_mpu6050_irq_handler(int irq, void *p);
 irqreturn_t inv_mpu6050_read_fifo(int irq, void *p);
-int inv_mpu6050_probe_trigger(struct iio_dev *indio_dev);
+int inv_mpu6050_probe_trigger(struct iio_dev *indio_dev, int irq_type);
 void inv_mpu6050_remove_trigger(struct inv_mpu6050_state *st);
 int inv_reset_fifo(struct iio_dev *indio_dev);
 int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask);
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
index ff81c6aa009d..672c3df2d1d1 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
@@ -127,8 +127,23 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
 	u8 data[INV_MPU6050_OUTPUT_DATA_SIZE];
 	u16 fifo_count;
 	s64 timestamp;
+	int int_status;
 
 	mutex_lock(&st->lock);
+
+	/* ack interrupt and check status */
+	result = regmap_read(st->map, st->reg->int_status, &int_status);
+	if (result) {
+		dev_err(regmap_get_device(st->map),
+			"failed to ack interrupt\n");
+		goto flush_fifo;
+	}
+	if (!(int_status & INV_MPU6050_BIT_RAW_DATA_RDY_INT)) {
+		dev_warn(regmap_get_device(st->map),
+			"spurious interrupt with status 0x%x\n", int_status);
+		goto end_session;
+	}
+
 	if (!(st->chip_config.accl_fifo_enable |
 		st->chip_config.gyro_fifo_enable))
 		goto end_session;
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
index f963f9fc98c0..b8c5584e4252 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
@@ -117,7 +117,7 @@ static const struct iio_trigger_ops inv_mpu_trigger_ops = {
 	.set_trigger_state = &inv_mpu_data_rdy_trigger_set_state,
 };
 
-int inv_mpu6050_probe_trigger(struct iio_dev *indio_dev)
+int inv_mpu6050_probe_trigger(struct iio_dev *indio_dev, int irq_type)
 {
 	int ret;
 	struct inv_mpu6050_state *st = iio_priv(indio_dev);
@@ -131,7 +131,7 @@ int inv_mpu6050_probe_trigger(struct iio_dev *indio_dev)
 
 	ret = devm_request_irq(&indio_dev->dev, st->irq,
 			       &iio_trigger_generic_data_rdy_poll,
-			       IRQF_TRIGGER_RISING,
+			       irq_type,
 			       "inv_mpu",
 			       st->trig);
 	if (ret)
-- 
2.11.0

[PATCH v7 2/2] dt-bindings: iio:imu:mpu6050: support more interrupt types

[Martin Kelly]

Document that the hardware supports falling edge, rising edge, level
low, and level high interrupt types, rather than just rising edge.

The language used is the same as that in st_lsm6dsx.txt.

Signed-off-by: Martin Kelly <mkelly@xevo.com>
---
 Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt b/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
index 2b4514592f83..6b106d5ef298 100644
--- a/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
+++ b/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
@@ -11,7 +11,12 @@ Required properties:
 		"invensense,icm20608"
  - reg : the I2C address of the sensor
  - interrupt-parent : should be the phandle for the interrupt controller
- - interrupts : interrupt mapping for GPIO IRQ
+ - interrupts: interrupt mapping for IRQ. It should be configured with flags
+   IRQ_TYPE_LEVEL_HIGH, IRQ_TYPE_EDGE_RISING, IRQ_TYPE_LEVEL_LOW or
+   IRQ_TYPE_EDGE_FALLING.
+
+  Refer to interrupt-controller/interrupts.txt for generic interrupt client node
+  bindings.
 
 Optional properties:
  - mount-matrix: an optional 3x3 mounting rotation matrix
@@ -24,7 +29,7 @@ Example:
 		compatible = "invensense,mpu6050";
 		reg = <0x68>;
 		interrupt-parent = <&gpio1>;
-		interrupts = <18 1>;
+		interrupts = <18 IRQ_TYPE_EDGE_RISING>;
 		mount-matrix = "-0.984807753012208",  /* x0 */
 		               "0",                   /* y0 */
 		               "-0.173648177666930",  /* z0 */
@@ -41,7 +46,7 @@ Example:
 		compatible = "invensense,mpu9250";
 		reg = <0x68>;
 		interrupt-parent = <&gpio3>;
-		interrupts = <21 1>;
+		interrupts = <21 IRQ_TYPE_LEVEL_HIGH>;
 		i2c-gate {
 			#address-cells = <1>;
 			#size-cells = <0>;
-- 
2.11.0

Re: [PATCH v7 1/2] iio:imu: inv_mpu6050: support more interrupt types

[Jean-Baptiste Maneyrol]

On 20/04/2018 18:54, Martin Kelly wrote:
> Currently, we support only rising edge interrupts, and in fact we assume
> that the interrupt we're given is rising edge (and things won't work if
> it's not). However, the device supports rising edge, falling edge, level
> low, and level high interrupts.
> 
> Empirically, on my system, switching to level interrupts has fixed a
> problem I had with significant (~40%) interrupt loss with edge
> interrupts. This issue is likely related to the SoC I'm using (Allwinner
> H3), but being able to switch the interrupt type is still a very useful
> workaround.
> 
> I tested this with each interrupt type and verified correct behavior in
> a logic analyzer.
> 
> Add support for these interrupt types while also eliminating the error
> case of the device tree and driver using different interrupt types.
> 
> Signed-off-by: Martin Kelly <mkelly@xevo.com>
> ---
> v7:
> - Actually flush the FIFO when we fail to ACK the interrupt, and end the session
>    when we get a spurious interrupt.
> v6:
> - Use -EINVAL if devicetree interrupt configuration is missing.
> - Use u8 for irq_mask.
> - Stop mixing register and bit defines, and group them separately.
> - If we fail to ACK the interrupt, flush the FIFO.
> v5:
> - Check interrupt status in all cases rather than only in the level triggered
>     case, and warn if we get spurious interrupts.
> - Rename INV_MPU6050_LEVEL_TRIGGERED to INV_MPU6050_LATCH_INT_EN to match
>     datasheet naming.
> - Write st->irq_mask prior to device power off to make sure it is fully set.
> - Write st->irq_mask instead of 0 in inv_mpu6050_deselect_bypass.
> - Make irq_type local instead of part of the driver state, as we use it only at
>     probe time and never again.
> - Remove the comment about bus lockups, as I have determined them to be
>     unrelated.
> - Add missing documentation for irq_type and irq_mask.
> v4:
> - Moved the ACK inside the mutex.
> v3:
> - Sent version 2 too quickly. Now that the ACK is moved to the top of the
>     function, the "goto out" logic is unnecessary, so we can clean it up.
> v2:
> - Changed to ACK level interrupts at the start of the bottom half thread instead
>     of at the end of it. Without this, the sample timestamps get distorted because
>     the time to handle the bottom half thread delays future interrupts. With this
>     change, the timestamps appear evenly distributed at the right frequency.
> 
>   drivers/iio/imu/inv_mpu6050/inv_mpu_core.c    | 33 ++++++++++++++++++++++++++-
>   drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c     |  5 ++--
>   drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h     | 15 ++++++++++--
>   drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c    | 15 ++++++++++++
>   drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c |  4 ++--
>   5 files changed, 64 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index 7d64be353403..c2cec7508451 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -24,6 +24,7 @@
>   #include <linux/spinlock.h>
>   #include <linux/iio/iio.h>
>   #include <linux/acpi.h>
> +#include <linux/platform_device.h>
>   #include "inv_mpu_iio.h"
>   
>   /*
> @@ -52,6 +53,7 @@ static const struct inv_mpu6050_reg_map reg_set_6500 = {
>   	.raw_accl               = INV_MPU6050_REG_RAW_ACCEL,
>   	.temperature            = INV_MPU6050_REG_TEMPERATURE,
>   	.int_enable             = INV_MPU6050_REG_INT_ENABLE,
> +	.int_status             = INV_MPU6050_REG_INT_STATUS,
>   	.pwr_mgmt_1             = INV_MPU6050_REG_PWR_MGMT_1,
>   	.pwr_mgmt_2             = INV_MPU6050_REG_PWR_MGMT_2,
>   	.int_pin_cfg		= INV_MPU6050_REG_INT_PIN_CFG,
> @@ -278,6 +280,10 @@ static int inv_mpu6050_init_config(struct iio_dev *indio_dev)
>   	if (result)
>   		return result;
>   
> +	result = regmap_write(st->map, st->reg->int_pin_cfg, st->irq_mask);
> +	if (result)
> +		return result;
> +
>   	memcpy(&st->chip_config, hw_info[st->chip_type].config,
>   	       sizeof(struct inv_mpu6050_chip_config));
>   	result = inv_mpu6050_set_power_itg(st, false);
> @@ -882,6 +888,8 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
>   	struct inv_mpu6050_platform_data *pdata;
>   	struct device *dev = regmap_get_device(regmap);
>   	int result;
> +	struct irq_data *desc;
> +	int irq_type;
>   
>   	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
>   	if (!indio_dev)
> @@ -913,6 +921,29 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
>   		st->plat_data = *pdata;
>   	}
>   
> +	desc = irq_get_irq_data(irq);
> +	if (!desc) {
> +		dev_err(dev, "Could not find IRQ %d\n", irq);
> +		return -EINVAL;
> +	}
> +
> +	irq_type = irqd_get_trigger_type(desc);
> +	if (irq_type == IRQF_TRIGGER_RISING)
> +		st->irq_mask = INV_MPU6050_ACTIVE_HIGH;
> +	else if (irq_type == IRQF_TRIGGER_FALLING)
> +		st->irq_mask = INV_MPU6050_ACTIVE_LOW;
> +	else if (irq_type == IRQF_TRIGGER_HIGH)
> +		st->irq_mask = INV_MPU6050_ACTIVE_HIGH |
> +			INV_MPU6050_LATCH_INT_EN;
> +	else if (irq_type == IRQF_TRIGGER_LOW)
> +		st->irq_mask = INV_MPU6050_ACTIVE_LOW |
> +			INV_MPU6050_LATCH_INT_EN;
> +	else {
> +		dev_err(dev, "Invalid interrupt type 0x%x specified\n",
> +			irq_type);
> +		return -EINVAL;
> +	}
> +
>   	/* power is turned on inside check chip type*/
>   	result = inv_check_and_setup_chip(st);
>   	if (result)
> @@ -948,7 +979,7 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
>   		dev_err(dev, "configure buffer fail %d\n", result);
>   		return result;
>   	}
> -	result = inv_mpu6050_probe_trigger(indio_dev);
> +	result = inv_mpu6050_probe_trigger(indio_dev, irq_type);
>   	if (result) {
>   		dev_err(dev, "trigger probe fail %d\n", result);
>   		goto out_unreg_ring;
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> index fcd7a92b6cf8..1b02d2b69174 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> @@ -44,8 +44,7 @@ static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id)
>   	if (!ret) {
>   		st->powerup_count++;
>   		ret = regmap_write(st->map, st->reg->int_pin_cfg,
> -				   INV_MPU6050_INT_PIN_CFG |
> -				   INV_MPU6050_BIT_BYPASS_EN);
> +			   st->irq_mask | INV_MPU6050_BIT_BYPASS_EN);
>   	}
>   write_error:
>   	mutex_unlock(&st->lock);
> @@ -60,7 +59,7 @@ static int inv_mpu6050_deselect_bypass(struct i2c_mux_core *muxc, u32 chan_id)
>   
>   	mutex_lock(&st->lock);
>   	/* It doesn't really mattter, if any of the calls fails */
> -	regmap_write(st->map, st->reg->int_pin_cfg, INV_MPU6050_INT_PIN_CFG);
> +	regmap_write(st->map, st->reg->int_pin_cfg, st->irq_mask);
>   	st->powerup_count--;
>   	if (!st->powerup_count)
>   		regmap_write(st->map, st->reg->pwr_mgmt_1,
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> index 065794162d65..eaa9158ac753 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> @@ -40,6 +40,7 @@
>    *  @raw_accl:		Address of first accel register.
>    *  @temperature:	temperature register
>    *  @int_enable:	Interrupt enable register.
> + *  @int_status:	Interrupt status register.
>    *  @pwr_mgmt_1:	Controls chip's power state and clock source.
>    *  @pwr_mgmt_2:	Controls power state of individual sensors.
>    *  @int_pin_cfg;	Controls interrupt pin configuration.
> @@ -60,6 +61,7 @@ struct inv_mpu6050_reg_map {
>   	u8 raw_accl;
>   	u8 temperature;
>   	u8 int_enable;
> +	u8 int_status;
>   	u8 pwr_mgmt_1;
>   	u8 pwr_mgmt_2;
>   	u8 int_pin_cfg;
> @@ -125,6 +127,7 @@ struct inv_mpu6050_hw {
>    *  @timestamps:        kfifo queue to store time stamp.
>    *  @map		regmap pointer.
>    *  @irq		interrupt number.
> + *  @irq_mask		the int_pin_cfg mask to configure interrupt type.
>    */
>   struct inv_mpu6050_state {
>   #define TIMESTAMP_FIFO_SIZE 16
> @@ -143,6 +146,7 @@ struct inv_mpu6050_state {
>   	DECLARE_KFIFO(timestamps, long long, TIMESTAMP_FIFO_SIZE);
>   	struct regmap *map;
>   	int irq;
> +	u8 irq_mask;
>   };
>   
>   /*register and associated bit definition*/
> @@ -166,6 +170,9 @@ struct inv_mpu6050_state {
>   #define INV_MPU6050_REG_TEMPERATURE         0x41
>   #define INV_MPU6050_REG_RAW_GYRO            0x43
>   
> +#define INV_MPU6050_REG_INT_STATUS          0x3A
> +#define INV_MPU6050_BIT_RAW_DATA_RDY_INT    0x01
> +
>   #define INV_MPU6050_REG_USER_CTRL           0x6A
>   #define INV_MPU6050_BIT_FIFO_RST            0x04
>   #define INV_MPU6050_BIT_DMP_RST             0x08
> @@ -215,8 +222,12 @@ struct inv_mpu6050_state {
>   #define INV_MPU6050_OUTPUT_DATA_SIZE         24
>   
>   #define INV_MPU6050_REG_INT_PIN_CFG	0x37
> +#define INV_MPU6050_ACTIVE_HIGH		0x00
> +#define INV_MPU6050_ACTIVE_LOW		0x80
> +/* enable level triggering */
> +#define INV_MPU6050_LATCH_INT_EN	0x20
>   #define INV_MPU6050_BIT_BYPASS_EN	0x2
> -#define INV_MPU6050_INT_PIN_CFG		0
> +
>   
>   /* init parameters */
>   #define INV_MPU6050_INIT_FIFO_RATE           50
> @@ -287,7 +298,7 @@ enum inv_mpu6050_clock_sel_e {
>   
>   irqreturn_t inv_mpu6050_irq_handler(int irq, void *p);
>   irqreturn_t inv_mpu6050_read_fifo(int irq, void *p);
> -int inv_mpu6050_probe_trigger(struct iio_dev *indio_dev);
> +int inv_mpu6050_probe_trigger(struct iio_dev *indio_dev, int irq_type);
>   void inv_mpu6050_remove_trigger(struct inv_mpu6050_state *st);
>   int inv_reset_fifo(struct iio_dev *indio_dev);
>   int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask);
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> index ff81c6aa009d..672c3df2d1d1 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> @@ -127,8 +127,23 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
>   	u8 data[INV_MPU6050_OUTPUT_DATA_SIZE];
>   	u16 fifo_count;
>   	s64 timestamp;
> +	int int_status;
>   
>   	mutex_lock(&st->lock);
> +
> +	/* ack interrupt and check status */
> +	result = regmap_read(st->map, st->reg->int_status, &int_status);
> +	if (result) {
> +		dev_err(regmap_get_device(st->map),
> +			"failed to ack interrupt\n");
> +		goto flush_fifo;
> +	}
> +	if (!(int_status & INV_MPU6050_BIT_RAW_DATA_RDY_INT)) {
> +		dev_warn(regmap_get_device(st->map),
> +			"spurious interrupt with status 0x%x\n", int_status);
> +		goto end_session;
> +	}
> +
>   	if (!(st->chip_config.accl_fifo_enable |
>   		st->chip_config.gyro_fifo_enable))
>   		goto end_session;
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> index f963f9fc98c0..b8c5584e4252 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> @@ -117,7 +117,7 @@ static const struct iio_trigger_ops inv_mpu_trigger_ops = {
>   	.set_trigger_state = &inv_mpu_data_rdy_trigger_set_state,
>   };
>   
> -int inv_mpu6050_probe_trigger(struct iio_dev *indio_dev)
> +int inv_mpu6050_probe_trigger(struct iio_dev *indio_dev, int irq_type)
>   {
>   	int ret;
>   	struct inv_mpu6050_state *st = iio_priv(indio_dev);
> @@ -131,7 +131,7 @@ int inv_mpu6050_probe_trigger(struct iio_dev *indio_dev)
>   
>   	ret = devm_request_irq(&indio_dev->dev, st->irq,
>   			       &iio_trigger_generic_data_rdy_poll,
> -			       IRQF_TRIGGER_RISING,
> +			       irq_type,
>   			       "inv_mpu",
>   			       st->trig);
>   	if (ret)
> 

All OK for me now.

Acked-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>

Re: [PATCH v7 2/2] dt-bindings: iio:imu:mpu6050: support more interrupt types

[Jean-Baptiste Maneyrol]

On 20/04/2018 18:54, Martin Kelly wrote:
> Document that the hardware supports falling edge, rising edge, level
> low, and level high interrupt types, rather than just rising edge.
> 
> The language used is the same as that in st_lsm6dsx.txt.
> 
> Signed-off-by: Martin Kelly <mkelly@xevo.com>
> ---
>   Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt | 11 ++++++++---
>   1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt b/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
> index 2b4514592f83..6b106d5ef298 100644
> --- a/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
> +++ b/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
> @@ -11,7 +11,12 @@ Required properties:
>   		"invensense,icm20608"
>    - reg : the I2C address of the sensor
>    - interrupt-parent : should be the phandle for the interrupt controller
> - - interrupts : interrupt mapping for GPIO IRQ
> + - interrupts: interrupt mapping for IRQ. It should be configured with flags
> +   IRQ_TYPE_LEVEL_HIGH, IRQ_TYPE_EDGE_RISING, IRQ_TYPE_LEVEL_LOW or
> +   IRQ_TYPE_EDGE_FALLING.
> +
> +  Refer to interrupt-controller/interrupts.txt for generic interrupt client node
> +  bindings.
>   
>   Optional properties:
>    - mount-matrix: an optional 3x3 mounting rotation matrix
> @@ -24,7 +29,7 @@ Example:
>   		compatible = "invensense,mpu6050";
>   		reg = <0x68>;
>   		interrupt-parent = <&gpio1>;
> -		interrupts = <18 1>;
> +		interrupts = <18 IRQ_TYPE_EDGE_RISING>;
>   		mount-matrix = "-0.984807753012208",  /* x0 */
>   		               "0",                   /* y0 */
>   		               "-0.173648177666930",  /* z0 */
> @@ -41,7 +46,7 @@ Example:
>   		compatible = "invensense,mpu9250";
>   		reg = <0x68>;
>   		interrupt-parent = <&gpio3>;
> -		interrupts = <21 1>;
> +		interrupts = <21 IRQ_TYPE_LEVEL_HIGH>;
>   		i2c-gate {
>   			#address-cells = <1>;
>   			#size-cells = <0>;
> 


Acked-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>

Re: [PATCH v7 1/2] iio:imu: inv_mpu6050: support more interrupt types

[Jonathan Cameron]

On Fri, 20 Apr 2018 19:04:17 +0200
Jean-Baptiste Maneyrol <jmaneyrol@invensense.com> wrote:

> On 20/04/2018 18:54, Martin Kelly wrote:
> > Currently, we support only rising edge interrupts, and in fact we assume
> > that the interrupt we're given is rising edge (and things won't work if
> > it's not). However, the device supports rising edge, falling edge, level
> > low, and level high interrupts.
> > 
> > Empirically, on my system, switching to level interrupts has fixed a
> > problem I had with significant (~40%) interrupt loss with edge
> > interrupts. This issue is likely related to the SoC I'm using (Allwinner
> > H3), but being able to switch the interrupt type is still a very useful
> > workaround.
> > 
> > I tested this with each interrupt type and verified correct behavior in
> > a logic analyzer.
> > 
> > Add support for these interrupt types while also eliminating the error
> > case of the device tree and driver using different interrupt types.
> > 
> > Signed-off-by: Martin Kelly <mkelly@xevo.com>
> > ---
> > v7:
> > - Actually flush the FIFO when we fail to ACK the interrupt, and end the session
> >    when we get a spurious interrupt.
> > v6:
> > - Use -EINVAL if devicetree interrupt configuration is missing.
> > - Use u8 for irq_mask.
> > - Stop mixing register and bit defines, and group them separately.
> > - If we fail to ACK the interrupt, flush the FIFO.
> > v5:
> > - Check interrupt status in all cases rather than only in the level triggered
> >     case, and warn if we get spurious interrupts.
> > - Rename INV_MPU6050_LEVEL_TRIGGERED to INV_MPU6050_LATCH_INT_EN to match
> >     datasheet naming.
> > - Write st->irq_mask prior to device power off to make sure it is fully set.
> > - Write st->irq_mask instead of 0 in inv_mpu6050_deselect_bypass.
> > - Make irq_type local instead of part of the driver state, as we use it only at
> >     probe time and never again.
> > - Remove the comment about bus lockups, as I have determined them to be
> >     unrelated.
> > - Add missing documentation for irq_type and irq_mask.
> > v4:
> > - Moved the ACK inside the mutex.
> > v3:
> > - Sent version 2 too quickly. Now that the ACK is moved to the top of the
> >     function, the "goto out" logic is unnecessary, so we can clean it up.
> > v2:
> > - Changed to ACK level interrupts at the start of the bottom half thread instead
> >     of at the end of it. Without this, the sample timestamps get distorted because
> >     the time to handle the bottom half thread delays future interrupts. With this
> >     change, the timestamps appear evenly distributed at the right frequency.
> > 
> >   drivers/iio/imu/inv_mpu6050/inv_mpu_core.c    | 33 ++++++++++++++++++++++++++-
> >   drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c     |  5 ++--
> >   drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h     | 15 ++++++++++--
> >   drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c    | 15 ++++++++++++
> >   drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c |  4 ++--
> >   5 files changed, 64 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> > index 7d64be353403..c2cec7508451 100644
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> > @@ -24,6 +24,7 @@
> >   #include <linux/spinlock.h>
> >   #include <linux/iio/iio.h>
> >   #include <linux/acpi.h>
> > +#include <linux/platform_device.h>
> >   #include "inv_mpu_iio.h"
> >   
> >   /*
> > @@ -52,6 +53,7 @@ static const struct inv_mpu6050_reg_map reg_set_6500 = {
> >   	.raw_accl               = INV_MPU6050_REG_RAW_ACCEL,
> >   	.temperature            = INV_MPU6050_REG_TEMPERATURE,
> >   	.int_enable             = INV_MPU6050_REG_INT_ENABLE,
> > +	.int_status             = INV_MPU6050_REG_INT_STATUS,
> >   	.pwr_mgmt_1             = INV_MPU6050_REG_PWR_MGMT_1,
> >   	.pwr_mgmt_2             = INV_MPU6050_REG_PWR_MGMT_2,
> >   	.int_pin_cfg		= INV_MPU6050_REG_INT_PIN_CFG,
> > @@ -278,6 +280,10 @@ static int inv_mpu6050_init_config(struct iio_dev *indio_dev)
> >   	if (result)
> >   		return result;
> >   
> > +	result = regmap_write(st->map, st->reg->int_pin_cfg, st->irq_mask);
> > +	if (result)
> > +		return result;
> > +
> >   	memcpy(&st->chip_config, hw_info[st->chip_type].config,
> >   	       sizeof(struct inv_mpu6050_chip_config));
> >   	result = inv_mpu6050_set_power_itg(st, false);
> > @@ -882,6 +888,8 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
> >   	struct inv_mpu6050_platform_data *pdata;
> >   	struct device *dev = regmap_get_device(regmap);
> >   	int result;
> > +	struct irq_data *desc;
> > +	int irq_type;
> >   
> >   	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
> >   	if (!indio_dev)
> > @@ -913,6 +921,29 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
> >   		st->plat_data = *pdata;
> >   	}
> >   
> > +	desc = irq_get_irq_data(irq);
> > +	if (!desc) {
> > +		dev_err(dev, "Could not find IRQ %d\n", irq);
> > +		return -EINVAL;
> > +	}
> > +
> > +	irq_type = irqd_get_trigger_type(desc);
> > +	if (irq_type == IRQF_TRIGGER_RISING)
> > +		st->irq_mask = INV_MPU6050_ACTIVE_HIGH;
> > +	else if (irq_type == IRQF_TRIGGER_FALLING)
> > +		st->irq_mask = INV_MPU6050_ACTIVE_LOW;
> > +	else if (irq_type == IRQF_TRIGGER_HIGH)
> > +		st->irq_mask = INV_MPU6050_ACTIVE_HIGH |
> > +			INV_MPU6050_LATCH_INT_EN;
> > +	else if (irq_type == IRQF_TRIGGER_LOW)
> > +		st->irq_mask = INV_MPU6050_ACTIVE_LOW |
> > +			INV_MPU6050_LATCH_INT_EN;
> > +	else {
> > +		dev_err(dev, "Invalid interrupt type 0x%x specified\n",
> > +			irq_type);
> > +		return -EINVAL;
> > +	}
> > +
> >   	/* power is turned on inside check chip type*/
> >   	result = inv_check_and_setup_chip(st);
> >   	if (result)
> > @@ -948,7 +979,7 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
> >   		dev_err(dev, "configure buffer fail %d\n", result);
> >   		return result;
> >   	}
> > -	result = inv_mpu6050_probe_trigger(indio_dev);
> > +	result = inv_mpu6050_probe_trigger(indio_dev, irq_type);
> >   	if (result) {
> >   		dev_err(dev, "trigger probe fail %d\n", result);
> >   		goto out_unreg_ring;
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> > index fcd7a92b6cf8..1b02d2b69174 100644
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> > @@ -44,8 +44,7 @@ static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id)
> >   	if (!ret) {
> >   		st->powerup_count++;
> >   		ret = regmap_write(st->map, st->reg->int_pin_cfg,
> > -				   INV_MPU6050_INT_PIN_CFG |
> > -				   INV_MPU6050_BIT_BYPASS_EN);
> > +			   st->irq_mask | INV_MPU6050_BIT_BYPASS_EN);
> >   	}
> >   write_error:
> >   	mutex_unlock(&st->lock);
> > @@ -60,7 +59,7 @@ static int inv_mpu6050_deselect_bypass(struct i2c_mux_core *muxc, u32 chan_id)
> >   
> >   	mutex_lock(&st->lock);
> >   	/* It doesn't really mattter, if any of the calls fails */
> > -	regmap_write(st->map, st->reg->int_pin_cfg, INV_MPU6050_INT_PIN_CFG);
> > +	regmap_write(st->map, st->reg->int_pin_cfg, st->irq_mask);
> >   	st->powerup_count--;
> >   	if (!st->powerup_count)
> >   		regmap_write(st->map, st->reg->pwr_mgmt_1,
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> > index 065794162d65..eaa9158ac753 100644
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> > @@ -40,6 +40,7 @@
> >    *  @raw_accl:		Address of first accel register.
> >    *  @temperature:	temperature register
> >    *  @int_enable:	Interrupt enable register.
> > + *  @int_status:	Interrupt status register.
> >    *  @pwr_mgmt_1:	Controls chip's power state and clock source.
> >    *  @pwr_mgmt_2:	Controls power state of individual sensors.
> >    *  @int_pin_cfg;	Controls interrupt pin configuration.
> > @@ -60,6 +61,7 @@ struct inv_mpu6050_reg_map {
> >   	u8 raw_accl;
> >   	u8 temperature;
> >   	u8 int_enable;
> > +	u8 int_status;
> >   	u8 pwr_mgmt_1;
> >   	u8 pwr_mgmt_2;
> >   	u8 int_pin_cfg;
> > @@ -125,6 +127,7 @@ struct inv_mpu6050_hw {
> >    *  @timestamps:        kfifo queue to store time stamp.
> >    *  @map		regmap pointer.
> >    *  @irq		interrupt number.
> > + *  @irq_mask		the int_pin_cfg mask to configure interrupt type.
> >    */
> >   struct inv_mpu6050_state {
> >   #define TIMESTAMP_FIFO_SIZE 16
> > @@ -143,6 +146,7 @@ struct inv_mpu6050_state {
> >   	DECLARE_KFIFO(timestamps, long long, TIMESTAMP_FIFO_SIZE);
> >   	struct regmap *map;
> >   	int irq;
> > +	u8 irq_mask;
> >   };
> >   
> >   /*register and associated bit definition*/
> > @@ -166,6 +170,9 @@ struct inv_mpu6050_state {
> >   #define INV_MPU6050_REG_TEMPERATURE         0x41
> >   #define INV_MPU6050_REG_RAW_GYRO            0x43
> >   
> > +#define INV_MPU6050_REG_INT_STATUS          0x3A
> > +#define INV_MPU6050_BIT_RAW_DATA_RDY_INT    0x01
> > +
> >   #define INV_MPU6050_REG_USER_CTRL           0x6A
> >   #define INV_MPU6050_BIT_FIFO_RST            0x04
> >   #define INV_MPU6050_BIT_DMP_RST             0x08
> > @@ -215,8 +222,12 @@ struct inv_mpu6050_state {
> >   #define INV_MPU6050_OUTPUT_DATA_SIZE         24
> >   
> >   #define INV_MPU6050_REG_INT_PIN_CFG	0x37
> > +#define INV_MPU6050_ACTIVE_HIGH		0x00
> > +#define INV_MPU6050_ACTIVE_LOW		0x80
> > +/* enable level triggering */
> > +#define INV_MPU6050_LATCH_INT_EN	0x20
> >   #define INV_MPU6050_BIT_BYPASS_EN	0x2
> > -#define INV_MPU6050_INT_PIN_CFG		0
> > +
> >   
> >   /* init parameters */
> >   #define INV_MPU6050_INIT_FIFO_RATE           50
> > @@ -287,7 +298,7 @@ enum inv_mpu6050_clock_sel_e {
> >   
> >   irqreturn_t inv_mpu6050_irq_handler(int irq, void *p);
> >   irqreturn_t inv_mpu6050_read_fifo(int irq, void *p);
> > -int inv_mpu6050_probe_trigger(struct iio_dev *indio_dev);
> > +int inv_mpu6050_probe_trigger(struct iio_dev *indio_dev, int irq_type);
> >   void inv_mpu6050_remove_trigger(struct inv_mpu6050_state *st);
> >   int inv_reset_fifo(struct iio_dev *indio_dev);
> >   int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask);
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> > index ff81c6aa009d..672c3df2d1d1 100644
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> > @@ -127,8 +127,23 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
> >   	u8 data[INV_MPU6050_OUTPUT_DATA_SIZE];
> >   	u16 fifo_count;
> >   	s64 timestamp;
> > +	int int_status;
> >   
> >   	mutex_lock(&st->lock);
> > +
> > +	/* ack interrupt and check status */
> > +	result = regmap_read(st->map, st->reg->int_status, &int_status);
> > +	if (result) {
> > +		dev_err(regmap_get_device(st->map),
> > +			"failed to ack interrupt\n");
> > +		goto flush_fifo;
> > +	}
> > +	if (!(int_status & INV_MPU6050_BIT_RAW_DATA_RDY_INT)) {
> > +		dev_warn(regmap_get_device(st->map),
> > +			"spurious interrupt with status 0x%x\n", int_status);
> > +		goto end_session;
> > +	}
> > +
> >   	if (!(st->chip_config.accl_fifo_enable |
> >   		st->chip_config.gyro_fifo_enable))
> >   		goto end_session;
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> > index f963f9fc98c0..b8c5584e4252 100644
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> > @@ -117,7 +117,7 @@ static const struct iio_trigger_ops inv_mpu_trigger_ops = {
> >   	.set_trigger_state = &inv_mpu_data_rdy_trigger_set_state,
> >   };
> >   
> > -int inv_mpu6050_probe_trigger(struct iio_dev *indio_dev)
> > +int inv_mpu6050_probe_trigger(struct iio_dev *indio_dev, int irq_type)
> >   {
> >   	int ret;
> >   	struct inv_mpu6050_state *st = iio_priv(indio_dev);
> > @@ -131,7 +131,7 @@ int inv_mpu6050_probe_trigger(struct iio_dev *indio_dev)
> >   
> >   	ret = devm_request_irq(&indio_dev->dev, st->irq,
> >   			       &iio_trigger_generic_data_rdy_poll,
> > -			       IRQF_TRIGGER_RISING,
> > +			       irq_type,
> >   			       "inv_mpu",
> >   			       st->trig);
> >   	if (ret)
> >   
> 
> All OK for me now.
> 
> Acked-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
Applied to the togreg branch of iio.git.  Note there was some fuzz
so please check I resolved things correctly.

Pushed out as testing for the autobuilders to play with it.

Thanks,

Jonathan

Re: [PATCH v7 2/2] dt-bindings: iio:imu:mpu6050: support more interrupt types

[Jonathan Cameron]

On Fri, 20 Apr 2018 19:05:12 +0200
Jean-Baptiste Maneyrol <jmaneyrol@invensense.com> wrote:

> On 20/04/2018 18:54, Martin Kelly wrote:
> > Document that the hardware supports falling edge, rising edge, level
> > low, and level high interrupt types, rather than just rising edge.
> > 
> > The language used is the same as that in st_lsm6dsx.txt.
> > 
> > Signed-off-by: Martin Kelly <mkelly@xevo.com>
Hi Martin,

Please pick up an acks / reviewed-by's from earlier versions.
The chances are I'll miss them some of the time otherwise.
Rob acked this one.

Applied with the two acks I know of to the togreg branch of iio.git
and pushed out as testing for the autobuilders to play with it.

Thanks,

Jonathan

> > ---
> >   Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt | 11 ++++++++---
> >   1 file changed, 8 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt b/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
> > index 2b4514592f83..6b106d5ef298 100644
> > --- a/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
> > +++ b/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
> > @@ -11,7 +11,12 @@ Required properties:
> >   		"invensense,icm20608"
> >    - reg : the I2C address of the sensor
> >    - interrupt-parent : should be the phandle for the interrupt controller
> > - - interrupts : interrupt mapping for GPIO IRQ
> > + - interrupts: interrupt mapping for IRQ. It should be configured with flags
> > +   IRQ_TYPE_LEVEL_HIGH, IRQ_TYPE_EDGE_RISING, IRQ_TYPE_LEVEL_LOW or
> > +   IRQ_TYPE_EDGE_FALLING.
> > +
> > +  Refer to interrupt-controller/interrupts.txt for generic interrupt client node
> > +  bindings.
> >   
> >   Optional properties:
> >    - mount-matrix: an optional 3x3 mounting rotation matrix
> > @@ -24,7 +29,7 @@ Example:
> >   		compatible = "invensense,mpu6050";
> >   		reg = <0x68>;
> >   		interrupt-parent = <&gpio1>;
> > -		interrupts = <18 1>;
> > +		interrupts = <18 IRQ_TYPE_EDGE_RISING>;
> >   		mount-matrix = "-0.984807753012208",  /* x0 */
> >   		               "0",                   /* y0 */
> >   		               "-0.173648177666930",  /* z0 */
> > @@ -41,7 +46,7 @@ Example:
> >   		compatible = "invensense,mpu9250";
> >   		reg = <0x68>;
> >   		interrupt-parent = <&gpio3>;
> > -		interrupts = <21 1>;
> > +		interrupts = <21 IRQ_TYPE_LEVEL_HIGH>;
> >   		i2c-gate {
> >   			#address-cells = <1>;
> >   			#size-cells = <0>;
> >   
> 
> 
> Acked-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 1/2] iio:imu: inv_mpu6050: support more interrupt types

[Martin Kelly]

On 04/21/2018 08:25 AM, Jonathan Cameron wrote:
> On Fri, 20 Apr 2018 19:04:17 +0200
> Jean-Baptiste Maneyrol <jmaneyrol@invensense.com> wrote:
> 
>> On 20/04/2018 18:54, Martin Kelly wrote:
>>> Currently, we support only rising edge interrupts, and in fact we assume
>>> that the interrupt we're given is rising edge (and things won't work if
>>> it's not). However, the device supports rising edge, falling edge, level
>>> low, and level high interrupts.
>>>
>>> Empirically, on my system, switching to level interrupts has fixed a
>>> problem I had with significant (~40%) interrupt loss with edge
>>> interrupts. This issue is likely related to the SoC I'm using (Allwinner
>>> H3), but being able to switch the interrupt type is still a very useful
>>> workaround.
>>>
>>> I tested this with each interrupt type and verified correct behavior in
>>> a logic analyzer.
>>>
>>> Add support for these interrupt types while also eliminating the error
>>> case of the device tree and driver using different interrupt types.
>>>
>>> Signed-off-by: Martin Kelly <mkelly@xevo.com>
>>> ---
>>> v7:
>>> - Actually flush the FIFO when we fail to ACK the interrupt, and end the session
>>>     when we get a spurious interrupt.
>>> v6:
>>> - Use -EINVAL if devicetree interrupt configuration is missing.
>>> - Use u8 for irq_mask.
>>> - Stop mixing register and bit defines, and group them separately.
>>> - If we fail to ACK the interrupt, flush the FIFO.
>>> v5:
>>> - Check interrupt status in all cases rather than only in the level triggered
>>>      case, and warn if we get spurious interrupts.
>>> - Rename INV_MPU6050_LEVEL_TRIGGERED to INV_MPU6050_LATCH_INT_EN to match
>>>      datasheet naming.
>>> - Write st->irq_mask prior to device power off to make sure it is fully set.
>>> - Write st->irq_mask instead of 0 in inv_mpu6050_deselect_bypass.
>>> - Make irq_type local instead of part of the driver state, as we use it only at
>>>      probe time and never again.
>>> - Remove the comment about bus lockups, as I have determined them to be
>>>      unrelated.
>>> - Add missing documentation for irq_type and irq_mask.
>>> v4:
>>> - Moved the ACK inside the mutex.
>>> v3:
>>> - Sent version 2 too quickly. Now that the ACK is moved to the top of the
>>>      function, the "goto out" logic is unnecessary, so we can clean it up.
>>> v2:
>>> - Changed to ACK level interrupts at the start of the bottom half thread instead
>>>      of at the end of it. Without this, the sample timestamps get distorted because
>>>      the time to handle the bottom half thread delays future interrupts. With this
>>>      change, the timestamps appear evenly distributed at the right frequency.
>>>
>>>    drivers/iio/imu/inv_mpu6050/inv_mpu_core.c    | 33 ++++++++++++++++++++++++++-
>>>    drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c     |  5 ++--
>>>    drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h     | 15 ++++++++++--
>>>    drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c    | 15 ++++++++++++
>>>    drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c |  4 ++--
>>>    5 files changed, 64 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>>> index 7d64be353403..c2cec7508451 100644
>>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>>> @@ -24,6 +24,7 @@
>>>    #include <linux/spinlock.h>
>>>    #include <linux/iio/iio.h>
>>>    #include <linux/acpi.h>
>>> +#include <linux/platform_device.h>
>>>    #include "inv_mpu_iio.h"
>>>    
>>>    /*
>>> @@ -52,6 +53,7 @@ static const struct inv_mpu6050_reg_map reg_set_6500 = {
>>>    	.raw_accl               = INV_MPU6050_REG_RAW_ACCEL,
>>>    	.temperature            = INV_MPU6050_REG_TEMPERATURE,
>>>    	.int_enable             = INV_MPU6050_REG_INT_ENABLE,
>>> +	.int_status             = INV_MPU6050_REG_INT_STATUS,
>>>    	.pwr_mgmt_1             = INV_MPU6050_REG_PWR_MGMT_1,
>>>    	.pwr_mgmt_2             = INV_MPU6050_REG_PWR_MGMT_2,
>>>    	.int_pin_cfg		= INV_MPU6050_REG_INT_PIN_CFG,
>>> @@ -278,6 +280,10 @@ static int inv_mpu6050_init_config(struct iio_dev *indio_dev)
>>>    	if (result)
>>>    		return result;
>>>    
>>> +	result = regmap_write(st->map, st->reg->int_pin_cfg, st->irq_mask);
>>> +	if (result)
>>> +		return result;
>>> +
>>>    	memcpy(&st->chip_config, hw_info[st->chip_type].config,
>>>    	       sizeof(struct inv_mpu6050_chip_config));
>>>    	result = inv_mpu6050_set_power_itg(st, false);
>>> @@ -882,6 +888,8 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
>>>    	struct inv_mpu6050_platform_data *pdata;
>>>    	struct device *dev = regmap_get_device(regmap);
>>>    	int result;
>>> +	struct irq_data *desc;
>>> +	int irq_type;
>>>    
>>>    	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
>>>    	if (!indio_dev)
>>> @@ -913,6 +921,29 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
>>>    		st->plat_data = *pdata;
>>>    	}
>>>    
>>> +	desc = irq_get_irq_data(irq);
>>> +	if (!desc) {
>>> +		dev_err(dev, "Could not find IRQ %d\n", irq);
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	irq_type = irqd_get_trigger_type(desc);
>>> +	if (irq_type == IRQF_TRIGGER_RISING)
>>> +		st->irq_mask = INV_MPU6050_ACTIVE_HIGH;
>>> +	else if (irq_type == IRQF_TRIGGER_FALLING)
>>> +		st->irq_mask = INV_MPU6050_ACTIVE_LOW;
>>> +	else if (irq_type == IRQF_TRIGGER_HIGH)
>>> +		st->irq_mask = INV_MPU6050_ACTIVE_HIGH |
>>> +			INV_MPU6050_LATCH_INT_EN;
>>> +	else if (irq_type == IRQF_TRIGGER_LOW)
>>> +		st->irq_mask = INV_MPU6050_ACTIVE_LOW |
>>> +			INV_MPU6050_LATCH_INT_EN;
>>> +	else {
>>> +		dev_err(dev, "Invalid interrupt type 0x%x specified\n",
>>> +			irq_type);
>>> +		return -EINVAL;
>>> +	}
>>> +
>>>    	/* power is turned on inside check chip type*/
>>>    	result = inv_check_and_setup_chip(st);
>>>    	if (result)
>>> @@ -948,7 +979,7 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
>>>    		dev_err(dev, "configure buffer fail %d\n", result);
>>>    		return result;
>>>    	}
>>> -	result = inv_mpu6050_probe_trigger(indio_dev);
>>> +	result = inv_mpu6050_probe_trigger(indio_dev, irq_type);
>>>    	if (result) {
>>>    		dev_err(dev, "trigger probe fail %d\n", result);
>>>    		goto out_unreg_ring;
>>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
>>> index fcd7a92b6cf8..1b02d2b69174 100644
>>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
>>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
>>> @@ -44,8 +44,7 @@ static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id)
>>>    	if (!ret) {
>>>    		st->powerup_count++;
>>>    		ret = regmap_write(st->map, st->reg->int_pin_cfg,
>>> -				   INV_MPU6050_INT_PIN_CFG |
>>> -				   INV_MPU6050_BIT_BYPASS_EN);
>>> +			   st->irq_mask | INV_MPU6050_BIT_BYPASS_EN);
>>>    	}
>>>    write_error:
>>>    	mutex_unlock(&st->lock);
>>> @@ -60,7 +59,7 @@ static int inv_mpu6050_deselect_bypass(struct i2c_mux_core *muxc, u32 chan_id)
>>>    
>>>    	mutex_lock(&st->lock);
>>>    	/* It doesn't really mattter, if any of the calls fails */
>>> -	regmap_write(st->map, st->reg->int_pin_cfg, INV_MPU6050_INT_PIN_CFG);
>>> +	regmap_write(st->map, st->reg->int_pin_cfg, st->irq_mask);
>>>    	st->powerup_count--;
>>>    	if (!st->powerup_count)
>>>    		regmap_write(st->map, st->reg->pwr_mgmt_1,
>>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
>>> index 065794162d65..eaa9158ac753 100644
>>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
>>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
>>> @@ -40,6 +40,7 @@
>>>     *  @raw_accl:		Address of first accel register.
>>>     *  @temperature:	temperature register
>>>     *  @int_enable:	Interrupt enable register.
>>> + *  @int_status:	Interrupt status register.
>>>     *  @pwr_mgmt_1:	Controls chip's power state and clock source.
>>>     *  @pwr_mgmt_2:	Controls power state of individual sensors.
>>>     *  @int_pin_cfg;	Controls interrupt pin configuration.
>>> @@ -60,6 +61,7 @@ struct inv_mpu6050_reg_map {
>>>    	u8 raw_accl;
>>>    	u8 temperature;
>>>    	u8 int_enable;
>>> +	u8 int_status;
>>>    	u8 pwr_mgmt_1;
>>>    	u8 pwr_mgmt_2;
>>>    	u8 int_pin_cfg;
>>> @@ -125,6 +127,7 @@ struct inv_mpu6050_hw {
>>>     *  @timestamps:        kfifo queue to store time stamp.
>>>     *  @map		regmap pointer.
>>>     *  @irq		interrupt number.
>>> + *  @irq_mask		the int_pin_cfg mask to configure interrupt type.
>>>     */
>>>    struct inv_mpu6050_state {
>>>    #define TIMESTAMP_FIFO_SIZE 16
>>> @@ -143,6 +146,7 @@ struct inv_mpu6050_state {
>>>    	DECLARE_KFIFO(timestamps, long long, TIMESTAMP_FIFO_SIZE);
>>>    	struct regmap *map;
>>>    	int irq;
>>> +	u8 irq_mask;
>>>    };
>>>    
>>>    /*register and associated bit definition*/
>>> @@ -166,6 +170,9 @@ struct inv_mpu6050_state {
>>>    #define INV_MPU6050_REG_TEMPERATURE         0x41
>>>    #define INV_MPU6050_REG_RAW_GYRO            0x43
>>>    
>>> +#define INV_MPU6050_REG_INT_STATUS          0x3A
>>> +#define INV_MPU6050_BIT_RAW_DATA_RDY_INT    0x01
>>> +
>>>    #define INV_MPU6050_REG_USER_CTRL           0x6A
>>>    #define INV_MPU6050_BIT_FIFO_RST            0x04
>>>    #define INV_MPU6050_BIT_DMP_RST             0x08
>>> @@ -215,8 +222,12 @@ struct inv_mpu6050_state {
>>>    #define INV_MPU6050_OUTPUT_DATA_SIZE         24
>>>    
>>>    #define INV_MPU6050_REG_INT_PIN_CFG	0x37
>>> +#define INV_MPU6050_ACTIVE_HIGH		0x00
>>> +#define INV_MPU6050_ACTIVE_LOW		0x80
>>> +/* enable level triggering */
>>> +#define INV_MPU6050_LATCH_INT_EN	0x20
>>>    #define INV_MPU6050_BIT_BYPASS_EN	0x2
>>> -#define INV_MPU6050_INT_PIN_CFG		0
>>> +
>>>    
>>>    /* init parameters */
>>>    #define INV_MPU6050_INIT_FIFO_RATE           50
>>> @@ -287,7 +298,7 @@ enum inv_mpu6050_clock_sel_e {
>>>    
>>>    irqreturn_t inv_mpu6050_irq_handler(int irq, void *p);
>>>    irqreturn_t inv_mpu6050_read_fifo(int irq, void *p);
>>> -int inv_mpu6050_probe_trigger(struct iio_dev *indio_dev);
>>> +int inv_mpu6050_probe_trigger(struct iio_dev *indio_dev, int irq_type);
>>>    void inv_mpu6050_remove_trigger(struct inv_mpu6050_state *st);
>>>    int inv_reset_fifo(struct iio_dev *indio_dev);
>>>    int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask);
>>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
>>> index ff81c6aa009d..672c3df2d1d1 100644
>>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
>>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
>>> @@ -127,8 +127,23 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
>>>    	u8 data[INV_MPU6050_OUTPUT_DATA_SIZE];
>>>    	u16 fifo_count;
>>>    	s64 timestamp;
>>> +	int int_status;
>>>    
>>>    	mutex_lock(&st->lock);
>>> +
>>> +	/* ack interrupt and check status */
>>> +	result = regmap_read(st->map, st->reg->int_status, &int_status);
>>> +	if (result) {
>>> +		dev_err(regmap_get_device(st->map),
>>> +			"failed to ack interrupt\n");
>>> +		goto flush_fifo;
>>> +	}
>>> +	if (!(int_status & INV_MPU6050_BIT_RAW_DATA_RDY_INT)) {
>>> +		dev_warn(regmap_get_device(st->map),
>>> +			"spurious interrupt with status 0x%x\n", int_status);
>>> +		goto end_session;
>>> +	}
>>> +
>>>    	if (!(st->chip_config.accl_fifo_enable |
>>>    		st->chip_config.gyro_fifo_enable))
>>>    		goto end_session;
>>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
>>> index f963f9fc98c0..b8c5584e4252 100644
>>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
>>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
>>> @@ -117,7 +117,7 @@ static const struct iio_trigger_ops inv_mpu_trigger_ops = {
>>>    	.set_trigger_state = &inv_mpu_data_rdy_trigger_set_state,
>>>    };
>>>    
>>> -int inv_mpu6050_probe_trigger(struct iio_dev *indio_dev)
>>> +int inv_mpu6050_probe_trigger(struct iio_dev *indio_dev, int irq_type)
>>>    {
>>>    	int ret;
>>>    	struct inv_mpu6050_state *st = iio_priv(indio_dev);
>>> @@ -131,7 +131,7 @@ int inv_mpu6050_probe_trigger(struct iio_dev *indio_dev)
>>>    
>>>    	ret = devm_request_irq(&indio_dev->dev, st->irq,
>>>    			       &iio_trigger_generic_data_rdy_poll,
>>> -			       IRQF_TRIGGER_RISING,
>>> +			       irq_type,
>>>    			       "inv_mpu",
>>>    			       st->trig);
>>>    	if (ret)
>>>    
>>
>> All OK for me now.
>>
>> Acked-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
> Applied to the togreg branch of iio.git.  Note there was some fuzz
> so please check I resolved things correctly.
> 

Looks fine to me; thanks!

Re: [PATCH v7 2/2] dt-bindings: iio:imu:mpu6050: support more interrupt types

[Martin Kelly]

On 04/21/2018 08:28 AM, Jonathan Cameron wrote:
> On Fri, 20 Apr 2018 19:05:12 +0200
> Jean-Baptiste Maneyrol <jmaneyrol@invensense.com> wrote:
> 
>> On 20/04/2018 18:54, Martin Kelly wrote:
>>> Document that the hardware supports falling edge, rising edge, level
>>> low, and level high interrupt types, rather than just rising edge.
>>>
>>> The language used is the same as that in st_lsm6dsx.txt.
>>>
>>> Signed-off-by: Martin Kelly <mkelly@xevo.com>
> Hi Martin,
> 
> Please pick up an acks / reviewed-by's from earlier versions.
> The chances are I'll miss them some of the time otherwise.
> Rob acked this one.
> 
> Applied with the two acks I know of to the togreg branch of iio.git
> and pushed out as testing for the autobuilders to play with it.
> 
> Thanks,
> 
> Jonathan
> 

OK, I'll try to remember to do that in the future.