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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241019121214.262af972@jic23-huawei>
Date: Sat, 19 Oct 2024 12:12:14 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Hans de Goede <hdegoede@...hat.com>
Cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
 linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org, Lars-Peter Clausen
 <lars@...afoo.de>
Subject: Re: [PATCH v1 1/1] iio: gyro: bmg160: Drop most likely fake ACPI
 IDs

On Sat, 19 Oct 2024 12:33:55 +0200
Hans de Goede <hdegoede@...hat.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@...ux.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@...hat.com>
> 
> Regards,
> 
> Hans
> 
>