Re: New ujoy(4) device for USB gamecontrollers
On Fri, Jan 22, 2021 at 02:26:34PM -0500, Bryan Steele wrote: > On Fri, Jan 22, 2021 at 07:00:57PM +0100, Marcus Glocker wrote: > > On Fri, 15 Jan 2021 22:41:13 +0100 > > Marcus Glocker wrote: > > > > > On Fri, 15 Jan 2021 11:37:47 -0500 > > > Bryan Steele wrote: > > > > > > > On Fri, Jan 15, 2021 at 06:23:01AM -0700, Thomas Frohwein wrote: > > > > > On Sat, Jan 09, 2021 at 10:16:16AM +0100, Marcus Glocker wrote: > > > > > > > > > > > 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. > > > > > > > > > > So with the hid_is_collection() problem not easy to mitigate [1], > > > > > should we table the ujoy(4) proposal for now pending a fix for the > > > > > problems with the PS4 controller? Or is this interesting enough > > > > > for some to work on moving forward despite this issue and finding > > > > > a solution for this specific (and in some ways unusual) device > > > > > later? > > > > > > > > Considering the hid_is_collection() fix from NetBSD proposed by > > > > Marcus fixes the issue with ujoy(4) but breaks some other drivers > > > > (imt(4) and ims(4)), could we not inline it into ujoy(4) for now? > > > > It's fairly small helper function. Like hid_is_joy_collection()? > > > > > > I think that could be an XXX workaround until somebody finds a proper, > > > generic fix for hid_is_collection() ... > > > > That's what you meant, yes? > > Yep, this is what I meant. Hopefully it won't have to stay for long, but > t would be nice to get ujoy(4) working as many controllers as possible > so that it can go in. > > Can you send a new full diff thfr@? I still need to test with my USB > SNES controller, but tentatively this is still ok brynet@ > > -Bryan. Yep, works fine with my generic USB SNES controller, tested with usbhidctl -f /dev/ujoy/0 -l, shows all button presses. uhidev4 at uhub0 port 12 configuration 1 interface 0 "vendor 0x0810 usb gamepad" rev 1.00/1.06 addr 2 uhidev4: iclass 3/0, 1 report id ujoy0 at uhidev4 reportid 1: input=7, output=0, feature=0 I think this is ready to go in. :-) -Bryan.
Re: New ujoy(4) device for USB gamecontrollers
On Fri, Jan 22, 2021 at 01:52:24PM -0700, Thomas Frohwein wrote:
> On Fri, Jan 22, 2021 at 02:26:31PM -0500, Bryan Steele wrote:
> > On Fri, Jan 22, 2021 at 07:00:57PM +0100, Marcus Glocker wrote:
>
> [...]
>
> > > > > Considering the hid_is_collection() fix from NetBSD proposed by
> > > > > Marcus fixes the issue with ujoy(4) but breaks some other drivers
> > > > > (imt(4) and ims(4)), could we not inline it into ujoy(4) for now?
> > > > > It's fairly small helper function. Like hid_is_joy_collection()?
> > > >
> > > > I think that could be an XXX workaround until somebody finds a proper,
> > > > generic fix for hid_is_collection() ...
> > >
> > > That's what you meant, yes?
> >
> > Yep, this is what I meant. Hopefully it won't have to stay for long, but
> > t would be nice to get ujoy(4) working as many controllers as possible
> > so that it can go in.
> >
> > Can you send a new full diff thfr@? I still need to test with my USB
> > SNES controller, but tentatively this is still ok brynet@
>
> Below is the updated diff, containing the original ujoy(4) diff and
> Marcus' ujoy.c that includes ujoy_hid_is_collection().
>
> [...]
>
> > > Can this been updated in the overall diff and re-verified if it still
> > > works fine with the other controllers? It (obviously) works with my
> > > PS4 controller.
>
> I built the kernel and device nodes anew with this diff and tested all the
> joystick and gamecontroller devices that I have access to with
> usbhidctl(1) (XBox 360 gamepad, Logitech F310 in D and X mode, Logitech
> Dual Action, Microsoft Sidewinder Precision 2) and all work as expected
> via /dev/ujoy/0.
Thanks.
Can you please also reset the tags of the new files to
/* $OpenBSD$ */
just to be sure that nothing nasty happens with the versions during
the import?
Otherwise this is ok mglocker@ under the condition that you guys take
care about the ports adaption :-)
> Index: etc/MAKEDEV.common
> ===
> RCS file: /cvs/src/etc/MAKEDEV.common,v
> retrieving revision 1.111
> diff -u -p -r1.111 MAKEDEV.common
> --- etc/MAKEDEV.common6 Jul 2020 06:11:26 - 1.111
> +++ etc/MAKEDEV.common22 Jan 2021 20:04:39 -
> @@ -181,6 +181,7 @@ dnl
> target(usb, usb, 0, 1, 2, 3, 4, 5, 6, 7)dnl
> target(usb, uhid, 0, 1, 2, 3, 4, 5, 6, 7)dnl
> twrget(usb, fido, fido)dnl
> +twrget(usb, ujoy, ujoy)dnl
> target(usb, ulpt, 0, 1)dnl
> target(usb, ugen, 0, 1, 2, 3, 4, 5, 6, 7)dnl
> target(usb, ttyU, 0, 1, 2, 3)dnl
> @@ -365,6 +366,10 @@ __devitem(fido, fido, fido/* nodes, fido
> _mkdev(fido, fido, {-RMlist[${#RMlist[*]}]=";mkdir -p fido;rm -f" n=0
> while [ $n -lt 4 ];do M fido/$n c major_fido_c $n 666;n=Add($n, 1);done
> MKlist[${#MKlist[*]}]=";chmod 555 fido"-})dnl
> +__devitem(ujoy, ujoy, ujoy/* nodes, ujoy)dnl
> +_mkdev(ujoy, ujoy, {-RMlist[${#RMlist[*]}]=";mkdir -p ujoy;rm -f" n=0
> + while [ $n -lt 4 ];do M ujoy/$n c major_ujoy_c $n 444;n=Add($n, 1);done
> + MKlist[${#MKlist[*]}]=";chmod 555 ujoy"-})dnl
> __devitem(ulpt, ulpt*, Printer devices)dnl
> _mcdev({-ulpt-}, ulpt*, {-ulpt-}, {-major_ulpt_c-}, 600)dnl
> __devitem(ttyU, ttyU*, USB serial ports,ucom)dnl
> Index: etc/etc.alpha/MAKEDEV.md
> ===
> RCS file: /cvs/src/etc/etc.alpha/MAKEDEV.md,v
> retrieving revision 1.76
> diff -u -p -r1.76 MAKEDEV.md
> --- etc/etc.alpha/MAKEDEV.md 6 Jul 2020 06:11:26 - 1.76
> +++ etc/etc.alpha/MAKEDEV.md 22 Jan 2021 20:04:39 -
> @@ -56,6 +56,7 @@ _DEV(uall)
> _DEV(ugen, 48)
> _DEV(uhid, 46)
> _DEV(fido, 70)
> +_DEV(ujoy, 72)
> _DEV(ulpt, 47)
> _DEV(usb, 45)
> _TITLE(spec)
> Index: etc/etc.amd64/MAKEDEV.md
> ===
> RCS file: /cvs/src/etc/etc.amd64/MAKEDEV.md,v
> retrieving revision 1.76
> diff -u -p -r1.76 MAKEDEV.md
> --- etc/etc.amd64/MAKEDEV.md 6 Jul 2020 06:11:26 - 1.76
> +++ etc/etc.amd64/MAKEDEV.md 22 Jan 2021 20:04:39 -
> @@ -60,6 +60,7 @@ _DEV(uall)
> _DEV(ugen, 63)
> _DEV(uhid, 62)
> _DEV(fido, 98)
> +_DEV(ujoy, 100)
> _DEV(ulpt, 64)
> _DEV(usb, 61)
> _TITLE(spec)
> Index: etc/etc.arm64/MAKEDEV.md
> ===
> RCS file: /cvs/src/etc/etc.arm64/MAKEDEV.md,v
> retrieving revision 1.10
> diff -u -p -r1.10 MAKEDEV.md
> --- etc/etc.arm64/MAKEDEV.md 6 Jul 2020 06:11:26 - 1.10
> +++ etc/etc.arm64/MAKEDEV.md 22 Jan 2021 20:04:39 -
> @@ -52,6 +52,7 @@ _DEV(uall)
> _DEV(ugen, 63)
> _DEV(uhid, 62)
> _DEV(fido, 98)
> +_DEV(ujoy, 100)
> _DEV(ulpt, 64)
> _DEV(usb, 61)
> _TITLE(spec)
> Index: etc/etc.armv7/MAKEDEV.md
> ===
> RCS file: /cvs/src/etc/etc.armv7/MAKEDEV.md,v
> retrieving revision 1.18
> diff -u -p -r1.18 MAKEDEV.md
> --- etc/etc.armv7/MAKEDEV.md 6 Jul 2020 06:11:26 - 1.18
> +++
Re: New ujoy(4) device for USB gamecontrollers
On Fri, Jan 22, 2021 at 02:26:31PM -0500, Bryan Steele wrote:
> On Fri, Jan 22, 2021 at 07:00:57PM +0100, Marcus Glocker wrote:
[...]
> > > > Considering the hid_is_collection() fix from NetBSD proposed by
> > > > Marcus fixes the issue with ujoy(4) but breaks some other drivers
> > > > (imt(4) and ims(4)), could we not inline it into ujoy(4) for now?
> > > > It's fairly small helper function. Like hid_is_joy_collection()?
> > >
> > > I think that could be an XXX workaround until somebody finds a proper,
> > > generic fix for hid_is_collection() ...
> >
> > That's what you meant, yes?
>
> Yep, this is what I meant. Hopefully it won't have to stay for long, but
> t would be nice to get ujoy(4) working as many controllers as possible
> so that it can go in.
>
> Can you send a new full diff thfr@? I still need to test with my USB
> SNES controller, but tentatively this is still ok brynet@
Below is the updated diff, containing the original ujoy(4) diff and
Marcus' ujoy.c that includes ujoy_hid_is_collection().
[...]
> > Can this been updated in the overall diff and re-verified if it still
> > works fine with the other controllers? It (obviously) works with my
> > PS4 controller.
I built the kernel and device nodes anew with this diff and tested all the
joystick and gamecontroller devices that I have access to with
usbhidctl(1) (XBox 360 gamepad, Logitech F310 in D and X mode, Logitech
Dual Action, Microsoft Sidewinder Precision 2) and all work as expected
via /dev/ujoy/0.
Index: etc/MAKEDEV.common
===
RCS file: /cvs/src/etc/MAKEDEV.common,v
retrieving revision 1.111
diff -u -p -r1.111 MAKEDEV.common
--- etc/MAKEDEV.common 6 Jul 2020 06:11:26 - 1.111
+++ etc/MAKEDEV.common 22 Jan 2021 20:04:39 -
@@ -181,6 +181,7 @@ dnl
target(usb, usb, 0, 1, 2, 3, 4, 5, 6, 7)dnl
target(usb, uhid, 0, 1, 2, 3, 4, 5, 6, 7)dnl
twrget(usb, fido, fido)dnl
+twrget(usb, ujoy, ujoy)dnl
target(usb, ulpt, 0, 1)dnl
target(usb, ugen, 0, 1, 2, 3, 4, 5, 6, 7)dnl
target(usb, ttyU, 0, 1, 2, 3)dnl
@@ -365,6 +366,10 @@ __devitem(fido, fido, fido/* nodes, fido
_mkdev(fido, fido, {-RMlist[${#RMlist[*]}]=";mkdir -p fido;rm -f" n=0
while [ $n -lt 4 ];do M fido/$n c major_fido_c $n 666;n=Add($n, 1);done
MKlist[${#MKlist[*]}]=";chmod 555 fido"-})dnl
+__devitem(ujoy, ujoy, ujoy/* nodes, ujoy)dnl
+_mkdev(ujoy, ujoy, {-RMlist[${#RMlist[*]}]=";mkdir -p ujoy;rm -f" n=0
+ while [ $n -lt 4 ];do M ujoy/$n c major_ujoy_c $n 444;n=Add($n, 1);done
+ MKlist[${#MKlist[*]}]=";chmod 555 ujoy"-})dnl
__devitem(ulpt, ulpt*, Printer devices)dnl
_mcdev({-ulpt-}, ulpt*, {-ulpt-}, {-major_ulpt_c-}, 600)dnl
__devitem(ttyU, ttyU*, USB serial ports,ucom)dnl
Index: etc/etc.alpha/MAKEDEV.md
===
RCS file: /cvs/src/etc/etc.alpha/MAKEDEV.md,v
retrieving revision 1.76
diff -u -p -r1.76 MAKEDEV.md
--- etc/etc.alpha/MAKEDEV.md6 Jul 2020 06:11:26 - 1.76
+++ etc/etc.alpha/MAKEDEV.md22 Jan 2021 20:04:39 -
@@ -56,6 +56,7 @@ _DEV(uall)
_DEV(ugen, 48)
_DEV(uhid, 46)
_DEV(fido, 70)
+_DEV(ujoy, 72)
_DEV(ulpt, 47)
_DEV(usb, 45)
_TITLE(spec)
Index: etc/etc.amd64/MAKEDEV.md
===
RCS file: /cvs/src/etc/etc.amd64/MAKEDEV.md,v
retrieving revision 1.76
diff -u -p -r1.76 MAKEDEV.md
--- etc/etc.amd64/MAKEDEV.md6 Jul 2020 06:11:26 - 1.76
+++ etc/etc.amd64/MAKEDEV.md22 Jan 2021 20:04:39 -
@@ -60,6 +60,7 @@ _DEV(uall)
_DEV(ugen, 63)
_DEV(uhid, 62)
_DEV(fido, 98)
+_DEV(ujoy, 100)
_DEV(ulpt, 64)
_DEV(usb, 61)
_TITLE(spec)
Index: etc/etc.arm64/MAKEDEV.md
===
RCS file: /cvs/src/etc/etc.arm64/MAKEDEV.md,v
retrieving revision 1.10
diff -u -p -r1.10 MAKEDEV.md
--- etc/etc.arm64/MAKEDEV.md6 Jul 2020 06:11:26 - 1.10
+++ etc/etc.arm64/MAKEDEV.md22 Jan 2021 20:04:39 -
@@ -52,6 +52,7 @@ _DEV(uall)
_DEV(ugen, 63)
_DEV(uhid, 62)
_DEV(fido, 98)
+_DEV(ujoy, 100)
_DEV(ulpt, 64)
_DEV(usb, 61)
_TITLE(spec)
Index: etc/etc.armv7/MAKEDEV.md
===
RCS file: /cvs/src/etc/etc.armv7/MAKEDEV.md,v
retrieving revision 1.18
diff -u -p -r1.18 MAKEDEV.md
--- etc/etc.armv7/MAKEDEV.md6 Jul 2020 06:11:26 - 1.18
+++ etc/etc.armv7/MAKEDEV.md22 Jan 2021 20:04:39 -
@@ -61,6 +61,7 @@ _DEV(uall)
_DEV(ugen, 70)
_DEV(uhid, 65)
_DEV(fido, 106)
+_DEV(ujoy, 108)
_DEV(ulpt, 66)
_DEV(usb, 64)
_TITLE(spec)
Index: etc/etc.hppa/MAKEDEV.md
===
RCS file: /cvs/src/etc/etc.hppa/MAKEDEV.md,v
retrieving revision 1.65
diff -u -p -r1.65 MAKEDEV.md
--- etc/etc.hppa/MAKEDEV.md 6 Jul 2020 06:11:26 - 1.65
+++ etc/etc.hppa/MAKEDEV.md 22 Jan 2021 20:04:39 -
@@ -54,6 +54,7 @@ _DEV(uall)
Re: New ujoy(4) device for USB gamecontrollers
On Fri, Jan 22, 2021 at 07:00:57PM +0100, Marcus Glocker wrote:
> On Fri, 15 Jan 2021 22:41:13 +0100
> Marcus Glocker wrote:
>
> > On Fri, 15 Jan 2021 11:37:47 -0500
> > Bryan Steele wrote:
> >
> > > On Fri, Jan 15, 2021 at 06:23:01AM -0700, Thomas Frohwein wrote:
> > > > On Sat, Jan 09, 2021 at 10:16:16AM +0100, Marcus Glocker wrote:
> > > >
> > > > > 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.
> > > >
> > > > So with the hid_is_collection() problem not easy to mitigate [1],
> > > > should we table the ujoy(4) proposal for now pending a fix for the
> > > > problems with the PS4 controller? Or is this interesting enough
> > > > for some to work on moving forward despite this issue and finding
> > > > a solution for this specific (and in some ways unusual) device
> > > > later?
> > >
> > > Considering the hid_is_collection() fix from NetBSD proposed by
> > > Marcus fixes the issue with ujoy(4) but breaks some other drivers
> > > (imt(4) and ims(4)), could we not inline it into ujoy(4) for now?
> > > It's fairly small helper function. Like hid_is_joy_collection()?
> >
> > I think that could be an XXX workaround until somebody finds a proper,
> > generic fix for hid_is_collection() ...
>
> That's what you meant, yes?
Yep, this is what I meant. Hopefully it won't have to stay for long, but
t would be nice to get ujoy(4) working as many controllers as possible
so that it can go in.
Can you send a new full diff thfr@? I still need to test with my USB
SNES controller, but tentatively this is still ok brynet@
-Bryan.
> Can this been updated in the overall diff and re-verified if it still
> works fine with the other controllers? It (obviously) works with my
> PS4 controller.
>
>
> Index: sys/dev/usb/ujoy.c
> ===
> RCS file: sys/dev/usb/ujoy.c
> diff -N sys/dev/usb/ujoy.c
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ sys/dev/usb/ujoy.c22 Jan 2021 17:54:46 -
> @@ -0,0 +1,149 @@
> +/* $OpenBSD$ */
> +
> +/*
> + * Copyright (c) 2020 Thomas Frohwein
> + * Copyright (c) 2020 Bryan Steele
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +
> +int ujoy_match(struct device *, void *, void *);
> +
> +struct cfdriver ujoy_cd = {
> + NULL, "ujoy", DV_DULL
> +};
> +
> +const struct cfattach ujoy_ca = {
> + sizeof(struct uhid_softc),
> + ujoy_match,
> + uhid_attach,
> + uhid_detach,
> +};
> +
> +/*
> + * XXX workaround:
> + *
> + * This is a copy of sys/dev/hid/hid.c:hid_is_collection(), synced up to the
> + * NetBSD version. Our current hid_is_collection() is not playing nice with
> + * all HID devices like the PS4 controller. But applying this version
> + * globally breaks other HID devices like ims(4) and imt(4). Until our
> global
> + * hid_is_collection() can't be fixed to play nice with all HID devices, we
> + * go for this dedicated ujoy(4) version.
> + */
> +int
> +ujoy_hid_is_collection(const void *desc, int size, uint8_t id, int32_t usage)
> +{
> + struct hid_data *hd;
> + struct hid_item hi;
> + uint32_t coll_usage = ~0;
> +
> + hd = hid_start_parse(desc, size, hid_input);
> + if (hd == NULL)
> + return (0);
> +
> + while (hid_get_item(hd, )) {
> + if (hi.kind == hid_collection &&
Re: New ujoy(4) device for USB gamecontrollers
On Fri, 15 Jan 2021 22:41:13 +0100
Marcus Glocker wrote:
> On Fri, 15 Jan 2021 11:37:47 -0500
> Bryan Steele wrote:
>
> > On Fri, Jan 15, 2021 at 06:23:01AM -0700, Thomas Frohwein wrote:
> > > On Sat, Jan 09, 2021 at 10:16:16AM +0100, Marcus Glocker wrote:
> > >
> > > > 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.
> > >
> > > So with the hid_is_collection() problem not easy to mitigate [1],
> > > should we table the ujoy(4) proposal for now pending a fix for the
> > > problems with the PS4 controller? Or is this interesting enough
> > > for some to work on moving forward despite this issue and finding
> > > a solution for this specific (and in some ways unusual) device
> > > later?
> >
> > Considering the hid_is_collection() fix from NetBSD proposed by
> > Marcus fixes the issue with ujoy(4) but breaks some other drivers
> > (imt(4) and ims(4)), could we not inline it into ujoy(4) for now?
> > It's fairly small helper function. Like hid_is_joy_collection()?
>
> I think that could be an XXX workaround until somebody finds a proper,
> generic fix for hid_is_collection() ...
That's what you meant, yes?
Can this been updated in the overall diff and re-verified if it still
works fine with the other controllers? It (obviously) works with my
PS4 controller.
Index: sys/dev/usb/ujoy.c
===
RCS file: sys/dev/usb/ujoy.c
diff -N sys/dev/usb/ujoy.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ sys/dev/usb/ujoy.c 22 Jan 2021 17:54:46 -
@@ -0,0 +1,149 @@
+/* $OpenBSD$ */
+
+/*
+ * Copyright (c) 2020 Thomas Frohwein
+ * Copyright (c) 2020 Bryan Steele
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+
+#include
+#include
+#include
+
+#include
+#include
+
+int ujoy_match(struct device *, void *, void *);
+
+struct cfdriver ujoy_cd = {
+ NULL, "ujoy", DV_DULL
+};
+
+const struct cfattach ujoy_ca = {
+ sizeof(struct uhid_softc),
+ ujoy_match,
+ uhid_attach,
+ uhid_detach,
+};
+
+/*
+ * XXX workaround:
+ *
+ * This is a copy of sys/dev/hid/hid.c:hid_is_collection(), synced up to the
+ * NetBSD version. Our current hid_is_collection() is not playing nice with
+ * all HID devices like the PS4 controller. But applying this version
+ * globally breaks other HID devices like ims(4) and imt(4). Until our global
+ * hid_is_collection() can't be fixed to play nice with all HID devices, we
+ * go for this dedicated ujoy(4) version.
+ */
+int
+ujoy_hid_is_collection(const void *desc, int size, uint8_t id, int32_t usage)
+{
+ struct hid_data *hd;
+ struct hid_item hi;
+ uint32_t coll_usage = ~0;
+
+ hd = hid_start_parse(desc, size, hid_input);
+ if (hd == NULL)
+ return (0);
+
+ while (hid_get_item(hd, )) {
+ if (hi.kind == hid_collection &&
+ hi.collection == HCOLL_APPLICATION)
+ coll_usage = hi.usage;
+
+ if (hi.kind == hid_endcollection)
+ coll_usage = ~0;
+
+ if (hi.kind == hid_input &&
+ coll_usage == usage &&
+ hi.report_ID == id) {
+ hid_end_parse(hd);
+ return (1);
+ }
+ }
+ hid_end_parse(hd);
+
+ return (0);
+}
+
+int
+ujoy_match(struct device *parent, void *match, void *aux)
+{
+ struct uhidev_attach_arg *uha = (struct
Re: New ujoy(4) device for USB gamecontrollers
On Fri, 15 Jan 2021 11:37:47 -0500 Bryan Steele wrote: > On Fri, Jan 15, 2021 at 06:23:01AM -0700, Thomas Frohwein wrote: > > On Sat, Jan 09, 2021 at 10:16:16AM +0100, Marcus Glocker wrote: > > > 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. > > > > So with the hid_is_collection() problem not easy to mitigate [1], > > should we table the ujoy(4) proposal for now pending a fix for the > > problems with the PS4 controller? Or is this interesting enough for > > some to work on moving forward despite this issue and finding a > > solution for this specific (and in some ways unusual) device later? > > > > Considering the hid_is_collection() fix from NetBSD proposed by Marcus > fixes the issue with ujoy(4) but breaks some other drivers (imt(4) > and ims(4)), could we not inline it into ujoy(4) for now? It's fairly > small helper function. Like hid_is_joy_collection()? I think that could be an XXX workaround until somebody finds a proper, generic fix for hid_is_collection() ...
Re: New ujoy(4) device for USB gamecontrollers
On Fri, 15 Jan 2021 06:23:01 -0700 Thomas Frohwein wrote: > On Sat, Jan 09, 2021 at 10:16:16AM +0100, Marcus Glocker wrote: > > 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. > > So with the hid_is_collection() problem not easy to mitigate [1], > should we table the ujoy(4) proposal for now pending a fix for the > problems with the PS4 controller? Or is this interesting enough for > some to work on moving forward despite this issue and finding a > solution for this specific (and in some ways unusual) device later? > > 3-4 have tested and reported to me so far. It seems so far that the > only new breakage is with the PS4 controller, and there is probably > another solution that can be found later that doesn't break other > drivers like [1]? > > [1] https://marc.info/?l=openbsd-tech=161043081617336=mbox What I don't like when we import ujoy(4) as is; It will break currently supported devices like the PS4 controller. The kernel change won't even be the problem since the PS4 controller still would attach to uhid(4), same as before. But after the import we will need to start changing the ports for those controllers which attach correctly, and require the ports to query the new ujoy(4) device. At this point the PS4 controller won't work anymore ... And unfortunately it doesn't look like there is a proper HID fix in sight currently which would work for all the devices.
Re: New ujoy(4) device for USB gamecontrollers
On Fri, Jan 15, 2021 at 06:23:01AM -0700, Thomas Frohwein wrote: > On Sat, Jan 09, 2021 at 10:16:16AM +0100, Marcus Glocker wrote: > > 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. > > So with the hid_is_collection() problem not easy to mitigate [1], > should we table the ujoy(4) proposal for now pending a fix for the > problems with the PS4 controller? Or is this interesting enough for > some to work on moving forward despite this issue and finding a > solution for this specific (and in some ways unusual) device later? Considering the hid_is_collection() fix from NetBSD proposed by Marcus fixes the issue with ujoy(4) but breaks some other drivers (imt(4) and ims(4)), could we not inline it into ujoy(4) for now? It's fairly small helper function. Like hid_is_joy_collection()? -Bryan. > 3-4 have tested and reported to me so far. It seems so far that the > only new breakage is with the PS4 controller, and there is probably > another solution that can be found later that doesn't break other > drivers like [1]? > > [1] https://marc.info/?l=openbsd-tech=161043081617336=mbox > > > > > 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
Re: New ujoy(4) device for USB gamecontrollers
On Fri, Jan 15, 2021 at 01:23:01PM GMT, Thomas Frohwein wrote: > On Sat, Jan 09, 2021 at 10:16:16AM +0100, Marcus Glocker wrote: > > 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. > > So with the hid_is_collection() problem not easy to mitigate [1], > should we table the ujoy(4) proposal for now pending a fix for the > problems with the PS4 controller? Or is this interesting enough for > some to work on moving forward despite this issue and finding a > solution for this specific (and in some ways unusual) device later? > > 3-4 have tested and reported to me so far. It seems so far that the > only new breakage is with the PS4 controller, and there is probably > another solution that can be found later that doesn't break other > drivers like [1]? > > [1] https://marc.info/?l=openbsd-tech=161043081617336=mbox Hi Thomas, Hadn't had a chance to test your diff yet but, FWIW, I rely on PS4 controller working so would appreciate if it remained in a working state :^) Cheers, Raf
Re: New ujoy(4) device for USB gamecontrollers
On Sat, Jan 09, 2021 at 10:16:16AM +0100, Marcus Glocker wrote: > 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. So with the hid_is_collection() problem not easy to mitigate [1], should we table the ujoy(4) proposal for now pending a fix for the problems with the PS4 controller? Or is this interesting enough for some to work on moving forward despite this issue and finding a solution for this specific (and in some ways unusual) device later? 3-4 have tested and reported to me so far. It seems so far that the only new breakage is with the PS4 controller, and there is probably another solution that can be found later that doesn't break other drivers like [1]? [1] https://marc.info/?l=openbsd-tech=161043081617336=mbox > > 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
Re: New ujoy(4) device for USB gamecontrollers
On Sat, Jan 09, 2021 at 10:16:16AM +0100, Marcus Glocker wrote: [...] > 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: Nice find. [...] > 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. While I can't speak for all 11,000 packages, I am yet to encounter a port that uses gamecontrollers/joysticks without going through sdl or sdl2. And for those 2 ports it's literally a 1-line diff each. I'm not sure if looking at all the ports with 'usbhid' in WANTLIB would be helpful. I have a list of 55 of those from sqlports, excluding anything with 'sdl' in its name.
Re: New ujoy(4) device for USB gamecontrollers
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
Re: New ujoy(4) device for USB gamecontrollers
On Thu, 7 Jan 2021 10:20:34 -0700 Thomas Frohwein wrote: > On Wed, Jan 06, 2021 at 10:48:58PM +0100, Marcus Glocker wrote: > > [...] > > > The implementation as such looks fine to me. > > But I quickly gave the diff a spin before on amd64 using my PS > > controller, and the result is not what I would expect. > > > > With uhid, I can access the controller on /dev/uhid6. The attach > > looks like this: > > > > imac /bsd: uaudio0 at uhub4 port 4 configuration 1 interface 1 "Sony > > Interactive Entertainment Wireless Controller" rev 2.00/1.00 addr 6 > > imac /bsd: uaudio0: class v1, full-speed, sync, channels: 2 play, 1 > > rec, 4 ctls > > imac /bsd: audio1 at uaudio0 > > imac /bsd: uhidev6 at uhub4 port 4 configuration 1 interface 3 "Sony > > Interactive Entertainment Wireless Controller" rev 2.00/1.00 addr 6 > > imac /bsd: uhidev6: iclass 3/0, 180 report ids > > imac /bsd: uhid6 at uhidev6 reportid 1: input=63, output=0, > > feature=0 > > [...] > > > imac /bsd: uhid50 at uhidev6 reportid 179: input=0, output=0, > > feature=63 imac /bsd: uhid51 at uhidev6 reportid 180: input=0, > > output=0, feature=63 > > > > ujoy instead attached to previous uhid51, and there it is of no use > > obviously. I still can access the controller through uhid6. The > > attach with ujoy looks like this: > > > > imac /bsd: uaudio0 at uhub4 port 4 configuration 1 interface 1 "Sony > > Interactive Entertainment Wireless Controller" rev 2.00/1.00 addr 6 > > imac /bsd: uaudio0: class v1, full-speed, sync, channels: 2 play, 1 > > rec, 4 ctls > > imac /bsd: audio1 at uaudio0 > > imac /bsd: uhidev6 at uhub4 port 4 configuration 1 interface 3 "Sony > > Interactive Entertainment Wireless Controller" rev 2.00/1.00 addr 6 > > imac /bsd: uhidev6: iclass 3/0, 180 report ids > > [...] > > > imac /bsd: uhid50 at uhidev6 reportid 179: input=0, output=0, > > feature=63 imac /bsd: ujoy0 at uhidev6 reportid 180: input=0, > > output=0, feature=63 > > > > I haven't analyzed yet what happens in the code. > > I can provide an lsusb of the controller if it's more obvious to > > you. > > 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. Bus 001 Device 006: ID 054c:09cc Sony Corp. Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 2.00 bDeviceClass0 (Defined at Interface level) bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize064 idVendor 0x054c Sony Corp. idProduct 0x09cc bcdDevice1.00 iManufacturer 1 Sony Interactive Entertainment iProduct2 Wireless Controller iSerial 0 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 225 bNumInterfaces 4 bConfigurationValue 1 iConfiguration 0 bmAttributes 0xc0 Self Powered MaxPower 500mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 0 bNumEndpoints 0 bInterfaceClass 1 Audio bInterfaceSubClass 1 Control Device bInterfaceProtocol 0 iInterface 0 AudioControl Interface Descriptor: bLength10 bDescriptorType36 bDescriptorSubtype 1 (HEADER) bcdADC 1.00 wTotalLength 71 bInCollection 2 baInterfaceNr( 0) 1 baInterfaceNr( 1) 2 AudioControl Interface Descriptor: bLength12 bDescriptorType36 bDescriptorSubtype 2 (INPUT_TERMINAL) bTerminalID 1 wTerminalType 0x0101 USB Streaming bAssocTerminal 6 bNrChannels 2 wChannelConfig 0x0003 Left Front (L) Right Front (R) iChannelNames 0 iTerminal 0 AudioControl Interface Descriptor: bLength10 bDescriptorType36 bDescriptorSubtype 6 (FEATURE_UNIT) bUnitID 2 bSourceID 1 bControlSize1 bmaControls( 0) 0x03 Mute Control Volume Control bmaControls( 1) 0x00 bmaControls( 2) 0x00 iFeature0 AudioControl Interface Descriptor: bLength 9 bDescriptorType36 bDescriptorSubtype 3 (OUTPUT_TERMINAL)
Re: New ujoy(4) device for USB gamecontrollers
On Wed, Jan 06, 2021 at 10:48:58PM +0100, Marcus Glocker wrote: [...] > The implementation as such looks fine to me. > But I quickly gave the diff a spin before on amd64 using my PS > controller, and the result is not what I would expect. > > With uhid, I can access the controller on /dev/uhid6. The attach looks > like this: > > imac /bsd: uaudio0 at uhub4 port 4 configuration 1 interface 1 "Sony > Interactive Entertainment Wireless Controller" rev 2.00/1.00 addr 6 > imac /bsd: uaudio0: class v1, full-speed, sync, channels: 2 play, 1 rec, > 4 ctls > imac /bsd: audio1 at uaudio0 > imac /bsd: uhidev6 at uhub4 port 4 configuration 1 interface 3 "Sony > Interactive Entertainment Wireless Controller" rev 2.00/1.00 addr 6 > imac /bsd: uhidev6: iclass 3/0, 180 report ids > imac /bsd: uhid6 at uhidev6 reportid 1: input=63, output=0, feature=0 [...] > imac /bsd: uhid50 at uhidev6 reportid 179: input=0, output=0, feature=63 > imac /bsd: uhid51 at uhidev6 reportid 180: input=0, output=0, feature=63 > > ujoy instead attached to previous uhid51, and there it is of no use > obviously. I still can access the controller through uhid6. The attach > with ujoy looks like this: > > imac /bsd: uaudio0 at uhub4 port 4 configuration 1 interface 1 "Sony > Interactive Entertainment Wireless Controller" rev 2.00/1.00 addr 6 > imac /bsd: uaudio0: class v1, full-speed, sync, channels: 2 play, 1 rec, > 4 ctls > imac /bsd: audio1 at uaudio0 > imac /bsd: uhidev6 at uhub4 port 4 configuration 1 interface 3 "Sony > Interactive Entertainment Wireless Controller" rev 2.00/1.00 addr 6 > imac /bsd: uhidev6: iclass 3/0, 180 report ids [...] > imac /bsd: uhid50 at uhidev6 reportid 179: input=0, output=0, feature=63 > imac /bsd: ujoy0 at uhidev6 reportid 180: input=0, output=0, feature=63 > > I haven't analyzed yet what happens in the code. > I can provide an lsusb of the controller if it's more obvious to you. 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!
Re: New ujoy(4) device for USB gamecontrollers
On Mon, Dec 28, 2020 at 05:03:14PM -0700, Thomas Frohwein wrote: > Hi, > > This is a diff to propose a new device type for USB gamecontrollers, > 'ujoy'. > > Rationale > - > > Since the tightening of security around USB devices, USB > gamecontrollers that generally attach to the kitchen-sink uhid device > don't work out of the box anymore since read permissions are absent. As > a consequence, USB gamecontrollers don't work out of the box anymore; > requiring a root user to find what uhid device the controller is > attached to and chmod +r that one. This poses a barrier to using those > devices, as well as in my opinion a security risk by enticing users to > indiscriminately do '# chmod +r /dev/uhid*' in order to not have to > re-apply permissions on different uhid devices multiple times. > > The proposed solution is inspired by the implementation of fido(4) [1]; > a similarly narrow use case for a certain type of USB devices. It > creates a device for gamecontrollers that has the minimum required > permissions, but won't require chmod(1)'ing device files to get the > controller working. > > Implementation in this Diff > --- > > This diff is largely based on reyk's commit for the fido(4) device from > December 2019 [1]. It creates a ujoy device and attaches USB devices > identified as HUG_JOYSTICK or HUG_GAME_PAD. > > ujoyopen() is set up such that it only allows read-only access. If it is > opened with write permissions, it should error with EPERM. Only a > subset of ioctls is allowed in ujoyioctl(), based on what I could find > that is used in devel/sdl2 (USB_GET_DEVICEINFO,USB_GET_REPORT, > USB_GET_REPORT_DESC,USB_GET_REPORT_ID), as well as FIONBIO and > FIOASYNC. > > This contains conf.c and MAKEDEV.md bits for the same architectures as > fido(4). A small man page, as well as updates to other pertinent man > pages (usb(4), uhidev(4)) are included, again following the example of > the commits for the fido(4) device. > > Credit to brynet@ who helped me extensively with this diff. > > Testing > --- > > A simple way to test this without needing to do a full release is > building a kernel with the diff and booting into it, then creating the > updated MAKEDEV script with: > > $ cd /usr/src/etc/etc.amd64/ > $ make# this will create MAKEDEV in etc.amd64 > $ cd /dev > # sh /usr/src/etc/etc.amd64/MAKEDEV ujoy > > This creates 2 device ujoy/0 and ujoy/1 by default. The device is > read-only by default (444). > > Plug in your USB gamecontroller and check in dmesg that it attaches as > ujoy, not uhid. > > The simplest way to test is with usbhidctl(1): > > $ usbhidctl -f /dev/ujoy/0 -l > > This will loop through printing the state of buttons, sticks etc. until > exited with Crtl-C. > > Another way to test is with games/sdl-jstest. As the SDL backends use > /dev/uhid* to find gamecontrollers, the way I tested this is by > repurposing /dev/uhid0 with mknod(8) (note major 100 is for amd64): > > $ cd /dev > # rm uhid0 > # mknod -m 444 uhid0 c 100 0 > > I put the diff through a release(8) without xenocara on amd64, and it > built base without problems and /dev/MAKEDEV and the /dev/ujoy directory > all look correct. > > I have tested usbhidctl and sdl-jstest with an XBox 360 gamepad and the > Logitech F310 gamepad, the latter both in DInput and XInput mode. All > of those work as expected. > > Issues/Follow-up Tasks > -- > > As ujoy devices don't attach to /dev/uhid* anymore, ports that use > gamecontrollers will need to be patched. This seems to be mostly (if > not exclusively) sdl/sdl2-based ports, so I anticipate that patching > these 2 ports will take care of the bulk of the use cases. > > On other OS's, writing to gamecontrollers is used at times for the > "rumble" functionality of the controller, or setting LEDs (like the > circle LED on XBox 360 controllers that can reflect what player number > the gamepad is for). Those have to my knowledge never worked on OpenBSD > and are not of any immediate interest as far as I can tell. > > ok? comments? The implementation as such looks fine to me. But I quickly gave the diff a spin before on amd64 using my PS controller, and the result is not what I would expect. With uhid, I can access the controller on /dev/uhid6. The attach looks like this: imac /bsd: uaudio0 at uhub4 port 4 configuration 1 interface 1 "Sony Interactive Entertainment Wireless Controller" rev 2.00/1.00 addr 6 imac /bsd: uaudio0: class v1, full-speed, sync, channels: 2 play, 1 rec, 4 ctls imac /bsd: audio1 at uaudio0 imac /bsd: uhidev6 at uhub4 port 4 configuration 1 interface 3 "Sony Interactive Entertainment Wireless Controller" rev 2.00/1.00 addr 6 imac /bsd: uhidev6: iclass 3/0, 180 report ids imac /bsd: uhid6 at uhidev6 reportid 1: input=63, output=0, feature=0 imac /bsd: uhid7 at uhidev6 reportid 2: input=0, output=0, feature=36 imac /bsd: uhid8 at uhidev6 reportid 4: input=0,
Re: New ujoy(4) device for USB gamecontrollers
On Mon, Dec 28, 2020 at 05:03:14PM -0700, Thomas Frohwein wrote: > Hi, > > This is a diff to propose a new device type for USB gamecontrollers, > 'ujoy'. > > Rationale > - > > Since the tightening of security around USB devices, USB > gamecontrollers that generally attach to the kitchen-sink uhid device > don't work out of the box anymore since read permissions are absent. As > a consequence, USB gamecontrollers don't work out of the box anymore; > requiring a root user to find what uhid device the controller is > attached to and chmod +r that one. This poses a barrier to using those > devices, as well as in my opinion a security risk by enticing users to > indiscriminately do '# chmod +r /dev/uhid*' in order to not have to > re-apply permissions on different uhid devices multiple times. > > The proposed solution is inspired by the implementation of fido(4) [1]; > a similarly narrow use case for a certain type of USB devices. It > creates a device for gamecontrollers that has the minimum required > permissions, but won't require chmod(1)'ing device files to get the > controller working. > > Implementation in this Diff > --- > > This diff is largely based on reyk's commit for the fido(4) device from > December 2019 [1]. It creates a ujoy device and attaches USB devices > identified as HUG_JOYSTICK or HUG_GAME_PAD. > > ujoyopen() is set up such that it only allows read-only access. If it is > opened with write permissions, it should error with EPERM. Only a > subset of ioctls is allowed in ujoyioctl(), based on what I could find > that is used in devel/sdl2 (USB_GET_DEVICEINFO,USB_GET_REPORT, > USB_GET_REPORT_DESC,USB_GET_REPORT_ID), as well as FIONBIO and > FIOASYNC. > > This contains conf.c and MAKEDEV.md bits for the same architectures as > fido(4). A small man page, as well as updates to other pertinent man > pages (usb(4), uhidev(4)) are included, again following the example of > the commits for the fido(4) device. > > Credit to brynet@ who helped me extensively with this diff. > > Testing > --- > > A simple way to test this without needing to do a full release is > building a kernel with the diff and booting into it, then creating the > updated MAKEDEV script with: > > $ cd /usr/src/etc/etc.amd64/ > $ make# this will create MAKEDEV in etc.amd64 > $ cd /dev > # sh /usr/src/etc/etc.amd64/MAKEDEV ujoy > > This creates 2 device ujoy/0 and ujoy/1 by default. The device is > read-only by default (444). > > Plug in your USB gamecontroller and check in dmesg that it attaches as > ujoy, not uhid. > > The simplest way to test is with usbhidctl(1): > > $ usbhidctl -f /dev/ujoy/0 -l > > This will loop through printing the state of buttons, sticks etc. until > exited with Crtl-C. > > Another way to test is with games/sdl-jstest. As the SDL backends use > /dev/uhid* to find gamecontrollers, the way I tested this is by > repurposing /dev/uhid0 with mknod(8) (note major 100 is for amd64): > > $ cd /dev > # rm uhid0 > # mknod -m 444 uhid0 c 100 0 > > I put the diff through a release(8) without xenocara on amd64, and it > built base without problems and /dev/MAKEDEV and the /dev/ujoy directory > all look correct. > > I have tested usbhidctl and sdl-jstest with an XBox 360 gamepad and the > Logitech F310 gamepad, the latter both in DInput and XInput mode. All > of those work as expected. > > Issues/Follow-up Tasks > -- > > As ujoy devices don't attach to /dev/uhid* anymore, ports that use > gamecontrollers will need to be patched. This seems to be mostly (if > not exclusively) sdl/sdl2-based ports, so I anticipate that patching > these 2 ports will take care of the bulk of the use cases. > > On other OS's, writing to gamecontrollers is used at times for the > "rumble" functionality of the controller, or setting LEDs (like the > circle LED on XBox 360 controllers that can reflect what player number > the gamepad is for). Those have to my knowledge never worked on OpenBSD > and are not of any immediate interest as far as I can tell. > > ok? comments? > > [1] https://marc.info/?l=openbsd-cvs=157658815523208=mbox Nice work! I think this makes a lot of sense (slightly biased), I agree that it felt very wrong advising users to change permissions for uhid* when they plugged in a gamepad, exposing a bunch of kernel functionality to games that don't need it. Having them Just-Work (tm) by default, while doing so in a clean and safe way will help the experience a lot for gamers (and slacking developers) alike! :-) Some ports patches are still required if this goes in, as mentioned devel/sdl2 at the very least, but the current situation as it stands controllers already don't work out of the box. thfr@ already has my ok and some general "good idea" from others, but feedback is appreciated, as well as any tips for getting this ready to go in. -Bryan. > Index: etc/MAKEDEV.common >
New ujoy(4) device for USB gamecontrollers
Hi,
This is a diff to propose a new device type for USB gamecontrollers,
'ujoy'.
Rationale
-
Since the tightening of security around USB devices, USB
gamecontrollers that generally attach to the kitchen-sink uhid device
don't work out of the box anymore since read permissions are absent. As
a consequence, USB gamecontrollers don't work out of the box anymore;
requiring a root user to find what uhid device the controller is
attached to and chmod +r that one. This poses a barrier to using those
devices, as well as in my opinion a security risk by enticing users to
indiscriminately do '# chmod +r /dev/uhid*' in order to not have to
re-apply permissions on different uhid devices multiple times.
The proposed solution is inspired by the implementation of fido(4) [1];
a similarly narrow use case for a certain type of USB devices. It
creates a device for gamecontrollers that has the minimum required
permissions, but won't require chmod(1)'ing device files to get the
controller working.
Implementation in this Diff
---
This diff is largely based on reyk's commit for the fido(4) device from
December 2019 [1]. It creates a ujoy device and attaches USB devices
identified as HUG_JOYSTICK or HUG_GAME_PAD.
ujoyopen() is set up such that it only allows read-only access. If it is
opened with write permissions, it should error with EPERM. Only a
subset of ioctls is allowed in ujoyioctl(), based on what I could find
that is used in devel/sdl2 (USB_GET_DEVICEINFO,USB_GET_REPORT,
USB_GET_REPORT_DESC,USB_GET_REPORT_ID), as well as FIONBIO and
FIOASYNC.
This contains conf.c and MAKEDEV.md bits for the same architectures as
fido(4). A small man page, as well as updates to other pertinent man
pages (usb(4), uhidev(4)) are included, again following the example of
the commits for the fido(4) device.
Credit to brynet@ who helped me extensively with this diff.
Testing
---
A simple way to test this without needing to do a full release is
building a kernel with the diff and booting into it, then creating the
updated MAKEDEV script with:
$ cd /usr/src/etc/etc.amd64/
$ make # this will create MAKEDEV in etc.amd64
$ cd /dev
# sh /usr/src/etc/etc.amd64/MAKEDEV ujoy
This creates 2 device ujoy/0 and ujoy/1 by default. The device is
read-only by default (444).
Plug in your USB gamecontroller and check in dmesg that it attaches as
ujoy, not uhid.
The simplest way to test is with usbhidctl(1):
$ usbhidctl -f /dev/ujoy/0 -l
This will loop through printing the state of buttons, sticks etc. until
exited with Crtl-C.
Another way to test is with games/sdl-jstest. As the SDL backends use
/dev/uhid* to find gamecontrollers, the way I tested this is by
repurposing /dev/uhid0 with mknod(8) (note major 100 is for amd64):
$ cd /dev
# rm uhid0
# mknod -m 444 uhid0 c 100 0
I put the diff through a release(8) without xenocara on amd64, and it
built base without problems and /dev/MAKEDEV and the /dev/ujoy directory
all look correct.
I have tested usbhidctl and sdl-jstest with an XBox 360 gamepad and the
Logitech F310 gamepad, the latter both in DInput and XInput mode. All
of those work as expected.
Issues/Follow-up Tasks
--
As ujoy devices don't attach to /dev/uhid* anymore, ports that use
gamecontrollers will need to be patched. This seems to be mostly (if
not exclusively) sdl/sdl2-based ports, so I anticipate that patching
these 2 ports will take care of the bulk of the use cases.
On other OS's, writing to gamecontrollers is used at times for the
"rumble" functionality of the controller, or setting LEDs (like the
circle LED on XBox 360 controllers that can reflect what player number
the gamepad is for). Those have to my knowledge never worked on OpenBSD
and are not of any immediate interest as far as I can tell.
ok? comments?
[1] https://marc.info/?l=openbsd-cvs=157658815523208=mbox
Index: etc/MAKEDEV.common
===
RCS file: /cvs/src/etc/MAKEDEV.common,v
retrieving revision 1.111
diff -u -p -r1.111 MAKEDEV.common
--- etc/MAKEDEV.common 6 Jul 2020 06:11:26 - 1.111
+++ etc/MAKEDEV.common 28 Dec 2020 03:25:04 -
@@ -181,6 +181,7 @@ dnl
target(usb, usb, 0, 1, 2, 3, 4, 5, 6, 7)dnl
target(usb, uhid, 0, 1, 2, 3, 4, 5, 6, 7)dnl
twrget(usb, fido, fido)dnl
+twrget(usb, ujoy, ujoy)dnl
target(usb, ulpt, 0, 1)dnl
target(usb, ugen, 0, 1, 2, 3, 4, 5, 6, 7)dnl
target(usb, ttyU, 0, 1, 2, 3)dnl
@@ -365,6 +366,10 @@ __devitem(fido, fido, fido/* nodes, fido
_mkdev(fido, fido, {-RMlist[${#RMlist[*]}]=";mkdir -p fido;rm -f" n=0
while [ $n -lt 4 ];do M fido/$n c major_fido_c $n 666;n=Add($n, 1);done
MKlist[${#MKlist[*]}]=";chmod 555 fido"-})dnl
+__devitem(ujoy, ujoy, ujoy/* nodes, ujoy)dnl
+_mkdev(ujoy, ujoy, {-RMlist[${#RMlist[*]}]=";mkdir -p ujoy;rm -f" n=0
+ while [ $n -lt 2 ];do M ujoy/$n c major_ujoy_c $n 444;n=Add($n, 1);done
+ MKlist[${#MKlist[*]}]=";chmod
