[ros-diffs] [mjmartin] 50302: [usb/usbehci]: - Fix bug in InitializeUsbDevice that caused endpoints to be duplicated in internal config structure. - Fix implementation of UsbRemoveDevice. - Implement SetDevice...

Author: mjmartin
Date: Thu Jan  6 17:46:59 2011
New Revision: 50302

URL: http://svn.reactos.org/svn/reactos?rev=50302&view=rev
Log:
[usb/usbehci]:
- Fix bug in InitializeUsbDevice that caused endpoints to be duplicated in internal config
structure.
- Fix implementation of UsbRemoveDevice.
- Implement SetDeviceHandleData and RestoreUsbDevice.
- SubmitControlTransfer: Not all control request need data, in which case dont create a
MDL and a PID_CODE_IN_TOKEN TD Descriptor.
- Fix bug in ReleaseMemory that caused memory not to be marked as free.
- Fix incorrect memory header size that resulted in memory corruption.

Modified:
    trunk/reactos/drivers/usb/usbehci/physmem.c
    trunk/reactos/drivers/usb/usbehci/physmem.h
    trunk/reactos/drivers/usb/usbehci/transfer.c
    trunk/reactos/drivers/usb/usbehci/usbehci.h
    trunk/reactos/drivers/usb/usbehci/usbiffn.c

Modified: trunk/reactos/drivers/usb/usbehci/physmem.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/usb/usbehci/physme…
==============================================================================
--- trunk/reactos/drivers/usb/usbehci/physmem.c [iso-8859-1] (original)
+++ trunk/reactos/drivers/usb/usbehci/physmem.c [iso-8859-1] Thu Jan  6 17:46:59 2011
@@ -31,7 +31,6 @@
 
     Size = ((Size + SMALL_ALLOCATION_SIZE - 1) / SMALL_ALLOCATION_SIZE) *
SMALL_ALLOCATION_SIZE;
     BlocksNeeded = Size / SMALL_ALLOCATION_SIZE;
-
     do
     {
         if (MemoryPage->IsFull)
@@ -52,6 +51,11 @@
                 freeCount = 0;
             }
 
