From patchwork Mon Nov 20 19:33:10 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 745536 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="Ivy+jPoe" Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F00BDA0 for ; Mon, 20 Nov 2023 11:33:36 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1700508816; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=rWC7UT1Zzcw/Y2k1xOhXr9syyucSNGdXDxAuei1Stls=; b=Ivy+jPoe78dY/mpSiM0zqkIFh5+8xJprtmNtLD3JVXuSe9kiulK+h+TPogIzQ47AOHsTM2 HDuMpJSpa+aefFe7C8dtzXyh2uGbHYf02VTEtYbtW96uZOezPhHeLaEJHsa55FCyWzdxVN wSoWLb+fmF+vRl7egvrlJS1nvFFLNhY= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-616-Z_u3IeoCPVSa3JBplv7_CA-1; Mon, 20 Nov 2023 14:33:33 -0500 X-MC-Unique: Z_u3IeoCPVSa3JBplv7_CA-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 42A9985A58C; Mon, 20 Nov 2023 19:33:32 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.193.19]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8E80E2166B26; Mon, 20 Nov 2023 19:33:30 +0000 (UTC) From: Hans de Goede To: Jiri Kosina , Benjamin Tissoires Cc: Hans de Goede , Douglas Anderson , Julian Sax , ahormann@gmx.net, Bruno Jesus , Dietrich , kloxdami@yahoo.com, Tim Aldridge , Rene Wagner , Federico Ricchiuto , linux-input@vger.kernel.org Subject: [RFC v2 4/7] HID: i2c-hid: Move i2c_hid_finish_hwreset() to after reading the report-descriptor Date: Mon, 20 Nov 2023 20:33:10 +0100 Message-ID: <20231120193313.666912-5-hdegoede@redhat.com> In-Reply-To: <20231120193313.666912-1-hdegoede@redhat.com> References: <20231120193313.666912-1-hdegoede@redhat.com> Precedence: bulk X-Mailing-List: linux-input@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.6 A recent bug made me look at Microsoft's i2c-hid docs again and I noticed the following: """ 4. Issue a RESET (Host Initiated Reset) to the Device. 5. Retrieve report descriptor from the device. Note: Steps 4 and 5 may be done in parallel to optimize for time on I²C. Since report descriptors are (a) static and (b) quite long, Windows 8 may issue a request for 5 while it is waiting for a response from the device on 4. """ Which made me think that maybe on some touchpads the reset ack is delayed till after the report descriptor is read ? Testing a T-BAO Tbook Air 12.5 with a 0911:5288 (SIPODEV SP1064?) touchpad, for which the I2C_HID_QUIRK_NO_IRQ_AFTER_RESET quirk was first introduced, shows that reading the report descriptor before waiting for the reset helps with the missing reset IRQ. Now the reset does get acked properly, but the ack sometimes still does not happen unfortunately. Still moving the wait for ack to after reading the report-descriptor, is probably a good idea, both to make i2c-hid's behavior closer to Windows as well as to speed up probing i2c-hid devices. While at it drop the dbg_hid() for a malloc failure, malloc failures already get logged extensively by malloc itself. Link: https://bugzilla.redhat.com/show_bug.cgi?id=2247751 Signed-off-by: Hans de Goede --- Changes in v2: - Adjust commit message to note that moving the wait-for-reset to after reading thr report-descriptor only partially fixes the missing reset IRQ problem - Adjust for the reset_lock now being taken in the callers of i2c_hid_start_hwreset() / i2c_hid_finish_hwreset() --- drivers/hid/i2c-hid/i2c-hid-core.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c index 5e535480fed7..48582c75a51b 100644 --- a/drivers/hid/i2c-hid/i2c-hid-core.c +++ b/drivers/hid/i2c-hid/i2c-hid-core.c @@ -741,12 +741,9 @@ static int i2c_hid_parse(struct hid_device *hid) return -EINVAL; } + mutex_lock(&ihid->reset_lock); do { - mutex_lock(&ihid->reset_lock); ret = i2c_hid_start_hwreset(ihid); - if (ret == 0) - ret = i2c_hid_finish_hwreset(ihid); - mutex_unlock(&ihid->reset_lock); if (ret) msleep(1000); } while (tries-- > 0 && ret); @@ -764,8 +761,8 @@ static int i2c_hid_parse(struct hid_device *hid) rdesc = kzalloc(rsize, GFP_KERNEL); if (!rdesc) { - dbg_hid("couldn't allocate rdesc memory\n"); - return -ENOMEM; + ret = -ENOMEM; + goto abort_reset; } i2c_hid_dbg(ihid, "asking HID report descriptor\n"); @@ -775,10 +772,23 @@ static int i2c_hid_parse(struct hid_device *hid) rdesc, rsize); if (ret) { hid_err(hid, "reading report descriptor failed\n"); - goto out; + goto abort_reset; } } + /* + * Windows directly reads the report-descriptor after sending reset + * and then waits for resets completion afterwards. Some touchpads + * actually wait for the report-descriptor to be read before signalling + * reset completion. + */ + ret = i2c_hid_finish_hwreset(ihid); +abort_reset: + clear_bit(I2C_HID_RESET_PENDING, &ihid->flags); + mutex_unlock(&ihid->reset_lock); + if (ret) + goto out; + i2c_hid_dbg(ihid, "Report Descriptor: %*ph\n", rsize, rdesc); ret = hid_parse_report(hid, rdesc, rsize); From patchwork Mon Nov 20 19:33:11 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 745535 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="QxOGKxwX" Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4DBC8A0 for ; Mon, 20 Nov 2023 11:33:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1700508819; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=hRd8YKtJ3xndnfbZqjT2r7/Sdl/4CADPH8N4wuA68KE=; b=QxOGKxwXqO+WlEcP3T1jSFT+y7eOeDkvgphrYAZpqtFzzbkCT/AGnLdwQ0N5qHRo1a3k2j r0K4I7k6/wREvRAxcYCNzTpQ58dteCYWdJ0mpzcdCrA88xoWwCdLuAzYEYm+jjxAIoYDmp 17j3cQWKr7q2Gf9ZkJ0iItRolx3MUtk= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-622-JQMnrB8cNK-UzKFBYtDG6A-1; Mon, 20 Nov 2023 14:33:35 -0500 X-MC-Unique: JQMnrB8cNK-UzKFBYtDG6A-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 2B24385A58C; Mon, 20 Nov 2023 19:33:34 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.193.19]) by smtp.corp.redhat.com (Postfix) with ESMTP id 73EC52166B26; Mon, 20 Nov 2023 19:33:32 +0000 (UTC) From: Hans de Goede To: Jiri Kosina , Benjamin Tissoires Cc: Hans de Goede , Douglas Anderson , Julian Sax , ahormann@gmx.net, Bruno Jesus , Dietrich , kloxdami@yahoo.com, Tim Aldridge , Rene Wagner , Federico Ricchiuto , linux-input@vger.kernel.org Subject: [RFC v2 5/7] HID: i2c-hid: Turn missing reset ack into a warning Date: Mon, 20 Nov 2023 20:33:11 +0100 Message-ID: <20231120193313.666912-6-hdegoede@redhat.com> In-Reply-To: <20231120193313.666912-1-hdegoede@redhat.com> References: <20231120193313.666912-1-hdegoede@redhat.com> Precedence: bulk X-Mailing-List: linux-input@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.6 On all i2c-hid devices seen sofar the reset-ack either works, or the hw is somehow buggy and does not (always) ack the reset properly, yet it still works fine. Lower the very long reset timeout to 1 second which should be plenty and change the reset not getting acked from an error into a warning. This results in a bit cleaner code and avoids the need to add more I2C_HID_QUIRK_NO_IRQ_AFTER_RESET quirks in the future. Signed-off-by: Hans de Goede --- drivers/hid/i2c-hid/i2c-hid-core.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c index 48582c75a51b..96fb5aafc1fa 100644 --- a/drivers/hid/i2c-hid/i2c-hid-core.c +++ b/drivers/hid/i2c-hid/i2c-hid-core.c @@ -481,9 +481,9 @@ static int i2c_hid_finish_hwreset(struct i2c_hid *ihid) 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__); @@ -492,11 +492,6 @@ static int i2c_hid_finish_hwreset(struct i2c_hid *ihid) ret = i2c_hid_set_power(ihid, I2C_HID_PWR_ON); return ret; - -err_clear_reset: - clear_bit(I2C_HID_RESET_PENDING, &ihid->flags); - i2c_hid_set_power(ihid, I2C_HID_PWR_SLEEP); - return ret; } static void i2c_hid_get_input(struct i2c_hid *ihid) From patchwork Mon Nov 20 19:33:13 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 745534 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="jEEFWC8P" Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B6B68B9 for ; Mon, 20 Nov 2023 11:33:45 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1700508825; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=z3iOt6TF4m21HqgKip0VNs6/jW+2VNd+2q+MwQfbRg0=; b=jEEFWC8PaX4FfA+0ptzxCRsBkWCe98ArMSrdID01FpOXoYl/GeWQ9ahdtoip+aeXgWFq7e XU/64XE0xD3y6Fa7yOf2Spr82ww6XcpqqL/pD+uywTPsrj63IlZhAqwoOmIGomFBtthyw6 +qhTvwxC0vplTP6Dz8U/KNwwtv6pS58= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-144-nwnsRGyiOdWAYz0przMNAQ-1; Mon, 20 Nov 2023 14:33:38 -0500 X-MC-Unique: nwnsRGyiOdWAYz0przMNAQ-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 26DE43804524; Mon, 20 Nov 2023 19:33:38 +0000 (UTC) Received: from localhost.localdomain (unknown [10.39.193.19]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3F3132166B26; Mon, 20 Nov 2023 19:33:36 +0000 (UTC) From: Hans de Goede To: Jiri Kosina , Benjamin Tissoires Cc: Hans de Goede , Douglas Anderson , Julian Sax , ahormann@gmx.net, Bruno Jesus , Dietrich , kloxdami@yahoo.com, Tim Aldridge , Rene Wagner , Federico Ricchiuto , linux-input@vger.kernel.org Subject: [RFC v2 7/7] HID: i2c-hid: Renumber I2C_HID_QUIRK_ defines Date: Mon, 20 Nov 2023 20:33:13 +0100 Message-ID: <20231120193313.666912-8-hdegoede@redhat.com> In-Reply-To: <20231120193313.666912-1-hdegoede@redhat.com> References: <20231120193313.666912-1-hdegoede@redhat.com> Precedence: bulk X-Mailing-List: linux-input@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.6 The quirks variable and the I2C_HID_QUIRK_ defines are never used / exported outside of the i2c-hid code renumber them to start at BIT(0) again. Reviewed-by: Douglas Anderson Signed-off-by: Hans de Goede --- drivers/hid/i2c-hid/i2c-hid-core.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c index 3bdfd3e89de5..151d5a5c87ca 100644 --- a/drivers/hid/i2c-hid/i2c-hid-core.c +++ b/drivers/hid/i2c-hid/i2c-hid-core.c @@ -44,11 +44,11 @@ #include "i2c-hid.h" /* quirks to control the device */ -#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) -#define I2C_HID_QUIRK_NO_WAKEUP_AFTER_RESET BIT(7) +#define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET BIT(0) +#define I2C_HID_QUIRK_BOGUS_IRQ BIT(1) +#define I2C_HID_QUIRK_RESET_ON_RESUME BIT(2) +#define I2C_HID_QUIRK_BAD_INPUT_SIZE BIT(3) +#define I2C_HID_QUIRK_NO_WAKEUP_AFTER_RESET BIT(4) /* Command opcodes */ #define I2C_HID_OPCODE_RESET 0x01