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

[PATCH v3 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:

V3
- 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 (6):
  iio: imu: bmi270: Remove unused FREQUENCY / SCALE attributes
  iio: imu: bmi270: Provide chip info as configuration structure
  dt-bindings: iio: imu: bmi270: 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,bmi270.yaml        |   4 +-
 drivers/iio/imu/bmi270/Kconfig                |   1 +
 drivers/iio/imu/bmi270/bmi270.h               |  21 +-
 drivers/iio/imu/bmi270/bmi270_core.c          | 442 +++++++++++++++++-
 drivers/iio/imu/bmi270/bmi270_i2c.c           |  24 +-
 drivers/iio/imu/bmi270/bmi270_spi.c           |  19 +-
 6 files changed, 496 insertions(+), 15 deletions(-)


base-commit: 1a8b58362f6a6fef975032f7fceb7c4b80d20d60
-- 
2.47.0

[PATCH v3 1/6] iio: imu: bmi270: Remove unused FREQUENCY / SCALE attributes

[Justin Weiss]

These attributes are not currently wired up, and will always return
EINVAL.

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, 4 deletions(-)

diff --git a/drivers/iio/imu/bmi270/bmi270_core.c b/drivers/iio/imu/bmi270/bmi270_core.c
index aeda7c4228df..e598c642178f 100644
--- a/drivers/iio/imu/bmi270/bmi270_core.c
+++ b/drivers/iio/imu/bmi270/bmi270_core.c
@@ -121,8 +121,6 @@ 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_FREQUENCY),			\
 }
 
 #define BMI270_ANG_VEL_CHANNEL(_axis) {				\
@@ -130,8 +128,6 @@ 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_FREQUENCY),			\
 }
 
 static const struct iio_chan_spec bmi270_channels[] = {
-- 
2.47.0

[PATCH v3 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      | 11 ++++++++++-
 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, 40 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/imu/bmi270/bmi270.h b/drivers/iio/imu/bmi270/bmi270.h
index 8ac20ad7ee94..93e5f387607b 100644
--- a/drivers/iio/imu/bmi270/bmi270.h
+++ b/drivers/iio/imu/bmi270/bmi270.h
@@ -10,10 +10,19 @@ struct device;
 struct bmi270_data {
 	struct device *dev;
 	struct regmap *regmap;
+	const struct bmi270_chip_info *chip_info;
+};
+
+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 e598c642178f..5f08d786fa21 100644
--- a/drivers/iio/imu/bmi270/bmi270_core.c
+++ b/drivers/iio/imu/bmi270/bmi270_core.c
@@ -66,6 +66,13 @@ enum bmi270_scan {
 	BMI270_SCAN_GYRO_Z,
 };
 
+const struct bmi270_chip_info bmi270_chip_info = {
+	.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)
 {
@@ -150,7 +157,7 @@ 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)
+	if (chip_id != bmi270_device->chip_info->chip_id)
 		dev_info(dev, "Unknown chip id 0x%x", chip_id);
 
 	return 0;
@@ -183,7 +190,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");
 
@@ -270,7 +278,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;
@@ -283,6 +292,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)
@@ -290,7 +300,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 d59161f23f9a..394f27996059 100644
--- a/drivers/iio/imu/bmi270/bmi270_i2c.c
+++ b/drivers/iio/imu/bmi270/bmi270_i2c.c
@@ -17,22 +17,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 },
 	{ }
 };
 
 static const struct of_device_id bmi270_of_match[] = {
-	{ .compatible = "bosch,bmi270" },
+	{ .compatible = "bosch,bmi270", .data = &bmi270_chip_info },
 	{ }
 };
 
diff --git a/drivers/iio/imu/bmi270/bmi270_spi.c b/drivers/iio/imu/bmi270/bmi270_spi.c
index b53784d4a1f4..7c2062c660d9 100644
--- a/drivers/iio/imu/bmi270/bmi270_spi.c
+++ b/drivers/iio/imu/bmi270/bmi270_spi.c
@@ -49,6 +49,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);
@@ -56,16 +61,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 },
 	{ }
 };
 
 static const struct of_device_id bmi270_of_match[] = {
-	{ .compatible = "bosch,bmi270" },
+	{ .compatible = "bosch,bmi270", .data = &bmi270_chip_info },
 	{ }
 };
 
