2023-06-28 20:58:33

by Edson Juliano Drosdeck

[permalink] [raw]
Subject: [PATCH] platform/x86: wmi: add Positivo WMI key driver

Some function keys on the built in keyboard on Positivo's notebooks does
not produce any key events when pressed in combination with the function
key. Some of these keys do report that they are being pressed via WMI
events.

This driver reports key events for Fn+F10,Fn+F11 and a custom key to
launch a custom program.

Other WMI events that are reported by the hardware but not utilized by
this driver are Caps Lock(which already work).

Signed-off-by: Edson Juliano Drosdeck <[email protected]>
---
drivers/platform/x86/Kconfig | 11 +++
drivers/platform/x86/Makefile | 1 +
drivers/platform/x86/positivo-wmi.c | 136 ++++++++++++++++++++++++++++
3 files changed, 148 insertions(+)
create mode 100644 drivers/platform/x86/positivo-wmi.c

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 22052031c719..f3ad84479460 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -134,6 +134,17 @@ config YOGABOOK_WMI
To compile this driver as a module, choose M here: the module will
be called lenovo-yogabook-wmi.

+config POSITIVO_WMI
+ tristate "Positivo WMI key driver"
+ depends on ACPI_WMI
+ depends on INPUT
+ select INPUT_SPARSEKMAP
+ help
+ This driver provides support for Positvo WMI hotkeys.
+
+ To compile this driver as a module, choose M here: the module
+ will be called positivo-wmi.
+
config ACERHDF
tristate "Acer Aspire One temperature and fan driver"
depends on ACPI && THERMAL
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 2cafe51ec4d8..5458bb9a56d3 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_NVIDIA_WMI_EC_BACKLIGHT) += nvidia-wmi-ec-backlight.o
obj-$(CONFIG_XIAOMI_WMI) += xiaomi-wmi.o
obj-$(CONFIG_GIGABYTE_WMI) += gigabyte-wmi.o
obj-$(CONFIG_YOGABOOK_WMI) += lenovo-yogabook-wmi.o
+obj-$(CONFIG_POSITIVO_WMI) += positivo-wmi.o

