[PATCH v1 1/1] iio: gyro: bmg160: Drop most likely fake ACPI IDs

[PATCH v1 1/1] iio: gyro: bmg160: Drop most likely fake ACPI IDs

[Andy Shevchenko]

The commits in question do not proove that ACPI IDs exist.
Quite likely it was a cargo cult addition while doing that
for DT-based enumeration. Drop most likely fake ACPI IDs.

The to be removed IDs has been checked against the following resources:
1) DuckDuckGo
2) Google
3) MS catalog: https://www.catalog.update.microsoft.com/Search.aspx
This gives no useful results in regard to DSDT, moreover, the official
vendor IDs in the registry for Bosh are BSG and BOSC.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/gyro/bmg160_i2c.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/iio/gyro/bmg160_i2c.c b/drivers/iio/gyro/bmg160_i2c.c
index 672d0b720f61..a81814df5205 100644
--- a/drivers/iio/gyro/bmg160_i2c.c
+++ b/drivers/iio/gyro/bmg160_i2c.c
@@ -39,8 +39,6 @@ static void bmg160_i2c_remove(struct i2c_client *client)
 
 static const struct acpi_device_id bmg160_acpi_match[] = {
 	{"BMG0160", 0},
-	{"BMI055B", 0},
-	{"BMI088B", 0},
 	{},
 };
 
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v1 1/1] iio: gyro: bmg160: Drop most likely fake ACPI IDs

[Jonathan Cameron]

On Fri, 18 Oct 2024 17:57:32 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> The commits in question do not proove that ACPI IDs exist.
> Quite likely it was a cargo cult addition while doing that
> for DT-based enumeration. Drop most likely fake ACPI IDs.
> 
> The to be removed IDs has been checked against the following resources:
> 1) DuckDuckGo
> 2) Google
> 3) MS catalog: https://www.catalog.update.microsoft.com/Search.aspx
> This gives no useful results in regard to DSDT, moreover, the official
> vendor IDs in the registry for Bosh are BSG and BOSC.
> 

I'm nervous about Bosch drivers in the wild given recent report
from Justin
https://lore.kernel.org/linux-iio/87jzeboi3g.fsf@justinweiss.com/

Justin, I couldn't find the driver you were referring to for the bmi160,
is it online somewhere?

Also if you have other bosch drivers could you check for these + bmc150
IDs Andy is proposing dropping in:

https://lore.kernel.org/linux-iio/20241018145805.2181682-1-andriy.shevchenko@linux.intel.com/


> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/iio/gyro/bmg160_i2c.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/iio/gyro/bmg160_i2c.c b/drivers/iio/gyro/bmg160_i2c.c
> index 672d0b720f61..a81814df5205 100644
> --- a/drivers/iio/gyro/bmg160_i2c.c
> +++ b/drivers/iio/gyro/bmg160_i2c.c
> @@ -39,8 +39,6 @@ static void bmg160_i2c_remove(struct i2c_client *client)
>  
>  static const struct acpi_device_id bmg160_acpi_match[] = {
>  	{"BMG0160", 0},
> -	{"BMI055B", 0},
> -	{"BMI088B", 0},
>  	{},
>  };
>  

Re: [PATCH v1 1/1] iio: gyro: bmg160: Drop most likely fake ACPI IDs

[Justin Weiss]

Jonathan Cameron <jic23@kernel.org> writes:

> On Fri, 18 Oct 2024 17:57:32 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>
>> The commits in question do not proove that ACPI IDs exist.
>> Quite likely it was a cargo cult addition while doing that
>> for DT-based enumeration. Drop most likely fake ACPI IDs.
>> 
>> The to be removed IDs has been checked against the following resources:
>> 1) DuckDuckGo
>> 2) Google
>> 3) MS catalog: https://www.catalog.update.microsoft.com/Search.aspx
>> This gives no useful results in regard to DSDT, moreover, the official
>> vendor IDs in the registry for Bosh are BSG and BOSC.
>> 
>
> I'm nervous about Bosch drivers in the wild given recent report
> from Justin
> https://lore.kernel.org/linux-iio/87jzeboi3g.fsf@justinweiss.com/
>
> Justin, I couldn't find the driver you were referring to for the bmi160,
> is it online somewhere?

Yes, the BMI160/260 Windows driver is on this page:
https://ayaneo.com/support/download