-- 
2.47.0

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

[Justin Weiss]

Add compatible ID for Bosch BMI260 to BMI270 documentation.

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 v3 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 | 28 +++++++++++++++++++++++++++-
 drivers/iio/imu/bmi270/bmi270_i2c.c  | 13 +++++++++++++
 drivers/iio/imu/bmi270/bmi270_spi.c  |  8 ++++++++
 4 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/imu/bmi270/bmi270.h b/drivers/iio/imu/bmi270/bmi270.h
index 93e5f387607b..d8f6d7cf47fc 100644
--- a/drivers/iio/imu/bmi270/bmi270.h
+++ b/drivers/iio/imu/bmi270/bmi270.h
@@ -20,6 +20,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 5f08d786fa21..24e45d6f0706 100644
--- a/drivers/iio/imu/bmi270/bmi270_core.c
+++ b/drivers/iio/imu/bmi270/bmi270_core.c
@@ -11,6 +11,11 @@
 #include "bmi270.h"
 
 #define BMI270_CHIP_ID_REG				0x00
+
+/* Used 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)
 
@@ -55,6 +60,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 {
@@ -66,6 +72,13 @@ enum bmi270_scan {
 	BMI270_SCAN_GYRO_Z,
 };
 
+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,
@@ -157,8 +170,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..3d0d8f3e8b63 100644
--- a/drivers/iio/imu/bmi270/bmi270_i2c.c
+++ b/drivers/iio/imu/bmi270/bmi270_i2c.c
@@ -32,11 +32,23 @@ 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[] = {
+	/* OrangePi NEO */
+	{ "BMI0260",  (kernel_ulong_t)&bmi260_chip_info },
+	/* GPD Win Mini, Aya Neo AIR Pro, OXP Mini Pro, etc. */
+	{ "BMI0160",  (kernel_ulong_t)&bmi260_chip_info },
+	/* GPD Win Max 2 */
+	{ "10EC5280", (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 +56,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..7f42ed74023b 100644
--- a/drivers/iio/imu/bmi270/bmi270_spi.c
+++ b/drivers/iio/imu/bmi270/bmi270_spi.c
@@ -65,11 +65,18 @@ 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 acpi_device_id bmi270_acpi_match[] = {
+	{ "BOSC0260",  (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 },
 	{ }
 };
@@ -77,6 +84,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 v3 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 d8f6d7cf47fc..fdfad5784cc5 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 24e45d6f0706..5b643afae9f7 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"
 
@@ -70,6 +72,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 bmi260_chip_info = {
@@ -86,6 +99,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)
 {
@@ -141,6 +175,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) {				\
@@ -148,6 +189,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[] = {
@@ -157,6 +205,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)
@@ -327,9 +376,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 v3 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 | 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 5b643afae9f7..3e1e698c4af0 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>
 
@@ -38,6 +39,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
@@ -46,6 +50,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)
 
@@ -99,6 +106,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;
@@ -161,6 +427,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;
 	}
@@ -168,6 +496,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) {				\
@@ -175,6 +505,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',					\
@@ -189,6 +524,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

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

[Krzysztof Kozlowski]

On Sun, Oct 20, 2024 at 03:00:07PM -0700, Justin Weiss wrote:
> Add compatible ID for Bosch BMI260 to BMI270 documentation.

This we see from the diff. Say something about the hardware, are they
compatible? No? What are the differences?

Best regards,
Krzysztof

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

[Justin Weiss]

Justin Weiss <justin@justinweiss.com> writes:

> 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 | 28 +++++++++++++++++++++++++++-
>  drivers/iio/imu/bmi270/bmi270_i2c.c  | 13 +++++++++++++
>  drivers/iio/imu/bmi270/bmi270_spi.c  |  8 ++++++++
>  4 files changed, 49 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/imu/bmi270/bmi270.h b/drivers/iio/imu/bmi270/bmi270.h
> index 93e5f387607b..d8f6d7cf47fc 100644
> --- a/drivers/iio/imu/bmi270/bmi270.h
> +++ b/drivers/iio/imu/bmi270/bmi270.h
> @@ -20,6 +20,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 5f08d786fa21..24e45d6f0706 100644
> --- a/drivers/iio/imu/bmi270/bmi270_core.c
> +++ b/drivers/iio/imu/bmi270/bmi270_core.c
> @@ -11,6 +11,11 @@
>  #include "bmi270.h"
>  
>  #define BMI270_CHIP_ID_REG				0x00
> +
> +/* Used 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)
>  
> @@ -55,6 +60,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 {
> @@ -66,6 +72,13 @@ enum bmi270_scan {
>  	BMI270_SCAN_GYRO_Z,
>  };
>  
> +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,
> @@ -157,8 +170,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..3d0d8f3e8b63 100644
> --- a/drivers/iio/imu/bmi270/bmi270_i2c.c
> +++ b/drivers/iio/imu/bmi270/bmi270_i2c.c
> @@ -32,11 +32,23 @@ 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 },
>  	{ }
>  };
>

The ACPI IDs with device pointers are here:

> +static const struct acpi_device_id bmi270_acpi_match[] = {
> +	/* OrangePi NEO */
> +	{ "BMI0260",  (kernel_ulong_t)&bmi260_chip_info },
> +	/* GPD Win Mini, Aya Neo AIR Pro, OXP Mini Pro, etc. */
> +	{ "BMI0160",  (kernel_ulong_t)&bmi260_chip_info },
> +	/* GPD Win Max 2 */
> +	{ "10EC5280", (kernel_ulong_t)&bmi260_chip_info },
> +	{ }
> +};

I pulled DSDT device excerpts for the GPD Win Mini (which uses the
BMI0160 ACPI ID, even though it has a bmi260) and the OrangePi NEO
(which uses the BMI0260 ACPI ID).

I couldn't find a shipping device with a bmi260 using the 10EC5280 ACPI
ID. Some prototype devices with the bmi260 may have used them:
https://lore.kernel.org/all/CAFqHKTm2WRNkcSoBEE=oNbfu_9d9RagQHLydmv6q1=snO_MXyA@mail.gmail.com/

I can remove that ID from this changeset for now.


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 */
    }

    OperationRegion (CMS2, SystemIO, 0x72, 0x02)
    Field (CMS2, ByteAcc, NoLock, Preserve)
    {
        IND2,   8, 
        DAT2,   8
    }

    IndexField (IND2, DAT2, ByteAcc, NoLock, Preserve)
    {
        Offset (0x74), 
        BACS,   32
    }

    Method (ROMS, 0, NotSerialized)
    {
        Name (RBUF, Package (0x03)
        {
            "0 1 0", 
            "1 0 0", 
            "0 0 1"
        })
        Return (RBUF) /* \_SB_.I2CB.BMI2.ROMS.RBUF */
    }

    Method (CALS, 1, NotSerialized)
    {
        Local0 = Arg0
        If (((Local0 == Zero) || (Local0 == Ones)))
        {
            Return (Local0)
        }
        Else
        {
            BACS = Local0
        }
    }

    Method (_STA, 0, NotSerialized)  // _STA: Status
    {
        Return (0x0F)
    }
}