# Acer
obj-$(CONFIG_ACERHDF) += acerhdf.o
diff --git a/drivers/platform/x86/positivo-wmi.c b/drivers/platform/x86/positivo-wmi.c
new file mode 100644
index 000000000000..5fbb4cf42154
--- /dev/null
+++ b/drivers/platform/x86/positivo-wmi.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+/* WMI driver for Positivo Laptops
+ *
+ * Copyright (C) 2023 Edson Juliano Drosdeck <[email protected]>
+ *
+ * */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/kernel.h>
+#include <linux/acpi.h>
+#include <linux/input.h>
+#include <linux/input/sparse-keymap.h>
+#include <linux/dmi.h>
+#include <linux/module.h>
+
+MODULE_AUTHOR("Edson Juliano Drosdeck");
+MODULE_DESCRIPTION("Positivo WMI Hotkey Driver");
+MODULE_LICENSE("GPL");
+
+#define POSITIVO_WMI_EVENT_GUID "ABBC0F72-8EA1-11D1-00A0-C90629100000"
+
+MODULE_ALIAS("wmi:"POSITIVO_WMI_EVENT_GUID);
+
+static const struct key_entry positivo_wmi_keymap[] = {
+ { KE_KEY, 0x1c, { KEY_PROG1} },
+ { KE_KEY, 0x36, { KEY_WLAN } },
+ { KE_KEY, 0x37, { KEY_BLUETOOTH } },
+ { KE_END, 0},
+};
+
+static struct input_dev *positivo_wmi_input_dev;
+
+static void positivo_wmi_notify(u32 value, void *context)
+{
+ struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL };
+ union acpi_object *obj;
+ int eventcode;
+ acpi_status status;
+
+ status = wmi_get_event_data(value, &response);
+ if (status != AE_OK) {
+ pr_err("bad event status 0x%x\n", status);
+ return;
+ }
+
+ obj = (union acpi_object *)response.pointer;
+ if (obj && obj->type == ACPI_TYPE_INTEGER) {
+ eventcode = obj->integer.value;
+
+ if (!sparse_keymap_report_event(positivo_wmi_input_dev,
+ eventcode, 1, true))
+ pr_err("Unknown key %x pressed\n", eventcode);
+ }
+
+ kfree(response.pointer);
+}
+
+static int positivo_wmi_input_setup(void)
+{
+
+ int err;
+
+ positivo_wmi_input_dev = input_allocate_device();
+ if (!positivo_wmi_input_dev)
+ return -ENOMEM;
+
+ positivo_wmi_input_dev->name = "Positivo laptop WMI hotkeys";
+ positivo_wmi_input_dev->phys = "wmi/input0";
+ positivo_wmi_input_dev->id.bustype = BUS_HOST;
+
+ err = sparse_keymap_setup(positivo_wmi_input_dev,
+ positivo_wmi_keymap, NULL);
+ if (err)
+ goto err_free_dev;
+
+ err = input_register_device(positivo_wmi_input_dev);
+
+ if (err){
+ pr_info("Unable to register input device\n");
+ goto err_free_dev;
+ }
+
+ return 0;
+
+err_free_dev:
+ input_free_device(positivo_wmi_input_dev);
+ return err;
+}
+
+static const struct dmi_system_id positivo_wmi_dmi_table[] __initconst = {
+ {
+ .ident = "Positivo laptop",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Positivo Tecnologia SA"),
+ },
+ },
+ {}
+};
+
+static int __init positivo_wmi_init(void)
+{
+ int err;
+
+ if (!wmi_has_guid(POSITIVO_WMI_EVENT_GUID) ||
+ !dmi_check_system(positivo_wmi_dmi_table))
+ return -ENODEV;
+
+ err = positivo_wmi_input_setup();
+ if (err)
+ return err;
+
+ err = wmi_install_notify_handler(POSITIVO_WMI_EVENT_GUID,
+ positivo_wmi_notify, NULL);
+ if (ACPI_FAILURE(err)) {
+ pr_err("Unable to setup WMI notify handler\n");
+ goto err_free_input;
+ }
+
+ return 0;
+
+err_free_input:
+ input_unregister_device(positivo_wmi_input_dev);
+ return err;
+
+}
+
+static void __exit positivo_wmi_exit(void)
+{
+ wmi_remove_notify_handler(POSITIVO_WMI_EVENT_GUID);
+ input_free_device(positivo_wmi_input_dev);
+}
+
+module_init(positivo_wmi_init);
+module_exit(positivo_wmi_exit);
--
2.39.2



2023-06-28 21:17:26

by Armin Wolf

[permalink] [raw]
Subject: Re: [PATCH] platform/x86: wmi: add Positivo WMI key driver

Am 28.06.23 um 23:40 schrieb Edson Juliano Drosdeck:

> Some function keys on the built in keyboard on Positivo's notebooks does
> not produce any key events when pressed in combination with the function
> key. Some of these keys do report that they are being pressed via WMI
> events.
>
> This driver reports key events for Fn+F10,Fn+F11 and a custom key to
> launch a custom program.
>
> Other WMI events that are reported by the hardware but not utilized by
> this driver are Caps Lock(which already work).
>
> Signed-off-by: Edson Juliano Drosdeck <[email protected]>
> ---
> drivers/platform/x86/Kconfig | 11 +++
> drivers/platform/x86/Makefile | 1 +
> drivers/platform/x86/positivo-wmi.c | 136 ++++++++++++++++++++++++++++
> 3 files changed, 148 insertions(+)
> create mode 100644 drivers/platform/x86/positivo-wmi.c
>
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 22052031c719..f3ad84479460 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -134,6 +134,17 @@ config YOGABOOK_WMI
> To compile this driver as a module, choose M here: the module will
> be called lenovo-yogabook-wmi.
>
> +config POSITIVO_WMI
> + tristate "Positivo WMI key driver"
> + depends on ACPI_WMI
> + depends on INPUT
> + select INPUT_SPARSEKMAP
> + help
> + This driver provides support for Positvo WMI hotkeys.
> +
> + To compile this driver as a module, choose M here: the module
> + will be called positivo-wmi.
> +
> config ACERHDF
> tristate "Acer Aspire One temperature and fan driver"
> depends on ACPI && THERMAL
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index 2cafe51ec4d8..5458bb9a56d3 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_NVIDIA_WMI_EC_BACKLIGHT) += nvidia-wmi-ec-backlight.o
> obj-$(CONFIG_XIAOMI_WMI) += xiaomi-wmi.o
> obj-$(CONFIG_GIGABYTE_WMI) += gigabyte-wmi.o
> obj-$(CONFIG_YOGABOOK_WMI) += lenovo-yogabook-wmi.o
> +obj-$(CONFIG_POSITIVO_WMI) += positivo-wmi.o
>
> # Acer
> obj-$(CONFIG_ACERHDF) += acerhdf.o
> diff --git a/drivers/platform/x86/positivo-wmi.c b/drivers/platform/x86/positivo-wmi.c
> new file mode 100644
> index 000000000000..5fbb4cf42154
> --- /dev/null
> +++ b/drivers/platform/x86/positivo-wmi.c
> @@ -0,0 +1,136 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +/* WMI driver for Positivo Laptops
> + *
> + * Copyright (C) 2023 Edson Juliano Drosdeck <[email protected]>
> + *
> + * */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/kernel.h>
> +#include <linux/acpi.h>
> +#include <linux/input.h>
> +#include <linux/input/sparse-keymap.h>
> +#include <linux/dmi.h>
> +#include <linux/module.h>
> +
> +MODULE_AUTHOR("Edson Juliano Drosdeck");
> +MODULE_DESCRIPTION("Positivo WMI Hotkey Driver");
> +MODULE_LICENSE("GPL");
> +
> +#define POSITIVO_WMI_EVENT_GUID "ABBC0F72-8EA1-11D1-00A0-C90629100000"
> +
> +MODULE_ALIAS("wmi:"POSITIVO_WMI_EVENT_GUID);
> +
> +static const struct key_entry positivo_wmi_keymap[] = {
> + { KE_KEY, 0x1c, { KEY_PROG1} },
> + { KE_KEY, 0x36, { KEY_WLAN } },
> + { KE_KEY, 0x37, { KEY_BLUETOOTH } },
> + { KE_END, 0},
> +};
> +
> +static struct input_dev *positivo_wmi_input_dev;
> +
> +static void positivo_wmi_notify(u32 value, void *context)
> +{
> + struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL };
> + union acpi_object *obj;
> + int eventcode;
> + acpi_status status;
> +
> + status = wmi_get_event_data(value, &response);
> + if (status != AE_OK) {
> + pr_err("bad event status 0x%x\n", status);
> + return;
> + }
> +
> + obj = (union acpi_object *)response.pointer;
> + if (obj && obj->type == ACPI_TYPE_INTEGER) {
> + eventcode = obj->integer.value;
> +
> + if (!sparse_keymap_report_event(positivo_wmi_input_dev,
> + eventcode, 1, true))
> + pr_err("Unknown key %x pressed\n", eventcode);
> + }
> +
> + kfree(response.pointer);
> +}
> +
> +static int positivo_wmi_input_setup(void)
> +{
> +
> + int err;
> +
> + positivo_wmi_input_dev = input_allocate_device();
> + if (!positivo_wmi_input_dev)
> + return -ENOMEM;
> +
> + positivo_wmi_input_dev->name = "Positivo laptop WMI hotkeys";
> + positivo_wmi_input_dev->phys = "wmi/input0";
> + positivo_wmi_input_dev->id.bustype = BUS_HOST;
> +
> + err = sparse_keymap_setup(positivo_wmi_input_dev,
> + positivo_wmi_keymap, NULL);
> + if (err)
> + goto err_free_dev;
> +
> + err = input_register_device(positivo_wmi_input_dev);
> +
> + if (err){
> + pr_info("Unable to register input device\n");
> + goto err_free_dev;
> + }
> +
> + return 0;
> +
> +err_free_dev:
> + input_free_device(positivo_wmi_input_dev);
> + return err;
> +}
> +
> +static const struct dmi_system_id positivo_wmi_dmi_table[] __initconst = {
> + {
> + .ident = "Positivo laptop",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Positivo Tecnologia SA"),
> + },
> + },
> + {}
> +};

