Re: [PATCH] Add IdeaPad WMI Fn Keys driver

[Hans de Goede <hdegoede@redhat.com>]

Hi again,

On 9/11/22 17:18, Hans de Goede wrote:
> Hi Philipp,
> 
> On 9/11/22 18:04, Philipp Jungkamp wrote:
>> Create an input device for WMI events corresponding to some special
>> keys on the 'Lenovo Yoga' line.
>>
>> This include the 3 keys to the right on the 'Lenovo Yoga9 14IAP7' and
>> additionally the 'Lenovo Support' and 'Lenovo Favorites' (star with 'S'
>> inside) in the fn key row as well as the event emitted on 'Fn+R' which
>> toggles between 60Hz and 90Hz display refresh rate on windows.
>>
>> Signed-off-by: Philipp Jungkamp <p.jungkamp@gmx.net>
>> ---
>> I found this patch by poking in the DSDT. I have not submitted any
>> notable patches yet and hope you can help me improve in case I make
>> unfortunate choices during submission.
> 
> No worries at a first glance (I have not looked at this in any
> detail yet) this looks pretty good for a first submission.
> 
> And thank you for contributing to the Linux kernel!
> 
> 
>> Philipp Jungkamp
>>
>>  drivers/platform/x86/Kconfig       |  13 +++
>>  drivers/platform/x86/Makefile      |   1 +
>>  drivers/platform/x86/ideapad-wmi.c | 153 +++++++++++++++++++++++++++++
>>  3 files changed, 167 insertions(+)
>>  create mode 100644 drivers/platform/x86/ideapad-wmi.c
>>
>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>> index f2f98e942cf2..e7c5148e5cb4 100644
>> --- a/drivers/platform/x86/Kconfig
>> +++ b/drivers/platform/x86/Kconfig
>> @@ -140,6 +140,19 @@ config YOGABOOK_WMI
>>  	  To compile this driver as a module, choose M here: the module will
>>  	  be called lenovo-yogabook-wmi.
>>
>> +config IDEAPAD_WMI
>> +	tristate "Lenovo IdeaPad WMI Fn Keys"
>> +	depends on ACPI_WMI
>> +	depends on INPUT
>> +	select INPUT_SPARSEKMAP
>> +	help
>> +	  Say Y here if you want to receive key presses from some lenovo
>> +	  specific keys. (Star Key, Support Key, Virtual Background,
>> +	  Dark Mode Toggle, ...)
>> +
>> +	  To compile this driver as a module, choose M here: the module will
>> +	  be called ideapad-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 5a428caa654a..d8bec884d6bc 100644
>> --- a/drivers/platform/x86/Makefile
>> +++ b/drivers/platform/x86/Makefile
>> @@ -16,6 +16,7 @@ obj-$(CONFIG_PEAQ_WMI)			+= peaq-wmi.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_IDEAPAD_WMI)		+= ideapad-wmi.o
>>
>>  # Acer
>>  obj-$(CONFIG_ACERHDF)		+= acerhdf.o
>> diff --git a/drivers/platform/x86/ideapad-wmi.c b/drivers/platform/x86/ideapad-wmi.c
>> new file mode 100644
>> index 000000000000..38f7b3d0c171
>> --- /dev/null
>> +++ b/drivers/platform/x86/ideapad-wmi.c
>> @@ -0,0 +1,153 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * ideapad-wmi.c - Ideapad WMI fn keys driver
>> + *
>> + * Copyright (C) 2022 Philipp Jungkamp <p.jungkamp@gmx.net>
>> + */
>> +
>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>> +
>> +#include <linux/acpi.h>
>> +#include <linux/input.h>
>> +#include <linux/input/sparse-keymap.h>
>> +#include <linux/list.h>
>> +#include <linux/module.h>
>> +#include <linux/wmi.h>
>> +
>> +#define IDEAPAD_FN_KEY_EVENT_GUID	"8FC0DE0C-B4E4-43FD-B0F3-8871711C1294"
> 
> At a first hunch (basically huh, don't we have a driver for that
> already?) I grepped through the kernel sources and found:
> 
> drivers/platform/x86/ideapad-laptop.c
> 
> can you see if you can make things work with that driver?

So I have taken a quick look at this and it seems to me that this
really should be able to work with the existing ideapad-laptop.c code ?

For starters you could add a debug printk / dev_info to this block,

#if IS_ENABLED(CONFIG_ACPI_WMI)
        for (i = 0; i < ARRAY_SIZE(ideapad_wmi_fnesc_events); i++) {
                status = wmi_install_notify_handler(ideapad_wmi_fnesc_events[i],
                                                    ideapad_wmi_notify, priv);
                if (ACPI_SUCCESS(status)) {
                        priv->fnesc_guid = ideapad_wmi_fnesc_events[i];
                        break;
                }
        }

        if (ACPI_FAILURE(status) && status != AE_NOT_EXIST) {
                err = -EIO;
                goto notification_failed_wmi;
        }
#endif

checking which event GUID ideapad-laptop binds to on your laptop. Assuming that
it does bind to the GUID this driver is binding to too, then it would be
a matter of extending the existing ideapad_wmi_notify() to do the same
as your notify function in this stand-alone driver. Note you can get the
the equivalend of the union acpi_object *data argument in your wmi handler
by calling wmi_get_event_data().

Regards,

Hans



