[PATCH v2 0/6] Add i2c driver for Bosch BMI260 IMU

[PATCH v2 0/6] 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:

V2
- 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 (6):
  iio: imu: bmi270: Use INFO_SAMP_FREQ instead of INFO_FREQUENCY
  iio: imu: bmi270: Provide chip info as configuration structure
  dt-bindings: iio: imu: Add Bosch BMI260
  iio: imu: bmi270: Add support for BMI260
  iio: imu: bmi270: Add triggered buffer for Bosch BMI270 IMU
  iio: imu: bmi270: Add scale and sampling frequency to BMI270 IMU

 .../bindings/iio/imu/bosch,bmi260.yaml        |  77 +++
 MAINTAINERS                                   |   1 +
 drivers/iio/imu/bmi270/Kconfig                |   1 +
 drivers/iio/imu/bmi270/bmi270.h               |  25 +-
 drivers/iio/imu/bmi270/bmi270_core.c          | 441 +++++++++++++++++-
 drivers/iio/imu/bmi270/bmi270_i2c.c           |  24 +-
 drivers/iio/imu/bmi270/bmi270_spi.c           |  19 +-
 7 files changed, 574 insertions(+), 14 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/imu/bosch,bmi260.yaml


base-commit: 96be67caa0f0420d4128cb67f07bbd7a6f49e03a
-- 
2.47.0

[PATCH v2 1/6] iio: imu: bmi270: Use INFO_SAMP_FREQ instead of INFO_FREQUENCY

[Justin Weiss]

Use IIO_CHAN_INFO_SAMP_FREQ instead of IIO_CHAN_INFO_FREQUENCY
to match the BMI160 / BMI323 drivers.

Fixes: 3ea51548d6b2 ("iio: imu: Add i2c driver for bmi270 imu")
Signed-off-by: Justin Weiss <justin@justinweiss.com>
---
 drivers/iio/imu/bmi270/bmi270_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/imu/bmi270/bmi270_core.c b/drivers/iio/imu/bmi270/bmi270_core.c
index aeda7c4228df..87036f352698 100644
--- a/drivers/iio/imu/bmi270/bmi270_core.c
+++ b/drivers/iio/imu/bmi270/bmi270_core.c
@@ -122,7 +122,7 @@ static const struct iio_info bmi270_info = {
 	.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_FREQUENCY),			\
+		BIT(IIO_CHAN_INFO_SAMP_FREQ),			\
 }
 
 #define BMI270_ANG_VEL_CHANNEL(_axis) {				\
@@ -131,7 +131,7 @@ static const struct iio_info bmi270_info = {
 	.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_FREQUENCY),			\
+		BIT(IIO_CHAN_INFO_SAMP_FREQ),			\
 }
 
 static const struct iio_chan_spec bmi270_channels[] = {
-- 
2.47.0

[PATCH v2 2/6] iio: imu: bmi270: Provide chip info as configuration structure

[Justin Weiss]

Prepare the bmi270 driver to support similar devices like the bmi260.

Signed-off-by: Justin Weiss <justin@justinweiss.com>
---
 drivers/iio/imu/bmi270/bmi270.h      | 15 ++++++++++++++-
 drivers/iio/imu/bmi270/bmi270_core.c | 18 +++++++++++++++---
 drivers/iio/imu/bmi270/bmi270_i2c.c  | 11 ++++++++---
 drivers/iio/imu/bmi270/bmi270_spi.c  | 11 ++++++++---
 4 files changed, 45 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/imu/bmi270/bmi270.h b/drivers/iio/imu/bmi270/bmi270.h
index 8ac20ad7ee94..2e8d85a4e419 100644
--- a/drivers/iio/imu/bmi270/bmi270.h
+++ b/drivers/iio/imu/bmi270/bmi270.h
@@ -10,10 +10,23 @@ struct device;
 struct bmi270_data {
 	struct device *dev;
 	struct regmap *regmap;
+	const struct bmi270_chip_info *chip_info;
+};
+
+enum bmi270_device_type {
+	BMI270,
+};
+
+struct bmi270_chip_info {
+	const char *name;
+	int chip_id;
+	const char *fw_name;
 };
 
 extern const struct regmap_config bmi270_regmap_config;
+extern const struct bmi270_chip_info bmi270_chip_info[];
 
-int bmi270_core_probe(struct device *dev, struct regmap *regmap);
+int bmi270_core_probe(struct device *dev, struct regmap *regmap,
+		      const struct bmi270_chip_info *chip_info);
 
 #endif  /* BMI270_H_ */
diff --git a/drivers/iio/imu/bmi270/bmi270_core.c b/drivers/iio/imu/bmi270/bmi270_core.c
index 87036f352698..799df78ec862 100644
--- a/drivers/iio/imu/bmi270/bmi270_core.c
+++ b/drivers/iio/imu/bmi270/bmi270_core.c
@@ -66,6 +66,15 @@ enum bmi270_scan {
 	BMI270_SCAN_GYRO_Z,
 };
 
+const struct bmi270_chip_info bmi270_chip_info[] = {
+	[BMI270] = {
+		.name = "bmi270",
+		.chip_id = BMI270_CHIP_ID_VAL,
+		.fw_name = BMI270_INIT_DATA_FILE,
+	}
+};
+EXPORT_SYMBOL_NS_GPL(bmi270_chip_info, IIO_BMI270);
+
 static int bmi270_get_data(struct bmi270_data *bmi270_device,
 			   int chan_type, int axis, int *val)
 {
@@ -187,7 +196,8 @@ 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_INIT_DATA_FILE, dev);
+	ret = request_firmware(&init_data,
+			       bmi270_device->chip_info->fw_name, dev);
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to load init data file");
 
@@ -274,7 +284,8 @@ static int bmi270_chip_init(struct bmi270_data *bmi270_device)
 	return bmi270_configure_imu(bmi270_device);
 }
 