+            if ((i-freeCount+1 + BlocksNeeded) > sizeof(MemoryPage->Entry))
+            {
+                freeCount = 0;
+                continue;
+            }
             if (freeCount == BlocksNeeded)
             {
                 for (j = 0; j < freeCount; j++)
@@ -80,13 +84,13 @@
 ReleaseMemory(ULONG Address)
 {
     PMEM_HEADER MemoryPage;
-    ULONG Index, i;
+    ULONG Index, i, BlockSize;
 
     MemoryPage = (PMEM_HEADER)(Address & ~(PAGE_SIZE - 1));
 
     Index = (Address - ((ULONG)MemoryPage + sizeof(MEM_HEADER))) /
SMALL_ALLOCATION_SIZE;
-
-    for (i = 0; i < MemoryPage->Entry[Index].Blocks; i++)
+    BlockSize = MemoryPage->Entry[Index].Blocks;
+    for (i = 0; i < BlockSize; i++)
     {
         MemoryPage->Entry[Index + i].InUse = 0;
         MemoryPage->Entry[Index + i].Blocks = 0;

Modified: trunk/reactos/drivers/usb/usbehci/physmem.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/usb/usbehci/physme…
==============================================================================
--- trunk/reactos/drivers/usb/usbehci/physmem.h [iso-8859-1] (original)
+++ trunk/reactos/drivers/usb/usbehci/physmem.h [iso-8859-1] Thu Jan  6 17:46:59 2011
@@ -11,7 +11,8 @@
 typedef struct _MEM_HEADER
 {
     UCHAR IsFull;
-    MEM_ENTRY Entry[127];    
+    MEM_ENTRY Entry[124];
+    UCHAR Reserved[3];
 } MEM_HEADER, *PMEM_HEADER;
 
 VOID

Modified: trunk/reactos/drivers/usb/usbehci/transfer.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/usb/usbehci/transf…
==============================================================================
--- trunk/reactos/drivers/usb/usbehci/transfer.c [iso-8859-1] (original)
+++ trunk/reactos/drivers/usb/usbehci/transfer.c [iso-8859-1] Thu Jan  6 17:46:59 2011
@@ -77,6 +77,9 @@
     /* SET CONFIG */
         case URB_FUNCTION_SELECT_CONFIGURATION:
             CtrlSetup->bRequest = USB_REQUEST_SET_CONFIGURATION;
+            CtrlSetup->wValue.W =
Urb->UrbSelectConfiguration.ConfigurationDescriptor->bConfigurationValue;
+            CtrlSetup->wIndex.W = 0;
+            CtrlSetup->wLength = 0;            
             CtrlSetup->bmRequestType.B = 0x00;
             break;
 
@@ -111,7 +114,11 @@
 
     /* SET INTERFACE*/
         case URB_FUNCTION_SELECT_INTERFACE:
-            DPRINT1("Not implemented\n");
+            CtrlSetup->bRequest = USB_REQUEST_SET_INTERFACE;
+            CtrlSetup->wValue.W =
Urb->UrbSelectInterface.Interface.AlternateSetting;
+            CtrlSetup->wIndex.W =
Urb->UrbSelectInterface.Interface.InterfaceNumber;
+            CtrlSetup->wLength = 0;
+            CtrlSetup->bmRequestType.B = 0x01;
             break;
 
     /* SYNC FRAME */
@@ -150,18 +157,21 @@
         KeInitializeEvent(Event, NotificationEvent, FALSE);
     }
 
-    /* Allocate Mdl for Buffer */
-    pMdl = IoAllocateMdl(TransferBuffer,
-                         TransferBufferLength,
-                         FALSE,
-                         FALSE,
-                         NULL);
-
-    /* Lock Physical Pages */
-    MmBuildMdlForNonPagedPool(pMdl);
-    //MmProbeAndLockPages(pMdl, KernelMode, IoReadAccess);
-
-    MdlPhysicalAddr = MmGetPhysicalAddress((PVOID)TransferBuffer).LowPart;
+    if (TransferBuffer)
+    {
+        /* Allocate Mdl for Buffer */
+        pMdl = IoAllocateMdl(TransferBuffer,
+                             TransferBufferLength,
+                             FALSE,
+                             FALSE,
+                             NULL);
+
+        /* Lock Physical Pages */
+        MmBuildMdlForNonPagedPool(pMdl);
+        //MmProbeAndLockPages(pMdl, KernelMode, IoReadAccess);
+
+        MdlPhysicalAddr = MmGetPhysicalAddress((PVOID)TransferBuffer).LowPart;
+    }
 
     QueueHead = CreateQueueHead(hcd);
 
@@ -175,9 +185,12 @@
     /* Save the first descriptor */
     QueueHead->TransferDescriptor = Descriptor[0];
 
-    Descriptor[1] = CreateDescriptor(hcd,
-                                     PID_CODE_IN_TOKEN,
-                                     TransferBufferLength);
+    if (TransferBuffer)
+    {
+        Descriptor[1] = CreateDescriptor(hcd,
+                                         PID_CODE_IN_TOKEN,
+                                         TransferBufferLength);        
+    }
 
     Descriptor[2] = CreateDescriptor(hcd,
                                      PID_CODE_OUT_TOKEN,
@@ -186,17 +199,34 @@
     Descriptor[1]->Token.Bits.InterruptOnComplete = FALSE;
 
     /* Link the descriptors */
-    Descriptor[0]->NextDescriptor = Descriptor[1];
-    Descriptor[1]->NextDescriptor = Descriptor[2];
-    Descriptor[1]->PreviousDescriptor = Descriptor[0];
-    Descriptor[2]->PreviousDescriptor = Descriptor[1];
-
+
+    if (TransferBuffer)
+    {
+        Descriptor[0]->NextDescriptor = Descriptor[1];
+        Descriptor[1]->NextDescriptor = Descriptor[2];
+        Descriptor[1]->PreviousDescriptor = Descriptor[0];
+        Descriptor[2]->PreviousDescriptor = Descriptor[1];
+    }
+    else
+    {
+        Descriptor[0]->NextDescriptor = Descriptor[2];
+        Descriptor[2]->PreviousDescriptor = Descriptor[0];
+    }
+    
     /* Assign the descritors buffers */
     Descriptor[0]->BufferPointer[0] = (ULONG)CtrlPhysicalPA;
-    Descriptor[1]->BufferPointer[0] = MdlPhysicalAddr;
-
-    Descriptor[0]->NextPointer = Descriptor[1]->PhysicalAddr;
-    Descriptor[1]->NextPointer = Descriptor[2]->PhysicalAddr;
+
+    if (TransferBuffer)
+    {        
+        Descriptor[1]->BufferPointer[0] = MdlPhysicalAddr;
+        Descriptor[0]->NextPointer = Descriptor[1]->PhysicalAddr;
+        Descriptor[1]->NextPointer = Descriptor[2]->PhysicalAddr;   
+    }
+    else
+    {
+        Descriptor[0]->NextPointer = Descriptor[2]->PhysicalAddr;
+    }
+    
     QueueHead->NextPointer = Descriptor[0]->PhysicalAddr;
 
     QueueHead->IrpToComplete = IrpToComplete;

Modified: trunk/reactos/drivers/usb/usbehci/usbehci.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/usb/usbehci/usbehc…
==============================================================================
--- trunk/reactos/drivers/usb/usbehci/usbehci.h [iso-8859-1] (original)
+++ trunk/reactos/drivers/usb/usbehci/usbehci.h [iso-8859-1] Thu Jan  6 17:46:59 2011
@@ -4,7 +4,7 @@
 #include <ntifs.h>
 #include <ntddk.h>
 #include <stdio.h>
-#define	NDEBUG
+#define NDEBUG
 #include <debug.h>
 #include <hubbusif.h>
 #include <usbioctl.h>
@@ -12,15 +12,16 @@
 
 #define USB_POOL_TAG (ULONG)'ebsu'
 
-#define	DEVICEINTIALIZED		0x01
-#define	DEVICESTARTED			0x02
-#define	DEVICEBUSY			0x04
-#define DEVICESTOPPED			0x08
-#define DEVICESTALLED          0x10
-
-
-#define	MAX_USB_DEVICES			127
-#define	EHCI_MAX_SIZE_TRANSFER		0x100000
+#define DEVICEINTIALIZED        0x01
+#define DEVICESTARTED           0x02
+#define DEVICEBUSY              0x04
+#define DEVICESTOPPED           0x08
+#define DEVICESTALLED           0x10
+#define DEVICEREMOVED           0x20
+
+
+#define	MAX_USB_DEVICES         127
+#define	EHCI_MAX_SIZE_TRANSFER  0x100000
 
 #define C_HUB_LOCAL_POWER   0
 #define C_HUB_OVER_CURRENT  1
@@ -71,6 +72,8 @@
     BOOLEAN IsHub;
     USB_DEVICE_SPEED DeviceSpeed;
     USB_DEVICE_TYPE DeviceType;
+    ULONG DeviceState;
+    PDEVICE_OBJECT UsbDevicePdo;
     USB_DEVICE_DESCRIPTOR DeviceDescriptor;
     UNICODE_STRING LanguageIDs;
     UNICODE_STRING iManufacturer;

Modified: trunk/reactos/drivers/usb/usbehci/usbiffn.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/drivers/usb/usbehci/usbiff…
==============================================================================
--- trunk/reactos/drivers/usb/usbehci/usbiffn.c [iso-8859-1] (original)
+++ trunk/reactos/drivers/usb/usbehci/usbiffn.c [iso-8859-1] Thu Jan  6 17:46:59 2011
@@ -132,21 +132,21 @@
     CtrlSetup.wIndex.W = 0;
     CtrlSetup.wLength = sizeof(USB_DEVICE_DESCRIPTOR);    
     CtrlSetup.bmRequestType.B = 0x80;
-    
+
     SubmitControlTransfer(&FdoDeviceExtension->hcd,
                           &CtrlSetup,
                           &UsbDevice->DeviceDescriptor,
                           sizeof(USB_DEVICE_DESCRIPTOR),
                           NULL);
-                          
+
     //DumpDeviceDescriptor(&UsbDevice->DeviceDescriptor);
-    
+
     if (UsbDevice->DeviceDescriptor.bLength != 0x12)
     {
         DPRINT1("Failed to get Device Descriptor from device connected on port
%d\n", UsbDevice->Port);
         return STATUS_DEVICE_DATA_ERROR;
     }
-    
+
     if (UsbDevice->DeviceDescriptor.bNumConfigurations == 0)
     {
         DPRINT1("Device on port %d has no configurations!\n",
UsbDevice->Port);
@@ -171,7 +171,7 @@
     }
 
     Ptr = Buffer;
-    
+
     for (i = 0; i < UsbDevice->DeviceDescriptor.bNumConfigurations; i++)
     {
         /* Get the Device Configuration Descriptor */
@@ -184,7 +184,7 @@
         CtrlSetup.wValue.HiByte = USB_CONFIGURATION_DESCRIPTOR_TYPE;
         CtrlSetup.wIndex.W = 0;
         CtrlSetup.wLength = PAGE_SIZE;
-        
+
         SubmitControlTransfer(&FdoDeviceExtension->hcd,
                               &CtrlSetup,
                               Buffer,
@@ -223,14 +223,13 @@
                 UsbDevice->Configs[i]->Interfaces[j]->EndPoints[k] =
ExAllocatePoolWithTag(NonPagedPool, sizeof(USB_ENDPOINT), USB_POOL_TAG);
                
RtlCopyMemory(&UsbDevice->Configs[i]->Interfaces[j]->EndPoints[k]->EndPointDescriptor,
                               EndpointDesc, sizeof(USB_ENDPOINT_DESCRIPTOR));
+                Ptr += sizeof(USB_ENDPOINT_DESCRIPTOR);
             }
-
         }
     }
 
     UsbDevice->ActiveConfig = UsbDevice->Configs[0];
     UsbDevice->ActiveInterface = UsbDevice->Configs[0]->Interfaces[0];
-
     return STATUS_SUCCESS;
 
     /* Set the device address */
@@ -244,13 +243,14 @@
     CtrlSetup.wLength = 0;
 
     DPRINT1("Setting Address to %x\n", UsbDevice->Address);
-    
+
     SubmitControlTransfer(&FdoDeviceExtension->hcd,
                           &CtrlSetup,
                           NULL,
                           0,
                           NULL);
 
+    PdoDeviceExtension->UsbDevices[i]->DeviceState = DEVICEINTIALIZED;
     return STATUS_SUCCESS;
 }
 
@@ -334,15 +334,12 @@
             }
 
             ExFreePool(UsbDevice);
-
-            /* DeConfig Device */
             break;
-       case USBD_KEEP_DEVICE_DATA:
-            DPRINT1("USBD_KEEP_DEVICE_DATA Not implemented!\n");
-            break;
-
         case USBD_MARK_DEVICE_BUSY:
-            DPRINT1("USBD_MARK_DEVICE_BUSY Not implemented!\n");
+            UsbDevice->DeviceState |= DEVICEBUSY;
+            /* Fall through */
+        case USBD_KEEP_DEVICE_DATA:
+            UsbDevice->DeviceState |= DEVICEREMOVED;
             break;
         default:
             DPRINT1("Unknown Remove Flags %x\n", Flags);
@@ -354,8 +351,60 @@
 USB_BUSIFFN
 RestoreUsbDevice(PVOID BusContext, PUSB_DEVICE_HANDLE OldDeviceHandle, PUSB_DEVICE_HANDLE
NewDeviceHandle)
 {
-    DPRINT1("Ehci: RestoreUsbDevice not implemented! %x, %x, %x\n", BusContext,
OldDeviceHandle, NewDeviceHandle);
-    return STATUS_NOT_SUPPORTED;
+    PUSB_DEVICE OldUsbDevice;
+    PUSB_DEVICE NewUsbDevice;
+
+    DPRINT1("Ehci: RestoreUsbDevice %x, %x, %x\n", BusContext, OldDeviceHandle,
NewDeviceHandle);
+
+    OldUsbDevice = DeviceHandleToUsbDevice(BusContext, OldDeviceHandle);
+    NewUsbDevice = DeviceHandleToUsbDevice(BusContext, NewDeviceHandle);
+
+    if (!OldUsbDevice)
+    {
+        DPRINT1("OldDeviceHandle is invalid\n");
+        return STATUS_DEVICE_NOT_CONNECTED;
+    }
+
+    if (!(OldUsbDevice->DeviceState & DEVICEREMOVED))
+    {
+        DPRINT1("UsbDevice is not marked as Removed!\n");
+        return STATUS_UNSUCCESSFUL;
+    }
+
+    if (!NewUsbDevice)
+    {
+        DPRINT1("NewDeviceHandle is invalid\n");
+        return STATUS_DEVICE_NOT_CONNECTED;
+    }
+
+    if ((OldUsbDevice->DeviceDescriptor.idVendor ==
NewUsbDevice->DeviceDescriptor.idVendor) &&
+        (OldUsbDevice->DeviceDescriptor.idProduct ==
NewUsbDevice->DeviceDescriptor.idProduct))
+    {
+        PUSB_CONFIGURATION ConfigToDelete;
+        int i;
+
+        NewUsbDevice->DeviceState &= ~DEVICEBUSY;
+        NewUsbDevice->DeviceState &= ~DEVICEREMOVED;
+
+        NewUsbDevice->ActiveConfig = OldUsbDevice->ActiveConfig;
+        NewUsbDevice->ActiveInterface = OldUsbDevice->ActiveInterface;
+
+        for (i = 0; i < NewUsbDevice->DeviceDescriptor.bNumConfigurations; i++)
+        {
+            ConfigToDelete = NewUsbDevice->Configs[i];
+            ASSERT(OldUsbDevice->Configs[i]);
+            NewUsbDevice->Configs[i] = OldUsbDevice->Configs[i];
+            OldUsbDevice->Configs[i] = ConfigToDelete;
+        }
+
+        RemoveUsbDevice(BusContext, OldDeviceHandle, 0);
+        return STATUS_SUCCESS;
+    }
+    else
+    {
+        DPRINT1("VendorId or ProductId did not match!\n");
+        return STATUS_DEVICE_NOT_CONNECTED;
+    }
 }
 
 NTSTATUS
@@ -407,13 +456,24 @@
         return STATUS_INVALID_PARAMETER;
     }
 