Hello,

it would be better if the driver matches on the WMI GUID. More on this below.

> +
> +static int __init positivo_wmi_init(void)
> +{
> + int err;
> +
> + if (!wmi_has_guid(POSITIVO_WMI_EVENT_GUID) ||
> + !dmi_check_system(positivo_wmi_dmi_table))
> + return -ENODEV;
> +
> + err = positivo_wmi_input_setup();
> + if (err)
> + return err;
> +
> + err = wmi_install_notify_handler(POSITIVO_WMI_EVENT_GUID,
> + positivo_wmi_notify, NULL);

Please use the newer bus-based WMI interface. The legacy GUID-based interface is
deprecated an has several issues. I recommend you to take a look at the xiaomi-wmi
driver. It seems to do a very similar thing and uses the modern bus-based WMI interface.
The only big difference is that your driver uses the acpi_object passed by the notify
callback to determine the key code.

You can find more documentation regarding the bus-based WMI interface under:
https://www.kernel.org/doc/html/next/driver-api/wmi.html

Thanks,
Armin Wolf

> + if (ACPI_FAILURE(err)) {
> + pr_err("Unable to setup WMI notify handler\n");
> + goto err_free_input;
> + }
> +
> + return 0;
> +
> +err_free_input:
> + input_unregister_device(positivo_wmi_input_dev);
> + return err;
> +
> +}
> +
> +static void __exit positivo_wmi_exit(void)
> +{
> + wmi_remove_notify_handler(POSITIVO_WMI_EVENT_GUID);
> + input_free_device(positivo_wmi_input_dev);
> +}
> +
> +module_init(positivo_wmi_init);
> +module_exit(positivo_wmi_exit);

2023-06-28 21:57:40

by Armin Wolf

[permalink] [raw]
Subject: Re: [PATCH] platform/x86: wmi: add Positivo WMI key driver

Am 28.06.23 um 23:04 schrieb Armin Wolf:

