[PATCHv3 1/2] platform/x86: dell-sysman: remove match on www.dell.com

[PATCHv3 1/2] platform/x86: dell-sysman: remove match on www.dell.com

[Crag Wang]

The URL is dynamic and may change according to the OEM. It was mainly used
for old systems that do not have "Dell System" in the OEM String.

Signed-off-by: Crag Wang <crag_wang@dell.com>
---
 drivers/platform/x86/dell/dell-wmi-sysman/sysman.c | 3 +--
 1 file changed, 1 insertion(+), 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..c05474f1ed70 100644
--- a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
+++ b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
@@ -520,8 +520,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)) {
+	if (!dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "Dell System", NULL)) {
 		pr_err("Unable to run on non-Dell system\n");
 		return -ENODEV;
 	}
-- 
2.43.0

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

[Crag Wang]

Alienware supports firmware-attributes and has its own OEM string.

Signed-off-by: Crag Wang <crag_wang@dell.com>
---
 drivers/platform/x86/dell/dell-wmi-sysman/sysman.c | 3 ++-
 1 file changed, 2 insertions(+), 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 c05474f1ed70..68c63e1fbd27 100644
--- a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
+++ b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
@@ -520,7 +520,8 @@ static int __init sysman_init(void)
 {
 	int ret = 0;
 
-	if (!dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "Dell System", NULL)) {
+	if (!dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "Dell System", 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: [PATCHv3 1/2] platform/x86: dell-sysman: remove match on www.dell.com

[Ilpo Järvinen]

On Fri, 4 Oct 2024, Crag Wang wrote:

> The URL is dynamic and may change according to the OEM. It was mainly used
> for old systems that do not have "Dell System" in the OEM String.
> 
> Signed-off-by: Crag Wang <crag_wang@dell.com>
> ---
>  drivers/platform/x86/dell/dell-wmi-sysman/sysman.c | 3 +--
>  1 file changed, 1 insertion(+), 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..c05474f1ed70 100644
> --- a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> +++ b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> @@ -520,8 +520,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)) {
> +	if (!dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "Dell System", NULL)) {

I suggested making the changes in the opposite order, that was to 
faciliate easy revert of the URL patch if needed which is not the case if
things are changed in this order.

-- 
 i.

Re: [PATCHv3 1/2] platform/x86: dell-sysman: remove match on www.dell.com

[Hans de Goede]

Hi,

On 4-Oct-24 4:41 AM, Crag Wang wrote:
> The URL is dynamic and may change according to the OEM. It was mainly used
> for old systems that do not have "Dell System" in the OEM String.

But those old systems presumably still exist somewhere out there, right ?

And if they still exist then we do still want to support them, so I'm
not sure why you think it is a good idea to drop this test ?

Adding the alienware match seems fine, dropping the URL match seems
like a bad idea unless you are 100% sure that there are no systems
out there which rely in this match to load dell-wmi-sysman.

Regards,

Hans



> 
> Signed-off-by: Crag Wang <crag_wang@dell.com>
> ---
>  drivers/platform/x86/dell/dell-wmi-sysman/sysman.c | 3 +--
>  1 file changed, 1 insertion(+), 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..c05474f1ed70 100644
> --- a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> +++ b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> @@ -520,8 +520,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)) {
> +	if (!dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "Dell System", NULL)) {
>  		pr_err("Unable to run on non-Dell system\n");
>  		return -ENODEV;
>  	}

RE: [PATCHv3 1/2] platform/x86: dell-sysman: remove match on www.dell.com

[Wang, Crag]

> But those old systems presumably still exist somewhere out there, right ?

Products with this driver have Dell System in place.

> And if they still exist then we do still want to support them, so I'm not sure why
> you think it is a good idea to drop this test ?

We can keep the check as it's harmless though the match should rely on Dell System
more than the URL, which is mainly for super older cases without Dell System.

> Adding the alienware match seems fine, dropping the URL match seems like a
> bad idea unless you are 100% sure that there are no systems out there which
> rely in this match to load dell-wmi-sysman.

I'll re-send with just the addition.

Crag