[PATCH 00/24] platform/x86: ideapad-laptop: cleanup, keyboard backlight and "always on USB charging" control support, reenable touchpad control

[PATCH 00/24] platform/x86: ideapad-laptop: cleanup, keyboard backlight and "always on USB charging" control support, reenable touchpad control

[Barnabás Pőcze]

This series contains patches that aim to bring more consistency
to the code; add keyboard backlight control support; add
"always on USB charging" control support.
Furthermore, commit 7f363145992cebf4ea760447f1cfdf6f81459683 is reverted
since it made it impossible to disable/enable the touchpad via the
ideapad-laptop module and on some devices the method implemented in the
module works correctly to disable/enable the touchpad.

Barnabás Pőcze (24):
  platform/x86: ideapad-laptop: remove unnecessary dev_set_drvdata()
    call
  platform/x86: ideapad-laptop: use appropriately typed variable to
    store the return value of ACPI methods
  platform/x86: ideapad-laptop: sort includes lexicographically
  platform/x86: ideapad-laptop: use sysfs_emit()
  platform/x86: ideapad-laptop: use for_each_set_bit() helper to
    simplify event processing
  platform/x86: ideapad-laptop: use msecs_to_jiffies() helper instead of
    hand-crafted formula
  platform/x86: ideapad-laptop: use dev_{err,warn} or appropriate
    variant to display log messages
  platform/x86: ideapad-laptop: convert ACPI helpers to return -EIO in
    case of failure
  platform/x86: ideapad-laptop: always propagate error codes from device
    attributes' show() callback
  platform/x86: ideapad-laptop: misc. device attribute changes
  platform/x86: ideapad-laptop: group and separate (un)related constants
    into enums
  platform/x86: ideapad-laptop: rework and create new ACPI helpers
  platform/x86: ideapad-laptop: rework is_visible() logic
  platform/x86: ideapad-laptop: check for Fn-lock support in HALS
  platform/x86: ideapad-laptop: check for touchpad support in _CFG
  platform/x86: ideapad-laptop: change 'status' debugfs file format
  platform/x86: ideapad-laptop: change 'cfg' debugfs file format
  Revert "platform/x86: ideapad-laptop: Switch touchpad attribute to be
    RO"
  platform/x86: ideapad-laptop: fix checkpatch warnings, more consistent
    style
  platform/x86: ideapad-laptop: send notification about touchpad state
    change to sysfs
  platform/x86: ideapad-laptop: add keyboard backlight control support
  platform/x86: ideapad-laptop: add "always on USB charging" control
    support
  Documentation/ABI: sysfs-platform-ideapad-laptop: update device
    attribute paths
  Documentation/ABI: sysfs-platform-ideapad-laptop: conservation_mode
    and usb_charging

 .../ABI/testing/sysfs-platform-ideapad-laptop |   26 +-
 drivers/platform/x86/ideapad-laptop.c         | 1047 +++++++++++------
 2 files changed, 692 insertions(+), 381 deletions(-)

-- 
2.29.2

[PATCH 01/24] platform/x86: ideapad-laptop: remove unnecessary dev_set_drvdata() call

[Barnabás Pőcze]

The driver core already sets the driver specific data on bind failure
or removal. Thus the call is unnecessary.

Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 7598cd46cf60..9ed5bd8d955b 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -1071,7 +1071,6 @@ static int ideapad_acpi_remove(struct platform_device *pdev)
 	ideapad_input_exit(priv);
 	ideapad_debugfs_exit(priv);
 	ideapad_sysfs_exit(priv);
-	dev_set_drvdata(&pdev->dev, NULL);
 
 	return 0;
 }
-- 
2.29.2

[PATCH 02/24] platform/x86: ideapad-laptop: use appropriately typed variable to store the return value of ACPI methods

[Barnabás Pőcze]

Use a variable with type `acpi_status` to store the return value of ACPI
methods instead of a plain `int`. And use ACPI_{SUCCESS,FAILURE} macros
where possible instead of direct comparison.

Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 9ed5bd8d955b..8b86e5547b59 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -971,6 +971,7 @@ static int ideapad_acpi_add(struct platform_device *pdev)
 	int cfg;
 	struct ideapad_private *priv;
 	struct acpi_device *adev;
+	acpi_status acpi_err;
 
 	ret = acpi_bus_get_device(ACPI_HANDLE(&pdev->dev), &adev);
 	if (ret)
@@ -1018,22 +1019,26 @@ static int ideapad_acpi_add(struct platform_device *pdev)
 		if (ret && ret != -ENODEV)
 			goto backlight_failed;
 	}
