On Mon, Oct 18, 2010 at 10:12 AM, Dmitry Torokhov
<dmitry.torokhov@xxxxxxxxx> wrote:
> On Sun, Oct 17, 2010 at 10:53:04PM -0600, Joey Lee wrote:
>> Hi Dmitry,
>>
>> æ äï2010-10-13 æ 11:24 -0700ïDmitry Torokhov æåï
>> > Hi Joey,
>> >
>> > On Wed, Oct 13, 2010 at 11:47:40AM +0800, Lee, Chun-Yi wrote:
>> > Â+
>> > > + status = wmi_install_notify_handler(ACERWMID_EVENT_GUID,
>> > > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â acer_wmi_notify, NULL);
>> > > + if (ACPI_FAILURE(status)) {
>> > > + Â Â Â Â err = -EIO;
>> > > + Â Â Â Â goto err_unregister_input;
>> > > + }
>> > > +
>> > > + return 0;
>> > > +
>> > > +err_unregister_input:
>> > > + input_unregister_device(acer_wmi_input_dev);
>> > > +err_free_keymap:
>> > > + sparse_keymap_free(acer_wmi_input_dev);
>> > > +err_free_dev:
>> > > + input_free_device(acer_wmi_input_dev);
>> >
>> > If wmi_install_notify_handler() fails you'll end up doing
>> > input_unregister_device() + input_free_device() which is forbidden. To
>> > avoid this issue register the device after installing the handler. As
>> > long as input device is properly allocated (by calling
>> > input_allocate_device) it is safe to use it to send events (although
>> > they will not go anywhere).
>> >
>> > BTW, I think it is high time we allocate a dedicated key for touchpad
>> > on/off....
>> >
>>
>> Thank's for your review.
>> I changed the error handler priority:
>>
>> >From 9e31867c348a1f2119d16ddbcf9cf0b7de111cf9 Mon Sep 17 00:00:00 2001
>> From: Lee, Chun-Yi <jlee@xxxxxxxxxx>
>> Date: Mon, 18 Oct 2010 12:12:49 +0800
>> Subject: [PATCH 1/3] Add acer wmi hotkey events support
>>
>> Add acer wmi hotkey event support. Install a wmi notify handler to
>> transfer wmi event key to key code, then send out keycode through acer
>> wmi input device to userland.
>>
>> Signed-off-by: Lee, Chun-Yi <jlee@xxxxxxxxxx>
>
> Looks good to me, thank you for making changes.
>
>>
>> @@ -48,6 +50,7 @@ MODULE_LICENSE("GPL");
>> Â#define ACER_ERR KERN_ERR ACER_LOGPREFIX
>> Â#define ACER_NOTICE KERN_NOTICE ACER_LOGPREFIX
>> Â#define ACER_INFO KERN_INFO ACER_LOGPREFIX
>> +#define ACER_WARNING KERN_WARNING ACER_LOGPREFIX
>>
>
> As a separate change could you please move the driver to using pr_err()
> and friends?
>
>> Â/*
>> Â * Magic Number
>> @@ -83,8 +86,39 @@ MODULE_LICENSE("GPL");
>> Â#define WMID_GUID1 Â Â Â Â Â "6AF4F258-B401-42fd-BE91-3D4AC2D7C0D3"
>> Â#define WMID_GUID2 Â Â Â Â Â "95764E09-FB56-4e83-B31A-37761F60994A"
>>
>> +/*
>> + * Acer ACPI event GUIDs
>> + */
>> +#define ACERWMID_EVENT_GUID "676AA15E-6A47-4D9F-A2CC-1E6D18D14026"
>> +
>> ÂMODULE_ALIAS("wmi:67C3371D-95A3-4C37-BB61-DD47B491DAAB");
>> ÂMODULE_ALIAS("wmi:6AF4F258-B401-42fd-BE91-3D4AC2D7C0D3");
>> +MODULE_ALIAS("wmi:676AA15E-6A47-4D9F-A2CC-1E6D18D14026");
>> +
>> +enum acer_wmi_event_ids {
>> + Â Â WMID_HOTKEY_EVENT = 0x1,
>> +};
>> +
>> +static const struct key_entry acer_wmi_keymap[] = {
>> + Â Â {KE_KEY, 0x01, {KEY_WLAN} }, Â Â /* WiFi */
>> + Â Â {KE_KEY, 0x12, {KEY_BLUETOOTH} }, Â Â Â /* BT */
>> + Â Â {KE_KEY, 0x21, {KEY_PROG1} }, Â Â/* Backup */
>> + Â Â {KE_KEY, 0x22, {KEY_PROG2} }, Â Â/* Aracade */
>> + Â Â {KE_KEY, 0x23, {KEY_PROG3} }, Â Â/* P_Key */
>> + Â Â {KE_KEY, 0x24, {KEY_PROG4} }, Â Â/* Social networking_Key */
>> + Â Â {KE_KEY, 0x64, {KEY_SWITCHVIDEOMODE} }, /* Display Switch */
>> + Â Â {KE_KEY, 0x82, {KEY_F22} }, Â Â Â/* Touch Pad On/Off */
>
> We need to standardize this. Some people use F13/F14, here we have
> F22...
>
The good thing with F* keys, is that they are already mapped in X/Qt/SDL/etc..
But if we do, we could add KEY_TOUCHPADTOGGLE 0x1b8
Then bind it to XF86XK_TouchpadToggle (what's the right way to do that
? keymaps ?)
--
Corentin Chary
http://xf.iksaif.net
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
