All of lore.kernel.org
 help / color / mirror / Atom feed
* [ PATCH: 1/1]  dell smbios driver : Consider Alienware a valid OEM String
@ 2020-10-03 11:52 Gerardo Esteban Malazdrewicz
  2020-10-03 13:00 ` Hans de Goede
  0 siblings, 1 reply; 17+ messages in thread
From: Gerardo Esteban Malazdrewicz @ 2020-10-03 11:52 UTC (permalink / raw)
  To: Pali Rohár, Mario Limonciello; +Cc: platform-driver-x86

From bda6b6db0d76186ff37ffce8ac836379447ef2bc Mon Sep 17 00:00:00 2001
From: Gerardo Malazdrewicz <36243997+GerMalaz@users.noreply.github.com>
Date: Sat, 3 Oct 2020 07:43:02 -0300
Subject: [PATCH] dell-smbios-base: Consider Alienware a Dell system

Alienware has been a subsidiary of Dell since 2006.

2020 Alienware laptop:
$ sudo dmidecode | head -3
# dmidecode 3.2
Getting SMBIOS data from sysfs.
SMBIOS 3.2.0 present.
$ sudo dmidecode | grep -A 29 "OEM Strings"
OEM Strings
	String 1: Alienware
	String 2: 1[099B]
	String 3: 3[1.0]
	String 4: 4[0001]
	String 5: 5[0000]
	String 6: 6[D0, D4, D8, DA, DE]
	String 7: 7[]
	String 8: 8[]
	String 9: 9[]
	String 10: 10[1.3.0]
	String 11: 11[]
	String 12: 12[]
	String 13: 13[P38E002]
	String 14: 14[0]
	String 15: 15[0]
	String 16: 16[0]
	String 17: 17[0000000000000000]
	String 18: 18[0]
	String 19: 19[1]
	String 20: 20[]
	String 21: 21[]
	String 22: <BAD INDEX>
	String 23: <BAD INDEX>
	String 24: <BAD INDEX>
	String 25: <BAD INDEX>
	String 26: <BAD INDEX>
	String 27: <BAD INDEX>
	String 28: <BAD INDEX>

2013 Alienware laptop:
OEM Strings
        String 1: Dell System
        String 2: 1[05AA]
        String 3: 14[2]
        String 4: 15[0]
        String 5: String5 for Original Equipment Manufacturer

Don't know when the OEM String changed.
Change tested in the 2020 laptop, loads dell_smbios without further
issues.

Thanks,
        Gerardo

Signed-off-by: Gerardo E. Malazdrewicz <gerardo@malazdrewicz.com.ar>
---
 drivers/platform/x86/dell-smbios-base.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/dell-smbios-base.c
b/drivers/platform/x86/dell-smbios-base.c
index 2e2cd565926aa..5ad6f7c105cf3 100644
--- a/drivers/platform/x86/dell-smbios-base.c
+++ b/drivers/platform/x86/dell-smbios-base.c
@@ -564,7 +564,8 @@ static int __init dell_smbios_init(void)
 	int ret, wmi, smm;
 
 	if (!dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "Dell System",
NULL) &&
-	    !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "www.dell.com",
NULL)) {
+	    !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "www.dell.com",
NULL) &&
+	    !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "Alienware",
NULL)) {
 		pr_err("Unable to run on non-Dell system\n");
 		return -ENODEV;
 	}



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [ PATCH: 1/1] dell smbios driver : Consider Alienware a valid OEM String
  2020-10-03 11:52 [ PATCH: 1/1] dell smbios driver : Consider Alienware a valid OEM String Gerardo Esteban Malazdrewicz
@ 2020-10-03 13:00 ` Hans de Goede
  2020-10-05 12:45   ` Limonciello, Mario
  0 siblings, 1 reply; 17+ messages in thread
From: Hans de Goede @ 2020-10-03 13:00 UTC (permalink / raw)
  To: Gerardo Esteban Malazdrewicz, Pali Rohár, Mario Limonciello
  Cc: platform-driver-x86

Hi,

On 10/3/20 1:52 PM, Gerardo Esteban Malazdrewicz wrote:
>  From bda6b6db0d76186ff37ffce8ac836379447ef2bc Mon Sep 17 00:00:00 2001
> From: Gerardo Malazdrewicz <36243997+GerMalaz@users.noreply.github.com>
> Date: Sat, 3 Oct 2020 07:43:02 -0300
> Subject: [PATCH] dell-smbios-base: Consider Alienware a Dell system
> 
> Alienware has been a subsidiary of Dell since 2006.
> 
> 2020 Alienware laptop:
> $ sudo dmidecode | head -3
> # dmidecode 3.2
> Getting SMBIOS data from sysfs.
> SMBIOS 3.2.0 present.
> $ sudo dmidecode | grep -A 29 "OEM Strings"
> OEM Strings
> 	String 1: Alienware
> 	String 2: 1[099B]
> 	String 3: 3[1.0]
> 	String 4: 4[0001]
> 	String 5: 5[0000]
> 	String 6: 6[D0, D4, D8, DA, DE]
> 	String 7: 7[]
> 	String 8: 8[]
> 	String 9: 9[]
> 	String 10: 10[1.3.0]
> 	String 11: 11[]
> 	String 12: 12[]
> 	String 13: 13[P38E002]
> 	String 14: 14[0]
> 	String 15: 15[0]
> 	String 16: 16[0]
> 	String 17: 17[0000000000000000]
> 	String 18: 18[0]
> 	String 19: 19[1]
> 	String 20: 20[]
> 	String 21: 21[]
> 	String 22: <BAD INDEX>
> 	String 23: <BAD INDEX>
> 	String 24: <BAD INDEX>
> 	String 25: <BAD INDEX>
> 	String 26: <BAD INDEX>
> 	String 27: <BAD INDEX>
> 	String 28: <BAD INDEX>
> 
> 2013 Alienware laptop:
> OEM Strings
>          String 1: Dell System
>          String 2: 1[05AA]
>          String 3: 14[2]
>          String 4: 15[0]
>          String 5: String5 for Original Equipment Manufacturer
> 
> Don't know when the OEM String changed.
> Change tested in the 2020 laptop, loads dell_smbios without further
> issues.
> 
> Thanks,
>          Gerardo

The "Thanks, Gerado" bit is a bit weird for in a commit message,
otherwise this looks good to me (please wait for further feedback
before sending a v2 though).

Mario, what is your take on this, do you think this change is ok,
or might this cause some issues ?

Regards,

Hans






> 
> Signed-off-by: Gerardo E. Malazdrewicz <gerardo@malazdrewicz.com.ar>
> ---
>   drivers/platform/x86/dell-smbios-base.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/dell-smbios-base.c
> b/drivers/platform/x86/dell-smbios-base.c
> index 2e2cd565926aa..5ad6f7c105cf3 100644
> --- a/drivers/platform/x86/dell-smbios-base.c
> +++ b/drivers/platform/x86/dell-smbios-base.c
> @@ -564,7 +564,8 @@ static int __init dell_smbios_init(void)
>   	int ret, wmi, smm;
>   
>   	if (!dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "Dell System",
> NULL) &&
> -	    !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "www.dell.com",
> NULL)) {
> +	    !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "www.dell.com",
> NULL) &&
> +	    !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "Alienware",
> NULL)) {
>   		pr_err("Unable to run on non-Dell system\n");
>   		return -ENODEV;
>   	}
> 
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [ PATCH: 1/1] dell smbios driver : Consider Alienware a valid OEM String
  2020-10-03 13:00 ` Hans de Goede
