[USBEHCI_NEW]

- Modify BuildBulkTransferQueueHead to support TransferBufferLengths larger than PAGE_SIZE * 5.
- Acquire a SpinLock before adding QueueHeads to AsyncList and PendingList.
- Dont request a new QueueHead for incomplete transfers in QueueHeadCompletion, as the memory for the just completed QueueHead has not been released yet. Doing so overwrites the m_TransferDescriptor[x] members with new address resulting in memory leaks. Instead request a new QueueHead after the QueueHead has been freed in QueueHeadCleanup.
- Fix a bug where a QueueHead was removed from the m_CompletedRequestAsyncList instead of the m_PendingRequestAsyncList.
- Temporary hackfix InternalCalculateTransferLength to return the TransferBufferLength. This hack will be removed as soon as possible.
- With these changes the hub and ehci driver allow viewing content of and transfers to/from usb disks.

svn path=/branches/usb-bringup/; revision=51684

drivers/usb/usbehci_new/usb_queue.cpp

@@ -138,7 +138,6 @@ CUSBQueue::GetPendingRequestCount()
     // has a IRP to complete.
     //
 
-    
     return 0;
 }
 
@@ -149,6 +148,7 @@ CUSBQueue::AddUSBRequest(
     PQUEUE_HEAD QueueHead;
     NTSTATUS Status;
     ULONG Type;
+    KIRQL OldLevel;
 
     //
     // sanity check
@@ -214,7 +214,9 @@ CUSBQueue::AddUSBRequest(
         //
         // Add it to the pending list
         //
+        KeAcquireSpinLock(&m_Lock, &OldLevel);
         LinkQueueHead(AsyncListQueueHead, QueueHead);
+        KeReleaseSpinLock(&m_Lock, OldLevel);
     }
 
 
@@ -432,42 +434,21 @@ CUSBQueue::QueueHeadCompletion(
     PQUEUE_HEAD CurrentQH,
     NTSTATUS Status)
 {
-    IUSBRequest *Request;
+    KIRQL OldLevel;
-    PQUEUE_HEAD NewQueueHead;
 
     //
     // now unlink the queue head
     // FIXME: implement chained queue heads
     //
+
+    KeAcquireSpinLock(&m_Lock, &OldLevel);
+
     UnlinkQueueHead(CurrentQH);
 
-    //
-    // get contained usb request
-    //
-    Request = (IUSBRequest*)CurrentQH->Request;
-
-    //
-    // check if the request is complete
-    //
-    if (Request->IsRequestComplete() == FALSE)
-    {
-        //
-        // request is still in complete
-        // get new queue head
-        //
-        Status = Request->GetQueueHead(&NewQueueHead);
-
-        //
-        // add to pending list
-        //
-        InsertTailList(&m_PendingRequestAsyncList, &NewQueueHead->LinkedQueueHeads);
-    }
-
-    //
-    // put queue head into completed queue head list
-    //
     InsertTailList(&m_CompletedRequestAsyncList, &CurrentQH->LinkedQueueHeads);
 
+    KeReleaseSpinLock(&m_Lock, OldLevel);
+
 }
 
 VOID
@@ -508,7 +489,6 @@ CUSBQueue::ProcessAsyncList(
         //
         Request = (IUSBRequest*)QueueHead->Request;
 
-
         //
         // move to next entry
         //
@@ -568,6 +548,7 @@ VOID
 CUSBQueue::QueueHeadCleanup(
     PQUEUE_HEAD CurrentQH)
 {
+    PQUEUE_HEAD NewQueueHead;
     IUSBRequest * Request;
     BOOLEAN ShouldReleaseWhenDone;
     USBD_STATUS UrbStatus;
@@ -625,9 +606,47 @@ CUSBQueue::QueueHeadCleanup(
     }
 
     //
-    // notify request that a queue head has been completed
+    // Check if the transfer was completed and if UrbStatus is ok
     //
-    Request->CompletionCallback(STATUS_SUCCESS /*FIXME*/, UrbStatus, CurrentQH);
+    if ((Request->IsRequestComplete() == FALSE) && (UrbStatus == USBD_STATUS_SUCCESS))
+    {
+        //
+        // let IUSBRequest free the queue head
+        //
+        Request->FreeQueueHead(CurrentQH);
+
+        //
+        // request is incomplete, get new queue head
+        //
+        if (Request->GetQueueHead(&NewQueueHead) == STATUS_SUCCESS)
+        {
+            //
+            // add to pending list
+            //
+            InsertTailList(&m_PendingRequestAsyncList, &NewQueueHead->LinkedQueueHeads);
+
+            //
+            // Done for now
+            //
+            return;
+        }
+        DPRINT1("Unable to create a new QueueHead\n");
+        PC_ASSERT(FALSE);
+
+        //
+        // Else there was a problem
+        // FIXME: Find better return
+        UrbStatus = USBD_STATUS_INSUFFICIENT_RESOURCES;
+    }
+
+    if (UrbStatus != USBD_STATUS_SUCCESS) PC_ASSERT(FALSE);
+
+    //
+    // notify request that a transfer has completed
+    //
+    Request->CompletionCallback(UrbStatus != USBD_STATUS_SUCCESS ?  STATUS_UNSUCCESSFUL : STATUS_SUCCESS,
+                                UrbStatus,
+                                CurrentQH);
 
     //
     // let IUSBRequest free the queue head
@@ -666,6 +685,7 @@ CUSBQueue::CompleteAsyncRequests()
     KIRQL OldLevel;
     PLIST_ENTRY Entry;
     PQUEUE_HEAD CurrentQH;
+    IUSBRequest *Request;
 
     DPRINT("CUSBQueue::CompleteAsyncRequests\n");
 
@@ -691,6 +711,11 @@ CUSBQueue::CompleteAsyncRequests()
         //
         CurrentQH = (PQUEUE_HEAD)CONTAINING_RECORD(Entry, QUEUE_HEAD, LinkedQueueHeads);
 
+        //
+        // Get the Request for this QueueHead
+        //
+        Request = (IUSBRequest*) CurrentQH->Request;
+
         //
         // complete request now
         //
@@ -705,7 +730,7 @@ CUSBQueue::CompleteAsyncRequests()
         //
         // remove first entry
         //
-        Entry = RemoveHeadList(&m_CompletedRequestAsyncList);
+        Entry = RemoveHeadList(&m_PendingRequestAsyncList);
 
         //
         // get queue head structure
@@ -713,7 +738,7 @@ CUSBQueue::CompleteAsyncRequests()
         CurrentQH = (PQUEUE_HEAD)CONTAINING_RECORD(Entry, QUEUE_HEAD, LinkedQueueHeads);
 
         //
-        // Add it to the pending list
+        // Add it to the AsyncList list
         //
         LinkQueueHead(AsyncListQueueHead, CurrentQH);
     }

drivers/usb/usbehci_new/usb_request.cpp

@@ -85,6 +85,16 @@ protected:
     //
     ULONG m_TransferBufferLength;
 
+    //
+    // current transfer length
+    //
+    ULONG m_TransferBufferLengthCompleted;
+
+    //
+    // Total Transfer Length
+    //
+    ULONG m_TotalBytesTransferred;
+
     //
     // transfer buffer MDL
     //
@@ -169,6 +179,12 @@ CUSBRequest::InitializeWithSetupPacket(
     m_TransferBufferMDL = TransferBuffer;
     m_DeviceAddress = DeviceAddress;
     m_EndpointDescriptor = EndpointDescriptor;
+    m_TotalBytesTransferred = 0;
+
+    //
+    // Set Length Completed to 0
+    //
+    m_TransferBufferLengthCompleted = 0;
 
     //
     // allocate completion event
@@ -208,6 +224,7 @@ CUSBRequest::InitializeWithIrp(
     PC_ASSERT(Irp);
 
     m_DmaManager = DmaManager;
+    m_TotalBytesTransferred = 0;
 
     //
     // get current irp stack location
@@ -244,7 +261,7 @@ CUSBRequest::InitializeWithIrp(
         case URB_FUNCTION_BULK_OR_INTERRUPT_TRANSFER:
         {
             //
-            // bulk / interrupt transfer
+            // bulk interrupt transfer
             //
             if (Urb->UrbBulkOrInterruptTransfer.TransferBufferLength)
             {
@@ -281,6 +298,10 @@ CUSBRequest::InitializeWithIrp(
                     // FIXME: Does hub driver already do this when passing MDL?
                     //
                     MmBuildMdlForNonPagedPool(m_TransferBufferMDL);
+
+                    //
+                    // Keep that ehci created the MDL and needs to free it.
+                    //
                 }
                 else
                 {
@@ -292,6 +313,11 @@ CUSBRequest::InitializeWithIrp(
                 //
                 m_TransferBufferLength = Urb->UrbBulkOrInterruptTransfer.TransferBufferLength;
 
+                //
+                // Set Length Completed to 0
+                //
+                m_TransferBufferLengthCompleted = 0;
+
                 //
                 // get endpoint descriptor
                 //
@@ -357,7 +383,6 @@ CUSBRequest::CompletionCallback(
         //
         // Check if the MDL was created
         //
-
         if (!Urb->UrbBulkOrInterruptTransfer.TransferBufferMDL)
         {
             //
@@ -439,6 +464,7 @@ CUSBRequest::CancelCallback(
         //
         // store urb status
         //
+        DPRINT1("Request Cancelled\n");
         Urb->UrbHeader.Status = USBD_STATUS_CANCELED;
         Urb->UrbHeader.Length = 0;
 
@@ -521,6 +547,18 @@ CUSBRequest::IsRequestComplete()
     //
     // FIXME: check if request was split
     //
+
+    //
+    // Check if the transfer was completed, only valid for Bulk Transfers
+    //
+    if ((m_TransferBufferLengthCompleted < m_TransferBufferLength)
+        && (GetTransferType() == USB_ENDPOINT_TYPE_BULK))
+    {
+        //
+        // Transfer not completed
+        //
+        return FALSE;
+    }
     return TRUE;
 }
 //----------------------------------------------------------------------------------------
@@ -751,7 +789,7 @@ CUSBRequest::BuildBulkTransferQueueHead(
     ULONG TransferDescriptorCount, Index;
     ULONG BytesAvailable, BufferIndex;
     PVOID Base;
-    ULONG PageOffset;
+    ULONG PageOffset, CurrentTransferBufferLength;
 
     //
     // Allocate the queue head
@@ -770,24 +808,59 @@ CUSBRequest::BuildBulkTransferQueueHead(
     // sanity checks
     //
     PC_ASSERT(QueueHead);
-
-    //
-    // FIXME: support more than one descriptor
-    //
-    PC_ASSERT(m_TransferBufferLength < PAGE_SIZE * 5);
     PC_ASSERT(m_TransferBufferLength);
 
-    TransferDescriptorCount = 1;
+    //
+    // Max default of 3 descriptors
+    //
+    TransferDescriptorCount = 3;
 
     //
     // get virtual base of mdl
     //
     Base = MmGetSystemAddressForMdlSafe(m_TransferBufferMDL, NormalPagePriority);
-    BytesAvailable = m_TransferBufferLength;
+
+    //
+    // Increase the size of last transfer, 0 in case this is the first
+    //
+    Base = (PVOID)((ULONG_PTR)Base + m_TransferBufferLengthCompleted);
 
     PC_ASSERT(m_EndpointDescriptor);
     PC_ASSERT(Base);
 
+    //
+    // Get the offset from page size
+    //
+    PageOffset = BYTE_OFFSET(Base);
+
+    //
+    // PageOffset should only be > 0 if this is the  first transfer for this requests
+    //
+    if ((PageOffset != 0) && (m_TransferBufferLengthCompleted != 0))
+    {
+        ASSERT(FALSE);
+    }
+
+    //
+    // Calculate the size of this transfer
+    //
+    if ((PageOffset != 0) && ((m_TransferBufferLength - m_TransferBufferLengthCompleted) >= (PAGE_SIZE * 4) + PageOffset))
+    {
+        CurrentTransferBufferLength = (PAGE_SIZE * 4) + PageOffset;
+    }
+    else if ((m_TransferBufferLength - m_TransferBufferLengthCompleted) >= PAGE_SIZE * 5)
+    {
+        CurrentTransferBufferLength = PAGE_SIZE * 5;
+    }
+    else
+        CurrentTransferBufferLength = (m_TransferBufferLength - m_TransferBufferLengthCompleted);
+
+    //
+    // Add current transfer length to transfer length completed
+    //
+    m_TransferBufferLengthCompleted += CurrentTransferBufferLength;
+    BytesAvailable = CurrentTransferBufferLength;
+    DPRINT("CurrentTransferBufferLength %x, m_TransferBufferLengthCompleted %x\n", CurrentTransferBufferLength, m_TransferBufferLengthCompleted);
 
     DPRINT("EndPointAddress %x\n", m_EndpointDescriptor->bEndpointAddress);
     DPRINT("EndPointDirection %x\n", USB_ENDPOINT_DIRECTION_IN(m_EndpointDescriptor->bEndpointAddress));
@@ -833,25 +906,15 @@ CUSBRequest::BuildBulkTransferQueueHead(
         for(BufferIndex = 0; BufferIndex < 5; BufferIndex++)
         {
             //
-            // setup buffer
+            // If this is the first buffer of the first descriptor and there is a PageOffset
             //
-            if (BufferIndex == 0)
+            if ((BufferIndex == 0) && (PageOffset != 0) && (Index == 0))
             {
                 //
                 // use physical address
                 //
                 m_TransferDescriptors[Index]->BufferPointer[0] = MmGetPhysicalAddress(Base).LowPart;
 
-                //
-                // get offset within page
-                //
-                PageOffset = BYTE_OFFSET(m_TransferDescriptors[Index]->BufferPointer[0]);
-
-                //
-                // check if request fills another page
-                //
-                if (PageOffset + BytesAvailable > PAGE_SIZE)
-                {
                 //
                 // move to next page
                 //
@@ -860,7 +923,10 @@ CUSBRequest::BuildBulkTransferQueueHead(
                 //
                 // increment transfer bytes
                 //
+                if (CurrentTransferBufferLength > PAGE_SIZE - PageOffset)
                     m_TransferDescriptors[Index]->Token.Bits.TotalBytesToTransfer = PAGE_SIZE - PageOffset;
+                else
+                    m_TransferDescriptors[Index]->Token.Bits.TotalBytesToTransfer = CurrentTransferBufferLength;
 
                 //
                 // decrement available byte count
@@ -871,19 +937,6 @@ CUSBRequest::BuildBulkTransferQueueHead(
                     BufferIndex, m_TransferDescriptors[Index]->Token.Bits.TotalBytesToTransfer, BytesAvailable);
             }
             else
-               {
-                   //
-                   // request ends on the first buffer page
-                   //
-                   m_TransferDescriptors[Index]->Token.Bits.TotalBytesToTransfer = BytesAvailable;
-                   BytesAvailable = 0;
-
-                   DPRINT("TransferDescriptor %p BufferPointer %p BufferIndex %lu TotalBytes %lu Remaining %lu\n", m_TransferDescriptors[Index], m_TransferDescriptors[Index]->BufferPointer[BufferIndex],
-                       BufferIndex, m_TransferDescriptors[Index]->Token.Bits.TotalBytesToTransfer, BytesAvailable);
-                   break;
-               }
-            }
-            else
             {
                 //
                 // the following pages always start on byte zero of each page
@@ -935,7 +988,7 @@ CUSBRequest::BuildBulkTransferQueueHead(
                     BytesAvailable -= BytesAvailable;
 
                     //
-                    // done
+                    // done as this is the last partial or full page
                     //
                     DPRINT("TransferDescriptor %p BufferPointer %p BufferIndex %lu TotalBytes %lu Remaining %lu\n", m_TransferDescriptors[Index], m_TransferDescriptors[Index]->BufferPointer[BufferIndex],
                             BufferIndex, m_TransferDescriptors[Index]->Token.Bits.TotalBytesToTransfer, BytesAvailable);
@@ -943,6 +996,12 @@ CUSBRequest::BuildBulkTransferQueueHead(
                     break;
                 }
             }
+
+            //
+            // Check if all bytes have been consumed
+            //
+            if (BytesAvailable == 0)
+                break;
         }
 
         //
@@ -971,6 +1030,12 @@ CUSBRequest::BuildBulkTransferQueueHead(
         //
         // FIXME need dead queue transfer descriptor?
         //
+
+        //
+        // Check if all bytes have been consumed
+        //
+        if (BytesAvailable == 0)
+            break;
     }
 
     //
@@ -1440,10 +1505,12 @@ VOID
 CUSBRequest::FreeQueueHead(
     IN struct _QUEUE_HEAD * QueueHead)
 {
+    LONG DescriptorCount;
+
     //
     // FIXME: support chained queue heads
     //
-    PC_ASSERT(QueueHead == m_QueueHead);
+    //PC_ASSERT(QueueHead == m_QueueHead);
 
     //
     // release queue head
@@ -1458,32 +1525,40 @@ CUSBRequest::FreeQueueHead(
     //
     // release transfer descriptors
     //
-
+    for (DescriptorCount = 0; DescriptorCount < 3; DescriptorCount++)
-    if (m_TransferDescriptors[0])
+    {
+        if (m_TransferDescriptors[DescriptorCount])
         {
             //
-        // release transfer descriptors
+            // Calculate Total Bytes Transferred
+            // FIXME: Is this the correct method of determine bytes transferred?
             //
-        m_DmaManager->Release(m_TransferDescriptors[0], sizeof(QUEUE_TRANSFER_DESCRIPTOR));
+            if (USB_ENDPOINT_TYPE_BULK == GetTransferType())
-        m_TransferDescriptors[0] = 0;
+            {
+                //
+                // sanity check
+                //
+                ASSERT(m_EndpointDescriptor);
+
+                if (USB_ENDPOINT_DIRECTION_IN(m_EndpointDescriptor->bEndpointAddress))
+                {
+                    DPRINT1("m_TotalBytesTransferred %x, %x - %x\n",
+                        m_TotalBytesTransferred,
+                        m_TransferDescriptors[DescriptorCount]->TotalBytesToTransfer,
+                        m_TransferDescriptors[DescriptorCount]->Token.Bits.TotalBytesToTransfer);
+
+                    m_TotalBytesTransferred +=
+                        m_TransferDescriptors[DescriptorCount]->TotalBytesToTransfer -
+                        m_TransferDescriptors[DescriptorCount]->Token.Bits.TotalBytesToTransfer;
+                }
             }
 
-    if (m_TransferDescriptors[1])
-    {
             //
             // release transfer descriptors
             //
-        m_DmaManager->Release(m_TransferDescriptors[1], sizeof(QUEUE_TRANSFER_DESCRIPTOR));
+            m_DmaManager->Release(m_TransferDescriptors[DescriptorCount], sizeof(QUEUE_TRANSFER_DESCRIPTOR));
-        m_TransferDescriptors[1] = 0;
+            m_TransferDescriptors[DescriptorCount] = 0;
         }
-
-    if (m_TransferDescriptors[2])
-    {
-        //
-        // release transfer descriptors
-        //
-        m_DmaManager->Release(m_TransferDescriptors[2], sizeof(QUEUE_TRANSFER_DESCRIPTOR));
-        m_TransferDescriptors[2] = 0;
     }
 
     if (m_DescriptorPacket)
@@ -1573,8 +1648,9 @@ CUSBRequest::InternalCalculateTransferLength()
     {
         //
         // bulk in request
+        // HACK: Properly determine transfer length
         //
-        return m_TransferDescriptors[0]->TotalBytesToTransfer - m_TransferDescriptors[0]->Token.Bits.TotalBytesToTransfer;
+        return m_TransferBufferLength;//m_TotalBytesTransferred;
     }
 
     //