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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 05 Jul 2011 15:26:08 +0800
From:	Ike Panhc <ike.pan@...onical.com>
To:	Keng-Yü Lin <kengyu@...onical.com>
CC:	Matthew Garrett <mjg@...hat.com>,
	platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org,
	vanhoof@...onical.com, kent.lin@...onical.com, sylee@...onical.com
Subject: Re: [PATCH 2/2] ideapad-laptop: Handle the App-controlled RF event

On 06/30/2011 06:34 PM, Keng-Yü Lin wrote:
> On Wed, Jun 29, 2011 at 2:56 PM, Ike Panhc <ike.pan@...onical.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@...onical.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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/