OrangePi NEO:

Device (BMI2)
{
    Name (_ADR, Zero)  // _ADR: Address
    Name (_HID, "BMI0260")  // _HID: Hardware ID
    Name (_CID, "BMI0260")  // _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 */
    }

    OperationRegion (CMS2, SystemIO, 0x72, 0x02)
    Field (CMS2, ByteAcc, NoLock, Preserve)
    {
        IND2,   8, 
        DAT2,   8
    }

    IndexField (IND2, DAT2, ByteAcc, NoLock, Preserve)
    {
        Offset (0x74), 
        BACS,   32
    }

    Method (ROMS, 0, NotSerialized)
    {
        Name (RBUF, Package (0x03)
        {
            "0 1 0", 
            "1 0 0", 
            "0 0 1"
        })
        Return (RBUF) /* \_SB_.I2CB.BMI2.ROMS.RBUF */
    }

    Method (CALS, 1, NotSerialized)
    {
        Local0 = Arg0
        If (((Local0 == Zero) || (Local0 == Ones)))
        {
            Return (Local0)
        }
        Else
        {
            BACS = Local0
        }
    }

    Method (_STA, 0, NotSerialized)  // _STA: Status
    {
        Return (0x0F)
    }
}

> +
>  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 +56,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..7f42ed74023b 100644
> --- a/drivers/iio/imu/bmi270/bmi270_spi.c
> +++ b/drivers/iio/imu/bmi270/bmi270_spi.c
> @@ -65,11 +65,18 @@ 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 acpi_device_id bmi270_acpi_match[] = {
> +	{ "BOSC0260",  (kernel_ulong_t)&bmi260_chip_info },
> +	{ }
> +};
> +

