Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 1 Jul 2008 16:51:51 GMT
From:      Hans Petter Selasky <hselasky@FreeBSD.org>
To:        Perforce Change Reviews <perforce@FreeBSD.org>
Subject:   PERFORCE change 144428 for review
Message-ID:  <200807011651.m61GppqV045688@repoman.freebsd.org>

next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=144428

Change 144428 by hselasky@hselasky_laptop001 on 2008/07/01 16:51:13

	
	Fix stopping of USB transfers. Some state stuff was
	a little bit broken.

Affected files ...

.. //depot/projects/usb/src/sys/dev/usb2/core/usb2_device.c#4 edit
.. //depot/projects/usb/src/sys/dev/usb2/core/usb2_transfer.c#7 edit

Differences ...

==== //depot/projects/usb/src/sys/dev/usb2/core/usb2_device.c#4 (text+ko) ====

@@ -511,6 +511,7 @@
 
 	if (!sx_xlocked(udev->default_sx + 1)) {
 		/* the caller is not properly locked */
+		DPRINTF(-1, "not locked\n");
 		return (USB_ERR_NOT_LOCKED);
 	}
 	/* detach all interface drivers */
@@ -529,7 +530,6 @@
 		return (err);
 	}
 	/* get the full config descriptor */
-
 	err = usb2_req_get_config_desc_full(udev,
 	    &Giant, &cdp, M_USB, index);
 	if (err) {
@@ -553,36 +553,42 @@
 			/* Must ask device. */
 			if (udev->flags.uq_power_claim) {
 				/*
-				 * Hub claims to be self powered, but isn't.
+				 * HUB claims to be self powered, but isn't.
 				 * It seems that the power status can be
-				 * determined by the hub characteristics.
+				 * determined by the HUB characteristics.
 				 */
 				err = usb2_req_get_hub_descriptor
-				    (udev, &Giant, &hd);
+				    (udev, &Giant, &hd, 1);
+				if (err) {
+					DPRINTF(-1, "could not read "
+					    "HUB descriptor: %s\n",
+					    usb2_errstr(err));
 
-				if (!err &&
-				    (UGETW(hd.wHubCharacteristics) &
-				    UHD_PWR_INDIVIDUAL)) {
+				} else if (UGETW(hd.wHubCharacteristics) &
+				    UHD_PWR_INDIVIDUAL) {
 					selfpowered = 1;
 				}
-				DPRINTF(0, "characteristics=0x%04x, error=%s\n",
-				    UGETW(hd.wHubCharacteristics),
-				    usb2_errstr(err));
+				DPRINTF(0, "characteristics=0x%04x\n",
+				    UGETW(hd.wHubCharacteristics));
 			} else {
+
+				usb2_pause_mtx(&Giant, 100);
+
 				err = usb2_req_get_device_status
 				    (udev, &Giant, &ds);
-
-				if (!err &&
-				    (UGETW(ds.wStatus) & UDS_SELF_POWERED)) {
+				if (err) {
+					DPRINTF(-1, "could not read "
+					    "device status: %s\n",
+					    usb2_errstr(err));
+				} else if (UGETW(ds.wStatus) & UDS_SELF_POWERED) {
 					selfpowered = 1;
 				}
-				DPRINTF(0, "status=0x%04x, error=%s\n",
-				    UGETW(ds.wStatus), usb2_errstr(err));
+				DPRINTF(0, "status=0x%04x \n",
+				    UGETW(ds.wStatus));
 			}
 		} else
 			selfpowered = 1;
 	}
-
 	DPRINTF(0, "udev=%p cdesc=%p (addr %d) cno=%d attr=0x%02x, "
 	    "selfpowered=%d, power=%d\n",
 	    udev, cdp,
@@ -612,8 +618,7 @@
 	udev->curr_config_index = index;
 
 	/* Set the actual configuration value. */
-	err = usb2_req_set_config(udev, &Giant, 
-cdp->bConfigurationValue);
+	err = usb2_req_set_config(udev, &Giant, cdp->bConfigurationValue);
 	if (err) {
 		goto error;
 	}
@@ -654,6 +659,7 @@
 
 	if (!sx_xlocked(udev->default_sx + 1)) {
 		/* the caller is not properly locked */
+		DPRINTF(-1, "not locked\n");
 		err = USB_ERR_NOT_LOCKED;
 		goto done;
 	}
@@ -1091,7 +1097,6 @@
 		if (usb2_probe_and_attach_sub(udev, &uaa)) {
 			/* ignore */
 		}
-
 		/* try generic interface drivers last */
 
 		uaa.use_generic = 1;
@@ -1452,7 +1457,7 @@
 	    udev->ddesc.iSerialNumber) {
 		/* read out the language ID string */
 		err = usb2_req_get_string_desc(udev, &Giant,
-		    scratch_ptr, 4, sizeof(udev->bus->scratch),
+		    scratch_ptr, 4, scratch_size,
 		    USB_LANGUAGE_TABLE);
 	} else {
 		err = USB_ERR_INVAL;

==== //depot/projects/usb/src/sys/dev/usb2/core/usb2_transfer.c#7 (text+ko) ====

@@ -1514,14 +1514,12 @@
 	 * Check if we can cancel the USB transfer immediately.
 	 */
 	if (xfer->flags_int.transferring) {
-		if (xfer->flags_int.can_cancel_immed) {
-			/*
-			 * The close method will be called by the
-			 * "usb2_callback_wrapper()" when it detects
-			 * that we are closed. This way we resolve
-			 * some races.
-			 */
-			usb2_transfer_done(xfer, USB_ERR_CANCELLED);
+		if (xfer->flags_int.can_cancel_immed &&
+		    (!xfer->flags_int.did_close)) {
+			DPRINTF(0, "close\n");
+			(xfer->pipe->methods->close) (xfer);
+			/* only close once */
+			xfer->flags_int.did_close = 1;
 		} else {
 			/* need to wait for the next done callback */
 		}
@@ -1530,6 +1528,11 @@
 
 		/* close here and now */
 		(xfer->pipe->methods->close) (xfer);
+
+		/*
+		 * Any additional DMA delay is done by
+		 * "usb2_transfer_unsetup()".
+		 */
 	}
 
 	mtx_unlock(xfer->usb2_mtx);
@@ -1734,7 +1737,7 @@
 	if ((!xfer->flags_int.open) &&
 	    (xfer->flags_int.started) &&
 	    (xfer->usb2_state == USB_ST_ERROR)) {
-		/* do nothing - just loop */
+		/* try to loop, but not recursivly */
 		usb2_command_wrapper(&(info->done_q), xfer);
 		return;
 	} else if (xfer->flags_int.draining &&
@@ -2125,14 +2128,15 @@
 	struct usb2_pipe *pipe;
 	uint32_t x;
 
-	if (!xfer->flags_int.open &&
-	    !xfer->flags_int.did_close) {
+	if ((!xfer->flags_int.open) &&
+	    (!xfer->flags_int.did_close)) {
 		DPRINTF(0, "close\n");
 		mtx_lock(xfer->usb2_mtx);
 		(xfer->pipe->methods->close) (xfer);
 		mtx_unlock(xfer->usb2_mtx);
 		/* only close once */
 		xfer->flags_int.did_close = 1;
+		return (1);		/* wait for new callback */
 	}
 	/*
 	 * If we have a non-hardware induced error we
@@ -2140,7 +2144,7 @@
 	 */
 	if (((xfer->error == USB_ERR_CANCELLED) ||
 	    (xfer->error == USB_ERR_TIMEOUT)) &&
-	    !xfer->flags_int.did_dma_delay) {
+	    (!xfer->flags_int.did_dma_delay)) {
 
 		uint32_t temp;
 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200807011651.m61GppqV045688>