IICHID: Reset responses are only two bytes
ClosedPublic
Actions

Authored by phk on Tue, Nov 18, 1:34 PM.

Details

Reviewers

wulf
emaste

Commits

rGd02a08f89932: iichid: The IICHID spec defines the response to the RESET command as two bytes…
rGd61be9d99470: iichid: The IICHID spec defines the response to the RESET command as two bytes…
rG5d53462af1d0: iichid: The IICHID spec defines the response to the RESET command as two bytes…

Summary

The IICHID spec defines the response to the RESET command as two bytes of zeros.

Our recent changes to iichid.c has caused us to attempt to read a full REPORT instead, and at least one keyboard hangs solid when we do that.

This patch changes us to be spec-compliant.

Test Plan

Tested on:

ELAN0676:00 04F3:3195 I2C HID device -- Worked fine without this patch.  (T14s GEN 2)
ELAN06F1:03 04F3:000D I2C HID device -- Needs this patch to work (T14s Snapdragon CPU)

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

phk created this revision.Tue, Nov 18, 1:34 PM

Herald added a subscriber: imp. · View Herald TranscriptTue, Nov 18, 1:34 PM

phk requested review of this revision.Tue, Nov 18, 1:34 PM

Can you upload the patch with full context (i.e. git diff -U9999999 or git show -U99999999)

In D53803#1228714, @emaste wrote:

Can you upload the patch with full context (i.e. git diff -U9999999 or git show -U99999999)

If this is a requirement, the UI should enforce it.

It's in the committer's guide and happens automatically if use git arc which is the recommended way to create reviews.

Anyhow I've merged the change into my WIP tree and will follow up with the results on my laptop

Another T14s, tested running:
FreeBSD g1-129.catwhisker.org 16.0-CURRENT FreeBSD 16.0-CURRENT #12 main-n281983-8012c61bef3b: Tue Nov 18 13:35:18 UTC 2025 root@g1-129.catwhisker.org:/usr/obj/usr/src/amd64.amd64/sys/GENERIC amd64 1600004 1600004

Seems OK both with & without the patch.

My 13th gen Framework 13 laptop works ok with or without this patch.

cy added a subscriber: cy.Tue, Nov 18, 2:39 PM

In D53803#1228714, @emaste wrote:

Can you upload the patch with full context (i.e. git diff -U9999999 or git show -U99999999)

Not specific to phabricator, git show output fails to apply using git apply whereas git diff output does. For phabricator, des@ has convinced me to use git arc instead.

That effectively reverts commit 548d3aa856a9 (https://github.com/freebsd/freebsd-src/commit/548d3aa856a97f4483554beceeb57fa9ba0ff913) and thus will broke some rare devices.

IMO, we should add new quirk to sys/dev/hid/hidquirk.c to limit RESET response read length and activate it for ELAN06F1

I've tested this patch on a Framework 13 and an HP 840 G5. Mind you, I had no problems with either prior to the patch.

That effectively reverts commit 548d3aa856a9

On the other side. if first short I2C read does not acknowledge interrupt, it will immediately result in full-sized second one read which should do the trick.

Unfortunately, I do not have access to such a hardware to test.

This revision is now accepted and ready to land.Tue, Nov 18, 3:57 PM

Works on my Framework 13" 11th gen Intel

libinput list-devices shoes:

Device:                  FRMW0001:01 32AC:0006 Consumer Control
Kernel:                  /dev/input/event9
Id:                      i2c:32ac:0006

Worked before this patch and continues to work now. Regular keyboard key events are on /dev/input/event3 which is AT keyboard, special function keys (e.g. brightness) come on /dev/input/event9.