[PATCH 0/3] BMI270 data ready interrupt support

[PATCH 0/3] BMI270 data ready interrupt support

[Gustavo Silva]

This series adds support for data ready interrupt to the BMI270 driver
using one of the available interrupt pins.

Additionally, this series includes some cleanups to simplify and improve
consistency across the driver.

Signed-off-by: Gustavo Silva <gustavograzs@gmail.com>
---
Gustavo Silva (3):
      iio: imu: bmi270: move private struct declaration to source file
      iio: imu: bmi270: rename variable bmi270_device to data
      iio: imu: bmi270: add support for data ready interrupt trigger

 drivers/iio/imu/bmi270/bmi270.h      |  17 +-
 drivers/iio/imu/bmi270/bmi270_core.c | 326 ++++++++++++++++++++++++++++++-----
 2 files changed, 281 insertions(+), 62 deletions(-)
---
base-commit: 4c571885898c5c98934d086f2ab11b5e27e4f41f
change-id: 20250219-bmi270-irq-9a2bc41faee3

Best regards,
-- 
Gustavo Silva <gustavograzs@gmail.com>

[PATCH 1/3] iio: imu: bmi270: move private struct declaration to source file

[Gustavo Silva]

The device's private data struct is currently declared in the header
file, but it does not need to be exposed there. Move it to the driver's
core source file to avoid unnecessary #include directives or forward
declarations in the header.

Signed-off-by: Gustavo Silva <gustavograzs@gmail.com>
---
 drivers/iio/imu/bmi270/bmi270.h      | 17 +----------------
 drivers/iio/imu/bmi270/bmi270_core.c | 15 +++++++++++++++
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/iio/imu/bmi270/bmi270.h b/drivers/iio/imu/bmi270/bmi270.h
index fdfad5784cc52043475b6816286619fac5824684..d94525f6aee85f21cc9e9ae1bc9c1db0dc00b927 100644
--- a/drivers/iio/imu/bmi270/bmi270.h
+++ b/drivers/iio/imu/bmi270/bmi270.h
@@ -6,22 +6,6 @@
 #include <linux/regmap.h>
 #include <linux/iio/iio.h>
 