I can't find any evidence of BOSC0260 being used, besides existing in
the Windows driver. As suggested in an earlier review, I added it here
to encourage people looking at this driver in the future to use the
correct ACPI ID.

Thanks,
Justin

>  static const struct of_device_id bmi270_of_match[] = {
> +	{ .compatible = "bosch,bmi260", .data = &bmi260_chip_info },
>  	{ .compatible = "bosch,bmi270", .data = &bmi270_chip_info },
>  	{ }
>  };
> @@ -77,6 +84,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,

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

[Justin Weiss]

Krzysztof Kozlowski <krzk@kernel.org> writes:

> On Sun, Oct 20, 2024 at 03:00:07PM -0700, Justin Weiss wrote:
>> Add compatible ID for Bosch BMI260 to BMI270 documentation.
>
> This we see from the diff. Say something about the hardware, are they
> compatible? No? What are the differences?

Got it, I'll update the commit message.

The bmi260 has nearly identical register maps and capabilities as the
bmi270, but the devices have different chip IDs and require different
initialization firmware.

Thanks,
Justin

> Best regards,
> Krzysztof

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

[Andy Shevchenko]

On Tue, Oct 22, 2024 at 08:50:43AM -0700, Justin Weiss wrote:
> Justin Weiss <justin@justinweiss.com> writes:

...

> The ACPI IDs with device pointers are here:
> 
> > +static const struct acpi_device_id bmi270_acpi_match[] = {
> > +	/* OrangePi NEO */
> > +	{ "BMI0260",  (kernel_ulong_t)&bmi260_chip_info },
> > +	/* GPD Win Mini, Aya Neo AIR Pro, OXP Mini Pro, etc. */
> > +	{ "BMI0160",  (kernel_ulong_t)&bmi260_chip_info },
> > +	/* GPD Win Max 2 */
> > +	{ "10EC5280", (kernel_ulong_t)&bmi260_chip_info },
> > +	{ }

Cool! But please, keep them alphabetically ordered by ID.

Can we push OrangePI NED to go and fix ACPI IDs eventually?

> > +};
> 
> I pulled DSDT device excerpts for the GPD Win Mini (which uses the
> BMI0160 ACPI ID, even though it has a bmi260) and the OrangePi NEO
> (which uses the BMI0260 ACPI ID).
> 
> I couldn't find a shipping device with a bmi260 using the 10EC5280 ACPI
> ID. Some prototype devices with the bmi260 may have used them:
> https://lore.kernel.org/all/CAFqHKTm2WRNkcSoBEE=oNbfu_9d9RagQHLydmv6q1=snO_MXyA@mail.gmail.com/
> 
> I can remove that ID from this changeset for now.

Yes, please do not add anything that has no evidence of existence in the wild
or approved vendor allocated ID.

> GPD Win Mini:

Add short parts of these to the commit message, or better split these to two
patches each of them adding a new ID to the table.

See below what I do want to see there (no need to have everything),
i.e. I removed unneeded lines:

> Device (BMI2)
> {
>     Name (_ADR, Zero)  // _ADR: Address

My gosh, can this be fixed (seems rhetorical)? The _ADR must NOT be present
together with _HID.  It's against the ACPI specifications.

>     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 */
>     }
      ...
> }
> 

> OrangePi NEO:

Same comments for this device.

...

> > +static const struct acpi_device_id bmi270_acpi_match[] = {
> > +	{ "BOSC0260",  (kernel_ulong_t)&bmi260_chip_info },
> > +	{ }
> > +};
> 
> I can't find any evidence of BOSC0260 being used, besides existing in
> the Windows driver. As suggested in an earlier review, I added it here
> to encourage people looking at this driver in the future to use the
> correct ACPI ID.

Are you official representative of Bosch or do you have a proof by the vendor
that they allocated this ID? Otherwise we may NOT allocate IDs on their behalf
and has not to be added.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/6] iio: imu: bmi270: Remove unused FREQUENCY / SCALE attributes

[Jonathan Cameron]

On Sun, 20 Oct 2024 15:00:05 -0700
Justin Weiss <justin@justinweiss.com> wrote:

> These attributes are not currently wired up, and will always return
> EINVAL.
> 
> Fixes: 3ea51548d6b2 ("iio: imu: Add i2c driver for bmi270 imu")
> Signed-off-by: Justin Weiss <justin@justinweiss.com>
Applied this one to the togreg branch of iio.git and pushed out as
testing.

Thanks,

Jonathan

> ---
>  drivers/iio/imu/bmi270/bmi270_core.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/iio/imu/bmi270/bmi270_core.c b/drivers/iio/imu/bmi270/bmi270_core.c
> index aeda7c4228df..e598c642178f 100644
> --- a/drivers/iio/imu/bmi270/bmi270_core.c
> +++ b/drivers/iio/imu/bmi270/bmi270_core.c
> @@ -121,8 +121,6 @@ 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_FREQUENCY),			\
>  }
>  
>  #define BMI270_ANG_VEL_CHANNEL(_axis) {				\
> @@ -130,8 +128,6 @@ 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_FREQUENCY),			\
>  }
>  
>  static const struct iio_chan_spec bmi270_channels[] = {

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

[Jonathan Cameron]

On Sun, 20 Oct 2024 15:00:06 -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>
In the interests of reducing how many patches are in flight I'll
pick this one as well. Can't go beyond this point though until patch 3
gets a DT RB.
> ---
>  drivers/iio/imu/bmi270/bmi270.h      | 11 ++++++++++-
>  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, 40 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/iio/imu/bmi270/bmi270.h b/drivers/iio/imu/bmi270/bmi270.h
> index 8ac20ad7ee94..93e5f387607b 100644
> --- a/drivers/iio/imu/bmi270/bmi270.h
> +++ b/drivers/iio/imu/bmi270/bmi270.h
> @@ -10,10 +10,19 @@ struct device;
>  struct bmi270_data {
>  	struct device *dev;
>  	struct regmap *regmap;
> +	const struct bmi270_chip_info *chip_info;
> +};
> +
> +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 e598c642178f..5f08d786fa21 100644
> --- a/drivers/iio/imu/bmi270/bmi270_core.c
> +++ b/drivers/iio/imu/bmi270/bmi270_core.c
> @@ -66,6 +66,13 @@ enum bmi270_scan {
>  	BMI270_SCAN_GYRO_Z,
>  };
>  
> +const struct bmi270_chip_info bmi270_chip_info = {
> +	.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)
>  {
> @@ -150,7 +157,7 @@ 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)
> +	if (chip_id != bmi270_device->chip_info->chip_id)
>  		dev_info(dev, "Unknown chip id 0x%x", chip_id);
>  
>  	return 0;
> @@ -183,7 +190,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");
>  
> @@ -270,7 +278,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;
> @@ -283,6 +292,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)
> @@ -290,7 +300,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 d59161f23f9a..394f27996059 100644
> --- a/drivers/iio/imu/bmi270/bmi270_i2c.c
> +++ b/drivers/iio/imu/bmi270/bmi270_i2c.c
> @@ -17,22 +17,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 },
>  	{ }
>  };
>  
>  static const struct of_device_id bmi270_of_match[] = {
> -	{ .compatible = "bosch,bmi270" },
> +	{ .compatible = "bosch,bmi270", .data = &bmi270_chip_info },
>  	{ }
>  };
>  
> diff --git a/drivers/iio/imu/bmi270/bmi270_spi.c b/drivers/iio/imu/bmi270/bmi270_spi.c
> index b53784d4a1f4..7c2062c660d9 100644
> --- a/drivers/iio/imu/bmi270/bmi270_spi.c
> +++ b/drivers/iio/imu/bmi270/bmi270_spi.c
> @@ -49,6 +49,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);
> @@ -56,16 +61,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 },
>  	{ }
>  };
>  
>  static const struct of_device_id bmi270_of_match[] = {
> -	{ .compatible = "bosch,bmi270" },
> +	{ .compatible = "bosch,bmi270", .data = &bmi270_chip_info },
>  	{ }
>  };
>  

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

