Date: Sat, 20 Dec 2008 07:26:27 GMT From: Weongyo Jeong <weongyo@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 155043 for review Message-ID: <200812200726.mBK7QRuL067824@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=155043 Change 155043 by weongyo@weongyo_ws on 2008/12/20 07:26:23 get rid of special handlings of the Interrupt IN pipe that the code is unified with the code which handles Interrupt OUT, Bulk IN/OUT. With this routines to manage are simplified and become less errors. Moreover some hacks due to abnormal behaviors of usbd_open_pipe_intr() are removed so I think the code becomes more easy to understand. Now some problems are fixed that it's occured when it try to halt th device. Affected files ... .. //depot/projects/ndisusb/sys/compat/ndis/subr_usbd.c#32 edit Differences ... ==== //depot/projects/ndisusb/sys/compat/ndis/subr_usbd.c#32 (text+ko) ==== @@ -75,7 +75,6 @@ static driver_object usbd_driver; static int32_t usbd_func_bulkintr(irp *); -static int32_t usbd_func_bulkintr_iin(irp *); static int32_t usbd_func_vendorclass(irp *); static int32_t usbd_func_selconf(irp *); static int32_t usbd_func_getdesc(irp *); @@ -85,8 +84,6 @@ static usbd_status usbd_init_ndispipe(irp *, usb_endpoint_descriptor_t *); static usbd_xfer_handle usbd_init_ndisxfer(irp *, usb_endpoint_descriptor_t *, void *, uint32_t); -static void usbd_intr(usbd_xfer_handle, usbd_private_handle, - usbd_status); static int32_t usbd_iodispatch(device_object *, irp *); static int32_t usbd_ioinvalid(device_object *, irp *); static int32_t usbd_pnp(device_object *, irp *); @@ -95,8 +92,6 @@ uint32_t, io_stack_location *); static void usbd_irpcancel(device_object *, irp *); static void usbd_irpcancel_cb(void *); -static void usbd_irpcancel_iin(device_object *, irp *); -static void usbd_irpcancel_iin_cb(void *); static int32_t usbd_submit_urb(irp *); static int32_t usbd_urb2nt(int32_t); static void usbd_xfereof(usbd_xfer_handle, usbd_private_handle, @@ -119,8 +114,6 @@ static usb_interface_descriptor_t *USBD_ParseConfigurationDescriptor( usb_config_descriptor_t *, uint8_t, uint8_t); -#define USBD_STATUS_IIN_ERROR 0x1 -static int usbd_iin_error; /* * We need to wrap these functions because these need `context switch' from * Windows to UNIX before it's called. @@ -130,7 +123,6 @@ static funcptr usbd_pnp_wrap; static funcptr usbd_power_wrap; static funcptr usbd_irpcancel_wrap; -static funcptr usbd_irpcancel_iin_wrap; static funcptr usbd_xfertask_wrap; int @@ -157,8 +149,6 @@ (funcptr *)&usbd_power_wrap, 2, WINDRV_WRAP_STDCALL); windrv_wrap((funcptr)usbd_irpcancel, (funcptr *)&usbd_irpcancel_wrap, 2, WINDRV_WRAP_STDCALL); - windrv_wrap((funcptr)usbd_irpcancel_iin, - (funcptr *)&usbd_irpcancel_iin_wrap, 2, WINDRV_WRAP_STDCALL); windrv_wrap((funcptr)usbd_xfertask, (funcptr *)&usbd_xfertask_wrap, 2, WINDRV_WRAP_STDCALL); @@ -199,7 +189,6 @@ windrv_unwrap(usbd_pnp_wrap); windrv_unwrap(usbd_power_wrap); windrv_unwrap(usbd_irpcancel_wrap); - windrv_unwrap(usbd_irpcancel_iin_wrap); windrv_unwrap(usbd_xfertask_wrap); free(usbd_driver.dro_drivername.us_buf, M_DEVBUF); @@ -722,126 +711,6 @@ return usbd_usb2urb(status); } -static void -usbd_irpcancel_iin_cb(priv) - void *priv; -{ - struct ndisusb_cancel *nc = priv; - struct ndis_softc *sc = device_get_softc(nc->dev); - usbd_status status; - - if (sc->ndisusb_status & NDISUSB_STATUS_DETACH) - goto exit; - - status = usbd_abort_pipe(sc->ndisusb_ep[NDISUSB_ENDPT_IIN]); - if (status != USBD_NORMAL_COMPLETION) - device_printf(nc->dev, "IIN can't be canceld"); -exit: - free(nc, M_USBDEV); -} - -static void -usbd_irpcancel_iin(dobj, ip) - device_object *dobj; - irp *ip; -{ - device_t dev = IRP_NDIS_DEV(ip); - struct ndisusb_cancel *nc; - struct usb_attach_arg *uaa = device_get_ivars(dev); - - /* XXX see the description of usbd_irpcancel() */ - nc = malloc(sizeof(struct ndisusb_cancel), M_USBDEV, M_NOWAIT | M_ZERO); - if (nc == NULL) { - ip->irp_cancel = FALSE; - IoReleaseCancelSpinLock(ip->irp_cancelirql); - return; - } - - nc->dev = dev; - nc->xfer = IRP_NDISUSB_XFER(ip); - usb_init_task(&nc->task, usbd_irpcancel_iin_cb, nc); - - IRP_NDISUSB_XFER(ip) = NULL; - usb_add_task(uaa->device, &nc->task, USB_TASKQ_DRIVER); - - ip->irp_cancel = TRUE; - IoReleaseCancelSpinLock(ip->irp_cancelirql); -} - -static int32_t -usbd_func_bulkintr_iin(ip) - irp *ip; -{ - char *iin_buf; - device_t dev = IRP_NDIS_DEV(ip); - struct ndis_softc *sc = device_get_softc(dev); - struct usb_attach_arg *uaa = device_get_ivars(dev); - struct usbd_urb_bulk_or_intr_transfer *ubi; - union usbd_urb *urb; - usb_endpoint_descriptor_t *ep; - usbd_interface_handle iface; - usbd_status status; -#ifdef NDISUSB_DEBUG - static irp *debug_irp = NULL; -#endif - - urb = usbd_geturb(ip); - - if (sc->ndisusb_ep[NDISUSB_ENDPT_IIN] != NULL) { -#ifdef NDISUSB_DEBUG - if (debug_irp != NULL && debug_irp != ip) - device_printf(dev, - "trying to re-initialize IIN with other IRP\n"); -#endif - /* don't need to open the NDISUSB_ENDPT_IIN pipe again. */ - USBD_URB_STATUS(urb) = USBD_STATUS_PENDING; - return (USBD_STATUS_PENDING); - } - - status = usbd_device2interface_handle(uaa->device, NDISUSB_IFACE_INDEX, - &iface); - if (status != USBD_NORMAL_COMPLETION) { - device_printf(dev, "could not get interface handle\n"); - return usbd_usb2urb(status); - } - - ubi = &urb->uu_bulkintr; - ep = ubi->ubi_epdesc; - - if (ubi->ubi_trans_buf != NULL && - MmIsAddressValid(ubi->ubi_trans_buf) == FALSE && - ubi->ubi_trans_buflen > 0) { - iin_buf = malloc(ubi->ubi_trans_buflen, M_USBDEV, - M_NOWAIT | M_ZERO); - if (iin_buf == NULL) - return USBD_STATUS_NO_MEMORY; - - sc->ndisusb_iin_buf = iin_buf; - } else - iin_buf = ubi->ubi_trans_buf; - - status = usbd_open_pipe_intr(iface, ep->bEndpointAddress, - USBD_SHORT_XFER_OK, &sc->ndisusb_ep[NDISUSB_ENDPT_IIN], ip, iin_buf, - ubi->ubi_trans_buflen, usbd_intr, USBD_DEFAULT_INTERVAL); - if (status != USBD_NORMAL_COMPLETION) { - device_printf(dev, "open IIN pipe failed: %s\n", - usbd_errstr(status)); - return usbd_usb2urb(status); - } - -#ifdef NDISUSB_DEBUG - debug_irp = ip; -#endif - - IoAcquireCancelSpinLock(&ip->irp_cancelirql); - ip->irp_iostat.isb_status = STATUS_SUCCESS; - ip->irp_cancelfunc = (cancel_func)usbd_irpcancel_iin_wrap; - USBD_URB_STATUS(urb) = USBD_STATUS_SUCCESS; - IoReleaseCancelSpinLock(ip->irp_cancelirql); - - return usbd_usb2urb(status); -} - static usbd_status usbd_init_ndispipe(ip, ep) irp *ip; @@ -860,13 +729,6 @@ return (status); } - if (!(UE_GET_DIR(ep->bEndpointAddress) == UE_DIR_IN || - UE_GET_DIR(ep->bEndpointAddress) == UE_DIR_OUT)) { - device_printf(dev, "unexpected direction 0x%x\n", - UE_GET_DIR(ep->bEndpointAddress)); - return (USBD_INVAL); - } - switch (UE_GET_XFERTYPE(ep->bmAttributes)) { case UE_BULK: if (UE_GET_DIR(ep->bEndpointAddress) == UE_DIR_IN) { @@ -888,9 +750,16 @@ USBD_EXCLUSIVE_USE, &sc->ndisusb_ep[NDISUSB_ENDPT_BOUT]); break; case UE_INTERRUPT: - if (UE_GET_DIR(ep->bEndpointAddress) == UE_DIR_IN) - /* incorrect call. */ - return (USBD_INVAL); + if (UE_GET_DIR(ep->bEndpointAddress) == UE_DIR_IN) { + /* Interrupt IN. */ + if (sc->ndisusb_ep[NDISUSB_ENDPT_IIN] != NULL) + return (USBD_NORMAL_COMPLETION); + + status = usbd_open_pipe(iface, ep->bEndpointAddress, + USBD_EXCLUSIVE_USE, + &sc->ndisusb_ep[NDISUSB_ENDPT_IIN]); + break; + } /* Interrupt OUT. */ if (sc->ndisusb_ep[NDISUSB_ENDPT_IOUT] != NULL) @@ -1043,42 +912,6 @@ } static void -usbd_intr(xfer, priv, status) - usbd_xfer_handle xfer; - usbd_private_handle priv; - usbd_status status; -{ - int set = 0; - - /* - * We should handle the interrupt IN pipe specially. For a example, - * ZD1211B NDIS driver uses the interrupt IN pipe so we opens its pipe - * using usbd_open_pipe_intr(). In a case of we forcibly plug out - * the ZD1211B USB adapter then firstly usbd_open_pipe_intr() calls our - * callback function whose the value of the status is USBD_IOERROR. - * As a next return, sometimes it returns USBD_CANCELLED. The problem - * is that at this case when we get USBD_IOERROR status we try to free - * IRP related with the interrupt IN pipe. As next if we get - * USBD_CANCELLED, in the previous we tried to free again so it causes - * a page fault. I don't know why the USB framework reports errors - * twice. To solve this problem we only handle first error. - * - * XXX I don't want to use a global variable and don't like this like - * a hack but no way to pass `sc' safely because `priv' can be a pointer - * which already be freed. - */ - if (status == USBD_NORMAL_COMPLETION) - usbd_iin_error &= ~USBD_STATUS_IIN_ERROR; - if (status == USBD_IOERROR) { - usbd_iin_error |= USBD_STATUS_IIN_ERROR; - set = 1; - } - - if ((!(usbd_iin_error & USBD_STATUS_IIN_ERROR)) || set == 1) - usbd_xferadd(xfer, priv, status, 0); -} - -static void usbd_xfereof(xfer, priv, status) usbd_xfer_handle xfer; usbd_private_handle priv; @@ -1209,18 +1042,6 @@ if (ep == NULL) return (USBD_STATUS_INVALID_PIPE_HANDLE); - if (UE_GET_XFERTYPE(ep->bmAttributes) == UE_INTERRUPT && - UE_GET_DIR(ep->bEndpointAddress) == UE_DIR_IN) { - /* - * when we handle Interrupt IN we should proceed this unlike - * what Windows does because in FreeBSD we normally try to use - * usbd_open_pipe_intr() and it will never be called again after - * setting. However, in cases of Windows, it looks that it try - * to re-set whenever it's completion routine is called. - */ - return usbd_func_bulkintr_iin(ip); - } - status = usbd_init_ndispipe(ip, ep); if (status != USBD_NORMAL_COMPLETION) return usbd_usb2urb(status); @@ -1232,35 +1053,37 @@ return (USBD_STATUS_NO_MEMORY); } + if (UE_GET_DIR(ep->bEndpointAddress) == UE_DIR_IN) { + xfer->flags |= USBD_SHORT_XFER_OK; + if (!(ubi->ubi_trans_flags & USBD_SHORT_TRANSFER_OK)) + xfer->flags &= ~USBD_SHORT_XFER_OK; + } + if (UE_GET_XFERTYPE(ep->bmAttributes) == UE_BULK) { - if (UE_GET_DIR(ep->bEndpointAddress) == UE_DIR_IN) { + if (UE_GET_DIR(ep->bEndpointAddress) == UE_DIR_IN) /* RX (bulk IN) */ - xfer->flags |= USBD_SHORT_XFER_OK; - if (!(ubi->ubi_trans_flags & USBD_SHORT_TRANSFER_OK)) - xfer->flags &= ~USBD_SHORT_XFER_OK; - usbd_setup_xfer(xfer, sc->ndisusb_ep[NDISUSB_ENDPT_BIN], ip, xfer->buffer, xfer->length, xfer->flags, USBD_NO_TIMEOUT, usbd_xfereof); - } else { + else { /* TX (bulk OUT) */ xfer->flags |= USBD_NO_COPY; - + usbd_setup_xfer(xfer, sc->ndisusb_ep[NDISUSB_ENDPT_BOUT], ip, xfer->buffer, xfer->length, xfer->flags, NDISUSB_TX_TIMEOUT, usbd_xfereof); } } else { - /* interrupt OUT */ - KASSERT(UE_GET_XFERTYPE(ep->bmAttributes) == UE_INTERRUPT && - UE_GET_DIR(ep->bEndpointAddress) == UE_DIR_OUT, - ("unexpected endpoint xfertype 0x%x dir 0x%x", - UE_GET_XFERTYPE(ep->bmAttributes), - UE_GET_DIR(ep->bEndpointAddress))); - - usbd_setup_xfer(xfer, sc->ndisusb_ep[NDISUSB_ENDPT_IOUT], - ip, xfer->buffer, xfer->length, xfer->flags, - NDISUSB_INTR_TIMEOUT, usbd_xfereof); + if (UE_GET_DIR(ep->bEndpointAddress) == UE_DIR_IN) + /* Interrupt IN */ + usbd_setup_xfer(xfer, sc->ndisusb_ep[NDISUSB_ENDPT_IIN], + ip, xfer->buffer, xfer->length, xfer->flags, + USBD_NO_TIMEOUT, usbd_xfereof); + else + /* Interrupt OUT */ + usbd_setup_xfer(xfer, sc->ndisusb_ep[NDISUSB_ENDPT_IOUT], + ip, xfer->buffer, xfer->length, xfer->flags, + NDISUSB_INTR_TIMEOUT, usbd_xfereof); } /* we've done to setup xfer. Let's transfer it. */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200812200726.mBK7QRuL067824>