It's at the very bottom, under "AYANEO Universal Gyro Driver." GPD also
has a copy inside their driver pack under the "Drivers & BIOS" tab here:
https://www.gpd.hk/gpdwinminifirmwaredriver, but the download is often
at capacity unless you're signed into Google.

> Also if you have other bosch drivers could you check for these + bmc150
> IDs Andy is proposing dropping in:
>
> https://lore.kernel.org/linux-iio/20241018145805.2181682-1-andriy.shevchenko@linux.intel.com/

Unfortunately, the 160/260 driver is the only one I have. I wanted gyro
support in Linux for a handheld PC I bought, and it was the IMU that
happened to be inside.

Justin

>
>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> ---
>>  drivers/iio/gyro/bmg160_i2c.c | 2 --
>>  1 file changed, 2 deletions(-)
>> 
>> diff --git a/drivers/iio/gyro/bmg160_i2c.c b/drivers/iio/gyro/bmg160_i2c.c
>> index 672d0b720f61..a81814df5205 100644
>> --- a/drivers/iio/gyro/bmg160_i2c.c
>> +++ b/drivers/iio/gyro/bmg160_i2c.c
>> @@ -39,8 +39,6 @@ static void bmg160_i2c_remove(struct i2c_client *client)
>>  
>>  static const struct acpi_device_id bmg160_acpi_match[] = {
>>  	{"BMG0160", 0},
>> -	{"BMI055B", 0},
>> -	{"BMI088B", 0},
>>  	{},
>>  };
>>  

Re: [PATCH v1 1/1] iio: gyro: bmg160: Drop most likely fake ACPI IDs

[Andy Shevchenko]

On Fri, Oct 18, 2024 at 01:27:57PM -0700, Justin Weiss wrote:
> Jonathan Cameron <jic23@kernel.org> writes:
> 
> > On Fri, 18 Oct 2024 17:57:32 +0300
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >
> >> The commits in question do not proove that ACPI IDs exist.
> >> Quite likely it was a cargo cult addition while doing that
> >> for DT-based enumeration. Drop most likely fake ACPI IDs.
> >> 
> >> The to be removed IDs has been checked against the following resources:
> >> 1) DuckDuckGo
> >> 2) Google
> >> 3) MS catalog: https://www.catalog.update.microsoft.com/Search.aspx
> >> This gives no useful results in regard to DSDT, moreover, the official
> >> vendor IDs in the registry for Bosh are BSG and BOSC.
> >> 
> >
> > I'm nervous about Bosch drivers in the wild given recent report
> > from Justin
> > https://lore.kernel.org/linux-iio/87jzeboi3g.fsf@justinweiss.com/

I know about Realtek case, but for BMI0/BMI it's interesting that MS
catalog shows hits on BMI160 (note no leading zero!), but not on BMI0160.

> > Justin, I couldn't find the driver you were referring to for the bmi160,
> > is it online somewhere?
> 
> Yes, the BMI160/260 Windows driver is on this page:
> https://ayaneo.com/support/download
> 
> It's at the very bottom, under "AYANEO Universal Gyro Driver." GPD also
> has a copy inside their driver pack under the "Drivers & BIOS" tab here:
> https://www.gpd.hk/gpdwinminifirmwaredriver, but the download is often
>>  at capacity unless you're signed into Google.

I was not Cc'ed on ACPI ID parts, please make sure that the patch
that adds it has a pointer to the actual device which uses it _and_
DSDT excerpt of the respective Device object. Without that info provided,
NAK from me.

> > Also if you have other bosch drivers could you check for these + bmc150
> > IDs Andy is proposing dropping in:
> >
> > https://lore.kernel.org/linux-iio/20241018145805.2181682-1-andriy.shevchenko@linux.intel.com/
> 
> Unfortunately, the 160/260 driver is the only one I have. I wanted gyro
> support in Linux for a handheld PC I bought, and it was the IMU that
> happened to be inside.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] iio: gyro: bmg160: Drop most likely fake ACPI IDs

[Jonathan Cameron]

