I have not much clue about HID, but when we did some testing for the
new ujoy(4) driver it turned out that the PS4 controller doesn't get
handled correctly by hid.c:hid_is_collection().  This made me peek in
to the NetBSD code where I could find an update in this function.

Syncing it up makes the PS4 controller attachment work correctly,
while not breaking my other HID devices.

The change was introduced in NetBSD with hid.c revision 1.25 on the
19.06.2006, obviously as part of a bigger change, not saying something
about the hid_is_collection() update itself:

--

Initial import of bluetooth stack on behalf of Iain Hibbert.  (plunky@,
NetBSD Foundation Membership still pending.)  This stack was written by
Iain under sponsorship from Itronix Inc.

The stack includes support for rfcomm networking (networking via your
bluetooth enabled cell phone), hid devices (keyboards/mice), and
headsets.

Drivers for both PCMCIA and USB bluetooth controllers are included.

--

http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/dev/usb/Attic/hid.c.diff?r1=1.24&r2=1.25&only_with_tag=MAIN

More testers?  Comments?  OKs?


Index: dev/hid/hid.c
===================================================================
RCS file: /cvs/src/sys/dev/hid/hid.c,v
retrieving revision 1.3
diff -u -p -u -p -r1.3 hid.c
--- dev/hid/hid.c       4 Jun 2020 23:03:43 -0000       1.3
+++ dev/hid/hid.c       9 Jan 2021 08:43:30 -0000
@@ -630,6 +630,25 @@ hid_get_udata(const uint8_t *buf, int le
         return (hid_get_data_sub(buf, len, loc, 0));
 }
 
+/*
+ * hid_is_collection(desc, size, id, usage)
+ *
+ * This function is broken in the following way.
+ *
+ * It is used to discover if the given 'id' is part of 'usage' collection
+ * in the descriptor in order to match report id against device type.
+ *
+ * The semantics of hid_start_parse() means though, that only a single
+ * kind of report is considered. The current HID code that uses this for
+ * matching is actually only looking for input reports, so this works
+ * for now.
+ *
+ * This function could try all report kinds (input, output and feature)
+ * consecutively if necessary, but it may be better to integrate the
+ * libusbhid code which can consider multiple report kinds simultaneously
+ *
+ * Needs some thought.
+ */
 int
 hid_is_collection(const void *desc, int size, uint8_t id, int32_t usage)
 {
@@ -637,17 +656,25 @@ hid_is_collection(const void *desc, int 
        struct hid_item hi;
        uint32_t coll_usage = ~0;
 
-       hd = hid_start_parse(desc, size, hid_none);
+       hd = hid_start_parse(desc, size, hid_input);
+       if (hd == NULL)
+               return (0);
 
        DPRINTF("%s: id=%d usage=0x%x\n", __func__, id, usage);
        while (hid_get_item(hd, &hi)) {
                DPRINTF("%s: kind=%d id=%d usage=0x%x(0x%x)\n", __func__,
                            hi.kind, hi.report_ID, hi.usage, coll_usage);
+
                if (hi.kind == hid_collection &&
                    hi.collection == HCOLL_APPLICATION)
                        coll_usage = hi.usage;
-               if (hi.kind == hid_endcollection &&
-                   coll_usage == usage && hi.report_ID == id) {
+
+               if (hi.kind == hid_endcollection)
+                       coll_usage = ~0;
+
+               if (hi.kind == hid_input &&
+                   coll_usage == usage &&
+                   hi.report_ID == id) {
                        DPRINTF("%s: found\n", __func__);
                        hid_end_parse(hd);
                        return (1);

Reply via email to