-    DeviceInfo->PortNumber = UsbDevice->Port;
-    DeviceInfo->HubAddress = 1;
+    DeviceInfo->HubAddress = 0;
     DeviceInfo->DeviceAddress = UsbDevice->Address;
     DeviceInfo->DeviceSpeed = UsbDevice->DeviceSpeed;
     DeviceInfo->DeviceType = UsbDevice->DeviceType;
-    DeviceInfo->CurrentConfigurationValue =
UsbDevice->ActiveConfig->ConfigurationDescriptor.bConfigurationValue;
-    DeviceInfo->NumberOfOpenPipes =
UsbDevice->ActiveInterface->InterfaceDescriptor.bNumEndpoints;
+
+    if (!UsbDevice->DeviceState)
+    {
+        DeviceInfo->CurrentConfigurationValue = 0;
+        DeviceInfo->NumberOfOpenPipes = 0;
+        DeviceInfo->PortNumber = 0;
+    }
+    else
+    {
+        DeviceInfo->CurrentConfigurationValue =
UsbDevice->ActiveConfig->ConfigurationDescriptor.bConfigurationValue;
+        /* FIXME: Use correct number of open pipes instead of all available */
+        DeviceInfo->NumberOfOpenPipes =
UsbDevice->ActiveInterface->InterfaceDescriptor.bNumEndpoints;
+        DeviceInfo->PortNumber = UsbDevice->Port;
+    }
 
     RtlCopyMemory(&DeviceInfo->DeviceDescriptor,
&UsbDevice->DeviceDescriptor, sizeof(USB_DEVICE_DESCRIPTOR));
 
