Hi Jeremy,
On 18/10/13(Fri) 09:11, Jeremy Evans wrote:
> This was originally submitted by Joe Gidi in November 2010, based on a
> FreeBSD commit by Ed Schouten from back in December 2005. See
> http://marc.info/?l=openbsd-tech&m=128924886803756&w=2 for previous
> thread. The only comment was from tedu@, that SMALL_KERNEL should be
> added, which I've done but I'm not sure correctly.
>
> Tested on amd64 using usbhidctl and some SDL-based emulators (mednafen,
> snes9x-gtk, desmume). The controller works fine except that the
> directional pad is not processed as a hat but as a series of buttons
> (that usbhidctl sees but SDL seems not to recognize). Also, the bottom
> L/R triggers are axes instead of buttons.
>
> I've built a release with this, and checked that the amd64 floppy
> still fits.
Good stuff, some comments below.
> Index: uhidev.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/uhidev.c,v
> retrieving revision 1.46
> diff -u -p -r1.46 uhidev.c
> --- uhidev.c 20 Sep 2013 15:34:50 -0000 1.46
> +++ uhidev.c 17 Oct 2013 23:32:55 -0000
> @@ -55,8 +55,11 @@
>
> #include <dev/usb/uhidev.h>
>
> -/* Report descriptor for broken Wacom Graphire */
> +/* Replacement report descriptors for devices shipped with broken ones */
> #include <dev/usb/ugraphire_rdesc.h>
> +#ifndef SMALL_KERNEL
> +#include <dev/usb/uxb360gp_rdesc.h>
> +#endif /* !SMALL_KERNEL */
Can you also include the ugraphire header in the #ifndef block, since we
don't include ums(4) in the ramdisk, this will same even more space!
> #ifdef UHIDEV_DEBUG
> #define DPRINTF(x) do { if (uhidevdebug) printf x; } while (0)
> @@ -99,8 +102,17 @@ uhidev_match(struct device *parent, void
> if (uaa->iface == NULL)
> return (UMATCH_NONE);
> id = usbd_get_interface_descriptor(uaa->iface);
> - if (id == NULL || id->bInterfaceClass != UICLASS_HID)
> - return (UMATCH_NONE);
> + if (id == NULL)
> + return (UMATCH_NONE);
> + if (id->bInterfaceClass != UICLASS_HID) {
> +#ifndef SMALL_KERNEL
> + /* The Xbox 360 gamepad doesn't use the HID class. */
> + if (id->bInterfaceClass != UICLASS_VENDOR ||
> + id->bInterfaceSubClass != UISUBCLASS_XBOX360_CONTROLLER
> ||
> + id->bInterfaceProtocol != UIPROTO_XBOX360_GAMEPAD)
> +#endif /* !SMALL_KERNEL */
I would prefer to add another quirk to usbd_quirks something like
UQ_FORCE_HID, rather than hardcoding the device id here.
> + return (UMATCH_NONE);
> + }
> if (usbd_get_quirks(uaa->device)->uq_flags & UQ_BAD_HID)
> return (UMATCH_NONE);
> if (uaa->matchlvl)
> @@ -200,7 +212,16 @@ uhidev_attach(struct device *parent, str
> /* Keep descriptor */
> break;
> }
> +#ifndef SMALL_KERNEL
> + } else if (id->bInterfaceClass == UICLASS_VENDOR &&
> + id->bInterfaceSubClass == UISUBCLASS_XBOX360_CONTROLLER &&
> + id->bInterfaceProtocol == UIPROTO_XBOX360_GAMEPAD) {
> + /* The Xbox 360 gamepad has no report descriptor. */
> + size = sizeof uhid_xb360gp_report_descr;
> + descptr = uhid_xb360gp_report_descr;
> +#endif /* !SMALL_KERNEL */
Can you merge this chunk into the switch just before since it will also
be #ifndef, and also use the uaa argument like for wacom devices?
> }
> +
>
> if (descptr) {
> desc = malloc(size, M_USBDEV, M_NOWAIT);
> Index: usb.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/usb.h,v
> retrieving revision 1.44
> diff -u -p -r1.44 usb.h
> --- usb.h 17 Apr 2013 11:53:10 -0000 1.44
> +++ usb.h 17 Oct 2013 18:11:59 -0000
> @@ -491,7 +491,8 @@ typedef struct {
> #define UIPROTO_IRDA 0
>
> #define UICLASS_VENDOR 0xff
> -
> +#define UISUBCLASS_XBOX360_CONTROLLER 0x5d
> +#define UIPROTO_XBOX360_GAMEPAD 0x01
I am not a big fan of having vendor/device specific defines in usb.h
which should be a generic header that's also included by userland.
> #define USB_HUB_MAX_DEPTH 5
>
> Index: uxb360gp_rdesc.h
> ===================================================================
> RCS file: uxb360gp_rdesc.h
> diff -N uxb360gp_rdesc.h
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ uxb360gp_rdesc.h 17 Oct 2013 18:23:56 -0000
> @@ -0,0 +1,126 @@
> +/*-
> + * Copyright (c) 2005 Ed Schouten <e...@freebsd.org>
> + * All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
> + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> + * SUCH DAMAGE.
> + *
> + * $FreeBSD: src/sys/dev/usb/uxb360gp_rdesc.h,v 1.3 2008/05/24 18:35:55 ed
> Exp $
> + */
> +
> +/*
> + * The descriptor has no output report format, thus preventing you from
> + * controlling the LEDs and the built-in rumblers.
> + */
> +#ifndef SMALL_KERNEL
You don't need that since you already protect the header inclusion.
> +static const uByte uhid_xb360gp_report_descr[] = {
> + 0x05, 0x01, /* USAGE PAGE (Generic Desktop) */
> + 0x09, 0x05, /* USAGE (Gamepad) */
> + 0xa1, 0x01, /* COLLECTION (Application) */
> + /* Unused */
> + 0x75, 0x08, /* REPORT SIZE (8) */
> + 0x95, 0x01, /* REPORT COUNT (1) */
> + 0x81, 0x01, /* INPUT (Constant) */
> + /* Byte count */
> + 0x75, 0x08, /* REPORT SIZE (8) */
> + 0x95, 0x01, /* REPORT COUNT (1) */
> + 0x05, 0x01, /* USAGE PAGE (Generic Desktop) */
> + 0x09, 0x3b, /* USAGE (Byte Count) */
> + 0x81, 0x01, /* INPUT (Constant) */
> + /* D-Pad */
> + 0x05, 0x01, /* USAGE PAGE (Generic Desktop) */
> + 0x09, 0x01, /* USAGE (Pointer) */
> + 0xa1, 0x00, /* COLLECTION (Physical) */
> + 0x75, 0x01, /* REPORT SIZE (1) */
> + 0x15, 0x00, /* LOGICAL MINIMUM (0) */
> + 0x25, 0x01, /* LOGICAL MAXIMUM (1) */
> + 0x35, 0x00, /* PHYSICAL MINIMUM (0) */
> + 0x45, 0x01, /* PHYSICAL MAXIMUM (1) */
> + 0x95, 0x04, /* REPORT COUNT (4) */
> + 0x05, 0x01, /* USAGE PAGE (Generic Desktop) */
> + 0x09, 0x90, /* USAGE (D-Pad Up) */
> + 0x09, 0x91, /* USAGE (D-Pad Down) */
> + 0x09, 0x93, /* USAGE (D-Pad Left) */
> + 0x09, 0x92, /* USAGE (D-Pad Right) */
> + 0x81, 0x02, /* INPUT (Data, Variable, Absolute) */
> + 0xc0, /* END COLLECTION */
> + /* Buttons 5-11 */
> + 0x75, 0x01, /* REPORT SIZE (1) */
> + 0x15, 0x00, /* LOGICAL MINIMUM (0) */
> + 0x25, 0x01, /* LOGICAL MAXIMUM (1) */
> + 0x35, 0x00, /* PHYSICAL MINIMUM (0) */
> + 0x45, 0x01, /* PHYSICAL MAXIMUM (1) */
> + 0x95, 0x07, /* REPORT COUNT (7) */
> + 0x05, 0x09, /* USAGE PAGE (Button) */
> + 0x09, 0x08, /* USAGE (Button 8) */
> + 0x09, 0x07, /* USAGE (Button 7) */
> + 0x09, 0x09, /* USAGE (Button 9) */
> + 0x09, 0x0a, /* USAGE (Button 10) */
> + 0x09, 0x05, /* USAGE (Button 5) */
> + 0x09, 0x06, /* USAGE (Button 6) */
> + 0x09, 0x0b, /* USAGE (Button 11) */
> + 0x81, 0x02, /* INPUT (Data, Variable, Absolute) */
> + /* Unused */
> + 0x75, 0x01, /* REPORT SIZE (1) */
> + 0x95, 0x01, /* REPORT COUNT (1) */
> + 0x81, 0x01, /* INPUT (Constant) */
> + /* Buttons 1-4 */
> + 0x75, 0x01, /* REPORT SIZE (1) */
> + 0x15, 0x00, /* LOGICAL MINIMUM (0) */
> + 0x25, 0x01, /* LOGICAL MAXIMUM (1) */
> + 0x35, 0x00, /* PHYSICAL MINIMUM (0) */
> + 0x45, 0x01, /* PHYSICAL MAXIMUM (1) */
> + 0x95, 0x04, /* REPORT COUNT (4) */
> + 0x05, 0x09, /* USAGE PAGE (Button) */
> + 0x19, 0x01, /* USAGE MINIMUM (Button 1) */
> + 0x29, 0x04, /* USAGE MAXIMUM (Button 4) */
> + 0x81, 0x02, /* INPUT (Data, Variable, Absolute) */
> + /* Triggers */
> + 0x75, 0x08, /* REPORT SIZE (8) */
> + 0x15, 0x00, /* LOGICAL MINIMUM (0) */
> + 0x26, 0xff, 0x00, /* LOGICAL MAXIMUM (255) */
> + 0x35, 0x00, /* PHYSICAL MINIMUM (0) */
> + 0x46, 0xff, 0x00, /* PHYSICAL MAXIMUM (255) */
> + 0x95, 0x02, /* REPORT SIZE (2) */
> + 0x05, 0x01, /* USAGE PAGE (Generic Desktop) */
> + 0x09, 0x32, /* USAGE (Z) */
> + 0x09, 0x35, /* USAGE (Rz) */
> + 0x81, 0x02, /* INPUT (Data, Variable, Absolute) */
> + /* Sticks */
> + 0x75, 0x10, /* REPORT SIZE (16) */
> + 0x16, 0x00, 0x80, /* LOGICAL MINIMUM (-32768) */
> + 0x26, 0xff, 0x7f, /* LOGICAL MAXIMUM (32767) */
> + 0x36, 0x00, 0x80, /* PHYSICAL MINIMUM (-32768) */
> + 0x46, 0xff, 0x7f, /* PHYSICAL MAXIMUM (32767) */
> + 0x95, 0x04, /* REPORT COUNT (4) */
> + 0x05, 0x01, /* USAGE PAGE (Generic Desktop) */
> + 0x09, 0x30, /* USAGE (X) */
> + 0x09, 0x31, /* USAGE (Y) */
> + 0x09, 0x33, /* USAGE (Rx) */
> + 0x09, 0x34, /* USAGE (Ry) */
> + 0x81, 0x02, /* INPUT (Data, Variable, Absolute) */
> + /* Unused */
> + 0x75, 0x30, /* REPORT SIZE (48) */
> + 0x95, 0x01, /* REPORT COUNT (1) */
> + 0x81, 0x01, /* INPUT (Constant) */
> + 0xc0, /* END COLLECTION */
> +};
> +#endif /* !SMALL_KERNEL */
>
