Hi Yong,
On Sat, Mar 20, 2010 at 02:03:41PM +0800, Yong Wang wrote:
> Changes since v1:
> 1. Use sparse keymap
> 2. Not send brightness input events
> 3. Fix memory leak of acpi_object
> 4. Set up input device before registering wmi notifier
> 5. Not return success if GUID is not found
> 6. Not check guid here in wmi_exit
>
> Matthew and Dmitry, thank you again for all your review comments!
>
Thank you for making changes, the driver shapes very nicely. Still a few
things left to do ;)
> ---
> Add a WMI driver for Eee PC laptops. Currently it only supports hotkeys.
>
> Signed-off-by: Yong Wang <yong.y.wang@xxxxxxxxx>
> ---
> drivers/platform/x86/Kconfig | 10 +++
> drivers/platform/x86/Makefile | 1
> drivers/platform/x86/eeepc-wmi.c | 158
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 169 insertions(+)
>
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index e631dbe..7bec458 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -385,6 +385,16 @@ config EEEPC_LAPTOP
>
> If you have an Eee PC laptop, say Y or M here.
>
> +config EEEPC_WMI
> + tristate "Eee PC WMI Hotkey Driver (EXPERIMENTAL)"
> + depends on ACPI_WMI
> + depends on INPUT
> + depends on EXPERIMENTAL
> + ---help---
> + Say Y here if you want to support WMI-based hotkeys on Eee PC laptops.
> +
> + To compile this driver as a module, choose M here: the module will
> + be called eeepc-wmi.
>
> config ACPI_WMI
> tristate "WMI"
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index 9cd9fa0..a906490 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -4,6 +4,7 @@
> #
> obj-$(CONFIG_ASUS_LAPTOP) += asus-laptop.o
> obj-$(CONFIG_EEEPC_LAPTOP) += eeepc-laptop.o
> +obj-$(CONFIG_EEEPC_WMI) += eeepc-wmi.o
> obj-$(CONFIG_MSI_LAPTOP) += msi-laptop.o
> obj-$(CONFIG_ACPI_CMPC) += classmate-laptop.o
> obj-$(CONFIG_COMPAL_LAPTOP) += compal-laptop.o
> diff --git a/drivers/platform/x86/eeepc-wmi.c b/drivers/platform/x86/eeepc-wmi.c
> new file mode 100644
> index 0000000..608414b
> --- /dev/null
> +++ b/drivers/platform/x86/eeepc-wmi.c
> @@ -0,0 +1,158 @@
> +/*
> + * Eee PC WMI hotkey driver
> + *
> + * Copyright(C) 2010 Intel Corporation.
> + *
> + * Portions based on wistron_btns.c:
> + * Copyright (C) 2005 Miloslav Trmac <mitr@xxxxxxxx>
> + * Copyright (C) 2005 Bernhard Rosenkraenzer <bero@xxxxxxxxxxxx>
> + * Copyright (C) 2005 Dmitry Torokhov <dtor@xxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/types.h>
> +#include <linux/input.h>
> +#include <linux/input/sparse-keymap.h>
> +#include <acpi/acpi_bus.h>
> +#include <acpi/acpi_drivers.h>
> +
> +MODULE_AUTHOR("Yong Wang <yong.y.wang@xxxxxxxxx>");
> +MODULE_DESCRIPTION("Eee PC WMI Hotkey Driver");
> +MODULE_LICENSE("GPL");
> +
> +#define EEEPC_WMI_EVENT_GUID "ABBC0F72-8EA1-11D1-00A0-C90629100000"
> +
> +MODULE_ALIAS("wmi:"EEEPC_WMI_EVENT_GUID);
> +
> +#define NOTIFY_BRNUP_MIN 0x11
> +#define NOTIFY_BRNUP_MAX 0x1f
> +#define NOTIFY_BRNDOWN_MIN 0x20
> +#define NOTIFY_BRNDOWN_MAX 0x2e
> +
> +static struct key_entry eeepc_wmi_keymap[] = {
Make it const as well?
> + /* Sleep already handled via generic ACPI code */
> + { KE_KEY, 0x5d, { KEY_WLAN } },
> + { KE_KEY, 0x32, { KEY_MUTE } },
> + { KE_KEY, 0x31, { KEY_VOLUMEDOWN } },
> + { KE_KEY, 0x30, { KEY_VOLUMEUP } },
> + { KE_KEY, NOTIFY_BRNDOWN_MIN, { KEY_BRIGHTNESSDOWN } },
> + { KE_KEY, NOTIFY_BRNUP_MIN, { KEY_BRIGHTNESSUP } },
> + { KE_KEY, 0xcc, { KEY_SWITCHVIDEOMODE } },
> + { KE_END, 0},
> +};
> +
> +static struct input_dev *eeepc_wmi_input_dev;
> +
> +static void eeepc_wmi_notify(u32 value, void *context)
> +{
> + struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL };
> + static struct key_entry *key;
> + union acpi_object *obj;
> + int code;
> +
> + wmi_get_event_data(value, &response);
I wonder if we need to check return value of the call to make sure
response data is valid.
> +
> + obj = (union acpi_object *)response.pointer;
> +
> + if (obj && obj->type == ACPI_TYPE_INTEGER) {
> + code = obj->integer.value;
> +
> + if (code >= NOTIFY_BRNUP_MIN && code <= NOTIFY_BRNUP_MAX)
> + code = NOTIFY_BRNUP_MIN;
> + else if (code >= NOTIFY_BRNDOWN_MIN && code <= NOTIFY_BRNDOWN_MAX)
> + code = NOTIFY_BRNDOWN_MIN;
> +
> + key = sparse_keymap_entry_from_scancode(eeepc_wmi_input_dev, code);
> +
> + if (!key)
> + printk(KERN_INFO "EEEPC WMI: Unknown key %x pressed\n", code);
> + else if (key->keycode == KEY_BRIGHTNESSDOWN ||
> + key->keycode == KEY_BRIGHTNESSUP)
> + ;
> + else
> + sparse_keymap_report_entry(eeepc_wmi_input_dev, key, 1, true);
I think you can mark NOTIFY_BRNDOWN_MIN and NOTIFY_BRNDOWN_MIN as KE_IGNORE
and then simply do:
if (!sparse_keymap_report_event(eeepc_wmi_input_dev,
code, 1, true))
pr_err("Unknown key %x pressed\n", code);
BTW, please consider switching to pr_err(), pr_warning(), etc.
> + }
> +
> + kfree(obj);
> +}
> +
> +static int eeepc_wmi_input_setup(void)
> +{
> + int err;
> +
> + eeepc_wmi_input_dev = input_allocate_device();
> + if (!eeepc_wmi_input_dev)
> + return -ENOMEM;
> +
> + eeepc_wmi_input_dev->name = "Eee PC WMI hotkeys";
> + eeepc_wmi_input_dev->phys = "wmi/input0";
> + eeepc_wmi_input_dev->id.bustype = BUS_HOST;
> +
> + err = sparse_keymap_setup(eeepc_wmi_input_dev, eeepc_wmi_keymap, NULL);
> + if (err)
> + goto err_free_dev;
> +
> + err = input_register_device(eeepc_wmi_input_dev);
> + if (err)
> + goto err_free_keymap;
> +
> + return 0;
> +
> +err_free_keymap:
> + sparse_keymap_free(eeepc_wmi_input_dev);
> +err_free_dev:
> + input_free_device(eeepc_wmi_input_dev);
> + return err;
> +}
> +
> +static int __init eeepc_wmi_init(void)
> +{
> + int err;
> + acpi_status status;
> +
> + if (!wmi_has_guid(EEEPC_WMI_EVENT_GUID)) {
> + printk(KERN_WARNING "EEEPC WMI: No known WMI GUID found\n");
> + return -ENODEV;
> + }
> +
> + err = eeepc_wmi_input_setup();
> + if (err)
> + return err;
> +
> + status = wmi_install_notify_handler(EEEPC_WMI_EVENT_GUID,
> + eeepc_wmi_notify, NULL);
> + if (ACPI_FAILURE(status)) {
> + input_unregister_device(eeepc_wmi_input_dev);
You need to free keymap as well. BTW, you need to do that after
unregistering device, because otherwise there is a chance someone
will try to modify keymap while device is still registered. I need
to fix a couple of drivers in input too...
> + printk(KERN_ERR
> + "EEEPC WMI: Unable to register notify handler - %d\n",
> + status);
> + return -ENODEV;
> + }
> +
> + return 0;
> +}
> +
> +static void __exit eeepc_wmi_exit(void)
> +{
> + wmi_remove_notify_handler(EEEPC_WMI_EVENT_GUID);
> + input_unregister_device(eeepc_wmi_input_dev);
Same here, free keymap.
> +}
> +
> +module_init(eeepc_wmi_init);
> +module_exit(eeepc_wmi_exit);
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