> Am 28.06.23 um 23:40 schrieb Edson Juliano Drosdeck:
>
>> Some function keys on the built in keyboard on Positivo's notebooks does
>> not produce any key events when pressed in combination with the function
>> key. Some of these keys do report that they are being pressed via WMI
>> events.
>>
>> This driver reports key events for Fn+F10,Fn+F11  and a custom key to
>> launch a custom program.
>>
>> Other WMI events that are reported by the hardware but not utilized by
>> this driver are Caps Lock(which already work).
>>
>> Signed-off-by: Edson Juliano Drosdeck <[email protected]>
>> ---
>>   drivers/platform/x86/Kconfig        |  11 +++
>>   drivers/platform/x86/Makefile       |   1 +
>>   drivers/platform/x86/positivo-wmi.c | 136 ++++++++++++++++++++++++++++
>>   3 files changed, 148 insertions(+)
>>   create mode 100644 drivers/platform/x86/positivo-wmi.c
>>
>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>> index 22052031c719..f3ad84479460 100644
>> --- a/drivers/platform/x86/Kconfig
>> +++ b/drivers/platform/x86/Kconfig
>> @@ -134,6 +134,17 @@ config YOGABOOK_WMI
>>         To compile this driver as a module, choose M here: the module
>> will
>>         be called lenovo-yogabook-wmi.
>>
>> +config POSITIVO_WMI
>> +    tristate "Positivo WMI key driver"
>> +    depends on ACPI_WMI
>> +    depends on INPUT
>> +    select INPUT_SPARSEKMAP
>> +    help
>> +      This driver provides support for Positvo WMI hotkeys.
>> +
>> +      To compile this driver as a module, choose M here: the module
>> +      will be called positivo-wmi.
>> +
>>   config ACERHDF
>>       tristate "Acer Aspire One temperature and fan driver"
>>       depends on ACPI && THERMAL
>> diff --git a/drivers/platform/x86/Makefile
>> b/drivers/platform/x86/Makefile
>> index 2cafe51ec4d8..5458bb9a56d3 100644
>> --- a/drivers/platform/x86/Makefile
>> +++ b/drivers/platform/x86/Makefile
>> @@ -15,6 +15,7 @@ obj-$(CONFIG_NVIDIA_WMI_EC_BACKLIGHT)    +=
>> nvidia-wmi-ec-backlight.o
>>   obj-$(CONFIG_XIAOMI_WMI)        += xiaomi-wmi.o
>>   obj-$(CONFIG_GIGABYTE_WMI)        += gigabyte-wmi.o
>>   obj-$(CONFIG_YOGABOOK_WMI)        += lenovo-yogabook-wmi.o
>> +obj-$(CONFIG_POSITIVO_WMI)        += positivo-wmi.o
>>
>>   # Acer
>>   obj-$(CONFIG_ACERHDF)        += acerhdf.o
>> diff --git a/drivers/platform/x86/positivo-wmi.c
>> b/drivers/platform/x86/positivo-wmi.c
>> new file mode 100644
>> index 000000000000..5fbb4cf42154
>> --- /dev/null
>> +++ b/drivers/platform/x86/positivo-wmi.c
>> @@ -0,0 +1,136 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +
>> +/* WMI driver for Positivo Laptops
>> + *
>> + * Copyright (C) 2023 Edson Juliano Drosdeck <[email protected]>
>> + *
>> + * */
>> +
>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/acpi.h>
>> +#include <linux/input.h>
>> +#include <linux/input/sparse-keymap.h>
>> +#include <linux/dmi.h>
>> +#include <linux/module.h>
>> +
>> +MODULE_AUTHOR("Edson Juliano Drosdeck");
>> +MODULE_DESCRIPTION("Positivo WMI Hotkey Driver");
>> +MODULE_LICENSE("GPL");
>> +
>> +#define POSITIVO_WMI_EVENT_GUID "ABBC0F72-8EA1-11D1-00A0-C90629100000"
>> +
>> +MODULE_ALIAS("wmi:"POSITIVO_WMI_EVENT_GUID);
>> +
>> +static const struct key_entry positivo_wmi_keymap[] = {
>> +    { KE_KEY, 0x1c, { KEY_PROG1} },
>> +    { KE_KEY, 0x36, { KEY_WLAN } },
>> +    { KE_KEY, 0x37, { KEY_BLUETOOTH } },
>> +    { KE_END, 0},
>> +};
>> +
>> +static struct input_dev *positivo_wmi_input_dev;
>> +
>> +static void positivo_wmi_notify(u32 value, void *context)
>> +{
>> +    struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL };
>> +    union acpi_object *obj;
>> +    int eventcode;
>> +    acpi_status status;
>> +
>> +    status = wmi_get_event_data(value, &response);
>> +    if (status != AE_OK) {
>> +        pr_err("bad event status 0x%x\n", status);
>> +        return;
>> +    }
>> +
>> +    obj = (union acpi_object *)response.pointer;
>> +    if (obj && obj->type == ACPI_TYPE_INTEGER) {
>> +        eventcode = obj->integer.value;
>> +
>> +        if (!sparse_keymap_report_event(positivo_wmi_input_dev,
>> +                        eventcode, 1, true))
>> +            pr_err("Unknown key %x pressed\n", eventcode);
>> +    }
>> +
>> +    kfree(response.pointer);
>> +}
>> +
>> +static int positivo_wmi_input_setup(void)
>> +{
>> +
>> +    int err;
>> +
>> +    positivo_wmi_input_dev = input_allocate_device();
>> +    if (!positivo_wmi_input_dev)
>> +        return -ENOMEM;
>> +
>> +    positivo_wmi_input_dev->name = "Positivo laptop WMI hotkeys";
>> +    positivo_wmi_input_dev->phys = "wmi/input0";
>> +    positivo_wmi_input_dev->id.bustype = BUS_HOST;
>> +
>> +    err = sparse_keymap_setup(positivo_wmi_input_dev,
>> +                  positivo_wmi_keymap, NULL);
>> +    if (err)
>> +        goto err_free_dev;
>> +
>> +    err = input_register_device(positivo_wmi_input_dev);
>> +
>> +    if (err){
>> +        pr_info("Unable to register input device\n");
>> +        goto err_free_dev;
>> +    }
>> +
>> +    return 0;
>> +
>> +err_free_dev:
>> +    input_free_device(positivo_wmi_input_dev);
>> +    return err;
>> +}
>> +
>> +static const struct dmi_system_id positivo_wmi_dmi_table[]
>> __initconst = {
>> +    {
>> +        .ident = "Positivo laptop",
>> +        .matches = {
>> +            DMI_MATCH(DMI_SYS_VENDOR, "Positivo Tecnologia SA"),
>> +        },
>> +    },
>> +    {}
>> +};
>
> Hello,
>
> it would be better if the driver matches on the WMI GUID. More on this
> below.

