[PATCH v1 1/1] iio: light: ltr501: Drop most likely fake ACPI ID

[PATCH v1 1/1] iio: light: ltr501: Drop most likely fake ACPI ID

[Andy Shevchenko]

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@linux.intel.com>
---
 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(&ltr501_pm_ops),
-		.acpi_match_table = ltr_acpi_match,
 	},
 	.probe = ltr501_probe,
 	.remove	= ltr501_remove,
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v1 1/1] iio: light: ltr501: Drop most likely fake ACPI ID

[Hans de Goede]

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@linux.intel.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

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(&ltr501_pm_ops),
> -		.acpi_match_table = ltr_acpi_match,
>  	},
>  	.probe = ltr501_probe,
>  	.remove	= ltr501_remove,

Re: [PATCH v1 1/1] iio: light: ltr501: Drop most likely fake ACPI ID

[Andy Shevchenko]

On Thu, Sep 12, 2024 at 03:51:09PM +0200, 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@linux.intel.com>
> 
> Thanks, patch looks good to me:

Have you grepped over your collection of real DSDTs?

> Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Thank you!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] iio: light: ltr501: Drop most likely fake ACPI ID

[Jonathan Cameron]

On Fri, 13 Sep 2024 12:31:31 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Thu, Sep 12, 2024 at 03:51:09PM +0200, 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@linux.intel.com>  
> > 
> > Thanks, patch looks good to me:  
> 
> Have you grepped over your collection of real DSDTs?
> 
> > Reviewed-by: Hans de Goede <hdegoede@redhat.com>  
> 
> Thank you!
> 
I'll pick these up in the meantime. Applied to the testing
branch of iio.git.

Re: [PATCH v1 1/1] iio: light: ltr501: Drop most likely fake ACPI ID

[Hans de Goede]

Hi,

On 9/14/24 4:25 PM, Jonathan Cameron wrote:
> On Fri, 13 Sep 2024 12:31:31 +0300
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
>> On Thu, Sep 12, 2024 at 03:51:09PM +0200, 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@linux.intel.com>  
>>>
>>> Thanks, patch looks good to me:  
>>
>> Have you grepped over your collection of real DSDTs?
>>
>>> Reviewed-by: Hans de Goede <hdegoede@redhat.com>  
>>
>> Thank you!
>>
> I'll pick these up in the meantime. Applied to the testing
> branch of iio.git.

As mentioned earlier today, at least the LTER0301 ACPI Hardware ID
is real, so please drop this one. The kmx61 patch is fine to keep.

Regards,

Hans

Re: [PATCH v1 1/1] iio: light: ltr501: Drop most likely fake ACPI ID

[Jonathan Cameron]

On Sat, 14 Sep 2024 16:30:00 +0200
Hans de Goede <hdegoede@redhat.com> wrote:

> Hi,
> 
> On 9/14/24 4:25 PM, Jonathan Cameron wrote:
> > On Fri, 13 Sep 2024 12:31:31 +0300
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >   
> >> On Thu, Sep 12, 2024 at 03:51:09PM +0200, 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@linux.intel.com>    
> >>>
> >>> Thanks, patch looks good to me:    
> >>
> >> Have you grepped over your collection of real DSDTs?
> >>  
> >>> Reviewed-by: Hans de Goede <hdegoede@redhat.com>    
> >>
> >> Thank you!
> >>  
> > I'll pick these up in the meantime. Applied to the testing
> > branch of iio.git.  
> 
> As mentioned earlier today, at least the LTER0301 ACPI Hardware ID
> is real, so please drop this one. The kmx61 patch is fine to keep.
Done.

Thanks,

J
> 
> Regards,
> 
> Hans
> 
> 
> 

Re: [PATCH v1 1/1] iio: light: ltr501: Drop most likely fake ACPI ID

[Hans de Goede]

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@linux.intel.com>

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@redhat.com>

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(&ltr501_pm_ops),
>> -		.acpi_match_table = ltr_acpi_match,
>>  	},
>>  	.probe = ltr501_probe,
>>  	.remove	= ltr501_remove,
> 

Re: [PATCH v1 1/1] iio: light: ltr501: Drop most likely fake ACPI ID

[Andy Shevchenko]

On Sat, Sep 14, 2024 at 03:45:58PM +0200, Hans de Goede wrote:
> On 9/12/24 3:51 PM, Hans de Goede wrote:
> > On 9/11/24 11:22 PM, Andy Shevchenko wrote:
> 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.

Very good finding! Bookmarked to check any other ACPI ID case with that as well.

> 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@redhat.com>
> 
> 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.

I'll update the patch with reference to that catalog.

> Note I do not have any hw with a ltr303 light sensor, so
> I cannot test this.

Neither can I. So, let's drop 'LTER05' and add a comment WRT the 0x01.

-- 
With Best Regards,
Andy Shevchenko