@@ -495,15 +555,15 @@
         DPRINT1("InformationLevel should really be set to 0. Ignoring\n");
     }
 
-    UsbExtHubInfo->NumberOfPorts = 8;
+    UsbExtHubInfo->NumberOfPorts =
FdoDeviceExntension->hcd.ECHICaps.HCSParams.PortCount;
 
     for (i=0; i < UsbExtHubInfo->NumberOfPorts; i++)
     {
         UsbExtHubInfo->Port[i].PhysicalPortNumber = i + 1;
-        UsbExtHubInfo->Port[i].PortLabelNumber =
FdoDeviceExntension->hcd.ECHICaps.HCSParams.PortCount;
+        UsbExtHubInfo->Port[i].PortLabelNumber = i + 1;
         UsbExtHubInfo->Port[i].VidOverride = 0;
         UsbExtHubInfo->Port[i].PidOverride = 0;
-        UsbExtHubInfo->Port[i].PortAttributes = USB_PORTATTR_SHARED_USB2;
+        UsbExtHubInfo->Port[i].PortAttributes = USB_PORTATTR_SHARED_USB2;// |
USB_PORTATTR_OWNED_BY_CC;
     }
 
     *LengthReturned = FIELD_OFFSET(USB_EXTHUB_INFORMATION_0, Port[8]);
