linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Jiri Kosina <jikos@kernel.org>,
	Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: Hans de Goede <hdegoede@redhat.com>,
	Douglas Anderson <dianders@chromium.org>,
	Julian Sax <jsbc@gmx.de>,
	ahormann@gmx.net, Bruno Jesus <bruno.fl.jesus@gmail.com>,
	Dietrich <enaut.w@googlemail.com>,
	kloxdami@yahoo.com, Tim Aldridge <taldridge@mac.com>,
	Rene Wagner <redhatbugzilla@callerid.de>,
	Federico Ricchiuto <fed.ricchiuto@gmail.com>,
	linux-input@vger.kernel.org
Subject: [PATCH 5/7] HID: i2c-hid: Remove I2C_HID_QUIRK_NO_IRQ_AFTER_RESET quirks
Date: Sat,  4 Nov 2023 12:17:41 +0100	[thread overview]
Message-ID: <20231104111743.14668-6-hdegoede@redhat.com> (raw)
In-Reply-To: <20231104111743.14668-1-hdegoede@redhat.com>

Now that i2c-hid-core waits for the reset after reading the HID report
descriptor, the I2C_HID_QUIRK_NO_IRQ_AFTER_RESET quirks appear to
no longer be necessary (confirmed on one of the quirked models).

Except on laptops where the HID report descriptor is missing and is
provided by a DMI quirk instead. On these models the HID report descriptor
is not read at all, so moving the wait for reset is a no-op. These laptops
have 0911:5288 touchpads for which I2C_HID_QUIRK_NO_IRQ_AFTER_RESET was
set, keep the existing behavior there.

There might still be devices which really do not ack the reset
at all, so to avoid this causing regressions also:

1. Change the wait for ack timeout from an error into a warning; and
2. Lower reset timeout from the very long 5s to 1s which should be plenty

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/hid/i2c-hid/i2c-hid-core.c | 29 +++++++++--------------------
 1 file changed, 9 insertions(+), 20 deletions(-)

diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 3bd0c3d77d99..df5577fc73c5 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -45,7 +45,6 @@
 
 /* quirks to control the device */
 #define I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV	BIT(0)
-#define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET	BIT(1)
 #define I2C_HID_QUIRK_BOGUS_IRQ			BIT(4)
 #define I2C_HID_QUIRK_RESET_ON_RESUME		BIT(5)
 #define I2C_HID_QUIRK_BAD_INPUT_SIZE		BIT(6)