@ 2020-10-05 12:45   ` Limonciello, Mario
  2020-10-06  3:23     ` [ PATCH v2 " Gerardo Esteban Malazdrewicz
  0 siblings, 1 reply; 17+ messages in thread
From: Limonciello, Mario @ 2020-10-05 12:45 UTC (permalink / raw)
  To: Hans de Goede, Gerardo Esteban Malazdrewicz, Pali Rohár
  Cc: platform-driver-x86



> -----Original Message-----
> From: Hans de Goede <hdegoede@redhat.com>
> Sent: Saturday, October 3, 2020 8:01
> To: Gerardo Esteban Malazdrewicz; Pali Rohár; Limonciello, Mario
> Cc: platform-driver-x86@vger.kernel.org
> Subject: Re: [ PATCH: 1/1] dell smbios driver : Consider Alienware a valid OEM
> String
> 
> 
> [EXTERNAL EMAIL]
> 
> Hi,
> 
> On 10/3/20 1:52 PM, Gerardo Esteban Malazdrewicz wrote:
> >  From bda6b6db0d76186ff37ffce8ac836379447ef2bc Mon Sep 17 00:00:00 2001
> > From: Gerardo Malazdrewicz <36243997+GerMalaz@users.noreply.github.com>
> > Date: Sat, 3 Oct 2020 07:43:02 -0300
> > Subject: [PATCH] dell-smbios-base: Consider Alienware a Dell system
> >
> > Alienware has been a subsidiary of Dell since 2006.
> >
> > 2020 Alienware laptop:
> > $ sudo dmidecode | head -3
> > # dmidecode 3.2
> > Getting SMBIOS data from sysfs.
> > SMBIOS 3.2.0 present.
> > $ sudo dmidecode | grep -A 29 "OEM Strings"
> > OEM Strings
> > 	String 1: Alienware
> > 	String 2: 1[099B]
> > 	String 3: 3[1.0]
> > 	String 4: 4[0001]
> > 	String 5: 5[0000]
> > 	String 6: 6[D0, D4, D8, DA, DE]
> > 	String 7: 7[]
> > 	String 8: 8[]
> > 	String 9: 9[]
> > 	String 10: 10[1.3.0]
> > 	String 11: 11[]
> > 	String 12: 12[]
> > 	String 13: 13[P38E002]
> > 	String 14: 14[0]
> > 	String 15: 15[0]
> > 	String 16: 16[0]
> > 	String 17: 17[0000000000000000]
> > 	String 18: 18[0]
> > 	String 19: 19[1]
> > 	String 20: 20[]
> > 	String 21: 21[]
> > 	String 22: <BAD INDEX>
> > 	String 23: <BAD INDEX>
> > 	String 24: <BAD INDEX>
> > 	String 25: <BAD INDEX>
> > 	String 26: <BAD INDEX>
> > 	String 27: <BAD INDEX>
> > 	String 28: <BAD INDEX>
> >
> > 2013 Alienware laptop:
> > OEM Strings
> >          String 1: Dell System
> >          String 2: 1[05AA]
> >          String 3: 14[2]
> >          String 4: 15[0]
> >          String 5: String5 for Original Equipment Manufacturer
> >
> > Don't know when the OEM String changed.
> > Change tested in the 2020 laptop, loads dell_smbios without further
> > issues.
> >
> > Thanks,
> >          Gerardo
> 
> The "Thanks, Gerado" bit is a bit weird for in a commit message,
> otherwise this looks good to me (please wait for further feedback
> before sending a v2 though).
> 
> Mario, what is your take on this, do you think this change is ok,
> or might this cause some issues ?
> 
> Regards,
> 
> Hans
> 
> 
> 
> 
> 
> 
> >
> > Signed-off-by: Gerardo E. Malazdrewicz <gerardo@malazdrewicz.com.ar>
> > ---
> >   drivers/platform/x86/dell-smbios-base.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/platform/x86/dell-smbios-base.c
> > b/drivers/platform/x86/dell-smbios-base.c
> > index 2e2cd565926aa..5ad6f7c105cf3 100644
> > --- a/drivers/platform/x86/dell-smbios-base.c
> > +++ b/drivers/platform/x86/dell-smbios-base.c
> > @@ -564,7 +564,8 @@ static int __init dell_smbios_init(void)
> >   	int ret, wmi, smm;
> >
> >   	if (!dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "Dell System",
> > NULL) &&
> > -	    !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "www.dell.com",
> > NULL)) {
> > +	    !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "www.dell.com",
> > NULL) &&
> > +	    !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "Alienware",
> > NULL)) {
> >   		pr_err("Unable to run on non-Dell system\n");
> >   		return -ENODEV;
> >   	}
> >
> >

Yes, this should be fine.  There are other checks to make sure the
interface is really there, and as pointed out Alienware is a Dell brand.

