On Mon, 2022-09-19 at 08:54 +0100, Hans de Goede wrote:
> 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@xxxxxxx>
> > > ---
> > > 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@xxxxxxx>
> > > + */
> > > +
> > > +#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
>
Hello Hans,
I did actually start by doing that.
The problem lies with the wmi_get_event_data() function not working
for my ACPI.
wmi_get_event_data() takes the event notify_id as input and is supposed
to give the data associated with the event back. This occures by
calling _WED on the first WMI block that contains said notify_id.
drivers/platform/x86/wmi.c:657:
list_for_each_entry(wblock, &wmi_block_list, list) {
struct guid_block *gblock = &wblock->gblock;
if ((gblock->flags & ACPI_WMI_EVENT) && gblock-
>notify_id == event)
return get_event_data(wblock, out);
}
The ACPI of the Lenovo Yoga 9 14IAP7 contains multiple WMI event
blocks hat contain the same notify_id 0xD0.
Here are two of the four WMI objects found in the DSDT:
in _WDG of block WMIY:
06129D99-6083-4164-81AD-F092F9D773A6:
notify_id: 0xD0
instance_count: 1
flags: 0x8 ACPI_WMI_EVENT
in _WDG of block WMIU:
8FC0DE0C-B4E4-43FD-B0F3-8871711C1294:
notify_id: 0xD0
instance_count: 1
flags: 0x8 ACPI_WMI_EVENT
These event block belong to different WMI devices and report
unrelated values from different _WED handlers. WMIY for example
triggers its event on "mode changes", e.g. laptop/tablet/tent based
on the accelometers/hinge.
WMIU is the WMI block with the event which reports the special keys
I'm interested in.
WMIY comes before WMIU in the wmi_block_list.
Calling wmi_get_event_data() in ideapad_wmi_notify() calls the wrong
_WED (the one of WMIY) and thus returns the wrong event data.
I noticed that the wmi_driver interface does not incur the problem
with the event because it binds to a wmi_block and calls the _WED
directly without searching through other WMI devices.
I thought of changing the signature of wmi_get_event_data() to include
the GUID of the correct WMI block, but chose wmi_driver instead.
Would you consider adding a wmi_get_event_data_for_guid() function to
the wmi module and using that in the ideapad_wmi_notify function to be
a better solution than the one in the patch presented here?
Regards,
Philipp
>
>
> > > +
> > > +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@xxxxxxx>");
> > > +MODULE_DESCRIPTION("Ideapad WMI fn keys driver");
> > > +MODULE_LICENSE("GPL");
> > > --
> > > 2.37.3
> > >
>
