[ros-diffs] [janderwald] 51888: [USBOHCI] - Add sanity checks - Preserve command status value when notifying the hc that a control / bulk endpoint was added - Re-enable root hub notification interrupt when res...

Author: janderwald
Date: Tue May 24 17:57:00 2011
New Revision: 51888

URL: http://svn.reactos.org/svn/reactos?rev=51888&view=rev
Log:
[USBOHCI]
- Add sanity checks
- Preserve  command status value when notifying the hc that a control / bulk endpoint was
added
- Re-enable root hub notification interrupt when reset port has been completed
- Dispatch processing to USBQeueu when DoneHead event arrives
- Scan endpoints to find the logical endpoint which was completed by the hardware
- Signal completion to IUSBRequest by calling CompletionCallback
- Create a final transfer descriptor which is linked to the endpoint descriptor
- Control transfers now appear to be working (Device retrieves device descriptor /
configuration descriptor and succeeds in setting device address)
- Configuration parsing code needs more work now (currently fails there due to invalid /
unexpected configuration descriptor from device)

Modified:
    branches/usb-bringup/drivers/usb/usbohci/hardware.cpp
    branches/usb-bringup/drivers/usb/usbohci/hardware.h
    branches/usb-bringup/drivers/usb/usbohci/interfaces.h
    branches/usb-bringup/drivers/usb/usbohci/usb_device.cpp
    branches/usb-bringup/drivers/usb/usbohci/usb_queue.cpp
    branches/usb-bringup/drivers/usb/usbohci/usb_request.cpp

Modified: branches/usb-bringup/drivers/usb/usbohci/hardware.cpp
URL:
http://svn.reactos.org/svn/reactos/branches/usb-bringup/drivers/usb/usbohci…
==============================================================================
--- branches/usb-bringup/drivers/usb/usbohci/hardware.cpp [iso-8859-1] (original)
+++ branches/usb-bringup/drivers/usb/usbohci/hardware.cpp [iso-8859-1] Tue May 24 17:57:00
2011
@@ -540,6 +540,7 @@
     // assert that the controller has been started
     //
     ASSERT((Control & OHCI_HC_FUNCTIONAL_STATE_MASK) ==
OHCI_HC_FUNCTIONAL_STATE_OPERATIONAL);
+    ASSERT((Control & OHCI_ENABLE_LIST) == OHCI_ENABLE_LIST);
 
     //
     // get frame interval
@@ -689,20 +690,30 @@
 CUSBHardwareDevice::HeadEndpointDescriptorModified(
     ULONG Type)
 {
+    ULONG Value = READ_REGISTER_ULONG((PULONG)((PUCHAR)m_Base +
OHCI_COMMAND_STATUS_OFFSET));
+
     if (Type == USB_ENDPOINT_TYPE_CONTROL)
     {
         //
         // notify controller
         //
-        WRITE_REGISTER_ULONG((PULONG)((PUCHAR)m_Base + OHCI_COMMAND_STATUS_OFFSET),
OHCI_CONTROL_LIST_FILLED);
+        //WRITE_REGISTER_ULONG((PULONG)((PUCHAR)m_Base + OHCI_CONTROL_HEAD_ED_OFFSET),
m_ControlEndpointDescriptor->NextPhysicalEndpoint);
+        //WRITE_REGISTER_ULONG((PULONG)((PUCHAR)m_Base + OHCI_CONTROL_CURRENT_ED_OFFSET),
m_ControlEndpointDescriptor->NextPhysicalEndpoint);
+        WRITE_REGISTER_ULONG((PULONG)((PUCHAR)m_Base + OHCI_COMMAND_STATUS_OFFSET), Value
| OHCI_CONTROL_LIST_FILLED);
     }
     else if (Type == USB_ENDPOINT_TYPE_BULK)
     {
         //
         // notify controller
         //
-        WRITE_REGISTER_ULONG((PULONG)((PUCHAR)m_Base + OHCI_COMMAND_STATUS_OFFSET),
OHCI_BULK_LIST_FILLED);
-    }
+        WRITE_REGISTER_ULONG((PULONG)((PUCHAR)m_Base + OHCI_COMMAND_STATUS_OFFSET), Value
| OHCI_BULK_LIST_FILLED);
+    }
+
+    Value = READ_REGISTER_ULONG((PULONG)((PUCHAR)m_Base + OHCI_COMMAND_STATUS_OFFSET));
+
+
+    DPRINT1("HeadEndpointDescriptorModified Value %x Type %x\n", Value, Type);
+
 }
 
 NTSTATUS
