[PATCH v2 00/17] Fixes for the mma9553 driver

[PATCH v2 00/17] Fixes for the mma9553 driver

[Irina Tirdea]

This is a set of follow-up patches for the mma9553 driver that fixes
issues as suggested by Hartmut and Daniel.

Changes in v2:
 - split some of the the v1 patches into smaller patches
as suggested by Jonathan
 - reordered the series to have bug fixes first followed by style fixes
 - use Reported-by for bug fixes
 - added more fixes for issues detected by checkpatch.pl --strict

Irina Tirdea (17):
  iio: accel: mma9553: fix endianness issue when reading status
  iio: accel: mma9553: check input value for activity period
  iio: accel: mma9551_core: prevent buffer overrun
  iio: accel: mma9553: add enable channel for activity
  iio: accel: mma9551_core: wrong doc fixes
  iio: accel: mma9551_core: typo fix in RSC APP ID
  iio: accel: mma9553: check for error in reading initial activity and
    stepcnt
  iio: accel: mma9553: return 0 as indication of success
  iio: accel: mma9553: comment and error message fixes
  iio: accel: mma9553: use GENMASK
  iio: accel: mma9553: prefix naming fixes
  iio: accel: mma9553: fix gpio bitnum init value
  iio: accel: mma9553: refactor mma9553_read_raw
  iio: accel: mma9551_core: use size in words for word buffers
  iio: accel: mma9553: fix alignment issues
  iio: accel: mma9553: document use of mutex
  iio: accel: mma9553: use unsigned counters

 drivers/iio/accel/mma9551_core.c |  70 ++++++-----
 drivers/iio/accel/mma9551_core.h |   8 +-
 drivers/iio/accel/mma9553.c      | 253 ++++++++++++++++++---------------------
 3 files changed, 160 insertions(+), 171 deletions(-)

-- 
1.9.1

[PATCH v2 01/17] iio: accel: mma9553: fix endianness issue when reading status

[Irina Tirdea]

Refactor code for simplicity and clarity.

This also fixes an endianness issue with the original code.
When reading multiple registers, the received buffer of
16-bytes words is little endian (status, step count). On
big endian machines, casting them to u32 would result in
reversed order in the buffer (step count, status) leading
to incorrect values for step count and activity.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Reported-by: Hartmut Knaack <knaack.h@gmx.de>
---
 drivers/iio/accel/mma9553.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
index 2df1af7..607dbfc 100644
--- a/drivers/iio/accel/mma9553.c
+++ b/drivers/iio/accel/mma9553.c
@@ -316,22 +316,19 @@ static int mma9553_set_config(struct mma9553_data *data, u16 reg,
 static int mma9553_read_activity_stepcnt(struct mma9553_data *data,
 					 u8 *activity, u16 *stepcnt)
 {
-	u32 status_stepcnt;
-	u16 status;
+	u16 buf[2];
 	int ret;
 
 	ret = mma9551_read_status_words(data->client, MMA9551_APPID_PEDOMETER,
-					MMA9553_REG_STATUS, sizeof(u32),
-					(u16 *) &status_stepcnt);
+					MMA9553_REG_STATUS, sizeof(u32), buf);
 	if (ret < 0) {
 		dev_err(&data->client->dev,
 			"error reading status and stepcnt\n");
 		return ret;
 	}
 
-	status = status_stepcnt & MMA9553_MASK_CONF_WORD;
-	*activity = mma9553_get_bits(status, MMA9553_MASK_STATUS_ACTIVITY);
-	*stepcnt = status_stepcnt >> 16;
+	*activity = mma9553_get_bits(buf[0], MMA9553_MASK_STATUS_ACTIVITY);
+	*stepcnt = buf[1];
 
 	return 0;
 }
-- 
1.9.1

[PATCH v2 02/17] iio: accel: mma9553: check input value for activity period

[Irina Tirdea]

When setting the activity period, the value introduced by
the user in sysfs is not checked for validity.

Add a boundary check so that only allowed values are
reported as successfully written to device.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Reported-by: Hartmut Knaack <knaack.h@gmx.de>
---
 drivers/iio/accel/mma9553.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
index 607dbfc..03120fb 100644
--- a/drivers/iio/accel/mma9553.c
+++ b/drivers/iio/accel/mma9553.c
@@ -54,6 +54,7 @@
 #define MMA9553_MASK_CONF_STEPCOALESCE		GENMASK(7, 0)
 
 #define MMA9553_REG_CONF_ACTTHD			0x0E
+#define MMA9553_MAX_ACTTHD			GENMASK(15, 0)
 
 /* Pedometer status registers (R-only) */
 #define MMA9553_REG_STATUS			0x00
