Hi
2022. október 4., kedd 23:43 keltezéssel, Gergo Koteles <soyer@xxxxxx> írta:
> This is a WMI driver for the Mode Control switch for Lenovo Yoga
> notebooks.
> The mode is mapped to a SW_TABLET_MODE switch capable input device.
>
> It should work with models like the Yoga C940, Ideapad flex 14API,
> Yoga 9 14IAP7, Yoga 7 14ARB7.
> The 14ARB7 model must trigger the EC to send mode change events.
> This might be a firmware bug.
Is it known how that works on Windows?
>
> Introduces a global variable in the ideapad-laptop driver.
> I would like some advice on how to do it without the variable,
> or how to do it better.
> ---
> drivers/platform/x86/Kconfig | 10 ++
> drivers/platform/x86/Makefile | 1 +
> drivers/platform/x86/ideapad-laptop.c | 18 +++
> drivers/platform/x86/lenovo-ymc.c | 185 ++++++++++++++++++++++++++
> 4 files changed, 214 insertions(+)
> create mode 100644 drivers/platform/x86/lenovo-ymc.c
> [...]
> diff --git a/drivers/platform/x86/lenovo-ymc.c b/drivers/platform/x86/lenovo-ymc.c
> new file mode 100644
> index 000000000000..0b899b02e12f
> --- /dev/null
> +++ b/drivers/platform/x86/lenovo-ymc.c
> @@ -0,0 +1,185 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * lenovo-ymc.c - Lenovo Yoga Mode Control driver
> + *
> + * Copyright © 2022 Gergo Koteles <soyer@xxxxxx>
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/acpi.h>
> +#include <linux/dmi.h>
> +#include <linux/input.h>
> +#include <linux/input/sparse-keymap.h>
> +#include <linux/wmi.h>
> +
> +#define LENOVO_YMC_EVENT_GUID "06129D99-6083-4164-81AD-F092F9D773A6"
> +#define LENOVO_YMC_QUERY_GUID "09B0EE6E-C3FD-4243-8DA1-7911FF80BB8C"
> +
> +#define LENOVO_YMC_QUERY_INSTANCE 0
> +#define LENOVO_YMC_QUERY_METHOD 0xAB
> +
> +extern void ideapad_trigger_ymc_next_read(void);
> +
> +static bool ec_trigger __read_mostly;
> +module_param(ec_trigger, bool, 0644);
> +MODULE_PARM_DESC(ec_trigger, "Enable EC triggering to emit YMC events");
> +
> +static int enable_ec_trigger(const struct dmi_system_id *id)
> +{
> + pr_debug("Lenovo YMC enable EC triggering.\n");
> + ec_trigger = true;
> + return 0;
You seem to be using spaces instead of tabs here. Please run scripts/checkpatch.pl.
The biggest problem currently is probably the missing Signed-off-by tag.
In any case, I think you don't actually need this function because you can do
ec_trigger |= dmi_check_system(ec_trigger_quirk_dmi_table);
in the probe function.
> +}
> +
> +static const struct dmi_system_id ec_trigger_quirk_dmi_table[] = {
> + {
> + // Lenovo Yoga 7 14ARB7
> + .callback = enable_ec_trigger,
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "82QF"),
> + },
> + },
> + { },
> +};
> +
> +static void lenovo_ymc_trigger_ec(void) {
> + if (ec_trigger) {
> + ideapad_trigger_ymc_next_read();
> + }
> +}
> +
> +
> +struct lenovo_ymc_private {
> + struct input_dev *input_dev;
> +};
> +
> +
> +static const struct key_entry lenovo_ymc_keymap[] = {
> + // Laptop
> + { KE_SW, 0x01, { .sw = { SW_TABLET_MODE, 0 } } },
> + // Tablet
> + { KE_SW, 0x02, { .sw = { SW_TABLET_MODE, 1 } } },
> + // Drawing Board
> + { KE_SW, 0x03, { .sw = { SW_TABLET_MODE, 1 } } },
> + // Tent
> + { KE_SW, 0x04, { .sw = { SW_TABLET_MODE, 1 } } },
> + { KE_END },
> +};
> +
> +static void lenovo_ymc_notify(struct wmi_device *wdev, union acpi_object *data)
> +{
> + struct lenovo_ymc_private *priv = dev_get_drvdata(&wdev->dev);
> +
> + u32 input_val = 0;
> + struct acpi_buffer input = {sizeof(input), &input_val};
Shouldn't it be `sizeof(input_val)`?
> + struct acpi_buffer output = {ACPI_ALLOCATE_BUFFER, NULL};
> + acpi_status status;
> + union acpi_object *obj;
> + int code;
> +
> + status = wmi_evaluate_method(LENOVO_YMC_QUERY_GUID,
> + LENOVO_YMC_QUERY_INSTANCE,
> + LENOVO_YMC_QUERY_METHOD,
> + &input, &output);
> +
> + if (ACPI_FAILURE(status)) {
> + dev_warn(&wdev->dev,
> + "Failed to evaluate query method %ud\n", status);
I believe it should be "%u", or maybe even better:
"%s" and `acpi_format_exception(status)`.
> + return;
> + }
> +
> + obj = (union acpi_object *)output.pointer;
Small thing, but this cast is not strictly necessary.
> +
> + if (obj->type != ACPI_TYPE_INTEGER) {
> + dev_warn(&wdev->dev,
> + "WMI event data is not an integer\n");
> + goto free_obj;
> + }
> + code = obj->integer.value;
> +
> + if (!sparse_keymap_report_event(priv->input_dev, code, 1, true))
> + dev_warn(&wdev->dev, "Unknown key %d pressed\n", code);
> +
> +free_obj:
> + kfree(obj);
> + lenovo_ymc_trigger_ec();
> +}
> +
> +static void lenovo_ymc_remove(struct wmi_device *wdev)
> +{
> + struct lenovo_ymc_private *priv = dev_get_drvdata(&wdev->dev);
> +
> + input_unregister_device(priv->input_dev);
> +}
> +
> +static int lenovo_ymc_probe(struct wmi_device *wdev, const void *ctx)
> +{
> + struct input_dev *input_dev;
> + struct lenovo_ymc_private *priv;
> + int err;
> +
> + dmi_check_system(ec_trigger_quirk_dmi_table);
> +
> + priv = devm_kzalloc(&wdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + input_dev = input_allocate_device();
Is there any reason why you're not using `devm_input_allocate_device()` here?
Then you could get rid of `lenovo_ymc_remove()` and the `err_free_dev` label.
> + if (!input_dev) {
> + return -ENOMEM;
> + }
> +
> + input_dev->name = "Lenovo Yoga Mode Control switch";
> + input_dev->phys = LENOVO_YMC_EVENT_GUID "/input0";
> + input_dev->id.bustype = BUS_HOST;
> + input_dev->dev.parent = &wdev->dev;
> +
> + input_set_capability(input_dev, EV_SW, SW_TABLET_MODE);
> +
> + err = sparse_keymap_setup(input_dev, lenovo_ymc_keymap, NULL);
> + if (err) {
> + dev_err(&wdev->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(&wdev->dev,
> + "Could not register input device: %d\n", err);
> + goto err_free_dev;
> + }
> +
> + priv->input_dev = input_dev;
> + dev_set_drvdata(&wdev->dev, priv);
> + lenovo_ymc_trigger_ec();
> + return 0;
> +
> +err_free_dev:
> + input_free_device(input_dev);
> + return err;
> +}
> +
> +static const struct wmi_device_id lenovo_ymc_wmi_id_table[] = {
> + { .guid_string = LENOVO_YMC_EVENT_GUID },
> + { }
> +};
> +MODULE_DEVICE_TABLE(wmi, lenovo_ymc_wmi_id_table);
> +
> +static struct wmi_driver lenovo_ymc_driver = {
> + .driver = {
> + .name = "lenovo-ymc",
> + },
> + .id_table = lenovo_ymc_wmi_id_table,
> + .probe = lenovo_ymc_probe,
> + .remove = lenovo_ymc_remove,
> + .notify = lenovo_ymc_notify,
> +};
> +
> +module_wmi_driver(lenovo_ymc_driver);
> +
> +MODULE_AUTHOR("Gergo Koteles <soyer@xxxxxx>");
> +MODULE_DESCRIPTION("Lenovo Yoga Mode Control driver");
> +MODULE_LICENSE("GPL");
> --
> 2.37.3
Regards,
Barnabás Pőcze
