[PATCH v4 0/9] iio: accel: bma400: Add buffer, step and activity/inactivity

[PATCH v4 0/9] iio: accel: bma400: Add buffer, step and activity/inactivity

[Jagath Jog J]

This patch series adds trigger buffer support with data ready interrupt,
separate channel for step counter, an event for step change interrupt,
activity recognition and activity/inactivity event support.

changes since v3
1. Removed all the unnecessary mutex locking for regmap.
2. Corrected the mutex locking and unlocking for device private data
members.
3. Mutex locking and unlocking is used to protect the device private
structure members.
4. Using DMA safe buffer for regmap_bulk_write() and regmap_bulk_read().

1/9: Fixed the comment.

3/9: Added () for the function name in the comment.

4/9: Handling error cases with goto in bma400_trigger_handler().
     Mutex locking and unlocking is used to protect the data->buffer.
     Using DMA safe buffer for regmap_bulk_read().
     Mutex locking and unlocking is used to protect the data->status in
     bma400_interrupt.

5/9: Using DMA safe buffers to read steps value by allocating memory internally.
     Using DMA safe buffers for regmap_bulk_write(). 
     Removed the lock for regmap().

6/9: Removed the duplication of code for enabling step, added function to handle
     the step enable. 

7/9: Removed the lock for regmap().
     Mutex locking and unlocking is used to protect the data members.

8/9: Removed the lock for regmap().

9/9. Added __be16 duration in struct bma400_data.
     Fixed the warning - impossible condition '(reg < 0) => (0-255 < 0)'
     Fixed error: call to __compiletime_assert_272
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

changes since v2
1. Reordering of header includes in the separate patch.
2. Matching the IIO syntax for multiline comment.
3. Following the preference in the interrupt handler for returning.
4. Add support for activity recognition.
5. Add support for debugfs to access registers from userspace.
6. Add support for activity and inactivity events

changes since v1
1. Added comment section that describes the math for scale calculation.
2. Added separate devm_add_action_or_reset() calls to disable regulator
   and to put the sensor in power down mode.
3. Remove the err_reg_disable and out, goto labels and returning directly
   if error occurs.
4. Added mutex calls while putting sensor in power down.
5. Added ___cacheline_aligned for device data.
6. Ordering the header includes.
7. Handling erroneous and spurious interrupts in the interrupt handler
   by returning IRQ_NONE.
8. Using dev_err_probe() instead of dev_err().
9. Configured the interrupt to open drain.
10. Using le16_to_cpu() to fix the sparse warning.
11. Checking the step change event is enabled or not.
12. Enabling the step change event will also enable the step channel.
13. Using FIELD_GET() instead of bitwise operation.
14. Removal of dead code in the _event_config().

Jagath Jog J (9):
  iio: accel: bma400: Fix the scale min and max macro values
  iio: accel: bma400: Reordering of header files
  iio: accel: bma400: conversion to device-managed function
  iio: accel: bma400: Add triggered buffer support
  iio: accel: bma400: Add separate channel for step counter
  iio: accel: bma400: Add step change event
  iio: accel: bma400: Add activity recognition support
  iio: accel: bma400: Add debugfs register access support
  iio: accel: bma400: Add support for activity and inactivity events

 drivers/iio/accel/Kconfig       |   2 +
 drivers/iio/accel/bma400.h      |  50 ++-
 drivers/iio/accel/bma400_core.c | 694 +++++++++++++++++++++++++++++---
 drivers/iio/accel/bma400_i2c.c  |  10 +-
 drivers/iio/accel/bma400_spi.c  |   8 +-
 5 files changed, 697 insertions(+), 67 deletions(-)

-- 
2.17.1

[PATCH v4 1/9] iio: accel: bma400: Fix the scale min and max macro values

[Jagath Jog J]

Changing the scale macro values to match the bma400 sensitivity
for 1 LSB of all the available ranges.

Fixes: 465c811f1f20 ("iio: accel: Add driver for the BMA400")
Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
---
 drivers/iio/accel/bma400.h | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/accel/bma400.h b/drivers/iio/accel/bma400.h
index c4c8d74155c2..80330c7ce17f 100644
--- a/drivers/iio/accel/bma400.h
+++ b/drivers/iio/accel/bma400.h
@@ -83,8 +83,27 @@
 #define BMA400_ACC_ODR_MIN_WHOLE_HZ 25
 #define BMA400_ACC_ODR_MIN_HZ       12
 
-#define BMA400_SCALE_MIN            38357
-#define BMA400_SCALE_MAX            306864
+/*
+ * BMA400_SCALE_MIN macro value represents m/s^2 for 1 LSB before
+ * converting to micro values for +-2g range.
+ *
+ * For +-2g - 1 LSB = 0.976562 milli g = 0.009576 m/s^2
+ * For +-4g - 1 LSB = 1.953125 milli g = 0.019153 m/s^2
+ * For +-16g - 1 LSB = 7.8125 milli g = 0.076614 m/s^2
+ *
+ * The raw value which is used to select the different ranges is determined
+ * by the first bit set position from the scale value, so BMA400_SCALE_MIN
+ * should be odd.
+ *
+ * Scale values for +-2g, +-4g, +-8g and +-16g are populated into bma400_scales
+ * array by left shifting BMA400_SCALE_MIN.
+ * e.g.:
+ * To select +-2g = 9577 << 0 = raw value to write is 0.
+ * To select +-8g = 9577 << 2 = raw value to write is 2.
+ * To select +-16g = 9577 << 3 = raw value to write is 3.
+ */
+#define BMA400_SCALE_MIN            9577
+#define BMA400_SCALE_MAX            76617
 
 #define BMA400_NUM_REGULATORS       2
 #define BMA400_VDD_REGULATOR        0
-- 
2.17.1

[PATCH v4 2/9] iio: accel: bma400: Reordering of header files

[Jagath Jog J]

Reordering of header files and removing the iio/sysfs.h since
custom attributes are not being used in the driver.

Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/iio/accel/bma400_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
index 043002fe6f63..25ad1f7339bc 100644
--- a/drivers/iio/accel/bma400_core.c
+++ b/drivers/iio/accel/bma400_core.c
@@ -13,14 +13,14 @@
 
 #include <linux/bitops.h>
 #include <linux/device.h>
-#include <linux/iio/iio.h>
-#include <linux/iio/sysfs.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 
+#include <linux/iio/iio.h>
+
 #include "bma400.h"
 
 /*
-- 
2.17.1

[PATCH v4 3/9] iio: accel: bma400: conversion to device-managed function

[Jagath Jog J]

This is a conversion to device-managed by using devm_iio_device_register()
inside probe function. Previously the bma400 was not put into power down
mode in some error paths in probe where it now is, but that should cause
no harm.

The dev_set_drvdata() call, bma400_remove() function and hooks in the I2C
and SPI driver struct is removed as devm_iio_device_register() function is
used to automatically unregister on driver detach.

Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/iio/accel/bma400.h      |  2 -
 drivers/iio/accel/bma400_core.c | 77 ++++++++++++++++-----------------
 drivers/iio/accel/bma400_i2c.c  |  8 ----
 drivers/iio/accel/bma400_spi.c  |  6 ---
 4 files changed, 38 insertions(+), 55 deletions(-)

diff --git a/drivers/iio/accel/bma400.h b/drivers/iio/accel/bma400.h
index 80330c7ce17f..1c8c47a9a317 100644
--- a/drivers/iio/accel/bma400.h
+++ b/drivers/iio/accel/bma400.h
@@ -113,6 +113,4 @@ extern const struct regmap_config bma400_regmap_config;
 
 int bma400_probe(struct device *dev, struct regmap *regmap, const char *name);
 
-void bma400_remove(struct device *dev);
-
 #endif
diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
index 25ad1f7339bc..07674d89d978 100644
--- a/drivers/iio/accel/bma400_core.c
+++ b/drivers/iio/accel/bma400_core.c
@@ -560,6 +560,26 @@ static void bma400_init_tables(void)
 	}
 }
 
+static void bma400_regulators_disable(void *data_ptr)
+{
+	struct bma400_data *data = data_ptr;
+
+	regulator_bulk_disable(ARRAY_SIZE(data->regulators), data->regulators);
+}
+
+static void bma400_power_disable(void *data_ptr)
+{
+	struct bma400_data *data = data_ptr;
+	int ret;
+
+	mutex_lock(&data->mutex);
+	ret = bma400_set_power_mode(data, POWER_MODE_SLEEP);
+	mutex_unlock(&data->mutex);
+	if (ret)
+		dev_warn(data->dev, "Failed to put device into sleep mode (%pe)\n",
+			 ERR_PTR(ret));
+}
+
 static int bma400_init(struct bma400_data *data)
 {
 	unsigned int val;
@@ -569,13 +589,12 @@ static int bma400_init(struct bma400_data *data)
 	ret = regmap_read(data->regmap, BMA400_CHIP_ID_REG, &val);
 	if (ret) {
 		dev_err(data->dev, "Failed to read chip id register\n");
-		goto out;
+		return ret;
 	}
 
 	if (val != BMA400_ID_REG_VAL) {
 		dev_err(data->dev, "Chip ID mismatch\n");
-		ret = -ENODEV;
-		goto out;
+		return -ENODEV;
 	}
 
 	data->regulators[BMA400_VDD_REGULATOR].supply = "vdd";
@@ -589,27 +608,31 @@ static int bma400_init(struct bma400_data *data)
 				"Failed to get regulators: %d\n",
 				ret);
 
-		goto out;
+		return ret;
 	}
 	ret = regulator_bulk_enable(ARRAY_SIZE(data->regulators),
 				    data->regulators);
 	if (ret) {
 		dev_err(data->dev, "Failed to enable regulators: %d\n",
 			ret);
-		goto out;
+		return ret;
 	}
 
+	ret = devm_add_action_or_reset(data->dev, bma400_regulators_disable, data);
+	if (ret)
+		return ret;
+
 	ret = bma400_get_power_mode(data);
 	if (ret) {
 		dev_err(data->dev, "Failed to get the initial power-mode\n");
-		goto err_reg_disable;
+		return ret;
 	}
 
 	if (data->power_mode != POWER_MODE_NORMAL) {
 		ret = bma400_set_power_mode(data, POWER_MODE_NORMAL);
 		if (ret) {
 			dev_err(data->dev, "Failed to wake up the device\n");
-			goto err_reg_disable;
+			return ret;
 		}
 		/*
 		 * TODO: The datasheet waits 1500us here in the example, but
@@ -618,19 +641,23 @@ static int bma400_init(struct bma400_data *data)
 		usleep_range(1500, 2000);
 	}
 
+	ret = devm_add_action_or_reset(data->dev, bma400_power_disable, data);
+	if (ret)
+		return ret;
+
 	bma400_init_tables();
 
 	ret = bma400_get_accel_output_data_rate(data);
 	if (ret)
-		goto err_reg_disable;
+		return ret;
 
 	ret = bma400_get_accel_oversampling_ratio(data);
 	if (ret)
-		goto err_reg_disable;
+		return ret;
 
 	ret = bma400_get_accel_scale(data);
 	if (ret)
-		goto err_reg_disable;
+		return ret;
 
 	/*
 	 * Once the interrupt engine is supported we might use the
@@ -639,12 +666,6 @@ static int bma400_init(struct bma400_data *data)
 	 * channel.
 	 */
 	return regmap_write(data->regmap, BMA400_ACC_CONFIG2_REG, 0x00);
-
-err_reg_disable:
-	regulator_bulk_disable(ARRAY_SIZE(data->regulators),
-			       data->regulators);
-out:
-	return ret;
 }
 
 static int bma400_read_raw(struct iio_dev *indio_dev,
@@ -822,32 +843,10 @@ int bma400_probe(struct device *dev, struct regmap *regmap, const char *name)
 	indio_dev->num_channels = ARRAY_SIZE(bma400_channels);
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
-	dev_set_drvdata(dev, indio_dev);
-
-	return iio_device_register(indio_dev);
+	return devm_iio_device_register(dev, indio_dev);
 }
 EXPORT_SYMBOL_NS(bma400_probe, IIO_BMA400);
 
-void bma400_remove(struct device *dev)
-{
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
-	struct bma400_data *data = iio_priv(indio_dev);
-	int ret;
-
-	mutex_lock(&data->mutex);
-	ret = bma400_set_power_mode(data, POWER_MODE_SLEEP);
-	mutex_unlock(&data->mutex);
-
-	if (ret)
-		dev_warn(dev, "Failed to put device into sleep mode (%pe)\n", ERR_PTR(ret));
-
-	regulator_bulk_disable(ARRAY_SIZE(data->regulators),
-			       data->regulators);
-
-	iio_device_unregister(indio_dev);
-}
-EXPORT_SYMBOL_NS(bma400_remove, IIO_BMA400);
-
 MODULE_AUTHOR("Dan Robertson <dan@dlrobertson.com>");
 MODULE_DESCRIPTION("Bosch BMA400 triaxial acceleration sensor core");
 MODULE_LICENSE("GPL");
diff --git a/drivers/iio/accel/bma400_i2c.c b/drivers/iio/accel/bma400_i2c.c
index da104ffd3fe0..4f6e01a3b3a1 100644
--- a/drivers/iio/accel/bma400_i2c.c
+++ b/drivers/iio/accel/bma400_i2c.c
@@ -27,13 +27,6 @@ static int bma400_i2c_probe(struct i2c_client *client,
 	return bma400_probe(&client->dev, regmap, id->name);
 }
 
-static int bma400_i2c_remove(struct i2c_client *client)
-{
-	bma400_remove(&client->dev);
-
-	return 0;
-}
-
 static const struct i2c_device_id bma400_i2c_ids[] = {
 	{ "bma400", 0 },
 	{ }
@@ -52,7 +45,6 @@ static struct i2c_driver bma400_i2c_driver = {
 		.of_match_table = bma400_of_i2c_match,
 	},
 	.probe    = bma400_i2c_probe,
-	.remove   = bma400_i2c_remove,
 	.id_table = bma400_i2c_ids,
 };
 
diff --git a/drivers/iio/accel/bma400_spi.c b/drivers/iio/accel/bma400_spi.c
index 51f23bdc0ea5..28e240400a3f 100644
--- a/drivers/iio/accel/bma400_spi.c
+++ b/drivers/iio/accel/bma400_spi.c
@@ -87,11 +87,6 @@ static int bma400_spi_probe(struct spi_device *spi)
 	return bma400_probe(&spi->dev, regmap, id->name);
 }
 
-static void bma400_spi_remove(struct spi_device *spi)
-{
-	bma400_remove(&spi->dev);
-}
-
 static const struct spi_device_id bma400_spi_ids[] = {
 	{ "bma400", 0 },
 	{ }
@@ -110,7 +105,6 @@ static struct spi_driver bma400_spi_driver = {
 		.of_match_table = bma400_of_spi_match,
 	},
 	.probe    = bma400_spi_probe,
-	.remove   = bma400_spi_remove,
 	.id_table = bma400_spi_ids,
 };
 
-- 
2.17.1

[PATCH v4 4/9] iio: accel: bma400: Add triggered buffer support

[Jagath Jog J]

Added trigger buffer support to read continuous acceleration
data from device with data ready interrupt which is mapped
to INT1 pin.

Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
---
 drivers/iio/accel/Kconfig       |   2 +
 drivers/iio/accel/bma400.h      |  10 +-
 drivers/iio/accel/bma400_core.c | 162 +++++++++++++++++++++++++++++++-
 drivers/iio/accel/bma400_i2c.c  |   2 +-
 drivers/iio/accel/bma400_spi.c  |   2 +-
 5 files changed, 170 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index eac3f02662ae..958097814232 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -204,6 +204,8 @@ config BMA220
 config BMA400
 	tristate "Bosch BMA400 3-Axis Accelerometer Driver"
 	select REGMAP
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
 	select BMA400_I2C if I2C
 	select BMA400_SPI if SPI
 	help
diff --git a/drivers/iio/accel/bma400.h b/drivers/iio/accel/bma400.h
index 1c8c47a9a317..907e1a6c0a38 100644
--- a/drivers/iio/accel/bma400.h
+++ b/drivers/iio/accel/bma400.h
@@ -62,6 +62,13 @@
 #define BMA400_ACC_CONFIG2_REG      0x1b
 #define BMA400_CMD_REG              0x7e
 