Since this is pretty straightforward I would think Hans can just fixup
the commit message to drop the "Thanks".  Otherwise feel free to add
for the content to this for v2:
Reviewed-by: Mario Limonciello <mario.limonciello@dell.com>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [ PATCH v2 1/1] dell smbios driver : Consider Alienware a valid OEM String
  2020-10-05 12:45   ` Limonciello, Mario
@ 2020-10-06  3:23     ` Gerardo Esteban Malazdrewicz
  2020-10-07 14:21       ` Hans de Goede
  0 siblings, 1 reply; 17+ messages in thread
From: Gerardo Esteban Malazdrewicz @ 2020-10-06  3:23 UTC (permalink / raw)
  To: Limonciello, Mario, Hans de Goede, Pali Rohár; +Cc: platform-driver-x86

From bda6b6db0d76186ff37ffce8ac836379447ef2bc Mon Sep 17 00:00:00 2001
From: Gerardo Malazdrewicz <36243997+GerMalaz@users.noreply.github.com>
Date: Sat, 3 Oct 2020 07:43:02 -0300
Subject: [PATCH] dell-smbios-base: Consider Alienware a Dell system

Alienware has been a subsidiary of Dell since 2006.

2020 Alienware laptop:
$ sudo dmidecode | head -3
# dmidecode 3.2
Getting SMBIOS data from sysfs.
SMBIOS 3.2.0 present.
$ sudo dmidecode | grep -A 29 "OEM Strings"
OEM Strings
	String 1: Alienware
	String 2: 1[099B]
	String 3: 3[1.0]
	String 4: 4[0001]
	String 5: 5[0000]
	String 6: 6[D0, D4, D8, DA, DE]
	String 7: 7[]
	String 8: 8[]
	String 9: 9[]
	String 10: 10[1.3.0]
	String 11: 11[]
	String 12: 12[]
	String 13: 13[P38E002]
	String 14: 14[0]
	String 15: 15[0]
	String 16: 16[0]
	String 17: 17[0000000000000000]
	String 18: 18[0]
	String 19: 19[1]
	String 20: 20[]
	String 21: 21[]
	String 22: <BAD INDEX>
	String 23: <BAD INDEX>
	String 24: <BAD INDEX>
	String 25: <BAD INDEX>
	String 26: <BAD INDEX>
	String 27: <BAD INDEX>
	String 28: <BAD INDEX>

2013 Alienware laptop:
OEM Strings
        String 1: Dell System
        String 2: 1[05AA]
        String 3: 14[2]
        String 4: 15[0]
        String 5: String5 for Original Equipment Manufacturer

Don't know when the OEM String changed.
Change tested in the 2020 laptop, loads dell_smbios without further
issues.

Signed-off-by: Gerardo E. Malazdrewicz <gerardo@malazdrewicz.com.ar> 
Reviewed-by: Mario Limonciello <mario.limonciello@dell.com>
---
 drivers/platform/x86/dell-smbios-base.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/dell-smbios-base.c
b/drivers/platform/x86/dell-smbios-base.c
index 2e2cd565926aa..5ad6f7c105cf3 100644
--- a/drivers/platform/x86/dell-smbios-base.c
+++ b/drivers/platform/x86/dell-smbios-base.c
@@ -564,7 +564,8 @@ static int __init dell_smbios_init(void)
 	int ret, wmi, smm;
 
 	if (!dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "Dell System",
NULL) &&
-	    !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "www.dell.com",
NULL)) {
+	    !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "www.dell.com",
NULL) &&
+	    !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "Alienware",
NULL)) {
 		pr_err("Unable to run on non-Dell system\n");
 		return -ENODEV;
 	}




^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [ PATCH v2 1/1] dell smbios driver : Consider Alienware a valid OEM String
  2020-10-06  3:23     ` [ PATCH v2 " Gerardo Esteban Malazdrewicz
@ 2020-10-07 14:21       ` Hans de Goede
  2020-10-07 14:33         ` Pali Rohár
  0 siblings, 1 reply; 17+ messages in thread
From: Hans de Goede @ 2020-10-07 14:21 UTC (permalink / raw)
  To: Gerardo Esteban Malazdrewicz, Limonciello, Mario, Pali Rohár
  Cc: platform-driver-x86

Hi,

On 10/6/20 5:23 AM, Gerardo Esteban Malazdrewicz wrote:
>  From bda6b6db0d76186ff37ffce8ac836379447ef2bc Mon Sep 17 00:00:00 2001
> From: Gerardo Malazdrewicz <36243997+GerMalaz@users.noreply.github.com>
> Date: Sat, 3 Oct 2020 07:43:02 -0300
> Subject: [PATCH] dell-smbios-base: Consider Alienware a Dell system

These lines are no supposed to be part of the body of the email
and the actual patch itself has been line-wrapped breaking it.

I've fixed this up manually this time. But next time please use
git send-email to submit patches to avoid breakage like this.

Thank you for your patch, I've applied this patch to me review-hans branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up there once I've pushed my local branch there,
which might take a while.

Once I've run some tests on this branch the patches there will be added
to the platform-drivers-x86/for-next branch and eventually will be
included in the pdx86 pull-request to Linus for the next merge-window.

Regards,

Hans



> 
> Alienware has been a subsidiary of Dell since 2006.
> 
> 2020 Alienware laptop:
> $ sudo dmidecode | head -3
> # dmidecode 3.2
> Getting SMBIOS data from sysfs.
> SMBIOS 3.2.0 present.
> $ sudo dmidecode | grep -A 29 "OEM Strings"
> OEM Strings
> 	String 1: Alienware
> 	String 2: 1[099B]
> 	String 3: 3[1.0]
> 	String 4: 4[0001]
> 	String 5: 5[0000]
> 	String 6: 6[D0, D4, D8, DA, DE]
> 	String 7: 7[]
> 	String 8: 8[]
> 	String 9: 9[]
> 	String 10: 10[1.3.0]
> 	String 11: 11[]
> 	String 12: 12[]
> 	String 13: 13[P38E002]
> 	String 14: 14[0]
> 	String 15: 15[0]
> 	String 16: 16[0]
> 	String 17: 17[0000000000000000]
> 	String 18: 18[0]
> 	String 19: 19[1]
> 	String 20: 20[]
> 	String 21: 21[]
> 	String 22: <BAD INDEX>
> 	String 23: <BAD INDEX>
> 	String 24: <BAD INDEX>
> 	String 25: <BAD INDEX>
> 	String 26: <BAD INDEX>
> 	String 27: <BAD INDEX>
> 	String 28: <BAD INDEX>
> 
> 2013 Alienware laptop:
> OEM Strings
>          String 1: Dell System
>          String 2: 1[05AA]
>          String 3: 14[2]
>          String 4: 15[0]
>          String 5: String5 for Original Equipment Manufacturer
> 
> Don't know when the OEM String changed.
> Change tested in the 2020 laptop, loads dell_smbios without further
> issues.
> 
> Signed-off-by: Gerardo E. Malazdrewicz <gerardo@malazdrewicz.com.ar>
> Reviewed-by: Mario Limonciello <mario.limonciello@dell.com>
> ---
>   drivers/platform/x86/dell-smbios-base.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/dell-smbios-base.c
> b/drivers/platform/x86/dell-smbios-base.c
> index 2e2cd565926aa..5ad6f7c105cf3 100644
> --- a/drivers/platform/x86/dell-smbios-base.c
> +++ b/drivers/platform/x86/dell-smbios-base.c
> @@ -564,7 +564,8 @@ static int __init dell_smbios_init(void)
>   	int ret, wmi, smm;
>   
>   	if (!dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "Dell System",
> NULL) &&
> -	    !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "www.dell.com",
> NULL)) {
> +	    !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "www.dell.com",
> NULL) &&
> +	    !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "Alienware",
> NULL)) {
>   		pr_err("Unable to run on non-Dell system\n");
>   		return -ENODEV;
>   	}
> 
> 
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [ PATCH v2 1/1] dell smbios driver : Consider Alienware a valid OEM String
  2020-10-07 14:21       ` Hans de Goede
@ 2020-10-07 14:33         ` Pali Rohár
  2020-10-07 15:53           ` Limonciello, Mario
  0 siblings, 1 reply; 17+ messages in thread
From: Pali Rohár @ 2020-10-07 14:33 UTC (permalink / raw)
  To: Hans de Goede, Gerardo Esteban Malazdrewicz
  Cc: Limonciello, Mario, platform-driver-x86

On Wednesday 07 October 2020 16:21:12 Hans de Goede wrote:
> Hi,
> 
> On 10/6/20 5:23 AM, Gerardo Esteban Malazdrewicz wrote:
> >  From bda6b6db0d76186ff37ffce8ac836379447ef2bc Mon Sep 17 00:00:00 2001
> > From: Gerardo Malazdrewicz <36243997+GerMalaz@users.noreply.github.com>
> > Date: Sat, 3 Oct 2020 07:43:02 -0300
> > Subject: [PATCH] dell-smbios-base: Consider Alienware a Dell system
> 
> These lines are no supposed to be part of the body of the email
> and the actual patch itself has been line-wrapped breaking it.
> 
> I've fixed this up manually this time. But next time please use
> git send-email to submit patches to avoid breakage like this.

Gerardo, if you have a problems with sending patches, you can look into
kernel howto guide where are written lot of details:

https://www.kernel.org/doc/html/latest/process/submitting-patches.html

And in case you cannot use preferred 'git send-email' command for
sending patches as Hans pointed, look into following page where are
written hints how to configure different email clients to not mangle /
line wrap patches:

https://www.kernel.org/doc/html/latest/process/email-clients.html#email-clients

> Thank you for your patch, I've applied this patch to me review-hans branch:
> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
> 
> Note it will show up there once I've pushed my local branch there,
> which might take a while.
> 
> Once I've run some tests on this branch the patches there will be added
> to the platform-drivers-x86/for-next branch and eventually will be
> included in the pdx86 pull-request to Linus for the next merge-window.
> 
> Regards,
> 
> Hans

Hans, there are more drivers which checks for Dell DMI strings. Probably
it would be needed to update Alienware on more places, not only in
dell-smbios-base.c driver.

> 
> 
> > 
> > Alienware has been a subsidiary of Dell since 2006.
> > 
> > 2020 Alienware laptop:
> > $ sudo dmidecode | head -3
> > # dmidecode 3.2
> > Getting SMBIOS data from sysfs.
> > SMBIOS 3.2.0 present.
> > $ sudo dmidecode | grep -A 29 "OEM Strings"
> > OEM Strings
> > 	String 1: Alienware
> > 	String 2: 1[099B]
> > 	String 3: 3[1.0]
> > 	String 4: 4[0001]
> > 	String 5: 5[0000]
> > 	String 6: 6[D0, D4, D8, DA, DE]
> > 	String 7: 7[]
> > 	String 8: 8[]
> > 	String 9: 9[]
> > 	String 10: 10[1.3.0]
> > 	String 11: 11[]
> > 	String 12: 12[]
> > 	String 13: 13[P38E002]
> > 	String 14: 14[0]
> > 	String 15: 15[0]
> > 	String 16: 16[0]
> > 	String 17: 17[0000000000000000]
> > 	String 18: 18[0]
> > 	String 19: 19[1]
> > 	String 20: 20[]
> > 	String 21: 21[]
> > 	String 22: <BAD INDEX>
> > 	String 23: <BAD INDEX>
> > 	String 24: <BAD INDEX>
> > 	String 25: <BAD INDEX>
> > 	String 26: <BAD INDEX>
> > 	String 27: <BAD INDEX>
> > 	String 28: <BAD INDEX>
> > 
> > 2013 Alienware laptop:
> > OEM Strings
> >          String 1: Dell System
> >          String 2: 1[05AA]
> >          String 3: 14[2]
> >          String 4: 15[0]
> >          String 5: String5 for Original Equipment Manufacturer
> > 
> > Don't know when the OEM String changed.
> > Change tested in the 2020 laptop, loads dell_smbios without further
> > issues.
> > 
> > Signed-off-by: Gerardo E. Malazdrewicz <gerardo@malazdrewicz.com.ar>
> > Reviewed-by: Mario Limonciello <mario.limonciello@dell.com>
> > ---
> >   drivers/platform/x86/dell-smbios-base.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/platform/x86/dell-smbios-base.c
> > b/drivers/platform/x86/dell-smbios-base.c
> > index 2e2cd565926aa..5ad6f7c105cf3 100644
> > --- a/drivers/platform/x86/dell-smbios-base.c
> > +++ b/drivers/platform/x86/dell-smbios-base.c
> > @@ -564,7 +564,8 @@ static int __init dell_smbios_init(void)
> >   	int ret, wmi, smm;
> >   	if (!dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "Dell System",
> > NULL) &&
> > -	    !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "www.dell.com",
> > NULL)) {
> > +	    !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "www.dell.com",
> > NULL) &&
> > +	    !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "Alienware",
> > NULL)) {
> >   		pr_err("Unable to run on non-Dell system\n");
> >   		return -ENODEV;
> >   	}
> > 
> > 
> > 
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [ PATCH v2 1/1] dell smbios driver : Consider Alienware a valid OEM String
  2020-10-07 14:33         ` Pali Rohár
