[USBSTOR]

- Add asserts
- Rewrite queue methods to accept the FDO only 
- Fix multiple synchronization bugs
- Don't start next irp before current irp has completed
- Use contiouslogicalbytes blocks from srb
- Should fix race conditions

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

drivers/usb/usbstor/disk.c

@@ -31,6 +31,11 @@ USBSTOR_HandleInternalDeviceControl(
     //
     Request = (PSCSI_REQUEST_BLOCK)IoStack->Parameters.Others.Argument1;
 
+    //
+    // sanity check
+    //
+    ASSERT(Request);
+
     //
     // get device extension
     //
@@ -39,8 +44,7 @@ USBSTOR_HandleInternalDeviceControl(
     //
     // sanity check
     //
-    ASSERT(Request);
+    ASSERT(PDODeviceExtension->Common.IsFDO == FALSE);
-    ASSERT(PDODeviceExtension);
 
     switch(Request->Function)
     {
@@ -87,7 +91,7 @@ USBSTOR_HandleInternalDeviceControl(
             //
             // add the request
             //
-            if (!USBSTOR_QueueAddIrp(DeviceObject, Irp))
+            if (!USBSTOR_QueueAddIrp(PDODeviceExtension->LowerDeviceObject, Irp))
             {
                 //
                 // irp was not added to the queue
@@ -154,8 +158,7 @@ USBSTOR_HandleInternalDeviceControl(
             //
             // release queue
             //
-            USBSTOR_QueueRelease(DeviceObject);
+            USBSTOR_QueueRelease(PDODeviceExtension->LowerDeviceObject);
-
 
             //
             // set status success
@@ -173,7 +176,7 @@ USBSTOR_HandleInternalDeviceControl(
             //
             // flush all requests
             //
-            USBSTOR_QueueFlushIrps(DeviceObject);
+            USBSTOR_QueueFlushIrps(PDODeviceExtension->LowerDeviceObject);
 
             //
             // set status success

drivers/usb/usbstor/queue.c

@@ -15,6 +15,10 @@ VOID
 USBSTOR_QueueInitialize(
     PFDO_DEVICE_EXTENSION FDODeviceExtension)
 {
+    //
+    // sanity check
+    //
+    ASSERT(FDODeviceExtension->Common.IsFDO);
 
     //
     // initialize queue lock
@@ -85,21 +89,19 @@ USBSTOR_QueueAddIrp(
 {
     PDRIVER_CANCEL OldDriverCancel;
     KIRQL OldLevel;
-    PPDO_DEVICE_EXTENSION PDODeviceExtension;
     PFDO_DEVICE_EXTENSION FDODeviceExtension;
     BOOLEAN IrpListFreeze;
     BOOLEAN SrbProcessing;
 
-    //
-    // get pdo device extension
-    //
-    PDODeviceExtension = (PPDO_DEVICE_EXTENSION)DeviceObject->DeviceExtension;
-    ASSERT(PDODeviceExtension->Common.IsFDO == FALSE);
-
     //
     // get FDO device extension
     //
-    FDODeviceExtension = (PFDO_DEVICE_EXTENSION)PDODeviceExtension->LowerDeviceObject->DeviceExtension;
+    FDODeviceExtension = (PFDO_DEVICE_EXTENSION)DeviceObject->DeviceExtension;
+
+    //
+    // sanity check
+    //
+    ASSERT(FDODeviceExtension->Common.IsFDO);
 
     //
     // mark irp pending
@@ -183,25 +185,19 @@ USBSTOR_RemoveIrp(
     IN PDEVICE_OBJECT DeviceObject)
 {
     KIRQL OldLevel;
-    PPDO_DEVICE_EXTENSION PDODeviceExtension;
     PFDO_DEVICE_EXTENSION FDODeviceExtension;
     PLIST_ENTRY Entry;
     PIRP Irp = NULL;
 
     //
-    // get pdo device extension
+    // get FDO device extension
     //
-    PDODeviceExtension = (PPDO_DEVICE_EXTENSION)DeviceObject->DeviceExtension;
+    FDODeviceExtension = (PFDO_DEVICE_EXTENSION)DeviceObject->DeviceExtension;
 
     //
     // sanity check
     //
-    ASSERT(PDODeviceExtension->Common.IsFDO == FALSE);
+    ASSERT(FDODeviceExtension->Common.IsFDO);
-
-    //
-    // get FDO device extension
-    //
-    FDODeviceExtension = (PFDO_DEVICE_EXTENSION)PDODeviceExtension->LowerDeviceObject->DeviceExtension;
 
     //
     // acquire lock
@@ -240,7 +236,6 @@ USBSTOR_QueueFlushIrps(
     IN PDEVICE_OBJECT DeviceObject)
 {
     KIRQL OldLevel;
-    PPDO_DEVICE_EXTENSION PDODeviceExtension;
     PFDO_DEVICE_EXTENSION FDODeviceExtension;
     PLIST_ENTRY Entry;
     PIRP Irp;
@@ -248,19 +243,14 @@ USBSTOR_QueueFlushIrps(
     PSCSI_REQUEST_BLOCK Request;
 
     //
-    // get pdo device extension
+    // get FDO device extension
     //
-    PDODeviceExtension = (PPDO_DEVICE_EXTENSION)DeviceObject->DeviceExtension;
+    FDODeviceExtension = (PFDO_DEVICE_EXTENSION)DeviceObject->DeviceExtension;
 
     //
     // sanity check
     //
-    ASSERT(PDODeviceExtension->Common.IsFDO == FALSE);
+    ASSERT(FDODeviceExtension->Common.IsFDO);
-
-    //
-    // get FDO device extension
-    //
-    FDODeviceExtension = (PFDO_DEVICE_EXTENSION)PDODeviceExtension->LowerDeviceObject->DeviceExtension;
 
     //
     // acquire lock
@@ -319,11 +309,59 @@ USBSTOR_QueueFlushIrps(
     KeReleaseSpinLock(&FDODeviceExtension->IrpListLock, OldLevel);
 }
 
+VOID
+USBSTOR_QueueTerminateRequest(
+    IN PDEVICE_OBJECT FDODeviceObject,
+    IN BOOLEAN ModifySrbState)
+{
+    KIRQL OldLevel;
+    PFDO_DEVICE_EXTENSION FDODeviceExtension;
+
+    //
+    // get FDO device extension
+    //
+    FDODeviceExtension = (PFDO_DEVICE_EXTENSION)FDODeviceObject->DeviceExtension;
+
+    //
+    // sanity check
+    //
+    ASSERT(FDODeviceExtension->Common.IsFDO);
+
+    //
+    // acquire lock
+    //
+    KeAcquireSpinLock(&FDODeviceExtension->IrpListLock, &OldLevel);
+
+    //
+    // decrement pending irp count
+    //
+    FDODeviceExtension->IrpPendingCount--;
+
+    if (ModifySrbState)
+    {
+        //
+        // sanity check
+        //
+        ASSERT(FDODeviceExtension->SrbActive == TRUE);
+
+        //
+        // indicate processing is completed
+        //
+        FDODeviceExtension->SrbActive = FALSE;
+    }
+
+    //
+    // release lock
+    //
+    KeReleaseSpinLock(&FDODeviceExtension->IrpListLock, OldLevel);
+
+}
+
 VOID
 USBSTOR_QueueNextRequest(
     IN PDEVICE_OBJECT DeviceObject)
 {
-    PPDO_DEVICE_EXTENSION PDODeviceExtension;
+    PFDO_DEVICE_EXTENSION FDODeviceExtension;
     PIRP Irp;
     PIO_STACK_LOCATION IoStack;
     PSCSI_REQUEST_BLOCK Request;
@@ -331,12 +369,12 @@ USBSTOR_QueueNextRequest(
     //
     // get pdo device extension
     //
-    PDODeviceExtension = (PPDO_DEVICE_EXTENSION)DeviceObject->DeviceExtension;
+    FDODeviceExtension = (PFDO_DEVICE_EXTENSION)DeviceObject->DeviceExtension;
 
     //
     // sanity check
     //
-    ASSERT(PDODeviceExtension->Common.IsFDO == FALSE);
+    ASSERT(FDODeviceExtension->Common.IsFDO);
 
     //
     // remove first irp from list
@@ -351,6 +389,7 @@ USBSTOR_QueueNextRequest(
         //
         // no work to do
         //
+        IoStartNextPacket(DeviceObject, TRUE);
         return;
     }
 
@@ -364,17 +403,26 @@ USBSTOR_QueueNextRequest(
     //
     Request = (PSCSI_REQUEST_BLOCK)IoStack->Parameters.Others.Argument1;
 
+    //
+    // sanity check
+    //
+    ASSERT(Request);
+
     //
     // start next packet
     //
-    IoStartPacket(PDODeviceExtension->LowerDeviceObject, Irp, &Request->QueueSortKey, USBSTOR_CancelIo);
+    IoStartPacket(DeviceObject, Irp, &Request->QueueSortKey, USBSTOR_CancelIo);
+
+    //
+    // start next request
+    //
+    IoStartNextPacket(DeviceObject, TRUE);
 }
 
 VOID
 USBSTOR_QueueRelease(
     IN PDEVICE_OBJECT DeviceObject)
 {
-    PPDO_DEVICE_EXTENSION PDODeviceExtension;
     PFDO_DEVICE_EXTENSION FDODeviceExtension;
     PIRP Irp;
     KIRQL OldLevel;
@@ -382,19 +430,14 @@ USBSTOR_QueueRelease(
     PSCSI_REQUEST_BLOCK Request;
 
     //
-    // get pdo device extension
+    // get FDO device extension
     //
-    PDODeviceExtension = (PPDO_DEVICE_EXTENSION)DeviceObject->DeviceExtension;
+    FDODeviceExtension = (PFDO_DEVICE_EXTENSION)DeviceObject->DeviceExtension;
 
     //
     // sanity check
     //
-    ASSERT(PDODeviceExtension->Common.IsFDO == FALSE);
+    ASSERT(FDODeviceExtension->Common.IsFDO);
-
-    //
-    // get FDO device extension
-    //
-    FDODeviceExtension = (PFDO_DEVICE_EXTENSION)PDODeviceExtension->LowerDeviceObject->DeviceExtension;
 
     //
     // acquire lock
@@ -440,7 +483,7 @@ USBSTOR_QueueRelease(
     //
     // start new packet
     //
-    IoStartPacket(PDODeviceExtension->LowerDeviceObject, // FDO
+    IoStartPacket(DeviceObject,
                   Irp,
                   &Request->QueueSortKey, 
                   USBSTOR_CancelIo);
@@ -511,12 +554,8 @@ USBSTOR_StartIo(
             //
             // queue next request
             //
+            USBSTOR_QueueTerminateRequest(DeviceObject, FALSE);
             USBSTOR_QueueNextRequest(DeviceObject);
-
-            //
-            // start next request
-            //
-            IoStartNextPacket(DeviceObject, TRUE);
         }
 
         //
@@ -541,6 +580,12 @@ USBSTOR_StartIo(
     ResetInProgress = FDODeviceExtension->ResetInProgress;
     ASSERT(ResetInProgress == FALSE);
 
+    //
+    // sanity check
+    //
+    ASSERT(FDODeviceExtension->SrbActive == FALSE);
+    FDODeviceExtension->SrbActive = TRUE;
+
     //
     // release lock
     //
@@ -572,37 +617,16 @@ USBSTOR_StartIo(
         Irp->IoStatus.Information = 0;
         Irp->IoStatus.Status = STATUS_DEVICE_DOES_NOT_EXIST;
         IoCompleteRequest(Irp, IO_NO_INCREMENT);
-    }
+        USBSTOR_QueueTerminateRequest(DeviceObject, TRUE);
-    else
+        return;
-    {
-        //
-        // execute scsi
-        //
-        Status = USBSTOR_HandleExecuteSCSI(IoStack->DeviceObject, Irp);
-
-        //
-        // acquire lock
-        //
-        KeAcquireSpinLock(&FDODeviceExtension->IrpListLock, &OldLevel);
-
-        //
-        // FIXME: synchronize with error handler
-        //
-        FDODeviceExtension->IrpPendingCount--;
-
-        //
-        // release lock
-        //
-        KeReleaseSpinLock(&FDODeviceExtension->IrpListLock, OldLevel);
     }
 
     //
-    // FIXME: synchronize action with error handling
+    // execute scsi
     //
-    USBSTOR_QueueNextRequest(IoStack->DeviceObject);
+    Status = USBSTOR_HandleExecuteSCSI(IoStack->DeviceObject, Irp);
 
     //
-    // start next request
+    // FIXME: handle error
     //
-    IoStartNextPacket(DeviceObject, TRUE);
 }

drivers/usb/usbstor/scsi.c

@@ -233,6 +233,16 @@ USBSTOR_CSWCompletionRoutine(
         // complete request
         //
         IoCompleteRequest(Context->Irp, IO_NO_INCREMENT);
+
+        //
+        // terminate current request
+        //
+        USBSTOR_QueueTerminateRequest(Context->PDODeviceExtension->LowerDeviceObject, TRUE);
+
+        //
+        // start next request
+        //
+        USBSTOR_QueueNextRequest(Context->PDODeviceExtension->LowerDeviceObject);
     }
 
     if (Context->Event)
@@ -432,7 +442,7 @@ USBSTOR_SendRequest(
     IN PDEVICE_OBJECT DeviceObject,
     IN PIRP OriginalRequest,
     IN OPTIONAL PKEVENT Event,
-    IN ULONG CommandLength,
+    IN UCHAR CommandLength,
     IN PUCHAR Command,
     IN ULONG TransferDataLength,
     IN PUCHAR TransferData)
@@ -740,11 +750,11 @@ USBSTOR_SendModeSenseCmd(
     UFI_SENSE_CMD Cmd;
     NTSTATUS Status;
     PVOID Response;
-    PPDO_DEVICE_EXTENSION PDODeviceExtension;
     PCBW OutControl;
     PCDB pCDB;
     PUFI_MODE_PARAMETER_HEADER Header;
 #endif
+    PPDO_DEVICE_EXTENSION PDODeviceExtension;
     PIO_STACK_LOCATION IoStack;
     PSCSI_REQUEST_BLOCK Request;
 
@@ -764,6 +774,26 @@ USBSTOR_SendModeSenseCmd(
     Irp->IoStatus.Status = STATUS_SUCCESS;
     IoCompleteRequest(Irp, IO_NO_INCREMENT);
 
+    //
+    // get PDO device extension
+    //
+    PDODeviceExtension = (PPDO_DEVICE_EXTENSION)DeviceObject->DeviceExtension;
+
+    //
+    // sanity check
+    //
+    ASSERT(PDODeviceExtension->Common.IsFDO == FALSE);
+
+    //
+    // terminate current request
+    //
+    USBSTOR_QueueTerminateRequest(PDODeviceExtension->LowerDeviceObject, TRUE);
+
+    //
+    // start next request
+    //
+    USBSTOR_QueueNextRequest(PDODeviceExtension->LowerDeviceObject);
+
     return STATUS_SUCCESS;
 
 #if 0
@@ -956,7 +986,8 @@ USBSTOR_SendReadWriteCmd(
     RtlZeroMemory(&Cmd, sizeof(UFI_READ_WRITE_CMD));
     Cmd.Code = pCDB->AsByte[0];
     Cmd.LUN = (PDODeviceExtension->LUN & MAX_LUN);
-    Cmd.ContiguousLogicBlocks = _byteswap_ushort(BlockCount);
+    Cmd.ContiguousLogicBlocksByte0 = pCDB->CDB10.TransferBlocksMsb;
+    Cmd.ContiguousLogicBlocksByte1 = pCDB->CDB10.TransferBlocksLsb;
     Cmd.LogicalBlockByte0 = pCDB->CDB10.LogicalBlockByte0;
     Cmd.LogicalBlockByte1 = pCDB->CDB10.LogicalBlockByte1;
     Cmd.LogicalBlockByte2 = pCDB->CDB10.LogicalBlockByte2;
@@ -1023,6 +1054,17 @@ USBSTOR_HandleExecuteSCSI(
     NTSTATUS Status;
     PIO_STACK_LOCATION IoStack;
     PSCSI_REQUEST_BLOCK Request;
+    PPDO_DEVICE_EXTENSION PDODeviceExtension;
+
+    //
+    // get PDO device extension
+    //
+    PDODeviceExtension = (PPDO_DEVICE_EXTENSION)DeviceObject->DeviceExtension;
+
+    //
+    // sanity check
+    //
+    ASSERT(PDODeviceExtension->Common.IsFDO == FALSE);
 
     //
     // get current stack location
@@ -1082,6 +1124,17 @@ USBSTOR_HandleExecuteSCSI(
         Irp->IoStatus.Status = STATUS_SUCCESS;
         Irp->IoStatus.Information = Request->DataTransferLength;
         IoCompleteRequest(Irp, IO_NO_INCREMENT);
+
+        //
+        // terminate current request
+        //
+        USBSTOR_QueueTerminateRequest(PDODeviceExtension->LowerDeviceObject, TRUE);
+
+        //
+        // start next request
+        //
+        USBSTOR_QueueNextRequest(PDODeviceExtension->LowerDeviceObject);
+
         return STATUS_SUCCESS;
     }
     else if (pCDB->MODE_SENSE.OperationCode == SCSIOP_TEST_UNIT_READY)

drivers/usb/usbstor/usbstor.h

@@ -68,6 +68,7 @@ typedef struct
     BOOLEAN IrpListFreeze;                                                               // if true the irp list is freezed
     BOOLEAN ResetInProgress;                                                             // if hard reset is in progress
     ULONG IrpPendingCount;                                                               // count of irp pending
+    BOOLEAN SrbActive;                                                                   // debug field if srb is pending
 }FDO_DEVICE_EXTENSION, *PFDO_DEVICE_EXTENSION;
 
 typedef struct
@@ -164,7 +165,8 @@ typedef struct
     UCHAR LogicalBlockByte2;                                         // lba byte 2
     UCHAR LogicalBlockByte3;                                         // lba byte 3
     UCHAR Reserved;                                                  // reserved 0x00
-    USHORT ContiguousLogicBlocks;                                    // num of contiguous logical blocks
+    UCHAR ContiguousLogicBlocksByte0;                                // msb contigious logic blocks byte
+    UCHAR ContiguousLogicBlocksByte1;                                // msb contigious logic blocks
     UCHAR Reserved1[3];                                              // reserved 0x00
 }UFI_READ_WRITE_CMD;
 
@@ -424,3 +426,11 @@ VOID
 USBSTOR_QueueInitialize(
     PFDO_DEVICE_EXTENSION FDODeviceExtension);
 
+VOID
+USBSTOR_QueueNextRequest(
+    IN PDEVICE_OBJECT DeviceObject);
+
+VOID
+USBSTOR_QueueTerminateRequest(
+    IN PDEVICE_OBJECT DeviceObject,
+    IN BOOLEAN ModifySrbState);