[PATCH v3 0/6] iio:bmi160: add drdy interrupt support

[PATCH v3 0/6] iio:bmi160: add drdy interrupt support

[Martin Kelly]

From: Martin Kelly <martin@martingkelly.com>

This patch series adds drdy interrupt support to the BMI160 IMU. It also updates
the relevant DT bindings and does a little bit of related cleanup.

v2:
- Drop "BOTH" interrupt setting.
- Change to "if (ret)" instead of "if (ret < 0)".
- Stylistic changes suggested by Jonathan Cameron.
- Fix bogus return check after iio_trigger_get.

v3:
- More cleanup changes.
- Directly get INT1/INT2 by name instead of using I2C/SPI IRQs. Move this code
  to the probe function.
- Fix EDGE/LEVEL #define naming. The code was correct but the names were wrong.

Martin Kelly (6):
  iio:bmi160: add SPDX identifiers
  iio:bmi160: add drdy interrupt support
  dt-bindings: fix incorrect bmi160 IRQ note
  dt-bindings: document open-drain property
  iio:bmi160: use iio_pollfunc_store_time
  iio:bmi160: use if (ret) instead of if (ret < 0)

 .../devicetree/bindings/iio/imu/bmi160.txt         |   6 +-
 drivers/iio/imu/bmi160/bmi160.h                    |  11 +
 drivers/iio/imu/bmi160/bmi160_core.c               | 317 +++++++++++++++++++--
 drivers/iio/imu/bmi160/bmi160_i2c.c                |   5 +-
 drivers/iio/imu/bmi160/bmi160_spi.c                |   4 +-
 5 files changed, 304 insertions(+), 39 deletions(-)

--
2.11.0

[PATCH v3 1/6] iio:bmi160: add SPDX identifiers

[Martin Kelly]

From: Martin Kelly <martin@martingkelly.com>

Add SPDX identifiers (GPL 2) for the BMI160 driver. bmi160.h had an
identifier, but the other files did not.

Signed-off-by: Martin Kelly <martin@martingkelly.com>
---
 drivers/iio/imu/bmi160/bmi160_core.c | 5 +----
 drivers/iio/imu/bmi160/bmi160_i2c.c  | 5 +----
 drivers/iio/imu/bmi160/bmi160_spi.c  | 4 +---
 3 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/imu/bmi160/bmi160_core.c b/drivers/iio/imu/bmi160/bmi160_core.c
index b10330b0f93f..ce61026d84c3 100644
--- a/drivers/iio/imu/bmi160/bmi160_core.c
+++ b/drivers/iio/imu/bmi160/bmi160_core.c
@@ -1,12 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  * BMI160 - Bosch IMU (accel, gyro plus external magnetometer)
  *
  * Copyright (c) 2016, Intel Corporation.
  *
- * This file is subject to the terms and conditions of version 2 of
- * the GNU General Public License.  See the file COPYING in the main
- * directory of this archive for more details.
- *
  * IIO core driver for BMI160, with support for I2C/SPI busses
  *
  * TODO: magnetometer, interrupts, hardware FIFO
diff --git a/drivers/iio/imu/bmi160/bmi160_i2c.c b/drivers/iio/imu/bmi160/bmi160_i2c.c
index 5b1f7e6af651..e36f5e82d400 100644
--- a/drivers/iio/imu/bmi160/bmi160_i2c.c
+++ b/drivers/iio/imu/bmi160/bmi160_i2c.c
@@ -1,12 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  * BMI160 - Bosch IMU, I2C bits
  *
  * Copyright (c) 2016, Intel Corporation.
  *
- * This file is subject to the terms and conditions of version 2 of
- * the GNU General Public License.  See the file COPYING in the main
- * directory of this archive for more details.
- *
  * 7-bit I2C slave address is:
  *      - 0x68 if SDO is pulled to GND
  *      - 0x69 if SDO is pulled to VDDIO
diff --git a/drivers/iio/imu/bmi160/bmi160_spi.c b/drivers/iio/imu/bmi160/bmi160_spi.c
index e521ad14eeac..c19e3df35559 100644
--- a/drivers/iio/imu/bmi160/bmi160_spi.c
+++ b/drivers/iio/imu/bmi160/bmi160_spi.c
@@ -1,11 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  * BMI160 - Bosch IMU, SPI bits
  *
  * Copyright (c) 2016, Intel Corporation.
  *
- * This file is subject to the terms and conditions of version 2 of
- * the GNU General Public License.  See the file COPYING in the main
- * directory of this archive for more details.
  */
 #include <linux/acpi.h>
 #include <linux/module.h>