@ 2020-10-07 15:53           ` Limonciello, Mario
  2020-10-07 16:00             ` Hans de Goede
       [not found]             ` <45e82b6dabb591de630ac0e91a3ebb7937245fb1.camel@malazdrewicz.com.ar>
  0 siblings, 2 replies; 17+ messages in thread
From: Limonciello, Mario @ 2020-10-07 15:53 UTC (permalink / raw)
  To: Pali Rohár, Hans de Goede, Gerardo Esteban Malazdrewicz
  Cc: platform-driver-x86

> 
> Hans, there are more drivers which checks for Dell DMI strings. Probably
> it would be needed to update Alienware on more places, not only in
> dell-smbios-base.c driver.

I would prefer that each of those be checked on a case by case basis and only
added if actually necessary.  Gerardo if you can please check any other drivers
that should need this string added to their allow list.

> 
> >
> >
> > >
> > > Alienware has been a subsidiary of Dell since 2006.
> > >
> > > 2020 Alienware laptop:
> > > $ sudo dmidecode | head -3
> > > # dmidecode 3.2
> > > Getting SMBIOS data from sysfs.
> > > SMBIOS 3.2.0 present.
> > > $ sudo dmidecode | grep -A 29 "OEM Strings"
> > > OEM Strings
> > > 	String 1: Alienware
> > > 	String 2: 1[099B]
> > > 	String 3: 3[1.0]
> > > 	String 4: 4[0001]
> > > 	String 5: 5[0000]
> > > 	String 6: 6[D0, D4, D8, DA, DE]
> > > 	String 7: 7[]
> > > 	String 8: 8[]
> > > 	String 9: 9[]
> > > 	String 10: 10[1.3.0]
> > > 	String 11: 11[]
> > > 	String 12: 12[]
> > > 	String 13: 13[P38E002]
> > > 	String 14: 14[0]
> > > 	String 15: 15[0]
> > > 	String 16: 16[0]
> > > 	String 17: 17[0000000000000000]
> > > 	String 18: 18[0]
> > > 	String 19: 19[1]
> > > 	String 20: 20[]
> > > 	String 21: 21[]
> > > 	String 22: <BAD INDEX>
> > > 	String 23: <BAD INDEX>
> > > 	String 24: <BAD INDEX>
> > > 	String 25: <BAD INDEX>
> > > 	String 26: <BAD INDEX>
> > > 	String 27: <BAD INDEX>
> > > 	String 28: <BAD INDEX>
> > >
> > > 2013 Alienware laptop:
> > > OEM Strings
> > >          String 1: Dell System
> > >          String 2: 1[05AA]
> > >          String 3: 14[2]
> > >          String 4: 15[0]
> > >          String 5: String5 for Original Equipment Manufacturer
> > >
> > > Don't know when the OEM String changed.
> > > Change tested in the 2020 laptop, loads dell_smbios without further
> > > issues.
> > >
> > > Signed-off-by: Gerardo E. Malazdrewicz <gerardo@malazdrewicz.com.ar>
> > > Reviewed-by: Mario Limonciello <mario.limonciello@dell.com>
> > > ---
> > >   drivers/platform/x86/dell-smbios-base.c | 3 ++-
> > >   1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/platform/x86/dell-smbios-base.c
> > > b/drivers/platform/x86/dell-smbios-base.c
> > > index 2e2cd565926aa..5ad6f7c105cf3 100644
> > > --- a/drivers/platform/x86/dell-smbios-base.c
> > > +++ b/drivers/platform/x86/dell-smbios-base.c
> > > @@ -564,7 +564,8 @@ static int __init dell_smbios_init(void)
> > >   	int ret, wmi, smm;
> > >   	if (!dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "Dell System",
> > > NULL) &&
> > > -	    !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "www.dell.com",
> > > NULL)) {
> > > +	    !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "www.dell.com",
> > > NULL) &&
> > > +	    !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "Alienware",
> > > NULL)) {
> > >   		pr_err("Unable to run on non-Dell system\n");
> > >   		return -ENODEV;
> > >   	}
> > >
> > >
> > >
> >

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [ PATCH v2 1/1] dell smbios driver : Consider Alienware a valid OEM String
  2020-10-07 15:53           ` Limonciello, Mario
@ 2020-10-07 16:00             ` Hans de Goede
       [not found]             ` <45e82b6dabb591de630ac0e91a3ebb7937245fb1.camel@malazdrewicz.com.ar>
  1 sibling, 0 replies; 17+ messages in thread
From: Hans de Goede @ 2020-10-07 16:00 UTC (permalink / raw)
  To: Limonciello, Mario, Pali Rohár, Gerardo Esteban Malazdrewicz
  Cc: platform-driver-x86

Hi,

On 10/7/20 5:53 PM, Limonciello, Mario wrote:
>>
>> Hans, there are more drivers which checks for Dell DMI strings. Probably
>> it would be needed to update Alienware on more places, not only in
>> dell-smbios-base.c driver.
> 
> I would prefer that each of those be checked on a case by case basis and only
> added if actually necessary.  Gerardo if you can please check any other drivers
> that should need this string added to their allow list.

Ack, that is my take on this to (check on a case by case basis
if we need any of those other drivers on Alienware models too).

Regards,

Hans



>>>> Alienware has been a subsidiary of Dell since 2006.
>>>>
>>>> 2020 Alienware laptop:
>>>> $ sudo dmidecode | head -3
>>>> # dmidecode 3.2
>>>> Getting SMBIOS data from sysfs.
>>>> SMBIOS 3.2.0 present.
>>>> $ sudo dmidecode | grep -A 29 "OEM Strings"
>>>> OEM Strings
>>>> 	String 1: Alienware
>>>> 	String 2: 1[099B]
>>>> 	String 3: 3[1.0]
>>>> 	String 4: 4[0001]
>>>> 	String 5: 5[0000]
>>>> 	String 6: 6[D0, D4, D8, DA, DE]
>>>> 	String 7: 7[]
>>>> 	String 8: 8[]
>>>> 	String 9: 9[]
>>>> 	String 10: 10[1.3.0]
>>>> 	String 11: 11[]
>>>> 	String 12: 12[]
>>>> 	String 13: 13[P38E002]
>>>> 	String 14: 14[0]
>>>> 	String 15: 15[0]
>>>> 	String 16: 16[0]
>>>> 	String 17: 17[0000000000000000]
>>>> 	String 18: 18[0]
>>>> 	String 19: 19[1]
>>>> 	String 20: 20[]
>>>> 	String 21: 21[]
>>>> 	String 22: <BAD INDEX>
>>>> 	String 23: <BAD INDEX>
>>>> 	String 24: <BAD INDEX>
>>>> 	String 25: <BAD INDEX>
>>>> 	String 26: <BAD INDEX>
>>>> 	String 27: <BAD INDEX>
>>>> 	String 28: <BAD INDEX>
>>>>
>>>> 2013 Alienware laptop:
>>>> OEM Strings
>>>>           String 1: Dell System
>>>>           String 2: 1[05AA]
>>>>           String 3: 14[2]
>>>>           String 4: 15[0]
>>>>           String 5: String5 for Original Equipment Manufacturer
>>>>
>>>> Don't know when the OEM String changed.
>>>> Change tested in the 2020 laptop, loads dell_smbios without further
>>>> issues.
>>>>
>>>> Signed-off-by: Gerardo E. Malazdrewicz <gerardo@malazdrewicz.com.ar>
>>>> Reviewed-by: Mario Limonciello <mario.limonciello@dell.com>
>>>> ---
>>>>    drivers/platform/x86/dell-smbios-base.c | 3 ++-
>>>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/platform/x86/dell-smbios-base.c
>>>> b/drivers/platform/x86/dell-smbios-base.c
>>>> index 2e2cd565926aa..5ad6f7c105cf3 100644
>>>> --- a/drivers/platform/x86/dell-smbios-base.c
>>>> +++ b/drivers/platform/x86/dell-smbios-base.c
>>>> @@ -564,7 +564,8 @@ static int __init dell_smbios_init(void)
>>>>    	int ret, wmi, smm;
>>>>    	if (!dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "Dell System",
>>>> NULL) &&
>>>> -	    !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "www.dell.com",
>>>> NULL)) {
>>>> +	    !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "www.dell.com",
>>>> NULL) &&
>>>> +	    !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "Alienware",
>>>> NULL)) {
>>>>    		pr_err("Unable to run on non-Dell system\n");
>>>>    		return -ENODEV;
>>>>    	}
>>>>
>>>>
>>>>
>>>
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [ PATCH v2 1/1] dell smbios driver : Consider Alienware a valid OEM String
       [not found]               ` <DM6PR19MB26363563F46E95E50AD28854FA0A0@DM6PR19MB2636.namprd19.prod.outlook.com>
@ 2020-10-07 20:38                 ` Hans de Goede
  2020-10-07 21:26                   ` Limonciello, Mario
  2020-10-09  5:33                   ` Gerardo Esteban Malazdrewicz
  0 siblings, 2 replies; 17+ messages in thread
