[ros-diffs] [janderwald] 55634: [USBEHCI] - Abort pipe when performing sync reset request - add assert to check for bogus interface descriptors - use endpoint max packet size when available - flip data toggle ...

Author: janderwald
Date: Thu Feb 16 14:49:59 2012
New Revision: 55634

URL: http://svn.reactos.org/svn/reactos?rev=55634&view=rev
Log:
[USBEHCI]
- Abort pipe when performing sync reset request
- add assert to check for bogus interface descriptors
- use endpoint max packet size when available
- flip data toggle after each transfer
- remove dead code
- use maximum of 4 pages or rest current buffer size when performing a bulk requests
- use nak reload count 3
- perform 1 transaction per frame


Modified:
    trunk/reactos/drivers/usb/usbehci/hub_controller.cpp
    trunk/reactos/drivers/usb/usbehci/usb_device.cpp
    trunk/reactos/drivers/usb/usbehci/usb_queue.cpp
    trunk/reactos/drivers/usb/usbehci/usb_request.cpp

Modified: trunk/reactos/drivers/usb/usbehci/hub_controller.cpp
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/usb/usbehci/hub_co…
==============================================================================
--- trunk/reactos/drivers/usb/usbehci/hub_controller.cpp [iso-8859-1] (original)
+++ trunk/reactos/drivers/usb/usbehci/hub_controller.cpp [iso-8859-1] Thu Feb 16 14:49:59
2012
@@ -1740,13 +1740,26 @@
     //
     if (!ValidateUsbDevice(PUSBDEVICE(Urb->UrbHeader.UsbdDeviceHandle)))
     {
-        DPRINT1("HandleAbortPipe invalid device handle %p\n",
Urb->UrbHeader.UsbdDeviceHandle);
+        DPRINT1("HandleSyncResetAndClearStall invalid device handle %p\n",
Urb->UrbHeader.UsbdDeviceHandle);
 
         //
         // invalid device handle
         //
         return STATUS_DEVICE_NOT_CONNECTED;
     }
+
+    //
+    // abort pipe
+    //
+    Status = HandleAbortPipe(Irp, Urb);
+    if (!NT_SUCCESS(Status))
+    {
+        //
+        // failed
+        //
+        DPRINT1("[USBEHCI] failed to reset pipe %x\n", Status)
+    }
+ 
 
     //
     // get endpoint descriptor

Modified: trunk/reactos/drivers/usb/usbehci/usb_device.cpp
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/usb/usbehci/usb_de…
==============================================================================
--- trunk/reactos/drivers/usb/usbehci/usb_device.cpp [iso-8859-1] (original)
+++ trunk/reactos/drivers/usb/usbehci/usb_device.cpp [iso-8859-1] Thu Feb 16 14:49:59
2012
@@ -819,6 +819,7 @@
             //
             // move to next descriptor
             //
+            ASSERT(InterfaceDescriptor->bLength);
             InterfaceDescriptor =
(PUSB_INTERFACE_DESCRIPTOR)((ULONG_PTR)InterfaceDescriptor +
InterfaceDescriptor->bLength);
         }
 

Modified: trunk/reactos/drivers/usb/usbehci/usb_queue.cpp
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/usb/usbehci/usb_qu…
==============================================================================
--- trunk/reactos/drivers/usb/usbehci/usb_queue.cpp [iso-8859-1] (original)
+++ trunk/reactos/drivers/usb/usbehci/usb_queue.cpp [iso-8859-1] Thu Feb 16 14:49:59 2012
@@ -427,7 +427,7 @@
     //
     // Link the LIST_ENTRYs
     //
-    ASSERT(IsListEmpty(&HeadQueueHead->LinkedQueueHeads));
+    //ASSERT(IsListEmpty(&HeadQueueHead->LinkedQueueHeads));
     InsertTailList(&HeadQueueHead->LinkedQueueHeads,
&NewQueueHead->LinkedQueueHeads);
 
     //
@@ -435,7 +435,7 @@
     //
     Entry = NewQueueHead->LinkedQueueHeads.Blink;
     LastQueueHead = CONTAINING_RECORD(Entry, QUEUE_HEAD, LinkedQueueHeads);