@@ -1035,7 +1046,9 @@
         //
         // re-enable root hub change
         //
-        WRITE_REGISTER_ULONG((PULONG)((PUCHAR)m_Base + OHCI_INTERRUPT_ENABLE_OFFSET),
OHCI_ROOT_HUB_STATUS_CHANGE);
+        Value = READ_REGISTER_ULONG((PULONG)((PUCHAR)m_Base +
OHCI_INTERRUPT_ENABLE_OFFSET));
+        WRITE_REGISTER_ULONG((PULONG)((PUCHAR)m_Base + OHCI_INTERRUPT_ENABLE_OFFSET),
Value | OHCI_ROOT_HUB_STATUS_CHANGE);
+
     }
 
     if (Status == C_PORT_CONNECTION)
@@ -1231,11 +1244,6 @@
      {
          //
          // head completed
-         //
-         DPRINT1("InterruptServiceRoutine> Done Head completion\n");
-         ASSERT(FALSE);
-         //
-         // FIXME: handle event
          //
          Acknowledge |= OHCI_WRITEBACK_DONE_HEAD;
     }
@@ -1307,6 +1315,8 @@
 {
     CUSBHardwareDevice *This;
     ULONG CStatus, Index, PortStatus;
+    POHCI_ENDPOINT_DESCRIPTOR EndpointDescriptor;
+    ULONG DoneHead;
 
     //
     // get parameters
@@ -1314,6 +1324,25 @@
     This = (CUSBHardwareDevice*) SystemArgument1;
     CStatus = (ULONG) SystemArgument2;
 
+	DPRINT1("OhciDefferedRoutine Status %x\n", CStatus);
+
+    if (CStatus & OHCI_WRITEBACK_DONE_HEAD)
+    {
+        //
+        // descriptor completion, get done head
+        //
+        DoneHead = This->m_HCCA->DoneHead;
+
+        //
+        // clear out lower bits, ed are 16 byte aligned
+        //
+        DoneHead &= ~0xF;
+
+        //
+        // notify queue of event
+        //
+        This->m_UsbQueue->TransferDescriptorCompletionCallback(DoneHead);
+    }
     if (CStatus & OHCI_ROOT_HUB_STATUS_CHANGE)
     {
         //

Modified: branches/usb-bringup/drivers/usb/usbohci/hardware.h
URL:
http://svn.reactos.org/svn/reactos/branches/usb-bringup/drivers/usb/usbohci…
==============================================================================
--- branches/usb-bringup/drivers/usb/usbohci/hardware.h [iso-8859-1] (original)
+++ branches/usb-bringup/drivers/usb/usbohci/hardware.h [iso-8859-1] Tue May 24 17:57:00
2011
@@ -202,8 +202,9 @@
 
     // Software part
     PHYSICAL_ADDRESS  PhysicalAddress;
+    PVOID HeadLogicalDescriptor;
+    PVOID NextDescriptor;
     PVOID Request;
-    PVOID NextDescriptor;
 }OHCI_ENDPOINT_DESCRIPTOR, *POHCI_ENDPOINT_DESCRIPTOR;
 
 
@@ -238,9 +239,9 @@
     ULONG LastPhysicalByteAddress; // Physical pointer to buffer end
     // Software part
     PHYSICAL_ADDRESS  PhysicalAddress;           // Physical address of this descriptor
+    PVOID NextLogicalDescriptor;
     ULONG  BufferSize;                // Size of the buffer
     PVOID    BufferLogical;            // Logical pointer to the buffer
-    PVOID  Request;                   // pointer to IUSBRequest
 }OHCI_GENERAL_TD, *POHCI_GENERAL_TD;
 
 

