as the ugen(4) is too flexible (and unsafe) than other usb device drivers,
i don't like this work to extend ugen(4)'s area.

I know many userland applications which supports multiple platform using
ugen type interface (because they usually use libusb or ugen apis), but
it is no reason to support them without considerations.

.....

>From usb developer's view (include me), this work is seems to the so
useful to use "unsupported" / "partially supported" device. hmm...

-- Yojiro UO


On 2011/01/04, at 8:18, Jacob Meuser wrote:

> when a USB device is inserted, usbd_probe_and_attach() first tries to
> find a driver that claims to support the device by matching the
> vendor/product IDs.  if no such driver is found, usbd_probe_and_attach()
> loops through each interface of each of the devices configurations,
> trying to match "interface" (aka class) drivers to an interface.  if
> the device has more than one interface *in a single configuration*
> that matches drivers, then drivers are allowed to attach to each
> interface.  for example:
>
> uvideo0 at uhub5 port 1 configuration 1 interface 0 "Logitech product
0x09a2" rev 2.00/0.08 addr 4
> video0 at uvideo0
> uaudio0 at uhub5 port 1 configuration 1 interface 2 "Logitech product
0x09a2" rev 2.00/0.08 addr 4
> uaudio0: audio rev 1.00, 2 mixer controls
> audio1 at uaudio0
>
> note how they are both attached "at uhub5 port 1 configuration 1".
>
> and:
>
> uhidev0 at uhub1 port 2 configuration 1 interface 0 "Logitech USB Receiver"
rev 2.00/16.00 addr 2
> uhidev0: iclass 3/1
> ums0 at uhidev0: 16 buttons, Z dir
> wsmouse1 at ums0 mux 0
> uhidev1 at uhub1 port 2 configuration 1 interface 1 "Logitech USB Receiver"
rev 2.00/16.00 addr 2
> uhidev1: iclass 3/0, 17 report ids
> uhid0 at uhidev1 reportid 3: input=4, output=0, feature=0
> uhid1 at uhidev1 reportid 16: input=6, output=6, feature=0
> uhid2 at uhidev1 reportid 17: input=19, output=19, feature=0
>
> note how both uhidev(4)s are attached "at uhub1 port 2 configuration 1".
>
> currently, usbd_probe_and_attach() makes a copy of the array of pointers
> to the interface descriptors and passes these to the match/attach functions
> in struct usb_attach_args (uaa).  when an interface is matched, the pointer
> to the interface descriptor in uaa is set to NULL.  and, if a driver uses
> other interfaces, the driver sets the pointer to those interface
> descriptors in the uaa to NULL.  this way, what interfaces are available
> is known, but this only really works at attach time.
>
> currently, ugen(4) assumes it can use any interface in any configuration.
> for this reason, ugen(4) is only attached if no interfaces have drivers
> attached.
>
> the following allows ugen(4) to be safely attached when there are unused
> interfaces.  this is accomplished by:
>
> * instead of NULLing pointers to interface descriptors in the uaa, mark
>  interfaces as being claimed in the usbd_device's copy of the interface
>  descriptors
> * allow ugen(4) to be attached if there are unused interfaces in a
>  configuration that has had drivers attached
> * make ugen(4) aware that it may be sharing a device with (an)other
>  driver(s), and if so:
>  * do not let ugen(4) change the configuration
>  * do not let ugen(4) access the already claimed interfaces
>
> this allows, for example:
>
> ulpt0 at uhub0 port 1 configuration 1 interface 1 "HP Officejet 4500
G510g-m" rev 2.00/1.00 addr 2
> ulpt0: using bi-directional mode
> ugen0 at uhub0 port 1 configuration 1 "HP Officejet 4500 G510g-m" rev
2.00/1.00 addr 2
>
> which means this MFP can be used by both lpd and sane, simultaneously.
> this also requires a patch to libusb, to let it know that ugen(4) might
> not be the first driver attached to the device.  that patch is at
> http://jakemsr.trancell.org/libusb.diff
>
> thoughts?  oks?
>
> --
> jake...@sdf.lonestar.org
> SDF Public Access UNIX System - http://sdf.lonestar.org
>
> Index: if_cdce.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/if_cdce.c,v
> retrieving revision 1.47
> diff -u -p if_cdce.c
> --- if_cdce.c 27 Oct 2010 17:51:11 -0000      1.47
> +++ if_cdce.c 3 Jan 2011 21:52:27 -0000
> @@ -224,12 +224,12 @@ cdce_attach(struct device *parent, struct device
*self
>               DPRINTF(("cdce_attach: union interface: ctl=%d, data=%d\n",
>                   ctl_ifcno, data_ifcno));
>               for (i = 0; i < uaa->nifaces; i++) {
> -                     if (uaa->ifaces[i] == NULL)
> +                     if (usbd_iface_claimed(sc->cdce_udev, i))
>                               continue;
>                       id = usbd_get_interface_descriptor(uaa->ifaces[i]);
>                       if (id != NULL && id->bInterfaceNumber == data_ifcno) {
>                               sc->cdce_data_iface = uaa->ifaces[i];
> -                             uaa->ifaces[i] = NULL;
> +                             usbd_claim_iface(sc->cdce_udev, i);
>                       }
>               }
>       }
> Index: if_urndis.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/if_urndis.c,v
> retrieving revision 1.27
> diff -u -p if_urndis.c
> --- if_urndis.c       27 Oct 2010 17:51:11 -0000      1.27
> +++ if_urndis.c       3 Jan 2011 21:52:27 -0000
> @@ -1389,12 +1389,12 @@ urndis_attach(struct device *parent, struct device
*se
>               DPRINTF(("urndis_attach: union interface: ctl %u, data %u\n",
>                   if_ctl, if_data));
>               for (i = 0; i < uaa->nifaces; i++) {
> -                     if (uaa->ifaces[i] == NULL)
> +                     if (usbd_iface_claimed(sc->sc_udev, i))
>                               continue;
>                       id = usbd_get_interface_descriptor(uaa->ifaces[i]);
>                       if (id && id->bInterfaceNumber == if_data) {
>                               sc->sc_iface_data = uaa->ifaces[i];
> -                             uaa->ifaces[i] = NULL;
> +                             usbd_claim_iface(sc->sc_udev, i);
>                       }
>               }
>       }
> Index: uaudio.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
> retrieving revision 1.89
> diff -u -p uaudio.c
> --- uaudio.c  18 Aug 2010 22:54:58 -0000      1.89
> +++ uaudio.c  3 Jan 2011 21:52:27 -0000
> @@ -210,7 +210,6 @@ struct uaudio_softc {
>       struct device    sc_dev;        /* base device */
>       usbd_device_handle sc_udev;     /* USB device */
>       int              sc_ac_iface;   /* Audio Control interface */
> -     usbd_interface_handle   sc_ac_ifaceh;
>       struct chan      sc_playchan;   /* play channel */
>       struct chan      sc_recchan;    /* record channel */
>       int              sc_nullalt;
> @@ -520,10 +519,9 @@ uaudio_attach(struct device *parent, struct device *se
>               return;
>       }
>
> -     sc->sc_ac_ifaceh = uaa->iface;
>       /* Pick up the AS interface. */
>       for (i = 0; i < uaa->nifaces; i++) {
> -             if (uaa->ifaces[i] == NULL)
> +             if (usbd_iface_claimed(sc->sc_udev, i))
>                       continue;
>               id = usbd_get_interface_descriptor(uaa->ifaces[i]);
>               if (id == NULL)
> @@ -537,7 +535,7 @@ uaudio_attach(struct device *parent, struct device *se
>                       }
>               }
>               if (found)
> -                     uaa->ifaces[i] = NULL;
> +                     usbd_claim_iface(sc->sc_udev, i);
>       }
>
>       for (j = 0; j < sc->sc_nalts; j++) {
> Index: ugen.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/ugen.c,v
> retrieving revision 1.62
> diff -u -p ugen.c
> --- ugen.c    24 Sep 2010 08:33:59 -0000      1.62
> +++ ugen.c    3 Jan 2011 21:52:28 -0000
> @@ -103,6 +103,7 @@ struct ugen_softc {
>
>       int sc_refcnt;
>       u_char sc_dying;
> +     u_char sc_secondary;
> };
>
> void ugenintr(usbd_xfer_handle xfer, usbd_private_handle addr,
> @@ -166,13 +167,18 @@ ugen_attach(struct device *parent, struct device
*self
>
>       sc->sc_udev = udev = uaa->device;
>
> -     /* First set configuration index 0, the default one for ugen. */
> -     err = usbd_set_config_index(udev, 0, 0);
> -     if (err) {
> -             printf("%s: setting configuration index 0 failed\n",
> -                    sc->sc_dev.dv_xname);
> -             sc->sc_dying = 1;
> -             return;
> +     if (usbd_get_devcnt(udev) > 0)
> +             sc->sc_secondary = 1;
> +
> +     if (!sc->sc_secondary) {
> +             /* First set configuration index 0, the default one for ugen. */
> +             err = usbd_set_config_index(udev, 0, 0);
> +             if (err) {
> +                     printf("%s: setting configuration index 0 failed\n",
> +                            sc->sc_dev.dv_xname);
> +                     sc->sc_dying = 1;
> +                     return;
> +             }
>       }
>       conf = usbd_get_config_descriptor(udev)->bConfigurationValue;
>
> @@ -220,9 +226,15 @@ ugen_set_config(struct ugen_softc *sc, int configno)
>       /* Avoid setting the current value. */
>       cdesc = usbd_get_config_descriptor(dev);
>       if (!cdesc || cdesc->bConfigurationValue != configno) {
> -             err = usbd_set_config_no(dev, configno, 1);
> -             if (err)
> -                     return (err);
> +             if (sc->sc_secondary) {
> +                     printf("%s: secondary, not changing config to %d\n",
> +                         __func__, configno);
> +                     return (USBD_IN_USE);
> +             } else {
> +                     err = usbd_set_config_no(dev, configno, 1);
> +                     if (err)
> +                             return (err);
> +             }
>       }
>
>       err = usbd_interface_count(dev, &niface);
> @@ -231,6 +243,10 @@ ugen_set_config(struct ugen_softc *sc, int configno)
>       memset(sc->sc_endpoints, 0, sizeof sc->sc_endpoints);
>       for (ifaceno = 0; ifaceno < niface; ifaceno++) {
>               DPRINTFN(1,("ugen_set_config: ifaceno %d\n", ifaceno));
> +             if (usbd_iface_claimed(dev, ifaceno)) {
> +                     DPRINTF(("%s: iface %d claimed\n", __func__, ifaceno));
> +                     continue;
> +             }
>               err = usbd_device2interface_handle(dev, ifaceno, &iface);
>               if (err)
>                       return (err);
> Index: umodem.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/umodem.c,v
> retrieving revision 1.39
> diff -u -p umodem.c
> --- umodem.c  2 Dec 2010 01:37:45 -0000       1.39
> +++ umodem.c  3 Jan 2011 21:52:28 -0000
> @@ -280,12 +280,12 @@ umodem_attach(struct device *parent, struct device
*se
>
>       /* Get the data interface too. */
>       for (i = 0; i < uaa->nifaces; i++) {
> -             if (uaa->ifaces[i] != NULL) {
> +             if (!usbd_iface_claimed(sc->sc_udev, i)) {
>                       id = usbd_get_interface_descriptor(uaa->ifaces[i]);
>                       if (id != NULL &&
>                             id->bInterfaceNumber == data_iface_no) {
>                               sc->sc_data_iface = uaa->ifaces[i];
> -                             uaa->ifaces[i] = NULL;
> +                             usbd_claim_iface(sc->sc_udev, i);
>                       }
>               }
>       }
> ? usb-n.diff
> Index: usb_subr.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/usb_subr.c,v
> retrieving revision 1.77
> diff -u -p usb_subr.c
> --- usb_subr.c        17 Dec 2010 22:38:54 -0000      1.77
> +++ usb_subr.c        3 Jan 2011 21:52:28 -0000
> @@ -777,7 +777,7 @@ usbd_set_config_index(usbd_device_handle dev, int inde
>       /* Allocate and fill interface data. */
>       nifc = cdp->bNumInterface;
>       dev->ifaces = malloc(nifc * sizeof(struct usbd_interface),
> -         M_USB, M_NOWAIT);
> +         M_USB, M_NOWAIT | M_ZERO);
>       if (dev->ifaces == NULL) {
>               err = USBD_NOMEM;
>               goto bad;
> @@ -859,14 +859,13 @@ usbd_getnewaddr(usbd_bus_handle bus)
>       return (-1);
> }
>
> -
> usbd_status
> usbd_probe_and_attach(struct device *parent, usbd_device_handle dev, int
port,
>     int addr)
> {
>       struct usb_attach_arg uaa;
>       usb_device_descriptor_t *dd = &dev->ddesc;
> -     int found, i, confi, nifaces, len;
> +     int i, confi, nifaces, len;
>       usbd_status err;
>       struct device *dv;
>       usbd_interface_handle *ifaces;
> @@ -895,8 +894,8 @@ usbd_probe_and_attach(struct device *parent, usbd_devi
>                       err = USBD_NOMEM;
>                       goto fail;
>               }
> -             dev->subdevs[0] = dv;
> -             dev->subdevs[1] = 0;
> +             dev->subdevs[dev->ndevs++] = dv;
> +             dev->subdevs[dev->ndevs] = 0;
>               err = USBD_NORMAL_COMPLETION;
>               goto fail;
>       }
> @@ -934,7 +933,9 @@ usbd_probe_and_attach(struct device *parent, usbd_devi
>                       ifaces[i] = &dev->ifaces[i];
>               uaa.ifaces = ifaces;
>               uaa.nifaces = nifaces;
> -             len = (nifaces+1) * sizeof dv;
> +
> +             /* add 1 for possible ugen and 1 for NULL terminator */
> +             len = (nifaces + 2) * sizeof dv;
>               dev->subdevs = malloc(len, M_USB, M_NOWAIT | M_ZERO);
>               if (dev->subdevs == NULL) {
>                       free(ifaces, M_USB);
> @@ -942,25 +943,31 @@ usbd_probe_and_attach(struct device *parent,
usbd_devi
>                       goto fail;
>               }
>
> -             found = 0;
>               for (i = 0; i < nifaces; i++) {
> -                     if (ifaces[i] == NULL)
> -                             continue; /* interface already claimed */
> +                     if (usbd_iface_claimed(dev, i))
> +                             continue;
>                       uaa.iface = ifaces[i];
>                       uaa.ifaceno = ifaces[i]->idesc->bInterfaceNumber;
>                       dv = config_found_sm(parent, &uaa, usbd_print,
>                                          usbd_submatch);
> -
>                       if (dv != NULL) {
> -                             dev->subdevs[found++] = dv;
> -                             ifaces[i] = NULL; /* consumed */
> +                             dev->subdevs[dev->ndevs++] = dv;
> +                             usbd_claim_iface(dev, i);
>                       }
>               }
>               free(ifaces, M_USB);
> -             if (found != 0) {
> -                     err = USBD_NORMAL_COMPLETION;
> -                     goto fail;
> +
> +             if (dev->ndevs > 0) {
> +                     for (i = 0; i < nifaces; i++) {
> +                             if (!usbd_iface_claimed(dev, i))
> +                                     break;
> +                     }
> +                     if (i < nifaces)
> +                             goto generic;
> +                      else
> +                             goto fail;
>               }
> +
>               free(dev->subdevs, M_USB);
>               dev->subdevs = 0;
>       }
> @@ -971,20 +978,24 @@ usbd_probe_and_attach(struct device *parent,
usbd_devi
>
>       DPRINTF(("usbd_probe_and_attach: no interface drivers found\n"));
>
> +generic:
>       /* Finally try the generic driver. */
>       uaa.iface = NULL;
>       uaa.usegeneric = 1;
> -     uaa.configno = UHUB_UNK_CONFIGURATION;
> +     uaa.configno = dev->ndevs == 0 ? UHUB_UNK_CONFIGURATION :
> +         dev->cdesc->bConfigurationValue;
>       uaa.ifaceno = UHUB_UNK_INTERFACE;
>       dv = config_found_sm(parent, &uaa, usbd_print, usbd_submatch);
>       if (dv != NULL) {
> -             dev->subdevs = malloc(2 * sizeof dv, M_USB, M_NOWAIT);
> -             if (dev->subdevs == 0) {
> -                     err = USBD_NOMEM;
> -                     goto fail;
> +             if (dev->ndevs == 0) {
> +                     dev->subdevs = malloc(2 * sizeof dv, M_USB, M_NOWAIT);
> +                     if (dev->subdevs == NULL) {
> +                             err = USBD_NOMEM;
> +                             goto fail;
> +                     }
>               }
> -             dev->subdevs[0] = dv;
> -             dev->subdevs[1] = 0;
> +             dev->subdevs[dev->ndevs++] = dv;
> +             dev->subdevs[dev->ndevs] = 0;
>               err = USBD_NORMAL_COMPLETION;
>               goto fail;
>       }
> Index: usbdi.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/usbdi.c,v
> retrieving revision 1.42
> diff -u -p usbdi.c
> --- usbdi.c   30 Dec 2010 05:10:35 -0000      1.42
> +++ usbdi.c   3 Jan 2011 21:52:28 -0000
> @@ -117,6 +117,24 @@ usbd_ref_wait(usbd_device_handle dev)
>               tsleep(&dev->ref_cnt, PWAIT, "usbref", hz * 60);
> }
>
> +int
> +usbd_get_devcnt(usbd_device_handle dev)
> +{
> +     return (dev->ndevs);
> +}
> +
> +void
> +usbd_claim_iface(usbd_device_handle dev, int ifaceidx)
> +{
> +     dev->ifaces[ifaceidx].claimed = 1;
> +}
> +
> +int
> +usbd_iface_claimed(usbd_device_handle dev, int ifaceidx)
> +{
> +     return (dev->ifaces[ifaceidx].claimed);
> +}
> +
> static __inline int
> usbd_xfer_isread(usbd_xfer_handle xfer)
> {
> Index: usbdi.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/usbdi.h,v
> retrieving revision 1.37
> diff -u -p usbdi.h
> --- usbdi.h   30 Dec 2010 05:10:35 -0000      1.37
> +++ usbdi.h   3 Jan 2011 21:52:28 -0000
> @@ -165,6 +165,10 @@ usbd_status usbd_reload_device_desc(usbd_device_handle
>
> int usbd_ratecheck(struct timeval *last);
>
> +int usbd_get_devcnt(usbd_device_handle);
> +void usbd_claim_iface(usbd_device_handle, int);
> +int usbd_iface_claimed(usbd_device_handle, int);
> +
> int usbd_is_dying(usbd_device_handle);
> void usbd_deactivate(usbd_device_handle);
>
> Index: usbdivar.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/usbdivar.h,v
> retrieving revision 1.41
> diff -u -p usbdivar.h
> --- usbdivar.h        30 Dec 2010 05:10:35 -0000      1.41
> +++ usbdivar.h        3 Jan 2011 21:52:28 -0000
> @@ -147,6 +147,7 @@ struct usbd_device {
>       const struct usbd_quirks     *quirks;  /* device quirks, always set */
>       struct usbd_hub        *hub;           /* only if this is a hub */
>       struct device         **subdevs;       /* sub-devices, 0 terminated */
> +     int                     ndevs;         /* # of subdevs */
> };
>
> struct usbd_interface {
> @@ -157,6 +158,7 @@ struct usbd_interface {
>       struct usbd_endpoint   *endpoints;
>       void                   *priv;
>       LIST_HEAD(, usbd_pipe)  pipes;
> +     u_int8_t                claimed;
> };
>
> struct usbd_pipe {
> Index: uvideo.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/uvideo.c,v
> retrieving revision 1.147
> diff -u -p uvideo.c
> --- uvideo.c  24 Nov 2010 19:53:07 -0000      1.147
> +++ uvideo.c  3 Jan 2011 21:52:28 -0000
> @@ -468,13 +468,24 @@ uvideo_attach(struct device *parent, struct device
*se
> {
>       struct uvideo_softc *sc = (struct uvideo_softc *)self;
>       struct usb_attach_arg *uaa = aux;
> +     usb_interface_descriptor_t *id;
> +     int i;
>
>       sc->sc_udev = uaa->device;
>       sc->sc_nifaces = uaa->nifaces;
> -     sc->sc_ifaces = malloc(uaa->nifaces * sizeof(usbd_interface_handle),
> -         M_USB, M_WAITOK);
> -     bcopy(uaa->ifaces, sc->sc_ifaces,
> -         uaa->nifaces * sizeof(usbd_interface_handle));
> +     /*
> +      * Claim all video interfaces.  Interfaces must be claimed during
> +      * attach, during attach hooks is too late.
> +      */
> +     for (i = 0; i < sc->sc_nifaces; i++) {
> +             if (usbd_iface_claimed(sc->sc_udev, i))
> +                     continue;
> +             id = usbd_get_interface_descriptor(&sc->sc_udev->ifaces[i]);
> +             if (id == NULL)
> +                     continue;
> +             if (id->bInterfaceClass == UICLASS_VIDEO)
> +                     usbd_claim_iface(sc->sc_udev, i);
> +     }
>
>       /* maybe the device has quirks */
>       sc->sc_quirk = uvideo_lookup(uaa->vendor, uaa->product);
> @@ -557,8 +568,6 @@ uvideo_detach(struct device *self, int flags)
>       struct uvideo_softc *sc = (struct uvideo_softc *)self;
>       int rv = 0;
>
> -     free(sc->sc_ifaces, M_USB);
> -
>       /* Wait for outstanding requests to complete */
>       usbd_delay_ms(sc->sc_udev, UVIDEO_NFRAMES_MAX);
>
> @@ -837,12 +846,13 @@ uvideo_vs_parse_desc(struct uvideo_softc *sc,
usb_conf
>       for (i = 0; i < sc->sc_desc_vc_header.fix->bInCollection; i++) {
>               iface = sc->sc_desc_vc_header.baInterfaceNr[i];
>
> -             id = usbd_get_interface_descriptor(sc->sc_ifaces[iface]);
> +             id = usbd_get_interface_descriptor(&sc->sc_udev->ifaces[iface]);
>               if (id == NULL) {
>                       printf("%s: can't get VS interface %d!\n",
>                           DEVNAME(sc), iface);
>                       return (USBD_INVAL);
>               }
> +             usbd_claim_iface(sc->sc_udev, iface);
>
>               numalts = usbd_get_no_alts(cdesc, id->bInterfaceNumber);
>
> @@ -1197,7 +1207,7 @@ uvideo_vs_parse_desc_alt(struct uvideo_softc *sc, int
>
>               /* save endpoint with largest bandwidth */
>               if (UGETW(ed->wMaxPacketSize) > vs->psize) {
> -                     vs->ifaceh = sc->sc_ifaces[iface];
> +                     vs->ifaceh = &sc->sc_udev->ifaces[iface];
>                       vs->endpoint = ed->bEndpointAddress;
>                       vs->numalts = numalts;
>                       vs->curalt = id->bAlternateSetting;

Reply via email to