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

["Tian, Feng" <feng.tian@intel.com>]

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