Modified: branches/usb-bringup/drivers/usb/usbohci/interfaces.h
URL:
http://svn.reactos.org/svn/reactos/branches/usb-bringup/drivers/usb/usbohci…
==============================================================================
--- branches/usb-bringup/drivers/usb/usbohci/interfaces.h [iso-8859-1] (original)
+++ branches/usb-bringup/drivers/usb/usbohci/interfaces.h [iso-8859-1] Tue May 24 17:57:00
2011
@@ -436,6 +436,13 @@
 
     virtual BOOLEAN IsRequestInitialized() = 0;
 
+//-----------------------------------------------------------------------------------------
+//
+// CompletionCallback
+//
+// Description: notifies request that the endpoint descriptor is complete
+
+    virtual VOID CompletionCallback(struct _OHCI_ENDPOINT_DESCRIPTOR * OutDescriptor) =
0;
 };
 
 
@@ -495,6 +502,14 @@
 // Description: creates an usb request
 
     virtual NTSTATUS CreateUSBRequest(IUSBRequest **OutRequest) = 0;
+
+//-----------------------------------------------------------------------------------------
+//
+// TransferDescriptorCompletionCallback
+//
+// Description: notifies the queue that a transfer was completed
+
+    virtual VOID TransferDescriptorCompletionCallback(ULONG
TransferDescriptorLogicalAddress) = 0;
 
 };
 

Modified: branches/usb-bringup/drivers/usb/usbohci/usb_device.cpp
URL:
http://svn.reactos.org/svn/reactos/branches/usb-bringup/drivers/usb/usbohci…
==============================================================================
--- branches/usb-bringup/drivers/usb/usbohci/usb_device.cpp [iso-8859-1] (original)
+++ branches/usb-bringup/drivers/usb/usbohci/usb_device.cpp [iso-8859-1] Tue May 24
17:57:00 2011
@@ -707,6 +707,11 @@
     }
 
     //
+    // zero buffer
+    //
+    RtlZeroMemory(Buffer, PAGE_SIZE);
+
+    //
     // build setup packet
     //
     CtrlSetup.bmRequestType._BM.Recipient = BMREQUEST_TO_DEVICE;

Modified: branches/usb-bringup/drivers/usb/usbohci/usb_queue.cpp
URL:
http://svn.reactos.org/svn/reactos/branches/usb-bringup/drivers/usb/usbohci…
==============================================================================
--- branches/usb-bringup/drivers/usb/usbohci/usb_queue.cpp [iso-8859-1] (original)
+++ branches/usb-bringup/drivers/usb/usbohci/usb_queue.cpp [iso-8859-1] Tue May 24
17:57:00 2011
@@ -33,11 +33,17 @@
         return m_Ref;
     }
 
+    // com
     virtual NTSTATUS Initialize(IN PUSBHARDWAREDEVICE Hardware, PDMA_ADAPTER
AdapterObject, IN PDMAMEMORYMANAGER MemManager, IN OPTIONAL PKSPIN_LOCK Lock);
     virtual ULONG GetPendingRequestCount();
     virtual NTSTATUS AddUSBRequest(IUSBRequest * Request);
     virtual NTSTATUS CancelRequests();
     virtual NTSTATUS CreateUSBRequest(IUSBRequest **OutRequest);
