| Message ID | 20181102041120.7603-2-ayman.bagabas@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Huawei laptops WMI & sound fixes | expand |
On Fri, Nov 2, 2018 at 6:11 AM Ayman Bagabas <ayman.bagabas@gmail.com> wrote: > > This driver adds support for missing hotkeys on some Huawei laptops. > Currently, only Huawei Matebook X Pro is supported. The driver > recognizes the following keys: brightness keys, micmute, wlan, and > Huawei special key. The brightness keys are ignored since they work out > of the box. > +config HUAWEI_LAPTOP > + tristate "Huawei WMI hotkeys driver" > + depends on ACPI > + depends on ACPI_WMI > + depends on INPUT > + select INPUT_SPARSEKMAP > + help > + This driver provides support for Huawei WMI hotkeys. > + It enables the missing keys and adds support to micmute > + led found on these laptops.q > + Supported devices are: > + - Matebook X Pro > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Huawei WMI Hotkeys Driver > + * > + * Copyright (C) 2018 Ayman Bagabas <ayman.bagabas@gmail.com> > + * > + * 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, see <https://www.gnu.org/licenses/>. > + * > + */ > +MODULE_LICENSE("GPL"); Here you have the following issues: - inconsistency between IDs for license (fix accordingly) - SDPX _and_ License header (remove latter) > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/init.h> Choose one of them. > +#include <linux/input.h> > +#include <linux/input/sparse-keymap.h> > +#include <linux/acpi.h> > +#include <linux/wmi.h> Please, keep in order. > +static const struct key_entry huawei_wmi_keymap[] __initconst = { > + { KE_IGNORE, 0x281, { KEY_BRIGHTNESSDOWN } }, > + { KE_IGNORE, 0x282, { KEY_BRIGHTNESSUP } }, > + { KE_KEY, 0x287, { KEY_MICMUTE } }, > + { KE_KEY, 0x289, { KEY_WLAN } }, > + // Huawei |M| button > + { KE_KEY, 0x28a, { KEY_PROG1 } }, > + { KE_END, 0 } > +}; > + > +struct huawei_wmi_device { > + struct input_dev *inputdev; > +}; Apparently no need to have this, you may use struct input_dev directly, isn't it? > +static struct huawei_wmi_device *wmi_device; No global variables, please. > +int huawei_wmi_micmute_led_set(bool on) > +{ > + u32 args = (on) ? MICMUTE_LED_ON : MICMUTE_LED_OFF; Redundant parens. > + struct acpi_buffer input = { (acpi_size)sizeof(args), &args }; > + acpi_status status; > + > + status = wmi_evaluate_method(AMW0_GUID, 0, 1, &input, NULL); > + if (ACPI_FAILURE(status)) > + return status; > + > + return 0; > +} > +static void huawei_wmi_process_key(struct input_dev *input_dev, int code) > +{ > + const struct key_entry *key; > + > + key = sparse_keymap_entry_from_scancode(input_dev, code); > + This blank line is redundant. > + if (!key) { > + pr_info("%s: Unknown key pressed, code: 0x%04x\n", > + MODULE_NAME, code); dev_info(); no MODULE_NAME, please. > + return; > + } > + > + sparse_keymap_report_entry(input_dev, key, 1, true); > +} > +static void huawei_wmi_notify(u32 value, void *context) > +{ > + struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL }; > + union acpi_object *obj; > + acpi_status status; > + > + status = wmi_get_event_data(value, &response); > + if (ACPI_FAILURE(status)) { > + pr_err("%s: Bad event status 0x%x\n", > + MODULE_NAME, status); No MODULE_NAME, please. If needed, use pr_fmt() macro. But I'm pretty sure you may pass pointer to input device and use dev_err(). > + return; > + } > + > + obj = (union acpi_object *)response.pointer; > + No redundant blank lines, please. > + if (!obj) > + return; > + > + if (obj->type == ACPI_TYPE_INTEGER) > + huawei_wmi_process_key(wmi_device->inputdev, > + obj->integer.value); > + else > + pr_info("%s: Unknown response received %d\n", > + MODULE_NAME, obj->type); Ditto about module name. > + > + kfree(response.pointer); > +} > + > +static int huawei_input_init(void) > +{ > + acpi_status status; > + int err; > + status = wmi_install_notify_handler(EVENT_GUID, > + huawei_wmi_notify, > + NULL); Pointer to the input device instead of NULL. > + No redundant blank lines, please. > + if (ACPI_FAILURE(status)) { > + err = -EIO; > + goto err_free_dev; > + } > +} > + wmi_device = kmalloc(sizeof(struct huawei_wmi_device), GFP_KERNEL); sizeof(*wmi_device) > + if (!wmi_device) > + return -ENOMEM; > +} > + pr_debug("%s: Driver unloaded successfully\n", MODULE_NAME); Noise. -- With Best Regards, Andy Shevchenko
On Fri, 2018-11-02 at 20:20 +0200, Andy Shevchenko wrote: > On Fri, Nov 2, 2018 at 6:11 AM Ayman Bagabas <ayman.bagabas@gmail.com > > wrote: > > > > This driver adds support for missing hotkeys on some Huawei > > laptops. > > Currently, only Huawei Matebook X Pro is supported. The driver > > recognizes the following keys: brightness keys, micmute, wlan, and > > Huawei special key. The brightness keys are ignored since they work > > out > > of the box. > > +config HUAWEI_LAPTOP > > + tristate "Huawei WMI hotkeys driver" > > + depends on ACPI > > + depends on ACPI_WMI > > > > + depends on INPUT > > + select INPUT_SPARSEKMAP > > + help > > + This driver provides support for Huawei WMI hotkeys. > > + It enables the missing keys and adds support to micmute > > + led found on these laptops.q > > + Supported devices are: > > + - Matebook X Pro > > > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Huawei WMI Hotkeys Driver > > + * > > + * Copyright (C) 2018 Ayman Bagabas < > > ayman.bagabas@gmail.com> > > + * > > + * 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, see < > > https://www.gnu.org/licenses/>. > > + * > > + */ > > +MODULE_LICENSE("GPL"); > > > Here you have the following issues: > - inconsistency between IDs for license (fix accordingly) > - SDPX _and_ License header (remove latter) > > > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > +#include <linux/init.h> > > Choose one of them. > > > +#include <linux/input.h> > > +#include <linux/input/sparse-keymap.h> > > +#include <linux/acpi.h> > > +#include <linux/wmi.h> > > Please, keep in order. What order do I have to follow? Does this look right? module.h init.h apci.h wmi.h input.h sparse-keymap.h > > > +static const struct key_entry huawei_wmi_keymap[] __initconst = { > > + { KE_IGNORE, 0x281, { KEY_BRIGHTNESSDOWN } }, > > + { KE_IGNORE, 0x282, { KEY_BRIGHTNESSUP } }, > > + { KE_KEY, 0x287, { KEY_MICMUTE } }, > > + { KE_KEY, 0x289, { KEY_WLAN } }, > > + // Huawei |M| button > > + { KE_KEY, 0x28a, { KEY_PROG1 } }, > > + { KE_END, 0 } > > +}; > > + > > +struct huawei_wmi_device { > > + struct input_dev *inputdev; > > +}; > > Apparently no need to have this, you may use struct input_dev > directly, isn't it? > > > +static struct huawei_wmi_device *wmi_device; > > No global variables, please. What about input_dev as a global variable? How would input_unregister_device access input_dev on exit if it wasn't global? > > > +int huawei_wmi_micmute_led_set(bool on) > > +{ > > + u32 args = (on) ? MICMUTE_LED_ON : MICMUTE_LED_OFF; > > Redundant parens. > > > + struct acpi_buffer input = { (acpi_size)sizeof(args), &args > > }; > > + acpi_status status; > > + > > + status = wmi_evaluate_method(AMW0_GUID, 0, 1, &input, > > NULL); > > + if (ACPI_FAILURE(status)) > > + return status; > > + > > + return 0; > > +} > > +static void huawei_wmi_process_key(struct input_dev *input_dev, > > int code) > > +{ > > + const struct key_entry *key; > > + > > + key = sparse_keymap_entry_from_scancode(input_dev, code); > > + > > This blank line is redundant. > > > + if (!key) { > > + pr_info("%s: Unknown key pressed, code: 0x%04x\n", > > + MODULE_NAME, code); > > dev_info(); no MODULE_NAME, please. > > > + return; > > + } > > + > > + sparse_keymap_report_entry(input_dev, key, 1, true); > > +} > > +static void huawei_wmi_notify(u32 value, void *context) > > +{ > > + struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL > > }; > > + union acpi_object *obj; > > + acpi_status status; > > + > > + status = wmi_get_event_data(value, &response); > > + if (ACPI_FAILURE(status)) { > > + pr_err("%s: Bad event status 0x%x\n", > > + MODULE_NAME, status); > > No MODULE_NAME, please. > If needed, use pr_fmt() macro. > > But I'm pretty sure you may pass pointer to input device and use > dev_err(). > > > + return; > > + } > > + > > + obj = (union acpi_object *)response.pointer; > > + > > No redundant blank lines, please. > > > + if (!obj) > > + return; > > + > > + if (obj->type == ACPI_TYPE_INTEGER) > > + huawei_wmi_process_key(wmi_device->inputdev, > > + obj- > > >integer.value); > > + else > > + pr_info("%s: Unknown response received %d\n", > > + MODULE_NAME, obj->type); > > Ditto about module name. > > > + > > + kfree(response.pointer); > > +} > > + > > +static int huawei_input_init(void) > > +{ > > + acpi_status status; > > + int err; > > + status = wmi_install_notify_handler(EVENT_GUID, > > + huawei_wmi_notify, > > + NULL); > > Pointer to the input device instead of NULL. > > > + > > No redundant blank lines, please. > > > + if (ACPI_FAILURE(status)) { > > + err = -EIO; > > + goto err_free_dev; > > + } > > +} > > > > + wmi_device = kmalloc(sizeof(struct huawei_wmi_device), > > GFP_KERNEL); > > sizeof(*wmi_device) > > > + if (!wmi_device) > > + return -ENOMEM; > > +} > > > > + pr_debug("%s: Driver unloaded successfully\n", > > MODULE_NAME); > > Noise. > > -- > With Best Regards, > Andy Shevchenko
On Sat, 03 Nov 2018 00:10:18 +0100, <ayman.bagabas@gmail.com> wrote: > > On Fri, 2018-11-02 at 20:20 +0200, Andy Shevchenko wrote: > > > +#include <linux/input.h> > > > +#include <linux/input/sparse-keymap.h> > > > +#include <linux/acpi.h> > > > +#include <linux/wmi.h> > > > > Please, keep in order. > > What order do I have to follow? Does this look right? > module.h > init.h > apci.h > wmi.h > input.h > sparse-keymap.h A most common pattern is the alphabetical order. thanks, Takashi
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 0c1aa6c314f5..c6813981e45c 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -1229,6 +1229,19 @@ config I2C_MULTI_INSTANTIATE To compile this driver as a module, choose M here: the module will be called i2c-multi-instantiate. +config HUAWEI_LAPTOP + tristate "Huawei WMI hotkeys driver" + depends on ACPI + depends on ACPI_WMI + depends on INPUT + select INPUT_SPARSEKMAP + help + This driver provides support for Huawei WMI hotkeys. + It enables the missing keys and adds support to micmute + led found on these laptops.q + Supported devices are: + - Matebook X Pro + endif # X86_PLATFORM_DEVICES config PMC_ATOM diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile index e6d1becf81ce..5984354e18ff 100644 --- a/drivers/platform/x86/Makefile +++ b/drivers/platform/x86/Makefile @@ -29,6 +29,7 @@ obj-$(CONFIG_ACERHDF) += acerhdf.o obj-$(CONFIG_HP_ACCEL) += hp_accel.o obj-$(CONFIG_HP_WIRELESS) += hp-wireless.o obj-$(CONFIG_HP_WMI) += hp-wmi.o +obj-$(CONFIG_HUAWEI_LAPTOP) += huawei_wmi.o obj-$(CONFIG_AMILO_RFKILL) += amilo-rfkill.o obj-$(CONFIG_GPD_POCKET_FAN) += gpd-pocket-fan.o obj-$(CONFIG_TC1100_WMI) += tc1100-wmi.o diff --git a/drivers/platform/x86/huawei_wmi.c b/drivers/platform/x86/huawei_wmi.c new file mode 100644 index 000000000000..83545217ac19 --- /dev/null +++ b/drivers/platform/x86/huawei_wmi.c @@ -0,0 +1,223 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Huawei WMI Hotkeys Driver + * + * Copyright (C) 2018 Ayman Bagabas <ayman.bagabas@gmail.com> + * + * 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, see <https://www.gnu.org/licenses/>. + * + */ + +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/init.h> +#include <linux/input.h> +#include <linux/input/sparse-keymap.h> +#include <linux/acpi.h> +#include <linux/wmi.h> + +MODULE_AUTHOR("Ayman Bagabas <ayman.bagabas@gmail.com>"); +MODULE_DESCRIPTION("Huawei WMI hotkeys"); +MODULE_LICENSE("GPL"); + +#define DEVICE_NAME "huawei" +#define MODULE_NAME DEVICE_NAME"_wmi" + +/* + * Huawei WMI Devices GUIDs + */ +#define AMW0_GUID "ABBC0F5B-8EA1-11D1-A000-C90629100000" // \_SB.AMW0 + +/* + * Huawei WMI Events GUIDs + */ +#define EVENT_GUID "ABBC0F5C-8EA1-11D1-A000-C90629100000" + +MODULE_ALIAS("wmi:"AMW0_GUID); +MODULE_ALIAS("wmi:"EVENT_GUID); + +enum { + MICMUTE_LED_ON = 0x00010B04, + MICMUTE_LED_OFF = 0x00000B04, +}; + +static const struct key_entry huawei_wmi_keymap[] __initconst = { + { KE_IGNORE, 0x281, { KEY_BRIGHTNESSDOWN } }, + { KE_IGNORE, 0x282, { KEY_BRIGHTNESSUP } }, + { KE_KEY, 0x287, { KEY_MICMUTE } }, + { KE_KEY, 0x289, { KEY_WLAN } }, + // Huawei |M| button + { KE_KEY, 0x28a, { KEY_PROG1 } }, + { KE_END, 0 } +}; + +struct huawei_wmi_device { + struct input_dev *inputdev; +}; +static struct huawei_wmi_device *wmi_device; + +int huawei_wmi_micmute_led_set(bool on) +{ + u32 args = (on) ? MICMUTE_LED_ON : MICMUTE_LED_OFF; + struct acpi_buffer input = { (acpi_size)sizeof(args), &args }; + acpi_status status; + + status = wmi_evaluate_method(AMW0_GUID, 0, 1, &input, NULL); + if (ACPI_FAILURE(status)) + return status; + + return 0; +} +EXPORT_SYMBOL_GPL(huawei_wmi_micmute_led_set); + +static void huawei_wmi_process_key(struct input_dev *input_dev, int code) +{ + const struct key_entry *key; + + key = sparse_keymap_entry_from_scancode(input_dev, code); + + if (!key) { + pr_info("%s: Unknown key pressed, code: 0x%04x\n", + MODULE_NAME, code); + return; + } + + sparse_keymap_report_entry(input_dev, key, 1, true); +} + +static void huawei_wmi_notify(u32 value, void *context) +{ + struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL }; + union acpi_object *obj; + acpi_status status; + + status = wmi_get_event_data(value, &response); + if (ACPI_FAILURE(status)) { + pr_err("%s: Bad event status 0x%x\n", + MODULE_NAME, status); + return; + } + + obj = (union acpi_object *)response.pointer; + + if (!obj) + return; + + if (obj->type == ACPI_TYPE_INTEGER) + huawei_wmi_process_key(wmi_device->inputdev, + obj->integer.value); + else + pr_info("%s: Unknown response received %d\n", + MODULE_NAME, obj->type); + + kfree(response.pointer); +} + +static int huawei_input_init(void) +{ + acpi_status status; + int err; + + wmi_device->inputdev = input_allocate_device(); + if (!wmi_device->inputdev) + return -ENOMEM; + + wmi_device->inputdev->name = "Huawei WMI hotkeys"; + wmi_device->inputdev->phys = "wmi/input0"; + wmi_device->inputdev->id.bustype = BUS_HOST; + + err = sparse_keymap_setup(wmi_device->inputdev, + huawei_wmi_keymap, NULL); + if (err) + goto err_free_dev; + + status = wmi_install_notify_handler(EVENT_GUID, + huawei_wmi_notify, + NULL); + + if (ACPI_FAILURE(status)) { + err = -EIO; + goto err_free_dev; + } + + err = input_register_device(wmi_device->inputdev); + if (err) + goto err_remove_notifier; + + return 0; + + +err_remove_notifier: + wmi_remove_notify_handler(EVENT_GUID); +err_free_dev: + input_free_device(wmi_device->inputdev); + return err; +} + +static void huawei_input_exit(void) +{ + wmi_remove_notify_handler(EVENT_GUID); + input_unregister_device(wmi_device->inputdev); +} + +static int __init huawei_wmi_setup(void) +{ + int err; + + wmi_device = kmalloc(sizeof(struct huawei_wmi_device), GFP_KERNEL); + if (!wmi_device) + return -ENOMEM; + + err = huawei_input_init(); + if (err) + goto err_input; + + return 0; + +err_input: + return err; +} + +static void huawei_wmi_destroy(void) +{ + huawei_input_exit(); + kfree(wmi_device); +} + +static int __init huawei_wmi_init(void) +{ + int err; + + if (!wmi_has_guid(EVENT_GUID)) { + pr_warn("%s: No known WMI GUID found\n", MODULE_NAME); + return -ENODEV; + } + + err = huawei_wmi_setup(); + if (err) { + pr_err("%s: Failed to setup device\n", MODULE_NAME); + return err; + } + + return 0; +} + +static void __exit huawei_wmi_exit(void) +{ + huawei_wmi_destroy(); + pr_debug("%s: Driver unloaded successfully\n", MODULE_NAME); +} + +module_init(huawei_wmi_init); +module_exit(huawei_wmi_exit);
This driver adds support for missing hotkeys on some Huawei laptops. Currently, only Huawei Matebook X Pro is supported. The driver recognizes the following keys: brightness keys, micmute, wlan, and Huawei special key. The brightness keys are ignored since they work out of the box. Signed-off-by: Ayman Bagabas <ayman.bagabas@gmail.com> --- drivers/platform/x86/Kconfig | 13 ++ drivers/platform/x86/Makefile | 1 + drivers/platform/x86/huawei_wmi.c | 223 ++++++++++++++++++++++++++++++ 3 files changed, 237 insertions(+) create mode 100644 drivers/platform/x86/huawei_wmi.c