On Sun, 4 Feb 2024 18:53:42 +0100
Jesus Miguel Gonzalez Herrero <jesusmgh@xxxxxxxxx> wrote:
> Hello Mr. Cameron
>
> First of all thank you for reviewing the patch.
>
> And I most definitely agree with you and Mr. Shevchenko: this absolutely
> is a firmware bug that manufacturers should fix. For this reason some
> people started talks with affected manufacturers to change it. In my
> case it was with GPD, together with some others, including some which
> historically had a more direct line with them. This was finally dismissed
> as WONTFIX, since their main focus is Windows and their driver supports
> the ID, so the end result of those conversations is a lack of a fixed
> firmware, and a surplus of frustration.
>
> As far as I know people have been in talks with Aya too, and I do not
> know the status of conversations with Lenovo or other manufacturers. I
> do not know of any conversation with Realtek, besides what was mentioned
> in those emails you linked to from 2021.
>
> I will amend the patch to include a big disclaimer and the reason as
> a comment in the code, and send it again in reply to this message. I
> don't think I'd go as far as tainting the kernel, but I'm not opposed,
> happy anyway if the IMU finally becomes usable, and VERY far from any
> expertise whatsoever concerning kernel development!
>
> Here is the relevant extract from the DSDT of my GPD Win Max 2 (AMD
> 6800U model) with the latest firmware 1.05 installed.
Great - This is exactly the info that should support such a patch like this.
Include the blob below and a statement that GPD have refused to fix due
to the windows driver in the patch description and I'm fine with taking this.
I may see if I can dig out a Lenovo contact as they are big enough and
have been around long enough to know better... (big company mess however
I'm sure...)
Jonathan
>
> Scope (_SB.I2CC) {
> Device (BMA2) {
> Name (_ADR, Zero) // _ADR: Address Name (_HID, "10EC5280")
> // _HID: Hardware ID Name (_CID, "10EC5280") // _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 (0x0069, ControllerInitiated,
> 0x00061A80,
> AddressingMode7Bit, "\\_SB.I2CC", 0x00,
> ResourceConsumer, , Exclusive, )
> }) Return (RBUF) /* \_SB_.I2CC.BMA2._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_.I2CC.BMA2.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)
> }
> }
> }
>
> Thank you for taking this into consideration!
>
> Jesus Gonzalez
>
>
>
> On 04/02/2024 15:00, Jonathan Cameron wrote:
> > On Fri, 2 Feb 2024 18:30:41 +0100
> > Jesus Gonzalez <jesusmgh@xxxxxxxxx> wrote:
> >
> >> "10EC5280" is used by several manufacturers like Lenovo, GPD, or AYA (and
> >> probably others) in their ACPI table as the ID for the bmi160 IMU. This
> >> means the bmi160_i2c driver won't bind to it, and the IMU is unavailable
> >> to the user. Manufacturers have been approached on several occasions to
> >> try getting a BIOS with a fixed ID, mostly without actual positive
> >> results, and since affected devices are already a few years old, this is
> >> not expected to change. This patch enables using the bmi160_i2c driver for
> >> the bmi160 IMU on these devices.
> > Hi Jesus,
> >
> > https://lore.kernel.org/lkml/CAHp75Vct-AXnU7QQmdE7nyYZT-=n=p67COPLiiZTet7z7snL-g@xxxxxxxxxxxxxx/
> > Lays out what Andy (and for that matter I) consider necessary for such
> > a patch.
> >
> > In short, we want to see devices called out here - with a DSDT section.
> > + a clear comment in the code.
> >
> > The big problem here is this tramples on Realtech's ID space. It's not just
> > a made up code (incidentally the BMI0160 isn't valid either),
> > it's a valid code but for an entirely different (PCI) device.
> >
> > So we need as much info as possible in the patch description and the driver
> > itself to justify carrying this. Tempting to add a firmware bug taint on
> > it as well but that might scare people :)
> >
> > Jonathan
> >
> >
> >> Signed-off-by: Jesus Gonzalez <jesusmgh@xxxxxxxxx>
> >> ---
> >> A device-specific transformation matrix can then be provided in a second
> >> step through udev hwdb.
> >>
> >> This has been discussed before in 2021, see here:
> >> https://lore.kernel.org/lkml/CACAwPwYQHRcrabw9=0tvenPzAcwwW1pTaR6a+AEWBF9Hqf_wXQ@xxxxxxxxxxxxxx/
> >>
> >> Lenovo, as an example of a big manufacturer, is also using this ID:
> >> https://www.reddit.com/r/linux/comments/r6f9de/comment/hr8bdfs/?context=3
> >>
> >> At least some discussions with GPD took place on the GPD server Discord,
> >> for which I can provide proof on demand via screenshot (if not accessible
> >> directly).
> >>
> >> I have read the patch submission instructions and followed them to the
> >> best of my knowledge. Still, this is my first kernel patch submission,
> >> so I'd be glad if you could please point out any mistakes. Thank you!
> >>
> >>
> >> drivers/iio/imu/bmi160/bmi160_spi.c | 1 +
> >> 1 file changed, 1 insertion(+)
> >>
> >> diff --git a/drivers/iio/imu/bmi160/bmi160_spi.c b/drivers/iio/imu/bmi160/bmi160_spi.c
> >> index 8b573ea99af2..0874c37c6670 100644
> >> --- a/drivers/iio/imu/bmi160/bmi160_spi.c
> >> +++ b/drivers/iio/imu/bmi160/bmi160_spi.c
> >> @@ -41,6 +41,7 @@ MODULE_DEVICE_TABLE(spi, bmi160_spi_id);
> >>
> >> static const struct acpi_device_id bmi160_acpi_match[] = {
> >> {"BMI0160", 0},
> >> + {"10EC5280", 0},
> >> { },
> >> };
> >> MODULE_DEVICE_TABLE(acpi, bmi160_acpi_match);
>
