Hi,

On 5/5/21 4:18 PM, Andy Shevchenko wrote:

On Wed, May 5, 2021 at 5:11 PM Hans de Goede [off-list ref] wrote:

quoted

On 5/5/21 3:53 PM, Andy Shevchenko wrote:

quoted

On Wed, May 5, 2021 at 4:39 PM Hans de Goede [off-list ref] wrote:

quoted

On 5/5/21 3:22 PM, Andy Shevchenko wrote:

quoted

On Wed, May 5, 2021 at 11:36 AM Jonathan Cameron
[off-list ref] wrote:

quoted

On Wed, 5 May 2021 09:32:35 +0100
Jonathan Cameron [off-list ref] wrote:

quoted

On Tue, 4 May 2021 11:00:52 -0700
Guenter Roeck [off-list ref] 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@

quoted

quoted

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.

quoted

quoted

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

Ok I cannot find any reference to those in the DSDT-s which I have,
nor in systemd's hwdb.

Regards,

Hans