From: Hans de Goede @ 2020-10-07 20:38 UTC (permalink / raw)
  To: Limonciello, Mario, Gerardo Esteban Malazdrewicz, Pali Rohár
  Cc: platform-driver-x86

Hi,

On 10/7/20 9:58 PM, Limonciello, Mario wrote:
>> -----Original Message-----
>> From: Gerardo Esteban Malazdrewicz <gerardo@malazdrewicz.com.ar>
>> Sent: Wednesday, October 7, 2020 14:55
>> To: Limonciello, Mario; Pali Rohár; Hans de Goede
>> Subject: Re: [ PATCH v2 1/1] dell smbios driver : Consider Alienware a valid
>> OEM String
>>
>>
>> [EXTERNAL EMAIL]
>>
>> El mié, 07-10-2020 a las 15:53 +0000, Limonciello, Mario escribió:
>>>> Hans, there are more drivers which checks for Dell DMI strings.
>>>> Probably
>>>> it would be needed to update Alienware on more places, not only in
>>>> dell-smbios-base.c driver.
>>>
>>> I would prefer that each of those be checked on a case by case basis
>>> and only
>>> added if actually necessary.  Gerardo if you can please check any
>>> other drivers
>>> that should need this string added to their allow list.
>>
>> I didn't find other instances of that string in this subsystem, but see
>> below.
>>
>> There is one in pci, another in hotplug.
>>
>> However, this is an extract from kernel logs:
>>
>> [  138.093686] dell-smbios A80593CE-A997-11DA-B012-B622A1EF5492: WMI
>> SMBIOS userspace interface not supported(0), try upgrading to a newer
>> BIOS
> 
> Considering that messaging - does the non-WMI interface actually work?
> dell-smbios has two backends available.

Yes that is a very good question.

Gerardo, I guess you started looking into this because of the:

	pr_err("Unable to run on non-Dell system\n");

In dell-smbios-base.c triggering on your system?

As Pali mentioned in another mail, you probably should
be looking at the dell-laptop code, which also has a
has a DMI string check and which uses the dell-smbios code,
another consumer of the dell-smbios code is the dell-wmi
driver.

If neither of those drivers add additional functionality
(e.g. extra hotkey events, being able the control the kbd
backlight), then the right fix might be to silence the
error you see being thrown by dell-smbios-base.c, rather
then allowing it to load.

For now I'll drop your patch from my review-hans branch,
as we first need to clear this up.

> The SMI based backend you can check by using dcdbas.
> 
> I had presumed from your patch that it actually worked.
> 
>> [ 1275.987716] dell_smm_hwmon: not running on a supported Dell system.
>> [ 1275.987734] dell_smm_hwmon: vendor=Alienware, model=Alienware Area-
>> 51m R2, version=1.3.0
>>
>>
>> dell_smm_hwmon ignore_dmi=1
>>
>> /sys/class/hwmon/hwmonX/pwm{1,3} access correctly the left and right
>> fan, respectively

Ok, so that looks good and fixing the DMI check there probably
makes sense. Note that this code is independent from the
dell-smbios code from drivers/platform/x86 so you can
do another patch to fix the DMI check there independent of the
dell-smbios discussion.

Regards,

Hans


^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [ PATCH v2 1/1] dell smbios driver : Consider Alienware a valid OEM String
  2020-10-07 20:38                 ` Hans de Goede
@ 2020-10-07 21:26                   ` Limonciello, Mario
  2020-10-07 21:30                     ` Hans de Goede
  2020-10-09  5:33                   ` Gerardo Esteban Malazdrewicz
  1 sibling, 1 reply; 17+ messages in thread
From: Limonciello, Mario @ 2020-10-07 21:26 UTC (permalink / raw)
  To: Hans de Goede, Gerardo Esteban Malazdrewicz, Pali Rohár
  Cc: platform-driver-x86

> On 10/7/20 9:58 PM, Limonciello, Mario wrote:
> >> -----Original Message-----
> >> From: Gerardo Esteban Malazdrewicz <gerardo@malazdrewicz.com.ar>
> >> Sent: Wednesday, October 7, 2020 14:55
> >> To: Limonciello, Mario; Pali Rohár; Hans de Goede
> >> Subject: Re: [ PATCH v2 1/1] dell smbios driver : Consider Alienware a
> valid
> >> OEM String
> >>
> >>
> >> [EXTERNAL EMAIL]
> >>
> >> El mié, 07-10-2020 a las 15:53 +0000, Limonciello, Mario escribió:
> >>>> Hans, there are more drivers which checks for Dell DMI strings.
> >>>> Probably
> >>>> it would be needed to update Alienware on more places, not only in
> >>>> dell-smbios-base.c driver.
> >>>
> >>> I would prefer that each of those be checked on a case by case basis
> >>> and only
> >>> added if actually necessary.  Gerardo if you can please check any
> >>> other drivers
> >>> that should need this string added to their allow list.
> >>
> >> I didn't find other instances of that string in this subsystem, but see
> >> below.
> >>
> >> There is one in pci, another in hotplug.
> >>
> >> However, this is an extract from kernel logs:
> >>
> >> [  138.093686] dell-smbios A80593CE-A997-11DA-B012-B622A1EF5492: WMI
> >> SMBIOS userspace interface not supported(0), try upgrading to a newer
> >> BIOS
> >
> > Considering that messaging - does the non-WMI interface actually work?
> > dell-smbios has two backends available.
> 
> Yes that is a very good question.
> 
> Gerardo, I guess you started looking into this because of the:
> 
> 	pr_err("Unable to run on non-Dell system\n");
> 
> In dell-smbios-base.c triggering on your system?

If that's the case, I would ask why this driver even auto-loaded?
The module load table is very prescriptive.
https://github.com/torvalds/linux/blob/master/drivers/platform/x86/dell-smbios-wmi.c#L277
https://github.com/torvalds/linux/blob/master/drivers/platform/x86/dell-smbios-smm.c#L56
Was it because it was compiled into the kernel?

> 
> As Pali mentioned in another mail, you probably should
> be looking at the dell-laptop code, which also has a
> has a DMI string check and which uses the dell-smbios code,
> another consumer of the dell-smbios code is the dell-wmi
> driver.
> 
> If neither of those drivers add additional functionality
> (e.g. extra hotkey events, being able the control the kbd
> backlight), then the right fix might be to silence the
> error you see being thrown by dell-smbios-base.c, rather
> then allowing it to load.

I have no opposition to dropping that pr_err or at least downgrading
it to pr_debug.