-	ret = acpi_install_notify_handler(adev->handle,
-		ACPI_DEVICE_NOTIFY, ideapad_acpi_notify, priv);
-	if (ret)
+	acpi_err = acpi_install_notify_handler(adev->handle,
+			ACPI_DEVICE_NOTIFY, ideapad_acpi_notify, priv);
+	if (ACPI_FAILURE(acpi_err)) {
+		ret = -EIO;
 		goto notification_failed;
+	}
 
 #if IS_ENABLED(CONFIG_ACPI_WMI)
 	for (i = 0; i < ARRAY_SIZE(ideapad_wmi_fnesc_events); i++) {
-		ret = wmi_install_notify_handler(ideapad_wmi_fnesc_events[i],
-						 ideapad_wmi_notify, priv);
-		if (ret == AE_OK) {
+		acpi_err = wmi_install_notify_handler(ideapad_wmi_fnesc_events[i],
+						      ideapad_wmi_notify, priv);
+		if (ACPI_SUCCESS(acpi_err)) {
 			priv->fnesc_guid = ideapad_wmi_fnesc_events[i];
 			break;
 		}
 	}
-	if (ret != AE_OK && ret != AE_NOT_EXIST)
+	if (ACPI_FAILURE(acpi_err) && acpi_err != AE_NOT_EXIST) {
+		ret = -EIO;
 		goto notification_failed_wmi;
+	}
 #endif
 
 	return 0;
-- 
2.29.2

[PATCH 03/24] platform/x86: ideapad-laptop: sort includes lexicographically

[Barnabás Pőcze]

Managing includes is easier when they are sorted, so
sort them lexicographically.

Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 8b86e5547b59..aefe83996be6 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -8,23 +8,23 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/init.h>
-#include <linux/types.h>
+#include <acpi/video.h>
 #include <linux/acpi.h>
-#include <linux/rfkill.h>
-#include <linux/platform_device.h>
-#include <linux/input.h>
-#include <linux/input/sparse-keymap.h>
 #include <linux/backlight.h>
-#include <linux/fb.h>
 #include <linux/debugfs.h>
-#include <linux/seq_file.h>
-#include <linux/i8042.h>
-#include <linux/dmi.h>
 #include <linux/device.h>
-#include <acpi/video.h>
+#include <linux/dmi.h>
+#include <linux/fb.h>
+#include <linux/i8042.h>
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/input/sparse-keymap.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/rfkill.h>
+#include <linux/seq_file.h>
+#include <linux/types.h>
 
 #define IDEAPAD_RFKILL_DEV_NUM	(3)
 
-- 
2.29.2

[PATCH 04/24] platform/x86: ideapad-laptop: use sysfs_emit()

[Barnabás Pőcze]

sysfs_emit() has been introduced to make it less ambiguous which function
is preferred when writing to the output buffer in a device attribute's
show() callback. Convert the ideapad-laptop module to utilize this
new helper function.

Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index aefe83996be6..11df791d702c 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -24,6 +24,7 @@
 #include <linux/platform_device.h>
 #include <linux/rfkill.h>
 #include <linux/seq_file.h>
+#include <linux/sysfs.h>
 #include <linux/types.h>
 
 #define IDEAPAD_RFKILL_DEV_NUM	(3)
@@ -344,8 +345,8 @@ static ssize_t show_ideapad_cam(struct device *dev,
 	struct ideapad_private *priv = dev_get_drvdata(dev);
 
 	if (read_ec_data(priv->adev->handle, VPCCMD_R_CAMERA, &result))
-		return sprintf(buf, "-1\n");
-	return sprintf(buf, "%lu\n", result);
+		return sysfs_emit(buf, "-1\n");
+	return sysfs_emit(buf, "%lu\n", result);
 }
 
 static ssize_t store_ideapad_cam(struct device *dev,
@@ -375,8 +376,8 @@ static ssize_t show_ideapad_fan(struct device *dev,
 	struct ideapad_private *priv = dev_get_drvdata(dev);
 
 	if (read_ec_data(priv->adev->handle, VPCCMD_R_FAN, &result))
-		return sprintf(buf, "-1\n");
-	return sprintf(buf, "%lu\n", result);
+		return sysfs_emit(buf, "-1\n");
+	return sysfs_emit(buf, "%lu\n", result);
 }
 
 static ssize_t store_ideapad_fan(struct device *dev,
@@ -408,8 +409,8 @@ static ssize_t touchpad_show(struct device *dev,
 	unsigned long result;
 
 	if (read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &result))
-		return sprintf(buf, "-1\n");
-	return sprintf(buf, "%lu\n", result);
+		return sysfs_emit(buf, "-1\n");
+	return sysfs_emit(buf, "%lu\n", result);
 }
 
 /* Switch to RO for now: It might be revisited in the future */
@@ -441,8 +442,8 @@ static ssize_t conservation_mode_show(struct device *dev,
 	unsigned long result;
 
 	if (method_gbmd(priv->adev->handle, &result))
-		return sprintf(buf, "-1\n");
-	return sprintf(buf, "%u\n", test_bit(BM_CONSERVATION_BIT, &result));
+		return sysfs_emit(buf, "-1\n");
+	return sysfs_emit(buf, "%u\n", test_bit(BM_CONSERVATION_BIT, &result));
 }
 
 static ssize_t conservation_mode_store(struct device *dev,
@@ -477,10 +478,10 @@ static ssize_t fn_lock_show(struct device *dev,
 	int fail = read_method_int(priv->adev->handle, "HALS", &hals);
 
 	if (fail)
-		return sprintf(buf, "-1\n");
+		return sysfs_emit(buf, "-1\n");
 
 	result = hals;
-	return sprintf(buf, "%u\n", test_bit(HA_FNLOCK_BIT, &result));
+	return sysfs_emit(buf, "%u\n", test_bit(HA_FNLOCK_BIT, &result));
 }
 
 static ssize_t fn_lock_store(struct device *dev,
-- 
2.29.2

[PATCH 05/24] platform/x86: ideapad-laptop: use for_each_set_bit() helper to simplify event processing

[Barnabás Pőcze]

The current code used the combination of a for loop + test_bit, which can
be simplified using for_each_set_bit(), so utilize that.

Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 11df791d702c..22e1b3fd3df5 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -11,6 +11,7 @@
 #include <acpi/video.h>
 #include <linux/acpi.h>
 #include <linux/backlight.h>
+#include <linux/bitops.h>
 #include <linux/debugfs.h>
 #include <linux/device.h>
 #include <linux/dmi.h>
@@ -738,22 +739,20 @@ static void ideapad_check_special_buttons(struct ideapad_private *priv)
 
 	read_ec_data(priv->adev->handle, VPCCMD_R_SPECIAL_BUTTONS, &value);
 
-	for (bit = 0; bit < 16; bit++) {
-		if (test_bit(bit, &value)) {
-			switch (bit) {
-			case 0:	/* Z580 */
-			case 6:	/* Z570 */
-				/* Thermal Management button */
-				ideapad_input_report(priv, 65);
-				break;
-			case 1:
-				/* OneKey Theater button */
-				ideapad_input_report(priv, 64);
-				break;
-			default:
-				pr_info("Unknown special button: %lu\n", bit);
-				break;
-			}
+	for_each_set_bit(bit, &value, 16) {
+		switch (bit) {
+		case 0:	/* Z580 */
+		case 6:	/* Z570 */
+			/* Thermal Management button */
+			ideapad_input_report(priv, 65);
+			break;
+		case 1:
+			/* OneKey Theater button */
+			ideapad_input_report(priv, 64);
+			break;
+		default:
+			pr_info("Unknown special button: %lu\n", bit);
+			break;
 		}
 	}
 }
@@ -884,7 +883,7 @@ static void ideapad_sync_touchpad_state(struct ideapad_private *priv)
 static void ideapad_acpi_notify(acpi_handle handle, u32 event, void *data)
 {
 	struct ideapad_private *priv = data;
-	unsigned long vpc1, vpc2, vpc_bit;
+	unsigned long vpc1, vpc2, bit;
 
 	if (read_ec_data(handle, VPCCMD_R_VPC1, &vpc1))
 		return;
@@ -892,44 +891,42 @@ static void ideapad_acpi_notify(acpi_handle handle, u32 event, void *data)
 		return;
 
 	vpc1 = (vpc2 << 8) | vpc1;
-	for (vpc_bit = 0; vpc_bit < 16; vpc_bit++) {
-		if (test_bit(vpc_bit, &vpc1)) {
-			switch (vpc_bit) {
-			case 9:
-				ideapad_sync_rfk_state(priv);
-				break;
-			case 13:
-			case 11:
-			case 8:
-			case 7:
-			case 6:
-				ideapad_input_report(priv, vpc_bit);
-				break;
-			case 5:
-				ideapad_sync_touchpad_state(priv);
-				break;
-			case 4:
-				ideapad_backlight_notify_brightness(priv);
-				break;
-			case 3:
-				ideapad_input_novokey(priv);
-				break;
-			case 2:
-				ideapad_backlight_notify_power(priv);
-				break;
-			case 0:
-				ideapad_check_special_buttons(priv);
-				break;
-			case 1:
-				/* Some IdeaPads report event 1 every ~20
-				 * seconds while on battery power; some
-				 * report this when changing to/from tablet
-				 * mode. Squelch this event.
-				 */
-				break;
-			default:
-				pr_info("Unknown event: %lu\n", vpc_bit);
-			}
+	for_each_set_bit(bit, &vpc1, 16) {
+		switch (bit) {
+		case 9:
+			ideapad_sync_rfk_state(priv);
+			break;
+		case 13:
+		case 11:
+		case 8:
+		case 7:
+		case 6:
+			ideapad_input_report(priv, bit);
+			break;
+		case 5:
+			ideapad_sync_touchpad_state(priv);
+			break;
+		case 4:
+			ideapad_backlight_notify_brightness(priv);
+			break;
+		case 3:
+			ideapad_input_novokey(priv);
+			break;
+		case 2:
+			ideapad_backlight_notify_power(priv);
+			break;
+		case 0:
+			ideapad_check_special_buttons(priv);
+			break;
+		case 1:
+			/* Some IdeaPads report event 1 every ~20
+			 * seconds while on battery power; some
+			 * report this when changing to/from tablet
+			 * mode. Squelch this event.
+			 */
+			break;
+		default:
+			pr_info("Unknown event: %lu\n", bit);
 		}
 	}
 }
-- 
2.29.2

[PATCH 06/24] platform/x86: ideapad-laptop: use msecs_to_jiffies() helper instead of hand-crafted formula

[Barnabás Pőcze]

The current code used a hand-crafted formulate to convert milliseconds to
jiffies, replace it with the msecs_to_jiffies() function.

Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 22e1b3fd3df5..798723c88a68 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -20,6 +20,7 @@
 #include <linux/init.h>
 #include <linux/input.h>
 #include <linux/input/sparse-keymap.h>
+#include <linux/jiffies.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
@@ -188,7 +189,7 @@ static int read_ec_data(acpi_handle handle, int cmd, unsigned long *data)
 	if (method_vpcw(handle, 1, cmd))
 		return -1;
 
-	for (end_jiffies = jiffies+(HZ)*IDEAPAD_EC_TIMEOUT/1000+1;
+	for (end_jiffies = jiffies + msecs_to_jiffies(IDEAPAD_EC_TIMEOUT) + 1;
 	     time_before(jiffies, end_jiffies);) {
 		schedule();
 		if (method_vpcr(handle, 1, &val))
@@ -214,7 +215,7 @@ static int write_ec_cmd(acpi_handle handle, int cmd, unsigned long data)
 	if (method_vpcw(handle, 1, cmd))
 		return -1;
 
-	for (end_jiffies = jiffies+(HZ)*IDEAPAD_EC_TIMEOUT/1000+1;
+	for (end_jiffies = jiffies + msecs_to_jiffies(IDEAPAD_EC_TIMEOUT) + 1;
 	     time_before(jiffies, end_jiffies);) {
 		schedule();
 		if (method_vpcr(handle, 1, &val))
-- 
2.29.2

[PATCH 07/24] platform/x86: ideapad-laptop: use dev_{err,warn} or appropriate variant to display log messages

[Barnabás Pőcze]

Having the device name in the log message makes it easier to determine in
the context of which device the message was printed, so utilize the
appropriate variants of dev_{err,warn,...} when printing log messages.

Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 798723c88a68..d9ac96f6b465 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -201,7 +201,7 @@ static int read_ec_data(acpi_handle handle, int cmd, unsigned long *data)
 			return 0;
 		}
 	}
-	pr_err("timeout in %s\n", __func__);
+	acpi_handle_err(handle, "timeout in %s\n", __func__);
 	return -1;
 }
 
@@ -223,7 +223,7 @@ static int write_ec_cmd(acpi_handle handle, int cmd, unsigned long data)
 		if (val == 0)
 			return 0;
 	}
-	pr_err("timeout in %s\n", __func__);
+	acpi_handle_err(handle, "timeout in %s\n", __func__);
 	return -1;
 }
 
@@ -692,13 +692,15 @@ static int ideapad_input_init(struct ideapad_private *priv)
 
 	error = sparse_keymap_setup(inputdev, ideapad_keymap, NULL);
 	if (error) {
-		pr_err("Unable to setup input device keymap\n");
+		dev_err(&priv->platform_device->dev,
+			"Unable to setup input device keymap\n");
 		goto err_free_dev;
 	}
 
 	error = input_register_device(inputdev);
 	if (error) {
-		pr_err("Unable to register input device\n");
+		dev_err(&priv->platform_device->dev,
+			"Unable to register input device\n");
 		goto err_free_dev;
 	}
 
@@ -752,7 +754,8 @@ static void ideapad_check_special_buttons(struct ideapad_private *priv)
 			ideapad_input_report(priv, 64);
 			break;
 		default:
-			pr_info("Unknown special button: %lu\n", bit);
+			dev_warn(&priv->platform_device->dev,
+				 "Unknown special button: %lu\n", bit);
 			break;
 		}
 	}
@@ -818,7 +821,8 @@ static int ideapad_backlight_init(struct ideapad_private *priv)
 					      &ideapad_backlight_ops,
 					      &props);
 	if (IS_ERR(blightdev)) {
-		pr_err("Could not register backlight device\n");
+		dev_warn(&priv->platform_device->dev,
+			 "Could not register backlight device\n");
 		return PTR_ERR(blightdev);
 	}
 
@@ -927,7 +931,8 @@ static void ideapad_acpi_notify(acpi_handle handle, u32 event, void *data)
 			 */
 			break;
 		default:
-			pr_info("Unknown event: %lu\n", bit);
+			dev_warn(&priv->platform_device->dev,
+				 "Unknown event: %lu\n", bit);
 		}
 	}
 }