-- 
2.11.0

[PATCH v3 2/6] iio:bmi160: add drdy interrupt support

[Martin Kelly]

From: Martin Kelly <martin@martingkelly.com>

Add interrupt support for the data ready signal on the BMI160, which fires
an interrupt whenever new accelerometer/gyroscope data is ready to read.

Signed-off-by: Martin Kelly <martin@martingkelly.com>
---
 drivers/iio/imu/bmi160/bmi160.h      |  11 ++
 drivers/iio/imu/bmi160/bmi160_core.c | 270 ++++++++++++++++++++++++++++++++++-
 2 files changed, 278 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/imu/bmi160/bmi160.h b/drivers/iio/imu/bmi160/bmi160.h
index 2351049d930b..621f5309d735 100644
--- a/drivers/iio/imu/bmi160/bmi160.h
+++ b/drivers/iio/imu/bmi160/bmi160.h
@@ -2,9 +2,20 @@
 #ifndef BMI160_H_
 #define BMI160_H_
 
+#include <linux/iio/iio.h>
+
+struct bmi160_data {
+	struct regmap *regmap;
+	struct iio_trigger *trig;
+};
+
 extern const struct regmap_config bmi160_regmap_config;
 
 int bmi160_core_probe(struct device *dev, struct regmap *regmap,
 		      const char *name, bool use_spi);
 
+int bmi160_enable_irq(struct regmap *regmap, bool enable);
+
+int bmi160_probe_trigger(struct iio_dev *indio_dev, int irq, u32 irq_type);
+
 #endif  /* BMI160_H_ */
diff --git a/drivers/iio/imu/bmi160/bmi160_core.c b/drivers/iio/imu/bmi160/bmi160_core.c
index ce61026d84c3..70739e3920af 100644
--- a/drivers/iio/imu/bmi160/bmi160_core.c
+++ b/drivers/iio/imu/bmi160/bmi160_core.c
@@ -3,21 +3,25 @@
  * BMI160 - Bosch IMU (accel, gyro plus external magnetometer)
  *
  * Copyright (c) 2016, Intel Corporation.
+ * Copyright (c) 2019, Martin Kelly.
  *
  * IIO core driver for BMI160, with support for I2C/SPI busses
  *
- * TODO: magnetometer, interrupts, hardware FIFO
+ * TODO: magnetometer, hardware FIFO
  */
 #include <linux/module.h>
 #include <linux/regmap.h>
 #include <linux/acpi.h>
 #include <linux/delay.h>
+#include <linux/irq.h>
+#include <linux/of_irq.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/triggered_buffer.h>
 #include <linux/iio/trigger_consumer.h>
 #include <linux/iio/buffer.h>
 #include <linux/iio/sysfs.h>
+#include <linux/iio/trigger.h>
 
 #include "bmi160.h"
 
@@ -61,8 +65,32 @@
 #define BMI160_CMD_GYRO_PM_FAST_STARTUP	0x17
 #define BMI160_CMD_SOFTRESET		0xB6
 
+#define BMI160_REG_INT_EN		0x51
+#define BMI160_DRDY_INT_EN		BIT(4)
+
+#define BMI160_REG_INT_OUT_CTRL		0x53
+#define BMI160_INT_OUT_CTRL_MASK	0x0f
+#define BMI160_INT1_OUT_CTRL_SHIFT	0
+#define BMI160_INT2_OUT_CTRL_SHIFT	4
+#define BMI160_EDGE_TRIGGERED		BIT(0)
+#define BMI160_ACTIVE_HIGH		BIT(1)
+#define BMI160_OPEN_DRAIN		BIT(2)
+#define BMI160_OUTPUT_EN		BIT(3)
+
+#define BMI160_REG_INT_LATCH		0x54
+#define BMI160_INT1_LATCH_MASK		BIT(4)
+#define BMI160_INT2_LATCH_MASK		BIT(5)
+
+/* INT1 and INT2 are in the opposite order as in INT_OUT_CTRL! */
+#define BMI160_REG_INT_MAP		0x56
+#define BMI160_INT1_MAP_DRDY_EN		0x80
+#define BMI160_INT2_MAP_DRDY_EN		0x08
+
 #define BMI160_REG_DUMMY		0x7F
 
+#define BMI160_NORMAL_WRITE_USLEEP	2
+#define BMI160_SUSPENDED_WRITE_USLEEP	450
+
 #define BMI160_ACCEL_PMU_MIN_USLEEP	3800
 #define BMI160_GYRO_PMU_MIN_USLEEP	80000
 #define BMI160_SOFTRESET_USLEEP		1000
