Hi Lee,
On Fri, Oct 01, 2010 at 09:51:27PM +0800, Lee, Chun-Yi wrote:
> +
> +struct key_entry {
> + char type; /* See KE_* below */
> + u16 code;
> + u16 keycode;
> +};
> +
> +enum { KE_KEY, KE_END };
> +
Like Corentin said, please use sparse_keymap, it will cut the code in
half.
> +
> + set_bit(EV_SW, acer_wmi_input_dev->evbit);
> +
I do not see you sending SW_* events...
> + err = input_register_device(acer_wmi_input_dev);
> +
> + if (err) {
> + input_free_device(acer_wmi_input_dev);
> + return err;
> + }
> +
> + return 0;
> +}
> +
> /*
> * debugfs functions
> */
> @@ -1327,6 +1518,18 @@ static int __init acer_wmi_init(void)
> "generic video driver\n");
> }
>
> + if (wmi_has_guid(ACERWMID_EVENT_GUID)) {
> + err = wmi_install_notify_handler(ACERWMID_EVENT_GUID,
> + acer_wmi_notify, NULL);
> + if (ACPI_FAILURE(err))
> + return -EINVAL;
> + err = acer_wmi_input_setup();
You really want to set up the device first and install notify handler
later. What will happen if event will fire up while input device has not
been created yet?
> + if (err) {
> + wmi_remove_notify_handler(ACERWMID_EVENT_GUID);
> + return err;
> + }
> + }
> +
> err = platform_driver_register(&acer_platform_driver);
> if (err) {
> printk(ACER_ERR "Unable to register platform driver.\n");
> @@ -1368,11 +1571,21 @@ error_device_add:
> error_device_alloc:
> platform_driver_unregister(&acer_platform_driver);
> error_platform_register:
> + if (wmi_has_guid(ACERWMID_EVENT_GUID)) {
> + input_unregister_device(acer_wmi_input_dev);
> + wmi_remove_notify_handler(ACERWMID_EVENT_GUID);
Same here, first shut off notified, then remove the device.
> + }
> +
> return err;
> }
>
> static void __exit acer_wmi_exit(void)
> {
> + if (wmi_has_guid(ACERWMID_EVENT_GUID)) {
> + wmi_remove_notify_handler(ACERWMID_EVENT_GUID);
> + input_unregister_device(acer_wmi_input_dev);
Same here.
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
