[RFC PATCH 0/9] iio: Fix ABBA deadlock in inv-mpu6050

[RFC PATCH 0/9] iio: Fix ABBA deadlock in inv-mpu6050

[Daniel Baluta]

Sending this as an RFC because I don't know if style fixes are appropriate
for this driver and also not sure if deadlock fix is the best solution.

I2C people should only look at patches 8/9 and 9/9.

The mpu6050 accel+gyro combo has an auxiliary I2C bus, allowing for
an auxiliary sensor to be connected (eg. a magnetometer).

The mpu can either act as an I2C master (functionality not currently
implemented in the driver) or it can use a bypass multiplexer which
directly connects the auxiliary I2C bus pins to the main I2C bus pins [1]
Currently the driver implements the bypass mode via an I2C mux.

This patchset fixes a deadlock in inv-mpu6050 IMU which happens when
magnetometer (on auxiliary I2C bus) runs in parallel with accel or gyro.

First 7 patches do some cleanup in order to make INV MPU6050 code easier
to read.

Patch number 8 allows passing NULL select callbacks to i2c_mux_master_xfer
and i2c_mux_smbus_xfer.

Patch number 9 actually fixes the deadlock.

[1] (page 28, https://www.cdiweb.com/datasheets/invensense/MPU-6050_DataSheet_V3%204.pdf)

Adriana Reus (2):
  i2c: i2c-mux: Allow for NULL select callback
  iio: imu: inv_mpu6050: Fix deadlock between i2c adapter lock and mpu
    lock

Daniel Baluta (7):
  iio: imu: inv_mpu6050: Fix multiline comments style
  iio: imu: inv_mpu6050: Fix Yoda conditions
  iio: imu: inv_mpu6050: Fix newlines to make code easier to read
  iio: imu: inv_mpu6050: Remove unnecessary parentheses
  iio: imu: inv_mpu6050: Delete space before comma
  iio: imu: inv_mpu6050: Fix code indent for if statement
  iio: imu: inv_mpu6050: Fix alignment with open parenthesis

 drivers/i2c/i2c-mux.c                         |  10 ++-
 drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c    |   6 +-
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c    | 101 ++++++++++++++------------
 drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c     |  90 ++++-------------------
 drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c    |  23 +++---
 drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c |  22 +++---
 6 files changed, 102 insertions(+), 150 deletions(-)

-- 
2.5.0

[RFC PATCH 1/9] iio: imu: inv_mpu6050: Fix multiline comments style

[Daniel Baluta]

The preffered style for long (multi-line) comments is:

/*
 * this is a multiline
 * comment
 */

This also fixes checkpatch.pl warning:
WARNING: Block comments use * on subsequent lines

Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 37 +++++++++++++++++++-----------
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index 2258600..84e014c 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -79,10 +79,11 @@ int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask)
 {
 	unsigned int d, mgmt_1;
 	int result;
-
-	/* switch clock needs to be careful. Only when gyro is on, can
-	   clock source be switched to gyro. Otherwise, it must be set to
-	   internal clock */
+	/*
+	 * switch clock needs to be careful. Only when gyro is on, can
+	 * clock source be switched to gyro. Otherwise, it must be set to
+	 * internal clock
+	 */
 	if (INV_MPU6050_BIT_PWR_GYRO_STBY == mask) {
 		result = regmap_read(st->map, st->reg->pwr_mgmt_1, &mgmt_1);
 		if (result)
@@ -92,8 +93,10 @@ int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask)
 	}
 
 	if ((INV_MPU6050_BIT_PWR_GYRO_STBY == mask) && (!en)) {
-		/* turning off gyro requires switch to internal clock first.
-		   Then turn off gyro engine */
+		/*
+		 * turning off gyro requires switch to internal clock first.
+		 * Then turn off gyro engine
+		 */
 		mgmt_1 |= INV_CLK_INTERNAL;
 		result = regmap_write(st->map, st->reg->pwr_mgmt_1, mgmt_1);
 		if (result)
@@ -391,8 +394,10 @@ static int inv_mpu6050_write_raw(struct iio_dev *indio_dev,
 	int result;
 
 	mutex_lock(&indio_dev->mlock);
-	/* we should only update scale when the chip is disabled, i.e.,
-		not running */
+	/*
+	 * we should only update scale when the chip is disabled, i.e.
+	 * not running
+	 */
 	if (st->chip_config.enable) {
 		result = -EBUSY;
 		goto error_write_raw;
@@ -529,8 +534,10 @@ static ssize_t inv_attr_show(struct device *dev,
 	s8 *m;
 
 	switch (this_attr->address) {
-	/* In MPU6050, the two matrix are the same because gyro and accel
-	   are integrated in one chip */
+	/*
+	 * In MPU6050, the two matrix are the same because gyro and accel
+	 * are integrated in one chip
+	 */
 	case ATTR_GYRO_MATRIX:
 	case ATTR_ACCL_MATRIX:
 		m = st->plat_data.orientation;
@@ -654,10 +661,12 @@ static int inv_check_and_setup_chip(struct inv_mpu6050_state *st)
 	if (result)
 		return result;
 	msleep(INV_MPU6050_POWER_UP_TIME);
-	/* toggle power state. After reset, the sleep bit could be on
-		or off depending on the OTP settings. Toggling power would
-		make it in a definite state as well as making the hardware
-		state align with the software state */
+	/*
+	 * toggle power state. After reset, the sleep bit could be on
+	 * or off depending on the OTP settings. Toggling power would
+	 * make it in a definite state as well as making the hardware
+	 * state align with the software state
+	 */
 	result = inv_mpu6050_set_power_itg(st, false);
 	if (result)
 		return result;
-- 
2.5.0

[RFC PATCH 2/9] iio: imu: inv_mpu6050: Fix Yoda conditions

[Daniel Baluta]

This fixes the following checkpatch warning:
	* WARNING: Comparisons should place the constant
	  on the right side of the test

Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 6 +++---
 drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index 84e014c..c550ebb 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -84,7 +84,7 @@ int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask)
 	 * clock source be switched to gyro. Otherwise, it must be set to
 	 * internal clock
 	 */
-	if (INV_MPU6050_BIT_PWR_GYRO_STBY == mask) {
+	if (mask == INV_MPU6050_BIT_PWR_GYRO_STBY) {
 		result = regmap_read(st->map, st->reg->pwr_mgmt_1, &mgmt_1);
 		if (result)
 			return result;
@@ -92,7 +92,7 @@ int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask)
 		mgmt_1 &= ~INV_MPU6050_BIT_CLK_MASK;
 	}
 
-	if ((INV_MPU6050_BIT_PWR_GYRO_STBY == mask) && (!en)) {
+	if ((mask == INV_MPU6050_BIT_PWR_GYRO_STBY) && (!en)) {
 		/*
 		 * turning off gyro requires switch to internal clock first.
 		 * Then turn off gyro engine
@@ -117,7 +117,7 @@ int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask)
 	if (en) {
 		/* Wait for output stabilize */
 		msleep(INV_MPU6050_TEMP_UP_TIME);
-		if (INV_MPU6050_BIT_PWR_GYRO_STBY == mask) {
+		if (mask == INV_MPU6050_BIT_PWR_GYRO_STBY) {
 			/* switch internal clock to PLL */
 			mgmt_1 |= INV_CLK_PLL;
 			result = regmap_write(st->map,
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
index 1fc5fd9..441080b 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
@@ -168,7 +168,7 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
 
 		result = kfifo_out(&st->timestamps, &timestamp, 1);
 		/* when there is no timestamp, put timestamp as 0 */
-		if (0 == result)
+		if (result == 0)
 			timestamp = 0;
 
 		result = iio_push_to_buffers_with_timestamp(indio_dev, data,
-- 
2.5.0

[RFC PATCH 3/9] iio: imu: inv_mpu6050: Fix newlines to make code easier to read

[Daniel Baluta]

This fixes the following checkpatch.pl warnings:
	* WARNING: Missing a blank line after declarations
	* CHECK: Blank lines aren't necessary before a close brace '}'

Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
 drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c | 2 +-
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
index 0bcfa8d..231a7af 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
@@ -186,7 +186,6 @@ int inv_mpu_acpi_create_mux_client(struct i2c_client *client)
 		st->mux_client = i2c_new_device(st->mux_adapter, &info);
 		if (!st->mux_client)
 			return -ENODEV;
-
 	}
 
 	return 0;
@@ -195,6 +194,7 @@ int inv_mpu_acpi_create_mux_client(struct i2c_client *client)
 void inv_mpu_acpi_delete_mux_client(struct i2c_client *client)
 {
 	struct inv_mpu6050_state *st = iio_priv(dev_get_drvdata(&client->dev));
+
 	if (st->mux_client)
 		i2c_unregister_device(st->mux_client);
 }
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index c550ebb..72cc478 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -365,6 +365,7 @@ static int inv_write_raw_get_fmt(struct iio_dev *indio_dev,
 
 	return -EINVAL;
 }
+
 static int inv_mpu6050_write_accel_scale(struct inv_mpu6050_state *st, int val)
 {
 	int result, i;
-- 
2.5.0

[RFC PATCH 4/9] iio: imu: inv_mpu6050: Remove unnecessary parentheses

[Daniel Baluta]

Fixes the following checkpatch warning:
CHECK: Unnecessary parentheses around cpm->package.elements[i]

Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
 drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
index 231a7af..2771106 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
@@ -66,11 +66,11 @@ static int asus_acpi_get_sensor_info(struct acpi_device *adev,
 		union acpi_object *elem;
 		int j;
 
-		elem = &(cpm->package.elements[i]);
+		elem = &cpm->package.elements[i];
 		for (j = 0; j < elem->package.count; ++j) {
 			union acpi_object *sub_elem;
 
-			sub_elem = &(elem->package.elements[j]);
+			sub_elem = &elem->package.elements[j];
 			if (sub_elem->type == ACPI_TYPE_STRING)
 				strlcpy(info->type, sub_elem->string.pointer,
 					sizeof(info->type));
-- 
2.5.0

[RFC PATCH 5/9] iio: imu: inv_mpu6050: Delete space before comma

[Daniel Baluta]

This fixes the following checkpatch.pl warning:

ERROR: space prohibited before that ',' (ctx:WxE)
		.shift = 0 ,

Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index 72cc478..48ab575 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -582,7 +582,7 @@ static int inv_mpu6050_validate_trigger(struct iio_dev *indio_dev,
 				.sign = 's',                          \
 				.realbits = 16,                       \
 				.storagebits = 16,                    \
-				.shift = 0 ,                          \
+				.shift = 0,                           \
 				.endianness = IIO_BE,                 \
 			     },                                       \
 	}
-- 
2.5.0

[RFC PATCH 6/9] iio: imu: inv_mpu6050: Fix code indent for if statement

[Daniel Baluta]

This fixes the following checkpatch.pl warning:

WARNING: suspect code indent for conditional statements (8, 24)
+       if (kfifo_len(&st->timestamps) >
[...]
+                       goto flush_fifo;

Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
 drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
index 441080b..0bc5091 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
@@ -158,8 +158,8 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
 		goto flush_fifo;
 	/* Timestamp mismatch. */
 	if (kfifo_len(&st->timestamps) >
-		fifo_count / bytes_per_datum + INV_MPU6050_TIME_STAMP_TOR)
-			goto flush_fifo;
+	    fifo_count / bytes_per_datum + INV_MPU6050_TIME_STAMP_TOR)
+		goto flush_fifo;
 	while (fifo_count >= bytes_per_datum) {
 		result = regmap_bulk_read(st->map, st->reg->fifo_r_w,
 					  data, bytes_per_datum);
-- 
2.5.0

[RFC PATCH 7/9] iio: imu: inv_mpu6050: Fix alignment with open parenthesis

[Daniel Baluta]

This makes code consistent around inv_mpu6050 driver and
fixes the following checkpatch.pl warning:
CHECK: Alignment should match open parenthesis

Note that there were few cases were it was not possible to
fix this due to making the line too long, but we can live with that.

Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c    | 55 +++++++++++++--------------
 drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c     |  2 +-
 drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c    | 17 ++++-----
 drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c | 22 +++++------
 4 files changed, 47 insertions(+), 49 deletions(-)

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index 48ab575..1643cd2 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -121,7 +121,7 @@ int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask)
 			/* switch internal clock to PLL */
 			mgmt_1 |= INV_CLK_PLL;
 			result = regmap_write(st->map,
-					st->reg->pwr_mgmt_1, mgmt_1);
+					      st->reg->pwr_mgmt_1, mgmt_1);
 			if (result)
 				return result;
 		}
@@ -196,14 +196,14 @@ static int inv_mpu6050_init_config(struct iio_dev *indio_dev)
 		return result;
 
 	memcpy(&st->chip_config, hw_info[st->chip_type].config,
-		sizeof(struct inv_mpu6050_chip_config));
+	       sizeof(struct inv_mpu6050_chip_config));
 	result = inv_mpu6050_set_power_itg(st, false);
 
 	return result;
 }
 
 static int inv_mpu6050_sensor_show(struct inv_mpu6050_state  *st, int reg,
-				int axis, int *val)
+				   int axis, int *val)
 {
 	int ind, result;
 	__be16 d;
@@ -217,11 +217,10 @@ static int inv_mpu6050_sensor_show(struct inv_mpu6050_state  *st, int reg,
 	return IIO_VAL_INT;
 }
 
-static int inv_mpu6050_read_raw(struct iio_dev *indio_dev,
-			      struct iio_chan_spec const *chan,
-			      int *val,
-			      int *val2,
-			      long mask) {
+static int
+inv_mpu6050_read_raw(struct iio_dev *indio_dev,
+		     struct iio_chan_spec const *chan,
+		     int *val, int *val2, long mask) {
 	struct inv_mpu6050_state  *st = iio_priv(indio_dev);
 
 	switch (mask) {
@@ -241,16 +240,16 @@ static int inv_mpu6050_read_raw(struct iio_dev *indio_dev,
 		switch (chan->type) {
 		case IIO_ANGL_VEL:
 			if (!st->chip_config.gyro_fifo_enable ||
-					!st->chip_config.enable) {
+			    !st->chip_config.enable) {
 				result = inv_mpu6050_switch_engine(st, true,
 						INV_MPU6050_BIT_PWR_GYRO_STBY);
 				if (result)
 					goto error_read_raw;
 			}
 			ret =  inv_mpu6050_sensor_show(st, st->reg->raw_gyro,
-						chan->channel2, val);
+						       chan->channel2, val);
 			if (!st->chip_config.gyro_fifo_enable ||
-					!st->chip_config.enable) {
+			    !st->chip_config.enable) {
 				result = inv_mpu6050_switch_engine(st, false,
 						INV_MPU6050_BIT_PWR_GYRO_STBY);
 				if (result)
@@ -259,16 +258,16 @@ static int inv_mpu6050_read_raw(struct iio_dev *indio_dev,
 			break;
 		case IIO_ACCEL:
 			if (!st->chip_config.accl_fifo_enable ||
-					!st->chip_config.enable) {
+			    !st->chip_config.enable) {
 				result = inv_mpu6050_switch_engine(st, true,
 						INV_MPU6050_BIT_PWR_ACCL_STBY);
 				if (result)
 					goto error_read_raw;
 			}
 			ret = inv_mpu6050_sensor_show(st, st->reg->raw_accl,
-						chan->channel2, val);
+						      chan->channel2, val);
 			if (!st->chip_config.accl_fifo_enable ||
-					!st->chip_config.enable) {
+			    !st->chip_config.enable) {
 				result = inv_mpu6050_switch_engine(st, false,
 						INV_MPU6050_BIT_PWR_ACCL_STBY);
 				if (result)
@@ -279,7 +278,7 @@ static int inv_mpu6050_read_raw(struct iio_dev *indio_dev,
 			/* wait for stablization */
 			msleep(INV_MPU6050_SENSOR_UP_TIME);
 			inv_mpu6050_sensor_show(st, st->reg->temperature,
-							IIO_MOD_X, val);
+						IIO_MOD_X, val);
 			break;
 		default:
 			ret = -EINVAL;
@@ -387,10 +386,8 @@ static int inv_mpu6050_write_accel_scale(struct inv_mpu6050_state *st, int val)
 }
 
 static int inv_mpu6050_write_raw(struct iio_dev *indio_dev,
-			       struct iio_chan_spec const *chan,
-			       int val,
-			       int val2,
-			       long mask) {
+				 struct iio_chan_spec const *chan,
+				 int val, int val2, long mask) {
 	struct inv_mpu6050_state  *st = iio_priv(indio_dev);
 	int result;
 
@@ -467,8 +464,9 @@ static int inv_mpu6050_set_lpf(struct inv_mpu6050_state *st, int rate)
 /**
  * inv_mpu6050_fifo_rate_store() - Set fifo rate.
  */
-static ssize_t inv_mpu6050_fifo_rate_store(struct device *dev,
-	struct device_attribute *attr, const char *buf, size_t count)
+static ssize_t
+inv_mpu6050_fifo_rate_store(struct device *dev, struct device_attribute *attr,
+			    const char *buf, size_t count)
 {
 	s32 fifo_rate;
 	u8 d;
@@ -479,7 +477,7 @@ static ssize_t inv_mpu6050_fifo_rate_store(struct device *dev,
 	if (kstrtoint(buf, 10, &fifo_rate))
 		return -EINVAL;
 	if (fifo_rate < INV_MPU6050_MIN_FIFO_RATE ||
-				fifo_rate > INV_MPU6050_MAX_FIFO_RATE)
+	    fifo_rate > INV_MPU6050_MAX_FIFO_RATE)
 		return -EINVAL;
 	if (fifo_rate == st->chip_config.fifo_rate)
 		return count;
@@ -515,8 +513,9 @@ fifo_rate_fail:
 /**
  * inv_fifo_rate_show() - Get the current sampling rate.
  */
-static ssize_t inv_fifo_rate_show(struct device *dev,
-	struct device_attribute *attr, char *buf)
+static ssize_t
+inv_fifo_rate_show(struct device *dev, struct device_attribute *attr,
+		   char *buf)
 {
 	struct inv_mpu6050_state *st = iio_priv(dev_to_iio_dev(dev));
 
@@ -527,8 +526,8 @@ static ssize_t inv_fifo_rate_show(struct device *dev,
  * inv_attr_show() - calling this function will show current
  *                    parameters.
  */
-static ssize_t inv_attr_show(struct device *dev,
-	struct device_attribute *attr, char *buf)
+static ssize_t inv_attr_show(struct device *dev, struct device_attribute *attr,
+			     char *buf)
 {
 	struct inv_mpu6050_state *st = iio_priv(dev_to_iio_dev(dev));
 	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
@@ -676,11 +675,11 @@ static int inv_check_and_setup_chip(struct inv_mpu6050_state *st)
 		return result;
 
 	result = inv_mpu6050_switch_engine(st, false,
-					INV_MPU6050_BIT_PWR_ACCL_STBY);
+					   INV_MPU6050_BIT_PWR_ACCL_STBY);
 	if (result)
 		return result;
 	result = inv_mpu6050_switch_engine(st, false,
-					INV_MPU6050_BIT_PWR_GYRO_STBY);
+					   INV_MPU6050_BIT_PWR_GYRO_STBY);
 	if (result)
 		return result;
 
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
index af400dd..71bdaa3 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
@@ -111,7 +111,7 @@ static int inv_mpu6050_deselect_bypass(struct i2c_adapter *adap,
  *  Returns 0 on success, a negative error code otherwise.
  */
 static int inv_mpu_probe(struct i2c_client *client,
-	const struct i2c_device_id *id)
+			 const struct i2c_device_id *id)
 {
 	struct inv_mpu6050_state *st;
 	int result;
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
index 0bc5091..d070062 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
@@ -57,7 +57,7 @@ int inv_reset_fifo(struct iio_dev *indio_dev)
 
 	/* reset FIFO*/
 	result = regmap_write(st->map, st->reg->user_ctrl,
-					INV_MPU6050_BIT_FIFO_RST);
+			      INV_MPU6050_BIT_FIFO_RST);
 	if (result)
 		goto reset_fifo_fail;
 
@@ -68,13 +68,13 @@ int inv_reset_fifo(struct iio_dev *indio_dev)
 	if (st->chip_config.accl_fifo_enable ||
 	    st->chip_config.gyro_fifo_enable) {
 		result = regmap_write(st->map, st->reg->int_enable,
-					INV_MPU6050_BIT_DATA_RDY_EN);
+				      INV_MPU6050_BIT_DATA_RDY_EN);
 		if (result)
 			return result;
 	}
 	/* enable FIFO reading and I2C master interface*/
 	result = regmap_write(st->map, st->reg->user_ctrl,
-					INV_MPU6050_BIT_FIFO_EN);
+			      INV_MPU6050_BIT_FIFO_EN);
 	if (result)
 		goto reset_fifo_fail;
 	/* enable sensor output to FIFO */
@@ -92,7 +92,7 @@ int inv_reset_fifo(struct iio_dev *indio_dev)
 reset_fifo_fail:
 	dev_err(regmap_get_device(st->map), "reset fifo failed %d\n", result);
 	result = regmap_write(st->map, st->reg->int_enable,
-					INV_MPU6050_BIT_DATA_RDY_EN);
+			      INV_MPU6050_BIT_DATA_RDY_EN);
 
 	return result;
 }
@@ -109,7 +109,7 @@ irqreturn_t inv_mpu6050_irq_handler(int irq, void *p)
 
 	timestamp = iio_get_time_ns();
 	kfifo_in_spinlocked(&st->timestamps, &timestamp, 1,
-				&st->time_stamp_lock);
+			    &st->time_stamp_lock);
 
 	return IRQ_WAKE_THREAD;
 }
@@ -143,9 +143,8 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
 	 * read fifo_count register to know how many bytes inside FIFO
 	 * right now
 	 */
-	result = regmap_bulk_read(st->map,
-				       st->reg->fifo_count_h,
-				       data, INV_MPU6050_FIFO_COUNT_BYTE);
+	result = regmap_bulk_read(st->map, st->reg->fifo_count_h, data,
+				  INV_MPU6050_FIFO_COUNT_BYTE);
 	if (result)
 		goto end_session;
 	fifo_count = be16_to_cpup((__be16 *)(&data[0]));
@@ -172,7 +171,7 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
 			timestamp = 0;
 
 		result = iio_push_to_buffers_with_timestamp(indio_dev, data,
-			timestamp);
+							    timestamp);
 		if (result)
 			goto flush_fifo;
 		fifo_count -= bytes_per_datum;
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
index 72d6aae..e8818d4 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
@@ -19,19 +19,19 @@ static void inv_scan_query(struct iio_dev *indio_dev)
 
 	st->chip_config.gyro_fifo_enable =
 		test_bit(INV_MPU6050_SCAN_GYRO_X,
-			indio_dev->active_scan_mask) ||
-			test_bit(INV_MPU6050_SCAN_GYRO_Y,
-			indio_dev->active_scan_mask) ||
-			test_bit(INV_MPU6050_SCAN_GYRO_Z,
-			indio_dev->active_scan_mask);
+			 indio_dev->active_scan_mask) ||
+		test_bit(INV_MPU6050_SCAN_GYRO_Y,
+			 indio_dev->active_scan_mask) ||
+		test_bit(INV_MPU6050_SCAN_GYRO_Z,
+			 indio_dev->active_scan_mask);
 
 	st->chip_config.accl_fifo_enable =
 		test_bit(INV_MPU6050_SCAN_ACCL_X,
-			indio_dev->active_scan_mask) ||
-			test_bit(INV_MPU6050_SCAN_ACCL_Y,
-			indio_dev->active_scan_mask) ||
-			test_bit(INV_MPU6050_SCAN_ACCL_Z,
-			indio_dev->active_scan_mask);
+			 indio_dev->active_scan_mask) ||
+		test_bit(INV_MPU6050_SCAN_ACCL_Y,
+			 indio_dev->active_scan_mask) ||
+		test_bit(INV_MPU6050_SCAN_ACCL_Z,
+			 indio_dev->active_scan_mask);
 }
 
 /**
@@ -101,7 +101,7 @@ static int inv_mpu6050_set_enable(struct iio_dev *indio_dev, bool enable)
  * @state: Desired trigger state
  */
 static int inv_mpu_data_rdy_trigger_set_state(struct iio_trigger *trig,
-						bool state)
+					      bool state)
 {
 	return inv_mpu6050_set_enable(iio_trigger_get_drvdata(trig), state);
 }
-- 
2.5.0

[RFC PATCH 8/9] i2c: i2c-mux: Allow for NULL select callback

[Daniel Baluta]

From: Adriana Reus <adriana.reus@intel.com>

Add a check in i2c_mux_master_xfer before calling the select callback.
This is necessary so that NULL callbacks can be safely registered.

Signed-off-by: Adriana Reus <adriana.reus@intel.com>
Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
 drivers/i2c/i2c-mux.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 00fc5b1..74d1700 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -46,11 +46,12 @@ static int i2c_mux_master_xfer(struct i2c_adapter *adap,
 {
 	struct i2c_mux_priv *priv = adap->algo_data;
 	struct i2c_adapter *parent = priv->parent;
-	int ret;
+	int ret = 0;
 
 	/* Switch to the right mux port and perform the transfer. */
 
-	ret = priv->select(parent, priv->mux_priv, priv->chan_id);
+	if (priv->select)
+		ret = priv->select(parent, priv->mux_priv, priv->chan_id);
 	if (ret >= 0)
 		ret = __i2c_transfer(parent, msgs, num);
 	if (priv->deselect)
@@ -66,11 +67,12 @@ static int i2c_mux_smbus_xfer(struct i2c_adapter *adap,
 {
 	struct i2c_mux_priv *priv = adap->algo_data;
 	struct i2c_adapter *parent = priv->parent;
-	int ret;
+	int ret = 0;
 
 	/* Select the right mux port and perform the transfer. */
 
-	ret = priv->select(parent, priv->mux_priv, priv->chan_id);
+	if (priv->select)
+		ret = priv->select(parent, priv->mux_priv, priv->chan_id);
 	if (ret >= 0)
 		ret = parent->algo->smbus_xfer(parent, addr, flags,
 					read_write, command, size, data);
-- 
2.5.0

[RFC PATCH 9/9] iio: imu: inv_mpu6050: Fix deadlock between i2c adapter lock and mpu lock

[Daniel Baluta]

From: Adriana Reus <adriana.reus@intel.com>

This deadlock occurs if the accel/gyro and the sensor on the auxiliary
I2C (in my setup it's an ak8975) are working at the same time.

Scenario:

      T1					T2
     ====				       ====
inv_mpu6050_read_fifo                  aux sensor op (eg. ak8975_read_raw)
        |                                     |
mutex_lock(&indio_dev->mlock)           i2c_transfer
        |                                     |
i2c transaction                         i2c adapter lock
        |                                     |
i2c adapter lock                        i2c_mux_master_xfer
                                              |
                                        inv_mpu6050_select_bypass
                                              |
                                        mutex_lock(&indio_dev->mlock)

When we operate on an mpu sensor the order of locking is mpu lock
followed by the i2c adapter lock. However, when we operate the auxiliary
sensor the order of locking is the other way around.

In order to avoid this enable the bypass mux bit once in the beginning
and remove the select/deselect_bypass functions.

This patch moves the bypass enabling in a separate function
that is called once at probe and removes the functionality from
inv_mpu_select/deselect_bypass.

Another advantage of this approach is that power-wise the mpu chip isn't
powered up at each auxiliary sensor i2c transaction so if only the
compass is used this would be more power efficient.

Signed-off-by: Adriana Reus <adriana.reus@intel.com>
Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
 drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c | 88 ++++++-------------------------
 1 file changed, 15 insertions(+), 73 deletions(-)

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
index 71bdaa3..8065355 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
@@ -25,82 +25,25 @@ static const struct regmap_config inv_mpu_regmap_config = {
 	.val_bits = 8,
 };
 
-/*
- * The i2c read/write needs to happen in unlocked mode. As the parent
- * adapter is common. If we use locked versions, it will fail as
- * the mux adapter will lock the parent i2c adapter, while calling
- * select/deselect functions.
- */
-static int inv_mpu6050_write_reg_unlocked(struct i2c_client *client,
-					  u8 reg, u8 d)
+static int inv_mpu6050_bypass_en(struct iio_dev *indio_dev)
 {
-	int ret;
-	u8 buf[2] = {reg, d};
-	struct i2c_msg msg[1] = {
-		{
-			.addr = client->addr,
-			.flags = 0,
-			.len = sizeof(buf),
-			.buf = buf,
-		}
-	};
-
-	ret = __i2c_transfer(client->adapter, msg, 1);
-	if (ret != 1)
-		return ret;
-
-	return 0;
-}
-
-static int inv_mpu6050_select_bypass(struct i2c_adapter *adap, void *mux_priv,
-				     u32 chan_id)
-{
-	struct i2c_client *client = mux_priv;
-	struct iio_dev *indio_dev = dev_get_drvdata(&client->dev);
 	struct inv_mpu6050_state *st = iio_priv(indio_dev);
 	int ret = 0;
 
-	/* Use the same mutex which was used everywhere to protect power-op */
-	mutex_lock(&indio_dev->mlock);
-	if (!st->powerup_count) {
-		ret = inv_mpu6050_write_reg_unlocked(client,
-						     st->reg->pwr_mgmt_1, 0);
-		if (ret)
-			goto write_error;
+	ret = inv_mpu6050_set_power_itg(st, true);
+	if (ret)
+		return ret;
 
-		msleep(INV_MPU6050_REG_UP_TIME);
-	}
-	if (!ret) {
-		st->powerup_count++;
-		ret = inv_mpu6050_write_reg_unlocked(client,
-						     st->reg->int_pin_cfg,
-						     INV_MPU6050_INT_PIN_CFG |
-						     INV_MPU6050_BIT_BYPASS_EN);
+	ret = regmap_write(st->map,
+			   st->reg->int_pin_cfg,
+			   INV_MPU6050_INT_PIN_CFG |
+			   INV_MPU6050_BIT_BYPASS_EN);
+	if (ret) {
+		inv_mpu6050_set_power_itg(st, false);
+		return ret;
 	}
-write_error:
-	mutex_unlock(&indio_dev->mlock);
 
-	return ret;
-}
-
-static int inv_mpu6050_deselect_bypass(struct i2c_adapter *adap,
-				       void *mux_priv, u32 chan_id)
-{
-	struct i2c_client *client = mux_priv;
-	struct iio_dev *indio_dev = dev_get_drvdata(&client->dev);
-	struct inv_mpu6050_state *st = iio_priv(indio_dev);
-
-	mutex_lock(&indio_dev->mlock);
-	/* It doesn't really mattter, if any of the calls fails */
-	inv_mpu6050_write_reg_unlocked(client, st->reg->int_pin_cfg,
-				       INV_MPU6050_INT_PIN_CFG);
-	st->powerup_count--;
-	if (!st->powerup_count)
-		inv_mpu6050_write_reg_unlocked(client, st->reg->pwr_mgmt_1,
-					       INV_MPU6050_BIT_SLEEP);
-	mutex_unlock(&indio_dev->mlock);
-
-	return 0;
+	return inv_mpu6050_set_power_itg(st, false);
 }
 
 /**
@@ -129,7 +72,8 @@ static int inv_mpu_probe(struct i2c_client *client,
 		return PTR_ERR(regmap);
 	}
 
-	result = inv_mpu_core_probe(regmap, client->irq, name, NULL);
+	result = inv_mpu_core_probe(regmap, client->irq, name,
+				    inv_mpu6050_bypass_en);
 	if (result < 0)
 		return result;
 
@@ -137,9 +81,7 @@ static int inv_mpu_probe(struct i2c_client *client,
 	st->mux_adapter = i2c_add_mux_adapter(client->adapter,
 					      &client->dev,
 					      client,
-					      0, 0, 0,
-					      inv_mpu6050_select_bypass,
-					      inv_mpu6050_deselect_bypass);
+					      0, 0, 0, NULL, NULL);
 	if (!st->mux_adapter) {
 		result = -ENODEV;
 		goto out_unreg_device;
-- 
2.5.0

Re: [RFC PATCH 9/9] iio: imu: inv_mpu6050: Fix deadlock between i2c adapter lock and mpu lock

[Srinivas Pandruvada]

On Thu, 2016-02-18 at 17:53 +0200, Daniel Baluta wrote:
> From: Adriana Reus <adriana.reus@intel.com>
> 
> This deadlock occurs if the accel/gyro and the sensor on the
> auxiliary
> I2C (in my setup it's an ak8975) are working at the same time.
> 
> Scenario:
> 
>       T1					T2
>      ====				       ====
> inv_mpu6050_read_fifo                  aux sensor op (eg.
> ak8975_read_raw)
>         |                                     |
> mutex_lock(&indio_dev->mlock)           i2c_transfer
>         |                                     |
> i2c transaction                         i2c adapter lock
>         |                                     |
> i2c adapter lock                        i2c_mux_master_xfer
>                                               |
>                                         inv_mpu6050_select_bypass
>                                               |
>                                         mutex_lock(&indio_dev->mlock)
> 
> When we operate on an mpu sensor the order of locking is mpu lock
> followed by the i2c adapter lock. However, when we operate the
> auxiliary
> sensor the order of locking is the other way around.
> 
> In order to avoid this enable the bypass mux bit once in the
> beginning
> and remove the select/deselect_bypass functions.
> 
> This patch moves the bypass enabling in a separate function
> that is called once at probe and removes the functionality from
> inv_mpu_select/deselect_bypass.
> 
> Another advantage of this approach is that power-wise the mpu chip
> isn't
> powered up at each auxiliary sensor i2c transaction so if only the
> compass is used this would be more power efficient.
There was one comment before that power off will cause the the bypass
i2c lines to disable for some MPU chips which this driver can support.
So get some confirmation from Invensense folks that this is OK for all
MPU chips.

Thanks,
Srinivas

> 
> Signed-off-by: Adriana Reus <adriana.reus@intel.com>
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
> ---
>  drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c | 88 ++++++---------------
> ----------
>  1 file changed, 15 insertions(+), 73 deletions(-)
> 
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> index 71bdaa3..8065355 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> @@ -25,82 +25,25 @@ static const struct regmap_config
> inv_mpu_regmap_config = {
>  	.val_bits = 8,
>  };
>  
> -/*
> - * The i2c read/write needs to happen in unlocked mode. As the
> parent
> - * adapter is common. If we use locked versions, it will fail as
> - * the mux adapter will lock the parent i2c adapter, while calling
> - * select/deselect functions.
> - */
> -static int inv_mpu6050_write_reg_unlocked(struct i2c_client *client,
> -					  u8 reg, u8 d)
> +static int inv_mpu6050_bypass_en(struct iio_dev *indio_dev)
>  {
> -	int ret;
> -	u8 buf[2] = {reg, d};
> -	struct i2c_msg msg[1] = {
> -		{
> -			.addr = client->addr,
> -			.flags = 0,
> -			.len = sizeof(buf),
> -			.buf = buf,
> -		}
> -	};
> -
> -	ret = __i2c_transfer(client->adapter, msg, 1);
> -	if (ret != 1)
> -		return ret;
> -
> -	return 0;
> -}
> -
> -static int inv_mpu6050_select_bypass(struct i2c_adapter *adap, void
> *mux_priv,
> -				     u32 chan_id)
> -{
> -	struct i2c_client *client = mux_priv;
> -	struct iio_dev *indio_dev = dev_get_drvdata(&client->dev);
>  	struct inv_mpu6050_state *st = iio_priv(indio_dev);
>  	int ret = 0;
>  
> -	/* Use the same mutex which was used everywhere to protect
> power-op */
> -	mutex_lock(&indio_dev->mlock);
> -	if (!st->powerup_count) {
> -		ret = inv_mpu6050_write_reg_unlocked(client,
> -						     st->reg-
> >pwr_mgmt_1, 0);
> -		if (ret)
> -			goto write_error;
> +	ret = inv_mpu6050_set_power_itg(st, true);
> +	if (ret)
> +		return ret;
>  
> -		msleep(INV_MPU6050_REG_UP_TIME);
> -	}
> -	if (!ret) {
> -		st->powerup_count++;
> -		ret = inv_mpu6050_write_reg_unlocked(client,
> -						     st->reg-
> >int_pin_cfg,
> -						     INV_MPU6050_INT
> _PIN_CFG |
> -						     INV_MPU6050_BIT
> _BYPASS_EN);
> +	ret = regmap_write(st->map,
> +			   st->reg->int_pin_cfg,
> +			   INV_MPU6050_INT_PIN_CFG |
> +			   INV_MPU6050_BIT_BYPASS_EN);
> +	if (ret) {
> +		inv_mpu6050_set_power_itg(st, false);
> +		return ret;
>  	}
> -write_error:
> -	mutex_unlock(&indio_dev->mlock);
>  
> -	return ret;
> -}
> -
> -static int inv_mpu6050_deselect_bypass(struct i2c_adapter *adap,
> -				       void *mux_priv, u32 chan_id)
> -{
> -	struct i2c_client *client = mux_priv;
> -	struct iio_dev *indio_dev = dev_get_drvdata(&client->dev);
> -	struct inv_mpu6050_state *st = iio_priv(indio_dev);
> -
> -	mutex_lock(&indio_dev->mlock);
> -	/* It doesn't really mattter, if any of the calls fails */
> -	inv_mpu6050_write_reg_unlocked(client, st->reg->int_pin_cfg,
> -				       INV_MPU6050_INT_PIN_CFG);
> -	st->powerup_count--;
> -	if (!st->powerup_count)
> -		inv_mpu6050_write_reg_unlocked(client, st->reg-
> >pwr_mgmt_1,
> -					       INV_MPU6050_BIT_SLEEP
> );
> -	mutex_unlock(&indio_dev->mlock);
> -
> -	return 0;
> +	return inv_mpu6050_set_power_itg(st, false);
>  }
>  
>  /**
> @@ -129,7 +72,8 @@ static int inv_mpu_probe(struct i2c_client
> *client,
>  		return PTR_ERR(regmap);
>  	}
>  
> -	result = inv_mpu_core_probe(regmap, client->irq, name,
> NULL);
> +	result = inv_mpu_core_probe(regmap, client->irq, name,
> +				    inv_mpu6050_bypass_en);
>  	if (result < 0)
>  		return result;
>  
> @@ -137,9 +81,7 @@ static int inv_mpu_probe(struct i2c_client
> *client,
>  	st->mux_adapter = i2c_add_mux_adapter(client->adapter,
>  					      &client->dev,
>  					      client,
> -					      0, 0, 0,
> -					      inv_mpu6050_select_byp
> ass,
> -					      inv_mpu6050_deselect_b
> ypass);
> +					      0, 0, 0, NULL, NULL);
>  	if (!st->mux_adapter) {
>  		result = -ENODEV;
>  		goto out_unreg_device;

Re: [RFC PATCH 2/9] iio: imu: inv_mpu6050: Fix Yoda conditions

[Crt Mori]

This is quite trivial.

Acked-by: Crt Mori <cmo@melexis.com>

On 18 February 2016 at 16:53, Daniel Baluta <daniel.baluta@intel.com> wrote:
> This fixes the following checkpatch warning:
>         * WARNING: Comparisons should place the constant
>           on the right side of the test
>
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
> ---
>  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 6 +++---
>  drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c | 2 +-
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index 84e014c..c550ebb 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -84,7 +84,7 @@ int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask)
>          * clock source be switched to gyro. Otherwise, it must be set to
>          * internal clock
>          */
> -       if (INV_MPU6050_BIT_PWR_GYRO_STBY == mask) {
> +       if (mask == INV_MPU6050_BIT_PWR_GYRO_STBY) {
>                 result = regmap_read(st->map, st->reg->pwr_mgmt_1, &mgmt_1);
>                 if (result)
>                         return result;
> @@ -92,7 +92,7 @@ int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask)
>                 mgmt_1 &= ~INV_MPU6050_BIT_CLK_MASK;
>         }
>
> -       if ((INV_MPU6050_BIT_PWR_GYRO_STBY == mask) && (!en)) {
> +       if ((mask == INV_MPU6050_BIT_PWR_GYRO_STBY) && (!en)) {
>                 /*
>                  * turning off gyro requires switch to internal clock first.
>                  * Then turn off gyro engine
> @@ -117,7 +117,7 @@ int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask)
>         if (en) {
>                 /* Wait for output stabilize */
>                 msleep(INV_MPU6050_TEMP_UP_TIME);
> -               if (INV_MPU6050_BIT_PWR_GYRO_STBY == mask) {
> +               if (mask == INV_MPU6050_BIT_PWR_GYRO_STBY) {
>                         /* switch internal clock to PLL */
>                         mgmt_1 |= INV_CLK_PLL;
>                         result = regmap_write(st->map,
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> index 1fc5fd9..441080b 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> @@ -168,7 +168,7 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
>
>                 result = kfifo_out(&st->timestamps, &timestamp, 1);
>                 /* when there is no timestamp, put timestamp as 0 */
> -               if (0 == result)
> +               if (result == 0)
>                         timestamp = 0;
>
>                 result = iio_push_to_buffers_with_timestamp(indio_dev, data,
> --
> 2.5.0
>

Re: [RFC PATCH 2/9] iio: imu: inv_mpu6050: Fix Yoda conditions

[Crt Mori]

This is quite trivial.

Acked-by: Crt Mori <cmo-fc6wVz46lShBDgjK7y7TUQ@public.gmane.org>

On 18 February 2016 at 16:53, Daniel Baluta <daniel.baluta-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> This fixes the following checkpatch warning:
>         * WARNING: Comparisons should place the constant
>           on the right side of the test
>
> Signed-off-by: Daniel Baluta <daniel.baluta-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 6 +++---
>  drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c | 2 +-
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index 84e014c..c550ebb 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -84,7 +84,7 @@ int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask)
>          * clock source be switched to gyro. Otherwise, it must be set to
>          * internal clock
>          */
> -       if (INV_MPU6050_BIT_PWR_GYRO_STBY == mask) {
> +       if (mask == INV_MPU6050_BIT_PWR_GYRO_STBY) {
>                 result = regmap_read(st->map, st->reg->pwr_mgmt_1, &mgmt_1);
>                 if (result)
>                         return result;
> @@ -92,7 +92,7 @@ int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask)
>                 mgmt_1 &= ~INV_MPU6050_BIT_CLK_MASK;
>         }
>
> -       if ((INV_MPU6050_BIT_PWR_GYRO_STBY == mask) && (!en)) {
> +       if ((mask == INV_MPU6050_BIT_PWR_GYRO_STBY) && (!en)) {
>                 /*
>                  * turning off gyro requires switch to internal clock first.
>                  * Then turn off gyro engine
> @@ -117,7 +117,7 @@ int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask)
>         if (en) {
>                 /* Wait for output stabilize */
>                 msleep(INV_MPU6050_TEMP_UP_TIME);
> -               if (INV_MPU6050_BIT_PWR_GYRO_STBY == mask) {
> +               if (mask == INV_MPU6050_BIT_PWR_GYRO_STBY) {
>                         /* switch internal clock to PLL */
>                         mgmt_1 |= INV_CLK_PLL;
>                         result = regmap_write(st->map,
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> index 1fc5fd9..441080b 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> @@ -168,7 +168,7 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
>
>                 result = kfifo_out(&st->timestamps, &timestamp, 1);
>                 /* when there is no timestamp, put timestamp as 0 */
> -               if (0 == result)
> +               if (result == 0)
>                         timestamp = 0;
>
>                 result = iio_push_to_buffers_with_timestamp(indio_dev, data,
> --
> 2.5.0
>

RE: [RFC PATCH 9/9] iio: imu: inv_mpu6050: Fix deadlock between i2c adapter lock and mpu lock

[Ge Gao]

"there was one comment before that power off will cause the the bypass i2c lines to disable for some MPU chips which this driver can support."
This is right. I remember there was some code about it earlier trying to take advantage of the secondary bus without enabling the MPU chip itself. This is impossible. The secondary bus interface is part of the chip and it will not function without enabling the chip. Without enabling gyro/accel engine, enabling the chip alone takes very little power. It is not worth the trouble to do that.

Thanks,

Ge



-----Original Message-----
From: Srinivas Pandruvada [mailto:srinivas.pandruvada@linux.intel.com] 
Sent: Thursday, February 18, 2016 10:17 AM
To: Daniel Baluta; jic23@kernel.org
Cc: knaack.h@gmx.de; lars@metafoo.de; pmeerw@pmeerw.net; linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org; wsa@the-dreams.de; linux-i2c@vger.kernel.org; lucas.demarchi@intel.com; Ge Gao; adi.reus@gmail.com; cmo@melexis.com; mwelling@ieee.org
Subject: Re: [RFC PATCH 9/9] iio: imu: inv_mpu6050: Fix deadlock between i2c adapter lock and mpu lock

On Thu, 2016-02-18 at 17:53 +0200, Daniel Baluta wrote:
> From: Adriana Reus <adriana.reus@intel.com>
> 
> This deadlock occurs if the accel/gyro and the sensor on the auxiliary 
> I2C (in my setup it's an ak8975) are working at the same time.
> 
> Scenario:
> 
>       T1					T2
>      ====				       ====
> inv_mpu6050_read_fifo                  aux sensor op (eg.
> ak8975_read_raw)
>         |                                     |
> mutex_lock(&indio_dev->mlock)           i2c_transfer
>         |                                     | i2c transaction                         
> i2c adapter lock
>         |                                     | i2c adapter lock                        
> i2c_mux_master_xfer
>                                               |
>                                         inv_mpu6050_select_bypass
>                                               |
>                                         mutex_lock(&indio_dev->mlock)
> 
> When we operate on an mpu sensor the order of locking is mpu lock 
> followed by the i2c adapter lock. However, when we operate the 
> auxiliary sensor the order of locking is the other way around.
> 
> In order to avoid this enable the bypass mux bit once in the beginning 
> and remove the select/deselect_bypass functions.
> 
> This patch moves the bypass enabling in a separate function that is 
> called once at probe and removes the functionality from 
> inv_mpu_select/deselect_bypass.
> 
> Another advantage of this approach is that power-wise the mpu chip 
> isn't powered up at each auxiliary sensor i2c transaction so if only 
> the compass is used this would be more power efficient.
There was one comment before that power off will cause the the bypass i2c lines to disable for some MPU chips which this driver can support.
So get some confirmation from Invensense folks that this is OK for all MPU chips.


Thanks,
Srinivas

> 
> Signed-off-by: Adriana Reus <adriana.reus@intel.com>
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
> ---
>  drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c | 88 ++++++---------------
> ----------
>  1 file changed, 15 insertions(+), 73 deletions(-)
> 
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> index 71bdaa3..8065355 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> @@ -25,82 +25,25 @@ static const struct regmap_config 
> inv_mpu_regmap_config = {
>  	.val_bits = 8,
>  };
>  
> -/*
> - * The i2c read/write needs to happen in unlocked mode. As the parent
> - * adapter is common. If we use locked versions, it will fail as
> - * the mux adapter will lock the parent i2c adapter, while calling
> - * select/deselect functions.
> - */
> -static int inv_mpu6050_write_reg_unlocked(struct i2c_client *client,
> -					  u8 reg, u8 d)
> +static int inv_mpu6050_bypass_en(struct iio_dev *indio_dev)
>  {
> -	int ret;
> -	u8 buf[2] = {reg, d};
> -	struct i2c_msg msg[1] = {
> -		{
> -			.addr = client->addr,
> -			.flags = 0,
> -			.len = sizeof(buf),
> -			.buf = buf,
> -		}
> -	};
> -
> -	ret = __i2c_transfer(client->adapter, msg, 1);
> -	if (ret != 1)
> -		return ret;
> -
> -	return 0;
> -}
> -
> -static int inv_mpu6050_select_bypass(struct i2c_adapter *adap, void 
> *mux_priv,
> -				     u32 chan_id)
> -{
> -	struct i2c_client *client = mux_priv;
> -	struct iio_dev *indio_dev = dev_get_drvdata(&client->dev);
>  	struct inv_mpu6050_state *st = iio_priv(indio_dev);
>  	int ret = 0;
>  
> -	/* Use the same mutex which was used everywhere to protect
> power-op */
> -	mutex_lock(&indio_dev->mlock);
> -	if (!st->powerup_count) {
> -		ret = inv_mpu6050_write_reg_unlocked(client,
> -						     st->reg-
> >pwr_mgmt_1, 0);
> -		if (ret)
> -			goto write_error;
> +	ret = inv_mpu6050_set_power_itg(st, true);
> +	if (ret)
> +		return ret;
>  
> -		msleep(INV_MPU6050_REG_UP_TIME);
> -	}
> -	if (!ret) {
> -		st->powerup_count++;
> -		ret = inv_mpu6050_write_reg_unlocked(client,
> -						     st->reg-
> >int_pin_cfg,
> -						     INV_MPU6050_INT
> _PIN_CFG |
> -						     INV_MPU6050_BIT
> _BYPASS_EN);
> +	ret = regmap_write(st->map,
> +			   st->reg->int_pin_cfg,
> +			   INV_MPU6050_INT_PIN_CFG |
> +			   INV_MPU6050_BIT_BYPASS_EN);
> +	if (ret) {
> +		inv_mpu6050_set_power_itg(st, false);
> +		return ret;
>  	}
> -write_error:
> -	mutex_unlock(&indio_dev->mlock);
>  
> -	return ret;
> -}
> -
> -static int inv_mpu6050_deselect_bypass(struct i2c_adapter *adap,
> -				       void *mux_priv, u32 chan_id)
> -{
> -	struct i2c_client *client = mux_priv;
> -	struct iio_dev *indio_dev = dev_get_drvdata(&client->dev);
> -	struct inv_mpu6050_state *st = iio_priv(indio_dev);
> -
> -	mutex_lock(&indio_dev->mlock);
> -	/* It doesn't really mattter, if any of the calls fails */
> -	inv_mpu6050_write_reg_unlocked(client, st->reg->int_pin_cfg,
> -				       INV_MPU6050_INT_PIN_CFG);
> -	st->powerup_count--;
> -	if (!st->powerup_count)
> -		inv_mpu6050_write_reg_unlocked(client, st->reg-
> >pwr_mgmt_1,
> -					       INV_MPU6050_BIT_SLEEP
> );
> -	mutex_unlock(&indio_dev->mlock);
> -
> -	return 0;
> +	return inv_mpu6050_set_power_itg(st, false);
>  }
>  
>  /**
> @@ -129,7 +72,8 @@ static int inv_mpu_probe(struct i2c_client *client,
>  		return PTR_ERR(regmap);
>  	}
>  
> -	result = inv_mpu_core_probe(regmap, client->irq, name,
> NULL);
> +	result = inv_mpu_core_probe(regmap, client->irq, name,
> +				    inv_mpu6050_bypass_en);
>  	if (result < 0)
>  		return result;
>  
> @@ -137,9 +81,7 @@ static int inv_mpu_probe(struct i2c_client *client,
>  	st->mux_adapter = i2c_add_mux_adapter(client->adapter,
>  					      &client->dev,
>  					      client,
> -					      0, 0, 0,
> -					      inv_mpu6050_select_byp
> ass,
> -					      inv_mpu6050_deselect_b
> ypass);
> +					      0, 0, 0, NULL, NULL);
>  	if (!st->mux_adapter) {
>  		result = -ENODEV;
>  		goto out_unreg_device;

RE: [RFC PATCH 9/9] iio: imu: inv_mpu6050: Fix deadlock between i2c adapter lock and mpu lock

[Ge Gao]

"there was one comment before that power off will cause the the bypass i2c lines to disable for some MPU chips which this driver can support."
This is right. I remember there was some code about it earlier trying to take advantage of the secondary bus without enabling the MPU chip itself. This is impossible. The secondary bus interface is part of the chip and it will not function without enabling the chip. Without enabling gyro/accel engine, enabling the chip alone takes very little power. It is not worth the trouble to do that.

Thanks,

Ge



-----Original Message-----
From: Srinivas Pandruvada [mailto:srinivas.pandruvada@linux.intel.com] 
Sent: Thursday, February 18, 2016 10:17 AM
To: Daniel Baluta; jic23@kernel.org
Cc: knaack.h@gmx.de; lars@metafoo.de; pmeerw@pmeerw.net; linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org; wsa@the-dreams.de; linux-i2c@vger.kernel.org; lucas.demarchi@intel.com; Ge Gao; adi.reus@gmail.com; cmo@melexis.com; mwelling@ieee.org
Subject: Re: [RFC PATCH 9/9] iio: imu: inv_mpu6050: Fix deadlock between i2c adapter lock and mpu lock

On Thu, 2016-02-18 at 17:53 +0200, Daniel Baluta wrote:
> From: Adriana Reus <adriana.reus@intel.com>
> 
> This deadlock occurs if the accel/gyro and the sensor on the auxiliary 
> I2C (in my setup it's an ak8975) are working at the same time.
> 
> Scenario:
> 
>       T1					T2
>      ====				       ====
> inv_mpu6050_read_fifo                  aux sensor op (eg.
> ak8975_read_raw)
>         |                                     |
> mutex_lock(&indio_dev->mlock)           i2c_transfer
>         |                                     | i2c transaction                         
> i2c adapter lock
>         |                                     | i2c adapter lock                        
> i2c_mux_master_xfer
>                                               |
>                                         inv_mpu6050_select_bypass
>                                               |
>                                         mutex_lock(&indio_dev->mlock)
> 
> When we operate on an mpu sensor the order of locking is mpu lock 
> followed by the i2c adapter lock. However, when we operate the 
> auxiliary sensor the order of locking is the other way around.
> 
> In order to avoid this enable the bypass mux bit once in the beginning 
> and remove the select/deselect_bypass functions.
> 
> This patch moves the bypass enabling in a separate function that is 
> called once at probe and removes the functionality from 
> inv_mpu_select/deselect_bypass.
> 
> Another advantage of this approach is that power-wise the mpu chip 
> isn't powered up at each auxiliary sensor i2c transaction so if only 
> the compass is used this would be more power efficient.
There was one comment before that power off will cause the the bypass i2c lines to disable for some MPU chips which this driver can support.
So get some confirmation from Invensense folks that this is OK for all MPU chips.


Thanks,
Srinivas

> 
> Signed-off-by: Adriana Reus <adriana.reus@intel.com>
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
> ---
>  drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c | 88 ++++++---------------
> ----------
>  1 file changed, 15 insertions(+), 73 deletions(-)
> 
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> index 71bdaa3..8065355 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> @@ -25,82 +25,25 @@ static const struct regmap_config 
> inv_mpu_regmap_config = {
>  	.val_bits = 8,
>  };
>  
> -/*
> - * The i2c read/write needs to happen in unlocked mode. As the parent
> - * adapter is common. If we use locked versions, it will fail as
> - * the mux adapter will lock the parent i2c adapter, while calling
> - * select/deselect functions.
> - */
> -static int inv_mpu6050_write_reg_unlocked(struct i2c_client *client,
> -					  u8 reg, u8 d)
> +static int inv_mpu6050_bypass_en(struct iio_dev *indio_dev)
>  {
> -	int ret;
> -	u8 buf[2] = {reg, d};
> -	struct i2c_msg msg[1] = {
> -		{
> -			.addr = client->addr,
> -			.flags = 0,
> -			.len = sizeof(buf),
> -			.buf = buf,
> -		}
> -	};
> -
> -	ret = __i2c_transfer(client->adapter, msg, 1);
> -	if (ret != 1)
> -		return ret;
> -
> -	return 0;
> -}
> -
> -static int inv_mpu6050_select_bypass(struct i2c_adapter *adap, void 
> *mux_priv,
> -				     u32 chan_id)
> -{
> -	struct i2c_client *client = mux_priv;
> -	struct iio_dev *indio_dev = dev_get_drvdata(&client->dev);
>  	struct inv_mpu6050_state *st = iio_priv(indio_dev);
>  	int ret = 0;
>  
> -	/* Use the same mutex which was used everywhere to protect
> power-op */
> -	mutex_lock(&indio_dev->mlock);
> -	if (!st->powerup_count) {
> -		ret = inv_mpu6050_write_reg_unlocked(client,
> -						     st->reg-
> >pwr_mgmt_1, 0);
> -		if (ret)
> -			goto write_error;
> +	ret = inv_mpu6050_set_power_itg(st, true);
> +	if (ret)
> +		return ret;
>  
> -		msleep(INV_MPU6050_REG_UP_TIME);
> -	}
> -	if (!ret) {
> -		st->powerup_count++;
> -		ret = inv_mpu6050_write_reg_unlocked(client,
> -						     st->reg-
> >int_pin_cfg,
> -						     INV_MPU6050_INT
> _PIN_CFG |
> -						     INV_MPU6050_BIT
> _BYPASS_EN);
> +	ret = regmap_write(st->map,
> +			   st->reg->int_pin_cfg,
> +			   INV_MPU6050_INT_PIN_CFG |
> +			   INV_MPU6050_BIT_BYPASS_EN);
> +	if (ret) {
> +		inv_mpu6050_set_power_itg(st, false);
> +		return ret;
>  	}
> -write_error:
> -	mutex_unlock(&indio_dev->mlock);
>  
> -	return ret;
> -}
> -
> -static int inv_mpu6050_deselect_bypass(struct i2c_adapter *adap,
> -				       void *mux_priv, u32 chan_id)
> -{
> -	struct i2c_client *client = mux_priv;
> -	struct iio_dev *indio_dev = dev_get_drvdata(&client->dev);
> -	struct inv_mpu6050_state *st = iio_priv(indio_dev);
> -
> -	mutex_lock(&indio_dev->mlock);
> -	/* It doesn't really mattter, if any of the calls fails */
> -	inv_mpu6050_write_reg_unlocked(client, st->reg->int_pin_cfg,
> -				       INV_MPU6050_INT_PIN_CFG);
> -	st->powerup_count--;
> -	if (!st->powerup_count)
> -		inv_mpu6050_write_reg_unlocked(client, st->reg-
> >pwr_mgmt_1,
> -					       INV_MPU6050_BIT_SLEEP
> );
> -	mutex_unlock(&indio_dev->mlock);
> -
> -	return 0;
> +	return inv_mpu6050_set_power_itg(st, false);
>  }
>  
>  /**
> @@ -129,7 +72,8 @@ static int inv_mpu_probe(struct i2c_client *client,
>  		return PTR_ERR(regmap);
>  	}
>  
> -	result = inv_mpu_core_probe(regmap, client->irq, name,
> NULL);
> +	result = inv_mpu_core_probe(regmap, client->irq, name,
> +				    inv_mpu6050_bypass_en);
>  	if (result < 0)
>  		return result;
>  
> @@ -137,9 +81,7 @@ static int inv_mpu_probe(struct i2c_client *client,
>  	st->mux_adapter = i2c_add_mux_adapter(client->adapter,
>  					      &client->dev,
>  					      client,
> -					      0, 0, 0,
> -					      inv_mpu6050_select_byp
> ass,
> -					      inv_mpu6050_deselect_b
> ypass);
> +					      0, 0, 0, NULL, NULL);
>  	if (!st->mux_adapter) {
>  		result = -ENODEV;
>  		goto out_unreg_device;

RE: [RFC PATCH 9/9] iio: imu: inv_mpu6050: Fix deadlock between i2c adapter lock and mpu lock

[Ge Gao]

InRoZXJlIHdhcyBvbmUgY29tbWVudCBiZWZvcmUgdGhhdCBwb3dlciBvZmYgd2lsbCBjYXVzZSB0
aGUgdGhlIGJ5cGFzcyBpMmMgbGluZXMgdG8gZGlzYWJsZSBmb3Igc29tZSBNUFUgY2hpcHMgd2hp
Y2ggdGhpcyBkcml2ZXIgY2FuIHN1cHBvcnQuIg0KVGhpcyBpcyByaWdodC4gSSByZW1lbWJlciB0
aGVyZSB3YXMgc29tZSBjb2RlIGFib3V0IGl0IGVhcmxpZXIgdHJ5aW5nIHRvIHRha2UgYWR2YW50
YWdlIG9mIHRoZSBzZWNvbmRhcnkgYnVzIHdpdGhvdXQgZW5hYmxpbmcgdGhlIE1QVSBjaGlwIGl0
c2VsZi4gVGhpcyBpcyBpbXBvc3NpYmxlLiBUaGUgc2Vjb25kYXJ5IGJ1cyBpbnRlcmZhY2UgaXMg
cGFydCBvZiB0aGUgY2hpcCBhbmQgaXQgd2lsbCBub3QgZnVuY3Rpb24gd2l0aG91dCBlbmFibGlu
ZyB0aGUgY2hpcC4gV2l0aG91dCBlbmFibGluZyBneXJvL2FjY2VsIGVuZ2luZSwgZW5hYmxpbmcg
dGhlIGNoaXAgYWxvbmUgdGFrZXMgdmVyeSBsaXR0bGUgcG93ZXIuIEl0IGlzIG5vdCB3b3J0aCB0
aGUgdHJvdWJsZSB0byBkbyB0aGF0Lg0KDQpUaGFua3MsDQoNCkdlDQoNCg0KDQotLS0tLU9yaWdp
bmFsIE1lc3NhZ2UtLS0tLQ0KRnJvbTogU3Jpbml2YXMgUGFuZHJ1dmFkYSBbbWFpbHRvOnNyaW5p
dmFzLnBhbmRydXZhZGFAbGludXguaW50ZWwuY29tXSANClNlbnQ6IFRodXJzZGF5LCBGZWJydWFy
eSAxOCwgMjAxNiAxMDoxNyBBTQ0KVG86IERhbmllbCBCYWx1dGE7IGppYzIzQGtlcm5lbC5vcmcN
CkNjOiBrbmFhY2suaEBnbXguZGU7IGxhcnNAbWV0YWZvby5kZTsgcG1lZXJ3QHBtZWVydy5uZXQ7
IGxpbnV4LWlpb0B2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4LWtlcm5lbEB2Z2VyLmtlcm5lbC5vcmc7
IHdzYUB0aGUtZHJlYW1zLmRlOyBsaW51eC1pMmNAdmdlci5rZXJuZWwub3JnOyBsdWNhcy5kZW1h
cmNoaUBpbnRlbC5jb207IEdlIEdhbzsgYWRpLnJldXNAZ21haWwuY29tOyBjbW9AbWVsZXhpcy5j
b207IG13ZWxsaW5nQGllZWUub3JnDQpTdWJqZWN0OiBSZTogW1JGQyBQQVRDSCA5LzldIGlpbzog
aW11OiBpbnZfbXB1NjA1MDogRml4IGRlYWRsb2NrIGJldHdlZW4gaTJjIGFkYXB0ZXIgbG9jayBh
bmQgbXB1IGxvY2sNCg0KT24gVGh1LCAyMDE2LTAyLTE4IGF0IDE3OjUzICswMjAwLCBEYW5pZWwg
QmFsdXRhIHdyb3RlOg0KPiBGcm9tOiBBZHJpYW5hIFJldXMgPGFkcmlhbmEucmV1c0BpbnRlbC5j
b20+DQo+IA0KPiBUaGlzIGRlYWRsb2NrIG9jY3VycyBpZiB0aGUgYWNjZWwvZ3lybyBhbmQgdGhl
IHNlbnNvciBvbiB0aGUgYXV4aWxpYXJ5IA0KPiBJMkMgKGluIG15IHNldHVwIGl0J3MgYW4gYWs4
OTc1KSBhcmUgd29ya2luZyBhdCB0aGUgc2FtZSB0aW1lLg0KPiANCj4gU2NlbmFyaW86DQo+IA0K
PiDCoMKgwqDCoMKgwqBUMQkJCQkJVDINCj4gwqDCoMKgwqDCoD09PT0JCQkJwqDCoMKgwqDCoMKg
wqA9PT09DQo+IGludl9tcHU2MDUwX3JlYWRfZmlmb8KgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg
wqDCoMKgwqDCoGF1eCBzZW5zb3Igb3AgKGVnLg0KPiBhazg5NzVfcmVhZF9yYXcpDQo+IMKgwqDC
oMKgwqDCoMKgwqB8wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDC
oMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqB8DQo+IG11dGV4X2xvY2soJmluZGlvX2Rldi0+
bWxvY2spwqDCoMKgwqDCoMKgwqDCoMKgwqDCoGkyY190cmFuc2Zlcg0KPiDCoMKgwqDCoMKgwqDC
oMKgfMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDC
oMKgwqDCoMKgwqDCoMKgwqDCoMKgfCBpMmMgdHJhbnNhY3Rpb27CoMKgwqDCoMKgwqDCoMKgwqDC
oMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoA0KPiBpMmMgYWRhcHRlciBsb2NrDQo+IMKg
wqDCoMKgwqDCoMKgwqB8wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg
wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqB8IGkyYyBhZGFwdGVyIGxvY2vCoMKgwqDC
oMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqANCj4gaTJjX211eF9tYXN0
ZXJfeGZlcg0KPiDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg
wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoHwNCj4gwqDCoMKg
wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDC
oMKgwqDCoMKgwqDCoMKgwqBpbnZfbXB1NjA1MF9zZWxlY3RfYnlwYXNzDQo+IMKgwqDCoMKgwqDC
oMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg
wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgfA0KPiDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg
wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoG11dGV4
X2xvY2soJmluZGlvX2Rldi0+bWxvY2spDQo+IA0KPiBXaGVuIHdlIG9wZXJhdGUgb24gYW4gbXB1
IHNlbnNvciB0aGUgb3JkZXIgb2YgbG9ja2luZyBpcyBtcHUgbG9jayANCj4gZm9sbG93ZWQgYnkg
dGhlIGkyYyBhZGFwdGVyIGxvY2suIEhvd2V2ZXIsIHdoZW4gd2Ugb3BlcmF0ZSB0aGUgDQo+IGF1
eGlsaWFyeSBzZW5zb3IgdGhlIG9yZGVyIG9mIGxvY2tpbmcgaXMgdGhlIG90aGVyIHdheSBhcm91
bmQuDQo+IA0KPiBJbiBvcmRlciB0byBhdm9pZCB0aGlzIGVuYWJsZSB0aGUgYnlwYXNzIG11eCBi
aXQgb25jZSBpbiB0aGUgYmVnaW5uaW5nIA0KPiBhbmQgcmVtb3ZlIHRoZSBzZWxlY3QvZGVzZWxl
Y3RfYnlwYXNzIGZ1bmN0aW9ucy4NCj4gDQo+IFRoaXMgcGF0Y2ggbW92ZXMgdGhlIGJ5cGFzcyBl
bmFibGluZyBpbiBhIHNlcGFyYXRlIGZ1bmN0aW9uIHRoYXQgaXMgDQo+IGNhbGxlZCBvbmNlIGF0
IHByb2JlIGFuZCByZW1vdmVzIHRoZSBmdW5jdGlvbmFsaXR5IGZyb20gDQo+IGludl9tcHVfc2Vs
ZWN0L2Rlc2VsZWN0X2J5cGFzcy4NCj4gDQo+IEFub3RoZXIgYWR2YW50YWdlIG9mIHRoaXMgYXBw
cm9hY2ggaXMgdGhhdCBwb3dlci13aXNlIHRoZSBtcHUgY2hpcCANCj4gaXNuJ3QgcG93ZXJlZCB1
cCBhdCBlYWNoIGF1eGlsaWFyeSBzZW5zb3IgaTJjIHRyYW5zYWN0aW9uIHNvIGlmIG9ubHkgDQo+
IHRoZSBjb21wYXNzIGlzIHVzZWQgdGhpcyB3b3VsZCBiZSBtb3JlIHBvd2VyIGVmZmljaWVudC4N
ClRoZXJlIHdhcyBvbmUgY29tbWVudCBiZWZvcmUgdGhhdCBwb3dlciBvZmYgd2lsbCBjYXVzZSB0
aGUgdGhlIGJ5cGFzcyBpMmMgbGluZXMgdG8gZGlzYWJsZSBmb3Igc29tZSBNUFUgY2hpcHMgd2hp
Y2ggdGhpcyBkcml2ZXIgY2FuIHN1cHBvcnQuDQpTbyBnZXQgc29tZSBjb25maXJtYXRpb24gZnJv
bSBJbnZlbnNlbnNlIGZvbGtzIHRoYXQgdGhpcyBpcyBPSyBmb3IgYWxsIE1QVSBjaGlwcy4NCg0K
DQpUaGFua3MsDQpTcmluaXZhcw0KDQo+IA0KPiBTaWduZWQtb2ZmLWJ5OiBBZHJpYW5hIFJldXMg
PGFkcmlhbmEucmV1c0BpbnRlbC5jb20+DQo+IFNpZ25lZC1vZmYtYnk6IERhbmllbCBCYWx1dGEg
PGRhbmllbC5iYWx1dGFAaW50ZWwuY29tPg0KPiAtLS0NCj4gwqBkcml2ZXJzL2lpby9pbXUvaW52
X21wdTYwNTAvaW52X21wdV9pMmMuYyB8IDg4ICsrKysrKy0tLS0tLS0tLS0tLS0tLQ0KPiAtLS0t
LS0tLS0tDQo+IMKgMSBmaWxlIGNoYW5nZWQsIDE1IGluc2VydGlvbnMoKyksIDczIGRlbGV0aW9u
cygtKQ0KPiANCj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvaWlvL2ltdS9pbnZfbXB1NjA1MC9pbnZf
bXB1X2kyYy5jDQo+IGIvZHJpdmVycy9paW8vaW11L2ludl9tcHU2MDUwL2ludl9tcHVfaTJjLmMN
Cj4gaW5kZXggNzFiZGFhMy4uODA2NTM1NSAxMDA2NDQNCj4gLS0tIGEvZHJpdmVycy9paW8vaW11
L2ludl9tcHU2MDUwL2ludl9tcHVfaTJjLmMNCj4gKysrIGIvZHJpdmVycy9paW8vaW11L2ludl9t
cHU2MDUwL2ludl9tcHVfaTJjLmMNCj4gQEAgLTI1LDgyICsyNSwyNSBAQCBzdGF0aWMgY29uc3Qg
c3RydWN0IHJlZ21hcF9jb25maWcgDQo+IGludl9tcHVfcmVnbWFwX2NvbmZpZyA9IHsNCj4gwqAJ
LnZhbF9iaXRzID0gOCwNCj4gwqB9Ow0KPiDCoA0KPiAtLyoNCj4gLSAqIFRoZSBpMmMgcmVhZC93
cml0ZSBuZWVkcyB0byBoYXBwZW4gaW4gdW5sb2NrZWQgbW9kZS4gQXMgdGhlIHBhcmVudA0KPiAt
ICogYWRhcHRlciBpcyBjb21tb24uIElmIHdlIHVzZSBsb2NrZWQgdmVyc2lvbnMsIGl0IHdpbGwg
ZmFpbCBhcw0KPiAtICogdGhlIG11eCBhZGFwdGVyIHdpbGwgbG9jayB0aGUgcGFyZW50IGkyYyBh
ZGFwdGVyLCB3aGlsZSBjYWxsaW5nDQo+IC0gKiBzZWxlY3QvZGVzZWxlY3QgZnVuY3Rpb25zLg0K
PiAtICovDQo+IC1zdGF0aWMgaW50IGludl9tcHU2MDUwX3dyaXRlX3JlZ191bmxvY2tlZChzdHJ1
Y3QgaTJjX2NsaWVudCAqY2xpZW50LA0KPiAtCQkJCQnCoMKgdTggcmVnLCB1OCBkKQ0KPiArc3Rh
dGljIGludCBpbnZfbXB1NjA1MF9ieXBhc3NfZW4oc3RydWN0IGlpb19kZXYgKmluZGlvX2RldikN
Cj4gwqB7DQo+IC0JaW50IHJldDsNCj4gLQl1OCBidWZbMl0gPSB7cmVnLCBkfTsNCj4gLQlzdHJ1
Y3QgaTJjX21zZyBtc2dbMV0gPSB7DQo+IC0JCXsNCj4gLQkJCS5hZGRyID0gY2xpZW50LT5hZGRy
LA0KPiAtCQkJLmZsYWdzID0gMCwNCj4gLQkJCS5sZW4gPSBzaXplb2YoYnVmKSwNCj4gLQkJCS5i
dWYgPSBidWYsDQo+IC0JCX0NCj4gLQl9Ow0KPiAtDQo+IC0JcmV0ID0gX19pMmNfdHJhbnNmZXIo
Y2xpZW50LT5hZGFwdGVyLCBtc2csIDEpOw0KPiAtCWlmIChyZXQgIT0gMSkNCj4gLQkJcmV0dXJu
IHJldDsNCj4gLQ0KPiAtCXJldHVybiAwOw0KPiAtfQ0KPiAtDQo+IC1zdGF0aWMgaW50IGludl9t
cHU2MDUwX3NlbGVjdF9ieXBhc3Moc3RydWN0IGkyY19hZGFwdGVyICphZGFwLCB2b2lkIA0KPiAq
bXV4X3ByaXYsDQo+IC0JCQkJwqDCoMKgwqDCoHUzMiBjaGFuX2lkKQ0KPiAtew0KPiAtCXN0cnVj
dCBpMmNfY2xpZW50ICpjbGllbnQgPSBtdXhfcHJpdjsNCj4gLQlzdHJ1Y3QgaWlvX2RldiAqaW5k
aW9fZGV2ID0gZGV2X2dldF9kcnZkYXRhKCZjbGllbnQtPmRldik7DQo+IMKgCXN0cnVjdCBpbnZf
bXB1NjA1MF9zdGF0ZSAqc3QgPSBpaW9fcHJpdihpbmRpb19kZXYpOw0KPiDCoAlpbnQgcmV0ID0g
MDsNCj4gwqANCj4gLQkvKiBVc2UgdGhlIHNhbWUgbXV0ZXggd2hpY2ggd2FzIHVzZWQgZXZlcnl3
aGVyZSB0byBwcm90ZWN0DQo+IHBvd2VyLW9wICovDQo+IC0JbXV0ZXhfbG9jaygmaW5kaW9fZGV2
LT5tbG9jayk7DQo+IC0JaWYgKCFzdC0+cG93ZXJ1cF9jb3VudCkgew0KPiAtCQlyZXQgPSBpbnZf
bXB1NjA1MF93cml0ZV9yZWdfdW5sb2NrZWQoY2xpZW50LA0KPiAtCQkJCQkJwqDCoMKgwqDCoHN0
LT5yZWctDQo+ID5wd3JfbWdtdF8xLCAwKTsNCj4gLQkJaWYgKHJldCkNCj4gLQkJCWdvdG8gd3Jp
dGVfZXJyb3I7DQo+ICsJcmV0ID0gaW52X21wdTYwNTBfc2V0X3Bvd2VyX2l0ZyhzdCwgdHJ1ZSk7
DQo+ICsJaWYgKHJldCkNCj4gKwkJcmV0dXJuIHJldDsNCj4gwqANCj4gLQkJbXNsZWVwKElOVl9N
UFU2MDUwX1JFR19VUF9USU1FKTsNCj4gLQl9DQo+IC0JaWYgKCFyZXQpIHsNCj4gLQkJc3QtPnBv
d2VydXBfY291bnQrKzsNCj4gLQkJcmV0ID0gaW52X21wdTYwNTBfd3JpdGVfcmVnX3VubG9ja2Vk
KGNsaWVudCwNCj4gLQkJCQkJCcKgwqDCoMKgwqBzdC0+cmVnLQ0KPiA+aW50X3Bpbl9jZmcsDQo+
IC0JCQkJCQnCoMKgwqDCoMKgSU5WX01QVTYwNTBfSU5UDQo+IF9QSU5fQ0ZHIHwNCj4gLQkJCQkJ
CcKgwqDCoMKgwqBJTlZfTVBVNjA1MF9CSVQNCj4gX0JZUEFTU19FTik7DQo+ICsJcmV0ID0gcmVn
bWFwX3dyaXRlKHN0LT5tYXAsDQo+ICsJCQnCoMKgwqBzdC0+cmVnLT5pbnRfcGluX2NmZywNCj4g
KwkJCcKgwqDCoElOVl9NUFU2MDUwX0lOVF9QSU5fQ0ZHIHwNCj4gKwkJCcKgwqDCoElOVl9NUFU2
MDUwX0JJVF9CWVBBU1NfRU4pOw0KPiArCWlmIChyZXQpIHsNCj4gKwkJaW52X21wdTYwNTBfc2V0
X3Bvd2VyX2l0ZyhzdCwgZmFsc2UpOw0KPiArCQlyZXR1cm4gcmV0Ow0KPiDCoAl9DQo+IC13cml0
ZV9lcnJvcjoNCj4gLQltdXRleF91bmxvY2soJmluZGlvX2Rldi0+bWxvY2spOw0KPiDCoA0KPiAt
CXJldHVybiByZXQ7DQo+IC19DQo+IC0NCj4gLXN0YXRpYyBpbnQgaW52X21wdTYwNTBfZGVzZWxl
Y3RfYnlwYXNzKHN0cnVjdCBpMmNfYWRhcHRlciAqYWRhcCwNCj4gLQkJCQnCoMKgwqDCoMKgwqDC
oHZvaWQgKm11eF9wcml2LCB1MzIgY2hhbl9pZCkNCj4gLXsNCj4gLQlzdHJ1Y3QgaTJjX2NsaWVu
dCAqY2xpZW50ID0gbXV4X3ByaXY7DQo+IC0Jc3RydWN0IGlpb19kZXYgKmluZGlvX2RldiA9IGRl
dl9nZXRfZHJ2ZGF0YSgmY2xpZW50LT5kZXYpOw0KPiAtCXN0cnVjdCBpbnZfbXB1NjA1MF9zdGF0
ZSAqc3QgPSBpaW9fcHJpdihpbmRpb19kZXYpOw0KPiAtDQo+IC0JbXV0ZXhfbG9jaygmaW5kaW9f
ZGV2LT5tbG9jayk7DQo+IC0JLyogSXQgZG9lc24ndCByZWFsbHkgbWF0dHRlciwgaWYgYW55IG9m
IHRoZSBjYWxscyBmYWlscyAqLw0KPiAtCWludl9tcHU2MDUwX3dyaXRlX3JlZ191bmxvY2tlZChj
bGllbnQsIHN0LT5yZWctPmludF9waW5fY2ZnLA0KPiAtCQkJCcKgwqDCoMKgwqDCoMKgSU5WX01Q
VTYwNTBfSU5UX1BJTl9DRkcpOw0KPiAtCXN0LT5wb3dlcnVwX2NvdW50LS07DQo+IC0JaWYgKCFz
dC0+cG93ZXJ1cF9jb3VudCkNCj4gLQkJaW52X21wdTYwNTBfd3JpdGVfcmVnX3VubG9ja2VkKGNs
aWVudCwgc3QtPnJlZy0NCj4gPnB3cl9tZ210XzEsDQo+IC0JCQkJCcKgwqDCoMKgwqDCoMKgSU5W
X01QVTYwNTBfQklUX1NMRUVQDQo+ICk7DQo+IC0JbXV0ZXhfdW5sb2NrKCZpbmRpb19kZXYtPm1s
b2NrKTsNCj4gLQ0KPiAtCXJldHVybiAwOw0KPiArCXJldHVybiBpbnZfbXB1NjA1MF9zZXRfcG93
ZXJfaXRnKHN0LCBmYWxzZSk7DQo+IMKgfQ0KPiDCoA0KPiDCoC8qKg0KPiBAQCAtMTI5LDcgKzcy
LDggQEAgc3RhdGljIGludCBpbnZfbXB1X3Byb2JlKHN0cnVjdCBpMmNfY2xpZW50ICpjbGllbnQs
DQo+IMKgCQlyZXR1cm4gUFRSX0VSUihyZWdtYXApOw0KPiDCoAl9DQo+IMKgDQo+IC0JcmVzdWx0
ID0gaW52X21wdV9jb3JlX3Byb2JlKHJlZ21hcCwgY2xpZW50LT5pcnEsIG5hbWUsDQo+IE5VTEwp
Ow0KPiArCXJlc3VsdCA9IGludl9tcHVfY29yZV9wcm9iZShyZWdtYXAsIGNsaWVudC0+aXJxLCBu
YW1lLA0KPiArCQkJCcKgwqDCoMKgaW52X21wdTYwNTBfYnlwYXNzX2VuKTsNCj4gwqAJaWYgKHJl
c3VsdCA8IDApDQo+IMKgCQlyZXR1cm4gcmVzdWx0Ow0KPiDCoA0KPiBAQCAtMTM3LDkgKzgxLDcg
QEAgc3RhdGljIGludCBpbnZfbXB1X3Byb2JlKHN0cnVjdCBpMmNfY2xpZW50ICpjbGllbnQsDQo+
IMKgCXN0LT5tdXhfYWRhcHRlciA9IGkyY19hZGRfbXV4X2FkYXB0ZXIoY2xpZW50LT5hZGFwdGVy
LA0KPiDCoAkJCQkJwqDCoMKgwqDCoMKgJmNsaWVudC0+ZGV2LA0KPiDCoAkJCQkJwqDCoMKgwqDC
oMKgY2xpZW50LA0KPiAtCQkJCQnCoMKgwqDCoMKgwqAwLCAwLCAwLA0KPiAtCQkJCQnCoMKgwqDC
oMKgwqBpbnZfbXB1NjA1MF9zZWxlY3RfYnlwDQo+IGFzcywNCj4gLQkJCQkJwqDCoMKgwqDCoMKg
aW52X21wdTYwNTBfZGVzZWxlY3RfYg0KPiB5cGFzcyk7DQo+ICsJCQkJCcKgwqDCoMKgwqDCoDAs
IDAsIDAsIE5VTEwsIE5VTEwpOw0KPiDCoAlpZiAoIXN0LT5tdXhfYWRhcHRlcikgew0KPiDCoAkJ
cmVzdWx0ID0gLUVOT0RFVjsNCj4gwqAJCWdvdG8gb3V0X3VucmVnX2RldmljZTsNCg==

Re: [RFC PATCH 1/9] iio: imu: inv_mpu6050: Fix multiline comments style

[Jonathan Cameron]

On 18/02/16 15:53, Daniel Baluta wrote:
> The preffered style for long (multi-line) comments is:
> 
> /*
>  * this is a multiline
>  * comment
>  */
> 
> This also fixes checkpatch.pl warning:
> WARNING: Block comments use * on subsequent lines
> 
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
Applied to the togreg branch of iio.git - pushed out as testing as normal.

Jonathan
> ---
>  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 37 +++++++++++++++++++-----------
>  1 file changed, 23 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index 2258600..84e014c 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -79,10 +79,11 @@ int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask)
>  {
>  	unsigned int d, mgmt_1;
>  	int result;
> -
> -	/* switch clock needs to be careful. Only when gyro is on, can
> -	   clock source be switched to gyro. Otherwise, it must be set to
> -	   internal clock */
> +	/*
> +	 * switch clock needs to be careful. Only when gyro is on, can
> +	 * clock source be switched to gyro. Otherwise, it must be set to
> +	 * internal clock
> +	 */
>  	if (INV_MPU6050_BIT_PWR_GYRO_STBY == mask) {
>  		result = regmap_read(st->map, st->reg->pwr_mgmt_1, &mgmt_1);
>  		if (result)
> @@ -92,8 +93,10 @@ int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask)
>  	}
>  
>  	if ((INV_MPU6050_BIT_PWR_GYRO_STBY == mask) && (!en)) {
> -		/* turning off gyro requires switch to internal clock first.
> -		   Then turn off gyro engine */
> +		/*
> +		 * turning off gyro requires switch to internal clock first.
> +		 * Then turn off gyro engine
> +		 */
>  		mgmt_1 |= INV_CLK_INTERNAL;
>  		result = regmap_write(st->map, st->reg->pwr_mgmt_1, mgmt_1);
>  		if (result)
> @@ -391,8 +394,10 @@ static int inv_mpu6050_write_raw(struct iio_dev *indio_dev,
>  	int result;
>  
>  	mutex_lock(&indio_dev->mlock);
> -	/* we should only update scale when the chip is disabled, i.e.,
> -		not running */
> +	/*
> +	 * we should only update scale when the chip is disabled, i.e.
> +	 * not running
> +	 */
>  	if (st->chip_config.enable) {
>  		result = -EBUSY;
>  		goto error_write_raw;
> @@ -529,8 +534,10 @@ static ssize_t inv_attr_show(struct device *dev,
>  	s8 *m;
>  
>  	switch (this_attr->address) {
> -	/* In MPU6050, the two matrix are the same because gyro and accel
> -	   are integrated in one chip */
> +	/*
> +	 * In MPU6050, the two matrix are the same because gyro and accel
> +	 * are integrated in one chip
> +	 */
>  	case ATTR_GYRO_MATRIX:
>  	case ATTR_ACCL_MATRIX:
>  		m = st->plat_data.orientation;
> @@ -654,10 +661,12 @@ static int inv_check_and_setup_chip(struct inv_mpu6050_state *st)
>  	if (result)
>  		return result;
>  	msleep(INV_MPU6050_POWER_UP_TIME);
> -	/* toggle power state. After reset, the sleep bit could be on
> -		or off depending on the OTP settings. Toggling power would
> -		make it in a definite state as well as making the hardware
> -		state align with the software state */
> +	/*
> +	 * toggle power state. After reset, the sleep bit could be on
> +	 * or off depending on the OTP settings. Toggling power would
> +	 * make it in a definite state as well as making the hardware
> +	 * state align with the software state
> +	 */
>  	result = inv_mpu6050_set_power_itg(st, false);
>  	if (result)
>  		return result;
> 

Re: [RFC PATCH 2/9] iio: imu: inv_mpu6050: Fix Yoda conditions

[Jonathan Cameron]

On 19/02/16 09:09, Crt Mori wrote:
> This is quite trivial.
> 
> Acked-by: Crt Mori <cmo@melexis.com>
Applied.
> 
> On 18 February 2016 at 16:53, Daniel Baluta <daniel.baluta@intel.com> wrote:
>> This fixes the following checkpatch warning:
>>         * WARNING: Comparisons should place the constant
>>           on the right side of the test
>>
>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
>> ---
>>  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 6 +++---
>>  drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c | 2 +-
>>  2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>> index 84e014c..c550ebb 100644
>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>> @@ -84,7 +84,7 @@ int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask)
>>          * clock source be switched to gyro. Otherwise, it must be set to
>>          * internal clock
>>          */
>> -       if (INV_MPU6050_BIT_PWR_GYRO_STBY == mask) {
>> +       if (mask == INV_MPU6050_BIT_PWR_GYRO_STBY) {
>>                 result = regmap_read(st->map, st->reg->pwr_mgmt_1, &mgmt_1);
>>                 if (result)
>>                         return result;
>> @@ -92,7 +92,7 @@ int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask)
>>                 mgmt_1 &= ~INV_MPU6050_BIT_CLK_MASK;
>>         }
>>
>> -       if ((INV_MPU6050_BIT_PWR_GYRO_STBY == mask) && (!en)) {
>> +       if ((mask == INV_MPU6050_BIT_PWR_GYRO_STBY) && (!en)) {
>>                 /*
>>                  * turning off gyro requires switch to internal clock first.
>>                  * Then turn off gyro engine
>> @@ -117,7 +117,7 @@ int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask)
>>         if (en) {
>>                 /* Wait for output stabilize */
>>                 msleep(INV_MPU6050_TEMP_UP_TIME);
>> -               if (INV_MPU6050_BIT_PWR_GYRO_STBY == mask) {
>> +               if (mask == INV_MPU6050_BIT_PWR_GYRO_STBY) {
>>                         /* switch internal clock to PLL */
>>                         mgmt_1 |= INV_CLK_PLL;
>>                         result = regmap_write(st->map,
>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
>> index 1fc5fd9..441080b 100644
>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
>> @@ -168,7 +168,7 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
>>
>>                 result = kfifo_out(&st->timestamps, &timestamp, 1);
>>                 /* when there is no timestamp, put timestamp as 0 */
>> -               if (0 == result)
>> +               if (result == 0)
>>                         timestamp = 0;
>>
>>                 result = iio_push_to_buffers_with_timestamp(indio_dev, data,
>> --
>> 2.5.0
>>

Re: [RFC PATCH 2/9] iio: imu: inv_mpu6050: Fix Yoda conditions

[Jonathan Cameron]

On 19/02/16 09:09, Crt Mori wrote:
> This is quite trivial.
> 
> Acked-by: Crt Mori <cmo-fc6wVz46lShBDgjK7y7TUQ@public.gmane.org>
Applied.
> 
> On 18 February 2016 at 16:53, Daniel Baluta <daniel.baluta-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
>> This fixes the following checkpatch warning:
>>         * WARNING: Comparisons should place the constant
>>           on the right side of the test
>>
>> Signed-off-by: Daniel Baluta <daniel.baluta-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> ---
>>  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 6 +++---
>>  drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c | 2 +-
>>  2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>> index 84e014c..c550ebb 100644
>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>> @@ -84,7 +84,7 @@ int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask)
>>          * clock source be switched to gyro. Otherwise, it must be set to
>>          * internal clock
>>          */
>> -       if (INV_MPU6050_BIT_PWR_GYRO_STBY == mask) {
>> +       if (mask == INV_MPU6050_BIT_PWR_GYRO_STBY) {
>>                 result = regmap_read(st->map, st->reg->pwr_mgmt_1, &mgmt_1);
>>                 if (result)
>>                         return result;
>> @@ -92,7 +92,7 @@ int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask)
>>                 mgmt_1 &= ~INV_MPU6050_BIT_CLK_MASK;
>>         }
>>
>> -       if ((INV_MPU6050_BIT_PWR_GYRO_STBY == mask) && (!en)) {
>> +       if ((mask == INV_MPU6050_BIT_PWR_GYRO_STBY) && (!en)) {
>>                 /*
>>                  * turning off gyro requires switch to internal clock first.
>>                  * Then turn off gyro engine
>> @@ -117,7 +117,7 @@ int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask)
>>         if (en) {
>>                 /* Wait for output stabilize */
>>                 msleep(INV_MPU6050_TEMP_UP_TIME);
>> -               if (INV_MPU6050_BIT_PWR_GYRO_STBY == mask) {
>> +               if (mask == INV_MPU6050_BIT_PWR_GYRO_STBY) {
>>                         /* switch internal clock to PLL */
>>                         mgmt_1 |= INV_CLK_PLL;
>>                         result = regmap_write(st->map,
>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
>> index 1fc5fd9..441080b 100644
>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
>> @@ -168,7 +168,7 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
>>
>>                 result = kfifo_out(&st->timestamps, &timestamp, 1);
>>                 /* when there is no timestamp, put timestamp as 0 */
>> -               if (0 == result)
>> +               if (result == 0)
>>                         timestamp = 0;
>>
>>                 result = iio_push_to_buffers_with_timestamp(indio_dev, data,
>> --
>> 2.5.0
>>

Re: [RFC PATCH 3/9] iio: imu: inv_mpu6050: Fix newlines to make code easier to read

[Jonathan Cameron]

On 18/02/16 15:53, Daniel Baluta wrote:
> This fixes the following checkpatch.pl warnings:
> 	* WARNING: Missing a blank line after declarations
> 	* CHECK: Blank lines aren't necessary before a close brace '}'
> 
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
Applied.
> ---
>  drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c | 2 +-
>  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
> index 0bcfa8d..231a7af 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
> @@ -186,7 +186,6 @@ int inv_mpu_acpi_create_mux_client(struct i2c_client *client)
>  		st->mux_client = i2c_new_device(st->mux_adapter, &info);
>  		if (!st->mux_client)
>  			return -ENODEV;
> -
>  	}
>  
>  	return 0;
> @@ -195,6 +194,7 @@ int inv_mpu_acpi_create_mux_client(struct i2c_client *client)
>  void inv_mpu_acpi_delete_mux_client(struct i2c_client *client)
>  {
>  	struct inv_mpu6050_state *st = iio_priv(dev_get_drvdata(&client->dev));
> +
>  	if (st->mux_client)
>  		i2c_unregister_device(st->mux_client);
>  }
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index c550ebb..72cc478 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -365,6 +365,7 @@ static int inv_write_raw_get_fmt(struct iio_dev *indio_dev,
>  
>  	return -EINVAL;
>  }
> +
>  static int inv_mpu6050_write_accel_scale(struct inv_mpu6050_state *st, int val)
>  {
>  	int result, i;
> 

Re: [RFC PATCH 4/9] iio: imu: inv_mpu6050: Remove unnecessary parentheses

[Jonathan Cameron]

On 18/02/16 15:53, Daniel Baluta wrote:
> Fixes the following checkpatch warning:
> CHECK: Unnecessary parentheses around cpm->package.elements[i]
> 
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
Applied (though I find it hard to care about this one!)

Jonathan
> ---
>  drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
> index 231a7af..2771106 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_acpi.c
> @@ -66,11 +66,11 @@ static int asus_acpi_get_sensor_info(struct acpi_device *adev,
>  		union acpi_object *elem;
>  		int j;
>  
> -		elem = &(cpm->package.elements[i]);
> +		elem = &cpm->package.elements[i];
>  		for (j = 0; j < elem->package.count; ++j) {
>  			union acpi_object *sub_elem;
>  
> -			sub_elem = &(elem->package.elements[j]);
> +			sub_elem = &elem->package.elements[j];
>  			if (sub_elem->type == ACPI_TYPE_STRING)
>  				strlcpy(info->type, sub_elem->string.pointer,
>  					sizeof(info->type));
> 

Re: [RFC PATCH 6/9] iio: imu: inv_mpu6050: Fix code indent for if statement

[Jonathan Cameron]

On 18/02/16 15:53, Daniel Baluta wrote:
> This fixes the following checkpatch.pl warning:
> 
> WARNING: suspect code indent for conditional statements (8, 24)
> +       if (kfifo_len(&st->timestamps) >
> [...]
> +                       goto flush_fifo;
> 
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
Applied.
> ---
>  drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> index 441080b..0bc5091 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> @@ -158,8 +158,8 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
>  		goto flush_fifo;
>  	/* Timestamp mismatch. */
>  	if (kfifo_len(&st->timestamps) >
> -		fifo_count / bytes_per_datum + INV_MPU6050_TIME_STAMP_TOR)
> -			goto flush_fifo;
> +	    fifo_count / bytes_per_datum + INV_MPU6050_TIME_STAMP_TOR)
> +		goto flush_fifo;
>  	while (fifo_count >= bytes_per_datum) {
>  		result = regmap_bulk_read(st->map, st->reg->fifo_r_w,
>  					  data, bytes_per_datum);
> 

Re: [RFC PATCH 7/9] iio: imu: inv_mpu6050: Fix alignment with open parenthesis

[Jonathan Cameron]

On 18/02/16 15:53, Daniel Baluta wrote:
> This makes code consistent around inv_mpu6050 driver and
> fixes the following checkpatch.pl warning:
> CHECK: Alignment should match open parenthesis
> 
> Note that there were few cases were it was not possible to
> fix this due to making the line too long, but we can live with that.
> 
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
Applied
> ---
>  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c    | 55 +++++++++++++--------------
>  drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c     |  2 +-
>  drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c    | 17 ++++-----
>  drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c | 22 +++++------
>  4 files changed, 47 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index 48ab575..1643cd2 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -121,7 +121,7 @@ int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask)
>  			/* switch internal clock to PLL */
>  			mgmt_1 |= INV_CLK_PLL;
>  			result = regmap_write(st->map,
> -					st->reg->pwr_mgmt_1, mgmt_1);
> +					      st->reg->pwr_mgmt_1, mgmt_1);
>  			if (result)
>  				return result;
>  		}
> @@ -196,14 +196,14 @@ static int inv_mpu6050_init_config(struct iio_dev *indio_dev)
>  		return result;
>  
>  	memcpy(&st->chip_config, hw_info[st->chip_type].config,
> -		sizeof(struct inv_mpu6050_chip_config));
> +	       sizeof(struct inv_mpu6050_chip_config));
>  	result = inv_mpu6050_set_power_itg(st, false);
>  
>  	return result;
>  }
>  
>  static int inv_mpu6050_sensor_show(struct inv_mpu6050_state  *st, int reg,
> -				int axis, int *val)
> +				   int axis, int *val)
>  {
>  	int ind, result;
>  	__be16 d;
> @@ -217,11 +217,10 @@ static int inv_mpu6050_sensor_show(struct inv_mpu6050_state  *st, int reg,
>  	return IIO_VAL_INT;
>  }
>  
> -static int inv_mpu6050_read_raw(struct iio_dev *indio_dev,
> -			      struct iio_chan_spec const *chan,
> -			      int *val,
> -			      int *val2,
> -			      long mask) {
> +static int
> +inv_mpu6050_read_raw(struct iio_dev *indio_dev,
> +		     struct iio_chan_spec const *chan,
> +		     int *val, int *val2, long mask) {
>  	struct inv_mpu6050_state  *st = iio_priv(indio_dev);
>  
>  	switch (mask) {
> @@ -241,16 +240,16 @@ static int inv_mpu6050_read_raw(struct iio_dev *indio_dev,
>  		switch (chan->type) {
>  		case IIO_ANGL_VEL:
>  			if (!st->chip_config.gyro_fifo_enable ||
> -					!st->chip_config.enable) {
> +			    !st->chip_config.enable) {
>  				result = inv_mpu6050_switch_engine(st, true,
>  						INV_MPU6050_BIT_PWR_GYRO_STBY);
>  				if (result)
>  					goto error_read_raw;
>  			}
>  			ret =  inv_mpu6050_sensor_show(st, st->reg->raw_gyro,
> -						chan->channel2, val);
> +						       chan->channel2, val);
>  			if (!st->chip_config.gyro_fifo_enable ||
> -					!st->chip_config.enable) {
> +			    !st->chip_config.enable) {
>  				result = inv_mpu6050_switch_engine(st, false,
>  						INV_MPU6050_BIT_PWR_GYRO_STBY);
>  				if (result)
> @@ -259,16 +258,16 @@ static int inv_mpu6050_read_raw(struct iio_dev *indio_dev,
>  			break;
>  		case IIO_ACCEL:
>  			if (!st->chip_config.accl_fifo_enable ||
> -					!st->chip_config.enable) {
> +			    !st->chip_config.enable) {
>  				result = inv_mpu6050_switch_engine(st, true,
>  						INV_MPU6050_BIT_PWR_ACCL_STBY);
>  				if (result)
>  					goto error_read_raw;
>  			}
>  			ret = inv_mpu6050_sensor_show(st, st->reg->raw_accl,
> -						chan->channel2, val);
> +						      chan->channel2, val);
>  			if (!st->chip_config.accl_fifo_enable ||
> -					!st->chip_config.enable) {
> +			    !st->chip_config.enable) {
>  				result = inv_mpu6050_switch_engine(st, false,
>  						INV_MPU6050_BIT_PWR_ACCL_STBY);
>  				if (result)
> @@ -279,7 +278,7 @@ static int inv_mpu6050_read_raw(struct iio_dev *indio_dev,
>  			/* wait for stablization */
>  			msleep(INV_MPU6050_SENSOR_UP_TIME);
>  			inv_mpu6050_sensor_show(st, st->reg->temperature,
> -							IIO_MOD_X, val);
> +						IIO_MOD_X, val);
>  			break;
>  		default:
>  			ret = -EINVAL;
> @@ -387,10 +386,8 @@ static int inv_mpu6050_write_accel_scale(struct inv_mpu6050_state *st, int val)
>  }
>  
>  static int inv_mpu6050_write_raw(struct iio_dev *indio_dev,
> -			       struct iio_chan_spec const *chan,
> -			       int val,
> -			       int val2,
> -			       long mask) {
> +				 struct iio_chan_spec const *chan,
> +				 int val, int val2, long mask) {
>  	struct inv_mpu6050_state  *st = iio_priv(indio_dev);
>  	int result;
>  
> @@ -467,8 +464,9 @@ static int inv_mpu6050_set_lpf(struct inv_mpu6050_state *st, int rate)
>  /**
>   * inv_mpu6050_fifo_rate_store() - Set fifo rate.
>   */
> -static ssize_t inv_mpu6050_fifo_rate_store(struct device *dev,
> -	struct device_attribute *attr, const char *buf, size_t count)
> +static ssize_t
> +inv_mpu6050_fifo_rate_store(struct device *dev, struct device_attribute *attr,
> +			    const char *buf, size_t count)
>  {
>  	s32 fifo_rate;
>  	u8 d;
> @@ -479,7 +477,7 @@ static ssize_t inv_mpu6050_fifo_rate_store(struct device *dev,
>  	if (kstrtoint(buf, 10, &fifo_rate))
>  		return -EINVAL;
>  	if (fifo_rate < INV_MPU6050_MIN_FIFO_RATE ||
> -				fifo_rate > INV_MPU6050_MAX_FIFO_RATE)
> +	    fifo_rate > INV_MPU6050_MAX_FIFO_RATE)
>  		return -EINVAL;
>  	if (fifo_rate == st->chip_config.fifo_rate)
>  		return count;
> @@ -515,8 +513,9 @@ fifo_rate_fail:
>  /**
>   * inv_fifo_rate_show() - Get the current sampling rate.
>   */
> -static ssize_t inv_fifo_rate_show(struct device *dev,
> -	struct device_attribute *attr, char *buf)
> +static ssize_t
> +inv_fifo_rate_show(struct device *dev, struct device_attribute *attr,
> +		   char *buf)
>  {
>  	struct inv_mpu6050_state *st = iio_priv(dev_to_iio_dev(dev));
>  
> @@ -527,8 +526,8 @@ static ssize_t inv_fifo_rate_show(struct device *dev,
>   * inv_attr_show() - calling this function will show current
>   *                    parameters.
>   */
> -static ssize_t inv_attr_show(struct device *dev,
> -	struct device_attribute *attr, char *buf)
> +static ssize_t inv_attr_show(struct device *dev, struct device_attribute *attr,
> +			     char *buf)
>  {
>  	struct inv_mpu6050_state *st = iio_priv(dev_to_iio_dev(dev));
>  	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
> @@ -676,11 +675,11 @@ static int inv_check_and_setup_chip(struct inv_mpu6050_state *st)
>  		return result;
>  
>  	result = inv_mpu6050_switch_engine(st, false,
> -					INV_MPU6050_BIT_PWR_ACCL_STBY);
> +					   INV_MPU6050_BIT_PWR_ACCL_STBY);
>  	if (result)
>  		return result;
>  	result = inv_mpu6050_switch_engine(st, false,
> -					INV_MPU6050_BIT_PWR_GYRO_STBY);
> +					   INV_MPU6050_BIT_PWR_GYRO_STBY);
>  	if (result)
>  		return result;
>  
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> index af400dd..71bdaa3 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> @@ -111,7 +111,7 @@ static int inv_mpu6050_deselect_bypass(struct i2c_adapter *adap,
>   *  Returns 0 on success, a negative error code otherwise.
>   */
>  static int inv_mpu_probe(struct i2c_client *client,
> -	const struct i2c_device_id *id)
> +			 const struct i2c_device_id *id)
>  {
>  	struct inv_mpu6050_state *st;
>  	int result;
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> index 0bc5091..d070062 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> @@ -57,7 +57,7 @@ int inv_reset_fifo(struct iio_dev *indio_dev)
>  
>  	/* reset FIFO*/
>  	result = regmap_write(st->map, st->reg->user_ctrl,
> -					INV_MPU6050_BIT_FIFO_RST);
> +			      INV_MPU6050_BIT_FIFO_RST);
>  	if (result)
>  		goto reset_fifo_fail;
>  
> @@ -68,13 +68,13 @@ int inv_reset_fifo(struct iio_dev *indio_dev)
>  	if (st->chip_config.accl_fifo_enable ||
>  	    st->chip_config.gyro_fifo_enable) {
>  		result = regmap_write(st->map, st->reg->int_enable,
> -					INV_MPU6050_BIT_DATA_RDY_EN);
> +				      INV_MPU6050_BIT_DATA_RDY_EN);
>  		if (result)
>  			return result;
>  	}
>  	/* enable FIFO reading and I2C master interface*/
>  	result = regmap_write(st->map, st->reg->user_ctrl,
> -					INV_MPU6050_BIT_FIFO_EN);
> +			      INV_MPU6050_BIT_FIFO_EN);
>  	if (result)
>  		goto reset_fifo_fail;
>  	/* enable sensor output to FIFO */
> @@ -92,7 +92,7 @@ int inv_reset_fifo(struct iio_dev *indio_dev)
>  reset_fifo_fail:
>  	dev_err(regmap_get_device(st->map), "reset fifo failed %d\n", result);
>  	result = regmap_write(st->map, st->reg->int_enable,
> -					INV_MPU6050_BIT_DATA_RDY_EN);
> +			      INV_MPU6050_BIT_DATA_RDY_EN);
>  
>  	return result;
>  }
> @@ -109,7 +109,7 @@ irqreturn_t inv_mpu6050_irq_handler(int irq, void *p)
>  
>  	timestamp = iio_get_time_ns();
>  	kfifo_in_spinlocked(&st->timestamps, &timestamp, 1,
> -				&st->time_stamp_lock);
> +			    &st->time_stamp_lock);
>  
>  	return IRQ_WAKE_THREAD;
>  }
> @@ -143,9 +143,8 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
>  	 * read fifo_count register to know how many bytes inside FIFO
>  	 * right now
>  	 */
> -	result = regmap_bulk_read(st->map,
> -				       st->reg->fifo_count_h,
> -				       data, INV_MPU6050_FIFO_COUNT_BYTE);
> +	result = regmap_bulk_read(st->map, st->reg->fifo_count_h, data,
> +				  INV_MPU6050_FIFO_COUNT_BYTE);
>  	if (result)
>  		goto end_session;
>  	fifo_count = be16_to_cpup((__be16 *)(&data[0]));
> @@ -172,7 +171,7 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
>  			timestamp = 0;
>  
>  		result = iio_push_to_buffers_with_timestamp(indio_dev, data,
> -			timestamp);
> +							    timestamp);
>  		if (result)
>  			goto flush_fifo;
>  		fifo_count -= bytes_per_datum;
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> index 72d6aae..e8818d4 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> @@ -19,19 +19,19 @@ static void inv_scan_query(struct iio_dev *indio_dev)
>  
>  	st->chip_config.gyro_fifo_enable =
>  		test_bit(INV_MPU6050_SCAN_GYRO_X,
> -			indio_dev->active_scan_mask) ||
> -			test_bit(INV_MPU6050_SCAN_GYRO_Y,
> -			indio_dev->active_scan_mask) ||
> -			test_bit(INV_MPU6050_SCAN_GYRO_Z,
> -			indio_dev->active_scan_mask);
> +			 indio_dev->active_scan_mask) ||
> +		test_bit(INV_MPU6050_SCAN_GYRO_Y,
> +			 indio_dev->active_scan_mask) ||
> +		test_bit(INV_MPU6050_SCAN_GYRO_Z,
> +			 indio_dev->active_scan_mask);
>  
>  	st->chip_config.accl_fifo_enable =
>  		test_bit(INV_MPU6050_SCAN_ACCL_X,
> -			indio_dev->active_scan_mask) ||
> -			test_bit(INV_MPU6050_SCAN_ACCL_Y,
> -			indio_dev->active_scan_mask) ||
> -			test_bit(INV_MPU6050_SCAN_ACCL_Z,
> -			indio_dev->active_scan_mask);
> +			 indio_dev->active_scan_mask) ||
> +		test_bit(INV_MPU6050_SCAN_ACCL_Y,
> +			 indio_dev->active_scan_mask) ||
> +		test_bit(INV_MPU6050_SCAN_ACCL_Z,
> +			 indio_dev->active_scan_mask);
>  }
>  
>  /**
> @@ -101,7 +101,7 @@ static int inv_mpu6050_set_enable(struct iio_dev *indio_dev, bool enable)
>   * @state: Desired trigger state
>   */
>  static int inv_mpu_data_rdy_trigger_set_state(struct iio_trigger *trig,
> -						bool state)
> +					      bool state)
>  {
>  	return inv_mpu6050_set_enable(iio_trigger_get_drvdata(trig), state);
>  }
> 

Re: [RFC PATCH 7/9] iio: imu: inv_mpu6050: Fix alignment with open parenthesis

[Jonathan Cameron]

On 18/02/16 15:53, Daniel Baluta wrote:
> This makes code consistent around inv_mpu6050 driver and
> fixes the following checkpatch.pl warning:
> CHECK: Alignment should match open parenthesis
> 
> Note that there were few cases were it was not possible to
> fix this due to making the line too long, but we can live with that.
> 
> Signed-off-by: Daniel Baluta <daniel.baluta-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Applied
> ---
>  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c    | 55 +++++++++++++--------------
>  drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c     |  2 +-
>  drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c    | 17 ++++-----
>  drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c | 22 +++++------
>  4 files changed, 47 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index 48ab575..1643cd2 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -121,7 +121,7 @@ int inv_mpu6050_switch_engine(struct inv_mpu6050_state *st, bool en, u32 mask)
>  			/* switch internal clock to PLL */
>  			mgmt_1 |= INV_CLK_PLL;
>  			result = regmap_write(st->map,
> -					st->reg->pwr_mgmt_1, mgmt_1);
> +					      st->reg->pwr_mgmt_1, mgmt_1);
>  			if (result)
>  				return result;
>  		}
> @@ -196,14 +196,14 @@ static int inv_mpu6050_init_config(struct iio_dev *indio_dev)
>  		return result;
>  
>  	memcpy(&st->chip_config, hw_info[st->chip_type].config,
> -		sizeof(struct inv_mpu6050_chip_config));
> +	       sizeof(struct inv_mpu6050_chip_config));
>  	result = inv_mpu6050_set_power_itg(st, false);
>  
>  	return result;
>  }
>  
>  static int inv_mpu6050_sensor_show(struct inv_mpu6050_state  *st, int reg,
> -				int axis, int *val)
> +				   int axis, int *val)
>  {
>  	int ind, result;
>  	__be16 d;
> @@ -217,11 +217,10 @@ static int inv_mpu6050_sensor_show(struct inv_mpu6050_state  *st, int reg,
>  	return IIO_VAL_INT;
>  }
>  
> -static int inv_mpu6050_read_raw(struct iio_dev *indio_dev,
> -			      struct iio_chan_spec const *chan,
> -			      int *val,
> -			      int *val2,
> -			      long mask) {
> +static int
> +inv_mpu6050_read_raw(struct iio_dev *indio_dev,
> +		     struct iio_chan_spec const *chan,
> +		     int *val, int *val2, long mask) {
>  	struct inv_mpu6050_state  *st = iio_priv(indio_dev);
>  
>  	switch (mask) {
> @@ -241,16 +240,16 @@ static int inv_mpu6050_read_raw(struct iio_dev *indio_dev,
>  		switch (chan->type) {
>  		case IIO_ANGL_VEL:
>  			if (!st->chip_config.gyro_fifo_enable ||
> -					!st->chip_config.enable) {
> +			    !st->chip_config.enable) {
>  				result = inv_mpu6050_switch_engine(st, true,
>  						INV_MPU6050_BIT_PWR_GYRO_STBY);
>  				if (result)
>  					goto error_read_raw;
>  			}
>  			ret =  inv_mpu6050_sensor_show(st, st->reg->raw_gyro,
> -						chan->channel2, val);
> +						       chan->channel2, val);
>  			if (!st->chip_config.gyro_fifo_enable ||
> -					!st->chip_config.enable) {
> +			    !st->chip_config.enable) {
>  				result = inv_mpu6050_switch_engine(st, false,
>  						INV_MPU6050_BIT_PWR_GYRO_STBY);
>  				if (result)
> @@ -259,16 +258,16 @@ static int inv_mpu6050_read_raw(struct iio_dev *indio_dev,
>  			break;
>  		case IIO_ACCEL:
>  			if (!st->chip_config.accl_fifo_enable ||
> -					!st->chip_config.enable) {
> +			    !st->chip_config.enable) {
>  				result = inv_mpu6050_switch_engine(st, true,
>  						INV_MPU6050_BIT_PWR_ACCL_STBY);
>  				if (result)
>  					goto error_read_raw;
>  			}
>  			ret = inv_mpu6050_sensor_show(st, st->reg->raw_accl,
> -						chan->channel2, val);
> +						      chan->channel2, val);
>  			if (!st->chip_config.accl_fifo_enable ||
> -					!st->chip_config.enable) {
> +			    !st->chip_config.enable) {
>  				result = inv_mpu6050_switch_engine(st, false,
>  						INV_MPU6050_BIT_PWR_ACCL_STBY);
>  				if (result)
> @@ -279,7 +278,7 @@ static int inv_mpu6050_read_raw(struct iio_dev *indio_dev,
>  			/* wait for stablization */
>  			msleep(INV_MPU6050_SENSOR_UP_TIME);
>  			inv_mpu6050_sensor_show(st, st->reg->temperature,
> -							IIO_MOD_X, val);
> +						IIO_MOD_X, val);
>  			break;
>  		default:
>  			ret = -EINVAL;
> @@ -387,10 +386,8 @@ static int inv_mpu6050_write_accel_scale(struct inv_mpu6050_state *st, int val)
>  }
>  
>  static int inv_mpu6050_write_raw(struct iio_dev *indio_dev,
> -			       struct iio_chan_spec const *chan,
> -			       int val,
> -			       int val2,
> -			       long mask) {
> +				 struct iio_chan_spec const *chan,
> +				 int val, int val2, long mask) {
>  	struct inv_mpu6050_state  *st = iio_priv(indio_dev);
>  	int result;
>  
> @@ -467,8 +464,9 @@ static int inv_mpu6050_set_lpf(struct inv_mpu6050_state *st, int rate)
>  /**
>   * inv_mpu6050_fifo_rate_store() - Set fifo rate.
>   */
> -static ssize_t inv_mpu6050_fifo_rate_store(struct device *dev,
> -	struct device_attribute *attr, const char *buf, size_t count)
> +static ssize_t
> +inv_mpu6050_fifo_rate_store(struct device *dev, struct device_attribute *attr,
> +			    const char *buf, size_t count)
>  {
>  	s32 fifo_rate;
>  	u8 d;
> @@ -479,7 +477,7 @@ static ssize_t inv_mpu6050_fifo_rate_store(struct device *dev,
>  	if (kstrtoint(buf, 10, &fifo_rate))
>  		return -EINVAL;
>  	if (fifo_rate < INV_MPU6050_MIN_FIFO_RATE ||
> -				fifo_rate > INV_MPU6050_MAX_FIFO_RATE)
> +	    fifo_rate > INV_MPU6050_MAX_FIFO_RATE)
>  		return -EINVAL;
>  	if (fifo_rate == st->chip_config.fifo_rate)
>  		return count;
> @@ -515,8 +513,9 @@ fifo_rate_fail:
>  /**
>   * inv_fifo_rate_show() - Get the current sampling rate.
>   */
> -static ssize_t inv_fifo_rate_show(struct device *dev,
> -	struct device_attribute *attr, char *buf)
> +static ssize_t
> +inv_fifo_rate_show(struct device *dev, struct device_attribute *attr,
> +		   char *buf)
>  {
>  	struct inv_mpu6050_state *st = iio_priv(dev_to_iio_dev(dev));
>  
> @@ -527,8 +526,8 @@ static ssize_t inv_fifo_rate_show(struct device *dev,
>   * inv_attr_show() - calling this function will show current
>   *                    parameters.
>   */
> -static ssize_t inv_attr_show(struct device *dev,
> -	struct device_attribute *attr, char *buf)
> +static ssize_t inv_attr_show(struct device *dev, struct device_attribute *attr,
> +			     char *buf)
>  {
>  	struct inv_mpu6050_state *st = iio_priv(dev_to_iio_dev(dev));
>  	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
> @@ -676,11 +675,11 @@ static int inv_check_and_setup_chip(struct inv_mpu6050_state *st)
>  		return result;
>  
>  	result = inv_mpu6050_switch_engine(st, false,
> -					INV_MPU6050_BIT_PWR_ACCL_STBY);
> +					   INV_MPU6050_BIT_PWR_ACCL_STBY);
>  	if (result)
>  		return result;
>  	result = inv_mpu6050_switch_engine(st, false,
> -					INV_MPU6050_BIT_PWR_GYRO_STBY);
> +					   INV_MPU6050_BIT_PWR_GYRO_STBY);
>  	if (result)
>  		return result;
>  
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> index af400dd..71bdaa3 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> @@ -111,7 +111,7 @@ static int inv_mpu6050_deselect_bypass(struct i2c_adapter *adap,
>   *  Returns 0 on success, a negative error code otherwise.
>   */
>  static int inv_mpu_probe(struct i2c_client *client,
> -	const struct i2c_device_id *id)
> +			 const struct i2c_device_id *id)
>  {
>  	struct inv_mpu6050_state *st;
>  	int result;
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> index 0bc5091..d070062 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> @@ -57,7 +57,7 @@ int inv_reset_fifo(struct iio_dev *indio_dev)
>  
>  	/* reset FIFO*/
>  	result = regmap_write(st->map, st->reg->user_ctrl,
> -					INV_MPU6050_BIT_FIFO_RST);
> +			      INV_MPU6050_BIT_FIFO_RST);
>  	if (result)
>  		goto reset_fifo_fail;
>  
> @@ -68,13 +68,13 @@ int inv_reset_fifo(struct iio_dev *indio_dev)
>  	if (st->chip_config.accl_fifo_enable ||
>  	    st->chip_config.gyro_fifo_enable) {
>  		result = regmap_write(st->map, st->reg->int_enable,
> -					INV_MPU6050_BIT_DATA_RDY_EN);
> +				      INV_MPU6050_BIT_DATA_RDY_EN);
>  		if (result)
>  			return result;
>  	}
>  	/* enable FIFO reading and I2C master interface*/
>  	result = regmap_write(st->map, st->reg->user_ctrl,
> -					INV_MPU6050_BIT_FIFO_EN);
> +			      INV_MPU6050_BIT_FIFO_EN);
>  	if (result)
>  		goto reset_fifo_fail;
>  	/* enable sensor output to FIFO */
> @@ -92,7 +92,7 @@ int inv_reset_fifo(struct iio_dev *indio_dev)
>  reset_fifo_fail:
>  	dev_err(regmap_get_device(st->map), "reset fifo failed %d\n", result);
>  	result = regmap_write(st->map, st->reg->int_enable,
> -					INV_MPU6050_BIT_DATA_RDY_EN);
> +			      INV_MPU6050_BIT_DATA_RDY_EN);
>  
>  	return result;
>  }
> @@ -109,7 +109,7 @@ irqreturn_t inv_mpu6050_irq_handler(int irq, void *p)
>  
>  	timestamp = iio_get_time_ns();
>  	kfifo_in_spinlocked(&st->timestamps, &timestamp, 1,
> -				&st->time_stamp_lock);
> +			    &st->time_stamp_lock);
>  
>  	return IRQ_WAKE_THREAD;
>  }
> @@ -143,9 +143,8 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
>  	 * read fifo_count register to know how many bytes inside FIFO
>  	 * right now
>  	 */
> -	result = regmap_bulk_read(st->map,
> -				       st->reg->fifo_count_h,
> -				       data, INV_MPU6050_FIFO_COUNT_BYTE);
> +	result = regmap_bulk_read(st->map, st->reg->fifo_count_h, data,
> +				  INV_MPU6050_FIFO_COUNT_BYTE);
>  	if (result)
>  		goto end_session;
>  	fifo_count = be16_to_cpup((__be16 *)(&data[0]));
> @@ -172,7 +171,7 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
>  			timestamp = 0;
>  
>  		result = iio_push_to_buffers_with_timestamp(indio_dev, data,
> -			timestamp);
> +							    timestamp);
>  		if (result)
>  			goto flush_fifo;
>  		fifo_count -= bytes_per_datum;
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> index 72d6aae..e8818d4 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> @@ -19,19 +19,19 @@ static void inv_scan_query(struct iio_dev *indio_dev)
>  
>  	st->chip_config.gyro_fifo_enable =
>  		test_bit(INV_MPU6050_SCAN_GYRO_X,
> -			indio_dev->active_scan_mask) ||
> -			test_bit(INV_MPU6050_SCAN_GYRO_Y,
> -			indio_dev->active_scan_mask) ||
> -			test_bit(INV_MPU6050_SCAN_GYRO_Z,
> -			indio_dev->active_scan_mask);
> +			 indio_dev->active_scan_mask) ||
> +		test_bit(INV_MPU6050_SCAN_GYRO_Y,
> +			 indio_dev->active_scan_mask) ||
> +		test_bit(INV_MPU6050_SCAN_GYRO_Z,
> +			 indio_dev->active_scan_mask);
>  
>  	st->chip_config.accl_fifo_enable =
>  		test_bit(INV_MPU6050_SCAN_ACCL_X,
> -			indio_dev->active_scan_mask) ||
> -			test_bit(INV_MPU6050_SCAN_ACCL_Y,
> -			indio_dev->active_scan_mask) ||
> -			test_bit(INV_MPU6050_SCAN_ACCL_Z,
> -			indio_dev->active_scan_mask);
> +			 indio_dev->active_scan_mask) ||
> +		test_bit(INV_MPU6050_SCAN_ACCL_Y,
> +			 indio_dev->active_scan_mask) ||
> +		test_bit(INV_MPU6050_SCAN_ACCL_Z,
> +			 indio_dev->active_scan_mask);
>  }
>  
>  /**
> @@ -101,7 +101,7 @@ static int inv_mpu6050_set_enable(struct iio_dev *indio_dev, bool enable)
>   * @state: Desired trigger state
>   */
>  static int inv_mpu_data_rdy_trigger_set_state(struct iio_trigger *trig,
> -						bool state)
> +					      bool state)
>  {
>  	return inv_mpu6050_set_enable(iio_trigger_get_drvdata(trig), state);
>  }
> 

Re: [RFC PATCH 7/9] iio: imu: inv_mpu6050: Fix alignment with open parenthesis

[Joe Perches]

On Sun, 2016-02-21 at 20:41 +0000, Jonathan Cameron wrote:
> On 18/02/16 15:53, Daniel Baluta wrote:
> > This makes code consistent around inv_mpu6050 driver and
> > fixes the following checkpatch.pl warning:
> > CHECK: Alignment should match open parenthesis
> > 
> > Note that there were few cases were it was not possible to
> > fix this due to making the line too long, but we can live with that.
> > 
> > Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
> Applied
[]
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
[]
> > @@ -217,11 +217,10 @@ static int inv_mpu6050_sensor_show(struct inv_mpu6050_state  *st, int reg,
> >  	return IIO_VAL_INT;
> >  }
> >  
> > -static int inv_mpu6050_read_raw(struct iio_dev *indio_dev,
> > -			      struct iio_chan_spec const *chan,
> > -			      int *val,
> > -			      int *val2,
> > -			      long mask) {
> > +static int
> > +inv_mpu6050_read_raw(struct iio_dev *indio_dev,
> > +		     struct iio_chan_spec const *chan,
> > +		     int *val, int *val2, long mask) {

Ideally, you'd also convert this form to use
the open brace on a new line like:

static int
inv_mpu6050_read_raw(struct iio_dev *indio_dev,
		     struct iio_chan_spec const *chan,
		     int *val, int *val2, long mask)
{

Re: [RFC PATCH 7/9] iio: imu: inv_mpu6050: Fix alignment with open parenthesis

[Joe Perches]

On Sun, 2016-02-21 at 20:41 +0000, Jonathan Cameron wrote:
> On 18/02/16 15:53, Daniel Baluta wrote:
> > This makes code consistent around inv_mpu6050 driver and
> > fixes the following checkpatch.pl warning:
> > CHECK: Alignment should match open parenthesis
> > 
> > Note that there were few cases were it was not possible to
> > fix this due to making the line too long, but we can live with that.
> > 
> > Signed-off-by: Daniel Baluta <daniel.baluta-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Applied
[]
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
[]
> > @@ -217,11 +217,10 @@ static int inv_mpu6050_sensor_show(struct inv_mpu6050_state  *st, int reg,
> >  	return IIO_VAL_INT;
> >  }
> >  
> > -static int inv_mpu6050_read_raw(struct iio_dev *indio_dev,
> > -			      struct iio_chan_spec const *chan,
> > -			      int *val,
> > -			      int *val2,
> > -			      long mask) {
> > +static int
> > +inv_mpu6050_read_raw(struct iio_dev *indio_dev,
> > +		     struct iio_chan_spec const *chan,
> > +		     int *val, int *val2, long mask) {

Ideally, you'd also convert this form to use
the open brace on a new line like:

static int
inv_mpu6050_read_raw(struct iio_dev *indio_dev,
		     struct iio_chan_spec const *chan,
		     int *val, int *val2, long mask)
{

Re: [RFC PATCH 7/9] iio: imu: inv_mpu6050: Fix alignment with open parenthesis

[Jonathan Cameron]

On 21/02/16 20:59, Joe Perches wrote:
> On Sun, 2016-02-21 at 20:41 +0000, Jonathan Cameron wrote:
>> On 18/02/16 15:53, Daniel Baluta wrote:
>>> This makes code consistent around inv_mpu6050 driver and
>>> fixes the following checkpatch.pl warning:
>>> CHECK: Alignment should match open parenthesis
>>>
>>> Note that there were few cases were it was not possible to
>>> fix this due to making the line too long, but we can live with that.
>>>
>>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
>> Applied
> []
>>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> []
>>> @@ -217,11 +217,10 @@ static int inv_mpu6050_sensor_show(struct inv_mpu6050_state  *st, int reg,
>>>  	return IIO_VAL_INT;
>>>  }
>>>  
>>> -static int inv_mpu6050_read_raw(struct iio_dev *indio_dev,
>>> -			      struct iio_chan_spec const *chan,
>>> -			      int *val,
>>> -			      int *val2,
>>> -			      long mask) {
>>> +static int
>>> +inv_mpu6050_read_raw(struct iio_dev *indio_dev,
>>> +		     struct iio_chan_spec const *chan,
>>> +		     int *val, int *val2, long mask) {
> 
> Ideally, you'd also convert this form to use
> the open brace on a new line like:
> 
> static int
> inv_mpu6050_read_raw(struct iio_dev *indio_dev,
> 		     struct iio_chan_spec const *chan,
> 		     int *val, int *val2, long mask)
> {
> 
Good point - as it fitted nicely in with Daniel's patch I applied a quick fixup
doing this one and the write_raw.

Thanks Joe,

Jonathan

Re: [RFC PATCH 7/9] iio: imu: inv_mpu6050: Fix alignment with open parenthesis

[Jonathan Cameron]

On 21/02/16 20:59, Joe Perches wrote:
> On Sun, 2016-02-21 at 20:41 +0000, Jonathan Cameron wrote:
>> On 18/02/16 15:53, Daniel Baluta wrote:
>>> This makes code consistent around inv_mpu6050 driver and
>>> fixes the following checkpatch.pl warning:
>>> CHECK: Alignment should match open parenthesis
>>>
>>> Note that there were few cases were it was not possible to
>>> fix this due to making the line too long, but we can live with that.
>>>
>>> Signed-off-by: Daniel Baluta <daniel.baluta-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> Applied
> []
>>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> []
>>> @@ -217,11 +217,10 @@ static int inv_mpu6050_sensor_show(struct inv_mpu6050_state  *st, int reg,
>>>  	return IIO_VAL_INT;
>>>  }
>>>  
>>> -static int inv_mpu6050_read_raw(struct iio_dev *indio_dev,
>>> -			      struct iio_chan_spec const *chan,
>>> -			      int *val,
>>> -			      int *val2,
>>> -			      long mask) {
>>> +static int
>>> +inv_mpu6050_read_raw(struct iio_dev *indio_dev,
>>> +		     struct iio_chan_spec const *chan,
>>> +		     int *val, int *val2, long mask) {
> 
> Ideally, you'd also convert this form to use
> the open brace on a new line like:
> 
> static int
> inv_mpu6050_read_raw(struct iio_dev *indio_dev,
> 		     struct iio_chan_spec const *chan,
> 		     int *val, int *val2, long mask)
> {
> 
Good point - as it fitted nicely in with Daniel's patch I applied a quick fixup
doing this one and the write_raw.

Thanks Joe,

Jonathan

Re: [RFC PATCH 0/9] iio: Fix ABBA deadlock in inv-mpu6050

[Wolfram Sang]

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

On Thu, Feb 18, 2016 at 05:53:05PM +0200, Daniel Baluta wrote:
> Sending this as an RFC because I don't know if style fixes are appropriate
> for this driver and also not sure if deadlock fix is the best solution.
> 
> I2C people should only look at patches 8/9 and 9/9.
> 
> The mpu6050 accel+gyro combo has an auxiliary I2C bus, allowing for
> an auxiliary sensor to be connected (eg. a magnetometer).
> 
> The mpu can either act as an I2C master (functionality not currently
> implemented in the driver) or it can use a bypass multiplexer which
> directly connects the auxiliary I2C bus pins to the main I2C bus pins [1]
> Currently the driver implements the bypass mode via an I2C mux.
> 
> This patchset fixes a deadlock in inv-mpu6050 IMU which happens when
> magnetometer (on auxiliary I2C bus) runs in parallel with accel or gyro.
> 
> First 7 patches do some cleanup in order to make INV MPU6050 code easier
> to read.
> 
> Patch number 8 allows passing NULL select callbacks to i2c_mux_master_xfer
> and i2c_mux_smbus_xfer.
> 
> Patch number 9 actually fixes the deadlock.

We recently had a bigger patch series fixing locking problems related to
muxes. I sadly didn't have the time to review it. Can you have a look if
it helps your case?

http://thread.gmane.org/gmane.linux.drivers.i2c/26169

Thanks,

   Wolfram


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

Re: [RFC PATCH 0/9] iio: Fix ABBA deadlock in inv-mpu6050

[Daniel Baluta]

On Mon, Feb 22, 2016 at 1:17 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
> On Thu, Feb 18, 2016 at 05:53:05PM +0200, Daniel Baluta wrote:
>> Sending this as an RFC because I don't know if style fixes are appropriate
>> for this driver and also not sure if deadlock fix is the best solution.
>>
>> I2C people should only look at patches 8/9 and 9/9.
>>
>> The mpu6050 accel+gyro combo has an auxiliary I2C bus, allowing for
>> an auxiliary sensor to be connected (eg. a magnetometer).
>>
>> The mpu can either act as an I2C master (functionality not currently
>> implemented in the driver) or it can use a bypass multiplexer which
>> directly connects the auxiliary I2C bus pins to the main I2C bus pins [1]
>> Currently the driver implements the bypass mode via an I2C mux.
>>
>> This patchset fixes a deadlock in inv-mpu6050 IMU which happens when
>> magnetometer (on auxiliary I2C bus) runs in parallel with accel or gyro.
>>
>> First 7 patches do some cleanup in order to make INV MPU6050 code easier
>> to read.
>>
>> Patch number 8 allows passing NULL select callbacks to i2c_mux_master_xfer
>> and i2c_mux_smbus_xfer.
>>
>> Patch number 9 actually fixes the deadlock.
>
> We recently had a bigger patch series fixing locking problems related to
> muxes. I sadly didn't have the time to review it. Can you have a look if
> it helps your case?
>
> http://thread.gmane.org/gmane.linux.drivers.i2c/26169

Thanks Wolfram,

Will look into it.

Re: [RFC PATCH 0/9] iio: Fix ABBA deadlock in inv-mpu6050

[Daniel Baluta]

On Mon, Feb 22, 2016 at 1:17 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
> On Thu, Feb 18, 2016 at 05:53:05PM +0200, Daniel Baluta wrote:
>> Sending this as an RFC because I don't know if style fixes are appropriate
>> for this driver and also not sure if deadlock fix is the best solution.
>>
>> I2C people should only look at patches 8/9 and 9/9.
>>
>> The mpu6050 accel+gyro combo has an auxiliary I2C bus, allowing for
>> an auxiliary sensor to be connected (eg. a magnetometer).
>>
>> The mpu can either act as an I2C master (functionality not currently
>> implemented in the driver) or it can use a bypass multiplexer which
>> directly connects the auxiliary I2C bus pins to the main I2C bus pins [1]
>> Currently the driver implements the bypass mode via an I2C mux.
>>
>> This patchset fixes a deadlock in inv-mpu6050 IMU which happens when
>> magnetometer (on auxiliary I2C bus) runs in parallel with accel or gyro.
>>
>> First 7 patches do some cleanup in order to make INV MPU6050 code easier
>> to read.
>>
>> Patch number 8 allows passing NULL select callbacks to i2c_mux_master_xfer
>> and i2c_mux_smbus_xfer.
>>
>> Patch number 9 actually fixes the deadlock.
>
> We recently had a bigger patch series fixing locking problems related to
> muxes. I sadly didn't have the time to review it. Can you have a look if
> it helps your case?
>
> http://thread.gmane.org/gmane.linux.drivers.i2c/26169


Hi Wolfram,

Tested this and the deadlock is still there :(.

It can be easily reproduced with following patch which enlarges
the race window:

iff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index a3f5070..279d6e0 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -232,6 +232,7 @@ inv_mpu6050_read_raw(struct iio_dev *indio_dev,
                ret = IIO_VAL_INT;
                result = 0;
                mutex_lock(&indio_dev->mlock);
+               msleep(12000);
                if (!st->chip_config.enable) {
                        result = inv_mpu6050_set_power_itg(st, true);
                        if (result)


using the following commands:

$ cat /sys/bus/iio/devices/iio:device0/in_accel_x_raw
$ cat /sys/bus/iio/devices/iio:device1/in_magn_x_raw

Off topic: I'm very curious why I didn't get a lockdep splat while running this,
might be because adapter lock is a rt_mutex.

thanks,
Daniel.

Re: [RFC PATCH 8/9] i2c: i2c-mux: Allow for NULL select callback

[Wolfram Sang]

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

On Thu, Feb 18, 2016 at 05:53:13PM +0200, Daniel Baluta wrote:
> From: Adriana Reus <adriana.reus@intel.com>
> 
> Add a check in i2c_mux_master_xfer before calling the select callback.
> This is necessary so that NULL callbacks can be safely registered.
> 
> Signed-off-by: Adriana Reus <adriana.reus@intel.com>
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>

Hmm, rather than supporting that in the core, I'd prefer to have the
driver pass an empty function instead. Then, in the driver, we can have
a comment explaining the special situation.


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

Re: [RFC PATCH 8/9] i2c: i2c-mux: Allow for NULL select callback

[Daniel Baluta]

On Tue, Mar 1, 2016 at 10:30 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> On Thu, Feb 18, 2016 at 05:53:13PM +0200, Daniel Baluta wrote:
>> From: Adriana Reus <adriana.reus@intel.com>
>>
>> Add a check in i2c_mux_master_xfer before calling the select callback.
>> This is necessary so that NULL callbacks can be safely registered.
>>
>> Signed-off-by: Adriana Reus <adriana.reus@intel.com>
>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
>
> Hmm, rather than supporting that in the core, I'd prefer to have the
> driver pass an empty function instead. Then, in the driver, we can have
> a comment explaining the special situation.

Agree. This seems a better idea forcing the user to explain the situation :).

Re: [RFC PATCH 9/9] iio: imu: inv_mpu6050: Fix deadlock between i2c adapter lock and mpu lock

[Wolfram Sang]

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

On Thu, Feb 18, 2016 at 05:53:14PM +0200, Daniel Baluta wrote:
> From: Adriana Reus <adriana.reus@intel.com>
> 
> This deadlock occurs if the accel/gyro and the sensor on the auxiliary
> I2C (in my setup it's an ak8975) are working at the same time.
> 
> Scenario:
> 
>       T1					T2
>      ====				       ====
> inv_mpu6050_read_fifo                  aux sensor op (eg. ak8975_read_raw)
>         |                                     |
> mutex_lock(&indio_dev->mlock)           i2c_transfer
>         |                                     |
> i2c transaction                         i2c adapter lock
>         |                                     |
> i2c adapter lock                        i2c_mux_master_xfer
>                                               |
>                                         inv_mpu6050_select_bypass
>                                               |
>                                         mutex_lock(&indio_dev->mlock)
> 
> When we operate on an mpu sensor the order of locking is mpu lock
> followed by the i2c adapter lock. However, when we operate the auxiliary
> sensor the order of locking is the other way around.
> 
> In order to avoid this enable the bypass mux bit once in the beginning
> and remove the select/deselect_bypass functions.
> 
> This patch moves the bypass enabling in a separate function
> that is called once at probe and removes the functionality from
> inv_mpu_select/deselect_bypass.
> 
> Another advantage of this approach is that power-wise the mpu chip isn't
> powered up at each auxiliary sensor i2c transaction so if only the
> compass is used this would be more power efficient.

I think the comments (power must be enabled on select) rendered this
solution not acceptable. What about using mutex_trylock in
inv_mpu6050_select_bypass() and returning -EAGAIN if the lock is already
held?


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

Re: [RFC PATCH 9/9] iio: imu: inv_mpu6050: Fix deadlock between i2c adapter lock and mpu lock

[Wolfram Sang]

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

On Thu, Feb 18, 2016 at 05:53:14PM +0200, Daniel Baluta wrote:
> From: Adriana Reus <adriana.reus-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> This deadlock occurs if the accel/gyro and the sensor on the auxiliary
> I2C (in my setup it's an ak8975) are working at the same time.
> 
> Scenario:
> 
>       T1					T2
>      ====				       ====
> inv_mpu6050_read_fifo                  aux sensor op (eg. ak8975_read_raw)
>         |                                     |
> mutex_lock(&indio_dev->mlock)           i2c_transfer
>         |                                     |
> i2c transaction                         i2c adapter lock
>         |                                     |
> i2c adapter lock                        i2c_mux_master_xfer
>                                               |
>                                         inv_mpu6050_select_bypass
>                                               |
>                                         mutex_lock(&indio_dev->mlock)
> 
> When we operate on an mpu sensor the order of locking is mpu lock
> followed by the i2c adapter lock. However, when we operate the auxiliary
> sensor the order of locking is the other way around.
> 
> In order to avoid this enable the bypass mux bit once in the beginning
> and remove the select/deselect_bypass functions.
> 
> This patch moves the bypass enabling in a separate function
> that is called once at probe and removes the functionality from
> inv_mpu_select/deselect_bypass.
> 
> Another advantage of this approach is that power-wise the mpu chip isn't
> powered up at each auxiliary sensor i2c transaction so if only the
> compass is used this would be more power efficient.

I think the comments (power must be enabled on select) rendered this
solution not acceptable. What about using mutex_trylock in
inv_mpu6050_select_bypass() and returning -EAGAIN if the lock is already
held?


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

Re: [RFC PATCH 2/9] iio: imu: inv_mpu6050: Fix Yoda conditions

[Wolfram Sang]

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


"Yoda conditions" :D Made my day...


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

Re: [RFC PATCH 2/9] iio: imu: inv_mpu6050: Fix Yoda conditions

[Wolfram Sang]

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


"Yoda conditions" :D Made my day...


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

Re: [RFC PATCH 9/9] iio: imu: inv_mpu6050: Fix deadlock between i2c adapter lock and mpu lock

[Daniel Baluta]

On Tue, Mar 1, 2016 at 10:50 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> On Thu, Feb 18, 2016 at 05:53:14PM +0200, Daniel Baluta wrote:
>> From: Adriana Reus <adriana.reus@intel.com>
>>
>> This deadlock occurs if the accel/gyro and the sensor on the auxiliary
>> I2C (in my setup it's an ak8975) are working at the same time.
>>
>> Scenario:
>>
>>       T1                                      T2
>>      ====                                    ====
>> inv_mpu6050_read_fifo                  aux sensor op (eg. ak8975_read_raw)
>>         |                                     |
>> mutex_lock(&indio_dev->mlock)           i2c_transfer
>>         |                                     |
>> i2c transaction                         i2c adapter lock
>>         |                                     |
>> i2c adapter lock                        i2c_mux_master_xfer
>>                                               |
>>                                         inv_mpu6050_select_bypass
>>                                               |
>>                                         mutex_lock(&indio_dev->mlock)
>>
>> When we operate on an mpu sensor the order of locking is mpu lock
>> followed by the i2c adapter lock. However, when we operate the auxiliary
>> sensor the order of locking is the other way around.
>>
>> In order to avoid this enable the bypass mux bit once in the beginning
>> and remove the select/deselect_bypass functions.
>>
>> This patch moves the bypass enabling in a separate function
>> that is called once at probe and removes the functionality from
>> inv_mpu_select/deselect_bypass.
>>
>> Another advantage of this approach is that power-wise the mpu chip isn't
>> powered up at each auxiliary sensor i2c transaction so if only the
>> compass is used this would be more power efficient.
>
> I think the comments (power must be enabled on select) rendered this
> solution not acceptable. What about using mutex_trylock in
> inv_mpu6050_select_bypass() and returning -EAGAIN if the lock is already
> held?

Well, yes this would be a very good temporary solution. I'm afraid tough
that reading at high rates accel/gyro data will starve the aux sensor readings.

I looked into the I2C adapter / mux code but I got lost rapidly :). It
feels like
the natural solution would be for the I2C core to not hold the adapter lock
while doing transactions on the muxed child adapter.

Anyhow, sometimes starving is better than deadlocking so I will send a patch
with your suggestion.

thanks,
Daniel.

Re: [RFC PATCH 9/9] iio: imu: inv_mpu6050: Fix deadlock between i2c adapter lock and mpu lock

[Wolfram Sang]

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


> I looked into the I2C adapter / mux code but I got lost rapidly :). It
> feels like the natural solution would be for the I2C core to not hold
> the adapter lock while doing transactions on the muxed child adapter.

The patch series I mentioned to you does exactly that. It locks only the
mux side of the muxed bus, not the whole parent adapter. It didn't work
for you because the mux driver maybe needed some adaptions as well?

However, I am still undecided if that series should go upstream because
it makes the mux code another magnitude more complex. And while this
seems to be the second issue which could be fixed by that series, both
issues are corner cases, so I am not sure it is worth the complexity.


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

Re: [RFC PATCH 9/9] iio: imu: inv_mpu6050: Fix deadlock between i2c adapter lock and mpu lock

[Wolfram Sang]

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


> I looked into the I2C adapter / mux code but I got lost rapidly :). It
> feels like the natural solution would be for the I2C core to not hold
> the adapter lock while doing transactions on the muxed child adapter.

The patch series I mentioned to you does exactly that. It locks only the
mux side of the muxed bus, not the whole parent adapter. It didn't work
for you because the mux driver maybe needed some adaptions as well?

However, I am still undecided if that series should go upstream because
it makes the mux code another magnitude more complex. And while this
seems to be the second issue which could be fixed by that series, both
issues are corner cases, so I am not sure it is worth the complexity.


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

Re: [RFC PATCH 0/9] iio: Fix ABBA deadlock in inv-mpu6050

[Peter Rosin]

From: Peter Rosin <peda@axentia.se>

Hi Daniel,

Daniel Baluta wrote:
>On Mon, Feb 22, 2016 at 1:17 AM, Wolfram Sang wrote:
>> On Thu, Feb 18, 2016 at 05:53:05PM +0200, Daniel Baluta wrote:
>>> Sending this as an RFC because I don't know if style fixes are appropriate
>>> for this driver and also not sure if deadlock fix is the best solution.

*snip*

>> We recently had a bigger patch series fixing locking problems related to
>> muxes. I sadly didn't have the time to review it. Can you have a look if
>> it helps your case?
>>
>> http://thread.gmane.org/gmane.linux.drivers.i2c/26169
> 
> Tested this and the deadlock is still there :(.

I assume that when you tested v3 of my series, you did nothing
to actually make use of the new stuff available in the mux-core?
If you didn't do anything to make use of the new stuff, the
driver should behave as before.

So, please try this patch on top of my recently posted v4 of the
"i2c mux cleanup and locking update" series [1]. I have not tested
this patch at all, but I have the feeling it could do the trick...

Cheers,
Peter

[1] https://marc.info/?l=linux-iio&m=145704469628330&w=3

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index 642f22013d17..02b56e631973 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -79,35 +79,6 @@ int inv_mpu6050_write_reg(struct inv_mpu6050_state *st, int reg, u8 d)
 	return i2c_smbus_write_i2c_block_data(st->client, reg, 1, &d);
 }
 
-/*
- * The i2c read/write needs to happen in unlocked mode. As the parent
- * adapter is common. If we use locked versions, it will fail as
- * the mux adapter will lock the parent i2c adapter, while calling
- * select/deselect functions.
- */
-static int inv_mpu6050_write_reg_unlocked(struct inv_mpu6050_state *st,
-					  u8 reg, u8 d)
-{
-	int ret;
-	u8 buf[2];
-	struct i2c_msg msg[1] = {
-		{
-			.addr = st->client->addr,
-			.flags = 0,
-			.len = sizeof(buf),
-			.buf = buf,
-		}
-	};
-
-	buf[0] = reg;
-	buf[1] = d;
-	ret = __i2c_transfer(st->client->adapter, msg, 1);
-	if (ret != 1)
-		return ret;
-
-	return 0;
-}
-
 static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id)
 {
 	struct iio_dev *indio_dev = i2c_mux_priv(muxc);
@@ -117,8 +88,7 @@ static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id)
 	/* Use the same mutex which was used everywhere to protect power-op */
 	mutex_lock(&indio_dev->mlock);
 	if (!st->powerup_count) {
-		ret = inv_mpu6050_write_reg_unlocked(st, st->reg->pwr_mgmt_1,
-						     0);
+		ret = inv_mpu6050_write_reg(st, st->reg->pwr_mgmt_1, 0);
 		if (ret)
 			goto write_error;
 
@@ -126,9 +96,9 @@ static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id)
 	}
 	if (!ret) {
 		st->powerup_count++;
-		ret = inv_mpu6050_write_reg_unlocked(st, st->reg->int_pin_cfg,
-						     st->client->irq |
-						     INV_MPU6050_BIT_BYPASS_EN);
+		ret = inv_mpu6050_write_reg(st, st->reg->int_pin_cfg,
+					    st->client->irq |
+					    INV_MPU6050_BIT_BYPASS_EN);
 	}
 write_error:
 	mutex_unlock(&indio_dev->mlock);
@@ -143,12 +113,11 @@ static int inv_mpu6050_deselect_bypass(struct i2c_mux_core *muxc, u32 chan_id)
 
 	mutex_lock(&indio_dev->mlock);
 	/* It doesn't really mattter, if any of the calls fails */
-	inv_mpu6050_write_reg_unlocked(st, st->reg->int_pin_cfg,
-				       st->client->irq);
+	inv_mpu6050_write_reg(st, st->reg->int_pin_cfg, st->client->irq);
 	st->powerup_count--;
 	if (!st->powerup_count)
-		inv_mpu6050_write_reg_unlocked(st, st->reg->pwr_mgmt_1,
-					       INV_MPU6050_BIT_SLEEP);
+		inv_mpu6050_write_reg(st, st->reg->pwr_mgmt_1,
+				      INV_MPU6050_BIT_SLEEP);
 	mutex_unlock(&indio_dev->mlock);
 
 	return 0;
@@ -839,8 +808,8 @@ static int inv_mpu_probe(struct i2c_client *client,
 		goto out_remove_trigger;
 	}
 
-	st->muxc = i2c_mux_one_adapter(client->adapter, &client->dev, 0, 0,
-				       0, 0, 0,
+	st->muxc = i2c_mux_one_adapter(client->adapter, &client->dev, 0,
+				       I2C_CONTROLLED_MUX, 0, 0, 0,
 				       inv_mpu6050_select_bypass,
 				       inv_mpu6050_deselect_bypass);
 	if (IS_ERR(st->muxc)) {
-- 
2.1.4

Re: [RFC PATCH 0/9] iio: Fix ABBA deadlock in inv-mpu6050

[Daniel Baluta]

On Fri, Mar 4, 2016 at 1:09 AM, Peter Rosin <peda@lysator.liu.se> wrote:
> From: Peter Rosin <peda@axentia.se>
>
> Hi Daniel,
>
> Daniel Baluta wrote:
>>On Mon, Feb 22, 2016 at 1:17 AM, Wolfram Sang wrote:
>>> On Thu, Feb 18, 2016 at 05:53:05PM +0200, Daniel Baluta wrote:
>>>> Sending this as an RFC because I don't know if style fixes are appropriate
>>>> for this driver and also not sure if deadlock fix is the best solution.
>
> *snip*
>
>>> We recently had a bigger patch series fixing locking problems related to
>>> muxes. I sadly didn't have the time to review it. Can you have a look if
>>> it helps your case?
>>>
>>> http://thread.gmane.org/gmane.linux.drivers.i2c/26169
>>
>> Tested this and the deadlock is still there :(.
>
> I assume that when you tested v3 of my series, you did nothing
> to actually make use of the new stuff available in the mux-core?
> If you didn't do anything to make use of the new stuff, the
> driver should behave as before.

You are right. I used v3 as it is.
>
> So, please try this patch on top of my recently posted v4 of the
> "i2c mux cleanup and locking update" series [1]. I have not tested
> this patch at all, but I have the feeling it could do the trick...

This patch fixes the problem! :P

> Cheers,
> Peter
>
> [1] https://marc.info/?l=linux-iio&m=145704469628330&w=3
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index 642f22013d17..02b56e631973 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -79,35 +79,6 @@ int inv_mpu6050_write_reg(struct inv_mpu6050_state *st, int reg, u8 d)
>         return i2c_smbus_write_i2c_block_data(st->client, reg, 1, &d);
>  }
>
> -/*
> - * The i2c read/write needs to happen in unlocked mode. As the parent
> - * adapter is common. If we use locked versions, it will fail as
> - * the mux adapter will lock the parent i2c adapter, while calling
> - * select/deselect functions.
> - */
> -static int inv_mpu6050_write_reg_unlocked(struct inv_mpu6050_state *st,
> -                                         u8 reg, u8 d)
> -{
> -       int ret;
> -       u8 buf[2];
> -       struct i2c_msg msg[1] = {
> -               {
> -                       .addr = st->client->addr,
> -                       .flags = 0,
> -                       .len = sizeof(buf),
> -                       .buf = buf,
> -               }
> -       };
> -
> -       buf[0] = reg;
> -       buf[1] = d;
> -       ret = __i2c_transfer(st->client->adapter, msg, 1);
> -       if (ret != 1)
> -               return ret;
> -
> -       return 0;
> -}
> -
>  static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id)
>  {
>         struct iio_dev *indio_dev = i2c_mux_priv(muxc);
> @@ -117,8 +88,7 @@ static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id)
>         /* Use the same mutex which was used everywhere to protect power-op */
>         mutex_lock(&indio_dev->mlock);
>         if (!st->powerup_count) {
> -               ret = inv_mpu6050_write_reg_unlocked(st, st->reg->pwr_mgmt_1,
> -                                                    0);
> +               ret = inv_mpu6050_write_reg(st, st->reg->pwr_mgmt_1, 0);
>                 if (ret)
>                         goto write_error;
>
> @@ -126,9 +96,9 @@ static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id)
>         }
>         if (!ret) {
>                 st->powerup_count++;
> -               ret = inv_mpu6050_write_reg_unlocked(st, st->reg->int_pin_cfg,
> -                                                    st->client->irq |
> -                                                    INV_MPU6050_BIT_BYPASS_EN);
> +               ret = inv_mpu6050_write_reg(st, st->reg->int_pin_cfg,
> +                                           st->client->irq |
> +                                           INV_MPU6050_BIT_BYPASS_EN);
>         }
>  write_error:
>         mutex_unlock(&indio_dev->mlock);
> @@ -143,12 +113,11 @@ static int inv_mpu6050_deselect_bypass(struct i2c_mux_core *muxc, u32 chan_id)
>
>         mutex_lock(&indio_dev->mlock);
>         /* It doesn't really mattter, if any of the calls fails */
> -       inv_mpu6050_write_reg_unlocked(st, st->reg->int_pin_cfg,
> -                                      st->client->irq);
> +       inv_mpu6050_write_reg(st, st->reg->int_pin_cfg, st->client->irq);
>         st->powerup_count--;
>         if (!st->powerup_count)
> -               inv_mpu6050_write_reg_unlocked(st, st->reg->pwr_mgmt_1,
> -                                              INV_MPU6050_BIT_SLEEP);
> +               inv_mpu6050_write_reg(st, st->reg->pwr_mgmt_1,
> +                                     INV_MPU6050_BIT_SLEEP);
>         mutex_unlock(&indio_dev->mlock);
>
>         return 0;
> @@ -839,8 +808,8 @@ static int inv_mpu_probe(struct i2c_client *client,
>                 goto out_remove_trigger;
>         }
>
> -       st->muxc = i2c_mux_one_adapter(client->adapter, &client->dev, 0, 0,
> -                                      0, 0, 0,
> +       st->muxc = i2c_mux_one_adapter(client->adapter, &client->dev, 0,
> +                                      I2C_CONTROLLED_MUX, 0, 0, 0,
>                                        inv_mpu6050_select_bypass,
>                                        inv_mpu6050_deselect_bypass);
>         if (IS_ERR(st->muxc)) {
> --
> 2.1.4
>
> --
> 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: [RFC PATCH 0/9] iio: Fix ABBA deadlock in inv-mpu6050

[Daniel Baluta]

On Fri, Mar 4, 2016 at 1:09 AM, Peter Rosin <peda-SamgB31n2u5IcsJQ0EH25Q@public.gmane.org> wrote:
> From: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
>
> Hi Daniel,
>
> Daniel Baluta wrote:
>>On Mon, Feb 22, 2016 at 1:17 AM, Wolfram Sang wrote:
>>> On Thu, Feb 18, 2016 at 05:53:05PM +0200, Daniel Baluta wrote:
>>>> Sending this as an RFC because I don't know if style fixes are appropriate
>>>> for this driver and also not sure if deadlock fix is the best solution.
>
> *snip*
>
>>> We recently had a bigger patch series fixing locking problems related to
>>> muxes. I sadly didn't have the time to review it. Can you have a look if
>>> it helps your case?
>>>
>>> http://thread.gmane.org/gmane.linux.drivers.i2c/26169
>>
>> Tested this and the deadlock is still there :(.
>
> I assume that when you tested v3 of my series, you did nothing
> to actually make use of the new stuff available in the mux-core?
> If you didn't do anything to make use of the new stuff, the
> driver should behave as before.

You are right. I used v3 as it is.
>
> So, please try this patch on top of my recently posted v4 of the
> "i2c mux cleanup and locking update" series [1]. I have not tested
> this patch at all, but I have the feeling it could do the trick...

This patch fixes the problem! :P

> Cheers,
> Peter
>
> [1] https://marc.info/?l=linux-iio&m=145704469628330&w=3
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index 642f22013d17..02b56e631973 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -79,35 +79,6 @@ int inv_mpu6050_write_reg(struct inv_mpu6050_state *st, int reg, u8 d)
>         return i2c_smbus_write_i2c_block_data(st->client, reg, 1, &d);
>  }
>
> -/*
> - * The i2c read/write needs to happen in unlocked mode. As the parent
> - * adapter is common. If we use locked versions, it will fail as
> - * the mux adapter will lock the parent i2c adapter, while calling
> - * select/deselect functions.
> - */
> -static int inv_mpu6050_write_reg_unlocked(struct inv_mpu6050_state *st,
> -                                         u8 reg, u8 d)
> -{
> -       int ret;
> -       u8 buf[2];
> -       struct i2c_msg msg[1] = {
> -               {
> -                       .addr = st->client->addr,
> -                       .flags = 0,
> -                       .len = sizeof(buf),
> -                       .buf = buf,
> -               }
> -       };
> -
> -       buf[0] = reg;
> -       buf[1] = d;
> -       ret = __i2c_transfer(st->client->adapter, msg, 1);
> -       if (ret != 1)
> -               return ret;
> -
> -       return 0;
> -}
> -
>  static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id)
>  {
>         struct iio_dev *indio_dev = i2c_mux_priv(muxc);
> @@ -117,8 +88,7 @@ static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id)
>         /* Use the same mutex which was used everywhere to protect power-op */
>         mutex_lock(&indio_dev->mlock);
>         if (!st->powerup_count) {
> -               ret = inv_mpu6050_write_reg_unlocked(st, st->reg->pwr_mgmt_1,
> -                                                    0);
> +               ret = inv_mpu6050_write_reg(st, st->reg->pwr_mgmt_1, 0);
>                 if (ret)
>                         goto write_error;
>
> @@ -126,9 +96,9 @@ static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id)
>         }
>         if (!ret) {
>                 st->powerup_count++;
> -               ret = inv_mpu6050_write_reg_unlocked(st, st->reg->int_pin_cfg,
> -                                                    st->client->irq |
> -                                                    INV_MPU6050_BIT_BYPASS_EN);
> +               ret = inv_mpu6050_write_reg(st, st->reg->int_pin_cfg,
> +                                           st->client->irq |
> +                                           INV_MPU6050_BIT_BYPASS_EN);
>         }
>  write_error:
>         mutex_unlock(&indio_dev->mlock);
> @@ -143,12 +113,11 @@ static int inv_mpu6050_deselect_bypass(struct i2c_mux_core *muxc, u32 chan_id)
>
>         mutex_lock(&indio_dev->mlock);
>         /* It doesn't really mattter, if any of the calls fails */
> -       inv_mpu6050_write_reg_unlocked(st, st->reg->int_pin_cfg,
> -                                      st->client->irq);
> +       inv_mpu6050_write_reg(st, st->reg->int_pin_cfg, st->client->irq);
>         st->powerup_count--;
>         if (!st->powerup_count)
> -               inv_mpu6050_write_reg_unlocked(st, st->reg->pwr_mgmt_1,
> -                                              INV_MPU6050_BIT_SLEEP);
> +               inv_mpu6050_write_reg(st, st->reg->pwr_mgmt_1,
> +                                     INV_MPU6050_BIT_SLEEP);
>         mutex_unlock(&indio_dev->mlock);
>
>         return 0;
> @@ -839,8 +808,8 @@ static int inv_mpu_probe(struct i2c_client *client,
>                 goto out_remove_trigger;
>         }
>
> -       st->muxc = i2c_mux_one_adapter(client->adapter, &client->dev, 0, 0,
> -                                      0, 0, 0,
> +       st->muxc = i2c_mux_one_adapter(client->adapter, &client->dev, 0,
> +                                      I2C_CONTROLLED_MUX, 0, 0, 0,
>                                        inv_mpu6050_select_bypass,
>                                        inv_mpu6050_deselect_bypass);
>         if (IS_ERR(st->muxc)) {
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html