@@ -105,8 +133,9 @@ enum bmi160_sensor_type {
 	BMI160_NUM_SENSORS /* must be last */
 };
 
-struct bmi160_data {
-	struct regmap *regmap;
+enum bmi160_int_pin {
+	BMI160_PIN_INT1,
+	BMI160_PIN_INT2
 };
 
 const struct regmap_config bmi160_regmap_config = {
@@ -495,6 +524,186 @@ static const char *bmi160_match_acpi_device(struct device *dev)
 	return dev_name(dev);
 }
 
+static int bmi160_write_conf_reg(struct regmap *regmap, unsigned int reg,
+				 unsigned int mask, unsigned int bits,
+				 unsigned int write_usleep)
+{
+	int ret;
+	unsigned int val;
+
+	ret = regmap_read(regmap, reg, &val);
+	if (ret)
+		return ret;
+
+	val = (val & ~mask) | bits;
+
+	ret = regmap_write(regmap, reg, val);
+	if (ret)
+		return ret;
+
+	/*
+	 * We need to wait after writing before we can write again. See the
+	 * datasheet, page 93.
+	 */
+	usleep_range(write_usleep, write_usleep + 1000);
+
+	return 0;
+}
+
+static int bmi160_config_pin(struct regmap *regmap, enum bmi160_int_pin pin,
+			     bool open_drain, u8 irq_mask,
+			     unsigned long write_usleep)
+{
+	int ret;
+	struct device *dev = regmap_get_device(regmap);
+	u8 int_out_ctrl_shift;
+	u8 int_latch_mask;
+	u8 int_map_mask;
+	u8 int_out_ctrl_mask;
+	u8 int_out_ctrl_bits;
+	const char *pin_name;
+
+	switch (pin) {
+	case BMI160_PIN_INT1:
+		int_out_ctrl_shift = BMI160_INT1_OUT_CTRL_SHIFT;
+		int_latch_mask = BMI160_INT1_LATCH_MASK;
+		int_map_mask = BMI160_INT1_MAP_DRDY_EN;
+		break;
+	case BMI160_PIN_INT2:
+		int_out_ctrl_shift = BMI160_INT2_OUT_CTRL_SHIFT;
+		int_latch_mask = BMI160_INT2_LATCH_MASK;
+		int_map_mask = BMI160_INT2_MAP_DRDY_EN;
+		break;
+	}
+	int_out_ctrl_mask = BMI160_INT_OUT_CTRL_MASK << int_out_ctrl_shift;
+
+	/*
+	 * Enable the requested pin with the right settings:
+	 * - Push-pull/open-drain
+	 * - Active low/high
+	 * - Edge/level triggered
+	 */
+	int_out_ctrl_bits = BMI160_OUTPUT_EN;
+	if (open_drain)
+		/* Default is push-pull. */
+		int_out_ctrl_bits |= BMI160_OPEN_DRAIN;
+	int_out_ctrl_bits |= irq_mask;
+	int_out_ctrl_bits <<= int_out_ctrl_shift;
+
+	ret = bmi160_write_conf_reg(regmap, BMI160_REG_INT_OUT_CTRL,
+				    int_out_ctrl_mask, int_out_ctrl_bits,
+				    write_usleep);
+	if (ret)
+		return ret;
+
+	/* Set the pin to input mode with no latching. */
+	ret = bmi160_write_conf_reg(regmap, BMI160_REG_INT_LATCH,
+				    int_latch_mask, int_latch_mask,
+				    write_usleep);
+	if (ret)
+		return ret;
+
+	/* Map interrupts to the requested pin. */
+	ret = bmi160_write_conf_reg(regmap, BMI160_REG_INT_MAP,
+				    int_map_mask, int_map_mask,
+				    write_usleep);
+	if (ret) {
+		switch (pin) {
+		case BMI160_PIN_INT1:
+			pin_name = "INT1";
+			break;
+		case BMI160_PIN_INT2:
+			pin_name = "INT2";
+			break;
+		}
+		dev_err(dev, "Failed to configure %s IRQ pin", pin_name);
+	}
+
+	return ret;
+}
+
+int bmi160_enable_irq(struct regmap *regmap, bool enable)
+{
+	unsigned int enable_bit = 0;
+
+	if (enable)
+		enable_bit = BMI160_DRDY_INT_EN;
+
+	return bmi160_write_conf_reg(regmap, BMI160_REG_INT_EN,
+				     BMI160_DRDY_INT_EN, enable_bit,
+				     BMI160_NORMAL_WRITE_USLEEP);
+}
+EXPORT_SYMBOL(bmi160_enable_irq);
+
+static int bmi160_get_irq(struct device_node *of_node, enum bmi160_int_pin *pin)
+{
+	int irq;
+
+	/* Use INT1 if possible, otherwise falling back to INT2. */
+	irq = of_irq_get_byname(of_node, "INT1");
+	if (irq > 0) {
+		*pin = BMI160_PIN_INT1;
+		return irq;
+	}
+
+	irq = of_irq_get_byname(of_node, "INT2");
+	if (irq > 0)
+		*pin = BMI160_PIN_INT2;
+
+	return irq;
+}
+
+static int bmi160_config_device_irq(struct iio_dev *indio_dev, int irq_type,
+				    enum bmi160_int_pin pin)
+{
+	bool open_drain;
+	u8 irq_mask;
+	struct bmi160_data *data = iio_priv(indio_dev);
+	struct device *dev = regmap_get_device(data->regmap);
+
+	/* Level-triggered, active-low is the default if we set all zeroes. */
+	if (irq_type == IRQF_TRIGGER_RISING)
+		irq_mask = BMI160_ACTIVE_HIGH | BMI160_EDGE_TRIGGERED;
+	else if (irq_type == IRQF_TRIGGER_FALLING)
+		irq_mask = BMI160_EDGE_TRIGGERED;
+	else if (irq_type == IRQF_TRIGGER_HIGH)
+		irq_mask = BMI160_ACTIVE_HIGH;
+	else if (irq_type == IRQF_TRIGGER_LOW)
+		irq_mask = 0;
+	else {
+		dev_err(&indio_dev->dev,
+			"Invalid interrupt type 0x%x specified\n", irq_type);
+		return -EINVAL;
+	}
+
+	open_drain = of_property_read_bool(dev->of_node, "bmi160,open-drain");
+
+	return bmi160_config_pin(data->regmap, pin, open_drain, irq_mask,
+				 BMI160_NORMAL_WRITE_USLEEP);
+}
+
+static int bmi160_setup_irq(struct iio_dev *indio_dev, int irq,
+			    enum bmi160_int_pin pin)
+{
+	struct irq_data *desc;
+	u32 irq_type;
+	int ret;
+
+	desc = irq_get_irq_data(irq);
+	if (!desc) {
+		dev_err(&indio_dev->dev, "Could not find IRQ %d\n", irq);
+		return -EINVAL;
+	}
+
+	irq_type = irqd_get_trigger_type(desc);
+
+	ret = bmi160_config_device_irq(indio_dev, irq_type, pin);
+	if (ret)
+		return ret;
+
+	return bmi160_probe_trigger(indio_dev, irq, irq_type);
+}
+
 static int bmi160_chip_init(struct bmi160_data *data, bool use_spi)
 {
 	int ret;
@@ -539,6 +748,49 @@ static int bmi160_chip_init(struct bmi160_data *data, bool use_spi)
 	return 0;
 }
 
+static int bmi160_data_rdy_trigger_set_state(struct iio_trigger *trig,
+					     bool enable)
+{
+	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
+	struct bmi160_data *data = iio_priv(indio_dev);
+
+	return bmi160_enable_irq(data->regmap, enable);
+}
+
+static const struct iio_trigger_ops bmi160_trigger_ops = {
+	.set_trigger_state = &bmi160_data_rdy_trigger_set_state,
+};
+
+int bmi160_probe_trigger(struct iio_dev *indio_dev, int irq, u32 irq_type)
+{
+	struct bmi160_data *data = iio_priv(indio_dev);
+	int ret;
+
+	data->trig = devm_iio_trigger_alloc(&indio_dev->dev, "%s-dev%d",
+					    indio_dev->name, indio_dev->id);
+
+	if (data->trig == NULL)
+		return -ENOMEM;
+
+	ret = devm_request_irq(&indio_dev->dev, irq,
+			       &iio_trigger_generic_data_rdy_poll,
+			       irq_type, "bmi160", data->trig);
+	if (ret < 0)
+		return ret;
+
+	data->trig->dev.parent = regmap_get_device(data->regmap);
+	data->trig->ops = &bmi160_trigger_ops;
+	iio_trigger_set_drvdata(data->trig, indio_dev);
+
+	ret = devm_iio_trigger_register(&indio_dev->dev, data->trig);
+	if (ret)
+		return ret;
+
+	indio_dev->trig = iio_trigger_get(data->trig);
+
+	return 0;
+}
+
 static void bmi160_chip_uninit(void *data)
 {
 	struct bmi160_data *bmi_data = data;
@@ -552,6 +804,8 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
 {
 	struct iio_dev *indio_dev;
 	struct bmi160_data *data;
+	int irq;
+	enum bmi160_int_pin int_pin;
 	int ret;
 
 	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
@@ -585,6 +839,16 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
 	if (ret < 0)
 		return ret;
 
+	irq = bmi160_get_irq(dev->of_node, &int_pin);
+	if (irq > 0) {
+		ret = bmi160_setup_irq(indio_dev, irq, int_pin);
+		if (ret)
+			dev_err(&indio_dev->dev, "Failed to setup IRQ %d\n",
+				irq);
+	} else {
+		dev_info(&indio_dev->dev, "Not setting up IRQ trigger\n");
+	}
+
 	ret = devm_iio_device_register(dev, indio_dev);
 	if (ret < 0)
 		return ret;
-- 
2.11.0

[PATCH v3 3/6] dt-bindings: fix incorrect bmi160 IRQ note

[Martin Kelly]

From: Martin Kelly <martin@martingkelly.com>

The bmi160 bindings say that the BMI160 requires level-triggered,
active-low interrupts, but it actually supports all interrupt types, so fix
the note to reflect that.

Signed-off-by: Martin Kelly <martin@martingkelly.com>
---
 Documentation/devicetree/bindings/iio/imu/bmi160.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/imu/bmi160.txt b/Documentation/devicetree/bindings/iio/imu/bmi160.txt
index 0c1c105fb503..1aec19997fb5 100644
--- a/Documentation/devicetree/bindings/iio/imu/bmi160.txt
+++ b/Documentation/devicetree/bindings/iio/imu/bmi160.txt
@@ -9,7 +9,7 @@ Required properties:
  - spi-max-frequency : set maximum clock frequency (only for SPI)
 
 Optional properties:
- - interrupts : interrupt mapping for IRQ, must be IRQ_TYPE_LEVEL_LOW
+ - interrupts : interrupt mapping for IRQ
  - interrupt-names : set to "INT1" if INT1 pin should be used as interrupt
    input, set to "INT2" if INT2 pin should be used instead
 
@@ -20,7 +20,7 @@ bmi160@68 {
 	reg = <0x68>;
 
 	interrupt-parent = <&gpio4>;
-	interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
+	interrupts = <12 IRQ_TYPE_EDGE_RISING>;
 	interrupt-names = "INT1";
 };
 
-- 
2.11.0

Re: [PATCH v3 3/6] dt-bindings: fix incorrect bmi160 IRQ note

[Rob Herring]

On Sun, 27 Jan 2019 12:39:08 -0800, Martin Kelly wrote:
> From: Martin Kelly <martin@martingkelly.com>
> 
> The bmi160 bindings say that the BMI160 requires level-triggered,
> active-low interrupts, but it actually supports all interrupt types, so fix
> the note to reflect that.
> 
> Signed-off-by: Martin Kelly <martin@martingkelly.com>
> ---
>  Documentation/devicetree/bindings/iio/imu/bmi160.txt | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v3 3/6] dt-bindings: fix incorrect bmi160 IRQ note

[Rob Herring]

On Sun, 27 Jan 2019 12:39:08 -0800, Martin Kelly wrote:
> From: Martin Kelly <martin@martingkelly.com>
> 
> The bmi160 bindings say that the BMI160 requires level-triggered,
> active-low interrupts, but it actually supports all interrupt types, so fix
> the note to reflect that.
> 
> Signed-off-by: Martin Kelly <martin@martingkelly.com>
> ---
>  Documentation/devicetree/bindings/iio/imu/bmi160.txt | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH v3 4/6] dt-bindings: document open-drain property

[Martin Kelly]

From: Martin Kelly <martin@martingkelly.com>

We have added an optional boolean property for configuring a BMI160
interrupt pin as open-drain, as opposed to the default push-pull, so
document this.

Signed-off-by: Martin Kelly <martin@martingkelly.com>
---
 Documentation/devicetree/bindings/iio/imu/bmi160.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/imu/bmi160.txt b/Documentation/devicetree/bindings/iio/imu/bmi160.txt
index 1aec19997fb5..7eb4b6016404 100644
--- a/Documentation/devicetree/bindings/iio/imu/bmi160.txt
+++ b/Documentation/devicetree/bindings/iio/imu/bmi160.txt
@@ -12,6 +12,8 @@ Optional properties:
  - interrupts : interrupt mapping for IRQ
  - interrupt-names : set to "INT1" if INT1 pin should be used as interrupt
    input, set to "INT2" if INT2 pin should be used instead
+- bmi160,open-drain : set if the specified interrupt pin should be configured as
+  open drain. If not set, defaults to push-pull.
 
 Examples:
 
-- 
2.11.0

Re: [PATCH v3 4/6] dt-bindings: document open-drain property

[Rob Herring]

On Sun, Jan 27, 2019 at 12:39:09PM -0800, Martin Kelly wrote:
> From: Martin Kelly <martin@martingkelly.com>
> 
> We have added an optional boolean property for configuring a BMI160
> interrupt pin as open-drain, as opposed to the default push-pull, so
> document this.
> 
> Signed-off-by: Martin Kelly <martin@martingkelly.com>
> ---
>  Documentation/devicetree/bindings/iio/imu/bmi160.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/imu/bmi160.txt b/Documentation/devicetree/bindings/iio/imu/bmi160.txt
> index 1aec19997fb5..7eb4b6016404 100644
> --- a/Documentation/devicetree/bindings/iio/imu/bmi160.txt
> +++ b/Documentation/devicetree/bindings/iio/imu/bmi160.txt
> @@ -12,6 +12,8 @@ Optional properties:
>   - interrupts : interrupt mapping for IRQ
>   - interrupt-names : set to "INT1" if INT1 pin should be used as interrupt
>     input, set to "INT2" if INT2 pin should be used instead
> +- bmi160,open-drain : set if the specified interrupt pin should be configured as
> +  open drain. If not set, defaults to push-pull.

Use 'drive-open-drain' like others.

>  
>  Examples:
>  
> -- 
> 2.11.0
> 

Re: [PATCH v3 4/6] dt-bindings: document open-drain property

[Martin Kelly]

On 1/30/19 8:57 AM, Rob Herring wrote:
> On Sun, Jan 27, 2019 at 12:39:09PM -0800, Martin Kelly wrote:
>> From: Martin Kelly <martin@martingkelly.com>
>>
>> We have added an optional boolean property for configuring a BMI160
>> interrupt pin as open-drain, as opposed to the default push-pull, so
>> document this.
>>
>> Signed-off-by: Martin Kelly <martin@martingkelly.com>
>> ---
>>   Documentation/devicetree/bindings/iio/imu/bmi160.txt | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/imu/bmi160.txt b/Documentation/devicetree/bindings/iio/imu/bmi160.txt
>> index 1aec19997fb5..7eb4b6016404 100644
>> --- a/Documentation/devicetree/bindings/iio/imu/bmi160.txt
>> +++ b/Documentation/devicetree/bindings/iio/imu/bmi160.txt
>> @@ -12,6 +12,8 @@ Optional properties:
>>    - interrupts : interrupt mapping for IRQ
>>    - interrupt-names : set to "INT1" if INT1 pin should be used as interrupt
>>      input, set to "INT2" if INT2 pin should be used instead
>> +- bmi160,open-drain : set if the specified interrupt pin should be configured as
>> +  open drain. If not set, defaults to push-pull.
> 
> Use 'drive-open-drain' like others.
> 

OK, I'll do this in the next revision.

>>   
>>   Examples:
>>   
>> -- 
>> 2.11.0
>>

[PATCH v3 5/6] iio:bmi160: use iio_pollfunc_store_time

[Martin Kelly]

From: Martin Kelly <martin@martingkelly.com>

Currently, we snap the timestamp after reading from the buffer and
processing the event. When the IIO poll function is triggered by an
interrupt,  we can get a slightly more accurate timestamp by snapping it
prior to reading the data, since the data was already generated prior to
entering the trigger handler. This is not going to make a huge difference,
but we might as well improve slightly.

Do this by using iio_pollfunc_store_time as other drivers do.

Signed-off-by: Martin Kelly <martin@martingkelly.com>
---
 drivers/iio/imu/bmi160/bmi160_core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/imu/bmi160/bmi160_core.c b/drivers/iio/imu/bmi160/bmi160_core.c
index 70739e3920af..de36fe20e023 100644
--- a/drivers/iio/imu/bmi160/bmi160_core.c
+++ b/drivers/iio/imu/bmi160/bmi160_core.c
@@ -425,8 +425,7 @@ static irqreturn_t bmi160_trigger_handler(int irq, void *p)
 		buf[j++] = sample;
 	}
 
-	iio_push_to_buffers_with_timestamp(indio_dev, buf,
-					   iio_get_time_ns(indio_dev));
+	iio_push_to_buffers_with_timestamp(indio_dev, buf, pf->timestamp);
 done:
 	iio_trigger_notify_done(indio_dev->trig);
 	return IRQ_HANDLED;
@@ -834,7 +833,8 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->info = &bmi160_info;
 
-	ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
+	ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
+					      iio_pollfunc_store_time,
 					      bmi160_trigger_handler, NULL);
 	if (ret < 0)
 		return ret;
-- 
2.11.0

[PATCH v3 6/6] iio:bmi160: use if (ret) instead of if (ret < 0)

[Martin Kelly]

From: Martin Kelly <martin@martingkelly.com>

We are using "if (ret < 0)" in many places in which the function returns 0
on success. Use "if (ret)" instead for better clarity and correctness.

Signed-off-by: Martin Kelly <martin@martingkelly.com>
---
 drivers/iio/imu/bmi160/bmi160_core.c | 40 ++++++++++++++++--------------------
 1 file changed, 18 insertions(+), 22 deletions(-)

diff --git a/drivers/iio/imu/bmi160/bmi160_core.c b/drivers/iio/imu/bmi160/bmi160_core.c
index de36fe20e023..6cc11afd395e 100644
--- a/drivers/iio/imu/bmi160/bmi160_core.c
+++ b/drivers/iio/imu/bmi160/bmi160_core.c
@@ -299,7 +299,7 @@ int bmi160_set_mode(struct bmi160_data *data, enum bmi160_sensor_type t,
 		cmd = bmi160_regs[t].pmu_cmd_suspend;
 
 	ret = regmap_write(data->regmap, BMI160_REG_CMD, cmd);
-	if (ret < 0)
+	if (ret)
 		return ret;
 
 	usleep_range(bmi160_pmu_time[t], bmi160_pmu_time[t] + 1000);
@@ -331,7 +331,7 @@ int bmi160_get_scale(struct bmi160_data *data, enum bmi160_sensor_type t,
 	int i, ret, val;
 
 	ret = regmap_read(data->regmap, bmi160_regs[t].range, &val);
-	if (ret < 0)
+	if (ret)
 		return ret;
 
 	for (i = 0; i < bmi160_scale_table[t].num; i++)
@@ -354,7 +354,7 @@ static int bmi160_get_data(struct bmi160_data *data, int chan_type,
 	reg = bmi160_regs[t].data + (axis - IIO_MOD_X) * sizeof(sample);
 
 	ret = regmap_bulk_read(data->regmap, reg, &sample, sizeof(sample));
-	if (ret < 0)
+	if (ret)
 		return ret;
 
 	*val = sign_extend32(le16_to_cpu(sample), 15);
@@ -388,7 +388,7 @@ static int bmi160_get_odr(struct bmi160_data *data, enum bmi160_sensor_type t,
 	int i, val, ret;
 
 	ret = regmap_read(data->regmap, bmi160_regs[t].config, &val);
-	if (ret < 0)
+	if (ret)
 		return ret;
 
 	val &= bmi160_regs[t].config_odr_mask;
@@ -420,7 +420,7 @@ static irqreturn_t bmi160_trigger_handler(int irq, void *p)
 			 indio_dev->masklength) {
 		ret = regmap_bulk_read(data->regmap, base + i * sizeof(sample),
 				       &sample, sizeof(sample));
-		if (ret < 0)
+		if (ret)
 			goto done;
 		buf[j++] = sample;
 	}
@@ -441,18 +441,18 @@ static int bmi160_read_raw(struct iio_dev *indio_dev,
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
 		ret = bmi160_get_data(data, chan->type, chan->channel2, val);
-		if (ret < 0)
+		if (ret)
 			return ret;
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
 		*val = 0;
 		ret = bmi160_get_scale(data,
 				       bmi160_to_sensor(chan->type), val2);
-		return ret < 0 ? ret : IIO_VAL_INT_PLUS_MICRO;
+		return ret ? ret : IIO_VAL_INT_PLUS_MICRO;
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		ret = bmi160_get_odr(data, bmi160_to_sensor(chan->type),
 				     val, val2);
-		return ret < 0 ? ret : IIO_VAL_INT_PLUS_MICRO;
+		return ret ? ret : IIO_VAL_INT_PLUS_MICRO;
 	default:
 		return -EINVAL;
 	}
@@ -710,7 +710,7 @@ static int bmi160_chip_init(struct bmi160_data *data, bool use_spi)
 	struct device *dev = regmap_get_device(data->regmap);
 
 	ret = regmap_write(data->regmap, BMI160_REG_CMD, BMI160_CMD_SOFTRESET);
-	if (ret < 0)
+	if (ret)
 		return ret;
 
 	usleep_range(BMI160_SOFTRESET_USLEEP, BMI160_SOFTRESET_USLEEP + 1);
@@ -721,12 +721,12 @@ static int bmi160_chip_init(struct bmi160_data *data, bool use_spi)
 	 */
 	if (use_spi) {
 		ret = regmap_read(data->regmap, BMI160_REG_DUMMY, &val);
-		if (ret < 0)
+		if (ret)
 			return ret;
 	}
 
 	ret = regmap_read(data->regmap, BMI160_REG_CHIP_ID, &val);
-	if (ret < 0) {
+	if (ret) {
 		dev_err(dev, "Error reading chip id\n");
 		return ret;
 	}
@@ -737,11 +737,11 @@ static int bmi160_chip_init(struct bmi160_data *data, bool use_spi)
 	}
 
 	ret = bmi160_set_mode(data, BMI160_ACCEL, true);
-	if (ret < 0)
+	if (ret)
 		return ret;
 
 	ret = bmi160_set_mode(data, BMI160_GYRO, true);
-	if (ret < 0)
+	if (ret)
 		return ret;
 
 	return 0;
@@ -774,7 +774,7 @@ int bmi160_probe_trigger(struct iio_dev *indio_dev, int irq, u32 irq_type)
 	ret = devm_request_irq(&indio_dev->dev, irq,
 			       &iio_trigger_generic_data_rdy_poll,
 			       irq_type, "bmi160", data->trig);
-	if (ret < 0)
+	if (ret)
 		return ret;
 
 	data->trig->dev.parent = regmap_get_device(data->regmap);
@@ -816,11 +816,11 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
 	data->regmap = regmap;
 
 	ret = bmi160_chip_init(data, use_spi);
-	if (ret < 0)
+	if (ret)
 		return ret;
 
 	ret = devm_add_action_or_reset(dev, bmi160_chip_uninit, data);
-	if (ret < 0)
+	if (ret)
 		return ret;
 
 	if (!name && ACPI_HANDLE(dev))
@@ -836,7 +836,7 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
 	ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
 					      iio_pollfunc_store_time,
 					      bmi160_trigger_handler, NULL);
-	if (ret < 0)
+	if (ret)
 		return ret;
 
 	irq = bmi160_get_irq(dev->of_node, &int_pin);
@@ -849,11 +849,7 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
 		dev_info(&indio_dev->dev, "Not setting up IRQ trigger\n");
 	}
 
-	ret = devm_iio_device_register(dev, indio_dev);
-	if (ret < 0)
-		return ret;
-
-	return 0;
+	return devm_iio_device_register(dev, indio_dev);
 }
 EXPORT_SYMBOL_GPL(bmi160_core_probe);
 
