[PATCH v7 0/3] Huawei laptops

[PATCH v7 0/3] Huawei laptops

[Ayman Bagabas]

This patch set is based on new audio LED triggers in branch topic/leds-trigger from
	git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git

Changes from v6:
* Use audio LED triggers patch set
* Use KEY_CONFIG (XF86Tools) instead of KEY_PROG1.
 In Windows, the key is used to launch Huawei PC manager. XF86Tools is
 used by default on most desktop environments i.e. Gnome.

Ayman Bagabas (3):
  ALSA: hda: fix front speakers on Huawei MBXP.
  x86: add support for Huawei WMI hotkeys.
  ALSA: hda: add support for Huawei WMI micmute LED

 drivers/platform/x86/Kconfig      |  15 ++
 drivers/platform/x86/Makefile     |   1 +
 drivers/platform/x86/huawei-wmi.c | 247 ++++++++++++++++++++++++++++++
 sound/pci/hda/patch_realtek.c     |  22 +++
 4 files changed, 285 insertions(+)
 create mode 100644 drivers/platform/x86/huawei-wmi.c

-- 
2.19.1

[PATCH v7 1/3] ALSA: hda: fix front speakers on Huawei MBXP.

[Ayman Bagabas]

This patch solves bug 200501 'Only 2 of 4 speakers playing sound.'
https://bugzilla.kernel.org/show_bug.cgi?id=200501
It enables the front speakers on Huawei Matebook X Pro laptops.
These laptops come with Dolby Atmos sound system and these pins
configuration enables the front speakers.

Signed-off-by: Ayman Bagabas <ayman.bagabas@gmail.com>
Reviewed-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/hda/patch_realtek.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 993d34c141c2..1326f32f4574 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -5490,6 +5490,7 @@ enum {
 	ALC298_FIXUP_TPT470_DOCK,
 	ALC255_FIXUP_DUMMY_LINEOUT_VERB,
 	ALC255_FIXUP_DELL_HEADSET_MIC,
+	ALC256_FIXUP_HUAWEI_MBXP_PINS,
 	ALC295_FIXUP_HP_X360,
 	ALC221_FIXUP_HP_HEADSET_MIC,
 };
@@ -5761,6 +5762,22 @@ static const struct hda_fixup alc269_fixups[] = {
 		.chained = true,
 		.chain_id = ALC269_FIXUP_HEADSET_MIC
 	},
