Date: Wed, 16 Apr 2008 18:00:06 GMT From: Hans Petter Selasky <hselasky@c2i.net> To: freebsd-usb@FreeBSD.org Subject: Re: usb/122819: Patch to provide dynamic additions to the usb quirks table Message-ID: <200804161800.m3GI06vk097202@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR usb/122819; it has been noted by GNATS.
From: Hans Petter Selasky <hselasky@c2i.net>
To: freebsd-usb@freebsd.org,
Maurice Castro <maurice@castro.aus.net>
Cc: FreeBSD-gnats-submit@freebsd.org
Subject: Re: usb/122819: Patch to provide dynamic additions to the usb quirks table
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
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"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200804161800.m3GI06vk097202>