-- 
2.11.0

Re: [PATCH v3 0/6] iio:bmi160: add drdy interrupt support

[Jonathan Cameron]

On Sun, 27 Jan 2019 12:39:05 -0800
Martin Kelly <martin@martingkelly.com> wrote:

> From: Martin Kelly <martin@martingkelly.com>
> 
> This patch series adds drdy interrupt support to the BMI160 IMU. It also updates
> the relevant DT bindings and does a little bit of related cleanup.
> 
> v2:
> - Drop "BOTH" interrupt setting.
> - Change to "if (ret)" instead of "if (ret < 0)".
> - Stylistic changes suggested by Jonathan Cameron.
> - Fix bogus return check after iio_trigger_get.
> 
> v3:
> - More cleanup changes.
> - Directly get INT1/INT2 by name instead of using I2C/SPI IRQs. Move this code
>   to the probe function.
> - Fix EDGE/LEVEL #define naming. The code was correct but the names were wrong.
> 
> Martin Kelly (6):
>   iio:bmi160: add SPDX identifiers
>   iio:bmi160: add drdy interrupt support
>   dt-bindings: fix incorrect bmi160 IRQ note
>   dt-bindings: document open-drain property
>   iio:bmi160: use iio_pollfunc_store_time
>   iio:bmi160: use if (ret) instead of if (ret < 0)
> 
>  .../devicetree/bindings/iio/imu/bmi160.txt         |   6 +-
>  drivers/iio/imu/bmi160/bmi160.h                    |  11 +
>  drivers/iio/imu/bmi160/bmi160_core.c               | 317 +++++++++++++++++++--
>  drivers/iio/imu/bmi160/bmi160_i2c.c                |   5 +-
>  drivers/iio/imu/bmi160/bmi160_spi.c                |   4 +-
>  5 files changed, 304 insertions(+), 39 deletions(-)
Other than addressing Rob's comments I think this is looking good.
Will take one last look at v4 of course!

Thanks,

Jonathan

> 
> --
> 2.11.0
>