-int bmi270_core_probe(struct device *dev, struct regmap *regmap)
+int bmi270_core_probe(struct device *dev, struct regmap *regmap,
+		      const struct bmi270_chip_info *chip_info)
 {
 	int ret;
 	struct bmi270_data *bmi270_device;
@@ -287,6 +298,7 @@ int bmi270_core_probe(struct device *dev, struct regmap *regmap)
 	bmi270_device = iio_priv(indio_dev);
 	bmi270_device->dev = dev;
 	bmi270_device->regmap = regmap;
+	bmi270_device->chip_info = chip_info;
 
 	ret = bmi270_chip_init(bmi270_device);
 	if (ret)
@@ -294,7 +306,7 @@ 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 = "bmi270";
+	indio_dev->name = chip_info->name;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->info = &bmi270_info;
 
diff --git a/drivers/iio/imu/bmi270/bmi270_i2c.c b/drivers/iio/imu/bmi270/bmi270_i2c.c
index e9025d22d5cc..742149701849 100644
--- a/drivers/iio/imu/bmi270/bmi270_i2c.c
+++ b/drivers/iio/imu/bmi270/bmi270_i2c.c
@@ -18,22 +18,27 @@ static int bmi270_i2c_probe(struct i2c_client *client)
 {
 	struct regmap *regmap;
 	struct device *dev = &client->dev;
+	const struct bmi270_chip_info *chip_info;
+
+	chip_info = i2c_get_match_data(client);
+	if (!chip_info)
+		return -ENODEV;
 
 	regmap = devm_regmap_init_i2c(client, &bmi270_i2c_regmap_config);
 	if (IS_ERR(regmap))
 		return dev_err_probe(dev, PTR_ERR(regmap),
 				     "Failed to init i2c regmap");
 
-	return bmi270_core_probe(dev, regmap);
+	return bmi270_core_probe(dev, regmap, chip_info);
 }
 
 static const struct i2c_device_id bmi270_i2c_id[] = {
-	{ "bmi270", 0 },
+	{ "bmi270", (kernel_ulong_t)&bmi270_chip_info[BMI270] },
 	{ }
 };
 
 static const struct of_device_id bmi270_of_match[] = {
-	{ .compatible = "bosch,bmi270" },
+	{ .compatible = "bosch,bmi270", .data = &bmi270_chip_info[BMI270] },
 	{ }
 };
 
diff --git a/drivers/iio/imu/bmi270/bmi270_spi.c b/drivers/iio/imu/bmi270/bmi270_spi.c
index 34d5ba6273bb..3d240f9651bc 100644
--- a/drivers/iio/imu/bmi270/bmi270_spi.c
+++ b/drivers/iio/imu/bmi270/bmi270_spi.c
@@ -50,6 +50,11 @@ static int bmi270_spi_probe(struct spi_device *spi)
 {
 	struct regmap *regmap;
 	struct device *dev = &spi->dev;
+	const struct bmi270_chip_info *chip_info;
+
+	chip_info = spi_get_device_match_data(spi);
+	if (!chip_info)
+		return -ENODEV;
 
 	regmap = devm_regmap_init(dev, &bmi270_regmap_bus, dev,
 				  &bmi270_spi_regmap_config);
@@ -57,16 +62,16 @@ static int bmi270_spi_probe(struct spi_device *spi)
 		return dev_err_probe(dev, PTR_ERR(regmap),
 				     "Failed to init i2c regmap");
 
-	return bmi270_core_probe(dev, regmap);
+	return bmi270_core_probe(dev, regmap, chip_info);
 }
 
 static const struct spi_device_id bmi270_spi_id[] = {
-	{ "bmi270" },
+	{ "bmi270", (kernel_ulong_t)&bmi270_chip_info[BMI270] },
 	{ }
 };
 
 static const struct of_device_id bmi270_of_match[] = {
-	{ .compatible = "bosch,bmi270" },
+	{ .compatible = "bosch,bmi270", .data = &bmi270_chip_info[BMI270] },
 	{ }
 };
 
-- 
2.47.0

[PATCH v2 3/6] dt-bindings: iio: imu: Add Bosch BMI260

[Justin Weiss]

Add devicetree description document for Bosch BMI260, a 6-Axis IMU.

Signed-off-by: Justin Weiss <justin@justinweiss.com>
---
 .../bindings/iio/imu/bosch,bmi260.yaml        | 77 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 78 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/imu/bosch,bmi260.yaml

diff --git a/Documentation/devicetree/bindings/iio/imu/bosch,bmi260.yaml b/Documentation/devicetree/bindings/iio/imu/bosch,bmi260.yaml
new file mode 100644
index 000000000000..6786b5e4d0fa
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/imu/bosch,bmi260.yaml
@@ -0,0 +1,77 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/imu/bosch,bmi260.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Bosch BMI260 6-Axis IMU
+
+maintainers:
+  - Justin Weiss <justin@justinweiss.com>
+
+description: |
+  BMI260 is a 6-axis inertial measurement unit that can measure acceleration and
+  angular velocity. The sensor also supports configurable interrupt events such
+  as motion detection and step counting. The sensor can communicate over
+  I2C or SPI.
+  https://www.bosch-sensortec.com/products/motion-sensors/imus/bmi260/
+
+properties:
+  compatible:
+    const: bosch,bmi260
+
+  reg:
+    maxItems: 1
+
+  vdd-supply: true
+  vddio-supply: true
+
+  interrupts:
+    minItems: 1
+    maxItems: 2
+
+  interrupt-names:
+    minItems: 1
+    maxItems: 2
+    items:
+      enum:
+        - INT1
+        - INT2
+
+  drive-open-drain:
+    description:
+      set if the specified interrupt pins should be configured as
+      open drain. If not set, defaults to push-pull.
+
+  mount-matrix:
+    description:
+      an optional 3x3 mounting rotation matrix.
+
+required:
+  - compatible
+  - reg
+  - vdd-supply
+  - vddio-supply
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        imu@68 {
+            compatible = "bosch,bmi260";
+            reg = <0x68>;
+            vdd-supply = <&vdd>;
+            vddio-supply = <&vddio>;
+            interrupt-parent = <&gpio1>;
+            interrupts = <16 IRQ_TYPE_EDGE_RISING>;
+            interrupt-names = "INT1";
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 6011af70c12e..73b6b7721dd8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4047,6 +4047,7 @@ BOSCH SENSORTEC BMI270 IMU IIO DRIVER
 M:	Alex Lanzano <lanzano.alex@gmail.com>
 L:	linux-iio@vger.kernel.org
 S:	Maintained
+F:	Documentation/devicetree/bindings/iio/imu/bosch,bmi260.yaml
 F:	Documentation/devicetree/bindings/iio/imu/bosch,bmi270.yaml
 F:	drivers/iio/imu/bmi270/
 
-- 
2.47.0

[PATCH v2 4/6] 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.

Signed-off-by: Justin Weiss <justin@justinweiss.com>
---
 drivers/iio/imu/bmi270/bmi270.h      |  1 +
 drivers/iio/imu/bmi270/bmi270_core.c | 25 +++++++++++++++++++++++--
 drivers/iio/imu/bmi270/bmi270_i2c.c  | 13 +++++++++++++
 drivers/iio/imu/bmi270/bmi270_spi.c  |  8 ++++++++
 4 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/imu/bmi270/bmi270.h b/drivers/iio/imu/bmi270/bmi270.h
index 2e8d85a4e419..51e374fd4290 100644
--- a/drivers/iio/imu/bmi270/bmi270.h
+++ b/drivers/iio/imu/bmi270/bmi270.h
@@ -14,6 +14,7 @@ struct bmi270_data {
 };
 
 enum bmi270_device_type {
+	BMI260,
 	BMI270,
 };
 
diff --git a/drivers/iio/imu/bmi270/bmi270_core.c b/drivers/iio/imu/bmi270/bmi270_core.c
index 799df78ec862..b30201dc4e22 100644
--- a/drivers/iio/imu/bmi270/bmi270_core.c
+++ b/drivers/iio/imu/bmi270/bmi270_core.c
@@ -11,6 +11,8 @@
 #include "bmi270.h"
 
 #define BMI270_CHIP_ID_REG				0x00
+#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)
 
@@ -55,6 +57,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 {
@@ -67,6 +70,11 @@ enum bmi270_scan {
 };
 
 const struct bmi270_chip_info bmi270_chip_info[] = {
+	[BMI260] = {
+		.name = "bmi260",
+		.chip_id = BMI260_CHIP_ID_VAL,
+		.fw_name = BMI260_INIT_DATA_FILE,
+	},
 	[BMI270] = {
 		.name = "bmi270",
 		.chip_id = BMI270_CHIP_ID_VAL,
@@ -163,8 +171,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");
 
-	if (chip_id != BMI270_CHIP_ID_VAL)
-		dev_info(dev, "Unknown chip id 0x%x", 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, "Unexpected chip id 0x%x", chip_id);
+
+	if (chip_id == BMI260_CHIP_ID_VAL)
+		bmi270_device->chip_info = &bmi270_chip_info[BMI260];
+	else if (chip_id == BMI270_CHIP_ID_VAL)
+		bmi270_device->chip_info = &bmi270_chip_info[BMI270];
 
 	return 0;
 }
diff --git a/drivers/iio/imu/bmi270/bmi270_i2c.c b/drivers/iio/imu/bmi270/bmi270_i2c.c
index 742149701849..d6417b7c799c 100644
--- a/drivers/iio/imu/bmi270/bmi270_i2c.c
+++ b/drivers/iio/imu/bmi270/bmi270_i2c.c
@@ -33,11 +33,23 @@ static int bmi270_i2c_probe(struct i2c_client *client)
 }
 
 static const struct i2c_device_id bmi270_i2c_id[] = {
+	{ "bmi260", (kernel_ulong_t)&bmi270_chip_info[BMI260] },
 	{ "bmi270", (kernel_ulong_t)&bmi270_chip_info[BMI270] },
 	{ }
 };
 
+static const struct acpi_device_id bmi270_acpi_match[] = {
+	/* OrangePi NEO */
+	{ "BMI0260",  (kernel_ulong_t)&bmi270_chip_info[BMI260] },
+	/* GPD Win Mini, Aya Neo AIR Pro, OXP Mini Pro, etc. */
+	{ "BMI0160",  (kernel_ulong_t)&bmi270_chip_info[BMI260] },
+	/* GPD Win Max 2 */
+	{ "10EC5280", (kernel_ulong_t)&bmi270_chip_info[BMI260] },
+	{ }
+};
+
 static const struct of_device_id bmi270_of_match[] = {
+	{ .compatible = "bosch,bmi260", .data = &bmi270_chip_info[BMI260] },
 	{ .compatible = "bosch,bmi270", .data = &bmi270_chip_info[BMI270] },
 	{ }
 };
@@ -45,6 +57,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 3d240f9651bc..88173e9a3529 100644
--- a/drivers/iio/imu/bmi270/bmi270_spi.c
+++ b/drivers/iio/imu/bmi270/bmi270_spi.c
@@ -66,11 +66,18 @@ static int bmi270_spi_probe(struct spi_device *spi)
 }
 
 static const struct spi_device_id bmi270_spi_id[] = {
+	{ "bmi260", (kernel_ulong_t)&bmi270_chip_info[BMI260] },
 	{ "bmi270", (kernel_ulong_t)&bmi270_chip_info[BMI270] },
 	{ }
 };
 
+static const struct acpi_device_id bmi270_acpi_match[] = {
+	{ "BOSC0260",  (kernel_ulong_t)&bmi270_chip_info[BMI260] },
+	{ }
+};
+
 static const struct of_device_id bmi270_of_match[] = {
+	{ .compatible = "bosch,bmi260", .data = &bmi270_chip_info[BMI260] },
 	{ .compatible = "bosch,bmi270", .data = &bmi270_chip_info[BMI270] },
 	{ }
 };
@@ -78,6 +85,7 @@ static const struct of_device_id bmi270_of_match[] = {
 static struct spi_driver bmi270_spi_driver = {
 	.driver = {
 		.name = "bmi270",
+		.acpi_match_table = bmi270_acpi_match,
 		.of_match_table = bmi270_of_match,
 	},
 	.probe = bmi270_spi_probe,
-- 
2.47.0

[PATCH v2 5/6] 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 51e374fd4290..844d70a7d32e 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 is 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_device_type {
diff --git a/drivers/iio/imu/bmi270/bmi270_core.c b/drivers/iio/imu/bmi270/bmi270_core.c
index b30201dc4e22..69b011f97324 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"
 
@@ -67,6 +69,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[] = {
@@ -83,6 +96,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)
 {
@@ -140,6 +174,13 @@ static const struct iio_info bmi270_info = {
 	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
 	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
 		BIT(IIO_CHAN_INFO_SAMP_FREQ),			\
+	.scan_index = BMI270_SCAN_ACCEL_##_axis,		\
+	.scan_type = {						\
+		.sign = 's',					\
+		.realbits = 16,					\
+		.storagebits = 16,				\
+		.endianness = IIO_LE				\
+	},	                                                \
 }
 
 #define BMI270_ANG_VEL_CHANNEL(_axis) {				\
@@ -149,6 +190,13 @@ static const struct iio_info bmi270_info = {
 	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
 	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
 		BIT(IIO_CHAN_INFO_SAMP_FREQ),			\
+	.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[] = {
@@ -158,6 +206,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)
@@ -328,9 +377,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 v2 6/6] 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 | 338 +++++++++++++++++++++++++++
 1 file changed, 338 insertions(+)

diff --git a/drivers/iio/imu/bmi270/bmi270_core.c b/drivers/iio/imu/bmi270/bmi270_core.c
index 69b011f97324..bc3604e66f85 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>
 
@@ -35,6 +36,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
@@ -43,6 +47,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)
 
@@ -96,6 +103,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;
@@ -158,6 +424,70 @@ 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;
+	}
+
+	return 0;
+}
+
+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;
 	}
@@ -165,6 +495,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) {				\
@@ -174,6 +506,9 @@ static const struct iio_info bmi270_info = {
 	.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',					\
@@ -190,6 +525,9 @@ static const struct iio_info bmi270_info = {
 	.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

Re: [PATCH v2 1/6] iio: imu: bmi270: Use INFO_SAMP_FREQ instead of INFO_FREQUENCY

[Jonathan Cameron]

On Fri, 18 Oct 2024 16:36:07 -0700
Justin Weiss <justin@justinweiss.com> wrote:

> Use IIO_CHAN_INFO_SAMP_FREQ instead of IIO_CHAN_INFO_FREQUENCY
> to match the BMI160 / BMI323 drivers.
> 
> Fixes: 3ea51548d6b2 ("iio: imu: Add i2c driver for bmi270 imu")
> Signed-off-by: Justin Weiss <justin@justinweiss.com>

Whilst this gets rid of the wrong attributes, they still aren't
wired up to anything either way so a read will always return an error.

For now, the fix is drop the bit and bring it back in a patch
that adds the read_raw handling for the sampling frequency
(patch 6 I think).

> ---
>  drivers/iio/imu/bmi270/bmi270_core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/imu/bmi270/bmi270_core.c b/drivers/iio/imu/bmi270/bmi270_core.c
> index aeda7c4228df..87036f352698 100644
> --- a/drivers/iio/imu/bmi270/bmi270_core.c
> +++ b/drivers/iio/imu/bmi270/bmi270_core.c
> @@ -122,7 +122,7 @@ static const struct iio_info bmi270_info = {
>  	.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_FREQUENCY),			\
> +		BIT(IIO_CHAN_INFO_SAMP_FREQ),			\
>  }
>  
>  #define BMI270_ANG_VEL_CHANNEL(_axis) {				\
> @@ -131,7 +131,7 @@ static const struct iio_info bmi270_info = {
>  	.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_FREQUENCY),			\
> +		BIT(IIO_CHAN_INFO_SAMP_FREQ),			\
>  }
>  
>  static const struct iio_chan_spec bmi270_channels[] = {

Re: [PATCH v2 2/6] iio: imu: bmi270: Provide chip info as configuration structure

[Jonathan Cameron]

On Fri, 18 Oct 2024 16:36:08 -0700
Justin Weiss <justin@justinweiss.com> wrote:

> Prepare the bmi270 driver to support similar devices like the bmi260.
> 
> Signed-off-by: Justin Weiss <justin@justinweiss.com>
One thing in here.  The enum ID thing tends to end up costing more than
the benefit it brings, so for newer drivers preferred option is separate
structure instances rather than an array.

> ---
>  drivers/iio/imu/bmi270/bmi270.h      | 15 ++++++++++++++-
>  drivers/iio/imu/bmi270/bmi270_core.c | 18 +++++++++++++++---
>  drivers/iio/imu/bmi270/bmi270_i2c.c  | 11 ++++++++---
>  drivers/iio/imu/bmi270/bmi270_spi.c  | 11 ++++++++---
>  4 files changed, 45 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/iio/imu/bmi270/bmi270.h b/drivers/iio/imu/bmi270/bmi270.h
> index 8ac20ad7ee94..2e8d85a4e419 100644
> --- a/drivers/iio/imu/bmi270/bmi270.h
> +++ b/drivers/iio/imu/bmi270/bmi270.h
> @@ -10,10 +10,23 @@ struct device;
>  struct bmi270_data {
>  	struct device *dev;
>  	struct regmap *regmap;
> +	const struct bmi270_chip_info *chip_info;
> +};
> +
> +enum bmi270_device_type {
> +	BMI270,

Whilst quite a few drivers do it this way, over time we've found that it's
much easier to just skip the array of structures and have independent ones.
Increase the extern lines to one per supported device, but removes
need for an enum here and generally gives slightly more readable code.


> +};

>  };
>  
>  static const struct of_device_id bmi270_of_match[] = {
> -	{ .compatible = "bosch,bmi270" },
> +	{ .compatible = "bosch,bmi270", .data = &bmi270_chip_info[BMI270] },

After dropping the enum this just becomes &bmi270_chip_info
and later you'll add bmi260_chip_info etc.

>  	{ }
>  };
>  

Re: [PATCH v2 3/6] dt-bindings: iio: imu: Add Bosch BMI260

[Jonathan Cameron]

On Fri, 18 Oct 2024 16:36:09 -0700
Justin Weiss <justin@justinweiss.com> wrote:

> Add devicetree description document for Bosch BMI260, a 6-Axis IMU.
> 
> Signed-off-by: Justin Weiss <justin@justinweiss.com>
Looks like this would be much better as an additional compatible id
in the existing bosch,bmi270.yaml binding doc.

From a quick comparison they look nearly identical.
Even if there are small differences the dt binding schema allows
those to be expressed in a single file.

Jonathan

> ---
>  .../bindings/iio/imu/bosch,bmi260.yaml        | 77 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 78 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/imu/bosch,bmi260.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/imu/bosch,bmi260.yaml b/Documentation/devicetree/bindings/iio/imu/bosch,bmi260.yaml
> new file mode 100644
> index 000000000000..6786b5e4d0fa
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/imu/bosch,bmi260.yaml
> @@ -0,0 +1,77 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/imu/bosch,bmi260.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Bosch BMI260 6-Axis IMU
> +
> +maintainers:
> +  - Justin Weiss <justin@justinweiss.com>
> +
> +description: |
> +  BMI260 is a 6-axis inertial measurement unit that can measure acceleration and
> +  angular velocity. The sensor also supports configurable interrupt events such
> +  as motion detection and step counting. The sensor can communicate over
> +  I2C or SPI.
> +  https://www.bosch-sensortec.com/products/motion-sensors/imus/bmi260/
> +
> +properties:
> +  compatible:
> +    const: bosch,bmi260
> +
> +  reg:
> +    maxItems: 1
> +
> +  vdd-supply: true
> +  vddio-supply: true
> +
> +  interrupts:
> +    minItems: 1
> +    maxItems: 2
> +
> +  interrupt-names:
> +    minItems: 1
> +    maxItems: 2
> +    items:
> +      enum:
> +        - INT1
> +        - INT2
> +
> +  drive-open-drain:
> +    description:
> +      set if the specified interrupt pins should be configured as
> +      open drain. If not set, defaults to push-pull.
> +
> +  mount-matrix:
> +    description:
> +      an optional 3x3 mounting rotation matrix.
> +
> +required:
> +  - compatible
> +  - reg
> +  - vdd-supply
> +  - vddio-supply
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        imu@68 {
> +            compatible = "bosch,bmi260";
> +            reg = <0x68>;
> +            vdd-supply = <&vdd>;
> +            vddio-supply = <&vddio>;
> +            interrupt-parent = <&gpio1>;
> +            interrupts = <16 IRQ_TYPE_EDGE_RISING>;
> +            interrupt-names = "INT1";
> +        };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 6011af70c12e..73b6b7721dd8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4047,6 +4047,7 @@ BOSCH SENSORTEC BMI270 IMU IIO DRIVER
>  M:	Alex Lanzano <lanzano.alex@gmail.com>
>  L:	linux-iio@vger.kernel.org
>  S:	Maintained
> +F:	Documentation/devicetree/bindings/iio/imu/bosch,bmi260.yaml
>  F:	Documentation/devicetree/bindings/iio/imu/bosch,bmi270.yaml
>  F:	drivers/iio/imu/bmi270/
>  

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

[Jonathan Cameron]

On Fri, 18 Oct 2024 16:36:10 -0700
Justin Weiss <justin@justinweiss.com> 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.
> 
> Signed-off-by: Justin Weiss <justin@justinweiss.com>
Trivial comments inline and a discussion on whether my earlier
don't use an array comment makes sense in this particular case.

Jonathan

> ---
>  drivers/iio/imu/bmi270/bmi270.h      |  1 +
>  drivers/iio/imu/bmi270/bmi270_core.c | 25 +++++++++++++++++++++++--
>  drivers/iio/imu/bmi270/bmi270_i2c.c  | 13 +++++++++++++
>  drivers/iio/imu/bmi270/bmi270_spi.c  |  8 ++++++++
>  4 files changed, 45 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/imu/bmi270/bmi270.h b/drivers/iio/imu/bmi270/bmi270.h
> index 2e8d85a4e419..51e374fd4290 100644
> --- a/drivers/iio/imu/bmi270/bmi270.h
> +++ b/drivers/iio/imu/bmi270/bmi270.h
> @@ -14,6 +14,7 @@ struct bmi270_data {
>  };
>  
>  enum bmi270_device_type {
> +	BMI260,
>  	BMI270,
>  };
>  
> diff --git a/drivers/iio/imu/bmi270/bmi270_core.c b/drivers/iio/imu/bmi270/bmi270_core.c
> index 799df78ec862..b30201dc4e22 100644
> --- a/drivers/iio/imu/bmi270/bmi270_core.c
> +++ b/drivers/iio/imu/bmi270/bmi270_core.c
> @@ -11,6 +11,8 @@
>  #include "bmi270.h"
>  
>  #define BMI270_CHIP_ID_REG				0x00
> +#define BMI160_CHIP_ID_VAL				0xD1

This one looks like a cut and paste error.

> +#define BMI260_CHIP_ID_VAL				0x27
>  #define BMI270_CHIP_ID_VAL				0x24
>  #define BMI270_CHIP_ID_MSK				GENMASK(7, 0)
>  
> @@ -55,6 +57,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 {
> @@ -67,6 +70,11 @@ enum bmi270_scan {
>  };
>  
>  const struct bmi270_chip_info bmi270_chip_info[] = {
> +	[BMI260] = {
> +		.name = "bmi260",
> +		.chip_id = BMI260_CHIP_ID_VAL,
> +		.fw_name = BMI260_INIT_DATA_FILE,
> +	},
>  	[BMI270] = {
>  		.name = "bmi270",
>  		.chip_id = BMI270_CHIP_ID_VAL,
> @@ -163,8 +171,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");
>  
> -	if (chip_id != BMI270_CHIP_ID_VAL)
> -		dev_info(dev, "Unknown chip id 0x%x", 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, "Unexpected chip id 0x%x", chip_id);
> +
> +	if (chip_id == BMI260_CHIP_ID_VAL)

Ah. My argument on separate IDs means you'd have to do it this way whereas
I was thinking maybe a loop would be a better idea.  Ah well if we
get a lot of supported chips, then we can rethink how to handle this.
For now what you have here is fine and should deal with lack of appropriate
ACPI ID mess.

> +		bmi270_device->chip_info = &bmi270_chip_info[BMI260];
> +	else if (chip_id == BMI270_CHIP_ID_VAL)
> +		bmi270_device->chip_info = &bmi270_chip_info[BMI270];
>  
>  	return 0;
>  }

Re: [PATCH v2 5/6] iio: imu: bmi270: Add triggered buffer for Bosch BMI270 IMU

[Jonathan Cameron]

On Fri, 18 Oct 2024 16:36:11 -0700
Justin Weiss <justin@justinweiss.com> wrote:

> Set up a triggered buffer for the accel and angl_vel values.
> 
> Signed-off-by: Justin Weiss <justin@justinweiss.com>
Looks good. One trivial comment inline.

Thanks,

Jonathan

> ---
>  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 51e374fd4290..844d70a7d32e 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 is larger than 8 bytes, align to that

maybe larger than 8 bytes
on x86_64 I think it is 8 bytes exactly, though I could be remembering that wrong.

> +	 * to ensure a DMA safe buffer.
> +	 */
> +	struct {
> +		__le16 channels[6];
> +		aligned_s64 timestamp;
> +	} data __aligned(IIO_DMA_MINALIGN);
>  };
>  

Re: [PATCH v2 6/6] iio: imu: bmi270: Add scale and sampling frequency to BMI270 IMU

[Jonathan Cameron]

On Fri, 18 Oct 2024 16:36:12 -0700
Justin Weiss <justin@justinweiss.com> wrote:

> Add read and write functions and create _available entries.
> 
> Signed-off-by: Justin Weiss <justin@justinweiss.com>
Hi Justin,

Just one trivial comment from me.

Jonathan



> +
> +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;
> +	}
> +
Unreachable code, so drop this return.
> +	return 0;
> +}
>

Re: [PATCH v2 1/6] iio: imu: bmi270: Use INFO_SAMP_FREQ instead of INFO_FREQUENCY

[Justin Weiss]

Jonathan Cameron <jic23@kernel.org> writes:

> On Fri, 18 Oct 2024 16:36:07 -0700
> Justin Weiss <justin@justinweiss.com> wrote:
>
>> Use IIO_CHAN_INFO_SAMP_FREQ instead of IIO_CHAN_INFO_FREQUENCY
>> to match the BMI160 / BMI323 drivers.
>> 
>> Fixes: 3ea51548d6b2 ("iio: imu: Add i2c driver for bmi270 imu")
>> Signed-off-by: Justin Weiss <justin@justinweiss.com>
>
> Whilst this gets rid of the wrong attributes, they still aren't
> wired up to anything either way so a read will always return an error.
>
> For now, the fix is drop the bit and bring it back in a patch
> that adds the read_raw handling for the sampling frequency
> (patch 6 I think).

Makes sense. I'll remove both SCALE and FREQUENCY here, because neither
of them are wired up right now.

Justin

>> ---
>>  drivers/iio/imu/bmi270/bmi270_core.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/iio/imu/bmi270/bmi270_core.c b/drivers/iio/imu/bmi270/bmi270_core.c
>> index aeda7c4228df..87036f352698 100644
>> --- a/drivers/iio/imu/bmi270/bmi270_core.c
>> +++ b/drivers/iio/imu/bmi270/bmi270_core.c
>> @@ -122,7 +122,7 @@ static const struct iio_info bmi270_info = {
>>  	.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_FREQUENCY),			\
>> +		BIT(IIO_CHAN_INFO_SAMP_FREQ),			\
>>  }
>>  
>>  #define BMI270_ANG_VEL_CHANNEL(_axis) {				\
>> @@ -131,7 +131,7 @@ static const struct iio_info bmi270_info = {
>>  	.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_FREQUENCY),			\
>> +		BIT(IIO_CHAN_INFO_SAMP_FREQ),			\
>>  }
>>  
>>  static const struct iio_chan_spec bmi270_channels[] = {

Re: [PATCH v2 2/6] iio: imu: bmi270: Provide chip info as configuration structure

[Justin Weiss]

Jonathan Cameron <jic23@kernel.org> writes:

> On Fri, 18 Oct 2024 16:36:08 -0700
> Justin Weiss <justin@justinweiss.com> wrote:
>
>> Prepare the bmi270 driver to support similar devices like the bmi260.
>> 
>> Signed-off-by: Justin Weiss <justin@justinweiss.com>
> One thing in here.  The enum ID thing tends to end up costing more than
> the benefit it brings, so for newer drivers preferred option is separate
> structure instances rather than an array.

That makes sense to me, even considering your comments on patch #4. I'll
switch to separate structures here and keep the if / else in that later
patch.

Justin

>> ---
>>  drivers/iio/imu/bmi270/bmi270.h      | 15 ++++++++++++++-
>>  drivers/iio/imu/bmi270/bmi270_core.c | 18 +++++++++++++++---
>>  drivers/iio/imu/bmi270/bmi270_i2c.c  | 11 ++++++++---
>>  drivers/iio/imu/bmi270/bmi270_spi.c  | 11 ++++++++---
>>  4 files changed, 45 insertions(+), 10 deletions(-)
>> 
>> diff --git a/drivers/iio/imu/bmi270/bmi270.h b/drivers/iio/imu/bmi270/bmi270.h
>> index 8ac20ad7ee94..2e8d85a4e419 100644
>> --- a/drivers/iio/imu/bmi270/bmi270.h
>> +++ b/drivers/iio/imu/bmi270/bmi270.h
>> @@ -10,10 +10,23 @@ struct device;
>>  struct bmi270_data {
>>  	struct device *dev;
>>  	struct regmap *regmap;
>> +	const struct bmi270_chip_info *chip_info;
>> +};
>> +
>> +enum bmi270_device_type {
>> +	BMI270,
>
> Whilst quite a few drivers do it this way, over time we've found that it's
> much easier to just skip the array of structures and have independent ones.
> Increase the extern lines to one per supported device, but removes
> need for an enum here and generally gives slightly more readable code.
>
>
>> +};
>
>>  };
>>  
>>  static const struct of_device_id bmi270_of_match[] = {
>> -	{ .compatible = "bosch,bmi270" },
>> +	{ .compatible = "bosch,bmi270", .data = &bmi270_chip_info[BMI270] },
>
> After dropping the enum this just becomes &bmi270_chip_info
> and later you'll add bmi260_chip_info etc.
>
>>  	{ }
>>  };
>>  

