[janderwald] 51998: [USBOHCI] - Fix OHCI_ISO_TD structure. Fixes host system crashes in Windows XP - Fix multiple bugs in isochronous transfer implementation - Still not yet working, as the interru...

Author: janderwald
Date: Sun May 29 19:54:55 2011
New Revision: 51998

URL: http://svn.reactos.org/svn/reactos?rev=51998&view=rev
Log:
[USBOHCI]
- Fix OHCI_ISO_TD structure. Fixes host system crashes in Windows XP
- Fix multiple bugs in isochronous transfer implementation
- Still not yet working, as the interrupt completion is not fired yet

Modified:
    branches/usb-bringup/drivers/usb/usbohci/hardware.h
    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.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] Sun May 29 19:54:55
2011
@@ -310,12 +310,21 @@
     ULONG BufferPhysical;                       // Physical page number of byte 0
     ULONG NextPhysicalDescriptor;               // Next isochronous transfer descriptor
     ULONG LastPhysicalByteAddress;              // Physical buffer end
-    ULONG Offset[OHCI_ITD_NOFFSET];             // Buffer offsets
+    USHORT Offset[OHCI_ITD_NOFFSET];             // Buffer offsets
 
     // Software part
     PHYSICAL_ADDRESS PhysicalAddress;             // Physical address of this descriptor
     struct _OHCI_ISO_TD_ * NextLogicalDescriptor; // Logical pointer next descriptor
 }OHCI_ISO_TD, *POHCI_ISO_TD;
+
+C_ASSERT(FIELD_OFFSET(OHCI_ISO_TD, Flags) == 0);
+C_ASSERT(FIELD_OFFSET(OHCI_ISO_TD, BufferPhysical) == 4);
+C_ASSERT(FIELD_OFFSET(OHCI_ISO_TD, NextPhysicalDescriptor) == 8);
+C_ASSERT(FIELD_OFFSET(OHCI_ISO_TD, LastPhysicalByteAddress) == 12);
+C_ASSERT(FIELD_OFFSET(OHCI_ISO_TD, Offset) == 16);
+C_ASSERT(FIELD_OFFSET(OHCI_ISO_TD, PhysicalAddress) == 32);
+C_ASSERT(FIELD_OFFSET(OHCI_ISO_TD, NextLogicalDescriptor) == 40);
+C_ASSERT(sizeof(OHCI_ISO_TD) == 48);
 
 #define OHCI_ITD_GET_STARTING_FRAME(x)          ((x) & 0x0000ffff)
 #define OHCI_ITD_SET_STARTING_FRAME(x)          ((x) & 0xffff)

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] Sun May 29
19:54:55 2011
@@ -43,8 +43,10 @@
 
     // local functions
     BOOLEAN IsTransferDescriptorInEndpoint(IN POHCI_ENDPOINT_DESCRIPTOR
EndpointDescriptor, IN ULONG TransferDescriptorLogicalAddress);
+    BOOLEAN IsTransferDescriptorInIsoEndpoint(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);
     NTSTATUS FindTransferDescriptorInInterruptHeadEndpoints(IN ULONG
TransferDescriptorLogicalAddress, OUT POHCI_ENDPOINT_DESCRIPTOR *OutEndpointDescriptor,
OUT POHCI_ENDPOINT_DESCRIPTOR *OutPreviousEndpointDescriptor);
+    NTSTATUS FindTransferDescriptorInIsochronousHeadEndpoints(IN ULONG
TransferDescriptorLogicalAddress, OUT POHCI_ENDPOINT_DESCRIPTOR *OutEndpointDescriptor,
OUT POHCI_ENDPOINT_DESCRIPTOR *OutPreviousEndpointDescriptor);
 
     VOID CleanupEndpointDescriptor(POHCI_ENDPOINT_DESCRIPTOR EndpointDescriptor,
POHCI_ENDPOINT_DESCRIPTOR PreviousEndpointDescriptor);
     POHCI_ENDPOINT_DESCRIPTOR FindInterruptEndpointDescriptor(UCHAR InterruptInterval);
@@ -102,6 +104,8 @@
     Hardware->GetControlHeadEndpointDescriptor(&m_ControlHeadEndpointDescriptor);
 
     //
+    // get isochronous endpoint
+    //
     Hardware->GetIsochronousHeadEndpointDescriptor(&m_IsoHeadEndpointDescriptor);
 
     //
