Changeset View
Standalone View
sys/dev/hid/hid.c
Show First 20 Lines • Show All 797 Lines • ▼ Show 20 Lines | |||||
*------------------------------------------------------------------------*/ | *------------------------------------------------------------------------*/ | ||||
int | int | ||||
hid_is_collection(const void *desc, hid_size_t size, int32_t usage) | hid_is_collection(const void *desc, hid_size_t size, int32_t usage) | ||||
{ | { | ||||
struct hid_data *hd; | struct hid_data *hd; | ||||
struct hid_item hi; | struct hid_item hi; | ||||
int err; | int err; | ||||
hd = hid_start_parse(desc, size, hid_input); | hd = hid_start_parse(desc, size, 0); | ||||
delphij: If I am understanding the code correctly (disclaimer: I am not familiar with this particular… | |||||
wulfUnsubmitted Not Done Inline ActionsPassing 0 is correct for our simple hid_is_collection() implementation. 1 << hid_input is required for more complex NetBSD version which tracks for input usage presence within a collection. See So we should replace hid_input with 0 and leave while loop as is or replace hid_input with 1 << hid_input and import NetBSD code. wulf: Passing `0` is correct for our simple hid_is_collection() implementation.
`1 << hid_input` is… | |||||
ankohuu_outlook.comAuthorUnsubmitted Done Inline Actions
I think hid_start_parse is for calling hid_get_item in a subsequent loop later. What kind of item we want to get in hid_get_item must be reflected in the third parameter kindset of hid_start_parse. In addition, items of type hid_collection/hid_endcollection will be returned regardless of the kindset.
Maybe simply that was not being implemented in sys/dev/hid/hid.c, when multiple bits are passed to hid_start_parse, hid_get_item can't get item position correctly, because pos in struct hid_data can't handle multiple kinds, functions in lib/libusbhid/parse.c work the same as described in the USBHID(3). ankohuu_outlook.com: > If I am understanding the code correctly (disclaimer: I am not familiar with this particular… | |||||
if (hd == NULL) | if (hd == NULL) | ||||
return (0); | return (0); | ||||
while ((err = hid_get_item(hd, &hi))) { | while ((err = hid_get_item(hd, &hi))) { | ||||
if (hi.kind == hid_collection && | if (hi.kind == hid_collection && | ||||
hi.usage == usage) | hi.usage == usage) | ||||
break; | break; | ||||
} | } | ||||
▲ Show 20 Lines • Show All 265 Lines • Show Last 20 Lines |
If I am understanding the code correctly (disclaimer: I am not familiar with this particular module so I could well be wrong), it appears like this should really be 1 << hid_input here. Passing 0 worked because when kind is collection, the kindset is blindly ignored, but for consistency with all other usages in the tree, I still think it's better to pass 1 << hid_input here.
BTW. despite documented as allowing a set of bits being passed in kindset, the code would fail when multiple bits are passed to hid_start_parse. It wasn't clear if that's a deliberate choice, or simply that was not being implemented, but the inconsistency of how kindset was kinda confusing.