@@ -122,12 +121,6 @@ static const struct i2c_hid_quirks {
 } i2c_hid_quirks[] = {
 	{ USB_VENDOR_ID_WEIDA, HID_ANY_ID,
 		I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV },
-	{ I2C_VENDOR_ID_HANTICK, I2C_PRODUCT_ID_HANTICK_5288,
-		I2C_HID_QUIRK_NO_IRQ_AFTER_RESET },
-	{ I2C_VENDOR_ID_ITE, I2C_DEVICE_ID_ITE_VOYO_WINPAD_A15,
-		I2C_HID_QUIRK_NO_IRQ_AFTER_RESET },
-	{ I2C_VENDOR_ID_RAYDIUM, I2C_PRODUCT_ID_RAYDIUM_3118,
-		I2C_HID_QUIRK_NO_IRQ_AFTER_RESET },
 	{ USB_VENDOR_ID_ALPS_JP, HID_ANY_ID,
 		 I2C_HID_QUIRK_RESET_ON_RESUME },
 	{ I2C_VENDOR_ID_SYNAPTICS, I2C_PRODUCT_ID_SYNAPTICS_SYNA2393,
@@ -470,11 +463,11 @@ static int i2c_hid_start_hwreset(struct i2c_hid *ihid)
 	return ret;
 }
 
-static int i2c_hid_finish_hwreset(struct i2c_hid *ihid)
+static int i2c_hid_finish_hwreset(struct i2c_hid *ihid, bool no_irq_after_reset)
 {
 	int ret = 0;
 
-	if (ihid->quirks & I2C_HID_QUIRK_NO_IRQ_AFTER_RESET) {
+	if (no_irq_after_reset) {
 		msleep(100);
 		clear_bit(I2C_HID_RESET_PENDING, &ihid->flags);
 	}
@@ -482,9 +475,9 @@ static int i2c_hid_finish_hwreset(struct i2c_hid *ihid)
 	i2c_hid_dbg(ihid, "%s: waiting...\n", __func__);
 	if (!wait_event_timeout(ihid->wait,
 				!test_bit(I2C_HID_RESET_PENDING, &ihid->flags),
-				msecs_to_jiffies(5000))) {
-		ret = -ENODATA;
-		goto err_clear_reset;
+				msecs_to_jiffies(1000))) {
+		dev_warn(&ihid->client->dev, "device did not ack reset within 1000 ms\n");
+		clear_bit(I2C_HID_RESET_PENDING, &ihid->flags);
 	}
 	i2c_hid_dbg(ihid, "%s: finished.\n", __func__);
 
@@ -494,12 +487,6 @@ static int i2c_hid_finish_hwreset(struct i2c_hid *ihid)
 
 	mutex_unlock(&ihid->reset_lock);
 	return ret;
-
-err_clear_reset:
-	clear_bit(I2C_HID_RESET_PENDING, &ihid->flags);
-	i2c_hid_set_power(ihid, I2C_HID_PWR_SLEEP);
-	mutex_unlock(&ihid->reset_lock);
-	return ret;
 }
 
 static void i2c_hid_abort_hwreset(struct i2c_hid *ihid)
@@ -736,6 +723,7 @@ static int i2c_hid_parse(struct hid_device *hid)
 	struct i2c_client *client = hid->driver_data;
 	struct i2c_hid *ihid = i2c_get_clientdata(client);
 	struct i2c_hid_desc *hdesc = &ihid->hdesc;
+	bool no_irq_after_reset = false;
 	unsigned int rsize;
 	char *rdesc;
 	int ret;
@@ -764,6 +752,7 @@ static int i2c_hid_parse(struct hid_device *hid)
 
 	if (use_override) {
 		rdesc = use_override;
+		no_irq_after_reset = true;
 		i2c_hid_dbg(ihid, "Using a HID report descriptor override\n");
 	} else {
 		rdesc = kzalloc(rsize, GFP_KERNEL);
@@ -790,7 +779,7 @@ static int i2c_hid_parse(struct hid_device *hid)
 	 * actually wait for the report-descriptor to be read before signalling
 	 * reset completion.
 	 */
-	ret = i2c_hid_finish_hwreset(ihid);
+	ret = i2c_hid_finish_hwreset(ihid, no_irq_after_reset);
 	if (ret)
 		goto out;
 
@@ -1005,7 +994,7 @@ static int i2c_hid_core_resume(struct i2c_hid *ihid)
 	if (ihid->quirks & I2C_HID_QUIRK_RESET_ON_RESUME) {
 		ret = i2c_hid_start_hwreset(ihid);
 		if (ret == 0)
-			ret = i2c_hid_finish_hwreset(ihid);
+			ret = i2c_hid_finish_hwreset(ihid, false);
 	} else {
 		ret = i2c_hid_set_power(ihid, I2C_HID_PWR_ON);
 	}
-- 
2.41.0


  parent reply	other threads:[~2023-11-04 11:19 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-04 11:17 [PATCH 0/7] HID: i2c-hid: Rework wait for reset to match Windows Hans de Goede
2023-11-04 11:17 ` [PATCH 1/7] HID: i2c-hid: Fold i2c_hid_execute_reset() into i2c_hid_hwreset() Hans de Goede
2023-11-06 18:50   ` Doug Anderson
2023-11-17 19:34     ` Hans de Goede
2023-11-04 11:17 ` [PATCH 2/7] HID: i2c-hid: Split i2c_hid_hwreset() in start() and finish() functions Hans de Goede
2023-11-06 18:53   ` Doug Anderson
2023-11-17 19:37     ` Hans de Goede
2023-11-04 11:17 ` [PATCH 3/7] HID: i2c-hid: Switch i2c_hid_parse() to goto style error handling Hans de Goede
2023-11-06 18:53   ` Doug Anderson
2023-11-04 11:17 ` [PATCH 4/7] HID: i2c-hid: Move i2c_hid_finish_hwreset() to after reading the report-descriptor Hans de Goede
2023-11-06 18:53   ` Doug Anderson
2023-11-17 19:42     ` Hans de Goede
2023-11-04 11:17 ` Hans de Goede [this message]
2023-11-06 18:54   ` [PATCH 5/7] HID: i2c-hid: Remove I2C_HID_QUIRK_NO_IRQ_AFTER_RESET quirks Doug Anderson
2023-11-04 11:17 ` [PATCH 6/7] HID: i2c-hid: Remove I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV quirk Hans de Goede
2023-11-06 18:55   ` Doug Anderson
2023-11-04 11:17 ` [PATCH 7/7] HID: i2c-hid: Renumber I2C_HID_QUIRK_ defines Hans de Goede
2023-11-06 18:55   ` Doug Anderson
2023-11-08 20:41 ` [PATCH 0/7] HID: i2c-hid: Rework wait for reset to match Windows Julian Sax
2023-11-16 16:46   ` Hans de Goede

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231104111743.14668-6-hdegoede@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=ahormann@gmx.net \
    --cc=benjamin.tissoires@redhat.com \
    --cc=bruno.fl.jesus@gmail.com \
    --cc=dianders@chromium.org \
    --cc=enaut.w@googlemail.com \
    --cc=fed.ricchiuto@gmail.com \
    --cc=jikos@kernel.org \
    --cc=jsbc@gmx.de \
    --cc=kloxdami@yahoo.com \
    --cc=linux-input@vger.kernel.org \
    --cc=redhatbugzilla@callerid.de \
    --cc=taldridge@mac.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).