@@ -170,6 +174,7 @@
     POHCI_ENDPOINT_DESCRIPTOR Descriptor;
     POHCI_ISO_TD CurrentDescriptor;
     ULONG FrameNumber;
+    USHORT Frame;
 
     DPRINT("CUSBQueue::AddUSBRequest\n");
 
@@ -244,9 +249,9 @@
         m_Hardware->GetCurrentFrameNumber(&FrameNumber);
 
         //
-        // increment frame number
-        //
-        FrameNumber++;
+        // FIXME: increment frame number
+        //
+        FrameNumber += 300; 
 
         //
         // apply frame number to iso transfer descriptors
@@ -254,18 +259,19 @@
         CurrentDescriptor = (POHCI_ISO_TD)Descriptor->HeadLogicalDescriptor;
 
         DPRINT1("ISO: NextFrameNumber %x\n", FrameNumber);
+        Frame = (FrameNumber & 0xFFFF);
 
         while(CurrentDescriptor)
         {
             //
             // set current frame number
             //
-            CurrentDescriptor->Flags |= OHCI_ITD_SET_STARTING_FRAME(FrameNumber);
+            CurrentDescriptor->Flags |= OHCI_ITD_SET_STARTING_FRAME(Frame);
 
             //
             // move to next frame number
             //
-            FrameNumber++;
+            Frame += OHCI_ITD_GET_FRAME_COUNT(CurrentDescriptor->Flags);
 
             //
             // move to next descriptor
@@ -275,14 +281,14 @@
     }
 
     //
+    // set descriptor active
+    //
+    Descriptor->Flags &= ~OHCI_ENDPOINT_SKIP;
+
+    //
     // insert endpoint at end
     //
     LinkEndpoint(HeadDescriptor, Descriptor);
-
-    //
-    // set descriptor active
-    //
-    Descriptor->Flags &= ~OHCI_ENDPOINT_SKIP;
 
     if (Type == USB_ENDPOINT_TYPE_CONTROL || Type == USB_ENDPOINT_TYPE_BULK)
     {
@@ -402,6 +408,97 @@
     return STATUS_NOT_FOUND;
 }
 
