> Date: Sat, 9 Jan 2021 10:38:20 +0100
> From: Marcus Glocker <[email protected]>
>
> 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?
Probably needs some testing with touchpad and touchscreen devices.
> 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);
>
>