I just found out that the WMI GUID used by the driver (ABBC0F72-8EA1-11D1-00A0-C90629100000),
which is supposed to be unique, is actually also used by the eeepc-wmi driver.

This _should_ normally not happen, but it seems that Positivio reused a WMI GUID found inside
the Microsoft ACPI WMI driver samples (line 227), which can be found under:
https://github.com/microsoft/Windows-driver-samples/blob/main/wmi/wmiacpi/acpimof.mof

Could you share the output of "acpidump"? I would be very interested to find out why this
WMI GUID is not unique.

Armin Wolf

>
>> +
>> +static int __init positivo_wmi_init(void)
>> +{
>> +    int err;
>> +
>> +    if (!wmi_has_guid(POSITIVO_WMI_EVENT_GUID) ||
>> +        !dmi_check_system(positivo_wmi_dmi_table))
>> +        return -ENODEV;
>> +
>> +    err = positivo_wmi_input_setup();
>> +    if (err)
>> +        return err;
>> +
>> +    err = wmi_install_notify_handler(POSITIVO_WMI_EVENT_GUID,
>> +                    positivo_wmi_notify, NULL);
>
> Please use the newer bus-based WMI interface. The legacy GUID-based
> interface is
> deprecated an has several issues. I recommend you to take a look at
> the xiaomi-wmi
> driver. It seems to do a very similar thing and uses the modern
> bus-based WMI interface.
> The only big difference is that your driver uses the acpi_object
> passed by the notify
> callback to determine the key code.
>
> You can find more documentation regarding the bus-based WMI interface
> under:
> https://www.kernel.org/doc/html/next/driver-api/wmi.html
>
> Thanks,
> Armin Wolf
>
>> +        if (ACPI_FAILURE(err)) {
>> +            pr_err("Unable to setup WMI notify handler\n");
>> +            goto err_free_input;
>> +        }
>> +
>> +    return 0;
>> +
>> +err_free_input:
>> +    input_unregister_device(positivo_wmi_input_dev);
>> +    return err;
>> +
>> +}
>> +
>> +static void __exit positivo_wmi_exit(void)
>> +{
>> +       wmi_remove_notify_handler(POSITIVO_WMI_EVENT_GUID);
>> +       input_free_device(positivo_wmi_input_dev);
>> +}
>> +
>> +module_init(positivo_wmi_init);
>> +module_exit(positivo_wmi_exit);