-    ASSERT(LastQueueHead == HeadQueueHead);
+    //ASSERT(LastQueueHead == HeadQueueHead);
     LastQueueHead->HorizontalLinkPointer = (NewQueueHead->PhysicalAddr |
QH_TYPE_QH);
 
     //
@@ -443,7 +443,7 @@
     //
     Entry = NewQueueHead->LinkedQueueHeads.Flink;
     NextQueueHead = CONTAINING_RECORD(Entry, QUEUE_HEAD, LinkedQueueHeads);
-    ASSERT(NextQueueHead == HeadQueueHead);
+    //ASSERT(NextQueueHead == HeadQueueHead);
     NewQueueHead->HorizontalLinkPointer = (NextQueueHead->PhysicalAddr |
QH_TYPE_QH);
 
     //
@@ -858,7 +858,6 @@
     KIRQL OldLevel;
     PLIST_ENTRY Entry;
     PQUEUE_HEAD CurrentQH;
-    IUSBRequest *Request;
 
     DPRINT("CUSBQueue::CompleteAsyncRequests\n");
 
@@ -883,11 +882,6 @@
         // get queue head structure
         //
         CurrentQH = (PQUEUE_HEAD)CONTAINING_RECORD(Entry, QUEUE_HEAD, LinkedQueueHeads);
-
-        //
-        // Get the Request for this QueueHead
-        //
-        Request = (IUSBRequest*) CurrentQH->Request;
 
         //
         // release lock
@@ -937,8 +931,82 @@
     IN UCHAR DeviceAddress,
     IN PUSB_ENDPOINT_DESCRIPTOR EndpointDescriptor)
 {
-    UNIMPLEMENTED
-    return STATUS_NOT_IMPLEMENTED;
+    KIRQL OldLevel;
+    PLIST_ENTRY Entry;
+    PQUEUE_HEAD QueueHead;
+    LIST_ENTRY ListHead;
+
+    //
+    // lock completed async list
+    //
+    KeAcquireSpinLock(m_Lock, &OldLevel);
+
+    DPRINT1("AbortDevicePipe DeviceAddress %x EndpointDescriptor %p Addr %x\n",
DeviceAddress, EndpointDescriptor, EndpointDescriptor->bEndpointAddress);
+
+    //
+    // init list head	
+    //
+    InitializeListHead(&ListHead);
+
+
+    //
+    // walk async list 
+    //
+    ASSERT(AsyncListQueueHead);
+    Entry = AsyncListQueueHead->LinkedQueueHeads.Flink;
+
+    while(Entry != &AsyncListQueueHead->LinkedQueueHeads)
+    {
+        //
+        // get queue head structure
+        //
+        QueueHead = (PQUEUE_HEAD)CONTAINING_RECORD(Entry, QUEUE_HEAD, LinkedQueueHeads);
+        ASSERT(QueueHead);
+
+        //
+        // move to next entry
+        //
+        Entry = Entry->Flink;
+
+        if (QueueHead->EndPointCharacteristics.DeviceAddress == DeviceAddress
&&
+            QueueHead->EndPointCharacteristics.EndPointNumber ==
(EndpointDescriptor->bEndpointAddress & 0xF) &&
QueueHead->Token.Bits.Halted)
+        {
+            //
+            // unlink queue head
+            //
+            UnlinkQueueHead(QueueHead);
+
+            //
+            // add to temp list
+            //
+            InsertTailList(&ListHead, &QueueHead->LinkedQueueHeads);
+        }
+    }
+
+    //
+    // release lock
+    //
+    KeReleaseSpinLock(m_Lock, OldLevel);
+
+    while(!IsListEmpty(&ListHead))
+    {
+        //
+        // remove entry
+        //
+        Entry = RemoveHeadList(&ListHead);
+
+        //
+        // get queue head structure
+        //
+        QueueHead = (PQUEUE_HEAD)CONTAINING_RECORD(Entry, QUEUE_HEAD, LinkedQueueHeads);
+        ASSERT(QueueHead);
+
+        //
+        // cleanup queue head
+        //
+        QueueHeadCleanup(QueueHead);
+    }
+    return STATUS_SUCCESS;
 }
 
 