On Mon, 21 Oct 2024 09:47:16 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Fri, Oct 18, 2024 at 01:27:57PM -0700, Justin Weiss wrote:
> > Jonathan Cameron <jic23@kernel.org> writes:
> >   
> > > On Fri, 18 Oct 2024 17:57:32 +0300
> > > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > >  
> > >> The commits in question do not proove that ACPI IDs exist.
> > >> Quite likely it was a cargo cult addition while doing that
> > >> for DT-based enumeration. Drop most likely fake ACPI IDs.
> > >> 
> > >> The to be removed IDs has been checked against the following resources:
> > >> 1) DuckDuckGo
> > >> 2) Google
> > >> 3) MS catalog: https://www.catalog.update.microsoft.com/Search.aspx
> > >> This gives no useful results in regard to DSDT, moreover, the official
> > >> vendor IDs in the registry for Bosh are BSG and BOSC.
> > >>   
> > >
> > > I'm nervous about Bosch drivers in the wild given recent report
> > > from Justin
> > > https://lore.kernel.org/linux-iio/87jzeboi3g.fsf@justinweiss.com/  
> 
> I know about Realtek case, but for BMI0/BMI it's interesting that MS
> catalog shows hits on BMI160 (note no leading zero!), but not on BMI0160.
> 
> > > Justin, I couldn't find the driver you were referring to for the bmi160,
> > > is it online somewhere?  
> > 
> > Yes, the BMI160/260 Windows driver is on this page:
> > https://ayaneo.com/support/download
> > 
> > It's at the very bottom, under "AYANEO Universal Gyro Driver." GPD also
> > has a copy inside their driver pack under the "Drivers & BIOS" tab here:
> > https://www.gpd.hk/gpdwinminifirmwaredriver, but the download is often  
> >>  at capacity unless you're signed into Google.  
> 
> I was not Cc'ed on ACPI ID parts, please make sure that the patch
> that adds it has a pointer to the actual device which uses it _and_
> DSDT excerpt of the respective Device object. Without that info provided,
> NAK from me.
Device we have in the patch comments. Justin if you can supply the DSDT
excerpts in reply to that patch that would be great.

I'll try and remember to check you are on the CC next time Andy.

Jonathan
> 
> > > Also if you have other bosch drivers could you check for these + bmc150
> > > IDs Andy is proposing dropping in:
> > >
> > > https://lore.kernel.org/linux-iio/20241018145805.2181682-1-andriy.shevchenko@linux.intel.com/  
> > 
> > Unfortunately, the 160/260 driver is the only one I have. I wanted gyro
> > support in Linux for a handheld PC I bought, and it was the IMU that
> > happened to be inside.  
> 

Re: [PATCH v1 1/1] iio: gyro: bmg160: Drop most likely fake ACPI IDs

[Hans de Goede]

Hi,

On 18-Oct-24 4:57 PM, Andy Shevchenko wrote:
> The commits in question do not proove that ACPI IDs exist.
> Quite likely it was a cargo cult addition while doing that
> for DT-based enumeration. Drop most likely fake ACPI IDs.
> 
> The to be removed IDs has been checked against the following resources:
> 1) DuckDuckGo
> 2) Google
> 3) MS catalog: https://www.catalog.update.microsoft.com/Search.aspx
> This gives no useful results in regard to DSDT, moreover, the official
> vendor IDs in the registry for Bosh are BSG and BOSC.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/iio/gyro/bmg160_i2c.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/iio/gyro/bmg160_i2c.c b/drivers/iio/gyro/bmg160_i2c.c
> index 672d0b720f61..a81814df5205 100644
> --- a/drivers/iio/gyro/bmg160_i2c.c
> +++ b/drivers/iio/gyro/bmg160_i2c.c
> @@ -39,8 +39,6 @@ static void bmg160_i2c_remove(struct i2c_client *client)
>  
>  static const struct acpi_device_id bmg160_acpi_match[] = {
>  	{"BMG0160", 0},
> -	{"BMI055B", 0},
> -	{"BMI088B", 0},
>  	{},
>  };
>  

Doing a grep on my acpidump collection shows that the BMI prefix is used
for some Bosch IMU-s. It seems that some of the Bosch ACPI HID
prefixes are like this:

Bosch Measurement Accel -> BMAxxxx
Bosch Measurement Gyro  -> BMGxxxx
Bosch Measurement IMU   -> BMIxxxx

In itself these 3 non official vendor prefixis seem to be quite
wildly used and such are not an indication that a HID is not in use.

