acer-wmi problem handling device states from WMI events

acer-wmi problem handling device states from WMI events

[Seth Forshee]

Hi Joey,

acer-wmi is indiscriminately using the device state from hotkey events
to update the various rfkill states. On the Aspire 1830 this can result
in a soft block on the wlan when the touchpad hotkey is pressed, as it
is reporting a non-zero device state that does not reflect the wireless
status.

I beleive the following (untested) patch is a roughly correct fix for
this issue.  It changes acer-wmi to only update the rfkill states when
the appropriate hotkeys are pressed, but I'm a little unsure about the
way I've split out the rfkill updates acording to the hotkeys. I don't
see any support in the driver for a 3G hotkey, so I've grouped it with
the wlan key, and I have split out bluetooth to be handled separately
from these. Does this patch look correct?

Thanks,
Seth


diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
index 005417b..592328d 100644
--- a/drivers/platform/x86/acer-wmi.c
+++ b/drivers/platform/x86/acer-wmi.c
@@ -1445,6 +1445,8 @@ static void acer_wmi_notify(u32 value, void *context)
 	union acpi_object *obj;
 	struct event_return_value return_value;
 	acpi_status status;
+	u16 device_state;
+	const struct key_entry *key;
 
 	status = wmi_get_event_data(value, &response);
 	if (status != AE_OK) {
@@ -1472,23 +1474,33 @@ static void acer_wmi_notify(u32 value, void *context)
 
 	switch (return_value.function) {
 	case WMID_HOTKEY_EVENT:
-		if (return_value.device_state) {
-			u16 device_state = return_value.device_state;
-			pr_debug("device state: 0x%x\n", device_state);
-			if (has_cap(ACER_CAP_WIRELESS))
-				rfkill_set_sw_state(wireless_rfkill,
-				!(device_state & ACER_WMID3_GDS_WIRELESS));
-			if (has_cap(ACER_CAP_BLUETOOTH))
-				rfkill_set_sw_state(bluetooth_rfkill,
-				!(device_state & ACER_WMID3_GDS_BLUETOOTH));
-			if (has_cap(ACER_CAP_THREEG))
-				rfkill_set_sw_state(threeg_rfkill,
-				!(device_state & ACER_WMID3_GDS_THREEG));
-		}
-		if (!sparse_keymap_report_event(acer_wmi_input_dev,
-				return_value.key_num, 1, true))
+		device_state = return_value.device_state;
+		pr_debug("device state: 0x%x\n", device_state);
+
+		key = sparse_keymap_entry_from_scancode(acer_wmi_input_dev,
+							return_value.key_num);
+		if (!key) {
 			pr_warn("Unknown key number - 0x%x\n",
 				return_value.key_num);
+		} else {
+			switch (key->keycode) {
+			case KEY_WLAN:
+				if (has_cap(ACER_CAP_WIRELESS))
+					rfkill_set_sw_state(wireless_rfkill,
+						!(device_state & ACER_WMID3_GDS_WIRELESS));
+				if (has_cap(ACER_CAP_THREEG))
+					rfkill_set_sw_state(threeg_rfkill,
+						!(device_state & ACER_WMID3_GDS_THREEG));
+				break;
+			case KEY_BLUETOOTH:
+				if (has_cap(ACER_CAP_BLUETOOTH))
+					rfkill_set_sw_state(bluetooth_rfkill,
+						!(device_state & ACER_WMID3_GDS_BLUETOOTH));
+				break;
+			}
+			sparse_keymap_report_entry(acer_wmi_input_dev, key,
+						   1, true);
+		}
 		break;
 	default:
 		pr_warn("Unknown function number - %d - %d\n",

Re: acer-wmi problem handling device states from WMI events

[Joey Lee]

Hi Seth, 

於 一，2011-06-20 於 14:06 -0500，Seth Forshee 提到：
> Hi Joey,
> 
> acer-wmi is indiscriminately using the device state from hotkey events
> to update the various rfkill states. On the Aspire 1830 this can result
> in a soft block on the wlan when the touchpad hotkey is pressed, as it
> is reporting a non-zero device state that does not reflect the wireless
> status.
> 

Thank's for you found out this issue, I didn't meet it because touchpad
hotkey didn't emit any wmi event on my Acer TravelMate 8572. 

I will double check it.

> I beleive the following (untested) patch is a roughly correct fix for
> this issue.  It changes acer-wmi to only update the rfkill states when
> the appropriate hotkeys are pressed, but I'm a little unsure about the
> way I've split out the rfkill updates acording to the hotkeys. I don't
> see any support in the driver for a 3G hotkey, so I've grouped it with
> the wlan key, and I have split out bluetooth to be handled separately
> from these. Does this patch look correct?
> 
> Thanks,
> Seth
> 

Yes, I thought your patch can avoid acer-wmi update killswitch state
base on the result from non-Communication button.

On some Acer machines only have one wireless key, the key only emit
KEY_WIRELESS but EC updates 3 communication devices' states, I thought
we can direct update 3 killswitch state when received KEY_WLAN or
KEY_BLUETOOTH. maybe like this:

+			switch (key->keycode) {
+			case KEY_WLAN:
+			case KEY_BLUETOOTH:
+				if (has_cap(ACER_CAP_WIRELESS))
+					rfkill_set_sw_state(wireless_rfkill,
+						!(device_state & ACER_WMID3_GDS_WIRELESS));
+				if (has_cap(ACER_CAP_THREEG))
+					rfkill_set_sw_state(threeg_rfkill,
+						!(device_state & ACER_WMID3_GDS_THREEG));
+				if (has_cap(ACER_CAP_BLUETOOTH))
+					rfkill_set_sw_state(bluetooth_rfkill,
+						!(device_state & ACER_WMID3_GDS_BLUETOOTH));
+				break;
+			}
+			sparse_keymap_report_entry(acer_wmi_input_dev, key,
+						   1, true);

Of course need more testing on my and your Acer machines.
I will double check this patch on my TravelMate 8572 then reply result
on this mail tomorrow.


Thank's a lot!
Joey Lee

> 
> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> index 005417b..592328d 100644
> --- a/drivers/platform/x86/acer-wmi.c
> +++ b/drivers/platform/x86/acer-wmi.c
> @@ -1445,6 +1445,8 @@ static void acer_wmi_notify(u32 value, void *context)
>  	union acpi_object *obj;
>  	struct event_return_value return_value;
>  	acpi_status status;
> +	u16 device_state;
> +	const struct key_entry *key;
>  
>  	status = wmi_get_event_data(value, &response);
>  	if (status != AE_OK) {
> @@ -1472,23 +1474,33 @@ static void acer_wmi_notify(u32 value, void *context)
>  
>  	switch (return_value.function) {
>  	case WMID_HOTKEY_EVENT:
> -		if (return_value.device_state) {
> -			u16 device_state = return_value.device_state;
> -			pr_debug("device state: 0x%x\n", device_state);
> -			if (has_cap(ACER_CAP_WIRELESS))
> -				rfkill_set_sw_state(wireless_rfkill,
> -				!(device_state & ACER_WMID3_GDS_WIRELESS));
> -			if (has_cap(ACER_CAP_BLUETOOTH))
> -				rfkill_set_sw_state(bluetooth_rfkill,
> -				!(device_state & ACER_WMID3_GDS_BLUETOOTH));
> -			if (has_cap(ACER_CAP_THREEG))
> -				rfkill_set_sw_state(threeg_rfkill,
> -				!(device_state & ACER_WMID3_GDS_THREEG));
> -		}
> -		if (!sparse_keymap_report_event(acer_wmi_input_dev,
> -				return_value.key_num, 1, true))
> +		device_state = return_value.device_state;
> +		pr_debug("device state: 0x%x\n", device_state);
> +
> +		key = sparse_keymap_entry_from_scancode(acer_wmi_input_dev,
> +							return_value.key_num);
> +		if (!key) {
>  			pr_warn("Unknown key number - 0x%x\n",
>  				return_value.key_num);
> +		} else {
> +			switch (key->keycode) {
> +			case KEY_WLAN:
> +				if (has_cap(ACER_CAP_WIRELESS))
> +					rfkill_set_sw_state(wireless_rfkill,
> +						!(device_state & ACER_WMID3_GDS_WIRELESS));
> +				if (has_cap(ACER_CAP_THREEG))
> +					rfkill_set_sw_state(threeg_rfkill,
> +						!(device_state & ACER_WMID3_GDS_THREEG));
> +				break;
> +			case KEY_BLUETOOTH:
> +				if (has_cap(ACER_CAP_BLUETOOTH))
> +					rfkill_set_sw_state(bluetooth_rfkill,
> +						!(device_state & ACER_WMID3_GDS_BLUETOOTH));
> +				break;
> +			}
> +			sparse_keymap_report_entry(acer_wmi_input_dev, key,
> +						   1, true);
> +		}
>  		break;
>  	default:
>  		pr_warn("Unknown function number - %d - %d\n",

Re: acer-wmi problem handling device states from WMI events

[Seth Forshee]

On Tue, Jun 21, 2011 at 04:16:38AM -0600, Joey Lee wrote:
> Hi Seth, 
> 
> 於 一，2011-06-20 於 14:06 -0500，Seth Forshee 提到：
> > Hi Joey,
> > 
> > acer-wmi is indiscriminately using the device state from hotkey events
> > to update the various rfkill states. On the Aspire 1830 this can result
> > in a soft block on the wlan when the touchpad hotkey is pressed, as it
> > is reporting a non-zero device state that does not reflect the wireless
> > status.
> > 
> 
> Thank's for you found out this issue, I didn't meet it because touchpad
> hotkey didn't emit any wmi event on my Acer TravelMate 8572. 
> 
> I will double check it.
> 
> > I beleive the following (untested) patch is a roughly correct fix for
> > this issue.  It changes acer-wmi to only update the rfkill states when
> > the appropriate hotkeys are pressed, but I'm a little unsure about the
> > way I've split out the rfkill updates acording to the hotkeys. I don't
> > see any support in the driver for a 3G hotkey, so I've grouped it with
> > the wlan key, and I have split out bluetooth to be handled separately
> > from these. Does this patch look correct?
> > 
> > Thanks,
> > Seth
> > 
> 
> Yes, I thought your patch can avoid acer-wmi update killswitch state
> base on the result from non-Communication button.
> 
> On some Acer machines only have one wireless key, the key only emit
> KEY_WIRELESS but EC updates 3 communication devices' states, I thought
> we can direct update 3 killswitch state when received KEY_WLAN or
> KEY_BLUETOOTH. maybe like this:
> 
> +			switch (key->keycode) {
> +			case KEY_WLAN:
> +			case KEY_BLUETOOTH:
> +				if (has_cap(ACER_CAP_WIRELESS))
> +					rfkill_set_sw_state(wireless_rfkill,
> +						!(device_state & ACER_WMID3_GDS_WIRELESS));
> +				if (has_cap(ACER_CAP_THREEG))
> +					rfkill_set_sw_state(threeg_rfkill,
> +						!(device_state & ACER_WMID3_GDS_THREEG));
> +				if (has_cap(ACER_CAP_BLUETOOTH))
> +					rfkill_set_sw_state(bluetooth_rfkill,
> +						!(device_state & ACER_WMID3_GDS_BLUETOOTH));
> +				break;
> +			}
> +			sparse_keymap_report_entry(acer_wmi_input_dev, key,
> +						   1, true);
> 
> Of course need more testing on my and your Acer machines.
> I will double check this patch on my TravelMate 8572 then reply result
> on this mail tomorrow.

That is the other way I was thinking of doing this, but I wasn't sure
which was correct, and the machine in question doesn't have bluetooth
for me to test. Based on your explanation I think doing it as above is
best.

I'll get some testing with the version above, and if it tests well for
both of us I will send an updated patch plus one other patch to support
the wlan hotkey on the 1830. I don't personally have this machine so it
may be a few days before I receive results back from my tester.

Thanks,
Seth

Re: acer-wmi problem handling device states from WMI events

[Seth Forshee]

On Tue, Jun 21, 2011 at 08:14:42AM -0500, Seth Forshee wrote:
> On Tue, Jun 21, 2011 at 04:16:38AM -0600, Joey Lee wrote:
> > Hi Seth, 
> > 
> > 於 一，2011-06-20 於 14:06 -0500，Seth Forshee 提到：
> > > Hi Joey,
> > > 
> > > acer-wmi is indiscriminately using the device state from hotkey events
> > > to update the various rfkill states. On the Aspire 1830 this can result
> > > in a soft block on the wlan when the touchpad hotkey is pressed, as it
> > > is reporting a non-zero device state that does not reflect the wireless
> > > status.
> > > 
> > 
> > Thank's for you found out this issue, I didn't meet it because touchpad
> > hotkey didn't emit any wmi event on my Acer TravelMate 8572. 
> > 
> > I will double check it.
> > 
> > > I beleive the following (untested) patch is a roughly correct fix for
> > > this issue.  It changes acer-wmi to only update the rfkill states when
> > > the appropriate hotkeys are pressed, but I'm a little unsure about the
> > > way I've split out the rfkill updates acording to the hotkeys. I don't
> > > see any support in the driver for a 3G hotkey, so I've grouped it with
> > > the wlan key, and I have split out bluetooth to be handled separately
> > > from these. Does this patch look correct?
> > > 
> > > Thanks,
> > > Seth
> > > 
> > 
> > Yes, I thought your patch can avoid acer-wmi update killswitch state
> > base on the result from non-Communication button.
> > 
> > On some Acer machines only have one wireless key, the key only emit
> > KEY_WIRELESS but EC updates 3 communication devices' states, I thought
> > we can direct update 3 killswitch state when received KEY_WLAN or
> > KEY_BLUETOOTH. maybe like this:
> > 
> > +			switch (key->keycode) {
> > +			case KEY_WLAN:
> > +			case KEY_BLUETOOTH:
> > +				if (has_cap(ACER_CAP_WIRELESS))
> > +					rfkill_set_sw_state(wireless_rfkill,
> > +						!(device_state & ACER_WMID3_GDS_WIRELESS));
> > +				if (has_cap(ACER_CAP_THREEG))
> > +					rfkill_set_sw_state(threeg_rfkill,
> > +						!(device_state & ACER_WMID3_GDS_THREEG));
> > +				if (has_cap(ACER_CAP_BLUETOOTH))
> > +					rfkill_set_sw_state(bluetooth_rfkill,
> > +						!(device_state & ACER_WMID3_GDS_BLUETOOTH));
> > +				break;
> > +			}
> > +			sparse_keymap_report_entry(acer_wmi_input_dev, key,
> > +						   1, true);
> > 
> > Of course need more testing on my and your Acer machines.
> > I will double check this patch on my TravelMate 8572 then reply result
> > on this mail tomorrow.
> 
> That is the other way I was thinking of doing this, but I wasn't sure
> which was correct, and the machine in question doesn't have bluetooth
> for me to test. Based on your explanation I think doing it as above is
> best.
> 
> I'll get some testing with the version above, and if it tests well for
> both of us I will send an updated patch plus one other patch to support
> the wlan hotkey on the 1830. I don't personally have this machine so it
> may be a few days before I receive results back from my tester.

I got test results back already, and the patch is working. I'll follow
up with both patches, that way if they work fine in your testing you can
go ahead and take them.

Thanks!
Seth

Re: acer-wmi problem handling device states from WMI events

[Joey Lee]

於 二，2011-06-21 於 10:16 +0000，joeyli(Joey Lee) 提到：
> Hi Seth, 
> 
> 於 一，2011-06-20 於 14:06 -0500，Seth Forshee 提到：
> > Hi Joey,
> > 
> > acer-wmi is indiscriminately using the device state from hotkey events
> > to update the various rfkill states. On the Aspire 1830 this can result
> > in a soft block on the wlan when the touchpad hotkey is pressed, as it
> > is reporting a non-zero device state that does not reflect the wireless
> > status.
> > 
> 
> Thank's for you found out this issue, I didn't meet it because touchpad
> hotkey didn't emit any wmi event on my Acer TravelMate 8572. 
> 
> I will double check it.
> 

Just checked, it also emit touchpad wmi event, I missed it.

Will test your patch, I will check KEY_WLAN and KEY_BLUETOOTH in the
same case to update killswitch states.


Thank's a lot1
Joey Lee

> > I beleive the following (untested) patch is a roughly correct fix for
> > this issue.  It changes acer-wmi to only update the rfkill states when
> > the appropriate hotkeys are pressed, but I'm a little unsure about the
> > way I've split out the rfkill updates acording to the hotkeys. I don't
> > see any support in the driver for a 3G hotkey, so I've grouped it with
> > the wlan key, and I have split out bluetooth to be handled separately
> > from these. Does this patch look correct?
> > 
> > Thanks,
> > Seth
> > 
> 
> Yes, I thought your patch can avoid acer-wmi update killswitch state
> base on the result from non-Communication button.
> 
> On some Acer machines only have one wireless key, the key only emit
> KEY_WIRELESS but EC updates 3 communication devices' states, I thought
> we can direct update 3 killswitch state when received KEY_WLAN or
> KEY_BLUETOOTH. maybe like this:
> 
> +			switch (key->keycode) {
> +			case KEY_WLAN:
> +			case KEY_BLUETOOTH:
> +				if (has_cap(ACER_CAP_WIRELESS))
> +					rfkill_set_sw_state(wireless_rfkill,
> +						!(device_state & ACER_WMID3_GDS_WIRELESS));
> +				if (has_cap(ACER_CAP_THREEG))
> +					rfkill_set_sw_state(threeg_rfkill,
> +						!(device_state & ACER_WMID3_GDS_THREEG));
> +				if (has_cap(ACER_CAP_BLUETOOTH))
> +					rfkill_set_sw_state(bluetooth_rfkill,
> +						!(device_state & ACER_WMID3_GDS_BLUETOOTH));
> +				break;
> +			}
> +			sparse_keymap_report_entry(acer_wmi_input_dev, key,
> +						   1, true);
> 
> Of course need more testing on my and your Acer machines.
> I will double check this patch on my TravelMate 8572 then reply result
> on this mail tomorrow.
> 
> 
> Thank's a lot!
> Joey Lee
> 
> > 
> > diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> > index 005417b..592328d 100644
> > --- a/drivers/platform/x86/acer-wmi.c
> > +++ b/drivers/platform/x86/acer-wmi.c
> > @@ -1445,6 +1445,8 @@ static void acer_wmi_notify(u32 value, void *context)
> >  	union acpi_object *obj;
> >  	struct event_return_value return_value;
> >  	acpi_status status;
> > +	u16 device_state;
> > +	const struct key_entry *key;
> >  
> >  	status = wmi_get_event_data(value, &response);
> >  	if (status != AE_OK) {
> > @@ -1472,23 +1474,33 @@ static void acer_wmi_notify(u32 value, void *context)
> >  
> >  	switch (return_value.function) {
> >  	case WMID_HOTKEY_EVENT:
> > -		if (return_value.device_state) {
> > -			u16 device_state = return_value.device_state;
> > -			pr_debug("device state: 0x%x\n", device_state);
> > -			if (has_cap(ACER_CAP_WIRELESS))
> > -				rfkill_set_sw_state(wireless_rfkill,
> > -				!(device_state & ACER_WMID3_GDS_WIRELESS));
> > -			if (has_cap(ACER_CAP_BLUETOOTH))
> > -				rfkill_set_sw_state(bluetooth_rfkill,
> > -				!(device_state & ACER_WMID3_GDS_BLUETOOTH));
> > -			if (has_cap(ACER_CAP_THREEG))
> > -				rfkill_set_sw_state(threeg_rfkill,
> > -				!(device_state & ACER_WMID3_GDS_THREEG));
> > -		}
> > -		if (!sparse_keymap_report_event(acer_wmi_input_dev,
> > -				return_value.key_num, 1, true))
> > +		device_state = return_value.device_state;
> > +		pr_debug("device state: 0x%x\n", device_state);
> > +
> > +		key = sparse_keymap_entry_from_scancode(acer_wmi_input_dev,
> > +							return_value.key_num);
> > +		if (!key) {
> >  			pr_warn("Unknown key number - 0x%x\n",
> >  				return_value.key_num);
> > +		} else {
> > +			switch (key->keycode) {
> > +			case KEY_WLAN:
> > +				if (has_cap(ACER_CAP_WIRELESS))
> > +					rfkill_set_sw_state(wireless_rfkill,
> > +						!(device_state & ACER_WMID3_GDS_WIRELESS));
> > +				if (has_cap(ACER_CAP_THREEG))
> > +					rfkill_set_sw_state(threeg_rfkill,
> > +						!(device_state & ACER_WMID3_GDS_THREEG));
> > +				break;
> > +			case KEY_BLUETOOTH:
> > +				if (has_cap(ACER_CAP_BLUETOOTH))
> > +					rfkill_set_sw_state(bluetooth_rfkill,
> > +						!(device_state & ACER_WMID3_GDS_BLUETOOTH));
> > +				break;
> > +			}
> > +			sparse_keymap_report_entry(acer_wmi_input_dev, key,
> > +						   1, true);
> > +		}
> >  		break;
> >  	default:
> >  		pr_warn("Unknown function number - %d - %d\n",
>