linux-kernel - [PATCH] HID: i2c-hid: Revert to using power commands to wake on resume

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <20241119235615.23902-1-kl@kl.wtf>
Date: Wed, 20 Nov 2024 00:53:17 +0100
From: Kenny Levinsen <kl@...wtf>
To: Jiri Kosina <jikos@...nel.org>,
	Benjamin Tissoires <benjamin.tissoires@...hat.com>
Cc: linux-input@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Michael <auslands-kv@....de>,
	Linux regressions mailing list <regressions@...ts.linux.dev>,
	Kenny Levinsen <kl@...wtf>,
	stable@...r.kernel.org
Subject: [PATCH] HID: i2c-hid: Revert to using power commands to wake on resume

7d6f065de37c ("HID: i2c-hid: Use address probe to wake on resume")
replaced the retry of power commands with the dummy read "bus probe" we
use on boot which accounts for a necessary delay before retry.

This made at least one Weida device (2575:0910 in an ASUS Vivobook S14)
very unhappy, as the bus probe despite being successful somehow lead to
the following power command failing so hard that the device never lets
go of the bus. This means that even retries of the power command would
fail on a timeout as the bus remains busy.

Remove the bus probe on resume and instead reintroduce retry of the
power command for wake-up purposes while respecting the newly
established wake-up retry timings.

Fixes: 7d6f065de37c ("HID: i2c-hid: Use address probe to wake on resume")
Cc: stable@...r.kernel.org
Reported-by: Michael <auslands-kv@....de>
Link: https://bugzilla.kernel.org/show_bug.cgi?id=219440
Link: https://lore.kernel.org/r/d5acb485-7377-4139-826d-4df04d21b5ed@leemhuis.info/
Signed-off-by: Kenny Levinsen <kl@...wtf>
---

As I don't have access to the hardware in question, a test by the
reporter (Michael) would be preferred to confirm the final patch.

 drivers/hid/i2c-hid/i2c-hid-core.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 43664a24176f..4e87380d3edd 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -414,7 +414,19 @@ static int i2c_hid_set_power(struct i2c_hid *ihid, int power_state)
 
 	i2c_hid_dbg(ihid, "%s\n", __func__);
 
+	/*
+	 * Some STM-based devices need 400µs after a rising clock edge to wake
+	 * from deep sleep, in which case the first request will fail due to
+	 * the address not being acknowledged. Try after a short sleep to see
+	 * if the device came alive on the bus. Certain Weida Tech devices also
+	 * need this.
+	 */
 	ret = i2c_hid_set_power_command(ihid, power_state);
+	if (ret && power_state == I2C_HID_PWR_ON) {
+		usleep_range(400, 500);
+		ret = i2c_hid_set_power_command(ihid, I2C_HID_PWR_ON);
+	}
+
 	if (ret)
 		dev_err(&ihid->client->dev,
 			"failed to change power setting.\n");
@@ -976,14 +988,6 @@ static int i2c_hid_core_resume(struct i2c_hid *ihid)
 
 	enable_irq(client->irq);
 
-	/* Make sure the device is awake on the bus */
-	ret = i2c_hid_probe_address(ihid);
-	if (ret < 0) {
-		dev_err(&client->dev, "nothing at address after resume: %d\n",
-			ret);
-		return -ENXIO;
-	}
-
 	/* On Goodix 27c6:0d42 wait extra time before device wakeup.
 	 * It's not clear why but if we send wakeup too early, the device will
 	 * never trigger input interrupts.
-- 
2.47.0