> 
> For now I'll drop your patch from my review-hans branch,
> as we first need to clear this up.
> 
> > The SMI based backend you can check by using dcdbas.
> >
> > I had presumed from your patch that it actually worked.
> >
> >> [ 1275.987716] dell_smm_hwmon: not running on a supported Dell system.
> >> [ 1275.987734] dell_smm_hwmon: vendor=Alienware, model=Alienware Area-
> >> 51m R2, version=1.3.0
> >>
> >>
> >> dell_smm_hwmon ignore_dmi=1
> >>
> >> /sys/class/hwmon/hwmonX/pwm{1,3} access correctly the left and right
> >> fan, respectively
> 
> Ok, so that looks good and fixing the DMI check there probably
> makes sense. Note that this code is independent from the
> dell-smbios code from drivers/platform/x86 so you can
> do another patch to fix the DMI check there independent of the
> dell-smbios discussion.
> 
> Regards,
> 
> Hans


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [ PATCH v2 1/1] dell smbios driver : Consider Alienware a valid OEM String
  2020-10-07 21:26                   ` Limonciello, Mario
@ 2020-10-07 21:30                     ` Hans de Goede
  2020-10-07 21:33                       ` Limonciello, Mario
  0 siblings, 1 reply; 17+ messages in thread
From: Hans de Goede @ 2020-10-07 21:30 UTC (permalink / raw)
  To: Limonciello, Mario, Gerardo Esteban Malazdrewicz, Pali Rohár
  Cc: platform-driver-x86

Hi,

On 10/7/20 11:26 PM, Limonciello, Mario wrote:
>> On 10/7/20 9:58 PM, Limonciello, Mario wrote:
>>>> -----Original Message-----
>>>> From: Gerardo Esteban Malazdrewicz <gerardo@malazdrewicz.com.ar>
>>>> Sent: Wednesday, October 7, 2020 14:55
>>>> To: Limonciello, Mario; Pali Rohár; Hans de Goede
>>>> Subject: Re: [ PATCH v2 1/1] dell smbios driver : Consider Alienware a
>> valid
>>>> OEM String
>>>>
>>>>
>>>> [EXTERNAL EMAIL]
>>>>
>>>> El mié, 07-10-2020 a las 15:53 +0000, Limonciello, Mario escribió:
>>>>>> Hans, there are more drivers which checks for Dell DMI strings.
>>>>>> Probably
>>>>>> it would be needed to update Alienware on more places, not only in
>>>>>> dell-smbios-base.c driver.
>>>>>
>>>>> I would prefer that each of those be checked on a case by case basis
>>>>> and only
>>>>> added if actually necessary.  Gerardo if you can please check any
>>>>> other drivers
>>>>> that should need this string added to their allow list.
>>>>
>>>> I didn't find other instances of that string in this subsystem, but see
>>>> below.
>>>>
>>>> There is one in pci, another in hotplug.
>>>>
>>>> However, this is an extract from kernel logs:
>>>>
>>>> [  138.093686] dell-smbios A80593CE-A997-11DA-B012-B622A1EF5492: WMI
>>>> SMBIOS userspace interface not supported(0), try upgrading to a newer
>>>> BIOS
>>>
>>> Considering that messaging - does the non-WMI interface actually work?
>>> dell-smbios has two backends available.
>>
>> Yes that is a very good question.
>>
>> Gerardo, I guess you started looking into this because of the:
>>
>> 	pr_err("Unable to run on non-Dell system\n");
>>
>> In dell-smbios-base.c triggering on your system?
> 
> If that's the case, I would ask why this driver even auto-loaded?
> The module load table is very prescriptive.
> https://github.com/torvalds/linux/blob/master/drivers/platform/x86/dell-smbios-wmi.c#L277
> https://github.com/torvalds/linux/blob/master/drivers/platform/x86/dell-smbios-smm.c#L56
> Was it because it was compiled into the kernel?

I believe that the laptops ACPI tables do define the WMI GUID
that dell-smbios-wmi.c has in its module-id-table otherwise
the "WMI SMBIOS userspace interface not supported(0)" error
would not trigger.

Regards,

Hans


^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [ PATCH v2 1/1] dell smbios driver : Consider Alienware a valid OEM String
  2020-10-07 21:30                     ` Hans de Goede
