hp-wmi rfkill change breaks systems

hp-wmi rfkill change breaks systems

[Matthew Garrett]

Hi Alex,

997daa1bd9aca412ab97955a35b26c460c0ec7a4 seems to break some systems 
(https://bugzilla.kernel.org/show_bug.cgi?id=69131 for example). The 
simplest approach seems to be to simply do something like the following:

@@ -883,8 +883,8 @@ static int __init hp_wmi_bios_setup(struct 
platform_device *device)
 	gps_rfkill = NULL;
 	rfkill2_count = 0;
 
-	if (hp_wmi_bios_2009_later() || hp_wmi_rfkill_setup(device))
-		hp_wmi_rfkill2_setup(device);
+	if (hp_wmi_rfkill2_setup(device))
+		hp_wmi_rfkill_setup(device);

Do you know if there are any cases that would be broken by this?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: hp-wmi rfkill change breaks systems

[Alex Hung]

Hi Matthew,

From the above code, I supposehp_wmi_rfkill_setup(device) successes
and hp_wmi_rfkill2_setup(device) fails. As hp_wmi_rfkill_setup(device)
does not run with 997daa1bd9aca412ab97955a35b26c460c0ec7a4.

From HP, it was claimed that HP WMI 0x05 is deprecated and replaced by
HP WMI 0x1B after 2009; however, this claim seems not true on on
nx6325. I would say this is a BIOS bug on either HP WMI 0x0D or WMI
0x1B.

The above code looks great on this bug except one problem:

On newer systems, WMI 0x1B (used in hp_wmi_rfkill2_setup) is also
depreciated with _OSI("Windows 2012") = TRUE, and it will return 04h
(invalid command type)*.

However, The Get for WMI 0x05 (used in hp_wmi_rfkill_setup) works but
Set for WMI 0x05 does not. If we are using
+       if (hp_wmi_rfkill2_setup(device))
+               hp_wmi_rfkill_setup(device);
we will have hp-wifi / hp-bluetooth in rfkill but we cannot change
their status on those systems.

I think a better solution may need to include testing both Set and Get
for HP WMI 0x05 in hp_wmi_rfkill_setup. We may be able to revert
997daa1bd9aca412ab97955a35b26c460c0ec7a4.

I will verify the above theory and update accordingly.

PS*: There needs no HP WMI for wireless. The wireless control is moved
to "HPQ6001" that I previous sent.

On Tue, Jan 21, 2014 at 11:17 PM, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> Hi Alex,
>
> 997daa1bd9aca412ab97955a35b26c460c0ec7a4 seems to break some systems
> (https://bugzilla.kernel.org/show_bug.cgi?id=69131 for example). The
> simplest approach seems to be to simply do something like the following:
>
> @@ -883,8 +883,8 @@ static int __init hp_wmi_bios_setup(struct
> platform_device *device)
>         gps_rfkill = NULL;
>         rfkill2_count = 0;
>
> -       if (hp_wmi_bios_2009_later() || hp_wmi_rfkill_setup(device))
> -               hp_wmi_rfkill2_setup(device);
> +       if (hp_wmi_rfkill2_setup(device))
> +               hp_wmi_rfkill_setup(device);
>
> Do you know if there are any cases that would be broken by this?
>
> --
> Matthew Garrett | mjg59@srcf.ucam.org
> --
> To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Cheers,
Alex Hung

Re: hp-wmi rfkill change breaks systems

[Alex Hung]

Hi Matthew,

I reverted 997daa1bd9aca412ab97955a35b26c460c0ec7a4 and add a check in
hp_wmi_rfkill_setup as below:

diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
index 8ba8956..65fee8a 100644
--- a/drivers/platform/x86/hp-wmi.c
+++ b/drivers/platform/x86/hp-wmi.c
@@ -54,7 +54,6 @@ MODULE_ALIAS("wmi:5FB7F034-2C63-45e9-BE91-3D44E2C707E4");
 #define HPWMI_HARDWARE_QUERY 0x4
 #define HPWMI_WIRELESS_QUERY 0x5
 #define HPWMI_HOTKEY_QUERY 0xc
-#define HPWMI_FEATURE_QUERY 0xd
 #define HPWMI_WIRELESS2_QUERY 0x1b
 #define HPWMI_POSTCODEERROR_QUERY 0x2a

@@ -293,17 +292,6 @@ static int hp_wmi_tablet_state(void)
        return (state & 0x4) ? 1 : 0;
 }

-static int hp_wmi_bios_2009_later(void)
-{
-       int state = 0;
-       int ret = hp_wmi_perform_query(HPWMI_FEATURE_QUERY, 0, &state,
-                                      sizeof(state), sizeof(state));
-       if (ret)
-               return ret;
-
-       return (state & 0x10) ? 1 : 0;
-}
-
 static int hp_wmi_set_block(void *data, bool blocked)
 {
        enum hp_wmi_radio r = (enum hp_wmi_radio) data;
@@ -696,6 +684,11 @@ static int hp_wmi_rfkill_setup(struct
platform_device *device)
        if (err)
                return err;

+       err = hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, 1,
+                                  &wireless, sizeof(wireless), 0);
+       if (err)
+               return err;
+
        if (wireless & 0x1) {
                wifi_rfkill = rfkill_alloc("hp-wifi", &device->dev,
                                           RFKILL_TYPE_WLAN,
@@ -883,7 +876,7 @@ static int __init hp_wmi_bios_setup(struct
platform_device *device)
        gps_rfkill = NULL;
        rfkill2_count = 0;

-       if (hp_wmi_bios_2009_later() || hp_wmi_rfkill_setup(device))
+       if (hp_wmi_rfkill_setup(device))
                hp_wmi_rfkill2_setup(device);

        err = device_create_file(&device->dev, &dev_attr_display);



It actually works better than 997daa1bd9aca412ab97955a35b26c460c0ec7a4
on new systems, but I don't have old hp systems for testing good HP
WMI 0x05 Set command.

Any suggestions are appreciated.

Re: hp-wmi rfkill change breaks systems

[Ward, David - 0665 - MITLL]

On 01/23/2014 02:25 AM, Alex Hung wrote:
> Hi Matthew,
>
> I reverted 997daa1bd9aca412ab97955a35b26c460c0ec7a4 and add a check in
> hp_wmi_rfkill_setup as below:
>
> diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
> index 8ba8956..65fee8a 100644
> --- a/drivers/platform/x86/hp-wmi.c
> +++ b/drivers/platform/x86/hp-wmi.c
> @@ -54,7 +54,6 @@ MODULE_ALIAS("wmi:5FB7F034-2C63-45e9-BE91-3D44E2C707E4");
>   #define HPWMI_HARDWARE_QUERY 0x4
>   #define HPWMI_WIRELESS_QUERY 0x5
>   #define HPWMI_HOTKEY_QUERY 0xc
> -#define HPWMI_FEATURE_QUERY 0xd
>   #define HPWMI_WIRELESS2_QUERY 0x1b
>   #define HPWMI_POSTCODEERROR_QUERY 0x2a
>
> @@ -293,17 +292,6 @@ static int hp_wmi_tablet_state(void)
>          return (state & 0x4) ? 1 : 0;
>   }
>
> -static int hp_wmi_bios_2009_later(void)
> -{
> -       int state = 0;
> -       int ret = hp_wmi_perform_query(HPWMI_FEATURE_QUERY, 0, &state,
> -                                      sizeof(state), sizeof(state));
> -       if (ret)
> -               return ret;
> -
> -       return (state & 0x10) ? 1 : 0;
> -}
> -
>   static int hp_wmi_set_block(void *data, bool blocked)
>   {
>          enum hp_wmi_radio r = (enum hp_wmi_radio) data;
> @@ -696,6 +684,11 @@ static int hp_wmi_rfkill_setup(struct
> platform_device *device)
>          if (err)
>                  return err;
>
> +       err = hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, 1,
> +                                  &wireless, sizeof(wireless), 0);
> +       if (err)
> +               return err;
> +
>          if (wireless & 0x1) {
>                  wifi_rfkill = rfkill_alloc("hp-wifi", &device->dev,
>                                             RFKILL_TYPE_WLAN,
> @@ -883,7 +876,7 @@ static int __init hp_wmi_bios_setup(struct
> platform_device *device)
>          gps_rfkill = NULL;
>          rfkill2_count = 0;
>
> -       if (hp_wmi_bios_2009_later() || hp_wmi_rfkill_setup(device))
> +       if (hp_wmi_rfkill_setup(device))
>                  hp_wmi_rfkill2_setup(device);
>
>          err = device_create_file(&device->dev, &dev_attr_display);
>
>
>
> It actually works better than 997daa1bd9aca412ab97955a35b26c460c0ec7a4
> on new systems, but I don't have old hp systems for testing good HP
> WMI 0x05 Set command.
>
> Any suggestions are appreciated.

Alex, Matthew, any update on this?

Commit f82bdd0 "hp-wmi: Enable hotkeys on some systems" added another 
use of the function hp_wmi_bios_2009_later(). Is the reason you removed 
the function above just because it wasn't called anymore, or is this 
problematic?

If you are looking for testing, maybe you should post (a revised version 
of) this patch to these bug reports, where users with older HP systems 
have indicated they are specifically affected by this:

https://bugzilla.kernel.org/show_bug.cgi?id=69131
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1290418

Thanks,

David