[PATCH v4 0/4] Add i2c driver for Bosch BMI260 IMU

[PATCH v4 0/4] Add i2c driver for Bosch BMI260 IMU

[Justin Weiss]

Add support for the Bosch BMI260 IMU to the BMI270 device driver.

The BMI270 and BMI260 have nearly identical register maps, but have
different chip IDs and firmware.

The BMI260 is the IMU on a number of handheld PCs. Unfortunately,
these devices often misidentify it in ACPI as a BMI160 ("BMI0160," for
example), and it can only be correctly identified using the chip
ID. To avoid conflicts with the bmi160 driver, this driver will not
probe if it detects a BMI160 chip ID.

Also add triggered buffer and scale / sampling frequency attributes,
which the input tools commonly used on handheld PCs require to support
IMUs.

Like the BMI270, the BMI260 requires firmware to be provided.
Signed-off-by: Justin Weiss <justin@justinweiss.com>
---

Changelog:

V4
- Move triggered buffer and attributes patches to the front of the set
- Add more detailed commit message to DT documentation patch
- Remove ACPI IDs from SPI driver
- Remove 10EC5280 and BMI0260 ACPI IDs from I2C driver
- Add DSDT excerpt for BMI0160 ACPI ID

V3
https://lore.kernel.org/lkml/20241020220011.212395-1-justin@justinweiss.com/
- Fix: Remove SCALE and FREQUENCY attributes
- Use separate configuration structures instead of an array
- Add bmi260 as compatible ID in bmi270 dt binding doc
- Check chip ID against value in configuration instead of constant
- Update comment for DMA alignment
- Remove unreachable return statement

V2
https://lore.kernel.org/all/20241018233723.28757-1-justin@justinweiss.com/
- Fix commit titles
- Fix: Change FREQUENCY to SAMP_FREQ
- Split chip_info refactor into a separate commit from adding bmi260
- Only fail probe when BMI160 is detected
- Update chip_info based on detected chip ID
- Add BMI260 to DT documentation
- Add BMI260 to of_device_id
- Add expected BMI260 ACPI ID to the SPI driver
- Remove unused/unexpected BMI260 ACPI IDs
- Remove trailing comma for null terminators
- Use DMA_MINALIGN for channel buffer
- Read channels in bulk
- Improve for loops for detecting scale / odr attrs
- Add missing masks
- Use FIELD_GET
- Use read_avail instead of custom attrs
- Misc. formatting and line wrapping improvements

V1
https://lore.kernel.org/all/20241011153751.65152-1-justin@justinweiss.com/

Justin Weiss (4):
  iio: imu: bmi270: Add triggered buffer for Bosch BMI270 IMU
  iio: imu: bmi270: Add scale and sampling frequency to BMI270 IMU
  dt-bindings: iio: imu: bmi270: Add Bosch BMI260
  iio: imu: bmi270: Add support for BMI260

 .../bindings/iio/imu/bosch,bmi270.yaml        |   4 +-
 drivers/iio/imu/bmi270/Kconfig                |   1 +
 drivers/iio/imu/bmi270/bmi270.h               |  10 +
 drivers/iio/imu/bmi270/bmi270_core.c          | 424 +++++++++++++++++-
 drivers/iio/imu/bmi270/bmi270_i2c.c           |   9 +
 drivers/iio/imu/bmi270/bmi270_spi.c           |   2 +
 6 files changed, 448 insertions(+), 2 deletions(-)


base-commit: 9090ececac9ff1e22fb7e042f3c886990a8fb090
-- 
2.47.0

[PATCH v4 1/4] iio: imu: bmi270: Add triggered buffer for Bosch BMI270 IMU

[Justin Weiss]

Set up a triggered buffer for the accel and angl_vel values.

Signed-off-by: Justin Weiss <justin@justinweiss.com>
---
 drivers/iio/imu/bmi270/Kconfig       |  1 +
 drivers/iio/imu/bmi270/bmi270.h      |  9 +++++
 drivers/iio/imu/bmi270/bmi270_core.c | 56 ++++++++++++++++++++++++++++
 3 files changed, 66 insertions(+)

diff --git a/drivers/iio/imu/bmi270/Kconfig b/drivers/iio/imu/bmi270/Kconfig
index 0ffd29794fda..6362acc706da 100644
--- a/drivers/iio/imu/bmi270/Kconfig
+++ b/drivers/iio/imu/bmi270/Kconfig
@@ -6,6 +6,7 @@
 config BMI270
 	tristate
 	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
 
 config BMI270_I2C
 	tristate "Bosch BMI270 I2C driver"
