On Thu, Jan 07, 2021 at 08:20:35PM +0100, Marcus Glocker wrote:
> > I have heard from others who tried the diff that the PS4 controller is
> > causing problems with the way it attaches. I ordered one to trial-and-
> > error this myself at home. Could you share output of lsusb -vv? Thanks
> > for giving it a try!
>
> Sure, here we go.
> If I can find anything myself in the meantime I let you know.
So the problem doesn't seem to be in your new ujoy(4) code, but how the
dev/hid/hid.c:hid_is_collection() function tries to cope with the PS4
controller.
I'm not much in to HID, but when I sync up the hid_is_collection()
function with NetBSD, the PS4 controller attaches to ujoy(4) as
expected, and also can be accessed by usbhidctl(1), while my other
HID devices continue to work as before:
uaudio0 at uhub4 port 4 configuration 1 interface 1 "Sony Interactive
Entertainment Wireless Controller" rev 2.00/1.00 addr 6
uaudio0: class v1, full-speed, sync, channels: 2 play, 1 rec, 4 ctls
audio1 at uaudio0
uhidev6 at uhub4 port 4 configuration 1 interface 3 "Sony Interactive
Entertainment Wireless Controller" rev 2.00/1.00 addr 6
uhidev6: iclass 3/0, 180 report ids
ujoy0 at uhidev6 reportid 1: input=63, output=0, feature=0 <--
uhid6 at uhidev6 reportid 2: input=0, output=0, feature=36
uhid7 at uhidev6 reportid 4: input=0, output=0, feature=36
uhid8 at uhidev6 reportid 5: input=0, output=31, feature=0
uhid9 at uhidev6 reportid 8: input=0, output=0, feature=3
uhid10 at uhidev6 reportid 16: input=0, output=0, feature=4
uhid11 at uhidev6 reportid 17: input=0, output=0, feature=2
uhid12 at uhidev6 reportid 18: input=0, output=0, feature=15
uhid13 at uhidev6 reportid 19: input=0, output=0, feature=22
uhid14 at uhidev6 reportid 20: input=0, output=0, feature=16
uhid15 at uhidev6 reportid 21: input=0, output=0, feature=44
uhid16 at uhidev6 reportid 128: input=0, output=0, feature=6
uhid17 at uhidev6 reportid 129: input=0, output=0, feature=6
uhid18 at uhidev6 reportid 130: input=0, output=0, feature=5
uhid19 at uhidev6 reportid 131: input=0, output=0, feature=1
uhid20 at uhidev6 reportid 132: input=0, output=0, feature=4
uhid21 at uhidev6 reportid 133: input=0, output=0, feature=6
uhid22 at uhidev6 reportid 134: input=0, output=0, feature=6
uhid23 at uhidev6 reportid 135: input=0, output=0, feature=35
uhid24 at uhidev6 reportid 136: input=0, output=0, feature=34
uhid25 at uhidev6 reportid 137: input=0, output=0, feature=2
uhid26 at uhidev6 reportid 144: input=0, output=0, feature=5
uhid27 at uhidev6 reportid 145: input=0, output=0, feature=3
uhid28 at uhidev6 reportid 146: input=0, output=0, feature=3
uhid29 at uhidev6 reportid 147: input=0, output=0, feature=12
uhid30 at uhidev6 reportid 160: input=0, output=0, feature=6
uhid31 at uhidev6 reportid 161: input=0, output=0, feature=1
uhid32 at uhidev6 reportid 162: input=0, output=0, feature=1
uhid33 at uhidev6 reportid 163: input=0, output=0, feature=48
uhid34 at uhidev6 reportid 164: input=0, output=0, feature=13
uhid35 at uhidev6 reportid 165: input=0, output=0, feature=21
uhid36 at uhidev6 reportid 166: input=0, output=0, feature=21
uhid37 at uhidev6 reportid 167: input=0, output=0, feature=1
uhid38 at uhidev6 reportid 168: input=0, output=0, feature=1
uhid39 at uhidev6 reportid 169: input=0, output=0, feature=8
uhid40 at uhidev6 reportid 170: input=0, output=0, feature=1
uhid41 at uhidev6 reportid 171: input=0, output=0, feature=57
uhid42 at uhidev6 reportid 172: input=0, output=0, feature=57
uhid43 at uhidev6 reportid 173: input=0, output=0, feature=11
uhid44 at uhidev6 reportid 174: input=0, output=0, feature=1
uhid45 at uhidev6 reportid 175: input=0, output=0, feature=2
uhid46 at uhidev6 reportid 176: input=0, output=0, feature=63
uhid47 at uhidev6 reportid 177: input=0, output=0, feature=2
uhid48 at uhidev6 reportid 178: input=0, output=0, feature=2
uhid49 at uhidev6 reportid 179: input=0, output=0, feature=63
uhid50 at uhidev6 reportid 180: input=0, output=0, feature=63
$ usbhidctl -f /dev/ujoy/0 -l
Game_Pad.X=126
Game_Pad.Y=126
Game_Pad.Z=125
Game_Pad.Rz=129
Game_Pad.Hat_Switch=8
Game_Pad.Button_1=0
Game_Pad.Button_2=0
Game_Pad.Button_3=0
Game_Pad.Button_4=0
Game_Pad.Button_5=0
Game_Pad.Button_6=0
Game_Pad.Button_7=0
Game_Pad.Button_8=0
Game_Pad.Button_9=0
Game_Pad.Button_10=0
Game_Pad.Button_11=0
Game_Pad.Button_12=0
Game_Pad.Button_13=0
Game_Pad.Button_14=0
Game_Pad.0x0020=28
Game_Pad.Rx=0
Game_Pad.Ry=0
[...]
If others want to regression test this diff, and somebody wants to OK
it. Maybe I should send it out on a separate thread.
Once this has been fixed I would continue to do some more ujoy(4)
testing, and check about a possible import. We also should agree on
who/how to fix the ports part.
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);
