[PATCH 1/2] dell-smbios-base: Extends support to Alienware products

[PATCH 1/2] dell-smbios-base: Extends support to Alienware products

[Kurt Borja]

Fixes the following error:

dell_smbios: Unable to run on non-Dell system

Which is triggered after dell-wmi driver fails to initialize on
Alienware systems, as it depends on dell-smbios.

This effectively adds dell-wmi and dell-smbios support to Alienware
products.

Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
 drivers/platform/x86/dell/dell-smbios-base.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/platform/x86/dell/dell-smbios-base.c b/drivers/platform/x86/dell/dell-smbios-base.c
index 73e41eb69..01c72b91a 100644
--- a/drivers/platform/x86/dell/dell-smbios-base.c
+++ b/drivers/platform/x86/dell/dell-smbios-base.c
@@ -576,6 +576,7 @@ 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, "Alienware", NULL) &&
 	    !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "www.dell.com", NULL)) {
 		pr_err("Unable to run on non-Dell system\n");
 		return -ENODEV;
-- 
2.47.0

[PATCH 2/2] dell-wmi-base: Handle Win-key Lock/Unlock events

[Kurt Borja]

Some Alienware devices have a key that locks/unlocks the Win-key. This
key triggers a WMI event that should be ignored, as it's handled
internally by the firmware.

Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
 drivers/platform/x86/dell/dell-wmi-base.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/platform/x86/dell/dell-wmi-base.c b/drivers/platform/x86/dell/dell-wmi-base.c
index 502783a7a..37fc0371a 100644
--- a/drivers/platform/x86/dell/dell-wmi-base.c
+++ b/drivers/platform/x86/dell/dell-wmi-base.c
@@ -80,6 +80,12 @@ static const struct dmi_system_id dell_wmi_smbios_list[] __initconst = {
 static const struct key_entry dell_wmi_keymap_type_0000[] = {
 	{ KE_IGNORE, 0x003a, { KEY_CAPSLOCK } },
 
+	/* Win-key Lock */
+	{ KE_IGNORE, 0xe000, {KEY_RESERVED} },
+
+	/* Win-key Unlock */
+	{ KE_IGNORE, 0xe001, {KEY_RESERVED} },
+
 	/* Key code is followed by brightness level */
 	{ KE_KEY,    0xe005, { KEY_BRIGHTNESSDOWN } },
 	{ KE_KEY,    0xe006, { KEY_BRIGHTNESSUP } },
-- 
2.47.0

Re: [PATCH 2/2] dell-wmi-base: Handle Win-key Lock/Unlock events

[Pali Rohár]

On Wednesday 30 October 2024 15:15:33 Kurt Borja wrote:
> Some Alienware devices have a key that locks/unlocks the Win-key. This

Please specify (in comment / commit message) which devices. It would
help other developers in future to track for which device is this event
needed.

> key triggers a WMI event that should be ignored, as it's handled
> internally by the firmware.

Can be this handling in FW ignored? So OS can use this key for any other
functionality?

Anyway, what is that Win-key and its lock?

> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> ---
>  drivers/platform/x86/dell/dell-wmi-base.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/platform/x86/dell/dell-wmi-base.c b/drivers/platform/x86/dell/dell-wmi-base.c
> index 502783a7a..37fc0371a 100644
> --- a/drivers/platform/x86/dell/dell-wmi-base.c
> +++ b/drivers/platform/x86/dell/dell-wmi-base.c
> @@ -80,6 +80,12 @@ static const struct dmi_system_id dell_wmi_smbios_list[] __initconst = {
>  static const struct key_entry dell_wmi_keymap_type_0000[] = {
>  	{ KE_IGNORE, 0x003a, { KEY_CAPSLOCK } },
>  
> +	/* Win-key Lock */
> +	{ KE_IGNORE, 0xe000, {KEY_RESERVED} },

nit: code style: spaces around KEY_RESERVED.

Is not there some better constant for this KEY_*?

> +
> +	/* Win-key Unlock */
> +	{ KE_IGNORE, 0xe001, {KEY_RESERVED} },
> +
>  	/* Key code is followed by brightness level */
>  	{ KE_KEY,    0xe005, { KEY_BRIGHTNESSDOWN } },
>  	{ KE_KEY,    0xe006, { KEY_BRIGHTNESSUP } },
> -- 
> 2.47.0
> 

Re: [PATCH 2/2] dell-wmi-base: Handle Win-key Lock/Unlock events

[Kurt Borja]

On Wed, Oct 30, 2024 at 07:34:36PM +0100, Pali Rohár wrote:
> On Wednesday 30 October 2024 15:15:33 Kurt Borja wrote:
> > Some Alienware devices have a key that locks/unlocks the Win-key. This
> 
> Please specify (in comment / commit message) which devices. It would
> help other developers in future to track for which device is this event
> needed.

I will!

> 
> > key triggers a WMI event that should be ignored, as it's handled
> > internally by the firmware.
> 
> Can be this handling in FW ignored? So OS can use this key for any other
> functionality?

Probably not. FW locks the Win-key regardless of how the event is
handled.

> 
> Anyway, what is that Win-key and its lock?

Win-key is the LEFTMETA key and the lock key is the RIGHTMETA key.

> 
> > Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> > ---
> >  drivers/platform/x86/dell/dell-wmi-base.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/platform/x86/dell/dell-wmi-base.c b/drivers/platform/x86/dell/dell-wmi-base.c
> > index 502783a7a..37fc0371a 100644
> > --- a/drivers/platform/x86/dell/dell-wmi-base.c
> > +++ b/drivers/platform/x86/dell/dell-wmi-base.c
> > @@ -80,6 +80,12 @@ static const struct dmi_system_id dell_wmi_smbios_list[] __initconst = {
> >  static const struct key_entry dell_wmi_keymap_type_0000[] = {
> >  	{ KE_IGNORE, 0x003a, { KEY_CAPSLOCK } },
> >  
> > +	/* Win-key Lock */
> > +	{ KE_IGNORE, 0xe000, {KEY_RESERVED} },
> 
> nit: code style: spaces around KEY_RESERVED.

I will fix it.

> 
> Is not there some better constant for this KEY_*?

We could assign it KEY_RIGHTMETA.

> 
> > +
> > +	/* Win-key Unlock */
> > +	{ KE_IGNORE, 0xe001, {KEY_RESERVED} },
> > +
> >  	/* Key code is followed by brightness level */
> >  	{ KE_KEY,    0xe005, { KEY_BRIGHTNESSDOWN } },
> >  	{ KE_KEY,    0xe006, { KEY_BRIGHTNESSUP } },
> > -- 
> > 2.47.0
> > 

Re: [PATCH 2/2] dell-wmi-base: Handle Win-key Lock/Unlock events

[Pali Rohár]

On Wednesday 30 October 2024 17:11:40 Kurt Borja wrote:
> On Wed, Oct 30, 2024 at 07:34:36PM +0100, Pali Rohár wrote:
> > On Wednesday 30 October 2024 15:15:33 Kurt Borja wrote:
> > > Some Alienware devices have a key that locks/unlocks the Win-key. This
> > 
> > Please specify (in comment / commit message) which devices. It would
> > help other developers in future to track for which device is this event
> > needed.
> 
> I will!
> 
> > 
> > > key triggers a WMI event that should be ignored, as it's handled
> > > internally by the firmware.
> > 
> > Can be this handling in FW ignored? So OS can use this key for any other
> > functionality?
> 
> Probably not. FW locks the Win-key regardless of how the event is
> handled.
> 
> > 
> > Anyway, what is that Win-key and its lock?
> 
> Win-key is the LEFTMETA key and the lock key is the RIGHTMETA key.

Ok, thanks for info.

So if the firmware handles and process RIGHTMETA key, it means that the
RIGHTMETA key is not usable for generic purposes on this machine?

> > 
> > > Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> > > ---
> > >  drivers/platform/x86/dell/dell-wmi-base.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/drivers/platform/x86/dell/dell-wmi-base.c b/drivers/platform/x86/dell/dell-wmi-base.c
> > > index 502783a7a..37fc0371a 100644
> > > --- a/drivers/platform/x86/dell/dell-wmi-base.c
> > > +++ b/drivers/platform/x86/dell/dell-wmi-base.c
> > > @@ -80,6 +80,12 @@ static const struct dmi_system_id dell_wmi_smbios_list[] __initconst = {
> > >  static const struct key_entry dell_wmi_keymap_type_0000[] = {
> > >  	{ KE_IGNORE, 0x003a, { KEY_CAPSLOCK } },
> > >  
> > > +	/* Win-key Lock */
> > > +	{ KE_IGNORE, 0xe000, {KEY_RESERVED} },
> > 
> > nit: code style: spaces around KEY_RESERVED.
> 
> I will fix it.
> 
> > 
> > Is not there some better constant for this KEY_*?
> 
> We could assign it KEY_RIGHTMETA.

Just to note that if the key is marked with KE_IGNORE, its value is not
used. But for documentation purposes it is a good idea to have written
there something other than KEY_RESERVED -- if it is possible.

For example it can be useful if somebody in future figure out how to
turn off processing of this key in firmware...

> > 
> > > +
> > > +	/* Win-key Unlock */
> > > +	{ KE_IGNORE, 0xe001, {KEY_RESERVED} },
> > > +
> > >  	/* Key code is followed by brightness level */
> > >  	{ KE_KEY,    0xe005, { KEY_BRIGHTNESSDOWN } },
> > >  	{ KE_KEY,    0xe006, { KEY_BRIGHTNESSUP } },
> > > -- 
> > > 2.47.0
> > > 

Re: [PATCH 2/2] dell-wmi-base: Handle Win-key Lock/Unlock events

[Kurt Borja]

On Wed, Oct 30, 2024 at 09:20:29PM +0100, Pali Rohár wrote:
> On Wednesday 30 October 2024 17:11:40 Kurt Borja wrote:
> > On Wed, Oct 30, 2024 at 07:34:36PM +0100, Pali Rohár wrote:
> > > On Wednesday 30 October 2024 15:15:33 Kurt Borja wrote:
> > > > Some Alienware devices have a key that locks/unlocks the Win-key. This
> > > 
> > > Please specify (in comment / commit message) which devices. It would
> > > help other developers in future to track for which device is this event
> > > needed.
> > 
> > I will!
> > 
> > > 
> > > > key triggers a WMI event that should be ignored, as it's handled
> > > > internally by the firmware.
> > > 
> > > Can be this handling in FW ignored? So OS can use this key for any other
> > > functionality?
> > 
> > Probably not. FW locks the Win-key regardless of how the event is
> > handled.
> > 
> > > 
> > > Anyway, what is that Win-key and its lock?
> > 
> > Win-key is the LEFTMETA key and the lock key is the RIGHTMETA key.
> 
> Ok, thanks for info.
> 
> So if the firmware handles and process RIGHTMETA key, it means that the
> RIGHTMETA key is not usable for generic purposes on this machine?

Yes, it is not. Unless, of course we find a way to change FW default
behavior.

> 
> > > 
> > > > Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> > > > ---
> > > >  drivers/platform/x86/dell/dell-wmi-base.c | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > > 
> > > > diff --git a/drivers/platform/x86/dell/dell-wmi-base.c b/drivers/platform/x86/dell/dell-wmi-base.c
> > > > index 502783a7a..37fc0371a 100644
> > > > --- a/drivers/platform/x86/dell/dell-wmi-base.c
> > > > +++ b/drivers/platform/x86/dell/dell-wmi-base.c
> > > > @@ -80,6 +80,12 @@ static const struct dmi_system_id dell_wmi_smbios_list[] __initconst = {
> > > >  static const struct key_entry dell_wmi_keymap_type_0000[] = {
> > > >  	{ KE_IGNORE, 0x003a, { KEY_CAPSLOCK } },
> > > >  
> > > > +	/* Win-key Lock */
> > > > +	{ KE_IGNORE, 0xe000, {KEY_RESERVED} },
> > > 
> > > nit: code style: spaces around KEY_RESERVED.
> > 
> > I will fix it.
> > 
> > > 
> > > Is not there some better constant for this KEY_*?
> > 
> > We could assign it KEY_RIGHTMETA.
> 
> Just to note that if the key is marked with KE_IGNORE, its value is not
> used. But for documentation purposes it is a good idea to have written
> there something other than KEY_RESERVED -- if it is possible.
> 
> For example it can be useful if somebody in future figure out how to
> turn off processing of this key in firmware...

Thanks I will replace it with KEY_RIGHTMETA.

Kurt

> 
> > > 
> > > > +
> > > > +	/* Win-key Unlock */
> > > > +	{ KE_IGNORE, 0xe001, {KEY_RESERVED} },
> > > > +
> > > >  	/* Key code is followed by brightness level */
> > > >  	{ KE_KEY,    0xe005, { KEY_BRIGHTNESSDOWN } },
> > > >  	{ KE_KEY,    0xe006, { KEY_BRIGHTNESSUP } },
> > > > -- 
> > > > 2.47.0
> > > > 

Re: [PATCH 2/2] dell-wmi-base: Handle Win-key Lock/Unlock events

[Pali Rohár]

On Wednesday 30 October 2024 17:37:38 Kurt Borja wrote:
> On Wed, Oct 30, 2024 at 09:20:29PM +0100, Pali Rohár wrote:
> > On Wednesday 30 October 2024 17:11:40 Kurt Borja wrote:
> > > On Wed, Oct 30, 2024 at 07:34:36PM +0100, Pali Rohár wrote:
> > > > On Wednesday 30 October 2024 15:15:33 Kurt Borja wrote:
> > > > > Some Alienware devices have a key that locks/unlocks the Win-key. This
> > > > 
> > > > Please specify (in comment / commit message) which devices. It would
> > > > help other developers in future to track for which device is this event
> > > > needed.
> > > 
> > > I will!
> > > 
> > > > 
> > > > > key triggers a WMI event that should be ignored, as it's handled
> > > > > internally by the firmware.
> > > > 
> > > > Can be this handling in FW ignored? So OS can use this key for any other
> > > > functionality?
> > > 
> > > Probably not. FW locks the Win-key regardless of how the event is
> > > handled.
> > > 
> > > > 
> > > > Anyway, what is that Win-key and its lock?
> > > 
> > > Win-key is the LEFTMETA key and the lock key is the RIGHTMETA key.
> > 
> > Ok, thanks for info.
> > 
> > So if the firmware handles and process RIGHTMETA key, it means that the
> > RIGHTMETA key is not usable for generic purposes on this machine?
> 
> Yes, it is not. Unless, of course we find a way to change FW default
> behavior.

That is sad... and also (negatively) surprising for me that another
generic-purpose key is behaving in this way. Mentioning this in commit
message would be useful as this is really something unexpected.

> > 
> > > > 
> > > > > Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> > > > > ---
> > > > >  drivers/platform/x86/dell/dell-wmi-base.c | 6 ++++++
> > > > >  1 file changed, 6 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/platform/x86/dell/dell-wmi-base.c b/drivers/platform/x86/dell/dell-wmi-base.c
> > > > > index 502783a7a..37fc0371a 100644
> > > > > --- a/drivers/platform/x86/dell/dell-wmi-base.c
> > > > > +++ b/drivers/platform/x86/dell/dell-wmi-base.c
> > > > > @@ -80,6 +80,12 @@ static const struct dmi_system_id dell_wmi_smbios_list[] __initconst = {
> > > > >  static const struct key_entry dell_wmi_keymap_type_0000[] = {
> > > > >  	{ KE_IGNORE, 0x003a, { KEY_CAPSLOCK } },
> > > > >  
> > > > > +	/* Win-key Lock */
> > > > > +	{ KE_IGNORE, 0xe000, {KEY_RESERVED} },
> > > > 
> > > > nit: code style: spaces around KEY_RESERVED.
> > > 
> > > I will fix it.
> > > 
> > > > 
> > > > Is not there some better constant for this KEY_*?
> > > 
> > > We could assign it KEY_RIGHTMETA.
> > 
> > Just to note that if the key is marked with KE_IGNORE, its value is not
> > used. But for documentation purposes it is a good idea to have written
> > there something other than KEY_RESERVED -- if it is possible.
> > 
> > For example it can be useful if somebody in future figure out how to
> > turn off processing of this key in firmware...
> 
> Thanks I will replace it with KEY_RIGHTMETA.
> 
> Kurt
> 
> > 
> > > > 
> > > > > +
> > > > > +	/* Win-key Unlock */
> > > > > +	{ KE_IGNORE, 0xe001, {KEY_RESERVED} },
> > > > > +
> > > > >  	/* Key code is followed by brightness level */
> > > > >  	{ KE_KEY,    0xe005, { KEY_BRIGHTNESSDOWN } },
> > > > >  	{ KE_KEY,    0xe006, { KEY_BRIGHTNESSUP } },
> > > > > -- 
> > > > > 2.47.0
> > > > > 

Re: [PATCH 1/2] dell-smbios-base: Extends support to Alienware products

[Pali Rohár]

On Wednesday 30 October 2024 15:12:45 Kurt Borja wrote:
> Fixes the following error:
> 
> dell_smbios: Unable to run on non-Dell system
> 
> Which is triggered after dell-wmi driver fails to initialize on
> Alienware systems, as it depends on dell-smbios.
> 
> This effectively adds dell-wmi and dell-smbios support to Alienware
> products.
> 
> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> ---
>  drivers/platform/x86/dell/dell-smbios-base.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/platform/x86/dell/dell-smbios-base.c b/drivers/platform/x86/dell/dell-smbios-base.c
> index 73e41eb69..01c72b91a 100644
> --- a/drivers/platform/x86/dell/dell-smbios-base.c
> +++ b/drivers/platform/x86/dell/dell-smbios-base.c
> @@ -576,6 +576,7 @@ 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, "Alienware", NULL) &&

Are we sure that all devices with "Alienware" OEM string supports this SMBIOS API?

>  	    !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "www.dell.com", NULL)) {
>  		pr_err("Unable to run on non-Dell system\n");
>  		return -ENODEV;
> -- 
> 2.47.0
> 

Re: [PATCH 1/2] dell-smbios-base: Extends support to Alienware products

[Kurt Borja]

On Wed, Oct 30, 2024 at 07:26:13PM +0100, Pali Rohár wrote:
> On Wednesday 30 October 2024 15:12:45 Kurt Borja wrote:
> > Fixes the following error:
> > 
> > dell_smbios: Unable to run on non-Dell system
> > 
> > Which is triggered after dell-wmi driver fails to initialize on
> > Alienware systems, as it depends on dell-smbios.
> > 
> > This effectively adds dell-wmi and dell-smbios support to Alienware
> > products.
> > 
> > Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> > ---
> >  drivers/platform/x86/dell/dell-smbios-base.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/platform/x86/dell/dell-smbios-base.c b/drivers/platform/x86/dell/dell-smbios-base.c
> > index 73e41eb69..01c72b91a 100644
> > --- a/drivers/platform/x86/dell/dell-smbios-base.c
> > +++ b/drivers/platform/x86/dell/dell-smbios-base.c
> > @@ -576,6 +576,7 @@ 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, "Alienware", NULL) &&
> 
> Are we sure that all devices with "Alienware" OEM string supports this SMBIOS API?

No, I am not sure.

However, I believe this driver is intended for general Dell SMBIOS 
implementations and automatically checks for support. I know this doesn't
necessarily extend to Alienware, so I hope some of the Dell maintainers
could shine some light on us.

I tested this on an Alienware x15 R1 and only some SMBIOS features
work, but no errors and dell-wmi-base works perfectly.

> 
> >  	    !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "www.dell.com", NULL)) {
> >  		pr_err("Unable to run on non-Dell system\n");
> >  		return -ENODEV;
> > -- 
> > 2.47.0
> > 

Re: [PATCH 1/2] dell-smbios-base: Extends support to Alienware products

[Pali Rohár]

On Wednesday 30 October 2024 17:01:11 Kurt Borja wrote:
> On Wed, Oct 30, 2024 at 07:26:13PM +0100, Pali Rohár wrote:
> > On Wednesday 30 October 2024 15:12:45 Kurt Borja wrote:
> > > Fixes the following error:
> > > 
> > > dell_smbios: Unable to run on non-Dell system
> > > 
> > > Which is triggered after dell-wmi driver fails to initialize on
> > > Alienware systems, as it depends on dell-smbios.
> > > 
> > > This effectively adds dell-wmi and dell-smbios support to Alienware
> > > products.
> > > 
> > > Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> > > ---
> > >  drivers/platform/x86/dell/dell-smbios-base.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/drivers/platform/x86/dell/dell-smbios-base.c b/drivers/platform/x86/dell/dell-smbios-base.c
> > > index 73e41eb69..01c72b91a 100644
> > > --- a/drivers/platform/x86/dell/dell-smbios-base.c
> > > +++ b/drivers/platform/x86/dell/dell-smbios-base.c
> > > @@ -576,6 +576,7 @@ 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, "Alienware", NULL) &&
> > 
> > Are we sure that all devices with "Alienware" OEM string supports this SMBIOS API?
> 
> No, I am not sure.
> 
> However, I believe this driver is intended for general Dell SMBIOS 
> implementations and automatically checks for support. I know this doesn't
> necessarily extend to Alienware, so I hope some of the Dell maintainers
> could shine some light on us.
> 
> I tested this on an Alienware x15 R1 and only some SMBIOS features
> work, but no errors and dell-wmi-base works perfectly.

This is good to know. You should write into commit message on which
platform you have tested this change. In case there would be some issues
in future, it will help to for example whitelist working machines.

I asked for this because for example dell hwmon driver has explicit list
of working and non-working devices. And on some dell some machines just
calling hwmon API cause issues...

So it is not just theoretical problem that allowing all devices from
Dell with some generic OEM string can cause issues.

> > 
> > >  	    !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "www.dell.com", NULL)) {
> > >  		pr_err("Unable to run on non-Dell system\n");
> > >  		return -ENODEV;
> > > -- 
> > > 2.47.0
> > > 

Re: [PATCH 1/2] dell-smbios-base: Extends support to Alienware products

[Kurt Borja]

On Wed, Oct 30, 2024 at 09:07:29PM +0100, Pali Rohár wrote:
> On Wednesday 30 October 2024 17:01:11 Kurt Borja wrote:
> > On Wed, Oct 30, 2024 at 07:26:13PM +0100, Pali Rohár wrote:
> > > On Wednesday 30 October 2024 15:12:45 Kurt Borja wrote:
> > > > Fixes the following error:
> > > > 
> > > > dell_smbios: Unable to run on non-Dell system
> > > > 
> > > > Which is triggered after dell-wmi driver fails to initialize on
> > > > Alienware systems, as it depends on dell-smbios.
> > > > 
> > > > This effectively adds dell-wmi and dell-smbios support to Alienware
> > > > products.
> > > > 
> > > > Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> > > > ---
> > > >  drivers/platform/x86/dell/dell-smbios-base.c | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > > 
> > > > diff --git a/drivers/platform/x86/dell/dell-smbios-base.c b/drivers/platform/x86/dell/dell-smbios-base.c
> > > > index 73e41eb69..01c72b91a 100644
> > > > --- a/drivers/platform/x86/dell/dell-smbios-base.c
> > > > +++ b/drivers/platform/x86/dell/dell-smbios-base.c
> > > > @@ -576,6 +576,7 @@ 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, "Alienware", NULL) &&
> > > 
> > > Are we sure that all devices with "Alienware" OEM string supports this SMBIOS API?
> > 
> > No, I am not sure.
> > 
> > However, I believe this driver is intended for general Dell SMBIOS 
> > implementations and automatically checks for support. I know this doesn't
> > necessarily extend to Alienware, so I hope some of the Dell maintainers
> > could shine some light on us.
> > 
> > I tested this on an Alienware x15 R1 and only some SMBIOS features
> > work, but no errors and dell-wmi-base works perfectly.
> 
> This is good to know. You should write into commit message on which
> platform you have tested this change. In case there would be some issues
> in future, it will help to for example whitelist working machines.

Perfect. I will.

> 
> I asked for this because for example dell hwmon driver has explicit list
> of working and non-working devices. And on some dell some machines just
> calling hwmon API cause issues...
> 
> So it is not just theoretical problem that allowing all devices from
> Dell with some generic OEM string can cause issues.

Yes, makes sense.

Also it is worth to mention that I checked a bunch of acpidumps of
different Dell models. I found three main ACPI implementations shared
between Dell's G-Series (which uses OEM STRING "Dell System") and 
Alienware X and M-Series. Obviously there might be some key differences I'm
ignoring, but it tells me the SMBIOS interface is probably implemented
similarily.

These three laptop series are all relatively new, this probably doesn't
apply to older Alienware devices.

Kurt

> 
> > > 
> > > >  	    !dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "www.dell.com", NULL)) {
> > > >  		pr_err("Unable to run on non-Dell system\n");
> > > >  		return -ENODEV;
> > > > -- 
> > > > 2.47.0
> > > >