Hi Andy, On 9/12/24 3:51 PM, Hans de Goede wrote: > Hi, > > On 9/11/24 11:22 PM, Andy Shevchenko wrote: >> The commit in question does not proove that ACPI ID exists. >> Quite likely it was a cargo cult addition while doint that >> for DT-based enumeration. Drop most likely fake ACPI ID. >> >> Googling for LTERxxxx gives no useful results in regard to DSDT. >> Moreover, there is no "LTER" official vendor ID in the registry. >> >> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> On 9/13/24 11:31 AM, Andy Shevchenko wrote: > Have you grepped over your collection of real DSDTs? Yes I did, but I just double-checked looking for only LTER and there are several DSDTs using LTER0303 for an ambient light sensor. duckduckgo-ing for LTER0303 finds: https://www.catalog.update.microsoft.com/Search.aspx?q=lter0303 which is actually quite an interesting URL to search for ACPI HID-s used in any Windows drivers. Checking for LTER0301: https://www.catalog.update.microsoft.com/Search.aspx?q=lter0301 Shows that that HID is also actually used, so: > Thanks, patch looks good to me: > > Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx> Correction, at least the LTER0301 ACPI id seems to actually be real: https://www.catalog.update.microsoft.com/Search.aspx?q=lter0301 So NACK for dropping all 3 HIDs. It seems to me that the LTER05xx HIDs can be dropped and a LTER0303 HID should be added instead of dropping all HIDs. Note I do not have any hw with a ltr303 light sensor, so I cannot test this. Regards, Hans >> --- >> drivers/iio/light/ltr501.c | 23 ----------------------- >> 1 file changed, 23 deletions(-) >> >> diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c >> index 8c516ede9116..0e0420573286 100644 >> --- a/drivers/iio/light/ltr501.c >> +++ b/drivers/iio/light/ltr501.c >> @@ -15,7 +15,6 @@ >> #include <linux/err.h> >> #include <linux/delay.h> >> #include <linux/regmap.h> >> -#include <linux/acpi.h> >> #include <linux/regulator/consumer.h> >> >> #include <linux/iio/iio.h> >> @@ -1422,17 +1421,6 @@ static int ltr501_powerdown(struct ltr501_data *data) >> data->ps_contr & ~LTR501_CONTR_ACTIVE); >> } >> >> -static const char *ltr501_match_acpi_device(struct device *dev, int *chip_idx) >> -{ >> - const struct acpi_device_id *id; >> - >> - id = acpi_match_device(dev->driver->acpi_match_table, dev); >> - if (!id) >> - return NULL; >> - *chip_idx = id->driver_data; >> - return dev_name(dev); >> -} >> - >> static int ltr501_probe(struct i2c_client *client) >> { >> const struct i2c_device_id *id = i2c_client_get_device_id(client); >> @@ -1523,8 +1511,6 @@ static int ltr501_probe(struct i2c_client *client) >> if (id) { >> name = id->name; >> chip_idx = id->driver_data; >> - } else if (ACPI_HANDLE(&client->dev)) { >> - name = ltr501_match_acpi_device(&client->dev, &chip_idx); >> } else { >> return -ENODEV; >> } >> @@ -1609,14 +1595,6 @@ static int ltr501_resume(struct device *dev) >> >> static DEFINE_SIMPLE_DEV_PM_OPS(ltr501_pm_ops, ltr501_suspend, ltr501_resume); >> >> -static const struct acpi_device_id ltr_acpi_match[] = { >> - { "LTER0501", ltr501 }, >> - { "LTER0559", ltr559 }, >> - { "LTER0301", ltr301 }, >> - { }, >> -}; >> -MODULE_DEVICE_TABLE(acpi, ltr_acpi_match); >> - >> static const struct i2c_device_id ltr501_id[] = { >> { "ltr501", ltr501 }, >> { "ltr559", ltr559 }, >> @@ -1640,7 +1618,6 @@ static struct i2c_driver ltr501_driver = { >> .name = LTR501_DRV_NAME, >> .of_match_table = ltr501_of_match, >> .pm = pm_sleep_ptr(<r501_pm_ops), >> - .acpi_match_table = ltr_acpi_match, >> }, >> .probe = ltr501_probe, >> .remove = ltr501_remove, >