2023-07-12 15:59:24

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH] platform/x86: wmi: add Positivo WMI key driver

Hi Edson,

On 6/28/23 23:40, Edson Juliano Drosdeck wrote:
> Some function keys on the built in keyboard on Positivo's notebooks does
> not produce any key events when pressed in combination with the function
> key. Some of these keys do report that they are being pressed via WMI
> events.
>
> This driver reports key events for Fn+F10,Fn+F11 and a custom key to
> launch a custom program.
>
> Other WMI events that are reported by the hardware but not utilized by
> this driver are Caps Lock(which already work).
>
> Signed-off-by: Edson Juliano Drosdeck <[email protected]>

Thank you for your patch.

As Armin has mentioned, please change the driver to use the
WMI bus model and make it a wmi_driver.

A simple example (except for it using multiple WMI GUIDs) of such
a driver would be drivers/platform/x86/xiaomi-wmi.c

Please model your driver after this. This means you will be able
to drop the wmi_has_guid(POSITIVO_WMI_EVENT_GUID) from your
module_init() function (which will now become a probe function).

You do still need to keep the DMI check, since as Armin pointed
out the GUID unfortunately is not unique, but instead used the
Microsoft WMI docs example GUID.

Armin, we have seen this (using Microsoft WMI example GUID)
before and there is nothing we can do about this except add
a DMI check. This works for Windows because the WMI driver
for this for Windows is part of the factory image and presumably
has never been added to Windows Updates avoiding the GUID conflict.

Regards,

Hans




