[PATCH 0/9] enhance UFS

[PATCH 0/9] enhance UFS

[Haojian Zhuang]

v1:
  1. Fix issues in UFS driver.
  2. Add PhyInit in UFS driver.
  3. Add retry in ScsiDisk for UFS device.

Haojian Zhuang (9):
  Ufs: fix data direction checking
  Ufs: fix to identify 32 bits address mode
  Ufs: fix the bit in UFS_HC_UTRLDBR_OFFSET register
  Ufs: fix to set UTRLBA and UTRLBAU register
  Ufs: fix to set PRDT length
  Ufs: add PhyInit
  Ufs: fix initialize OCS value to 0x0F
  Ufs: fix to add cache operation
  ScsiDisk: retry if device detected power failure

 MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c       |  6 +++
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c  |  2 +
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h  |  1 +
 .../Bus/Ufs/UfsPassThruDxe/UfsPassThruDxe.inf      |  1 +
 .../Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c        | 48 +++++++++++++++++-----
 MdeModulePkg/Include/Protocol/UfsHostController.h  |  7 ++++
 6 files changed, 55 insertions(+), 10 deletions(-)

-- 
2.7.4

[PATCH 1/9] Ufs: fix data direction checking

[Haojian Zhuang]

BIT6 is used in read operation, and BIT5 is used in write operation.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
index 3bd6dad..9b77a89 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
@@ -1564,7 +1564,7 @@ UfsExecScsiCmds (
 
   if (TransReq->Trd->Ocs == 0) {
     if (Packet->DataDirection == EFI_EXT_SCSI_DATA_DIRECTION_READ) {
-      if ((Response->Flags & BIT5) == BIT5) {
+      if ((Response->Flags & BIT6) == BIT6) {
         ResTranCount = Response->ResTranCount;
         SwapLittleEndianToBigEndian ((UINT8*)&ResTranCount, sizeof (UINT32));
         Packet->InTransferLength -= ResTranCount;
@@ -2321,7 +2321,7 @@ ProcessAsyncTaskList (
 
         if (TransReq->Trd->Ocs == 0) {
           if (Packet->DataDirection == EFI_EXT_SCSI_DATA_DIRECTION_READ) {
-            if ((Response->Flags & BIT5) == BIT5) {
+            if ((Response->Flags & BIT6) == BIT6) {
               ResTranCount = Response->ResTranCount;
               SwapLittleEndianToBigEndian ((UINT8*)&ResTranCount, sizeof (UINT32));
               Packet->InTransferLength -= ResTranCount;
-- 
2.7.4

[PATCH 2/9] Ufs: fix to identify 32 bits address mode

[Haojian Zhuang]

When UFS_HC_CAP_64ADDR bit is set, it means 64-bit address,
not 32-bit address.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
index 9b77a89..f160d6a 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
@@ -1749,9 +1749,9 @@ UfsAllocateAlignCommonBuffer (
   EDKII_UFS_HOST_CONTROLLER_PROTOCOL   *UfsHc;
 
   if ((Private->Capabilities & UFS_HC_CAP_64ADDR) == UFS_HC_CAP_64ADDR) {
-    Is32BitAddr = TRUE;
-  } else {
     Is32BitAddr = FALSE;
+  } else {
+    Is32BitAddr = TRUE;
   }
 
   UfsHc  = Private->UfsHostController;
-- 
2.7.4

[PATCH 3/9] Ufs: fix the bit in UFS_HC_UTRLDBR_OFFSET register

[Haojian Zhuang]

When UPIU packet is sent, (BIT0 << Slot) should be set according
to context.  But BIT0 is used without Slot when UfsWaitMemSet ()
is invoked.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
index f160d6a..e556b62 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
@@ -954,7 +954,7 @@ UfsRwDeviceDesc (
   //
   // Wait for the completion of the transfer request.
   //  
-  Status = UfsWaitMemSet (Private, UFS_HC_UTRLDBR_OFFSET, BIT0, 0, Packet.Timeout);
+  Status = UfsWaitMemSet (Private, UFS_HC_UTRLDBR_OFFSET, BIT0 << Slot, 0, Packet.Timeout);
   if (EFI_ERROR (Status)) {
     goto Exit;
   }
@@ -1077,7 +1077,7 @@ UfsRwAttributes (
   //
   // Wait for the completion of the transfer request.
   //  
-  Status = UfsWaitMemSet (Private, UFS_HC_UTRLDBR_OFFSET, BIT0, 0, Packet.Timeout);
+  Status = UfsWaitMemSet (Private, UFS_HC_UTRLDBR_OFFSET, BIT0 << Slot, 0, Packet.Timeout);
   if (EFI_ERROR (Status)) {
     goto Exit;
   }
@@ -1201,7 +1201,7 @@ UfsRwFlags (
   //
   // Wait for the completion of the transfer request.
   //  
-  Status = UfsWaitMemSet (Private, UFS_HC_UTRLDBR_OFFSET, BIT0, 0, Packet.Timeout);
+  Status = UfsWaitMemSet (Private, UFS_HC_UTRLDBR_OFFSET, BIT0 << Slot, 0, Packet.Timeout);
   if (EFI_ERROR (Status)) {
     goto Exit;
   }
@@ -1368,7 +1368,7 @@ UfsExecNopCmds (
   //
   // Wait for the completion of the transfer request.
   //  
-  Status = UfsWaitMemSet (Private, UFS_HC_UTRLDBR_OFFSET, BIT0, 0, UFS_TIMEOUT);
+  Status = UfsWaitMemSet (Private, UFS_HC_UTRLDBR_OFFSET, BIT0 << Slot, 0, UFS_TIMEOUT);
   if (EFI_ERROR (Status)) {
     goto Exit;
   }
@@ -1534,7 +1534,7 @@ UfsExecScsiCmds (
   //
   // Wait for the completion of the transfer request.
   // 
-  Status = UfsWaitMemSet (Private, UFS_HC_UTRLDBR_OFFSET, BIT0, 0, Packet->Timeout);
+  Status = UfsWaitMemSet (Private, UFS_HC_UTRLDBR_OFFSET, BIT0 << TransReq->Slot, 0, Packet->Timeout);
   if (EFI_ERROR (Status)) {
     goto Exit;
   }
-- 
2.7.4

[PATCH 4/9] Ufs: fix to set UTRLBA and UTRLBAU register

[Haojian Zhuang]

Although UfsInitTransferRequestList () assigns UTRLBA && UTRLBAU
registers, UfsCreateScsiCommandDesc () creates a page of command
buffer. It means that UTRLBA && UTRLBAU registers should be updated.

Always set UTRLBA && UTRLBAU registers before sending UPIU packet.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
index e556b62..5c256a9 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
@@ -937,6 +937,8 @@ UfsRwDeviceDesc (
   if (EFI_ERROR (Status)) {
     return Status;
   }
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBA_OFFSET, (UINTN)Trd & 0xffffffff);
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBAU_OFFSET, ((UINTN)Trd >> 32) & 0xffffffff);
 
   //
   // Check the transfer request result.
@@ -1060,6 +1062,8 @@ UfsRwAttributes (
   if (EFI_ERROR (Status)) {
     return Status;
   }
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBA_OFFSET, (UINTN)Trd & 0xffffffff);
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBAU_OFFSET, ((UINTN)Trd >> 32) & 0xffffffff);
 
   //
   // Check the transfer request result.
@@ -1184,6 +1188,8 @@ UfsRwFlags (
   if (EFI_ERROR (Status)) {
     return Status;
   }
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBA_OFFSET, (UINTN)Trd & 0xffffffff);
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBAU_OFFSET, ((UINTN)Trd >> 32) & 0xffffffff);
 
   //
   // Check the transfer request result.
@@ -1351,6 +1357,8 @@ UfsExecNopCmds (
   if (EFI_ERROR (Status)) {
     return Status;
   }
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBA_OFFSET, (UINTN)Trd & 0xffffffff);
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBAU_OFFSET, ((UINTN)Trd >> 32) & 0xffffffff);
 
   //
   // Check the transfer request result.
@@ -1473,6 +1481,8 @@ UfsExecScsiCmds (
   if (EFI_ERROR (Status)) {
     return Status;
   }
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBA_OFFSET, (UINTN)TransReq->Trd & 0xffffffff);
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBAU_OFFSET, ((UINTN)TransReq->Trd >> 32) & 0xffffffff);
 
   TransReq->CmdDescSize = TransReq->Trd->PrdtO * sizeof (UINT32) + TransReq->Trd->PrdtL * sizeof (UTP_TR_PRD);
 
-- 
2.7.4

[PATCH 5/9] Ufs: fix to set PRDT length

[Haojian Zhuang]

Should set the length of PRDT, not the number of PRDT.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
index 5c256a9..218e9f5 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
@@ -555,7 +555,7 @@ UfsCreateScsiCommandDesc (
   Trd->UcdBaU = (UINT32)RShiftU64 ((UINT64)CmdDescPhyAddr, 32);
   Trd->RuL    = (UINT16)DivU64x32 ((UINT64)ROUNDUP8 (sizeof (UTP_RESPONSE_UPIU)), sizeof (UINT32));
   Trd->RuO    = (UINT16)DivU64x32 ((UINT64)ROUNDUP8 (sizeof (UTP_COMMAND_UPIU)), sizeof (UINT32));
-  Trd->PrdtL  = (UINT16)PrdtNumber;
+  Trd->PrdtL  = (UINT16)DivU64x32 ((UINT64)ROUNDUP8 (PrdtNumber * sizeof (UTP_TR_PRD)), sizeof (UINT32));
   Trd->PrdtO  = (UINT16)DivU64x32 ((UINT64)(ROUNDUP8 (sizeof (UTP_COMMAND_UPIU)) + ROUNDUP8 (sizeof (UTP_RESPONSE_UPIU))), sizeof (UINT32));
   return EFI_SUCCESS;
 }
-- 
2.7.4

[PATCH 6/9] Ufs: add PhyInit

[Haojian Zhuang]

Add PhyInit() to support designware UFS controller.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c    | 2 ++
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 8 ++++++++
 MdeModulePkg/Include/Protocol/UfsHostController.h    | 7 +++++++
 3 files changed, 17 insertions(+)

diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
index 7c831e9..761dc8e 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
@@ -853,6 +853,8 @@ UfsPassThruDriverBindingStart (
     goto Error;
   }
 
+  MicroSecondDelay (100000);
+
   //
   // Get Ufs Device's Lun Info by reading Configuration Descriptor.
   //
diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
index 218e9f5..7c01d57 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
@@ -2089,6 +2089,7 @@ UfsControllerInit (
   )
 {
   EFI_STATUS             Status;
+  EDKII_UFS_HOST_CONTROLLER_PROTOCOL *UfsHc;
 
   Status = UfsEnableHostController (Private);
   if (EFI_ERROR (Status)) {
@@ -2096,6 +2097,13 @@ UfsControllerInit (
     return Status;
   }
 
+  UfsHc  = Private->UfsHostController;
+  Status = UfsHc->PhyInit (UfsHc);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((EFI_D_ERROR, "UfsControllerInit: Phy Init Fails, Status = %r\n", Status));
+    return Status;
+  }
+
   Status = UfsDeviceDetection (Private);
   if (EFI_ERROR (Status)) {
     DEBUG ((EFI_D_ERROR, "UfsControllerInit: Device Detection Fails, Status = %r\n", Status));
diff --git a/MdeModulePkg/Include/Protocol/UfsHostController.h b/MdeModulePkg/Include/Protocol/UfsHostController.h
index 909c981..1f3605f 100644
--- a/MdeModulePkg/Include/Protocol/UfsHostController.h
+++ b/MdeModulePkg/Include/Protocol/UfsHostController.h
@@ -221,6 +221,12 @@ EFI_STATUS
   IN OUT VOID                                      *Buffer
   );
 
+typedef
+EFI_STATUS
+(EFIAPI *EDKII_UFS_HC_PHY_INIT)(
+  IN     EDKII_UFS_HOST_CONTROLLER_PROTOCOL        *This
+  );
+
 ///
 ///  UFS Host Controller Protocol structure.
 ///
@@ -233,6 +239,7 @@ struct _EDKII_UFS_HOST_CONTROLLER_PROTOCOL {
   EDKII_UFS_HC_FLUSH                  Flush;
   EDKII_UFS_HC_MMIO_READ_WRITE        Read;
   EDKII_UFS_HC_MMIO_READ_WRITE        Write;
+  EDKII_UFS_HC_PHY_INIT               PhyInit;
 };
 
 ///
-- 
2.7.4

[PATCH 7/9] Ufs: fix initialize OCS value to 0x0F

[Haojian Zhuang]

The OCS value should be initiliazed as 0x0F according to UFS spec.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
index 7c01d57..db70fb1 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
@@ -551,6 +551,7 @@ UfsCreateScsiCommandDesc (
   Trd->Int    = UFS_INTERRUPT_COMMAND;
   Trd->Dd     = DataDirection;
   Trd->Ct     = UFS_STORAGE_COMMAND_TYPE;
+  Trd->Ocs    = 0x0F;
   Trd->UcdBa  = (UINT32)RShiftU64 ((UINT64)CmdDescPhyAddr, 7);
   Trd->UcdBaU = (UINT32)RShiftU64 ((UINT64)CmdDescPhyAddr, 32);
   Trd->RuL    = (UINT16)DivU64x32 ((UINT64)ROUNDUP8 (sizeof (UTP_RESPONSE_UPIU)), sizeof (UINT32));
@@ -719,6 +720,7 @@ UfsCreateNopCommandDesc (
   Trd->Int    = UFS_INTERRUPT_COMMAND;
   Trd->Dd     = 0x00;
   Trd->Ct     = UFS_STORAGE_COMMAND_TYPE;
+  Trd->Ocs    = 0x0F;
   Trd->UcdBa  = (UINT32)RShiftU64 ((UINT64)CmdDescPhyAddr, 7);
   Trd->UcdBaU = (UINT32)RShiftU64 ((UINT64)CmdDescPhyAddr, 32);
   Trd->RuL    = (UINT16)DivU64x32 ((UINT64)ROUNDUP8 (sizeof (UTP_NOP_IN_UPIU)), sizeof (UINT32));
-- 
2.7.4

[PATCH 8/9] Ufs: fix to add cache operation

[Haojian Zhuang]

Since command UPIU is initialized with virtual address that CPU accesses,
need to add cache operation.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h      | 1 +
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruDxe.inf | 1 +
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c   | 8 ++++++++
 3 files changed, 10 insertions(+)

diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
index 7fc82ba..af13757 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
@@ -26,6 +26,7 @@
 #include <Library/BaseMemoryLib.h>
 #include <Library/MemoryAllocationLib.h>
 #include <Library/UefiBootServicesTableLib.h>
+#include <Library/CacheMaintenanceLib.h>
 #include <Library/DevicePathLib.h>
 #include <Library/TimerLib.h>
 
diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruDxe.inf b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruDxe.inf
index c90c72f..254f51a 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruDxe.inf
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruDxe.inf
@@ -49,6 +49,7 @@
   BaseMemoryLib
   UefiLib
   BaseLib
+  CacheMaintenanceLib
   UefiDriverEntryPoint
   DebugLib
   DevicePathLib
diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
index db70fb1..98a17ac 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
@@ -1449,6 +1449,7 @@ UfsExecScsiCmds (
   UTP_TR_PRD                           *PrdtBase;
   EFI_TPL                              OldTpl;
   UFS_PASS_THRU_TRANS_REQ              *TransReq;
+  UINTN                                TotalLen;
 
   TransReq       = AllocateZeroPool (sizeof (UFS_PASS_THRU_TRANS_REQ));
   if (TransReq == NULL) {
@@ -1521,6 +1522,13 @@ UfsExecScsiCmds (
   UfsInitUtpPrdt (PrdtBase, (VOID*)(UINTN)DataBufPhyAddr, DataLen);
 
   //
+  // Flush & invalidate data cache since CmdDescHost is virtual address
+  // and Command UPIU is updated after Map ().
+  //
+  TotalLen = (TransReq->Trd->PrdtO << 2) + (TransReq->Trd->PrdtL << 2);
+  WriteBackInvalidateDataCacheRange (TransReq->CmdDescHost, TotalLen);
+
+  //
   // Insert the async SCSI cmd to the Async I/O list
   //
   if (Event != NULL) {
-- 
2.7.4

[PATCH 9/9] ScsiDisk: retry if device detected power failure

[Haojian Zhuang]

If device detected power failure, just retry. This operation is common
in linux kernel.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
index b5eff25..a7b62ec 100644
--- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
+++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
@@ -2251,6 +2251,12 @@ ScsiDiskTestUnitReady (
   if (SenseDataLength != 0) {
     *NumberOfSenseKeys = SenseDataLength / sizeof (EFI_SCSI_SENSE_DATA);
     *SenseDataArray    = ScsiDiskDevice->SenseData;
+    if (((*SenseDataArray)->Sense_Key == EFI_SCSI_SK_UNIT_ATTENTION) &&
+        ((*SenseDataArray)->Addnl_Sense_Code == 0x29) &&
+        ((*SenseDataArray)->Addnl_Sense_Code_Qualifier == 0)) {
+      *NeedRetry = TRUE;
+      return EFI_NOT_READY;
+    }
     return EFI_SUCCESS;
   }
 
-- 
2.7.4

Re: [PATCH 1/9] Ufs: fix data direction checking

[Tian, Feng]

Haojian,

The bit 5 is Flags.U of RESPONSE UPIU, which means the Target has less data bytes to transfer than the Initiator requested.

That's why we use "Packet->InTransferLength -= ResTranCount;" to return actual transfer data length.

It's not related data direction checking.

The bit 6 is Flags.O, which means the Target has more data bytes to transfer than the Initiator requested. It's not designated for your below intention.

Thanks
Feng

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Haojian Zhuang
Sent: Friday, January 6, 2017 2:52 PM
To: Tian, Feng <feng.tian@intel.com>; leif.lindholm@linaro.org; ard.biesheuvel@linaro.org; edk2-devel@lists.01.org
Cc: Haojian Zhuang <haojian.zhuang@linaro.org>
Subject: [edk2] [PATCH 1/9] Ufs: fix data direction checking

BIT6 is used in read operation, and BIT5 is used in write operation.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
index 3bd6dad..9b77a89 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
@@ -1564,7 +1564,7 @@ UfsExecScsiCmds (
 
   if (TransReq->Trd->Ocs == 0) {
     if (Packet->DataDirection == EFI_EXT_SCSI_DATA_DIRECTION_READ) {
-      if ((Response->Flags & BIT5) == BIT5) {
+      if ((Response->Flags & BIT6) == BIT6) {
         ResTranCount = Response->ResTranCount;
         SwapLittleEndianToBigEndian ((UINT8*)&ResTranCount, sizeof (UINT32));
         Packet->InTransferLength -= ResTranCount; @@ -2321,7 +2321,7 @@ ProcessAsyncTaskList (
 
         if (TransReq->Trd->Ocs == 0) {
           if (Packet->DataDirection == EFI_EXT_SCSI_DATA_DIRECTION_READ) {
-            if ((Response->Flags & BIT5) == BIT5) {
+            if ((Response->Flags & BIT6) == BIT6) {
               ResTranCount = Response->ResTranCount;
               SwapLittleEndianToBigEndian ((UINT8*)&ResTranCount, sizeof (UINT32));
               Packet->InTransferLength -= ResTranCount;
--
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 2/9] Ufs: fix to identify 32 bits address mode

[Tian, Feng]

Reviewed-by: Feng Tian <feng.tian@intel.com>

Thanks
Feng

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Haojian Zhuang
Sent: Friday, January 6, 2017 2:52 PM
To: Tian, Feng <feng.tian@intel.com>; leif.lindholm@linaro.org; ard.biesheuvel@linaro.org; edk2-devel@lists.01.org
Cc: Haojian Zhuang <haojian.zhuang@linaro.org>
Subject: [edk2] [PATCH 2/9] Ufs: fix to identify 32 bits address mode

When UFS_HC_CAP_64ADDR bit is set, it means 64-bit address, not 32-bit address.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
index 9b77a89..f160d6a 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
@@ -1749,9 +1749,9 @@ UfsAllocateAlignCommonBuffer (
   EDKII_UFS_HOST_CONTROLLER_PROTOCOL   *UfsHc;
 
   if ((Private->Capabilities & UFS_HC_CAP_64ADDR) == UFS_HC_CAP_64ADDR) {
-    Is32BitAddr = TRUE;
-  } else {
     Is32BitAddr = FALSE;
+  } else {
+    Is32BitAddr = TRUE;
   }
 
   UfsHc  = Private->UfsHostController;
--
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 3/9] Ufs: fix the bit in UFS_HC_UTRLDBR_OFFSET register

[Tian, Feng]

Could you please update the same bug in UfsBlockIoPei driver as well?

And please update all the log headers to: MdeModulePkg/Ufs: xxxx.

Others look good to me.

Reviewed-by: Feng Tian <feng.tian@intel.com>

Thanks
Feng

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Haojian Zhuang
Sent: Friday, January 6, 2017 2:52 PM
To: Tian, Feng <feng.tian@intel.com>; leif.lindholm@linaro.org; ard.biesheuvel@linaro.org; edk2-devel@lists.01.org
Cc: Haojian Zhuang <haojian.zhuang@linaro.org>
Subject: [edk2] [PATCH 3/9] Ufs: fix the bit in UFS_HC_UTRLDBR_OFFSET register

When UPIU packet is sent, (BIT0 << Slot) should be set according to context.  But BIT0 is used without Slot when UfsWaitMemSet () is invoked.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
index f160d6a..e556b62 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
@@ -954,7 +954,7 @@ UfsRwDeviceDesc (
   //
   // Wait for the completion of the transfer request.
   //
-  Status = UfsWaitMemSet (Private, UFS_HC_UTRLDBR_OFFSET, BIT0, 0, Packet.Timeout);
+  Status = UfsWaitMemSet (Private, UFS_HC_UTRLDBR_OFFSET, BIT0 << Slot, 
+ 0, Packet.Timeout);
   if (EFI_ERROR (Status)) {
     goto Exit;
   }
@@ -1077,7 +1077,7 @@ UfsRwAttributes (
   //
   // Wait for the completion of the transfer request.
   //
-  Status = UfsWaitMemSet (Private, UFS_HC_UTRLDBR_OFFSET, BIT0, 0, Packet.Timeout);
+  Status = UfsWaitMemSet (Private, UFS_HC_UTRLDBR_OFFSET, BIT0 << Slot, 
+ 0, Packet.Timeout);
   if (EFI_ERROR (Status)) {
     goto Exit;
   }
@@ -1201,7 +1201,7 @@ UfsRwFlags (
   //
   // Wait for the completion of the transfer request.
   //
-  Status = UfsWaitMemSet (Private, UFS_HC_UTRLDBR_OFFSET, BIT0, 0, Packet.Timeout);
+  Status = UfsWaitMemSet (Private, UFS_HC_UTRLDBR_OFFSET, BIT0 << Slot, 
+ 0, Packet.Timeout);
   if (EFI_ERROR (Status)) {
     goto Exit;
   }
@@ -1368,7 +1368,7 @@ UfsExecNopCmds (
   //
   // Wait for the completion of the transfer request.
   //
-  Status = UfsWaitMemSet (Private, UFS_HC_UTRLDBR_OFFSET, BIT0, 0, UFS_TIMEOUT);
+  Status = UfsWaitMemSet (Private, UFS_HC_UTRLDBR_OFFSET, BIT0 << Slot, 
+ 0, UFS_TIMEOUT);
   if (EFI_ERROR (Status)) {
     goto Exit;
   }
@@ -1534,7 +1534,7 @@ UfsExecScsiCmds (
   //
   // Wait for the completion of the transfer request.
   //
-  Status = UfsWaitMemSet (Private, UFS_HC_UTRLDBR_OFFSET, BIT0, 0, Packet->Timeout);
+  Status = UfsWaitMemSet (Private, UFS_HC_UTRLDBR_OFFSET, BIT0 << 
+ TransReq->Slot, 0, Packet->Timeout);
   if (EFI_ERROR (Status)) {
     goto Exit;
   }
--
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 7/9] Ufs: fix initialize OCS value to 0x0F

[Tian, Feng]

Same with previous patches, could you please fix Ufs PEI driver as well?

Reviewed-by: Feng Tian <feng.tian@intel.com>

Thanks
Feng

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Haojian Zhuang
Sent: Friday, January 6, 2017 2:52 PM
To: Tian, Feng <feng.tian@intel.com>; leif.lindholm@linaro.org; ard.biesheuvel@linaro.org; edk2-devel@lists.01.org
Cc: Haojian Zhuang <haojian.zhuang@linaro.org>
Subject: [edk2] [PATCH 7/9] Ufs: fix initialize OCS value to 0x0F

The OCS value should be initiliazed as 0x0F according to UFS spec.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
index 7c01d57..db70fb1 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
@@ -551,6 +551,7 @@ UfsCreateScsiCommandDesc (
   Trd->Int    = UFS_INTERRUPT_COMMAND;
   Trd->Dd     = DataDirection;
   Trd->Ct     = UFS_STORAGE_COMMAND_TYPE;
+  Trd->Ocs    = 0x0F;
   Trd->UcdBa  = (UINT32)RShiftU64 ((UINT64)CmdDescPhyAddr, 7);
   Trd->UcdBaU = (UINT32)RShiftU64 ((UINT64)CmdDescPhyAddr, 32);
   Trd->RuL    = (UINT16)DivU64x32 ((UINT64)ROUNDUP8 (sizeof (UTP_RESPONSE_UPIU)), sizeof (UINT32));
@@ -719,6 +720,7 @@ UfsCreateNopCommandDesc (
   Trd->Int    = UFS_INTERRUPT_COMMAND;
   Trd->Dd     = 0x00;
   Trd->Ct     = UFS_STORAGE_COMMAND_TYPE;
+  Trd->Ocs    = 0x0F;
   Trd->UcdBa  = (UINT32)RShiftU64 ((UINT64)CmdDescPhyAddr, 7);
   Trd->UcdBaU = (UINT32)RShiftU64 ((UINT64)CmdDescPhyAddr, 32);
   Trd->RuL    = (UINT16)DivU64x32 ((UINT64)ROUNDUP8 (sizeof (UTP_NOP_IN_UPIU)), sizeof (UINT32));
-- 
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 6/9] Ufs: add PhyInit

[Tian, Feng]

Could this be done at lower layer rather than changing existing interface? 

Thanks
Feng

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Haojian Zhuang
Sent: Friday, January 6, 2017 2:52 PM
To: Tian, Feng <feng.tian@intel.com>; leif.lindholm@linaro.org; ard.biesheuvel@linaro.org; edk2-devel@lists.01.org
Cc: Haojian Zhuang <haojian.zhuang@linaro.org>
Subject: [edk2] [PATCH 6/9] Ufs: add PhyInit

Add PhyInit() to support designware UFS controller.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c    | 2 ++
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 8 ++++++++
 MdeModulePkg/Include/Protocol/UfsHostController.h    | 7 +++++++
 3 files changed, 17 insertions(+)

diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
index 7c831e9..761dc8e 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
@@ -853,6 +853,8 @@ UfsPassThruDriverBindingStart (
     goto Error;
   }
 
+  MicroSecondDelay (100000);
+
   //
   // Get Ufs Device's Lun Info by reading Configuration Descriptor.
   //
diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
index 218e9f5..7c01d57 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
@@ -2089,6 +2089,7 @@ UfsControllerInit (
   )
 {
   EFI_STATUS             Status;
+  EDKII_UFS_HOST_CONTROLLER_PROTOCOL *UfsHc;
 
   Status = UfsEnableHostController (Private);
   if (EFI_ERROR (Status)) {
@@ -2096,6 +2097,13 @@ UfsControllerInit (
     return Status;
   }
 
+  UfsHc  = Private->UfsHostController;
+  Status = UfsHc->PhyInit (UfsHc);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((EFI_D_ERROR, "UfsControllerInit: Phy Init Fails, Status = %r\n", Status));
+    return Status;
+  }
+
   Status = UfsDeviceDetection (Private);
   if (EFI_ERROR (Status)) {
     DEBUG ((EFI_D_ERROR, "UfsControllerInit: Device Detection Fails, Status = %r\n", Status)); diff --git a/MdeModulePkg/Include/Protocol/UfsHostController.h b/MdeModulePkg/Include/Protocol/UfsHostController.h
index 909c981..1f3605f 100644
--- a/MdeModulePkg/Include/Protocol/UfsHostController.h
+++ b/MdeModulePkg/Include/Protocol/UfsHostController.h
@@ -221,6 +221,12 @@ EFI_STATUS
   IN OUT VOID                                      *Buffer
   );
 
+typedef
+EFI_STATUS
+(EFIAPI *EDKII_UFS_HC_PHY_INIT)(
+  IN     EDKII_UFS_HOST_CONTROLLER_PROTOCOL        *This
+  );
+
 ///
 ///  UFS Host Controller Protocol structure.
 ///
@@ -233,6 +239,7 @@ struct _EDKII_UFS_HOST_CONTROLLER_PROTOCOL {
   EDKII_UFS_HC_FLUSH                  Flush;
   EDKII_UFS_HC_MMIO_READ_WRITE        Read;
   EDKII_UFS_HC_MMIO_READ_WRITE        Write;
+  EDKII_UFS_HC_PHY_INIT               PhyInit;
 };
 
 ///
--
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 7/9] Ufs: fix initialize OCS value to 0x0F

[Haojian Zhuang]

Sure.

Best Regards
Haojian

On 6 January 2017 at 15:26, Tian, Feng <feng.tian@intel.com> wrote:
> Same with previous patches, could you please fix Ufs PEI driver as well?
>
> Reviewed-by: Feng Tian <feng.tian@intel.com>
>
> Thanks
> Feng
>
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Haojian Zhuang
> Sent: Friday, January 6, 2017 2:52 PM
> To: Tian, Feng <feng.tian@intel.com>; leif.lindholm@linaro.org; ard.biesheuvel@linaro.org; edk2-devel@lists.01.org
> Cc: Haojian Zhuang <haojian.zhuang@linaro.org>
> Subject: [edk2] [PATCH 7/9] Ufs: fix initialize OCS value to 0x0F
>
> The OCS value should be initiliazed as 0x0F according to UFS spec.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> ---
>  MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
> index 7c01d57..db70fb1 100644
> --- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
> +++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
> @@ -551,6 +551,7 @@ UfsCreateScsiCommandDesc (
>    Trd->Int    = UFS_INTERRUPT_COMMAND;
>    Trd->Dd     = DataDirection;
>    Trd->Ct     = UFS_STORAGE_COMMAND_TYPE;
> +  Trd->Ocs    = 0x0F;
>    Trd->UcdBa  = (UINT32)RShiftU64 ((UINT64)CmdDescPhyAddr, 7);
>    Trd->UcdBaU = (UINT32)RShiftU64 ((UINT64)CmdDescPhyAddr, 32);
>    Trd->RuL    = (UINT16)DivU64x32 ((UINT64)ROUNDUP8 (sizeof (UTP_RESPONSE_UPIU)), sizeof (UINT32));
> @@ -719,6 +720,7 @@ UfsCreateNopCommandDesc (
>    Trd->Int    = UFS_INTERRUPT_COMMAND;
>    Trd->Dd     = 0x00;
>    Trd->Ct     = UFS_STORAGE_COMMAND_TYPE;
> +  Trd->Ocs    = 0x0F;
>    Trd->UcdBa  = (UINT32)RShiftU64 ((UINT64)CmdDescPhyAddr, 7);
>    Trd->UcdBaU = (UINT32)RShiftU64 ((UINT64)CmdDescPhyAddr, 32);
>    Trd->RuL    = (UINT16)DivU64x32 ((UINT64)ROUNDUP8 (sizeof (UTP_NOP_IN_UPIU)), sizeof (UINT32));
> --
> 2.7.4
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 6/9] Ufs: add PhyInit

[Haojian Zhuang]

It's a bit hard to implement it in lower layer. Since PhyInit ()
should be executed
between UFS controller enabled and UicDmeLinkStartup command.

Best Regards
Haojian

On 6 January 2017 at 15:27, Tian, Feng <feng.tian@intel.com> wrote:
> Could this be done at lower layer rather than changing existing interface?
>
> Thanks
> Feng
>
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Haojian Zhuang
> Sent: Friday, January 6, 2017 2:52 PM
> To: Tian, Feng <feng.tian@intel.com>; leif.lindholm@linaro.org; ard.biesheuvel@linaro.org; edk2-devel@lists.01.org
> Cc: Haojian Zhuang <haojian.zhuang@linaro.org>
> Subject: [edk2] [PATCH 6/9] Ufs: add PhyInit
>
> Add PhyInit() to support designware UFS controller.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> ---
>  MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c    | 2 ++
>  MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 8 ++++++++
>  MdeModulePkg/Include/Protocol/UfsHostController.h    | 7 +++++++
>  3 files changed, 17 insertions(+)
>
> diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> index 7c831e9..761dc8e 100644
> --- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> +++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> @@ -853,6 +853,8 @@ UfsPassThruDriverBindingStart (
>      goto Error;
>    }
>
> +  MicroSecondDelay (100000);
> +
>    //
>    // Get Ufs Device's Lun Info by reading Configuration Descriptor.
>    //
> diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
> index 218e9f5..7c01d57 100644
> --- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
> +++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
> @@ -2089,6 +2089,7 @@ UfsControllerInit (
>    )
>  {
>    EFI_STATUS             Status;
> +  EDKII_UFS_HOST_CONTROLLER_PROTOCOL *UfsHc;
>
>    Status = UfsEnableHostController (Private);
>    if (EFI_ERROR (Status)) {
> @@ -2096,6 +2097,13 @@ UfsControllerInit (
>      return Status;
>    }
>
> +  UfsHc  = Private->UfsHostController;
> +  Status = UfsHc->PhyInit (UfsHc);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((EFI_D_ERROR, "UfsControllerInit: Phy Init Fails, Status = %r\n", Status));
> +    return Status;
> +  }
> +
>    Status = UfsDeviceDetection (Private);
>    if (EFI_ERROR (Status)) {
>      DEBUG ((EFI_D_ERROR, "UfsControllerInit: Device Detection Fails, Status = %r\n", Status)); diff --git a/MdeModulePkg/Include/Protocol/UfsHostController.h b/MdeModulePkg/Include/Protocol/UfsHostController.h
> index 909c981..1f3605f 100644
> --- a/MdeModulePkg/Include/Protocol/UfsHostController.h
> +++ b/MdeModulePkg/Include/Protocol/UfsHostController.h
> @@ -221,6 +221,12 @@ EFI_STATUS
>    IN OUT VOID                                      *Buffer
>    );
>
> +typedef
> +EFI_STATUS
> +(EFIAPI *EDKII_UFS_HC_PHY_INIT)(
> +  IN     EDKII_UFS_HOST_CONTROLLER_PROTOCOL        *This
> +  );
> +
>  ///
>  ///  UFS Host Controller Protocol structure.
>  ///
> @@ -233,6 +239,7 @@ struct _EDKII_UFS_HOST_CONTROLLER_PROTOCOL {
>    EDKII_UFS_HC_FLUSH                  Flush;
>    EDKII_UFS_HC_MMIO_READ_WRITE        Read;
>    EDKII_UFS_HC_MMIO_READ_WRITE        Write;
> +  EDKII_UFS_HC_PHY_INIT               PhyInit;
>  };
>
>  ///
> --
> 2.7.4
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 9/9] ScsiDisk: retry if device detected power failure

[Tian, Feng]

Hi, Haojian

We have handled ASC code 0x29 at DetectMediaParsingSenseKeys(). TestUnitReady will retry if it's that case.

So I don't understand this fix.

Thanks
Feng

-----Original Message-----
From: Haojian Zhuang [mailto:haojian.zhuang@linaro.org] 
Sent: Friday, January 6, 2017 2:52 PM
To: Tian, Feng <feng.tian@intel.com>; leif.lindholm@linaro.org; ard.biesheuvel@linaro.org; edk2-devel@lists.01.org
Cc: Haojian Zhuang <haojian.zhuang@linaro.org>
Subject: [PATCH 9/9] ScsiDisk: retry if device detected power failure

If device detected power failure, just retry. This operation is common in linux kernel.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
index b5eff25..a7b62ec 100644
--- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
+++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
@@ -2251,6 +2251,12 @@ ScsiDiskTestUnitReady (
   if (SenseDataLength != 0) {
     *NumberOfSenseKeys = SenseDataLength / sizeof (EFI_SCSI_SENSE_DATA);
     *SenseDataArray    = ScsiDiskDevice->SenseData;
+    if (((*SenseDataArray)->Sense_Key == EFI_SCSI_SK_UNIT_ATTENTION) &&
+        ((*SenseDataArray)->Addnl_Sense_Code == 0x29) &&
+        ((*SenseDataArray)->Addnl_Sense_Code_Qualifier == 0)) {
+      *NeedRetry = TRUE;
+      return EFI_NOT_READY;
+    }
     return EFI_SUCCESS;
   }
 
--
2.7.4

Re: [PATCH 9/9] ScsiDisk: retry if device detected power failure

[Haojian Zhuang]

Hi Feng,

Yes, it's a bit redudant. Let's ignore it.

Best Regards
Haojian

On 6 January 2017 at 16:22, Tian, Feng <feng.tian@intel.com> wrote:
> Hi, Haojian
>
> We have handled ASC code 0x29 at DetectMediaParsingSenseKeys(). TestUnitReady will retry if it's that case.
>
> So I don't understand this fix.
>
> Thanks
> Feng
>
> -----Original Message-----
> From: Haojian Zhuang [mailto:haojian.zhuang@linaro.org]
> Sent: Friday, January 6, 2017 2:52 PM
> To: Tian, Feng <feng.tian@intel.com>; leif.lindholm@linaro.org; ard.biesheuvel@linaro.org; edk2-devel@lists.01.org
> Cc: Haojian Zhuang <haojian.zhuang@linaro.org>
> Subject: [PATCH 9/9] ScsiDisk: retry if device detected power failure
>
> If device detected power failure, just retry. This operation is common in linux kernel.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> ---
>  MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> index b5eff25..a7b62ec 100644
> --- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> +++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> @@ -2251,6 +2251,12 @@ ScsiDiskTestUnitReady (
>    if (SenseDataLength != 0) {
>      *NumberOfSenseKeys = SenseDataLength / sizeof (EFI_SCSI_SENSE_DATA);
>      *SenseDataArray    = ScsiDiskDevice->SenseData;
> +    if (((*SenseDataArray)->Sense_Key == EFI_SCSI_SK_UNIT_ATTENTION) &&
> +        ((*SenseDataArray)->Addnl_Sense_Code == 0x29) &&
> +        ((*SenseDataArray)->Addnl_Sense_Code_Qualifier == 0)) {
> +      *NeedRetry = TRUE;
> +      return EFI_NOT_READY;
> +    }
>      return EFI_SUCCESS;
>    }
>
> --
> 2.7.4
>

Re: [PATCH 1/9] Ufs: fix data direction checking

[Haojian Zhuang]

Hi Feng,

Yes, it's a different flag. My fix is wrong.

Best Regards
Haojian

On 6 January 2017 at 15:08, Tian, Feng <feng.tian@intel.com> wrote:
> Haojian,
>
> The bit 5 is Flags.U of RESPONSE UPIU, which means the Target has less data bytes to transfer than the Initiator requested.
>
> That's why we use "Packet->InTransferLength -= ResTranCount;" to return actual transfer data length.
>
> It's not related data direction checking.
>
> The bit 6 is Flags.O, which means the Target has more data bytes to transfer than the Initiator requested. It's not designated for your below intention.
>
> Thanks
> Feng
>
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Haojian Zhuang
> Sent: Friday, January 6, 2017 2:52 PM
> To: Tian, Feng <feng.tian@intel.com>; leif.lindholm@linaro.org; ard.biesheuvel@linaro.org; edk2-devel@lists.01.org
> Cc: Haojian Zhuang <haojian.zhuang@linaro.org>
> Subject: [edk2] [PATCH 1/9] Ufs: fix data direction checking
>
> BIT6 is used in read operation, and BIT5 is used in write operation.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> ---
>  MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
> index 3bd6dad..9b77a89 100644
> --- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
> +++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
> @@ -1564,7 +1564,7 @@ UfsExecScsiCmds (
>
>    if (TransReq->Trd->Ocs == 0) {
>      if (Packet->DataDirection == EFI_EXT_SCSI_DATA_DIRECTION_READ) {
> -      if ((Response->Flags & BIT5) == BIT5) {
> +      if ((Response->Flags & BIT6) == BIT6) {
>          ResTranCount = Response->ResTranCount;
>          SwapLittleEndianToBigEndian ((UINT8*)&ResTranCount, sizeof (UINT32));
>          Packet->InTransferLength -= ResTranCount; @@ -2321,7 +2321,7 @@ ProcessAsyncTaskList (
>
>          if (TransReq->Trd->Ocs == 0) {
>            if (Packet->DataDirection == EFI_EXT_SCSI_DATA_DIRECTION_READ) {
> -            if ((Response->Flags & BIT5) == BIT5) {
> +            if ((Response->Flags & BIT6) == BIT6) {
>                ResTranCount = Response->ResTranCount;
>                SwapLittleEndianToBigEndian ((UINT8*)&ResTranCount, sizeof (UINT32));
>                Packet->InTransferLength -= ResTranCount;
> --
> 2.7.4
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 4/9] Ufs: fix to set UTRLBA and UTRLBAU register

[Tian, Feng]

Hi, Haojian

If the UTRLBA & UTRLBAU gets changed like below, the doorbell location also should be updated. So I don't suggest to update them.

Thanks
Feng

-----Original Message-----
From: Haojian Zhuang [mailto:haojian.zhuang@linaro.org] 
Sent: Friday, January 6, 2017 2:52 PM
To: Tian, Feng <feng.tian@intel.com>; leif.lindholm@linaro.org; ard.biesheuvel@linaro.org; edk2-devel@lists.01.org
Cc: Haojian Zhuang <haojian.zhuang@linaro.org>
Subject: [PATCH 4/9] Ufs: fix to set UTRLBA and UTRLBAU register

Although UfsInitTransferRequestList () assigns UTRLBA && UTRLBAU registers, UfsCreateScsiCommandDesc () creates a page of command buffer. It means that UTRLBA && UTRLBAU registers should be updated.

Always set UTRLBA && UTRLBAU registers before sending UPIU packet.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
index e556b62..5c256a9 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
@@ -937,6 +937,8 @@ UfsRwDeviceDesc (
   if (EFI_ERROR (Status)) {
     return Status;
   }
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBA_OFFSET, (UINTN)Trd & 
+ 0xffffffff);
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBAU_OFFSET, ((UINTN)Trd >> 32) & 
+ 0xffffffff);
 
   //
   // Check the transfer request result.
@@ -1060,6 +1062,8 @@ UfsRwAttributes (
   if (EFI_ERROR (Status)) {
     return Status;
   }
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBA_OFFSET, (UINTN)Trd & 
+ 0xffffffff);
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBAU_OFFSET, ((UINTN)Trd >> 32) & 
+ 0xffffffff);
 
   //
   // Check the transfer request result.
@@ -1184,6 +1188,8 @@ UfsRwFlags (
   if (EFI_ERROR (Status)) {
     return Status;
   }
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBA_OFFSET, (UINTN)Trd & 
+ 0xffffffff);
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBAU_OFFSET, ((UINTN)Trd >> 32) & 
+ 0xffffffff);
 
   //
   // Check the transfer request result.
@@ -1351,6 +1357,8 @@ UfsExecNopCmds (
   if (EFI_ERROR (Status)) {
     return Status;
   }
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBA_OFFSET, (UINTN)Trd & 
+ 0xffffffff);
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBAU_OFFSET, ((UINTN)Trd >> 32) & 
+ 0xffffffff);
 
   //
   // Check the transfer request result.
@@ -1473,6 +1481,8 @@ UfsExecScsiCmds (
   if (EFI_ERROR (Status)) {
     return Status;
   }
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBA_OFFSET, (UINTN)TransReq->Trd & 
+ 0xffffffff);
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBAU_OFFSET, ((UINTN)TransReq->Trd 
+ >> 32) & 0xffffffff);
 
   TransReq->CmdDescSize = TransReq->Trd->PrdtO * sizeof (UINT32) + TransReq->Trd->PrdtL * sizeof (UTP_TR_PRD);
 
--
2.7.4

Re: [PATCH 8/9] Ufs: fix to add cache operation

[Tian, Feng]

Haojian,

The CmdDescHost is a EdkiiUfsHcOperationBusMasterCommonBuffer common buffer, which means Host and DMA could access this region at the same time. So there is no cache coherent issue:)

Thanks
Feng

-----Original Message-----
From: Haojian Zhuang [mailto:haojian.zhuang@linaro.org] 
Sent: Friday, January 6, 2017 2:52 PM
To: Tian, Feng <feng.tian@intel.com>; leif.lindholm@linaro.org; ard.biesheuvel@linaro.org; edk2-devel@lists.01.org
Cc: Haojian Zhuang <haojian.zhuang@linaro.org>
Subject: [PATCH 8/9] Ufs: fix to add cache operation

Since command UPIU is initialized with virtual address that CPU accesses, need to add cache operation.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h      | 1 +
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruDxe.inf | 1 +
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c   | 8 ++++++++
 3 files changed, 10 insertions(+)

diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
index 7fc82ba..af13757 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
@@ -26,6 +26,7 @@
 #include <Library/BaseMemoryLib.h>
 #include <Library/MemoryAllocationLib.h>  #include <Library/UefiBootServicesTableLib.h>
+#include <Library/CacheMaintenanceLib.h>
 #include <Library/DevicePathLib.h>
 #include <Library/TimerLib.h>
 
diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruDxe.inf b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruDxe.inf
index c90c72f..254f51a 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruDxe.inf
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruDxe.inf
@@ -49,6 +49,7 @@
   BaseMemoryLib
   UefiLib
   BaseLib
+  CacheMaintenanceLib
   UefiDriverEntryPoint
   DebugLib
   DevicePathLib
diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
index db70fb1..98a17ac 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
@@ -1449,6 +1449,7 @@ UfsExecScsiCmds (
   UTP_TR_PRD                           *PrdtBase;
   EFI_TPL                              OldTpl;
   UFS_PASS_THRU_TRANS_REQ              *TransReq;
+  UINTN                                TotalLen;
 
   TransReq       = AllocateZeroPool (sizeof (UFS_PASS_THRU_TRANS_REQ));
   if (TransReq == NULL) {
@@ -1521,6 +1522,13 @@ UfsExecScsiCmds (
   UfsInitUtpPrdt (PrdtBase, (VOID*)(UINTN)DataBufPhyAddr, DataLen);
 
   //
+  // Flush & invalidate data cache since CmdDescHost is virtual address  
+ // and Command UPIU is updated after Map ().
+  //
+  TotalLen = (TransReq->Trd->PrdtO << 2) + (TransReq->Trd->PrdtL << 2);  
+ WriteBackInvalidateDataCacheRange (TransReq->CmdDescHost, TotalLen);
+
+  //
   // Insert the async SCSI cmd to the Async I/O list
   //
   if (Event != NULL) {
--
2.7.4

Re: [PATCH 6/9] Ufs: add PhyInit

[Tian, Feng]

Yes, I am thinking moving UFS HC enable to lower layer, this way should be able to eliminate interface change...

Thanks
Feng

-----Original Message-----
From: Haojian Zhuang [mailto:haojian.zhuang@linaro.org] 
Sent: Friday, January 6, 2017 4:22 PM
To: Tian, Feng <feng.tian@intel.com>
Cc: leif.lindholm@linaro.org; ard.biesheuvel@linaro.org; edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH 6/9] Ufs: add PhyInit

It's a bit hard to implement it in lower layer. Since PhyInit () should be executed between UFS controller enabled and UicDmeLinkStartup command.

Best Regards
Haojian

On 6 January 2017 at 15:27, Tian, Feng <feng.tian@intel.com> wrote:
> Could this be done at lower layer rather than changing existing interface?
>
> Thanks
> Feng
>
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of 
> Haojian Zhuang
> Sent: Friday, January 6, 2017 2:52 PM
> To: Tian, Feng <feng.tian@intel.com>; leif.lindholm@linaro.org; 
> ard.biesheuvel@linaro.org; edk2-devel@lists.01.org
> Cc: Haojian Zhuang <haojian.zhuang@linaro.org>
> Subject: [edk2] [PATCH 6/9] Ufs: add PhyInit
>
> Add PhyInit() to support designware UFS controller.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> ---
>  MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c    | 2 ++
>  MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 8 ++++++++
>  MdeModulePkg/Include/Protocol/UfsHostController.h    | 7 +++++++
>  3 files changed, 17 insertions(+)
>
> diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c 
> b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> index 7c831e9..761dc8e 100644
> --- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> +++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> @@ -853,6 +853,8 @@ UfsPassThruDriverBindingStart (
>      goto Error;
>    }
>
> +  MicroSecondDelay (100000);
> +
>    //
>    // Get Ufs Device's Lun Info by reading Configuration Descriptor.
>    //
> diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c 
> b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
> index 218e9f5..7c01d57 100644
> --- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
> +++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
> @@ -2089,6 +2089,7 @@ UfsControllerInit (
>    )
>  {
>    EFI_STATUS             Status;
> +  EDKII_UFS_HOST_CONTROLLER_PROTOCOL *UfsHc;
>
>    Status = UfsEnableHostController (Private);
>    if (EFI_ERROR (Status)) {
> @@ -2096,6 +2097,13 @@ UfsControllerInit (
>      return Status;
>    }
>
> +  UfsHc  = Private->UfsHostController;  Status = UfsHc->PhyInit 
> + (UfsHc);  if (EFI_ERROR (Status)) {
> +    DEBUG ((EFI_D_ERROR, "UfsControllerInit: Phy Init Fails, Status = %r\n", Status));
> +    return Status;
> +  }
> +
>    Status = UfsDeviceDetection (Private);
>    if (EFI_ERROR (Status)) {
>      DEBUG ((EFI_D_ERROR, "UfsControllerInit: Device Detection Fails, 
> Status = %r\n", Status)); diff --git 
> a/MdeModulePkg/Include/Protocol/UfsHostController.h 
> b/MdeModulePkg/Include/Protocol/UfsHostController.h
> index 909c981..1f3605f 100644
> --- a/MdeModulePkg/Include/Protocol/UfsHostController.h
> +++ b/MdeModulePkg/Include/Protocol/UfsHostController.h
> @@ -221,6 +221,12 @@ EFI_STATUS
>    IN OUT VOID                                      *Buffer
>    );
>
> +typedef
> +EFI_STATUS
> +(EFIAPI *EDKII_UFS_HC_PHY_INIT)(
> +  IN     EDKII_UFS_HOST_CONTROLLER_PROTOCOL        *This
> +  );
> +
>  ///
>  ///  UFS Host Controller Protocol structure.
>  ///
> @@ -233,6 +239,7 @@ struct _EDKII_UFS_HOST_CONTROLLER_PROTOCOL {
>    EDKII_UFS_HC_FLUSH                  Flush;
>    EDKII_UFS_HC_MMIO_READ_WRITE        Read;
>    EDKII_UFS_HC_MMIO_READ_WRITE        Write;
> +  EDKII_UFS_HC_PHY_INIT               PhyInit;
>  };
>
>  ///
> --
> 2.7.4
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 4/9] Ufs: fix to set UTRLBA and UTRLBAU register

[Haojian Zhuang]

Hi Feng,



I don’t understand why doorbell location should be updated too. If we don’t update UTRLBA & UTRLBAU for sending each UPIU packet, we have to extend the size of UTRD packet. Then we could avoid to create a new command buffer.



Best Regards

Haojian



From: Tian, Feng<mailto:feng.tian@intel.com>
Sent: 2017年1月6日 16:36
To: Haojian Zhuang<mailto:haojian.zhuang@linaro.org>; leif.lindholm@linaro.org<mailto:leif.lindholm@linaro.org>; ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Cc: Tian, Feng<mailto:feng.tian@intel.com>
Subject: RE: [PATCH 4/9] Ufs: fix to set UTRLBA and UTRLBAU register



Hi, Haojian

If the UTRLBA & UTRLBAU gets changed like below, the doorbell location also should be updated. So I don't suggest to update them.

Thanks
Feng

-----Original Message-----
From: Haojian Zhuang [mailto:haojian.zhuang@linaro.org]
Sent: Friday, January 6, 2017 2:52 PM
To: Tian, Feng <feng.tian@intel.com>; leif.lindholm@linaro.org; ard.biesheuvel@linaro.org; edk2-devel@lists.01.org
Cc: Haojian Zhuang <haojian.zhuang@linaro.org>
Subject: [PATCH 4/9] Ufs: fix to set UTRLBA and UTRLBAU register

Although UfsInitTransferRequestList () assigns UTRLBA && UTRLBAU registers, UfsCreateScsiCommandDesc () creates a page of command buffer. It means that UTRLBA && UTRLBAU registers should be updated.

Always set UTRLBA && UTRLBAU registers before sending UPIU packet.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
index e556b62..5c256a9 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
@@ -937,6 +937,8 @@ UfsRwDeviceDesc (
   if (EFI_ERROR (Status)) {
     return Status;
   }
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBA_OFFSET, (UINTN)Trd &
+ 0xffffffff);
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBAU_OFFSET, ((UINTN)Trd >> 32) &
+ 0xffffffff);

   //
   // Check the transfer request result.
@@ -1060,6 +1062,8 @@ UfsRwAttributes (
   if (EFI_ERROR (Status)) {
     return Status;
   }
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBA_OFFSET, (UINTN)Trd &
+ 0xffffffff);
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBAU_OFFSET, ((UINTN)Trd >> 32) &
+ 0xffffffff);

   //
   // Check the transfer request result.
@@ -1184,6 +1188,8 @@ UfsRwFlags (
   if (EFI_ERROR (Status)) {
     return Status;
   }
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBA_OFFSET, (UINTN)Trd &
+ 0xffffffff);
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBAU_OFFSET, ((UINTN)Trd >> 32) &
+ 0xffffffff);

   //
   // Check the transfer request result.
@@ -1351,6 +1357,8 @@ UfsExecNopCmds (
   if (EFI_ERROR (Status)) {
     return Status;
   }
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBA_OFFSET, (UINTN)Trd &
+ 0xffffffff);
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBAU_OFFSET, ((UINTN)Trd >> 32) &
+ 0xffffffff);

   //
   // Check the transfer request result.
@@ -1473,6 +1481,8 @@ UfsExecScsiCmds (
   if (EFI_ERROR (Status)) {
     return Status;
   }
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBA_OFFSET, (UINTN)TransReq->Trd &
+ 0xffffffff);
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBAU_OFFSET, ((UINTN)TransReq->Trd
+ >> 32) & 0xffffffff);

   TransReq->CmdDescSize = TransReq->Trd->PrdtO * sizeof (UINT32) + TransReq->Trd->PrdtL * sizeof (UTP_TR_PRD);

--
2.7.4

Re: [PATCH 4/9] Ufs: fix to set UTRLBA and UTRLBAU register

[Tian, Feng]

Haojian,

Please refer to below pic.

For example, we set UTRL as 0x10000, and we found 5th slot is free, then we should doorbell the 5th bit of UTRLDBR.

Your change will set UTRL as 0x100050,(assume each TRD uses 0x10bytes for easy understanding), then you have to set doorbell to 0.

Besides this, your fix also disobeys the UFS spec, which requests the UTRLBA is 1KB align.

[cid:image003.jpg@01D26A5A.4655E180]

Thanks
Feng

From: Haojian Zhuang [mailto:haojian.zhuang@linaro.org]
Sent: Friday, January 6, 2017 5:06 PM
To: Tian, Feng <feng.tian@intel.com>; leif.lindholm@linaro.org; ard.biesheuvel@linaro.org; edk2-devel@lists.01.org
Cc: Tian, Feng <feng.tian@intel.com>
Subject: RE: [PATCH 4/9] Ufs: fix to set UTRLBA and UTRLBAU register


Hi Feng,



I don’t understand why doorbell location should be updated too. If we don’t update UTRLBA & UTRLBAU for sending each UPIU packet, we have to extend the size of UTRD packet. Then we could avoid to create a new command buffer.



Best Regards

Haojian



From: Tian, Feng<mailto:feng.tian@intel.com>
Sent: 2017年1月6日 16:36
To: Haojian Zhuang<mailto:haojian.zhuang@linaro.org>; leif.lindholm@linaro.org<mailto:leif.lindholm@linaro.org>; ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Cc: Tian, Feng<mailto:feng.tian@intel.com>
Subject: RE: [PATCH 4/9] Ufs: fix to set UTRLBA and UTRLBAU register


Hi, Haojian

If the UTRLBA & UTRLBAU gets changed like below, the doorbell location also should be updated. So I don't suggest to update them.

Thanks
Feng

-----Original Message-----
From: Haojian Zhuang [mailto:haojian.zhuang@linaro.org]
Sent: Friday, January 6, 2017 2:52 PM
To: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>; leif.lindholm@linaro.org<mailto:leif.lindholm@linaro.org>; ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Cc: Haojian Zhuang <haojian.zhuang@linaro.org<mailto:haojian.zhuang@linaro.org>>
Subject: [PATCH 4/9] Ufs: fix to set UTRLBA and UTRLBAU register

Although UfsInitTransferRequestList () assigns UTRLBA && UTRLBAU registers, UfsCreateScsiCommandDesc () creates a page of command buffer. It means that UTRLBA && UTRLBAU registers should be updated.

Always set UTRLBA && UTRLBAU registers before sending UPIU packet.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org<mailto:haojian.zhuang@linaro.org>>
---
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
index e556b62..5c256a9 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
@@ -937,6 +937,8 @@ UfsRwDeviceDesc (
   if (EFI_ERROR (Status)) {
     return Status;
   }
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBA_OFFSET, (UINTN)Trd &
+ 0xffffffff);
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBAU_OFFSET, ((UINTN)Trd >> 32) &
+ 0xffffffff);

   //
   // Check the transfer request result.
@@ -1060,6 +1062,8 @@ UfsRwAttributes (
   if (EFI_ERROR (Status)) {
     return Status;
   }
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBA_OFFSET, (UINTN)Trd &
+ 0xffffffff);
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBAU_OFFSET, ((UINTN)Trd >> 32) &
+ 0xffffffff);

   //
   // Check the transfer request result.
@@ -1184,6 +1188,8 @@ UfsRwFlags (
   if (EFI_ERROR (Status)) {
     return Status;
   }
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBA_OFFSET, (UINTN)Trd &
+ 0xffffffff);
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBAU_OFFSET, ((UINTN)Trd >> 32) &
+ 0xffffffff);

   //
   // Check the transfer request result.
@@ -1351,6 +1357,8 @@ UfsExecNopCmds (
   if (EFI_ERROR (Status)) {
     return Status;
   }
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBA_OFFSET, (UINTN)Trd &
+ 0xffffffff);
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBAU_OFFSET, ((UINTN)Trd >> 32) &
+ 0xffffffff);

   //
   // Check the transfer request result.
@@ -1473,6 +1481,8 @@ UfsExecScsiCmds (
   if (EFI_ERROR (Status)) {
     return Status;
   }
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBA_OFFSET, (UINTN)TransReq->Trd &
+ 0xffffffff);
+  UfsMmioWrite32 (Private, UFS_HC_UTRLBAU_OFFSET, ((UINTN)TransReq->Trd
+ >> 32) & 0xffffffff);

   TransReq->CmdDescSize = TransReq->Trd->PrdtO * sizeof (UINT32) + TransReq->Trd->PrdtL * sizeof (UTP_TR_PRD);

--
2.7.4