[PATCH 1/1] Add 10EC5280 to bmi160_i2c ACPI IDs to allow binding on some devices

[PATCH 1/1] Add 10EC5280 to bmi160_i2c ACPI IDs to allow binding on some devices

[Jesus Gonzalez]

"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.

Signed-off-by: Jesus Gonzalez <jesusmgh@gmail.com>
---
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@mail.gmail.com/

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);
-- 
2.43.0

Re: [PATCH 1/1] Add 10EC5280 to bmi160_i2c ACPI IDs to allow binding on some devices

[Jonathan Cameron]

On Fri,  2 Feb 2024 18:30:41 +0100
Jesus Gonzalez <jesusmgh@gmail.com> 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@mail.gmail.com/
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@gmail.com>
> ---
> 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@mail.gmail.com/
> 
> 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);

Re: [PATCH 1/1] Add 10EC5280 to bmi160_i2c ACPI IDs to allow binding on some devices

[Jesus Miguel Gonzalez Herrero]

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.

     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@gmail.com> 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@mail.gmail.com/
> 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@gmail.com>
>> ---
>> 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@mail.gmail.com/
>>
>> 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);

Re: [PATCH 1/1] Add 10EC5280 to bmi160_i2c ACPI IDs to allow binding on some devices

[Jonathan Cameron]

On Sun, 4 Feb 2024 18:53:42 +0100
Jesus Miguel Gonzalez Herrero <jesusmgh@gmail.com> 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@gmail.com> 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@mail.gmail.com/
> > 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@gmail.com>
> >> ---
> >> 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@mail.gmail.com/
> >>
> >> 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);  
> 

[PATCH 1/1] Add 10EC5280 to bmi160_i2c ACPI IDs to allow binding on some devices

[Jesus Gonzalez]

"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.

Signed-off-by: Jesus Gonzalez <jesusmgh@gmail.com>
---
 drivers/iio/imu/bmi160/bmi160_spi.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/iio/imu/bmi160/bmi160_spi.c b/drivers/iio/imu/bmi160/bmi160_spi.c
index 8b573ea99af2..0df961a129bb 100644
--- a/drivers/iio/imu/bmi160/bmi160_spi.c
+++ b/drivers/iio/imu/bmi160/bmi160_spi.c
@@ -41,6 +41,14 @@ MODULE_DEVICE_TABLE(spi, bmi160_spi_id);
 
 static const struct acpi_device_id bmi160_acpi_match[] = {
 	{"BMI0160", 0},
+	/* FIRMWARE BUG WORKAROUND
+	 * Some manufacturers like GPD, Lenovo or Aya used the incorrect
+	 * ID "10EC5280" for bmi160 in their DSDT. A fixed firmware is not
+	 * available as of Feb 2024 after trying to work with OEMs, and
+	 * this is not expected to change anymore since at least some of
+	 * the affected devices are from 2021/2022.
+	 */
+	{"10EC5280", 0},
 	{ },
 };
 MODULE_DEVICE_TABLE(acpi, bmi160_acpi_match);
-- 
2.43.0

Re: [PATCH 1/1] Add 10EC5280 to bmi160_i2c ACPI IDs to allow binding on some devices

[Jonathan Cameron]

On Sun,  4 Feb 2024 19:05:37 +0100
Jesus Gonzalez <jesusmgh@gmail.com> 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.
> 
> Signed-off-by: Jesus Gonzalez <jesusmgh@gmail.com>
Make sure to include v2 in the patch title.  
git format-patch -v2 ...

+ as per other reply, I'd like to see more info in here including the
DSDT blob.

Thanks,

Jonathan

> ---
>  drivers/iio/imu/bmi160/bmi160_spi.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/iio/imu/bmi160/bmi160_spi.c b/drivers/iio/imu/bmi160/bmi160_spi.c
> index 8b573ea99af2..0df961a129bb 100644
> --- a/drivers/iio/imu/bmi160/bmi160_spi.c
> +++ b/drivers/iio/imu/bmi160/bmi160_spi.c
> @@ -41,6 +41,14 @@ MODULE_DEVICE_TABLE(spi, bmi160_spi_id);
>  
>  static const struct acpi_device_id bmi160_acpi_match[] = {
>  	{"BMI0160", 0},
> +	/* FIRMWARE BUG WORKAROUND
> +	 * Some manufacturers like GPD, Lenovo or Aya used the incorrect
> +	 * ID "10EC5280" for bmi160 in their DSDT. A fixed firmware is not
> +	 * available as of Feb 2024 after trying to work with OEMs, and
> +	 * this is not expected to change anymore since at least some of
> +	 * the affected devices are from 2021/2022.
> +	 */
> +	{"10EC5280", 0},
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(acpi, bmi160_acpi_match);

Re: [PATCH 1/1] Add 10EC5280 to bmi160_i2c ACPI IDs to allow binding on some devices

[Andy Shevchenko]

On Sun, Feb 04, 2024 at 02:00:10PM +0000, Jonathan Cameron wrote:
> On Fri,  2 Feb 2024 18:30:41 +0100
> Jesus Gonzalez <jesusmgh@gmail.com> wrote:

...

> >  static const struct acpi_device_id bmi160_acpi_match[] = {
> >  	{"BMI0160", 0},
> > +	{"10EC5280", 0},
> >  	{ },
> >  };

Keep it ordered (digits are going before letters).


-- 
With Best Regards,
Andy Shevchenko