Hi Yong,
On Fri, Mar 19, 2010 at 09:39:24PM +0800, Yong Wang wrote:
> +
> +struct key_entry {
> + char type;
> + u8 code;
> + u16 keycode;
> +};
> +
Like Matthew mentioned, please switch to sparse-keymap library.
> +
> +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);
> +
> + 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 = eeepc_wmi_get_entry_by_scancode(code);
> +
> + if (key) {
> + input_report_key(eeepc_wmi_input_dev, key->keycode, 1);
> + input_sync(eeepc_wmi_input_dev);
> + input_report_key(eeepc_wmi_input_dev, key->keycode, 0);
> + input_sync(eeepc_wmi_input_dev);
> + } else
> + printk(KERN_INFO "EEEPC WMI: Unknown key %x pressed\n", code);
> + }
You are leaking memory pointed to by obj.
> +
> +static int __init eeepc_wmi_init(void)
> +{
> + int err;
> +
> + if (wmi_has_guid(EEEPC_WMI_EVENT_GUID)) {
> + err = wmi_install_notify_handler(EEEPC_WMI_EVENT_GUID,
> + eeepc_wmi_notify, NULL);
> + if (ACPI_FAILURE(err))
> + return -EINVAL;
> +
It is not a good idea to install notify handler before registering input
devive. If event comes too early you will oops. In fact this is a common
problem in many platfrom drivers.
> + err = eeepc_wmi_input_setup();
> + if (err) {
> + wmi_remove_notify_handler(EEEPC_WMI_EVENT_GUID);
> + return err;
> + }
> + }
> +
> + return 0;
Do not return success if GUID is not found - do
if (!wmi_has_guid(EEEPC_WMI_EVENT_GUID))
return -ENODEV;
instead.
> +}
> +
> +static void __exit eeepc_wmi_exit(void)
> +{
> + if (wmi_has_guid(EEEPC_WMI_EVENT_GUID)) {
... and you won't have to check guid here.
> + wmi_remove_notify_handler(EEEPC_WMI_EVENT_GUID);
> + input_unregister_device(eeepc_wmi_input_dev);
> + }
> +}
> +
> +module_init(eeepc_wmi_init);
> +module_exit(eeepc_wmi_exit);
Thanks.
--
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
