Date: Thu, 13 Nov 2008 21:49:09 GMT From: Hans Petter Selasky <hselasky@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 152950 for review Message-ID: <200811132149.mADLn9sC092232@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=152950 Change 152950 by hselasky@hselasky_laptop001 on 2008/11/13 21:48:35 Fix problem with USB-FS mode looping in the poll-handler, due to an invalid selwakeup. Add some more comments. Put a TAB character after some "#define" keywords. Reported by: Stefan Ehmann Affected files ... .. //depot/projects/usb/src/sys/dev/usb2/core/usb2_core.h#24 edit .. //depot/projects/usb/src/sys/dev/usb2/core/usb2_dev.c#40 edit .. //depot/projects/usb/src/sys/dev/usb2/core/usb2_generic.c#30 edit Differences ... ==== //depot/projects/usb/src/sys/dev/usb2/core/usb2_core.h#24 (text+ko) ==== @@ -159,12 +159,12 @@ #define usb2_callout_drain(c) callout_drain(&(c)->co) #define usb2_callout_pending(c) callout_pending(&(c)->co) -#define USB_BUS_LOCK(_b) mtx_lock(&(_b)->bus_mtx) -#define USB_BUS_UNLOCK(_b) mtx_unlock(&(_b)->bus_mtx) -#define USB_BUS_LOCK_ASSERT(_b, _t) mtx_assert(&(_b)->bus_mtx, _t) -#define USB_XFER_LOCK(_x) mtx_lock((_x)->xfer_mtx) -#define USB_XFER_UNLOCK(_x) mtx_unlock((_x)->xfer_mtx) -#define USB_XFER_LOCK_ASSERT(_x, _t) mtx_assert((_x)->xfer_mtx, _t) +#define USB_BUS_LOCK(_b) mtx_lock(&(_b)->bus_mtx) +#define USB_BUS_UNLOCK(_b) mtx_unlock(&(_b)->bus_mtx) +#define USB_BUS_LOCK_ASSERT(_b, _t) mtx_assert(&(_b)->bus_mtx, _t) +#define USB_XFER_LOCK(_x) mtx_lock((_x)->xfer_mtx) +#define USB_XFER_UNLOCK(_x) mtx_unlock((_x)->xfer_mtx) +#define USB_XFER_LOCK_ASSERT(_x, _t) mtx_assert((_x)->xfer_mtx, _t) /* structure prototypes */ struct file; @@ -401,13 +401,14 @@ struct usb2_fifo *rxfifo; struct usb2_fifo *txfifo; uint32_t devloc; /* original devloc */ - uint16_t bus_index; - uint8_t dev_index; - uint8_t iface_index; - uint8_t ep_index; - uint8_t is_read; - uint8_t is_write; - uint8_t is_uref; + uint16_t bus_index; /* bus index */ + uint8_t dev_index; /* device index */ + uint8_t iface_index; /* interface index */ + uint8_t ep_index; /* endpoint index */ + uint8_t is_read; /* set if location has read access */ + uint8_t is_write; /* set if location has write access */ + uint8_t is_uref; /* set if USB refcount decr. needed */ + uint8_t is_usbfs; /* set if USB-FS is active */ }; /* external variables */ ==== //depot/projects/usb/src/sys/dev/usb2/core/usb2_dev.c#40 (text+ko) ==== @@ -537,7 +537,11 @@ ploc->rxfifo = NULL; ploc->is_write = 0; ploc->is_read = 0; + ploc->is_usbfs = 0; } else { + /* initialise "is_usbfs" flag */ + ploc->is_usbfs = 0; + /* check for write */ if (fflags & FWRITE) { ppf = ploc->udev->fifo; @@ -549,6 +553,10 @@ (f->curr_file != fp)) { goto error; } + /* check if USB-FS is active */ + if (f->fs_ep_max != 0) { + ploc->is_usbfs = 1; + } } else { ploc->txfifo = NULL; ploc->is_write = 0; /* no ref */ @@ -565,6 +573,10 @@ (f->curr_file != fp)) { goto error; } + /* check if USB-FS is active */ + if (f->fs_ep_max != 0) { + ploc->is_usbfs = 1; + } } else { ploc->rxfifo = NULL; ploc->is_read = 0; /* no ref */ @@ -1594,7 +1606,6 @@ struct usb2_mbuf *m; int fflags; int revents; - uint8_t usbfs_active = 0; revents = usb2_ref_device(fp, &loc, 1 /* no uref */ );; if (revents) { @@ -1602,20 +1613,6 @@ } fflags = fp->f_flag; - /* figure out if the USB File System is active */ - - if (fflags & FWRITE) { - f = loc.txfifo; - if (f->fs_ep_max != 0) { - usbfs_active = 1; - } - } - if (fflags & FREAD) { - f = loc.rxfifo; - if (f->fs_ep_max != 0) { - usbfs_active = 1; - } - } /* Figure out who needs service */ if ((events & (POLLOUT | POLLWRNORM)) && @@ -1625,7 +1622,7 @@ mtx_lock(f->priv_mtx); - if (!usbfs_active) { + if (!loc.is_usbfs) { if (f->flag_iserror) { /* we got an error */ m = (void *)1; @@ -1664,7 +1661,7 @@ mtx_lock(f->priv_mtx); - if (!usbfs_active) { + if (!loc.is_usbfs) { if (f->flag_iserror) { /* we have and error */ m = (void *)1; @@ -1693,8 +1690,10 @@ f->flag_isselect = 1; selrecord(td, &f->selinfo); - /* start reading data */ - (f->methods->f_start_read) (f); + if (!loc.is_usbfs) { + /* start reading data */ + (f->methods->f_start_read) (f); + } } mtx_unlock(f->priv_mtx); @@ -1739,22 +1738,23 @@ mtx_lock(f->priv_mtx); + /* check for permanent read error */ if (f->flag_iserror) { err = EIO; goto done; } + /* check if USB-FS interface is active */ + if (loc.is_usbfs) { + /* + * The queue is used for events that should be + * retrieved using the "USB_FS_COMPLETE" ioctl. + */ + err = EINVAL; + goto done; + } while (uio->uio_resid > 0) { - if (f->fs_ep_max == 0) { - USB_IF_DEQUEUE(&f->used_q, m); - } else { - /* - * The queue is used for events that should be - * retrieved using the "USB_FS_COMPLETE" - * ioctl. - */ - m = NULL; - } + USB_IF_DEQUEUE(&f->used_q, m); if (m == NULL) { @@ -1883,26 +1883,27 @@ mtx_lock(f->priv_mtx); + /* check for permanent write error */ if (f->flag_iserror) { err = EIO; goto done; } - if ((f->queue_data == NULL) && (f->fs_ep_max == 0)) { + /* check if USB-FS interface is active */ + if (loc.is_usbfs) { + /* + * The queue is used for events that should be + * retrieved using the "USB_FS_COMPLETE" ioctl. + */ + err = EINVAL; + goto done; + } + if (f->queue_data == NULL) { /* start write transfer, if not already started */ (f->methods->f_start_write) (f); } /* we allow writing zero length data */ do { - if (f->fs_ep_max == 0) { - USB_IF_DEQUEUE(&f->free_q, m); - } else { - /* - * The queue is used for events that should be - * retrieved using the "USB_FS_COMPLETE" - * ioctl. - */ - m = NULL; - } + USB_IF_DEQUEUE(&f->free_q, m); if (m == NULL) { ==== //depot/projects/usb/src/sys/dev/usb2/core/usb2_generic.c#30 (text+ko) ==== @@ -209,10 +209,6 @@ /* transfers are already opened */ return (0); } - if (f->fs_xfer) { - /* should not happen */ - return (EINVAL); - } bzero(usb2_config, sizeof(usb2_config)); usb2_config[1].type = UE_CONTROL; @@ -281,10 +277,6 @@ /* transfers are already opened */ return (0); } - if (f->fs_xfer) { - /* should not happen */ - return (EINVAL); - } bzero(usb2_config, sizeof(usb2_config)); usb2_config[1].type = UE_CONTROL;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200811132149.mADLn9sC092232>