+/* Interrupt registers */
+#define BMA400_INT_CONFIG0_REG	    0x1f
+#define BMA400_INT_CONFIG1_REG	    0x20
+#define BMA400_INT1_MAP_REG	    0x21
+#define BMA400_INT_IO_CTRL_REG	    0x24
+#define BMA400_INT_DRDY_MSK	    BIT(7)
+
 /* Chip ID of BMA 400 devices found in the chip ID register. */
 #define BMA400_ID_REG_VAL           0x90
 
@@ -111,6 +118,7 @@
 
 extern const struct regmap_config bma400_regmap_config;
 
-int bma400_probe(struct device *dev, struct regmap *regmap, const char *name);
+int bma400_probe(struct device *dev, struct regmap *regmap, int irq,
+		 const char *name);
 
 #endif
diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
index 07674d89d978..57910ccf9180 100644
--- a/drivers/iio/accel/bma400_core.c
+++ b/drivers/iio/accel/bma400_core.c
@@ -11,6 +11,7 @@
  *  - Create channel for sensor time
  */
 
+#include <linux/bitfield.h>
 #include <linux/bitops.h>
 #include <linux/device.h>
 #include <linux/kernel.h>
@@ -20,6 +21,10 @@
 #include <linux/regulator/consumer.h>
 
 #include <linux/iio/iio.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
 
 #include "bma400.h"
 
@@ -61,6 +66,14 @@ struct bma400_data {
 	struct bma400_sample_freq sample_freq;
 	int oversampling_ratio;
 	int scale;
+	struct iio_trigger *trig;
+	/* Correct time stamp alignment */
+	struct {
+		__le16 buff[3];
+		u8 temperature;
+		s64 ts __aligned(8);
+	} buffer ____cacheline_aligned;
+	__le16 status;
 };
 
 static bool bma400_is_writable_reg(struct device *dev, unsigned int reg)
@@ -152,7 +165,7 @@ static const struct iio_chan_spec_ext_info bma400_ext_info[] = {
 	{ }
 };
 
-#define BMA400_ACC_CHANNEL(_axis) { \
+#define BMA400_ACC_CHANNEL(_index, _axis) { \
 	.type = IIO_ACCEL, \
 	.modified = 1, \
 	.channel2 = IIO_MOD_##_axis, \
@@ -164,17 +177,32 @@ static const struct iio_chan_spec_ext_info bma400_ext_info[] = {
 		BIT(IIO_CHAN_INFO_SCALE) | \
 		BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
 	.ext_info = bma400_ext_info, \
+	.scan_index = _index,	\
+	.scan_type = {		\
+		.sign = 's',	\
+		.realbits = 12,		\
+		.storagebits = 16,	\
+		.endianness = IIO_LE,	\
+	},				\
 }
 
 static const struct iio_chan_spec bma400_channels[] = {
-	BMA400_ACC_CHANNEL(X),
-	BMA400_ACC_CHANNEL(Y),
-	BMA400_ACC_CHANNEL(Z),
+	BMA400_ACC_CHANNEL(0, X),
+	BMA400_ACC_CHANNEL(1, Y),
+	BMA400_ACC_CHANNEL(2, Z),
 	{
 		.type = IIO_TEMP,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.scan_index = 3,
+		.scan_type = {
+			.sign = 's',
+			.realbits = 8,
+			.storagebits = 8,
+			.endianness = IIO_LE,
+		},
 	},
+	IIO_CHAN_SOFT_TIMESTAMP(4),
 };
 
 static int bma400_get_temp_reg(struct bma400_data *data, int *val, int *val2)
@@ -659,6 +687,10 @@ static int bma400_init(struct bma400_data *data)
 	if (ret)
 		return ret;
 
+	/* Configure INT1 pin to open drain */
+	ret = regmap_write(data->regmap, BMA400_INT_IO_CTRL_REG, 0x06);
+	if (ret)
+		return ret;
 	/*
 	 * Once the interrupt engine is supported we might use the
 	 * data_src_reg, but for now ensure this is set to the
@@ -807,6 +839,29 @@ static int bma400_write_raw_get_fmt(struct iio_dev *indio_dev,
 	}
 }
 
+static int bma400_data_rdy_trigger_set_state(struct iio_trigger *trig,
+					     bool state)
+{
+	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
+	struct bma400_data *data = iio_priv(indio_dev);
+	int ret;
+
+	ret = regmap_update_bits(data->regmap, BMA400_INT_CONFIG0_REG,
+				 BMA400_INT_DRDY_MSK,
+				 FIELD_PREP(BMA400_INT_DRDY_MSK, state));
+	if (ret)
+		return ret;
+
+	return regmap_update_bits(data->regmap, BMA400_INT1_MAP_REG,
+				  BMA400_INT_DRDY_MSK,
+				  FIELD_PREP(BMA400_INT_DRDY_MSK, state));
+}
+
+static const unsigned long bma400_avail_scan_masks[] = {
+	GENMASK(3, 0),
+	0
+};
+
 static const struct iio_info bma400_info = {
 	.read_raw          = bma400_read_raw,
 	.read_avail        = bma400_read_avail,
@@ -814,7 +869,72 @@ static const struct iio_info bma400_info = {
 	.write_raw_get_fmt = bma400_write_raw_get_fmt,
 };
 
-int bma400_probe(struct device *dev, struct regmap *regmap, const char *name)
+static const struct iio_trigger_ops bma400_trigger_ops = {
+	.set_trigger_state = &bma400_data_rdy_trigger_set_state,
+	.validate_device = &iio_trigger_validate_own_device,
+};
+
+static irqreturn_t bma400_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct bma400_data *data = iio_priv(indio_dev);
+	int ret, temp;
+
+	/* Lock to protect the data->buffer */
+	mutex_lock(&data->mutex);
+
+	/* bulk read six registers, with the base being the LSB register */
+	ret = regmap_bulk_read(data->regmap, BMA400_X_AXIS_LSB_REG,
+			       &data->buffer.buff, sizeof(data->buffer.buff));
+	if (ret)
+		goto unlock_err;
+
+	ret = regmap_read(data->regmap, BMA400_TEMP_DATA_REG, &temp);
+	if (ret)
+		goto unlock_err;
+
+	data->buffer.temperature = temp;
+
+	iio_push_to_buffers_with_timestamp(indio_dev, &data->buffer,
+					   iio_get_time_ns(indio_dev));
+
+	mutex_unlock(&data->mutex);
+	iio_trigger_notify_done(indio_dev->trig);
+	return IRQ_HANDLED;
+
+unlock_err:
+	mutex_unlock(&data->mutex);
+	return IRQ_NONE;
+}
+
+static irqreturn_t bma400_interrupt(int irq, void *private)
+{
+	struct iio_dev *indio_dev = private;
+	struct bma400_data *data = iio_priv(indio_dev);
+	int ret;
+
+	/* Lock to protect the data->status */
+	mutex_lock(&data->mutex);
+	ret = regmap_bulk_read(data->regmap, BMA400_INT_STAT0_REG,
+			       &data->status,
+			       sizeof(data->status));
+	if (ret)
+		goto unlock_err;
+
+	if (FIELD_GET(BMA400_INT_DRDY_MSK, le16_to_cpu(data->status))) {
+		mutex_unlock(&data->mutex);
+		iio_trigger_poll_chained(data->trig);
+		return IRQ_HANDLED;
+	}
+
+unlock_err:
+	mutex_unlock(&data->mutex);
+	return IRQ_NONE;
+}
+
+int bma400_probe(struct device *dev, struct regmap *regmap, int irq,
+		 const char *name)
 {
 	struct iio_dev *indio_dev;
 	struct bma400_data *data;
@@ -841,8 +961,40 @@ int bma400_probe(struct device *dev, struct regmap *regmap, const char *name)
 	indio_dev->info = &bma400_info;
 	indio_dev->channels = bma400_channels;
 	indio_dev->num_channels = ARRAY_SIZE(bma400_channels);
+	indio_dev->available_scan_masks = bma400_avail_scan_masks;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
+	if (irq > 0) {
+		data->trig = devm_iio_trigger_alloc(dev, "%s-dev%d",
+						    indio_dev->name,
+						    iio_device_id(indio_dev));
+		if (!data->trig)
+			return -ENOMEM;
+
+		data->trig->ops = &bma400_trigger_ops;
+		iio_trigger_set_drvdata(data->trig, indio_dev);
+
+		ret = devm_iio_trigger_register(data->dev, data->trig);
+		if (ret)
+			return dev_err_probe(data->dev, ret,
+					     "iio trigger register fail\n");
+
+		indio_dev->trig = iio_trigger_get(data->trig);
+		ret = devm_request_threaded_irq(dev, irq, NULL,
+						&bma400_interrupt,
+						IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+						indio_dev->name, indio_dev);
+		if (ret)
+			return dev_err_probe(data->dev, ret,
+					     "request irq %d failed\n", irq);
+	}
+
+	ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
+					      &bma400_trigger_handler, NULL);
+	if (ret)
+		return dev_err_probe(data->dev, ret,
+				     "iio triggered buffer setup failed\n");
+
 	return devm_iio_device_register(dev, indio_dev);
 }
 EXPORT_SYMBOL_NS(bma400_probe, IIO_BMA400);
diff --git a/drivers/iio/accel/bma400_i2c.c b/drivers/iio/accel/bma400_i2c.c
index 4f6e01a3b3a1..1ba2a982ea73 100644
--- a/drivers/iio/accel/bma400_i2c.c
+++ b/drivers/iio/accel/bma400_i2c.c
@@ -24,7 +24,7 @@ static int bma400_i2c_probe(struct i2c_client *client,
 		return PTR_ERR(regmap);
 	}
 
-	return bma400_probe(&client->dev, regmap, id->name);
+	return bma400_probe(&client->dev, regmap, client->irq, id->name);
 }
 
 static const struct i2c_device_id bma400_i2c_ids[] = {
diff --git a/drivers/iio/accel/bma400_spi.c b/drivers/iio/accel/bma400_spi.c
index 28e240400a3f..ec13c044b304 100644
--- a/drivers/iio/accel/bma400_spi.c
+++ b/drivers/iio/accel/bma400_spi.c
@@ -84,7 +84,7 @@ static int bma400_spi_probe(struct spi_device *spi)
 	if (ret)
 		dev_err(&spi->dev, "Failed to read chip id register\n");
 
-	return bma400_probe(&spi->dev, regmap, id->name);
+	return bma400_probe(&spi->dev, regmap, spi->irq, id->name);
 }
 
 static const struct spi_device_id bma400_spi_ids[] = {
-- 
2.17.1

[PATCH v4 5/9] iio: accel: bma400: Add separate channel for step counter

[Jagath Jog J]

Added channel for step counter which can be enable or disable
through the sysfs interface.

Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
---
 drivers/iio/accel/bma400.h      |  2 +
 drivers/iio/accel/bma400_core.c | 67 +++++++++++++++++++++++++++++++--
 2 files changed, 65 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/accel/bma400.h b/drivers/iio/accel/bma400.h
index 907e1a6c0a38..32c08f8b0b98 100644
--- a/drivers/iio/accel/bma400.h
+++ b/drivers/iio/accel/bma400.h
@@ -53,6 +53,8 @@
 #define BMA400_STEP_CNT1_REG        0x16
 #define BMA400_STEP_CNT3_REG        0x17
 #define BMA400_STEP_STAT_REG        0x18
+#define BMA400_STEP_INT_MSK         BIT(0)
+#define BMA400_STEP_RAW_LEN         0x03
 
 /*
  * Read-write configuration registers
diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
index 57910ccf9180..aafb5a40944d 100644
--- a/drivers/iio/accel/bma400_core.c
+++ b/drivers/iio/accel/bma400_core.c
@@ -19,6 +19,9 @@
 #include <linux/mutex.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+
+#include <asm/unaligned.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/buffer.h>
@@ -67,6 +70,7 @@ struct bma400_data {
 	int oversampling_ratio;
 	int scale;
 	struct iio_trigger *trig;
+	int steps_enabled;
 	/* Correct time stamp alignment */
 	struct {
 		__le16 buff[3];
@@ -202,6 +206,12 @@ static const struct iio_chan_spec bma400_channels[] = {
 			.endianness = IIO_LE,
 		},
 	},
+	{
+		.type = IIO_STEPS,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
+				      BIT(IIO_CHAN_INFO_ENABLE),
+		.scan_index = -1, /* No buffer support */
+	},
 	IIO_CHAN_SOFT_TIMESTAMP(4),
 };
 
@@ -570,6 +580,38 @@ static int bma400_set_power_mode(struct bma400_data *data,
 	return 0;
 }
 
