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@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
>>>> ---
>>>> Â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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/