@@ -935,12 +940,15 @@ static void ideapad_acpi_notify(acpi_handle handle, u32 event, void *data)
 #if IS_ENABLED(CONFIG_ACPI_WMI)
 static void ideapad_wmi_notify(u32 value, void *context)
 {
+	struct ideapad_private *priv = context;
+
 	switch (value) {
 	case 128:
-		ideapad_input_report(context, value);
+		ideapad_input_report(priv, value);
 		break;
 	default:
-		pr_info("Unknown WMI event %u\n", value);
+		dev_warn(&priv->platform_device->dev,
+			 "Unknown WMI event: %u\n", value);
 	}
 }
 #endif
-- 
2.29.2

[PATCH 08/24] platform/x86: ideapad-laptop: convert ACPI helpers to return -EIO in case of failure

[Barnabás Pőcze]

ACPI helpers returned -1 in case of failure. Convert these functions to
return appropriate error codes, and convert their users to propagate
these error codes accordingly.

Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index d9ac96f6b465..1d43894d557e 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -115,7 +115,7 @@ static int read_method_int(acpi_handle handle, const char *method, int *val)
 	status = acpi_evaluate_integer(handle, (char *)method, NULL, &result);
 	if (ACPI_FAILURE(status)) {
 		*val = -1;
-		return -1;
+		return -EIO;
 	}
 	*val = result;
 	return 0;
@@ -136,7 +136,7 @@ static int method_int1(acpi_handle handle, char *method, int cmd)
 	acpi_status status;
 
 	status = acpi_execute_simple_method(handle, method, cmd);
-	return ACPI_FAILURE(status) ? -1 : 0;
+	return ACPI_FAILURE(status) ? -EIO : 0;
 }
 
 static int method_vpcr(acpi_handle handle, int cmd, int *ret)
@@ -155,7 +155,7 @@ static int method_vpcr(acpi_handle handle, int cmd, int *ret)
 
 	if (ACPI_FAILURE(status)) {
 		*ret = -1;
-		return -1;
+		return -EIO;
 	}
 	*ret = result;
 	return 0;
@@ -177,54 +177,60 @@ static int method_vpcw(acpi_handle handle, int cmd, int data)
 
 	status = acpi_evaluate_object(handle, "VPCW", &params, NULL);
 	if (status != AE_OK)