But using BMI which is for IMU-s in a non IMU driver is suspect.
Note that has Jonathan has pointed out there is a valid BMI0160
HID used by the drivers/iio/imu/bmi160/ code.

I also notice a "BMI055A" HID in the bmc150-accel-[i2c|spi].c drivers,
which seems equally wrong.

It seems that if anything there should be a "BMI055" (no suffix) HID
for drivers/iio/imu/bno055/bno055_i2c.c, but we should only add that
if we actually encounter it in the wild.

TL;DR:

I agree with the removal of the "BMI055B" and "BMI088B" ACPI HIDs
from this driver, because if these are valid for anything they
are valid for the bno055 + some unknown 088 IMU drivers and not
for a gyro driver.

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans

Re: [PATCH v1 1/1] iio: gyro: bmg160: Drop most likely fake ACPI IDs

[Jonathan Cameron]

On Sat, 19 Oct 2024 12:33:55 +0200
Hans de Goede <hdegoede@redhat.com> wrote:

> Hi,
> 
> On 18-Oct-24 4:57 PM, Andy Shevchenko wrote:
> > The commits in question do not proove that ACPI IDs exist.
> > Quite likely it was a cargo cult addition while doing that
> > for DT-based enumeration. Drop most likely fake ACPI IDs.
> > 
> > The to be removed IDs has been checked against the following resources:
> > 1) DuckDuckGo
> > 2) Google
> > 3) MS catalog: https://www.catalog.update.microsoft.com/Search.aspx
> > This gives no useful results in regard to DSDT, moreover, the official
> > vendor IDs in the registry for Bosh are BSG and BOSC.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >  drivers/iio/gyro/bmg160_i2c.c | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/drivers/iio/gyro/bmg160_i2c.c b/drivers/iio/gyro/bmg160_i2c.c
> > index 672d0b720f61..a81814df5205 100644
> > --- a/drivers/iio/gyro/bmg160_i2c.c
> > +++ b/drivers/iio/gyro/bmg160_i2c.c
> > @@ -39,8 +39,6 @@ static void bmg160_i2c_remove(struct i2c_client *client)
> >  
> >  static const struct acpi_device_id bmg160_acpi_match[] = {
> >  	{"BMG0160", 0},
> > -	{"BMI055B", 0},
> > -	{"BMI088B", 0},
> >  	{},
> >  };
> >    
> 
> Doing a grep on my acpidump collection shows that the BMI prefix is used
> for some Bosch IMU-s. It seems that some of the Bosch ACPI HID
> prefixes are like this:
> 
> Bosch Measurement Accel -> BMAxxxx
> Bosch Measurement Gyro  -> BMGxxxx
> Bosch Measurement IMU   -> BMIxxxx
> 
> In itself these 3 non official vendor prefixis seem to be quite
> wildly used and such are not an indication that a HID is not in use.
> 
> But using BMI which is for IMU-s in a non IMU driver is suspect.
> Note that has Jonathan has pointed out there is a valid BMI0160
> HID used by the drivers/iio/imu/bmi160/ code.
> 
> I also notice a "BMI055A" HID in the bmc150-accel-[i2c|spi].c drivers,
> which seems equally wrong.
> 
> It seems that if anything there should be a "BMI055" (no suffix) HID
> for drivers/iio/imu/bno055/bno055_i2c.c, but we should only add that
> if we actually encounter it in the wild.
> 
> TL;DR:
> 
> I agree with the removal of the "BMI055B" and "BMI088B" ACPI HIDs
> from this driver, because if these are valid for anything they
> are valid for the bno055 + some unknown 088 IMU drivers and not
> for a gyro driver.

Not so simple.  The BMI055 is a multi sensor package. So two
MEMS devices shoved in the same chip package. Bus is shared, but separate
chip selects and entirely unrelated register sets. We handle these
in DT via _gyro / _accel postfixes on the compatible.

These used to be common in IMUs though it seems fully integrated
ones have replaced them.

https://lore.kernel.org/all/1409655597-28781-1-git-send-email-irina.tirdea@intel.com/

The A/B thing makes me particularly suspicious as that's a novel
invention for something not in the wild.

Still, if we have no evidence this one is actually used (and it's an
ancient device so I doubt we'll get new users!) I'm not against
removing the IDs and see if anyone shouts.  On that basis I'll queue
this up.

Jonathan


> 
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> 
> Regards,
> 
> Hans
> 
>