diff mbox

[v3,0/2] platform/x86: Add driver for Yoga Tablet mode switch

Message ID 20230329014559.44494-1-kallmeyeras@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andrew Kallmeyer March 29, 2023, 1:45 a.m. UTC
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

Comments

Armin Wolf March 31, 2023, 5:48 p.m. UTC | #1
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
André Apitzsch April 5, 2023, 6:27 p.m. UTC | #2
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é
diff mbox

Patch

--- 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);