On Tue, Nov 27, 2012 at 3:55 PM, Maxim Mikityanskiy <maxtram95@xxxxxxxxx> wrote:
> 2012/11/27 Corentin Chary <corentin.chary@xxxxxxxxx>:
>> On Tue, Nov 27, 2012 at 8:14 AM, joeyli <jlee@xxxxxxxx> wrote:
>>> Hi Maxim,
>>>
>>> 於 日，2012-11-25 於 00:29 +0200，Maxim Mikityanskiy 提到：
>>>> Create a driver to support WMI found on MSI Wind laptops. This driver
>>>> allows us to receive some additional keycodes for keys that are not
>>>> handled without this driver.
>>>>
>>>> Signed-off-by: Maxim Mikityanskiy <maxtram95@xxxxxxxxx>
>>>
>>> I simply review this patch and looks there have many code the same with
>>> msi-wmi.c. Does there have any reason didn't direct patch msi-wmi but
>>> need generate a new driver?
>>
>> Right, if it's only a keymap and guid change, plus 1 or 2 quirks, it
>> may makes sense to be in msi-wind.c
>
> Compare msi-wind-wmi.c and dell-wmi-aio.c. They look very similar,
> even more similar than msi-wind-wmi.c and msi-wmi.c. Why do you
> recommend to merge msi-wind-wmi features into msi-wmi, but not
> dell-wmi-aio?
Well, because both are msi ? But as long as you took a look and think
that's not worth it, I'm ok with that.
> Look at dell-wmi.c, dell-wmi-aio.c and msi-wmi.c. They have lots of
> common code and they differ only in GUIDs, keymap tables and some
> quirks, but they are still different modules.
>
> msi-wind-wmi is WMI driver that only supports hotkey handling. Each
> WMI driver providing this feature contains such code, so there is
> nothing strange in that msi-wmi driver also contains such code.
> msi-wmi is a driver for fully different laptop with fully different
> WMI. It also handles backlight brightness and needs a quirk for hotkey
> handling. MSI Wind WMI does not send brightness change events and does
> not need that quirk.
>
> Do you still think I should merge msi-wind-wmi code into msi-wmi?
>
>>
>>>
>>> Thanks a lot!
>>> Joey Lee
>>>
>>>> ---
>>>> drivers/platform/x86/Kconfig | 13 +++
>>>> drivers/platform/x86/Makefile | 1 +
>>>> drivers/platform/x86/msi-wind-wmi.c | 169 ++++++++++++++++++++++++++++++++++++
>>>> 3 files changed, 183 insertions(+)
>>>> create mode 100644 drivers/platform/x86/msi-wind-wmi.c
>>>>
>>>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>>>> index c86bae8..46f269e5 100644
>>>> --- a/drivers/platform/x86/Kconfig
>>>> +++ b/drivers/platform/x86/Kconfig
>>>> @@ -561,6 +561,19 @@ config MSI_WMI
>>>> To compile this driver as a module, choose M here: the module will
>>>> be called msi-wmi.
>>>>
>>>> +config MSI_WIND_WMI
>>>> + tristate "MSI Wind WMI Driver"
>>>> + depends on ACPI_WMI
>>>> + depends on INPUT
>>>> + select INPUT_SPARSEKMAP
>>>> + ---help---
>>>> + Say Y here if you want to support WMI-based hotkeys on MSI Wind
>>>> + laptops. MSI Wind WMI differs from WMI found on other MSI laptops, so
>>>> + say Y here if you have MSI Wind, otherwise select "MSI WMI extras".
>>>> +
>>>> + To compile this driver as a module, choose M here: the module will
>>>> + be called msi-wind-wmi.
>>>> +
>>>> config TOPSTAR_LAPTOP
>>>> tristate "Topstar Laptop Extras"
>>>> depends on ACPI
>>>> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
>>>> index bf7e4f9..44389c0 100644
>>>> --- a/drivers/platform/x86/Makefile
>>>> +++ b/drivers/platform/x86/Makefile
>>>> @@ -30,6 +30,7 @@ obj-$(CONFIG_INTEL_MENLOW) += intel_menlow.o
>>>> obj-$(CONFIG_ACPI_WMI) += wmi.o
>>>> obj-$(CONFIG_MSI_WMI) += msi-wmi.o
>>>> obj-$(CONFIG_TOPSTAR_LAPTOP) += topstar-laptop.o
>>>> +obj-$(CONFIG_MSI_WIND_WMI) += msi-wind-wmi.o
>>>>
>>>> # toshiba_acpi must link after wmi to ensure that wmi devices are found
>>>> # before toshiba_acpi initializes
>>>> diff --git a/drivers/platform/x86/msi-wind-wmi.c b/drivers/platform/x86/msi-wind-wmi.c
>>>> new file mode 100644
>>>> index 0000000..4f19434
>>>> --- /dev/null
>>>> +++ b/drivers/platform/x86/msi-wind-wmi.c
>>>> @@ -0,0 +1,169 @@
>>>> +/*
>>>> + * MSI Wind WMI hotkeys
>>>> + *
>>>> + * Copyright (C) 2012 Maxim Mikityanskiy <maxtram95@xxxxxxxxx>
>>>> + *
>>>> + * 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
>>>> + */
>>>> +
>>>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>>>> +
>>>> +#include <linux/module.h>
>>>> +#include <linux/input.h>
>>>> +#include <linux/input/sparse-keymap.h>
>>>> +#include <linux/acpi.h>
>>>> +
>>>> +MODULE_AUTHOR("Maxim Mikityanskiy <maxtram95@xxxxxxxxx>");
>>>> +MODULE_DESCRIPTION("MSI Wind laptop WMI hotkeys driver");
>>>> +MODULE_LICENSE("GPL");
>>>> +
>>>> +#define WMI_EVENT_GUID "5B3CC38A-40D9-7245-8AE6-1145B751BE3F"
>>>> +
>>>> +MODULE_ALIAS("wmi:" WMI_EVENT_GUID);
>>>> +
>>>> +/* Fn+F3 touchpad toggle */
>>>> +#define WIND_KEY_TOUCHPAD 0x08
>>>> +/* Fn+F11 Bluetooth toggle */
>>>> +#define WIND_KEY_BLUETOOTH 0x56
>>>> +/* Fn+F6 webcam toggle */
>>>> +#define WIND_KEY_CAMERA 0x57
>>>> +/* Fn+F11 Wi-Fi toggle */
>>>> +#define WIND_KEY_WLAN 0x5f
>>>> +/* Fn+F10 turbo mode toggle */
>>>> +#define WIND_KEY_TURBO 0x60
>>>> +/* Fn+F10 ECO mode toggle */
>>>> +#define WIND_KEY_ECO 0x69
>>>> +
>>>> +static struct key_entry wind_keymap[] = {
>>>> + /* These keys work without WMI. Ignore them to avoid double keycodes */
>>>> + { KE_IGNORE, WIND_KEY_TOUCHPAD, { KEY_TOUCHPAD_TOGGLE } },
>>>> + { KE_IGNORE, WIND_KEY_BLUETOOTH, { KEY_BLUETOOTH } },
>>>> + { KE_IGNORE, WIND_KEY_CAMERA, { KEY_CAMERA } },
>>>> + { KE_IGNORE, WIND_KEY_WLAN, { KEY_WLAN } },
>>>> + /* These are keys that should be handled via WMI */
>>>> + { KE_KEY, WIND_KEY_TURBO, { KEY_PROG1 } },
>>>> + { KE_KEY, WIND_KEY_ECO, { KEY_PROG2 } },
>>>> + { KE_END, 0 }
>>>> +};
>>>> +
>>>> +static struct input_dev *wind_input_dev;
>>>> +
>>>> +static void wind_wmi_notify(u32 value, void *context)
>>>> +{
>>>> + struct acpi_buffer response = {
>>>> + .length = ACPI_ALLOCATE_BUFFER,
>>>> + .pointer = NULL
>>>> + };
>>>> + acpi_status status = wmi_get_event_data(value, &response);
>>>> + union acpi_object *obj = response.pointer;
>>>> + int code;
>>>> +
>>>> + if (status != AE_OK) {
>>>> + pr_warn("Bad event status %#x\n", status);
>>>> + return;
>>>> + }
>>>> +
>>>> + if (!obj || obj->type != ACPI_TYPE_INTEGER) {
>>>> + pr_warn("Unknown event received\n");
>>>> + goto wind_wmi_notify_free;
>>>> + }
>>>> +
>>>> + code = obj->integer.value;
>>>> + pr_debug("Event code: %#x\n", code);
>>>> +
>>>> + if (code == 0x00 || code == 0x62 || code == 0x63) {
>>>> + /* Unknown events - drop them for now */
>>>> + goto wind_wmi_notify_free;
>>>> + }
>>>> +
>>>> + if (!sparse_keymap_report_event(wind_input_dev, code, 1, true))
>>>> + pr_warn("Unknown key %#x pressed\n", code);
>>>> +
>>>> +wind_wmi_notify_free:
>>>> + kfree(response.pointer);
>>>> +}
>>>> +
>>>> +static int __init wind_input_setup(void)
>>>> +{
>>>> + int err;
>>>> +
>>>> + wind_input_dev = input_allocate_device();
>>>> + if (!wind_input_dev)
>>>> + return -ENOMEM;
>>>> +
>>>> + wind_input_dev->name = "MSI WMI hotkeys";
>>>> + wind_input_dev->phys = "wmi/input0";
>>>> + wind_input_dev->id.bustype = BUS_HOST;
>>>> +
>>>> + err = sparse_keymap_setup(wind_input_dev, wind_keymap, NULL);
>>>> + if (err)
>>>> + goto wind_input_setup_free_dev;
>>>> +
>>>> + err = input_register_device(wind_input_dev);
>>>> + if (err)
>>>> + goto wind_input_setup_free_keymap;
>>>> +
>>>> + return 0;
>>>> +
>>>> +wind_input_setup_free_keymap:
>>>> + sparse_keymap_free(wind_input_dev);
>>>> +wind_input_setup_free_dev:
>>>> + input_free_device(wind_input_dev);
>>>> + return err;
>>>> +}
>>>> +
>>>> +static int __init wind_wmi_init(void)
>>>> +{
>>>> + int err;
>>>> +
>>>> + if (!wmi_has_guid(WMI_EVENT_GUID)) {
>>>> + pr_err("No MSI Wind WMI found\n");
>>>> + return -ENODEV;
>>>> + }
>>>> +
>>>> + err = wind_input_setup();
>>>> + if (err) {
>>>> + pr_err("Unable to setup input device\n");
>>>> + return err;
>>>> + }
>>>> +
>>>> + if (ACPI_FAILURE(wmi_install_notify_handler(WMI_EVENT_GUID,
>>>> + wind_wmi_notify, NULL))) {
>>>> + pr_err("Unable to install WMI notify handler\n");
>>>> + err = -EIO;
>>>> + goto wind_wmi_init_unregister_input;
>>>> + }
>>>> +
>>>> + pr_debug("Event handler installed\n");
>>>> +
>>>> + return 0;
>>>> +
>>>> +wind_wmi_init_unregister_input:
>>>> + input_unregister_device(wind_input_dev);
>>>> + sparse_keymap_free(wind_input_dev);
>>>> + input_free_device(wind_input_dev);
>>>> + return err;
>>>> +}
>>>> +
>>>> +static void __exit wind_wmi_exit(void)
>>>> +{
>>>> + wmi_remove_notify_handler(WMI_EVENT_GUID);
>>>> + input_unregister_device(wind_input_dev);
>>>> + sparse_keymap_free(wind_input_dev);
>>>> + input_free_device(wind_input_dev);
>>>> +}
>>>> +
>>>> +module_init(wind_wmi_init);
>>>> +module_exit(wind_wmi_exit);
>>>
>>>
>>> --
>>> 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
>>
>>
>>
>> --
>> Corentin Chary
>> http://xf.iksaif.net
--
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