[Jonathan Cameron]

On Tue, 22 Oct 2024 19:54:03 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Tue, Oct 22, 2024 at 08:50:43AM -0700, Justin Weiss wrote:
> > Justin Weiss <justin@justinweiss.com> writes:  
> 
> ...
> 
> > The ACPI IDs with device pointers are here:
> >   
> > > +static const struct acpi_device_id bmi270_acpi_match[] = {
> > > +	/* OrangePi NEO */
> > > +	{ "BMI0260",  (kernel_ulong_t)&bmi260_chip_info },
> > > +	/* GPD Win Mini, Aya Neo AIR Pro, OXP Mini Pro, etc. */
> > > +	{ "BMI0160",  (kernel_ulong_t)&bmi260_chip_info },
> > > +	/* GPD Win Max 2 */
> > > +	{ "10EC5280", (kernel_ulong_t)&bmi260_chip_info },
> > > +	{ }  
> 
> Cool! But please, keep them alphabetically ordered by ID.
> 
> Can we push OrangePI NED to go and fix ACPI IDs eventually?
> 
> > > +};  
> > 
> > I pulled DSDT device excerpts for the GPD Win Mini (which uses the
> > BMI0160 ACPI ID, even though it has a bmi260) and the OrangePi NEO
> > (which uses the BMI0260 ACPI ID).
> > 
> > I couldn't find a shipping device with a bmi260 using the 10EC5280 ACPI
> > ID. Some prototype devices with the bmi260 may have used them:
> > https://lore.kernel.org/all/CAFqHKTm2WRNkcSoBEE=oNbfu_9d9RagQHLydmv6q1=snO_MXyA@mail.gmail.com/
> > 
> > I can remove that ID from this changeset for now.  
> 
> Yes, please do not add anything that has no evidence of existence in the wild
> or approved vendor allocated ID.
> 
> > GPD Win Mini:  
> 
> Add short parts of these to the commit message, or better split these to two
> patches each of them adding a new ID to the table.
> 
> See below what I do want to see there (no need to have everything),
> i.e. I removed unneeded lines:
> 
> > Device (BMI2)
> > {
> >     Name (_ADR, Zero)  // _ADR: Address  
> 
> My gosh, can this be fixed (seems rhetorical)? The _ADR must NOT be present
> together with _HID.  It's against the ACPI specifications.
> 
> >     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 */
> >     }  
>       ...
> > }
> >   
> 
> > OrangePi NEO:  
> 
> Same comments for this device.
> 
> ...
> 
> > > +static const struct acpi_device_id bmi270_acpi_match[] = {
> > > +	{ "BOSC0260",  (kernel_ulong_t)&bmi260_chip_info },
> > > +	{ }
> > > +};  
> > 
> > I can't find any evidence of BOSC0260 being used, besides existing in
> > the Windows driver. As suggested in an earlier review, I added it here
> > to encourage people looking at this driver in the future to use the
> > correct ACPI ID.  
> 
> Are you official representative of Bosch or do you have a proof by the vendor
> that they allocated this ID? Otherwise we may NOT allocate IDs on their behalf
> and has not to be added.
Fair point. The provenance of the driver is a little disconnected from Bosch:
https://ayaneo-1305909189.cos.accelerate.myqcloud.com/ayaneo_com/download/2023/UMDF2.0_BMI260_V1.0.23_5ID_signed_20H1.zip

