2012/11/27 Corentin Chary <corentin.chary@xxxxxxxxx>:
> 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 ?
In spite of this, that's totally different laptops, although they both are MSI.
> But as long as you took a look and think
> that's not worth it, I'm ok with that.
OK, thanks.
>> 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
