On Thu, Jun 10, 2021 at 12:56 PM Stephan Gerhold <stephan@xxxxxxxxxxx> wrote:
>
> Commit 0ad4bf370176 ("iio:accel:bmc150-accel: Use the chip ID to detect
> sensor variant") stopped using the I2C/ACPI match data to look up the
> bmc150_accel_chip_info. However, the bmc150_accel_chip_info_tbl remained
> as-is, with multiple entries with the same chip_id (e.g. 0xFA for
> BMC150/BMI055/BMA255). This is redundant now because actually the driver
> will always select the first entry with a matching chip_id.
>
> So even if a device probes e.g. with BMA0255 it will end up using the
> chip_info for BMC150. And in general that's fine for now, the entries
> for BMC150/BMI055/BMA255 were exactly the same anyway (except for the
> name, which is replaced with the more accurate one later).
>
> But in this case it's misleading because it suggests that one should
> add even more entries with the same chip_id when adding support for
> new variants. Let's make that more clear by removing the enum with
> the chip identifiers entirely and instead have only one entry per
> chip_id.
>
> Note that we may need to bring back some mechanism to differentiate
> between different chips with the same chip_id in the future.
> For example, BMA250 (currently supported by the bma180 driver) has the
> same chip_id = 0x03 as BMA222 even though they have different channel
> sizes (8 bits vs 10 bits). But in any case, that mechanism would
> need to look quite different from what we have right now.
...
> static const struct bmc150_accel_chip_info bmc150_accel_chip_info_tbl[] = {
Perhaps sort this by chip_id value?
...
> 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},
> + {"BSBA0150"},
> + {"BMC150A"},
> + {"BMI055A"},
> + {"BMA0255"},
> + {"BMA250E"},
> + {"BMA222"},
> + {"BMA222E"},
> + {"BMA0280"},
> {"BOSC0200"},
> {"DUAL250E"},
I have noticed during review patch 3 that the arrays are unsorted, can
we at the same time sort them by ID, please?
...
> static const struct i2c_device_id bmc150_accel_id[] = {
Ditto.
> {}
> };
...
> static const struct acpi_device_id bmc150_accel_acpi_match[] = {
Ditto.
> { },
> };
...
> static const struct spi_device_id bmc150_accel_id[] = {
Ditto.
> {}
> };
--
With Best Regards,
Andy Shevchenko