Justin, if you have contacts at ayaneo, maybe they can confirm if the IDs come
from Bosch. Or maybe we can find someone at Bosch?

Jonathan


> 

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

[Philip Müller]

On 22/10/24 22:50, Justin Weiss wrote:
 > I couldn't find a shipping device with a bmi260 using the 10EC5280 ACPI
 > ID. Some prototype devices with the bmi260 may have used them:
 > https://lore.kernel.org/all/
 > CAFqHKTm2WRNkcSoBEE=oNbfu_9d9RagQHLydmv6q1=snO_MXyA@mail.gmail.com/

The Arch wiki has some recordings of that. Most likely got fixed in 
newer BIOSs to the BMI0XXX coding.

https://wiki.archlinux.org/title/AYA_NEO_2021#IMU_(Accelerometer_+_Gyro)
https://wiki.archlinux.org/title/GPD_Win_Max#IMU_(Accelerometer_+_Gyro)

On 22/10/24 22:50, Justin Weiss wrote:
> I can't find any evidence of BOSC0260 being used, besides existing in
> the Windows driver. As suggested in an earlier review, I added it here
> to encourage people looking at this driver in the future to use the
> correct ACPI ID.

Based on the BIOS code from the OrangePi Neo the default value was 
10EC5280 which got commented out and replaced by BMI0260. For BIOS v1.19 
however OrangePi will use BOSC0260. I might provide a new DSDT dump as 
soon as I get the newer BIOS from the vendor.

-- 
Best, Philip

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

[Andy Shevchenko]

On Thu, Oct 24, 2024 at 01:40:50PM +0700, Philip Müller wrote:
> On 22/10/24 22:50, Justin Weiss wrote:
> > I couldn't find a shipping device with a bmi260 using the 10EC5280 ACPI
> > ID. Some prototype devices with the bmi260 may have used them:
> > https://lore.kernel.org/all/
> > CAFqHKTm2WRNkcSoBEE=oNbfu_9d9RagQHLydmv6q1=snO_MXyA@mail.gmail.com/
> 
> The Arch wiki has some recordings of that. Most likely got fixed in newer
> BIOSs to the BMI0XXX coding.
> 
> https://wiki.archlinux.org/title/AYA_NEO_2021#IMU_(Accelerometer_+_Gyro)
> https://wiki.archlinux.org/title/GPD_Win_Max#IMU_(Accelerometer_+_Gyro)
> 
> On 22/10/24 22:50, Justin Weiss wrote:
> > I can't find any evidence of BOSC0260 being used, besides existing in
> > the Windows driver. As suggested in an earlier review, I added it here
> > to encourage people looking at this driver in the future to use the
> > correct ACPI ID.
> 
> Based on the BIOS code from the OrangePi Neo the default value was 10EC5280
> which got commented out and replaced by BMI0260. For BIOS v1.19 however
> OrangePi will use BOSC0260. I might provide a new DSDT dump as soon as I get
> the newer BIOS from the vendor.

Okay, at least that will be correct ID from the specification perspective.
Still, do  we have an (written) approval from Bosch to use that ID?

Anyway, that needs to be in its own patch with the DSDT excerpt and other
information.

P.S. And thanks for working on this, at least we are coming better to have
this mess to be sorted out and possibly preventing fake IDs from appearing
in the future.

-- 
With Best Regards,
Andy Shevchenko

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

[Justin Weiss]

Jonathan Cameron <jic23@kernel.org> writes:

> On Tue, 22 Oct 2024 19:54:03 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>
>> On Tue, Oct 22, 2024 at 08:50:43AM -0700, Justin Weiss wrote:
>> > Justin Weiss <justin@justinweiss.com> writes:
>> > ...
>> > I can't find any evidence of BOSC0260 being used, besides existing in
>> > the Windows driver. As suggested in an earlier review, I added it here
>> > to encourage people looking at this driver in the future to use the
>> > correct ACPI ID.  
>> 
>> Are you official representative of Bosch or do you have a proof by the vendor
>> that they allocated this ID? Otherwise we may NOT allocate IDs on their behalf
>> and has not to be added.
> Fair point. The provenance of the driver is a little disconnected from Bosch:
> https://ayaneo-1305909189.cos.accelerate.myqcloud.com/ayaneo_com/download/2023/UMDF2.0_BMI260_V1.0.23_5ID_signed_20H1.zip
>
> Justin, if you have contacts at ayaneo, maybe they can confirm if the IDs come
> from Bosch. Or maybe we can find someone at Bosch?
>
> Jonathan

I've asked around a bit but haven't been able to find a contact at Bosch
to help answer these questions. I also haven't heard back from AYANEO.

In the meantime, I can reorder the patches to add the triggers and
attributes first and leave the BMI260 support / ACPI ID additions at the
end.

I'll include the BMI0160 ID (and DSDT) in the patch adding the initial
support, since we know that one exists in the wild, and hold on adding
BOSC0260 until there's a way to move forward on it.

I'll also remove all ACPI IDs from the SPI driver since we haven't seen
them at all yet. If we get clarification on the correct ACPI IDs to use,
we can add it later on.

Justin

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

[Andy Shevchenko]

On Fri, Oct 25, 2024 at 08:42:59AM -0700, Justin Weiss wrote:
> Jonathan Cameron <jic23@kernel.org> writes:
> > On Tue, 22 Oct 2024 19:54:03 +0300
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >> On Tue, Oct 22, 2024 at 08:50:43AM -0700, Justin Weiss wrote:
> >> > Justin Weiss <justin@justinweiss.com> writes:

...

> >> > I can't find any evidence of BOSC0260 being used, besides existing in
> >> > the Windows driver. As suggested in an earlier review, I added it here
> >> > to encourage people looking at this driver in the future to use the
> >> > correct ACPI ID.  
> >> 
> >> Are you official representative of Bosch or do you have a proof by the vendor
> >> that they allocated this ID? Otherwise we may NOT allocate IDs on their behalf
> >> and has not to be added.
> > Fair point. The provenance of the driver is a little disconnected from Bosch:
> > https://ayaneo-1305909189.cos.accelerate.myqcloud.com/ayaneo_com/download/2023/UMDF2.0_BMI260_V1.0.23_5ID_signed_20H1.zip
> >
> > Justin, if you have contacts at ayaneo, maybe they can confirm if the IDs come
> > from Bosch. Or maybe we can find someone at Bosch?
> 
> I've asked around a bit but haven't been able to find a contact at Bosch
> to help answer these questions. I also haven't heard back from AYANEO.

Hmm... Maybe we can grep for the added BOSC IDs in the kernel and see if it
gives any contacts / clues whom to contact, but I'm not insisting you to go
this way, only if you want to.

> In the meantime, I can reorder the patches to add the triggers and
> attributes first and leave the BMI260 support / ACPI ID additions at the
> end.
> 
> I'll include the BMI0160 ID (and DSDT) in the patch adding the initial
> support, since we know that one exists in the wild, and hold on adding
> BOSC0260 until there's a way to move forward on it.

Sounds like a plan!

> I'll also remove all ACPI IDs from the SPI driver since we haven't seen
> them at all yet. If we get clarification on the correct ACPI IDs to use,
> we can add it later on.

Since ACPI has the different resource types for these busses it's usually okay
to have the same ID in both, but there is no special requirement for following
or not following that. And taking a history of fake ACPI IDs in the past,
I would go defensive way as you put it here, i.e. no ID should be added until
we have a proof of them being used in the real products.

-- 
With Best Regards,
Andy Shevchenko