kern/49391: Fixes to XHCI driver command ring and status TRB

[webpages%sprow.co.uk@localhost]

>Number:         49391
>Category:       kern
>Synopsis:       Fixes to XHCI driver command ring and status TRB
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Nov 13 19:15:01 +0000 2014
>Originator:     Robert Sprowson
>Release:        
>Organization:
>Environment:
>Description:
Encountered a few problems porting the driver to an embedded platform, as described below.

In xhcireg.h:
~~~~~~~~~~~~~
The value of XHCI_COMMAND_RING_SEGMENTS_ALIGN doesn't match table 49 in the XHCI spec. The CRCR uses bits 5:0 as flags or reserved bits, so the address must be 64 byte aligned (even though the commands in the ring are 16 bytes each) not 16. In my port the DMA memory comes from a pool which just does the minimum aligning necessary to meet the request, so ended up with bits 5 or 4 set & the command ring never ran, I guess NetBSD allocates to page size alignment normally.

In xhci.c:
~~~~~~~~~~
usb_allocmem_flags() with a last argument of 0 is equivalent to usb_allocmem(). This cuts down on the number of functions needed to port the driver.

In the status stage TRB sets the TDSZ bits in the status word, however this TRB type defines bits 21:17 to be reserved - so they should be zero.

In the status stage TRB the logic to determine if the control word should have DIR_IN set doesn't match table 7 of the XHCI spec, only for reads of non zero length is the status outgoing.

Typo on "intrerrupt".

>How-To-Repeat:

>Fix:
--- xhci.c.1.27	2014-10-16 11:50:38 +0100
+++ xhci.c	2014-11-13 18:42:20 +0000
@@ -1383,9 +1383,9 @@
 	usbd_status err;
 
 	XHCIHIST_FUNC(); XHCIHIST_CALLED();
 
-	err = usb_allocmem_flags(&sc->sc_bus, size, 0, dma, 0);
+	err = usb_allocmem(&sc->sc_bus, size, 0, dma);
 #if 0
 	if (err == USBD_NOMEM)
 		err = usb_reserve_allocm(&sc->sc_dma_reserve, dma, size);
 #endif
@@ -2490,9 +2490,9 @@
 
 	xfer->hcpriv = NULL;
 }
 
-/* root hub intrerrupt */
+/* root hub interrupt */
 
 static usbd_status
 xhci_root_intr_transfer(usbd_xfer_handle xfer)
 {
@@ -2656,11 +2656,11 @@
 	xhci_trb_put(&xx->xx_trb[i++], parameter, status, control);
 
 no_data:
 	parameter = 0;
-	status = XHCI_TRB_2_IRQ_SET(0) | XHCI_TRB_2_TDSZ_SET(1);
+	status = XHCI_TRB_2_IRQ_SET(0);
 	/* the status stage has inverted direction */
-	control = (isread ? 0 : XHCI_TRB_3_DIR_IN) |
+	control = ((isread && (len > 0)) ? 0 : XHCI_TRB_3_DIR_IN) |
 	    XHCI_TRB_3_TYPE_SET(XHCI_TRB_TYPE_STATUS_STAGE) |
 	    XHCI_TRB_3_CHAIN_BIT | XHCI_TRB_3_ENT_BIT;
 	xhci_trb_put(&xx->xx_trb[i++], parameter, status, control);
 
--- xhcireg.h.1.1	2014-10-20 15:16:20 +0100
+++ xhcireg.h	2014-11-13 18:28:56 +0000
@@ -211,9 +211,9 @@
 #define XHCI_ENDPOINT_CONTEXT_ALIGN 32
 #define XHCI_STREAM_CONTEXT_ALIGN 16
 #define XHCI_STREAM_ARRAY_ALIGN 16
 #define XHCI_TRANSFER_RING_SEGMENTS_ALIGN 16
-#define XHCI_COMMAND_RING_SEGMENTS_ALIGN 16
+#define XHCI_COMMAND_RING_SEGMENTS_ALIGN 64
 #define XHCI_EVENT_RING_SEGMENTS_ALIGN 64
 #define XHCI_EVENT_RING_SEGMENT_TABLE_ALIGN 64
 #define XHCI_SCRATCHPAD_BUFFER_ARRAY_ALIGN 64
 #define XHCI_SCRATCHPAD_BUFFERS_ALIGN XHCI_PAGE_SIZE