+static int bma400_enable_steps(struct bma400_data *data, int val)
+{
+	int ret;
+
+	if (data->steps_enabled == val)
+		return 0;
+
+	ret = regmap_update_bits(data->regmap, BMA400_INT_CONFIG1_REG,
+				 BMA400_STEP_INT_MSK,
+				 FIELD_PREP(BMA400_STEP_INT_MSK, !!val));
+	data->steps_enabled = val;
+	return ret;
+}
+
+static int bma400_get_steps_reg(struct bma400_data *data, int *val)
+{
+	u8 *steps_raw;
+	int ret;
+
+	steps_raw = kmalloc(BMA400_STEP_RAW_LEN, GFP_KERNEL);
+	if (!steps_raw)
+		return -ENOMEM;
+
+	ret = regmap_bulk_read(data->regmap, BMA400_STEP_CNT0_REG,
+			       steps_raw, BMA400_STEP_RAW_LEN);
+	if (ret)
+		return ret;
+	*val = get_unaligned_le24(steps_raw);
+	kfree(steps_raw);
+	return IIO_VAL_INT;
+}
+
 static void bma400_init_tables(void)
 {
 	int raw;
@@ -709,10 +751,17 @@ static int bma400_read_raw(struct iio_dev *indio_dev,
 
 	switch (mask) {
 	case IIO_CHAN_INFO_PROCESSED:
-		mutex_lock(&data->mutex);
-		ret = bma400_get_temp_reg(data, val, val2);
-		mutex_unlock(&data->mutex);
-		return ret;
+		switch (chan->type) {
+		case IIO_TEMP:
+			mutex_lock(&data->mutex);
+			ret = bma400_get_temp_reg(data, val, val2);
+			mutex_unlock(&data->mutex);
+			return ret;
+		case IIO_STEPS:
+			return bma400_get_steps_reg(data, val);
+		default:
+			return -EINVAL;
+		}
 	case IIO_CHAN_INFO_RAW:
 		mutex_lock(&data->mutex);
 		ret = bma400_get_accel_reg(data, chan, val);
@@ -753,6 +802,9 @@ static int bma400_read_raw(struct iio_dev *indio_dev,
 
 		*val = data->oversampling_ratio;
 		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_ENABLE:
+		*val = data->steps_enabled;
+		return IIO_VAL_INT;
 	default:
 		return -EINVAL;
 	}
@@ -818,6 +870,11 @@ static int bma400_write_raw(struct iio_dev *indio_dev,
 		ret = bma400_set_accel_oversampling_ratio(data, val);
 		mutex_unlock(&data->mutex);
 		return ret;
+	case IIO_CHAN_INFO_ENABLE:
+		mutex_lock(&data->mutex);
+		ret = bma400_enable_steps(data, val);
+		mutex_unlock(&data->mutex);
+		return ret;
 	default:
 		return -EINVAL;
 	}
@@ -834,6 +891,8 @@ static int bma400_write_raw_get_fmt(struct iio_dev *indio_dev,
 		return IIO_VAL_INT_PLUS_MICRO;
 	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
 		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_ENABLE:
+		return IIO_VAL_INT;
 	default:
 		return -EINVAL;
 	}
-- 
2.17.1

[PATCH v4 6/9] iio: accel: bma400: Add step change event

[Jagath Jog J]

Added support for event when there is a detection of step change.
INT1 pin is used to interrupt and event is pushed to userspace.

Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
---
 drivers/iio/accel/bma400.h      |  2 +
 drivers/iio/accel/bma400_core.c | 75 +++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+)

diff --git a/drivers/iio/accel/bma400.h b/drivers/iio/accel/bma400.h
index 32c08f8b0b98..0faa40fdbbf8 100644
--- a/drivers/iio/accel/bma400.h
+++ b/drivers/iio/accel/bma400.h
@@ -39,6 +39,7 @@
 #define BMA400_INT_STAT0_REG        0x0e
 #define BMA400_INT_STAT1_REG        0x0f
 #define BMA400_INT_STAT2_REG        0x10
+#define BMA400_INT12_MAP_REG        0x23
 
 /* Temperature register */
 #define BMA400_TEMP_DATA_REG        0x11
@@ -55,6 +56,7 @@
 #define BMA400_STEP_STAT_REG        0x18
 #define BMA400_STEP_INT_MSK         BIT(0)
 #define BMA400_STEP_RAW_LEN         0x03
+#define BMA400_STEP_STAT_MASK       GENMASK(9, 8)
 
 /*
  * Read-write configuration registers
diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
index aafb5a40944d..fe101df7b773 100644
--- a/drivers/iio/accel/bma400_core.c
+++ b/drivers/iio/accel/bma400_core.c
@@ -25,6 +25,7 @@
 
 #include <linux/iio/iio.h>
 #include <linux/iio/buffer.h>
+#include <linux/iio/events.h>
 #include <linux/iio/trigger.h>
 #include <linux/iio/trigger_consumer.h>
 #include <linux/iio/triggered_buffer.h>
@@ -71,6 +72,7 @@ struct bma400_data {
 	int scale;
 	struct iio_trigger *trig;
 	int steps_enabled;
+	bool step_event_en;
 	/* Correct time stamp alignment */
 	struct {
 		__le16 buff[3];
@@ -169,6 +171,12 @@ static const struct iio_chan_spec_ext_info bma400_ext_info[] = {
 	{ }
 };
 
+static const struct iio_event_spec bma400_step_detect_event = {
+	.type = IIO_EV_TYPE_CHANGE,
+	.dir = IIO_EV_DIR_NONE,
+	.mask_separate = BIT(IIO_EV_INFO_ENABLE),
+};
+
 #define BMA400_ACC_CHANNEL(_index, _axis) { \
 	.type = IIO_ACCEL, \
 	.modified = 1, \
@@ -211,6 +219,8 @@ static const struct iio_chan_spec bma400_channels[] = {
 		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
 				      BIT(IIO_CHAN_INFO_ENABLE),
 		.scan_index = -1, /* No buffer support */
+		.event_spec = &bma400_step_detect_event,
+		.num_event_specs = 1,
 	},
 	IIO_CHAN_SOFT_TIMESTAMP(4),
 };
@@ -898,6 +908,58 @@ static int bma400_write_raw_get_fmt(struct iio_dev *indio_dev,
 	}
 }
 
+static int bma400_read_event_config(struct iio_dev *indio_dev,
+				    const struct iio_chan_spec *chan,
+				    enum iio_event_type type,
+				    enum iio_event_direction dir)
+{
+	struct bma400_data *data = iio_priv(indio_dev);
+
+	switch (chan->type) {
+	case IIO_STEPS:
+		return data->step_event_en;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int bma400_steps_event_enable(struct bma400_data *data, int state)
+{
+	int ret;
+
+	ret = bma400_enable_steps(data, 1);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(data->regmap, BMA400_INT12_MAP_REG,
+				 BMA400_STEP_INT_MSK,
+				 FIELD_PREP(BMA400_STEP_INT_MSK,
+					    state));
+	if (ret)
+		return ret;
+	data->step_event_en = state;
+	return 0;
+}
+
+static int bma400_write_event_config(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan,
+				     enum iio_event_type type,
+				     enum iio_event_direction dir, int state)
+{
+	struct bma400_data *data = iio_priv(indio_dev);
+	int ret;
+
+	switch (chan->type) {
+	case IIO_STEPS:
+		mutex_lock(&data->mutex);
+		ret = bma400_steps_event_enable(data, state);
+		mutex_unlock(&data->mutex);
+		return ret;
+	default:
+		return -EINVAL;
+	}
+}
+
 static int bma400_data_rdy_trigger_set_state(struct iio_trigger *trig,
 					     bool state)
 {
@@ -926,6 +988,8 @@ static const struct iio_info bma400_info = {
 	.read_avail        = bma400_read_avail,
 	.write_raw         = bma400_write_raw,
 	.write_raw_get_fmt = bma400_write_raw_get_fmt,
+	.read_event_config = bma400_read_event_config,
+	.write_event_config = bma400_write_event_config,
 };
 
 static const struct iio_trigger_ops bma400_trigger_ops = {
@@ -971,6 +1035,7 @@ static irqreturn_t bma400_interrupt(int irq, void *private)
 {
 	struct iio_dev *indio_dev = private;
 	struct bma400_data *data = iio_priv(indio_dev);
+	s64 timestamp = iio_get_time_ns(indio_dev);
 	int ret;
 
 	/* Lock to protect the data->status */
@@ -981,6 +1046,16 @@ static irqreturn_t bma400_interrupt(int irq, void *private)
 	if (ret)
 		goto unlock_err;
 
+	if (FIELD_GET(BMA400_STEP_STAT_MASK, le16_to_cpu(data->status))) {
+		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),
+			       timestamp);
+		mutex_unlock(&data->mutex);
+		return IRQ_HANDLED;
+	}
+
 	if (FIELD_GET(BMA400_INT_DRDY_MSK, le16_to_cpu(data->status))) {
 		mutex_unlock(&data->mutex);
 		iio_trigger_poll_chained(data->trig);
-- 
2.17.1

[PATCH v4 7/9] iio: accel: bma400: Add activity recognition support

[Jagath Jog J]

Add support for activity recognition like STILL, WALKING, RUNNING
and these events are pushed to the userspace whenever the STEP
interrupt occurs.

Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
---
 drivers/iio/accel/bma400_core.c | 86 +++++++++++++++++++++++++++++++++
 1 file changed, 86 insertions(+)

diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
index fe101df7b773..073fff7d64a3 100644
--- a/drivers/iio/accel/bma400_core.c
+++ b/drivers/iio/accel/bma400_core.c
@@ -60,6 +60,12 @@ struct bma400_sample_freq {
 	int uhz;
 };
 
+enum bma400_activity {
+	BMA400_STILL,
+	BMA400_WALKING,
+	BMA400_RUNNING,
+};
+
 struct bma400_data {
 	struct device *dev;
 	struct regmap *regmap;
@@ -73,6 +79,7 @@ struct bma400_data {
 	struct iio_trigger *trig;
 	int steps_enabled;
 	bool step_event_en;
+	bool activity_event_en;
 	/* Correct time stamp alignment */
 	struct {
 		__le16 buff[3];
@@ -177,6 +184,12 @@ static const struct iio_event_spec bma400_step_detect_event = {
 	.mask_separate = BIT(IIO_EV_INFO_ENABLE),
 };
 
+static const struct iio_event_spec bma400_activity_event = {
+	.type = IIO_EV_TYPE_CHANGE,
+	.dir = IIO_EV_DIR_NONE,
+	.mask_shared_by_type = BIT(IIO_EV_INFO_ENABLE),
+};
+
 #define BMA400_ACC_CHANNEL(_index, _axis) { \
 	.type = IIO_ACCEL, \
 	.modified = 1, \
@@ -198,6 +211,16 @@ static const struct iio_event_spec bma400_step_detect_event = {
 	},				\
 }
 
+#define BMA400_ACTIVITY_CHANNEL(_chan2) {	\
+	.type = IIO_ACTIVITY,			\
+	.modified = 1,				\
+	.channel2 = _chan2,			\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),	\
+	.scan_index = -1, /* No buffer support */		\
+	.event_spec = &bma400_activity_event,			\
+	.num_event_specs = 1,					\
+}
+
 static const struct iio_chan_spec bma400_channels[] = {
 	BMA400_ACC_CHANNEL(0, X),
 	BMA400_ACC_CHANNEL(1, Y),
@@ -222,6 +245,9 @@ static const struct iio_chan_spec bma400_channels[] = {
 		.event_spec = &bma400_step_detect_event,
 		.num_event_specs = 1,
 	},
+	BMA400_ACTIVITY_CHANNEL(IIO_MOD_STILL),
+	BMA400_ACTIVITY_CHANNEL(IIO_MOD_WALKING),
+	BMA400_ACTIVITY_CHANNEL(IIO_MOD_RUNNING),
 	IIO_CHAN_SOFT_TIMESTAMP(4),
 };
 
@@ -660,6 +686,20 @@ static void bma400_power_disable(void *data_ptr)
 			 ERR_PTR(ret));
 }
 
+static enum iio_modifier bma400_act_to_mod(enum bma400_activity activity)
+{
+	switch (activity) {
+	case BMA400_STILL:
+		return IIO_MOD_STILL;
+	case BMA400_WALKING:
+		return IIO_MOD_WALKING;
+	case BMA400_RUNNING:
+		return IIO_MOD_RUNNING;
+	default:
+		return IIO_NO_MOD;
+	}
+}
+
 static int bma400_init(struct bma400_data *data)
 {
 	unsigned int val;
@@ -758,6 +798,7 @@ static int bma400_read_raw(struct iio_dev *indio_dev,
 {
 	struct bma400_data *data = iio_priv(indio_dev);
 	int ret;
+	unsigned int activity;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_PROCESSED:
@@ -769,6 +810,21 @@ static int bma400_read_raw(struct iio_dev *indio_dev,
 			return ret;
 		case IIO_STEPS:
 			return bma400_get_steps_reg(data, val);
+		case IIO_ACTIVITY:
+			ret = regmap_read(data->regmap, BMA400_STEP_STAT_REG,
+					  &activity);
+			if (ret)
+				return ret;
+			/*
+			 * The device does not support confidence value levels,
+			 * so we will always have 100% for current activity and
+			 * 0% for the others.
+			 */
+			if (chan->channel2 == bma400_act_to_mod(activity))
+				*val = 100;
+			else
+				*val = 0;
+			return IIO_VAL_INT;
 		default:
 			return -EINVAL;
 		}
@@ -918,6 +974,8 @@ static int bma400_read_event_config(struct iio_dev *indio_dev,
 	switch (chan->type) {
 	case IIO_STEPS:
 		return data->step_event_en;
+	case IIO_ACTIVITY:
+		return data->activity_event_en;
 	default:
 		return -EINVAL;
 	}
@@ -955,6 +1013,18 @@ static int bma400_write_event_config(struct iio_dev *indio_dev,
 		ret = bma400_steps_event_enable(data, state);
 		mutex_unlock(&data->mutex);
 		return ret;
+	case IIO_ACTIVITY:
+		mutex_lock(&data->mutex);
+		if (!data->step_event_en) {
+			ret = bma400_steps_event_enable(data, true);
+			if (ret) {
+				mutex_unlock(&data->mutex);
+				return ret;
+			}
+		}
+		data->activity_event_en = state;
+		mutex_unlock(&data->mutex);
+		return 0;
 	default:
 		return -EINVAL;
 	}
@@ -1037,6 +1107,7 @@ static irqreturn_t bma400_interrupt(int irq, void *private)
 	struct bma400_data *data = iio_priv(indio_dev);
 	s64 timestamp = iio_get_time_ns(indio_dev);
 	int ret;
+	unsigned int act;
 
 	/* Lock to protect the data->status */
 	mutex_lock(&data->mutex);
@@ -1052,6 +1123,21 @@ static irqreturn_t bma400_interrupt(int irq, void *private)
 					      IIO_EV_DIR_NONE,
 					      IIO_EV_TYPE_CHANGE, 0, 0, 0),
 			       timestamp);
+
+		if (data->activity_event_en) {
+			ret = regmap_read(data->regmap, BMA400_STEP_STAT_REG,
+					  &act);
+			if (ret)
+				goto unlock_err;
+
+			iio_push_event(indio_dev,
+				       IIO_EVENT_CODE(IIO_ACTIVITY, 0,
+						      bma400_act_to_mod(act),
+						      IIO_EV_DIR_NONE,
+						      IIO_EV_TYPE_CHANGE, 0,
+						      0, 0),
+				       timestamp);
+		}
 		mutex_unlock(&data->mutex);
 		return IRQ_HANDLED;
 	}
-- 
2.17.1

[PATCH v4 8/9] iio: accel: bma400: Add debugfs register access support

[Jagath Jog J]

Add support to read/write byte from/to bma400 device from the userspace
using debugfs interface.

Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
---
 drivers/iio/accel/bma400_core.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
index 073fff7d64a3..5b1b28972ef9 100644
--- a/drivers/iio/accel/bma400_core.c
+++ b/drivers/iio/accel/bma400_core.c
@@ -1030,6 +1030,19 @@ static int bma400_write_event_config(struct iio_dev *indio_dev,
 	}
 }
 
+static int bma400_debugfs_reg_access(struct iio_dev *indio_dev,
+				     unsigned int reg,
+				     unsigned int writeval,
+				     unsigned int *readval)
+{
+	struct bma400_data *data = iio_priv(indio_dev);
+
+	if (readval)
+		return regmap_read(data->regmap, reg, readval);
+	else
+		return regmap_write(data->regmap, reg, writeval);
+}
+
 static int bma400_data_rdy_trigger_set_state(struct iio_trigger *trig,
 					     bool state)
 {
@@ -1060,6 +1073,7 @@ static const struct iio_info bma400_info = {
 	.write_raw_get_fmt = bma400_write_raw_get_fmt,
 	.read_event_config = bma400_read_event_config,
 	.write_event_config = bma400_write_event_config,
+	.debugfs_reg_access = bma400_debugfs_reg_access,
 };
 
 static const struct iio_trigger_ops bma400_trigger_ops = {
-- 
2.17.1

[PATCH v4 9/9] iio: accel: bma400: Add support for activity and inactivity events

[Jagath Jog J]

Add support for activity and inactivity events for all axis based on the
threshold, duration and hysteresis value set from the userspace. INT1 pin
is used to interrupt and event is pushed to userspace.

Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
---
 drivers/iio/accel/bma400.h      |  11 ++
 drivers/iio/accel/bma400_core.c | 217 ++++++++++++++++++++++++++++++++
 2 files changed, 228 insertions(+)

diff --git a/drivers/iio/accel/bma400.h b/drivers/iio/accel/bma400.h
index 0faa40fdbbf8..e8f802a82300 100644
--- a/drivers/iio/accel/bma400.h
+++ b/drivers/iio/accel/bma400.h
@@ -94,6 +94,17 @@
 #define BMA400_ACC_ODR_MIN_WHOLE_HZ 25
 #define BMA400_ACC_ODR_MIN_HZ       12
 
+/* Generic interrupts register */
+#define BMA400_GEN1INT_CONFIG0      0x3f
+#define BMA400_GEN2INT_CONFIG0      0x4A
+#define BMA400_GEN_CONFIG1_OFF      0x01
+#define BMA400_GEN_CONFIG2_OFF      0x02
+#define BMA400_GEN_CONFIG3_OFF      0x03
+#define BMA400_GEN_CONFIG31_OFF     0x04
+#define BMA400_INT_GEN1_MSK         BIT(2)
+#define BMA400_INT_GEN2_MSK         BIT(3)
+#define BMA400_GEN_HYST_MSK         GENMASK(1, 0)
+
 /*
  * BMA400_SCALE_MIN macro value represents m/s^2 for 1 LSB before
  * converting to micro values for +-2g range.
diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
index 5b1b28972ef9..792336b3b9ca 100644
--- a/drivers/iio/accel/bma400_core.c
+++ b/drivers/iio/accel/bma400_core.c
@@ -80,6 +80,7 @@ struct bma400_data {
 	int steps_enabled;
 	bool step_event_en;
 	bool activity_event_en;
+	unsigned int generic_event_en;
 	/* Correct time stamp alignment */
 	struct {
 		__le16 buff[3];
@@ -87,6 +88,7 @@ struct bma400_data {
 		s64 ts __aligned(8);
 	} buffer ____cacheline_aligned;
 	__le16 status;
+	__be16 duration;
 };
 
 static bool bma400_is_writable_reg(struct device *dev, unsigned int reg)
@@ -190,6 +192,25 @@ static const struct iio_event_spec bma400_activity_event = {
 	.mask_shared_by_type = BIT(IIO_EV_INFO_ENABLE),
 };
 
+static const struct iio_event_spec bma400_accel_event[] = {
+	{
+		.type = IIO_EV_TYPE_MAG,
+		.dir = IIO_EV_DIR_FALLING,
+		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
+				       BIT(IIO_EV_INFO_PERIOD) |
+				       BIT(IIO_EV_INFO_HYSTERESIS) |
+				       BIT(IIO_EV_INFO_ENABLE),
+	},
+	{
+		.type = IIO_EV_TYPE_MAG,
+		.dir = IIO_EV_DIR_RISING,
+		.mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
+				       BIT(IIO_EV_INFO_PERIOD) |
+				       BIT(IIO_EV_INFO_HYSTERESIS) |
+				       BIT(IIO_EV_INFO_ENABLE),
+	},
+};
+
 #define BMA400_ACC_CHANNEL(_index, _axis) { \
 	.type = IIO_ACCEL, \
 	.modified = 1, \
@@ -209,6 +230,8 @@ static const struct iio_event_spec bma400_activity_event = {
 		.storagebits = 16,	\
 		.endianness = IIO_LE,	\
 	},				\
+	.event_spec = bma400_accel_event,			\
+	.num_event_specs = ARRAY_SIZE(bma400_accel_event)	\
 }
 
 #define BMA400_ACTIVITY_CHANNEL(_chan2) {	\
@@ -972,6 +995,17 @@ static int bma400_read_event_config(struct iio_dev *indio_dev,
 	struct bma400_data *data = iio_priv(indio_dev);
 
 	switch (chan->type) {
+	case IIO_ACCEL:
+		switch (dir) {
+		case IIO_EV_DIR_RISING:
+			return FIELD_GET(BMA400_INT_GEN1_MSK,
+					 data->generic_event_en);
+		case IIO_EV_DIR_FALLING:
+			return FIELD_GET(BMA400_INT_GEN2_MSK,
+					 data->generic_event_en);
+		default:
+			return -EINVAL;
+		}
 	case IIO_STEPS:
 		return data->step_event_en;
 	case IIO_ACTIVITY:
@@ -999,6 +1033,63 @@ static int bma400_steps_event_enable(struct bma400_data *data, int state)
 	return 0;
 }
 
+static int bma400_activity_event_en(struct bma400_data *data,
+				    enum iio_event_direction dir,
+				    int state)
+{
+	int ret, reg, msk, value, field_value;
+
+	switch (dir) {
+	case IIO_EV_DIR_RISING:
+		reg = BMA400_GEN1INT_CONFIG0;
+		msk = BMA400_INT_GEN1_MSK;
+		value = 2;
+		field_value = FIELD_PREP(msk, state);
+		break;
+	case IIO_EV_DIR_FALLING:
+		reg = BMA400_GEN2INT_CONFIG0;
+		msk = BMA400_INT_GEN2_MSK;
+		value = 0;
+		field_value = FIELD_PREP(msk, state);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* Enabling all axis for interrupt evaluation */
+	ret = regmap_write(data->regmap, reg, 0xF8);
+	if (ret)
+		return ret;
+
+	/* OR combination of all axis for interrupt evaluation */
+	ret = regmap_write(data->regmap, reg + BMA400_GEN_CONFIG1_OFF, value);
+	if (ret)
+		return ret;
+
+	/* Initial value to avoid interrupts while enabling*/
+	ret = regmap_write(data->regmap, reg + BMA400_GEN_CONFIG2_OFF, 0x0A);
+	if (ret)
+		return ret;
+
+	/* Initial duration value to avoid interrupts while enabling*/
+	ret = regmap_write(data->regmap, reg + BMA400_GEN_CONFIG31_OFF, 0x0F);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(data->regmap, BMA400_INT1_MAP_REG, msk,
+				 field_value);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(data->regmap, BMA400_INT_CONFIG0_REG, msk,
+				 field_value);
+	if (ret)
+		return ret;
+
+	set_mask_bits(&data->generic_event_en, msk, field_value);
+	return 0;
+}
+
 static int bma400_write_event_config(struct iio_dev *indio_dev,
 				     const struct iio_chan_spec *chan,
 				     enum iio_event_type type,
@@ -1008,6 +1099,11 @@ static int bma400_write_event_config(struct iio_dev *indio_dev,
 	int ret;
 
 	switch (chan->type) {
+	case IIO_ACCEL:
+		mutex_lock(&data->mutex);
+		ret = bma400_activity_event_en(data, dir, state);
+		mutex_unlock(&data->mutex);
+		return ret;
 	case IIO_STEPS:
 		mutex_lock(&data->mutex);
 		ret = bma400_steps_event_enable(data, state);
@@ -1030,6 +1126,108 @@ static int bma400_write_event_config(struct iio_dev *indio_dev,
 	}
 }
 
+static int get_gen_config_reg(enum iio_event_direction dir)
+{
+	switch (dir) {
+	case IIO_EV_DIR_FALLING:
+		return BMA400_GEN2INT_CONFIG0;
+	case IIO_EV_DIR_RISING:
+		return BMA400_GEN1INT_CONFIG0;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int bma400_read_event_value(struct iio_dev *indio_dev,
+				   const struct iio_chan_spec *chan,
+				   enum iio_event_type type,
+				   enum iio_event_direction dir,
+				   enum iio_event_info info,
+				   int *val, int *val2)
+{
+	struct bma400_data *data = iio_priv(indio_dev);
+	int ret, reg;
+
+	reg = get_gen_config_reg(dir);
+	if (reg < 0)
+		return -EINVAL;
+
+	*val2 = 0;
+	switch (info) {
+	case IIO_EV_INFO_VALUE:
+		ret = regmap_read(data->regmap, reg + BMA400_GEN_CONFIG2_OFF,
+				  val);
+		if (ret)
+			return ret;
+		return IIO_VAL_INT;
+	case IIO_EV_INFO_PERIOD:
+		mutex_lock(&data->mutex);
+		ret = regmap_bulk_read(data->regmap,
+				       reg + BMA400_GEN_CONFIG3_OFF,
+				       &data->duration, sizeof(data->duration));
+		if (ret) {
+			mutex_unlock(&data->mutex);
+			return ret;
+		}
+		*val = be16_to_cpu(data->duration);
+		mutex_unlock(&data->mutex);
+		return IIO_VAL_INT;
+	case IIO_EV_INFO_HYSTERESIS:
+		ret = regmap_read(data->regmap, reg, val);
+		if (ret)
+			return ret;
+		*val = FIELD_GET(BMA400_GEN_HYST_MSK, *val);
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int bma400_write_event_value(struct iio_dev *indio_dev,
+				    const struct iio_chan_spec *chan,
+				    enum iio_event_type type,
+				    enum iio_event_direction dir,
+				    enum iio_event_info info,
+				    int val, int val2)
+{
+	struct bma400_data *data = iio_priv(indio_dev);
+	int reg, ret;
+
+	reg = get_gen_config_reg(dir);
+	if (reg < 0)
+		return -EINVAL;
+
+	switch (info) {
+	case IIO_EV_INFO_VALUE:
+		if (val < 1 || val > 255)
+			return -EINVAL;
+
+		return regmap_write(data->regmap, reg + BMA400_GEN_CONFIG2_OFF,
+				    val);
+	case IIO_EV_INFO_PERIOD:
+		if (val < 1 || val > 65535)
+			return -EINVAL;
+
+		mutex_lock(&data->mutex);
+		put_unaligned_be16(val, &data->duration);
+		ret = regmap_bulk_write(data->regmap,
+					reg + BMA400_GEN_CONFIG3_OFF,
+					&data->duration,
+					sizeof(data->duration));
+		mutex_unlock(&data->mutex);
+		return ret;
+	case IIO_EV_INFO_HYSTERESIS:
+		if (val < 0 || val > 3)
+			return -EINVAL;
+
+		return regmap_update_bits(data->regmap, reg,
+					  BMA400_GEN_HYST_MSK,
+					  FIELD_PREP(BMA400_GEN_HYST_MSK, val));
+	default:
+		return -EINVAL;
+	}
+}
+
 static int bma400_debugfs_reg_access(struct iio_dev *indio_dev,
 				     unsigned int reg,
 				     unsigned int writeval,
@@ -1074,6 +1272,8 @@ static const struct iio_info bma400_info = {
 	.read_event_config = bma400_read_event_config,
 	.write_event_config = bma400_write_event_config,
 	.debugfs_reg_access = bma400_debugfs_reg_access,
+	.write_event_value = bma400_write_event_value,
+	.read_event_value = bma400_read_event_value,
 };
 
 static const struct iio_trigger_ops bma400_trigger_ops = {
@@ -1122,6 +1322,7 @@ static irqreturn_t bma400_interrupt(int irq, void *private)
 	s64 timestamp = iio_get_time_ns(indio_dev);
 	int ret;
 	unsigned int act;
+	unsigned int ev_dir = IIO_EV_DIR_NONE;
 
 	/* Lock to protect the data->status */
 	mutex_lock(&data->mutex);
@@ -1131,6 +1332,22 @@ static irqreturn_t bma400_interrupt(int irq, void *private)
 	if (ret)
 		goto unlock_err;
 
+	if (FIELD_GET(BMA400_INT_GEN1_MSK, le16_to_cpu(data->status)))
+		ev_dir = IIO_EV_DIR_RISING;
+
+	if (FIELD_GET(BMA400_INT_GEN2_MSK, le16_to_cpu(data->status)))
+		ev_dir = IIO_EV_DIR_FALLING;
+
+	if (ev_dir != IIO_EV_DIR_NONE) {
+		iio_push_event(indio_dev,
+			       IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
+						  IIO_MOD_X_OR_Y_OR_Z,
+						  IIO_EV_TYPE_MAG, ev_dir),
+			       timestamp);
+		mutex_unlock(&data->mutex);
+		return IRQ_HANDLED;
+	}
+
 	if (FIELD_GET(BMA400_STEP_STAT_MASK, le16_to_cpu(data->status))) {
 		iio_push_event(indio_dev,
 			       IIO_EVENT_CODE(IIO_STEPS, 0, IIO_NO_MOD,
-- 
2.17.1

Re: [PATCH v4 9/9] iio: accel: bma400: Add support for activity and inactivity events

[kernel test robot]

Hi Jagath,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on linus/master v5.18-rc3 next-20220420]
[cannot apply to linux/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Jagath-Jog-J/iio-accel-bma400-Add-buffer-step-and-activity-inactivity/20220421-051244
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: mips-randconfig-r031-20220420 (https://download.01.org/0day-ci/archive/20220421/202204211211.febbJ6fy-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project bac6cd5bf85669e3376610cfc4c4f9ca015e7b9b)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install mips cross compiling tool for clang build
        # apt-get install binutils-mips-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/b33d9910aa7588ec8db7c1694dbc03c3ed200ebb
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Jagath-Jog-J/iio-accel-bma400-Add-buffer-step-and-activity-inactivity/20220421-051244
        git checkout b33d9910aa7588ec8db7c1694dbc03c3ed200ebb
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/iio/accel/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/iio/accel/bma400_core.c:1047:17: warning: result of comparison of constant 18446744073709551615 with expression of type 'typeof (_Generic((msk), char: (unsigned char)0, unsigned char: (unsigned char)0, signed char: (unsigned char)0, unsigned short: (unsigned short)0, short: (unsigned short)0, unsigned int: (unsigned int)0, int: (unsigned int)0, unsigned long: (unsigned long)0, long: (unsigned long)0, unsigned long long: (unsigned long long)0, long long: (unsigned long long)0, default: (msk)))' (aka 'unsigned int') is always false [-Wtautological-constant-out-of-range-compare]
                   field_value = FIELD_PREP(msk, state);
                                 ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/bitfield.h:114:3: note: expanded from macro 'FIELD_PREP'
                   __BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: ");    \
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/bitfield.h:71:53: note: expanded from macro '__BF_FIELD_CHECK'
                   BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask) >     \
                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
   include/linux/build_bug.h:39:58: note: expanded from macro 'BUILD_BUG_ON_MSG'
   #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
                                       ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
   include/linux/compiler_types.h:352:22: note: expanded from macro 'compiletime_assert'
           _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
           ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:340:23: note: expanded from macro '_compiletime_assert'
           __compiletime_assert(condition, msg, prefix, suffix)
           ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:332:9: note: expanded from macro '__compiletime_assert'
                   if (!(condition))                                       \
                         ^~~~~~~~~
   drivers/iio/accel/bma400_core.c:1053:17: warning: result of comparison of constant 18446744073709551615 with expression of type 'typeof (_Generic((msk), char: (unsigned char)0, unsigned char: (unsigned char)0, signed char: (unsigned char)0, unsigned short: (unsigned short)0, short: (unsigned short)0, unsigned int: (unsigned int)0, int: (unsigned int)0, unsigned long: (unsigned long)0, long: (unsigned long)0, unsigned long long: (unsigned long long)0, long long: (unsigned long long)0, default: (msk)))' (aka 'unsigned int') is always false [-Wtautological-constant-out-of-range-compare]
                   field_value = FIELD_PREP(msk, state);
                                 ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/bitfield.h:114:3: note: expanded from macro 'FIELD_PREP'
                   __BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: ");    \
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/bitfield.h:71:53: note: expanded from macro '__BF_FIELD_CHECK'
                   BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask) >     \
                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
   include/linux/build_bug.h:39:58: note: expanded from macro 'BUILD_BUG_ON_MSG'
   #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
                                       ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
   include/linux/compiler_types.h:352:22: note: expanded from macro 'compiletime_assert'
           _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
           ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:340:23: note: expanded from macro '_compiletime_assert'
           __compiletime_assert(condition, msg, prefix, suffix)
           ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:332:9: note: expanded from macro '__compiletime_assert'
                   if (!(condition))                                       \
                         ^~~~~~~~~
   2 warnings generated.


vim +1047 drivers/iio/accel/bma400_core.c

  1035	
  1036	static int bma400_activity_event_en(struct bma400_data *data,
  1037					    enum iio_event_direction dir,
  1038					    int state)
  1039	{
  1040		int ret, reg, msk, value, field_value;
  1041	
  1042		switch (dir) {
  1043		case IIO_EV_DIR_RISING:
  1044			reg = BMA400_GEN1INT_CONFIG0;
  1045			msk = BMA400_INT_GEN1_MSK;
  1046			value = 2;
> 1047			field_value = FIELD_PREP(msk, state);
  1048			break;
  1049		case IIO_EV_DIR_FALLING:
  1050			reg = BMA400_GEN2INT_CONFIG0;
  1051			msk = BMA400_INT_GEN2_MSK;
  1052			value = 0;
  1053			field_value = FIELD_PREP(msk, state);
  1054			break;
  1055		default:
  1056			return -EINVAL;
  1057		}
  1058	
  1059		/* Enabling all axis for interrupt evaluation */
  1060		ret = regmap_write(data->regmap, reg, 0xF8);
  1061		if (ret)
  1062			return ret;
  1063	
  1064		/* OR combination of all axis for interrupt evaluation */
  1065		ret = regmap_write(data->regmap, reg + BMA400_GEN_CONFIG1_OFF, value);
  1066		if (ret)
  1067			return ret;
  1068	
  1069		/* Initial value to avoid interrupts while enabling*/
  1070		ret = regmap_write(data->regmap, reg + BMA400_GEN_CONFIG2_OFF, 0x0A);
  1071		if (ret)
  1072			return ret;
  1073	
  1074		/* Initial duration value to avoid interrupts while enabling*/
  1075		ret = regmap_write(data->regmap, reg + BMA400_GEN_CONFIG31_OFF, 0x0F);
  1076		if (ret)
  1077			return ret;
  1078	
  1079		ret = regmap_update_bits(data->regmap, BMA400_INT1_MAP_REG, msk,
  1080					 field_value);
  1081		if (ret)
  1082			return ret;
  1083	
  1084		ret = regmap_update_bits(data->regmap, BMA400_INT_CONFIG0_REG, msk,
  1085					 field_value);
  1086		if (ret)
  1087			return ret;
  1088	
  1089		set_mask_bits(&data->generic_event_en, msk, field_value);
  1090		return 0;
  1091	}
  1092	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH v4 9/9] iio: accel: bma400: Add support for activity and inactivity events

[Jonathan Cameron]

On Thu, 21 Apr 2022 14:45:05 +0800
kernel test robot <lkp@intel.com> wrote:

> Hi Jagath,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> [auto build test WARNING on jic23-iio/togreg]
> [also build test WARNING on linus/master v5.18-rc3 next-20220420]
> [cannot apply to linux/master]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Jagath-Jog-J/iio-accel-bma400-Add-buffer-step-and-activity-inactivity/20220421-051244
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
> config: mips-randconfig-r031-20220420 (https://download.01.org/0day-ci/archive/20220421/202204211211.febbJ6fy-lkp@intel.com/config)
> compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project bac6cd5bf85669e3376610cfc4c4f9ca015e7b9b)
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # install mips cross compiling tool for clang build
>         # apt-get install binutils-mips-linux-gnu
>         # https://github.com/intel-lab-lkp/linux/commit/b33d9910aa7588ec8db7c1694dbc03c3ed200ebb
>         git remote add linux-review https://github.com/intel-lab-lkp/linux
>         git fetch --no-tags linux-review Jagath-Jog-J/iio-accel-bma400-Add-buffer-step-and-activity-inactivity/20220421-051244
>         git checkout b33d9910aa7588ec8db7c1694dbc03c3ed200ebb
>         # save the config file
>         mkdir build_dir && cp config build_dir/.config
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/iio/accel/
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All warnings (new ones prefixed by >>):
> 
> >> drivers/iio/accel/bma400_core.c:1047:17: warning: result of comparison of constant 18446744073709551615 with expression of type 'typeof (_Generic((msk), char: (unsigned char)0, unsigned char: (unsigned char)0, signed char: (unsigned char)0, unsigned short: (unsigned short)0, short: (unsigned short)0, unsigned int: (unsigned int)0, int: (unsigned int)0, unsigned long: (unsigned long)0, long: (unsigned long)0, unsigned long long: (unsigned long long)0, long long: (unsigned long long)0, default: (msk)))' (aka 'unsigned int') is always false [-Wtautological-constant-out-of-range-compare]  
>                    field_value = FIELD_PREP(msk, state);
>                                  ^~~~~~~~~~~~~~~~~~~~~~
>    include/linux/bitfield.h:114:3: note: expanded from macro 'FIELD_PREP'
>                    __BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: ");    \
>                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    include/linux/bitfield.h:71:53: note: expanded from macro '__BF_FIELD_CHECK'
>                    BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask) >     \
>                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
>    include/linux/build_bug.h:39:58: note: expanded from macro 'BUILD_BUG_ON_MSG'
>    #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
>                                        ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
>    include/linux/compiler_types.h:352:22: note: expanded from macro 'compiletime_assert'
>            _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
>            ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    include/linux/compiler_types.h:340:23: note: expanded from macro '_compiletime_assert'
>            __compiletime_assert(condition, msg, prefix, suffix)
>            ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    include/linux/compiler_types.h:332:9: note: expanded from macro '__compiletime_assert'
>                    if (!(condition))                                       \
>                          ^~~~~~~~~
>    drivers/iio/accel/bma400_core.c:1053:17: warning: result of comparison of constant 18446744073709551615 with expression of type 'typeof (_Generic((msk), char: (unsigned char)0, unsigned char: (unsigned char)0, signed char: (unsigned char)0, unsigned short: (unsigned short)0, short: (unsigned short)0, unsigned int: (unsigned int)0, int: (unsigned int)0, unsigned long: (unsigned long)0, long: (unsigned long)0, unsigned long long: (unsigned long long)0, long long: (unsigned long long)0, default: (msk)))' (aka 'unsigned int') is always false [-Wtautological-constant-out-of-range-compare]
>                    field_value = FIELD_PREP(msk, state);
>                                  ^~~~~~~~~~~~~~~~~~~~~~
>    include/linux/bitfield.h:114:3: note: expanded from macro 'FIELD_PREP'
>                    __BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: ");    \
>                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    include/linux/bitfield.h:71:53: note: expanded from macro '__BF_FIELD_CHECK'
>                    BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask) >     \
>                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
>    include/linux/build_bug.h:39:58: note: expanded from macro 'BUILD_BUG_ON_MSG'
>    #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
>                                        ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
>    include/linux/compiler_types.h:352:22: note: expanded from macro 'compiletime_assert'
>            _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
>            ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    include/linux/compiler_types.h:340:23: note: expanded from macro '_compiletime_assert'
>            __compiletime_assert(condition, msg, prefix, suffix)
>            ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    include/linux/compiler_types.h:332:9: note: expanded from macro '__compiletime_assert'
>                    if (!(condition))                                       \
>                          ^~~~~~~~~
>    2 warnings generated.
> 
> 
> vim +1047 drivers/iio/accel/bma400_core.c
> 
>   1035	
>   1036	static int bma400_activity_event_en(struct bma400_data *data,
>   1037					    enum iio_event_direction dir,
>   1038					    int state)
>   1039	{
>   1040		int ret, reg, msk, value, field_value;
>   1041	
>   1042		switch (dir) {
>   1043		case IIO_EV_DIR_RISING:
>   1044			reg = BMA400_GEN1INT_CONFIG0;
>   1045			msk = BMA400_INT_GEN1_MSK;
>   1046			value = 2;
> > 1047			field_value = FIELD_PREP(msk, state);

Ah.  Will need to clamp state to 0/1

				field_value = FIELD_PREP(msk, state ? 1 : 0);
perhaps?
  
>   1048			break;
>   1049		case IIO_EV_DIR_FALLING:
>   1050			reg = BMA400_GEN2INT_CONFIG0;
>   1051			msk = BMA400_INT_GEN2_MSK;
>   1052			value = 0;
>   1053			field_value = FIELD_PREP(msk, state);
>   1054			break;
>   1055		default:
>   1056			return -EINVAL;
>   1057		}
>   1058	
>   1059		/* Enabling all axis for interrupt evaluation */
>   1060		ret = regmap_write(data->regmap, reg, 0xF8);
>   1061		if (ret)
>   1062			return ret;
>   1063	
>   1064		/* OR combination of all axis for interrupt evaluation */
>   1065		ret = regmap_write(data->regmap, reg + BMA400_GEN_CONFIG1_OFF, value);
>   1066		if (ret)
>   1067			return ret;
>   1068	
>   1069		/* Initial value to avoid interrupts while enabling*/
>   1070		ret = regmap_write(data->regmap, reg + BMA400_GEN_CONFIG2_OFF, 0x0A);
>   1071		if (ret)
>   1072			return ret;
>   1073	
>   1074		/* Initial duration value to avoid interrupts while enabling*/
>   1075		ret = regmap_write(data->regmap, reg + BMA400_GEN_CONFIG31_OFF, 0x0F);
>   1076		if (ret)
>   1077			return ret;
>   1078	
>   1079		ret = regmap_update_bits(data->regmap, BMA400_INT1_MAP_REG, msk,
>   1080					 field_value);
>   1081		if (ret)
>   1082			return ret;
>   1083	
>   1084		ret = regmap_update_bits(data->regmap, BMA400_INT_CONFIG0_REG, msk,
>   1085					 field_value);
>   1086		if (ret)
>   1087			return ret;
>   1088	
>   1089		set_mask_bits(&data->generic_event_en, msk, field_value);
>   1090		return 0;
>   1091	}
>   1092	
> 

Re: [PATCH v4 0/9] iio: accel: bma400: Add buffer, step and activity/inactivity

[Andy Shevchenko]

On Wed, Apr 20, 2022 at 11:11 PM Jagath Jog J <jagathjog1996@gmail.com> wrote:
>
> This patch series adds trigger buffer support with data ready interrupt,
> separate channel for step counter, an event for step change interrupt,
> activity recognition and activity/inactivity event support.

You forgot to add tags from the previous round of review, please be
respectful to reviewers.


> changes since v3
> 1. Removed all the unnecessary mutex locking for regmap.
> 2. Corrected the mutex locking and unlocking for device private data
> members.
> 3. Mutex locking and unlocking is used to protect the device private
> structure members.
> 4. Using DMA safe buffer for regmap_bulk_write() and regmap_bulk_read().
>
> 1/9: Fixed the comment.
>
> 3/9: Added () for the function name in the comment.
>
> 4/9: Handling error cases with goto in bma400_trigger_handler().
>      Mutex locking and unlocking is used to protect the data->buffer.
>      Using DMA safe buffer for regmap_bulk_read().
>      Mutex locking and unlocking is used to protect the data->status in
>      bma400_interrupt.
>
> 5/9: Using DMA safe buffers to read steps value by allocating memory internally.
>      Using DMA safe buffers for regmap_bulk_write().
>      Removed the lock for regmap().
>
> 6/9: Removed the duplication of code for enabling step, added function to handle
>      the step enable.
>
> 7/9: Removed the lock for regmap().
>      Mutex locking and unlocking is used to protect the data members.
>
> 8/9: Removed the lock for regmap().
>
> 9/9. Added __be16 duration in struct bma400_data.
>      Fixed the warning - impossible condition '(reg < 0) => (0-255 < 0)'
>      Fixed error: call to __compiletime_assert_272
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> changes since v2
> 1. Reordering of header includes in the separate patch.
> 2. Matching the IIO syntax for multiline comment.
> 3. Following the preference in the interrupt handler for returning.
> 4. Add support for activity recognition.
> 5. Add support for debugfs to access registers from userspace.
> 6. Add support for activity and inactivity events
>
> changes since v1
> 1. Added comment section that describes the math for scale calculation.
> 2. Added separate devm_add_action_or_reset() calls to disable regulator
>    and to put the sensor in power down mode.
> 3. Remove the err_reg_disable and out, goto labels and returning directly
>    if error occurs.
> 4. Added mutex calls while putting sensor in power down.
> 5. Added ___cacheline_aligned for device data.
> 6. Ordering the header includes.
> 7. Handling erroneous and spurious interrupts in the interrupt handler
>    by returning IRQ_NONE.
> 8. Using dev_err_probe() instead of dev_err().
> 9. Configured the interrupt to open drain.
> 10. Using le16_to_cpu() to fix the sparse warning.
> 11. Checking the step change event is enabled or not.
> 12. Enabling the step change event will also enable the step channel.
> 13. Using FIELD_GET() instead of bitwise operation.
> 14. Removal of dead code in the _event_config().
>
> Jagath Jog J (9):
>   iio: accel: bma400: Fix the scale min and max macro values
>   iio: accel: bma400: Reordering of header files
>   iio: accel: bma400: conversion to device-managed function
>   iio: accel: bma400: Add triggered buffer support
>   iio: accel: bma400: Add separate channel for step counter
>   iio: accel: bma400: Add step change event
>   iio: accel: bma400: Add activity recognition support
>   iio: accel: bma400: Add debugfs register access support
>   iio: accel: bma400: Add support for activity and inactivity events
>
>  drivers/iio/accel/Kconfig       |   2 +
>  drivers/iio/accel/bma400.h      |  50 ++-
>  drivers/iio/accel/bma400_core.c | 694 +++++++++++++++++++++++++++++---
>  drivers/iio/accel/bma400_i2c.c  |  10 +-
>  drivers/iio/accel/bma400_spi.c  |   8 +-
>  5 files changed, 697 insertions(+), 67 deletions(-)
>
> --
> 2.17.1
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 0/9] iio: accel: bma400: Add buffer, step and activity/inactivity

[Andy Shevchenko]

On Tue, Apr 26, 2022 at 1:04 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Wed, Apr 20, 2022 at 11:11 PM Jagath Jog J <jagathjog1996@gmail.com> wrote:
> >
> > This patch series adds trigger buffer support with data ready interrupt,
> > separate channel for step counter, an event for step change interrupt,
> > activity recognition and activity/inactivity event support.
>
> You forgot to add tags from the previous round of review, please be
> respectful to reviewers.

That said, send a new version with properly added tags for what it was given.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 0/9] iio: accel: bma400: Add buffer, step and activity/inactivity

[Jagath Jog J]

Hello Andy,

On Tue, Apr 26, 2022 at 4:35 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Tue, Apr 26, 2022 at 1:04 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> >
> > On Wed, Apr 20, 2022 at 11:11 PM Jagath Jog J <jagathjog1996@gmail.com> wrote:
> > >
> > > This patch series adds trigger buffer support with data ready interrupt,
> > > separate channel for step counter, an event for step change interrupt,
> > > activity recognition and activity/inactivity event support.
> >
> > You forgot to add tags from the previous round of review, please be
> > respectful to reviewers.
>
> That said, send a new version with properly added tags for what it was given.

I have followed your comments from previous series and added the 'Fixes' tag for
patch 1, also added your 'Reviewed-by' tag for patch number 2 and 3 for which
you have given previously.

>
> --
> With Best Regards,
> Andy Shevchenko

Re: [PATCH v4 9/9] iio: accel: bma400: Add support for activity and inactivity events

[Jagath Jog J]

Hello Jonathan,

On Sun, Apr 24, 2022 at 05:20:02PM +0100, Jonathan Cameron wrote:
> On Thu, 21 Apr 2022 14:45:05 +0800
> kernel test robot <lkp@intel.com> wrote:
> 
> > Hi Jagath,
> > 
> > Thank you for the patch! Perhaps something to improve:
> > 
> > [auto build test WARNING on jic23-iio/togreg]
> > [also build test WARNING on linus/master v5.18-rc3 next-20220420]
> > [cannot apply to linux/master]
> > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > And when submitting patch, we suggest to use '--base' as documented in
> > https://git-scm.com/docs/git-format-patch]
> > 
> > url:    https://github.com/intel-lab-lkp/linux/commits/Jagath-Jog-J/iio-accel-bma400-Add-buffer-step-and-activity-inactivity/20220421-051244
> > base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
> > config: mips-randconfig-r031-20220420 (https://download.01.org/0day-ci/archive/20220421/202204211211.febbJ6fy-lkp@intel.com/config)
> > compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project bac6cd5bf85669e3376610cfc4c4f9ca015e7b9b)
> > reproduce (this is a W=1 build):
> >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         # install mips cross compiling tool for clang build
> >         # apt-get install binutils-mips-linux-gnu
> >         # https://github.com/intel-lab-lkp/linux/commit/b33d9910aa7588ec8db7c1694dbc03c3ed200ebb
> >         git remote add linux-review https://github.com/intel-lab-lkp/linux
> >         git fetch --no-tags linux-review Jagath-Jog-J/iio-accel-bma400-Add-buffer-step-and-activity-inactivity/20220421-051244
> >         git checkout b33d9910aa7588ec8db7c1694dbc03c3ed200ebb
> >         # save the config file
> >         mkdir build_dir && cp config build_dir/.config
> >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/iio/accel/
> > 
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> > 
> > All warnings (new ones prefixed by >>):
> > 
> > >> drivers/iio/accel/bma400_core.c:1047:17: warning: result of comparison of constant 18446744073709551615 with expression of type 'typeof (_Generic((msk), char: (unsigned char)0, unsigned char: (unsigned char)0, signed char: (unsigned char)0, unsigned short: (unsigned short)0, short: (unsigned short)0, unsigned int: (unsigned int)0, int: (unsigned int)0, unsigned long: (unsigned long)0, long: (unsigned long)0, unsigned long long: (unsigned long long)0, long long: (unsigned long long)0, default: (msk)))' (aka 'unsigned int') is always false [-Wtautological-constant-out-of-range-compare]  
> >                    field_value = FIELD_PREP(msk, state);
> >                                  ^~~~~~~~~~~~~~~~~~~~~~
> >    include/linux/bitfield.h:114:3: note: expanded from macro 'FIELD_PREP'
> >                    __BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: ");    \
> >                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >    include/linux/bitfield.h:71:53: note: expanded from macro '__BF_FIELD_CHECK'
> >                    BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask) >     \
> >                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
> >    include/linux/build_bug.h:39:58: note: expanded from macro 'BUILD_BUG_ON_MSG'
> >    #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
> >                                        ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
> >    include/linux/compiler_types.h:352:22: note: expanded from macro 'compiletime_assert'
> >            _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> >            ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >    include/linux/compiler_types.h:340:23: note: expanded from macro '_compiletime_assert'
> >            __compiletime_assert(condition, msg, prefix, suffix)
> >            ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >    include/linux/compiler_types.h:332:9: note: expanded from macro '__compiletime_assert'
> >                    if (!(condition))                                       \
> >                          ^~~~~~~~~
> >    drivers/iio/accel/bma400_core.c:1053:17: warning: result of comparison of constant 18446744073709551615 with expression of type 'typeof (_Generic((msk), char: (unsigned char)0, unsigned char: (unsigned char)0, signed char: (unsigned char)0, unsigned short: (unsigned short)0, short: (unsigned short)0, unsigned int: (unsigned int)0, int: (unsigned int)0, unsigned long: (unsigned long)0, long: (unsigned long)0, unsigned long long: (unsigned long long)0, long long: (unsigned long long)0, default: (msk)))' (aka 'unsigned int') is always false [-Wtautological-constant-out-of-range-compare]
> >                    field_value = FIELD_PREP(msk, state);
> >                                  ^~~~~~~~~~~~~~~~~~~~~~
> >    include/linux/bitfield.h:114:3: note: expanded from macro 'FIELD_PREP'
> >                    __BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: ");    \
> >                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >    include/linux/bitfield.h:71:53: note: expanded from macro '__BF_FIELD_CHECK'
> >                    BUILD_BUG_ON_MSG(__bf_cast_unsigned(_mask, _mask) >     \
> >                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
> >    include/linux/build_bug.h:39:58: note: expanded from macro 'BUILD_BUG_ON_MSG'
> >    #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
> >                                        ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
> >    include/linux/compiler_types.h:352:22: note: expanded from macro 'compiletime_assert'
> >            _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> >            ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >    include/linux/compiler_types.h:340:23: note: expanded from macro '_compiletime_assert'
> >            __compiletime_assert(condition, msg, prefix, suffix)
> >            ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >    include/linux/compiler_types.h:332:9: note: expanded from macro '__compiletime_assert'
> >                    if (!(condition))                                       \
> >                          ^~~~~~~~~
> >    2 warnings generated.
> > 
> > 
> > vim +1047 drivers/iio/accel/bma400_core.c
> > 
> >   1035	
> >   1036	static int bma400_activity_event_en(struct bma400_data *data,
> >   1037					    enum iio_event_direction dir,
> >   1038					    int state)
> >   1039	{
> >   1040		int ret, reg, msk, value, field_value;
> >   1041	
> >   1042		switch (dir) {
> >   1043		case IIO_EV_DIR_RISING:
> >   1044			reg = BMA400_GEN1INT_CONFIG0;
> >   1045			msk = BMA400_INT_GEN1_MSK;
> >   1046			value = 2;
> > > 1047			field_value = FIELD_PREP(msk, state);
> 
> Ah.  Will need to clamp state to 0/1
> 
> 				field_value = FIELD_PREP(msk, state ? 1 : 0);
> perhaps?

For this also compiler shows warning.

To compile for mips I used
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir
ARCH=mips SHELL=/bin/bash drivers/iio/accel/

../drivers/iio/accel/bma400_core.c:1047:17: warning: result of comparison of
constant 18446744073709551615 with expression of type 'typeof (_Generic((msk)...

                field_value = FIELD_PREP(msk, state ? 1 : 0);
                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../include/linux/bitfield.h:114:3: note: expanded from macro 'FIELD_PREP'
                __BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: ");    \



To avoid warning can I do like this
field_value = FIELD_PREP(BMA400_INT_GEN1_MSK, state);

>   
> >   1048			break;
> >   1049		case IIO_EV_DIR_FALLING:
> >   1050			reg = BMA400_GEN2INT_CONFIG0;
> >   1051			msk = BMA400_INT_GEN2_MSK;
> >   1052			value = 0;
> >   1053			field_value = FIELD_PREP(msk, state);
> >   1054			break;
> >   1055		default:
> >   1056			return -EINVAL;
> >   1057		}
> >   1058	
> >   1059		/* Enabling all axis for interrupt evaluation */
> >   1060		ret = regmap_write(data->regmap, reg, 0xF8);
> >   1061		if (ret)
> >   1062			return ret;
> >   1063	
> >   1064		/* OR combination of all axis for interrupt evaluation */
> >   1065		ret = regmap_write(data->regmap, reg + BMA400_GEN_CONFIG1_OFF, value);
> >   1066		if (ret)
> >   1067			return ret;
> >   1068	
> >   1069		/* Initial value to avoid interrupts while enabling*/
> >   1070		ret = regmap_write(data->regmap, reg + BMA400_GEN_CONFIG2_OFF, 0x0A);
> >   1071		if (ret)
> >   1072			return ret;
> >   1073	
> >   1074		/* Initial duration value to avoid interrupts while enabling*/
> >   1075		ret = regmap_write(data->regmap, reg + BMA400_GEN_CONFIG31_OFF, 0x0F);
> >   1076		if (ret)
> >   1077			return ret;
> >   1078	
> >   1079		ret = regmap_update_bits(data->regmap, BMA400_INT1_MAP_REG, msk,
> >   1080					 field_value);
> >   1081		if (ret)
> >   1082			return ret;
> >   1083	
> >   1084		ret = regmap_update_bits(data->regmap, BMA400_INT_CONFIG0_REG, msk,
> >   1085					 field_value);
> >   1086		if (ret)
> >   1087			return ret;
> >   1088	
> >   1089		set_mask_bits(&data->generic_event_en, msk, field_value);
> >   1090		return 0;
> >   1091	}
> >   1092	
> > 
> 


Thank you,
Jagath

Re: [PATCH v4 1/9] iio: accel: bma400: Fix the scale min and max macro values

[Andy Shevchenko]

On Wed, Apr 20, 2022 at 11:11 PM Jagath Jog J <jagathjog1996@gmail.com> wrote:
>
> Changing the scale macro values to match the bma400 sensitivity
> for 1 LSB of all the available ranges.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Fixes: 465c811f1f20 ("iio: accel: Add driver for the BMA400")
> Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
> ---
>  drivers/iio/accel/bma400.h | 23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/accel/bma400.h b/drivers/iio/accel/bma400.h
> index c4c8d74155c2..80330c7ce17f 100644
> --- a/drivers/iio/accel/bma400.h
> +++ b/drivers/iio/accel/bma400.h
> @@ -83,8 +83,27 @@
>  #define BMA400_ACC_ODR_MIN_WHOLE_HZ 25
>  #define BMA400_ACC_ODR_MIN_HZ       12
>
> -#define BMA400_SCALE_MIN            38357
> -#define BMA400_SCALE_MAX            306864
> +/*
> + * BMA400_SCALE_MIN macro value represents m/s^2 for 1 LSB before
> + * converting to micro values for +-2g range.
> + *
> + * For +-2g - 1 LSB = 0.976562 milli g = 0.009576 m/s^2
> + * For +-4g - 1 LSB = 1.953125 milli g = 0.019153 m/s^2
> + * For +-16g - 1 LSB = 7.8125 milli g = 0.076614 m/s^2
> + *
> + * The raw value which is used to select the different ranges is determined
> + * by the first bit set position from the scale value, so BMA400_SCALE_MIN
> + * should be odd.
> + *
> + * Scale values for +-2g, +-4g, +-8g and +-16g are populated into bma400_scales
> + * array by left shifting BMA400_SCALE_MIN.
> + * e.g.:
> + * To select +-2g = 9577 << 0 = raw value to write is 0.
> + * To select +-8g = 9577 << 2 = raw value to write is 2.
> + * To select +-16g = 9577 << 3 = raw value to write is 3.
> + */
> +#define BMA400_SCALE_MIN            9577
> +#define BMA400_SCALE_MAX            76617
>
>  #define BMA400_NUM_REGULATORS       2
>  #define BMA400_VDD_REGULATOR        0
> --
> 2.17.1
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 5/9] iio: accel: bma400: Add separate channel for step counter

[Andy Shevchenko]

On Wed, Apr 20, 2022 at 11:11 PM Jagath Jog J <jagathjog1996@gmail.com> wrote:
>
> Added channel for step counter which can be enable or disable
> through the sysfs interface.

...

> +static int bma400_enable_steps(struct bma400_data *data, int val)
> +{
> +       int ret;
> +
> +       if (data->steps_enabled == val)
> +               return 0;
> +
> +       ret = regmap_update_bits(data->regmap, BMA400_INT_CONFIG1_REG,
> +                                BMA400_STEP_INT_MSK,
> +                                FIELD_PREP(BMA400_STEP_INT_MSK, !!val));

> +       data->steps_enabled = val;

This will update the value even if we got an error and actual device
state is unknown here. Does this make sense?

> +       return ret;
> +}

...

I perhaps missed why kmalloc() is needed now. Any pointers to the discussion?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 4/9] iio: accel: bma400: Add triggered buffer support

[Andy Shevchenko]

On Wed, Apr 20, 2022 at 11:11 PM Jagath Jog J <jagathjog1996@gmail.com> wrote:
>
> Added trigger buffer support to read continuous acceleration
> data from device with data ready interrupt which is mapped
> to INT1 pin.

LGTM,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
> ---
>  drivers/iio/accel/Kconfig       |   2 +
>  drivers/iio/accel/bma400.h      |  10 +-
>  drivers/iio/accel/bma400_core.c | 162 +++++++++++++++++++++++++++++++-
>  drivers/iio/accel/bma400_i2c.c  |   2 +-
>  drivers/iio/accel/bma400_spi.c  |   2 +-
>  5 files changed, 170 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index eac3f02662ae..958097814232 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -204,6 +204,8 @@ config BMA220
>  config BMA400
>         tristate "Bosch BMA400 3-Axis Accelerometer Driver"
>         select REGMAP
> +       select IIO_BUFFER
> +       select IIO_TRIGGERED_BUFFER
>         select BMA400_I2C if I2C
>         select BMA400_SPI if SPI
>         help
> diff --git a/drivers/iio/accel/bma400.h b/drivers/iio/accel/bma400.h
> index 1c8c47a9a317..907e1a6c0a38 100644
> --- a/drivers/iio/accel/bma400.h
> +++ b/drivers/iio/accel/bma400.h
> @@ -62,6 +62,13 @@
>  #define BMA400_ACC_CONFIG2_REG      0x1b
>  #define BMA400_CMD_REG              0x7e
>
> +/* Interrupt registers */
> +#define BMA400_INT_CONFIG0_REG     0x1f
> +#define BMA400_INT_CONFIG1_REG     0x20
> +#define BMA400_INT1_MAP_REG        0x21
> +#define BMA400_INT_IO_CTRL_REG     0x24
> +#define BMA400_INT_DRDY_MSK        BIT(7)
> +
>  /* Chip ID of BMA 400 devices found in the chip ID register. */
>  #define BMA400_ID_REG_VAL           0x90
>
> @@ -111,6 +118,7 @@
>
>  extern const struct regmap_config bma400_regmap_config;
>
> -int bma400_probe(struct device *dev, struct regmap *regmap, const char *name);
> +int bma400_probe(struct device *dev, struct regmap *regmap, int irq,
> +                const char *name);
>
>  #endif
> diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
> index 07674d89d978..57910ccf9180 100644
> --- a/drivers/iio/accel/bma400_core.c
> +++ b/drivers/iio/accel/bma400_core.c
> @@ -11,6 +11,7 @@
>   *  - Create channel for sensor time
>   */
>
> +#include <linux/bitfield.h>
>  #include <linux/bitops.h>
>  #include <linux/device.h>
>  #include <linux/kernel.h>
> @@ -20,6 +21,10 @@
>  #include <linux/regulator/consumer.h>
>
>  #include <linux/iio/iio.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
>
>  #include "bma400.h"
>
> @@ -61,6 +66,14 @@ struct bma400_data {
>         struct bma400_sample_freq sample_freq;
>         int oversampling_ratio;
>         int scale;
> +       struct iio_trigger *trig;
> +       /* Correct time stamp alignment */
> +       struct {
> +               __le16 buff[3];
> +               u8 temperature;
> +               s64 ts __aligned(8);
> +       } buffer ____cacheline_aligned;
> +       __le16 status;
>  };
>
>  static bool bma400_is_writable_reg(struct device *dev, unsigned int reg)
> @@ -152,7 +165,7 @@ static const struct iio_chan_spec_ext_info bma400_ext_info[] = {
>         { }
>  };
>
> -#define BMA400_ACC_CHANNEL(_axis) { \
> +#define BMA400_ACC_CHANNEL(_index, _axis) { \
>         .type = IIO_ACCEL, \
>         .modified = 1, \
>         .channel2 = IIO_MOD_##_axis, \
> @@ -164,17 +177,32 @@ static const struct iio_chan_spec_ext_info bma400_ext_info[] = {
>                 BIT(IIO_CHAN_INFO_SCALE) | \
>                 BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
>         .ext_info = bma400_ext_info, \
> +       .scan_index = _index,   \
> +       .scan_type = {          \
> +               .sign = 's',    \
> +               .realbits = 12,         \
> +               .storagebits = 16,      \
> +               .endianness = IIO_LE,   \
> +       },                              \
>  }
>
>  static const struct iio_chan_spec bma400_channels[] = {
> -       BMA400_ACC_CHANNEL(X),
> -       BMA400_ACC_CHANNEL(Y),
> -       BMA400_ACC_CHANNEL(Z),
> +       BMA400_ACC_CHANNEL(0, X),
> +       BMA400_ACC_CHANNEL(1, Y),
> +       BMA400_ACC_CHANNEL(2, Z),
>         {
>                 .type = IIO_TEMP,
>                 .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
>                 .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> +               .scan_index = 3,
> +               .scan_type = {
> +                       .sign = 's',
> +                       .realbits = 8,
> +                       .storagebits = 8,
> +                       .endianness = IIO_LE,
> +               },
>         },
> +       IIO_CHAN_SOFT_TIMESTAMP(4),
>  };
>
>  static int bma400_get_temp_reg(struct bma400_data *data, int *val, int *val2)
> @@ -659,6 +687,10 @@ static int bma400_init(struct bma400_data *data)
>         if (ret)
>                 return ret;
>
> +       /* Configure INT1 pin to open drain */
> +       ret = regmap_write(data->regmap, BMA400_INT_IO_CTRL_REG, 0x06);
> +       if (ret)
> +               return ret;
>         /*
>          * Once the interrupt engine is supported we might use the
>          * data_src_reg, but for now ensure this is set to the
> @@ -807,6 +839,29 @@ static int bma400_write_raw_get_fmt(struct iio_dev *indio_dev,
>         }
>  }
>
> +static int bma400_data_rdy_trigger_set_state(struct iio_trigger *trig,
> +                                            bool state)
> +{
> +       struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
> +       struct bma400_data *data = iio_priv(indio_dev);
> +       int ret;
> +
> +       ret = regmap_update_bits(data->regmap, BMA400_INT_CONFIG0_REG,
> +                                BMA400_INT_DRDY_MSK,
> +                                FIELD_PREP(BMA400_INT_DRDY_MSK, state));
> +       if (ret)
> +               return ret;
> +
> +       return regmap_update_bits(data->regmap, BMA400_INT1_MAP_REG,
> +                                 BMA400_INT_DRDY_MSK,
> +                                 FIELD_PREP(BMA400_INT_DRDY_MSK, state));
> +}
> +
> +static const unsigned long bma400_avail_scan_masks[] = {
> +       GENMASK(3, 0),
> +       0
> +};
> +
>  static const struct iio_info bma400_info = {
>         .read_raw          = bma400_read_raw,
>         .read_avail        = bma400_read_avail,
> @@ -814,7 +869,72 @@ static const struct iio_info bma400_info = {
>         .write_raw_get_fmt = bma400_write_raw_get_fmt,
>  };
>
> -int bma400_probe(struct device *dev, struct regmap *regmap, const char *name)
> +static const struct iio_trigger_ops bma400_trigger_ops = {
> +       .set_trigger_state = &bma400_data_rdy_trigger_set_state,
> +       .validate_device = &iio_trigger_validate_own_device,
> +};
> +
> +static irqreturn_t bma400_trigger_handler(int irq, void *p)
> +{
> +       struct iio_poll_func *pf = p;
> +       struct iio_dev *indio_dev = pf->indio_dev;
> +       struct bma400_data *data = iio_priv(indio_dev);
> +       int ret, temp;
> +
> +       /* Lock to protect the data->buffer */
> +       mutex_lock(&data->mutex);
> +
> +       /* bulk read six registers, with the base being the LSB register */
> +       ret = regmap_bulk_read(data->regmap, BMA400_X_AXIS_LSB_REG,
> +                              &data->buffer.buff, sizeof(data->buffer.buff));
> +       if (ret)
> +               goto unlock_err;
> +
> +       ret = regmap_read(data->regmap, BMA400_TEMP_DATA_REG, &temp);
> +       if (ret)
> +               goto unlock_err;
> +
> +       data->buffer.temperature = temp;
> +
> +       iio_push_to_buffers_with_timestamp(indio_dev, &data->buffer,
> +                                          iio_get_time_ns(indio_dev));
> +
> +       mutex_unlock(&data->mutex);
> +       iio_trigger_notify_done(indio_dev->trig);
> +       return IRQ_HANDLED;
> +
> +unlock_err:
> +       mutex_unlock(&data->mutex);
> +       return IRQ_NONE;
> +}
> +
> +static irqreturn_t bma400_interrupt(int irq, void *private)
> +{
> +       struct iio_dev *indio_dev = private;
> +       struct bma400_data *data = iio_priv(indio_dev);
> +       int ret;
> +
> +       /* Lock to protect the data->status */
> +       mutex_lock(&data->mutex);
> +       ret = regmap_bulk_read(data->regmap, BMA400_INT_STAT0_REG,
> +                              &data->status,
> +                              sizeof(data->status));
> +       if (ret)
> +               goto unlock_err;
> +
> +       if (FIELD_GET(BMA400_INT_DRDY_MSK, le16_to_cpu(data->status))) {
> +               mutex_unlock(&data->mutex);
> +               iio_trigger_poll_chained(data->trig);
> +               return IRQ_HANDLED;
> +       }
> +
> +unlock_err:
> +       mutex_unlock(&data->mutex);
> +       return IRQ_NONE;
> +}
> +
> +int bma400_probe(struct device *dev, struct regmap *regmap, int irq,
> +                const char *name)
>  {
>         struct iio_dev *indio_dev;
>         struct bma400_data *data;
> @@ -841,8 +961,40 @@ int bma400_probe(struct device *dev, struct regmap *regmap, const char *name)
>         indio_dev->info = &bma400_info;
>         indio_dev->channels = bma400_channels;
>         indio_dev->num_channels = ARRAY_SIZE(bma400_channels);
> +       indio_dev->available_scan_masks = bma400_avail_scan_masks;
>         indio_dev->modes = INDIO_DIRECT_MODE;
>
> +       if (irq > 0) {
> +               data->trig = devm_iio_trigger_alloc(dev, "%s-dev%d",
> +                                                   indio_dev->name,
> +                                                   iio_device_id(indio_dev));
> +               if (!data->trig)
> +                       return -ENOMEM;
> +
> +               data->trig->ops = &bma400_trigger_ops;
> +               iio_trigger_set_drvdata(data->trig, indio_dev);
> +
> +               ret = devm_iio_trigger_register(data->dev, data->trig);
> +               if (ret)
> +                       return dev_err_probe(data->dev, ret,
> +                                            "iio trigger register fail\n");
> +
> +               indio_dev->trig = iio_trigger_get(data->trig);
> +               ret = devm_request_threaded_irq(dev, irq, NULL,
> +                                               &bma400_interrupt,
> +                                               IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> +                                               indio_dev->name, indio_dev);
> +               if (ret)
> +                       return dev_err_probe(data->dev, ret,
> +                                            "request irq %d failed\n", irq);
> +       }
> +
> +       ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
> +                                             &bma400_trigger_handler, NULL);
> +       if (ret)
> +               return dev_err_probe(data->dev, ret,
> +                                    "iio triggered buffer setup failed\n");
> +
>         return devm_iio_device_register(dev, indio_dev);
>  }
>  EXPORT_SYMBOL_NS(bma400_probe, IIO_BMA400);
> diff --git a/drivers/iio/accel/bma400_i2c.c b/drivers/iio/accel/bma400_i2c.c
> index 4f6e01a3b3a1..1ba2a982ea73 100644
> --- a/drivers/iio/accel/bma400_i2c.c
> +++ b/drivers/iio/accel/bma400_i2c.c
> @@ -24,7 +24,7 @@ static int bma400_i2c_probe(struct i2c_client *client,
>                 return PTR_ERR(regmap);
>         }
>
> -       return bma400_probe(&client->dev, regmap, id->name);
> +       return bma400_probe(&client->dev, regmap, client->irq, id->name);
>  }
>
>  static const struct i2c_device_id bma400_i2c_ids[] = {
> diff --git a/drivers/iio/accel/bma400_spi.c b/drivers/iio/accel/bma400_spi.c
> index 28e240400a3f..ec13c044b304 100644
> --- a/drivers/iio/accel/bma400_spi.c
> +++ b/drivers/iio/accel/bma400_spi.c
> @@ -84,7 +84,7 @@ static int bma400_spi_probe(struct spi_device *spi)
>         if (ret)
>                 dev_err(&spi->dev, "Failed to read chip id register\n");
>
> -       return bma400_probe(&spi->dev, regmap, id->name);
> +       return bma400_probe(&spi->dev, regmap, spi->irq, id->name);
>  }
>
>  static const struct spi_device_id bma400_spi_ids[] = {
> --
> 2.17.1
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 9/9] iio: accel: bma400: Add support for activity and inactivity events

[Andy Shevchenko]

On Wed, Apr 27, 2022 at 5:01 AM Jagath Jog J <jagathjog1996@gmail.com> wrote:
> On Sun, Apr 24, 2022 at 05:20:02PM +0100, Jonathan Cameron wrote:
> > On Thu, 21 Apr 2022 14:45:05 +0800
> > kernel test robot <lkp@intel.com> wrote:
> > > Thank you for the patch! Perhaps something to improve:

...

> To avoid warning can I do like this
> field_value = FIELD_PREP(BMA400_INT_GEN1_MSK, state);


Can the same be used as below ?

> > >   1089              set_mask_bits(&data->generic_event_en, msk, field_value);

In other words, look for function macros in the bitfield.h.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 5/9] iio: accel: bma400: Add separate channel for step counter

[Jagath Jog J]

Hi Andy,

On Wed, Apr 27, 2022 at 6:04 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Wed, Apr 20, 2022 at 11:11 PM Jagath Jog J <jagathjog1996@gmail.com> wrote:
> >
> > Added channel for step counter which can be enable or disable
> > through the sysfs interface.
>
> ...
>
> > +static int bma400_enable_steps(struct bma400_data *data, int val)
> > +{
> > +       int ret;
> > +
> > +       if (data->steps_enabled == val)
> > +               return 0;
> > +
> > +       ret = regmap_update_bits(data->regmap, BMA400_INT_CONFIG1_REG,
> > +                                BMA400_STEP_INT_MSK,
> > +                                FIELD_PREP(BMA400_STEP_INT_MSK, !!val));
>
> > +       data->steps_enabled = val;
>
> This will update the value even if we got an error and actual device
> state is unknown here. Does this make sense?

I will correct this in the next series.

>
> > +       return ret;
> > +}
>
> ...
>
> I perhaps missed why kmalloc() is needed now. Any pointers to the discussion?

Here step is a 24-bit value and since this is a sysfs channel read (slow path),
kmalloc() is used to make the buffer DMA safe to read the multibyte value
using regmap_bulk_read().

>
> --
> With Best Regards,
> Andy Shevchenko

Thank you,
Jagath

Re: [PATCH v4 4/9] iio: accel: bma400: Add triggered buffer support

[Jonathan Cameron]

On Wed, 27 Apr 2022 14:34:57 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Wed, Apr 20, 2022 at 11:11 PM Jagath Jog J <jagathjog1996@gmail.com> wrote:
> >
> > Added trigger buffer support to read continuous acceleration
> > data from device with data ready interrupt which is mapped
> > to INT1 pin.  
> 
> LGTM,
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Agreed.  A couple of 'comments' inline but no actual need to change anything.
One is contingent on a fix I've not sent out yet for the rest of IIO.
The other is potentially a minor improvement for the future.

Thanks,

Jonathan

> 
> > Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
> > ---
> >  drivers/iio/accel/Kconfig       |   2 +
> >  drivers/iio/accel/bma400.h      |  10 +-
> >  drivers/iio/accel/bma400_core.c | 162 +++++++++++++++++++++++++++++++-
> >  drivers/iio/accel/bma400_i2c.c  |   2 +-
> >  drivers/iio/accel/bma400_spi.c  |   2 +-
> >  5 files changed, 170 insertions(+), 8 deletions(-)

> >
> >  #include "bma400.h"
> >
> > @@ -61,6 +66,14 @@ struct bma400_data {
> >         struct bma400_sample_freq sample_freq;
> >         int oversampling_ratio;
> >         int scale;
> > +       struct iio_trigger *trig;
> > +       /* Correct time stamp alignment */
> > +       struct {
> > +               __le16 buff[3];
> > +               u8 temperature;
> > +               s64 ts __aligned(8);
> > +       } buffer ____cacheline_aligned;

If you are rolling again, could you change this to
__aligned(IIO_ALIGN);  See
https://lore.kernel.org/linux-iio/20220419121241.00002e42@Huawei.com/
for why.
Note that I'll be sending a fix patch out for IIO_ALIGN to define
it as ARCH_KMALLOC_ALIGN in next few days.

If you'd pref not to get caught up in that, send it as it stands
and I'll fix up once that fix is in place.  What's one more driver
on top of the 80+ I have to do anyway :)



> > +       __le16 status;
> >  };
> >

> > +
> > +static const unsigned long bma400_avail_scan_masks[] = {
> > +       GENMASK(3, 0),
> > +       0
> > +};
> > +
> >  static const struct iio_info bma400_info = {
> >         .read_raw          = bma400_read_raw,
> >         .read_avail        = bma400_read_avail,
> > @@ -814,7 +869,72 @@ static const struct iio_info bma400_info = {
> >         .write_raw_get_fmt = bma400_write_raw_get_fmt,
> >  };
> >
> > -int bma400_probe(struct device *dev, struct regmap *regmap, const char *name)
> > +static const struct iio_trigger_ops bma400_trigger_ops = {
> > +       .set_trigger_state = &bma400_data_rdy_trigger_set_state,
> > +       .validate_device = &iio_trigger_validate_own_device,
> > +};
> > +
> > +static irqreturn_t bma400_trigger_handler(int irq, void *p)
> > +{
> > +       struct iio_poll_func *pf = p;
> > +       struct iio_dev *indio_dev = pf->indio_dev;
> > +       struct bma400_data *data = iio_priv(indio_dev);
> > +       int ret, temp;
> > +
> > +       /* Lock to protect the data->buffer */
> > +       mutex_lock(&data->mutex);
> > +
> > +       /* bulk read six registers, with the base being the LSB register */
> > +       ret = regmap_bulk_read(data->regmap, BMA400_X_AXIS_LSB_REG,
> > +                              &data->buffer.buff, sizeof(data->buffer.buff));
> > +       if (ret)
> > +               goto unlock_err;
> > +
> > +       ret = regmap_read(data->regmap, BMA400_TEMP_DATA_REG, &temp);

Given the temperature read is a separate action, it seems like you could sensible
add another entry to bma400_avail_scan_masks() for just the accelerometer axis
and then only perform this read if the temperature is requested.

It would be a feature though, so no need to have it in this patch if you
prefer not to.

> > +       if (ret)
> > +               goto unlock_err;
> > +
> > +       data->buffer.temperature = temp;
> > +
> > +       iio_push_to_buffers_with_timestamp(indio_dev, &data->buffer,
> > +                                          iio_get_time_ns(indio_dev));
> > +
> > +       mutex_unlock(&data->mutex);
> > +       iio_trigger_notify_done(indio_dev->trig);
> > +       return IRQ_HANDLED;
> > +
> > +unlock_err:
> > +       mutex_unlock(&data->mutex);
> > +       return IRQ_NONE;
> > +}

Re: [PATCH v4 6/9] iio: accel: bma400: Add step change event

[Jonathan Cameron]

On Thu, 21 Apr 2022 02:41:02 +0530
Jagath Jog J <jagathjog1996@gmail.com> wrote:

> Added support for event when there is a detection of step change.
> INT1 pin is used to interrupt and event is pushed to userspace.
> 
> Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
Hi Jagath,

A query about handling of multiple interrupts...

> ---
>  drivers/iio/accel/bma400.h      |  2 +
>  drivers/iio/accel/bma400_core.c | 75 +++++++++++++++++++++++++++++++++
>  2 files changed, 77 insertions(+)
> 
>   * Read-write configuration registers
> diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
> index aafb5a40944d..fe101df7b773 100644
> --- a/drivers/iio/accel/bma400_core.c
> +++ b/drivers/iio/accel/bma400_core.c

>  
>  static const struct iio_trigger_ops bma400_trigger_ops = {
> @@ -971,6 +1035,7 @@ static irqreturn_t bma400_interrupt(int irq, void *private)
>  {
>  	struct iio_dev *indio_dev = private;
>  	struct bma400_data *data = iio_priv(indio_dev);
> +	s64 timestamp = iio_get_time_ns(indio_dev);
>  	int ret;
>  
>  	/* Lock to protect the data->status */
> @@ -981,6 +1046,16 @@ static irqreturn_t bma400_interrupt(int irq, void *private)
>  	if (ret)
>  		goto unlock_err;
>  
> +	if (FIELD_GET(BMA400_STEP_STAT_MASK, le16_to_cpu(data->status))) {
> +		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),
> +			       timestamp);
> +		mutex_unlock(&data->mutex);

Is it possible for two interrupt sources to be active at the same time?
Given the device is clearing interrupts on read (which is unusual enough to
make me check that on the datasheet) you will loose any other events.

Normal trick is to act on all set bits and if any of them were acted on
return HANDLED.

> +		return IRQ_HANDLED;
> +	}
> +
>  	if (FIELD_GET(BMA400_INT_DRDY_MSK, le16_to_cpu(data->status))) {
>  		mutex_unlock(&data->mutex);
>  		iio_trigger_poll_chained(data->trig);

Re: [PATCH v4 8/9] iio: accel: bma400: Add debugfs register access support

[Jonathan Cameron]

On Thu, 21 Apr 2022 02:41:04 +0530
Jagath Jog J <jagathjog1996@gmail.com> wrote:

> Add support to read/write byte from/to bma400 device from the userspace
> using debugfs interface.
> 
> Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
> ---
>  drivers/iio/accel/bma400_core.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
> index 073fff7d64a3..5b1b28972ef9 100644
> --- a/drivers/iio/accel/bma400_core.c
> +++ b/drivers/iio/accel/bma400_core.c
> @@ -1030,6 +1030,19 @@ static int bma400_write_event_config(struct iio_dev *indio_dev,
>  	}
>  }
>  
> +static int bma400_debugfs_reg_access(struct iio_dev *indio_dev,
> +				     unsigned int reg,
> +				     unsigned int writeval,
> +				     unsigned int *readval)
> +{
> +	struct bma400_data *data = iio_priv(indio_dev);
> +
> +	if (readval)
> +		return regmap_read(data->regmap, reg, readval);
> +	else
> +		return regmap_write(data->regmap, reg, writeval);

Hmm. So normally reads are safe, but not on this device because a read
of the status register has side effects.  As such, I'd either block
reading that register or simply not provide debugfs access at all.

Writes are often likely to break things, so users tend to be aware
of that...

Jonathan

> +}
> +
>  static int bma400_data_rdy_trigger_set_state(struct iio_trigger *trig,
>  					     bool state)
>  {
> @@ -1060,6 +1073,7 @@ static const struct iio_info bma400_info = {
>  	.write_raw_get_fmt = bma400_write_raw_get_fmt,
>  	.read_event_config = bma400_read_event_config,
>  	.write_event_config = bma400_write_event_config,
> +	.debugfs_reg_access = bma400_debugfs_reg_access,
>  };
>  
>  static const struct iio_trigger_ops bma400_trigger_ops = {

Re: [PATCH v4 0/9] iio: accel: bma400: Add buffer, step and activity/inactivity

[Jonathan Cameron]

On Thu, 21 Apr 2022 02:40:56 +0530
Jagath Jog J <jagathjog1996@gmail.com> wrote:

> This patch series adds trigger buffer support with data ready interrupt,
> separate channel for step counter, an event for step change interrupt,
> activity recognition and activity/inactivity event support.

Hi Jagath,

This is coming together nicely. I'm fine with all the patches
I haven't sent specific replies for.

Thanks,

Jonathan

> 
> changes since v3
> 1. Removed all the unnecessary mutex locking for regmap.
> 2. Corrected the mutex locking and unlocking for device private data
> members.
> 3. Mutex locking and unlocking is used to protect the device private
> structure members.
> 4. Using DMA safe buffer for regmap_bulk_write() and regmap_bulk_read().
> 
> 1/9: Fixed the comment.
> 
> 3/9: Added () for the function name in the comment.
> 
> 4/9: Handling error cases with goto in bma400_trigger_handler().
>      Mutex locking and unlocking is used to protect the data->buffer.
>      Using DMA safe buffer for regmap_bulk_read().
>      Mutex locking and unlocking is used to protect the data->status in
>      bma400_interrupt.
> 
> 5/9: Using DMA safe buffers to read steps value by allocating memory internally.
>      Using DMA safe buffers for regmap_bulk_write(). 
>      Removed the lock for regmap().
> 
> 6/9: Removed the duplication of code for enabling step, added function to handle
>      the step enable. 
> 
> 7/9: Removed the lock for regmap().
>      Mutex locking and unlocking is used to protect the data members.
> 
> 8/9: Removed the lock for regmap().
> 
> 9/9. Added __be16 duration in struct bma400_data.
>      Fixed the warning - impossible condition '(reg < 0) => (0-255 < 0)'
>      Fixed error: call to __compiletime_assert_272
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> changes since v2
> 1. Reordering of header includes in the separate patch.
> 2. Matching the IIO syntax for multiline comment.
> 3. Following the preference in the interrupt handler for returning.
> 4. Add support for activity recognition.
> 5. Add support for debugfs to access registers from userspace.
> 6. Add support for activity and inactivity events
> 
> changes since v1
> 1. Added comment section that describes the math for scale calculation.
> 2. Added separate devm_add_action_or_reset() calls to disable regulator
>    and to put the sensor in power down mode.
> 3. Remove the err_reg_disable and out, goto labels and returning directly
>    if error occurs.
> 4. Added mutex calls while putting sensor in power down.
> 5. Added ___cacheline_aligned for device data.
> 6. Ordering the header includes.
> 7. Handling erroneous and spurious interrupts in the interrupt handler
>    by returning IRQ_NONE.
> 8. Using dev_err_probe() instead of dev_err().
> 9. Configured the interrupt to open drain.
> 10. Using le16_to_cpu() to fix the sparse warning.
> 11. Checking the step change event is enabled or not.
> 12. Enabling the step change event will also enable the step channel.
> 13. Using FIELD_GET() instead of bitwise operation.
> 14. Removal of dead code in the _event_config().
> 
> Jagath Jog J (9):
>   iio: accel: bma400: Fix the scale min and max macro values
>   iio: accel: bma400: Reordering of header files
>   iio: accel: bma400: conversion to device-managed function
>   iio: accel: bma400: Add triggered buffer support
>   iio: accel: bma400: Add separate channel for step counter
>   iio: accel: bma400: Add step change event
>   iio: accel: bma400: Add activity recognition support
>   iio: accel: bma400: Add debugfs register access support
>   iio: accel: bma400: Add support for activity and inactivity events
> 
>  drivers/iio/accel/Kconfig       |   2 +
>  drivers/iio/accel/bma400.h      |  50 ++-
>  drivers/iio/accel/bma400_core.c | 694 +++++++++++++++++++++++++++++---
>  drivers/iio/accel/bma400_i2c.c  |  10 +-
>  drivers/iio/accel/bma400_spi.c  |   8 +-
>  5 files changed, 697 insertions(+), 67 deletions(-)
> 

Re: [PATCH v4 4/9] iio: accel: bma400: Add triggered buffer support

[Jagath Jog J]

Hi Jonathan,

On Sun, May 1, 2022 at 9:42 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Wed, 27 Apr 2022 14:34:57 +0200
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>
> > On Wed, Apr 20, 2022 at 11:11 PM Jagath Jog J <jagathjog1996@gmail.com> wrote:
> > >
> > > Added trigger buffer support to read continuous acceleration
> > > data from device with data ready interrupt which is mapped
> > > to INT1 pin.
> >
> > LGTM,
> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Agreed.  A couple of 'comments' inline but no actual need to change anything.
> One is contingent on a fix I've not sent out yet for the rest of IIO.
> The other is potentially a minor improvement for the future.
>
> Thanks,
>
> Jonathan
>
> >
> > > Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
> > > ---
> > >  drivers/iio/accel/Kconfig       |   2 +
> > >  drivers/iio/accel/bma400.h      |  10 +-
> > >  drivers/iio/accel/bma400_core.c | 162 +++++++++++++++++++++++++++++++-
> > >  drivers/iio/accel/bma400_i2c.c  |   2 +-
> > >  drivers/iio/accel/bma400_spi.c  |   2 +-
> > >  5 files changed, 170 insertions(+), 8 deletions(-)
>
> > >
> > >  #include "bma400.h"
> > >
> > > @@ -61,6 +66,14 @@ struct bma400_data {
> > >         struct bma400_sample_freq sample_freq;
> > >         int oversampling_ratio;
> > >         int scale;
> > > +       struct iio_trigger *trig;
> > > +       /* Correct time stamp alignment */
> > > +       struct {
> > > +               __le16 buff[3];
> > > +               u8 temperature;
> > > +               s64 ts __aligned(8);
> > > +       } buffer ____cacheline_aligned;
>
> If you are rolling again, could you change this to
> __aligned(IIO_ALIGN);  See
> https://lore.kernel.org/linux-iio/20220419121241.00002e42@Huawei.com/
> for why.
> Note that I'll be sending a fix patch out for IIO_ALIGN to define
> it as ARCH_KMALLOC_ALIGN in next few days.
>
> If you'd pref not to get caught up in that, send it as it stands
> and I'll fix up once that fix is in place.  What's one more driver
> on top of the 80+ I have to do anyway :)
>
>

Sure, I will change that to __aligned(IIO_ALIGN); in the next series.

>
> > > +       __le16 status;
> > >  };
> > >
>
> > > +
> > > +static const unsigned long bma400_avail_scan_masks[] = {
> > > +       GENMASK(3, 0),
> > > +       0
> > > +};
> > > +
> > >  static const struct iio_info bma400_info = {
> > >         .read_raw          = bma400_read_raw,
> > >         .read_avail        = bma400_read_avail,
> > > @@ -814,7 +869,72 @@ static const struct iio_info bma400_info = {
> > >         .write_raw_get_fmt = bma400_write_raw_get_fmt,
> > >  };
> > >
> > > -int bma400_probe(struct device *dev, struct regmap *regmap, const char *name)
> > > +static const struct iio_trigger_ops bma400_trigger_ops = {
> > > +       .set_trigger_state = &bma400_data_rdy_trigger_set_state,
> > > +       .validate_device = &iio_trigger_validate_own_device,
> > > +};
> > > +
> > > +static irqreturn_t bma400_trigger_handler(int irq, void *p)
> > > +{
> > > +       struct iio_poll_func *pf = p;
> > > +       struct iio_dev *indio_dev = pf->indio_dev;
> > > +       struct bma400_data *data = iio_priv(indio_dev);
> > > +       int ret, temp;
> > > +
> > > +       /* Lock to protect the data->buffer */
> > > +       mutex_lock(&data->mutex);
> > > +
> > > +       /* bulk read six registers, with the base being the LSB register */
> > > +       ret = regmap_bulk_read(data->regmap, BMA400_X_AXIS_LSB_REG,
> > > +                              &data->buffer.buff, sizeof(data->buffer.buff));
> > > +       if (ret)
> > > +               goto unlock_err;
> > > +
> > > +       ret = regmap_read(data->regmap, BMA400_TEMP_DATA_REG, &temp);
>
> Given the temperature read is a separate action, it seems like you could sensible
> add another entry to bma400_avail_scan_masks() for just the accelerometer axis
> and then only perform this read if the temperature is requested.
>
> It would be a feature though, so no need to have it in this patch if you
> prefer not to.

Sure I will add another entry only for the accelerometer axis and I
will make changes
accordingly in the next series.

Do I need to add 'Reviewed-by' tag if the patch gets modified again
after getting the
tag?


>
> > > +       if (ret)
> > > +               goto unlock_err;
> > > +
> > > +       data->buffer.temperature = temp;
> > > +
> > > +       iio_push_to_buffers_with_timestamp(indio_dev, &data->buffer,
> > > +                                          iio_get_time_ns(indio_dev));
> > > +
> > > +       mutex_unlock(&data->mutex);
> > > +       iio_trigger_notify_done(indio_dev->trig);
> > > +       return IRQ_HANDLED;
> > > +
> > > +unlock_err:
> > > +       mutex_unlock(&data->mutex);
> > > +       return IRQ_NONE;
> > > +}
>

Re: [PATCH v4 6/9] iio: accel: bma400: Add step change event

[Jagath Jog J]

Hi Jonathan,

On Sun, May 1, 2022 at 9:52 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Thu, 21 Apr 2022 02:41:02 +0530
> Jagath Jog J <jagathjog1996@gmail.com> wrote:
>
> > Added support for event when there is a detection of step change.
> > INT1 pin is used to interrupt and event is pushed to userspace.
> >
> > Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
> Hi Jagath,
>
> A query about handling of multiple interrupts...
>
> > ---
> >  drivers/iio/accel/bma400.h      |  2 +
> >  drivers/iio/accel/bma400_core.c | 75 +++++++++++++++++++++++++++++++++
> >  2 files changed, 77 insertions(+)
> >
> >   * Read-write configuration registers
> > diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
> > index aafb5a40944d..fe101df7b773 100644
> > --- a/drivers/iio/accel/bma400_core.c
> > +++ b/drivers/iio/accel/bma400_core.c
>
> >
> >  static const struct iio_trigger_ops bma400_trigger_ops = {
> > @@ -971,6 +1035,7 @@ static irqreturn_t bma400_interrupt(int irq, void *private)
> >  {
> >       struct iio_dev *indio_dev = private;
> >       struct bma400_data *data = iio_priv(indio_dev);
> > +     s64 timestamp = iio_get_time_ns(indio_dev);
> >       int ret;
> >
> >       /* Lock to protect the data->status */
> > @@ -981,6 +1046,16 @@ static irqreturn_t bma400_interrupt(int irq, void *private)
> >       if (ret)
> >               goto unlock_err;
> >
> > +     if (FIELD_GET(BMA400_STEP_STAT_MASK, le16_to_cpu(data->status))) {
> > +             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),
> > +                            timestamp);
> > +             mutex_unlock(&data->mutex);
>
> Is it possible for two interrupt sources to be active at the same time?

Yeah, it is possible when multiple interrupts are enabled like data ready,
step and generic interrupts.

> Given the device is clearing interrupts on read (which is unusual enough to
> make me check that on the datasheet) you will loose any other events.
>
> Normal trick is to act on all set bits and if any of them were acted on
> return HANDLED.

Then I will push all the events that occurred and then in the end I will return
HANDLED so that none of the events are missed.
I will change this in the next version.

Thank you,
Jagath


>
> > +             return IRQ_HANDLED;
> > +     }
> > +
> >       if (FIELD_GET(BMA400_INT_DRDY_MSK, le16_to_cpu(data->status))) {
> >               mutex_unlock(&data->mutex);
> >               iio_trigger_poll_chained(data->trig);
>

Re: [PATCH v4 4/9] iio: accel: bma400: Add triggered buffer support

[Jonathan Cameron]

On Mon, 2 May 2022 01:55:49 +0530
Jagath Jog J <jagathjog1996@gmail.com> wrote:

> Hi Jonathan,
> 
> On Sun, May 1, 2022 at 9:42 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Wed, 27 Apr 2022 14:34:57 +0200
> > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> >  
> > > On Wed, Apr 20, 2022 at 11:11 PM Jagath Jog J <jagathjog1996@gmail.com> wrote:  
> > > >
> > > > Added trigger buffer support to read continuous acceleration
> > > > data from device with data ready interrupt which is mapped
> > > > to INT1 pin.  
> > >
> > > LGTM,
> > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>  
> > Agreed.  A couple of 'comments' inline but no actual need to change anything.
> > One is contingent on a fix I've not sent out yet for the rest of IIO.
> > The other is potentially a minor improvement for the future.
> >
> > Thanks,
> >
> > Jonathan
> >  
> > >  
> > > > Signed-off-by: Jagath Jog J <jagathjog1996@gmail.com>
> > > > ---
> > > >  drivers/iio/accel/Kconfig       |   2 +
> > > >  drivers/iio/accel/bma400.h      |  10 +-
> > > >  drivers/iio/accel/bma400_core.c | 162 +++++++++++++++++++++++++++++++-
> > > >  drivers/iio/accel/bma400_i2c.c  |   2 +-
> > > >  drivers/iio/accel/bma400_spi.c  |   2 +-
> > > >  5 files changed, 170 insertions(+), 8 deletions(-)  
> >  
> > > >
> > > >  #include "bma400.h"
> > > >
> > > > @@ -61,6 +66,14 @@ struct bma400_data {
> > > >         struct bma400_sample_freq sample_freq;
> > > >         int oversampling_ratio;
> > > >         int scale;
> > > > +       struct iio_trigger *trig;
> > > > +       /* Correct time stamp alignment */
> > > > +       struct {
> > > > +               __le16 buff[3];
> > > > +               u8 temperature;
> > > > +               s64 ts __aligned(8);
> > > > +       } buffer ____cacheline_aligned;  
> >
> > If you are rolling again, could you change this to
> > __aligned(IIO_ALIGN);  See
> > https://lore.kernel.org/linux-iio/20220419121241.00002e42@Huawei.com/
> > for why.
> > Note that I'll be sending a fix patch out for IIO_ALIGN to define
> > it as ARCH_KMALLOC_ALIGN in next few days.
> >
> > If you'd pref not to get caught up in that, send it as it stands
> > and I'll fix up once that fix is in place.  What's one more driver
> > on top of the 80+ I have to do anyway :)
> >
> >  
> 
> Sure, I will change that to __aligned(IIO_ALIGN); in the next series.
> 
> >  
> > > > +       __le16 status;
> > > >  };
> > > >  
> >  
> > > > +
> > > > +static const unsigned long bma400_avail_scan_masks[] = {
> > > > +       GENMASK(3, 0),
> > > > +       0
> > > > +};
> > > > +
> > > >  static const struct iio_info bma400_info = {
> > > >         .read_raw          = bma400_read_raw,
> > > >         .read_avail        = bma400_read_avail,
> > > > @@ -814,7 +869,72 @@ static const struct iio_info bma400_info = {
> > > >         .write_raw_get_fmt = bma400_write_raw_get_fmt,
> > > >  };
> > > >
> > > > -int bma400_probe(struct device *dev, struct regmap *regmap, const char *name)
> > > > +static const struct iio_trigger_ops bma400_trigger_ops = {
> > > > +       .set_trigger_state = &bma400_data_rdy_trigger_set_state,
> > > > +       .validate_device = &iio_trigger_validate_own_device,
> > > > +};
> > > > +
> > > > +static irqreturn_t bma400_trigger_handler(int irq, void *p)
> > > > +{
> > > > +       struct iio_poll_func *pf = p;
> > > > +       struct iio_dev *indio_dev = pf->indio_dev;
> > > > +       struct bma400_data *data = iio_priv(indio_dev);
> > > > +       int ret, temp;
> > > > +
> > > > +       /* Lock to protect the data->buffer */
> > > > +       mutex_lock(&data->mutex);
> > > > +
> > > > +       /* bulk read six registers, with the base being the LSB register */
> > > > +       ret = regmap_bulk_read(data->regmap, BMA400_X_AXIS_LSB_REG,
> > > > +                              &data->buffer.buff, sizeof(data->buffer.buff));
> > > > +       if (ret)
> > > > +               goto unlock_err;
> > > > +
> > > > +       ret = regmap_read(data->regmap, BMA400_TEMP_DATA_REG, &temp);  
> >
> > Given the temperature read is a separate action, it seems like you could sensible
> > add another entry to bma400_avail_scan_masks() for just the accelerometer axis
> > and then only perform this read if the temperature is requested.
> >
> > It would be a feature though, so no need to have it in this patch if you
> > prefer not to.  
> 
> Sure I will add another entry only for the accelerometer axis and I
> will make changes
> accordingly in the next series.
> 
> Do I need to add 'Reviewed-by' tag if the patch gets modified again
> after getting the
> tag?
> 
It's something you have judge based on whether you think a change
is significant enough to warrant dropping tags.
If you do drop them you should always state why in the cover
letter or change log.

Thanks,

Jonathan

> 
> >  
> > > > +       if (ret)
> > > > +               goto unlock_err;
> > > > +
> > > > +       data->buffer.temperature = temp;
> > > > +
> > > > +       iio_push_to_buffers_with_timestamp(indio_dev, &data->buffer,
> > > > +                                          iio_get_time_ns(indio_dev));
> > > > +
> > > > +       mutex_unlock(&data->mutex);
> > > > +       iio_trigger_notify_done(indio_dev->trig);
> > > > +       return IRQ_HANDLED;
> > > > +
> > > > +unlock_err:
> > > > +       mutex_unlock(&data->mutex);
> > > > +       return IRQ_NONE;
> > > > +}  
> >