| Message ID | 20241011153751.65152-1-justin@justinweiss.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add i2c driver for Bosch BMI260 IMU | expand |
On Fri, 11 Oct 2024 08:37:46 -0700 Justin Weiss <justin@justinweiss.com> wrote: > Add support for the Bosch BMI260 IMU to the BMI270 device driver. > > The BMI270 and BMI260 have nearly identical register maps, but have > different chip IDs and firmware. > > The BMI260 is the IMU on a number of handheld PCs. Unfortunately, > these devices often misidentify it in ACPI as a BMI160 ("BMI0160," for > example), and it can only be correctly identified using the chip > ID. I've changed the driver to fail if the chip ID isn't recognized so > the firmware initialization data isn't sent to incompatible devices. So just to check, is the firmware always specific to an individual chip? Normally we strongly resist hard checks on mismatched IDs because they break the option for using fallback compatibles to get some support on older kernels for newer devices, but if the firmware is locked to a device then that is a good justification. Fallback compatibles in DT will never work here. Note that means you need a specific compatible in Documentation/devicetree/bindings/iio/imu/bosch,bmi270.yaml Technically you could match on a single ID and figure it out, but that will lead to potential confusion if an older kernel is used with a binding written against current kernel and the driver just doesn't work. Not a regression but in my view inelegant. Make sure you include this detail about specific firmware selection in there as well. Jonathan > > 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> > --- > > Justin Weiss (3): > iio: imu: Add i2c driver for bmi260 imu > iio: imu: Add triggered buffer for Bosch BMI270 IMU > iio: imu: Add scale and sampling frequency to BMI270 IMU > > drivers/iio/imu/bmi270/Kconfig | 1 + > drivers/iio/imu/bmi270/bmi270.h | 24 +- > drivers/iio/imu/bmi270/bmi270_core.c | 369 ++++++++++++++++++++++++++- > drivers/iio/imu/bmi270/bmi270_i2c.c | 22 +- > drivers/iio/imu/bmi270/bmi270_spi.c | 11 +- > 5 files changed, 413 insertions(+), 14 deletions(-) > > > base-commit: 96be67caa0f0420d4128cb67f07bbd7a6f49e03a
Thanks for the review! I really appreciate it. Jonathan Cameron <jic23@kernel.org> writes: > On Fri, 11 Oct 2024 08:37:46 -0700 > Justin Weiss <justin@justinweiss.com> wrote: > >> 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. I've changed the driver to fail if the chip ID isn't recognized so >> the firmware initialization data isn't sent to incompatible devices. > > So just to check, is the firmware always specific to an individual chip? For these devices, yes. The BMI160 does not have firmware initialization data. The 260 and 270 have different firmware data from each other. > Normally we strongly resist hard checks on mismatched IDs because they break > the option for using fallback compatibles to get some support on older > kernels for newer devices, but if the firmware is locked to a > device then that is a good justification. Fallback compatibles in DT > will never work here. The specific problem I'm trying to avoid with this hard check is the situation when a device actually has a BMI160, this driver matches "BMI0160", and sends the BMI260 firmware data to a BMI160 chip. I suppose this driver could target this situation by only failing to probe if it detects the BMI160 chip ID. That would imply that the device is a BMI160 and should not be handled by this driver. Then, as you suggested in another response, the driver could check the other chip IDs individually. If the driver detects the 260 chip ID, it would send the BMI260 firmware and so on with the 270. Otherwise, it would use the chip_info found by match_data. This would at least handle the problem we see in shipped devices while keeping it flexible for the future. I'm happy to make those changes if they make more sense to you. There's an older thread here that provides more background about the DSDT confusion: https://lore.kernel.org/all/CAFqHKTm2WRNkcSoBEE=oNbfu_9d9RagQHLydmv6q1=snO_MXyA@mail.gmail.com/ Justin
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. I've changed the driver to fail if the chip ID isn't recognized so the firmware initialization data isn't sent to incompatible devices. 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> --- Justin Weiss (3): iio: imu: Add i2c driver for bmi260 imu iio: imu: Add triggered buffer for Bosch BMI270 IMU iio: imu: Add scale and sampling frequency to BMI270 IMU drivers/iio/imu/bmi270/Kconfig | 1 + drivers/iio/imu/bmi270/bmi270.h | 24 +- drivers/iio/imu/bmi270/bmi270_core.c | 369 ++++++++++++++++++++++++++- drivers/iio/imu/bmi270/bmi270_i2c.c | 22 +- drivers/iio/imu/bmi270/bmi270_spi.c | 11 +- 5 files changed, 413 insertions(+), 14 deletions(-) base-commit: 96be67caa0f0420d4128cb67f07bbd7a6f49e03a