@@ -869,6 +870,9 @@ static int mma9553_write_event_value(struct iio_dev *indio_dev,
 	case IIO_EV_INFO_PERIOD:
 		switch (chan->type) {
 		case IIO_ACTIVITY:
+			if (val < 0 || val > MMA9553_ACTIVITY_THD_TO_SEC(
+			    MMA9553_MAX_ACTTHD))
+				return -EINVAL;
 			mutex_lock(&data->mutex);
 			ret = mma9553_set_config(data, MMA9553_REG_CONF_ACTTHD,
 						 &data->conf.actthd,
-- 
1.9.1

[PATCH v2 03/17] iio: accel: mma9551_core: prevent buffer overrun

[Irina Tirdea]

The mma9551 functions that read/write word arrays from the
device have a limit for the buffer size given by the device
specifications.

Check that the requested buffer length is within required limits
when transferring word arrays. This will prevent buffer overrun
in the mma9551_read/write_*_words functions and also in the
mma9551_transfer call when writing into the MBOX response/request
structure.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Reported-by: Hartmut Knaack <knaack.h@gmx.de>
---
 drivers/iio/accel/mma9551_core.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/mma9551_core.c b/drivers/iio/accel/mma9551_core.c
index 7f55a6d..c6d5a3a 100644
--- a/drivers/iio/accel/mma9551_core.c
+++ b/drivers/iio/accel/mma9551_core.c
@@ -389,7 +389,12 @@ int mma9551_read_config_words(struct i2c_client *client, u8 app_id,
 {
 	int ret, i;
 	int len_words = len / sizeof(u16);
-	__be16 be_buf[MMA9551_MAX_MAILBOX_DATA_REGS];
+	__be16 be_buf[MMA9551_MAX_MAILBOX_DATA_REGS / 2];
+
+	if (len_words > ARRAY_SIZE(be_buf)) {
+		dev_err(&client->dev, "Invalid buffer size %d\n", len);
+		return -EINVAL;
+	}
 
 	ret = mma9551_transfer(client, app_id, MMA9551_CMD_READ_CONFIG,
 			       reg, NULL, 0, (u8 *) be_buf, len);
@@ -424,7 +429,12 @@ int mma9551_read_status_words(struct i2c_client *client, u8 app_id,
 {
 	int ret, i;
 	int len_words = len / sizeof(u16);
-	__be16 be_buf[MMA9551_MAX_MAILBOX_DATA_REGS];
+	__be16 be_buf[MMA9551_MAX_MAILBOX_DATA_REGS / 2];
+
+	if (len_words > ARRAY_SIZE(be_buf)) {
+		dev_err(&client->dev, "Invalid buffer size %d\n", len);
+		return -EINVAL;
+	}
 
 	ret = mma9551_transfer(client, app_id, MMA9551_CMD_READ_STATUS,
 			       reg, NULL, 0, (u8 *) be_buf, len);
@@ -459,7 +469,12 @@ int mma9551_write_config_words(struct i2c_client *client, u8 app_id,
 {
 	int i;
 	int len_words = len / sizeof(u16);
-	__be16 be_buf[MMA9551_MAX_MAILBOX_DATA_REGS];
+	__be16 be_buf[(MMA9551_MAX_MAILBOX_DATA_REGS - 1) / 2];
+
+	if (len_words > ARRAY_SIZE(be_buf)) {
+		dev_err(&client->dev, "Invalid buffer size %d\n", len);
+		return -EINVAL;
+	}
 
 	for (i = 0; i < len_words; i++)
 		be_buf[i] = cpu_to_be16(buf[i]);
-- 
1.9.1

[PATCH v2 04/17] iio: accel: mma9553: add enable channel for activity

[Irina Tirdea]

Since we need to specifically power on the device if used
in polling mode, we have an enable channel for each type
(step count, distance, speed, calories, etc.).

Add also an enable channel for activity, so it can also
be polled independently of events or other channels.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Reported-by: Daniel Baluta <daniel.baluta@intel.com>
---
 drivers/iio/accel/mma9553.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
index 03120fb..365a109 100644
--- a/drivers/iio/accel/mma9553.c
+++ b/drivers/iio/accel/mma9553.c
@@ -972,7 +972,8 @@ static const struct iio_chan_spec_ext_info mma9553_ext_info[] = {
 	.modified = 1,							\
 	.channel2 = _chan2,						\
 	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),		\
-	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_CALIBHEIGHT),	\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_CALIBHEIGHT) |	\
+				    BIT(IIO_CHAN_INFO_ENABLE),		\
 	.event_spec = mma9553_activity_events,				\
 	.num_event_specs = ARRAY_SIZE(mma9553_activity_events),		\
 	.ext_info = mma9553_ext_info,					\
-- 
1.9.1

[PATCH v2 05/17] iio: accel: mma9551_core: wrong doc fixes

[Irina Tirdea]

Fix docummentation for mma9553_read_* functions.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Reported-by: Hartmut Knaack <knaack.h@gmx.de>
---
 drivers/iio/accel/mma9551_core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/mma9551_core.c b/drivers/iio/accel/mma9551_core.c
index c6d5a3a..efe09a3 100644
--- a/drivers/iio/accel/mma9551_core.c
+++ b/drivers/iio/accel/mma9551_core.c
@@ -374,7 +374,7 @@ EXPORT_SYMBOL(mma9551_read_status_word);
  * @app_id:	Application ID
  * @reg:	Application register
  * @len:	Length of array to read in bytes
- * @val:	Array of words to read
+ * @buf:	Array of words to read
  *
  * Read multiple configuration registers (word-sized registers).
  *
@@ -414,7 +414,7 @@ EXPORT_SYMBOL(mma9551_read_config_words);
  * @app_id:	Application ID
  * @reg:	Application register
  * @len:	Length of array to read in bytes
- * @val:	Array of words to read
+ * @buf:	Array of words to read
  *
  * Read multiple status registers (word-sized registers).
  *
@@ -454,7 +454,7 @@ EXPORT_SYMBOL(mma9551_read_status_words);
  * @app_id:	Application ID
  * @reg:	Application register
  * @len:	Length of array to write in bytes
- * @val:	Array of words to write
+ * @buf:	Array of words to write
  *
  * Write multiple configuration registers (word-sized registers).
  *
-- 
1.9.1

[PATCH v2 06/17] iio: accel: mma9551_core: typo fix in RSC APP ID

[Irina Tirdea]

Fix typo in Reset/Suspend/Clear Application ID definition.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Reported-by: Hartmut Knaack <knaack.h@gmx.de>
---
 drivers/iio/accel/mma9551_core.c | 2 +-
 drivers/iio/accel/mma9551_core.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/accel/mma9551_core.c b/drivers/iio/accel/mma9551_core.c
index efe09a3..2fd2a99 100644
--- a/drivers/iio/accel/mma9551_core.c
+++ b/drivers/iio/accel/mma9551_core.c
@@ -800,7 +800,7 @@ EXPORT_SYMBOL(mma9551_read_accel_scale);
  */
 int mma9551_app_reset(struct i2c_client *client, u32 app_mask)
 {
-	return mma9551_write_config_byte(client, MMA9551_APPID_RCS,
+	return mma9551_write_config_byte(client, MMA9551_APPID_RSC,
 					 MMA9551_RSC_RESET +
 					 MMA9551_RSC_OFFSET(app_mask),
 					 MMA9551_RSC_VAL(app_mask));
diff --git a/drivers/iio/accel/mma9551_core.h b/drivers/iio/accel/mma9551_core.h
index edaa56b..79939e4 100644
--- a/drivers/iio/accel/mma9551_core.h
+++ b/drivers/iio/accel/mma9551_core.h
@@ -22,7 +22,7 @@
 #define MMA9551_APPID_TILT		0x0B
 #define MMA9551_APPID_SLEEP_WAKE	0x12
 #define MMA9551_APPID_PEDOMETER	        0x15
-#define MMA9551_APPID_RCS		0x17
+#define MMA9551_APPID_RSC		0x17
 #define MMA9551_APPID_NONE		0xff
 
 /* Reset/Suspend/Clear application app masks */
-- 
1.9.1

[PATCH v2 07/17] iio: accel: mma9553: check for error in reading initial activity and stepcnt

[Irina Tirdea]

When configuring gpio, we need to read initial values for activity and
step count. This function may fail due to i2c read errors.

Check the error code returned by mma9553_read_activity_stepcnt
and return the appropriate error in gpio config function.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Reported-by: Hartmut Knaack <knaack.h@gmx.de>
---
 drivers/iio/accel/mma9553.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
index 365a109..6f0a14b 100644
--- a/drivers/iio/accel/mma9553.c
+++ b/drivers/iio/accel/mma9553.c
@@ -363,9 +363,12 @@ static int mma9553_conf_gpio(struct mma9553_data *data)
 		return 0;
 
 	/* Save initial values for activity and stepcnt */
-	if (activity_enabled || ev_step_detect->enabled)
-		mma9553_read_activity_stepcnt(data, &data->activity,
-					      &data->stepcnt);
+	if (activity_enabled || ev_step_detect->enabled) {
+		ret = mma9553_read_activity_stepcnt(data, &data->activity,
+						    &data->stepcnt);
+		if (ret < 0)
+			return ret;
+	}
 
 	ret = mma9551_gpio_config(data->client,
 				  MMA9553_DEFAULT_GPIO_PIN,
-- 
1.9.1

[PATCH v2 08/17] iio: accel: mma9553: return 0 as indication of success

[Irina Tirdea]

Use return 0 instead of return ret to mark
clearly the success return path.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Suggested-by: Hartmut Knaack <knaack.h@gmx.de>
---
 drivers/iio/accel/mma9553.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
index 6f0a14b..0f30006 100644
--- a/drivers/iio/accel/mma9553.c
+++ b/drivers/iio/accel/mma9553.c
@@ -792,7 +792,7 @@ static int mma9553_write_event_config(struct iio_dev *indio_dev,
 
 	mutex_unlock(&data->mutex);
 
-	return ret;
+	return 0;
 
 err_conf_gpio:
 	if (state) {
-- 
1.9.1

[PATCH v2 09/17] iio: accel: mma9553: comment and error message fixes

[Irina Tirdea]

Use "GPIO" instead of "gpio" and "ACPI" instead of "acpi".

Includes a couple of small style fixes in comments
(missing full stop, whitespace, paranthesis).

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Suggested-by: Hartmut Knaack <knaack.h@gmx.de>
---
 drivers/iio/accel/mma9553.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
index 0f30006..a6b74de 100644
--- a/drivers/iio/accel/mma9553.c
+++ b/drivers/iio/accel/mma9553.c
@@ -76,14 +76,14 @@
 #define MMA9553_DEFAULT_GPIO_PIN	mma9551_gpio6
 #define MMA9553_DEFAULT_GPIO_POLARITY	0
 
-/* Bitnum used for gpio configuration = bit number in high status byte */
 #define STATUS_TO_BITNUM(bit)		(ffs(bit) - 9)
+/* Bitnum used for GPIO configuration = bit number in high status byte */
 
 #define MMA9553_DEFAULT_SAMPLE_RATE	30	/* Hz */
 
 /*
  * The internal activity level must be stable for ACTTHD samples before
- * ACTIVITY is updated.The ACTIVITY variable contains the current activity
+ * ACTIVITY is updated. The ACTIVITY variable contains the current activity
  * level and is updated every time a step is detected or once a second
  * if there are no steps.
  */
@@ -399,13 +399,13 @@ static int mma9553_init(struct mma9553_data *data)
 				      sizeof(data->conf), (u16 *) &data->conf);
 	if (ret < 0) {
 		dev_err(&data->client->dev,
-			"device is not MMA9553L: failed to read cfg regs\n");
+			"failed to read configuration registers\n");
 		return ret;
 	}
 
 
-	/* Reset gpio */
 	data->gpio_bitnum = -1;
+	/* Reset GPIO */
 	ret = mma9553_conf_gpio(data);
 	if (ret < 0)
 		return ret;
@@ -457,7 +457,8 @@ static int mma9553_read_raw(struct iio_dev *indio_dev,
 			 * The HW only counts steps and other dependent
 			 * parameters (speed, distance, calories, activity)
 			 * if power is on (from enabling an event or the
-			 * step counter */
+			 * step counter).
+			 */
 			powered_on =
 			    mma9553_is_any_event_enabled(data, false, 0) ||
 			    data->stepcnt_enabled;
@@ -900,7 +901,7 @@ static int mma9553_get_calibgender_mode(struct iio_dev *indio_dev,
 	gender = mma9553_get_bits(data->conf.filter, MMA9553_MASK_CONF_MALE);
 	/*
 	 * HW expects 0 for female and 1 for male,
-	 * while iio index is 0 for male and 1 for female
+	 * while iio index is 0 for male and 1 for female.
 	 */
 	return !gender;
 }
@@ -1113,16 +1114,16 @@ static int mma9553_gpio_probe(struct i2c_client *client)
 
 	dev = &client->dev;
 
-	/* data ready gpio interrupt pin */
+	/* data ready GPIO interrupt pin */
 	gpio = devm_gpiod_get_index(dev, MMA9553_GPIO_NAME, 0, GPIOD_IN);
 	if (IS_ERR(gpio)) {
-		dev_err(dev, "acpi gpio get index failed\n");
+		dev_err(dev, "ACPI GPIO get index failed\n");
 		return PTR_ERR(gpio);
 	}
 
 	ret = gpiod_to_irq(gpio);
 
-	dev_dbg(dev, "gpio resource, no:%d irq:%d\n", desc_to_gpio(gpio), ret);
+	dev_dbg(dev, "GPIO resource, no:%d irq:%d\n", desc_to_gpio(gpio), ret);
 
 	return ret;
 }
-- 
1.9.1

[PATCH v2 10/17] iio: accel: mma9553: use GENMASK

[Irina Tirdea]

Use GENMASK instead of BIT or direct value to
define a mask.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Suggested-by: Hartmut Knaack <knaack.h@gmx.de>
---
 drivers/iio/accel/mma9553.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
index a6b74de..47cb14b 100644
--- a/drivers/iio/accel/mma9553.c
+++ b/drivers/iio/accel/mma9553.c
@@ -63,8 +63,8 @@
 #define MMA9553_MASK_STATUS_STEPCHG		BIT(13)
 #define MMA9553_MASK_STATUS_ACTCHG		BIT(12)
 #define MMA9553_MASK_STATUS_SUSP		BIT(11)
-#define MMA9553_MASK_STATUS_ACTIVITY		(BIT(10) | BIT(9) | BIT(8))
-#define MMA9553_MASK_STATUS_VERSION		0x00FF
+#define MMA9553_MASK_STATUS_ACTIVITY		GENMASK(10, 8)
+#define MMA9553_MASK_STATUS_VERSION		GENMASK(7, 0)
 
 #define MMA9553_REG_STEPCNT			0x02
 #define MMA9553_REG_DISTANCE			0x04
-- 
1.9.1

[PATCH v2 11/17] iio: accel: mma9553: prefix naming fixes

[Irina Tirdea]

Add mma9553_ prefix to all local functions/declarations.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Suggested-by: Hartmut Knaack <knaack.h@gmx.de>
---
 drivers/iio/accel/mma9553.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
index 47cb14b..9573147 100644
--- a/drivers/iio/accel/mma9553.c
+++ b/drivers/iio/accel/mma9553.c
@@ -76,8 +76,8 @@
 #define MMA9553_DEFAULT_GPIO_PIN	mma9551_gpio6
 #define MMA9553_DEFAULT_GPIO_POLARITY	0
 
-#define STATUS_TO_BITNUM(bit)		(ffs(bit) - 9)
 /* Bitnum used for GPIO configuration = bit number in high status byte */
+#define MMA9553_STATUS_TO_BITNUM(bit)	(ffs(bit) - 9)
 
 #define MMA9553_DEFAULT_SAMPLE_RATE	30	/* Hz */
 
@@ -351,11 +351,11 @@ static int mma9553_conf_gpio(struct mma9553_data *data)
 	 * This bit is the logical OR of the SUSPCHG, STEPCHG, and ACTCHG flags.
 	 */
 	if (activity_enabled && ev_step_detect->enabled)
-		bitnum = STATUS_TO_BITNUM(MMA9553_MASK_STATUS_MRGFL);
+		bitnum = MMA9553_STATUS_TO_BITNUM(MMA9553_MASK_STATUS_MRGFL);
 	else if (ev_step_detect->enabled)
-		bitnum = STATUS_TO_BITNUM(MMA9553_MASK_STATUS_STEPCHG);
+		bitnum = MMA9553_STATUS_TO_BITNUM(MMA9553_MASK_STATUS_STEPCHG);
 	else if (activity_enabled)
-		bitnum = STATUS_TO_BITNUM(MMA9553_MASK_STATUS_ACTCHG);
+		bitnum = MMA9553_STATUS_TO_BITNUM(MMA9553_MASK_STATUS_ACTCHG);
 	else			/* Reset */
 		appid = MMA9551_APPID_NONE;
 
@@ -948,11 +948,11 @@ static const struct iio_event_spec mma9553_activity_events[] = {
 	},
 };
 
-static const char * const calibgender_modes[] = { "male", "female" };
+static const char * const mma9553_calibgender_modes[] = { "male", "female" };
 
 static const struct iio_enum mma9553_calibgender_enum = {
-	.items = calibgender_modes,
-	.num_items = ARRAY_SIZE(calibgender_modes),
+	.items = mma9553_calibgender_modes,
+	.num_items = ARRAY_SIZE(mma9553_calibgender_modes),
 	.get = mma9553_get_calibgender_mode,
 	.set = mma9553_set_calibgender_mode,
 };
-- 
1.9.1

[PATCH v2 12/17] iio: accel: mma9553: fix gpio bitnum init value

[Irina Tirdea]

Initial value of gpio bitnum is set to -1, but
the variable is declared as unsigned.

Use a positive invalid value for initial gpio
bitnum.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Suggested-by: Hartmut Knaack <knaack.h@gmx.de>
---
 drivers/iio/accel/mma9553.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
index 9573147..5755977 100644
--- a/drivers/iio/accel/mma9553.c
+++ b/drivers/iio/accel/mma9553.c
@@ -78,6 +78,7 @@
 
 /* Bitnum used for GPIO configuration = bit number in high status byte */
 #define MMA9553_STATUS_TO_BITNUM(bit)	(ffs(bit) - 9)
+#define MMA9553_MAX_BITNUM		MMA9553_STATUS_TO_BITNUM(BIT(16))
 
 #define MMA9553_DEFAULT_SAMPLE_RATE	30	/* Hz */
 
@@ -404,8 +405,8 @@ static int mma9553_init(struct mma9553_data *data)
 	}
 
 
-	data->gpio_bitnum = -1;
 	/* Reset GPIO */
+	data->gpio_bitnum = MMA9553_MAX_BITNUM;
 	ret = mma9553_conf_gpio(data);
 	if (ret < 0)
 		return ret;
-- 
1.9.1

[PATCH v2 13/17] iio: accel: mma9553: refactor mma9553_read_raw

[Irina Tirdea]

Refactor code for simplicity and clarity.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Suggested-by: Hartmut Knaack <knaack.h@gmx.de>
---
 drivers/iio/accel/mma9553.c | 101 +++++++++++++++-----------------------------
 1 file changed, 33 insertions(+), 68 deletions(-)

diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
index 5755977..8bfc618 100644
--- a/drivers/iio/accel/mma9553.c
+++ b/drivers/iio/accel/mma9553.c
@@ -440,6 +440,32 @@ static int mma9553_init(struct mma9553_data *data)
 	return mma9551_set_device_state(data->client, true);
 }
 
+static int mma9553_read_status_word(struct mma9553_data *data, u16 reg,
+				    u16 *tmp)
+{
+	bool powered_on;
+	int ret;
+
+	/*
+	 * The HW only counts steps and other dependent
+	 * parameters (speed, distance, calories, activity)
+	 * if power is on (from enabling an event or the
+	 * step counter).
+	 */
+	powered_on = mma9553_is_any_event_enabled(data, false, 0) ||
+		     data->stepcnt_enabled;
+	if (!powered_on) {
+		dev_err(&data->client->dev, "No channels enabled\n");
+		return -EINVAL;
+	}
+
+	mutex_lock(&data->mutex);
+	ret = mma9551_read_status_word(data->client, MMA9551_APPID_PEDOMETER,
+				       reg, tmp);
+	mutex_unlock(&data->mutex);
+	return ret;
+}
+
 static int mma9553_read_raw(struct iio_dev *indio_dev,
 			    struct iio_chan_spec const *chan,
 			    int *val, int *val2, long mask)
@@ -448,70 +474,30 @@ static int mma9553_read_raw(struct iio_dev *indio_dev,
 	int ret;
 	u16 tmp;
 	u8 activity;
-	bool powered_on;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_PROCESSED:
 		switch (chan->type) {
 		case IIO_STEPS:
-			/*
-			 * The HW only counts steps and other dependent
-			 * parameters (speed, distance, calories, activity)
-			 * if power is on (from enabling an event or the
-			 * step counter).
-			 */
-			powered_on =
-			    mma9553_is_any_event_enabled(data, false, 0) ||
-			    data->stepcnt_enabled;
-			if (!powered_on) {
-				dev_err(&data->client->dev,
-					"No channels enabled\n");
-				return -EINVAL;
-			}
-			mutex_lock(&data->mutex);
-			ret = mma9551_read_status_word(data->client,
-						       MMA9551_APPID_PEDOMETER,
+			ret = mma9553_read_status_word(data,
 						       MMA9553_REG_STEPCNT,
 						       &tmp);
-			mutex_unlock(&data->mutex);
 			if (ret < 0)
 				return ret;
 			*val = tmp;
 			return IIO_VAL_INT;
 		case IIO_DISTANCE:
-			powered_on =
-			    mma9553_is_any_event_enabled(data, false, 0) ||
-			    data->stepcnt_enabled;
-			if (!powered_on) {
-				dev_err(&data->client->dev,
-					"No channels enabled\n");
-				return -EINVAL;
-			}
-			mutex_lock(&data->mutex);
-			ret = mma9551_read_status_word(data->client,
-						       MMA9551_APPID_PEDOMETER,
+			ret = mma9553_read_status_word(data,
 						       MMA9553_REG_DISTANCE,
 						       &tmp);
-			mutex_unlock(&data->mutex);
 			if (ret < 0)
 				return ret;
 			*val = tmp;
 			return IIO_VAL_INT;
 		case IIO_ACTIVITY:
-			powered_on =
-			    mma9553_is_any_event_enabled(data, false, 0) ||
-			    data->stepcnt_enabled;
-			if (!powered_on) {
-				dev_err(&data->client->dev,
-					"No channels enabled\n");
-				return -EINVAL;
-			}
-			mutex_lock(&data->mutex);
-			ret = mma9551_read_status_word(data->client,
-						       MMA9551_APPID_PEDOMETER,
+			ret = mma9553_read_status_word(data,
 						       MMA9553_REG_STATUS,
 						       &tmp);
-			mutex_unlock(&data->mutex);
 			if (ret < 0)
 				return ret;
 
@@ -536,38 +522,17 @@ static int mma9553_read_raw(struct iio_dev *indio_dev,
 		case IIO_VELOCITY:	/* m/h */
 			if (chan->channel2 != IIO_MOD_ROOT_SUM_SQUARED_X_Y_Z)
 				return -EINVAL;
-			powered_on =
-			    mma9553_is_any_event_enabled(data, false, 0) ||
-			    data->stepcnt_enabled;
-			if (!powered_on) {
-				dev_err(&data->client->dev,
-					"No channels enabled\n");
-				return -EINVAL;
-			}
-			mutex_lock(&data->mutex);
-			ret = mma9551_read_status_word(data->client,
-						       MMA9551_APPID_PEDOMETER,
-						       MMA9553_REG_SPEED, &tmp);
-			mutex_unlock(&data->mutex);
+			ret = mma9553_read_status_word(data,
+						       MMA9553_REG_SPEED,
+						       &tmp);
 			if (ret < 0)
 				return ret;
 			*val = tmp;
 			return IIO_VAL_INT;
 		case IIO_ENERGY:	/* Cal or kcal */
-			powered_on =
-			    mma9553_is_any_event_enabled(data, false, 0) ||
-			    data->stepcnt_enabled;
-			if (!powered_on) {
-				dev_err(&data->client->dev,
-					"No channels enabled\n");
-				return -EINVAL;
-			}
-			mutex_lock(&data->mutex);
-			ret = mma9551_read_status_word(data->client,
-						       MMA9551_APPID_PEDOMETER,
+			ret = mma9553_read_status_word(data,
 						       MMA9553_REG_CALORIES,
 						       &tmp);
-			mutex_unlock(&data->mutex);
 			if (ret < 0)
 				return ret;
 			*val = tmp;
-- 
1.9.1

[PATCH v2 14/17] iio: accel: mma9551_core: use size in words for word buffers

[Irina Tirdea]

Change the prototype for the mma9551_read/write_*_words functions
to receive the length of the buffer in words (instead of bytes) since
we are using a word buffer. This will prevent users from sending an
odd number of bytes for a word array.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
---
 drivers/iio/accel/mma9551_core.c | 27 ++++++++++++---------------
 drivers/iio/accel/mma9553.c      |  9 ++++++---
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/iio/accel/mma9551_core.c b/drivers/iio/accel/mma9551_core.c
index 2fd2a99..583660b 100644
--- a/drivers/iio/accel/mma9551_core.c
+++ b/drivers/iio/accel/mma9551_core.c
@@ -373,7 +373,7 @@ EXPORT_SYMBOL(mma9551_read_status_word);
  * @client:	I2C client
  * @app_id:	Application ID
  * @reg:	Application register
- * @len:	Length of array to read in bytes
+ * @len:	Length of array to read (in words)
  * @buf:	Array of words to read
  *
  * Read multiple configuration registers (word-sized registers).
@@ -388,20 +388,19 @@ int mma9551_read_config_words(struct i2c_client *client, u8 app_id,
 			     u16 reg, u8 len, u16 *buf)
 {
 	int ret, i;
-	int len_words = len / sizeof(u16);
 	__be16 be_buf[MMA9551_MAX_MAILBOX_DATA_REGS / 2];
 
-	if (len_words > ARRAY_SIZE(be_buf)) {
+	if (len > ARRAY_SIZE(be_buf)) {
 		dev_err(&client->dev, "Invalid buffer size %d\n", len);
 		return -EINVAL;
 	}
 
 	ret = mma9551_transfer(client, app_id, MMA9551_CMD_READ_CONFIG,
-			       reg, NULL, 0, (u8 *) be_buf, len);
+			       reg, NULL, 0, (u8 *)be_buf, len * sizeof(u16));
 	if (ret < 0)
 		return ret;
 
-	for (i = 0; i < len_words; i++)
+	for (i = 0; i < len; i++)
 		buf[i] = be16_to_cpu(be_buf[i]);
 
 	return 0;
@@ -413,7 +412,7 @@ EXPORT_SYMBOL(mma9551_read_config_words);
  * @client:	I2C client
  * @app_id:	Application ID
  * @reg:	Application register
- * @len:	Length of array to read in bytes
+ * @len:	Length of array to read (in words)
  * @buf:	Array of words to read
  *
  * Read multiple status registers (word-sized registers).
@@ -428,20 +427,19 @@ int mma9551_read_status_words(struct i2c_client *client, u8 app_id,
 			      u16 reg, u8 len, u16 *buf)
 {
 	int ret, i;
-	int len_words = len / sizeof(u16);
 	__be16 be_buf[MMA9551_MAX_MAILBOX_DATA_REGS / 2];
 
-	if (len_words > ARRAY_SIZE(be_buf)) {
+	if (len > ARRAY_SIZE(be_buf)) {
 		dev_err(&client->dev, "Invalid buffer size %d\n", len);
 		return -EINVAL;
 	}
 
 	ret = mma9551_transfer(client, app_id, MMA9551_CMD_READ_STATUS,
-			       reg, NULL, 0, (u8 *) be_buf, len);
+			       reg, NULL, 0, (u8 *)be_buf, len * sizeof(u16));
 	if (ret < 0)
 		return ret;
 
-	for (i = 0; i < len_words; i++)
+	for (i = 0; i < len; i++)
 		buf[i] = be16_to_cpu(be_buf[i]);
 
 	return 0;
@@ -453,7 +451,7 @@ EXPORT_SYMBOL(mma9551_read_status_words);
  * @client:	I2C client
  * @app_id:	Application ID
  * @reg:	Application register
- * @len:	Length of array to write in bytes
+ * @len:	Length of array to write (in words)
  * @buf:	Array of words to write
  *
  * Write multiple configuration registers (word-sized registers).
@@ -468,19 +466,18 @@ int mma9551_write_config_words(struct i2c_client *client, u8 app_id,
 			       u16 reg, u8 len, u16 *buf)
 {
 	int i;
-	int len_words = len / sizeof(u16);
 	__be16 be_buf[(MMA9551_MAX_MAILBOX_DATA_REGS - 1) / 2];
 
-	if (len_words > ARRAY_SIZE(be_buf)) {
+	if (len > ARRAY_SIZE(be_buf)) {
 		dev_err(&client->dev, "Invalid buffer size %d\n", len);
 		return -EINVAL;
 	}
 
-	for (i = 0; i < len_words; i++)
+	for (i = 0; i < len; i++)
 		be_buf[i] = cpu_to_be16(buf[i]);
 
 	return mma9551_transfer(client, app_id, MMA9551_CMD_WRITE_CONFIG,
-				reg, (u8 *) be_buf, len, NULL, 0);
+				reg, (u8 *)be_buf, len * sizeof(u16), NULL, 0);
 }
 EXPORT_SYMBOL(mma9551_write_config_words);
 
diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
index 8bfc618..06c8707 100644
--- a/drivers/iio/accel/mma9553.c
+++ b/drivers/iio/accel/mma9553.c
@@ -322,7 +322,8 @@ static int mma9553_read_activity_stepcnt(struct mma9553_data *data,
 	int ret;
 
 	ret = mma9551_read_status_words(data->client, MMA9551_APPID_PEDOMETER,
-					MMA9553_REG_STATUS, sizeof(u32), buf);
+					MMA9553_REG_STATUS, ARRAY_SIZE(buf),
+					buf);
 	if (ret < 0) {
 		dev_err(&data->client->dev,
 			"error reading status and stepcnt\n");
@@ -397,7 +398,8 @@ static int mma9553_init(struct mma9553_data *data)
 	ret =
 	    mma9551_read_config_words(data->client, MMA9551_APPID_PEDOMETER,
 				      MMA9553_REG_CONF_SLEEPMIN,
-				      sizeof(data->conf), (u16 *) &data->conf);
+				      sizeof(data->conf) / sizeof(u16),
+				      (u16 *)&data->conf);
 	if (ret < 0) {
 		dev_err(&data->client->dev,
 			"failed to read configuration registers\n");
@@ -430,7 +432,8 @@ static int mma9553_init(struct mma9553_data *data)
 	ret =
 	    mma9551_write_config_words(data->client, MMA9551_APPID_PEDOMETER,
 				       MMA9553_REG_CONF_SLEEPMIN,
-				       sizeof(data->conf), (u16 *) &data->conf);
+				       sizeof(data->conf) / sizeof(u16),
+				       (u16 *)&data->conf);
 	if (ret < 0) {
 		dev_err(&data->client->dev,
 			"failed to write configuration registers\n");
-- 
1.9.1

[PATCH v2 15/17] iio: accel: mma9553: fix alignment issues

[Irina Tirdea]

Fix code alignment and wrap parameters.
Fix issues reported by checkpatch.pl --strict.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Suggested-by: Hartmut Knaack <knaack.h@gmx.de>
---
 drivers/iio/accel/mma9551_core.c |  8 ++---
 drivers/iio/accel/mma9551_core.h |  6 ++--
 drivers/iio/accel/mma9553.c      | 76 +++++++++++++++++++---------------------
 3 files changed, 43 insertions(+), 47 deletions(-)

diff --git a/drivers/iio/accel/mma9551_core.c b/drivers/iio/accel/mma9551_core.c
index 583660b..c34c5ce 100644
--- a/drivers/iio/accel/mma9551_core.c
+++ b/drivers/iio/accel/mma9551_core.c
@@ -297,7 +297,7 @@ EXPORT_SYMBOL(mma9551_read_status_byte);
  * Returns: 0 on success, negative value on failure.
  */
 int mma9551_read_config_word(struct i2c_client *client, u8 app_id,
-			    u16 reg, u16 *val)
+			     u16 reg, u16 *val)
 {
 	int ret;
 	__be16 v;
@@ -328,12 +328,12 @@ EXPORT_SYMBOL(mma9551_read_config_word);
  * Returns: 0 on success, negative value on failure.
  */
 int mma9551_write_config_word(struct i2c_client *client, u8 app_id,
-			     u16 reg, u16 val)
+			      u16 reg, u16 val)
 {
 	__be16 v = cpu_to_be16(val);
 
 	return mma9551_transfer(client, app_id, MMA9551_CMD_WRITE_CONFIG, reg,
-				(u8 *) &v, 2, NULL, 0);
+				(u8 *)&v, 2, NULL, 0);
 }
 EXPORT_SYMBOL(mma9551_write_config_word);
 
@@ -385,7 +385,7 @@ EXPORT_SYMBOL(mma9551_read_status_word);
  * Returns: 0 on success, negative value on failure.
  */
 int mma9551_read_config_words(struct i2c_client *client, u8 app_id,
-			     u16 reg, u8 len, u16 *buf)
+			      u16 reg, u8 len, u16 *buf)
 {
 	int ret, i;
 	__be16 be_buf[MMA9551_MAX_MAILBOX_DATA_REGS / 2];
diff --git a/drivers/iio/accel/mma9551_core.h b/drivers/iio/accel/mma9551_core.h
index 79939e4..5e88e64 100644
--- a/drivers/iio/accel/mma9551_core.h
+++ b/drivers/iio/accel/mma9551_core.h
@@ -53,13 +53,13 @@ int mma9551_write_config_byte(struct i2c_client *client, u8 app_id,
 int mma9551_read_status_byte(struct i2c_client *client, u8 app_id,
 			     u16 reg, u8 *val);
 int mma9551_read_config_word(struct i2c_client *client, u8 app_id,
-			    u16 reg, u16 *val);
+			     u16 reg, u16 *val);
 int mma9551_write_config_word(struct i2c_client *client, u8 app_id,
-			     u16 reg, u16 val);
+			      u16 reg, u16 val);
 int mma9551_read_status_word(struct i2c_client *client, u8 app_id,
 			     u16 reg, u16 *val);
 int mma9551_read_config_words(struct i2c_client *client, u8 app_id,
-			     u16 reg, u8 len, u16 *buf);
+			      u16 reg, u8 len, u16 *buf);
 int mma9551_read_status_words(struct i2c_client *client, u8 app_id,
 			      u16 reg, u8 len, u16 *buf);
 int mma9551_write_config_words(struct i2c_client *client, u8 app_id,
diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
index 06c8707..08f28c3 100644
--- a/drivers/iio/accel/mma9553.c
+++ b/drivers/iio/accel/mma9553.c
@@ -343,10 +343,10 @@ static int mma9553_conf_gpio(struct mma9553_data *data)
 	struct mma9553_event *ev_step_detect;
 	bool activity_enabled;
 
-	activity_enabled =
-	    mma9553_is_any_event_enabled(data, true, IIO_ACTIVITY);
-	ev_step_detect =
-	    mma9553_get_event(data, IIO_STEPS, IIO_NO_MOD, IIO_EV_DIR_NONE);
+	activity_enabled = mma9553_is_any_event_enabled(data, true,
+							IIO_ACTIVITY);
+	ev_step_detect = mma9553_get_event(data, IIO_STEPS, IIO_NO_MOD,
+					   IIO_EV_DIR_NONE);
 
 	/*
 	 * If both step detector and activity are enabled, use the MRGFL bit.
@@ -372,9 +372,8 @@ static int mma9553_conf_gpio(struct mma9553_data *data)
 			return ret;
 	}
 
-	ret = mma9551_gpio_config(data->client,
-				  MMA9553_DEFAULT_GPIO_PIN,
-				  appid, bitnum, MMA9553_DEFAULT_GPIO_POLARITY);
+	ret = mma9551_gpio_config(data->client, MMA9553_DEFAULT_GPIO_PIN, appid,
+				  bitnum, MMA9553_DEFAULT_GPIO_POLARITY);
 	if (ret < 0)
 		return ret;
 	data->gpio_bitnum = bitnum;
@@ -395,18 +394,16 @@ static int mma9553_init(struct mma9553_data *data)
 	 * a device identification command to differentiate the MMA9553L
 	 * from the MMA9550L.
 	 */
-	ret =
-	    mma9551_read_config_words(data->client, MMA9551_APPID_PEDOMETER,
-				      MMA9553_REG_CONF_SLEEPMIN,
-				      sizeof(data->conf) / sizeof(u16),
-				      (u16 *)&data->conf);
+	ret = mma9551_read_config_words(data->client, MMA9551_APPID_PEDOMETER,
+					MMA9553_REG_CONF_SLEEPMIN,
+					sizeof(data->conf) / sizeof(u16),
+					(u16 *)&data->conf);
 	if (ret < 0) {
 		dev_err(&data->client->dev,
 			"failed to read configuration registers\n");
 		return ret;
 	}
 
-
 	/* Reset GPIO */
 	data->gpio_bitnum = MMA9553_MAX_BITNUM;
 	ret = mma9553_conf_gpio(data);
@@ -421,19 +418,18 @@ static int mma9553_init(struct mma9553_data *data)
 	data->conf.sleepmin = MMA9553_DEFAULT_SLEEPMIN;
 	data->conf.sleepmax = MMA9553_DEFAULT_SLEEPMAX;
 	data->conf.sleepthd = MMA9553_DEFAULT_SLEEPTHD;
-	data->conf.config =
-	    mma9553_set_bits(data->conf.config, 1, MMA9553_MASK_CONF_CONFIG);
+	data->conf.config = mma9553_set_bits(data->conf.config, 1,
+					     MMA9553_MASK_CONF_CONFIG);
 	/*
 	 * Clear the activity debounce counter when the activity level changes,
 	 * so that the confidence level applies for any activity level.
 	 */
 	data->conf.config = mma9553_set_bits(data->conf.config, 1,
 					     MMA9553_MASK_CONF_ACT_DBCNTM);
-	ret =
-	    mma9551_write_config_words(data->client, MMA9551_APPID_PEDOMETER,
-				       MMA9553_REG_CONF_SLEEPMIN,
-				       sizeof(data->conf) / sizeof(u16),
-				       (u16 *)&data->conf);
+	ret = mma9551_write_config_words(data->client, MMA9551_APPID_PEDOMETER,
+					 MMA9553_REG_CONF_SLEEPMIN,
+					 sizeof(data->conf) / sizeof(u16),
+					 (u16 *)&data->conf);
 	if (ret < 0) {
 		dev_err(&data->client->dev,
 			"failed to write configuration registers\n");
@@ -570,7 +566,7 @@ static int mma9553_read_raw(struct iio_dev *indio_dev,
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_CALIBHEIGHT:
 		tmp = mma9553_get_bits(data->conf.height_weight,
-					MMA9553_MASK_CONF_HEIGHT);
+				       MMA9553_MASK_CONF_HEIGHT);
 		*val = tmp / 100;	/* cm to m */
 		*val2 = (tmp % 100) * 10000;
 		return IIO_VAL_INT_PLUS_MICRO;
@@ -722,7 +718,6 @@ static int mma9553_read_event_config(struct iio_dev *indio_dev,
 				     enum iio_event_type type,
 				     enum iio_event_direction dir)
 {
-
 	struct mma9553_data *data = iio_priv(indio_dev);
 	struct mma9553_event *event;
 
@@ -1029,22 +1024,22 @@ static irqreturn_t mma9553_event_handler(int irq, void *private)
 		return IRQ_HANDLED;
 	}
 
-	ev_prev_activity =
-	    mma9553_get_event(data, IIO_ACTIVITY,
-			      mma9553_activity_to_mod(data->activity),
-			      IIO_EV_DIR_FALLING);
-	ev_activity =
-	    mma9553_get_event(data, IIO_ACTIVITY,
-			      mma9553_activity_to_mod(activity),
-			      IIO_EV_DIR_RISING);
-	ev_step_detect =
-	    mma9553_get_event(data, IIO_STEPS, IIO_NO_MOD, IIO_EV_DIR_NONE);
+	ev_prev_activity = mma9553_get_event(data, IIO_ACTIVITY,
+					     mma9553_activity_to_mod(
+					     data->activity),
+					     IIO_EV_DIR_FALLING);
+	ev_activity = mma9553_get_event(data, IIO_ACTIVITY,
+					mma9553_activity_to_mod(activity),
+					IIO_EV_DIR_RISING);
+	ev_step_detect = mma9553_get_event(data, IIO_STEPS, IIO_NO_MOD,
+					   IIO_EV_DIR_NONE);
 
 	if (ev_step_detect->enabled && (stepcnt != data->stepcnt)) {
 		data->stepcnt = stepcnt;
 		iio_push_event(indio_dev,
 			       IIO_EVENT_CODE(IIO_STEPS, 0, IIO_NO_MOD,
-			       IIO_EV_DIR_NONE, IIO_EV_TYPE_CHANGE, 0, 0, 0),
+					      IIO_EV_DIR_NONE,
+					      IIO_EV_TYPE_CHANGE, 0, 0, 0),
 			       data->timestamp);
 	}
 
@@ -1054,17 +1049,19 @@ static irqreturn_t mma9553_event_handler(int irq, void *private)
 		if (ev_prev_activity && ev_prev_activity->enabled)
 			iio_push_event(indio_dev,
 				       IIO_EVENT_CODE(IIO_ACTIVITY, 0,
-				       ev_prev_activity->info->mod,
-				       IIO_EV_DIR_FALLING,
-				       IIO_EV_TYPE_THRESH, 0, 0, 0),
+						    ev_prev_activity->info->mod,
+						    IIO_EV_DIR_FALLING,
+						    IIO_EV_TYPE_THRESH, 0, 0,
+						    0),
 				       data->timestamp);
 
 		if (ev_activity && ev_activity->enabled)
 			iio_push_event(indio_dev,
 				       IIO_EVENT_CODE(IIO_ACTIVITY, 0,
-				       ev_activity->info->mod,
-				       IIO_EV_DIR_RISING,
-				       IIO_EV_TYPE_THRESH, 0, 0, 0),
+						      ev_activity->info->mod,
+						      IIO_EV_DIR_RISING,
+						      IIO_EV_TYPE_THRESH, 0, 0,
+						      0),
 				       data->timestamp);
 	}
 	mutex_unlock(&data->mutex);
@@ -1159,7 +1156,6 @@ static int mma9553_probe(struct i2c_client *client,
 				client->irq);
 			goto out_poweroff;
 		}
-
 	}
 
 	ret = iio_device_register(indio_dev);
-- 
1.9.1

[PATCH v2 16/17] iio: accel: mma9553: document use of mutex

[Irina Tirdea]

Fix checkpatch.pl --strict check:
CHECK: struct mutex definition without comment
+     struct mutex mutex;

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
---
 drivers/iio/accel/mma9553.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
index 08f28c3..a605280 100644
--- a/drivers/iio/accel/mma9553.c
+++ b/drivers/iio/accel/mma9553.c
@@ -182,6 +182,10 @@ struct mma9553_conf_regs {
 
 struct mma9553_data {
 	struct i2c_client *client;
+	/*
+	 * 1. Serialize access to HW (requested by mma9551_core API).
+	 * 2. Serialize sequences that power on/off the device and access HW.
+	 */
 	struct mutex mutex;
 	struct mma9553_conf_regs conf;
 	struct mma9553_event events[MMA9553_EVENTS_INFO_SIZE];
-- 
1.9.1

[PATCH v2 17/17] iio: accel: mma9553: use unsigned counters

[Irina Tirdea]

Use unsigned counters instead of signed when all the
possible values are positive.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Suggested-by: Hartmut Knaack <knaack.h@gmx.de>
---
 drivers/iio/accel/mma9551_core.c | 12 +++++++-----
 drivers/iio/accel/mma9553.c      |  8 ++++----
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/accel/mma9551_core.c b/drivers/iio/accel/mma9551_core.c
index c34c5ce..44b8243 100644
--- a/drivers/iio/accel/mma9551_core.c
+++ b/drivers/iio/accel/mma9551_core.c
@@ -115,8 +115,8 @@ struct mma9551_version_info {
 
 static int mma9551_transfer(struct i2c_client *client,
 			    u8 app_id, u8 command, u16 offset,
-			    u8 *inbytes, int num_inbytes,
-			    u8 *outbytes, int num_outbytes)
+			    u8 *inbytes, u8 num_inbytes,
+			    u8 *outbytes, u8 num_outbytes)
 {
 	struct mma9551_mbox_request req;
 	struct mma9551_mbox_response rsp;
@@ -387,7 +387,8 @@ EXPORT_SYMBOL(mma9551_read_status_word);
 int mma9551_read_config_words(struct i2c_client *client, u8 app_id,
 			      u16 reg, u8 len, u16 *buf)
 {
-	int ret, i;
+	int ret;
+	u8 i;
 	__be16 be_buf[MMA9551_MAX_MAILBOX_DATA_REGS / 2];
 
 	if (len > ARRAY_SIZE(be_buf)) {
@@ -426,7 +427,8 @@ EXPORT_SYMBOL(mma9551_read_config_words);
 int mma9551_read_status_words(struct i2c_client *client, u8 app_id,
 			      u16 reg, u8 len, u16 *buf)
 {
-	int ret, i;
+	int ret;
+	u8 i;
 	__be16 be_buf[MMA9551_MAX_MAILBOX_DATA_REGS / 2];
 
 	if (len > ARRAY_SIZE(be_buf)) {
@@ -465,7 +467,7 @@ EXPORT_SYMBOL(mma9551_read_status_words);
 int mma9551_write_config_words(struct i2c_client *client, u8 app_id,
 			       u16 reg, u8 len, u16 *buf)
 {
-	int i;
+	u8 i;
 	__be16 be_buf[(MMA9551_MAX_MAILBOX_DATA_REGS - 1) / 2];
 
 	if (len > ARRAY_SIZE(be_buf)) {
diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
index a605280..ad587ec 100644
--- a/drivers/iio/accel/mma9553.c
+++ b/drivers/iio/accel/mma9553.c
@@ -189,7 +189,7 @@ struct mma9553_data {
 	struct mutex mutex;
 	struct mma9553_conf_regs conf;
 	struct mma9553_event events[MMA9553_EVENTS_INFO_SIZE];
-	int num_events;
+	u8 num_events;
 	u8 gpio_bitnum;
 	/*
 	 * This is used for all features that depend on step count:
@@ -230,7 +230,7 @@ static enum iio_modifier mma9553_activity_to_mod(enum activity_level activity)
 
 static void mma9553_init_events(struct mma9553_data *data)
 {
-	int i;
+	u8 i;
 
 	data->num_events = MMA9553_EVENTS_INFO_SIZE;
 	for (i = 0; i < data->num_events; i++) {
@@ -244,7 +244,7 @@ static struct mma9553_event *mma9553_get_event(struct mma9553_data *data,
 					       enum iio_modifier mod,
 					       enum iio_event_direction dir)
 {
-	int i;
+	u8 i;
 
 	for (i = 0; i < data->num_events; i++)
 		if (data->events[i].info->type == type &&
@@ -259,7 +259,7 @@ static bool mma9553_is_any_event_enabled(struct mma9553_data *data,
 					 bool check_type,
 					 enum iio_chan_type type)
 {
-	int i;
+	u8 i;
 
 	for (i = 0; i < data->num_events; i++)
 		if ((check_type && data->events[i].info->type == type &&
-- 
1.9.1

Re: [PATCH v2 01/17] iio: accel: mma9553: fix endianness issue when reading status

[Jonathan Cameron]

On 13/04/15 16:40, Irina Tirdea wrote:
> Refactor code for simplicity and clarity.
> 
> This also fixes an endianness issue with the original code.
> When reading multiple registers, the received buffer of
> 16-bytes words is little endian (status, step count). On
> big endian machines, casting them to u32 would result in
> reversed order in the buffer (step count, status) leading
> to incorrect values for step count and activity.
> 
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> Reported-by: Hartmut Knaack <knaack.h@gmx.de>
Applied to the fixes-togreg branch of iio.git

Thanks
> ---
>  drivers/iio/accel/mma9553.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
> index 2df1af7..607dbfc 100644
> --- a/drivers/iio/accel/mma9553.c
> +++ b/drivers/iio/accel/mma9553.c
> @@ -316,22 +316,19 @@ static int mma9553_set_config(struct mma9553_data *data, u16 reg,
>  static int mma9553_read_activity_stepcnt(struct mma9553_data *data,
>  					 u8 *activity, u16 *stepcnt)
>  {
> -	u32 status_stepcnt;
> -	u16 status;
> +	u16 buf[2];
>  	int ret;
>  
>  	ret = mma9551_read_status_words(data->client, MMA9551_APPID_PEDOMETER,
> -					MMA9553_REG_STATUS, sizeof(u32),
> -					(u16 *) &status_stepcnt);
> +					MMA9553_REG_STATUS, sizeof(u32), buf);
>  	if (ret < 0) {
>  		dev_err(&data->client->dev,
>  			"error reading status and stepcnt\n");
>  		return ret;
>  	}
>  
> -	status = status_stepcnt & MMA9553_MASK_CONF_WORD;
> -	*activity = mma9553_get_bits(status, MMA9553_MASK_STATUS_ACTIVITY);
> -	*stepcnt = status_stepcnt >> 16;
> +	*activity = mma9553_get_bits(buf[0], MMA9553_MASK_STATUS_ACTIVITY);
> +	*stepcnt = buf[1];
>  
>  	return 0;
>  }
> 

Re: [PATCH v2 03/17] iio: accel: mma9551_core: prevent buffer overrun

[Jonathan Cameron]

On 13/04/15 16:40, Irina Tirdea wrote:
> The mma9551 functions that read/write word arrays from the
> device have a limit for the buffer size given by the device
> specifications.
> 
> Check that the requested buffer length is within required limits
> when transferring word arrays. This will prevent buffer overrun
> in the mma9551_read/write_*_words functions and also in the
> mma9551_transfer call when writing into the MBOX response/request
> structure.
> 
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> Reported-by: Hartmut Knaack <knaack.h@gmx.de>
Applied to the fixes-togreg branch of iio.git

Thanks,

Jonathan
> ---
>  drivers/iio/accel/mma9551_core.c | 21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/accel/mma9551_core.c b/drivers/iio/accel/mma9551_core.c
> index 7f55a6d..c6d5a3a 100644
> --- a/drivers/iio/accel/mma9551_core.c
> +++ b/drivers/iio/accel/mma9551_core.c
> @@ -389,7 +389,12 @@ int mma9551_read_config_words(struct i2c_client *client, u8 app_id,
>  {
>  	int ret, i;
>  	int len_words = len / sizeof(u16);
> -	__be16 be_buf[MMA9551_MAX_MAILBOX_DATA_REGS];
> +	__be16 be_buf[MMA9551_MAX_MAILBOX_DATA_REGS / 2];
> +
> +	if (len_words > ARRAY_SIZE(be_buf)) {
> +		dev_err(&client->dev, "Invalid buffer size %d\n", len);
> +		return -EINVAL;
> +	}
>  
>  	ret = mma9551_transfer(client, app_id, MMA9551_CMD_READ_CONFIG,
>  			       reg, NULL, 0, (u8 *) be_buf, len);
> @@ -424,7 +429,12 @@ int mma9551_read_status_words(struct i2c_client *client, u8 app_id,
>  {
>  	int ret, i;
>  	int len_words = len / sizeof(u16);
> -	__be16 be_buf[MMA9551_MAX_MAILBOX_DATA_REGS];
> +	__be16 be_buf[MMA9551_MAX_MAILBOX_DATA_REGS / 2];
> +
> +	if (len_words > ARRAY_SIZE(be_buf)) {
> +		dev_err(&client->dev, "Invalid buffer size %d\n", len);
> +		return -EINVAL;
> +	}
>  
>  	ret = mma9551_transfer(client, app_id, MMA9551_CMD_READ_STATUS,
>  			       reg, NULL, 0, (u8 *) be_buf, len);
> @@ -459,7 +469,12 @@ int mma9551_write_config_words(struct i2c_client *client, u8 app_id,
>  {
>  	int i;
>  	int len_words = len / sizeof(u16);
> -	__be16 be_buf[MMA9551_MAX_MAILBOX_DATA_REGS];
> +	__be16 be_buf[(MMA9551_MAX_MAILBOX_DATA_REGS - 1) / 2];
> +
> +	if (len_words > ARRAY_SIZE(be_buf)) {
> +		dev_err(&client->dev, "Invalid buffer size %d\n", len);
> +		return -EINVAL;
> +	}
>  
>  	for (i = 0; i < len_words; i++)
>  		be_buf[i] = cpu_to_be16(buf[i]);
> 

Re: [PATCH v2 04/17] iio: accel: mma9553: add enable channel for activity

[Jonathan Cameron]

On 13/04/15 16:40, Irina Tirdea wrote:
Err, this first bit is not relevant to this patch so I've dropped it.
> Since we need to specifically power on the device if used
> in polling mode, we have an enable channel for each type
> (step count, distance, speed, calories, etc.).
> 
> Add also an enable channel for activity, so it can also
> be polled independently of events or other channels.
> 
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> Reported-by: Daniel Baluta <daniel.baluta@intel.com>
Applied to the fixes-togreg branch of iio.git

Thanks,
> ---
>  drivers/iio/accel/mma9553.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
> index 03120fb..365a109 100644
> --- a/drivers/iio/accel/mma9553.c
> +++ b/drivers/iio/accel/mma9553.c
> @@ -972,7 +972,8 @@ static const struct iio_chan_spec_ext_info mma9553_ext_info[] = {
>  	.modified = 1,							\
>  	.channel2 = _chan2,						\
>  	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),		\
> -	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_CALIBHEIGHT),	\
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_CALIBHEIGHT) |	\
> +				    BIT(IIO_CHAN_INFO_ENABLE),		\
>  	.event_spec = mma9553_activity_events,				\
>  	.num_event_specs = ARRAY_SIZE(mma9553_activity_events),		\
>  	.ext_info = mma9553_ext_info,					\
> 

Re: [PATCH v2 02/17] iio: accel: mma9553: check input value for activity period

[Jonathan Cameron]

On 13/04/15 16:40, Irina Tirdea wrote:
> When setting the activity period, the value introduced by
> the user in sysfs is not checked for validity.
> 
> Add a boundary check so that only allowed values are
> reported as successfully written to device.
> 
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> Reported-by: Hartmut Knaack <knaack.h@gmx.de>
hmm.  Not that critical as it just gives miss information to
userspace that is writing an incorrect value.  Still it's small
so I've taken it in the fixes-togreg branch.

Thanks,

Jonathan
> ---
>  drivers/iio/accel/mma9553.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
> index 607dbfc..03120fb 100644
> --- a/drivers/iio/accel/mma9553.c
> +++ b/drivers/iio/accel/mma9553.c
> @@ -54,6 +54,7 @@
>  #define MMA9553_MASK_CONF_STEPCOALESCE		GENMASK(7, 0)
>  
>  #define MMA9553_REG_CONF_ACTTHD			0x0E
> +#define MMA9553_MAX_ACTTHD			GENMASK(15, 0)
>  
>  /* Pedometer status registers (R-only) */
>  #define MMA9553_REG_STATUS			0x00
> @@ -869,6 +870,9 @@ static int mma9553_write_event_value(struct iio_dev *indio_dev,
>  	case IIO_EV_INFO_PERIOD:
>  		switch (chan->type) {
>  		case IIO_ACTIVITY:
> +			if (val < 0 || val > MMA9553_ACTIVITY_THD_TO_SEC(
> +			    MMA9553_MAX_ACTTHD))
> +				return -EINVAL;
>  			mutex_lock(&data->mutex);
>  			ret = mma9553_set_config(data, MMA9553_REG_CONF_ACTTHD,
>  						 &data->conf.actthd,
> 

Re: [PATCH v2 05/17] iio: accel: mma9551_core: wrong doc fixes

[Jonathan Cameron]

On 13/04/15 16:40, Irina Tirdea wrote:
> Fix docummentation for mma9553_read_* functions.
> 
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> Reported-by: Hartmut Knaack <knaack.h@gmx.de>
Applied to the togreg branch of iio.git. Initially pushed
out as testing for the autobuilders to play with it.

Jonathan
> ---
>  drivers/iio/accel/mma9551_core.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/accel/mma9551_core.c b/drivers/iio/accel/mma9551_core.c
> index c6d5a3a..efe09a3 100644
> --- a/drivers/iio/accel/mma9551_core.c
> +++ b/drivers/iio/accel/mma9551_core.c
> @@ -374,7 +374,7 @@ EXPORT_SYMBOL(mma9551_read_status_word);
>   * @app_id:	Application ID
>   * @reg:	Application register
>   * @len:	Length of array to read in bytes
> - * @val:	Array of words to read
> + * @buf:	Array of words to read
>   *
>   * Read multiple configuration registers (word-sized registers).
>   *
> @@ -414,7 +414,7 @@ EXPORT_SYMBOL(mma9551_read_config_words);
>   * @app_id:	Application ID
>   * @reg:	Application register
>   * @len:	Length of array to read in bytes
> - * @val:	Array of words to read
> + * @buf:	Array of words to read
>   *
>   * Read multiple status registers (word-sized registers).
>   *
> @@ -454,7 +454,7 @@ EXPORT_SYMBOL(mma9551_read_status_words);
>   * @app_id:	Application ID
>   * @reg:	Application register
>   * @len:	Length of array to write in bytes
> - * @val:	Array of words to write
> + * @buf:	Array of words to write
>   *
>   * Write multiple configuration registers (word-sized registers).
>   *
> 

Re: [PATCH v2 06/17] iio: accel: mma9551_core: typo fix in RSC APP ID

[Jonathan Cameron]

On 13/04/15 16:40, Irina Tirdea wrote:
> Fix typo in Reset/Suspend/Clear Application ID definition.
> 
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> Reported-by: Hartmut Knaack <knaack.h@gmx.de>
applied to the togreg branch of iio.git

Thanks,
> ---
>  drivers/iio/accel/mma9551_core.c | 2 +-
>  drivers/iio/accel/mma9551_core.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/accel/mma9551_core.c b/drivers/iio/accel/mma9551_core.c
> index efe09a3..2fd2a99 100644
> --- a/drivers/iio/accel/mma9551_core.c
> +++ b/drivers/iio/accel/mma9551_core.c
> @@ -800,7 +800,7 @@ EXPORT_SYMBOL(mma9551_read_accel_scale);
>   */
>  int mma9551_app_reset(struct i2c_client *client, u32 app_mask)
>  {
> -	return mma9551_write_config_byte(client, MMA9551_APPID_RCS,
> +	return mma9551_write_config_byte(client, MMA9551_APPID_RSC,
>  					 MMA9551_RSC_RESET +
>  					 MMA9551_RSC_OFFSET(app_mask),
>  					 MMA9551_RSC_VAL(app_mask));
> diff --git a/drivers/iio/accel/mma9551_core.h b/drivers/iio/accel/mma9551_core.h
> index edaa56b..79939e4 100644
> --- a/drivers/iio/accel/mma9551_core.h
> +++ b/drivers/iio/accel/mma9551_core.h
> @@ -22,7 +22,7 @@
>  #define MMA9551_APPID_TILT		0x0B
>  #define MMA9551_APPID_SLEEP_WAKE	0x12
>  #define MMA9551_APPID_PEDOMETER	        0x15
> -#define MMA9551_APPID_RCS		0x17
> +#define MMA9551_APPID_RSC		0x17
>  #define MMA9551_APPID_NONE		0xff
>  
>  /* Reset/Suspend/Clear application app masks */
> 

Re: [PATCH v2 07/17] iio: accel: mma9553: check for error in reading initial activity and stepcnt

[Jonathan Cameron]

On 13/04/15 16:40, Irina Tirdea wrote:
> When configuring gpio, we need to read initial values for activity and
> step count. This function may fail due to i2c read errors.
> 
> Check the error code returned by mma9553_read_activity_stepcnt
> and return the appropriate error in gpio config function.
> 
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> Reported-by: Hartmut Knaack <knaack.h@gmx.de>
applied, togreg....
> ---
>  drivers/iio/accel/mma9553.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
> index 365a109..6f0a14b 100644
> --- a/drivers/iio/accel/mma9553.c
> +++ b/drivers/iio/accel/mma9553.c
> @@ -363,9 +363,12 @@ static int mma9553_conf_gpio(struct mma9553_data *data)
>  		return 0;
>  
>  	/* Save initial values for activity and stepcnt */
> -	if (activity_enabled || ev_step_detect->enabled)
> -		mma9553_read_activity_stepcnt(data, &data->activity,
> -					      &data->stepcnt);
> +	if (activity_enabled || ev_step_detect->enabled) {
> +		ret = mma9553_read_activity_stepcnt(data, &data->activity,
> +						    &data->stepcnt);
> +		if (ret < 0)
> +			return ret;
> +	}
>  
>  	ret = mma9551_gpio_config(data->client,
>  				  MMA9553_DEFAULT_GPIO_PIN,
> 

Re: [PATCH v2 09/17] iio: accel: mma9553: comment and error message fixes

[Jonathan Cameron]

On 13/04/15 16:40, Irina Tirdea wrote:
> Use "GPIO" instead of "gpio" and "ACPI" instead of "acpi".
> 
> Includes a couple of small style fixes in comments
> (missing full stop, whitespace, paranthesis).
> 
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> Suggested-by: Hartmut Knaack <knaack.h@gmx.de>
Applied to the togreg branch of iio.git

Thanks.

J
> ---
>  drivers/iio/accel/mma9553.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
> index 0f30006..a6b74de 100644
> --- a/drivers/iio/accel/mma9553.c
> +++ b/drivers/iio/accel/mma9553.c
> @@ -76,14 +76,14 @@
>  #define MMA9553_DEFAULT_GPIO_PIN	mma9551_gpio6
>  #define MMA9553_DEFAULT_GPIO_POLARITY	0
>  
> -/* Bitnum used for gpio configuration = bit number in high status byte */
>  #define STATUS_TO_BITNUM(bit)		(ffs(bit) - 9)
> +/* Bitnum used for GPIO configuration = bit number in high status byte */
>  
>  #define MMA9553_DEFAULT_SAMPLE_RATE	30	/* Hz */
>  
>  /*
>   * The internal activity level must be stable for ACTTHD samples before
> - * ACTIVITY is updated.The ACTIVITY variable contains the current activity
> + * ACTIVITY is updated. The ACTIVITY variable contains the current activity
>   * level and is updated every time a step is detected or once a second
>   * if there are no steps.
>   */
> @@ -399,13 +399,13 @@ static int mma9553_init(struct mma9553_data *data)
>  				      sizeof(data->conf), (u16 *) &data->conf);
>  	if (ret < 0) {
>  		dev_err(&data->client->dev,
> -			"device is not MMA9553L: failed to read cfg regs\n");
> +			"failed to read configuration registers\n");
>  		return ret;
>  	}
>  
>  
> -	/* Reset gpio */
>  	data->gpio_bitnum = -1;
> +	/* Reset GPIO */
>  	ret = mma9553_conf_gpio(data);
>  	if (ret < 0)
>  		return ret;
> @@ -457,7 +457,8 @@ static int mma9553_read_raw(struct iio_dev *indio_dev,
>  			 * The HW only counts steps and other dependent
>  			 * parameters (speed, distance, calories, activity)
>  			 * if power is on (from enabling an event or the
> -			 * step counter */
> +			 * step counter).
> +			 */
>  			powered_on =
>  			    mma9553_is_any_event_enabled(data, false, 0) ||
>  			    data->stepcnt_enabled;
> @@ -900,7 +901,7 @@ static int mma9553_get_calibgender_mode(struct iio_dev *indio_dev,
>  	gender = mma9553_get_bits(data->conf.filter, MMA9553_MASK_CONF_MALE);
>  	/*
>  	 * HW expects 0 for female and 1 for male,
> -	 * while iio index is 0 for male and 1 for female
> +	 * while iio index is 0 for male and 1 for female.
>  	 */
>  	return !gender;
>  }
> @@ -1113,16 +1114,16 @@ static int mma9553_gpio_probe(struct i2c_client *client)
>  
>  	dev = &client->dev;
>  
> -	/* data ready gpio interrupt pin */
> +	/* data ready GPIO interrupt pin */
>  	gpio = devm_gpiod_get_index(dev, MMA9553_GPIO_NAME, 0, GPIOD_IN);
>  	if (IS_ERR(gpio)) {
> -		dev_err(dev, "acpi gpio get index failed\n");
> +		dev_err(dev, "ACPI GPIO get index failed\n");
>  		return PTR_ERR(gpio);
>  	}
>  
>  	ret = gpiod_to_irq(gpio);
>  
> -	dev_dbg(dev, "gpio resource, no:%d irq:%d\n", desc_to_gpio(gpio), ret);
> +	dev_dbg(dev, "GPIO resource, no:%d irq:%d\n", desc_to_gpio(gpio), ret);
>  
>  	return ret;
>  }
> 

Re: [PATCH v2 10/17] iio: accel: mma9553: use GENMASK

[Jonathan Cameron]

On 13/04/15 16:40, Irina Tirdea wrote:
> Use GENMASK instead of BIT or direct value to
> define a mask.
> 
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> Suggested-by: Hartmut Knaack <knaack.h@gmx.de>
Applied.
> ---
>  drivers/iio/accel/mma9553.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
> index a6b74de..47cb14b 100644
> --- a/drivers/iio/accel/mma9553.c
> +++ b/drivers/iio/accel/mma9553.c
> @@ -63,8 +63,8 @@
>  #define MMA9553_MASK_STATUS_STEPCHG		BIT(13)
>  #define MMA9553_MASK_STATUS_ACTCHG		BIT(12)
>  #define MMA9553_MASK_STATUS_SUSP		BIT(11)
> -#define MMA9553_MASK_STATUS_ACTIVITY		(BIT(10) | BIT(9) | BIT(8))
> -#define MMA9553_MASK_STATUS_VERSION		0x00FF
> +#define MMA9553_MASK_STATUS_ACTIVITY		GENMASK(10, 8)
> +#define MMA9553_MASK_STATUS_VERSION		GENMASK(7, 0)
>  
>  #define MMA9553_REG_STEPCNT			0x02
>  #define MMA9553_REG_DISTANCE			0x04
> 

Re: [PATCH v2 11/17] iio: accel: mma9553: prefix naming fixes

[Jonathan Cameron]

On 13/04/15 16:40, Irina Tirdea wrote:
> Add mma9553_ prefix to all local functions/declarations.
> 
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> Suggested-by: Hartmut Knaack <knaack.h@gmx.de>
app
> ---
>  drivers/iio/accel/mma9553.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
> index 47cb14b..9573147 100644
> --- a/drivers/iio/accel/mma9553.c
> +++ b/drivers/iio/accel/mma9553.c
> @@ -76,8 +76,8 @@
>  #define MMA9553_DEFAULT_GPIO_PIN	mma9551_gpio6
>  #define MMA9553_DEFAULT_GPIO_POLARITY	0
>  
> -#define STATUS_TO_BITNUM(bit)		(ffs(bit) - 9)
>  /* Bitnum used for GPIO configuration = bit number in high status byte */
> +#define MMA9553_STATUS_TO_BITNUM(bit)	(ffs(bit) - 9)
>  
>  #define MMA9553_DEFAULT_SAMPLE_RATE	30	/* Hz */
>  
> @@ -351,11 +351,11 @@ static int mma9553_conf_gpio(struct mma9553_data *data)
>  	 * This bit is the logical OR of the SUSPCHG, STEPCHG, and ACTCHG flags.
>  	 */
>  	if (activity_enabled && ev_step_detect->enabled)
> -		bitnum = STATUS_TO_BITNUM(MMA9553_MASK_STATUS_MRGFL);
> +		bitnum = MMA9553_STATUS_TO_BITNUM(MMA9553_MASK_STATUS_MRGFL);
>  	else if (ev_step_detect->enabled)
> -		bitnum = STATUS_TO_BITNUM(MMA9553_MASK_STATUS_STEPCHG);
> +		bitnum = MMA9553_STATUS_TO_BITNUM(MMA9553_MASK_STATUS_STEPCHG);
>  	else if (activity_enabled)
> -		bitnum = STATUS_TO_BITNUM(MMA9553_MASK_STATUS_ACTCHG);
> +		bitnum = MMA9553_STATUS_TO_BITNUM(MMA9553_MASK_STATUS_ACTCHG);
>  	else			/* Reset */
>  		appid = MMA9551_APPID_NONE;
>  
> @@ -948,11 +948,11 @@ static const struct iio_event_spec mma9553_activity_events[] = {
>  	},
>  };
>  
> -static const char * const calibgender_modes[] = { "male", "female" };
> +static const char * const mma9553_calibgender_modes[] = { "male", "female" };
>  
>  static const struct iio_enum mma9553_calibgender_enum = {
> -	.items = calibgender_modes,
> -	.num_items = ARRAY_SIZE(calibgender_modes),
> +	.items = mma9553_calibgender_modes,
> +	.num_items = ARRAY_SIZE(mma9553_calibgender_modes),
>  	.get = mma9553_get_calibgender_mode,
>  	.set = mma9553_set_calibgender_mode,
>  };
> 

Re: [PATCH v2 12/17] iio: accel: mma9553: fix gpio bitnum init value

[Jonathan Cameron]

On 13/04/15 16:40, Irina Tirdea wrote:
> Initial value of gpio bitnum is set to -1, but
> the variable is declared as unsigned.
> 
> Use a positive invalid value for initial gpio
> bitnum.
> 
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> Suggested-by: Hartmut Knaack <knaack.h@gmx.de>
Applied.  I thought about adding this to the fixes-togreg
branch but got lazy and added it to the togreg one instead.

J
> ---
>  drivers/iio/accel/mma9553.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
> index 9573147..5755977 100644
> --- a/drivers/iio/accel/mma9553.c
> +++ b/drivers/iio/accel/mma9553.c
> @@ -78,6 +78,7 @@
>  
>  /* Bitnum used for GPIO configuration = bit number in high status byte */
>  #define MMA9553_STATUS_TO_BITNUM(bit)	(ffs(bit) - 9)
> +#define MMA9553_MAX_BITNUM		MMA9553_STATUS_TO_BITNUM(BIT(16))
>  
>  #define MMA9553_DEFAULT_SAMPLE_RATE	30	/* Hz */
>  
> @@ -404,8 +405,8 @@ static int mma9553_init(struct mma9553_data *data)
>  	}
>  
>  
> -	data->gpio_bitnum = -1;
>  	/* Reset GPIO */
> +	data->gpio_bitnum = MMA9553_MAX_BITNUM;
>  	ret = mma9553_conf_gpio(data);
>  	if (ret < 0)
>  		return ret;
> 

Re: [PATCH v2 13/17] iio: accel: mma9553: refactor mma9553_read_raw

[Jonathan Cameron]

On 13/04/15 16:41, Irina Tirdea wrote:
> Refactor code for simplicity and clarity.
> 
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> Suggested-by: Hartmut Knaack <knaack.h@gmx.de>
Applied.
> ---
>  drivers/iio/accel/mma9553.c | 101 +++++++++++++++-----------------------------
>  1 file changed, 33 insertions(+), 68 deletions(-)
> 
> diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
> index 5755977..8bfc618 100644
> --- a/drivers/iio/accel/mma9553.c
> +++ b/drivers/iio/accel/mma9553.c
> @@ -440,6 +440,32 @@ static int mma9553_init(struct mma9553_data *data)
>  	return mma9551_set_device_state(data->client, true);
>  }
>  
> +static int mma9553_read_status_word(struct mma9553_data *data, u16 reg,
> +				    u16 *tmp)
> +{
> +	bool powered_on;
> +	int ret;
> +
> +	/*
> +	 * The HW only counts steps and other dependent
> +	 * parameters (speed, distance, calories, activity)
> +	 * if power is on (from enabling an event or the
> +	 * step counter).
> +	 */
> +	powered_on = mma9553_is_any_event_enabled(data, false, 0) ||
> +		     data->stepcnt_enabled;
> +	if (!powered_on) {
> +		dev_err(&data->client->dev, "No channels enabled\n");
> +		return -EINVAL;
> +	}
> +
> +	mutex_lock(&data->mutex);
> +	ret = mma9551_read_status_word(data->client, MMA9551_APPID_PEDOMETER,
> +				       reg, tmp);
> +	mutex_unlock(&data->mutex);
> +	return ret;
> +}
> +
>  static int mma9553_read_raw(struct iio_dev *indio_dev,
>  			    struct iio_chan_spec const *chan,
>  			    int *val, int *val2, long mask)
> @@ -448,70 +474,30 @@ static int mma9553_read_raw(struct iio_dev *indio_dev,
>  	int ret;
>  	u16 tmp;
>  	u8 activity;
> -	bool powered_on;
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_PROCESSED:
>  		switch (chan->type) {
>  		case IIO_STEPS:
> -			/*
> -			 * The HW only counts steps and other dependent
> -			 * parameters (speed, distance, calories, activity)
> -			 * if power is on (from enabling an event or the
> -			 * step counter).
> -			 */
> -			powered_on =
> -			    mma9553_is_any_event_enabled(data, false, 0) ||
> -			    data->stepcnt_enabled;
> -			if (!powered_on) {
> -				dev_err(&data->client->dev,
> -					"No channels enabled\n");
> -				return -EINVAL;
> -			}
> -			mutex_lock(&data->mutex);
> -			ret = mma9551_read_status_word(data->client,
> -						       MMA9551_APPID_PEDOMETER,
> +			ret = mma9553_read_status_word(data,
>  						       MMA9553_REG_STEPCNT,
>  						       &tmp);
> -			mutex_unlock(&data->mutex);
>  			if (ret < 0)
>  				return ret;
>  			*val = tmp;
>  			return IIO_VAL_INT;
>  		case IIO_DISTANCE:
> -			powered_on =
> -			    mma9553_is_any_event_enabled(data, false, 0) ||
> -			    data->stepcnt_enabled;
> -			if (!powered_on) {
> -				dev_err(&data->client->dev,
> -					"No channels enabled\n");
> -				return -EINVAL;
> -			}
> -			mutex_lock(&data->mutex);
> -			ret = mma9551_read_status_word(data->client,
> -						       MMA9551_APPID_PEDOMETER,
> +			ret = mma9553_read_status_word(data,
>  						       MMA9553_REG_DISTANCE,
>  						       &tmp);
> -			mutex_unlock(&data->mutex);
>  			if (ret < 0)
>  				return ret;
>  			*val = tmp;
>  			return IIO_VAL_INT;
>  		case IIO_ACTIVITY:
> -			powered_on =
> -			    mma9553_is_any_event_enabled(data, false, 0) ||
> -			    data->stepcnt_enabled;
> -			if (!powered_on) {
> -				dev_err(&data->client->dev,
> -					"No channels enabled\n");
> -				return -EINVAL;
> -			}
> -			mutex_lock(&data->mutex);
> -			ret = mma9551_read_status_word(data->client,
> -						       MMA9551_APPID_PEDOMETER,
> +			ret = mma9553_read_status_word(data,
>  						       MMA9553_REG_STATUS,
>  						       &tmp);
> -			mutex_unlock(&data->mutex);
>  			if (ret < 0)
>  				return ret;
>  
> @@ -536,38 +522,17 @@ static int mma9553_read_raw(struct iio_dev *indio_dev,
>  		case IIO_VELOCITY:	/* m/h */
>  			if (chan->channel2 != IIO_MOD_ROOT_SUM_SQUARED_X_Y_Z)
>  				return -EINVAL;
> -			powered_on =
> -			    mma9553_is_any_event_enabled(data, false, 0) ||
> -			    data->stepcnt_enabled;
> -			if (!powered_on) {
> -				dev_err(&data->client->dev,
> -					"No channels enabled\n");
> -				return -EINVAL;
> -			}
> -			mutex_lock(&data->mutex);
> -			ret = mma9551_read_status_word(data->client,
> -						       MMA9551_APPID_PEDOMETER,
> -						       MMA9553_REG_SPEED, &tmp);
> -			mutex_unlock(&data->mutex);
> +			ret = mma9553_read_status_word(data,
> +						       MMA9553_REG_SPEED,
> +						       &tmp);
>  			if (ret < 0)
>  				return ret;
>  			*val = tmp;
>  			return IIO_VAL_INT;
>  		case IIO_ENERGY:	/* Cal or kcal */
> -			powered_on =
> -			    mma9553_is_any_event_enabled(data, false, 0) ||
> -			    data->stepcnt_enabled;
> -			if (!powered_on) {
> -				dev_err(&data->client->dev,
> -					"No channels enabled\n");
> -				return -EINVAL;
> -			}
> -			mutex_lock(&data->mutex);
> -			ret = mma9551_read_status_word(data->client,
> -						       MMA9551_APPID_PEDOMETER,
> +			ret = mma9553_read_status_word(data,
>  						       MMA9553_REG_CALORIES,
>  						       &tmp);
> -			mutex_unlock(&data->mutex);
>  			if (ret < 0)
>  				return ret;
>  			*val = tmp;
> 

Re: [PATCH v2 14/17] iio: accel: mma9551_core: use size in words for word buffers

[Jonathan Cameron]

On 13/04/15 16:41, Irina Tirdea wrote:
> Change the prototype for the mma9551_read/write_*_words functions
> to receive the length of the buffer in words (instead of bytes) since
> we are using a word buffer. This will prevent users from sending an
> odd number of bytes for a word array.
> 
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Lots of merge issues by this point in the series so I'll hold this one at
least until the fixes filter through.

Remind me if I seem to have forgotten (it wouldn't be the first time :)

Jonathan
> ---
>  drivers/iio/accel/mma9551_core.c | 27 ++++++++++++---------------
>  drivers/iio/accel/mma9553.c      |  9 ++++++---
>  2 files changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/iio/accel/mma9551_core.c b/drivers/iio/accel/mma9551_core.c
> index 2fd2a99..583660b 100644
> --- a/drivers/iio/accel/mma9551_core.c
> +++ b/drivers/iio/accel/mma9551_core.c
> @@ -373,7 +373,7 @@ EXPORT_SYMBOL(mma9551_read_status_word);
>   * @client:	I2C client
>   * @app_id:	Application ID
>   * @reg:	Application register
> - * @len:	Length of array to read in bytes
> + * @len:	Length of array to read (in words)
>   * @buf:	Array of words to read
>   *
>   * Read multiple configuration registers (word-sized registers).
> @@ -388,20 +388,19 @@ int mma9551_read_config_words(struct i2c_client *client, u8 app_id,
>  			     u16 reg, u8 len, u16 *buf)
>  {
>  	int ret, i;
> -	int len_words = len / sizeof(u16);
>  	__be16 be_buf[MMA9551_MAX_MAILBOX_DATA_REGS / 2];
>  
> -	if (len_words > ARRAY_SIZE(be_buf)) {
> +	if (len > ARRAY_SIZE(be_buf)) {
>  		dev_err(&client->dev, "Invalid buffer size %d\n", len);
>  		return -EINVAL;
>  	}
>  
>  	ret = mma9551_transfer(client, app_id, MMA9551_CMD_READ_CONFIG,
> -			       reg, NULL, 0, (u8 *) be_buf, len);
> +			       reg, NULL, 0, (u8 *)be_buf, len * sizeof(u16));
>  	if (ret < 0)
>  		return ret;
>  
> -	for (i = 0; i < len_words; i++)
> +	for (i = 0; i < len; i++)
>  		buf[i] = be16_to_cpu(be_buf[i]);
>  
>  	return 0;
> @@ -413,7 +412,7 @@ EXPORT_SYMBOL(mma9551_read_config_words);
>   * @client:	I2C client
>   * @app_id:	Application ID
>   * @reg:	Application register
> - * @len:	Length of array to read in bytes
> + * @len:	Length of array to read (in words)
>   * @buf:	Array of words to read
>   *
>   * Read multiple status registers (word-sized registers).
> @@ -428,20 +427,19 @@ int mma9551_read_status_words(struct i2c_client *client, u8 app_id,
>  			      u16 reg, u8 len, u16 *buf)
>  {
>  	int ret, i;
> -	int len_words = len / sizeof(u16);
>  	__be16 be_buf[MMA9551_MAX_MAILBOX_DATA_REGS / 2];
>  
> -	if (len_words > ARRAY_SIZE(be_buf)) {
> +	if (len > ARRAY_SIZE(be_buf)) {
>  		dev_err(&client->dev, "Invalid buffer size %d\n", len);
>  		return -EINVAL;
>  	}
>  
>  	ret = mma9551_transfer(client, app_id, MMA9551_CMD_READ_STATUS,
> -			       reg, NULL, 0, (u8 *) be_buf, len);
> +			       reg, NULL, 0, (u8 *)be_buf, len * sizeof(u16));
>  	if (ret < 0)
>  		return ret;
>  
> -	for (i = 0; i < len_words; i++)
> +	for (i = 0; i < len; i++)
>  		buf[i] = be16_to_cpu(be_buf[i]);
>  
>  	return 0;
> @@ -453,7 +451,7 @@ EXPORT_SYMBOL(mma9551_read_status_words);
>   * @client:	I2C client
>   * @app_id:	Application ID
>   * @reg:	Application register
> - * @len:	Length of array to write in bytes
> + * @len:	Length of array to write (in words)
>   * @buf:	Array of words to write
>   *
>   * Write multiple configuration registers (word-sized registers).
> @@ -468,19 +466,18 @@ int mma9551_write_config_words(struct i2c_client *client, u8 app_id,
>  			       u16 reg, u8 len, u16 *buf)
>  {
>  	int i;
> -	int len_words = len / sizeof(u16);
>  	__be16 be_buf[(MMA9551_MAX_MAILBOX_DATA_REGS - 1) / 2];
>  
> -	if (len_words > ARRAY_SIZE(be_buf)) {
> +	if (len > ARRAY_SIZE(be_buf)) {
>  		dev_err(&client->dev, "Invalid buffer size %d\n", len);
>  		return -EINVAL;
>  	}
>  
> -	for (i = 0; i < len_words; i++)
> +	for (i = 0; i < len; i++)
>  		be_buf[i] = cpu_to_be16(buf[i]);
>  
>  	return mma9551_transfer(client, app_id, MMA9551_CMD_WRITE_CONFIG,
> -				reg, (u8 *) be_buf, len, NULL, 0);
> +				reg, (u8 *)be_buf, len * sizeof(u16), NULL, 0);
>  }
>  EXPORT_SYMBOL(mma9551_write_config_words);
>  
> diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
> index 8bfc618..06c8707 100644
> --- a/drivers/iio/accel/mma9553.c
> +++ b/drivers/iio/accel/mma9553.c
> @@ -322,7 +322,8 @@ static int mma9553_read_activity_stepcnt(struct mma9553_data *data,
>  	int ret;
>  
>  	ret = mma9551_read_status_words(data->client, MMA9551_APPID_PEDOMETER,
> -					MMA9553_REG_STATUS, sizeof(u32), buf);
> +					MMA9553_REG_STATUS, ARRAY_SIZE(buf),
> +					buf);
>  	if (ret < 0) {
>  		dev_err(&data->client->dev,
>  			"error reading status and stepcnt\n");
> @@ -397,7 +398,8 @@ static int mma9553_init(struct mma9553_data *data)
>  	ret =
>  	    mma9551_read_config_words(data->client, MMA9551_APPID_PEDOMETER,
>  				      MMA9553_REG_CONF_SLEEPMIN,
> -				      sizeof(data->conf), (u16 *) &data->conf);
> +				      sizeof(data->conf) / sizeof(u16),
> +				      (u16 *)&data->conf);
>  	if (ret < 0) {
>  		dev_err(&data->client->dev,
>  			"failed to read configuration registers\n");
> @@ -430,7 +432,8 @@ static int mma9553_init(struct mma9553_data *data)
>  	ret =
>  	    mma9551_write_config_words(data->client, MMA9551_APPID_PEDOMETER,
>  				       MMA9553_REG_CONF_SLEEPMIN,
> -				       sizeof(data->conf), (u16 *) &data->conf);
> +				       sizeof(data->conf) / sizeof(u16),
> +				       (u16 *)&data->conf);
>  	if (ret < 0) {
>  		dev_err(&data->client->dev,
>  			"failed to write configuration registers\n");
> 

Re: [PATCH v2 00/17] Fixes for the mma9553 driver

[Hartmut Knaack]

Irina Tirdea schrieb am 13.04.2015 um 17:40:
> This is a set of follow-up patches for the mma9553 driver that fixes
> issues as suggested by Hartmut and Daniel.
> 

Finally, I got around to this series. Looks good to me.
Thanks,

Hartmut

> Changes in v2:
>  - split some of the the v1 patches into smaller patches
> as suggested by Jonathan
>  - reordered the series to have bug fixes first followed by style fixes
>  - use Reported-by for bug fixes
>  - added more fixes for issues detected by checkpatch.pl --strict
> 
> Irina Tirdea (17):
>   iio: accel: mma9553: fix endianness issue when reading status
>   iio: accel: mma9553: check input value for activity period
>   iio: accel: mma9551_core: prevent buffer overrun
>   iio: accel: mma9553: add enable channel for activity
>   iio: accel: mma9551_core: wrong doc fixes
>   iio: accel: mma9551_core: typo fix in RSC APP ID
>   iio: accel: mma9553: check for error in reading initial activity and
>     stepcnt
>   iio: accel: mma9553: return 0 as indication of success
>   iio: accel: mma9553: comment and error message fixes
>   iio: accel: mma9553: use GENMASK
>   iio: accel: mma9553: prefix naming fixes
>   iio: accel: mma9553: fix gpio bitnum init value
>   iio: accel: mma9553: refactor mma9553_read_raw
>   iio: accel: mma9551_core: use size in words for word buffers
>   iio: accel: mma9553: fix alignment issues
>   iio: accel: mma9553: document use of mutex
>   iio: accel: mma9553: use unsigned counters
> 
>  drivers/iio/accel/mma9551_core.c |  70 ++++++-----
>  drivers/iio/accel/mma9551_core.h |   8 +-
>  drivers/iio/accel/mma9553.c      | 253 ++++++++++++++++++---------------------
>  3 files changed, 160 insertions(+), 171 deletions(-)
> 

RE: [PATCH v2 14/17] iio: accel: mma9551_core: use size in words for word buffers

[Tirdea, Irina]

> -----Original Message-----
> From: Jonathan Cameron [mailto:jic23@kernel.org]
> Sent: 26 April, 2015 22:05
> To: Tirdea, Irina; linux-iio@vger.kernel.org; Hartmut Knaack
> Cc: linux-kernel@vger.kernel.org; Dogaru, Vlad
> Subject: Re: [PATCH v2 14/17] iio: accel: mma9551_core: use size in words for word buffers
> 
> On 13/04/15 16:41, Irina Tirdea wrote:
> > Change the prototype for the mma9551_read/write_*_words functions
> > to receive the length of the buffer in words (instead of bytes) since
> > we are using a word buffer. This will prevent users from sending an
> > odd number of bytes for a word array.
> >
> > Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> Lots of merge issues by this point in the series so I'll hold this one at
> least until the fixes filter through.
> 
> Remind me if I seem to have forgotten (it wouldn't be the first time :)
> 
Sure, I'll keep an eye on when the fixes will be merged in togreg branch.

Thanks,
Irina

> Jonathan
> > ---
> >  drivers/iio/accel/mma9551_core.c | 27 ++++++++++++---------------
> >  drivers/iio/accel/mma9553.c      |  9 ++++++---
> >  2 files changed, 18 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/iio/accel/mma9551_core.c b/drivers/iio/accel/mma9551_core.c
> > index 2fd2a99..583660b 100644
> > --- a/drivers/iio/accel/mma9551_core.c
> > +++ b/drivers/iio/accel/mma9551_core.c
> > @@ -373,7 +373,7 @@ EXPORT_SYMBOL(mma9551_read_status_word);
> >   * @client:	I2C client
> >   * @app_id:	Application ID
> >   * @reg:	Application register
> > - * @len:	Length of array to read in bytes
> > + * @len:	Length of array to read (in words)
> >   * @buf:	Array of words to read
> >   *
> >   * Read multiple configuration registers (word-sized registers).
> > @@ -388,20 +388,19 @@ int mma9551_read_config_words(struct i2c_client *client, u8 app_id,
> >  			     u16 reg, u8 len, u16 *buf)
> >  {
> >  	int ret, i;
> > -	int len_words = len / sizeof(u16);
> >  	__be16 be_buf[MMA9551_MAX_MAILBOX_DATA_REGS / 2];
> >
> > -	if (len_words > ARRAY_SIZE(be_buf)) {
> > +	if (len > ARRAY_SIZE(be_buf)) {
> >  		dev_err(&client->dev, "Invalid buffer size %d\n", len);
> >  		return -EINVAL;
> >  	}
> >
> >  	ret = mma9551_transfer(client, app_id, MMA9551_CMD_READ_CONFIG,
> > -			       reg, NULL, 0, (u8 *) be_buf, len);
> > +			       reg, NULL, 0, (u8 *)be_buf, len * sizeof(u16));
> >  	if (ret < 0)
> >  		return ret;
> >
> > -	for (i = 0; i < len_words; i++)
> > +	for (i = 0; i < len; i++)
> >  		buf[i] = be16_to_cpu(be_buf[i]);
> >
> >  	return 0;
> > @@ -413,7 +412,7 @@ EXPORT_SYMBOL(mma9551_read_config_words);
> >   * @client:	I2C client
> >   * @app_id:	Application ID
> >   * @reg:	Application register
> > - * @len:	Length of array to read in bytes
> > + * @len:	Length of array to read (in words)
> >   * @buf:	Array of words to read
> >   *
> >   * Read multiple status registers (word-sized registers).
> > @@ -428,20 +427,19 @@ int mma9551_read_status_words(struct i2c_client *client, u8 app_id,
> >  			      u16 reg, u8 len, u16 *buf)
> >  {
> >  	int ret, i;
> > -	int len_words = len / sizeof(u16);
> >  	__be16 be_buf[MMA9551_MAX_MAILBOX_DATA_REGS / 2];
> >
> > -	if (len_words > ARRAY_SIZE(be_buf)) {
> > +	if (len > ARRAY_SIZE(be_buf)) {
> >  		dev_err(&client->dev, "Invalid buffer size %d\n", len);
> >  		return -EINVAL;
> >  	}
> >
> >  	ret = mma9551_transfer(client, app_id, MMA9551_CMD_READ_STATUS,
> > -			       reg, NULL, 0, (u8 *) be_buf, len);
> > +			       reg, NULL, 0, (u8 *)be_buf, len * sizeof(u16));
> >  	if (ret < 0)
> >  		return ret;
> >
> > -	for (i = 0; i < len_words; i++)
> > +	for (i = 0; i < len; i++)
> >  		buf[i] = be16_to_cpu(be_buf[i]);
> >
> >  	return 0;
> > @@ -453,7 +451,7 @@ EXPORT_SYMBOL(mma9551_read_status_words);
> >   * @client:	I2C client
> >   * @app_id:	Application ID
> >   * @reg:	Application register
> > - * @len:	Length of array to write in bytes
> > + * @len:	Length of array to write (in words)
> >   * @buf:	Array of words to write
> >   *
> >   * Write multiple configuration registers (word-sized registers).
> > @@ -468,19 +466,18 @@ int mma9551_write_config_words(struct i2c_client *client, u8 app_id,
> >  			       u16 reg, u8 len, u16 *buf)
> >  {
> >  	int i;
> > -	int len_words = len / sizeof(u16);
> >  	__be16 be_buf[(MMA9551_MAX_MAILBOX_DATA_REGS - 1) / 2];
> >
> > -	if (len_words > ARRAY_SIZE(be_buf)) {
> > +	if (len > ARRAY_SIZE(be_buf)) {
> >  		dev_err(&client->dev, "Invalid buffer size %d\n", len);
> >  		return -EINVAL;
> >  	}
> >
> > -	for (i = 0; i < len_words; i++)
> > +	for (i = 0; i < len; i++)
> >  		be_buf[i] = cpu_to_be16(buf[i]);
> >
> >  	return mma9551_transfer(client, app_id, MMA9551_CMD_WRITE_CONFIG,
> > -				reg, (u8 *) be_buf, len, NULL, 0);
> > +				reg, (u8 *)be_buf, len * sizeof(u16), NULL, 0);
> >  }
> >  EXPORT_SYMBOL(mma9551_write_config_words);
> >
> > diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
> > index 8bfc618..06c8707 100644
> > --- a/drivers/iio/accel/mma9553.c
> > +++ b/drivers/iio/accel/mma9553.c
> > @@ -322,7 +322,8 @@ static int mma9553_read_activity_stepcnt(struct mma9553_data *data,
> >  	int ret;
> >
> >  	ret = mma9551_read_status_words(data->client, MMA9551_APPID_PEDOMETER,
> > -					MMA9553_REG_STATUS, sizeof(u32), buf);
> > +					MMA9553_REG_STATUS, ARRAY_SIZE(buf),
> > +					buf);
> >  	if (ret < 0) {
> >  		dev_err(&data->client->dev,
> >  			"error reading status and stepcnt\n");
> > @@ -397,7 +398,8 @@ static int mma9553_init(struct mma9553_data *data)
> >  	ret =
> >  	    mma9551_read_config_words(data->client, MMA9551_APPID_PEDOMETER,
> >  				      MMA9553_REG_CONF_SLEEPMIN,
> > -				      sizeof(data->conf), (u16 *) &data->conf);
> > +				      sizeof(data->conf) / sizeof(u16),
> > +				      (u16 *)&data->conf);
> >  	if (ret < 0) {
> >  		dev_err(&data->client->dev,
> >  			"failed to read configuration registers\n");
> > @@ -430,7 +432,8 @@ static int mma9553_init(struct mma9553_data *data)
> >  	ret =
> >  	    mma9551_write_config_words(data->client, MMA9551_APPID_PEDOMETER,
> >  				       MMA9553_REG_CONF_SLEEPMIN,
> > -				       sizeof(data->conf), (u16 *) &data->conf);
> > +				       sizeof(data->conf) / sizeof(u16),
> > +				       (u16 *)&data->conf);
> >  	if (ret < 0) {
> >  		dev_err(&data->client->dev,
> >  			"failed to write configuration registers\n");
> >

Re: [PATCH v2 14/17] iio: accel: mma9551_core: use size in words for word buffers

[Jonathan Cameron]

On 29/04/15 13:20, Tirdea, Irina wrote:
> 
> 
>> -----Original Message-----
>> From: Jonathan Cameron [mailto:jic23@kernel.org]
>> Sent: 26 April, 2015 22:05
>> To: Tirdea, Irina; linux-iio@vger.kernel.org; Hartmut Knaack
>> Cc: linux-kernel@vger.kernel.org; Dogaru, Vlad
>> Subject: Re: [PATCH v2 14/17] iio: accel: mma9551_core: use size in words for word buffers
>>
>> On 13/04/15 16:41, Irina Tirdea wrote:
>>> Change the prototype for the mma9551_read/write_*_words functions
>>> to receive the length of the buffer in words (instead of bytes) since
>>> we are using a word buffer. This will prevent users from sending an
>>> odd number of bytes for a word array.
>>>
>>> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
>> Lots of merge issues by this point in the series so I'll hold this one at
>> least until the fixes filter through.
>>
>> Remind me if I seem to have forgotten (it wouldn't be the first time :)
>>
> Sure, I'll keep an eye on when the fixes will be merged in togreg branch.
> 
They are there now, so applied to the togreg branch of iio.git, initially pushed
out as testing for the autobuilders to play with it.

Now looking at the merge window in 3 months time though to go upstream unfortunately.
Sorry for the delay, been a busy couple of months!

Jonathan
> Thanks,
> Irina
> 
>> Jonathan
>>> ---
>>>  drivers/iio/accel/mma9551_core.c | 27 ++++++++++++---------------
>>>  drivers/iio/accel/mma9553.c      |  9 ++++++---
>>>  2 files changed, 18 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/drivers/iio/accel/mma9551_core.c b/drivers/iio/accel/mma9551_core.c
>>> index 2fd2a99..583660b 100644
>>> --- a/drivers/iio/accel/mma9551_core.c
>>> +++ b/drivers/iio/accel/mma9551_core.c
>>> @@ -373,7 +373,7 @@ EXPORT_SYMBOL(mma9551_read_status_word);
>>>   * @client:	I2C client
>>>   * @app_id:	Application ID
>>>   * @reg:	Application register
>>> - * @len:	Length of array to read in bytes
>>> + * @len:	Length of array to read (in words)
>>>   * @buf:	Array of words to read
>>>   *
>>>   * Read multiple configuration registers (word-sized registers).
>>> @@ -388,20 +388,19 @@ int mma9551_read_config_words(struct i2c_client *client, u8 app_id,
>>>  			     u16 reg, u8 len, u16 *buf)
>>>  {
>>>  	int ret, i;
>>> -	int len_words = len / sizeof(u16);
>>>  	__be16 be_buf[MMA9551_MAX_MAILBOX_DATA_REGS / 2];
>>>
>>> -	if (len_words > ARRAY_SIZE(be_buf)) {
>>> +	if (len > ARRAY_SIZE(be_buf)) {
>>>  		dev_err(&client->dev, "Invalid buffer size %d\n", len);
>>>  		return -EINVAL;
>>>  	}
>>>
>>>  	ret = mma9551_transfer(client, app_id, MMA9551_CMD_READ_CONFIG,
>>> -			       reg, NULL, 0, (u8 *) be_buf, len);
>>> +			       reg, NULL, 0, (u8 *)be_buf, len * sizeof(u16));
>>>  	if (ret < 0)
>>>  		return ret;
>>>
>>> -	for (i = 0; i < len_words; i++)
>>> +	for (i = 0; i < len; i++)
>>>  		buf[i] = be16_to_cpu(be_buf[i]);
>>>
>>>  	return 0;
>>> @@ -413,7 +412,7 @@ EXPORT_SYMBOL(mma9551_read_config_words);
>>>   * @client:	I2C client
>>>   * @app_id:	Application ID
>>>   * @reg:	Application register
>>> - * @len:	Length of array to read in bytes
>>> + * @len:	Length of array to read (in words)
>>>   * @buf:	Array of words to read
>>>   *
>>>   * Read multiple status registers (word-sized registers).
>>> @@ -428,20 +427,19 @@ int mma9551_read_status_words(struct i2c_client *client, u8 app_id,
>>>  			      u16 reg, u8 len, u16 *buf)
>>>  {
>>>  	int ret, i;
>>> -	int len_words = len / sizeof(u16);
>>>  	__be16 be_buf[MMA9551_MAX_MAILBOX_DATA_REGS / 2];
>>>
>>> -	if (len_words > ARRAY_SIZE(be_buf)) {
>>> +	if (len > ARRAY_SIZE(be_buf)) {
>>>  		dev_err(&client->dev, "Invalid buffer size %d\n", len);
>>>  		return -EINVAL;
>>>  	}
>>>
>>>  	ret = mma9551_transfer(client, app_id, MMA9551_CMD_READ_STATUS,
>>> -			       reg, NULL, 0, (u8 *) be_buf, len);
>>> +			       reg, NULL, 0, (u8 *)be_buf, len * sizeof(u16));
>>>  	if (ret < 0)
>>>  		return ret;
>>>
>>> -	for (i = 0; i < len_words; i++)
>>> +	for (i = 0; i < len; i++)
>>>  		buf[i] = be16_to_cpu(be_buf[i]);
>>>
>>>  	return 0;
>>> @@ -453,7 +451,7 @@ EXPORT_SYMBOL(mma9551_read_status_words);
>>>   * @client:	I2C client
>>>   * @app_id:	Application ID
>>>   * @reg:	Application register
>>> - * @len:	Length of array to write in bytes
>>> + * @len:	Length of array to write (in words)
>>>   * @buf:	Array of words to write
>>>   *
>>>   * Write multiple configuration registers (word-sized registers).
>>> @@ -468,19 +466,18 @@ int mma9551_write_config_words(struct i2c_client *client, u8 app_id,
>>>  			       u16 reg, u8 len, u16 *buf)
>>>  {
>>>  	int i;
>>> -	int len_words = len / sizeof(u16);
>>>  	__be16 be_buf[(MMA9551_MAX_MAILBOX_DATA_REGS - 1) / 2];
>>>
>>> -	if (len_words > ARRAY_SIZE(be_buf)) {
>>> +	if (len > ARRAY_SIZE(be_buf)) {
>>>  		dev_err(&client->dev, "Invalid buffer size %d\n", len);
>>>  		return -EINVAL;
>>>  	}
>>>
>>> -	for (i = 0; i < len_words; i++)
>>> +	for (i = 0; i < len; i++)
>>>  		be_buf[i] = cpu_to_be16(buf[i]);
>>>
>>>  	return mma9551_transfer(client, app_id, MMA9551_CMD_WRITE_CONFIG,
>>> -				reg, (u8 *) be_buf, len, NULL, 0);
>>> +				reg, (u8 *)be_buf, len * sizeof(u16), NULL, 0);
>>>  }
>>>  EXPORT_SYMBOL(mma9551_write_config_words);
>>>
>>> diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
>>> index 8bfc618..06c8707 100644
>>> --- a/drivers/iio/accel/mma9553.c
>>> +++ b/drivers/iio/accel/mma9553.c
>>> @@ -322,7 +322,8 @@ static int mma9553_read_activity_stepcnt(struct mma9553_data *data,
>>>  	int ret;
>>>
>>>  	ret = mma9551_read_status_words(data->client, MMA9551_APPID_PEDOMETER,
>>> -					MMA9553_REG_STATUS, sizeof(u32), buf);
>>> +					MMA9553_REG_STATUS, ARRAY_SIZE(buf),
>>> +					buf);
>>>  	if (ret < 0) {
>>>  		dev_err(&data->client->dev,
>>>  			"error reading status and stepcnt\n");
>>> @@ -397,7 +398,8 @@ static int mma9553_init(struct mma9553_data *data)
>>>  	ret =
>>>  	    mma9551_read_config_words(data->client, MMA9551_APPID_PEDOMETER,
>>>  				      MMA9553_REG_CONF_SLEEPMIN,
>>> -				      sizeof(data->conf), (u16 *) &data->conf);
>>> +				      sizeof(data->conf) / sizeof(u16),
>>> +				      (u16 *)&data->conf);
>>>  	if (ret < 0) {
>>>  		dev_err(&data->client->dev,
>>>  			"failed to read configuration registers\n");
>>> @@ -430,7 +432,8 @@ static int mma9553_init(struct mma9553_data *data)
>>>  	ret =
>>>  	    mma9551_write_config_words(data->client, MMA9551_APPID_PEDOMETER,
>>>  				       MMA9553_REG_CONF_SLEEPMIN,
>>> -				       sizeof(data->conf), (u16 *) &data->conf);
>>> +				       sizeof(data->conf) / sizeof(u16),
>>> +				       (u16 *)&data->conf);
>>>  	if (ret < 0) {
>>>  		dev_err(&data->client->dev,
>>>  			"failed to write configuration registers\n");
>>>
> 

Re: [PATCH v2 15/17] iio: accel: mma9553: fix alignment issues

[Jonathan Cameron]

On 13/04/15 16:41, Irina Tirdea wrote:
> Fix code alignment and wrap parameters.
> Fix issues reported by checkpatch.pl --strict.
> 
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> Suggested-by: Hartmut Knaack <knaack.h@gmx.de>
Applied to the togreg branch of iio.git, initially pushed out
as testing etc. etc..
> ---
>  drivers/iio/accel/mma9551_core.c |  8 ++---
>  drivers/iio/accel/mma9551_core.h |  6 ++--
>  drivers/iio/accel/mma9553.c      | 76 +++++++++++++++++++---------------------
>  3 files changed, 43 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/iio/accel/mma9551_core.c b/drivers/iio/accel/mma9551_core.c
> index 583660b..c34c5ce 100644
> --- a/drivers/iio/accel/mma9551_core.c
> +++ b/drivers/iio/accel/mma9551_core.c
> @@ -297,7 +297,7 @@ EXPORT_SYMBOL(mma9551_read_status_byte);
>   * Returns: 0 on success, negative value on failure.
>   */
>  int mma9551_read_config_word(struct i2c_client *client, u8 app_id,
> -			    u16 reg, u16 *val)
> +			     u16 reg, u16 *val)
>  {
>  	int ret;
>  	__be16 v;
> @@ -328,12 +328,12 @@ EXPORT_SYMBOL(mma9551_read_config_word);
>   * Returns: 0 on success, negative value on failure.
>   */
>  int mma9551_write_config_word(struct i2c_client *client, u8 app_id,
> -			     u16 reg, u16 val)
> +			      u16 reg, u16 val)
>  {
>  	__be16 v = cpu_to_be16(val);
>  
>  	return mma9551_transfer(client, app_id, MMA9551_CMD_WRITE_CONFIG, reg,
> -				(u8 *) &v, 2, NULL, 0);
> +				(u8 *)&v, 2, NULL, 0);
>  }
>  EXPORT_SYMBOL(mma9551_write_config_word);
>  
> @@ -385,7 +385,7 @@ EXPORT_SYMBOL(mma9551_read_status_word);
>   * Returns: 0 on success, negative value on failure.
>   */
>  int mma9551_read_config_words(struct i2c_client *client, u8 app_id,
> -			     u16 reg, u8 len, u16 *buf)
> +			      u16 reg, u8 len, u16 *buf)
>  {
>  	int ret, i;
>  	__be16 be_buf[MMA9551_MAX_MAILBOX_DATA_REGS / 2];
> diff --git a/drivers/iio/accel/mma9551_core.h b/drivers/iio/accel/mma9551_core.h
> index 79939e4..5e88e64 100644
> --- a/drivers/iio/accel/mma9551_core.h
> +++ b/drivers/iio/accel/mma9551_core.h
> @@ -53,13 +53,13 @@ int mma9551_write_config_byte(struct i2c_client *client, u8 app_id,
>  int mma9551_read_status_byte(struct i2c_client *client, u8 app_id,
>  			     u16 reg, u8 *val);
>  int mma9551_read_config_word(struct i2c_client *client, u8 app_id,
> -			    u16 reg, u16 *val);
> +			     u16 reg, u16 *val);
>  int mma9551_write_config_word(struct i2c_client *client, u8 app_id,
> -			     u16 reg, u16 val);
> +			      u16 reg, u16 val);
>  int mma9551_read_status_word(struct i2c_client *client, u8 app_id,
>  			     u16 reg, u16 *val);
>  int mma9551_read_config_words(struct i2c_client *client, u8 app_id,
> -			     u16 reg, u8 len, u16 *buf);
> +			      u16 reg, u8 len, u16 *buf);
>  int mma9551_read_status_words(struct i2c_client *client, u8 app_id,
>  			      u16 reg, u8 len, u16 *buf);
>  int mma9551_write_config_words(struct i2c_client *client, u8 app_id,
> diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
> index 06c8707..08f28c3 100644
> --- a/drivers/iio/accel/mma9553.c
> +++ b/drivers/iio/accel/mma9553.c
> @@ -343,10 +343,10 @@ static int mma9553_conf_gpio(struct mma9553_data *data)
>  	struct mma9553_event *ev_step_detect;
>  	bool activity_enabled;
>  
> -	activity_enabled =
> -	    mma9553_is_any_event_enabled(data, true, IIO_ACTIVITY);
> -	ev_step_detect =
> -	    mma9553_get_event(data, IIO_STEPS, IIO_NO_MOD, IIO_EV_DIR_NONE);
> +	activity_enabled = mma9553_is_any_event_enabled(data, true,
> +							IIO_ACTIVITY);
> +	ev_step_detect = mma9553_get_event(data, IIO_STEPS, IIO_NO_MOD,
> +					   IIO_EV_DIR_NONE);
>  
>  	/*
>  	 * If both step detector and activity are enabled, use the MRGFL bit.
> @@ -372,9 +372,8 @@ static int mma9553_conf_gpio(struct mma9553_data *data)
>  			return ret;
>  	}
>  
> -	ret = mma9551_gpio_config(data->client,
> -				  MMA9553_DEFAULT_GPIO_PIN,
> -				  appid, bitnum, MMA9553_DEFAULT_GPIO_POLARITY);
> +	ret = mma9551_gpio_config(data->client, MMA9553_DEFAULT_GPIO_PIN, appid,
> +				  bitnum, MMA9553_DEFAULT_GPIO_POLARITY);
>  	if (ret < 0)
>  		return ret;
>  	data->gpio_bitnum = bitnum;
> @@ -395,18 +394,16 @@ static int mma9553_init(struct mma9553_data *data)
>  	 * a device identification command to differentiate the MMA9553L
>  	 * from the MMA9550L.
>  	 */
> -	ret =
> -	    mma9551_read_config_words(data->client, MMA9551_APPID_PEDOMETER,
> -				      MMA9553_REG_CONF_SLEEPMIN,
> -				      sizeof(data->conf) / sizeof(u16),
> -				      (u16 *)&data->conf);
> +	ret = mma9551_read_config_words(data->client, MMA9551_APPID_PEDOMETER,
> +					MMA9553_REG_CONF_SLEEPMIN,
> +					sizeof(data->conf) / sizeof(u16),
> +					(u16 *)&data->conf);
>  	if (ret < 0) {
>  		dev_err(&data->client->dev,
>  			"failed to read configuration registers\n");
>  		return ret;
>  	}
>  
> -
>  	/* Reset GPIO */
>  	data->gpio_bitnum = MMA9553_MAX_BITNUM;
>  	ret = mma9553_conf_gpio(data);
> @@ -421,19 +418,18 @@ static int mma9553_init(struct mma9553_data *data)
>  	data->conf.sleepmin = MMA9553_DEFAULT_SLEEPMIN;
>  	data->conf.sleepmax = MMA9553_DEFAULT_SLEEPMAX;
>  	data->conf.sleepthd = MMA9553_DEFAULT_SLEEPTHD;
> -	data->conf.config =
> -	    mma9553_set_bits(data->conf.config, 1, MMA9553_MASK_CONF_CONFIG);
> +	data->conf.config = mma9553_set_bits(data->conf.config, 1,
> +					     MMA9553_MASK_CONF_CONFIG);
>  	/*
>  	 * Clear the activity debounce counter when the activity level changes,
>  	 * so that the confidence level applies for any activity level.
>  	 */
>  	data->conf.config = mma9553_set_bits(data->conf.config, 1,
>  					     MMA9553_MASK_CONF_ACT_DBCNTM);
> -	ret =
> -	    mma9551_write_config_words(data->client, MMA9551_APPID_PEDOMETER,
> -				       MMA9553_REG_CONF_SLEEPMIN,
> -				       sizeof(data->conf) / sizeof(u16),
> -				       (u16 *)&data->conf);
> +	ret = mma9551_write_config_words(data->client, MMA9551_APPID_PEDOMETER,
> +					 MMA9553_REG_CONF_SLEEPMIN,
> +					 sizeof(data->conf) / sizeof(u16),
> +					 (u16 *)&data->conf);
>  	if (ret < 0) {
>  		dev_err(&data->client->dev,
>  			"failed to write configuration registers\n");
> @@ -570,7 +566,7 @@ static int mma9553_read_raw(struct iio_dev *indio_dev,
>  		return IIO_VAL_INT;
>  	case IIO_CHAN_INFO_CALIBHEIGHT:
>  		tmp = mma9553_get_bits(data->conf.height_weight,
> -					MMA9553_MASK_CONF_HEIGHT);
> +				       MMA9553_MASK_CONF_HEIGHT);
>  		*val = tmp / 100;	/* cm to m */
>  		*val2 = (tmp % 100) * 10000;
>  		return IIO_VAL_INT_PLUS_MICRO;
> @@ -722,7 +718,6 @@ static int mma9553_read_event_config(struct iio_dev *indio_dev,
>  				     enum iio_event_type type,
>  				     enum iio_event_direction dir)
>  {
> -
>  	struct mma9553_data *data = iio_priv(indio_dev);
>  	struct mma9553_event *event;
>  
> @@ -1029,22 +1024,22 @@ static irqreturn_t mma9553_event_handler(int irq, void *private)
>  		return IRQ_HANDLED;
>  	}
>  
> -	ev_prev_activity =
> -	    mma9553_get_event(data, IIO_ACTIVITY,
> -			      mma9553_activity_to_mod(data->activity),
> -			      IIO_EV_DIR_FALLING);
> -	ev_activity =
> -	    mma9553_get_event(data, IIO_ACTIVITY,
> -			      mma9553_activity_to_mod(activity),
> -			      IIO_EV_DIR_RISING);
> -	ev_step_detect =
> -	    mma9553_get_event(data, IIO_STEPS, IIO_NO_MOD, IIO_EV_DIR_NONE);
> +	ev_prev_activity = mma9553_get_event(data, IIO_ACTIVITY,
> +					     mma9553_activity_to_mod(
> +					     data->activity),
> +					     IIO_EV_DIR_FALLING);
> +	ev_activity = mma9553_get_event(data, IIO_ACTIVITY,
> +					mma9553_activity_to_mod(activity),
> +					IIO_EV_DIR_RISING);
> +	ev_step_detect = mma9553_get_event(data, IIO_STEPS, IIO_NO_MOD,
> +					   IIO_EV_DIR_NONE);
>  
>  	if (ev_step_detect->enabled && (stepcnt != data->stepcnt)) {
>  		data->stepcnt = stepcnt;
>  		iio_push_event(indio_dev,
>  			       IIO_EVENT_CODE(IIO_STEPS, 0, IIO_NO_MOD,
> -			       IIO_EV_DIR_NONE, IIO_EV_TYPE_CHANGE, 0, 0, 0),
> +					      IIO_EV_DIR_NONE,
> +					      IIO_EV_TYPE_CHANGE, 0, 0, 0),
>  			       data->timestamp);
>  	}
>  
> @@ -1054,17 +1049,19 @@ static irqreturn_t mma9553_event_handler(int irq, void *private)
>  		if (ev_prev_activity && ev_prev_activity->enabled)
>  			iio_push_event(indio_dev,
>  				       IIO_EVENT_CODE(IIO_ACTIVITY, 0,
> -				       ev_prev_activity->info->mod,
> -				       IIO_EV_DIR_FALLING,
> -				       IIO_EV_TYPE_THRESH, 0, 0, 0),
> +						    ev_prev_activity->info->mod,
> +						    IIO_EV_DIR_FALLING,
> +						    IIO_EV_TYPE_THRESH, 0, 0,
> +						    0),
>  				       data->timestamp);
>  
>  		if (ev_activity && ev_activity->enabled)
>  			iio_push_event(indio_dev,
>  				       IIO_EVENT_CODE(IIO_ACTIVITY, 0,
> -				       ev_activity->info->mod,
> -				       IIO_EV_DIR_RISING,
> -				       IIO_EV_TYPE_THRESH, 0, 0, 0),
> +						      ev_activity->info->mod,
> +						      IIO_EV_DIR_RISING,
> +						      IIO_EV_TYPE_THRESH, 0, 0,
> +						      0),
>  				       data->timestamp);
>  	}
>  	mutex_unlock(&data->mutex);
> @@ -1159,7 +1156,6 @@ static int mma9553_probe(struct i2c_client *client,
>  				client->irq);
>  			goto out_poweroff;
>  		}
> -
>  	}
>  
>  	ret = iio_device_register(indio_dev);
> 

Re: [PATCH v2 16/17] iio: accel: mma9553: document use of mutex

[Jonathan Cameron]

On 13/04/15 16:41, Irina Tirdea wrote:
> Fix checkpatch.pl --strict check:
> CHECK: struct mutex definition without comment
> +     struct mutex mutex;
> 
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Applied.

Thanks,


> ---
>  drivers/iio/accel/mma9553.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
> index 08f28c3..a605280 100644
> --- a/drivers/iio/accel/mma9553.c
> +++ b/drivers/iio/accel/mma9553.c
> @@ -182,6 +182,10 @@ struct mma9553_conf_regs {
>  
>  struct mma9553_data {
>  	struct i2c_client *client;
> +	/*
> +	 * 1. Serialize access to HW (requested by mma9551_core API).
> +	 * 2. Serialize sequences that power on/off the device and access HW.
> +	 */
>  	struct mutex mutex;
>  	struct mma9553_conf_regs conf;
>  	struct mma9553_event events[MMA9553_EVENTS_INFO_SIZE];
> 

Re: [PATCH v2 17/17] iio: accel: mma9553: use unsigned counters

[Jonathan Cameron]

On 13/04/15 16:41, Irina Tirdea wrote:
> Use unsigned counters instead of signed when all the
> possible values are positive.
> 
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> Suggested-by: Hartmut Knaack <knaack.h@gmx.de>
Hmm. I'm actually going to drop this one.  I'm not sure
there is a significant gain to be had by using minimal
sized integers as loop iterators.

int is just fine.

Jonathan
> ---
>  drivers/iio/accel/mma9551_core.c | 12 +++++++-----
>  drivers/iio/accel/mma9553.c      |  8 ++++----
>  2 files changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iio/accel/mma9551_core.c b/drivers/iio/accel/mma9551_core.c
> index c34c5ce..44b8243 100644
> --- a/drivers/iio/accel/mma9551_core.c
> +++ b/drivers/iio/accel/mma9551_core.c
> @@ -115,8 +115,8 @@ struct mma9551_version_info {
>  
>  static int mma9551_transfer(struct i2c_client *client,
>  			    u8 app_id, u8 command, u16 offset,
> -			    u8 *inbytes, int num_inbytes,
> -			    u8 *outbytes, int num_outbytes)
> +			    u8 *inbytes, u8 num_inbytes,
> +			    u8 *outbytes, u8 num_outbytes)
>  {
>  	struct mma9551_mbox_request req;
>  	struct mma9551_mbox_response rsp;
> @@ -387,7 +387,8 @@ EXPORT_SYMBOL(mma9551_read_status_word);
>  int mma9551_read_config_words(struct i2c_client *client, u8 app_id,
>  			      u16 reg, u8 len, u16 *buf)
>  {
> -	int ret, i;
> +	int ret;
> +	u8 i;
>  	__be16 be_buf[MMA9551_MAX_MAILBOX_DATA_REGS / 2];
>  
>  	if (len > ARRAY_SIZE(be_buf)) {
> @@ -426,7 +427,8 @@ EXPORT_SYMBOL(mma9551_read_config_words);
>  int mma9551_read_status_words(struct i2c_client *client, u8 app_id,
>  			      u16 reg, u8 len, u16 *buf)
>  {
> -	int ret, i;
> +	int ret;
> +	u8 i;
>  	__be16 be_buf[MMA9551_MAX_MAILBOX_DATA_REGS / 2];
>  
>  	if (len > ARRAY_SIZE(be_buf)) {
> @@ -465,7 +467,7 @@ EXPORT_SYMBOL(mma9551_read_status_words);
>  int mma9551_write_config_words(struct i2c_client *client, u8 app_id,
>  			       u16 reg, u8 len, u16 *buf)
>  {
> -	int i;
> +	u8 i;
>  	__be16 be_buf[(MMA9551_MAX_MAILBOX_DATA_REGS - 1) / 2];
>  
>  	if (len > ARRAY_SIZE(be_buf)) {
> diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
> index a605280..ad587ec 100644
> --- a/drivers/iio/accel/mma9553.c
> +++ b/drivers/iio/accel/mma9553.c
> @@ -189,7 +189,7 @@ struct mma9553_data {
>  	struct mutex mutex;
>  	struct mma9553_conf_regs conf;
>  	struct mma9553_event events[MMA9553_EVENTS_INFO_SIZE];
> -	int num_events;
> +	u8 num_events;
>  	u8 gpio_bitnum;
>  	/*
>  	 * This is used for all features that depend on step count:
> @@ -230,7 +230,7 @@ static enum iio_modifier mma9553_activity_to_mod(enum activity_level activity)
>  
>  static void mma9553_init_events(struct mma9553_data *data)
>  {
> -	int i;
> +	u8 i;
>  
>  	data->num_events = MMA9553_EVENTS_INFO_SIZE;
>  	for (i = 0; i < data->num_events; i++) {
> @@ -244,7 +244,7 @@ static struct mma9553_event *mma9553_get_event(struct mma9553_data *data,
>  					       enum iio_modifier mod,
>  					       enum iio_event_direction dir)
>  {
> -	int i;
> +	u8 i;
>  
>  	for (i = 0; i < data->num_events; i++)
>  		if (data->events[i].info->type == type &&
> @@ -259,7 +259,7 @@ static bool mma9553_is_any_event_enabled(struct mma9553_data *data,
>  					 bool check_type,
>  					 enum iio_chan_type type)
>  {
> -	int i;
> +	u8 i;
>  
>  	for (i = 0; i < data->num_events; i++)
>  		if ((check_type && data->events[i].info->type == type &&
> 

RE: [PATCH v2 14/17] iio: accel: mma9551_core: use size in words for word buffers

[Tirdea, Irina]

> -----Original Message-----
> From: Jonathan Cameron [mailto:jic23@kernel.org]
> Sent: 14 June, 2015 18:01
> To: Tirdea, Irina; linux-iio@vger.kernel.org; Hartmut Knaack
> Cc: linux-kernel@vger.kernel.org; Dogaru, Vlad
> Subject: Re: [PATCH v2 14/17] iio: accel: mma9551_core: use size in words for word buffers
> 
> On 29/04/15 13:20, Tirdea, Irina wrote:
> >
> >
> >> -----Original Message-----
> >> From: Jonathan Cameron [mailto:jic23@kernel.org]
> >> Sent: 26 April, 2015 22:05
> >> To: Tirdea, Irina; linux-iio@vger.kernel.org; Hartmut Knaack
> >> Cc: linux-kernel@vger.kernel.org; Dogaru, Vlad
> >> Subject: Re: [PATCH v2 14/17] iio: accel: mma9551_core: use size in words for word buffers
> >>
> >> On 13/04/15 16:41, Irina Tirdea wrote:
> >>> Change the prototype for the mma9551_read/write_*_words functions
> >>> to receive the length of the buffer in words (instead of bytes) since
> >>> we are using a word buffer. This will prevent users from sending an
> >>> odd number of bytes for a word array.
> >>>
> >>> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> >> Lots of merge issues by this point in the series so I'll hold this one at
> >> least until the fixes filter through.
> >>
> >> Remind me if I seem to have forgotten (it wouldn't be the first time :)
> >>
> > Sure, I'll keep an eye on when the fixes will be merged in togreg branch.
> >
> They are there now, so applied to the togreg branch of iio.git, initially pushed
> out as testing for the autobuilders to play with it.
> 
> Now looking at the merge window in 3 months time though to go upstream unfortunately.
> Sorry for the delay, been a busy couple of months!
> 
No problem, thanks for keeping an eye on when the fixes got merged.

Thanks,
Irina

> Jonathan
> > Thanks,
> > Irina
> >
> >> Jonathan
> >>> ---
> >>>  drivers/iio/accel/mma9551_core.c | 27 ++++++++++++---------------
> >>>  drivers/iio/accel/mma9553.c      |  9 ++++++---
> >>>  2 files changed, 18 insertions(+), 18 deletions(-)
> >>>
> >>> diff --git a/drivers/iio/accel/mma9551_core.c b/drivers/iio/accel/mma9551_core.c
> >>> index 2fd2a99..583660b 100644
> >>> --- a/drivers/iio/accel/mma9551_core.c
> >>> +++ b/drivers/iio/accel/mma9551_core.c
> >>> @@ -373,7 +373,7 @@ EXPORT_SYMBOL(mma9551_read_status_word);
> >>>   * @client:	I2C client
> >>>   * @app_id:	Application ID
> >>>   * @reg:	Application register
> >>> - * @len:	Length of array to read in bytes
> >>> + * @len:	Length of array to read (in words)
> >>>   * @buf:	Array of words to read
> >>>   *
> >>>   * Read multiple configuration registers (word-sized registers).
> >>> @@ -388,20 +388,19 @@ int mma9551_read_config_words(struct i2c_client *client, u8 app_id,
> >>>  			     u16 reg, u8 len, u16 *buf)
> >>>  {
> >>>  	int ret, i;
> >>> -	int len_words = len / sizeof(u16);
> >>>  	__be16 be_buf[MMA9551_MAX_MAILBOX_DATA_REGS / 2];
> >>>
> >>> -	if (len_words > ARRAY_SIZE(be_buf)) {
> >>> +	if (len > ARRAY_SIZE(be_buf)) {
> >>>  		dev_err(&client->dev, "Invalid buffer size %d\n", len);
> >>>  		return -EINVAL;
> >>>  	}
> >>>
> >>>  	ret = mma9551_transfer(client, app_id, MMA9551_CMD_READ_CONFIG,
> >>> -			       reg, NULL, 0, (u8 *) be_buf, len);
> >>> +			       reg, NULL, 0, (u8 *)be_buf, len * sizeof(u16));
> >>>  	if (ret < 0)
> >>>  		return ret;
> >>>
> >>> -	for (i = 0; i < len_words; i++)
> >>> +	for (i = 0; i < len; i++)
> >>>  		buf[i] = be16_to_cpu(be_buf[i]);
> >>>
> >>>  	return 0;
> >>> @@ -413,7 +412,7 @@ EXPORT_SYMBOL(mma9551_read_config_words);
> >>>   * @client:	I2C client
> >>>   * @app_id:	Application ID
> >>>   * @reg:	Application register
> >>> - * @len:	Length of array to read in bytes
> >>> + * @len:	Length of array to read (in words)
> >>>   * @buf:	Array of words to read
> >>>   *
> >>>   * Read multiple status registers (word-sized registers).
> >>> @@ -428,20 +427,19 @@ int mma9551_read_status_words(struct i2c_client *client, u8 app_id,
> >>>  			      u16 reg, u8 len, u16 *buf)
> >>>  {
> >>>  	int ret, i;
> >>> -	int len_words = len / sizeof(u16);
> >>>  	__be16 be_buf[MMA9551_MAX_MAILBOX_DATA_REGS / 2];
> >>>
> >>> -	if (len_words > ARRAY_SIZE(be_buf)) {
> >>> +	if (len > ARRAY_SIZE(be_buf)) {
> >>>  		dev_err(&client->dev, "Invalid buffer size %d\n", len);
> >>>  		return -EINVAL;
> >>>  	}
> >>>
> >>>  	ret = mma9551_transfer(client, app_id, MMA9551_CMD_READ_STATUS,
> >>> -			       reg, NULL, 0, (u8 *) be_buf, len);
> >>> +			       reg, NULL, 0, (u8 *)be_buf, len * sizeof(u16));
> >>>  	if (ret < 0)
> >>>  		return ret;
> >>>
> >>> -	for (i = 0; i < len_words; i++)
> >>> +	for (i = 0; i < len; i++)
> >>>  		buf[i] = be16_to_cpu(be_buf[i]);
> >>>
> >>>  	return 0;
> >>> @@ -453,7 +451,7 @@ EXPORT_SYMBOL(mma9551_read_status_words);
> >>>   * @client:	I2C client
> >>>   * @app_id:	Application ID
> >>>   * @reg:	Application register
> >>> - * @len:	Length of array to write in bytes
> >>> + * @len:	Length of array to write (in words)
> >>>   * @buf:	Array of words to write
> >>>   *
> >>>   * Write multiple configuration registers (word-sized registers).
> >>> @@ -468,19 +466,18 @@ int mma9551_write_config_words(struct i2c_client *client, u8 app_id,
> >>>  			       u16 reg, u8 len, u16 *buf)
> >>>  {
> >>>  	int i;
> >>> -	int len_words = len / sizeof(u16);
> >>>  	__be16 be_buf[(MMA9551_MAX_MAILBOX_DATA_REGS - 1) / 2];
> >>>
> >>> -	if (len_words > ARRAY_SIZE(be_buf)) {
> >>> +	if (len > ARRAY_SIZE(be_buf)) {
> >>>  		dev_err(&client->dev, "Invalid buffer size %d\n", len);
> >>>  		return -EINVAL;
> >>>  	}
> >>>
> >>> -	for (i = 0; i < len_words; i++)
> >>> +	for (i = 0; i < len; i++)
> >>>  		be_buf[i] = cpu_to_be16(buf[i]);
> >>>
> >>>  	return mma9551_transfer(client, app_id, MMA9551_CMD_WRITE_CONFIG,
> >>> -				reg, (u8 *) be_buf, len, NULL, 0);
> >>> +				reg, (u8 *)be_buf, len * sizeof(u16), NULL, 0);
> >>>  }
> >>>  EXPORT_SYMBOL(mma9551_write_config_words);
> >>>
> >>> diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
> >>> index 8bfc618..06c8707 100644
> >>> --- a/drivers/iio/accel/mma9553.c
> >>> +++ b/drivers/iio/accel/mma9553.c
> >>> @@ -322,7 +322,8 @@ static int mma9553_read_activity_stepcnt(struct mma9553_data *data,
> >>>  	int ret;
> >>>
> >>>  	ret = mma9551_read_status_words(data->client, MMA9551_APPID_PEDOMETER,
> >>> -					MMA9553_REG_STATUS, sizeof(u32), buf);
> >>> +					MMA9553_REG_STATUS, ARRAY_SIZE(buf),
> >>> +					buf);
> >>>  	if (ret < 0) {
> >>>  		dev_err(&data->client->dev,
> >>>  			"error reading status and stepcnt\n");
> >>> @@ -397,7 +398,8 @@ static int mma9553_init(struct mma9553_data *data)
> >>>  	ret =
> >>>  	    mma9551_read_config_words(data->client, MMA9551_APPID_PEDOMETER,
> >>>  				      MMA9553_REG_CONF_SLEEPMIN,
> >>> -				      sizeof(data->conf), (u16 *) &data->conf);
> >>> +				      sizeof(data->conf) / sizeof(u16),
> >>> +				      (u16 *)&data->conf);
> >>>  	if (ret < 0) {
> >>>  		dev_err(&data->client->dev,
> >>>  			"failed to read configuration registers\n");
> >>> @@ -430,7 +432,8 @@ static int mma9553_init(struct mma9553_data *data)
> >>>  	ret =
> >>>  	    mma9551_write_config_words(data->client, MMA9551_APPID_PEDOMETER,
> >>>  				       MMA9553_REG_CONF_SLEEPMIN,
> >>> -				       sizeof(data->conf), (u16 *) &data->conf);
> >>> +				       sizeof(data->conf) / sizeof(u16),
> >>> +				       (u16 *)&data->conf);
> >>>  	if (ret < 0) {
> >>>  		dev_err(&data->client->dev,
> >>>  			"failed to write configuration registers\n");
> >>>
> >

RE: [PATCH v2 17/17] iio: accel: mma9553: use unsigned counters

[Tirdea, Irina]

> -----Original Message-----
> From: Jonathan Cameron [mailto:jic23@kernel.org]
> Sent: 14 June, 2015 18:04
> To: Tirdea, Irina; linux-iio@vger.kernel.org; Hartmut Knaack
> Cc: linux-kernel@vger.kernel.org; Dogaru, Vlad
> Subject: Re: [PATCH v2 17/17] iio: accel: mma9553: use unsigned counters
> 
> On 13/04/15 16:41, Irina Tirdea wrote:
> > Use unsigned counters instead of signed when all the
> > possible values are positive.
> >
> > Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> > Suggested-by: Hartmut Knaack <knaack.h@gmx.de>
> Hmm. I'm actually going to drop this one.  I'm not sure
> there is a significant gain to be had by using minimal
> sized integers as loop iterators.
> 
> int is just fine.
> 

Fair enough, wasn't sure about this one either.

Thanks,
Irina

> Jonathan
> > ---
> >  drivers/iio/accel/mma9551_core.c | 12 +++++++-----
> >  drivers/iio/accel/mma9553.c      |  8 ++++----
> >  2 files changed, 11 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/iio/accel/mma9551_core.c b/drivers/iio/accel/mma9551_core.c
> > index c34c5ce..44b8243 100644
> > --- a/drivers/iio/accel/mma9551_core.c
> > +++ b/drivers/iio/accel/mma9551_core.c
> > @@ -115,8 +115,8 @@ struct mma9551_version_info {
> >
> >  static int mma9551_transfer(struct i2c_client *client,
> >  			    u8 app_id, u8 command, u16 offset,
> > -			    u8 *inbytes, int num_inbytes,
> > -			    u8 *outbytes, int num_outbytes)
> > +			    u8 *inbytes, u8 num_inbytes,
> > +			    u8 *outbytes, u8 num_outbytes)
> >  {
> >  	struct mma9551_mbox_request req;
> >  	struct mma9551_mbox_response rsp;
> > @@ -387,7 +387,8 @@ EXPORT_SYMBOL(mma9551_read_status_word);
> >  int mma9551_read_config_words(struct i2c_client *client, u8 app_id,
> >  			      u16 reg, u8 len, u16 *buf)
> >  {
> > -	int ret, i;
> > +	int ret;
> > +	u8 i;
> >  	__be16 be_buf[MMA9551_MAX_MAILBOX_DATA_REGS / 2];
> >
> >  	if (len > ARRAY_SIZE(be_buf)) {
> > @@ -426,7 +427,8 @@ EXPORT_SYMBOL(mma9551_read_config_words);
> >  int mma9551_read_status_words(struct i2c_client *client, u8 app_id,
> >  			      u16 reg, u8 len, u16 *buf)
> >  {
> > -	int ret, i;
> > +	int ret;
> > +	u8 i;
> >  	__be16 be_buf[MMA9551_MAX_MAILBOX_DATA_REGS / 2];
> >
> >  	if (len > ARRAY_SIZE(be_buf)) {
> > @@ -465,7 +467,7 @@ EXPORT_SYMBOL(mma9551_read_status_words);
> >  int mma9551_write_config_words(struct i2c_client *client, u8 app_id,
> >  			       u16 reg, u8 len, u16 *buf)
> >  {
> > -	int i;
> > +	u8 i;
> >  	__be16 be_buf[(MMA9551_MAX_MAILBOX_DATA_REGS - 1) / 2];
> >
> >  	if (len > ARRAY_SIZE(be_buf)) {
> > diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
> > index a605280..ad587ec 100644
> > --- a/drivers/iio/accel/mma9553.c
> > +++ b/drivers/iio/accel/mma9553.c
> > @@ -189,7 +189,7 @@ struct mma9553_data {
> >  	struct mutex mutex;
> >  	struct mma9553_conf_regs conf;
> >  	struct mma9553_event events[MMA9553_EVENTS_INFO_SIZE];
> > -	int num_events;
> > +	u8 num_events;
> >  	u8 gpio_bitnum;
> >  	/*
> >  	 * This is used for all features that depend on step count:
> > @@ -230,7 +230,7 @@ static enum iio_modifier mma9553_activity_to_mod(enum activity_level activity)
> >
> >  static void mma9553_init_events(struct mma9553_data *data)
> >  {
> > -	int i;
> > +	u8 i;
> >
> >  	data->num_events = MMA9553_EVENTS_INFO_SIZE;
> >  	for (i = 0; i < data->num_events; i++) {
> > @@ -244,7 +244,7 @@ static struct mma9553_event *mma9553_get_event(struct mma9553_data *data,
> >  					       enum iio_modifier mod,
> >  					       enum iio_event_direction dir)
> >  {
> > -	int i;
> > +	u8 i;
> >
> >  	for (i = 0; i < data->num_events; i++)
> >  		if (data->events[i].info->type == type &&
> > @@ -259,7 +259,7 @@ static bool mma9553_is_any_event_enabled(struct mma9553_data *data,
> >  					 bool check_type,
> >  					 enum iio_chan_type type)
> >  {
> > -	int i;
> > +	u8 i;
> >
> >  	for (i = 0; i < data->num_events; i++)
> >  		if ((check_type && data->events[i].info->type == type &&
> >