On 01/29/15 20:57, Chagin Dmitry wrote:
On Thu, Jan 29, 2015 at 04:04:03PM +0100, Hans Petter Selasky wrote:
On 01/29/15 13:25, Kohji Okuno wrote:
Hi HPS,

I found a bug in xHCI device driver.

Acording to extensible-host-controler-interface-usb-xhci.pdf:"3.2.9
Control Transfers"...

A Data Stage TD consists of a Data Stage TRB followed by zero or more
Normal TRBs. If the data is not physically contiguous, Normal TRBs may
be chained to the Data Stage TRB.


But, in the current imprementation, when two or more TRBs are needed,
the device driver set XHCI_TRB_TYPE_DATA_STAGE to all TRBs.
This is the violation of the spec.

In my minor xHCI, I encountered strange bubble error in a control
transfer. After I changed as the following, I succeeded its control
transfer.

Would you check the following (****)?


Hi Kohji,

You are correct there is a bug, but your patch is not correct.

In FreeBSD we allow SETUP and DATA stages to be done as separate jobs.
That means at the entry of creating a new DATA chain, we need to check
if it is there first DATA packet or not.

Can you test the attached patch and see if it works for you?

patch is lost somewhere, Hans.


Trying again.

I think Kohji got it.

--HPS
Index: sys/dev/usb/controller/xhci.c
===================================================================
--- sys/dev/usb/controller/xhci.c	(revision 277724)
+++ sys/dev/usb/controller/xhci.c	(working copy)
@@ -1866,6 +1866,15 @@
 				    XHCI_TRB_3_TYPE_SET(XHCI_TRB_TYPE_DATA_STAGE);
 				if (temp->direction == UE_DIR_IN)
 					dword |= XHCI_TRB_3_DIR_IN | XHCI_TRB_3_ISP_BIT;
+				/*
+				 * Section 3.2.9 in the XHCI
+				 * specification about control
+				 * transfers says that we should use a
+				 * normal-TRB if there are more TRBs
+				 * extending the data-stage
+				 * TRB. Update the "trb_type".
+				 */
+				temp->trb_type = XHCI_TRB_TYPE_NORMAL;
 				break;
 			case XHCI_TRB_TYPE_STATUS_STAGE:
 				dword = XHCI_TRB_3_CHAIN_BIT | XHCI_TRB_3_CYCLE_BIT |
@@ -2106,7 +2115,8 @@
 		mult = 1;
 		temp.isoc_delta = 0;
 		temp.isoc_frame = 0;
-		temp.trb_type = XHCI_TRB_TYPE_DATA_STAGE;
+		temp.trb_type = usbd_control_transfer_did_data(xfer) ?
+		    XHCI_TRB_TYPE_NORMAL : XHCI_TRB_TYPE_DATA_STAGE;
 	} else {
 		x = 0;
 		mult = 1;
Index: sys/dev/usb/usb_transfer.c
===================================================================
--- sys/dev/usb/usb_transfer.c	(revision 277724)
+++ sys/dev/usb/usb_transfer.c	(working copy)
@@ -1409,6 +1409,31 @@
 }
 
 /*------------------------------------------------------------------------*
+ *	usbd_control_transfer_did_data
+ *
+ * This function returns non-zero in USB host mode if a control
+ * endpoint has done the first DATA packet after the SETUP packet.
+ * Else it return zero.
+ *------------------------------------------------------------------------*/
+uint8_t
+usbd_control_transfer_did_data(struct usb_xfer *xfer)
+{
+	struct usb_device_request req;
+
+	if (xfer->flags_int.usb_mode == USB_MODE_DEVICE ||
+	    xfer->flags_int.control_xfr == 0)
+		return (0);
+
+	/* copy out the USB request header */
+
+	usbd_copy_out(xfer->frbuffers, 0, &req, sizeof(req));
+
+	/* compare remainder to the initial value */
+
+	return (xfer->flags_int.control_rem != UGETW(req.wLength));
+}
+
+/*------------------------------------------------------------------------*
  *	usbd_setup_ctrl_transfer
  *
  * This function handles initialisation of control transfers. Control
Index: sys/dev/usb/usbdi.h
===================================================================
--- sys/dev/usb/usbdi.h	(revision 277724)
+++ sys/dev/usb/usbdi.h	(working copy)
@@ -529,6 +529,7 @@
 void	usbd_transfer_stop(struct usb_xfer *xfer);
 void	usbd_transfer_unsetup(struct usb_xfer **pxfer, uint16_t n_setup);
 void	usbd_transfer_poll(struct usb_xfer **ppxfer, uint16_t max);
+uint8_t	usbd_control_transfer_did_data(struct usb_xfer *xfer);
 void	usbd_set_parent_iface(struct usb_device *udev, uint8_t iface_index,
 	    uint8_t parent_index);
 uint8_t	usbd_get_bus_index(struct usb_device *udev);
_______________________________________________
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to "freebsd-usb-unsubscr...@freebsd.org"

Reply via email to