[PATCH] platform/x86: dell-sysman: add support for alienware products

[PATCH] platform/x86: dell-sysman: add support for alienware products

[Crag Wang]

Use SMBIOS Type 1 manfacturer instead OEM strings to verify product
compatibility. Also, add Alienware products to the support scope.

Signed-off-by: Crag Wang <crag_wang@dell.com>
---
 .../x86/dell/dell-wmi-sysman/sysman.c         | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
index 9def7983d7d6..2f3f7e307b29 100644
--- a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
+++ b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
@@ -516,12 +516,27 @@ static int init_bios_attributes(int attr_type, const char *guid)
 	return retval;
 }
 
+static const struct dmi_system_id sysman_dev_table[] __initconst = {
+	{
+		.ident = "Dell Inc.",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+		},
+	},
+	{
+		.ident = "Alienware",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Alienware"),
+		},
+	},
+	{}
+};
+
 static int __init sysman_init(void)
 {
 	int ret = 0;
 
-	if (!dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "Dell System", NULL) &&
-	    !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "www.dell.com", NULL)) {
+	if (!dmi_check_system(sysman_dev_table)) {
 		pr_err("Unable to run on non-Dell system\n");
 		return -ENODEV;
 	}
-- 
2.43.0

Re: [PATCH] platform/x86: dell-sysman: add support for alienware products

[Mario Limonciello]

On 9/23/2024 01:36, Crag Wang wrote:
> Use SMBIOS Type 1 manfacturer instead OEM strings to verify product
> compatibility. Also, add Alienware products to the support scope.

Alienware systems don't use OEM string "Dell System"?  TIL.

> 
> Signed-off-by: Crag Wang <crag_wang@dell.com>
> ---
>   .../x86/dell/dell-wmi-sysman/sysman.c         | 19 +++++++++++++++++--
>   1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> index 9def7983d7d6..2f3f7e307b29 100644
> --- a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> +++ b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> @@ -516,12 +516,27 @@ static int init_bios_attributes(int attr_type, const char *guid)
>   	return retval;
>   }
>   
> +static const struct dmi_system_id sysman_dev_table[] __initconst = {
> +	{
> +		.ident = "Dell Inc.",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> +		},
> +	},
> +	{
> +		.ident = "Alienware",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Alienware"),
> +		},
> +	},
> +	{}
> +};
> +
>   static int __init sysman_init(void)
>   {
>   	int ret = 0;
>   
> -	if (!dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "Dell System", NULL) &&
> -	    !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "www.dell.com", NULL)) {
> +	if (!dmi_check_system(sysman_dev_table)) {

Would it perhaps make sense to be an && condition instead of remove the 
dmi_find_device() conditions?

IE:

if (!dmi_find_device() && !dmi_find_device() && !dmi_check_system())

Then you can continue to cover anything that has been rebranded too.

>   		pr_err("Unable to run on non-Dell system\n");
>   		return -ENODEV;
>   	}

RE: [PATCH] platform/x86: dell-sysman: add support for alienware products

[Wang, Crag]

>
> Would it perhaps make sense to be an && condition instead of remove the
> dmi_find_device() conditions?
>
> IE:
>
> if (!dmi_find_device() && !dmi_find_device() && !dmi_check_system())
>
> Then you can continue to cover anything that has been rebranded too.
>

I will do v2 to add the new string and remove the URL check to accommodate
the OEM use case.


Internal Use - Confidential

[PATCHv2 1/1] platform/x86: dell-sysman: add support for alienware products

[Crag Wang]

Remove the URL check and add alienware string for broader support.

Signed-off-by: Crag Wang <crag_wang@dell.com>
---
 drivers/platform/x86/dell/dell-wmi-sysman/sysman.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
index 9def7983d7d6..68c63e1fbd27 100644
--- a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
+++ b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
@@ -521,7 +521,7 @@ static int __init sysman_init(void)
 	int ret = 0;
 
 	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, "Alienware", NULL)) {
 		pr_err("Unable to run on non-Dell system\n");
 		return -ENODEV;
 	}
-- 
2.43.0

Re: [PATCHv2 1/1] platform/x86: dell-sysman: add support for alienware products

[Mario Limonciello]

On 9/24/2024 00:02, Crag Wang wrote:
> Remove the URL check and add alienware string for broader support.
> 
> Signed-off-by: Crag Wang <crag_wang@dell.com>
> ---
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
>   drivers/platform/x86/dell/dell-wmi-sysman/sysman.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> index 9def7983d7d6..68c63e1fbd27 100644
> --- a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> +++ b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> @@ -521,7 +521,7 @@ static int __init sysman_init(void)
>   	int ret = 0;
>   
>   	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, "Alienware", NULL)) {
>   		pr_err("Unable to run on non-Dell system\n");
>   		return -ENODEV;
>   	}