-		return -1;
+		return -EIO;
 	return 0;
 }
 
 static int read_ec_data(acpi_handle handle, int cmd, unsigned long *data)
 {
-	int val;
+	int val, err;
 	unsigned long int end_jiffies;
 
-	if (method_vpcw(handle, 1, cmd))
-		return -1;
+	err = method_vpcw(handle, 1, cmd);
+	if (err)
+		return err;
 
 	for (end_jiffies = jiffies + msecs_to_jiffies(IDEAPAD_EC_TIMEOUT) + 1;
 	     time_before(jiffies, end_jiffies);) {
 		schedule();
-		if (method_vpcr(handle, 1, &val))
-			return -1;
+		err = method_vpcr(handle, 1, &val);
+		if (err)
+			return err;
 		if (val == 0) {
-			if (method_vpcr(handle, 0, &val))
-				return -1;
+			err = method_vpcr(handle, 0, &val);
+			if (err)
+				return err;
 			*data = val;
 			return 0;
 		}
 	}
 	acpi_handle_err(handle, "timeout in %s\n", __func__);
-	return -1;
+	return -ETIMEDOUT;
 }
 
 static int write_ec_cmd(acpi_handle handle, int cmd, unsigned long data)
 {
-	int val;
+	int val, err;
 	unsigned long int end_jiffies;
 
-	if (method_vpcw(handle, 0, data))
-		return -1;
-	if (method_vpcw(handle, 1, cmd))
-		return -1;
+	err = method_vpcw(handle, 0, data);
+	if (err)
+		return err;
+	err = method_vpcw(handle, 1, cmd);
+	if (err)
+		return err;
 
 	for (end_jiffies = jiffies + msecs_to_jiffies(IDEAPAD_EC_TIMEOUT) + 1;
 	     time_before(jiffies, end_jiffies);) {
 		schedule();
-		if (method_vpcr(handle, 1, &val))
-			return -1;
+		err = method_vpcr(handle, 1, &val);
+		if (err)
+			return err;
 		if (val == 0)
 			return 0;
 	}
 	acpi_handle_err(handle, "timeout in %s\n", __func__);
-	return -1;
+	return -ETIMEDOUT;
 }
 
 /*
@@ -363,8 +369,8 @@ static ssize_t store_ideapad_cam(struct device *dev,
 	if (sscanf(buf, "%i", &state) != 1)
 		return -EINVAL;
 	ret = write_ec_cmd(priv->adev->handle, VPCCMD_W_CAMERA, state);
-	if (ret < 0)
-		return -EIO;
+	if (ret)
+		return ret;
 	return count;
 }
 
@@ -396,8 +402,8 @@ static ssize_t store_ideapad_fan(struct device *dev,
 	if (state < 0 || state > 4 || state == 3)
 		return -EINVAL;
 	ret = write_ec_cmd(priv->adev->handle, VPCCMD_W_FAN, state);
-	if (ret < 0)
-		return -EIO;
+	if (ret)
+		return ret;
 	return count;
 }
 
@@ -429,8 +435,8 @@ static ssize_t __maybe_unused touchpad_store(struct device *dev,
 		return ret;
 
 	ret = write_ec_cmd(priv->adev->handle, VPCCMD_W_TOUCHPAD, state);
-	if (ret < 0)
-		return -EIO;
+	if (ret)
+		return ret;
 	return count;
 }
 
@@ -463,8 +469,8 @@ static ssize_t conservation_mode_store(struct device *dev,
 	ret = method_int1(priv->adev->handle, "SBMC", state ?
 					      BMCMD_CONSERVATION_ON :
 					      BMCMD_CONSERVATION_OFF);
-	if (ret < 0)
-		return -EIO;
+	if (ret)
+		return ret;
 	return count;
 }
 
@@ -501,8 +507,8 @@ static ssize_t fn_lock_store(struct device *dev,
 	ret = method_int1(priv->adev->handle, "SALS", state ?
 			  HACMD_FNLOCK_ON :
 			  HACMD_FNLOCK_OFF);
-	if (ret < 0)
-		return -EIO;
+	if (ret)
+		return ret;
 	return count;
 }
 
@@ -740,7 +746,8 @@ static void ideapad_check_special_buttons(struct ideapad_private *priv)
 {
 	unsigned long bit, value;
 
-	read_ec_data(priv->adev->handle, VPCCMD_R_SPECIAL_BUTTONS, &value);
+	if (read_ec_data(priv->adev->handle, VPCCMD_R_SPECIAL_BUTTONS, &value))
+		return;
 
 	for_each_set_bit(bit, &value, 16) {
 		switch (bit) {
@@ -768,28 +775,33 @@ static int ideapad_backlight_get_brightness(struct backlight_device *blightdev)
 {
 	struct ideapad_private *priv = bl_get_data(blightdev);
 	unsigned long now;
+	int err;
 
 	if (!priv)
 		return -EINVAL;
 
-	if (read_ec_data(priv->adev->handle, VPCCMD_R_BL, &now))
-		return -EIO;
+	err = read_ec_data(priv->adev->handle, VPCCMD_R_BL, &now);
+	if (err)
+		return err;
 	return now;
 }
 
 static int ideapad_backlight_update_status(struct backlight_device *blightdev)
 {
 	struct ideapad_private *priv = bl_get_data(blightdev);
+	int err;
 
 	if (!priv)
 		return -EINVAL;
 
-	if (write_ec_cmd(priv->adev->handle, VPCCMD_W_BL,
-			 blightdev->props.brightness))
-		return -EIO;
-	if (write_ec_cmd(priv->adev->handle, VPCCMD_W_BL_POWER,
-			 blightdev->props.power == FB_BLANK_POWERDOWN ? 0 : 1))
-		return -EIO;
+	err = write_ec_cmd(priv->adev->handle, VPCCMD_W_BL,
+			   blightdev->props.brightness);
+	if (err)
+		return err;
+	err = write_ec_cmd(priv->adev->handle, VPCCMD_W_BL_POWER,
+			   blightdev->props.power != FB_BLANK_POWERDOWN);
+	if (err)
+		return err;
 
 	return 0;
 }
@@ -804,13 +816,17 @@ static int ideapad_backlight_init(struct ideapad_private *priv)
 	struct backlight_device *blightdev;
 	struct backlight_properties props;
 	unsigned long max, now, power;
-
-	if (read_ec_data(priv->adev->handle, VPCCMD_R_BL_MAX, &max))
-		return -EIO;
-	if (read_ec_data(priv->adev->handle, VPCCMD_R_BL, &now))
-		return -EIO;
-	if (read_ec_data(priv->adev->handle, VPCCMD_R_BL_POWER, &power))
-		return -EIO;
+	int err;
+
+	err = read_ec_data(priv->adev->handle, VPCCMD_R_BL_MAX, &max);
+	if (err)
+		return err;
+	err = read_ec_data(priv->adev->handle, VPCCMD_R_BL, &now);
+	if (err)
+		return err;
+	err = read_ec_data(priv->adev->handle, VPCCMD_R_BL_POWER, &power);
+	if (err)
+		return err;
 
 	memset(&props, 0, sizeof(struct backlight_properties));
 	props.max_brightness = max;
-- 
2.29.2

[PATCH 09/24] platform/x86: ideapad-laptop: always propagate error codes from device attributes' show() callback

[Barnabás Pőcze]

Consumers can differentiate an error from a successful read much more
easily if the read() call fails with the appropriate errno instead of
returning a magic string like "-1".

Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 1d43894d557e..b6b5a508a8b8 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -351,9 +351,11 @@ static ssize_t show_ideapad_cam(struct device *dev,
 {
 	unsigned long result;
 	struct ideapad_private *priv = dev_get_drvdata(dev);
+	int err;
 
-	if (read_ec_data(priv->adev->handle, VPCCMD_R_CAMERA, &result))
-		return sysfs_emit(buf, "-1\n");
+	err = read_ec_data(priv->adev->handle, VPCCMD_R_CAMERA, &result);
+	if (err)
+		return err;
 	return sysfs_emit(buf, "%lu\n", result);
 }
 
@@ -382,9 +384,11 @@ static ssize_t show_ideapad_fan(struct device *dev,
 {
 	unsigned long result;
 	struct ideapad_private *priv = dev_get_drvdata(dev);
+	int err;
 
-	if (read_ec_data(priv->adev->handle, VPCCMD_R_FAN, &result))
-		return sysfs_emit(buf, "-1\n");
+	err = read_ec_data(priv->adev->handle, VPCCMD_R_FAN, &result);
+	if (err)
+		return err;
 	return sysfs_emit(buf, "%lu\n", result);
 }
 
@@ -415,9 +419,11 @@ static ssize_t touchpad_show(struct device *dev,
 {
 	struct ideapad_private *priv = dev_get_drvdata(dev);
 	unsigned long result;
+	int err;
 
-	if (read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &result))
-		return sysfs_emit(buf, "-1\n");
+	err = read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &result);
+	if (err)
+		return err;
 	return sysfs_emit(buf, "%lu\n", result);
 }
 
@@ -448,9 +454,11 @@ static ssize_t conservation_mode_show(struct device *dev,
 {
 	struct ideapad_private *priv = dev_get_drvdata(dev);
 	unsigned long result;
+	int err;
 
-	if (method_gbmd(priv->adev->handle, &result))
-		return sysfs_emit(buf, "-1\n");
+	err = method_gbmd(priv->adev->handle, &result);
+	if (err)
+		return err;
 	return sysfs_emit(buf, "%u\n", test_bit(BM_CONSERVATION_BIT, &result));
 }
 
@@ -486,7 +494,7 @@ static ssize_t fn_lock_show(struct device *dev,
 	int fail = read_method_int(priv->adev->handle, "HALS", &hals);
 
 	if (fail)
-		return sysfs_emit(buf, "-1\n");
+		return fail;
 
 	result = hals;
 	return sysfs_emit(buf, "%u\n", test_bit(HA_FNLOCK_BIT, &result));
-- 
2.29.2

[PATCH 10/24] platform/x86: ideapad-laptop: misc. device attribute changes

[Barnabás Pőcze]

Do not handle zero length buffer separately. Use kstrtouint() instead
of sscanf().

Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index b6b5a508a8b8..a615c42d1beb 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -363,13 +363,13 @@ static ssize_t store_ideapad_cam(struct device *dev,
 				 struct device_attribute *attr,
 				 const char *buf, size_t count)
 {
-	int ret, state;
+	int ret;
 	struct ideapad_private *priv = dev_get_drvdata(dev);
+	unsigned int state;
 
-	if (!count)
-		return 0;
-	if (sscanf(buf, "%i", &state) != 1)
-		return -EINVAL;
+	ret = kstrtouint(buf, 0, &state);
+	if (ret)
+		return ret;
 	ret = write_ec_cmd(priv->adev->handle, VPCCMD_W_CAMERA, state);
 	if (ret)
 		return ret;
@@ -396,14 +396,14 @@ static ssize_t store_ideapad_fan(struct device *dev,
 				 struct device_attribute *attr,
 				 const char *buf, size_t count)
 {
-	int ret, state;
+	int ret;
 	struct ideapad_private *priv = dev_get_drvdata(dev);
+	unsigned int state;
 
-	if (!count)
-		return 0;
-	if (sscanf(buf, "%i", &state) != 1)
-		return -EINVAL;
-	if (state < 0 || state > 4 || state == 3)
+	ret = kstrtouint(buf, 0, &state);
+	if (ret)
+		return ret;
+	if (state > 4 || state == 3)
 		return -EINVAL;
 	ret = write_ec_cmd(priv->adev->handle, VPCCMD_W_FAN, state);
 	if (ret)
-- 
2.29.2

[PATCH 11/24] platform/x86: ideapad-laptop: group and separate (un)related constants into enums

[Barnabás Pőcze]

Group and rename constants depending on which ACPI interface they
pertain to, and rename CFG_X constants to CFG_CAP_X.

Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index a615c42d1beb..ffe41bf5585f 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -31,14 +31,6 @@
 
 #define IDEAPAD_RFKILL_DEV_NUM	(3)
 
-#define BM_CONSERVATION_BIT (5)
-#define HA_FNLOCK_BIT       (10)
-
-#define CFG_BT_BIT	(16)
-#define CFG_3G_BIT	(17)
-#define CFG_WIFI_BIT	(18)
-#define CFG_CAMERA_BIT	(19)
-
 #if IS_ENABLED(CONFIG_ACPI_WMI)
 static const char *const ideapad_wmi_fnesc_events[] = {
 	"26CAB2E5-5CF1-46AE-AAC3-4A12B6BA50E6", /* Yoga 3 */
