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

[Ike Panhc <ike.pan@canonical.com>]

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.