> ---
> drivers/platform/x86/Kconfig | 11 +++
> drivers/platform/x86/Makefile | 1 +
> drivers/platform/x86/positivo-wmi.c | 136 ++++++++++++++++++++++++++++
> 3 files changed, 148 insertions(+)
> create mode 100644 drivers/platform/x86/positivo-wmi.c
>
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 22052031c719..f3ad84479460 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -134,6 +134,17 @@ config YOGABOOK_WMI
> To compile this driver as a module, choose M here: the module will
> be called lenovo-yogabook-wmi.
>
> +config POSITIVO_WMI
> + tristate "Positivo WMI key driver"
> + depends on ACPI_WMI
> + depends on INPUT
> + select INPUT_SPARSEKMAP
> + help
> + This driver provides support for Positvo WMI hotkeys.
> +
> + To compile this driver as a module, choose M here: the module
> + will be called positivo-wmi.
> +
> config ACERHDF
> tristate "Acer Aspire One temperature and fan driver"
> depends on ACPI && THERMAL
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index 2cafe51ec4d8..5458bb9a56d3 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_NVIDIA_WMI_EC_BACKLIGHT) += nvidia-wmi-ec-backlight.o
> obj-$(CONFIG_XIAOMI_WMI) += xiaomi-wmi.o
> obj-$(CONFIG_GIGABYTE_WMI) += gigabyte-wmi.o
> obj-$(CONFIG_YOGABOOK_WMI) += lenovo-yogabook-wmi.o
> +obj-$(CONFIG_POSITIVO_WMI) += positivo-wmi.o
>
> # Acer
> obj-$(CONFIG_ACERHDF) += acerhdf.o
> diff --git a/drivers/platform/x86/positivo-wmi.c b/drivers/platform/x86/positivo-wmi.c
> new file mode 100644
> index 000000000000..5fbb4cf42154
> --- /dev/null
> +++ b/drivers/platform/x86/positivo-wmi.c
> @@ -0,0 +1,136 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +/* WMI driver for Positivo Laptops
> + *
> + * Copyright (C) 2023 Edson Juliano Drosdeck <[email protected]>
> + *
> + * */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/kernel.h>
> +#include <linux/acpi.h>
> +#include <linux/input.h>
> +#include <linux/input/sparse-keymap.h>
> +#include <linux/dmi.h>
> +#include <linux/module.h>
> +
> +MODULE_AUTHOR("Edson Juliano Drosdeck");
> +MODULE_DESCRIPTION("Positivo WMI Hotkey Driver");
> +MODULE_LICENSE("GPL");
> +
> +#define POSITIVO_WMI_EVENT_GUID "ABBC0F72-8EA1-11D1-00A0-C90629100000"
> +
> +MODULE_ALIAS("wmi:"POSITIVO_WMI_EVENT_GUID);
> +
> +static const struct key_entry positivo_wmi_keymap[] = {
> + { KE_KEY, 0x1c, { KEY_PROG1} },
> + { KE_KEY, 0x36, { KEY_WLAN } },
> + { KE_KEY, 0x37, { KEY_BLUETOOTH } },
> + { KE_END, 0},
> +};
> +
> +static struct input_dev *positivo_wmi_input_dev;
> +
> +static void positivo_wmi_notify(u32 value, void *context)
> +{
> + struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL };
> + union acpi_object *obj;
> + int eventcode;
> + acpi_status status;
> +
> + status = wmi_get_event_data(value, &response);
> + if (status != AE_OK) {
> + pr_err("bad event status 0x%x\n", status);
> + return;
> + }
> +
> + obj = (union acpi_object *)response.pointer;
> + if (obj && obj->type == ACPI_TYPE_INTEGER) {
> + eventcode = obj->integer.value;
> +
> + if (!sparse_keymap_report_event(positivo_wmi_input_dev,
> + eventcode, 1, true))
> + pr_err("Unknown key %x pressed\n", eventcode);
> + }
> +
> + kfree(response.pointer);
> +}
> +
> +static int positivo_wmi_input_setup(void)
> +{
> +
> + int err;
> +
> + positivo_wmi_input_dev = input_allocate_device();
> + if (!positivo_wmi_input_dev)
> + return -ENOMEM;
> +
> + positivo_wmi_input_dev->name = "Positivo laptop WMI hotkeys";
> + positivo_wmi_input_dev->phys = "wmi/input0";
> + positivo_wmi_input_dev->id.bustype = BUS_HOST;
> +
> + err = sparse_keymap_setup(positivo_wmi_input_dev,
> + positivo_wmi_keymap, NULL);
> + if (err)
> + goto err_free_dev;
> +
> + err = input_register_device(positivo_wmi_input_dev);
> +
> + if (err){
> + pr_info("Unable to register input device\n");
> + goto err_free_dev;
> + }
> +
> + return 0;
> +
> +err_free_dev:
> + input_free_device(positivo_wmi_input_dev);
> + return err;
> +}
> +
> +static const struct dmi_system_id positivo_wmi_dmi_table[] __initconst = {
> + {
> + .ident = "Positivo laptop",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Positivo Tecnologia SA"),
> + },
> + },
> + {}
> +};
> +
> +static int __init positivo_wmi_init(void)
> +{
> + int err;
> +
> + if (!wmi_has_guid(POSITIVO_WMI_EVENT_GUID) ||
> + !dmi_check_system(positivo_wmi_dmi_table))
> + return -ENODEV;
> +
> + err = positivo_wmi_input_setup();
> + if (err)
> + return err;
> +
> + err = wmi_install_notify_handler(POSITIVO_WMI_EVENT_GUID,
> + positivo_wmi_notify, NULL);
> + if (ACPI_FAILURE(err)) {
> + pr_err("Unable to setup WMI notify handler\n");
> + goto err_free_input;
> + }
> +
> + return 0;
> +
> +err_free_input:
> + input_unregister_device(positivo_wmi_input_dev);
> + return err;
> +
> +}
> +
> +static void __exit positivo_wmi_exit(void)
> +{
> + wmi_remove_notify_handler(POSITIVO_WMI_EVENT_GUID);
> + input_free_device(positivo_wmi_input_dev);
> +}
> +
> +module_init(positivo_wmi_init);
> +module_exit(positivo_wmi_exit);