@@ -47,10 +39,28 @@ static const char *const ideapad_wmi_fnesc_events[] = {
 #endif
 
 enum {
-	BMCMD_CONSERVATION_ON = 3,
-	BMCMD_CONSERVATION_OFF = 5,
-	HACMD_FNLOCK_ON = 0xe,
-	HACMD_FNLOCK_OFF = 0xf,
+	CFG_CAP_BT_BIT   = 16,
+	CFG_CAP_3G_BIT   = 17,
+	CFG_CAP_WIFI_BIT = 18,
+	CFG_CAP_CAM_BIT  = 19,
+};
+
+enum {
+	GBMD_CONSERVATION_STATE_BIT = 5,
+};
+
+enum {
+	SMBC_CONSERVATION_ON  = 3,
+	SMBC_CONSERVATION_OFF = 5,
+};
+
+enum {
+	HALS_FNLOCK_STATE_BIT = 10,
+};
+
+enum {
+	SALS_FNLOCK_ON  = 0xe,
+	SALS_FNLOCK_OFF = 0xf,
 };
 
 enum {
@@ -276,7 +286,7 @@ static int debugfs_status_show(struct seq_file *s, void *data)
 
 	if (!method_gbmd(priv->adev->handle, &value)) {
 		seq_printf(s, "Conservation mode:\t%s(%lu)\n",
-			   test_bit(BM_CONSERVATION_BIT, &value) ? "On" : "Off",
+			   test_bit(GBMD_CONSERVATION_STATE_BIT, &value) ? "On" : "Off",
 			   value);
 	}
 
@@ -293,13 +303,13 @@ static int debugfs_cfg_show(struct seq_file *s, void *data)
 	} else {
 		seq_printf(s, "cfg: 0x%.8lX\n\nCapability: ",
 			   priv->cfg);
-		if (test_bit(CFG_BT_BIT, &priv->cfg))
+		if (test_bit(CFG_CAP_BT_BIT, &priv->cfg))
 			seq_printf(s, "Bluetooth ");
-		if (test_bit(CFG_3G_BIT, &priv->cfg))
+		if (test_bit(CFG_CAP_3G_BIT, &priv->cfg))
 			seq_printf(s, "3G ");
-		if (test_bit(CFG_WIFI_BIT, &priv->cfg))
+		if (test_bit(CFG_CAP_WIFI_BIT, &priv->cfg))
 			seq_printf(s, "Wireless ");
-		if (test_bit(CFG_CAMERA_BIT, &priv->cfg))
+		if (test_bit(CFG_CAP_CAM_BIT, &priv->cfg))
 			seq_printf(s, "Camera ");
 		seq_printf(s, "\nGraphic: ");
 		switch ((priv->cfg)&0x700) {
@@ -459,7 +469,7 @@ static ssize_t conservation_mode_show(struct device *dev,
 	err = method_gbmd(priv->adev->handle, &result);
 	if (err)
 		return err;
-	return sysfs_emit(buf, "%u\n", test_bit(BM_CONSERVATION_BIT, &result));
+	return sysfs_emit(buf, "%u\n", test_bit(GBMD_CONSERVATION_STATE_BIT, &result));
 }
 
 static ssize_t conservation_mode_store(struct device *dev,
@@ -475,8 +485,8 @@ static ssize_t conservation_mode_store(struct device *dev,
 		return ret;
 
 	ret = method_int1(priv->adev->handle, "SBMC", state ?
-					      BMCMD_CONSERVATION_ON :
-					      BMCMD_CONSERVATION_OFF);
+					      SMBC_CONSERVATION_ON :
+					      SMBC_CONSERVATION_OFF);
 	if (ret)
 		return ret;
 	return count;
@@ -497,7 +507,7 @@ static ssize_t fn_lock_show(struct device *dev,
 		return fail;
 
 	result = hals;
-	return sysfs_emit(buf, "%u\n", test_bit(HA_FNLOCK_BIT, &result));
+	return sysfs_emit(buf, "%u\n", test_bit(HALS_FNLOCK_STATE_BIT, &result));
 }
 
 static ssize_t fn_lock_store(struct device *dev,
@@ -513,8 +523,8 @@ static ssize_t fn_lock_store(struct device *dev,
 		return ret;
 
 	ret = method_int1(priv->adev->handle, "SALS", state ?
-			  HACMD_FNLOCK_ON :
-			  HACMD_FNLOCK_OFF);
+			  SALS_FNLOCK_ON :
+			  SALS_FNLOCK_OFF);
 	if (ret)
 		return ret;
 	return count;
@@ -541,7 +551,7 @@ static umode_t ideapad_is_visible(struct kobject *kobj,
 	bool supported;
 
 	if (attr == &dev_attr_camera_power.attr)
-		supported = test_bit(CFG_CAMERA_BIT, &(priv->cfg));
+		supported = test_bit(CFG_CAP_CAM_BIT, &priv->cfg);
 	else if (attr == &dev_attr_fan_mode.attr) {
 		unsigned long value;
 		supported = !read_ec_data(priv->adev->handle, VPCCMD_R_FAN,
@@ -574,9 +584,9 @@ struct ideapad_rfk_data {
 };
 
 static const struct ideapad_rfk_data ideapad_rfk_data[] = {
-	{ "ideapad_wlan",    CFG_WIFI_BIT, VPCCMD_W_WIFI, RFKILL_TYPE_WLAN },
-	{ "ideapad_bluetooth", CFG_BT_BIT, VPCCMD_W_BT, RFKILL_TYPE_BLUETOOTH },
-	{ "ideapad_3g",        CFG_3G_BIT, VPCCMD_W_3G, RFKILL_TYPE_WWAN },
+	{ "ideapad_wlan",      CFG_CAP_WIFI_BIT, VPCCMD_W_WIFI, RFKILL_TYPE_WLAN },
+	{ "ideapad_bluetooth", CFG_CAP_BT_BIT,   VPCCMD_W_BT,   RFKILL_TYPE_BLUETOOTH },
+	{ "ideapad_3g",        CFG_CAP_3G_BIT,   VPCCMD_W_3G,   RFKILL_TYPE_WWAN },
 };
 
 static int ideapad_rfk_set(void *data, bool blocked)
-- 
2.29.2

[PATCH 12/24] platform/x86: ideapad-laptop: rework and create new ACPI helpers

[Barnabás Pőcze]

Create dedicated helper functions for accessing the main ACPI methods:
GBMD, SMBC, HALS, SALS; and utilize them. Use `unsigned long` consistently
in every ACPI helper wherever possible. Change names to better express
purpose. Do not assign values to output parameters in case of failure.

Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index ffe41bf5585f..795978e0d13e 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -117,41 +117,47 @@ MODULE_PARM_DESC(no_bt_rfkill, "No rfkill for bluetooth.");
  */
 #define IDEAPAD_EC_TIMEOUT (200) /* in ms */
 
-static int read_method_int(acpi_handle handle, const char *method, int *val)
+static int eval_int(acpi_handle handle, const char *method, unsigned long *val)
 {
-	acpi_status status;
+	acpi_status acpi_err;
 	unsigned long long result;
 
-	status = acpi_evaluate_integer(handle, (char *)method, NULL, &result);
-	if (ACPI_FAILURE(status)) {
-		*val = -1;
+	acpi_err = acpi_evaluate_integer(handle, (char *)method, NULL, &result);
+	if (ACPI_FAILURE(acpi_err))
 		return -EIO;
-	}
 	*val = result;
 	return 0;
+}
 
+static int eval_simple_method(acpi_handle handle, char *method, u64 arg)
+{
+	acpi_status acpi_err = acpi_execute_simple_method(handle, method, arg);
+	return ACPI_FAILURE(acpi_err) ? -EIO : 0;
 }
 
-static int method_gbmd(acpi_handle handle, unsigned long *ret)
+static int eval_gbmd(acpi_handle handle, unsigned long *val)
 {
-	int result, val;
+	return eval_int(handle, "GBMD", val);
+}
 
-	result = read_method_int(handle, "GBMD", &val);
-	*ret = val;
-	return result;
+static int eval_smbc(acpi_handle handle, unsigned long arg)
+{
+	return eval_simple_method(handle, "SMBC", arg);
 }
 
-static int method_int1(acpi_handle handle, char *method, int cmd)
+static int eval_hals(acpi_handle handle, unsigned long *val)
 {
-	acpi_status status;
+	return eval_int(handle, "HALS", val);
+}
 
-	status = acpi_execute_simple_method(handle, method, cmd);
-	return ACPI_FAILURE(status) ? -EIO : 0;
+static int eval_sals(acpi_handle handle, unsigned long arg)
+{
+	return eval_simple_method(handle, "SALS", arg);
 }
 
-static int method_vpcr(acpi_handle handle, int cmd, int *ret)
+static int eval_vpcr(acpi_handle handle, unsigned long cmd, unsigned long *val)
 {
-	acpi_status status;
+	acpi_status acpi_err;
 	unsigned long long result;
 	struct acpi_object_list params;
 	union acpi_object in_obj;
@@ -161,22 +167,20 @@ static int method_vpcr(acpi_handle handle, int cmd, int *ret)
 	in_obj.type = ACPI_TYPE_INTEGER;
 	in_obj.integer.value = cmd;
 
-	status = acpi_evaluate_integer(handle, "VPCR", &params, &result);
+	acpi_err = acpi_evaluate_integer(handle, "VPCR", &params, &result);
 
-	if (ACPI_FAILURE(status)) {
-		*ret = -1;
+	if (ACPI_FAILURE(acpi_err))
 		return -EIO;
-	}
-	*ret = result;
+	*val = result;
 	return 0;
 
 }
 
-static int method_vpcw(acpi_handle handle, int cmd, int data)
+static int eval_vpcw(acpi_handle handle, unsigned long cmd, unsigned long data)
 {
 	struct acpi_object_list params;
 	union acpi_object in_obj[2];
-	acpi_status status;
+	acpi_status acpi_err;
 
 	params.count = 2;
 	params.pointer = in_obj;
@@ -185,55 +189,50 @@ static int method_vpcw(acpi_handle handle, int cmd, int data)
 	in_obj[1].type = ACPI_TYPE_INTEGER;
 	in_obj[1].integer.value = data;
 
-	status = acpi_evaluate_object(handle, "VPCW", &params, NULL);
-	if (status != AE_OK)
+	acpi_err = acpi_evaluate_object(handle, "VPCW", &params, NULL);
+	if (ACPI_FAILURE(acpi_err))
 		return -EIO;
 	return 0;
 }
 
-static int read_ec_data(acpi_handle handle, int cmd, unsigned long *data)
+static int read_ec_data(acpi_handle handle, unsigned long cmd, unsigned long *data)
 {
-	int val, err;
-	unsigned long int end_jiffies;
+	int err;
+	unsigned long int end_jiffies, val;
 
-	err = method_vpcw(handle, 1, cmd);
+	err = eval_vpcw(handle, 1, cmd);
 	if (err)
 		return err;
 
 	for (end_jiffies = jiffies + msecs_to_jiffies(IDEAPAD_EC_TIMEOUT) + 1;
 	     time_before(jiffies, end_jiffies);) {
 		schedule();
-		err = method_vpcr(handle, 1, &val);
+		err = eval_vpcr(handle, 1, &val);
 		if (err)
 			return err;
-		if (val == 0) {
-			err = method_vpcr(handle, 0, &val);
-			if (err)
-				return err;
-			*data = val;
-			return 0;
-		}
+		if (val == 0)
+			return eval_vpcr(handle, 0, data);
 	}
 	acpi_handle_err(handle, "timeout in %s\n", __func__);
 	return -ETIMEDOUT;
 }
 
-static int write_ec_cmd(acpi_handle handle, int cmd, unsigned long data)
+static int write_ec_cmd(acpi_handle handle, unsigned long cmd, unsigned long data)
 {
-	int val, err;
-	unsigned long int end_jiffies;
+	int err;
+	unsigned long end_jiffies, val;
 
-	err = method_vpcw(handle, 0, data);
+	err = eval_vpcw(handle, 0, data);
 	if (err)
 		return err;
-	err = method_vpcw(handle, 1, cmd);
+	err = eval_vpcw(handle, 1, cmd);
 	if (err)
 		return err;
 
 	for (end_jiffies = jiffies + msecs_to_jiffies(IDEAPAD_EC_TIMEOUT) + 1;
 	     time_before(jiffies, end_jiffies);) {
 		schedule();
-		err = method_vpcr(handle, 1, &val);
+		err = eval_vpcr(handle, 1, &val);
 		if (err)
 			return err;
 		if (val == 0)
@@ -284,7 +283,7 @@ static int debugfs_status_show(struct seq_file *s, void *data)
 			   value ? "On" : "Off", value);
 	seq_puts(s, "=====================\n");
 
-	if (!method_gbmd(priv->adev->handle, &value)) {
+	if (!eval_gbmd(priv->adev->handle, &value)) {
 		seq_printf(s, "Conservation mode:\t%s(%lu)\n",
 			   test_bit(GBMD_CONSERVATION_STATE_BIT, &value) ? "On" : "Off",
 			   value);
@@ -466,7 +465,7 @@ static ssize_t conservation_mode_show(struct device *dev,
 	unsigned long result;
 	int err;
 
-	err = method_gbmd(priv->adev->handle, &result);
+	err = eval_gbmd(priv->adev->handle, &result);
 	if (err)
 		return err;
 	return sysfs_emit(buf, "%u\n", test_bit(GBMD_CONSERVATION_STATE_BIT, &result));
@@ -484,9 +483,8 @@ static ssize_t conservation_mode_store(struct device *dev,
 	if (ret)
 		return ret;
 
-	ret = method_int1(priv->adev->handle, "SBMC", state ?
-					      SMBC_CONSERVATION_ON :
-					      SMBC_CONSERVATION_OFF);
+	ret = eval_smbc(priv->adev->handle,
+			state ? SMBC_CONSERVATION_ON : SMBC_CONSERVATION_OFF);
 	if (ret)
 		return ret;
 	return count;
@@ -499,15 +497,13 @@ static ssize_t fn_lock_show(struct device *dev,
 			    char *buf)
 {
 	struct ideapad_private *priv = dev_get_drvdata(dev);
-	unsigned long result;
-	int hals;
-	int fail = read_method_int(priv->adev->handle, "HALS", &hals);
+	unsigned long hals;
+	int fail = eval_hals(priv->adev->handle, &hals);
 
 	if (fail)
 		return fail;
 
-	result = hals;
-	return sysfs_emit(buf, "%u\n", test_bit(HALS_FNLOCK_STATE_BIT, &result));
+	return sysfs_emit(buf, "%u\n", test_bit(HALS_FNLOCK_STATE_BIT, &hals));
 }
 
 static ssize_t fn_lock_store(struct device *dev,
@@ -522,9 +518,8 @@ static ssize_t fn_lock_store(struct device *dev,
 	if (ret)
 		return ret;
 
-	ret = method_int1(priv->adev->handle, "SALS", state ?
-			  SALS_FNLOCK_ON :
-			  SALS_FNLOCK_OFF);
+	ret = eval_sals(priv->adev->handle,
+			state ? SALS_FNLOCK_ON : SALS_FNLOCK_OFF);
 	if (ret)
 		return ret;
 	return count;
@@ -1009,7 +1004,7 @@ static const struct dmi_system_id hw_rfkill_list[] = {
 static int ideapad_acpi_add(struct platform_device *pdev)
 {
 	int ret, i;
-	int cfg;
+	unsigned long cfg;
 	struct ideapad_private *priv;
 	struct acpi_device *adev;
 	acpi_status acpi_err;
@@ -1018,7 +1013,7 @@ static int ideapad_acpi_add(struct platform_device *pdev)
 	if (ret)
 		return -ENODEV;
 
-	if (read_method_int(adev->handle, "_CFG", &cfg))
+	if (eval_int(adev->handle, "_CFG", &cfg))
 		return -ENODEV;
 
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
-- 
2.29.2

Re: [PATCH 00/24] platform/x86: ideapad-laptop: cleanup, keyboard backlight and "always on USB charging" control support, reenable touchpad control

[Barnabás Pőcze]

2020. december 16., szerda 2:39 keltezéssel, Barnabás Pőcze írta:

> This series contains patches that aim to bring more consistency
> to the code; add keyboard backlight control support; add
> "always on USB charging" control support.
> Furthermore, commit 7f363145992cebf4ea760447f1cfdf6f81459683 is reverted
> since it made it impossible to disable/enable the touchpad via the
> ideapad-laptop module and on some devices the method implemented in the
> module works correctly to disable/enable the touchpad.
>
> Barnabás Pőcze (24):
> platform/x86: ideapad-laptop: remove unnecessary dev_set_drvdata()
> call
> platform/x86: ideapad-laptop: use appropriately typed variable to
> store the return value of ACPI methods
> platform/x86: ideapad-laptop: sort includes lexicographically
> platform/x86: ideapad-laptop: use sysfs_emit()
> platform/x86: ideapad-laptop: use for_each_set_bit() helper to
> simplify event processing
> platform/x86: ideapad-laptop: use msecs_to_jiffies() helper instead of
> hand-crafted formula
> platform/x86: ideapad-laptop: use dev_{err,warn} or appropriate
> variant to display log messages
> platform/x86: ideapad-laptop: convert ACPI helpers to return -EIO in
> case of failure
> platform/x86: ideapad-laptop: always propagate error codes from device
> attributes' show() callback
> platform/x86: ideapad-laptop: misc. device attribute changes
> platform/x86: ideapad-laptop: group and separate (un)related constants
> into enums
> platform/x86: ideapad-laptop: rework and create new ACPI helpers
> platform/x86: ideapad-laptop: rework is_visible() logic
> platform/x86: ideapad-laptop: check for Fn-lock support in HALS
> platform/x86: ideapad-laptop: check for touchpad support in _CFG
> platform/x86: ideapad-laptop: change 'status' debugfs file format
> platform/x86: ideapad-laptop: change 'cfg' debugfs file format
> Revert "platform/x86: ideapad-laptop: Switch touchpad attribute to be
> RO"
> platform/x86: ideapad-laptop: fix checkpatch warnings, more consistent
> style
> platform/x86: ideapad-laptop: send notification about touchpad state
> change to sysfs
> platform/x86: ideapad-laptop: add keyboard backlight control support
> platform/x86: ideapad-laptop: add "always on USB charging" control
> support
> Documentation/ABI: sysfs-platform-ideapad-laptop: update device
> attribute paths
> Documentation/ABI: sysfs-platform-ideapad-laptop: conservation_mode
> and usb_charging
>
> .../ABI/testing/sysfs-platform-ideapad-laptop | 26 +-
> drivers/platform/x86/ideapad-laptop.c | 1047 +++++++++++------
> 2 files changed, 692 insertions(+), 381 deletions(-)
>
> ---
>
> 2.29.2

A patch in the series a serious flaw which I have just noticed;
I will send a new version when the situation about a conflicting
patch[1] becomes clear.

[1]: https://www.spinics.net/lists/platform-driver-x86/msg24007.html

Re: [PATCH 00/24] platform/x86: ideapad-laptop: cleanup, keyboard backlight and "always on USB charging" control support, reenable touchpad control

[Hans de Goede]

Hi Barnabás,

On 1/4/21 1:03 PM, Barnabás Pőcze wrote:
> 2020. december 16., szerda 2:39 keltezéssel, Barnabás Pőcze írta:
> 
>> This series contains patches that aim to bring more consistency
>> to the code; add keyboard backlight control support; add
>> "always on USB charging" control support.
>> Furthermore, commit 7f363145992cebf4ea760447f1cfdf6f81459683 is reverted
>> since it made it impossible to disable/enable the touchpad via the
>> ideapad-laptop module and on some devices the method implemented in the
>> module works correctly to disable/enable the touchpad.
>>
>> Barnabás Pőcze (24):
>> platform/x86: ideapad-laptop: remove unnecessary dev_set_drvdata()
>> call
>> platform/x86: ideapad-laptop: use appropriately typed variable to
>> store the return value of ACPI methods
>> platform/x86: ideapad-laptop: sort includes lexicographically
>> platform/x86: ideapad-laptop: use sysfs_emit()
>> platform/x86: ideapad-laptop: use for_each_set_bit() helper to
>> simplify event processing
>> platform/x86: ideapad-laptop: use msecs_to_jiffies() helper instead of
>> hand-crafted formula
>> platform/x86: ideapad-laptop: use dev_{err,warn} or appropriate
>> variant to display log messages
>> platform/x86: ideapad-laptop: convert ACPI helpers to return -EIO in
>> case of failure
>> platform/x86: ideapad-laptop: always propagate error codes from device
>> attributes' show() callback
>> platform/x86: ideapad-laptop: misc. device attribute changes
>> platform/x86: ideapad-laptop: group and separate (un)related constants
>> into enums
>> platform/x86: ideapad-laptop: rework and create new ACPI helpers
>> platform/x86: ideapad-laptop: rework is_visible() logic
>> platform/x86: ideapad-laptop: check for Fn-lock support in HALS
>> platform/x86: ideapad-laptop: check for touchpad support in _CFG
>> platform/x86: ideapad-laptop: change 'status' debugfs file format
>> platform/x86: ideapad-laptop: change 'cfg' debugfs file format
>> Revert "platform/x86: ideapad-laptop: Switch touchpad attribute to be
>> RO"
>> platform/x86: ideapad-laptop: fix checkpatch warnings, more consistent
>> style
>> platform/x86: ideapad-laptop: send notification about touchpad state
>> change to sysfs
>> platform/x86: ideapad-laptop: add keyboard backlight control support
>> platform/x86: ideapad-laptop: add "always on USB charging" control
>> support
>> Documentation/ABI: sysfs-platform-ideapad-laptop: update device
>> attribute paths
>> Documentation/ABI: sysfs-platform-ideapad-laptop: conservation_mode
>> and usb_charging
>>
>> .../ABI/testing/sysfs-platform-ideapad-laptop | 26 +-
>> drivers/platform/x86/ideapad-laptop.c | 1047 +++++++++++------
>> 2 files changed, 692 insertions(+), 381 deletions(-)
>>
>> ---
>>
>> 2.29.2
> 
> A patch in the series a serious flaw which I have just noticed;
> I will send a new version when the situation about a conflicting
> patch[1] becomes clear.
> 
> [1]: https://www.spinics.net/lists/platform-driver-x86/msg24007.html

Thank you for the headsup, I will try to make some time to review v1
of this series, so that you can take any review-remarks which I might
have into account when posting v2 (or add my reviewed-by for patches
for which I have no remark).

May I ask which patch is flawed and what the flaw is ? Then I can skip
that while reviewing. I hope to get around to reviewing v1 (this version)
of this series this Wednesday (but no promises).

Regards,

Hans

Re: [PATCH 00/24] platform/x86: ideapad-laptop: cleanup, keyboard backlight and "always on USB charging" control support, reenable touchpad control

[Barnabás Pőcze]

2021. január 4., hétfő 15:03 keltezéssel, Hans de Goede írta:

> Hi Barnabás,
>
> On 1/4/21 1:03 PM, Barnabás Pőcze wrote:
>
> > 2020.  december 16., szerda 2:39 keltezéssel, Barnabás Pőcze írta:
> >
> > > This series contains patches that aim to bring more consistency
> > > to the code; add keyboard backlight control support; add
> > > "always on USB charging" control support.
> > > Furthermore, commit 7f363145992cebf4ea760447f1cfdf6f81459683 is reverted
> > > since it made it impossible to disable/enable the touchpad via the
> > > ideapad-laptop module and on some devices the method implemented in the
> > > module works correctly to disable/enable the touchpad.
> > > Barnabás Pőcze (24):
> > > platform/x86: ideapad-laptop: remove unnecessary dev_set_drvdata()
> > > call
> > > platform/x86: ideapad-laptop: use appropriately typed variable to
> > > store the return value of ACPI methods
> > > platform/x86: ideapad-laptop: sort includes lexicographically
> > > platform/x86: ideapad-laptop: use sysfs_emit()
> > > platform/x86: ideapad-laptop: use for_each_set_bit() helper to
> > > simplify event processing
> > > platform/x86: ideapad-laptop: use msecs_to_jiffies() helper instead of
> > > hand-crafted formula
> > > platform/x86: ideapad-laptop: use dev_{err,warn} or appropriate
> > > variant to display log messages
> > > platform/x86: ideapad-laptop: convert ACPI helpers to return -EIO in
> > > case of failure
> > > platform/x86: ideapad-laptop: always propagate error codes from device
> > > attributes' show() callback
> > > platform/x86: ideapad-laptop: misc. device attribute changes
> > > platform/x86: ideapad-laptop: group and separate (un)related constants
> > > into enums
> > > platform/x86: ideapad-laptop: rework and create new ACPI helpers
> > > platform/x86: ideapad-laptop: rework is_visible() logic
> > > platform/x86: ideapad-laptop: check for Fn-lock support in HALS
> > > platform/x86: ideapad-laptop: check for touchpad support in _CFG
> > > platform/x86: ideapad-laptop: change 'status' debugfs file format
> > > platform/x86: ideapad-laptop: change 'cfg' debugfs file format
> > > Revert "platform/x86: ideapad-laptop: Switch touchpad attribute to be
> > > RO"
> > > platform/x86: ideapad-laptop: fix checkpatch warnings, more consistent
> > > style
> > > platform/x86: ideapad-laptop: send notification about touchpad state
> > > change to sysfs
> > > platform/x86: ideapad-laptop: add keyboard backlight control support
> > > platform/x86: ideapad-laptop: add "always on USB charging" control
> > > support
> > > Documentation/ABI: sysfs-platform-ideapad-laptop: update device
> > > attribute paths
> > > Documentation/ABI: sysfs-platform-ideapad-laptop: conservation_mode
> > > and usb_charging
> > > .../ABI/testing/sysfs-platform-ideapad-laptop | 26 +-
> > > drivers/platform/x86/ideapad-laptop.c | 1047 +++++++++++------
> > > 2 files changed, 692 insertions(+), 381 deletions(-)
> > >
> > > 2.29.2
> >
> > A patch in the series a serious flaw which I have just noticed;
> > I will send a new version when the situation about a conflicting
> > patch1 becomes clear.
>
> Thank you for the headsup, I will try to make some time to review v1
> of this series, so that you can take any review-remarks which I might
> have into account when posting v2 (or add my reviewed-by for patches
> for which I have no remark).
>
> May I ask which patch is flawed and what the flaw is ? Then I can skip
> that while reviewing. I hope to get around to reviewing v1 (this version)
> of this series this Wednesday (but no promises).
>

Hi


In "platform/x86: ideapad-laptop: check for touchpad support in _CFG", the bit
is off by one. It should be 30, not 31.


Regards,
Barnabás Pőcze

Re: [PATCH 00/24] platform/x86: ideapad-laptop: cleanup, keyboard backlight and "always on USB charging" control support, reenable touchpad control

[Hans de Goede]

Hi,

On 12/16/20 2:39 AM, Barnabás Pőcze wrote:
> This series contains patches that aim to bring more consistency
> to the code; add keyboard backlight control support; add
> "always on USB charging" control support.
> Furthermore, commit 7f363145992cebf4ea760447f1cfdf6f81459683 is reverted
> since it made it impossible to disable/enable the touchpad via the
> ideapad-laptop module and on some devices the method implemented in the
> module works correctly to disable/enable the touchpad.

Thank you for this series, it is good to see all the
cleanups, as well as to see the new functionality.

Patches 1-20 and 22-24 look good to me and you may add my:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

To them for v2 of this patch-set.

I have some remarks about patch 21 I will reply to that one
separately.

One minor remark about patch 3/24, normally we put all
the #include <linux/foo.h> includes first (sorted
alphabetically as you already do in the patch) and then
follow up by other / subsys specific include such as
acpi/video.h. Again sorted alphabetically for the file-names
after the subsys dir. I don't think there really is any
preferred order for which subsys headers to include first,
but typically the generic linux/foo.h headers are included
first.

Regards,

Hans

p.s.

About merging this series vs other outstanding ideapad-laptop
changes. The other outstanding changes are quite small, so easy
to rebase. As such I would actually prefer to merge this series
first. So if you can send out a v2 soon-ish, then that would be
great.



> 
> Barnabás Pőcze (24):
>   platform/x86: ideapad-laptop: remove unnecessary dev_set_drvdata()
>     call
>   platform/x86: ideapad-laptop: use appropriately typed variable to
>     store the return value of ACPI methods
>   platform/x86: ideapad-laptop: sort includes lexicographically
>   platform/x86: ideapad-laptop: use sysfs_emit()
>   platform/x86: ideapad-laptop: use for_each_set_bit() helper to
>     simplify event processing
>   platform/x86: ideapad-laptop: use msecs_to_jiffies() helper instead of
>     hand-crafted formula
>   platform/x86: ideapad-laptop: use dev_{err,warn} or appropriate
>     variant to display log messages
>   platform/x86: ideapad-laptop: convert ACPI helpers to return -EIO in
>     case of failure
>   platform/x86: ideapad-laptop: always propagate error codes from device
>     attributes' show() callback
>   platform/x86: ideapad-laptop: misc. device attribute changes
>   platform/x86: ideapad-laptop: group and separate (un)related constants
>     into enums
>   platform/x86: ideapad-laptop: rework and create new ACPI helpers
>   platform/x86: ideapad-laptop: rework is_visible() logic
>   platform/x86: ideapad-laptop: check for Fn-lock support in HALS
>   platform/x86: ideapad-laptop: check for touchpad support in _CFG
>   platform/x86: ideapad-laptop: change 'status' debugfs file format
>   platform/x86: ideapad-laptop: change 'cfg' debugfs file format
>   Revert "platform/x86: ideapad-laptop: Switch touchpad attribute to be
>     RO"
>   platform/x86: ideapad-laptop: fix checkpatch warnings, more consistent
>     style
>   platform/x86: ideapad-laptop: send notification about touchpad state
>     change to sysfs
>   platform/x86: ideapad-laptop: add keyboard backlight control support
>   platform/x86: ideapad-laptop: add "always on USB charging" control
>     support
>   Documentation/ABI: sysfs-platform-ideapad-laptop: update device
>     attribute paths
>   Documentation/ABI: sysfs-platform-ideapad-laptop: conservation_mode
>     and usb_charging
> 
>  .../ABI/testing/sysfs-platform-ideapad-laptop |   26 +-
>  drivers/platform/x86/ideapad-laptop.c         | 1047 +++++++++++------
>  2 files changed, 692 insertions(+), 381 deletions(-)
> 

Re: [PATCH 00/24] platform/x86: ideapad-laptop: cleanup, keyboard backlight and "always on USB charging" control support, reenable touchpad control

[Barnabás Pőcze]

Hi


2021. január 6., szerda 19:23 keltezéssel, Hans de Goede írta:

> [...]
> >  #include <acpi/video.h>
> > +#include <dt-bindings/leds/common.h>
> >  #include <linux/acpi.h>
> >  #include <linux/backlight.h>
> >  #include <linux/bitops.h>
> > @@ -22,6 +23,7 @@
> >  #include <linux/input/sparse-keymap.h>
> >  #include <linux/jiffies.h>
> >  #include <linux/kernel.h>
> > +#include <linux/leds.h>
> >  #include <linux/module.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/rfkill.h>
>
> I guess you need a "depends on LEDS_CLASS" in the Kconfig for this?
>

You're right, I was under the impression for some reason that leds.h defines
stub functions when LED support is disabled, it turns out that's not the case
(at least not for all functions). Interestingly, some other drivers "select"
LEDS_CLASS (e.g. thinkpad_acpi), others "depend" on it (e.g. alienware-wmi)...
I'm unsure about what should be done here.


> [...]
> > +static void ideapad_kbd_bl_notify(struct ideapad_private *priv)
> > +{
> > +	int brightness;
> > +
> > +	if (!priv->kbd_bl.initialized)
> > +		return;
> > +
> > +	brightness = ideapad_kbd_bl_brightness_get(priv);
> > +	if (brightness < 0)
> > +		return;
> > +
> > +	if (brightness == priv->kbd_bl.last_brightness)
> > +		return;
> > +
> > +	priv->kbd_bl.last_brightness = brightness;
> > +
> > +	led_classdev_notify_brightness_hw_changed(&priv->kbd_bl.led, brightness);
> > +}
>
> So I guess that there is some hotkey combo on the laptops keyboards which
> directly changes the state of the kbd backlight "underneath" us and that
> is why this is necessary ?
>

Yes, more specifically, Fn + space on the machine I tested it.


> > +
> > +static int ideapad_kbd_bl_init(struct ideapad_private *priv)
> > +{
> > +	unsigned long hals;
> > +	int err;
> > +
> > +	err = eval_hals(priv->adev->handle, &hals);
> > +	if (err)
> > +		return err;
>
> So you are checking for presence of the HALS method here, but not
> for SALS which is being used in ideapad_kbd_bl_led_cdev_brightness_set()
> and you are needlessly re-evaluating HALS here. Would it not be better
> to add a features.kbd_bl flag and set that from ideapad_check_features()
> as done for other features ? That would avoid an extra evaluation of
> the HALS method and also check for SALS.
> [...]

You're right, it would be better.


Regards,
Barnabás Pőcze

Re: [PATCH 00/24] platform/x86: ideapad-laptop: cleanup, keyboard backlight and "always on USB charging" control support, reenable touchpad control

[Barnabás Pőcze]

2021. január 6., szerda 19:23 keltezéssel, Hans de Goede írta:

> [...]
> Thank you for this series, it is good to see all the
> cleanups, as well as to see the new functionality.
>
> Patches 1-20 and 22-24 look good to me and you may add my:
>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
>
> To them for v2 of this patch-set.
>

Thanks for the review.


> I have some remarks about patch 21 I will reply to that one
> separately.
>
> One minor remark about patch 3/24, normally we put all
> the #include <linux/foo.h> includes first (sorted
> alphabetically as you already do in the patch) and then
> follow up by other / subsys specific include such as
> acpi/video.h. Again sorted alphabetically for the file-names
> after the subsys dir. I don't think there really is any
> preferred order for which subsys headers to include first,
> but typically the generic linux/foo.h headers are included
> first.
>

I will change the order as requested.


> Regards,
>
> Hans
>
> p.s.
>
> About merging this series vs other outstanding ideapad-laptop
> changes. The other outstanding changes are quite small, so easy
> to rebase. As such I would actually prefer to merge this series
> first. So if you can send out a v2 soon-ish, then that would be
> great.

That would make it harder to backport, no? As far as I remember, the patch[1] was
sent to stable@kernel.org as well.

[1]: https://lore.kernel.org/platform-driver-x86/20210103033651.47580-1-jiaxun.yang@flygoat.com/#t


Regards,
Barnabás Pőcze


p.s.
My previous reply to this thread was meant to be a reply to
https://lore.kernel.org/platform-driver-x86/770007e6-a06f-eb0a-112c-17e2eb396ae5@redhat.com/

Re: [PATCH 00/24] platform/x86: ideapad-laptop: cleanup, keyboard backlight and "always on USB charging" control support, reenable touchpad control

[Hans de Goede]

Hi,

On 1/6/21 9:49 PM, Barnabás Pőcze wrote:
> 2021. január 6., szerda 19:23 keltezéssel, Hans de Goede írta:
> 
>> [...]
>> Thank you for this series, it is good to see all the
>> cleanups, as well as to see the new functionality.
>>
>> Patches 1-20 and 22-24 look good to me and you may add my:
>>
>> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
>>
>> To them for v2 of this patch-set.
>>
> 
> Thanks for the review.
> 
> 
>> I have some remarks about patch 21 I will reply to that one
>> separately.
>>
>> One minor remark about patch 3/24, normally we put all
>> the #include <linux/foo.h> includes first (sorted
>> alphabetically as you already do in the patch) and then
>> follow up by other / subsys specific include such as
>> acpi/video.h. Again sorted alphabetically for the file-names
>> after the subsys dir. I don't think there really is any
>> preferred order for which subsys headers to include first,
>> but typically the generic linux/foo.h headers are included
>> first.
>>
> 
> I will change the order as requested.
> 
> 
>> Regards,
>>
>> Hans
>>
>> p.s.
>>
>> About merging this series vs other outstanding ideapad-laptop
>> changes. The other outstanding changes are quite small, so easy
>> to rebase. As such I would actually prefer to merge this series
>> first. So if you can send out a v2 soon-ish, then that would be
>> great.
> 
> That would make it harder to backport, no? As far as I remember, the patch[1] was
> sent to stable@kernel.org as well.
> 
> [1]: https://lore.kernel.org/platform-driver-x86/20210103033651.47580-1-jiaxun.yang@flygoat.com/#t

That is true. So lets get that patch merged first, and then you can base your v2 on top of that.

Jiaxun, can you send out a v4 of your "[PATCH fixes v3] platform/x86: ideapad-laptop: Disable touchpad_switch for ELAN0634"
patch addressing my review remark on v3 ?

Regards,

Hans