@@ -600,7 +660,17 @@
 USB_BUSIFFN
 SetDeviceHandleData(PVOID BusContext, PVOID DeviceHandle, PDEVICE_OBJECT UsbDevicePdo)
 {
-    DPRINT1("Ehci: SetDeviceHandleData not implemented %x, %x, %x\n",
BusContext, DeviceHandle, UsbDevicePdo);
+    PUSB_DEVICE UsbDevice;
+    
+    DPRINT1("Ehci: SetDeviceHandleData %x, %x, %x\n", BusContext, DeviceHandle,
UsbDevicePdo);
+    UsbDevice = DeviceHandleToUsbDevice(BusContext, DeviceHandle);
+    if (!UsbDevice)
+    {
+        DPRINT1("Invalid DeviceHandle or device not connected\n");
+        return;
+    }    
+    
+    UsbDevice->UsbDevicePdo = UsbDevicePdo;
 }
 
 
@@ -654,6 +724,6 @@
 USB_BUSIFFN
 EnumLogEntry(PVOID BusContext, ULONG DriverTag, ULONG EnumTag, ULONG P1, ULONG P2)
 {
-    DPRINT1("Ehci: EnumLogEntry called\n");
-    return STATUS_SUCCESS;
-}
+    DPRINT1("Ehci: EnumLogEntry called %x, %x, %x, %x\n", DriverTag, EnumTag,
P1, P2);
+    return STATUS_SUCCESS;
+}