Re: [PATCH v2 3/6] dt-bindings: iio: imu: Add Bosch BMI260

[Justin Weiss]

Jonathan Cameron <jic23@kernel.org> writes:

> On Fri, 18 Oct 2024 16:36:09 -0700
> Justin Weiss <justin@justinweiss.com> wrote:
>
>> Add devicetree description document for Bosch BMI260, a 6-Axis IMU.
>> 
>> Signed-off-by: Justin Weiss <justin@justinweiss.com>
> Looks like this would be much better as an additional compatible id
> in the existing bosch,bmi270.yaml binding doc.
>
> From a quick comparison they look nearly identical.
> Even if there are small differences the dt binding schema allows
> those to be expressed in a single file.
>
> Jonathan

Sounds good, will update in v3.

Justin

>> ---
>>  .../bindings/iio/imu/bosch,bmi260.yaml        | 77 +++++++++++++++++++
>>  MAINTAINERS                                   |  1 +
>>  2 files changed, 78 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/iio/imu/bosch,bmi260.yaml
>> 
>> diff --git
>> a/Documentation/devicetree/bindings/iio/imu/bosch,bmi260.yaml
>> b/Documentation/devicetree/bindings/iio/imu/bosch,bmi260.yaml
>> new file mode 100644
>> index 000000000000..6786b5e4d0fa
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/imu/bosch,bmi260.yaml
>> @@ -0,0 +1,77 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/iio/imu/bosch,bmi260.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Bosch BMI260 6-Axis IMU
>> +
>> +maintainers:
>> +  - Justin Weiss <justin@justinweiss.com>
>> +
>> +description: |
>> +  BMI260 is a 6-axis inertial measurement unit that can measure acceleration and
>> +  angular velocity. The sensor also supports configurable interrupt events such
>> +  as motion detection and step counting. The sensor can communicate over
>> +  I2C or SPI.
>> +  https://www.bosch-sensortec.com/products/motion-sensors/imus/bmi260/
>> +
>> +properties:
>> +  compatible:
>> +    const: bosch,bmi260
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  vdd-supply: true
>> +  vddio-supply: true
>> +
>> +  interrupts:
>> +    minItems: 1
>> +    maxItems: 2
>> +
>> +  interrupt-names:
>> +    minItems: 1
>> +    maxItems: 2
>> +    items:
>> +      enum:
>> +        - INT1
>> +        - INT2
>> +
>> +  drive-open-drain:
>> +    description:
>> +      set if the specified interrupt pins should be configured as
>> +      open drain. If not set, defaults to push-pull.
>> +
>> +  mount-matrix:
>> +    description:
>> +      an optional 3x3 mounting rotation matrix.
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - vdd-supply
>> +  - vddio-supply
>> +
>> +allOf:
>> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +    i2c {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +
>> +        imu@68 {
>> +            compatible = "bosch,bmi260";
>> +            reg = <0x68>;
>> +            vdd-supply = <&vdd>;
>> +            vddio-supply = <&vddio>;
>> +            interrupt-parent = <&gpio1>;
>> +            interrupts = <16 IRQ_TYPE_EDGE_RISING>;
>> +            interrupt-names = "INT1";
>> +        };
>> +    };
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 6011af70c12e..73b6b7721dd8 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -4047,6 +4047,7 @@ BOSCH SENSORTEC BMI270 IMU IIO DRIVER
>>  M:	Alex Lanzano <lanzano.alex@gmail.com>
>>  L:	linux-iio@vger.kernel.org
>>  S:	Maintained
>> +F:	Documentation/devicetree/bindings/iio/imu/bosch,bmi260.yaml
>>  F:	Documentation/devicetree/bindings/iio/imu/bosch,bmi270.yaml
>>  F:	drivers/iio/imu/bmi270/
>>  

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

