The current USB implementation uses a quirks table that is compiled into the kernel. Under most circumstances this approach works. However, some manufacturers of USB devices have reused keys used in the table and hence changing the compiled in table will result in an inappropriate entries being present. A localised method of changing the quirks values without recompiling their kernel would assist developers and users facing this problem. USB developers in particular can benefit from the ability to prevent a device inappropriately identifying itself as a hid device without having to recompile their kernel. The supplied patch uses entries in the kernel environment to create a dynamic quirks table. The data is available at boot time so devices that are connected across a reboot are correctly handled. This table can also be updated after boot time by calling an IOCTL.
Hi, Maybe you can prefix the string version of the quirks by the USB module name ? For example: MS_REVZ -> UMS_REVZ AU_NO_FRAC -> UAUDIO_NO_FRAC Then it is easier to know where the quirk belongs. Add a short description of what the quirk does in the "usb" manpage. There might be a race condition reading and writing the quirks. Probably a mutex is appropriate. You can improve the quirk string to numerical conversion code by using binary search. Else your patch looks fine to me. --HPS On Wednesday 16 April 2008, Maurice Castro wrote: > >Number: 122819 > >Category: usb > >Synopsis: Patch to provide dynamic additions to the usb quirks table > >Confidential: no > >Severity: non-critical > >Priority: medium > >Responsible: freebsd-usb > >State: open > >Quarter: > >Keywords: > >Date-Required: > >Class: change-request > >Submitter-Id: current-users > >Arrival-Date: Wed Apr 16 15:40:01 UTC 2008 > >Closed-Date: > >Last-Modified: > >Originator: Maurice Castro > >Release: FreeBSD 7.0-RELEASE i386 > >Organization: > >Environment: > > System: FreeBSD atum.castro.aus.net 7.0-RELEASE FreeBSD 7.0-RELEASE #9: Wed > Apr 16 17:42:53 EST 2008 > root@atum.castro.aus.net:/scratch/src/sys/i386/compile/USBTEST i386 > > > > FreeBSD all versions > > >Description: > > The current USB implementation uses a quirks table that is compiled > into the kernel. Under most circumstances this approach works. > However, some manufacturers of USB devices have reused keys used > in the table and hence changing the compiled in table will result > in an inappropriate entries being present. A localised method of > changing the quirks values without recompiling their kernel would > assist developers and users facing this problem. USB developers in > particular can benefit from the ability to prevent a device > inappropriately identifying itself as a hid device without having > to recompile their kernel. > > The supplied patch uses entries in the kernel environment to create > a dynamic quirks table. The data is available at boot time so > devices that are connected across a reboot are correctly handled. > This table can also be updated after boot time by calling an IOCTL. > > >How-To-Repeat: > >Fix: > > diff -ur /usr/src/share/man/man4/usb.4 /scratch/src/share/man/man4/usb.4 > --- /usr/src/share/man/man4/usb.4 2008-04-11 22:43:31.000000000 +1000 > +++ /scratch/src/share/man/man4/usb.4 2008-04-17 00:23:48.000000000 +1000 > @@ -288,6 +288,66 @@ > .Em DANGEROUS > and should be used with great care since it > can destroy the bus integrity. > +.It Dv USB_SETDYNQUIRKS > +This command will cause the dynamic quirks table to be rebuilt from the > +contents of the kernel environment. Environment strings of the form > +.Pp > +.Ic usb.quirk.N="VENDOR PRODUCT REVISION FLAGS" > +.Pp > +where > +.Ic N > +is a number between 0 and 9 and quirks must be numbered contiguously; > +.Ic VENDOR PRODUCT > +and > +.Ic REVISION > +are constants that identify the device (the value 0xffff for > +.Ic REVISION > +denotes all revisions); and > +.Ic FLAGS > +is any combination of > +.Bl -bullet -offset indent -compact > +.It > +SWAP_UNICODE > +.It > +MS_REVZ > +.It > +NO_STRINGS > +.It > +BAD_ADC > +.It > +BUS_POWERED > +.It > +BAD_AUDIO > +.It > +SPUR_BUT_UP > +.It > +AU_NO_XU > +.It > +POWER_CLAIM > +.It > +AU_NO_FRAC > +.It > +AU_INP_ASYNC > +.It > +BROKEN_BIDIR > +.It > +OPEN_CLEARSTALL > +.It > +HID_IGNORE > +.It > +KBD_IGNORE > +.It > +MS_BAD_CLASS > +.It > +MS_LEADING_BYTE > +.El > +separated by "|" characters. These lines set the quirks for each device > +identified. > +.Pp > +The dynamic quirks table is designed to supplement the quirks table built > +in to the kernel. It is of particular use to developers working with > devices +that inappropriately share vendor, product and revision > information and hence +cannot be correctly added in to the kernel's quirks > table. > .El > .Pp > The include file > diff -ur /usr/src/sys/dev/usb/usb.c /scratch/src/sys/dev/usb/usb.c > --- /usr/src/sys/dev/usb/usb.c 2008-04-11 22:43:56.000000000 +1000 > +++ /scratch/src/sys/dev/usb/usb.c 2008-04-16 23:23:55.000000000 +1000 > @@ -668,6 +668,10 @@ > *(struct usb_device_stats *)data = sc->sc_bus->stats; > break; > > + case USB_SETDYNQUIRKS: > + usbd_populate_dynamic_quirks(); > + break; > + > default: > return (EINVAL); > } > diff -ur /usr/src/sys/dev/usb/usb.h /scratch/src/sys/dev/usb/usb.h > --- /usr/src/sys/dev/usb/usb.h 2008-04-11 22:43:56.000000000 +1000 > +++ /scratch/src/sys/dev/usb/usb.h 2008-04-16 23:22:34.000000000 +1000 > @@ -673,6 +673,7 @@ > #define USB_DISCOVER _IO ('U', 3) > #define USB_DEVICEINFO _IOWR('U', 4, struct usb_device_info) > #define USB_DEVICESTATS _IOR ('U', 5, struct usb_device_stats) > +#define USB_SETDYNQUIRKS _IO ('U', 6) > > /* Generic HID device */ > #define USB_GET_REPORT_DESC _IOR ('U', 21, struct usb_ctl_report_desc) > diff -ur /usr/src/sys/dev/usb/usb_quirks.c > /scratch/src/sys/dev/usb/usb_quirks.c --- > /usr/src/sys/dev/usb/usb_quirks.c 2008-04-11 22:43:56.000000000 +1000 +++ > /scratch/src/sys/dev/usb/usb_quirks.c 2008-04-16 17:42:12.000000000 +1000 > @@ -42,8 +42,11 @@ > > #include <sys/param.h> > #include <sys/systm.h> > +#include <sys/kernel.h> > +#include <sys/ctype.h> > > #include <dev/usb/usb.h> > +#include <sys/kenv.h> > > #include "usbdevs.h" > #include <dev/usb/usb_quirks.h> > @@ -54,12 +57,14 @@ > > #define ANY 0xffff > > -static const struct usbd_quirk_entry { > +struct usbd_quirk_entry { > u_int16_t idVendor; > u_int16_t idProduct; > u_int16_t bcdDevice; > struct usbd_quirks quirks; > -} usb_quirks[] = { > +}; > + > +static struct usbd_quirk_entry usb_quirks[] = { > { USB_VENDOR_INSIDEOUT, USB_PRODUCT_INSIDEOUT_EDGEPORT4, > 0x094, { UQ_SWAP_UNICODE}}, > { USB_VENDOR_DALLAS, USB_PRODUCT_DALLAS_J6502, 0x0a2, { UQ_BAD_ADC > }}, @@ -117,15 +122,24 @@ > > const struct usbd_quirks usbd_no_quirk = { 0 }; > > -const struct usbd_quirks * > -usbd_find_quirk(usb_device_descriptor_t *d) > +#define MAX_DYNAMIC_USB_QUIRKS 10 > +#define ENVNAMEROOT "usb.quirk." > +#define SEPCHAR "|" > + > +static struct usbd_quirk_entry dynamic_usb_quirks[MAX_DYNAMIC_USB_QUIRKS]; > + > +static struct usbd_quirks * > +usbd_search_quirk(struct usbd_quirk_entry *quirks, usb_device_descriptor_t > *d); + > +static struct usbd_quirks * > +usbd_search_quirk(struct usbd_quirk_entry *quirks, usb_device_descriptor_t > *d) { > - const struct usbd_quirk_entry *t; > + struct usbd_quirk_entry *t; > u_int16_t vendor = UGETW(d->idVendor); > u_int16_t product = UGETW(d->idProduct); > u_int16_t revision = UGETW(d->bcdDevice); > > - for (t = usb_quirks; t->idVendor != 0; t++) { > + for (t = quirks; t->idVendor != 0; t++) { > if (t->idVendor == vendor && > t->idProduct == product && > (t->bcdDevice == ANY || t->bcdDevice == revision)) > @@ -139,3 +153,134 @@ > #endif > return (&t->quirks); > } > + > +const struct usbd_quirks * > +usbd_find_quirk(usb_device_descriptor_t *d) > +{ > + struct usbd_quirks *quirks; > + /* check the dynamic quirks list first for local entries */ > + quirks = usbd_search_quirk((struct usbd_quirk_entry *) > dynamic_usb_quirks, d); + if (quirks->uq_flags != 0) > + return quirks; > + /* check the compiled in quirks list if dynamic entry not set */ > + return(usbd_search_quirk((struct usbd_quirk_entry *) usb_quirks, d)); > +} > + > +void usbd_populate_dynamic_quirks() > +{ > + /* the size of envkey must exceed the length of ENVNAMEROOT */ > + /* and the maximum number of digits in MAX_DYNAMIC_USB_QUIRKS plus 1 */ > + /* as the environment size is limitted to 512 entries and a maximum */ > + /* of 128 usb devices are supported 3 digits is appropriate for > + all valid values of MAX_DYNAMIC_USB_QUIRKS */ > + const int envkeysz = strlen(ENVNAMEROOT)+3+1; > + char envkey[envkeysz]; > + int i; > + char *env; > + char *pt; > + char *e; > + int n; > + for (i=0; i<MAX_DYNAMIC_USB_QUIRKS; i++) > + { > + dynamic_usb_quirks[i].quirks.uq_flags = 0; > + dynamic_usb_quirks[i].idVendor = 0; > + dynamic_usb_quirks[i].idProduct = 0; > + dynamic_usb_quirks[i].bcdDevice = 0; > + snprintf(envkey,envkeysz,"%s%d",ENVNAMEROOT,i); > + if (testenv(envkey)) > + { > +#ifdef USB_DEBUG > + printf("usbd config %s\n", envkey); > +#endif > + env = getenv(envkey); > + dynamic_usb_quirks[i].idVendor = strtoul(env, &pt, 0); > + dynamic_usb_quirks[i].idProduct = strtoul(pt, &pt, 0); > + dynamic_usb_quirks[i].bcdDevice = strtoul(pt, &pt, 0); > + /* skip anything which isn't a flag */ > + while (*pt && !isalpha(*pt)) pt++; > + /* read in flags */ > + while (*pt) > + { > + e = strstr(pt,SEPCHAR); > + if (!e) > + { > + n = strlen(pt); > + } > + else > + { > + n = e - pt; > + } > + if (!strncmp("SWAP_UNICODE", pt, n)) > + dynamic_usb_quirks[i].quirks.uq_flags |= UQ_SWAP_UNICODE; > + if (!strncmp("MS_REVZ", pt, n)) > + dynamic_usb_quirks[i].quirks.uq_flags |= UQ_MS_REVZ; > + if (!strncmp("NO_STRINGS", pt, n)) > + dynamic_usb_quirks[i].quirks.uq_flags |= UQ_NO_STRINGS; > + if (!strncmp("BAD_ADC", pt, n)) > + dynamic_usb_quirks[i].quirks.uq_flags |= UQ_BAD_ADC; > + if (!strncmp("BUS_POWERED", pt, n)) > + dynamic_usb_quirks[i].quirks.uq_flags |= UQ_BUS_POWERED; > + if (!strncmp("BAD_AUDIO", pt, n)) > + dynamic_usb_quirks[i].quirks.uq_flags |= UQ_BAD_AUDIO; > + if (!strncmp("SPUR_BUT_UP", pt, n)) > + dynamic_usb_quirks[i].quirks.uq_flags |= UQ_SPUR_BUT_UP; > + if (!strncmp("AU_NO_XU", pt, n)) > + dynamic_usb_quirks[i].quirks.uq_flags |= UQ_AU_NO_XU; > + if (!strncmp("POWER_CLAIM", pt, n)) > + dynamic_usb_quirks[i].quirks.uq_flags |= UQ_POWER_CLAIM; > + if (!strncmp("AU_NO_FRAC", pt, n)) > + dynamic_usb_quirks[i].quirks.uq_flags |= UQ_AU_NO_FRAC; > + if (!strncmp("AU_INP_ASYNC", pt, n)) > + dynamic_usb_quirks[i].quirks.uq_flags |= UQ_AU_INP_ASYNC; > + if (!strncmp("BROKEN_BIDIR", pt, n)) > + dynamic_usb_quirks[i].quirks.uq_flags |= UQ_BROKEN_BIDIR; > + if (!strncmp("OPEN_CLEARSTALL", pt, n)) > + dynamic_usb_quirks[i].quirks.uq_flags |= UQ_OPEN_CLEARSTALL; > + if (!strncmp("HID_IGNORE", pt, n)) > + dynamic_usb_quirks[i].quirks.uq_flags |= UQ_HID_IGNORE; > + if (!strncmp("KBD_IGNORE", pt, n)) > + dynamic_usb_quirks[i].quirks.uq_flags |= UQ_KBD_IGNORE; > + if (!strncmp("MS_BAD_CLASS", pt, n)) > + dynamic_usb_quirks[i].quirks.uq_flags |= UQ_MS_BAD_CLASS; > + if (!strncmp("MS_LEADING_BYTE", pt, n)) > + dynamic_usb_quirks[i].quirks.uq_flags |= UQ_MS_LEADING_BYTE; > + pt += n; > + pt += strspn(pt, SEPCHAR); > + } > +#ifdef USB_DEBUG > + printf("usbd quirk %d %x %x %x %x\n", > + dynamic_usb_quirks[i].quirks.uq_flags, > + dynamic_usb_quirks[i].idVendor, > + dynamic_usb_quirks[i].idProduct, > + dynamic_usb_quirks[i].bcdDevice, > + dynamic_usb_quirks[i].quirks.uq_flags > + ); > +#endif > + freeenv(env); > + } > + else > + { > + break; > + } > + } > + for (; i<MAX_DYNAMIC_USB_QUIRKS; i++) > + { > +#ifdef USB_DEBUG > + printf("usbd clear dynamic quirk %d\n", i); > +#endif > + dynamic_usb_quirks[i].quirks.uq_flags = 0; > + dynamic_usb_quirks[i].idVendor = 0; > + dynamic_usb_quirks[i].idProduct = 0; > + dynamic_usb_quirks[i].bcdDevice = 0; > + snprintf(envkey,envkeysz,"%s%d",ENVNAMEROOT,i); > + if (testenv(envkey)) > + { > +#ifdef USB_DEBUG > + printf("usbd key %s invalid as earlier entry missing\n", envkey); > +#endif > + } > + } > +} > + > +SYSINIT(usbd_populate_dynamic_quirks, SI_SUB_KLD, SI_ORDER_MIDDLE, > + usbd_populate_dynamic_quirks, NULL); > diff -ur /usr/src/sys/dev/usb/usb_quirks.h > /scratch/src/sys/dev/usb/usb_quirks.h --- > /usr/src/sys/dev/usb/usb_quirks.h 2008-04-11 22:43:56.000000000 +1000 +++ > /scratch/src/sys/dev/usb/usb_quirks.h 2008-04-16 11:09:35.000000000 +1000 > @@ -62,3 +62,5 @@ > extern const struct usbd_quirks usbd_no_quirk; > > const struct usbd_quirks *usbd_find_quirk(usb_device_descriptor_t *); > + > +void usbd_populate_dynamic_quirks(void); > > >Release-Note: > >Audit-Trail: > >Unformatted: > > _______________________________________________ > freebsd-usb@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-usb > To unsubscribe, send any mail to "freebsd-usb-unsubscribe@freebsd.org"
Hi Hans, in addition to the corrections below I have added an extra utility to the system this utility calls the ioctl to load the dynamic quirks table. Patch is attached. As noted below I have not incorporated the bsearch for the reasons stated. Maurice Castro From: Hans Petter Selasky <hselasky@c2i.net> Date: Wed, 16 Apr 2008 18:54:02 +0200 > Hi, > > Maybe you can prefix the string version of the quirks by the USB > module name ? > > For example: > > MS_REVZ -> UMS_REVZ > AU_NO_FRAC -> UAUDIO_NO_FRAC > Done > Then it is easier to know where the quirk belongs. > > Add a short description of what the quirk does in the "usb" manpage. > Done > There might be a race condition reading and writing the quirks. > Probably a > mutex is appropriate. > Done > You can improve the quirk string to numerical conversion code by > using binary > search. > Wrote code to use bsearch and then backed it out. Value of binary searching on lists less than 20 items is minimal and required additional memory copy operation and release, due to non-destructive parsing technique used on environment variables. > Else your patch looks fine to me.
From: hselasky@c2i.net > You need to do a little bit more work regarding the token naming. > There is no USB module called "UAU". Instead of "UAU_NO_FRAC" I > think you should have changed it to "UAUDIO_NO_FRAC". The same > applies for most of the other quirk tokens aswell. "UHID_IGNORE" is > fine. Hi Hans, changes made as requested. Summary of quirk, module quirk appears in and naming below. Please note that 2 of the quirks appear to be unused. Updated patch attached. Maurice Castro * UQ_SWAP_UNICODE /usr/src/sys/dev/usb/usbdi.c: int swap = dev->quirks->uq_flags & UQ_SWAP_UNICODE; USWAP_UNICODE -> USBDI_SWAP_UNICODE * UQ_NO_STRINGS /usr/src/sys/dev/usb/usbdi.c: if (dev->quirks->uq_flags & UQ_NO_STRINGS) UNO_STRINGS -> USBDI_NO_STRINGS * UQ_BAD_ADC /usr/src/sys/dev/sound/usb/uaudio.c: if (!(usbd_get_quirks(sc- >sc_udev)->uq_flags & UQ_BAD_ADC) && UBAD_ADC -> UAUDIO_BAD_ADC * UQ_BUS_POWERED /usr/src/sys/dev/usb/usb_subr.c: if (!(dev->quirks->uq_flags & UQ_BUS_POWERED) && UBUS_POWERED -> USB_BUS_POWERED * UQ_BAD_AUDIO /usr/src/sys/dev/sound/usb/uaudio.c: (usbd_get_quirks(uaa- >device)->uq_flags & UQ_BAD_AUDIO)) UBAD_AUDIO -> UAUDIO_BAD_AUDIO * UQ_SPUR_BUT_UP /usr/src/sys/dev/usb/ums.c: if (usbd_get_quirks(uaa->device)- >uq_flags & UQ_SPUR_BUT_UP) { USPUR_BUT_UP -> UMS_SPUR_BUT_UP * UQ_AU_NO_XU /usr/src/sys/dev/sound/usb/uaudio.c: if (usbd_get_quirks(sc- >sc_udev)->uq_flags & UQ_AU_NO_XU) UAU_NO_XU -> UAUDIO_NO_XU * UQ_POWER_CLAIM /usr/src/sys/dev/usb/usb_subr.c: if (dev- >quirks->uq_flags & UQ_POWER_CLAIM) { UPOWER_CLAIM -> USB_POWER_CLAIM * UQ_AU_NO_FRAC /usr/src/sys/dev/sound/usb/uaudio.c: if (usbd_get_quirks(sc- >sc_udev)->uq_flags & UQ_AU_NO_FRAC) UAU_NO_FRAC -> UAUDIO_AU_NO_FRAC * UQ_AU_INP_ASYNC /usr/src/sys/dev/sound/usb/uaudio.c: if ((usbd_get_quirks(sc- >sc_udev)->uq_flags & UQ_AU_INP_ASYNC) && UAU_INP_ASYNC -> UAUDIO_INP_ASYNC * UQ_BROKEN_BIDIR /usr/src/sys/dev/usb/ulpt.c: if (usbd_get_quirks(dev)->uq_flags & UQ_BROKEN_BIDIR) { UBROKEN_BIDIR -> ULPT_BROKEN_BIDIR * UQ_OPEN_CLEARSTALL /usr/src/sys/dev/usb/usb_subr.c: if (dev->quirks->uq_flags & UQ_OPEN_CLEARSTALL) { * UQ_HID_IGNORE /usr/src/sys/dev/usb/uhid.c: if (usbd_get_quirks(uaa->device)- >uq_flags & UQ_HID_IGNORE) UHID_IGNORE -> UHID_IGNORE * UQ_KBD_IGNORE /usr/src/sys/dev/usb/ukbd.c: if (usbd_get_quirks(uaa->device)- >uq_flags & UQ_KBD_IGNORE) UKBD_IGNORE -> UKBD_IGNORE * UQ_MS_BAD_CLASS /usr/src/sys/dev/usb/ums.c: if (usbd_get_quirks(uaa->device)- >uq_flags & UQ_MS_BAD_CLASS) { UMS_BAD_CLASS -> UMS_BAD_CLASS * UQ_MS_REVZ UMS_REVZ -> MS_REVZ * UQ_MS_LEADING_BYTE UMS_LEADING_BYTE -> MS_LEADING_BYTE
State Changed From-To: open->feedback What's the status of this PR? To my knowledge the new USB stack supports dynamic quirk tables.
After quickly reviewing the code in 8.1 there appears to be dynamic usb = quirks in the new usb stack. The patch is effectively orphaned as the = new usb stack provides the feature; but people using the old stack do = not have the functionality. To summarise the patch is relevant to the 7 = series of kernels, but the functionality appears to be available in the = 8 and beyond series of kernels. Maurice Castro=
State Changed From-To: feedback->suspended Thank you for your feedback. I've tagged this PR with [usb67]. Due to the fact that there's very little chance that this functionality will be committed to the old USB 1 stack, i've also set it into suspend state.
State Changed From-To: suspended->closed usb67 is no longer being developed; thank you for your contribution