RE: [PATCHv2 1/1] platform/x86: dell-sysman: add support for alienware products

[Tudor, Laurentiu]

Hi,


Internal Use - Confidential
-----Original Message-----
From: Mario Limonciello <mario.limonciello@amd.com>
Sent: Tuesday, September 24, 2024 4:24 PM

On 9/24/2024 00:02, Crag Wang wrote:
> Remove the URL check and add alienware string for broader support.

Couple of newbie questions: what's the reason for dropping the URL check? Would it make sense to include the reason in the commit msg?

---
Thanks & Best Regards, Laurentiu

> Signed-off-by: Crag Wang <crag_wang@dell.com>
> ---
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
>   drivers/platform/x86/dell/dell-wmi-sysman/sysman.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> index 9def7983d7d6..68c63e1fbd27 100644
> --- a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> +++ b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> @@ -521,7 +521,7 @@ static int __init sysman_init(void)
>       int ret = 0;
>
>       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, "Alienware", NULL)) {
>               pr_err("Unable to run on non-Dell system\n");
>               return -ENODEV;
>       }

RE: [PATCHv2 1/1] platform/x86: dell-sysman: add support for alienware products

[Wang, Crag]

>
> Couple of newbie questions: what's the reason for dropping the URL check?
> Would it make sense to include the reason in the commit msg?
>

URL in type 11 is subject to change according to OEM ID Module specification, for
standard Dell and OEM inclusive the string 'Dell System' should be sufficient.

Crag


Internal Use - Confidential

RE: [PATCHv2 1/1] platform/x86: dell-sysman: add support for alienware products

[Ilpo Järvinen]

On Tue, 24 Sep 2024, Wang, Crag wrote:

> > Couple of newbie questions: what's the reason for dropping the URL check?
> > Would it make sense to include the reason in the commit msg?
> 
> URL in type 11 is subject to change according to OEM ID Module specification, for
> standard Dell and OEM inclusive the string 'Dell System' should be sufficient.

Please add such information into the commit message so it gets properly 
recorded. But I'd prefer this to be split into two patches, one adding 
the Alienware and another that removes the URL making it easier to revert 
only the latter one if the need arises.

-- 
 i.

> Internal Use - Confidential

RE: [PATCH] platform/x86: dell-sysman: add support for alienware products

[Ksr, Prasanth]

> [EXTERNAL EMAIL]


Internal Use - Confidential
On 9/23/2024 01:36, Crag Wang wrote:
> Use SMBIOS Type 1 manfacturer instead OEM strings to verify product
> compatibility. Also, add Alienware products to the support scope.

> Alienware systems don't use OEM string "Dell System"?  TIL.
If the system is Dell-developed, the string value is “Dell Inc.”. System management software can identify a Dell system by checking for “Dell ” as the first five characters of this string. Alienware systems should show “Alienware”.
This is according to the Dell implementation of SMBIOS.

>
> Signed-off-by: Crag Wang <crag_wang@dell.com>
> ---
>   .../x86/dell/dell-wmi-sysman/sysman.c         | 19 +++++++++++++++++--
>   1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> index 9def7983d7d6..2f3f7e307b29 100644
> --- a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> +++ b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> @@ -516,12 +516,27 @@ static int init_bios_attributes(int attr_type, const char *guid)
>       return retval;
>   }
>
> +static const struct dmi_system_id sysman_dev_table[] __initconst = {
> +     {
> +             .ident = "Dell Inc.",
> +             .matches = {
> +                     DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> +             },
> +     },
> +     {
> +             .ident = "Alienware",
> +             .matches = {
> +                     DMI_MATCH(DMI_SYS_VENDOR, "Alienware"),
> +             },
> +     },
> +     {}
> +};
> +
>   static int __init sysman_init(void)
>   {
>       int ret = 0;
>
> -     if (!dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "Dell System", NULL) &&
> -         !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "www.dell.com", NULL)) {
> +     if (!dmi_check_system(sysman_dev_table)) {

> Would it perhaps make sense to be an && condition instead of remove the
> dmi_find_device() conditions?

Agreed on this.

> IE:

> if (!dmi_find_device() && !dmi_find_device() && !dmi_check_system())

> Then you can continue to cover anything that has been rebranded too.

>               pr_err("Unable to run on non-Dell system\n");
>               return -ENODEV;
>       }