[Justin Weiss]

Jonathan Cameron <jic23@kernel.org> writes:

> On Fri, 18 Oct 2024 16:36:10 -0700
> Justin Weiss <justin@justinweiss.com> 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.
>> 
>> Signed-off-by: Justin Weiss <justin@justinweiss.com>
> Trivial comments inline and a discussion on whether my earlier
> don't use an array comment makes sense in this particular case.
>
> Jonathan
>
>> ---
>>  drivers/iio/imu/bmi270/bmi270.h      |  1 +
>>  drivers/iio/imu/bmi270/bmi270_core.c | 25 +++++++++++++++++++++++--
>>  drivers/iio/imu/bmi270/bmi270_i2c.c  | 13 +++++++++++++
>>  drivers/iio/imu/bmi270/bmi270_spi.c  |  8 ++++++++
>>  4 files changed, 45 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/iio/imu/bmi270/bmi270.h b/drivers/iio/imu/bmi270/bmi270.h
>> index 2e8d85a4e419..51e374fd4290 100644
>> --- a/drivers/iio/imu/bmi270/bmi270.h
>> +++ b/drivers/iio/imu/bmi270/bmi270.h
>> @@ -14,6 +14,7 @@ struct bmi270_data {
>>  };
>>  
>>  enum bmi270_device_type {
>> +	BMI260,
>>  	BMI270,
>>  };
>>  
>> diff --git a/drivers/iio/imu/bmi270/bmi270_core.c b/drivers/iio/imu/bmi270/bmi270_core.c
>> index 799df78ec862..b30201dc4e22 100644
>> --- a/drivers/iio/imu/bmi270/bmi270_core.c
>> +++ b/drivers/iio/imu/bmi270/bmi270_core.c
>> @@ -11,6 +11,8 @@
>>  #include "bmi270.h"
>>  
>>  #define BMI270_CHIP_ID_REG				0x00
>> +#define BMI160_CHIP_ID_VAL				0xD1
>
> This one looks like a cut and paste error.

