[PATCH 1/2] ideapad-laptop: Ignore an unused VPC notification

[PATCH 1/2] ideapad-laptop: Ignore an unused VPC notification

[Keng-Yu Lin]

BIOS fires notifications in a constant period with VPC bit 1 set when the
power cord is unplugged.

This event is not known yet to have any effect, but it makes the
call to sparse_keymap_report_event() and generates many KEY_UNKNOWNs.

This causes the userspace (Gnome Power Manager in my case) unable
to enter the idle mode. As the result there is no screensaver showing up,
no dim backlight, etc.

Signed-off-by: Keng-Yu Lin <kengyu@canonical.com>
---
 drivers/platform/x86/ideapad-laptop.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index bfdda33..820f013 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -463,7 +463,8 @@ static void ideapad_acpi_notify(struct acpi_device *adevice, u32 event)
 				ideapad_sync_rfk_state(adevice);
 			else if (vpc_bit == 4)
 				read_ec_data(handle, 0x12, &vpc2);
-			else
+			else if (vpc_bit == 1) {
+			} else
 				ideapad_input_report(priv, vpc_bit);
 		}
 	}
-- 
1.7.4.1

[PATCH 2/2] ideapad-laptop: Handle the App-controlled RF event

[Keng-Yu Lin]

Notification with VPC2 bit 5 set is generated when the wireless
hotkey is pressed. BIOS fires this to notify a Windows
application to handle the RF switch.

On Linux, there is no such application. This patch implements
a toggle for all RF devices through accessing the EC I/O ports.

Without the explicit EC commands, the wifi LED is always on.

