* [PATCH v2 1/3] x86: add support for Huawei WMI hotkeys.
2018-11-02 4:11 [PATCH v2 0/3] Huawei laptops WMI & sound fixes Ayman Bagabas
@ 2018-11-02 4:11 ` Ayman Bagabas
2018-11-02 18:20 ` Andy Shevchenko
2018-11-02 4:11 ` [PATCH v2 2/3] ALSA: hda: fix front speakers on Huawei MBXP Ayman Bagabas
` (2 subsequent siblings)
3 siblings, 1 reply; 16+ messages in thread
From: Ayman Bagabas @ 2018-11-02 4:11 UTC (permalink / raw)
To: Darren Hart, Andy Shevchenko, Jaroslav Kysela, Takashi Iwai,
Ayman Bagabas, Kailang Yang, Hui Wang, linux-kernel,
platform-driver-x86, alsa-devel
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
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);
--
2.17.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v2 1/3] x86: add support for Huawei WMI hotkeys.
2018-11-02 4:11 ` [PATCH v2 1/3] x86: add support for Huawei WMI hotkeys Ayman Bagabas
@ 2018-11-02 18:20 ` Andy Shevchenko
2018-11-02 23:10 ` ayman.bagabas
0 siblings, 1 reply; 16+ messages in thread
From: Andy Shevchenko @ 2018-11-02 18:20 UTC (permalink / raw)
To: Ayman Bagabas
Cc: Darren Hart, Andy Shevchenko, Jaroslav Kysela, Takashi Iwai,
kailang, Hui Wang, Linux Kernel Mailing List, Platform Driver,
ALSA Development Mailing List
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
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2 1/3] x86: add support for Huawei WMI hotkeys.
2018-11-02 18:20 ` Andy Shevchenko
@ 2018-11-02 23:10 ` ayman.bagabas
2018-11-05 11:05 ` Takashi Iwai
0 siblings, 1 reply; 16+ messages in thread
From: ayman.bagabas @ 2018-11-02 23:10 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Darren Hart, Andy Shevchenko, Jaroslav Kysela, Takashi Iwai,
kailang, Hui Wang, Linux Kernel Mailing List, Platform Driver,
ALSA Development Mailing List
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
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2 1/3] x86: add support for Huawei WMI hotkeys.
2018-11-02 23:10 ` ayman.bagabas
@ 2018-11-05 11:05 ` Takashi Iwai
0 siblings, 0 replies; 16+ messages in thread
From: Takashi Iwai @ 2018-11-05 11:05 UTC (permalink / raw)
To: ayman.bagabas
Cc: Andy Shevchenko, ALSA Development Mailing List, Hui Wang,
Andy Shevchenko, Darren Hart, Jaroslav Kysela, kailang,
Linux Kernel Mailing List, Platform Driver
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/3] x86: add support for Huawei WMI hotkeys.
@ 2018-11-05 11:05 ` Takashi Iwai
0 siblings, 0 replies; 16+ messages in thread
From: Takashi Iwai @ 2018-11-05 11:05 UTC (permalink / raw)
To: ayman.bagabas
Cc: Andy Shevchenko, ALSA Development Mailing List, Hui Wang,
Andy Shevchenko, Darren Hart, Jaroslav Kysela, kailang,
Linux Kernel Mailing List, Platform Driver
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 2/3] ALSA: hda: fix front speakers on Huawei MBXP.
2018-11-02 4:11 [PATCH v2 0/3] Huawei laptops WMI & sound fixes Ayman Bagabas
2018-11-02 4:11 ` [PATCH v2 1/3] x86: add support for Huawei WMI hotkeys Ayman Bagabas
@ 2018-11-02 4:11 ` Ayman Bagabas
2018-11-02 18:07 ` Andy Shevchenko
2018-11-02 4:11 ` [PATCH v2 3/3] ALSA: hda: add support for Huawei WMI MicMute LED Ayman Bagabas
2018-11-02 18:05 ` [PATCH v2 0/3] Huawei laptops WMI & sound fixes Andy Shevchenko
3 siblings, 1 reply; 16+ messages in thread
From: Ayman Bagabas @ 2018-11-02 4:11 UTC (permalink / raw)
To: Darren Hart, Andy Shevchenko, Jaroslav Kysela, Takashi Iwai,
Ayman Bagabas, Kailang Yang, Hui Wang, linux-kernel,
platform-driver-x86, alsa-devel
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>
---
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 3ac7ba9b342d..4f7a39c7883c 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -5493,6 +5493,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,
};
@@ -6347,6 +6348,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},
+ { },
+ },
+ },
[ALC295_FIXUP_HP_X360] = {
.type = HDA_FIXUP_FUNC,
.v.func = alc295_fixup_hp_top_speakers,
@@ -6592,6 +6609,7 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
SND_PCI_QUIRK(0x17aa, 0x5109, "Thinkpad", ALC269_FIXUP_LIMIT_INT_MIC_BOOST),
SND_PCI_QUIRK(0x17aa, 0x511e, "Thinkpad", ALC298_FIXUP_TPT470_DOCK),
SND_PCI_QUIRK(0x17aa, 0x511f, "Thinkpad", ALC298_FIXUP_TPT470_DOCK),
+ SND_PCI_QUIRK(0x19e5, 0x3204, "Huawei MBXP", ALC256_FIXUP_HUAWEI_MBXP_PINS),
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(0x1b7d, 0xa831, "Ordissimo EVE2 ", ALC269VB_FIXUP_ORDISSIMO_EVE2), /* Also known as Malata PC-B1303 */
--
2.17.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v2 2/3] ALSA: hda: fix front speakers on Huawei MBXP.
2018-11-02 4:11 ` [PATCH v2 2/3] ALSA: hda: fix front speakers on Huawei MBXP Ayman Bagabas
@ 2018-11-02 18:07 ` Andy Shevchenko
0 siblings, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2018-11-02 18:07 UTC (permalink / raw)
To: Ayman Bagabas
Cc: Darren Hart, Andy Shevchenko, Jaroslav Kysela, Takashi Iwai,
kailang, Hui Wang, Linux Kernel Mailing List, Platform Driver,
ALSA Development Mailing List
On Fri, Nov 2, 2018 at 6:11 AM Ayman Bagabas <ayman.bagabas@gmail.com> wrote:
>
> 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.
> + [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},
> + { },
Terminators better w/o comma.
> + },
> + },
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 3/3] ALSA: hda: add support for Huawei WMI MicMute LED
2018-11-02 4:11 [PATCH v2 0/3] Huawei laptops WMI & sound fixes Ayman Bagabas
2018-11-02 4:11 ` [PATCH v2 1/3] x86: add support for Huawei WMI hotkeys Ayman Bagabas
2018-11-02 4:11 ` [PATCH v2 2/3] ALSA: hda: fix front speakers on Huawei MBXP Ayman Bagabas
@ 2018-11-02 4:11 ` Ayman Bagabas
2018-11-02 18:12 ` Andy Shevchenko
2018-11-02 18:05 ` [PATCH v2 0/3] Huawei laptops WMI & sound fixes Andy Shevchenko
3 siblings, 1 reply; 16+ messages in thread
From: Ayman Bagabas @ 2018-11-02 4:11 UTC (permalink / raw)
To: Darren Hart, Andy Shevchenko, Jaroslav Kysela, Takashi Iwai,
Ayman Bagabas, Kailang Yang, Hui Wang, linux-kernel,
platform-driver-x86, alsa-devel
Some of Huawei laptops come with a LED in the mic mute key. This patch
enables and disable this LED accordingly.
Signed-off-by: Ayman Bagabas <ayman.bagabas@gmail.com>
---
drivers/platform/x86/huawei_wmi.c | 1 +
include/linux/huawei_wmi.h | 7 +++++
sound/pci/hda/huawei_wmi_helper.c | 48 +++++++++++++++++++++++++++++++
sound/pci/hda/patch_realtek.c | 10 +++++++
4 files changed, 66 insertions(+)
create mode 100644 include/linux/huawei_wmi.h
create mode 100644 sound/pci/hda/huawei_wmi_helper.c
diff --git a/drivers/platform/x86/huawei_wmi.c b/drivers/platform/x86/huawei_wmi.c
index 83545217ac19..cc5492571727 100644
--- a/drivers/platform/x86/huawei_wmi.c
+++ b/drivers/platform/x86/huawei_wmi.c
@@ -26,6 +26,7 @@
#include <linux/input/sparse-keymap.h>
#include <linux/acpi.h>
#include <linux/wmi.h>
+#include <linux/huawei_wmi.h>
MODULE_AUTHOR("Ayman Bagabas <ayman.bagabas@gmail.com>");
MODULE_DESCRIPTION("Huawei WMI hotkeys");
diff --git a/include/linux/huawei_wmi.h b/include/linux/huawei_wmi.h
new file mode 100644
index 000000000000..69b656c5029b
--- /dev/null
+++ b/include/linux/huawei_wmi.h
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __HUAWEI_WMI_H__
+#define __HUAWEI_WMI_H__
+
+int huawei_wmi_micmute_led_set(bool on);
+
+#endif
diff --git a/sound/pci/hda/huawei_wmi_helper.c b/sound/pci/hda/huawei_wmi_helper.c
new file mode 100644
index 000000000000..77edb658cbf0
--- /dev/null
+++ b/sound/pci/hda/huawei_wmi_helper.c
@@ -0,0 +1,48 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Helper functions for Huawei WMI Mic Mute LED;
+ * to be included from codec driver
+ */
+
+#if IS_ENABLED(CONFIG_HUAWEI_LAPTOP)
+#include <linux/huawei_wmi.h>
+
+static int (*huawei_wmi_micmute_led_set_func)(bool);
+
+static void update_huawei_wmi_micmute_led(struct hda_codec *codec)
+{
+ struct hda_gen_spec *spec = codec->spec;
+
+ huawei_wmi_micmute_led_set_func(spec->micmute_led.led_value);
+}
+
+static void alc_fixup_huawei_wmi(struct hda_codec *codec,
+ const struct hda_fixup *fix, int action)
+{
+ bool removefunc = false;
+
+ if (action == HDA_FIXUP_ACT_PROBE) {
+ if (!huawei_wmi_micmute_led_set_func)
+ huawei_wmi_micmute_led_set_func = symbol_request(huawei_wmi_micmute_led_set);
+ if (!huawei_wmi_micmute_led_set_func) {
+ codec_warn(codec, "Failed to find huawei_wmi symbol huawei_wmi_micmute_led_set\n");
+ return;
+ }
+ removefunc = (huawei_wmi_micmute_led_set_func(false) < 0)
+ || (snd_hda_gen_add_micmute_led(codec, update_huawei_wmi_micmute_led) < 0);
+
+ }
+
+ if (huawei_wmi_micmute_led_set_func && (action == HDA_FIXUP_ACT_FREE || removefunc)) {
+ symbol_put(huawei_wmi_micmute_led_set);
+ huawei_wmi_micmute_led_set_func = NULL;
+ }
+}
+
+#else
+
+static void alc_fixup_huawei_wmi(struct hda_codec *codec,
+ const struct hda_fixup *fix, int action)
+{
+}
+
+#endif
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 4f7a39c7883c..d09457d2a4f3 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -5374,6 +5374,9 @@ static void alc_fixup_thinkpad_acpi(struct hda_codec *codec,
/* for alc295_fixup_hp_top_speakers */
#include "hp_x360_helper.c"
+/* for alc_fixup_huawei_micmute_led */
+#include "huawei_wmi_helper.c"
+
enum {
ALC269_FIXUP_SONY_VAIO,
ALC275_FIXUP_SONY_VAIO_GPIO2,
@@ -5494,6 +5497,7 @@ enum {
ALC255_FIXUP_DUMMY_LINEOUT_VERB,
ALC255_FIXUP_DELL_HEADSET_MIC,
ALC256_FIXUP_HUAWEI_MBXP_PINS,
+ ALC256_FIXUP_HUAWEI_WMI_MICMUTE_LED,
ALC295_FIXUP_HP_X360,
ALC221_FIXUP_HP_HEADSET_MIC,
};
@@ -6348,6 +6352,10 @@ static const struct hda_fixup alc269_fixups[] = {
.chained = true,
.chain_id = ALC269_FIXUP_HEADSET_MIC
},
+ [ALC256_FIXUP_HUAWEI_WMI_MICMUTE_LED] = {
+ .type = HDA_FIXUP_FUNC,
+ .v.func = alc_fixup_huawei_wmi
+ },
[ALC256_FIXUP_HUAWEI_MBXP_PINS] = {
.type = HDA_FIXUP_PINS,
.v.pins = (const struct hda_pintbl[]) {
@@ -6363,6 +6371,8 @@ static const struct hda_fixup alc269_fixups[] = {
{0x21, 0x04211020},
{ },
},
+ .chained = true,
+ .chain_id = ALC256_FIXUP_HUAWEI_WMI_MICMUTE_LED
},
[ALC295_FIXUP_HP_X360] = {
.type = HDA_FIXUP_FUNC,
--
2.17.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v2 3/3] ALSA: hda: add support for Huawei WMI MicMute LED
2018-11-02 4:11 ` [PATCH v2 3/3] ALSA: hda: add support for Huawei WMI MicMute LED Ayman Bagabas
@ 2018-11-02 18:12 ` Andy Shevchenko
2018-11-02 18:30 ` Takashi Iwai
2018-11-02 23:21 ` ayman.bagabas
0 siblings, 2 replies; 16+ messages in thread
From: Andy Shevchenko @ 2018-11-02 18:12 UTC (permalink / raw)
To: Ayman Bagabas
Cc: Darren Hart, Andy Shevchenko, Jaroslav Kysela, Takashi Iwai,
kailang, Hui Wang, Linux Kernel Mailing List, Platform Driver,
ALSA Development Mailing List
On Fri, Nov 2, 2018 at 6:12 AM Ayman Bagabas <ayman.bagabas@gmail.com> wrote:
>
> Some of Huawei laptops come with a LED in the mic mute key. This patch
> enables and disable this LED accordingly.
> --- a/drivers/platform/x86/huawei_wmi.c
> +++ b/drivers/platform/x86/huawei_wmi.c
> @@ -26,6 +26,7 @@
> #include <linux/input/sparse-keymap.h>
> #include <linux/acpi.h>
> #include <linux/wmi.h>
> +#include <linux/huawei_wmi.h>
Keep it in order and put under
include/linux/platform_data/x86/
folder.
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __HUAWEI_WMI_H__
> +#define __HUAWEI_WMI_H__
> +
> +int huawei_wmi_micmute_led_set(bool on);
> +
> +#endif
This has to cover !HUAWEI_LAPTOP case.
> +/* Helper functions for Huawei WMI Mic Mute LED;
> + * to be included from codec driver
> + */
Comment style.
> +#if IS_ENABLED(CONFIG_HUAWEI_LAPTOP)
See above
> +static int (*huawei_wmi_micmute_led_set_func)(bool);
Why is that?
> + if (action == HDA_FIXUP_ACT_PROBE) {
> + if (!huawei_wmi_micmute_led_set_func)
> + huawei_wmi_micmute_led_set_func = symbol_request(huawei_wmi_micmute_led_set);
> + if (!huawei_wmi_micmute_led_set_func) {
> + codec_warn(codec, "Failed to find huawei_wmi symbol huawei_wmi_micmute_led_set\n");
> + return;
> + }
> + removefunc = (huawei_wmi_micmute_led_set_func(false) < 0)
> + || (snd_hda_gen_add_micmute_led(codec, update_huawei_wmi_micmute_led) < 0);
> +
> + }
> +
> + if (huawei_wmi_micmute_led_set_func && (action == HDA_FIXUP_ACT_FREE || removefunc)) {
> + symbol_put(huawei_wmi_micmute_led_set);
> + huawei_wmi_micmute_led_set_func = NULL;
> + }
> +}
Takashi, is it a way how the rest sound drivers are written? B/c this
symbol_request(s) look to me a bit ugly.
> +/* for alc_fixup_huawei_micmute_led */
> +#include "huawei_wmi_helper.c"
Ditto.
Include *.c?! Huh?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2 3/3] ALSA: hda: add support for Huawei WMI MicMute LED
2018-11-02 18:12 ` Andy Shevchenko
@ 2018-11-02 18:30 ` Takashi Iwai
2018-11-02 23:21 ` ayman.bagabas
1 sibling, 0 replies; 16+ messages in thread
From: Takashi Iwai @ 2018-11-02 18:30 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Ayman Bagabas, ALSA Development Mailing List, Hui Wang,
Andy Shevchenko, Darren Hart, Jaroslav Kysela, kailang,
Linux Kernel Mailing List, Platform Driver
On Fri, 02 Nov 2018 19:12:44 +0100,
Andy Shevchenko wrote:
>
> > + if (action == HDA_FIXUP_ACT_PROBE) {
> > + if (!huawei_wmi_micmute_led_set_func)
> > + huawei_wmi_micmute_led_set_func = symbol_request(huawei_wmi_micmute_led_set);
> > + if (!huawei_wmi_micmute_led_set_func) {
> > + codec_warn(codec, "Failed to find huawei_wmi symbol huawei_wmi_micmute_led_set\n");
> > + return;
> > + }
> > + removefunc = (huawei_wmi_micmute_led_set_func(false) < 0)
> > + || (snd_hda_gen_add_micmute_led(codec, update_huawei_wmi_micmute_led) < 0);
> > +
> > + }
> > +
> > + if (huawei_wmi_micmute_led_set_func && (action == HDA_FIXUP_ACT_FREE || removefunc)) {
> > + symbol_put(huawei_wmi_micmute_led_set);
> > + huawei_wmi_micmute_led_set_func = NULL;
> > + }
> > +}
>
> Takashi, is it a way how the rest sound drivers are written? B/c this
> symbol_request(s) look to me a bit ugly.
It's a workaround for not having a hard dependency. The HD-audio
codec driver is generic, and we don't want to load the multiple WMI
drivers always for using a Realtek codec.
Ugly, yes, but simple enough.
thanks,
Takashi
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2 3/3] ALSA: hda: add support for Huawei WMI MicMute LED
@ 2018-11-02 18:30 ` Takashi Iwai
0 siblings, 0 replies; 16+ messages in thread
From: Takashi Iwai @ 2018-11-02 18:30 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Ayman Bagabas, ALSA Development Mailing List, Hui Wang,
Andy Shevchenko, Darren Hart, Jaroslav Kysela, kailang,
Linux Kernel Mailing List, Platform Driver
On Fri, 02 Nov 2018 19:12:44 +0100,
Andy Shevchenko wrote:
>
> > + if (action == HDA_FIXUP_ACT_PROBE) {
> > + if (!huawei_wmi_micmute_led_set_func)
> > + huawei_wmi_micmute_led_set_func = symbol_request(huawei_wmi_micmute_led_set);
> > + if (!huawei_wmi_micmute_led_set_func) {
> > + codec_warn(codec, "Failed to find huawei_wmi symbol huawei_wmi_micmute_led_set\n");
> > + return;
> > + }
> > + removefunc = (huawei_wmi_micmute_led_set_func(false) < 0)
> > + || (snd_hda_gen_add_micmute_led(codec, update_huawei_wmi_micmute_led) < 0);
> > +
> > + }
> > +
> > + if (huawei_wmi_micmute_led_set_func && (action == HDA_FIXUP_ACT_FREE || removefunc)) {
> > + symbol_put(huawei_wmi_micmute_led_set);
> > + huawei_wmi_micmute_led_set_func = NULL;
> > + }
> > +}
>
> Takashi, is it a way how the rest sound drivers are written? B/c this
> symbol_request(s) look to me a bit ugly.
It's a workaround for not having a hard dependency. The HD-audio
codec driver is generic, and we don't want to load the multiple WMI
drivers always for using a Realtek codec.
Ugly, yes, but simple enough.
thanks,
Takashi
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2 3/3] ALSA: hda: add support for Huawei WMI MicMute LED
2018-11-02 18:30 ` Takashi Iwai
(?)
@ 2018-11-02 18:33 ` Andy Shevchenko
-1 siblings, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2018-11-02 18:33 UTC (permalink / raw)
To: Takashi Iwai
Cc: Ayman Bagabas, ALSA Development Mailing List, Hui Wang,
Andy Shevchenko, Darren Hart, Jaroslav Kysela, kailang,
Linux Kernel Mailing List, Platform Driver
On Fri, Nov 2, 2018 at 8:30 PM Takashi Iwai <tiwai@suse.de> wrote:
> On Fri, 02 Nov 2018 19:12:44 +0100,
> Andy Shevchenko wrote:
> >
> > > + if (action == HDA_FIXUP_ACT_PROBE) {
> > > + if (!huawei_wmi_micmute_led_set_func)
> > > + huawei_wmi_micmute_led_set_func = symbol_request(huawei_wmi_micmute_led_set);
> > > + if (!huawei_wmi_micmute_led_set_func) {
> > > + codec_warn(codec, "Failed to find huawei_wmi symbol huawei_wmi_micmute_led_set\n");
> > > + return;
> > > + }
> > > + removefunc = (huawei_wmi_micmute_led_set_func(false) < 0)
> > > + || (snd_hda_gen_add_micmute_led(codec, update_huawei_wmi_micmute_led) < 0);
> > > +
> > > + }
> > > +
> > > + if (huawei_wmi_micmute_led_set_func && (action == HDA_FIXUP_ACT_FREE || removefunc)) {
> > > + symbol_put(huawei_wmi_micmute_led_set);
> > > + huawei_wmi_micmute_led_set_func = NULL;
> > > + }
> > > +}
> >
> > Takashi, is it a way how the rest sound drivers are written? B/c this
> > symbol_request(s) look to me a bit ugly.
>
> It's a workaround for not having a hard dependency. The HD-audio
> codec driver is generic, and we don't want to load the multiple WMI
> drivers always for using a Realtek codec.
>
> Ugly, yes, but simple enough.
Okay, thanks for elaboration.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 3/3] ALSA: hda: add support for Huawei WMI MicMute LED
2018-11-02 18:12 ` Andy Shevchenko
2018-11-02 18:30 ` Takashi Iwai
@ 2018-11-02 23:21 ` ayman.bagabas
2018-11-05 14:46 ` Andy Shevchenko
1 sibling, 1 reply; 16+ messages in thread
From: ayman.bagabas @ 2018-11-02 23:21 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Darren Hart, Andy Shevchenko, Jaroslav Kysela, Takashi Iwai,
kailang, Hui Wang, Linux Kernel Mailing List, Platform Driver,
ALSA Development Mailing List
On Fri, 2018-11-02 at 20:12 +0200, Andy Shevchenko wrote:
> On Fri, Nov 2, 2018 at 6:12 AM Ayman Bagabas <ayman.bagabas@gmail.com
> > wrote:
> >
> > Some of Huawei laptops come with a LED in the mic mute key. This
> > patch
> > enables and disable this LED accordingly.
> > --- a/drivers/platform/x86/huawei_wmi.c
> > +++ b/drivers/platform/x86/huawei_wmi.c
> > @@ -26,6 +26,7 @@
> > #include <linux/input/sparse-keymap.h>
> > #include <linux/acpi.h>
> > #include <linux/wmi.h>
> > +#include <linux/huawei_wmi.h>
>
> Keep it in order and put under
> include/linux/platform_data/x86/
> folder.
>
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef __HUAWEI_WMI_H__
> > +#define __HUAWEI_WMI_H__
> > +
> > +int huawei_wmi_micmute_led_set(bool on);
> > +
> > +#endif
>
> This has to cover !HUAWEI_LAPTOP case.
>
> > +/* Helper functions for Huawei WMI Mic Mute LED;
> > + * to be included from codec driver
> > + */
>
> Comment style.
/* Helper functions for Huawei WMI micmute led;
* to be included from codec driver
*/
>
> > +#if IS_ENABLED(CONFIG_HUAWEI_LAPTOP)
>
> See above
>
> > +static int (*huawei_wmi_micmute_led_set_func)(bool);
>
> Why is that?
This is used with symbol_request and then in the update function to
locate the function from the wmi device. But I guess you are right, we
could use the function defined in the header file directly.
>
> > + if (action == HDA_FIXUP_ACT_PROBE) {
> > + if (!huawei_wmi_micmute_led_set_func)
> > + huawei_wmi_micmute_led_set_func =
> > symbol_request(huawei_wmi_micmute_led_set);
> > + if (!huawei_wmi_micmute_led_set_func) {
> > + codec_warn(codec, "Failed to find
> > huawei_wmi symbol huawei_wmi_micmute_led_set\n");
> > + return;
> > + }
> > + removefunc =
> > (huawei_wmi_micmute_led_set_func(false) < 0)
> > + || (snd_hda_gen_add_micmute_led(codec,
> > update_huawei_wmi_micmute_led) < 0);
> > +
> > + }
> > +
> > + if (huawei_wmi_micmute_led_set_func && (action ==
> > HDA_FIXUP_ACT_FREE || removefunc)) {
> > + symbol_put(huawei_wmi_micmute_led_set);
> > + huawei_wmi_micmute_led_set_func = NULL;
> > + }
> > +}
>
> Takashi, is it a way how the rest sound drivers are written? B/c this
> symbol_request(s) look to me a bit ugly.
>
> > +/* for alc_fixup_huawei_micmute_led */
> > +#include "huawei_wmi_helper.c"
>
> Ditto.
>
> Include *.c?! Huh?
Is that the wrong way? Should I define the functions directly into
patch_realtek.c?
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2 3/3] ALSA: hda: add support for Huawei WMI MicMute LED
2018-11-02 23:21 ` ayman.bagabas
@ 2018-11-05 14:46 ` Andy Shevchenko
0 siblings, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2018-11-05 14:46 UTC (permalink / raw)
To: Ayman Bagabas
Cc: Darren Hart, Andy Shevchenko, Jaroslav Kysela, Takashi Iwai,
Kailang Yang, Hui Wang, Linux Kernel Mailing List,
Platform Driver, ALSA Development Mailing List
On Sat, Nov 3, 2018 at 1:21 AM <ayman.bagabas@gmail.com> wrote:
> On Fri, 2018-11-02 at 20:12 +0200, Andy Shevchenko wrote:
> > On Fri, Nov 2, 2018 at 6:12 AM Ayman Bagabas <ayman.bagabas@gmail.com
> > > wrote:
Takashi explained me, that is the way sound driver are using the
external symbols, so, follow his suggestion.
> > > +static int (*huawei_wmi_micmute_led_set_func)(bool);
> >
> > Why is that?
>
> This is used with symbol_request and then in the update function to
> locate the function from the wmi device. But I guess you are right, we
> could use the function defined in the header file directly.
> > Takashi, is it a way how the rest sound drivers are written? B/c this
> > symbol_request(s) look to me a bit ugly.
> >
> > > +/* for alc_fixup_huawei_micmute_led */
> > > +#include "huawei_wmi_helper.c"
> >
> > Ditto.
> >
> > Include *.c?! Huh?
>
> Is that the wrong way? Should I define the functions directly into
> patch_realtek.c?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 0/3] Huawei laptops WMI & sound fixes
2018-11-02 4:11 [PATCH v2 0/3] Huawei laptops WMI & sound fixes Ayman Bagabas
` (2 preceding siblings ...)
2018-11-02 4:11 ` [PATCH v2 3/3] ALSA: hda: add support for Huawei WMI MicMute LED Ayman Bagabas
@ 2018-11-02 18:05 ` Andy Shevchenko
3 siblings, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2018-11-02 18:05 UTC (permalink / raw)
To: Ayman Bagabas
Cc: Darren Hart, Andy Shevchenko, Jaroslav Kysela, Takashi Iwai,
kailang, Hui Wang, Linux Kernel Mailing List, Platform Driver,
ALSA Development Mailing List
On Fri, Nov 2, 2018 at 6:11 AM Ayman Bagabas <ayman.bagabas@gmail.com> wrote:
>
> This patch set fixes some of the issues with Huawei laptops.
>
Thanks for the series, I'll review it later on.
> [PATCH v2 1/3]
> The first patch adds support for missing hotkeys on some models. Some hotkeys,
> like brightness keys, work out of the box on these models.
>
> [PATCH v2 2/3]
> This one enables the front speakers on the Huawei Matebook X Pro (MBXP). This
> solves bug 200501 https://bugzilla.kernel.org/show_bug.cgi?id=200501
> It simply uses the pins configurations generated by hdajackretast using the
> settings posted on the bug page https://imgur.com/a/N1xsCVZ
>
> [PATCH v2 3/3]
> This enables the micmute LED on Huawei laptops. It calls an WMI method, using
> PATCH #1, to turn the micmute LED on/off.
>
> Ayman Bagabas (3):
> x86: add support for Huawei WMI hotkeys.
> ALSA: hda: fix front speakers on Huawei MBXP.
> ALSA: hda: add support for Huawei WMI MicMute LED
>
> drivers/platform/x86/Kconfig | 13 ++
> drivers/platform/x86/Makefile | 1 +
> drivers/platform/x86/huawei_wmi.c | 224 ++++++++++++++++++++++++++++++
> include/linux/huawei_wmi.h | 7 +
> sound/pci/hda/huawei_wmi_helper.c | 48 +++++++
> sound/pci/hda/patch_realtek.c | 28 ++++
> 6 files changed, 321 insertions(+)
> create mode 100644 drivers/platform/x86/huawei_wmi.c
> create mode 100644 include/linux/huawei_wmi.h
> create mode 100644 sound/pci/hda/huawei_wmi_helper.c
>
> --
> 2.17.2
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread