mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: Lucy Wong <lucywong2778@gmail.com>
Cc: "Hans de Goede" <hansg@kernel.org>,
	"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
	platform-driver-x86@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] platform/x86: add Xiaomi WMI key driver
Date: Mon, 8 Jun 2026 13:09:02 +0300 (EEST)	[thread overview]
Message-ID: <ddc819dc-4c5a-ba62-bced-3222e2e60bb7@linux.intel.com> (raw)
In-Reply-To: <20260601152703.94539-2-lucywong2778@gmail.com>

On Mon, 1 Jun 2026, Lucy Wong wrote:

> Certain Xiaomi laptops expose extra function keys through a single WMI
> event GUID (B74AF83F-8B2F-4069-ACAC-36D176F62FC0). Unlike the existing
> xiaomi-wmi driver where each key has its own GUID, here all keys share
> one GUID and the event buffer encodes both an event type and a keycode:
> 
>   buffer[1] = event type
>     0x02: press/release keys (5 keys with abstract icons)
>     0x03: firmware-handled toggles (touchpad, Fn lock)
>   buffer[2] = keycode within that type
> 
> The keys with abstract icons are mapped to programmable macro keys for
> userspace to bind. The touchpad and Fn lock keys are handled by the EC
> and are reported for notification purposes only.
> 
> Signed-off-by: Lucy Wong <lucywong2778@gmail.com>
> ---
>  MAINTAINERS                            |   6 ++
>  drivers/platform/x86/Kconfig           |  11 ++
>  drivers/platform/x86/Makefile          |   1 +
>  drivers/platform/x86/xiaomi-wmi-keys.c | 139 +++++++++++++++++++++++++
>  4 files changed, 157 insertions(+)
>  create mode 100644 drivers/platform/x86/xiaomi-wmi-keys.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 839e39825455..7969b82c8315 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -29124,6 +29124,12 @@ F:	fs/xfs/
>  F:	include/uapi/linux/dqblk_xfs.h
>  F:	include/uapi/linux/fsmap.h
>  
> +XIAOMI WMI KEYS DRIVER
> +M:	Lucy Wong <lucywong2778@gmail.com>
> +L:	platform-driver-x86@vger.kernel.org
> +S:	Maintained
> +F:	drivers/platform/x86/xiaomi-wmi-keys.c
> +
>  XILINX AMS DRIVER
>  M:	Salih Erim <salih.erim@amd.com>
>  M:	Conall O'Griofa <conall.ogriofa@amd.com>
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index b54b5212b204..541b0a255daa 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -90,6 +90,17 @@ config XIAOMI_WMI
>  	  To compile this driver as a module, choose M here: the module will
>  	  be called xiaomi-wmi.
>  
> +config XIAOMI_WMI_KEYS
> +	tristate "Xiaomi WMI key events driver"
> +	depends on ACPI_WMI
> +	depends on INPUT
> +	help
> +	  Say Y here if you want to support WMI-based hotkey events on certain
> +	  Xiaomi laptops.
> +
> +	  To compile this driver as a module, choose M here: the module will
> +	  be called xiaomi-wmi-keys.
> +
>  config REDMI_WMI
>  	tristate "Redmibook WMI key driver"
>  	depends on ACPI_WMI
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index 872ac3842391..d852bb0bf61b 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -12,6 +12,7 @@ obj-$(CONFIG_HUAWEI_WMI)		+= huawei-wmi.o
>  obj-$(CONFIG_MXM_WMI)			+= mxm-wmi.o
>  obj-$(CONFIG_NVIDIA_WMI_EC_BACKLIGHT)	+= nvidia-wmi-ec-backlight.o
>  obj-$(CONFIG_XIAOMI_WMI)		+= xiaomi-wmi.o
> +obj-$(CONFIG_XIAOMI_WMI_KEYS)		+= xiaomi-wmi-keys.o
>  obj-$(CONFIG_REDMI_WMI)			+= redmi-wmi.o
>  obj-$(CONFIG_GIGABYTE_WMI)		+= gigabyte-wmi.o
>  obj-$(CONFIG_BITLAND_MIFS_WMI)		+= bitland-mifs-wmi.o
> diff --git a/drivers/platform/x86/xiaomi-wmi-keys.c b/drivers/platform/x86/xiaomi-wmi-keys.c
> new file mode 100644
> index 000000000000..524ede0e64eb
> --- /dev/null
> +++ b/drivers/platform/x86/xiaomi-wmi-keys.c
> @@ -0,0 +1,139 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Xiaomi laptop WMI hotkey driver
> + *
> + * Copyright (C) 2026 Lucy Wong <lucywong2778@gmail.com>
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/device.h>
> +#include <linux/input.h>
> +#include <linux/input/sparse-keymap.h>
> +#include <linux/jiffies.h>
> +#include <linux/module.h>
> +#include <linux/wmi.h>
> +
> +#define XIAOMI_WMI_EVENT_GUID "B74AF83F-8B2F-4069-ACAC-36D176F62FC0"
> +
> +#define XIAOMI_TYPE_BACKLIGHT 0x01 /* keyboard backlight level (EC handled) */
> +#define XIAOMI_TYPE_KEY  0x02 /* press/release keys */
> +#define XIAOMI_TYPE_TOGGLE 0x03 /* firmware toggle keys (notify only) */

Please align values within a "group".

> +
> +#define XIAOMI_KEY_PRESS_MIN 0x01
> +#define XIAOMI_KEY_PRESS_MAX 0x05
> +#define XIAOMI_KEY_RELEASE_OFF 0x05

Ditto.

> +
> +#define XIAOMI_TOGGLE_DEDUP_MS 50
> +
> +struct xiaomi_wmi {
> +	struct input_dev *input_dev;
> +	unsigned long last_toggle;
> +	u8 last_toggle_code;
> +};
> +
> +static const struct key_entry xiaomi_wmi_keymap[] = {
> +	/* type 0x02: programmable keys with abstract icons */
> +	{ KE_KEY, 0x0201, { KEY_MACRO1 } },
> +	{ KE_KEY, 0x0202, { KEY_MACRO2 } },
> +	{ KE_KEY, 0x0203, { KEY_MACRO3 } },
> +	{ KE_KEY, 0x0204, { KEY_MACRO4 } },
> +	{ KE_KEY, 0x0205, { KEY_MACRO5 } },
> +	/* type 0x03: firmware-handled toggles, reported for OSD only */
> +	{ KE_KEY, 0x0300, { KEY_F21 } }, /* touchpad toggle */

There is KEY_TOUCHPAD_TOGGLE.

> +	{ KE_KEY, 0x0301, { KEY_FN_ESC } }, /* Fn lock */

I'm not sure if KEY_FN_ESC is right key for it.

And if these are handled by FW, shouldn't you use KEY_IGNORE?

Since you've space, align the upper comment to the one below for better 
readability.

> +	{ KE_END, 0 },
> +};
> +
> +static int xiaomi_wmi_probe(struct wmi_device *wdev, const void *context)
> +{
> +	struct xiaomi_wmi *data;
> +	int ret;
> +
> +	data = devm_kzalloc(&wdev->dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +	dev_set_drvdata(&wdev->dev, data);
> +
> +	data->input_dev = devm_input_allocate_device(&wdev->dev);
> +	if (!data->input_dev)
> +		return -ENOMEM;
> +
> +	data->input_dev->name = "Xiaomi WMI keys";
> +	data->input_dev->phys = "wmi/input0";
> +	data->input_dev->id.bustype = BUS_HOST;
> +
> +	ret = sparse_keymap_setup(data->input_dev, xiaomi_wmi_keymap, NULL);

Please add select INPUT_SPARSEKMAP.

> +	if (ret)
> +		return ret;
> +
> +	return input_register_device(data->input_dev);
> +}
> +
> +static void xiaomi_wmi_notify(struct wmi_device *wdev, union acpi_object *obj)
> +{
> +	struct xiaomi_wmi *data = dev_get_drvdata(&wdev->dev);
> +	u8 type, code;
> +
> +	if (obj->type != ACPI_TYPE_BUFFER || obj->buffer.length < 3)
> +		return;
> +
> +	type = obj->buffer.pointer[1];
> +	code = obj->buffer.pointer[2];
> +
> +	switch (type) {
> +	case XIAOMI_TYPE_KEY:
> +		if (code >= XIAOMI_KEY_PRESS_MIN &&
> +		    code <= XIAOMI_KEY_PRESS_MAX) {
> +			sparse_keymap_report_event(data->input_dev,
> +						   0x0200 | code, 1, false);

Please name literals.

> +		} else if (code > XIAOMI_KEY_PRESS_MAX &&
> +						code <= XIAOMI_KEY_PRESS_MAX +
> +							XIAOMI_KEY_RELEASE_OFF) {

Please align properly.

I think the defines are not optional as you seem to have two blocks of 
codes, both with n length, one doing press and the other release but your 
defines are not based one the code blocks being of same length.

> +			code -= XIAOMI_KEY_RELEASE_OFF;
> +			sparse_keymap_report_event(data->input_dev,
> +						   0x0200 | code, 0, false);
> +		}
> +		break;
> +
> +	case XIAOMI_TYPE_TOGGLE:
> +		/* deduplicate: each press emits two identical events */
> +		if (data->last_toggle_code == code &&
> +		    time_before(jiffies, data->last_toggle +

Is ->last_toggle initialized to a sensible value?
(found by sashiko.dev)

> +				msecs_to_jiffies(XIAOMI_TOGGLE_DEDUP_MS)))
> +			return;
> +		data->last_toggle_code = code;
> +		data->last_toggle = jiffies;

Locking needed?

Is it guaranteed that the duplicates are always in-order, that is, not two 
different toggles can get interleaved?

> +		sparse_keymap_report_event(data->input_dev,
> +					   0x0300 | code, 1, true);
> +		break;
> +
> +	case XIAOMI_TYPE_BACKLIGHT:
> +		/* keyboard backlight handled by EC, nothing to do */
> +		break;
> +
> +	default:
> +		dev_dbg(&wdev->dev, "unknown event type 0x%02x\n", type);

Add include.

> +		break;
> +	}
> +}
> +
> +static const struct wmi_device_id xiaomi_wmi_id_table[] = {
> +	{ .guid_string = XIAOMI_WMI_EVENT_GUID },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(wmi, xiaomi_wmi_id_table);
> +
> +static struct wmi_driver xiaomi_wmi_driver = {
> +	.driver = {
> +		.name = "xiaomi-wmi-keys",
> +	},
> +	.id_table = xiaomi_wmi_id_table,
> +	.probe = xiaomi_wmi_probe,
> +	.notify = xiaomi_wmi_notify,

We're transitioning wmi core, so please convert to .notify_new.

You may also consider using .min_event_size.

> +	.no_singleton = true,
> +};
> +module_wmi_driver(xiaomi_wmi_driver);
> +
> +MODULE_AUTHOR("Lucy Wong <lucywong2778@gmail.com>");
> +MODULE_DESCRIPTION("Xiaomi laptop WMI hotkey driver");
> +MODULE_LICENSE("GPL");
> 

-- 
 i.


  reply	other threads:[~2026-06-08 10:09 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-01 15:27 [PATCH 0/1] " Lucy Wong
2026-06-01 15:27 ` [PATCH 1/1] " Lucy Wong
2026-06-08 10:09   ` Ilpo Järvinen [this message]
2026-06-10 17:40     ` Armin Wolf
  -- strict thread matches above, loose matches on Subject: below --
2026-06-01 15:14 [PATCH 0/1] " Lucy Wong
2026-06-01 15:14 ` [PATCH 1/1] " Lucy Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ddc819dc-4c5a-ba62-bced-3222e2e60bb7@linux.intel.com \
    --to=ilpo.jarvinen@linux.intel.com \
    --cc=hansg@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lucywong2778@gmail.com \
    --cc=platform-driver-x86@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®