Signed-off-by: Keng-Yu Lin <kengyu@canonical.com>
---
 drivers/platform/x86/ideapad-laptop.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 820f013..5e1e971 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -461,7 +461,11 @@ static void ideapad_acpi_notify(struct acpi_device *adevice, u32 event)
 		if (test_bit(vpc_bit, &vpc1)) {
 			if (vpc_bit == 9)
 				ideapad_sync_rfk_state(adevice);
-			else if (vpc_bit == 4)
+			else if (vpc_bit == 13) {
+				read_ec_data(handle, 0x23, &vpc2);
+				write_ec_cmd(handle, 0x24, !vpc2);
+				ideapad_sync_rfk_state(adevice);
+			} else if (vpc_bit == 4)
 				read_ec_data(handle, 0x12, &vpc2);
 			else if (vpc_bit == 1) {
 			} else
-- 
1.7.4.1

Re: [PATCH 2/2] ideapad-laptop: Handle the App-controlled RF event

[Ike Panhc]

On 06/28/2011 09:35 PM, Keng-Yu Lin wrote:
> Notification with VPC2 bit 5 set is generated when the wireless
> hotkey is pressed. BIOS fires this to notify a Windows
> application to handle the RF switch.
> 
> On Linux, there is no such application. This patch implements
> a toggle for all RF devices through accessing the EC I/O ports.
> 
> Without the explicit EC commands, the wifi LED is always on.
> 
> Signed-off-by: Keng-Yu Lin <kengyu@canonical.com>
> ---
>  drivers/platform/x86/ideapad-laptop.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
> index 820f013..5e1e971 100644
> --- a/drivers/platform/x86/ideapad-laptop.c
> +++ b/drivers/platform/x86/ideapad-laptop.c
> @@ -461,7 +461,11 @@ static void ideapad_acpi_notify(struct acpi_device *adevice, u32 event)
>  		if (test_bit(vpc_bit, &vpc1)) {
>  			if (vpc_bit == 9)
>  				ideapad_sync_rfk_state(adevice);
> -			else if (vpc_bit == 4)
> +			else if (vpc_bit == 13) {
> +				read_ec_data(handle, 0x23, &vpc2);
> +				write_ec_cmd(handle, 0x24, !vpc2);
> +				ideapad_sync_rfk_state(adevice);
> +			} else if (vpc_bit == 4)
>  				read_ec_data(handle, 0x12, &vpc2);
>  			else if (vpc_bit == 1) {
>  			} else

I think its better to implement the changes in ideapad_sync_rfk_state() because we have
read switch status there. It is not necessary to read the same data twice in an event.

Re: [PATCH 2/2] ideapad-laptop: Handle the App-controlled RF event

[Keng-Yü Lin]

On Wed, Jun 29, 2011 at 2:56 PM, Ike Panhc <ike.pan@canonical.com> wrote:
> On 06/28/2011 09:35 PM, Keng-Yu Lin wrote:
>> Notification with VPC2 bit 5 set is generated when the wireless
>> hotkey is pressed. BIOS fires this to notify a Windows
>> application to handle the RF switch.
>>
>> On Linux, there is no such application. This patch implements
>> a toggle for all RF devices through accessing the EC I/O ports.
>>
>> Without the explicit EC commands, the wifi LED is always on.
>>
>> Signed-off-by: Keng-Yu Lin <kengyu@canonical.com>
>> ---
>>  drivers/platform/x86/ideapad-laptop.c |    6 +++++-
>>  1 files changed, 5 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
>> index 820f013..5e1e971 100644
>> --- a/drivers/platform/x86/ideapad-laptop.c
>> +++ b/drivers/platform/x86/ideapad-laptop.c
>> @@ -461,7 +461,11 @@ static void ideapad_acpi_notify(struct acpi_device *adevice, u32 event)
>>               if (test_bit(vpc_bit, &vpc1)) {
>>                       if (vpc_bit == 9)
>>                               ideapad_sync_rfk_state(adevice);
>> -                     else if (vpc_bit == 4)
>> +                     else if (vpc_bit == 13) {
>> +                             read_ec_data(handle, 0x23, &vpc2);
>> +                             write_ec_cmd(handle, 0x24, !vpc2);
>> +                             ideapad_sync_rfk_state(adevice);
>> +                     } else if (vpc_bit == 4)
>>                               read_ec_data(handle, 0x12, &vpc2);
>>                       else if (vpc_bit == 1) {
>>                       } else
>
> I think its better to implement the changes in ideapad_sync_rfk_state() because we have
> read switch status there. It is not necessary to read the same data twice in an event.
>
>

Current ideapad_sync_rfk_state() code does not really toggle the
hardware swtich (I mean to write_ec_cmd with 0x24 command). It just
assumes that the BIOS or EC is responsible for the wifi switch-on/-off
and reports the state via rfkill_set_hw_state().

In my case, without explicit sending the 0x24 command to turn off the
radio device, the wifi LED is always on.

Though it might not be true or applicable on ideapads,  in my last
experience on Dell laptops that to switch off the RF radio in the
driver while the hardware claims to control it can cause some mess-up.

So to be safe I prefer not to move the toggle code in ideapad_sync_rfk_state().

  Thanks,
-kengyu

Re: [PATCH 2/2] ideapad-laptop: Handle the App-controlled RF event

[Ike Panhc]

On 06/30/2011 06:34 PM, Keng-Yü Lin wrote:
> On Wed, Jun 29, 2011 at 2:56 PM, Ike Panhc <ike.pan@canonical.com> wrote:
>> On 06/28/2011 09:35 PM, Keng-Yu Lin wrote:
>>> Notification with VPC2 bit 5 set is generated when the wireless
>>> hotkey is pressed. BIOS fires this to notify a Windows
>>> application to handle the RF switch.
>>>
>>> On Linux, there is no such application. This patch implements
>>> a toggle for all RF devices through accessing the EC I/O ports.
>>>
>>> Without the explicit EC commands, the wifi LED is always on.
>>>
>>> Signed-off-by: Keng-Yu Lin <kengyu@canonical.com>
>>> ---
>>>  drivers/platform/x86/ideapad-laptop.c |    6 +++++-
>>>  1 files changed, 5 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
>>> index 820f013..5e1e971 100644
>>> --- a/drivers/platform/x86/ideapad-laptop.c
>>> +++ b/drivers/platform/x86/ideapad-laptop.c
>>> @@ -461,7 +461,11 @@ static void ideapad_acpi_notify(struct acpi_device *adevice, u32 event)
>>>               if (test_bit(vpc_bit, &vpc1)) {
>>>                       if (vpc_bit == 9)
>>>                               ideapad_sync_rfk_state(adevice);
>>> -                     else if (vpc_bit == 4)
>>> +                     else if (vpc_bit == 13) {
>>> +                             read_ec_data(handle, 0x23, &vpc2);
>>> +                             write_ec_cmd(handle, 0x24, !vpc2);
>>> +                             ideapad_sync_rfk_state(adevice);
>>> +                     } else if (vpc_bit == 4)
>>>                               read_ec_data(handle, 0x12, &vpc2);
>>>                       else if (vpc_bit == 1) {
>>>                       } else
>>
>> I think its better to implement the changes in ideapad_sync_rfk_state() because we have
>> read switch status there. It is not necessary to read the same data twice in an event.
>>
>>
> 
> Current ideapad_sync_rfk_state() code does not really toggle the
> hardware swtich (I mean to write_ec_cmd with 0x24 command). It just
> assumes that the BIOS or EC is responsible for the wifi switch-on/-off
> and reports the state via rfkill_set_hw_state().
> 
> In my case, without explicit sending the 0x24 command to turn off the
> radio device, the wifi LED is always on.
> 
> Though it might not be true or applicable on ideapads,  in my last
> experience on Dell laptops that to switch off the RF radio in the
> driver while the hardware claims to control it can cause some mess-up.
> 
> So to be safe I prefer not to move the toggle code in ideapad_sync_rfk_state().
> 
>   Thanks,
> -kengyu
> 

Look twice and I do not think this patch is acceptable.

vpc_bit 13 is soft block key event (Fn+F5) and read_ec_data from 0x23 is
hardware switch status. This patch will mask the key report of inputdev
and to force update switch status in EC (0x24) is not a reasonable solution.

Re: [PATCH 2/2] ideapad-laptop: Handle the App-controlled RF event

[Keng-Yü Lin]

On Tue, Jul 5, 2011 at 3:26 PM, Ike Panhc <ike.pan@canonical.com> wrote:
> On 06/30/2011 06:34 PM, Keng-Yü Lin wrote:
>> On Wed, Jun 29, 2011 at 2:56 PM, Ike Panhc <ike.pan@canonical.com> wrote:
>>> On 06/28/2011 09:35 PM, Keng-Yu Lin wrote:
>>>> Notification with VPC2 bit 5 set is generated when the wireless
>>>> hotkey is pressed. BIOS fires this to notify a Windows
>>>> application to handle the RF switch.
>>>>
>>>> On Linux, there is no such application. This patch implements
>>>> a toggle for all RF devices through accessing the EC I/O ports.
>>>>
>>>> Without the explicit EC commands, the wifi LED is always on.
>>>>
>>>> Signed-off-by: Keng-Yu Lin <kengyu@canonical.com>
>>>> ---
>>>>  drivers/platform/x86/ideapad-laptop.c |    6 +++++-
>>>>  1 files changed, 5 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
>>>> index 820f013..5e1e971 100644
>>>> --- a/drivers/platform/x86/ideapad-laptop.c
>>>> +++ b/drivers/platform/x86/ideapad-laptop.c
>>>> @@ -461,7 +461,11 @@ static void ideapad_acpi_notify(struct acpi_device *adevice, u32 event)
>>>>               if (test_bit(vpc_bit, &vpc1)) {
>>>>                       if (vpc_bit == 9)
>>>>                               ideapad_sync_rfk_state(adevice);
>>>> -                     else if (vpc_bit == 4)
>>>> +                     else if (vpc_bit == 13) {
>>>> +                             read_ec_data(handle, 0x23, &vpc2);
>>>> +                             write_ec_cmd(handle, 0x24, !vpc2);
>>>> +                             ideapad_sync_rfk_state(adevice);
>>>> +                     } else if (vpc_bit == 4)
>>>>                               read_ec_data(handle, 0x12, &vpc2);
>>>>                       else if (vpc_bit == 1) {
>>>>                       } else
>>>
>>> I think its better to implement the changes in ideapad_sync_rfk_state() because we have
>>> read switch status there. It is not necessary to read the same data twice in an event.
>>>
>>>
>>
>> Current ideapad_sync_rfk_state() code does not really toggle the
>> hardware swtich (I mean to write_ec_cmd with 0x24 command). It just
>> assumes that the BIOS or EC is responsible for the wifi switch-on/-off
>> and reports the state via rfkill_set_hw_state().
>>
>> In my case, without explicit sending the 0x24 command to turn off the
>> radio device, the wifi LED is always on.
>>
>> Though it might not be true or applicable on ideapads,  in my last
>> experience on Dell laptops that to switch off the RF radio in the
>> driver while the hardware claims to control it can cause some mess-up.
>>
>> So to be safe I prefer not to move the toggle code in ideapad_sync_rfk_state().
>>
>>   Thanks,
>> -kengyu
>>
>
> Look twice and I do not think this patch is acceptable.
>
> vpc_bit 13 is soft block key event (Fn+F5) and read_ec_data from 0x23 is
> hardware switch status. This patch will mask the key report of inputdev
> and to force update switch status in EC (0x24) is not a reasonable solution.
>

In fact quite a few things involved in this:

since we get notified with VPC bit 13 set when Fn+F5 is pressed, I
think that it is ok to be treated as a hardware rfkill
blocking/unblocking.

However it is true that the bit/event is supposed be to be handled by
an application in Windows, it is a soft-killswitch by the definition.
Yet currently there is no such correspondent userspace app/daemon as I
know. This inspired the original intention of the patch, to handle the
app-controlled rfkill in the driver. I think that this is probably
common in other platform-x86 drivers.

I'd rather agree with you that it is a better idea to report a keycode.

Re: [PATCH 2/2] ideapad-laptop: Handle the App-controlled RF event

[Matthew Garrett]

On Tue, Jun 28, 2011 at 02:35:54PM +0100, Keng-Yu Lin wrote:

> On Linux, there is no such application. This patch implements
> a toggle for all RF devices through accessing the EC I/O ports.

Sure there is. Either the kernel can handle it via rfkill-input, or 
rfkilld can look after it. Please don't add your own implementation.

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

Re: [PATCH 2/2] ideapad-laptop: Handle the App-controlled RF event

[Joey Lee]

Hi Keng-Yu, 

於 四，2011-07-07 於 16:11 +0100，Matthew Garrett 提到：
> On Tue, Jun 28, 2011 at 02:35:54PM +0100, Keng-Yu Lin wrote:
> 
> > On Linux, there is no such application. This patch implements
> > a toggle for all RF devices through accessing the EC I/O ports.
> 
> Sure there is. Either the kernel can handle it via rfkill-input, or 
> rfkilld can look after it. Please don't add your own implementation.
> 

Welcome to use urfkill deaemon:
http://www.freedesktop.org/wiki/Software/urfkill


Thank's a lot!
Joey

Re: [PATCH 1/2] ideapad-laptop: Ignore an unused VPC notification

[Ike Panhc]

On 06/28/2011 09:35 PM, Keng-Yu Lin wrote:
> BIOS fires notifications in a constant period with VPC bit 1 set when the
> power cord is unplugged.
> 
> This event is not known yet to have any effect, but it makes the
> call to sparse_keymap_report_event() and generates many KEY_UNKNOWNs.
> 
> This causes the userspace (Gnome Power Manager in my case) unable
> to enter the idle mode. As the result there is no screensaver showing up,
> no dim backlight, etc.
> 
> Signed-off-by: Keng-Yu Lin <kengyu@canonical.com>
> ---
>  drivers/platform/x86/ideapad-laptop.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
> index bfdda33..820f013 100644
> --- a/drivers/platform/x86/ideapad-laptop.c
> +++ b/drivers/platform/x86/ideapad-laptop.c
> @@ -463,7 +463,8 @@ static void ideapad_acpi_notify(struct acpi_device *adevice, u32 event)
>  				ideapad_sync_rfk_state(adevice);
>  			else if (vpc_bit == 4)
>  				read_ec_data(handle, 0x12, &vpc2);
> -			else
> +			else if (vpc_bit == 1) {
> +			} else
>  				ideapad_input_report(priv, vpc_bit);
>  		}
>  	}

So far I know this event (coming with vpc_bit=1) is for Lenovo PM utility
on Windows. For most of the machine this event is not implemented in BIOS.

Its ok that we just ignore the event.

I think it will be better if we can let sparse-keymap or any upper level
application to ignore all unknown scancode or KEY_UNKNOWN but so far I do
not know a better way then this patch.

Re: [PATCH 1/2] ideapad-laptop: Ignore an unused VPC notification

[Keng-Yü Lin]

On Wed, Jun 29, 2011 at 3:26 PM, Ike Panhc <ike.pan@canonical.com> wrote:
> On 06/28/2011 09:35 PM, Keng-Yu Lin wrote:
>> BIOS fires notifications in a constant period with VPC bit 1 set when the
>> power cord is unplugged.
>>
>> This event is not known yet to have any effect, but it makes the
>> call to sparse_keymap_report_event() and generates many KEY_UNKNOWNs.
>>
>> This causes the userspace (Gnome Power Manager in my case) unable
>> to enter the idle mode. As the result there is no screensaver showing up,
>> no dim backlight, etc.
>>
>> Signed-off-by: Keng-Yu Lin <kengyu@canonical.com>
>> ---
>>  drivers/platform/x86/ideapad-laptop.c |    3 ++-
>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
>> index bfdda33..820f013 100644
>> --- a/drivers/platform/x86/ideapad-laptop.c
>> +++ b/drivers/platform/x86/ideapad-laptop.c
>> @@ -463,7 +463,8 @@ static void ideapad_acpi_notify(struct acpi_device *adevice, u32 event)
>>                               ideapad_sync_rfk_state(adevice);
>>                       else if (vpc_bit == 4)
>>                               read_ec_data(handle, 0x12, &vpc2);
>> -                     else
>> +                     else if (vpc_bit == 1) {
>> +                     } else
>>                               ideapad_input_report(priv, vpc_bit);
>>               }
>>       }
>
> So far I know this event (coming with vpc_bit=1) is for Lenovo PM utility
> on Windows. For most of the machine this event is not implemented in BIOS.
>
> Its ok that we just ignore the event.
>
> I think it will be better if we can let sparse-keymap or any upper level
> application to ignore all unknown scancode or KEY_UNKNOWN but so far I do
> not know a better way then this patch.

I think sparse_keymap_report_event() sends the KEY_UNKNOWN on purpose
as a debugging aid.

We should prevent the call to it in ideapad-laptop. I think the patch
is probably the simplest way.

  Thanks,
-kengyu

Re: [PATCH 1/2] ideapad-laptop: Ignore an unused VPC notification

[Matthew Garrett]

On Tue, Jun 28, 2011 at 02:35:53PM +0100, Keng-Yu Lin wrote:
> BIOS fires notifications in a constant period with VPC bit 1 set when the
> power cord is unplugged.

How frequently? That's going to do bad things to power management. It'd 
be nice to find a way to just make it stop doing that.

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

Re: [PATCH 1/2] ideapad-laptop: Ignore an unused VPC notification

[Keng-Yü Lin]

On Thu, Jul 7, 2011 at 11:10 PM, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> On Tue, Jun 28, 2011 at 02:35:53PM +0100, Keng-Yu Lin wrote:
>> BIOS fires notifications in a constant period with VPC bit 1 set when the
>> power cord is unplugged.
>
> How frequently? That's going to do bad things to power management. It'd
> be nice to find a way to just make it stop doing that.
>

About every 22 seconds.

May 18 13:46:36 ubuntu-Rev-1-0 kernel: [ 1359.734293] keyboard: can't
emulate rawmode for keycode 240
May 18 13:46:36 ubuntu-Rev-1-0 kernel: [ 1359.734324] keyboard: can't
emulate rawmode for keycode 240
May 18 13:46:58 ubuntu-Rev-1-0 kernel: [ 1382.195371] keyboard: can't
emulate rawmode for keycode 240
May 18 13:46:58 ubuntu-Rev-1-0 kernel: [ 1382.195388] keyboard: can't
emulate rawmode for keycode 240
May 18 13:47:21 ubuntu-Rev-1-0 kernel: [ 1404.650451] keyboard: can't
emulate rawmode for keycode 240
May 18 13:47:21 ubuntu-Rev-1-0 kernel: [ 1404.650470] keyboard: can't
emulate rawmode for keycode 240
May 18 13:47:43 ubuntu-Rev-1-0 kernel: [ 1427.103195] keyboard: can't
emulate rawmode for keycode 240
May 18 13:47:43 ubuntu-Rev-1-0 kernel: [ 1427.103214] keyboard: can't
emulate rawmode for keycode 240
May 18 13:48:06 ubuntu-Rev-1-0 kernel: [ 1449.568874] keyboard: can't
emulate rawmode for keycode 240
May 18 13:48:06 ubuntu-Rev-1-0 kernel: [ 1449.568884] keyboard: can't
emulate rawmode for keycode 240
May 18 13:48:28 ubuntu-Rev-1-0 kernel: [ 1472.103599] keyboard: can't
emulate rawmode for keycode 240
May 18 13:48:28 ubuntu-Rev-1-0 kernel: [ 1472.103612] keyboard: can't
emulate rawmode for keycode 240

(skip)


It'd be ideal if we can find the real use of the notification and
assign it a corresponding keycode, but now I am not able to find any.

Since it occurs in a period when AC cord unplugged, I suspect it is
just a notification to the Windows App, probably due to that the App
is badly written and the App does not want to poll the battery
information. In Linux the userspace can access the battery information
in an easier way through the ACPI proc/sysfs and it is probably not
true in Windows. It could be the reason/purpose why this notification
is sent in a period.

If so, it should be safe to filter out.

Re: [PATCH 1/2] ideapad-laptop: Ignore an unused VPC notification

[Matthew Garrett]

On Mon, Jul 11, 2011 at 03:29:09PM +0800, Keng-Yü Lin wrote:
> On Thu, Jul 7, 2011 at 11:10 PM, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> > On Tue, Jun 28, 2011 at 02:35:53PM +0100, Keng-Yu Lin wrote:
> >> BIOS fires notifications in a constant period with VPC bit 1 set when the
> >> power cord is unplugged.
> >
> > How frequently? That's going to do bad things to power management. It'd
> > be nice to find a way to just make it stop doing that.
> >
> 
> About every 22 seconds.

Ok, this seems fair then.

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