| Message ID | 20230329014559.44494-1-kallmeyeras@gmail.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
Am 29.03.23 um 03:45 schrieb Andrew Kallmeyer: > This driver maps the Lenovo Yoga tablet mode switch to a SW_TABLET_MODE input > device. This will make the tablet status available to desktop environments. > > This patch series is the result of the discussion at > https://lore.kernel.org/r/CAG4kvq9US=-NjyXFMzJYu2zCJryJWtOc7FGZbrewpgCDjdAkbg@mail.gmail.com/ > > I decided to follow-up on the patch Gergo wrote and respond to the review > comments to get it merged and available for everyone. > > Gergo and I have tested this on the Yoga 7 14ARB7, and the Yoga 7 14AIL7 > respectively. Additionally, according to reports at > https://github.com/lukas-w/yoga-usage-mode, which uses the same WMI devices, > this driver should work with: > Yoga C940, Ideapad flex 14API, Yoga 9 14IAP7, Yoga 7 14ARB7, etc. > > v1: https://lore.kernel.org/r/20230310041726.217447-1-kallmeyeras@gmail.com/ > v2: https://lore.kernel.org/r/20230323025200.5462-1-kallmeyeras@gmail.com/ > The diff since v2 is as follows: > > --- a/drivers/platform/x86/lenovo-ymc.c > +++ b/drivers/platform/x86/lenovo-ymc.c > @@ -21,8 +21,8 @@ > #define LENOVO_YMC_QUERY_METHOD 0x01 > > static bool ec_trigger __read_mostly; > -module_param(ec_trigger, bool, 0644); > -MODULE_PARM_DESC(ec_trigger, "Enable EC triggering to emit YMC events"); > +module_param(ec_trigger, bool, 0444); > +MODULE_PARM_DESC(ec_trigger, "Enable EC triggering work-around to force emitting tablet mode events"); > > static const struct dmi_system_id ec_trigger_quirk_dmi_table[] = { > { > @@ -43,8 +43,7 @@ struct lenovo_ymc_private { > static void lenovo_ymc_trigger_ec(struct wmi_device *wdev, struct lenovo_ymc_private *priv) > { > int err; > - > - if (!ec_trigger || !priv || !priv->ec_acpi_dev) > + if (!priv->ec_acpi_dev) > return; > err = write_ec_cmd(priv->ec_acpi_dev->handle, VPCCMD_W_YMC, 1); > if (err) > @@ -103,11 +102,7 @@ static void lenovo_ymc_notify(struct wmi_device *wdev, union acpi_object *data) > lenovo_ymc_trigger_ec(wdev, priv); > } > > -static void lenovo_ymc_remove(struct wmi_device *wdev) > -{ > - struct lenovo_ymc_private *priv = dev_get_drvdata(&wdev->dev); > - acpi_dev_put(priv->ec_acpi_dev); > -} > +static void acpi_dev_put_helper(void *p) { acpi_dev_put(p); } > > static int lenovo_ymc_probe(struct wmi_device *wdev, const void *ctx) > { > @@ -124,10 +119,18 @@ static int lenovo_ymc_probe(struct wmi_device *wdev, const void *ctx) > if (ec_trigger) { > pr_debug("Lenovo YMC enable EC triggering.\n"); > priv->ec_acpi_dev = acpi_dev_get_first_match_dev("VPC2004", NULL, -1); > + > if (!priv->ec_acpi_dev) { > dev_err(&wdev->dev, "Could not find EC ACPI device.\n"); > return -ENODEV; > } > + err = devm_add_action_or_reset(&wdev->dev, > + acpi_dev_put_helper, priv->ec_acpi_dev); > + if (err) { > + dev_err(&wdev->dev, > + "Could not clean up EC ACPI device: %d\n", err); > + return err; > + } > } > > input_dev = devm_input_allocate_device(&wdev->dev); > @@ -138,9 +141,6 @@ static int lenovo_ymc_probe(struct wmi_device *wdev, const void *ctx) > 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, > @@ -177,7 +177,6 @@ static struct wmi_driver lenovo_ymc_driver = { > .id_table = lenovo_ymc_wmi_id_table, > .probe = lenovo_ymc_probe, > .notify = lenovo_ymc_notify, > - .remove = lenovo_ymc_remove, > }; > > module_wmi_driver(lenovo_ymc_driver); > > Andrew Kallmeyer (1): > platform/x86: Move ideapad ACPI helpers to a new header > > Gergo Koteles (1): > platform/x86: Add driver for Yoga Tablet Mode switch > > drivers/platform/x86/Kconfig | 10 ++ > drivers/platform/x86/Makefile | 1 + > drivers/platform/x86/ideapad-laptop.c | 135 +------------------ > drivers/platform/x86/ideapad-laptop.h | 152 +++++++++++++++++++++ > drivers/platform/x86/lenovo-ymc.c | 185 ++++++++++++++++++++++++++ > 5 files changed, 349 insertions(+), 134 deletions(-) > create mode 100644 drivers/platform/x86/ideapad-laptop.h > create mode 100644 drivers/platform/x86/lenovo-ymc.c > Hi, while compile-testing your patch series, i noticed that there are a couple of checkpatch warnings. For lenovo-ymc.c: WARNING: Missing a blank line after declarations #46: FILE: drivers/platform/x86/lenovo-ymc.c:46: + int err; + if (!priv->ec_acpi_dev) CHECK: Alignment should match open parenthesis #77: FILE: drivers/platform/x86/lenovo-ymc.c:77: + status = wmi_evaluate_method(LENOVO_YMC_QUERY_GUID, + LENOVO_YMC_QUERY_INSTANCE, CHECK: Alignment should match open parenthesis #83: FILE: drivers/platform/x86/lenovo-ymc.c:83: + dev_warn(&wdev->dev, + "Failed to evaluate query method: %s\n", CHECK: Alignment should match open parenthesis #92: FILE: drivers/platform/x86/lenovo-ymc.c:92: + dev_warn(&wdev->dev, + "WMI event data is not an integer\n"); CHECK: Alignment should match open parenthesis #128: FILE: drivers/platform/x86/lenovo-ymc.c:128: + err = devm_add_action_or_reset(&wdev->dev, + acpi_dev_put_helper, priv->ec_acpi_dev); For ideapad-laptop.h: WARNING: Improper SPDX comment style for 'drivers/platform/x86/ideapad-laptop.h', please use '/*' instead #1: FILE: drivers/platform/x86/ideapad-laptop.h:1: +// SPDX-License-Identifier: GPL-2.0-or-later WARNING: Missing or malformed SPDX-License-Identifier tag in line 1 #1: FILE: drivers/platform/x86/ideapad-laptop.h:1: +// SPDX-License-Identifier: GPL-2.0-or-later CHECK: line length of 112 exceeds 100 columns #44: FILE: drivers/platform/x86/ideapad-laptop.h:44: +static inline int eval_int_with_arg(acpi_handle handle, const char *name, unsigned long arg, unsigned long *res) All these checkpatch warnings are minor style issues (except the malformed SPDX tag in ideapad-laptop.h), but it would be nice if those would get fixed too. Armin Wolf
Am Dienstag, dem 28.03.2023 um 18:45 -0700 schrieb Andrew Kallmeyer: > This driver maps the Lenovo Yoga tablet mode switch to a SW_TABLET_MODE input > device. This will make the tablet status available to desktop environments. > > This patch series is the result of the discussion at > https://lore.kernel.org/r/CAG4kvq9US=-NjyXFMzJYu2zCJryJWtOc7FGZbrewpgCDjdAkbg@mail.gmail.com/ > > I decided to follow-up on the patch Gergo wrote and respond to the review > comments to get it merged and available for everyone. > > Gergo and I have tested this on the Yoga 7 14ARB7, and the Yoga 7 14AIL7 > respectively. Additionally, according to reports at > https://github.com/lukas-w/yoga-usage-mode, which uses the same WMI devices, > this driver should work with: > Yoga C940, Ideapad flex 14API, Yoga 9 14IAP7, Yoga 7 14ARB7, etc. > > v1: https://lore.kernel.org/r/20230310041726.217447-1-kallmeyeras@gmail.com/ > v2: https://lore.kernel.org/r/20230323025200.5462-1-kallmeyeras@gmail.com/ > The diff since v2 is as follows: > > [..] > > Andrew Kallmeyer (1): > platform/x86: Move ideapad ACPI helpers to a new header > > Gergo Koteles (1): > platform/x86: Add driver for Yoga Tablet Mode switch > > drivers/platform/x86/Kconfig | 10 ++ > drivers/platform/x86/Makefile | 1 + > drivers/platform/x86/ideapad-laptop.c | 135 +------------------ > drivers/platform/x86/ideapad-laptop.h | 152 +++++++++++++++++++++ > drivers/platform/x86/lenovo-ymc.c | 185 ++++++++++++++++++++++++++ > 5 files changed, 349 insertions(+), 134 deletions(-) > create mode 100644 drivers/platform/x86/ideapad-laptop.h > create mode 100644 drivers/platform/x86/lenovo-ymc.c > Hi Andrew, it seems my last tested-by got lost, when preparing the this patch version or was ignored because of the amount of code changes. Anyway, tested again on Lenovo ThinkBook 14s Yoga ITL. Tested-by: André Apitzsch <git@apitzsch.eu> Best regards, André
--- a/drivers/platform/x86/lenovo-ymc.c +++ b/drivers/platform/x86/lenovo-ymc.c @@ -21,8 +21,8 @@ #define LENOVO_YMC_QUERY_METHOD 0x01 static bool ec_trigger __read_mostly; -module_param(ec_trigger, bool, 0644); -MODULE_PARM_DESC(ec_trigger, "Enable EC triggering to emit YMC events"); +module_param(ec_trigger, bool, 0444); +MODULE_PARM_DESC(ec_trigger, "Enable EC triggering work-around to force emitting tablet mode events"); static const struct dmi_system_id ec_trigger_quirk_dmi_table[] = { { @@ -43,8 +43,7 @@ struct lenovo_ymc_private { static void lenovo_ymc_trigger_ec(struct wmi_device *wdev, struct lenovo_ymc_private *priv) { int err; - - if (!ec_trigger || !priv || !priv->ec_acpi_dev) + if (!priv->ec_acpi_dev) return; err = write_ec_cmd(priv->ec_acpi_dev->handle, VPCCMD_W_YMC, 1); if (err) @@ -103,11 +102,7 @@ static void lenovo_ymc_notify(struct wmi_device *wdev, union acpi_object *data) lenovo_ymc_trigger_ec(wdev, priv); } -static void lenovo_ymc_remove(struct wmi_device *wdev) -{ - struct lenovo_ymc_private *priv = dev_get_drvdata(&wdev->dev); - acpi_dev_put(priv->ec_acpi_dev); -} +static void acpi_dev_put_helper(void *p) { acpi_dev_put(p); } static int lenovo_ymc_probe(struct wmi_device *wdev, const void *ctx) { @@ -124,10 +119,18 @@ static int lenovo_ymc_probe(struct wmi_device *wdev, const void *ctx) if (ec_trigger) { pr_debug("Lenovo YMC enable EC triggering.\n"); priv->ec_acpi_dev = acpi_dev_get_first_match_dev("VPC2004", NULL, -1); + if (!priv->ec_acpi_dev) { dev_err(&wdev->dev, "Could not find EC ACPI device.\n"); return -ENODEV; } + err = devm_add_action_or_reset(&wdev->dev, + acpi_dev_put_helper, priv->ec_acpi_dev); + if (err) { + dev_err(&wdev->dev, + "Could not clean up EC ACPI device: %d\n", err); + return err; + } } input_dev = devm_input_allocate_device(&wdev->dev); @@ -138,9 +141,6 @@ static int lenovo_ymc_probe(struct wmi_device *wdev, const void *ctx) 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, @@ -177,7 +177,6 @@ static struct wmi_driver lenovo_ymc_driver = { .id_table = lenovo_ymc_wmi_id_table, .probe = lenovo_ymc_probe, .notify = lenovo_ymc_notify, - .remove = lenovo_ymc_remove, }; module_wmi_driver(lenovo_ymc_driver);