mbox series

[0/7] HID: i2c-hid: Rework wait for reset to match Windows

Message ID 20231104111743.14668-1-hdegoede@redhat.com (mailing list archive)
Headers show
Series HID: i2c-hid: Rework wait for reset to match Windows | expand

Message

Hans de Goede Nov. 4, 2023, 11:17 a.m. UTC
Hi All,

Looking at:

"2247751 - Touchpad not working on Lenovo Thinkbook 15 Gen 5"
https://bugzilla.redhat.com/show_bug.cgi?id=2247751

I thought at first that this was another touchpad needing
a I2C_HID_QUIRK_NO_IRQ_AFTER_RESET quirk, further testing
has shown this seems to be an IRQ not working issue though,
so this series does not help for that bug.

The bug has made me revisit I2C_HID_QUIRK_NO_IRQ_AFTER_RESET though and it
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 about 1 ms after the report descriptor read finishes the reset
indeed does get acked.

So this series refactors the reset handling to move the waiting for
the ack to after reading the report-descriptor, so that
the I2C_HID_QUIRK_NO_IRQ_AFTER_RESET quirk is no longer necessary.

Julian, I've added you to the Cc because you developed the code
for touchpads where the HID report descriptors are missing and are
provided by the kernel through a DMI quirk. The behavior for these
touchpads should be unchanged, but if you can test on a laptop
with such a touchpad that would be great.

Regards,

Hans


Hans de Goede (7):
  HID: i2c-hid: Fold i2c_hid_execute_reset() into i2c_hid_hwreset()
  HID: i2c-hid: Split i2c_hid_hwreset() in start() and finish()
    functions
  HID: i2c-hid: Switch i2c_hid_parse() to goto style error handling
  HID: i2c-hid: Move i2c_hid_finish_hwreset() to after reading the
    report-descriptor
  HID: i2c-hid: Remove I2C_HID_QUIRK_NO_IRQ_AFTER_RESET quirks
  HID: i2c-hid: Remove I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV quirk
  HID: i2c-hid: Renumber I2C_HID_QUIRK_ defines

 drivers/hid/i2c-hid/i2c-hid-core.c | 155 +++++++++++++++--------------
 1 file changed, 79 insertions(+), 76 deletions(-)

Comments

Julian Sax Nov. 8, 2023, 8:41 p.m. UTC | #1
Am 04.11.23 um 12:17 schrieb Hans de Goede:
> Julian, I've added you to the Cc because you developed the code
> for touchpads where the HID report descriptors are missing and are
> provided by the kernel through a DMI quirk. The behavior for these
> touchpads should be unchanged, but if you can test on a laptop
> with such a touchpad that would be great.
>

Hi Hans,

I just tested the touchpad with these patches, everything still works fine.


Regards,

Julian
Hans de Goede Nov. 16, 2023, 4:46 p.m. UTC | #2
Hi,

On 11/8/23 21:41, Julian Sax wrote:
> 
> Am 04.11.23 um 12:17 schrieb Hans de Goede:
>> Julian, I've added you to the Cc because you developed the code
>> for touchpads where the HID report descriptors are missing and are
>> provided by the kernel through a DMI quirk. The behavior for these
>> touchpads should be unchanged, but if you can test on a laptop
>> with such a touchpad that would be great.
>>
> 
> Hi Hans,
> 
> I just tested the touchpad with these patches, everything still works fine.

That is good to hear, thank you for testing.

Regards,

Hans