Hi 2023. március 23., csütörtök 3:52 keltezéssel, Andrew Kallmeyer <kallmeyeras@xxxxxxxxx> írta: > From: Gergo Koteles <soyer@xxxxxx> > > This WMI driver for the tablet mode control switch for Lenovo Yoga > notebooks was originally written by Gergo Koteles. The mode is mapped to > a SW_TABLET_MODE switch capable input device. > > Andrew followed the suggestions that were posted in reply to Gergo's RFC > patch, and on the v1 version of this patch to follow-up and get it > merged. > > Changes from Gergo's RFC: > > - Refactored obtaining a reference to the EC ACPI device needed for the > quirk implementation as suggested by Hans de Goede > - Applied small fixes and switched to devm_input_allocate_device() and > removing input_unregister_device() as suggested by Barnabás Pőcze. > - Merged the lenovo_ymc_trigger_ec function with the > ideapad_trigger_ymc_next_read function since it was no longer > external. > - Added the word "Tablet" to the driver description to hopefully make > it more clear. > - Fixed the LENOVO_YMC_QUERY_METHOD ID and the name string for the EC > APCI device trigged for the quirk > - Triggered the input event on probe so that the initial tablet mode > state when the driver is loaded is reported to userspace as suggested > by Armin Wolf. > > We have tested this on the Yoga 7 14AIL7 for the non-quirk path and on > the Yoga 7 14ARB7 which has the firmware bug that requires triggering > the embedded controller to send the mode change events. This workaround > is also used by the Windows drivers. > > According to reports at https://github.com/lukas-w/yoga-usage-mode, > which uses the same WMI devices, the following models should also work: > Yoga C940, Ideapad flex 14API, Yoga 9 14IAP7, Yoga 7 14ARB7, etc. > > Signed-off-by: Gergo Koteles <soyer@xxxxxx> > Co-developed-by: Andrew Kallmeyer <kallmeyeras@xxxxxxxxx> > Signed-off-by: Andrew Kallmeyer <kallmeyeras@xxxxxxxxx> > Link: https://lore.kernel.org/r/20221004214332.35934-1-soyer@xxxxxx/ > Link: https://lore.kernel.org/platform-driver-x86/20230310041726.217447-1-kallmeyeras@xxxxxxxxx/ > --- > [...] > +struct lenovo_ymc_private { > + struct input_dev *input_dev; > + struct acpi_device *ec_acpi_dev; > +}; > + > +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) I think just the `!priv->ec_acpi_dev` check is sufficient. > + return; > + err = write_ec_cmd(priv->ec_acpi_dev->handle, VPCCMD_W_YMC, 1); > + if (err) > + dev_warn(&wdev->dev, "Could not write YMC: %d\n", err); > +} > [...] > +static int lenovo_ymc_probe(struct wmi_device *wdev, const void *ctx) > +{ > + struct input_dev *input_dev; > + struct lenovo_ymc_private *priv; > + int err; > + > + ec_trigger |= dmi_check_system(ec_trigger_quirk_dmi_table); > + > + priv = devm_kzalloc(&wdev->dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + if (ec_trigger) { > + pr_debug("Lenovo YMC enable EC triggering.\n"); > + priv->ec_acpi_dev = acpi_dev_get_first_match_dev("VPC2004", NULL, -1); The reference is leaked in case of an error. Use `devm_add_action_or_reset()`: static void acpi_dev_put_helper(void *p) { acpi_dev_put(p); } [...] priv->ec_acpi_dev = acpi_dev_get_first_match_dev("VPC2004", NULL, -1); if (!priv->ec_acpi_dev) ... err = devm_add_action_or_reset(&wdev->dev, acpi_dev_put_helper, priv->ec_acpi_dev) if (err) ... And then you can remove `lenovo_ymc_remove()` altogether. > + if (!priv->ec_acpi_dev) { > + dev_err(&wdev->dev, "Could not find EC ACPI device.\n"); > + return -ENODEV; > + } > + } > + > + input_dev = devm_input_allocate_device(&wdev->dev); > + if (!input_dev) > + return -ENOMEM; > + > + input_dev->name = "Lenovo Yoga Tablet 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); As far as I can tell `sparse_keymap_setup()` already sets the above. > + > + 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); > + return err; > + } > + > + err = input_register_device(input_dev); > + if (err) { > + dev_err(&wdev->dev, > + "Could not register input device: %d\n", err); > + return err; > + } > + > + priv->input_dev = input_dev; > + dev_set_drvdata(&wdev->dev, priv); > + > + // Report the state for the first time on probe > + lenovo_ymc_trigger_ec(wdev, priv); > + lenovo_ymc_notify(wdev, NULL); > + return 0; > +} > [...] Regards, Barnabás Pőcze