Hi, On Thu, Dec 9, 2010 at 9:53 PM, Thadeu Lima de Souza Cascardo <cascardo@xxxxxxxxxxxxxx> wrote: > This driver supports some keys for Megaware notebook models. This particular > driver was sponsored by Ulevel <http://ulevel.com>. > > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@xxxxxxxxxxxxxx> > --- > > Perhaps, some function names are a little odd or some of the functions > should be open-coded instead. Comments, please? > > Note that it depends on both the last fix for WMI and the recent touchpad keys > in input.h. > > Thanks, > Cascardo. > > --- > Âdrivers/platform/x86/Kconfig    Â|  Â8 ++ > Âdrivers/platform/x86/Makefile    |  Â1 + > Âdrivers/platform/x86/megaware-wmi.c | Â226 +++++++++++++++++++++++++++++++++++ > Â3 files changed, 235 insertions(+), 0 deletions(-) > Âcreate mode 100644 drivers/platform/x86/megaware-wmi.c > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index 4c7f8b9..9e20ee0 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -651,4 +651,12 @@ config XO1_RFKILL >     ÂSupport for enabling/disabling the WLAN interface on the OLPC XO-1 >     Âlaptop. > > +config MEGAWARE_WMI > +    tristate "Megaware WMI input keys" > +    depends on ACPI_WMI > +    select INPUT depends on INPUT, only use select for small stuff. > +    select INPUT_SPARSEKMAP > +    ---help--- > +     This adds support for the keys in some Megaware notebook models. > + > Âendif # X86_PLATFORM_DEVICES > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile > index 4ec4ff8..e7942f8 100644 > --- a/drivers/platform/x86/Makefile > +++ b/drivers/platform/x86/Makefile > @@ -34,3 +34,4 @@ obj-$(CONFIG_INTEL_IPS)        += intel_ips.o > Âobj-$(CONFIG_GPIO_INTEL_PMIC) Â+= intel_pmic_gpio.o > Âobj-$(CONFIG_XO1_RFKILL)    += xo1-rfkill.o > Âobj-$(CONFIG_IBM_RTL)     Â+= ibm_rtl.o > +obj-$(CONFIG_MEGAWARE_WMI)   += megaware-wmi.o > diff --git a/drivers/platform/x86/megaware-wmi.c b/drivers/platform/x86/megaware-wmi.c > new file mode 100644 > index 0000000..0ea0dac > --- /dev/null > +++ b/drivers/platform/x86/megaware-wmi.c > @@ -0,0 +1,226 @@ > +/* > + * ÂCopyright (C) 2010 ÂThadeu Lima de Souza Cascardo <cascardo@xxxxxxxxxxxxxx> > + * > + * Â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., > + * Â51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. > + */ > + > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/slab.h> > +#include <linux/input.h> > +#include <linux/input/sparse-keymap.h> > +#include <linux/platform_device.h> > +#include <linux/acpi.h> > + > +MODULE_LICENSE("GPL"); > +MODULE_AUTHOR("Thadeu Lima de Souza Cascardo <cascardo@xxxxxxxxxxxxxx>"); > + > +#define MEGAWARE_DRIVER_NAME "megaware" > + > +#define MEGAWARE_GUID_CODE "39142400-C6A3-40FA-BADB-8A2652834100" > +#define MEGAWARE_GUID_EVENT "59142400-C6A3-40FA-BADB-8A2652834100" > +#define MEGAWARE_GUID_INIT "79142400-C6A3-40FA-BADB-8A2652834100" > + > +MODULE_ALIAS("wmi:"MEGAWARE_GUID_INIT); > + > +struct megaware_device { > +    struct input_dev *idev; > +}; > + > +static acpi_status megaware_call_wq00(acpi_handle handle, > +                unsigned long long *ret) > +{ > +    struct acpi_buffer output; > +    union acpi_object *obj; > +    acpi_status status; add an empty line between variable declaration and code ? I don't know if it's in the coding style, but I find it more readable. Apply this comment to all functions. > +    output.length = ACPI_ALLOCATE_BUFFER; > +    output.pointer = NULL; > +    status = wmi_query_block(MEGAWARE_GUID_CODE, 1, &output); > +    if (status) > +        return status; > +    obj = output.pointer; > +    if (obj && obj->type == ACPI_TYPE_INTEGER) > +        *ret = obj->integer.value; > +    else > +        status = AE_TYPE; > +    kfree(obj); > +    return status; > +} > + > +static acpi_status megaware_call_init(struct megaware_device *megaware) > +{ > +    struct acpi_buffer input; > +    unsigned long long value = 0; > +    acpi_status status; > +    input.length = sizeof(value); > +    input.pointer = &value; > +    status = wmi_evaluate_method(MEGAWARE_GUID_INIT, 1, 0, &input, NULL); > +    return status; > +} > + > +static const struct key_entry megaware_keys[] = { > +    { KE_KEY, 0x100, { KEY_WLAN } }, > +    { KE_KEY, 0x101, { KEY_BLUETOOTH } }, > +    { KE_KEY, 0x102, { KEY_PROG1 } }, Maybe you could add a comment to describe the PROG1 key ? > +    { KE_KEY, 0x107, { KEY_SWITCHVIDEOMODE } }, > +    { KE_KEY, 0x108, { KEY_TOUCHPAD_TOGGLE } }, > +    { KE_KEY, 0x109, { KEY_MUTE } }, > +    { KE_KEY, 0x10A, { KEY_VOLUMEUP } }, > +    { KE_KEY, 0x10B, { KEY_VOLUMEDOWN } }, > +    { KE_END, } > +}; > + > +static int megaware_add(struct platform_device *dev) > +{ > +    struct megaware_device *megaware = dev_get_drvdata(&dev->dev); > +    struct input_dev *inputdev; > +    int error; > +    acpi_status status; > + > +    status = megaware_call_init(megaware); > +    if (!ACPI_SUCCESS(status)) > +        return -EIO; > + > +    inputdev = input_allocate_device(); > +    if (!inputdev) > +        return -ENOMEM; > +    inputdev->name = MEGAWARE_DRIVER_NAME "_keys"; > +    inputdev->dev.parent = &dev->dev; > + > +    error = sparse_keymap_setup(inputdev, megaware_keys, NULL); > +    if (error) > +        goto err_alloc; > + > +    error = input_register_device(inputdev); > +    if (error) > +        goto err_keymap; > +    megaware->idev = inputdev; > +    return 0; > + > +err_keymap: > +    sparse_keymap_free(inputdev); > +err_alloc: > +    input_free_device(inputdev); > +    return error; > +} > + > +static int megaware_del(struct platform_device *dev) > +{ > +    struct megaware_device *megaware = dev_get_drvdata(&dev->dev); > +    struct input_dev *inputdev = megaware->idev; > +    sparse_keymap_free(inputdev); > +    input_unregister_device(inputdev); > +    return 0; > +} > + > +static void megaware_notify(u32 event, void *data) > +{ > +    struct megaware_device *megaware = data; > +    struct input_dev *inputdev = megaware->idev; > +    acpi_status status; > +    unsigned long long value = 0; > +    if (event != 0x80) > +        return; > +    status = megaware_call_wq00(megaware, &value); > +    if (!ACPI_SUCCESS(status)) > +        return; > +    sparse_keymap_report_event(inputdev, value, 1, true); Add a trace for non-mapped keys ? Will help to support new models. > +} > + > +static __devinit int megaware_probe(struct platform_device *dev) > +{ > +    struct megaware_device *megaware = dev_get_drvdata(&dev->dev); > +    int ret; > +    acpi_status status; > +    ret = megaware_add(dev); > +    if (ret) > +        return ret; > +    status = wmi_install_notify_handler(MEGAWARE_GUID_EVENT, > +            megaware_notify, megaware); > +    if (ACPI_FAILURE(status)) { > +        megaware_del(dev); > +        ret = -EIO; > +    } > +    return ret; > +} > + > +static __devexit int megaware_remove(struct platform_device *dev) > +{ > +    int ret; > +    wmi_remove_notify_handler(MEGAWARE_GUID_EVENT); > +    ret = megaware_del(dev); > +    return ret; > +} > + > +static struct platform_driver megaware_driver = { > +    .driver = { > +        .owner = THIS_MODULE, > +        .name = MEGAWARE_DRIVER_NAME, > +    }, I'd suggest that you don't use .probe / .remove, because if probe fail you won't know about it and the module will still be loaded. I recently sent a patch for eeepc-wmi (it's in linux-next) to fix the same issue, calling the megaware_add like function directly in the module init function. Another way to go is to use platform_driver_probe/platform_create_bundle. > +    .probe = megaware_probe, > +    .remove = __devexit_p(megaware_remove), > +}; > + > +static struct platform_device *megaware_device; > + > +static __init int megaware_init(void) > +{ > +    struct megaware_device *megaware; > +    int ret; > +    if (!wmi_has_guid(MEGAWARE_GUID_INIT)) { Maybe you could also check MEGAWARE_GUID_CODE here ? Or even the three GUID ? (but the event guid should already be checked when installing the notify handler). > +        printk(KERN_WARNING "Could not find megaware WMI device.\n"); Use pr_* functions instead of printk ? This way it will add a prefix automatically. > +        ret = -ENODEV; > +        goto out_guid; > +    } > +    megaware = kzalloc(sizeof(*megaware), GFP_KERNEL); > +    if (!megaware) { > +        ret = -ENOMEM; > +        goto out_alloc; > +    } > +    megaware_device = platform_device_alloc(MEGAWARE_DRIVER_NAME, 0); > +    if (!megaware_device) { > +        ret = -ENOMEM; > +        goto out_devalloc; > +    } > +    ret = platform_device_add(megaware_device); > +    if (ret) > +        goto out_devadd; > +    dev_set_drvdata(&megaware_device->dev, megaware); > +    ret = platform_driver_register(&megaware_driver); > +    if (ret) > +        goto out_driver; > +    return 0; > +out_driver: > +    platform_device_del(megaware_device); > +out_devadd: > +    platform_device_put(megaware_device); > +out_devalloc: > +    kfree(megaware); > +out_alloc: > +out_guid: > +    return ret; > +} > + > +static __exit void megaware_exit(void) > +{ > +    struct megaware_device *megaware; > +    platform_driver_unregister(&megaware_driver); > +    megaware = dev_get_drvdata(&megaware_device->dev); > +    platform_device_unregister(megaware_device); > +    kfree(megaware); > +} > + > +module_init(megaware_init); > +module_exit(megaware_exit); > -- > 1.7.2.3 Otherwise, this looks like a clean wmi driver :) Thanks. -- Corentin Chary http://xf.iksaif.net -- 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
