| Message ID | 20231224213629.395741-2-hdegoede@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | i2c-i801 / dell-smo8800: Move instantiation of lis3lv02d i2c_client from i2c-i801 to dell-smo8800 | expand |
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 >
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 --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");
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(+)