>> +
>> +struct ideapad_wmi_private {
>> +	struct wmi_device *wmi_device;
>> +	struct input_dev *input_dev;
>> +};
>> +
>> +static const struct key_entry ideapad_wmi_fn_key_keymap[] = {
>> +	/* FnLock (handled by the firmware) */
>> +	{ KE_IGNORE,	0x02 },
>> +	/* Customizable Lenovo Hotkey ("star" with 'S' inside) */
>> +	{ KE_KEY,	0x01, { KEY_FAVORITES } },
>> +	/* Dark mode toggle */
>> +	{ KE_KEY,	0x13, { KEY_PROG1 } },
>> +	/* Sound profile switch */
>> +	{ KE_KEY,	0x12, { KEY_PROG2 } },
>> +	/* Lenovo Virtual Background application */
>> +	{ KE_KEY,	0x28, { KEY_PROG3 } },
>> +	/* Lenovo Support */
>> +	{ KE_KEY,	0x27, { KEY_HELP } },
>> +	/* Refresh Rate Toggle */
>> +	{ KE_KEY,	0x0a, { KEY_DISPLAYTOGGLE } },
>> +	{ KE_END },
>> +};
>> +
>> +static int ideapad_wmi_input_init(struct ideapad_wmi_private *priv)
>> +{
>> +	struct input_dev *input_dev;
>> +	int err;
>> +
>> +	input_dev = input_allocate_device();
>> +	if (!input_dev) {
>> +		return -ENOMEM;
>> +	}
>> +
>> +	input_dev->name = "Ideapad WMI Fn Keys";
>> +	input_dev->phys = IDEAPAD_FN_KEY_EVENT_GUID "/input0";
>> +	input_dev->id.bustype = BUS_HOST;
>> +	input_dev->dev.parent = &priv->wmi_device->dev;
>> +
>> +	err = sparse_keymap_setup(input_dev, ideapad_wmi_fn_key_keymap, NULL);
>> +	if (err) {
>> +		dev_err(&priv->wmi_device->dev,
>> +			"Could not set up input device keymap: %d\n", err);
>> +		goto err_free_dev;
>> +	}
>> +
>> +	err = input_register_device(input_dev);
>> +	if (err) {
>> +		dev_err(&priv->wmi_device->dev,
>> +			"Could not register input device: %d\n", err);
>> +		goto err_free_dev;
>> +	}
>> +
>> +	priv->input_dev = input_dev;
>> +	return 0;
>> +
>> +err_free_dev:
>> +	input_free_device(input_dev);
>> +	return err;
>> +}
>> +
>> +static void ideapad_wmi_input_exit(struct ideapad_wmi_private *priv)
>> +{
>> +	input_unregister_device(priv->input_dev);
>> +	priv->input_dev = NULL;
>> +}
>> +
>> +static void ideapad_wmi_input_report(struct ideapad_wmi_private *priv,
>> +				     unsigned int scancode)
>> +{
>> +	sparse_keymap_report_event(priv->input_dev, scancode, 1, true);
>> +}
>> +
>> +static int ideapad_wmi_probe(struct wmi_device *wdev, const void *ctx)
>> +{
>> +	struct ideapad_wmi_private *priv;
>> +	int err;
>> +
>> +	priv = devm_kzalloc(&wdev->dev, sizeof(*priv), GFP_KERNEL);
>> +	if (!priv)
>> +		return -ENOMEM;
>> +
>> +	dev_set_drvdata(&wdev->dev, priv);
>> +
>> +	priv->wmi_device = wdev;
>> +
>> +	err = ideapad_wmi_input_init(priv);
>> +	if (err)
>> +		return err;
>> +
>> +	return 0;
>> +}
>> +
>> +static void ideapad_wmi_remove(struct wmi_device *wdev)
>> +{
>> +	struct ideapad_wmi_private *priv = dev_get_drvdata(&wdev->dev);
>> +
>> +	ideapad_wmi_input_exit(priv);
>> +}
>> +
>> +static void ideapad_wmi_notify(struct wmi_device *wdev, union acpi_object *data)
>> +{
>> +	struct ideapad_wmi_private *priv = dev_get_drvdata(&wdev->dev);
>> +
>> +	if(data->type != ACPI_TYPE_INTEGER) {
>> +		dev_warn(&priv->wmi_device->dev,
>> +			"WMI event data is not an integer\n");
>> +		return;
>> +	}
>> +
>> +	ideapad_wmi_input_report(priv, data->integer.value);
>> +}
>> +
>> +static const struct wmi_device_id ideapad_wmi_id_table[] = {
>> +	{	/* Special Keys on the Yoga 9 14IAP7 */
>> +		.guid_string = IDEAPAD_FN_KEY_EVENT_GUID
>> +	},
>> +	{ }
>> +};
>> +
>> +static struct wmi_driver ideapad_wmi_driver = {
>> +	.driver = {
>> +		.name = "ideapad-wmi",
>> +	},
>> +	.id_table = ideapad_wmi_id_table,
>> +	.probe = ideapad_wmi_probe,
>> +	.remove = ideapad_wmi_remove,
>> +	.notify = ideapad_wmi_notify,
>> +};
>> +
>> +module_wmi_driver(ideapad_wmi_driver);
>> +
>> +MODULE_DEVICE_TABLE(wmi, ideapad_wmi_id_table);
>> +MODULE_AUTHOR("Philipp Jungkamp <p.jungkamp@gmx.net>");
>> +MODULE_DESCRIPTION("Ideapad WMI fn keys driver");
>> +MODULE_LICENSE("GPL");
>> --
>> 2.37.3
>>