Modified: trunk/reactos/drivers/usb/usbehci/usb_request.cpp
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/usb/usbehci/usb_re…
==============================================================================
--- trunk/reactos/drivers/usb/usbehci/usb_request.cpp [iso-8859-1] (original)
+++ trunk/reactos/drivers/usb/usbehci/usb_request.cpp [iso-8859-1] Thu Feb 16 14:49:59
2012
@@ -62,10 +62,9 @@
     NTSTATUS BuildSetupPacket();
     NTSTATUS BuildSetupPacketFromURB();
     ULONG InternalCalculateTransferLength();
-    NTSTATUS BuildTransferDescriptorChain(IN PQUEUE_HEAD QueueHead, IN PVOID
TransferBuffer, IN ULONG TransferBufferLength, IN UCHAR PidCode, IN UCHAR
InitialDataToggle, OUT PQUEUE_TRANSFER_DESCRIPTOR * OutFirstDescriptor, OUT
PQUEUE_TRANSFER_DESCRIPTOR * OutLastDescriptor, OUT PUCHAR OutDataToggle, OUT PULONG
OutTransferBufferOffset);
+    NTSTATUS BuildTransferDescriptorChain(IN PQUEUE_HEAD QueueHead, IN PVOID
TransferBuffer, IN ULONG TransferBufferLength, IN UCHAR PidCode, IN UCHAR
InitialDataToggle, IN PQUEUE_TRANSFER_DESCRIPTOR AlternativeDescriptor, OUT
PQUEUE_TRANSFER_DESCRIPTOR * OutFirstDescriptor, OUT PQUEUE_TRANSFER_DESCRIPTOR *
OutLastDescriptor, OUT PUCHAR OutDataToggle, OUT PULONG OutTransferBufferOffset);
     VOID InitDescriptor(IN PQUEUE_TRANSFER_DESCRIPTOR CurrentDescriptor, IN PVOID
TransferBuffer, IN ULONG TransferBufferLength, IN UCHAR PidCode, IN UCHAR DataToggle, OUT
PULONG OutDescriptorLength);
     VOID DumpQueueHead(IN PQUEUE_HEAD QueueHead);
-
 
     // constructor / destructor
     CUSBRequest(IUnknown *OuterUnknown){}
@@ -734,6 +733,7 @@
     IN ULONG TransferBufferLength,
     IN UCHAR PidCode,
     IN UCHAR InitialDataToggle,
+    IN  PQUEUE_TRANSFER_DESCRIPTOR AlternativeDescriptor,
     OUT PQUEUE_TRANSFER_DESCRIPTOR * OutFirstDescriptor,
     OUT PQUEUE_TRANSFER_DESCRIPTOR * OutLastDescriptor,
     OUT PUCHAR OutDataToggle,
@@ -742,6 +742,19 @@
     PQUEUE_TRANSFER_DESCRIPTOR FirstDescriptor = NULL, CurrentDescriptor, LastDescriptor
= NULL;
     NTSTATUS Status;
     ULONG DescriptorLength, TransferBufferOffset  = 0;
+    ULONG MaxPacketSize = 0, TransferSize;
+
+    //
+    // is there an endpoint descriptor
+    //
+    if (m_EndpointDescriptor)
+    {
+        //
+        // use endpoint packet size
+        //
+        MaxPacketSize = m_EndpointDescriptor->EndPointDescriptor.wMaxPacketSize;
+    }
+
 
     do
     {
@@ -754,8 +767,22 @@
             //
             // failed to allocate transfer descriptor
             //
-            ASSERT(FALSE);
-            return Status;
+            return STATUS_INSUFFICIENT_RESOURCES;
+        }
+
+        if (MaxPacketSize)
+        {
+            //
+            // transfer size is minimum available buffer or endpoint size
+            //
+            TransferSize = min(TransferBufferLength - TransferBufferOffset,
MaxPacketSize);
+        }
+        else
+        {
+            //
+            // use available buffer
+            //
+            TransferSize = TransferBufferLength - TransferBufferOffset;
         }
 
         //
@@ -763,7 +790,7 @@
         //
         InitDescriptor(CurrentDescriptor, 
                        (PVOID)((ULONG_PTR)TransferBuffer + TransferBufferOffset),
-                       TransferBufferLength - TransferBufferOffset,
+                       TransferSize,
                        PidCode,
                        InitialDataToggle,
                        &DescriptorLength);