+NTSTATUS
+CUSBQueue::FindTransferDescriptorInIsochronousHeadEndpoints(
+    IN ULONG TransferDescriptorLogicalAddress, 
+    OUT POHCI_ENDPOINT_DESCRIPTOR *OutEndpointDescriptor, 
+    OUT POHCI_ENDPOINT_DESCRIPTOR *OutPreviousEndpointDescriptor)
+{
+    POHCI_ENDPOINT_DESCRIPTOR EndpointDescriptor;
+    POHCI_ENDPOINT_DESCRIPTOR LastDescriptor = m_IsoHeadEndpointDescriptor;
+
+
+    //
+    // skip first endpoint head
+    //
+    EndpointDescriptor =
(POHCI_ENDPOINT_DESCRIPTOR)m_IsoHeadEndpointDescriptor->NextDescriptor;
+
+    while(EndpointDescriptor)
+    {
+        //
+        // check if the transfer descriptor is inside the list
+        //
+        if (IsTransferDescriptorInIsoEndpoint(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::IsTransferDescriptorInIsoEndpoint(
+    IN POHCI_ENDPOINT_DESCRIPTOR EndpointDescriptor,
+    IN ULONG TransferDescriptorLogicalAddress)
+{
+    POHCI_ISO_TD Descriptor;
+
+    //
+    // get first general transfer descriptor
+    //
+    Descriptor = (POHCI_ISO_TD)EndpointDescriptor->HeadLogicalDescriptor;
+
+    //
+    // sanity check
+    //
+    ASSERT(Descriptor);
+
+    do
+    {
+        if (Descriptor->PhysicalAddress.LowPart == TransferDescriptorLogicalAddress)
+        {
+            //
+            // found descriptor
+            //
+            return TRUE;
+        }
+
+        //
+        // move to next
+        //
+        Descriptor = (POHCI_ISO_TD)Descriptor->NextLogicalDescriptor;
+    }while(Descriptor);
+
+    //
+    // no descriptor found
+    //
+    return FALSE;
+}
+
+
 BOOLEAN
 CUSBQueue::IsTransferDescriptorInEndpoint(
     IN POHCI_ENDPOINT_DESCRIPTOR EndpointDescriptor,
@@ -476,6 +573,8 @@
     //
     //ASSERT(Request->IsRequestComplete());
 
+    Request->FreeEndpointDescriptor(EndpointDescriptor);
+
     //
     // release request
     //
@@ -511,7 +610,7 @@
     POHCI_ENDPOINT_DESCRIPTOR EndpointDescriptor, PreviousEndpointDescriptor;
     NTSTATUS Status;
 
-    DPRINT("CUSBQueue::TransferDescriptorCompletionCallback transfer descriptor
%x\n", TransferDescriptorLogicalAddress);
+    DPRINT1("CUSBQueue::TransferDescriptorCompletionCallback transfer descriptor
%x\n", TransferDescriptorLogicalAddress);
 
     //
     // find transfer descriptor in control list
@@ -564,13 +663,30 @@
         return;
     }
 
+    //
+    // last try: find the descriptor in isochronous list
+    //
+    Status =
FindTransferDescriptorInIsochronousHeadEndpoints(TransferDescriptorLogicalAddress,
&EndpointDescriptor, &PreviousEndpointDescriptor);
+    if (NT_SUCCESS(Status))
+    {
+        //
+        // cleanup endpoint
+        //
+        DPRINT1("ISO endpoint complete\n");
+ASSERT(FALSE);
+        CleanupEndpointDescriptor(EndpointDescriptor, PreviousEndpointDescriptor);
+
+        //
+        // done
+        //
+        return;
+    }
 
     //
     // hardware reported dead endpoint completed
     //
-    DPRINT("CUSBQueue::TransferDescriptorCompletionCallback invalid transfer
descriptor %x\n", TransferDescriptorLogicalAddress);
+    DPRINT1("CUSBQueue::TransferDescriptorCompletionCallback invalid transfer
descriptor %x\n", TransferDescriptorLogicalAddress);
     ASSERT(FALSE);
-
 }
 
 POHCI_ENDPOINT_DESCRIPTOR

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] Sun May 29
19:54:55 2011
@@ -299,7 +299,7 @@
                     m_TransferBufferMDL =
Urb->UrbIsochronousTransfer.TransferBufferMDL;
                 }
             }
-
+ 
             //
             // save buffer length
             //
@@ -656,13 +656,12 @@
 {
     POHCI_ISO_TD FirstDescriptor, PreviousDescriptor = NULL, CurrentDescriptor;
     POHCI_ENDPOINT_DESCRIPTOR EndpointDescriptor;
-    ULONG Index = 0, SubIndex, NumberOfPackets, PageOffset;
+    ULONG Index = 0, SubIndex, NumberOfPackets, PageOffset, Page;
     NTSTATUS Status;
     PVOID Buffer;
     PIO_STACK_LOCATION IoStack;
     PURB Urb;
 
-    DPRINT1("cp\n");
     //
     // get current irp stack location
     //
@@ -690,23 +689,27 @@
         //
         // failed to create setup descriptor
         //
+        ASSERT(FALSE);
         return Status;
     }
-    DPRINT1("cp\n");
+
     //
     // get buffer
     //
     Buffer = MmGetSystemAddressForMdlSafe(m_TransferBufferMDL, NormalPagePriority);
     ASSERT(Buffer);
 
-    DPRINT1("cp\n");
+    //
+    // FIXME: support requests which spans serveral pages
+    //
+    ASSERT(ADDRESS_AND_SIZE_TO_SPAN_PAGES(MmGetMdlVirtualAddress(m_TransferBufferMDL),
MmGetMdlByteCount(m_TransferBufferMDL)) <= 2);
+
     while(Index < Urb->UrbIsochronousTransfer.NumberOfPackets)
     {
         //
         // get number of packets remaining
         //
         NumberOfPackets = min(Urb->UrbIsochronousTransfer.NumberOfPackets - Index,
OHCI_ITD_NOFFSET);
-    DPRINT1("cp Number Packets %lu\n", NumberOfPackets);
         //
         // allocate iso descriptor
         //
@@ -720,23 +723,29 @@
             ASSERT(FALSE);
             return Status;
         }
-    DPRINT1("cp\n");
+
+        //
+        // get physical page
+        //
+        Page = MmGetPhysicalAddress(Buffer).LowPart;
+
+        //
+        // get page offset
+        //
+        PageOffset = MmGetMdlByteOffset(m_TransferBufferMDL);
+
         //
         // initialize descriptor
         //
-        CurrentDescriptor->BufferPhysical = (MmGetPhysicalAddress(Buffer).LowPart
& ~ (PAGE_SIZE - 1));
-
-        //
-        // get page offset
-        //
-        PageOffset = BYTE_OFFSET(MmGetPhysicalAddress(Buffer).LowPart);
-    DPRINT1("cp\n");
+        CurrentDescriptor->BufferPhysical = Page - PageOffset;
+
         for(SubIndex = 0; SubIndex < NumberOfPackets; SubIndex++)
         {
             //
             // store buffer offset
             //
-            CurrentDescriptor->Offset[SubIndex] =
Urb->UrbIsochronousTransfer.IsoPacket[Index].Offset + PageOffset;
+            CurrentDescriptor->Offset[SubIndex] =
Urb->UrbIsochronousTransfer.IsoPacket[Index+SubIndex].Offset + PageOffset;
+            DPRINT1("Index %lu PacketOffset %lu FinalOffset %lu\n",
SubIndex+Index, Urb->UrbIsochronousTransfer.IsoPacket[Index+SubIndex].Offset,
CurrentDescriptor->Offset[SubIndex]);
         }
 
         //
@@ -752,19 +761,14 @@
             //
             // end of transfer
             //
-            CurrentDescriptor->LastPhysicalByteAddress =
CurrentDescriptor->BufferPhysical + m_TransferBufferLength - 1;
+            CurrentDescriptor->LastPhysicalByteAddress =
CurrentDescriptor->BufferPhysical + PageOffset + m_TransferBufferLength - 1;
         }
         else
         {
             //
             // use start address of next packet - 1
             //
-            CurrentDescriptor->LastPhysicalByteAddress =
CurrentDescriptor->BufferPhysical + PageOffset +
Urb->UrbIsochronousTransfer.IsoPacket[Index + 1].Offset - 1;
-
-            //
-            // move buffer to next address
-            //
-            Buffer = (PVOID)((ULONG_PTR)Buffer +
Urb->UrbIsochronousTransfer.IsoPacket[Index + 1].Offset);
+            CurrentDescriptor->LastPhysicalByteAddress =
CurrentDescriptor->BufferPhysical + PageOffset +
Urb->UrbIsochronousTransfer.IsoPacket[Index].Offset - 1;
         }
 
         //
@@ -790,8 +794,13 @@
         // store as previous descriptor
         //
         PreviousDescriptor = CurrentDescriptor;
-    }
-    DPRINT1("cp\n");
+        DPRINT1("Current Descriptor %p Logical %lx StartAddress %x EndAddress
%x\n", CurrentDescriptor, CurrentDescriptor->PhysicalAddress.LowPart,
CurrentDescriptor->BufferPhysical, CurrentDescriptor->LastPhysicalByteAddress);
+
+    //
+    // fire interrupt as soon transfer is finished
+    //
+    CurrentDescriptor->Flags |=
OHCI_TD_SET_DELAY_INTERRUPT(OHCI_TD_INTERRUPT_IMMEDIATE);
+    }
 
     //
     // clear interrupt mask for last transfer descriptor
@@ -814,7 +823,7 @@
     EndpointDescriptor->HeadPhysicalDescriptor =
FirstDescriptor->PhysicalAddress.LowPart;
     EndpointDescriptor->TailPhysicalDescriptor =
CurrentDescriptor->PhysicalAddress.LowPart;
     EndpointDescriptor->HeadLogicalDescriptor = FirstDescriptor;
-    DPRINT1("cp\n");
+
     //
     // store result
     //
@@ -1451,48 +1460,95 @@
     struct _OHCI_ENDPOINT_DESCRIPTOR * OutDescriptor)
 {
     POHCI_GENERAL_TD TransferDescriptor, NextTransferDescriptor;
+    POHCI_ISO_TD IsoTransferDescriptor, IsoNextTransferDescriptor;
+    ULONG Index, PacketCount;
 
     DPRINT("CUSBRequest::FreeEndpointDescriptor EndpointDescriptor %p Logical
%x\n", OutDescriptor, OutDescriptor->PhysicalAddress.LowPart);
 
-    //
-    // get first general transfer descriptor
-    //
-    TransferDescriptor = (POHCI_GENERAL_TD)OutDescriptor->HeadLogicalDescriptor;
-
-    //
-    // release endpoint descriptor
-    //
-    m_DmaManager->Release(OutDescriptor, sizeof(OHCI_ENDPOINT_DESCRIPTOR));
-
-    while(TransferDescriptor)
-    {
-        //
-        // get next
-        //
-        NextTransferDescriptor =
(POHCI_GENERAL_TD)TransferDescriptor->NextLogicalDescriptor;
-
-        //
-        // is there a buffer associated
-        //
-        if (TransferDescriptor->BufferSize)
-        {
-            //
-            // release buffer
-            //
-            m_DmaManager->Release(TransferDescriptor->BufferLogical,
TransferDescriptor->BufferSize);
-        }
-
-        DPRINT("CUSBRequest::FreeEndpointDescriptor Descriptor %p Logical %x Buffer
Physical %x EndAddress %x\n", TransferDescriptor,
TransferDescriptor->PhysicalAddress.LowPart, TransferDescriptor->BufferPhysical,
TransferDescriptor->LastPhysicalByteAddress);
-
-        //
-        // release descriptor
-        //
-        m_DmaManager->Release(TransferDescriptor, sizeof(OHCI_GENERAL_TD));
-
-        //
-        // move to next
-        //
-        TransferDescriptor = NextTransferDescriptor;
+    if (OutDescriptor->Flags & OHCI_ENDPOINT_ISOCHRONOUS_FORMAT)
+    {
+        //
+        // get first iso transfer descriptor
+        //
+        IsoTransferDescriptor = (POHCI_ISO_TD)OutDescriptor->HeadLogicalDescriptor;
+
+        //
+        // release endpoint descriptor
+        //
+        m_DmaManager->Release(OutDescriptor, sizeof(OHCI_ENDPOINT_DESCRIPTOR));
+
+        while(IsoTransferDescriptor)
+        {
+            //
+            // get next
+            //
+            IsoNextTransferDescriptor = IsoTransferDescriptor->NextLogicalDescriptor;
+
+            //
+            // get packet count
+            //
+            PacketCount = OHCI_ITD_GET_FRAME_COUNT(IsoTransferDescriptor->Flags);
+
+            DPRINT1("CUSBRequest::FreeEndpointDescriptor Descriptor %p Logical %x
Buffer Physical %x EndAddress %x PacketCount %lu\n", IsoTransferDescriptor,
IsoTransferDescriptor->PhysicalAddress.LowPart,
IsoTransferDescriptor->BufferPhysical,
IsoTransferDescriptor->LastPhysicalByteAddress, PacketCount);
+
+            for(Index = 0; Index < PacketCount; Index++)
+            {
+                DPRINT1("PSW Index %lu Value %x\n", Index,
IsoTransferDescriptor->Offset[Index]);
+            }
+
+            //
+            // release descriptor
+            //
+            m_DmaManager->Release(IsoTransferDescriptor, sizeof(OHCI_ISO_TD));
+
+            //
+            // move to next
+            //
+            IsoTransferDescriptor = IsoNextTransferDescriptor;
+        }
+    }
+    else
+    {
+        //
+        // get first general transfer descriptor
+        //
+        TransferDescriptor = (POHCI_GENERAL_TD)OutDescriptor->HeadLogicalDescriptor;
+
+        //
+        // release endpoint descriptor
+        //
+        m_DmaManager->Release(OutDescriptor, sizeof(OHCI_ENDPOINT_DESCRIPTOR));
+
+        while(TransferDescriptor)
+        {
+            //
+            // get next
+            //
+            NextTransferDescriptor =
(POHCI_GENERAL_TD)TransferDescriptor->NextLogicalDescriptor;
+
+            //
+            // is there a buffer associated
+            //
+            if (TransferDescriptor->BufferSize)
+            {
+                //
+                // release buffer
+                //
+                m_DmaManager->Release(TransferDescriptor->BufferLogical,
TransferDescriptor->BufferSize);
+            }
+
+            DPRINT("CUSBRequest::FreeEndpointDescriptor Descriptor %p Logical %x
Buffer Physical %x EndAddress %x\n", TransferDescriptor,
TransferDescriptor->PhysicalAddress.LowPart, TransferDescriptor->BufferPhysical,
TransferDescriptor->LastPhysicalByteAddress);
+
+            //
+            // release descriptor
+            //
+            m_DmaManager->Release(TransferDescriptor, sizeof(OHCI_GENERAL_TD));
+
+            //
+            // move to next
+            //
+            TransferDescriptor = NextTransferDescriptor;
+        }
     }
 
 }