+	[ALC256_FIXUP_HUAWEI_MBXP_PINS] = {
+		.type = HDA_FIXUP_PINS,
+		.v.pins = (const struct hda_pintbl[]) {
+			{0x12, 0x90a60130},
+			{0x13, 0x40000000},
+			{0x14, 0x90170110},
+			{0x18, 0x411111f0},
+			{0x19, 0x04a11040},
+			{0x1a, 0x411111f0},
+			{0x1b, 0x90170112},
+			{0x1d, 0x40759a05},
+			{0x1e, 0x411111f0},
+			{0x21, 0x04211020},
+			{ }
+		}
+	},
 	[ALC269_FIXUP_ASUS_X101_FUNC] = {
 		.type = HDA_FIXUP_FUNC,
 		.v.func = alc269_fixup_x101_headset_mic,
@@ -6591,6 +6608,7 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
 	SND_PCI_QUIRK(0x17aa, 0x511f, "Thinkpad", ALC298_FIXUP_TPT470_DOCK),
 	SND_PCI_QUIRK(0x17aa, 0x3bf8, "Quanta FL1", ALC269_FIXUP_PCM_44K),
 	SND_PCI_QUIRK(0x17aa, 0x9e54, "LENOVO NB", ALC269_FIXUP_LENOVO_EAPD),
+	SND_PCI_QUIRK(0x19e5, 0x3204, "Huawei MBXP", ALC256_FIXUP_HUAWEI_MBXP_PINS),
 	SND_PCI_QUIRK(0x1b7d, 0xa831, "Ordissimo EVE2 ", ALC269VB_FIXUP_ORDISSIMO_EVE2), /* Also known as Malata PC-B1303 */
 
 #if 0
-- 
2.19.1

[PATCH v7 2/3] x86: add support for Huawei WMI hotkeys.

[Ayman Bagabas]

This driver adds support for missing hotkeys on some Huawei laptops.
Currently, only Huawei Matebook X and Matebook X Pro is supported.

Signed-off-by: Ayman Bagabas <ayman.bagabas@gmail.com>
---
 drivers/platform/x86/Kconfig      |  15 ++
 drivers/platform/x86/Makefile     |   1 +
 drivers/platform/x86/huawei-wmi.c | 247 ++++++++++++++++++++++++++++++
 3 files changed, 263 insertions(+)
 create mode 100644 drivers/platform/x86/huawei-wmi.c

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 87f70e8f4dd0..0f9db4275fb4 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -1292,6 +1292,21 @@ config INTEL_ATOMISP2_PM
 	  To compile this driver as a module, choose M here: the module
 	  will be called intel_atomisp2_pm.
 
+config HUAWEI_WMI
+	tristate "Huawei WMI hotkeys driver"
+	depends on ACPI_WMI
+	depends on INPUT
+	select INPUT_SPARSEKMAP
+	select LEDS_TRIGGERS
+	select LEDS_TRIGGER_AUDIO
+	help
+	  This driver provides support for Huawei WMI hotkeys.
+	  It enables the missing keys and adds support to the micmute
+	  LED found on some of these laptops.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called huawei-wmi.
+
 endif # X86_PLATFORM_DEVICES
 
 config PMC_ATOM
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 39ae94135406..d841c550e3cc 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -32,6 +32,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_WMI)		+= 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..b6157c5b7f4a
--- /dev/null
+++ b/drivers/platform/x86/huawei-wmi.c
@@ -0,0 +1,247 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ *  Huawei WMI hotkeys
+ *
+ *  Copyright (C) 2018	      Ayman Bagabas <ayman.bagabas@gmail.com>
+ */
+
+#include <linux/acpi.h>
+#include <linux/input.h>
+#include <linux/input/sparse-keymap.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+
+MODULE_AUTHOR("Ayman Bagabas <ayman.bagabas@gmail.com>");
+MODULE_DESCRIPTION("Huawei WMI hotkeys");
+MODULE_LICENSE("GPL v2");
+
+/*
+ * Huawei WMI Events GUIDs
+ */
+#define MBX_EVENT_GUID "59142400-C6A3-40fa-BADB-8A2652834100"
+#define MBXP_EVENT_GUID "ABBC0F5C-8EA1-11D1-A000-C90629100000"
+
+MODULE_ALIAS("wmi:"MBX_EVENT_GUID);
+MODULE_ALIAS("wmi:"MBXP_EVENT_GUID);
+
+static const struct key_entry huawei_wmi_keymap[] __initconst = {
+		{ KE_KEY,    0x281, { KEY_BRIGHTNESSDOWN } },
+		{ KE_KEY,    0x282, { KEY_BRIGHTNESSUP } },
+		{ KE_KEY,    0x284, { KEY_MUTE } },
+		{ KE_KEY,    0x285, { KEY_VOLUMEDOWN } },
+		{ KE_KEY,    0x286, { KEY_VOLUMEUP } },
+		{ KE_KEY,    0x287, { KEY_MICMUTE } },
+		{ KE_KEY,    0x289, { KEY_WLAN } },
+		// Huawei |M| button
+		{ KE_KEY,    0x28a, { KEY_CONFIG } },
+		// Keyboard light
+		{ KE_IGNORE, 0x293, { KEY_KBDILLUMTOGGLE } },
+		{ KE_IGNORE, 0x294, { KEY_KBDILLUMUP } },
+		{ KE_IGNORE, 0x295, { KEY_KBDILLUMUP } },
+		{ KE_END,    0 }
+};
+
+static char *event_guid;
+static struct input_dev *inputdev;
+
+static int huawei_wmi_micmute_led_set(bool on)
+{
+	acpi_handle handle;
+	char *method;
+	union acpi_object args[3];
+	struct acpi_object_list arg_list = {
+		.pointer = args,
+		.count = ARRAY_SIZE(args),
+	};
+
+	handle = ACPI_HANDLE(&inputdev->dev);
+	args[0].type = args[1].type = args[2].type = ACPI_TYPE_INTEGER;
+	args[1].integer.value = 0x04;
+
+	if (acpi_has_method(handle, method = "\\_SB.PCI0.LPCB.EC0.SPIN")) {
+		args[0].integer.value = 0;
+		args[2].integer.value = on ? 1 : 0;
+	} else if (acpi_has_method(handle, method = "\\_SB.PCI0.LPCB.EC0.WPIN")) {
+		args[0].integer.value = 1;
+		args[2].integer.value = on ? 0 : 1;
+	} else {
+		dev_err(&inputdev->dev, "Unable to find ACPI method\n");
+		return -ENOSYS;
+	}
+
+	acpi_evaluate_object(handle, method, &arg_list, NULL);
+
+	return 0;
+}
+
+static int micmute_led_set(struct led_classdev *led_cdev,
+		enum led_brightness brightness)
+{
+	int state = brightness != LED_OFF;
+	int err;
+
+	err = huawei_wmi_micmute_led_set(state);
+	return err < 0 ? err : 0;
+}
+
+static struct led_classdev micmute_led_cdev = {
+	.name = "huawei::micmute",
+	.max_brightness = 1,
+	.brightness_set_blocking = micmute_led_set,
+	.default_trigger = "audio-micmute",
+};
+
+static void huawei_wmi_process_key(struct input_dev *inputdev, int code)
+{
+	const struct key_entry *key;
+
+	/*
+	 * MBX uses code 0x80 to indicate a hotkey event.
+	 * The actual key is fetched from the method WQ00.
+	 */
+	if (code == 0x80) {
+		acpi_status status;
+		unsigned long long result;
+		const char *method = "\\WMI0.WQ00";
+		union acpi_object args[1];
+		struct acpi_object_list arg_list = {
+			.pointer = args,
+			.count = ARRAY_SIZE(args),
+		};
+
+		args[0].type = ACPI_TYPE_INTEGER;
+		args[0].integer.value = 0;
+
+		status = acpi_evaluate_integer(ACPI_HANDLE(&inputdev->dev), (char *)method, &arg_list, &result);
+		if (ACPI_FAILURE(status)) {
+			dev_err(&inputdev->dev, "Unable to evaluate ACPI method %s\n", method);
+			return;
+		}
+
+		code = result;
+	}
+
+	key = sparse_keymap_entry_from_scancode(inputdev, code);
+	if (!key) {
+		dev_info(&inputdev->dev, "Unknown key pressed, code: 0x%04x\n", code);
+		return;
+	}
+
+	/*
+	 * The MBXP handles backlight natively using ACPI,
+	 * but not the MBX. If MBXP is being used, skip reporting event.
+	 */
+	if ((key->sw.code == KEY_BRIGHTNESSUP || key->sw.code == KEY_BRIGHTNESSDOWN)
+			&& strcmp(event_guid, MBXP_EVENT_GUID) == 0)
+		return;
+
+	sparse_keymap_report_entry(inputdev, key, 1, true);
+}
+
+static void huawei_wmi_notify(u32 value, void *context)
+{
+	struct input_dev *inputdev = 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)) {
+		dev_err(&inputdev->dev, "Bad event status 0x%x\n", status);
+		return;
+	}
+
+	obj = (union acpi_object *)response.pointer;
+	if (!obj)
+		return;
+
+	if (obj->type == ACPI_TYPE_INTEGER)
+		huawei_wmi_process_key(inputdev, obj->integer.value);
+	else
+		dev_info(&inputdev->dev, "Unknown response received %d\n", obj->type);
+
+	kfree(response.pointer);
+}
+
+static int huawei_wmi_input_init(void)
+{
+	acpi_status status;
+	int err;
+
+	inputdev = input_allocate_device();
+	if (!inputdev)
+		return -ENOMEM;
+
+	inputdev->name = "Huawei WMI hotkeys";
+	inputdev->phys = "wmi/input0";
+	inputdev->id.bustype = BUS_HOST;
+
+	err = sparse_keymap_setup(inputdev, huawei_wmi_keymap, NULL);
+	if (err)
+		goto err_free_dev;
+
+	status = wmi_install_notify_handler(event_guid,
+			huawei_wmi_notify,
+			inputdev);
+	if (ACPI_FAILURE(status)) {
+		err = -EIO;
+		goto err_free_dev;
+	}
+
+	err = input_register_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(inputdev);
+	return err;
+}
+
+static void huawei_wmi_input_exit(void)
+{
+	wmi_remove_notify_handler(event_guid);
+	input_unregister_device(inputdev);
+}
+
+static int __init huawei_wmi_init(void)
+{
+	int err;
+
+	if (wmi_has_guid(MBX_EVENT_GUID)) {
+		event_guid = MBX_EVENT_GUID;
+	} else if (wmi_has_guid(MBXP_EVENT_GUID)) {
+		event_guid = MBXP_EVENT_GUID;
+	} else {
+		pr_warn("Compatible WMI GUID not found\n");
+		return -ENODEV;
+	}
+
+	err = huawei_wmi_input_init();
+	if (err)
+		goto err_input;
+
+	micmute_led_cdev.brightness = ledtrig_audio_get(LED_AUDIO_MICMUTE);
+	err = led_classdev_register(&inputdev->dev, &micmute_led_cdev);
+	if (err)
+		goto err_leds;
+
+	return 0;
+
+err_leds:
+	huawei_wmi_input_exit();
+err_input:
+	return err;
+}
+
+static void __exit huawei_wmi_exit(void)
+{
+	led_classdev_unregister(&micmute_led_cdev);
+	huawei_wmi_input_exit();
+}
+
+module_init(huawei_wmi_init);
+module_exit(huawei_wmi_exit);
-- 
2.19.1

