From owner-freebsd-usb@FreeBSD.ORG Tue Jan 18 13:00:55 2005 Return-Path: Delivered-To: freebsd-usb@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id B885616A4CF for ; Tue, 18 Jan 2005 13:00:55 +0000 (GMT) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id 73B1443D3F for ; Tue, 18 Jan 2005 13:00:55 +0000 (GMT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.13.1/8.13.1) with ESMTP id j0ID0ta4001473 for ; Tue, 18 Jan 2005 13:00:55 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.13.1/8.13.1/Submit) id j0ID0tu1001472; Tue, 18 Jan 2005 13:00:55 GMT (envelope-from gnats) Date: Tue, 18 Jan 2005 13:00:55 GMT Message-Id: <200501181300.j0ID0tu1001472@freefall.freebsd.org> To: freebsd-usb@FreeBSD.org From: Peter Pentchev Subject: Re: usb/76240: USB camera panics kernel X-BeenThere: freebsd-usb@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list Reply-To: Peter Pentchev List-Id: FreeBSD support for USB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 18 Jan 2005 13:00:55 -0000 The following reply was made to PR usb/76240; it has been noted by GNATS. From: Peter Pentchev To: bug-followup@FreeBSD.org Cc: Subject: Re: usb/76240: USB camera panics kernel Date: Tue, 18 Jan 2005 14:54:55 +0200 On Fri, Jan 14, 2005 at 08:56:15AM +0000, Mark Ovens wrote: > >Synopsis: USB camera panics kernel [snip] > ugen1: Canon Inc. Canon Digital Camera, rev 1.10/0.01, addr 2 [snip] > I've done that, but now instead of "Can't connect to camera" the > computer crashes with a kernel panic [snip] > WARNING: Driver mistake: destroy_dev on 0/0 > panic: don't do that Just to add this to the audit trail: Claude Buisson submitted a patch in the <41E7DA39.6000204@nerim.net> message to the freebsd-usb list. His patch merges most of the -CURRENT version of ugen.c to the RELENG_5 branch. Here's a slightly modified version, which takes care of the 6.0-CURRENT-specific d_purge_t reference by conditional compilation based on __FreeBSD_version. G'luck, Peter Index: src/sys/dev/usb/ugen.c =================================================================== RCS file: /home/ncvs/src/sys/dev/usb/ugen.c,v retrieving revision 1.88.2.2 diff -u -r1.88.2.2 ugen.c --- src/sys/dev/usb/ugen.c 31 Dec 2004 08:01:48 -0000 1.88.2.2 +++ src/sys/dev/usb/ugen.c 18 Jan 2005 12:51:10 -0000 @@ -70,9 +70,10 @@ #else #include #endif -#include #include #include +#include +#include #include #include @@ -136,7 +137,30 @@ #define OUT 0 #define IN 1 +#if defined(__FreeBSD__) && __FreeBSD_version > 600003 +#define UGEN_DEV_REF(dev, sc) \ + if ((sc)->sc_dying || dev_refthread(dev) == NULL) \ + return (ENXIO) +#define UGEN_DEV_RELE(dev, sc) \ + dev_relthread(dev) +#define UGEN_DEV_OPEN(dev, sc) \ + /* handled by dev layer */ +#define UGEN_DEV_CLOSE(dev, sc) \ + /* handled by dev layer */ +#else int sc_refcnt; +#define UGEN_DEV_REF(dev, sc) \ + if ((sc)->sc_dying) \ + return (ENXIO); \ + (sc)->sc_refcnt++ +#define UGEN_DEV_RELE(dev, sc) \ + if (--(sc)->sc_refcnt < 0) \ + usb_detach_wakeup(USBDEV((sc)->sc_dev)) +#define UGEN_DEV_OPEN(dev, sc) \ + (sc)->sc_refcnt++ +#define UGEN_DEV_CLOSE(dev, sc) \ + UGEN_DEV_RELE(dev, sc) +#endif u_char sc_dying; }; @@ -149,7 +173,24 @@ d_write_t ugenwrite; d_ioctl_t ugenioctl; d_poll_t ugenpoll; +#if defined(__FreeBSD__) && __FreeBSD_version > 600003 +d_purge_t ugenpurge; +#endif +Static struct cdevsw ugenctl_cdevsw = { + .d_version = D_VERSION, + .d_flags = D_NEEDGIANT, + .d_open = ugenopen, + .d_close = ugenclose, + .d_ioctl = ugenioctl, +#if defined(__FreeBSD__) && __FreeBSD_version > 600003 + .d_purge = ugenpurge, +#endif + .d_name = "ugenctl", +#if __FreeBSD_version < 500014 + .d_bmaj -1 +#endif +}; Static struct cdevsw ugen_cdevsw = { .d_version = D_VERSION, @@ -160,6 +201,9 @@ .d_write = ugenwrite, .d_ioctl = ugenioctl, .d_poll = ugenpoll, +#if defined(__FreeBSD__) && __FreeBSD_version > 600003 + .d_purge = ugenpurge, +#endif .d_name = "ugen", #if __FreeBSD_version < 500014 .d_bmaj -1 @@ -241,8 +285,10 @@ #if defined(__FreeBSD__) /* the main device, ctrl endpoint */ - sc->dev = make_dev(&ugen_cdevsw, UGENMINOR(USBDEVUNIT(sc->sc_dev), 0), - UID_ROOT, GID_OPERATOR, 0644, "%s", USBDEVNAME(sc->sc_dev)); + sc->dev = make_dev(&ugenctl_cdevsw, + UGENMINOR(USBDEVUNIT(sc->sc_dev), 0), UID_ROOT, GID_OPERATOR, 0644, + "%s", USBDEVNAME(sc->sc_dev)); + ugen_make_devnodes(sc); #endif USB_ATTACH_SUCCESS_RETURN; @@ -271,6 +317,7 @@ UID_ROOT, GID_OPERATOR, 0644, "%s.%d", USBDEVNAME(sc->sc_dev), endptno); + dev_depends(sc->dev, dev); if (sc->sc_endpoints[endptno][IN].sc != NULL) sc->sc_endpoints[endptno][IN].dev = dev; if (sc->sc_endpoints[endptno][OUT].sc != NULL) @@ -322,10 +369,6 @@ DPRINTFN(1,("ugen_set_config: %s to configno %d, sc=%p\n", USBDEVNAME(sc->sc_dev), configno, sc)); -#if defined(__FreeBSD__) - ugen_destroy_devnodes(sc); -#endif - /* We start at 1, not 0, because we don't care whether the * control endpoint is open or not. It is always present. */ @@ -437,10 +480,6 @@ } } -#if defined(__FreeBSD__) - ugen_make_devnodes(sc); -#endif - return (USBD_NORMAL_COMPLETION); } @@ -471,7 +510,7 @@ if (endpt == USB_CONTROL_ENDPOINT) { sc->sc_is_open[USB_CONTROL_ENDPOINT] = 1; - sc->sc_refcnt++; + UGEN_DEV_OPEN(dev, sc); return (0); } @@ -479,7 +518,7 @@ for (dir = OUT; dir <= IN; dir++) { if (flag & (dir == OUT ? FWRITE : FREAD)) { sce = &sc->sc_endpoints[endpt][dir]; - if (sce == 0 || sce->edesc == 0) + if (sce->edesc == 0) return (ENXIO); } } @@ -582,7 +621,7 @@ } } sc->sc_is_open[endpt] = 1; - sc->sc_refcnt++; + UGEN_DEV_OPEN(dev, sc); return (0); } @@ -610,8 +649,7 @@ if (endpt == USB_CONTROL_ENDPOINT) { DPRINTFN(5, ("ugenclose: close control\n")); sc->sc_is_open[endpt] = 0; - if (--sc->sc_refcnt == 0) - usb_detach_wakeup(USBDEV(sc->sc_dev)); + UGEN_DEV_CLOSE(dev, sc); return (0); } @@ -619,7 +657,7 @@ if (!(flag & (dir == OUT ? FWRITE : FREAD))) continue; sce = &sc->sc_endpoints[endpt][dir]; - if (sce == NULL || sce->pipeh == NULL) + if (sce->pipeh == NULL) continue; DPRINTFN(5, ("ugenclose: endpt=%d dir=%d sce=%p\n", endpt, dir, sce)); @@ -648,8 +686,7 @@ } } sc->sc_is_open[endpt] = 0; - if (--sc->sc_refcnt == 0) - usb_detach_wakeup(USBDEV(sc->sc_dev)); + UGEN_DEV_CLOSE(dev, sc); return (0); } @@ -674,9 +711,6 @@ if (endpt == USB_CONTROL_ENDPOINT) return (ENODEV); - if (sce == NULL) - return (EINVAL); - #ifdef DIAGNOSTIC if (sce->edesc == NULL) { printf("ugenread: no edesc\n"); @@ -693,7 +727,7 @@ /* Block until activity occurred. */ s = splusb(); while (sce->q.c_cc == 0) { - if (flag & IO_NDELAY) { + if (flag & O_NONBLOCK) { splx(s); return (EWOULDBLOCK); } @@ -734,10 +768,10 @@ DPRINTFN(1, ("ugenread: start transfer %d bytes\n",n)); tn = n; err = usbd_bulk_transfer( - xfer, sce->pipeh, - sce->state & UGEN_SHORT_OK ? - USBD_SHORT_XFER_OK : 0, - sce->timeout, buf, &tn, "ugenrb"); + xfer, sce->pipeh, + sce->state & UGEN_SHORT_OK ? + USBD_SHORT_XFER_OK : 0, + sce->timeout, buf, &tn, "ugenrb"); if (err) { if (err == USBD_INTERRUPTED) error = EINTR; @@ -757,7 +791,7 @@ case UE_ISOCHRONOUS: s = splusb(); while (sce->cur == sce->fill) { - if (flag & IO_NDELAY) { + if (flag & O_NONBLOCK) { splx(s); return (EWOULDBLOCK); } @@ -808,7 +842,9 @@ USB_GET_SC(ugen, UGENUNIT(dev), sc); + UGEN_DEV_REF(dev, sc); error = ugen_do_read(sc, endpt, uio, flag); + UGEN_DEV_RELE(dev, sc); return (error); } @@ -830,9 +866,6 @@ if (endpt == USB_CONTROL_ENDPOINT) return (ENODEV); - if (sce == NULL) - return (EINVAL); - #ifdef DIAGNOSTIC if (sce->edesc == NULL) { printf("ugenwrite: no edesc\n"); @@ -907,7 +940,9 @@ USB_GET_SC(ugen, UGENUNIT(dev), sc); + UGEN_DEV_REF(dev, sc); error = ugen_do_write(sc, endpt, uio, flag); + UGEN_DEV_RELE(dev, sc); return (error); } @@ -929,14 +964,33 @@ } #endif +#if defined(__FreeBSD__) && __FreeBSD_version > 600003 +void +ugenpurge(struct cdev *dev) +{ + int endpt = UGENENDPOINT(dev); + struct ugen_endpoint *sce; + struct ugen_softc *sc; + + if (endpt == USB_CONTROL_ENDPOINT) + return; + USB_GET_SC(ugen, UGENUNIT(dev), sc); + sce = &sc->sc_endpoints[endpt][IN]; + if (sce->pipeh) + usbd_abort_pipe(sce->pipeh); +} +#endif + USB_DETACH(ugen) { USB_DETACH_START(ugen, sc); struct ugen_endpoint *sce; int i, dir; +/* Not sure if this declaration must be moved inside the + defined(__NetBSD__) || defined(__OpenBSD__) below, or not */ int s; #if defined(__NetBSD__) || defined(__OpenBSD__) - int maj, mn; + int maj, mn, c; #endif #if defined(__NetBSD__) || defined(__OpenBSD__) @@ -950,11 +1004,13 @@ for (i = 0; i < USB_MAX_ENDPOINTS; i++) { for (dir = OUT; dir <= IN; dir++) { sce = &sc->sc_endpoints[i][dir]; - if (sce && sce->pipeh) + if (sce->pipeh) usbd_abort_pipe(sce->pipeh); } } +/* Not sure if this instruction group must be moved inside the + defined(__NetBSD__) || defined(__OpenBSD__) below, or not */ s = splusb(); if (sc->sc_refcnt > 0) { /* Wake everyone */ @@ -967,6 +1023,17 @@ splx(s); #if defined(__NetBSD__) || defined(__OpenBSD__) + /* Wait for opens to go away. */ + do { + c = 0; + for (i = 0; i < USB_MAX_ENDPOINTS; i++) { + if (sc->sc_is_open[i]) + c++; + } + if (c != 0) + tsleep(&sc->sc_dying, PZERO, "ugendr", hz); + } while (c != 0); + /* locate the major number */ for (maj = 0; maj < nchrdev; maj++) if (cdevsw[maj].d_open == ugenopen) @@ -978,7 +1045,6 @@ #elif defined(__FreeBSD__) /* destroy the device for the control endpoint */ destroy_dev(sc->dev); - ugen_destroy_devnodes(sc); #endif return (0); @@ -1230,6 +1296,7 @@ switch (cmd) { case FIONBIO: + case FIOASYNC: /* All handled in the upper FS layer. */ return (0); case USB_SET_SHORT_XFER: @@ -1237,8 +1304,6 @@ return (EINVAL); /* This flag only affects read */ sce = &sc->sc_endpoints[endpt][IN]; - if (sce == NULL) - return (EINVAL); if (sce->pipeh == NULL) { printf("ugenioctl: USB_SET_SHORT_XFER, no pipe\n"); @@ -1252,8 +1317,6 @@ return (0); case USB_SET_TIMEOUT: sce = &sc->sc_endpoints[endpt][IN]; - if (sce == NULL) - return (EINVAL); sce->timeout = *(int *)addr; return (0); default: @@ -1281,6 +1344,9 @@ err = ugen_set_config(sc, *(int *)addr); switch (err) { case USBD_NORMAL_COMPLETION: +#if defined(__FreeBSD__) + ugen_make_devnodes(sc); +#endif break; case USBD_IN_USE: return (EBUSY); @@ -1488,7 +1554,9 @@ USB_GET_SC(ugen, UGENUNIT(dev), sc); + UGEN_DEV_REF(dev, sc); error = ugen_do_ioctl(sc, endpt, cmd, addr, flag, p); + UGEN_DEV_RELE(dev, sc); return (error); } @@ -1507,9 +1575,6 @@ /* XXX always IN */ sce = &sc->sc_endpoints[UGENENDPOINT(dev)][IN]; - if (sce == NULL) - return (EINVAL); - #ifdef DIAGNOSTIC if (!sce->edesc) { printf("ugenpoll: no edesc\n"); -- Peter Pentchev roam@ringlet.net roam@cnsys.bg roam@FreeBSD.org PGP key: http://people.FreeBSD.org/~roam/roam.key.asc Key fingerprint FDBA FD79 C26F 3C51 C95E DF9E ED18 B68D 1619 4553 I had to translate this sentence into English because I could not read the original Sanskrit.