Download raw body.
imt(4)/umt(4): finding HID report IDs
On 7/16/25 00:07, Mark Kettenis wrote:
>> Date: Tue, 15 Jul 2025 23:08:29 +0200
>> From: Ulf Brosziewski <ulf.brosziewski@t-online.de>
>>
>> Anybody?
>
> The same comment about "top-level usage" not being a thing in the spec
> and being inconsistent with the existing comments applies here as well.
>
> Otherwise this makes (some) sense to me.
In hid_find_report(), "top-level usage" is the more accurate term because
it refers to collections without a parent collection in the descriptor.
However, this should be an application collection ("Each top level
collection must be an application collection", hid1_11, p. 67). I could
rename "top_usage" to "app_usage" and add the check for the collection
type (h.collection == HCOLL_APPLICATION).
Is something else missing? More tests would be nice, of course, but it's
summertime. I had checked a userland version of hid_find_report() with a
set of report descriptors grabbed from the web.
Skipping unknown collections is a recommendation from the specification:
"If the Usage attached to a collection is unknown to an application,
then the application should ignore the collection and all Usages
contained in the collection. A collection can be used to modify the
meaning of the Usages that it contains [...]" (hud1_6, p. 20)
>
>> On 7/2/25 01:37, Ulf Brosziewski wrote:
>>> The way imt(4) and umt(4) search for report IDs is broken. Depending
>>> on the structure and distribution of IDs in a report descriptor, it might
>>> succeed, or fail with wrong ID values (as has been reported by Thomas
>>> Dettbarn, see https://marc.info/?l=openbsd-tech&m=174841257723328&w=2 ).
>>>
>>> The hid_*_collection functions only provide partial information, which,
>>> in isolation, may be useless if you aren't lucky and deal with simple,
>>> uniform descriptors.
>>>
>>> The patch adds another helper function to hid.c, which permits searching
>>> for several report items at once, and filters out "unknown" collections
>>> (see the comment in the diff). It doesn't put further constraints on the
>>> search and the structure of the descriptor, but the heuristic might be
>>> safe enough for most practical purposes. However, my experience with the
>>> real HID world is limited, comments and hints would be appreciated.
>>>
>>> The patch moves the search for "PTP" reports to hidmt, and adapts imt
>>> and umt accordingly. Please note that it also contains a fix for another
>>> bug in hidmt (see https://marc.info/?l=openbsd-tech&m=175101436027656&w=2 ).
>>> It may be hidden by the faulty report search; that's why for tests, it
>>> makes sense to combine the patches.
>>>
>>> Tests, comments, and OKs would be welcome.
>>>
>>>
>>> Index: dev/hid/hid.c
>>> ===================================================================
>>> RCS file: /cvs/src/sys/dev/hid/hid.c,v
>>> retrieving revision 1.7
>>> diff -u -p -r1.7 hid.c
>>> --- dev/hid/hid.c 10 May 2024 10:49:10 -0000 1.7
>>> +++ dev/hid/hid.c 1 Jul 2025 20:11:57 -0000
>>> @@ -706,3 +706,70 @@ hid_get_id_of_collection(const void *des
>>> hid_end_parse(hd);
>>> return -1;
>>> }
>>> +
>>> +/*
>>> + * Find the first report that contains each of the given "usages"
>>> + * and belongs to a top-level collection with the 'top_usage' type.
>>> + * The size of the 'usages' array must be in the range [1..32].
>>> + *
>>> + * If 'coll_usages' is NULL, the search will skip collections with
>>> + * usages from vendor pages (0xFF00 - 0xFFFF).
>>> + *
>>> + * If 'coll_usages' is non-NULL, it must point to a 0-terminated
>>> + * sequence of collection usages, and the search will skip collections
>>> + * with usages not present in this set. (It isn't necessary to include
>>> + * the top-level usage here).
>>> + *
>>> + * Return Values:
>>> + * -1: No match
>>> + * 0: Success (single report without an ID)
>>> + * [1..255]: Report ID
>>> + */
>>> +int
>>> +hid_find_report(const void *desc, int len, enum hid_kind kind,
>>> + int32_t top_usage, int n_usages, int32_t *usages, int32_t *coll_usages)
>>> +{
>>> + struct hid_data *hd;
>>> + struct hid_item h;
>>> + uint32_t matches;
>>> + int i, cur_id, skip;
>>> +
>>> + hd = hid_start_parse(desc, len, hid_all);
>>> + for (cur_id = -1, skip = 0; hid_get_item(hd, &h); ) {
>>> + if (cur_id != h.report_ID) {
>>> + matches = 0;
>>> + cur_id = h.report_ID;
>>> + }
>>> + if (h.kind == hid_collection) {
>>> + if (skip)
>>> + continue;
>>> + if (h.collevel == 1) {
>>> + if (h.usage != top_usage)
>>> + skip = 1;
>>> + } else if (coll_usages != NULL) {
>>> + for (i = 0; coll_usages[i] != h.usage; i++)
>>> + if (coll_usages[i] == 0) {
>>> + skip = h.collevel;
>>> + break;
>>> + }
>>> + } else if (((h.usage >> 16) & 0xffff) >= 0xff00) {
>>> + skip = h.collevel;
>>> + }
>>> + } else if (h.kind == hid_endcollection) {
>>> + if (h.collevel < skip)
>>> + skip = 0;
>>> + }
>>> + if (h.kind != kind || skip)
>>> + continue;
>>> + for (i = 0; i < n_usages; i++)
>>> + if (h.usage == usages[i] && !(matches & (1 << i))) {
>>> + matches |= (1 << i);
>>> + if (matches != (1 << n_usages) - 1)
>>> + break;
>>> + hid_end_parse(hd);
>>> + return (h.report_ID);
>>> + }
>>> + }
>>> + hid_end_parse(hd);
>>> + return (-1);
>>> +}
>>> Index: dev/hid/hid.h
>>> ===================================================================
>>> RCS file: /cvs/src/sys/dev/hid/hid.h,v
>>> retrieving revision 1.11
>>> diff -u -p -r1.11 hid.h
>>> --- dev/hid/hid.h 12 Aug 2023 20:47:06 -0000 1.11
>>> +++ dev/hid/hid.h 1 Jul 2025 20:11:57 -0000
>>> @@ -95,6 +95,8 @@ uint32_t hid_get_udata(const uint8_t *bu
>>> int hid_is_collection(const void *, int, uint8_t, int32_t);
>>> struct hid_data *hid_get_collection_data(const void *, int, int32_t, uint32_t);
>>> int hid_get_id_of_collection(const void *, int, int32_t, uint32_t);
>>> +int hid_find_report(const void *, int len, enum hid_kind, int32_t,
>>> + int, int32_t *, int32_t *);
>>>
>>> #endif /* _KERNEL */
>>>
>>> Index: dev/hid/hidmt.c
>>> ===================================================================
>>> RCS file: /cvs/src/sys/dev/hid/hidmt.c,v
>>> retrieving revision 1.13
>>> diff -u -p -r1.13 hidmt.c
>>> --- dev/hid/hidmt.c 16 Oct 2022 20:17:08 -0000 1.13
>>> +++ dev/hid/hidmt.c 1 Jul 2025 20:11:57 -0000
>>> @@ -42,6 +42,9 @@
>>> #define DPRINTF(x)
>>> #endif
>>>
>>> +#define IS_TOP_LEVEL_USAGE(u) ((((u) >> 16) & 0xffff) == HUP_BUTTON || \
>>> + (u) == HID_USAGE2(HUP_DIGITIZERS, HUD_CONTACTCOUNT))
>>> +
>>> #define HID_UNIT_CM 0x11
>>> #define HID_UNIT_INCH 0x13
>>>
>>> @@ -373,7 +376,7 @@ hidmt_input(struct hidmt *mt, uint8_t *d
>>>
>>> bzero(&hc, sizeof(struct hidmt_contact));
>>> }
>>> - else if (!firstu)
>>> + else if (!firstu && !IS_TOP_LEVEL_USAGE(hi->usage))
>>> firstu = hi->usage;
>>>
>>> switch (hi->usage) {
>>> @@ -530,4 +533,41 @@ void
>>> hidmt_disable(struct hidmt *mt)
>>> {
>>> mt->sc_enabled = 0;
>>> +}
>>> +
>>> +int
>>> +hidmt_find_winptp_reports(const void *desc, int len, int *input_rid,
>>> + int *config_rid, int *cap_rid)
>>> +{
>>> + static int32_t ptp_collections[] = {
>>> + HID_USAGE2(HUP_DIGITIZERS, HUD_FINGER), 0
>>> + };
>>> + static int32_t input_usages[] = {
>>> + /* top-level */
>>> + HID_USAGE2(HUP_DIGITIZERS, HUD_CONTACTCOUNT),
>>> + /* contact-level */
>>> + HID_USAGE2(HUP_GENERIC_DESKTOP, HUG_X),
>>> + HID_USAGE2(HUP_GENERIC_DESKTOP, HUG_Y),
>>> + HID_USAGE2(HUP_DIGITIZERS, HUD_CONFIDENCE),
>>> + HID_USAGE2(HUP_DIGITIZERS, HUD_TIP_SWITCH),
>>> + HID_USAGE2(HUP_DIGITIZERS, HUD_CONTACTID),
>>> + };
>>> + static int32_t cfg_usages[] = {
>>> + HID_USAGE2(HUP_DIGITIZERS, HUD_INPUT_MODE),
>>> + };
>>> + static int32_t cap_usages[] = {
>>> + HID_USAGE2(HUP_DIGITIZERS, HUD_CONTACT_MAX),
>>> + };
>>> +
>>> + *input_rid = hid_find_report(desc, len, hid_input,
>>> + HID_USAGE2(HUP_DIGITIZERS, HUD_TOUCHPAD),
>>> + nitems(input_usages), input_usages, ptp_collections);
>>> + *config_rid = hid_find_report(desc, len, hid_feature,
>>> + HID_USAGE2(HUP_DIGITIZERS, HUD_CONFIG),
>>> + nitems(cfg_usages), cfg_usages, ptp_collections);
>>> + *cap_rid = hid_find_report(desc, len, hid_feature,
>>> + HID_USAGE2(HUP_DIGITIZERS, HUD_TOUCHPAD),
>>> + nitems(cap_usages), cap_usages, ptp_collections);
>>> +
>>> + return (*input_rid > 0 && *config_rid > 0 && *cap_rid > 0);
>>> }
>>> Index: dev/hid/hidmtvar.h
>>> ===================================================================
>>> RCS file: /cvs/src/sys/dev/hid/hidmtvar.h,v
>>> retrieving revision 1.8
>>> diff -u -p -r1.8 hidmtvar.h
>>> --- dev/hid/hidmtvar.h 8 Nov 2019 01:20:22 -0000 1.8
>>> +++ dev/hid/hidmtvar.h 1 Jul 2025 20:11:57 -0000
>>> @@ -77,3 +77,4 @@ int hidmt_enable(struct hidmt *);
>>> void hidmt_input(struct hidmt *, uint8_t *, u_int);
>>> int hidmt_ioctl(struct hidmt *, u_long, caddr_t, int, struct proc *);
>>> int hidmt_setup(struct device *, struct hidmt *, void *, int);
>>> +int hidmt_find_winptp_reports(const void *, int, int *, int *, int *);
>>> Index: dev/i2c/imt.c
>>> ===================================================================
>>> RCS file: /cvs/src/sys/dev/i2c/imt.c,v
>>> retrieving revision 1.6
>>> diff -u -p -r1.6 imt.c
>>> --- dev/i2c/imt.c 13 May 2024 01:15:50 -0000 1.6
>>> +++ dev/i2c/imt.c 1 Jul 2025 20:11:57 -0000
>>> @@ -56,8 +56,6 @@ const struct wsmouse_accessops imt_acces
>>> };
>>>
>>> int imt_match(struct device *, void *, void *);
>>> -int imt_find_winptp_reports(struct ihidev_softc *, void *, int,
>>> - struct imt_softc *);
>>> void imt_attach(struct device *, struct device *, void *);
>>> int imt_hidev_get_report(struct device *, int, int, void *, int);
>>> int imt_hidev_set_report(struct device *, int, int, void *, int);
>>> @@ -78,16 +76,17 @@ int
>>> imt_match(struct device *parent, void *match, void *aux)
>>> {
>>> struct ihidev_attach_arg *iha = (struct ihidev_attach_arg *)aux;
>>> - struct imt_softc sc;
>>> + int input_rid, conf_rid, cap_rid;
>>> int size;
>>> void *desc;
>>>
>>> if (iha->reportid == IHIDEV_CLAIM_MULTIPLEID) {
>>> ihidev_get_report_desc(iha->parent, &desc, &size);
>>> - if (imt_find_winptp_reports(iha->parent, desc, size, &sc)) {
>>> - iha->claims[0] = sc.sc_rep_input;
>>> - iha->claims[1] = sc.sc_rep_config;
>>> - iha->claims[2] = sc.sc_rep_cap;
>>> + if (hidmt_find_winptp_reports(desc, size,
>>> + &input_rid, &conf_rid, &cap_rid)) {
>>> + iha->claims[0] = input_rid;
>>> + iha->claims[1] = conf_rid;
>>> + iha->claims[2] = cap_rid;
>>> iha->nclaims = 3;
>>> return (IMATCH_DEVCLASS_DEVSUBCLASS);
>>> }
>>> @@ -96,49 +95,6 @@ imt_match(struct device *parent, void *m
>>> return (IMATCH_NONE);
>>> }
>>>
>>> -int
>>> -imt_find_winptp_reports(struct ihidev_softc *parent, void *desc, int size,
>>> - struct imt_softc *sc)
>>> -{
>>> - int repid;
>>> - int input = 0, conf = 0, cap = 0;
>>> -
>>> - if (sc != NULL) {
>>> - sc->sc_rep_input = -1;
>>> - sc->sc_rep_config = -1;
>>> - sc->sc_rep_cap = -1;
>>> - }
>>> -
>>> - for (repid = 0; repid < parent->sc_nrepid; repid++) {
>>> - if (hid_report_size(desc, size, hid_input, repid) == 0 &&
>>> - hid_report_size(desc, size, hid_output, repid) == 0 &&
>>> - hid_report_size(desc, size, hid_feature, repid) == 0)
>>> - continue;
>>> -
>>> - if (hid_is_collection(desc, size, repid,
>>> - HID_USAGE2(HUP_DIGITIZERS, HUD_TOUCHPAD))) {
>>> - input = 1;
>>> - if (sc != NULL && sc->sc_rep_input == -1)
>>> - sc->sc_rep_input = repid;
>>> - } else if (hid_is_collection(desc, size, repid,
>>> - HID_USAGE2(HUP_DIGITIZERS, HUD_CONFIG))) {
>>> - conf = 1;
>>> - if (sc != NULL && sc->sc_rep_config == -1)
>>> - sc->sc_rep_config = repid;
>>> - }
>>> -
>>> - /* capabilities report could be anywhere */
>>> - if (hid_locate(desc, size, HID_USAGE2(HUP_DIGITIZERS,
>>> - HUD_CONTACT_MAX), repid, hid_feature, NULL, NULL)) {
>>> - cap = 1;
>>> - if (sc != NULL && sc->sc_rep_cap == -1)
>>> - sc->sc_rep_cap = repid;
>>> - }
>>> - }
>>> -
>>> - return (conf && input && cap);
>>> -}
>>> -
>>> void
>>> imt_attach(struct device *parent, struct device *self, void *aux)
>>> {
>>> @@ -152,7 +108,8 @@ imt_attach(struct device *parent, struct
>>> sc->sc_hdev.sc_parent = iha->parent;
>>>
>>> ihidev_get_report_desc(iha->parent, &desc, &size);
>>> - imt_find_winptp_reports(iha->parent, desc, size, sc);
>>> + hidmt_find_winptp_reports(desc, size, &sc->sc_rep_input,
>>> + &sc->sc_rep_config, &sc->sc_rep_cap);
>>>
>>> memset(mt, 0, sizeof(sc->sc_mt));
>>>
>>> Index: dev/usb/umt.c
>>> ===================================================================
>>> RCS file: /cvs/src/sys/dev/usb/umt.c,v
>>> retrieving revision 1.8
>>> diff -u -p -r1.8 umt.c
>>> --- dev/usb/umt.c 26 May 2024 20:06:27 -0000 1.8
>>> +++ dev/usb/umt.c 1 Jul 2025 20:11:57 -0000
>>> @@ -60,8 +60,6 @@ const struct wsmouse_accessops umt_acces
>>> };
>>>
>>> int umt_match(struct device *, void *, void *);
>>> -int umt_find_winptp_reports(struct uhidev_softc *, void *, int, int *,
>>> - int *, int *);
>>> void umt_attach(struct device *, struct device *, void *);
>>> int umt_hidev_get_report(struct device *, int, int, void *, int);
>>> int umt_hidev_set_report(struct device *, int, int, void *, int);
>>> @@ -88,7 +86,7 @@ umt_match(struct device *parent, void *m
>>>
>>> if (UHIDEV_CLAIM_MULTIPLE_REPORTID(uha)) {
>>> uhidev_get_report_desc(uha->parent, &desc, &size);
>>> - if (umt_find_winptp_reports(uha->parent, desc, size, &input,
>>> + if (hidmt_find_winptp_reports(desc, size, &input,
>>> &conf, &cap)) {
>>> uha->claimed[input] = 1;
>>> uha->claimed[conf] = 1;
>>> @@ -100,50 +98,6 @@ umt_match(struct device *parent, void *m
>>> return (UMATCH_NONE);
>>> }
>>>
>>> -int
>>> -umt_find_winptp_reports(struct uhidev_softc *parent, void *desc, int size,
>>> - int *input, int *config, int *cap)
>>> -{
>>> - int repid;
>>> - int finput = 0, fconf = 0, fcap = 0;
>>> -
>>> - if (input != NULL)
>>> - *input = -1;
>>> - if (config != NULL)
>>> - *config = -1;
>>> - if (cap != NULL)
>>> - *cap = -1;
>>> -
>>> - for (repid = 0; repid < parent->sc_nrepid; repid++) {
>>> - if (hid_report_size(desc, size, hid_input, repid) == 0 &&
>>> - hid_report_size(desc, size, hid_output, repid) == 0 &&
>>> - hid_report_size(desc, size, hid_feature, repid) == 0)
>>> - continue;
>>> -
>>> - if (hid_is_collection(desc, size, repid,
>>> - HID_USAGE2(HUP_DIGITIZERS, HUD_TOUCHPAD))) {
>>> - finput = 1;
>>> - if (input != NULL && *input == -1)
>>> - *input = repid;
>>> - } else if (hid_is_collection(desc, size, repid,
>>> - HID_USAGE2(HUP_DIGITIZERS, HUD_CONFIG))) {
>>> - fconf = 1;
>>> - if (config != NULL && *config == -1)
>>> - *config = repid;
>>> - }
>>> -
>>> - /* capabilities report could be anywhere */
>>> - if (hid_locate(desc, size, HID_USAGE2(HUP_DIGITIZERS,
>>> - HUD_CONTACT_MAX), repid, hid_feature, NULL, NULL)) {
>>> - fcap = 1;
>>> - if (cap != NULL && *cap == -1)
>>> - *cap = repid;
>>> - }
>>> - }
>>> -
>>> - return (fconf && finput && fcap);
>>> -}
>>> -
>>> void
>>> umt_attach(struct device *parent, struct device *self, void *aux)
>>> {
>>> @@ -163,7 +117,7 @@ umt_attach(struct device *parent, struct
>>> sc->sc_quirks = usbd_get_quirks(sc->sc_hdev.sc_udev)->uq_flags;
>>>
>>> uhidev_get_report_desc(uha->parent, &desc, &size);
>>> - umt_find_winptp_reports(uha->parent, desc, size, &sc->sc_rep_input,
>>> + hidmt_find_winptp_reports(desc, size, &sc->sc_rep_input,
>>> &sc->sc_rep_config, &sc->sc_rep_cap);
>>>
>>> memset(mt, 0, sizeof(sc->sc_mt));
>>>
>>
>>
imt(4)/umt(4): finding HID report IDs