+    virtual VOID TransferDescriptorCompletionCallback(ULONG
TransferDescriptorLogicalAddress);
+
+    // local functions
+    BOOLEAN IsTransferDescriptorInEndpoint(IN POHCI_ENDPOINT_DESCRIPTOR
EndpointDescriptor, IN ULONG TransferDescriptorLogicalAddress);
+    NTSTATUS FindTransferDescriptorInEndpoint(IN POHCI_ENDPOINT_DESCRIPTOR
EndpointDescriptor, IN ULONG TransferDescriptorLogicalAddress, OUT
POHCI_ENDPOINT_DESCRIPTOR *OutEndpointDescriptor, OUT POHCI_ENDPOINT_DESCRIPTOR
*OutPreviousEndpointDescriptor);
 
     // constructor / destructor
     CUSBQueue(IUnknown *OuterUnknown){}
@@ -218,13 +224,14 @@
     // set descriptor active
     //
     Descriptor->Flags &= ~OHCI_ENDPOINT_SKIP;
+    //HeadDescriptor->Flags &= ~OHCI_ENDPOINT_SKIP;
 
     //
     // notify hardware of our request
     //
     m_Hardware->HeadEndpointDescriptorModified(Type);
 
-    DPRINT1("Request added to queue\n");
+    DPRINT1("Request %x %x added to queue\n", Descriptor,
Descriptor->PhysicalAddress);
 
 
     return STATUS_SUCCESS;
@@ -255,6 +262,142 @@
     return Status;
 }
 