diff --git a/drivers/iio/imu/bmi270/bmi270.h b/drivers/iio/imu/bmi270/bmi270.h
index 93e5f387607b..6173be929bac 100644
--- a/drivers/iio/imu/bmi270/bmi270.h
+++ b/drivers/iio/imu/bmi270/bmi270.h
@@ -11,6 +11,15 @@ 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 {
diff --git a/drivers/iio/imu/bmi270/bmi270_core.c b/drivers/iio/imu/bmi270/bmi270_core.c
index 5f08d786fa21..1608cb2c8fb5 100644
--- a/drivers/iio/imu/bmi270/bmi270_core.c
+++ b/drivers/iio/imu/bmi270/bmi270_core.c
@@ -7,6 +7,8 @@
 #include <linux/regmap.h>
 
 #include <linux/iio/iio.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger_consumer.h>
 
 #include "bmi270.h"
 
@@ -64,6 +66,17 @@ enum bmi270_scan {
 	BMI270_SCAN_GYRO_X,
 	BMI270_SCAN_GYRO_Y,
 	BMI270_SCAN_GYRO_Z,
+	BMI270_SCAN_TIMESTAMP,
+};
+
+static const unsigned long bmi270_avail_scan_masks[] = {
+	(BIT(BMI270_SCAN_ACCEL_X) |
+	 BIT(BMI270_SCAN_ACCEL_Y) |
+	 BIT(BMI270_SCAN_ACCEL_Z) |
+	 BIT(BMI270_SCAN_GYRO_X) |
+	 BIT(BMI270_SCAN_GYRO_Y) |
+	 BIT(BMI270_SCAN_GYRO_Z)),
+	0
 };
 
 const struct bmi270_chip_info bmi270_chip_info = {
@@ -73,6 +86,27 @@ const struct bmi270_chip_info bmi270_chip_info = {
 };
 EXPORT_SYMBOL_NS_GPL(bmi270_chip_info, IIO_BMI270);
 
+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);
+	int ret;
+
+	ret = regmap_bulk_read(bmi270_device->regmap, BMI270_ACCEL_X_REG,
+			       &bmi270_device->data.channels,
+			       sizeof(bmi270_device->data.channels));
+
+	if (ret)
+		goto done;
+
+	iio_push_to_buffers_with_timestamp(indio_dev, &bmi270_device->data,
+					   pf->timestamp);
+done:
+	iio_trigger_notify_done(indio_dev->trig);
+	return IRQ_HANDLED;
+}
+
 static int bmi270_get_data(struct bmi270_data *bmi270_device,
 			   int chan_type, int axis, int *val)
 {
@@ -128,6 +162,13 @@ static const struct iio_info bmi270_info = {
 	.modified = 1,						\
 	.channel2 = IIO_MOD_##_axis,				\
 	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
+	.scan_index = BMI270_SCAN_ACCEL_##_axis,		\
+	.scan_type = {						\
+		.sign = 's',					\
+		.realbits = 16,					\
+		.storagebits = 16,				\
+		.endianness = IIO_LE				\
+	},	                                                \
 }
 
 #define BMI270_ANG_VEL_CHANNEL(_axis) {				\
@@ -135,6 +176,13 @@ static const struct iio_info bmi270_info = {
 	.modified = 1,						\
 	.channel2 = IIO_MOD_##_axis,				\
 	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
+	.scan_index = BMI270_SCAN_GYRO_##_axis,			\
+	.scan_type = {						\
+		.sign = 's',					\
+		.realbits = 16,					\
+		.storagebits = 16,				\
+		.endianness = IIO_LE				\
+	},	                                                \
 }
 
 static const struct iio_chan_spec bmi270_channels[] = {
@@ -144,6 +192,7 @@ static const struct iio_chan_spec bmi270_channels[] = {
 	BMI270_ANG_VEL_CHANNEL(X),
 	BMI270_ANG_VEL_CHANNEL(Y),
 	BMI270_ANG_VEL_CHANNEL(Z),
+	IIO_CHAN_SOFT_TIMESTAMP(BMI270_SCAN_TIMESTAMP),
 };
 
 static int bmi270_validate_chip_id(struct bmi270_data *bmi270_device)
@@ -301,9 +350,16 @@ int bmi270_core_probe(struct device *dev, struct regmap *regmap,
 	indio_dev->channels = bmi270_channels;
 	indio_dev->num_channels = ARRAY_SIZE(bmi270_channels);
 	indio_dev->name = chip_info->name;
+	indio_dev->available_scan_masks = bmi270_avail_scan_masks;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->info = &bmi270_info;
 
+	ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
+					      iio_pollfunc_store_time,
+					      bmi270_trigger_handler, NULL);
+	if (ret)
+		return ret;
+
 	return devm_iio_device_register(dev, indio_dev);
 }
 EXPORT_SYMBOL_NS_GPL(bmi270_core_probe, IIO_BMI270);
-- 
2.47.0

[PATCH v4 2/4] iio: imu: bmi270: Add scale and sampling frequency to BMI270 IMU

[Justin Weiss]

Add read and write functions and create _available entries.

Signed-off-by: Justin Weiss <justin@justinweiss.com>
---
 drivers/iio/imu/bmi270/bmi270_core.c | 340 +++++++++++++++++++++++++++
 1 file changed, 340 insertions(+)

diff --git a/drivers/iio/imu/bmi270/bmi270_core.c b/drivers/iio/imu/bmi270/bmi270_core.c
index 1608cb2c8fb5..27e501a15095 100644
--- a/drivers/iio/imu/bmi270/bmi270_core.c
+++ b/drivers/iio/imu/bmi270/bmi270_core.c
@@ -7,6 +7,7 @@
 #include <linux/regmap.h>
 
 #include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
 #include <linux/iio/triggered_buffer.h>
 #include <linux/iio/trigger_consumer.h>
 
@@ -33,6 +34,9 @@
 #define BMI270_ACC_CONF_BWP_NORMAL_MODE			0x02
 #define BMI270_ACC_CONF_FILTER_PERF_MSK			BIT(7)
 
+#define BMI270_ACC_CONF_RANGE_REG			0x41
+#define BMI270_ACC_CONF_RANGE_MSK			GENMASK(1, 0)
+
 #define BMI270_GYR_CONF_REG				0x42
 #define BMI270_GYR_CONF_ODR_MSK				GENMASK(3, 0)
 #define BMI270_GYR_CONF_ODR_200HZ			0x09
@@ -41,6 +45,9 @@
 #define BMI270_GYR_CONF_NOISE_PERF_MSK			BIT(6)
 #define BMI270_GYR_CONF_FILTER_PERF_MSK			BIT(7)
 
+#define BMI270_GYR_CONF_RANGE_REG			0x43
+#define BMI270_GYR_CONF_RANGE_MSK			GENMASK(2, 0)
+
 #define BMI270_INIT_CTRL_REG				0x59
 #define BMI270_INIT_CTRL_LOAD_DONE_MSK			BIT(0)
 
@@ -86,6 +93,265 @@ const struct bmi270_chip_info bmi270_chip_info = {
 };
 EXPORT_SYMBOL_NS_GPL(bmi270_chip_info, IIO_BMI270);
 
+enum bmi270_sensor_type {
+	BMI270_ACCEL	= 0,
+	BMI270_GYRO,
+};
+
+struct bmi270_scale {
+	int scale;
+	int uscale;
+};
+
+struct bmi270_odr {
+	int odr;
+	int uodr;
+};
+
+static const struct bmi270_scale bmi270_accel_scale[] = {
+	{ 0, 598 },
+	{ 0, 1197 },
+	{ 0, 2394 },
+	{ 0, 4788 },
+};
+
+static const struct bmi270_scale bmi270_gyro_scale[] = {
+	{ 0, 1065 },
+	{ 0, 532 },
+	{ 0, 266 },
+	{ 0, 133 },
+	{ 0, 66 },
+};
+
+struct bmi270_scale_item {
+	const struct bmi270_scale *tbl;
+	int num;
+};
+
+static const struct bmi270_scale_item bmi270_scale_table[] = {
+	[BMI270_ACCEL] = {
+		.tbl	= bmi270_accel_scale,
+		.num	= ARRAY_SIZE(bmi270_accel_scale),
+	},
+	[BMI270_GYRO] = {
+		.tbl	= bmi270_gyro_scale,
+		.num	= ARRAY_SIZE(bmi270_gyro_scale),
+	},
+};
+
+static const struct bmi270_odr bmi270_accel_odr[] = {
+	{ 0, 781250 },
+	{ 1, 562500 },
+	{ 3, 125000 },
+	{ 6, 250000 },
+	{ 12, 500000 },
+	{ 25, 0 },
+	{ 50, 0 },
+	{ 100, 0 },
+	{ 200, 0 },
+	{ 400, 0 },
+	{ 800, 0 },
+	{ 1600, 0 },
+};
+
+static const u8 bmi270_accel_odr_vals[] = {
+	0x01,
+	0x02,
+	0x03,
+	0x04,
+	0x05,
+	0x06,
+	0x07,
+	0x08,
+	0x09,
+	0x0A,
+	0x0B,
+	0x0C,
+};
+
+static const struct bmi270_odr bmi270_gyro_odr[] = {
+	{ 25, 0 },
+	{ 50, 0 },
+	{ 100, 0 },
+	{ 200, 0 },
+	{ 400, 0 },
+	{ 800, 0 },
+	{ 1600, 0 },
+	{ 3200, 0 },
+};
+
+static const u8 bmi270_gyro_odr_vals[] = {
+	0x06,
+	0x07,
+	0x08,
+	0x09,
+	0x0A,
+	0x0B,
+	0x0C,
+	0x0D,
+};
+
+struct bmi270_odr_item {
+	const struct bmi270_odr *tbl;
+	const u8 *vals;
+	int num;
+};
+
+static const struct  bmi270_odr_item bmi270_odr_table[] = {
+	[BMI270_ACCEL] = {
+		.tbl	= bmi270_accel_odr,
+		.vals   = bmi270_accel_odr_vals,
+		.num	= ARRAY_SIZE(bmi270_accel_odr),
+	},
+	[BMI270_GYRO] = {
+		.tbl	= bmi270_gyro_odr,
+		.vals   = bmi270_gyro_odr_vals,
+		.num	= ARRAY_SIZE(bmi270_gyro_odr),
+	},
+};
+
+static int bmi270_set_scale(struct bmi270_data *data,
+			    int chan_type, int uscale)
+{
+	int i;
+	int reg, mask;
+	struct bmi270_scale_item bmi270_scale_item;
+
+	switch (chan_type) {
+	case IIO_ACCEL:
+		reg = BMI270_ACC_CONF_RANGE_REG;
+		mask = BMI270_ACC_CONF_RANGE_MSK;
+		bmi270_scale_item = bmi270_scale_table[BMI270_ACCEL];
+		break;
+	case IIO_ANGL_VEL:
+		reg = BMI270_GYR_CONF_RANGE_REG;
+		mask = BMI270_GYR_CONF_RANGE_MSK;
+		bmi270_scale_item = bmi270_scale_table[BMI270_GYRO];
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	for (i = 0; i < bmi270_scale_item.num; i++) {
+		if (bmi270_scale_item.tbl[i].uscale != uscale)
+			continue;
+
+		return regmap_update_bits(data->regmap, reg, mask, i);
+	}
+
+	return -EINVAL;
+}
+
+static int bmi270_get_scale(struct bmi270_data *bmi270_device,
+			    int chan_type, int *uscale)
+{
+	int ret;
+	unsigned int val;
+	struct bmi270_scale_item bmi270_scale_item;
+
+	switch (chan_type) {
+	case IIO_ACCEL:
+		ret = regmap_read(bmi270_device->regmap,
+				  BMI270_ACC_CONF_RANGE_REG, &val);
+		if (ret)
+			return ret;
+
+		val = FIELD_GET(BMI270_ACC_CONF_RANGE_MSK, val);
+		bmi270_scale_item = bmi270_scale_table[BMI270_ACCEL];
+		break;
+	case IIO_ANGL_VEL:
+		ret = regmap_read(bmi270_device->regmap,
+				  BMI270_GYR_CONF_RANGE_REG, &val);
+		if (ret)
+			return ret;
+
+		val = FIELD_GET(BMI270_GYR_CONF_RANGE_MSK, val);
+		bmi270_scale_item = bmi270_scale_table[BMI270_GYRO];
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (val >= bmi270_scale_item.num)
+		return -EINVAL;
+
+	*uscale = bmi270_scale_item.tbl[val].uscale;
+	return 0;
+}
+
+static int bmi270_set_odr(struct bmi270_data *data, int chan_type,
+			  int odr, int uodr)
+{
+	int i;
+	int reg, mask;
+	struct bmi270_odr_item bmi270_odr_item;
+
+	switch (chan_type) {
+	case IIO_ACCEL:
+		reg = BMI270_ACC_CONF_REG;
+		mask = BMI270_ACC_CONF_ODR_MSK;
+		bmi270_odr_item = bmi270_odr_table[BMI270_ACCEL];
+		break;
+	case IIO_ANGL_VEL:
+		reg = BMI270_GYR_CONF_REG;
+		mask = BMI270_GYR_CONF_ODR_MSK;
+		bmi270_odr_item = bmi270_odr_table[BMI270_GYRO];
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	for (i = 0; i < bmi270_odr_item.num; i++) {
+		if (bmi270_odr_item.tbl[i].odr != odr ||
+		    bmi270_odr_item.tbl[i].uodr != uodr)
+			continue;
+
+		return regmap_update_bits(data->regmap, reg, mask,
+					  bmi270_odr_item.vals[i]);
+	}
+
+	return -EINVAL;
+}
+
+static int bmi270_get_odr(struct bmi270_data *data, int chan_type,
+			  int *odr, int *uodr)
+{
+	int i, val, ret;
+	struct bmi270_odr_item bmi270_odr_item;
+
+	switch (chan_type) {
+	case IIO_ACCEL:
+		ret = regmap_read(data->regmap, BMI270_ACC_CONF_REG, &val);
+		if (ret)
+			return ret;
+
+		val = FIELD_GET(BMI270_ACC_CONF_ODR_MSK, val);
+		bmi270_odr_item = bmi270_odr_table[BMI270_ACCEL];
+		break;
+	case IIO_ANGL_VEL:
+		ret = regmap_read(data->regmap, BMI270_GYR_CONF_REG, &val);
+		if (ret)
+			return ret;
+
+		val = FIELD_GET(BMI270_GYR_CONF_ODR_MSK, val);
+		bmi270_odr_item = bmi270_odr_table[BMI270_GYRO];
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	for (i = 0; i < bmi270_odr_item.num; i++) {
+		if (val != bmi270_odr_item.vals[i])
+			continue;
+
+		*odr = bmi270_odr_item.tbl[i].odr;
+		*uodr = bmi270_odr_item.tbl[i].uodr;
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
 static irqreturn_t bmi270_trigger_handler(int irq, void *p)
 {
 	struct iio_poll_func *pf = p;
@@ -148,6 +414,68 @@ static int bmi270_read_raw(struct iio_dev *indio_dev,
 			return ret;
 
 		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		*val = 0;
+		ret = bmi270_get_scale(bmi270_device, chan->type, val2);
+		return ret ? ret : IIO_VAL_INT_PLUS_MICRO;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		ret = bmi270_get_odr(bmi270_device, chan->type, val, val2);
+		return ret ? ret : IIO_VAL_INT_PLUS_MICRO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int bmi270_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int val, int val2, long mask)
+{
+	struct bmi270_data *data = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		return bmi270_set_scale(data, chan->type, val2);
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		return bmi270_set_odr(data, chan->type, val, val2);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int bmi270_read_avail(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     const int **vals, int *type, int *length,
+			     long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		*type = IIO_VAL_INT_PLUS_MICRO;
+		switch (chan->type) {
+		case IIO_ANGL_VEL:
+			*vals = (const int *)bmi270_gyro_scale;
+			*length = ARRAY_SIZE(bmi270_gyro_scale) * 2;
+			return IIO_AVAIL_LIST;
+		case IIO_ACCEL:
+			*vals = (const int *)bmi270_accel_scale;
+			*length = ARRAY_SIZE(bmi270_accel_scale) * 2;
+			return IIO_AVAIL_LIST;
+		default:
+			return -EINVAL;
+		}
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*type = IIO_VAL_INT_PLUS_MICRO;
+		switch (chan->type) {
+		case IIO_ANGL_VEL:
+			*vals = (const int *)bmi270_gyro_odr;
+			*length = ARRAY_SIZE(bmi270_gyro_odr) * 2;
+			return IIO_AVAIL_LIST;
+		case IIO_ACCEL:
+			*vals = (const int *)bmi270_accel_odr;
+			*length = ARRAY_SIZE(bmi270_accel_odr) * 2;
+			return IIO_AVAIL_LIST;
+		default:
+			return -EINVAL;
+		}
 	default:
 		return -EINVAL;
 	}
@@ -155,6 +483,8 @@ static int bmi270_read_raw(struct iio_dev *indio_dev,
 
 static const struct iio_info bmi270_info = {
 	.read_raw = bmi270_read_raw,
+	.write_raw = bmi270_write_raw,
+	.read_avail = bmi270_read_avail,
 };
 
 #define BMI270_ACCEL_CHANNEL(_axis) {				\
@@ -162,6 +492,11 @@ static const struct iio_info bmi270_info = {
 	.modified = 1,						\
 	.channel2 = IIO_MOD_##_axis,				\
 	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
+		BIT(IIO_CHAN_INFO_SAMP_FREQ),			\
+	.info_mask_shared_by_type_available =			\
+		BIT(IIO_CHAN_INFO_SCALE) |			\
+		BIT(IIO_CHAN_INFO_SAMP_FREQ),			\
 	.scan_index = BMI270_SCAN_ACCEL_##_axis,		\
 	.scan_type = {						\
 		.sign = 's',					\
@@ -176,6 +511,11 @@ static const struct iio_info bmi270_info = {
 	.modified = 1,						\
 	.channel2 = IIO_MOD_##_axis,				\
 	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
+		BIT(IIO_CHAN_INFO_SAMP_FREQ),			\
+	.info_mask_shared_by_type_available =			\
+		BIT(IIO_CHAN_INFO_SCALE) |			\
+		BIT(IIO_CHAN_INFO_SAMP_FREQ),			\
 	.scan_index = BMI270_SCAN_GYRO_##_axis,			\
 	.scan_type = {						\
 		.sign = 's',					\
-- 
2.47.0

[PATCH v4 3/4] dt-bindings: iio: imu: bmi270: Add Bosch BMI260

[Justin Weiss]

The BMI260's register map, configuration, and capabilities are nearly
identical to the BMI270, but the devices have different chip IDs and
require different initialization firmware.

Signed-off-by: Justin Weiss <justin@justinweiss.com>
---
 Documentation/devicetree/bindings/iio/imu/bosch,bmi270.yaml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iio/imu/bosch,bmi270.yaml b/Documentation/devicetree/bindings/iio/imu/bosch,bmi270.yaml
index 792d1483af3c..7b0cde1c9b0a 100644
--- a/Documentation/devicetree/bindings/iio/imu/bosch,bmi270.yaml
+++ b/Documentation/devicetree/bindings/iio/imu/bosch,bmi270.yaml
@@ -18,7 +18,9 @@ description: |
 
 properties:
   compatible:
-    const: bosch,bmi270
+    enum:
+      - bosch,bmi260
+      - bosch,bmi270
 
   reg:
     maxItems: 1
-- 
2.47.0

[PATCH v4 4/4] iio: imu: bmi270: Add support for BMI260

[Justin Weiss]

Adds support for the Bosch BMI260 6-axis IMU to the Bosch BMI270
driver. Setup and operation is nearly identical to the Bosch BMI270,
but has a different chip ID and requires different firmware.

Firmware is requested and loaded from userspace.

Adds ACPI ID BMI0160, used by several devices including the GPD Win
Mini, Aya Neo AIR Pro, and OXP Mini Pro.

GPD Win Mini:

Device (BMI2)
{
    Name (_ADR, Zero)  // _ADR: Address
    Name (_HID, "BMI0160")  // _HID: Hardware ID
    Name (_CID, "BMI0160")  // _CID: Compatible ID
    Name (_DDN, "Accelerometer")  // _DDN: DOS Device Name
    Name (_UID, One)  // _UID: Unique ID
    Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
    {
        Name (RBUF, ResourceTemplate ()
        {
            I2cSerialBusV2 (0x0068, ControllerInitiated, 0x00061A80,
                AddressingMode7Bit, "\\_SB.I2CB",
                0x00, ResourceConsumer, , Exclusive,
                )
            GpioInt (Edge, ActiveLow, Exclusive, PullDefault, 0x0000,
                "\\_SB.GPIO", 0x00, ResourceConsumer, ,
                )
                {   // Pin list
                    0x008B
                }
        })
        Return (RBUF) /* \_SB_.I2CB.BMI2._CRS.RBUF */
    }
    ...
}

Signed-off-by: Justin Weiss <justin@justinweiss.com>
---
 drivers/iio/imu/bmi270/bmi270.h      |  1 +
 drivers/iio/imu/bmi270/bmi270_core.c | 28 +++++++++++++++++++++++++++-
 drivers/iio/imu/bmi270/bmi270_i2c.c  |  9 +++++++++
 drivers/iio/imu/bmi270/bmi270_spi.c  |  2 ++
 4 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/imu/bmi270/bmi270.h b/drivers/iio/imu/bmi270/bmi270.h
index 6173be929bac..fdfad5784cc5 100644
--- a/drivers/iio/imu/bmi270/bmi270.h
+++ b/drivers/iio/imu/bmi270/bmi270.h
@@ -29,6 +29,7 @@ struct bmi270_chip_info {
 };
 
 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;
 
 int bmi270_core_probe(struct device *dev, struct regmap *regmap,
diff --git a/drivers/iio/imu/bmi270/bmi270_core.c b/drivers/iio/imu/bmi270/bmi270_core.c
index 27e501a15095..938cf14d46bd 100644
--- a/drivers/iio/imu/bmi270/bmi270_core.c
+++ b/drivers/iio/imu/bmi270/bmi270_core.c
@@ -14,6 +14,11 @@
 #include "bmi270.h"
 
 #define BMI270_CHIP_ID_REG				0x00
+
+/* Checked to prevent sending incompatible firmware to BMI160 devices */
+#define BMI160_CHIP_ID_VAL				0xD1
+
+#define BMI260_CHIP_ID_VAL				0x27
 #define BMI270_CHIP_ID_VAL				0x24
 #define BMI270_CHIP_ID_MSK				GENMASK(7, 0)
 
@@ -64,6 +69,7 @@
 #define BMI270_PWR_CTRL_ACCEL_EN_MSK			BIT(2)
 #define BMI270_PWR_CTRL_TEMP_EN_MSK			BIT(3)
 
+#define BMI260_INIT_DATA_FILE "bmi260-init-data.fw"
 #define BMI270_INIT_DATA_FILE "bmi270-init-data.fw"
 
 enum bmi270_scan {
@@ -86,6 +92,13 @@ static const unsigned long bmi270_avail_scan_masks[] = {
 	0
 };
 
+const struct bmi270_chip_info bmi260_chip_info = {
+	.name = "bmi260",
+	.chip_id = BMI260_CHIP_ID_VAL,
+	.fw_name = BMI260_INIT_DATA_FILE,
+};
+EXPORT_SYMBOL_NS_GPL(bmi260_chip_info, IIO_BMI270);
+
 const struct bmi270_chip_info bmi270_chip_info = {
 	.name = "bmi270",
 	.chip_id = BMI270_CHIP_ID_VAL,
@@ -546,8 +559,21 @@ static int bmi270_validate_chip_id(struct bmi270_data *bmi270_device)
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to read chip id");
 
+	/*
+	 * Some manufacturers use "BMI0160" for both the BMI160 and
+	 * BMI260. If the device is actually a BMI160, the bmi160
+	 * driver should handle it and this driver should not.
+	 */
+	if (chip_id == BMI160_CHIP_ID_VAL)
+		return -ENODEV;
+
 	if (chip_id != bmi270_device->chip_info->chip_id)
-		dev_info(dev, "Unknown chip id 0x%x", 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;
+	else if (chip_id == bmi270_chip_info.chip_id)
+		bmi270_device->chip_info = &bmi270_chip_info;
 
 	return 0;
 }
diff --git a/drivers/iio/imu/bmi270/bmi270_i2c.c b/drivers/iio/imu/bmi270/bmi270_i2c.c
index 394f27996059..6bd82e4362ab 100644
--- a/drivers/iio/imu/bmi270/bmi270_i2c.c
+++ b/drivers/iio/imu/bmi270/bmi270_i2c.c
@@ -32,11 +32,19 @@ static int bmi270_i2c_probe(struct i2c_client *client)
 }
 
 static const struct i2c_device_id bmi270_i2c_id[] = {
+	{ "bmi260", (kernel_ulong_t)&bmi260_chip_info },
 	{ "bmi270", (kernel_ulong_t)&bmi270_chip_info },
 	{ }
 };
 
+static const struct acpi_device_id bmi270_acpi_match[] = {
+	/* GPD Win Mini, Aya Neo AIR Pro, OXP Mini Pro, etc. */
+	{ "BMI0160",  (kernel_ulong_t)&bmi260_chip_info },
+	{ }
+};
+
 static const struct of_device_id bmi270_of_match[] = {
+	{ .compatible = "bosch,bmi260", .data = &bmi260_chip_info },
 	{ .compatible = "bosch,bmi270", .data = &bmi270_chip_info },
 	{ }
 };
@@ -44,6 +52,7 @@ static const struct of_device_id bmi270_of_match[] = {
 static struct i2c_driver bmi270_i2c_driver = {
 	.driver = {
 		.name = "bmi270_i2c",
+		.acpi_match_table = bmi270_acpi_match,
 		.of_match_table = bmi270_of_match,
 	},
 	.probe = bmi270_i2c_probe,
diff --git a/drivers/iio/imu/bmi270/bmi270_spi.c b/drivers/iio/imu/bmi270/bmi270_spi.c
index 7c2062c660d9..b1fd9fc5dc98 100644
--- a/drivers/iio/imu/bmi270/bmi270_spi.c
+++ b/drivers/iio/imu/bmi270/bmi270_spi.c
@@ -65,11 +65,13 @@ static int bmi270_spi_probe(struct spi_device *spi)
 }
 
 static const struct spi_device_id bmi270_spi_id[] = {
+	{ "bmi260", (kernel_ulong_t)&bmi260_chip_info},
 	{ "bmi270", (kernel_ulong_t)&bmi270_chip_info },
 	{ }
 };
 
 static const struct of_device_id bmi270_of_match[] = {
+	{ .compatible = "bosch,bmi260", .data = &bmi260_chip_info },
 	{ .compatible = "bosch,bmi270", .data = &bmi270_chip_info },
 	{ }
 };
-- 
2.47.0

Re: [PATCH v4 3/4] dt-bindings: iio: imu: bmi270: Add Bosch BMI260

[Krzysztof Kozlowski]

On Sun, Oct 27, 2024 at 10:20:24AM -0700, Justin Weiss wrote:
> The BMI260's register map, configuration, and capabilities are nearly
> identical to the BMI270, but the devices have different chip IDs and
> require different initialization firmware.
> 
> Signed-off-by: Justin Weiss <justin@justinweiss.com>

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v4 4/4] iio: imu: bmi270: Add support for BMI260

[Andy Shevchenko]

On Sun, Oct 27, 2024 at 10:20:25AM -0700, Justin Weiss wrote:
> Adds support for the Bosch BMI260 6-axis IMU to the Bosch BMI270
> driver. Setup and operation is nearly identical to the Bosch BMI270,
> but has a different chip ID and requires different firmware.
> 
> Firmware is requested and loaded from userspace.
> 
> Adds ACPI ID BMI0160, used by several devices including the GPD Win
> Mini, Aya Neo AIR Pro, and OXP Mini Pro.
> 
> GPD Win Mini:
> 
> Device (BMI2)
> {
>     Name (_ADR, Zero)  // _ADR: Address
>     Name (_HID, "BMI0160")  // _HID: Hardware ID
>     Name (_CID, "BMI0160")  // _CID: Compatible ID
>     Name (_DDN, "Accelerometer")  // _DDN: DOS Device Name
>     Name (_UID, One)  // _UID: Unique ID
>     Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
>     {
>         Name (RBUF, ResourceTemplate ()
>         {
>             I2cSerialBusV2 (0x0068, ControllerInitiated, 0x00061A80,
>                 AddressingMode7Bit, "\\_SB.I2CB",
>                 0x00, ResourceConsumer, , Exclusive,
>                 )
>             GpioInt (Edge, ActiveLow, Exclusive, PullDefault, 0x0000,
>                 "\\_SB.GPIO", 0x00, ResourceConsumer, ,
>                 )
>                 {   // Pin list
>                     0x008B
>                 }
>         })
>         Return (RBUF) /* \_SB_.I2CB.BMI2._CRS.RBUF */
>     }
>     ...
> }

LGTM from ACPI ID perspective,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

...

>  static const struct spi_device_id bmi270_spi_id[] = {
> +	{ "bmi260", (kernel_ulong_t)&bmi260_chip_info},

Missed space.

>  	{ "bmi270", (kernel_ulong_t)&bmi270_chip_info },
>  	{ }
>  };

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 1/4] iio: imu: bmi270: Add triggered buffer for Bosch BMI270 IMU

[Andy Shevchenko]

On Sun, Oct 27, 2024 at 10:20:22AM -0700, Justin Weiss wrote:
> Set up a triggered buffer for the accel and angl_vel values.

...

>  	.channel2 = IIO_MOD_##_axis,				\
>  	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> +	.scan_index = BMI270_SCAN_ACCEL_##_axis,		\
> +	.scan_type = {						\
> +		.sign = 's',					\
> +		.realbits = 16,					\
> +		.storagebits = 16,				\
> +		.endianness = IIO_LE				\

Leave trailing comma here.

> +	},	                                                \

...

>  	.channel2 = IIO_MOD_##_axis,				\
>  	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> +	.scan_index = BMI270_SCAN_GYRO_##_axis,			\
> +	.scan_type = {						\
> +		.sign = 's',					\
> +		.realbits = 16,					\
> +		.storagebits = 16,				\
> +		.endianness = IIO_LE				\

Ditto.

> +	},	                                                \

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 2/4] iio: imu: bmi270: Add scale and sampling frequency to BMI270 IMU

[Andy Shevchenko]

On Sun, Oct 27, 2024 at 10:20:23AM -0700, Justin Weiss wrote:
> Add read and write functions and create _available entries.

...

> +static int bmi270_set_scale(struct bmi270_data *data,
> +			    int chan_type, int uscale)

There is available space in the previous line, (And I would even join them
despite being 83 characters long.)

...

> +static int bmi270_get_scale(struct bmi270_data *bmi270_device,
> +			    int chan_type, int *uscale)

Ditto (for chan_type).

...

> +static int bmi270_set_odr(struct bmi270_data *data, int chan_type,
> +			  int odr, int uodr)

Ditto.

...

> +	for (i = 0; i < bmi270_odr_item.num; i++) {
> +		if (bmi270_odr_item.tbl[i].odr != odr ||
> +		    bmi270_odr_item.tbl[i].uodr != uodr)
> +			continue;
> +
> +		return regmap_update_bits(data->regmap, reg, mask,
> +					  bmi270_odr_item.vals[i]);
> +	}
> +
> +	return -EINVAL;

Wouldn't be better to use regular patterns, i.e. checking for errors first?

	for (i = 0; i < bmi270_odr_item.num; i++) {
		if (bmi270_odr_item.tbl[i].odr == odr ||
		    bmi270_odr_item.tbl[i].uodr == uodr)
			break;
	}
	if (i == bmi270_odr_item.num)
		return -EINVAL;

	return regmap_update_bits(data->regmap, reg, mask, bmi270_odr_item.vals[i]);

...

> +static int bmi270_get_odr(struct bmi270_data *data, int chan_type,
> +			  int *odr, int *uodr)

As per above.

> +	for (i = 0; i < bmi270_odr_item.num; i++) {
> +		if (val != bmi270_odr_item.vals[i])
> +			continue;
> +
> +		*odr = bmi270_odr_item.tbl[i].odr;
> +		*uodr = bmi270_odr_item.tbl[i].uodr;
> +		return 0;
> +	}
> +
> +	return -EINVAL;

As per above.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 1/4] iio: imu: bmi270: Add triggered buffer for Bosch BMI270 IMU

[Jonathan Cameron]

On Mon, 28 Oct 2024 11:25:06 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Sun, Oct 27, 2024 at 10:20:22AM -0700, Justin Weiss wrote:
> > Set up a triggered buffer for the accel and angl_vel values.  
> 
> ...
> 
> >  	.channel2 = IIO_MOD_##_axis,				\
> >  	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> > +	.scan_index = BMI270_SCAN_ACCEL_##_axis,		\
> > +	.scan_type = {						\
> > +		.sign = 's',					\
> > +		.realbits = 16,					\
> > +		.storagebits = 16,				\
> > +		.endianness = IIO_LE				\  
> 
> Leave trailing comma here.
> 
> > +	},	                                                \  
> 
> ...
> 
> >  	.channel2 = IIO_MOD_##_axis,				\
> >  	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> > +	.scan_index = BMI270_SCAN_GYRO_##_axis,			\
> > +	.scan_type = {						\
> > +		.sign = 's',					\
> > +		.realbits = 16,					\
> > +		.storagebits = 16,				\
> > +		.endianness = IIO_LE				\  
> 
> Ditto.
> 
> > +	},	                                                \  
> 
Agreed. Tweaked whilst applying

Jonathan

Re: [PATCH v4 2/4] iio: imu: bmi270: Add scale and sampling frequency to BMI270 IMU

[Jonathan Cameron]

On Mon, 28 Oct 2024 11:32:55 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Sun, Oct 27, 2024 at 10:20:23AM -0700, Justin Weiss wrote:
> > Add read and write functions and create _available entries.  
> 
> ...
> 
> > +static int bmi270_set_scale(struct bmi270_data *data,
> > +			    int chan_type, int uscale)  
> 
> There is available space in the previous line, (And I would even join them
> despite being 83 characters long.)
> 
> ...
> 
> > +static int bmi270_get_scale(struct bmi270_data *bmi270_device,
> > +			    int chan_type, int *uscale)  
> 
> Ditto (for chan_type).
> 
> ...
> 
> > +static int bmi270_set_odr(struct bmi270_data *data, int chan_type,
> > +			  int odr, int uodr)  
> 
> Ditto.
> 
> ...
> 
> > +	for (i = 0; i < bmi270_odr_item.num; i++) {
> > +		if (bmi270_odr_item.tbl[i].odr != odr ||
> > +		    bmi270_odr_item.tbl[i].uodr != uodr)
> > +			continue;
> > +
> > +		return regmap_update_bits(data->regmap, reg, mask,
> > +					  bmi270_odr_item.vals[i]);
> > +	}
> > +
> > +	return -EINVAL;  
> 
> Wouldn't be better to use regular patterns, i.e. checking for errors first?

Hmm. This was my suggestion :(. For a simple case of match and do
something if true, this is a reasonably common pattern - particularly
in cases where there is a fallback option. I.e. you'd do something
after the loop only if there is no match.

Anyhow, given I suggested it I feel mean asking Justin to revert
to what he had in the first place.  I don't feel that strongly about it
though so if the two of you agree this is neater, send a follow up patch.

Tweaked the line wraps whilst applying.
> 
> 	for (i = 0; i < bmi270_odr_item.num; i++) {
> 		if (bmi270_odr_item.tbl[i].odr == odr ||
> 		    bmi270_odr_item.tbl[i].uodr == uodr)

That would be a bad idea && is fine though .

> 			break;
> 	}
> 	if (i == bmi270_odr_item.num)
> 		return -EINVAL;
> 
> 	return regmap_update_bits(data->regmap, reg, mask, bmi270_odr_item.vals[i]);
> 
> ...
> 
> > +static int bmi270_get_odr(struct bmi270_data *data, int chan_type,
> > +			  int *odr, int *uodr)  
> 
> As per above.
> 
> > +	for (i = 0; i < bmi270_odr_item.num; i++) {
> > +		if (val != bmi270_odr_item.vals[i])
> > +			continue;
> > +
> > +		*odr = bmi270_odr_item.tbl[i].odr;
> > +		*uodr = bmi270_odr_item.tbl[i].uodr;
> > +		return 0;
> > +	}
> > +
> > +	return -EINVAL;  
> 
> As per above.
> 

Re: [PATCH v4 0/4] Add i2c driver for Bosch BMI260 IMU

[Jonathan Cameron]

On Sun, 27 Oct 2024 10:20:21 -0700
Justin Weiss <justin@justinweiss.com> wrote:

> Add support for the Bosch BMI260 IMU to the BMI270 device driver.
> 
> The BMI270 and BMI260 have nearly identical register maps, but have
> different chip IDs and firmware.
> 
> The BMI260 is the IMU on a number of handheld PCs. Unfortunately,
> these devices often misidentify it in ACPI as a BMI160 ("BMI0160," for
> example), and it can only be correctly identified using the chip
> ID. To avoid conflicts with the bmi160 driver, this driver will not
> probe if it detects a BMI160 chip ID.
> 
> Also add triggered buffer and scale / sampling frequency attributes,
> which the input tools commonly used on handheld PCs require to support
> IMUs.
> 
> Like the BMI270, the BMI260 requires firmware to be provided.
> Signed-off-by: Justin Weiss <justin@justinweiss.com>

Applied with a few tweaks thanks to Andy's review.

I'll push this out as testing to let 0-day poke at it before it goes
into linux-next in a few days time.

Thanks,

Jonathan

> ---
> 
> Changelog:
> 
> V4
> - Move triggered buffer and attributes patches to the front of the set
> - Add more detailed commit message to DT documentation patch
> - Remove ACPI IDs from SPI driver
> - Remove 10EC5280 and BMI0260 ACPI IDs from I2C driver
> - Add DSDT excerpt for BMI0160 ACPI ID
> 
> V3
> https://lore.kernel.org/lkml/20241020220011.212395-1-justin@justinweiss.com/
> - Fix: Remove SCALE and FREQUENCY attributes
> - Use separate configuration structures instead of an array
> - Add bmi260 as compatible ID in bmi270 dt binding doc
> - Check chip ID against value in configuration instead of constant
> - Update comment for DMA alignment
> - Remove unreachable return statement
> 
> V2
> https://lore.kernel.org/all/20241018233723.28757-1-justin@justinweiss.com/
> - Fix commit titles
> - Fix: Change FREQUENCY to SAMP_FREQ
> - Split chip_info refactor into a separate commit from adding bmi260
> - Only fail probe when BMI160 is detected
> - Update chip_info based on detected chip ID
> - Add BMI260 to DT documentation
> - Add BMI260 to of_device_id
> - Add expected BMI260 ACPI ID to the SPI driver
> - Remove unused/unexpected BMI260 ACPI IDs
> - Remove trailing comma for null terminators
> - Use DMA_MINALIGN for channel buffer
> - Read channels in bulk
> - Improve for loops for detecting scale / odr attrs
> - Add missing masks
> - Use FIELD_GET
> - Use read_avail instead of custom attrs
> - Misc. formatting and line wrapping improvements
> 
> V1
> https://lore.kernel.org/all/20241011153751.65152-1-justin@justinweiss.com/
> 
> Justin Weiss (4):
>   iio: imu: bmi270: Add triggered buffer for Bosch BMI270 IMU
>   iio: imu: bmi270: Add scale and sampling frequency to BMI270 IMU
>   dt-bindings: iio: imu: bmi270: Add Bosch BMI260
>   iio: imu: bmi270: Add support for BMI260
> 
>  .../bindings/iio/imu/bosch,bmi270.yaml        |   4 +-
>  drivers/iio/imu/bmi270/Kconfig                |   1 +
>  drivers/iio/imu/bmi270/bmi270.h               |  10 +
>  drivers/iio/imu/bmi270/bmi270_core.c          | 424 +++++++++++++++++-
>  drivers/iio/imu/bmi270/bmi270_i2c.c           |   9 +
>  drivers/iio/imu/bmi270/bmi270_spi.c           |   2 +
>  6 files changed, 448 insertions(+), 2 deletions(-)
> 
> 
> base-commit: 9090ececac9ff1e22fb7e042f3c886990a8fb090

Re: [PATCH v4 0/4] Add i2c driver for Bosch BMI260 IMU

[Justin Weiss]

Jonathan Cameron <jic23@kernel.org> writes:

> Applied with a few tweaks thanks to Andy's review.
>
> I'll push this out as testing to let 0-day poke at it before it goes
> into linux-next in a few days time.
>

Thank you, and thanks for all the help!
Justin

> Thanks,
>
> Jonathan