@ 2020-10-07 21:33                       ` Limonciello, Mario
  0 siblings, 0 replies; 17+ messages in thread
From: Limonciello, Mario @ 2020-10-07 21:33 UTC (permalink / raw)
  To: Hans de Goede, Gerardo Esteban Malazdrewicz, Pali Rohár
  Cc: platform-driver-x86

> On 10/7/20 11:26 PM, Limonciello, Mario wrote:
> >> On 10/7/20 9:58 PM, Limonciello, Mario wrote:
> >>>> -----Original Message-----
> >>>> From: Gerardo Esteban Malazdrewicz <gerardo@malazdrewicz.com.ar>
> >>>> Sent: Wednesday, October 7, 2020 14:55
> >>>> To: Limonciello, Mario; Pali Rohár; Hans de Goede
> >>>> Subject: Re: [ PATCH v2 1/1] dell smbios driver : Consider Alienware a
> >> valid
> >>>> OEM String
> >>>>
> >>>>
> >>>> [EXTERNAL EMAIL]
> >>>>
> >>>> El mié, 07-10-2020 a las 15:53 +0000, Limonciello, Mario escribió:
> >>>>>> Hans, there are more drivers which checks for Dell DMI strings.
> >>>>>> Probably
> >>>>>> it would be needed to update Alienware on more places, not only in
> >>>>>> dell-smbios-base.c driver.
> >>>>>
> >>>>> I would prefer that each of those be checked on a case by case basis
> >>>>> and only
> >>>>> added if actually necessary.  Gerardo if you can please check any
> >>>>> other drivers
> >>>>> that should need this string added to their allow list.
> >>>>
> >>>> I didn't find other instances of that string in this subsystem, but see
> >>>> below.
> >>>>
> >>>> There is one in pci, another in hotplug.
> >>>>
> >>>> However, this is an extract from kernel logs:
> >>>>
> >>>> [  138.093686] dell-smbios A80593CE-A997-11DA-B012-B622A1EF5492: WMI
> >>>> SMBIOS userspace interface not supported(0), try upgrading to a newer
> >>>> BIOS
> >>>
> >>> Considering that messaging - does the non-WMI interface actually work?
> >>> dell-smbios has two backends available.
> >>
> >> Yes that is a very good question.
> >>
> >> Gerardo, I guess you started looking into this because of the:
> >>
> >> 	pr_err("Unable to run on non-Dell system\n");
> >>
> >> In dell-smbios-base.c triggering on your system?
> >
> > If that's the case, I would ask why this driver even auto-loaded?
> > The module load table is very prescriptive.
> > https://github.com/torvalds/linux/blob/master/drivers/platform/x86/dell-
> smbios-wmi.c#L277
> > https://github.com/torvalds/linux/blob/master/drivers/platform/x86/dell-
> smbios-smm.c#L56
> > Was it because it was compiled into the kernel?
> 
> I believe that the laptops ACPI tables do define the WMI GUID
> that dell-smbios-wmi.c has in its module-id-table otherwise
> the "WMI SMBIOS userspace interface not supported(0)" error
> would not trigger.
> 

That's a good point, you're right thanks.  I should note - that is a
warning, it's valid and it only turns off userspace access.  If the BIOS doesn't
advertise that hotfix many calls can fail.  Some of the more simple
calls used by the kernel modules such as dell-laptop would not be prohibited.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [ PATCH v2 1/1] dell smbios driver : Consider Alienware a valid OEM String
  2020-10-07 20:38                 ` Hans de Goede
  2020-10-07 21:26                   ` Limonciello, Mario
@ 2020-10-09  5:33                   ` Gerardo Esteban Malazdrewicz
  2020-10-09  8:26                     ` Pali Rohár
  1 sibling, 1 reply; 17+ messages in thread
From: Gerardo Esteban Malazdrewicz @ 2020-10-09  5:33 UTC (permalink / raw)
  To: Hans de Goede, Limonciello, Mario, Pali Rohár; +Cc: platform-driver-x86

El mié, 07-10-2020 a las 22:38 +0200, Hans de Goede escribió:
> Hi,
> 
> On 10/7/20 9:58 PM, Limonciello, Mario wrote:
> > > -----Original Message-----
> > > From: Gerardo Esteban Malazdrewicz <gerardo@malazdrewicz.com.ar>
> > > Sent: Wednesday, October 7, 2020 14:55
> > > To: Limonciello, Mario; Pali Rohár; Hans de Goede
> > > Subject: Re: [ PATCH v2 1/1] dell smbios driver : Consider
> > > Alienware a valid
> > > OEM String
> > > 
> > > 
> > > [EXTERNAL EMAIL]
> > > 
> > > El mié, 07-10-2020 a las 15:53 +0000, Limonciello, Mario
> > > escribió:
> > > > > Hans, there are more drivers which checks for Dell DMI
> > > > > strings.
> > > > > Probably
> > > > > it would be needed to update Alienware on more places, not
> > > > > only in
> > > > > dell-smbios-base.c driver.
> > > > 
> > > > I would prefer that each of those be checked on a case by case
> > > > basis
> > > > and only
> > > > added if actually necessary.  Gerardo if you can please check
> > > > any
> > > > other drivers
> > > > that should need this string added to their allow list.
> > > 
> > > I didn't find other instances of that string in this subsystem,
> > > but see
> > > below.
> > > 
> > > There is one in pci, another in hotplug.
> > > 
> > > However, this is an extract from kernel logs:
> > > 
> > > [  138.093686] dell-smbios A80593CE-A997-11DA-B012-B622A1EF5492:
> > > WMI
> > > SMBIOS userspace interface not supported(0), try upgrading to a
> > > newer
> > > BIOS
> > 
> > Considering that messaging - does the non-WMI interface actually
> > work?
> > dell-smbios has two backends available.
> 
> Yes that is a very good question.
> 
> Gerardo, I guess you started looking into this because of the:
> 
> 	pr_err("Unable to run on non-Dell system\n");
> 
> In dell-smbios-base.c triggering on your system?

Correct, plus the top of dmidecode output, declaring SMBIOS present.

# dmidecode 3.2
Getting SMBIOS data from sysfs.
SMBIOS 3.2.0 present.
Table at 0x848B6000.


> 
> As Pali mentioned in another mail, you probably should
> be looking at the dell-laptop code, which also has a
> has a DMI string check and which uses the dell-smbios code,
> another consumer of the dell-smbios code is the dell-wmi
> driver.
> 
> If neither of those drivers add additional functionality
> (e.g. extra hotkey events, being able the control the kbd
> backlight), then the right fix might be to silence the
> error you see being thrown by dell-smbios-base.c, rather
> then allowing it to load.
> 

dell-laptop doesn't even load as is ('No such device')

However, adding an entry in dell_device_table, based on dmidecode
output (Vendor: Alienware, Type: 10), allows it to load.

But I don't know how to test for any of this additional functionality.

What should I look for?

If any is found, would do a v3.

dell|alienware lsmod output, if that helps:

$ sudo lsmod | egrep "(dell|alienware)"
dell_laptop            28672  0
dell_rbtn              20480  0
dell_rbu               20480  0
dell_wmi               20480  0
dell_smo8800           20480  0
dell_smbios            32768  2 dell_wmi,dell_laptop
dell_smm_hwmon         24576  0
ledtrig_audio          16384  3
snd_hda_codec_generic,snd_sof,dell_laptop
dcdbas                 20480  1 dell_smbios
alienware_wmi          20480  0
dell_wmi_descriptor    20480  2 dell_wmi,dell_smbios
rfkill                 28672  11
bluetooth,dell_laptop,dell_rbtn,cfg80211
sparse_keymap          16384  2 intel_hid,dell_wmi
wmi                    36864  7
intel_wmi_thunderbolt,alienware_wmi,dell_wmi,wmi_bmof,dell_smbios,dell_
wmi_descriptor,mxm_wmi
video                  53248  3 dell_wmi,dell_laptop,i915
dell|alienware lsmod output, if that helps:

> For now I'll drop your patch from my review-hans branch,
> as we first need to clear this up.
> 
> > The SMI based backend you can check by using dcdbas.
> > 
> > I had presumed from your patch that it actually worked.
> > 
> > > [ 1275.987716] dell_smm_hwmon: not running on a supported Dell
> > > system.
> > > [ 1275.987734] dell_smm_hwmon: vendor=Alienware, model=Alienware
> > > Area-
> > > 51m R2, version=1.3.0
> > > 
> > > 
> > > dell_smm_hwmon ignore_dmi=1
> > > 
> > > /sys/class/hwmon/hwmonX/pwm{1,3} access correctly the left and
> > > right
> > > fan, respectively
> 
> Ok, so that looks good and fixing the DMI check there probably
> makes sense. Note that this code is independent from the
> dell-smbios code from drivers/platform/x86 so you can
> do another patch to fix the DMI check there independent of the
> dell-smbios discussion.
> 
> Regards,
> 
> Hans

Regards,      Gerardo
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [ PATCH v2 1/1] dell smbios driver : Consider Alienware a valid OEM String
  2020-10-09  5:33                   ` Gerardo Esteban Malazdrewicz
@ 2020-10-09  8:26                     ` Pali Rohár
  2020-10-09  9:14                       ` Hans de Goede
  0 siblings, 1 reply; 17+ messages in thread
From: Pali Rohár @ 2020-10-09  8:26 UTC (permalink / raw)
  To: Gerardo Esteban Malazdrewicz
  Cc: Hans de Goede, Limonciello, Mario, platform-driver-x86

On Friday 09 October 2020 02:33:49 Gerardo Esteban Malazdrewicz wrote:
> dell-laptop doesn't even load as is ('No such device')
> 
> However, adding an entry in dell_device_table, based on dmidecode
> output (Vendor: Alienware, Type: 10), allows it to load.
> 
> But I don't know how to test for any of this additional functionality.
> 
> What should I look for?

Hello! dell-laptop driver provides following features:

* rfkill interface for enabling/disabling wifi and bluetooth
  - check presence of "*dell*" by /sbin/rfkill utility

* backlight interface for controlling display brightness
  - check presence of "dell_backlight" in /sys/class/backlight/

* touchpad led (if your touchpad has some led)
  - check presence of "dell-laptop::touchpad" in /sys/class/leds

* configuring keyboard backlight
  - check presence of "dell::kbd_backlight" in /sys/class/leds

* led for microphone mute
  - check presence of "platform::micmute" in /sys/class/leds

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [ PATCH v2 1/1] dell smbios driver : Consider Alienware a valid OEM String
  2020-10-09  8:26                     ` Pali Rohár
@ 2020-10-09  9:14                       ` Hans de Goede
  2020-10-09  9:24                         ` Pali Rohár
  0 siblings, 1 reply; 17+ messages in thread
From: Hans de Goede @ 2020-10-09  9:14 UTC (permalink / raw)
  To: Pali Rohár, Gerardo Esteban Malazdrewicz
  Cc: Limonciello, Mario, platform-driver-x86

Hi,

On 10/9/20 10:26 AM, Pali Rohár wrote:
> On Friday 09 October 2020 02:33:49 Gerardo Esteban Malazdrewicz wrote:
>> dell-laptop doesn't even load as is ('No such device')
>>
>> However, adding an entry in dell_device_table, based on dmidecode
>> output (Vendor: Alienware, Type: 10), allows it to load.
>>
>> But I don't know how to test for any of this additional functionality.
>>
>> What should I look for?
> 
> Hello! dell-laptop driver provides following features:
> 
> * rfkill interface for enabling/disabling wifi and bluetooth
>    - check presence of "*dell*" by /sbin/rfkill utility
> 
> * backlight interface for controlling display brightness
>    - check presence of "dell_backlight" in /sys/class/backlight/
> 
> * touchpad led (if your touchpad has some led)
>    - check presence of "dell-laptop::touchpad" in /sys/class/leds
> 
> * configuring keyboard backlight
>    - check presence of "dell::kbd_backlight" in /sys/class/leds
> 
> * led for microphone mute
>    - check presence of "platform::micmute" in /sys/class/leds

Thanks Pali, that is a great answer.

Note that the dell-wmi driver also depends on the
dell-smbios-base module, so you should also check if the
dell-wmi driver offers any additional functionality
on your laptop (if it does then that would also be a reason
to move forward with your dell-smbios-base patch).

Pali, can you perhaps make a similar feature list for
the dell-wmi driver ?

Regards,

Hans


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [ PATCH v2 1/1] dell smbios driver : Consider Alienware a valid OEM String
  2020-10-09  9:14                       ` Hans de Goede