No, this was intentional -- I added the BMI160 chip ID here so it could
be checked later to avoid conflicting with the existing bmi160 driver. I
could add newlines before and after this group of _ID_VAL #defines if it
makes it clearer.

>> +#define BMI260_CHIP_ID_VAL				0x27
>>  #define BMI270_CHIP_ID_VAL				0x24
>>  #define BMI270_CHIP_ID_MSK				GENMASK(7, 0)
>>  
>> @@ -55,6 +57,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 {
>> @@ -67,6 +70,11 @@ enum bmi270_scan {
>>  };
>>  
>>  const struct bmi270_chip_info bmi270_chip_info[] = {
>> +	[BMI260] = {
>> +		.name = "bmi260",
>> +		.chip_id = BMI260_CHIP_ID_VAL,
>> +		.fw_name = BMI260_INIT_DATA_FILE,
>> +	},
>>  	[BMI270] = {
>>  		.name = "bmi270",
>>  		.chip_id = BMI270_CHIP_ID_VAL,
>> @@ -163,8 +171,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");
>>  
>> -	if (chip_id != BMI270_CHIP_ID_VAL)
>> -		dev_info(dev, "Unknown chip id 0x%x", 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;

This is where that BMI160 chip ID is checked.

>> +
>> +	if (chip_id != bmi270_device->chip_info->chip_id)
>> +		dev_info(dev, "Unexpected chip id 0x%x", chip_id);
>> +
>> +	if (chip_id == BMI260_CHIP_ID_VAL)
>
> Ah. My argument on separate IDs means you'd have to do it this way whereas
> I was thinking maybe a loop would be a better idea.  Ah well if we
> get a lot of supported chips, then we can rethink how to handle this.
> For now what you have here is fine and should deal with lack of appropriate
> ACPI ID mess.

I like the idea of separate structures, so I'll keep the if / else
here. I think it would be straightforward to change later without
conflicts if there are more supported chips.

I will change this to check against bmi260_chip_info.chip_id,
etc. instead of the constants, to make sure they stay consistent.

Justin

>> +		bmi270_device->chip_info = &bmi270_chip_info[BMI260];
>> +	else if (chip_id == BMI270_CHIP_ID_VAL)
>> +		bmi270_device->chip_info = &bmi270_chip_info[BMI270];
>>  
>>  	return 0;
>>  }

Re: [PATCH v2 5/6] iio: imu: bmi270: Add triggered buffer for Bosch BMI270 IMU

[Justin Weiss]

Jonathan Cameron <jic23@kernel.org> writes:

> On Fri, 18 Oct 2024 16:36:11 -0700
> Justin Weiss <justin@justinweiss.com> wrote:
>
>> Set up a triggered buffer for the accel and angl_vel values.
>> 
>> Signed-off-by: Justin Weiss <justin@justinweiss.com>
> Looks good. One trivial comment inline.
>
> Thanks,
>
> Jonathan
>
>> ---
>>  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 51e374fd4290..844d70a7d32e 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 is larger than 8 bytes, align to that
>
> maybe larger than 8 bytes
> on x86_64 I think it is 8 bytes exactly, though I could be remembering that wrong.

Got it. I'll change this to "... IIO_DMA_MINALIGN may be larger than 8
bytes, ..." in v3.

Justin

>> +	 * to ensure a DMA safe buffer.
>> +	 */
>> +	struct {
>> +		__le16 channels[6];
>> +		aligned_s64 timestamp;
>> +	} data __aligned(IIO_DMA_MINALIGN);
>>  };
>>  

Re: [PATCH v2 6/6] iio: imu: bmi270: Add scale and sampling frequency to BMI270 IMU

[Justin Weiss]

Jonathan Cameron <jic23@kernel.org> writes:

> On Fri, 18 Oct 2024 16:36:12 -0700
> Justin Weiss <justin@justinweiss.com> wrote:
>
>> Add read and write functions and create _available entries.
>> 
>> Signed-off-by: Justin Weiss <justin@justinweiss.com>
> Hi Justin,
>
> Just one trivial comment from me.
>
> Jonathan
>
>
>
>> +
>> +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;
>> +	}
>> +
> Unreachable code, so drop this return.

Will remove in v3.

Justin

>> +	return 0;
>> +}
>>

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

[Jonathan Cameron]

> >>  #define BMI270_CHIP_ID_REG				0x00
> >> +#define BMI160_CHIP_ID_VAL				0xD1  
> >
> > This one looks like a cut and paste error.  
> 
> No, this was intentional -- I added the BMI160 chip ID here so it could
> be checked later to avoid conflicting with the existing bmi160 driver. I
> could add newlines before and after this group of _ID_VAL #defines if it
> makes it clearer.

Got it. Just add a comment that it's to exclude known bad firmwares and
that is fine to keep.