+NTSTATUS
+CUSBQueue::FindTransferDescriptorInEndpoint(
+    IN POHCI_ENDPOINT_DESCRIPTOR EndpointDescriptor,
+    IN ULONG TransferDescriptorLogicalAddress,
+    OUT POHCI_ENDPOINT_DESCRIPTOR *OutEndpointDescriptor,
+    OUT POHCI_ENDPOINT_DESCRIPTOR *OutPreviousEndpointDescriptor)
+{
+    POHCI_ENDPOINT_DESCRIPTOR LastDescriptor = EndpointDescriptor;
+
+
+    //
+    // skip first endpoint head
+    //
+    EndpointDescriptor =
(POHCI_ENDPOINT_DESCRIPTOR)EndpointDescriptor->NextDescriptor;
+
+    while(EndpointDescriptor)
+    {
+        //
+        // check if the transfer descriptor is inside the list
+        //
+        if (IsTransferDescriptorInEndpoint(EndpointDescriptor,
TransferDescriptorLogicalAddress))
+        {
+            //
+            // found endpoint
+            //
+            *OutEndpointDescriptor = EndpointDescriptor;
+            *OutPreviousEndpointDescriptor = LastDescriptor;
+
+            //
+            // done
+            //
+            return STATUS_SUCCESS;
+        }
+
+        //
+        // store last endpoint
+        //
+        LastDescriptor = EndpointDescriptor;
+
+        //
+        // move to next
+        //
+        EndpointDescriptor =
(POHCI_ENDPOINT_DESCRIPTOR)EndpointDescriptor->NextDescriptor;
+    }
+
+    //
+    // failed to endpoint
+    //
+    return STATUS_NOT_FOUND;
+}
+
+
+BOOLEAN
+CUSBQueue::IsTransferDescriptorInEndpoint(
+    IN POHCI_ENDPOINT_DESCRIPTOR EndpointDescriptor,
+    IN ULONG TransferDescriptorLogicalAddress)
+{
+    POHCI_GENERAL_TD Descriptor;
+
+    //
+    // get first general transfer descriptor
+    //
+    Descriptor = (POHCI_GENERAL_TD)EndpointDescriptor->HeadLogicalDescriptor;
+
+    //
+    // sanity check
+    //
+    ASSERT(Descriptor);
+
+    do
+    {
+        if (Descriptor->PhysicalAddress.LowPart == TransferDescriptorLogicalAddress)
+        {
+            //
+            // found descriptor
+            //
+            return TRUE;
+        }
+
+        //
+        // move to next
+        //
+        Descriptor = (POHCI_GENERAL_TD)Descriptor->NextLogicalDescriptor;
+    }while(Descriptor);
+
+
+    //
+    // no descriptor found
+    //
+    return FALSE;
+}
+
+
+VOID
+CUSBQueue::TransferDescriptorCompletionCallback(
+    ULONG TransferDescriptorLogicalAddress)
+{
+    POHCI_ENDPOINT_DESCRIPTOR EndpointDescriptor, PreviousEndpointDescriptor;
+    NTSTATUS Status;
+    PUSBREQUEST Request;
+
+    //
+    // find transfer descriptor in control list
+    //
+    Status = FindTransferDescriptorInEndpoint(m_ControlHeadEndpointDescriptor,
TransferDescriptorLogicalAddress, &EndpointDescriptor,
&PreviousEndpointDescriptor);
+    if (NT_SUCCESS(Status))
+    {
+        //
+        // FIXME: make sure this is ok
+        // unlink descriptor
+        //
+        PreviousEndpointDescriptor->NextDescriptor =
EndpointDescriptor->NextDescriptor;
+        PreviousEndpointDescriptor->NextPhysicalEndpoint =
EndpointDescriptor->NextPhysicalEndpoint;
+
+        //
+        // get corresponding request
+        //
+        Request = PUSBREQUEST(EndpointDescriptor->Request);
+
+        //
+        // notify of completion
+        //
+        Request->CompletionCallback(EndpointDescriptor);
+
+        //
+        // FIXME: check if complete
+        //
+
+        //
+        // release request
+        //
+        Request->Release();
+    }
+
+}
+
 
 NTSTATUS
 CreateUSBQueue(

Modified: branches/usb-bringup/drivers/usb/usbohci/usb_request.cpp
URL:
http://svn.reactos.org/svn/reactos/branches/usb-bringup/drivers/usb/usbohci…
==============================================================================
--- branches/usb-bringup/drivers/usb/usbohci/usb_request.cpp [iso-8859-1] (original)
+++ branches/usb-bringup/drivers/usb/usbohci/usb_request.cpp [iso-8859-1] Tue May 24
17:57:00 2011
@@ -44,7 +44,7 @@
     virtual VOID GetResultStatus(OUT OPTIONAL NTSTATUS *NtStatusCode, OUT OPTIONAL PULONG
UrbStatusCode);
     virtual BOOLEAN IsRequestInitialized();
     virtual BOOLEAN IsQueueHeadComplete(struct _QUEUE_HEAD * QueueHead);
-
+    virtual VOID CompletionCallback(struct _OHCI_ENDPOINT_DESCRIPTOR * OutDescriptor);
 
     // local functions
     ULONG InternalGetTransferType();
@@ -635,7 +635,7 @@
 CUSBRequest::BuildControlTransferDescriptor(
     POHCI_ENDPOINT_DESCRIPTOR * OutEndpointDescriptor)
 {
-    POHCI_GENERAL_TD SetupDescriptor, StatusDescriptor, DataDescriptor = NULL;
+    POHCI_GENERAL_TD SetupDescriptor, StatusDescriptor, DataDescriptor = NULL,
LastDescriptor;
     POHCI_ENDPOINT_DESCRIPTOR EndpointDescriptor;
     NTSTATUS Status;
 
@@ -680,6 +680,22 @@
         return Status;
     }
 
+    //
+    // finally create the last descriptor
+    //
+    Status = CreateGeneralTransferDescriptor(&LastDescriptor, 0);
+    if (!NT_SUCCESS(Status))
+    {
+        //
+        // failed to create status descriptor
+        //
+        FreeDescriptor(SetupDescriptor);
+        FreeDescriptor(StatusDescriptor);
+        m_DmaManager->Release(EndpointDescriptor, sizeof(OHCI_ENDPOINT_DESCRIPTOR));
+        return Status;
+    }
+
+
     if (m_TransferBufferLength)
     {
         //
@@ -690,7 +706,7 @@
         //
         // now create the data descriptor
         //
-        Status = CreateGeneralTransferDescriptor(&DataDescriptor, 0);
+        Status = CreateGeneralTransferDescriptor(&DataDescriptor,
m_TransferBufferLength);
         if (!NT_SUCCESS(Status))
         {
             //
@@ -699,6 +715,7 @@
             m_DmaManager->Release(EndpointDescriptor,
sizeof(OHCI_ENDPOINT_DESCRIPTOR));
             FreeDescriptor(SetupDescriptor);
             FreeDescriptor(StatusDescriptor);
+            FreeDescriptor(LastDescriptor);
             return Status;
         }
 
@@ -762,11 +779,20 @@
          // link setup descriptor to data descriptor
          //
          SetupDescriptor->NextPhysicalDescriptor =
DataDescriptor->PhysicalAddress.LowPart;
-
-         //
-         // FIXME: should link to last data descriptor to status descriptor
+         SetupDescriptor->NextLogicalDescriptor = DataDescriptor;
+
+         //
+         // link data descriptor to status descriptor
+         // FIXME: check if there are more data descriptors
          //
          DataDescriptor->NextPhysicalDescriptor =
StatusDescriptor->PhysicalAddress.LowPart;
+         DataDescriptor->NextLogicalDescriptor = StatusDescriptor;
+
+         //
+         // link status descriptor to last descriptor
+         //
+         StatusDescriptor->NextPhysicalDescriptor =
LastDescriptor->PhysicalAddress.LowPart;
+         StatusDescriptor->NextLogicalDescriptor = LastDescriptor;
     }
     else
     {
@@ -774,14 +800,21 @@
          // link setup descriptor to status descriptor
          //
          SetupDescriptor->NextPhysicalDescriptor =
StatusDescriptor->PhysicalAddress.LowPart;
+         SetupDescriptor->NextLogicalDescriptor = StatusDescriptor;
+
+         //
+         // link status descriptor to last descriptor
+         //
+         StatusDescriptor->NextPhysicalDescriptor =
LastDescriptor->PhysicalAddress.LowPart;
+         StatusDescriptor->NextLogicalDescriptor = LastDescriptor;
     }
 
     //
     // now link descriptor to endpoint
     //
     EndpointDescriptor->HeadPhysicalDescriptor =
SetupDescriptor->PhysicalAddress.LowPart;
-    EndpointDescriptor->TailPhysicalDescriptor =
SetupDescriptor->PhysicalAddress.LowPart;
-    DPRINT1("CUSBRequest::BuildControlTransferDescriptor done\n");
+    EndpointDescriptor->TailPhysicalDescriptor =
LastDescriptor->PhysicalAddress.LowPart;
+    EndpointDescriptor->HeadLogicalDescriptor = SetupDescriptor;
 
     //
     // store result
@@ -887,6 +920,32 @@
 }
 
 
+VOID
+CUSBRequest::CompletionCallback(
+    struct _OHCI_ENDPOINT_DESCRIPTOR * OutDescriptor)
+{
+    DPRINT1("CUSBRequest::CompletionCallback\n");
+
+    //
+    // set status code
+    //
+    m_NtStatusCode = STATUS_SUCCESS;
+    m_UrbStatusCode = USBD_STATUS_SUCCESS;
+
+    ASSERT(!m_Irp);
+
+    //
+    // FIXME: cleanup descriptors
+    //
+
+    //
+    // signal completion event
+    //
+    PC_ASSERT(m_CompletionEvent);
+    KeSetEvent(m_CompletionEvent, 0, FALSE);
+}
+
+

//-----------------------------------------------------------------------------------------
 BOOLEAN
 CUSBRequest::IsRequestInitialized()