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.0Justin 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,
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
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
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
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
>
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
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
© 2016 - 2024 Red Hat, Inc.