NetBSD Problem Report #25960
Received: (qmail 5088 invoked by uid 605); 18 Jun 2004 01:01:35 -0000
Message-Id: <200406180100.i5I10c30016676@clare.home.matthewgream.net>
Date: Fri, 18 Jun 2004 01:00:38 GMT
From: matthew.gream@pobox.com
Sender: gnats-bugs-owner@NetBSD.org
Reply-To: matthew.gream@pobox.com
To: gnats-bugs@gnats.NetBSD.org
Subject: NetBSD-current usb ugen(4) isochronous fix/enhance
X-Send-Pr-Version: 3.95
>Number: 25960
>Category: kern
>Synopsis: ugen(4) isochronous handling correction and tx support
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: kern-bug-people
>State: open
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Fri Jun 18 01:02:00 +0000 2004
>Closed-Date:
>Last-Modified: Sun Apr 30 11:02:17 +0000 2006
>Originator: Matthew Gream <matthew.gream@pobox.com>
>Release: NetBSD-current (2004/06/18 CVS HEAD)
>Organization:
>Environment:
System: NetBSD usbdev 2.0F NetBSD 2.0F (USBDEV) #101: Thu Jun 17 23:30:20 UTC 2004 root@dev:/usr/src/sys/arch/i386/compile/USBDEV i386
Architecture: i386
Machine: i386
>Description:
Problems with ugen(4) isoc handling:
1. panic during ugenopen as a result of early isr completion in
in ohci_device_isoc_transfer, causing
ohci_device_isoc_start(SIMPLEQ_FIRST(&xfer->pipe->queue)) to
deliver null pointer resulting in panic in
ohci_device_isoc_start(usbd_xfer_handle xfer) on deref of xfer.
2. hcpriv = NULL in ohci_device_isoc_done(usbd_xfer_handle xfer)
would overwrite newly set value from usbd_transfer call in
ugen_isoc_rintr, causing subseqently isoc transfers to fail.
3. circular buffer broken and generally poor.
4. supports read only, no write.
(FreeBSD/NetBSD/OpenBSD PR's cover some of this)
>How-To-Repeat:
Using full-duplex 256kbps USB A/D w/ isoc 80 byte endpoints;
150Mhz Pentium/MMX, USB 1.0 OHCI.
>Fix:
Solutions:
1. locked usb around usbd_transfers in ugenopen.
2. rolled hcpriv = NULL from all _done methods up into ohci_softintr,
and moved post-callback xfer dereferences in usbd_transfer_complete
into temporaries.
3. implemented IBUF_* macros.
4. implemented write support using:
- ioctl USB_SET_ISOC_RATE, USB_GET_ISOC_RATE to specify
outgoing frame sizes (which may be lower than maximum
supported by interface);
- updated fs methods (do_write, do_write, do_poll, etc)
to support transfer and select handling for isoc IN and
OUT cases.
Notes:
- isorate for endpoints is inherited from sce (device) isorate,
both can be directly manipulated by ioctl, would be nicer to allow
user to directly specify outgoing frame sizes.
Limitations:
- first isoc tx xfers will be null filled frames.
- if write queue empty, isoc tx xfers will be silently null filled.
Tested:
With above configuration, stable under moderate load, not stress
tested (yet).
Patched:
: ohci.c
: ohcivar.h
: ugen.c
: usb.h
: usbdi.c
Index: ohci.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/ohci.c,v
retrieving revision 1.146
diff -u -u -r1.146 ohci.c
--- ohci.c 29 Dec 2003 08:17:10 -0000 1.146
+++ ohci.c 18 Jun 2004 00:12:15 -0000
@@ -1443,6 +1443,7 @@
if (uedir == UE_DIR_IN &&
xfer->status == USBD_NORMAL_COMPLETION)
xfer->actlen = actlen;
+ xfer->hcpriv = NULL;
s = splusb();
usb_transfer_complete(xfer);
@@ -1471,7 +1472,6 @@
panic("ohci_device_ctrl_done: not a request");
}
#endif
- xfer->hcpriv = NULL;
}
void
@@ -1486,8 +1486,6 @@
DPRINTFN(10,("ohci_device_intr_done: xfer=%p, actlen=%d\n",
xfer, xfer->actlen));
- xfer->hcpriv = NULL;
-
if (xfer->pipe->repeat) {
data = opipe->tail.td;
tail = ohci_alloc_std(sc); /* XXX should reuse TD */
@@ -1523,8 +1521,6 @@
{
DPRINTFN(10,("ohci_device_bulk_done: xfer=%p, actlen=%d\n",
xfer, xfer->actlen));
-
- xfer->hcpriv = NULL;
}
void
@@ -1564,13 +1560,13 @@
void
ohci_root_intr_done(usbd_xfer_handle xfer)
{
- xfer->hcpriv = NULL;
+ DPRINTFN(10,("ohci_root_intr_done: xfer=%p\n", xfer));
}
void
ohci_root_ctrl_done(usbd_xfer_handle xfer)
{
- xfer->hcpriv = NULL;
+ DPRINTFN(10,("ohci_root_ctrl_done: xfer=%p\n", xfer));
}
/*
@@ -3374,10 +3370,7 @@
void
ohci_device_isoc_done(usbd_xfer_handle xfer)
{
-
- DPRINTFN(1,("ohci_device_isoc_done: xfer=%p\n", xfer));
-
- xfer->hcpriv = NULL;
+ DPRINTFN(10,("ohci_device_isoc_done: xfer=%p\n", xfer));
}
usbd_status
Index: ohcivar.h
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/ohcivar.h,v
retrieving revision 1.32
diff -u -u -r1.32 ohcivar.h
--- ohcivar.h 22 Feb 2003 05:24:17 -0000 1.32
+++ ohcivar.h 18 Jun 2004 00:12:15 -0000
@@ -146,7 +146,7 @@
struct usb_task abort_task;
};
-#define OXFER(xfer) ((struct ehci_xfer *)(xfer))
+#define OXFER(xfer) ((struct ohci_xfer *)(xfer))
usbd_status ohci_init(ohci_softc_t *);
int ohci_intr(void *);
Index: ugen.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/ugen.c,v
retrieving revision 1.67
diff -u -u -r1.67 ugen.c
--- ugen.c 23 Apr 2004 17:25:25 -0000 1.67
+++ ugen.c 18 Jun 2004 00:12:17 -0000
@@ -83,8 +83,8 @@
#define UGEN_BBSIZE 1024
#define UGEN_NISOFRAMES 500 /* 0.5 seconds worth */
-#define UGEN_NISOREQS 6 /* number of outstanding xfer requests */
-#define UGEN_NISORFRMS 4 /* number of frames (miliseconds) per req */
+#define UGEN_NISOREQS 4 /* number of outstanding xfer requests */
+#define UGEN_NISORFRMS 10 /* number of frames (milliseconds) per req */
struct ugen_endpoint {
struct ugen_softc *sc;
@@ -101,6 +101,9 @@
u_char *limit; /* end of circular buffer (isoc) */
u_char *cur; /* current read location (isoc) */
u_int32_t timeout;
+ int isorate;
+ int isoflen;
+ int isofres;
struct isoreq {
struct ugen_endpoint *sce;
usbd_xfer_handle xfer;
@@ -109,6 +112,61 @@
} isoreqs[UGEN_NISOREQS];
};
+#define ISOC_CALC_FLEN(r) \
+ ((r / USB_FRAMES_PER_SECOND) & (~1))
+#define ISOC_CALC_FRES(r) \
+ (((r - USB_FRAMES_PER_SECOND * ISOC_CALC_FLEN(r)) / \
+ (USB_FRAMES_PER_SECOND / UGEN_NISORFRMS)) >> 1)
+#define ISOC_MAKE_FLEN(l,r,o) \
+ (l + ((o < r) ? 2 : 0))
+
+#define IBUF_SPACE_ALLOC(sce,s,d) { \
+ sce->ibuf = malloc(s, d, M_WAITOK); \
+ sce->cur = sce->fill = sce->ibuf; \
+ sce->limit = sce->ibuf + (s); \
+}
+#define IBUF_SPACE_FREE(sce,d) \
+ if (sce->ibuf != NULL) { \
+ free(sce->ibuf, d); \
+ sce->ibuf = NULL; \
+ }
+#define IBUF_SPACE_PNTR(sce) \
+ (sce->ibuf)
+#define IBUF_SPACE_WR_PNTR(sce) \
+ (sce->fill)
+#define IBUF_SPACE_WR_SIZE(sce) \
+ (sce->cur > sce->fill ? \
+ (sce->cur - sce->fill) - 1 : \
+ (sce->limit - sce->fill) + (sce->cur - sce->ibuf) - 1)
+#define IBUF_SPACE_WR_ZERO(sce) \
+ (sce->fill + 1 == sce->cur) || \
+ (sce->fill + 1 == sce->limit && sce->cur == sce->ibuf)
+#define IBUF_SPACE_WR_CHNK(sce) \
+ (sce->cur > sce->fill ? \
+ (sce->cur - sce->fill) - 1 : \
+ (sce->limit - sce->fill) - (sce->cur == sce->ibuf ? 1 : 0))
+#define IBUF_SPACE_WR_INCR(sce, n) { \
+ sce->fill += (n); \
+ if (sce->fill >= sce->limit) \
+ sce->fill = sce->ibuf + (sce->fill - sce->limit); \
+ }
+#define IBUF_SPACE_RD_PNTR(sce) \
+ (sce->cur)
+#define IBUF_SPACE_RD_SIZE(sce) \
+ (sce->fill >= sce->cur ? \
+ sce->fill - sce->cur : \
+ (sce->limit - sce->cur) + (sce->fill - sce->ibuf))
+#define IBUF_SPACE_RD_ZERO(sce) \
+ (sce->fill == sce->cur)
+#define IBUF_SPACE_RD_CHNK(sce) \
+ (sce->fill > sce->cur ? \
+ sce->fill - sce->cur : sce->limit - sce->cur)
+#define IBUF_SPACE_RD_INCR(sce, n) { \
+ sce->cur += (n); \
+ if (sce->cur >= sce->limit) \
+ sce->cur = sce->ibuf + (sce->cur - sce->limit); \
+ }
+
struct ugen_softc {
USBBASEDEVICE sc_dev; /* base device */
usbd_device_handle sc_udev;
@@ -117,6 +175,7 @@
struct ugen_endpoint sc_endpoints[USB_MAX_ENDPOINTS][2];
#define OUT 0
#define IN 1
+ int sc_isorate;
int sc_refcnt;
u_char sc_dying;
@@ -167,12 +226,13 @@
Static void ugenintr(usbd_xfer_handle xfer, usbd_private_handle addr,
usbd_status status);
-Static void ugen_isoc_rintr(usbd_xfer_handle xfer, usbd_private_handle addr,
- usbd_status status);
+Static void ugenintr_isoc(usbd_xfer_handle xfer, usbd_private_handle addr,
+ usbd_status status);
Static int ugen_do_read(struct ugen_softc *, int, struct uio *, int);
Static int ugen_do_write(struct ugen_softc *, int, struct uio *, int);
Static int ugen_do_ioctl(struct ugen_softc *, int, u_long,
caddr_t, int, usb_proc_ptr);
+Static int ugen_do_poll(struct ugen_softc *, int, int, usb_proc_ptr);
Static int ugen_set_config(struct ugen_softc *sc, int configno);
Static usb_config_descriptor_t *ugen_get_cdesc(struct ugen_softc *sc,
int index, int *lenp);
@@ -324,6 +384,7 @@
usbd_xfer_handle xfer;
void *buf;
int i, j;
+ int s;
USB_GET_SC_OPEN(ugen, unit, sc);
@@ -366,18 +427,22 @@
isize = UGETW(edesc->wMaxPacketSize);
if (isize == 0) /* shouldn't happen */
return (EINVAL);
- sce->ibuf = malloc(isize, M_USBDEV, M_WAITOK);
DPRINTFN(5, ("ugenopen: intr endpt=%d,isize=%d\n",
endpt, isize));
- if (clalloc(&sce->q, UGEN_IBSIZE, 0) == -1)
+ IBUF_SPACE_ALLOC(sce, isize, M_USBDEV);
+ if (!IBUF_SPACE_PNTR(sce))
+ return (ENOMEM);
+ if (clalloc(&sce->q, UGEN_IBSIZE, 0) == -1) {
+ IBUF_SPACE_FREE(sce, M_USBDEV);
return (ENOMEM);
+ }
err = usbd_open_pipe_intr(sce->iface,
edesc->bEndpointAddress,
USBD_SHORT_XFER_OK, &sce->pipeh, sce,
- sce->ibuf, isize, ugenintr,
+ IBUF_SPACE_PNTR(sce), isize, ugenintr,
USBD_DEFAULT_INTERVAL);
if (err) {
- free(sce->ibuf, M_USBDEV);
+ IBUF_SPACE_FREE(sce, M_USBDEV);
clfree(&sce->q);
return (EIO);
}
@@ -390,25 +455,25 @@
return (EIO);
break;
case UE_ISOCHRONOUS:
- if (dir == OUT)
- return (EINVAL);
isize = UGETW(edesc->wMaxPacketSize);
if (isize == 0) /* shouldn't happen */
return (EINVAL);
- sce->ibuf = malloc(isize * UGEN_NISOFRAMES,
- M_USBDEV, M_WAITOK);
- sce->cur = sce->fill = sce->ibuf;
- sce->limit = sce->ibuf + isize * UGEN_NISOFRAMES;
DPRINTFN(5, ("ugenopen: isoc endpt=%d, isize=%d\n",
endpt, isize));
+ IBUF_SPACE_ALLOC(sce, isize * UGEN_NISOFRAMES,
+ M_USBDEV);
+ if (!IBUF_SPACE_PNTR(sce))
+ return (ENOMEM);
err = usbd_open_pipe(sce->iface,
edesc->bEndpointAddress, 0, &sce->pipeh);
if (err) {
- free(sce->ibuf, M_USBDEV);
+ IBUF_SPACE_FREE(sce, M_USBDEV);
return (EIO);
}
+ sce->isorate = sc->sc_isorate;
+ sce->isoflen = ISOC_CALC_FLEN(sce->isorate);
+ sce->isofres = ISOC_CALC_FRES(sce->isorate);
for(i = 0; i < UGEN_NISOREQS; ++i) {
- sce->isoreqs[i].sce = sce;
xfer = usbd_alloc_xfer(sc->sc_udev);
if (xfer == 0)
goto bad;
@@ -420,20 +485,30 @@
goto bad;
}
sce->isoreqs[i].dmabuf = buf;
+ sce->isoreqs[i].sce = sce;
+ if (dir == OUT)
+ memset(buf, 0, isize * UGEN_NISORFRMS);
for(j = 0; j < UGEN_NISORFRMS; ++j)
- sce->isoreqs[i].sizes[j] = isize;
+ sce->isoreqs[i].sizes[j] =
+ (dir == IN) ? isize :
+ ISOC_MAKE_FLEN(sce->isoflen,
+ sce->isofres, j);
usbd_setup_isoc_xfer
(xfer, sce->pipeh, &sce->isoreqs[i],
sce->isoreqs[i].sizes,
UGEN_NISORFRMS, USBD_NO_COPY,
- ugen_isoc_rintr);
- (void)usbd_transfer(xfer);
+ ugenintr_isoc);
}
+ s = splusb();
+ for(i = 0; i < UGEN_NISOREQS; ++i)
+ (void)usbd_transfer(sce->isoreqs[i].xfer);
+ splx(s);
DPRINTFN(5, ("ugenopen: isoc open done\n"));
break;
bad:
while (--i >= 0) /* implicit buffer free */
usbd_free_xfer(sce->isoreqs[i].xfer);
+ IBUF_SPACE_FREE(sce, M_USBDEV);
return (ENOMEM);
case UE_CONTROL:
sce->timeout = USBD_DEFAULT_TIMEOUT;
@@ -497,11 +572,7 @@
break;
}
- if (sce->ibuf != NULL) {
- free(sce->ibuf, M_USBDEV);
- sce->ibuf = NULL;
- clfree(&sce->q);
- }
+ IBUF_SPACE_FREE(sce, M_USBDEV);
}
sc->sc_is_open[endpt] = 0;
@@ -607,43 +678,33 @@
break;
case UE_ISOCHRONOUS:
s = splusb();
- while (sce->cur == sce->fill) {
- if (flag & IO_NDELAY) {
- splx(s);
- return (EWOULDBLOCK);
- }
- sce->state |= UGEN_ASLP;
- DPRINTFN(5, ("ugenread: sleep on %p\n", sce));
- error = tsleep(sce, PZERO | PCATCH, "ugenri", 0);
- DPRINTFN(5, ("ugenread: woke, error=%d\n", error));
- if (sc->sc_dying)
- error = EIO;
- if (error) {
- sce->state &= ~UGEN_ASLP;
- break;
+ while (uio->uio_resid > 0 && !error) {
+ if (IBUF_SPACE_RD_ZERO(sce)) {
+ if (flag & IO_NDELAY) {
+ error = EWOULDBLOCK;
+ break;
+ }
+ sce->state |= UGEN_ASLP;
+ DPRINTFN(5, ("ugenread: sleep on %p\n", sce));
+ error = tsleep(sce, PZERO | PCATCH, "ugenri", 0);
+ DPRINTFN(5, ("ugenread: woke, error=%d\n", error));
+ if (sc->sc_dying)
+ error = EIO;
+ if (error)
+ sce->state &= ~UGEN_ASLP;
+ } else {
+ n = min(IBUF_SPACE_RD_CHNK(sce), uio->uio_resid);
+
+ DPRINTFN(5, ("ugenread: isoc read %d bytes\n", n));
+
+ error = uiomove(IBUF_SPACE_RD_PNTR(sce), n, uio);
+ if (!error)
+ IBUF_SPACE_RD_INCR(sce, n);
}
}
-
- while (sce->cur != sce->fill && uio->uio_resid > 0 && !error) {
- if(sce->fill > sce->cur)
- n = min(sce->fill - sce->cur, uio->uio_resid);
- else
- n = min(sce->limit - sce->cur, uio->uio_resid);
-
- DPRINTFN(5, ("ugenread: isoc got %d chars\n", n));
-
- /* Copy the data to the user process. */
- error = uiomove(sce->cur, n, uio);
- if (error)
- break;
- sce->cur += n;
- if(sce->cur >= sce->limit)
- sce->cur = sce->ibuf;
- }
splx(s);
break;
-
default:
return (ENXIO);
}
@@ -675,6 +736,7 @@
char buf[UGEN_BBSIZE];
usbd_xfer_handle xfer;
usbd_status err;
+ int s;
DPRINTFN(5, ("%s: ugenwrite: %d\n", USBDEVNAME(sc->sc_dev), endpt));
@@ -719,6 +781,35 @@
}
usbd_free_xfer(xfer);
break;
+ case UE_ISOCHRONOUS:
+ s = splusb();
+ while(uio->uio_resid > 0 && !error) {
+ if (IBUF_SPACE_WR_ZERO(sce)) {
+ if (flag & IO_NDELAY) {
+ error = EWOULDBLOCK;
+ break;
+ }
+ sce->state |= UGEN_ASLP;
+ DPRINTFN(5, ("ugenwrite: sleep on %p\n", sce));
+ error = tsleep(sce, PZERO | PCATCH, "ugenri", 0);
+ DPRINTFN(5, ("ugenwrite: woke, error=%d\n", error));
+ if (sc->sc_dying)
+ error = EIO;
+ if (error)
+ sce->state &= ~UGEN_ASLP;
+ } else {
+ n = min(IBUF_SPACE_WR_CHNK(sce), uio->uio_resid);
+ DPRINTFN(5, ("ugenwrite: isoc write %d bytes\n", n));
+
+ error = uiomove(IBUF_SPACE_WR_PNTR(sce), n, uio);
+
+ if (!error)
+ IBUF_SPACE_WR_INCR(sce, n);
+ }
+ }
+ splx(s);
+ break;
+
default:
return (ENXIO);
}
@@ -853,59 +944,73 @@
}
Static void
-ugen_isoc_rintr(usbd_xfer_handle xfer, usbd_private_handle addr,
- usbd_status status)
+ugenintr_isoc(usbd_xfer_handle xfer, usbd_private_handle addr,
+ usbd_status status)
{
struct isoreq *req = addr;
struct ugen_endpoint *sce = req->sce;
- u_int32_t count, n;
- int i, isize;
+ u_int32_t count;
+ u_char* buf;
+ int i, dir, psize, n, len;
/* Return if we are aborting. */
if (status == USBD_CANCELLED)
return;
+ dir = UE_GET_DIR(sce->edesc->bEndpointAddress) == UE_DIR_IN ? IN : OUT;
+ psize = UGETW(sce->edesc->wMaxPacketSize);
+
usbd_get_xfer_status(xfer, NULL, NULL, &count, NULL);
- DPRINTFN(5,("ugen_isoc_rintr: xfer %ld, count=%d\n",
- (long)(req - sce->isoreqs), count));
+ DPRINTFN(5,("ugenintr_isoc: xfer %ld, dir %s, count=%d\n",
+ (long)(req - sce->isoreqs), (dir == IN) ? "IN" : "OUT", count));
- /* throw away oldest input if the buffer is full */
- if(sce->fill < sce->cur && sce->cur <= sce->fill + count) {
- sce->cur += count;
- if(sce->cur >= sce->limit)
- sce->cur = sce->ibuf + (sce->limit - sce->cur);
- DPRINTFN(5, ("ugen_isoc_rintr: throwing away %d bytes\n",
- count));
- }
-
- isize = UGETW(sce->edesc->wMaxPacketSize);
- for (i = 0; i < UGEN_NISORFRMS; i++) {
- u_int32_t actlen = req->sizes[i];
- char const *buf = (char const *)req->dmabuf + isize * i;
-
- /* copy data to buffer */
- while (actlen > 0) {
- n = min(actlen, sce->limit - sce->fill);
- memcpy(sce->fill, buf, n);
-
- buf += n;
- actlen -= n;
- sce->fill += n;
- if(sce->fill == sce->limit)
- sce->fill = sce->ibuf;
+ buf = (u_char *)req->dmabuf;
+
+ if (dir == OUT) {
+ for (i = 0; i < UGEN_NISORFRMS; i++) {
+ len = ISOC_MAKE_FLEN(sce->isoflen, sce->isofres, i);
+ req->sizes[i] = len;
+
+ for (; len > 0 && !IBUF_SPACE_RD_ZERO(sce);
+ buf += n, len -= n) {
+ n = min(IBUF_SPACE_RD_CHNK(sce), len);
+ memcpy(buf, IBUF_SPACE_RD_PNTR(sce), n);
+ IBUF_SPACE_RD_INCR(sce, n);
+ }
+
+ if (len > 0) {
+ memset(buf, 0, len);
+ buf += len;
+ }
}
+ } else {
+ for (i = 0; i < UGEN_NISORFRMS; i++) {
+ len = req->sizes[i];
+
+ if ((n = IBUF_SPACE_WR_SIZE(sce)) < len) {
+ IBUF_SPACE_RD_INCR(sce, len - n);
+ DPRINTFN(5, ("ugenintr_isoc: throw %d bytes\n",
+ len - n));
+ }
+
+ for (; len > 0; buf += n, len -= n) {
+ n = min(IBUF_SPACE_WR_CHNK(sce), len);
+ memcpy(IBUF_SPACE_WR_PNTR(sce), buf, n);
+ IBUF_SPACE_WR_INCR(sce, n);
+ }
- /* setup size for next transfer */
- req->sizes[i] = isize;
+ buf += (psize - req->sizes[i]);
+ req->sizes[i] = psize;
+ }
}
usbd_setup_isoc_xfer(xfer, sce->pipeh, req, req->sizes, UGEN_NISORFRMS,
- USBD_NO_COPY, ugen_isoc_rintr);
+ USBD_NO_COPY, ugenintr_isoc);
(void)usbd_transfer(xfer);
if (sce->state & UGEN_ASLP) {
sce->state &= ~UGEN_ASLP;
- DPRINTFN(5, ("ugen_isoc_rintr: waking %p\n", sce));
+ DPRINTFN(5, ("ugenintr_isoc: waking %p\n", sce));
wakeup(sce);
}
selnotify(&sce->rsel, 0);
@@ -1059,6 +1164,28 @@
return (EINVAL);
sce->timeout = *(int *)addr;
return (0);
+ case USB_GET_ISOC_RATE:
+ if (endpt == USB_CONTROL_ENDPOINT)
+ *(int *)addr = sc->sc_isorate;
+ else {
+ sce = &sc->sc_endpoints[endpt][OUT];
+ if (sce == NULL || sce->edesc == NULL)
+ return (EINVAL);
+ *(int *)addr = sce->isorate;
+ }
+ return (0);
+ case USB_SET_ISOC_RATE:
+ if (endpt == USB_CONTROL_ENDPOINT)
+ sc->sc_isorate = *(int *)addr;
+ else {
+ sce = &sc->sc_endpoints[endpt][OUT];
+ if (sce == NULL || sce->edesc == NULL)
+ return (EINVAL);
+ sce->isorate = *(int *)addr;
+ sce->isoflen = ISOC_CALC_FLEN(sce->isorate);
+ sce->isofres = ISOC_CALC_FRES(sce->isorate);
+ }
+ return (0);
default:
break;
}
@@ -1296,59 +1423,58 @@
return (error);
}
-int
-ugenpoll(dev_t dev, int events, usb_proc_ptr p)
+Static int
+ugen_do_poll(struct ugen_softc *sc, int endpt, int events, usb_proc_ptr p)
{
- struct ugen_softc *sc;
- struct ugen_endpoint *sce;
+ struct ugen_endpoint *sce_in, *sce_out;
+ usb_endpoint_descriptor_t *edesc;
int revents = 0;
int s;
- USB_GET_SC(ugen, UGENUNIT(dev), sc);
-
if (sc->sc_dying)
return (EIO);
- /* XXX always IN */
- sce = &sc->sc_endpoints[UGENENDPOINT(dev)][IN];
- if (sce == NULL)
+ sce_in = &sc->sc_endpoints[endpt][IN];
+ sce_out = &sc->sc_endpoints[endpt][OUT];
+ if (sce_in == NULL && sce_out == NULL)
return (EINVAL);
-#ifdef DIAGNOSTIC
- if (!sce->edesc) {
- printf("ugenpoll: no edesc\n");
- return (EIO);
- }
- if (!sce->pipeh) {
- printf("ugenpoll: no pipe\n");
+
+ if (sce_in && sce_in->edesc)
+ edesc = sce_in->edesc;
+ else if (sce_out && sce_out->edesc)
+ edesc = sce_out->edesc;
+ else /* no edesc! */
return (EIO);
- }
-#endif
+
s = splusb();
- switch (sce->edesc->bmAttributes & UE_XFERTYPE) {
+ switch (edesc->bmAttributes & UE_XFERTYPE) {
case UE_INTERRUPT:
- if (events & (POLLIN | POLLRDNORM)) {
- if (sce->q.c_cc > 0)
+ if (sce_in && events & (POLLIN | POLLRDNORM)) {
+ if (sce_in->q.c_cc > 0)
revents |= events & (POLLIN | POLLRDNORM);
else
- selrecord(p, &sce->rsel);
+ selrecord(p, &sce_in->rsel);
}
break;
case UE_ISOCHRONOUS:
- if (events & (POLLIN | POLLRDNORM)) {
- if (sce->cur != sce->fill)
+ if (sce_in && events & (POLLIN | POLLRDNORM)) {
+ if (!IBUF_SPACE_RD_ZERO(sce_in))
revents |= events & (POLLIN | POLLRDNORM);
else
- selrecord(p, &sce->rsel);
+ selrecord(p, &sce_in->rsel);
+ }
+ if (sce_out && events & (POLLOUT | POLLWRNORM)) {
+ if (!IBUF_SPACE_WR_ZERO(sce_out))
+ revents |= events & (POLLOUT | POLLWRNORM);
+ else
+ selrecord(p, &sce_out->rsel);
}
break;
case UE_BULK:
- /*
- * We have no easy way of determining if a read will
- * yield any data or a write will happen.
- * Pretend they will.
- */
- revents |= events &
- (POLLIN | POLLRDNORM | POLLOUT | POLLWRNORM);
+ if (sce_in && events & (POLLIN | POLLRDNORM))
+ revents |= events & (POLLIN | POLLRDNORM);
+ if (sce_out && events & (POLLOUT | POLLWRNORM))
+ revents |= events & (POLLOUT | POLLWRNORM);
break;
default:
break;
@@ -1357,6 +1483,22 @@
return (revents);
}
+int
+ugenpoll(dev_t dev, int events, usb_proc_ptr p)
+{
+ int endpt = UGENENDPOINT(dev);
+ struct ugen_softc *sc;
+ int error;
+
+ USB_GET_SC(ugen, UGENUNIT(dev), sc);
+
+ sc->sc_refcnt++;
+ error = ugen_do_poll(sc, endpt, events, p);
+ if (--sc->sc_refcnt < 0)
+ usb_detach_wakeup(USBDEV(sc->sc_dev));
+ return (error);
+}
+
static void
filt_ugenrdetach(struct knote *kn)
{
@@ -1382,14 +1524,22 @@
{
struct ugen_endpoint *sce = kn->kn_hook;
- if (sce->cur == sce->fill)
+ if (IBUF_SPACE_RD_ZERO(sce))
return (0);
- if (sce->cur < sce->fill)
- kn->kn_data = sce->fill - sce->cur;
- else
- kn->kn_data = (sce->limit - sce->cur) +
- (sce->fill - sce->ibuf);
+ kn->kn_data = IBUF_SPACE_RD_SIZE(sce);
+
+ return (1);
+}
+static int
+filt_ugenwrite_isoc(struct knote *kn, long hint)
+{
+ struct ugen_endpoint *sce = kn->kn_hook;
+
+ if (IBUF_SPACE_WR_ZERO(sce))
+ return (0);
+
+ kn->kn_data = IBUF_SPACE_WR_SIZE(sce);
return (1);
}
@@ -1399,6 +1549,8 @@
static const struct filterops ugenread_isoc_filtops =
{ 1, NULL, filt_ugenrdetach, filt_ugenread_isoc };
+static const struct filterops ugenwrite_isoc_filtops =
+ { 1, NULL, filt_ugenrdetach, filt_ugenwrite_isoc };
static const struct filterops ugen_seltrue_filtops =
{ 1, NULL, filt_ugenrdetach, filt_seltrue };
@@ -1416,13 +1568,11 @@
if (sc->sc_dying)
return (1);
- /* XXX always IN */
- sce = &sc->sc_endpoints[UGENENDPOINT(dev)][IN];
- if (sce == NULL)
- return (1);
-
switch (kn->kn_filter) {
case EVFILT_READ:
+ sce = &sc->sc_endpoints[UGENENDPOINT(dev)][IN];
+ if (sce == NULL || sce->edesc == NULL)
+ return (1);
klist = &sce->rsel.sel_klist;
switch (sce->edesc->bmAttributes & UE_XFERTYPE) {
case UE_INTERRUPT:
@@ -1445,13 +1595,19 @@
break;
case EVFILT_WRITE:
+ sce = &sc->sc_endpoints[UGENENDPOINT(dev)][OUT];
+ if (sce == NULL || sce->edesc == NULL)
+ return (1);
klist = &sce->rsel.sel_klist;
switch (sce->edesc->bmAttributes & UE_XFERTYPE) {
case UE_INTERRUPT:
- case UE_ISOCHRONOUS:
/* XXX poll doesn't support this */
return (1);
+ case UE_ISOCHRONOUS:
+ kn->kn_fop = &ugenwrite_isoc_filtops;
+ break;
+
case UE_BULK:
/*
* We have no easy way of determining if a read will
Index: usb.h
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/usb.h,v
retrieving revision 1.69
diff -u -u -r1.69 usb.h
--- usb.h 22 Sep 2002 23:20:50 -0000 1.69
+++ usb.h 18 Jun 2004 00:12:17 -0000
@@ -682,6 +682,8 @@
#define USB_GET_DEVICEINFO _IOR ('U', 112, struct usb_device_info)
#define USB_SET_SHORT_XFER _IOW ('U', 113, int)
#define USB_SET_TIMEOUT _IOW ('U', 114, int)
+#define USB_GET_ISOC_RATE _IOR ('U', 115, int)
+#define USB_SET_ISOC_RATE _IOW ('U', 116, int)
/* Modem device */
#define USB_GET_CM_OVER_DATA _IOR ('U', 130, int)
Index: usbdi.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/usbdi.c,v
retrieving revision 1.103
diff -u -u -r1.103 usbdi.c
--- usbdi.c 27 Sep 2002 15:37:38 -0000 1.103
+++ usbdi.c 18 Jun 2004 00:12:17 -0000
@@ -758,6 +758,9 @@
{
usbd_pipe_handle pipe = xfer->pipe;
usb_dma_t *dmap = &xfer->dmabuf;
+ int sync = xfer->flags & USBD_SYNCRONOUS;
+ int errd = xfer->status == USBD_CANCELLED ||
+ xfer->status == USBD_TIMEOUT;
int repeat = pipe->repeat;
int polling;
@@ -842,14 +845,12 @@
pipe->methods->done(xfer);
#endif
- if ((xfer->flags & USBD_SYNCHRONOUS) && !polling)
+ if (sync && !polling)
wakeup(xfer);
if (!repeat) {
/* XXX should we stop the queue on all errors? */
- if ((xfer->status == USBD_CANCELLED ||
- xfer->status == USBD_TIMEOUT) &&
- pipe->iface != NULL) /* not control pipe */
+ if (errd && pipe->iface != NULL) /* not control pipe */
pipe->running = 0;
else
usbd_start_next(pipe);
>Release-Note:
>Audit-Trail:
From: Julian Coleman <jdc@coris.org.uk>
To: matthew.gream@pobox.com
Cc: gnats-bugs@gnats.NetBSD.org
Subject: Re: kern/25960: NetBSD-current usb ugen(4) isochronous fix/enhance
Date: Sun, 27 Jun 2004 10:51:19 +0100
> Problems with ugen(4) isoc handling:
>
> 1. panic during ugenopen as a result of early isr completion in
> in ohci_device_isoc_transfer, causing
> ohci_device_isoc_start(SIMPLEQ_FIRST(&xfer->pipe->queue)) to
> deliver null pointer resulting in panic in
> ohci_device_isoc_start(usbd_xfer_handle xfer) on deref of xfer.
I see this when trying to use pkgsrc/graphics/vid to capture images from a
USB webcam on:
Digital AlphaPC 164LX 599 MHz, s/n
ohci0 at pci0 dev 5 function 0: Opti RM861HA (rev. 0x20)
so I was interested in your patches. The good news is that the machine no
longer panics. The bad news is that it keeps reading from /dev/ugen0.01
~forever and generates no output. Any ideas?
Thanks,
J
> Index: usbdi.c
> ===================================================================
> RCS file: /cvsroot/src/sys/dev/usb/usbdi.c,v
> retrieving revision 1.103
> diff -u -u -r1.103 usbdi.c
> --- usbdi.c 27 Sep 2002 15:37:38 -0000 1.103
> +++ usbdi.c 18 Jun 2004 00:12:17 -0000
> @@ -758,6 +758,9 @@
> {
> usbd_pipe_handle pipe = xfer->pipe;
> usb_dma_t *dmap = &xfer->dmabuf;
> + int sync = xfer->flags & USBD_SYNCRONOUS;
That should be USBD_SYNCHRONOUS.
> + int errd = xfer->status == USBD_CANCELLED ||
> + xfer->status == USBD_TIMEOUT;
> int repeat = pipe->repeat;
> int polling;
>
--
My other computer also runs NetBSD / Sailing at Newbiggin
http://www.netbsd.org/ / http://www.newbigginsailingclub.org/
From: "Matthew Gream" <matthew.gream@pobox.com>
To: "'Julian Coleman'" <jdc@coris.org.uk>
Cc: <gnats-bugs@gnats.NetBSD.org>
Subject: RE: kern/25960: NetBSD-current usb ugen(4) isochronous fix/enhance
Date: Tue, 29 Jun 2004 09:54:56 +0100
> > + int sync = xfer->flags & USBD_SYNCRONOUS;
>
> That should be USBD_SYNCHRONOUS.
Yes, correct: my typo in manually preparing the diff for send-pr.
From: "Charles M. Hannum" <mycroft@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: pr/25960 CVS commit: src/sys/dev/usb
Date: Sat, 17 Jul 2004 20:16:13 +0000 (UTC)
Module Name: src
Committed By: mycroft
Date: Sat Jul 17 20:16:13 UTC 2004
Modified Files:
src/sys/dev/usb: usbdi.c
Log Message:
ugen_isoc_rintr() may recycle the xfer immediately. Therefore, we avoid
touching the xfer after calling the callback in usb_transfer_complete().
From PR 25960.
To generate a diff of this commit:
cvs rdiff -r1.103 -r1.104 src/sys/dev/usb/usbdi.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: "Charles M. Hannum" <mycroft@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: pr/25960 CVS commit: src/sys/dev/usb
Date: Sat, 17 Jul 2004 20:24:15 +0000 (UTC)
Module Name: src
Committed By: mycroft
Date: Sat Jul 17 20:24:15 UTC 2004
Modified Files:
src/sys/dev/usb: ohci.c ohcivar.h
Log Message:
Avoid touching the xfer after calling usb_transfer_complete(), as the
driver callback may have recycled it. From PR 25960.
To generate a diff of this commit:
cvs rdiff -r1.150 -r1.151 src/sys/dev/usb/ohci.c
cvs rdiff -r1.32 -r1.33 src/sys/dev/usb/ohcivar.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: Matthias Scheler <tron@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: pr/25960 CVS commit: [netbsd-2-0] src/sys/dev/usb
Date: Fri, 23 Jul 2004 15:44:54 +0000 (UTC)
Module Name: src
Committed By: tron
Date: Fri Jul 23 15:44:54 UTC 2004
Modified Files:
src/sys/dev/usb [netbsd-2-0]: ohci.c
Log Message:
Pull up revision 1.151 (requested by mycroft in ticket #688):
Avoid touching the xfer after calling usb_transfer_complete(), as the
driver callback may have recycled it. From PR 25960.
To generate a diff of this commit:
cvs rdiff -r1.146.2.4 -r1.146.2.5 src/sys/dev/usb/ohci.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: Matthias Scheler <tron@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: pr/25960 CVS commit: [netbsd-2-0] src/sys/dev/usb
Date: Fri, 23 Jul 2004 15:45:15 +0000 (UTC)
Module Name: src
Committed By: tron
Date: Fri Jul 23 15:45:15 UTC 2004
Modified Files:
src/sys/dev/usb [netbsd-2-0]: ohcivar.h
Log Message:
Pull up revision 1.33 (requested by mycroft in ticket #688):
Avoid touching the xfer after calling usb_transfer_complete(), as the
driver callback may have recycled it. From PR 25960.
To generate a diff of this commit:
cvs rdiff -r1.32 -r1.32.4.1 src/sys/dev/usb/ohcivar.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: Matthias Scheler <tron@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: pr/25960 CVS commit: [netbsd-2-0] src/sys/dev/usb
Date: Fri, 23 Jul 2004 15:46:54 +0000 (UTC)
Module Name: src
Committed By: tron
Date: Fri Jul 23 15:46:54 UTC 2004
Modified Files:
src/sys/dev/usb [netbsd-2-0]: usbdi.c
Log Message:
Pull up revision 1.104 (requested by mycroft in ticket #688):
ugen_isoc_rintr() may recycle the xfer immediately. Therefore, we avoid
touching the xfer after calling the callback in usb_transfer_complete().
From PR 25960.
To generate a diff of this commit:
cvs rdiff -r1.103 -r1.103.8.1 src/sys/dev/usb/usbdi.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: Hans Petter Selasky <hselasky@c2i.net>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: kern/25960
Date: Wed, 8 Jun 2005 17:55:19 +0200
Hi,
sce->isorate must be initialized somewhere.
Also it must be range checked.
Is USBD_SET_RATE or USBD_GET_RATE documented somewhere.
I think it is not so smart to pass the total number of
bytes per second. What is important is the frame size,
to synchronize data. Therefore USBD_SET_FRAME_SIZE and
USBD_GET_FRAME_SIZE would be better. And more generic!
Can you change this?
Remember there is USB2.0 with 1000*8 frames per second.
So USBD_SET_RATE might require two different values for
the same application, which is not very good.
PS: EHCI ISOCHRONOUS support at:
http://home.c2i.net/hselasky/isdn4bsd/privat/usb/
--HPS
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2007
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.
Home