-struct device;
-struct bmi270_data {
-	struct device *dev;
-	struct regmap *regmap;
-	const struct bmi270_chip_info *chip_info;
-
-	/*
-	 * Where IIO_DMA_MINALIGN may be larger than 8 bytes, align to
-	 * that to ensure a DMA safe buffer.
-	 */
-	struct {
-		__le16 channels[6];
-		aligned_s64 timestamp;
-	} data __aligned(IIO_DMA_MINALIGN);
-};
-
 struct bmi270_chip_info {
 	const char *name;
 	int chip_id;
@@ -32,6 +16,7 @@ extern const struct regmap_config bmi270_regmap_config;
 extern const struct bmi270_chip_info bmi260_chip_info;
 extern const struct bmi270_chip_info bmi270_chip_info;
 
+struct device;
 int bmi270_core_probe(struct device *dev, struct regmap *regmap,
 		      const struct bmi270_chip_info *chip_info);
 
diff --git a/drivers/iio/imu/bmi270/bmi270_core.c b/drivers/iio/imu/bmi270/bmi270_core.c
index 464dcdd657c4ba27fdb7620c80e9f05e2a831910..9f24d4044ed6f00f67fd3a8f5adda821c3140a18 100644
--- a/drivers/iio/imu/bmi270/bmi270_core.c
+++ b/drivers/iio/imu/bmi270/bmi270_core.c
@@ -78,6 +78,21 @@
 #define BMI260_INIT_DATA_FILE "bmi260-init-data.fw"
 #define BMI270_INIT_DATA_FILE "bmi270-init-data.fw"
 
+struct bmi270_data {
+	struct device *dev;
+	struct regmap *regmap;
+	const struct bmi270_chip_info *chip_info;
+
+	/*
+	 * Where IIO_DMA_MINALIGN may be larger than 8 bytes, align to
+	 * that to ensure a DMA safe buffer.
+	 */
+	struct {
+		__le16 channels[6];
+		aligned_s64 timestamp;
+	} data __aligned(IIO_DMA_MINALIGN);
+};
+
 enum bmi270_scan {
 	BMI270_SCAN_ACCEL_X,
 	BMI270_SCAN_ACCEL_Y,

-- 
2.48.1

[PATCH 2/3] iio: imu: bmi270: rename variable bmi270_device to data

[Gustavo Silva]

Rename all instances of 'struct bmi270_data' to 'data', to ensure
consistency across the driver.

Signed-off-by: Gustavo Silva <gustavograzs@gmail.com>
---
 drivers/iio/imu/bmi270/bmi270_core.c | 79 ++++++++++++++++++------------------
 1 file changed, 39 insertions(+), 40 deletions(-)

diff --git a/drivers/iio/imu/bmi270/bmi270_core.c b/drivers/iio/imu/bmi270/bmi270_core.c
index 9f24d4044ed6f00f67fd3a8f5adda821c3140a18..56ebd887fcec94b684dc23f1b4503b719fb39239 100644
--- a/drivers/iio/imu/bmi270/bmi270_core.c
+++ b/drivers/iio/imu/bmi270/bmi270_core.c
@@ -90,7 +90,7 @@ struct bmi270_data {
 	struct {
 		__le16 channels[6];
 		aligned_s64 timestamp;
-	} data __aligned(IIO_DMA_MINALIGN);
+	} buffer __aligned(IIO_DMA_MINALIGN);
 };
 
 enum bmi270_scan {
@@ -284,7 +284,7 @@ static int bmi270_set_scale(struct bmi270_data *data, int chan_type, int uscale)
 	return -EINVAL;
 }
 
-static int bmi270_get_scale(struct bmi270_data *bmi270_device, int chan_type,
+static int bmi270_get_scale(struct bmi270_data *data, int chan_type,
 			    int *scale, int *uscale)
 {
 	int ret;
@@ -293,7 +293,7 @@ static int bmi270_get_scale(struct bmi270_data *bmi270_device, int chan_type,
 
 	switch (chan_type) {
 	case IIO_ACCEL:
-		ret = regmap_read(bmi270_device->regmap,
+		ret = regmap_read(data->regmap,
 				  BMI270_ACC_CONF_RANGE_REG, &val);
 		if (ret)
 			return ret;
@@ -302,7 +302,7 @@ static int bmi270_get_scale(struct bmi270_data *bmi270_device, int chan_type,
 		bmi270_scale_item = bmi270_scale_table[BMI270_ACCEL];
 		break;
 	case IIO_ANGL_VEL:
-		ret = regmap_read(bmi270_device->regmap,
+		ret = regmap_read(data->regmap,
 				  BMI270_GYR_CONF_RANGE_REG, &val);
 		if (ret)
 			return ret;
@@ -403,24 +403,24 @@ static irqreturn_t bmi270_trigger_handler(int irq, void *p)
 {
 	struct iio_poll_func *pf = p;
 	struct iio_dev *indio_dev = pf->indio_dev;
-	struct bmi270_data *bmi270_device = iio_priv(indio_dev);
+	struct bmi270_data *data = iio_priv(indio_dev);
 	int ret;
 
-	ret = regmap_bulk_read(bmi270_device->regmap, BMI270_ACCEL_X_REG,
-			       &bmi270_device->data.channels,
-			       sizeof(bmi270_device->data.channels));
+	ret = regmap_bulk_read(data->regmap, BMI270_ACCEL_X_REG,
+			       &data->buffer.channels,
+			       sizeof(data->buffer.channels));
 
 	if (ret)
 		goto done;
 
-	iio_push_to_buffers_with_timestamp(indio_dev, &bmi270_device->data,
+	iio_push_to_buffers_with_timestamp(indio_dev, &data->buffer,
 					   pf->timestamp);
 done:
 	iio_trigger_notify_done(indio_dev->trig);
 	return IRQ_HANDLED;
 }
 
-static int bmi270_get_data(struct bmi270_data *bmi270_device,
+static int bmi270_get_data(struct bmi270_data *data,
 			   int chan_type, int axis, int *val)
 {
 	__le16 sample;
@@ -441,7 +441,7 @@ static int bmi270_get_data(struct bmi270_data *bmi270_device,
 		return -EINVAL;
 	}
 
-	ret = regmap_bulk_read(bmi270_device->regmap, reg, &sample, sizeof(sample));
+	ret = regmap_bulk_read(data->regmap, reg, &sample, sizeof(sample));
 	if (ret)
 		return ret;
 
@@ -455,17 +455,17 @@ static int bmi270_read_raw(struct iio_dev *indio_dev,
 			   int *val, int *val2, long mask)
 {
 	int ret;
-	struct bmi270_data *bmi270_device = iio_priv(indio_dev);
+	struct bmi270_data *data = iio_priv(indio_dev);
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		ret = bmi270_get_data(bmi270_device, chan->type, chan->channel2, val);
+		ret = bmi270_get_data(data, chan->type, chan->channel2, val);
 		if (ret)
 			return ret;
 
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
-		ret = bmi270_get_scale(bmi270_device, chan->type, val, val2);
+		ret = bmi270_get_scale(data, chan->type, val, val2);
 		return ret ? ret : IIO_VAL_INT_PLUS_MICRO;
 	case IIO_CHAN_INFO_OFFSET:
 		switch (chan->type) {
@@ -476,7 +476,7 @@ static int bmi270_read_raw(struct iio_dev *indio_dev,
 			return -EINVAL;
 		}
 	case IIO_CHAN_INFO_SAMP_FREQ:
-		ret = bmi270_get_odr(bmi270_device, chan->type, val, val2);
+		ret = bmi270_get_odr(data, chan->type, val, val2);
 		return ret ? ret : IIO_VAL_INT_PLUS_MICRO;
 	default:
 		return -EINVAL;
@@ -599,12 +599,12 @@ static const struct iio_chan_spec bmi270_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(BMI270_SCAN_TIMESTAMP),
 };
 
-static int bmi270_validate_chip_id(struct bmi270_data *bmi270_device)
+static int bmi270_validate_chip_id(struct bmi270_data *data)
 {
 	int chip_id;
 	int ret;
-	struct device *dev = bmi270_device->dev;
-	struct regmap *regmap = bmi270_device->regmap;
+	struct device *dev = data->dev;
+	struct regmap *regmap = data->regmap;
 
 	ret = regmap_read(regmap, BMI270_CHIP_ID_REG, &chip_id);
 	if (ret)
@@ -618,24 +618,24 @@ static int bmi270_validate_chip_id(struct bmi270_data *bmi270_device)
 	if (chip_id == BMI160_CHIP_ID_VAL)
 		return -ENODEV;
 
-	if (chip_id != bmi270_device->chip_info->chip_id)
+	if (chip_id != data->chip_info->chip_id)
 		dev_info(dev, "Unexpected chip id 0x%x", chip_id);
 
 	if (chip_id == bmi260_chip_info.chip_id)
-		bmi270_device->chip_info = &bmi260_chip_info;
+		data->chip_info = &bmi260_chip_info;
 	else if (chip_id == bmi270_chip_info.chip_id)
-		bmi270_device->chip_info = &bmi270_chip_info;
+		data->chip_info = &bmi270_chip_info;
 
 	return 0;
 }
 
-static int bmi270_write_calibration_data(struct bmi270_data *bmi270_device)
+static int bmi270_write_calibration_data(struct bmi270_data *data)
 {
 	int ret;
 	int status = 0;
 	const struct firmware *init_data;
-	struct device *dev = bmi270_device->dev;
-	struct regmap *regmap = bmi270_device->regmap;
+	struct device *dev = data->dev;
+	struct regmap *regmap = data->regmap;
 
 	ret = regmap_clear_bits(regmap, BMI270_PWR_CONF_REG,
 				BMI270_PWR_CONF_ADV_PWR_SAVE_MSK);
@@ -656,8 +656,7 @@ static int bmi270_write_calibration_data(struct bmi270_data *bmi270_device)
 		return dev_err_probe(dev, ret,
 				     "Failed to prepare device to load init data");
 
-	ret = request_firmware(&init_data,
-			       bmi270_device->chip_info->fw_name, dev);
+	ret = request_firmware(&init_data, data->chip_info->fw_name, dev);
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to load init data file");
 
@@ -689,11 +688,11 @@ static int bmi270_write_calibration_data(struct bmi270_data *bmi270_device)
 	return 0;
 }
 
-static int bmi270_configure_imu(struct bmi270_data *bmi270_device)
+static int bmi270_configure_imu(struct bmi270_data *data)
 {
 	int ret;
-	struct device *dev = bmi270_device->dev;
-	struct regmap *regmap = bmi270_device->regmap;
+	struct device *dev = data->dev;
+	struct regmap *regmap = data->regmap;
 
 	ret = regmap_set_bits(regmap, BMI270_PWR_CTRL_REG,
 			      BMI270_PWR_CTRL_AUX_EN_MSK |
@@ -730,38 +729,38 @@ static int bmi270_configure_imu(struct bmi270_data *bmi270_device)
 	return 0;
 }
 
-static int bmi270_chip_init(struct bmi270_data *bmi270_device)
+static int bmi270_chip_init(struct bmi270_data *data)
 {
 	int ret;
 
-	ret = bmi270_validate_chip_id(bmi270_device);
+	ret = bmi270_validate_chip_id(data);
 	if (ret)
 		return ret;
 
-	ret = bmi270_write_calibration_data(bmi270_device);
+	ret = bmi270_write_calibration_data(data);
 	if (ret)
 		return ret;
 
-	return bmi270_configure_imu(bmi270_device);
+	return bmi270_configure_imu(data);
 }
 
 int bmi270_core_probe(struct device *dev, struct regmap *regmap,
 		      const struct bmi270_chip_info *chip_info)
 {
 	int ret;
-	struct bmi270_data *bmi270_device;
+	struct bmi270_data *data;
 	struct iio_dev *indio_dev;
 
-	indio_dev = devm_iio_device_alloc(dev, sizeof(*bmi270_device));
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
 	if (!indio_dev)
 		return -ENOMEM;
 
-	bmi270_device = iio_priv(indio_dev);
-	bmi270_device->dev = dev;
-	bmi270_device->regmap = regmap;
-	bmi270_device->chip_info = chip_info;
+	data = iio_priv(indio_dev);
+	data->dev = dev;
+	data->regmap = regmap;
+	data->chip_info = chip_info;
 
-	ret = bmi270_chip_init(bmi270_device);
+	ret = bmi270_chip_init(data);
 	if (ret)
 		return ret;
 

-- 
2.48.1

[PATCH 3/3] iio: imu: bmi270: add support for data ready interrupt trigger

[Gustavo Silva]

The BMI270 sensor provides two interrupt pins that can be used for
different interrupt sources, including a data ready signal. Add support
for configuring one the pins as a trigger source.

The interrupt pin can be configured with various options: active high or
low, push-pull or open-drain, and latched or non-latched.

Signed-off-by: Gustavo Silva <gustavograzs@gmail.com>
---
 drivers/iio/imu/bmi270/bmi270_core.c | 234 +++++++++++++++++++++++++++++++++--
 1 file changed, 227 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/imu/bmi270/bmi270_core.c b/drivers/iio/imu/bmi270/bmi270_core.c
index 56ebd887fcec94b684dc23f1b4503b719fb39239..378943dfe69ebdb3937f724a7c57f7556a5c452e 100644
--- a/drivers/iio/imu/bmi270/bmi270_core.c
+++ b/drivers/iio/imu/bmi270/bmi270_core.c
@@ -4,11 +4,13 @@
 #include <linux/firmware.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/regmap.h>
 #include <linux/units.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
+#include <linux/iio/trigger.h>
 #include <linux/iio/triggered_buffer.h>
 #include <linux/iio/trigger_consumer.h>
 
@@ -26,6 +28,9 @@
 #define BMI270_ACCEL_X_REG				0x0c
 #define BMI270_ANG_VEL_X_REG				0x12
 
+#define BMI270_INT_STATUS_1_REG				0x1d
+#define BMI270_INT_STATUS_1_ACC_GYR_DRDY_MSK		GENMASK(7, 6)
+
 #define BMI270_INTERNAL_STATUS_REG			0x21
 #define BMI270_INTERNAL_STATUS_MSG_MSK			GENMASK(3, 0)
 #define BMI270_INTERNAL_STATUS_MSG_INIT_OK		0x01
@@ -55,6 +60,20 @@
 #define BMI270_GYR_CONF_RANGE_REG			0x43
 #define BMI270_GYR_CONF_RANGE_MSK			GENMASK(2, 0)
 
+#define BMI270_INT1_IO_CTRL_REG				0x53
+#define BMI270_INT2_IO_CTRL_REG				0x54
+#define BMI270_INT_IO_CTRL_LVL_MSK			BIT(1)
+#define BMI270_INT_IO_CTRL_OD_MSK			BIT(2)
+#define BMI270_INT_IO_CTRL_OP_MSK			BIT(3)
+#define BMI270_INT_IO_LVL_OD_OP_MSK			GENMASK(3, 1)
+
+#define BMI270_INT_LATCH_REG				0x55
+#define BMI270_INT_LATCH_REG_MSK			BIT(0)
+
+#define BMI270_INT_MAP_DATA_REG				0x58
+#define BMI270_INT_MAP_DATA_DRDY_INT1_MSK		BIT(2)
+#define BMI270_INT_MAP_DATA_DRDY_INT2_MSK		BIT(6)
+
 #define BMI270_INIT_CTRL_REG				0x59
 #define BMI270_INIT_CTRL_LOAD_DONE_MSK			BIT(0)
 
@@ -78,10 +97,20 @@
 #define BMI260_INIT_DATA_FILE "bmi260-init-data.fw"
 #define BMI270_INIT_DATA_FILE "bmi270-init-data.fw"
 
+enum bmi270_irq_pin {
+	BMI270_IRQ_DISABLED,
+	BMI270_IRQ_INT1,
+	BMI270_IRQ_INT2,
+};
+
 struct bmi270_data {
 	struct device *dev;
 	struct regmap *regmap;
 	const struct bmi270_chip_info *chip_info;
+	enum bmi270_irq_pin irq_pin;
+	struct iio_trigger *trig;
+	 /* Protect device's private data from concurrent access */
+	struct mutex mutex;
 
 	/*
 	 * Where IIO_DMA_MINALIGN may be larger than 8 bytes, align to
@@ -274,6 +303,8 @@ static int bmi270_set_scale(struct bmi270_data *data, int chan_type, int uscale)
 		return -EINVAL;
 	}
 
+	guard(mutex)(&data->mutex);
+
 	for (i = 0; i < bmi270_scale_item.num; i++) {
 		if (bmi270_scale_item.tbl[i].uscale != uscale)
 			continue;
@@ -291,6 +322,8 @@ static int bmi270_get_scale(struct bmi270_data *data, int chan_type,
 	unsigned int val;
 	struct bmi270_scale_item bmi270_scale_item;
 
+	guard(mutex)(&data->mutex);
+
 	switch (chan_type) {
 	case IIO_ACCEL:
 		ret = regmap_read(data->regmap,
@@ -348,6 +381,8 @@ static int bmi270_set_odr(struct bmi270_data *data, int chan_type, int odr,
 		return -EINVAL;
 	}
 
+	guard(mutex)(&data->mutex);
+
 	for (i = 0; i < bmi270_odr_item.num; i++) {
 		if (bmi270_odr_item.tbl[i].odr != odr ||
 		    bmi270_odr_item.tbl[i].uodr != uodr)
@@ -366,6 +401,8 @@ static int bmi270_get_odr(struct bmi270_data *data, int chan_type, int *odr,
 	int i, val, ret;
 	struct bmi270_odr_item bmi270_odr_item;
 
+	guard(mutex)(&data->mutex);
+
 	switch (chan_type) {
 	case IIO_ACCEL:
 		ret = regmap_read(data->regmap, BMI270_ACC_CONF_REG, &val);
@@ -399,6 +436,60 @@ static int bmi270_get_odr(struct bmi270_data *data, int chan_type, int *odr,
 	return -EINVAL;
 }
 
+static irqreturn_t bmi270_irq_thread_handler(int irq, void *private)
+{
+	struct iio_dev *indio_dev = private;
+	struct bmi270_data *data = iio_priv(indio_dev);
+	unsigned int status;
+	int ret;
+
+	scoped_guard(mutex, &data->mutex) {
+		ret = regmap_read(data->regmap, BMI270_INT_STATUS_1_REG,
+				  &status);
+		if (ret)
+			return IRQ_NONE;
+	}
+
+	if (FIELD_GET(BMI270_INT_STATUS_1_ACC_GYR_DRDY_MSK, status))
+		iio_trigger_poll_nested(data->trig);
+
+	return IRQ_HANDLED;
+}
+
+static int bmi270_data_rdy_trigger_set_state(struct iio_trigger *trig,
+					     bool state)
+{
+	struct bmi270_data *data = iio_trigger_get_drvdata(trig);
+	unsigned int field_value = 0;
+	unsigned int mask;
+
+	guard(mutex)(&data->mutex);
+
+	switch (data->irq_pin) {
+	case BMI270_IRQ_INT1:
+		mask = BMI270_INT_MAP_DATA_DRDY_INT1_MSK;
+		set_mask_bits(&field_value, BMI270_INT_MAP_DATA_DRDY_INT1_MSK,
+			      FIELD_PREP(BMI270_INT_MAP_DATA_DRDY_INT1_MSK,
+					 state));
+		break;
+	case BMI270_IRQ_INT2:
+		mask = BMI270_INT_MAP_DATA_DRDY_INT2_MSK;
+		set_mask_bits(&field_value, BMI270_INT_MAP_DATA_DRDY_INT2_MSK,
+			      FIELD_PREP(BMI270_INT_MAP_DATA_DRDY_INT2_MSK,
+					 state));
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return regmap_update_bits(data->regmap, BMI270_INT_MAP_DATA_REG, mask,
+				  field_value);
+}
+
+static const struct iio_trigger_ops bmi270_trigger_ops = {
+	.set_trigger_state = &bmi270_data_rdy_trigger_set_state,
+};
+
 static irqreturn_t bmi270_trigger_handler(int irq, void *p)
 {
 	struct iio_poll_func *pf = p;
@@ -406,6 +497,8 @@ static irqreturn_t bmi270_trigger_handler(int irq, void *p)
 	struct bmi270_data *data = iio_priv(indio_dev);
 	int ret;
 
+	guard(mutex)(&data->mutex);
+
 	ret = regmap_bulk_read(data->regmap, BMI270_ACCEL_X_REG,
 			       &data->buffer.channels,
 			       sizeof(data->buffer.channels));
@@ -441,13 +534,15 @@ static int bmi270_get_data(struct bmi270_data *data,
 		return -EINVAL;
 	}
 
+	guard(mutex)(&data->mutex);
+
 	ret = regmap_bulk_read(data->regmap, reg, &sample, sizeof(sample));
 	if (ret)
 		return ret;
 
 	*val = sign_extend32(le16_to_cpu(sample), 15);
 
-	return 0;
+	return IIO_VAL_INT;
 }
 
 static int bmi270_read_raw(struct iio_dev *indio_dev,
@@ -459,11 +554,11 @@ static int bmi270_read_raw(struct iio_dev *indio_dev,
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
 		ret = bmi270_get_data(data, chan->type, chan->channel2, val);
-		if (ret)
-			return ret;
-
-		return IIO_VAL_INT;
+		iio_device_release_direct(indio_dev);
+		return ret;
 	case IIO_CHAN_INFO_SCALE:
 		ret = bmi270_get_scale(data, chan->type, val, val2);
 		return ret ? ret : IIO_VAL_INT_PLUS_MICRO;
@@ -488,12 +583,21 @@ static int bmi270_write_raw(struct iio_dev *indio_dev,
 			    int val, int val2, long mask)
 {
 	struct bmi270_data *data = iio_priv(indio_dev);
+	int ret;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_SCALE:
-		return bmi270_set_scale(data, chan->type, val2);
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
+		ret = bmi270_set_scale(data, chan->type, val2);
+		iio_device_release_direct(indio_dev);
+		return ret;
 	case IIO_CHAN_INFO_SAMP_FREQ:
-		return bmi270_set_odr(data, chan->type, val, val2);
+		if (!iio_device_claim_direct(indio_dev))
+			return -EBUSY;
+		ret = bmi270_set_odr(data, chan->type, val, val2);
+		iio_device_release_direct(indio_dev);
+		return ret;
 	default:
 		return -EINVAL;
 	}
@@ -599,6 +703,116 @@ static const struct iio_chan_spec bmi270_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(BMI270_SCAN_TIMESTAMP),
 };
 
+static int bmi270_int_pin_config(struct bmi270_data *data,
+				 enum bmi270_irq_pin irq_pin,
+				 bool active_high, bool open_drain, bool latch)
+{
+	unsigned int reg, field_value;
+	int ret;
+
+	ret = regmap_update_bits(data->regmap, BMI270_INT_LATCH_REG,
+				 BMI270_INT_LATCH_REG_MSK,
+				 FIELD_PREP(BMI270_INT_LATCH_REG_MSK, latch));
+	if (ret)
+		return ret;
+
+	switch (irq_pin) {
+	case BMI270_IRQ_INT1:
+		reg = BMI270_INT1_IO_CTRL_REG;
+		break;
+	case BMI270_IRQ_INT2:
+		reg = BMI270_INT2_IO_CTRL_REG;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	field_value = FIELD_PREP(BMI270_INT_IO_CTRL_LVL_MSK, active_high) |
+		      FIELD_PREP(BMI270_INT_IO_CTRL_OD_MSK, open_drain) |
+		      FIELD_PREP(BMI270_INT_IO_CTRL_OP_MSK, 1);
+	return regmap_update_bits(data->regmap, reg,
+				  BMI270_INT_IO_LVL_OD_OP_MSK, field_value);
+}
+
+static int bmi270_trigger_probe(struct bmi270_data *data,
+				struct iio_dev *indio_dev)
+{
+	bool open_drain, active_high, latch;
+	struct fwnode_handle *fwnode;
+	enum bmi270_irq_pin irq_pin;
+	int ret, irq, irq_type;
+
+	fwnode = dev_fwnode(data->dev);
+	if (!fwnode)
+		return -ENODEV;
+
+	irq = fwnode_irq_get_byname(fwnode, "INT1");
+	if (irq > 0) {
+		irq_pin = BMI270_IRQ_INT1;
+	} else {
+		irq = fwnode_irq_get_byname(fwnode, "INT2");
+		if (irq < 0)
+			return 0;
+
+		irq_pin = BMI270_IRQ_INT2;
+	}
+
+	irq_type = irq_get_trigger_type(irq);
+	switch (irq_type) {
+	case IRQF_TRIGGER_RISING:
+		latch = false;
+		active_high = true;
+		break;
+	case IRQF_TRIGGER_HIGH:
+		latch = true;
+		active_high = true;
+		break;
+	case IRQF_TRIGGER_FALLING:
+		latch = false;
+		active_high = false;
+		break;
+	case IRQF_TRIGGER_LOW:
+		latch = true;
+		active_high = false;
+		break;
+	default:
+		return dev_err_probe(data->dev, -EINVAL,
+				     "Invalid interrupt type 0x%x specified\n",
+				     irq_type);
+	}
+
+	open_drain = fwnode_property_read_bool(fwnode, "drive-open-drain");
+
+	ret = bmi270_int_pin_config(data, irq_pin, active_high, open_drain,
+				    latch);
+	if (ret)
+		return dev_err_probe(data->dev, ret,
+				     "Failed to configure irq line\n");
+
+	data->trig = devm_iio_trigger_alloc(data->dev, "%s-trig-%d",
+					    indio_dev->name, irq_pin);
+	if (!data->trig)
+		return -ENOMEM;
+
+	data->trig->ops = &bmi270_trigger_ops;
+	iio_trigger_set_drvdata(data->trig, data);
+
+	ret = devm_request_threaded_irq(data->dev, irq, NULL,
+					bmi270_irq_thread_handler,
+					IRQF_ONESHOT, "bmi270-int", indio_dev);
+	if (ret)
+		return dev_err_probe(data->dev, ret, "Failed to request IRQ\n");
+
+	ret = devm_iio_trigger_register(data->dev, data->trig);
+	if (ret)
+		return dev_err_probe(data->dev, ret,
+				     "Trigger registration failed\n");
+
+	data->irq_pin = irq_pin;
+
+	return 0;
+}
+
 static int bmi270_validate_chip_id(struct bmi270_data *data)
 {
 	int chip_id;
@@ -759,6 +973,8 @@ int bmi270_core_probe(struct device *dev, struct regmap *regmap,
 	data->dev = dev;
 	data->regmap = regmap;
 	data->chip_info = chip_info;
+	data->irq_pin = BMI270_IRQ_DISABLED;
+	mutex_init(&data->mutex);
 
 	ret = bmi270_chip_init(data);
 	if (ret)
@@ -771,6 +987,10 @@ int bmi270_core_probe(struct device *dev, struct regmap *regmap,
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->info = &bmi270_info;
 
+	ret = bmi270_trigger_probe(data, indio_dev);
+	if (ret)
+		return ret;
+
 	ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
 					      iio_pollfunc_store_time,
 					      bmi270_trigger_handler, NULL);

-- 
2.48.1

Re: [PATCH 1/3] iio: imu: bmi270: move private struct declaration to source file

[Alex Lanzano]

On Wed, Feb 19, 2025 at 08:54:45PM -0300, Gustavo Silva wrote:
> The device's private data struct is currently declared in the header
> file, but it does not need to be exposed there. Move it to the driver's
> core source file to avoid unnecessary #include directives or forward
> declarations in the header.
> 
> Signed-off-by: Gustavo Silva <gustavograzs@gmail.com>
Acked-by: Alex Lanzano <lanzano.alex@gmail.com>
> ---
>  drivers/iio/imu/bmi270/bmi270.h      | 17 +----------------
>  drivers/iio/imu/bmi270/bmi270_core.c | 15 +++++++++++++++
>  2 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/iio/imu/bmi270/bmi270.h b/drivers/iio/imu/bmi270/bmi270.h
> index fdfad5784cc52043475b6816286619fac5824684..d94525f6aee85f21cc9e9ae1bc9c1db0dc00b927 100644
> --- a/drivers/iio/imu/bmi270/bmi270.h
> +++ b/drivers/iio/imu/bmi270/bmi270.h
> @@ -6,22 +6,6 @@
>  #include <linux/regmap.h>
>  #include <linux/iio/iio.h>
>  
> -struct device;
> -struct bmi270_data {
> -	struct device *dev;
> -	struct regmap *regmap;
> -	const struct bmi270_chip_info *chip_info;
> -
> -	/*
> -	 * Where IIO_DMA_MINALIGN may be larger than 8 bytes, align to
> -	 * that to ensure a DMA safe buffer.
> -	 */
> -	struct {
> -		__le16 channels[6];
> -		aligned_s64 timestamp;
> -	} data __aligned(IIO_DMA_MINALIGN);
> -};
> -
>  struct bmi270_chip_info {
>  	const char *name;
>  	int chip_id;
> @@ -32,6 +16,7 @@ extern const struct regmap_config bmi270_regmap_config;
>  extern const struct bmi270_chip_info bmi260_chip_info;
>  extern const struct bmi270_chip_info bmi270_chip_info;
>  
> +struct device;
>  int bmi270_core_probe(struct device *dev, struct regmap *regmap,
>  		      const struct bmi270_chip_info *chip_info);
>  
> diff --git a/drivers/iio/imu/bmi270/bmi270_core.c b/drivers/iio/imu/bmi270/bmi270_core.c
> index 464dcdd657c4ba27fdb7620c80e9f05e2a831910..9f24d4044ed6f00f67fd3a8f5adda821c3140a18 100644
> --- a/drivers/iio/imu/bmi270/bmi270_core.c
> +++ b/drivers/iio/imu/bmi270/bmi270_core.c
> @@ -78,6 +78,21 @@
>  #define BMI260_INIT_DATA_FILE "bmi260-init-data.fw"
>  #define BMI270_INIT_DATA_FILE "bmi270-init-data.fw"
>  
> +struct bmi270_data {
> +	struct device *dev;
> +	struct regmap *regmap;
> +	const struct bmi270_chip_info *chip_info;
> +
> +	/*
> +	 * Where IIO_DMA_MINALIGN may be larger than 8 bytes, align to
> +	 * that to ensure a DMA safe buffer.
> +	 */
> +	struct {
> +		__le16 channels[6];
> +		aligned_s64 timestamp;
> +	} data __aligned(IIO_DMA_MINALIGN);
> +};
> +
>  enum bmi270_scan {
>  	BMI270_SCAN_ACCEL_X,
>  	BMI270_SCAN_ACCEL_Y,
> 
> -- 
> 2.48.1
> 

Re: [PATCH 2/3] iio: imu: bmi270: rename variable bmi270_device to data

[Alex Lanzano]

On Wed, Feb 19, 2025 at 08:54:46PM -0300, Gustavo Silva wrote:
> Rename all instances of 'struct bmi270_data' to 'data', to ensure
> consistency across the driver.
> 
> Signed-off-by: Gustavo Silva <gustavograzs@gmail.com>

Just to note, you should also include that you changed the name of the
nested struct in bmi270_data from 'data' to 'buffer' in your commit
message as well.

Acked-by: Alex Lanzano <lanzano.alex@gmail.com>

Re: [PATCH 1/3] iio: imu: bmi270: move private struct declaration to source file

[Jonathan Cameron]

On Sat, 22 Feb 2025 11:25:21 -0500
Alex Lanzano <lanzano.alex@gmail.com> wrote:

> On Wed, Feb 19, 2025 at 08:54:45PM -0300, Gustavo Silva wrote:
> > The device's private data struct is currently declared in the header
> > file, but it does not need to be exposed there. Move it to the driver's
> > core source file to avoid unnecessary #include directives or forward
> > declarations in the header.
> > 
> > Signed-off-by: Gustavo Silva <gustavograzs@gmail.com>  
> Acked-by: Alex Lanzano <lanzano.alex@gmail.com>
A good cleanup. So applied before I even look at 2 and 3!

Thanks,

Jonathan

> > ---
> >  drivers/iio/imu/bmi270/bmi270.h      | 17 +----------------
> >  drivers/iio/imu/bmi270/bmi270_core.c | 15 +++++++++++++++
> >  2 files changed, 16 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/iio/imu/bmi270/bmi270.h b/drivers/iio/imu/bmi270/bmi270.h
> > index fdfad5784cc52043475b6816286619fac5824684..d94525f6aee85f21cc9e9ae1bc9c1db0dc00b927 100644
> > --- a/drivers/iio/imu/bmi270/bmi270.h
> > +++ b/drivers/iio/imu/bmi270/bmi270.h
> > @@ -6,22 +6,6 @@
> >  #include <linux/regmap.h>
> >  #include <linux/iio/iio.h>
> >  
> > -struct device;
> > -struct bmi270_data {
> > -	struct device *dev;
> > -	struct regmap *regmap;
> > -	const struct bmi270_chip_info *chip_info;
> > -
> > -	/*
> > -	 * Where IIO_DMA_MINALIGN may be larger than 8 bytes, align to
> > -	 * that to ensure a DMA safe buffer.
> > -	 */
> > -	struct {
> > -		__le16 channels[6];
> > -		aligned_s64 timestamp;
> > -	} data __aligned(IIO_DMA_MINALIGN);
> > -};
> > -
> >  struct bmi270_chip_info {
> >  	const char *name;
> >  	int chip_id;
> > @@ -32,6 +16,7 @@ extern const struct regmap_config bmi270_regmap_config;
> >  extern const struct bmi270_chip_info bmi260_chip_info;
> >  extern const struct bmi270_chip_info bmi270_chip_info;
> >  
> > +struct device;
> >  int bmi270_core_probe(struct device *dev, struct regmap *regmap,
> >  		      const struct bmi270_chip_info *chip_info);
> >  
> > diff --git a/drivers/iio/imu/bmi270/bmi270_core.c b/drivers/iio/imu/bmi270/bmi270_core.c
> > index 464dcdd657c4ba27fdb7620c80e9f05e2a831910..9f24d4044ed6f00f67fd3a8f5adda821c3140a18 100644
> > --- a/drivers/iio/imu/bmi270/bmi270_core.c
> > +++ b/drivers/iio/imu/bmi270/bmi270_core.c
> > @@ -78,6 +78,21 @@
> >  #define BMI260_INIT_DATA_FILE "bmi260-init-data.fw"
> >  #define BMI270_INIT_DATA_FILE "bmi270-init-data.fw"
> >  
> > +struct bmi270_data {
> > +	struct device *dev;
> > +	struct regmap *regmap;
> > +	const struct bmi270_chip_info *chip_info;
> > +
> > +	/*
> > +	 * Where IIO_DMA_MINALIGN may be larger than 8 bytes, align to
> > +	 * that to ensure a DMA safe buffer.
> > +	 */
> > +	struct {
> > +		__le16 channels[6];
> > +		aligned_s64 timestamp;
> > +	} data __aligned(IIO_DMA_MINALIGN);
> > +};
> > +
> >  enum bmi270_scan {
> >  	BMI270_SCAN_ACCEL_X,
> >  	BMI270_SCAN_ACCEL_Y,
> > 
> > -- 
> > 2.48.1
> >   

Re: [PATCH 2/3] iio: imu: bmi270: rename variable bmi270_device to data

[Jonathan Cameron]

On Wed, 19 Feb 2025 20:54:46 -0300
Gustavo Silva <gustavograzs@gmail.com> wrote:

> Rename all instances of 'struct bmi270_data' to 'data', to ensure
> consistency across the driver.
> 
> Signed-off-by: Gustavo Silva <gustavograzs@gmail.com>
> ---
>  drivers/iio/imu/bmi270/bmi270_core.c | 79 ++++++++++++++++++------------------
>  1 file changed, 39 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/iio/imu/bmi270/bmi270_core.c b/drivers/iio/imu/bmi270/bmi270_core.c
> index 9f24d4044ed6f00f67fd3a8f5adda821c3140a18..56ebd887fcec94b684dc23f1b4503b719fb39239 100644
> --- a/drivers/iio/imu/bmi270/bmi270_core.c
> +++ b/drivers/iio/imu/bmi270/bmi270_core.c
> @@ -90,7 +90,7 @@ struct bmi270_data {
>  	struct {
>  		__le16 channels[6];
>  		aligned_s64 timestamp;
> -	} data __aligned(IIO_DMA_MINALIGN);
> +	} buffer __aligned(IIO_DMA_MINALIGN);
>  };
>  
>  enum bmi270_scan {
> @@ -284,7 +284,7 @@ static int bmi270_set_scale(struct bmi270_data *data, int chan_type, int uscale)
>  	return -EINVAL;
>  }
>  
> -static int bmi270_get_scale(struct bmi270_data *bmi270_device, int chan_type,
> +static int bmi270_get_scale(struct bmi270_data *data, int chan_type,
>  			    int *scale, int *uscale)
>  {
>  	int ret;
> @@ -293,7 +293,7 @@ static int bmi270_get_scale(struct bmi270_data *bmi270_device, int chan_type,
>  
>  	switch (chan_type) {
>  	case IIO_ACCEL:
> -		ret = regmap_read(bmi270_device->regmap,
> +		ret = regmap_read(data->regmap,
>  				  BMI270_ACC_CONF_RANGE_REG, &val);

Given shorter naming, a few places like this should be re line wrapped.

>  		if (ret)
>  			return ret;

Re: [PATCH 3/3] iio: imu: bmi270: add support for data ready interrupt trigger

[Alex Lanzano]

On Wed, Feb 19, 2025 at 08:54:47PM -0300, Gustavo Silva wrote:
> The BMI270 sensor provides two interrupt pins that can be used for
> different interrupt sources, including a data ready signal. Add support
> for configuring one the pins as a trigger source.
> 
> The interrupt pin can be configured with various options: active high or
> low, push-pull or open-drain, and latched or non-latched.
> 
> Signed-off-by: Gustavo Silva <gustavograzs@gmail.com>

Most of this patch seems to based off of the implementation done in
bmi323. Double checked the relevant registers in the datasheet. All
looks correct.

Acked-by: Alex Lanzano <lanzano.alex@gmail.com