diff mbox series

[1/6] platform/x86: dell-smo8800: Only load on Dell laptops

Message ID 20231224213629.395741-2-hdegoede@redhat.com (mailing list archive)
State Superseded, archived
Headers show
Series i2c-i801 / dell-smo8800: Move instantiation of lis3lv02d i2c_client from i2c-i801 to dell-smo8800 | expand

Commit Message

Hans de Goede Dec. 24, 2023, 9:36 p.m. UTC
The SMO8xxx ACPI HIDs are generic accelerometer ids which are also used
by other vendors. Add a sys_vendor check to ensure that the dell-smo8800
driver only loads on Dell laptops.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/dell/dell-smo8800.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Pali Rohár Dec. 24, 2023, 9:48 p.m. UTC | #1
On Sunday 24 December 2023 22:36:17 Hans de Goede wrote:
> The SMO8xxx ACPI HIDs are generic accelerometer ids which are also used
> by other vendors. Add a sys_vendor check to ensure that the dell-smo8800
> driver only loads on Dell laptops.

I saw that this driver was used also on some Acer laptops. As you wrote
above, and now after years I have also feeling that these ACPI HIDs are
generic, not Dell specific.

So I would propose to not restrict smo8800 driver just for Dell laptops
like this patch is introducing.

Maybe better option would be to move this driver out of the dell
namespace.

Are we able to collect dmesg outputs and verify this information on how
many non-dell laptops is this driver loaded?

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/platform/x86/dell/dell-smo8800.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/platform/x86/dell/dell-smo8800.c b/drivers/platform/x86/dell/dell-smo8800.c
> index f7ec17c56833..4d5f778fb599 100644
> --- a/drivers/platform/x86/dell/dell-smo8800.c
> +++ b/drivers/platform/x86/dell/dell-smo8800.c
> @@ -10,6 +10,7 @@
>  
>  #define DRIVER_NAME "smo8800"
>  
> +#include <linux/dmi.h>
>  #include <linux/fs.h>
>  #include <linux/interrupt.h>
>  #include <linux/kernel.h>
> @@ -108,6 +109,9 @@ static int smo8800_probe(struct platform_device *device)
>  	int err;
>  	struct smo8800_device *smo8800;
>  
> +	if (!dmi_match(DMI_SYS_VENDOR, "Dell Inc."))
> +		return false;
> +
>  	smo8800 = devm_kzalloc(&device->dev, sizeof(*smo8800), GFP_KERNEL);
>  	if (!smo8800) {
>  		dev_err(&device->dev, "failed to allocate device data\n");
> -- 
> 2.43.0
>
Hans de Goede Jan. 5, 2024, 4:25 p.m. UTC | #2
Hi Pali,

Thank you for your feedback on this series.

On 12/24/23 22:48, Pali Rohár wrote:
> On Sunday 24 December 2023 22:36:17 Hans de Goede wrote:
>> The SMO8xxx ACPI HIDs are generic accelerometer ids which are also used
>> by other vendors. Add a sys_vendor check to ensure that the dell-smo8800
>> driver only loads on Dell laptops.
> 
> I saw that this driver was used also on some Acer laptops. As you wrote
> above, and now after years I have also feeling that these ACPI HIDs are
> generic, not Dell specific.
> 
> So I would propose to not restrict smo8800 driver just for Dell laptops
> like this patch is introducing.

Ok, I was not aware that some Acer laptops were also using this.

I was just trying to honor the vendor check done in
the i2c-i801 driver after the move of the i2c-client instantiation
to dell-smo8800.

Given that we are keeping the existing DMI matching I don't 
think keeping the sys-vendor check is necessary. So I'll
drop the patch from v2 of the series.

Regards,

Hans





> Maybe better option would be to move this driver out of the dell
> namespace.
> 
> Are we able to collect dmesg outputs and verify this information on how
> many non-dell laptops is this driver loaded?
> 
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/platform/x86/dell/dell-smo8800.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/platform/x86/dell/dell-smo8800.c b/drivers/platform/x86/dell/dell-smo8800.c
>> index f7ec17c56833..4d5f778fb599 100644
>> --- a/drivers/platform/x86/dell/dell-smo8800.c
>> +++ b/drivers/platform/x86/dell/dell-smo8800.c
>> @@ -10,6 +10,7 @@
>>  
>>  #define DRIVER_NAME "smo8800"
>>  
>> +#include <linux/dmi.h>
>>  #include <linux/fs.h>
>>  #include <linux/interrupt.h>
>>  #include <linux/kernel.h>
>> @@ -108,6 +109,9 @@ static int smo8800_probe(struct platform_device *device)
>>  	int err;
>>  	struct smo8800_device *smo8800;
>>  
>> +	if (!dmi_match(DMI_SYS_VENDOR, "Dell Inc."))
>> +		return false;
>> +
>>  	smo8800 = devm_kzalloc(&device->dev, sizeof(*smo8800), GFP_KERNEL);
>>  	if (!smo8800) {
>>  		dev_err(&device->dev, "failed to allocate device data\n");
>> -- 
>> 2.43.0
>>
>
diff mbox series

Patch

diff --git a/drivers/platform/x86/dell/dell-smo8800.c b/drivers/platform/x86/dell/dell-smo8800.c
index f7ec17c56833..4d5f778fb599 100644
--- a/drivers/platform/x86/dell/dell-smo8800.c
+++ b/drivers/platform/x86/dell/dell-smo8800.c
@@ -10,6 +10,7 @@ 
 
 #define DRIVER_NAME "smo8800"
 
+#include <linux/dmi.h>
 #include <linux/fs.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
@@ -108,6 +109,9 @@  static int smo8800_probe(struct platform_device *device)
 	int err;
 	struct smo8800_device *smo8800;
 
+	if (!dmi_match(DMI_SYS_VENDOR, "Dell Inc."))
+		return false;
+
 	smo8800 = devm_kzalloc(&device->dev, sizeof(*smo8800), GFP_KERNEL);
 	if (!smo8800) {
 		dev_err(&device->dev, "failed to allocate device data\n");