@@ -783,9 +810,17 @@
             //
             // link to current descriptor
             //
-            LastDescriptor->AlternateNextPointer =
CurrentDescriptor->PhysicalAddr;
             LastDescriptor->NextPointer = CurrentDescriptor->PhysicalAddr;
             LastDescriptor = CurrentDescriptor;
+
+            if (AlternativeDescriptor)
+            {
+                //
+                // link to alternative next pointer
+                //
+                LastDescriptor->AlternateNextPointer =
AlternativeDescriptor->PhysicalAddr;
+            }
+
         }
         else
         {
@@ -795,6 +830,11 @@
             LastDescriptor = FirstDescriptor = CurrentDescriptor;
         }
 
+        //
+        // flip data toggle
+        //
+        InitialDataToggle = !InitialDataToggle;
+
         if(TransferBufferLength == TransferBufferOffset)
         {
             //
@@ -802,6 +842,7 @@
             //
             break;
         }
+
     }while(TRUE);
 
     if (OutFirstDescriptor)
@@ -822,11 +863,6 @@
 
     if (OutDataToggle)
     {
-        //
-        // flip data toggle
-        //
-        InitialDataToggle = !InitialDataToggle;
-
         //
         // store result data toggle
         //
@@ -918,14 +954,7 @@
     //
     QueueHead->EndPointCharacteristics.DeviceAddress = GetDeviceAddress();
 
-    if (m_EndpointDescriptor)
-    {
-        //
-        // set endpoint address and max packet length
-        //
-        QueueHead->EndPointCharacteristics.EndPointNumber =
m_EndpointDescriptor->EndPointDescriptor.bEndpointAddress & 0x0F;
-        QueueHead->EndPointCharacteristics.MaximumPacketLength =
m_EndpointDescriptor->EndPointDescriptor.wMaxPacketSize;
-    }
+    ASSERT(m_EndpointDescriptor == FALSE);
 
     //
     // init setup descriptor
@@ -954,6 +983,7 @@
                                               m_TransferBufferLength,
                                               InternalGetPidDirection(),
                                               TRUE,
+                                              NULL,
                                               &FirstDescriptor,
                                               &LastDescriptor,
                                               NULL,
@@ -1137,13 +1167,19 @@
     ASSERT(m_EndpointDescriptor);
 
     //
+    // use 4 * PAGE_SIZE at max for each new request
+    //
+    ULONG MaxTransferLength = min(4 * PAGE_SIZE, m_TransferBufferLength -
m_TransferBufferLengthCompleted);
+
+    //
     // build bulk transfer descriptor chain
     //
     Status = BuildTransferDescriptorChain(QueueHead,
                                           Base,
-                                          m_TransferBufferLength -
m_TransferBufferLengthCompleted,
+                                          MaxTransferLength,
                                           InternalGetPidDirection(),
                                           m_EndpointDescriptor->DataToggle,
+                                          NULL,
                                           &FirstDescriptor,
                                           &LastDescriptor,
                                           &m_EndpointDescriptor->DataToggle,
@@ -1152,7 +1188,7 @@
     //
     // FIXME: handle errors
     //
-    ASSERT(ChainDescriptorLength == m_TransferBufferLength);
+    //ASSERT(ChainDescriptorLength == m_TransferBufferLength);
 
     //
     // move to next offset
@@ -1279,7 +1315,7 @@
     //
     // Set NakCountReload to max value possible
     //
-    QueueHead->EndPointCharacteristics.NakCountReload = 0xF;
+    QueueHead->EndPointCharacteristics.NakCountReload = 0x3;
 
     //
     // Get the Initial Data Toggle from the QEDT
@@ -1290,7 +1326,7 @@
     // FIXME: check if High Speed Device
     //
     QueueHead->EndPointCharacteristics.EndPointSpeed = QH_ENDPOINT_HIGHSPEED;
-    QueueHead->EndPointCapabilities.NumberOfTransactionPerFrame = 0x03;
+    QueueHead->EndPointCapabilities.NumberOfTransactionPerFrame = 0x01;
     QueueHead->Token.DWord = 0;
     QueueHead->Token.Bits.InterruptOnComplete = FALSE;