[PATCH v7 3/3] ALSA: hda: add support for Huawei WMI micmute LED

[Ayman Bagabas]

Some of Huawei laptops come with a LED in the micmute key. This patch
enables the use of micmute LED for these devices:
1. Matebook X (19e5:3200), (19e5:3201)
2. Matebook X Pro (19e5:3204)

Signed-off-by: Ayman Bagabas <ayman.bagabas@gmail.com>
---
 sound/pci/hda/patch_realtek.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 1326f32f4574..9766fd249bdf 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -5776,7 +5776,9 @@ static const struct hda_fixup alc269_fixups[] = {
 			{0x1e, 0x411111f0},
 			{0x21, 0x04211020},
 			{ }
-		}
+		},
+		.chained = true,
+		.chain_id = ALC255_FIXUP_MIC_MUTE_LED
 	},
 	[ALC269_FIXUP_ASUS_X101_FUNC] = {
 		.type = HDA_FIXUP_FUNC,
@@ -6608,6 +6610,8 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
 	SND_PCI_QUIRK(0x17aa, 0x511f, "Thinkpad", ALC298_FIXUP_TPT470_DOCK),
 	SND_PCI_QUIRK(0x17aa, 0x3bf8, "Quanta FL1", ALC269_FIXUP_PCM_44K),
 	SND_PCI_QUIRK(0x17aa, 0x9e54, "LENOVO NB", ALC269_FIXUP_LENOVO_EAPD),
+	SND_PCI_QUIRK(0x19e5, 0x3200, "Huawei MBX", ALC255_FIXUP_MIC_MUTE_LED),
+	SND_PCI_QUIRK(0x19e5, 0x3201, "Huawei MBX", ALC255_FIXUP_MIC_MUTE_LED),
 	SND_PCI_QUIRK(0x19e5, 0x3204, "Huawei MBXP", ALC256_FIXUP_HUAWEI_MBXP_PINS),
 	SND_PCI_QUIRK(0x1b7d, 0xa831, "Ordissimo EVE2 ", ALC269VB_FIXUP_ORDISSIMO_EVE2), /* Also known as Malata PC-B1303 */
 
-- 
2.19.1

Re: [PATCH v7 2/3] x86: add support for Huawei WMI hotkeys.

[Takashi Iwai]

On Tue, 27 Nov 2018 03:57:48 +0100,
Ayman Bagabas wrote:
> 
> +static const struct key_entry huawei_wmi_keymap[] __initconst = {
> +		{ KE_KEY,    0x281, { KEY_BRIGHTNESSDOWN } },
> +		{ KE_KEY,    0x282, { KEY_BRIGHTNESSUP } },
> +		{ KE_KEY,    0x284, { KEY_MUTE } },
> +		{ KE_KEY,    0x285, { KEY_VOLUMEDOWN } },
> +		{ KE_KEY,    0x286, { KEY_VOLUMEUP } },
> +		{ KE_KEY,    0x287, { KEY_MICMUTE } },
> +		{ KE_KEY,    0x289, { KEY_WLAN } },
> +		// Huawei |M| button
> +		{ KE_KEY,    0x28a, { KEY_CONFIG } },
> +		// Keyboard light
> +		{ KE_IGNORE, 0x293, { KEY_KBDILLUMTOGGLE } },
> +		{ KE_IGNORE, 0x294, { KEY_KBDILLUMUP } },
> +		{ KE_IGNORE, 0x295, { KEY_KBDILLUMUP } },
> +		{ KE_END,    0 }

The indentation looks too deep here.

> +static int huawei_wmi_micmute_led_set(bool on)
> +{
> +	acpi_handle handle;
> +	char *method;
> +	union acpi_object args[3];
> +	struct acpi_object_list arg_list = {
> +		.pointer = args,
> +		.count = ARRAY_SIZE(args),
> +	};
> +
> +	handle = ACPI_HANDLE(&inputdev->dev);

Just wondering whether the ACPI handle is assigned properly for this
device...

> +	args[0].type = args[1].type = args[2].type = ACPI_TYPE_INTEGER;
> +	args[1].integer.value = 0x04;
> +
> +	if (acpi_has_method(handle, method = "\\_SB.PCI0.LPCB.EC0.SPIN")) {
> +		args[0].integer.value = 0;
> +		args[2].integer.value = on ? 1 : 0;
> +	} else if (acpi_has_method(handle, method = "\\_SB.PCI0.LPCB.EC0.WPIN")) {
> +		args[0].integer.value = 1;
> +		args[2].integer.value = on ? 0 : 1;
> +	} else {
> +		dev_err(&inputdev->dev, "Unable to find ACPI method\n");
> +		return -ENOSYS;
> +	}
> +
> +	acpi_evaluate_object(handle, method, &arg_list, NULL);
> +
> +	return 0;
> +}
> +
> +static int micmute_led_set(struct led_classdev *led_cdev,
> +		enum led_brightness brightness)
> +{
> +	int state = brightness != LED_OFF;
> +	int err;
> +
> +	err = huawei_wmi_micmute_led_set(state);
> +	return err < 0 ? err : 0;

No need for checking err here, you can return
huawei_wmi_mute_led_set() directly.

Or even you can fold two functions into one.

> +static int __init huawei_wmi_init(void)
> +{
> +	int err;
> +
> +	if (wmi_has_guid(MBX_EVENT_GUID)) {
> +		event_guid = MBX_EVENT_GUID;
> +	} else if (wmi_has_guid(MBXP_EVENT_GUID)) {
> +		event_guid = MBXP_EVENT_GUID;
> +	} else {
> +		pr_warn("Compatible WMI GUID not found\n");
> +		return -ENODEV;
> +	}
> +
> +	err = huawei_wmi_input_init();
> +	if (err)
> +		goto err_input;
> +
> +	micmute_led_cdev.brightness = ledtrig_audio_get(LED_AUDIO_MICMUTE);
> +	err = led_classdev_register(&inputdev->dev, &micmute_led_cdev);
> +	if (err)
> +		goto err_leds;
> +
> +	return 0;

Might it be cleaner to implement on top of wmi_driver?
Then you can create both input and led devices on wmi device.


thanks,

Takashi

Re: [PATCH v7 2/3] x86: add support for Huawei WMI hotkeys.

[Takashi Iwai]

Some more forgotten bits...

On Tue, 27 Nov 2018 03:57:48 +0100,
Ayman Bagabas wrote:
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
....
> +config HUAWEI_WMI
> +	tristate "Huawei WMI hotkeys driver"
> +	depends on ACPI_WMI
> +	depends on INPUT
> +	select INPUT_SPARSEKMAP
> +	select LEDS_TRIGGERS
> +	select LEDS_TRIGGER_AUDIO

I guess "select LEDS_CLASS" is needed as well.
Other drivers had already it.

> --- /dev/null
> +++ b/drivers/platform/x86/huawei-wmi.c
....
> +static int huawei_wmi_micmute_led_set(bool on)
> +{
> +	acpi_handle handle;
> +	char *method;
> +	union acpi_object args[3];
> +	struct acpi_object_list arg_list = {
> +		.pointer = args,
> +		.count = ARRAY_SIZE(args),
> +	};
> +
> +	handle = ACPI_HANDLE(&inputdev->dev);
> +	args[0].type = args[1].type = args[2].type = ACPI_TYPE_INTEGER;
> +	args[1].integer.value = 0x04;
> +
> +	if (acpi_has_method(handle, method = "\\_SB.PCI0.LPCB.EC0.SPIN")) {

This looks ugly...  And checkpatch also complains.

> +		args[0].integer.value = 0;
> +		args[2].integer.value = on ? 1 : 0;
> +	} else if (acpi_has_method(handle, method = "\\_SB.PCI0.LPCB.EC0.WPIN")) {
> +		args[0].integer.value = 1;
> +		args[2].integer.value = on ? 0 : 1;
> +	} else {
> +		dev_err(&inputdev->dev, "Unable to find ACPI method\n");
> +		return -ENOSYS;
> +	}

Can these checks be done at initialization phase?  It doesn't seem
needed to be executed at each call.


thanks,

Takashi

Re: [PATCH v7 2/3] x86: add support for Huawei WMI hotkeys.

[Andy Shevchenko]

On Tue, Nov 27, 2018 at 1:02 PM Takashi Iwai <tiwai@suse.de> wrote:
> On Tue, 27 Nov 2018 03:57:48 +0100,
> Ayman Bagabas wrote:

> > +     handle = ACPI_HANDLE(&inputdev->dev);
> > +     args[0].type = args[1].type = args[2].type = ACPI_TYPE_INTEGER;
> > +     args[1].integer.value = 0x04;
> > +
> > +     if (acpi_has_method(handle, method = "\\_SB.PCI0.LPCB.EC0.SPIN")) {
>
> This looks ugly...  And checkpatch also complains.

Good point.
Actually another question is what handle do we get here? Can we check
method name by relative path, i.e. "SPIN" / "WPIN" instead?

> > +             args[0].integer.value = 0;
> > +             args[2].integer.value = on ? 1 : 0;
> > +     } else if (acpi_has_method(handle, method = "\\_SB.PCI0.LPCB.EC0.WPIN")) {
> > +             args[0].integer.value = 1;
> > +             args[2].integer.value = on ? 0 : 1;
> > +     } else {
> > +             dev_err(&inputdev->dev, "Unable to find ACPI method\n");
> > +             return -ENOSYS;
> > +     }
>
> Can these checks be done at initialization phase?  It doesn't seem
> needed to be executed at each call.

Agree.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v7 2/3] x86: add support for Huawei WMI hotkeys.

[ayman.bagabas]

On Tue, 2018-11-27 at 17:52 +0200, Andy Shevchenko wrote:
> On Tue, Nov 27, 2018 at 1:02 PM Takashi Iwai <tiwai@suse.de> wrote:
> > On Tue, 27 Nov 2018 03:57:48 +0100,
> > Ayman Bagabas wrote:
> > > +     handle = ACPI_HANDLE(&inputdev->dev);
> > > +     args[0].type = args[1].type = args[2].type =
> > > ACPI_TYPE_INTEGER;
> > > +     args[1].integer.value = 0x04;
> > > +
> > > +     if (acpi_has_method(handle, method =
> > > "\\_SB.PCI0.LPCB.EC0.SPIN")) {
> > 
> > This looks ugly...  And checkpatch also complains.
> 
> Good point.
> Actually another question is what handle do we get here? Can we check
> method name by relative path, i.e. "SPIN" / "WPIN" instead?

The handle is a NULL handle at "\". Should I use acpi_get_handle(NULL,
"\\_SB.PCI0.LPCB.EC0", &handle)?  EC0 is a acpi device HID:PNP0C09.

> 
> > > +             args[0].integer.value = 0;
> > > +             args[2].integer.value = on ? 1 : 0;
> > > +     } else if (acpi_has_method(handle, method =
> > > "\\_SB.PCI0.LPCB.EC0.WPIN")) {
> > > +             args[0].integer.value = 1;
> > > +             args[2].integer.value = on ? 0 : 1;
> > > +     } else {
> > > +             dev_err(&inputdev->dev, "Unable to find ACPI
> > > method\n");
> > > +             return -ENOSYS;
> > > +     }
> > 
> > Can these checks be done at initialization phase?  It doesn't seem
> > needed to be executed at each call.
> 
> Agree.
> 

Even if it was done in init phase, we still need to check what acpi
method should be used and based on that and micmute status we set the
arguments args[0] and args[2], args[1] is the pin value. We could check
these using event_guid, but that would assume the methods exist for
each guid.
For example, if event_guid == MBX_EVENT_GUID, then method SPIN should
be used.

Re: [PATCH v7 2/3] x86: add support for Huawei WMI hotkeys.

[ayman.bagabas]

On Tue, 2018-11-27 at 10:45 +0100, Takashi Iwai wrote:
> On Tue, 27 Nov 2018 03:57:48 +0100,
> Ayman Bagabas wrote:
> > +static const struct key_entry huawei_wmi_keymap[] __initconst = {
> > +		{ KE_KEY,    0x281, { KEY_BRIGHTNESSDOWN } },
> > +		{ KE_KEY,    0x282, { KEY_BRIGHTNESSUP } },
> > +		{ KE_KEY,    0x284, { KEY_MUTE } },
> > +		{ KE_KEY,    0x285, { KEY_VOLUMEDOWN } },
> > +		{ KE_KEY,    0x286, { KEY_VOLUMEUP } },
> > +		{ KE_KEY,    0x287, { KEY_MICMUTE } },
> > +		{ KE_KEY,    0x289, { KEY_WLAN } },
> > +		// Huawei |M| button
> > +		{ KE_KEY,    0x28a, { KEY_CONFIG } },
> > +		// Keyboard light
> > +		{ KE_IGNORE, 0x293, { KEY_KBDILLUMTOGGLE } },
> > +		{ KE_IGNORE, 0x294, { KEY_KBDILLUMUP } },
> > +		{ KE_IGNORE, 0x295, { KEY_KBDILLUMUP } },
> > +		{ KE_END,    0 }
> 
> The indentation looks too deep here.
> 
> > +static int huawei_wmi_micmute_led_set(bool on)
> > +{
> > +	acpi_handle handle;
> > +	char *method;
> > +	union acpi_object args[3];
> > +	struct acpi_object_list arg_list = {
> > +		.pointer = args,
> > +		.count = ARRAY_SIZE(args),
> > +	};
> > +
> > +	handle = ACPI_HANDLE(&inputdev->dev);
> 
> Just wondering whether the ACPI handle is assigned properly for this
> device...

It is, but it returns a handle at "\".

> 
> > +	args[0].type = args[1].type = args[2].type = ACPI_TYPE_INTEGER;
> > +	args[1].integer.value = 0x04;
> > +
> > +	if (acpi_has_method(handle, method =
> > "\\_SB.PCI0.LPCB.EC0.SPIN")) {
> > +		args[0].integer.value = 0;
> > +		args[2].integer.value = on ? 1 : 0;
> > +	} else if (acpi_has_method(handle, method =
> > "\\_SB.PCI0.LPCB.EC0.WPIN")) {
> > +		args[0].integer.value = 1;
> > +		args[2].integer.value = on ? 0 : 1;
> > +	} else {
> > +		dev_err(&inputdev->dev, "Unable to find ACPI
> > method\n");
> > +		return -ENOSYS;
> > +	}
> > +
> > +	acpi_evaluate_object(handle, method, &arg_list, NULL);
> > +
> > +	return 0;
> > +}
> > +
> > +static int micmute_led_set(struct led_classdev *led_cdev,
> > +		enum led_brightness brightness)
> > +{
> > +	int state = brightness != LED_OFF;
> > +	int err;
> > +
> > +	err = huawei_wmi_micmute_led_set(state);
> > +	return err < 0 ? err : 0;
> 
> No need for checking err here, you can return
> huawei_wmi_mute_led_set() directly.
> 
> Or even you can fold two functions into one.

I think folding them make more sense.

> 
> > +static int __init huawei_wmi_init(void)
> > +{
> > +	int err;
> > +
> > +	if (wmi_has_guid(MBX_EVENT_GUID)) {
> > +		event_guid = MBX_EVENT_GUID;
> > +	} else if (wmi_has_guid(MBXP_EVENT_GUID)) {
> > +		event_guid = MBXP_EVENT_GUID;
> > +	} else {
> > +		pr_warn("Compatible WMI GUID not found\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	err = huawei_wmi_input_init();
> > +	if (err)
> > +		goto err_input;
> > +
> > +	micmute_led_cdev.brightness =
> > ledtrig_audio_get(LED_AUDIO_MICMUTE);
> > +	err = led_classdev_register(&inputdev->dev, &micmute_led_cdev);
> > +	if (err)
> > +		goto err_leds;
> > +
> > +	return 0;
> 
> Might it be cleaner to implement on top of wmi_driver?
> Then you can create both input and led devices on wmi device.
> 

Done! Now led devices get registered when wmi probes a new device.
Thanks.

> 
> thanks,
> 
> Takashi

Re: [PATCH v7 2/3] x86: add support for Huawei WMI hotkeys.

[Takashi Iwai]

On Wed, 28 Nov 2018 01:49:48 +0100,
ayman.bagabas@gmail.com wrote:
> 
> On Tue, 2018-11-27 at 17:52 +0200, Andy Shevchenko wrote:
> > On Tue, Nov 27, 2018 at 1:02 PM Takashi Iwai <tiwai@suse.de> wrote:
> > > On Tue, 27 Nov 2018 03:57:48 +0100,
> > > Ayman Bagabas wrote:
> > > > +     handle = ACPI_HANDLE(&inputdev->dev);
> > > > +     args[0].type = args[1].type = args[2].type =
> > > > ACPI_TYPE_INTEGER;
> > > > +     args[1].integer.value = 0x04;
> > > > +
> > > > +     if (acpi_has_method(handle, method =
> > > > "\\_SB.PCI0.LPCB.EC0.SPIN")) {
> > > 
> > > This looks ugly...  And checkpatch also complains.
> > 
> > Good point.
> > Actually another question is what handle do we get here? Can we check
> > method name by relative path, i.e. "SPIN" / "WPIN" instead?
> 
> The handle is a NULL handle at "\". Should I use acpi_get_handle(NULL,
> "\\_SB.PCI0.LPCB.EC0", &handle)?  EC0 is a acpi device HID:PNP0C09.

Then passing NULL explicitly avoid confusion, IMO.


> > > > +             args[0].integer.value = 0;
> > > > +             args[2].integer.value = on ? 1 : 0;
> > > > +     } else if (acpi_has_method(handle, method =
> > > > "\\_SB.PCI0.LPCB.EC0.WPIN")) {
> > > > +             args[0].integer.value = 1;
> > > > +             args[2].integer.value = on ? 0 : 1;
> > > > +     } else {
> > > > +             dev_err(&inputdev->dev, "Unable to find ACPI
> > > > method\n");
> > > > +             return -ENOSYS;
> > > > +     }
> > > 
> > > Can these checks be done at initialization phase?  It doesn't seem
> > > needed to be executed at each call.
> > 
> > Agree.
> > 
> 
> Even if it was done in init phase, we still need to check what acpi
> method should be used and based on that and micmute status we set the
> arguments args[0] and args[2], args[1] is the pin value. We could check
> these using event_guid, but that would assume the methods exist for
> each guid.
> For example, if event_guid == MBX_EVENT_GUID, then method SPIN should
> be used.

Hmm, I can't read it from your patch...  Where is event_guid checked
in huawei_wmi_mucmute_led_set()?  I thought it checks only
acpi_has_method().


thanks,

Takashi

[PATCH v7 2/3] x86: add support for Huawei WMI hotkeys.

[Ayman Bagabas]

This driver adds support for missing hotkeys on some Huawei laptops.
Currently, only Huawei Matebook X and Matebook X Pro is supported.

Signed-off-by: Ayman Bagabas <ayman.bagabas@gmail.com>
---
 drivers/platform/x86/Kconfig      |  15 ++
 drivers/platform/x86/Makefile     |   1 +
 drivers/platform/x86/huawei-wmi.c | 247 ++++++++++++++++++++++++++++++
 3 files changed, 263 insertions(+)
 create mode 100644 drivers/platform/x86/huawei-wmi.c

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 87f70e8f4dd0..0f9db4275fb4 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -1292,6 +1292,21 @@ config INTEL_ATOMISP2_PM
 	  To compile this driver as a module, choose M here: the module
 	  will be called intel_atomisp2_pm.
 
+config HUAWEI_WMI
+	tristate "Huawei WMI hotkeys driver"
+	depends on ACPI_WMI
+	depends on INPUT
+	select INPUT_SPARSEKMAP
+	select LEDS_TRIGGERS
+	select LEDS_TRIGGER_AUDIO
+	help
+	  This driver provides support for Huawei WMI hotkeys.
+	  It enables the missing keys and adds support to the micmute
+	  LED found on some of these laptops.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called huawei-wmi.
+
 endif # X86_PLATFORM_DEVICES
 
 config PMC_ATOM
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 39ae94135406..d841c550e3cc 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -32,6 +32,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_WMI)		+= 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..b6157c5b7f4a
--- /dev/null
+++ b/drivers/platform/x86/huawei-wmi.c
@@ -0,0 +1,247 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ *  Huawei WMI hotkeys
+ *
+ *  Copyright (C) 2018	      Ayman Bagabas <ayman.bagabas@gmail.com>
+ */
+
+#include <linux/acpi.h>
+#include <linux/input.h>
+#include <linux/input/sparse-keymap.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+
+MODULE_AUTHOR("Ayman Bagabas <ayman.bagabas@gmail.com>");
+MODULE_DESCRIPTION("Huawei WMI hotkeys");
+MODULE_LICENSE("GPL v2");
+
+/*
+ * Huawei WMI Events GUIDs
+ */
+#define MBX_EVENT_GUID "59142400-C6A3-40fa-BADB-8A2652834100"
+#define MBXP_EVENT_GUID "ABBC0F5C-8EA1-11D1-A000-C90629100000"
+
+MODULE_ALIAS("wmi:"MBX_EVENT_GUID);
+MODULE_ALIAS("wmi:"MBXP_EVENT_GUID);
+
+static const struct key_entry huawei_wmi_keymap[] __initconst = {
+		{ KE_KEY,    0x281, { KEY_BRIGHTNESSDOWN } },
+		{ KE_KEY,    0x282, { KEY_BRIGHTNESSUP } },
+		{ KE_KEY,    0x284, { KEY_MUTE } },
+		{ KE_KEY,    0x285, { KEY_VOLUMEDOWN } },
+		{ KE_KEY,    0x286, { KEY_VOLUMEUP } },
+		{ KE_KEY,    0x287, { KEY_MICMUTE } },
+		{ KE_KEY,    0x289, { KEY_WLAN } },
+		// Huawei |M| button
+		{ KE_KEY,    0x28a, { KEY_CONFIG } },
+		// Keyboard light
+		{ KE_IGNORE, 0x293, { KEY_KBDILLUMTOGGLE } },
+		{ KE_IGNORE, 0x294, { KEY_KBDILLUMUP } },
+		{ KE_IGNORE, 0x295, { KEY_KBDILLUMUP } },
+		{ KE_END,    0 }
+};
+
+static char *event_guid;
+static struct input_dev *inputdev;
+
+static int huawei_wmi_micmute_led_set(bool on)
+{
+	acpi_handle handle;
+	char *method;
+	union acpi_object args[3];
+	struct acpi_object_list arg_list = {
+		.pointer = args,
+		.count = ARRAY_SIZE(args),
+	};
+
+	handle = ACPI_HANDLE(&inputdev->dev);
+	args[0].type = args[1].type = args[2].type = ACPI_TYPE_INTEGER;
+	args[1].integer.value = 0x04;
+
+	if (acpi_has_method(handle, method = "\\_SB.PCI0.LPCB.EC0.SPIN")) {
+		args[0].integer.value = 0;
+		args[2].integer.value = on ? 1 : 0;
+	} else if (acpi_has_method(handle, method = "\\_SB.PCI0.LPCB.EC0.WPIN")) {
+		args[0].integer.value = 1;
+		args[2].integer.value = on ? 0 : 1;
+	} else {
+		dev_err(&inputdev->dev, "Unable to find ACPI method\n");
+		return -ENOSYS;
+	}
+
+	acpi_evaluate_object(handle, method, &arg_list, NULL);
+
+	return 0;
+}
+
+static int micmute_led_set(struct led_classdev *led_cdev,
+		enum led_brightness brightness)
+{
+	int state = brightness != LED_OFF;
+	int err;
+
+	err = huawei_wmi_micmute_led_set(state);
+	return err < 0 ? err : 0;
+}
+
+static struct led_classdev micmute_led_cdev = {
+	.name = "huawei::micmute",
+	.max_brightness = 1,
+	.brightness_set_blocking = micmute_led_set,
+	.default_trigger = "audio-micmute",
+};
+
+static void huawei_wmi_process_key(struct input_dev *inputdev, int code)
+{
+	const struct key_entry *key;
+
+	/*
+	 * MBX uses code 0x80 to indicate a hotkey event.
+	 * The actual key is fetched from the method WQ00.
+	 */
+	if (code == 0x80) {
+		acpi_status status;
+		unsigned long long result;
+		const char *method = "\\WMI0.WQ00";
+		union acpi_object args[1];
+		struct acpi_object_list arg_list = {
+			.pointer = args,
+			.count = ARRAY_SIZE(args),
+		};
+
+		args[0].type = ACPI_TYPE_INTEGER;
+		args[0].integer.value = 0;
+
+		status = acpi_evaluate_integer(ACPI_HANDLE(&inputdev->dev), (char *)method, &arg_list, &result);
+		if (ACPI_FAILURE(status)) {
+			dev_err(&inputdev->dev, "Unable to evaluate ACPI method %s\n", method);
+			return;
+		}
+
+		code = result;
+	}
+
+	key = sparse_keymap_entry_from_scancode(inputdev, code);
+	if (!key) {
+		dev_info(&inputdev->dev, "Unknown key pressed, code: 0x%04x\n", code);
+		return;
+	}
+
+	/*
+	 * The MBXP handles backlight natively using ACPI,
+	 * but not the MBX. If MBXP is being used, skip reporting event.
+	 */
+	if ((key->sw.code == KEY_BRIGHTNESSUP || key->sw.code == KEY_BRIGHTNESSDOWN)
+			&& strcmp(event_guid, MBXP_EVENT_GUID) == 0)
+		return;
+
+	sparse_keymap_report_entry(inputdev, key, 1, true);
+}
+
+static void huawei_wmi_notify(u32 value, void *context)
+{
+	struct input_dev *inputdev = 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)) {
+		dev_err(&inputdev->dev, "Bad event status 0x%x\n", status);
+		return;
+	}
+
+	obj = (union acpi_object *)response.pointer;
+	if (!obj)
+		return;
+
+	if (obj->type == ACPI_TYPE_INTEGER)
+		huawei_wmi_process_key(inputdev, obj->integer.value);
+	else
+		dev_info(&inputdev->dev, "Unknown response received %d\n", obj->type);
+
+	kfree(response.pointer);
+}
+
+static int huawei_wmi_input_init(void)
+{
+	acpi_status status;
+	int err;
+
+	inputdev = input_allocate_device();
+	if (!inputdev)
+		return -ENOMEM;
+
+	inputdev->name = "Huawei WMI hotkeys";
+	inputdev->phys = "wmi/input0";
+	inputdev->id.bustype = BUS_HOST;
+
+	err = sparse_keymap_setup(inputdev, huawei_wmi_keymap, NULL);
+	if (err)
+		goto err_free_dev;
+
+	status = wmi_install_notify_handler(event_guid,
+			huawei_wmi_notify,
+			inputdev);
+	if (ACPI_FAILURE(status)) {
+		err = -EIO;
+		goto err_free_dev;
+	}
+
+	err = input_register_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(inputdev);
+	return err;
+}
+
+static void huawei_wmi_input_exit(void)
+{
+	wmi_remove_notify_handler(event_guid);
+	input_unregister_device(inputdev);
+}
+
+static int __init huawei_wmi_init(void)
+{
+	int err;
+
+	if (wmi_has_guid(MBX_EVENT_GUID)) {
+		event_guid = MBX_EVENT_GUID;
+	} else if (wmi_has_guid(MBXP_EVENT_GUID)) {
+		event_guid = MBXP_EVENT_GUID;
+	} else {
+		pr_warn("Compatible WMI GUID not found\n");
+		return -ENODEV;
+	}
+
+	err = huawei_wmi_input_init();
+	if (err)
+		goto err_input;
+
+	micmute_led_cdev.brightness = ledtrig_audio_get(LED_AUDIO_MICMUTE);
+	err = led_classdev_register(&inputdev->dev, &micmute_led_cdev);
+	if (err)
+		goto err_leds;
+
+	return 0;
+
+err_leds:
+	huawei_wmi_input_exit();
+err_input:
+	return err;
+}
+
+static void __exit huawei_wmi_exit(void)
+{
+	led_classdev_unregister(&micmute_led_cdev);
+	huawei_wmi_input_exit();
+}
+
+module_init(huawei_wmi_init);
+module_exit(huawei_wmi_exit);
-- 
2.19.1