From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: Hans de Goede <hdegoede@redhat.com>
Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>,
"Rafael J. Wysocki" <rjw@rjwysocki.net>,
ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
Paul Menzel <paulepanter@users.sourceforge.net>,
Jacek Anaszewski <jacek.anaszewski@gmail.com>,
Pavel Machek <pavel@denx.de>, Guenter Roeck <linux@roeck-us.net>,
Jonathan Cameron <jic23@kernel.org>,
linux-iio <linux-iio@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
kernel test robot <lkp@intel.com>
Subject: Re: [PATCH] iio: bme680_i2c: Make bme680_acpi_match depend on CONFIG_ACPI
Date: Wed, 5 May 2021 17:18:57 +0300 [thread overview]
Message-ID: <CAHp75VciMKfxPvKmY349327FcoUcUMeFnvqkniw2erCyb71BoQ@mail.gmail.com> (raw)
In-Reply-To: <ede732cb-4a23-e5bc-6802-0280dc232876@redhat.com>
On Wed, May 5, 2021 at 5:11 PM Hans de Goede <hdegoede@redhat.com> wrote:
> On 5/5/21 3:53 PM, Andy Shevchenko wrote:
> > On Wed, May 5, 2021 at 4:39 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >> On 5/5/21 3:22 PM, Andy Shevchenko wrote:
> >>> On Wed, May 5, 2021 at 11:36 AM Jonathan Cameron
> >>> <Jonathan.Cameron@huawei.com> wrote:
> >>>> On Wed, 5 May 2021 09:32:35 +0100
> >>>> Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:
> >>>>> On Tue, 4 May 2021 11:00:52 -0700
> >>>>> Guenter Roeck <linux@roeck-us.net> wrote:
> >>>
> >>> +Cc: Paul (I hope you are related to coreboot somehow and can
> >>> communicate this further), Pavel and Jacek (LED subsystem suffered
> >>> with this as well), Hans, Rafael and linux-acpi@
> >>>
> >>>>> Dropping the ones we are fairly sure are spurious is even better!
> >>>>
> >>>> If I get bored I'll just do a scrub of all the instances of this that
> >>>> you haven't already cleaned up. It's worth noting that we do
> >>>> know some highly suspicious looking entries are out there in the wild.
> >>>
> >>> I have counted ~60 users of acpi_device_id in IIO. Brief looking at
> >>> the IDs themselves rings an alarm about half of them.
> >>>
> >>> So, here we may have a chicken and egg problem, i.e. somebody has been
> >>> using (or used) fake IDs from Linux kernel in the real products. What
> >>> I can consider as a course of action is the following:
> >>> 1. Clean up (by removing as quickly as possible) the IDs that have no
> >>> proof to be real from the Linux kernel sources (perhaps marked as
> >>> stable material)
> >>> 2. Notify ASWG / UEFI forum about all IDs that abuse ACPI
> >>> specification and are in Linux kernel, so at least we can keep some
> >>> kind of "reserved/do not use" list on the official level (Rafael?)
> >>> 3. Do not accept any IDs without an evidence provided that they are
> >>> being in use in the real products (this should be done on Linux
> >>> maintainer level in all subsystems that accept drivers
> >>
> >> So my 2 cents on this are that we need to be very careful with
> >> removing "bogus" ACPI-ids.
> >>
> >> A couple of examples from a quick check under drivers/iio/accel:
> >>
> >> drivers/iio/accel/bmc150-accel-i2c.c:
> >>
> >> static const struct i2c_device_id bmc150_accel_id[] = {
> >> {"bmc150_accel", bmc150},
> >> {"bmi055_accel", bmi055},
> >> {"bma255", bma255},
> >> {"bma250e", bma250e},
> >> {"bma222", bma222},
> >> {"bma222e", bma222e},
> >> {"bma280", bma280},
> >> {}
> >> };
> >>
> >> static const struct acpi_device_id bmc150_accel_acpi_match[] = {
> >> {"BSBA0150", bmc150},
> >> {"BMC150A", bmc150},
> >> {"BMI055A", bmi055},
> >> {"BMA0255", bma255},
> >> {"BMA250E", bma250e},
> >> {"BMA222", bma222},
> >> {"BMA222E", bma222e},
> >> {"BMA0280", bma280},
> >> {"BOSC0200"},
> >> { },
> >> };
> >>
> >> With the exception of the "BSBA0150" and "BOSC0200"
> >> ids, these look like they were invented. But at least the
> >> "BMA250E" one is actually being used! The other BMA###?
> >> ones are probably fake, but given that the "BMA250E"
> >> one is actually real ...
> >>
> >> drivers/iio/accel/bmc150-accel-spi.c
> >>
> >> This uses the same set of ACPI ids as bmc150-accel-i2c.c
> >> minus the "BOSC0200" one. I'm not aware if these
> >> being used in spi mode on any x86 devices, but again
> >> I'm not 100% sure ...
> >>
> >> drivers/iio/accel/da280.c
> >>
> >> static const struct acpi_device_id da280_acpi_match[] = {
> >> {"MIRAACC", da280},
> >> {},
> >> };
> >> MODULE_DEVICE_TABLE(acpi, da280_acpi_match);
> >>
> >> This looks like a fake-id, but it was actually added
> >> in a separate commit adding ACPI support because the
> >> chip is used with this id on a Linx 820 Windows tablet.
> >>
> >> So figuring out of any ids are real or not is really tricky
> >> and removing them if they are real will lead to regressions.
> >>
> >> So summarizing IMHO we need to be careful and not just
> >> start removing a whole bunch of these...
> >
> > That's all true. However, I have a few hints on how to distinguish
> > them (fake ones):
> > 1. The ID has been added from day 1 with I2C or SPI ID table with just
> > capitalized name
> > 2. If there are a few drivers by the same author and at least one of
> > the contributions has confirmed fake ID
> > 3. The ID is single in the list and mimics the part number (capitalized form)
> > 4. Google/DuckDuckGo/etc searches give no meaningful results
> >
> > Either combination of the above can be a good hint to at least be
> > sceptical that it's being used
> May I suggest for accelerometers to also grep for the id in
> 60-sensors.hwdb from systemd ? E.g. the BMA250E id can be found
> there.
Right, it's a very good suggestion! It will definitely tell us what
may not be removed even if we don't see any evidence otherwise.
> > So, Hans, as you already noticed, drivers with a long list of IDs or
> > when ID added separately can be considered less fakish, but we really
> > want evidence of the hardware that has it.
>
> If you want to move ahead with pruning some of these please Cc me
> on the patches, then I'll check them against my collection of
> Bay and Cherry Trail DSDTs, which are devices where these sensors
> are often found.
Currently the scope is of
AOS2315
BME0680
STK8312
--
With Best Regards,
Andy Shevchenko
next prev parent reply other threads:[~2021-05-05 14:19 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-04 17:40 [PATCH] iio: bme680_i2c: Make bme680_acpi_match depend on CONFIG_ACPI Guenter Roeck
2021-05-04 17:44 ` Andy Shevchenko
2021-05-04 17:46 ` Andy Shevchenko
2021-05-04 18:00 ` Guenter Roeck
2021-05-05 8:32 ` Jonathan Cameron
2021-05-05 8:34 ` Jonathan Cameron
2021-05-05 13:00 ` Guenter Roeck
2021-05-05 13:22 ` Andy Shevchenko
2021-05-05 13:39 ` Hans de Goede
2021-05-05 13:53 ` Andy Shevchenko
2021-05-05 14:04 ` Hans de Goede
2021-05-05 14:18 ` Andy Shevchenko [this message]
2021-05-05 15:19 ` Hans de Goede
2021-05-05 16:26 ` Andy Shevchenko
2021-05-05 16:30 ` Hans de Goede
2021-05-07 11:53 ` Pavel Machek
2021-05-07 12:39 ` Andy Shevchenko
2021-05-05 9:04 ` Andy Shevchenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAHp75VciMKfxPvKmY349327FcoUcUMeFnvqkniw2erCyb71BoQ@mail.gmail.com \
--to=andy.shevchenko@gmail.com \
--cc=Jonathan.Cameron@huawei.com \
--cc=hdegoede@redhat.com \
--cc=jacek.anaszewski@gmail.com \
--cc=jic23@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=lkp@intel.com \
--cc=paulepanter@users.sourceforge.net \
--cc=pavel@denx.de \
--cc=rjw@rjwysocki.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).