@ 2020-10-09  9:24                         ` Pali Rohár
  2020-10-09  9:40                           ` Hans de Goede
  0 siblings, 1 reply; 17+ messages in thread
From: Pali Rohár @ 2020-10-09  9:24 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Gerardo Esteban Malazdrewicz, Limonciello, Mario, platform-driver-x86

On Friday 09 October 2020 11:14:33 Hans de Goede wrote:
> Hi,
> 
> On 10/9/20 10:26 AM, Pali Rohár wrote:
> > On Friday 09 October 2020 02:33:49 Gerardo Esteban Malazdrewicz wrote:
> > > dell-laptop doesn't even load as is ('No such device')
> > > 
> > > However, adding an entry in dell_device_table, based on dmidecode
> > > output (Vendor: Alienware, Type: 10), allows it to load.
> > > 
> > > But I don't know how to test for any of this additional functionality.
> > > 
> > > What should I look for?
> > 
> > Hello! dell-laptop driver provides following features:
> > 
> > * rfkill interface for enabling/disabling wifi and bluetooth
> >    - check presence of "*dell*" by /sbin/rfkill utility
> > 
> > * backlight interface for controlling display brightness
> >    - check presence of "dell_backlight" in /sys/class/backlight/
> > 
> > * touchpad led (if your touchpad has some led)
> >    - check presence of "dell-laptop::touchpad" in /sys/class/leds
> > 
> > * configuring keyboard backlight
> >    - check presence of "dell::kbd_backlight" in /sys/class/leds
> > 
> > * led for microphone mute
> >    - check presence of "platform::micmute" in /sys/class/leds
> 
> Thanks Pali, that is a great answer.
...
> Pali, can you perhaps make a similar feature list for
> the dell-wmi driver ?

dell-wmi is just listener for events delivered by WMI interface. All
events are currently delivered only via input device to userspace as
most of the events are key pressed or other similar events which maps to
input device. Looking at the code, there is one exception about event
KEY_KBDILLUMTOGGLE which is delivered to dell-laptop driver to notify it
when dell firmware itself decided to change keyboard backlight level.

...
> Note that the dell-wmi driver also depends on the
> dell-smbios-base module, so you should also check if the
> dell-wmi driver offers any additional functionality
> on your laptop (if it does then that would also be a reason
> to move forward with your dell-smbios-base patch).

For two Dell laptops (Dell Inspiron M5110 and Dell Vostro V131) it is
needed to call special SMBIOS function to enable receiving those WMI
events. Therefore dell-wmi checks via DMI table if that special call is
required then use dell-smbios-base module to issue needed call.

So theoretically if some key press events are not delivered on
particular dell laptop, it is a good idea to add it on that
dell_wmi_smbios_list, special SMBIOS call would be issued and check if
something is changed... But for now we know only 2 laptops which
required it (or better only 2 people complained that not all key press
events are delivered and verified that special SMBIOS call was
required).

> Regards,
> 
> Hans
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [ PATCH v2 1/1] dell smbios driver : Consider Alienware a valid OEM String
  2020-10-09  9:24                         ` Pali Rohár
@ 2020-10-09  9:40                           ` Hans de Goede
  0 siblings, 0 replies; 17+ messages in thread
From: Hans de Goede @ 2020-10-09  9:40 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Gerardo Esteban Malazdrewicz, Limonciello, Mario, platform-driver-x86

Hi,

On 10/9/20 11:24 AM, Pali Rohár wrote:
> On Friday 09 October 2020 11:14:33 Hans de Goede wrote:
>> Hi,
>>
>> On 10/9/20 10:26 AM, Pali Rohár wrote:
>>> On Friday 09 October 2020 02:33:49 Gerardo Esteban Malazdrewicz wrote:
>>>> dell-laptop doesn't even load as is ('No such device')
>>>>
>>>> However, adding an entry in dell_device_table, based on dmidecode
>>>> output (Vendor: Alienware, Type: 10), allows it to load.
>>>>
>>>> But I don't know how to test for any of this additional functionality.
>>>>
>>>> What should I look for?
>>>
>>> Hello! dell-laptop driver provides following features:
>>>
>>> * rfkill interface for enabling/disabling wifi and bluetooth
>>>     - check presence of "*dell*" by /sbin/rfkill utility
>>>
>>> * backlight interface for controlling display brightness
>>>     - check presence of "dell_backlight" in /sys/class/backlight/
>>>
>>> * touchpad led (if your touchpad has some led)
>>>     - check presence of "dell-laptop::touchpad" in /sys/class/leds
>>>
>>> * configuring keyboard backlight
>>>     - check presence of "dell::kbd_backlight" in /sys/class/leds
>>>
>>> * led for microphone mute
>>>     - check presence of "platform::micmute" in /sys/class/leds
>>
>> Thanks Pali, that is a great answer.
> ...
>> Pali, can you perhaps make a similar feature list for
>> the dell-wmi driver ?
> 
> dell-wmi is just listener for events delivered by WMI interface. All
> events are currently delivered only via input device to userspace as
> most of the events are key pressed or other similar events which maps to
> input device. Looking at the code, there is one exception about event
> KEY_KBDILLUMTOGGLE which is delivered to dell-laptop driver to notify it
> when dell firmware itself decided to change keyboard backlight level.
> 
> ...
>> Note that the dell-wmi driver also depends on the
>> dell-smbios-base module, so you should also check if the
>> dell-wmi driver offers any additional functionality
>> on your laptop (if it does then that would also be a reason
>> to move forward with your dell-smbios-base patch).
> 
> For two Dell laptops (Dell Inspiron M5110 and Dell Vostro V131) it is
> needed to call special SMBIOS function to enable receiving those WMI
> events. Therefore dell-wmi checks via DMI table if that special call is
> required then use dell-smbios-base module to issue needed call.
> 
> So theoretically if some key press events are not delivered on
> particular dell laptop, it is a good idea to add it on that
> dell_wmi_smbios_list, special SMBIOS call would be issued and check if
> something is changed... But for now we know only 2 laptops which
> required it (or better only 2 people complained that not all key press
> events are delivered and verified that special SMBIOS call was
> required).

Right, so Gerardo, I guess the laptop keyboard will have
some special hotkeys (possibly in combination with the "Fn")
keys as shortcuts for various things.

It would be good if you can verify that all those hotkeys
generate events.

What you can do is run "sudo evemu-record" and then select e.g.
first the "AT Translated Set 2 keyboard" device and see which
(special hotkey) keys generate events there. Then strike those
of the list to check and next check the "Dell WMI hotkeys"
device and see if any hotkeys report events there.

Note the display brightness up/down hotkeys might very well be
delivered through the "Video Bus" event. More in general you may
have some other input devices which deal with some of the hotkeys.

Regards,

Hans


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2020-10-09  9:40 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-03 11:52 [ PATCH: 1/1] dell smbios driver : Consider Alienware a valid OEM String Gerardo Esteban Malazdrewicz
2020-10-03 13:00 ` Hans de Goede
2020-10-05 12:45   ` Limonciello, Mario
2020-10-06  3:23     ` [ PATCH v2 " Gerardo Esteban Malazdrewicz
2020-10-07 14:21       ` Hans de Goede
2020-10-07 14:33         ` Pali Rohár
2020-10-07 15:53           ` Limonciello, Mario
2020-10-07 16:00             ` Hans de Goede
     [not found]             ` <45e82b6dabb591de630ac0e91a3ebb7937245fb1.camel@malazdrewicz.com.ar>
     [not found]               ` <DM6PR19MB26363563F46E95E50AD28854FA0A0@DM6PR19MB2636.namprd19.prod.outlook.com>
2020-10-07 20:38                 ` Hans de Goede
2020-10-07 21:26                   ` Limonciello, Mario
2020-10-07 21:30                     ` Hans de Goede
2020-10-07 21:33                       ` Limonciello, Mario
2020-10-09  5:33                   ` Gerardo Esteban Malazdrewicz
2020-10-09  8:26                     ` Pali Rohár
2020-10-09  9:14                       ` Hans de Goede
2020-10-09  9:24                         ` Pali Rohár
2020-10-09  9:40                           ` Hans de Goede

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.