* [PATCH v6 0/3] MdeModulePkg: Fix UfsBlockIoPei timing problem
@ 2021-12-23 4:19 VincentX Ke
2021-12-23 4:19 ` [PATCH v6 1/3] MdeModulePkg: Replace with UFS_UNIT_DESC to fix timeout problem VincentX Ke
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: VincentX Ke @ 2021-12-23 4:19 UTC (permalink / raw)
To: devel; +Cc: VincentX Ke
*** BLURB HERE ***
VincentX Ke (3):
MdeModulePkg: Replace with UFS_UNIT_DESC to fix timeout problem
MdeModulePkg: Refactoring UFS DME request and fix timing problem
MdeModulePkg: Put off UFS HCS.DP checking to fix timing problem
.../Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c | 23 +-
MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c | 359 ++++++++++--------
2 files changed, 209 insertions(+), 173 deletions(-)
--
2.31.1.windows.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v6 1/3] MdeModulePkg: Replace with UFS_UNIT_DESC to fix timeout problem
2021-12-23 4:19 [PATCH v6 0/3] MdeModulePkg: Fix UfsBlockIoPei timing problem VincentX Ke
@ 2021-12-23 4:19 ` VincentX Ke
2021-12-23 5:44 ` Wu, Hao A
2021-12-23 4:19 ` [PATCH v6 2/3] MdeModulePkg: Refactoring UFS DME request and fix timing problem VincentX Ke
2021-12-23 4:19 ` [PATCH v6 3/3] MdeModulePkg: Put off UFS HCS.DP checking to " VincentX Ke
2 siblings, 1 reply; 7+ messages in thread
From: VincentX Ke @ 2021-12-23 4:19 UTC (permalink / raw)
To: devel; +Cc: VincentX Ke, Hao A Wu, Ray Ni, Ian Chiu, Maggie Chu
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3714
Replace with UFS_UNIT_DESC to fix response timeout problem.
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Ian Chiu <Ian.chiu@intel.com>
Cc: Maggie Chu <maggie.chu@intel.com>
Signed-off-by: VincentX Ke <vincentx.ke@intel.com>
---
.../Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c | 23 +++++++++----------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
index b331c0f3e3..b8651ff998 100644
--- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
+++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
@@ -1,6 +1,6 @@
/** @file
- Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2014 - 2021, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -1037,9 +1037,9 @@ InitializeUfsBlockIoPeim (
UFS_PEIM_HC_PRIVATE_DATA *Private;
EDKII_UFS_HOST_CONTROLLER_PPI *UfsHcPpi;
UINT32 Index;
- UFS_CONFIG_DESC Config;
UINTN MmioBase;
UINT8 Controller;
+ UFS_UNIT_DESC UnitDescriptor;
//
// Shadow this PEIM to run from memory
@@ -1126,19 +1126,18 @@ InitializeUfsBlockIoPeim (
}
//
- // Get Ufs Device's Lun Info by reading Configuration Descriptor.
+ // Check if 8 common luns are active and set corresponding bit mask.
//
- Status = UfsRwDeviceDesc (Private, TRUE, UfsConfigDesc, 0, 0, &Config, sizeof (UFS_CONFIG_DESC));
- if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR, "Ufs Get Configuration Descriptor Error, Status = %r\n", Status));
- Controller++;
- continue;
- }
-
for (Index = 0; Index < UFS_PEIM_MAX_LUNS; Index++) {
- if (Config.UnitDescConfParams[Index].LunEn != 0) {
- Private->Luns.BitMask |= (BIT0 << Index);
+ Status = UfsRwDeviceDesc (Private, TRUE, UfsUnitDesc, (UINT8)Index, 0, &UnitDescriptor, sizeof (UFS_UNIT_DESC));
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Fail to read UFS Unit Descriptor, Index = %X, Status = %r\n", Index, Status));
+ continue;
+ }
+
+ if (UnitDescriptor.LunEn == 0x1) {
DEBUG ((DEBUG_INFO, "Ufs %d Lun %d is enabled\n", Controller, Index));
+ Private->Luns.BitMask |= (BIT0 << Index);
}
}
--
2.31.1.windows.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v6 2/3] MdeModulePkg: Refactoring UFS DME request and fix timing problem
2021-12-23 4:19 [PATCH v6 0/3] MdeModulePkg: Fix UfsBlockIoPei timing problem VincentX Ke
2021-12-23 4:19 ` [PATCH v6 1/3] MdeModulePkg: Replace with UFS_UNIT_DESC to fix timeout problem VincentX Ke
@ 2021-12-23 4:19 ` VincentX Ke
2021-12-23 5:42 ` Wu, Hao A
2021-12-23 4:19 ` [PATCH v6 3/3] MdeModulePkg: Put off UFS HCS.DP checking to " VincentX Ke
2 siblings, 1 reply; 7+ messages in thread
From: VincentX Ke @ 2021-12-23 4:19 UTC (permalink / raw)
To: devel; +Cc: VincentX Ke, Hao A Wu, Ray Ni, Ian Chiu, Maggie Chu
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3775
Refactoring UFS DME request function and retry up to 5 times.
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Ian Chiu <Ian.chiu@intel.com>
Cc: Maggie Chu <maggie.chu@intel.com>
Signed-off-by: VincentX Ke <vincentx.ke@intel.com>
---
MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c | 312 +++++++++++---------
1 file changed, 179 insertions(+), 133 deletions(-)
diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
index 2baa57593e..cffe8e02a7 100644
--- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
@@ -1,6 +1,6 @@
/** @file
- Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2014 - 2021, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -557,7 +557,7 @@ UfsCreateDMCommandDesc (
return EFI_INVALID_PARAMETER;
}
- if ( ((Opcode == UtpQueryFuncOpcodeSetFlag) || (Opcode == UtpQueryFuncOpcodeClrFlag) || (Opcode == UtpQueryFuncOpcodeTogFlag))
+ if ( ((Opcode == UtpQueryFuncOpcodeClrFlag) || (Opcode == UtpQueryFuncOpcodeTogFlag))
&& ((DataSize != 0) || (Data != NULL)))
{
return EFI_INVALID_PARAMETER;
@@ -747,60 +747,91 @@ UfsStopExecCmd (
}
/**
- Read or write specified device descriptor of a UFS device.
+ Extracts return data from query response upiu.
- @param[in] Private The pointer to the UFS_PEIM_HC_PRIVATE_DATA data structure.
- @param[in] Read The boolean variable to show r/w direction.
- @param[in] DescId The ID of device descriptor.
- @param[in] Index The Index of device descriptor.
- @param[in] Selector The Selector of device descriptor.
- @param[in, out] Descriptor The buffer of device descriptor to be read or written.
- @param[in] DescSize The size of device descriptor buffer.
+ @param[in, out] Packet Pointer to the UFS_DEVICE_MANAGEMENT_REQUEST_PACKET.
+ @param[in] QueryResp Pointer to the query response.
- @retval EFI_SUCCESS The device descriptor was read/written successfully.
- @retval EFI_DEVICE_ERROR A device error occurred while attempting to r/w the device descriptor.
- @retval EFI_TIMEOUT A timeout occurred while waiting for the completion of r/w the device descriptor.
+ @retval EFI_INVALID_PARAMETER Packet or QueryResp are empty or opcode is invalid.
+ @retval EFI_DEVICE_ERROR Data returned from device is invalid.
+ @retval EFI_SUCCESS Data extracted.
**/
EFI_STATUS
-UfsRwDeviceDesc (
- IN UFS_PEIM_HC_PRIVATE_DATA *Private,
- IN BOOLEAN Read,
- IN UINT8 DescId,
- IN UINT8 Index,
- IN UINT8 Selector,
- IN OUT VOID *Descriptor,
- IN UINT32 DescSize
+UfsGetReturnDataFromQueryResponse (
+ IN OUT UFS_DEVICE_MANAGEMENT_REQUEST_PACKET *Packet,
+ IN UTP_QUERY_RESP_UPIU *QueryResp
)
{
- EFI_STATUS Status;
- UFS_DEVICE_MANAGEMENT_REQUEST_PACKET Packet;
- UINT8 Slot;
- UTP_TRD *Trd;
- UINTN Address;
- UTP_QUERY_RESP_UPIU *QueryResp;
- UINT8 *CmdDescBase;
- UINT32 CmdDescSize;
- UINT16 ReturnDataSize;
+ UINT16 ReturnDataSize;
- ZeroMem (&Packet, sizeof (UFS_DEVICE_MANAGEMENT_REQUEST_PACKET));
+ ReturnDataSize = 0;
- if (Read) {
- Packet.DataDirection = UfsDataIn;
- Packet.InDataBuffer = Descriptor;
- Packet.InTransferLength = DescSize;
- Packet.Opcode = UtpQueryFuncOpcodeRdDesc;
- } else {
- Packet.DataDirection = UfsDataOut;
- Packet.OutDataBuffer = Descriptor;
- Packet.OutTransferLength = DescSize;
- Packet.Opcode = UtpQueryFuncOpcodeWrDesc;
+ if ((Packet == NULL) || (QueryResp == NULL)) {
+ return EFI_INVALID_PARAMETER;
}
- Packet.DescId = DescId;
- Packet.Index = Index;
- Packet.Selector = Selector;
- Packet.Timeout = UFS_TIMEOUT;
+ switch (Packet->Opcode) {
+ case UtpQueryFuncOpcodeRdDesc:
+ ReturnDataSize = QueryResp->Tsf.Length;
+ SwapLittleEndianToBigEndian ((UINT8 *)&ReturnDataSize, sizeof (UINT16));
+ //
+ // Make sure the hardware device does not return more data than expected.
+ //
+ if (ReturnDataSize > Packet->InTransferLength) {
+ return EFI_DEVICE_ERROR;
+ }
+
+ CopyMem (Packet->InDataBuffer, (QueryResp + 1), ReturnDataSize);
+ Packet->InTransferLength = ReturnDataSize;
+ break;
+ case UtpQueryFuncOpcodeWrDesc:
+ ReturnDataSize = QueryResp->Tsf.Length;
+ SwapLittleEndianToBigEndian ((UINT8 *)&ReturnDataSize, sizeof (UINT16));
+ Packet->OutTransferLength = ReturnDataSize;
+ break;
+ case UtpQueryFuncOpcodeRdFlag:
+ case UtpQueryFuncOpcodeSetFlag:
+ case UtpQueryFuncOpcodeClrFlag:
+ case UtpQueryFuncOpcodeTogFlag:
+ //
+ // The 'FLAG VALUE' field is at byte offset 3 of QueryResp->Tsf.Value
+ //
+ *((UINT8 *)(Packet->OutDataBuffer)) = *((UINT8 *)&(QueryResp->Tsf.Value) + 3);
+ break;
+ default:
+ return EFI_INVALID_PARAMETER;
+ }
+
+ return EFI_SUCCESS;
+}
+
+/**
+ Creates Transfer Request descriptor and sends Query Request to the device.
+
+ @param[in] Private Pointer to the UFS_PEIM_HC_PRIVATE_DATA.
+ @param[in, out] Packet Pointer to the UFS_DEVICE_MANAGEMENT_REQUEST_PACKET.
+
+ @retval EFI_SUCCESS The device descriptor was read/written successfully.
+ @retval EFI_INVALID_PARAMETER The DescId, Index and Selector fields in Packet are invalid
+ combination to point to a type of UFS device descriptor.
+ @retval EFI_DEVICE_ERROR A device error occurred while attempting to r/w the device descriptor.
+ @retval EFI_TIMEOUT A timeout occurred while waiting for the completion of r/w the device descriptor.
+
+**/
+EFI_STATUS
+UfsSendDmRequestRetry (
+ IN UFS_PEIM_HC_PRIVATE_DATA *Private,
+ IN OUT UFS_DEVICE_MANAGEMENT_REQUEST_PACKET *Packet
+ )
+{
+ UINT8 Slot;
+ EFI_STATUS Status;
+ UTP_TRD *Trd;
+ UINTN Address;
+ UTP_QUERY_RESP_UPIU *QueryResp;
+ UINT8 *CmdDescBase;
+ UINT32 CmdDescSize;
//
// Find out which slot of transfer request list is available.
@@ -814,8 +845,9 @@ UfsRwDeviceDesc (
//
// Fill transfer request descriptor to this slot.
//
- Status = UfsCreateDMCommandDesc (Private, &Packet, Trd);
+ Status = UfsCreateDMCommandDesc (Private, Packet, Trd);
if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Failed to create DM command descriptor\n"));
return Status;
}
@@ -835,43 +867,116 @@ UfsRwDeviceDesc (
// Wait for the completion of the transfer request.
//
Address = Private->UfsHcBase + UFS_HC_UTRLDBR_OFFSET;
- Status = UfsWaitMemSet (Address, BIT0 << Slot, 0, Packet.Timeout);
+ Status = UfsWaitMemSet (Address, (BIT0 << Slot), 0, Packet->Timeout);
if (EFI_ERROR (Status)) {
goto Exit;
}
- if (QueryResp->QueryResp != 0) {
+ if ((Trd->Ocs != 0) || (QueryResp->QueryResp != 0)) {
+ DEBUG ((DEBUG_ERROR, "Failed to send query request, OCS = %X, QueryResp = %X\n", Trd->Ocs, QueryResp->QueryResp));
DumpQueryResponseResult (QueryResp->QueryResp);
Status = EFI_DEVICE_ERROR;
goto Exit;
}
- if (Trd->Ocs == 0) {
- ReturnDataSize = QueryResp->Tsf.Length;
- SwapLittleEndianToBigEndian ((UINT8 *)&ReturnDataSize, sizeof (UINT16));
+ Status = UfsGetReturnDataFromQueryResponse (Packet, QueryResp);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Failed to get return data from query response\n"));
+ goto Exit;
+ }
- if (Read) {
- //
- // Make sure the hardware device does not return more data than expected.
- //
- if (ReturnDataSize > Packet.InTransferLength) {
- Status = EFI_DEVICE_ERROR;
- goto Exit;
- }
+Exit:
+ UfsStopExecCmd (Private, Slot);
+ UfsPeimFreeMem (Private->Pool, CmdDescBase, CmdDescSize);
- CopyMem (Packet.InDataBuffer, (QueryResp + 1), ReturnDataSize);
- Packet.InTransferLength = ReturnDataSize;
- } else {
- Packet.OutTransferLength = ReturnDataSize;
+ return Status;
+}
+
+/**
+ Sends Query Request to the device. Query is sent until device responds correctly or counter runs out.
+
+ @param[in] Private Pointer to the UFS_PEIM_HC_PRIVATE_DATA.
+ @param[in, out] Packet Pointer to the UFS_DEVICE_MANAGEMENT_REQUEST_PACKET.
+
+ @retval EFI_SUCCESS The device responded correctly to the Query request.
+ @retval EFI_INVALID_PARAMETER The DescId, Index and Selector fields in Packet are invalid
+ combination to point to a type of UFS device descriptor.
+ @retval EFI_DEVICE_ERROR A device error occurred while waiting for the response from the device.
+ @retval EFI_TIMEOUT A timeout occurred while waiting for the completion of the operation.
+
+**/
+EFI_STATUS
+UfsSendDmRequest (
+ IN UFS_PEIM_HC_PRIVATE_DATA *Private,
+ IN OUT UFS_DEVICE_MANAGEMENT_REQUEST_PACKET *Packet
+ )
+{
+ EFI_STATUS Status;
+ UINT8 Retry;
+
+ Status = EFI_SUCCESS;
+
+ for (Retry = 0; Retry < 5; Retry++) {
+ Status = UfsSendDmRequestRetry (Private, Packet);
+ if (!EFI_ERROR (Status)) {
+ return EFI_SUCCESS;
}
+ }
+
+ DEBUG ((DEBUG_ERROR, "Failed to get response from the device after %d retries\n", Retry));
+ return Status;
+}
+
+/**
+ Read or write specified device descriptor of a UFS device.
+
+ @param[in] Private The pointer to the UFS_PEIM_HC_PRIVATE_DATA data structure.
+ @param[in] Read The boolean variable to show r/w direction.
+ @param[in] DescId The ID of device descriptor.
+ @param[in] Index The Index of device descriptor.
+ @param[in] Selector The Selector of device descriptor.
+ @param[in, out] Descriptor The buffer of device descriptor to be read or written.
+ @param[in] DescSize The size of device descriptor buffer.
+
+ @retval EFI_SUCCESS The device descriptor was read/written successfully.
+ @retval EFI_DEVICE_ERROR A device error occurred while attempting to r/w the device descriptor.
+ @retval EFI_TIMEOUT A timeout occurred while waiting for the completion of r/w the device descriptor.
+
+**/
+EFI_STATUS
+UfsRwDeviceDesc (
+ IN UFS_PEIM_HC_PRIVATE_DATA *Private,
+ IN BOOLEAN Read,
+ IN UINT8 DescId,
+ IN UINT8 Index,
+ IN UINT8 Selector,
+ IN OUT VOID *Descriptor,
+ IN UINT32 DescSize
+ )
+{
+ EFI_STATUS Status;
+ UFS_DEVICE_MANAGEMENT_REQUEST_PACKET Packet;
+
+ ZeroMem (&Packet, sizeof (UFS_DEVICE_MANAGEMENT_REQUEST_PACKET));
+
+ if (Read) {
+ Packet.DataDirection = UfsDataIn;
+ Packet.InDataBuffer = Descriptor;
+ Packet.InTransferLength = DescSize;
+ Packet.Opcode = UtpQueryFuncOpcodeRdDesc;
} else {
- Status = EFI_DEVICE_ERROR;
+ Packet.DataDirection = UfsDataOut;
+ Packet.OutDataBuffer = Descriptor;
+ Packet.OutTransferLength = DescSize;
+ Packet.Opcode = UtpQueryFuncOpcodeWrDesc;
}
-Exit:
- UfsStopExecCmd (Private, Slot);
- UfsPeimFreeMem (Private->Pool, CmdDescBase, CmdDescSize);
+ Packet.DescId = DescId;
+ Packet.Index = Index;
+ Packet.Selector = Selector;
+ Packet.Timeout = UFS_TIMEOUT;
+ Status = UfsSendDmRequest (Private, &Packet);
return Status;
}
@@ -898,12 +1003,6 @@ UfsRwFlags (
{
EFI_STATUS Status;
UFS_DEVICE_MANAGEMENT_REQUEST_PACKET Packet;
- UINT8 Slot;
- UTP_TRD *Trd;
- UINTN Address;
- UTP_QUERY_RESP_UPIU *QueryResp;
- UINT8 *CmdDescBase;
- UINT32 CmdDescSize;
if (Value == NULL) {
return EFI_INVALID_PARAMETER;
@@ -926,67 +1025,14 @@ UfsRwFlags (
}
}
- Packet.DescId = FlagId;
- Packet.Index = 0;
- Packet.Selector = 0;
- Packet.Timeout = UFS_TIMEOUT;
+ Packet.OutDataBuffer = (VOID *)Value;
+ Packet.DescId = FlagId;
+ Packet.Index = 0;
+ Packet.Selector = 0;
+ Packet.Timeout = UFS_TIMEOUT;
- //
- // Find out which slot of transfer request list is available.
- //
- Status = UfsFindAvailableSlotInTrl (Private, &Slot);
- if (EFI_ERROR (Status)) {
- return Status;
- }
-
- //
- // Fill transfer request descriptor to this slot.
- //
- Trd = ((UTP_TRD *)Private->UtpTrlBase) + Slot;
- Status = UfsCreateDMCommandDesc (Private, &Packet, Trd);
- if (EFI_ERROR (Status)) {
- return Status;
- }
-
- //
- // Check the transfer request result.
- //
- CmdDescBase = (UINT8 *)(UINTN)(LShiftU64 ((UINT64)Trd->UcdBaU, 32) | LShiftU64 ((UINT64)Trd->UcdBa, 7));
- QueryResp = (UTP_QUERY_RESP_UPIU *)(CmdDescBase + Trd->RuO * sizeof (UINT32));
- CmdDescSize = Trd->RuO * sizeof (UINT32) + Trd->RuL * sizeof (UINT32);
-
- //
- // Start to execute the transfer request.
- //
- UfsStartExecCmd (Private, Slot);
-
- //
- // Wait for the completion of the transfer request.
- //
- Address = Private->UfsHcBase + UFS_HC_UTRLDBR_OFFSET;
- Status = UfsWaitMemSet (Address, BIT0 << Slot, 0, Packet.Timeout);
- if (EFI_ERROR (Status)) {
- goto Exit;
- }
-
- if (QueryResp->QueryResp != 0) {
- DumpQueryResponseResult (QueryResp->QueryResp);
- Status = EFI_DEVICE_ERROR;
- goto Exit;
- }
-
- if (Trd->Ocs == 0) {
- //
- // The 'FLAG VALUE' field is at byte offset 3 of QueryResp->Tsf.Value
- //
- *Value = *((UINT8 *)&(QueryResp->Tsf.Value) + 3);
- } else {
- Status = EFI_DEVICE_ERROR;
- }
-
-Exit:
- UfsStopExecCmd (Private, Slot);
- UfsPeimFreeMem (Private->Pool, CmdDescBase, CmdDescSize);
+ Status = UfsSendDmRequest (Private, &Packet);
+ *Value = *((UINT8 *)(Packet.OutDataBuffer));
return Status;
}
--
2.31.1.windows.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v6 3/3] MdeModulePkg: Put off UFS HCS.DP checking to fix timing problem
2021-12-23 4:19 [PATCH v6 0/3] MdeModulePkg: Fix UfsBlockIoPei timing problem VincentX Ke
2021-12-23 4:19 ` [PATCH v6 1/3] MdeModulePkg: Replace with UFS_UNIT_DESC to fix timeout problem VincentX Ke
2021-12-23 4:19 ` [PATCH v6 2/3] MdeModulePkg: Refactoring UFS DME request and fix timing problem VincentX Ke
@ 2021-12-23 4:19 ` VincentX Ke
2021-12-23 5:44 ` [edk2-devel] " Wu, Hao A
2 siblings, 1 reply; 7+ messages in thread
From: VincentX Ke @ 2021-12-23 4:19 UTC (permalink / raw)
To: devel; +Cc: VincentX Ke, Hao A Wu, Ray Ni, Ian Chiu, Maggie Chu
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3776
Put off UFS HCS.DP (Device Attached) checking
until UfsDeviceDetection() to fix timing problem.
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Ian Chiu <Ian.chiu@intel.com>
Cc: Maggie Chu <maggie.chu@intel.com>
Signed-off-by: VincentX Ke <vincentx.ke@intel.com>
---
MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c | 47 +++++++++------------
1 file changed, 19 insertions(+), 28 deletions(-)
diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
index cffe8e02a7..864bf6928d 100644
--- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
@@ -1353,23 +1353,6 @@ UfsExecUicCommands (
}
}
- //
- // Check value of HCS.DP and make sure that there is a device attached to the Link.
- //
- Address = UfsHcBase + UFS_HC_STATUS_OFFSET;
- Data = MmioRead32 (Address);
- if ((Data & UFS_HC_HCS_DP) == 0) {
- Address = UfsHcBase + UFS_HC_IS_OFFSET;
- Status = UfsWaitMemSet (Address, UFS_HC_IS_ULSS, UFS_HC_IS_ULSS, UFS_TIMEOUT);
- if (EFI_ERROR (Status)) {
- return EFI_DEVICE_ERROR;
- }
-
- return EFI_NOT_FOUND;
- }
-
- DEBUG ((DEBUG_INFO, "UfsblockioPei: found a attached UFS device\n"));
-
return EFI_SUCCESS;
}
@@ -1443,6 +1426,8 @@ UfsDeviceDetection (
)
{
UINTN Retry;
+ UINTN Address;
+ UINT32 Data;
EFI_STATUS Status;
//
@@ -1451,22 +1436,28 @@ UfsDeviceDetection (
//
for (Retry = 0; Retry < 3; Retry++) {
Status = UfsExecUicCommands (Private, UfsUicDmeLinkStartup, 0, 0, 0);
- if (!EFI_ERROR (Status)) {
- break;
+ if (EFI_ERROR (Status)) {
+ return EFI_DEVICE_ERROR;
}
- if (Status == EFI_NOT_FOUND) {
- continue;
+ //
+ // Check value of HCS.DP and make sure that there is a device attached to the Link
+ //
+ Address = Private->UfsHcBase + UFS_HC_STATUS_OFFSET;
+ Data = MmioRead32 (Address);
+ if ((Data & UFS_HC_HCS_DP) == 0) {
+ Address = Private->UfsHcBase + UFS_HC_IS_OFFSET;
+ Status = UfsWaitMemSet (Address, UFS_HC_IS_ULSS, UFS_HC_IS_ULSS, UFS_TIMEOUT);
+ if (EFI_ERROR (Status)) {
+ return EFI_DEVICE_ERROR;
+ }
+ } else {
+ DEBUG ((DEBUG_INFO, "UfsblockioPei: found a attached UFS device\n"));
+ return EFI_SUCCESS;
}
-
- return EFI_DEVICE_ERROR;
}
- if (Retry == 3) {
- return EFI_NOT_FOUND;
- }
-
- return EFI_SUCCESS;
+ return EFI_NOT_FOUND;
}
/**
--
2.31.1.windows.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v6 2/3] MdeModulePkg: Refactoring UFS DME request and fix timing problem
2021-12-23 4:19 ` [PATCH v6 2/3] MdeModulePkg: Refactoring UFS DME request and fix timing problem VincentX Ke
@ 2021-12-23 5:42 ` Wu, Hao A
0 siblings, 0 replies; 7+ messages in thread
From: Wu, Hao A @ 2021-12-23 5:42 UTC (permalink / raw)
To: Ke, VincentX, devel@edk2.groups.io; +Cc: Ni, Ray, Chiu, Ian, Chu, Maggie
Inline comment below:
> -----Original Message-----
> From: Ke, VincentX <vincentx.ke@intel.com>
> Sent: Thursday, December 23, 2021 12:19 PM
> To: devel@edk2.groups.io
> Cc: Ke, VincentX <vincentx.ke@intel.com>; Wu, Hao A
> <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Chiu, Ian
> <Ian.chiu@intel.com>; Chu, Maggie <maggie.chu@intel.com>
> Subject: [PATCH v6 2/3] MdeModulePkg: Refactoring UFS DME request and
> fix timing problem
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3775
>
> Refactoring UFS DME request function and retry up to 5 times.
>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Ian Chiu <Ian.chiu@intel.com>
> Cc: Maggie Chu <maggie.chu@intel.com>
> Signed-off-by: VincentX Ke <vincentx.ke@intel.com>
> ---
> MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c | 312 +++++++++++--------
> -
> 1 file changed, 179 insertions(+), 133 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
> b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
> index 2baa57593e..cffe8e02a7 100644
> --- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
> +++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
> @@ -1,6 +1,6 @@
> /** @file
>
> - Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2014 - 2021, Intel Corporation. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -557,7 +557,7 @@ UfsCreateDMCommandDesc (
> return EFI_INVALID_PARAMETER;
> }
>
> - if ( ((Opcode == UtpQueryFuncOpcodeSetFlag) || (Opcode ==
> UtpQueryFuncOpcodeClrFlag) || (Opcode == UtpQueryFuncOpcodeTogFlag))
> + if ( ((Opcode == UtpQueryFuncOpcodeClrFlag) || (Opcode ==
> UtpQueryFuncOpcodeTogFlag))
Please help to just remove the above check.
> && ((DataSize != 0) || (Data != NULL)))
> {
> return EFI_INVALID_PARAMETER;
> @@ -747,60 +747,91 @@ UfsStopExecCmd (
> }
>
> /**
> - Read or write specified device descriptor of a UFS device.
> + Extracts return data from query response upiu.
>
> - @param[in] Private The pointer to the
> UFS_PEIM_HC_PRIVATE_DATA data structure.
> - @param[in] Read The boolean variable to show r/w direction.
> - @param[in] DescId The ID of device descriptor.
> - @param[in] Index The Index of device descriptor.
> - @param[in] Selector The Selector of device descriptor.
> - @param[in, out] Descriptor The buffer of device descriptor to be read or
> written.
> - @param[in] DescSize The size of device descriptor buffer.
> + @param[in, out] Packet Pointer to the
> UFS_DEVICE_MANAGEMENT_REQUEST_PACKET.
> + @param[in] QueryResp Pointer to the query response.
>
> - @retval EFI_SUCCESS The device descriptor was read/written
> successfully.
> - @retval EFI_DEVICE_ERROR A device error occurred while attempting to
> r/w the device descriptor.
> - @retval EFI_TIMEOUT A timeout occurred while waiting for the
> completion of r/w the device descriptor.
> + @retval EFI_INVALID_PARAMETER Packet or QueryResp are empty or
> opcode is invalid.
> + @retval EFI_DEVICE_ERROR Data returned from device is invalid.
> + @retval EFI_SUCCESS Data extracted.
>
> **/
> EFI_STATUS
> -UfsRwDeviceDesc (
> - IN UFS_PEIM_HC_PRIVATE_DATA *Private,
> - IN BOOLEAN Read,
> - IN UINT8 DescId,
> - IN UINT8 Index,
> - IN UINT8 Selector,
> - IN OUT VOID *Descriptor,
> - IN UINT32 DescSize
> +UfsGetReturnDataFromQueryResponse (
> + IN OUT UFS_DEVICE_MANAGEMENT_REQUEST_PACKET *Packet,
> + IN UTP_QUERY_RESP_UPIU *QueryResp
> )
> {
> - EFI_STATUS Status;
> - UFS_DEVICE_MANAGEMENT_REQUEST_PACKET Packet;
> - UINT8 Slot;
> - UTP_TRD *Trd;
> - UINTN Address;
> - UTP_QUERY_RESP_UPIU *QueryResp;
> - UINT8 *CmdDescBase;
> - UINT32 CmdDescSize;
> - UINT16 ReturnDataSize;
> + UINT16 ReturnDataSize;
>
> - ZeroMem (&Packet, sizeof
> (UFS_DEVICE_MANAGEMENT_REQUEST_PACKET));
> + ReturnDataSize = 0;
>
> - if (Read) {
> - Packet.DataDirection = UfsDataIn;
> - Packet.InDataBuffer = Descriptor;
> - Packet.InTransferLength = DescSize;
> - Packet.Opcode = UtpQueryFuncOpcodeRdDesc;
> - } else {
> - Packet.DataDirection = UfsDataOut;
> - Packet.OutDataBuffer = Descriptor;
> - Packet.OutTransferLength = DescSize;
> - Packet.Opcode = UtpQueryFuncOpcodeWrDesc;
> + if ((Packet == NULL) || (QueryResp == NULL)) {
> + return EFI_INVALID_PARAMETER;
> }
>
> - Packet.DescId = DescId;
> - Packet.Index = Index;
> - Packet.Selector = Selector;
> - Packet.Timeout = UFS_TIMEOUT;
> + switch (Packet->Opcode) {
> + case UtpQueryFuncOpcodeRdDesc:
> + ReturnDataSize = QueryResp->Tsf.Length;
> + SwapLittleEndianToBigEndian ((UINT8 *)&ReturnDataSize, sizeof
> (UINT16));
> + //
> + // Make sure the hardware device does not return more data than
> expected.
> + //
> + if (ReturnDataSize > Packet->InTransferLength) {
> + return EFI_DEVICE_ERROR;
> + }
> +
> + CopyMem (Packet->InDataBuffer, (QueryResp + 1), ReturnDataSize);
> + Packet->InTransferLength = ReturnDataSize;
> + break;
> + case UtpQueryFuncOpcodeWrDesc:
> + ReturnDataSize = QueryResp->Tsf.Length;
> + SwapLittleEndianToBigEndian ((UINT8 *)&ReturnDataSize, sizeof
> (UINT16));
> + Packet->OutTransferLength = ReturnDataSize;
> + break;
> + case UtpQueryFuncOpcodeRdFlag:
> + case UtpQueryFuncOpcodeSetFlag:
> + case UtpQueryFuncOpcodeClrFlag:
> + case UtpQueryFuncOpcodeTogFlag:
> + //
> + // The 'FLAG VALUE' field is at byte offset 3 of QueryResp->Tsf.Value
> + //
> + *((UINT8 *)(Packet->OutDataBuffer)) = *((UINT8 *)&(QueryResp-
> >Tsf.Value) + 3);
Use "OutDataBuffer" only for UtpQueryFuncOpcodeSetFlag, UtpQueryFuncOpcodeClrFlag and UtpQueryFuncOpcodeTogFlag.
Use "InDataBuffer" for UtpQueryFuncOpcodeRdFlag.
> + break;
> + default:
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + return EFI_SUCCESS;
> +}
> +
> +/**
> + Creates Transfer Request descriptor and sends Query Request to the
> device.
> +
> + @param[in] Private Pointer to the UFS_PEIM_HC_PRIVATE_DATA.
> + @param[in, out] Packet Pointer to the
> UFS_DEVICE_MANAGEMENT_REQUEST_PACKET.
> +
> + @retval EFI_SUCCESS The device descriptor was read/written
> successfully.
> + @retval EFI_INVALID_PARAMETER The DescId, Index and Selector fields in
> Packet are invalid
> + combination to point to a type of UFS device descriptor.
> + @retval EFI_DEVICE_ERROR A device error occurred while attempting to
> r/w the device descriptor.
> + @retval EFI_TIMEOUT A timeout occurred while waiting for the
> completion of r/w the device descriptor.
> +
> +**/
> +EFI_STATUS
> +UfsSendDmRequestRetry (
> + IN UFS_PEIM_HC_PRIVATE_DATA *Private,
> + IN OUT UFS_DEVICE_MANAGEMENT_REQUEST_PACKET *Packet
> + )
> +{
> + UINT8 Slot;
> + EFI_STATUS Status;
> + UTP_TRD *Trd;
> + UINTN Address;
> + UTP_QUERY_RESP_UPIU *QueryResp;
> + UINT8 *CmdDescBase;
> + UINT32 CmdDescSize;
>
> //
> // Find out which slot of transfer request list is available.
> @@ -814,8 +845,9 @@ UfsRwDeviceDesc (
> //
> // Fill transfer request descriptor to this slot.
> //
> - Status = UfsCreateDMCommandDesc (Private, &Packet, Trd);
> + Status = UfsCreateDMCommandDesc (Private, Packet, Trd);
> if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "Failed to create DM command descriptor\n"));
> return Status;
> }
>
> @@ -835,43 +867,116 @@ UfsRwDeviceDesc (
> // Wait for the completion of the transfer request.
> //
> Address = Private->UfsHcBase + UFS_HC_UTRLDBR_OFFSET;
> - Status = UfsWaitMemSet (Address, BIT0 << Slot, 0, Packet.Timeout);
> + Status = UfsWaitMemSet (Address, (BIT0 << Slot), 0, Packet->Timeout);
> if (EFI_ERROR (Status)) {
> goto Exit;
> }
>
> - if (QueryResp->QueryResp != 0) {
> + if ((Trd->Ocs != 0) || (QueryResp->QueryResp != 0)) {
Please use:
if ((Trd->Ocs != 0) || (QueryResp->QueryResp != UfsUtpQueryResponseSuccess)) {
> + DEBUG ((DEBUG_ERROR, "Failed to send query request, OCS = %X,
> QueryResp = %X\n", Trd->Ocs, QueryResp->QueryResp));
> DumpQueryResponseResult (QueryResp->QueryResp);
> Status = EFI_DEVICE_ERROR;
> goto Exit;
> }
>
> - if (Trd->Ocs == 0) {
> - ReturnDataSize = QueryResp->Tsf.Length;
> - SwapLittleEndianToBigEndian ((UINT8 *)&ReturnDataSize, sizeof
> (UINT16));
> + Status = UfsGetReturnDataFromQueryResponse (Packet, QueryResp);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "Failed to get return data from query
> response\n"));
> + goto Exit;
> + }
>
> - if (Read) {
> - //
> - // Make sure the hardware device does not return more data than
> expected.
> - //
> - if (ReturnDataSize > Packet.InTransferLength) {
> - Status = EFI_DEVICE_ERROR;
> - goto Exit;
> - }
> +Exit:
> + UfsStopExecCmd (Private, Slot);
> + UfsPeimFreeMem (Private->Pool, CmdDescBase, CmdDescSize);
>
> - CopyMem (Packet.InDataBuffer, (QueryResp + 1), ReturnDataSize);
> - Packet.InTransferLength = ReturnDataSize;
> - } else {
> - Packet.OutTransferLength = ReturnDataSize;
> + return Status;
> +}
> +
> +/**
> + Sends Query Request to the device. Query is sent until device responds
> correctly or counter runs out.
> +
> + @param[in] Private Pointer to the UFS_PEIM_HC_PRIVATE_DATA.
> + @param[in, out] Packet Pointer to the
> UFS_DEVICE_MANAGEMENT_REQUEST_PACKET.
> +
> + @retval EFI_SUCCESS The device responded correctly to the Query
> request.
> + @retval EFI_INVALID_PARAMETER The DescId, Index and Selector fields in
> Packet are invalid
> + combination to point to a type of UFS device descriptor.
> + @retval EFI_DEVICE_ERROR A device error occurred while waiting for
> the response from the device.
> + @retval EFI_TIMEOUT A timeout occurred while waiting for the
> completion of the operation.
> +
> +**/
> +EFI_STATUS
> +UfsSendDmRequest (
> + IN UFS_PEIM_HC_PRIVATE_DATA *Private,
> + IN OUT UFS_DEVICE_MANAGEMENT_REQUEST_PACKET *Packet
> + )
> +{
> + EFI_STATUS Status;
> + UINT8 Retry;
> +
> + Status = EFI_SUCCESS;
> +
> + for (Retry = 0; Retry < 5; Retry++) {
> + Status = UfsSendDmRequestRetry (Private, Packet);
> + if (!EFI_ERROR (Status)) {
> + return EFI_SUCCESS;
> }
> + }
> +
> + DEBUG ((DEBUG_ERROR, "Failed to get response from the device after %d
> retries\n", Retry));
> + return Status;
> +}
> +
> +/**
> + Read or write specified device descriptor of a UFS device.
> +
> + @param[in] Private The pointer to the
> UFS_PEIM_HC_PRIVATE_DATA data structure.
> + @param[in] Read The boolean variable to show r/w direction.
> + @param[in] DescId The ID of device descriptor.
> + @param[in] Index The Index of device descriptor.
> + @param[in] Selector The Selector of device descriptor.
> + @param[in, out] Descriptor The buffer of device descriptor to be read or
> written.
> + @param[in] DescSize The size of device descriptor buffer.
> +
> + @retval EFI_SUCCESS The device descriptor was read/written
> successfully.
> + @retval EFI_DEVICE_ERROR A device error occurred while attempting to
> r/w the device descriptor.
> + @retval EFI_TIMEOUT A timeout occurred while waiting for the
> completion of r/w the device descriptor.
> +
> +**/
> +EFI_STATUS
> +UfsRwDeviceDesc (
> + IN UFS_PEIM_HC_PRIVATE_DATA *Private,
> + IN BOOLEAN Read,
> + IN UINT8 DescId,
> + IN UINT8 Index,
> + IN UINT8 Selector,
> + IN OUT VOID *Descriptor,
> + IN UINT32 DescSize
> + )
> +{
> + EFI_STATUS Status;
> + UFS_DEVICE_MANAGEMENT_REQUEST_PACKET Packet;
> +
> + ZeroMem (&Packet, sizeof
> (UFS_DEVICE_MANAGEMENT_REQUEST_PACKET));
> +
> + if (Read) {
> + Packet.DataDirection = UfsDataIn;
> + Packet.InDataBuffer = Descriptor;
> + Packet.InTransferLength = DescSize;
> + Packet.Opcode = UtpQueryFuncOpcodeRdDesc;
> } else {
> - Status = EFI_DEVICE_ERROR;
> + Packet.DataDirection = UfsDataOut;
> + Packet.OutDataBuffer = Descriptor;
> + Packet.OutTransferLength = DescSize;
> + Packet.Opcode = UtpQueryFuncOpcodeWrDesc;
> }
>
> -Exit:
> - UfsStopExecCmd (Private, Slot);
> - UfsPeimFreeMem (Private->Pool, CmdDescBase, CmdDescSize);
> + Packet.DescId = DescId;
> + Packet.Index = Index;
> + Packet.Selector = Selector;
> + Packet.Timeout = UFS_TIMEOUT;
>
> + Status = UfsSendDmRequest (Private, &Packet);
> return Status;
> }
>
> @@ -898,12 +1003,6 @@ UfsRwFlags (
> {
> EFI_STATUS Status;
> UFS_DEVICE_MANAGEMENT_REQUEST_PACKET Packet;
> - UINT8 Slot;
> - UTP_TRD *Trd;
> - UINTN Address;
> - UTP_QUERY_RESP_UPIU *QueryResp;
> - UINT8 *CmdDescBase;
> - UINT32 CmdDescSize;
>
> if (Value == NULL) {
> return EFI_INVALID_PARAMETER;
> @@ -926,67 +1025,14 @@ UfsRwFlags (
> }
> }
>
> - Packet.DescId = FlagId;
> - Packet.Index = 0;
> - Packet.Selector = 0;
> - Packet.Timeout = UFS_TIMEOUT;
> + Packet.OutDataBuffer = (VOID *)Value;
Please use "Packet.OutDataBuffer" only for the write case, use "Packet.InDataBuffer" for read case.
Also, I suggest to set OutTransferLength and InTransferLength to 0 respectively.
> + Packet.DescId = FlagId;
> + Packet.Index = 0;
> + Packet.Selector = 0;
> + Packet.Timeout = UFS_TIMEOUT;
>
> - //
> - // Find out which slot of transfer request list is available.
> - //
> - Status = UfsFindAvailableSlotInTrl (Private, &Slot);
> - if (EFI_ERROR (Status)) {
> - return Status;
> - }
> -
> - //
> - // Fill transfer request descriptor to this slot.
> - //
> - Trd = ((UTP_TRD *)Private->UtpTrlBase) + Slot;
> - Status = UfsCreateDMCommandDesc (Private, &Packet, Trd);
> - if (EFI_ERROR (Status)) {
> - return Status;
> - }
> -
> - //
> - // Check the transfer request result.
> - //
> - CmdDescBase = (UINT8 *)(UINTN)(LShiftU64 ((UINT64)Trd->UcdBaU, 32) |
> LShiftU64 ((UINT64)Trd->UcdBa, 7));
> - QueryResp = (UTP_QUERY_RESP_UPIU *)(CmdDescBase + Trd->RuO *
> sizeof (UINT32));
> - CmdDescSize = Trd->RuO * sizeof (UINT32) + Trd->RuL * sizeof (UINT32);
> -
> - //
> - // Start to execute the transfer request.
> - //
> - UfsStartExecCmd (Private, Slot);
> -
> - //
> - // Wait for the completion of the transfer request.
> - //
> - Address = Private->UfsHcBase + UFS_HC_UTRLDBR_OFFSET;
> - Status = UfsWaitMemSet (Address, BIT0 << Slot, 0, Packet.Timeout);
> - if (EFI_ERROR (Status)) {
> - goto Exit;
> - }
> -
> - if (QueryResp->QueryResp != 0) {
> - DumpQueryResponseResult (QueryResp->QueryResp);
> - Status = EFI_DEVICE_ERROR;
> - goto Exit;
> - }
> -
> - if (Trd->Ocs == 0) {
> - //
> - // The 'FLAG VALUE' field is at byte offset 3 of QueryResp->Tsf.Value
> - //
> - *Value = *((UINT8 *)&(QueryResp->Tsf.Value) + 3);
> - } else {
> - Status = EFI_DEVICE_ERROR;
> - }
> -
> -Exit:
> - UfsStopExecCmd (Private, Slot);
> - UfsPeimFreeMem (Private->Pool, CmdDescBase, CmdDescSize);
> + Status = UfsSendDmRequest (Private, &Packet);
> + *Value = *((UINT8 *)(Packet.OutDataBuffer));
Similar comment as above, please use "Packet.InDataBuffer" for read case.
Best Regards,
Hao Wu
>
> return Status;
> }
> --
> 2.31.1.windows.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v6 1/3] MdeModulePkg: Replace with UFS_UNIT_DESC to fix timeout problem
2021-12-23 4:19 ` [PATCH v6 1/3] MdeModulePkg: Replace with UFS_UNIT_DESC to fix timeout problem VincentX Ke
@ 2021-12-23 5:44 ` Wu, Hao A
0 siblings, 0 replies; 7+ messages in thread
From: Wu, Hao A @ 2021-12-23 5:44 UTC (permalink / raw)
To: Ke, VincentX, devel@edk2.groups.io; +Cc: Ni, Ray, Chiu, Ian, Chu, Maggie
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Best Regards,
Hao Wu
> -----Original Message-----
> From: Ke, VincentX <vincentx.ke@intel.com>
> Sent: Thursday, December 23, 2021 12:19 PM
> To: devel@edk2.groups.io
> Cc: Ke, VincentX <vincentx.ke@intel.com>; Wu, Hao A
> <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Chiu, Ian
> <Ian.chiu@intel.com>; Chu, Maggie <maggie.chu@intel.com>
> Subject: [PATCH v6 1/3] MdeModulePkg: Replace with UFS_UNIT_DESC to fix
> timeout problem
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3714
>
> Replace with UFS_UNIT_DESC to fix response timeout problem.
>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Ian Chiu <Ian.chiu@intel.com>
> Cc: Maggie Chu <maggie.chu@intel.com>
> Signed-off-by: VincentX Ke <vincentx.ke@intel.com>
> ---
> .../Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c | 23 +++++++++----------
> 1 file changed, 11 insertions(+), 12 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
> b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
> index b331c0f3e3..b8651ff998 100644
> --- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
> +++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c
> @@ -1,6 +1,6 @@
> /** @file
>
> - Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2014 - 2021, Intel Corporation. All rights
> + reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -1037,9 +1037,9 @@ InitializeUfsBlockIoPeim (
> UFS_PEIM_HC_PRIVATE_DATA *Private;
> EDKII_UFS_HOST_CONTROLLER_PPI *UfsHcPpi;
> UINT32 Index;
> - UFS_CONFIG_DESC Config;
> UINTN MmioBase;
> UINT8 Controller;
> + UFS_UNIT_DESC UnitDescriptor;
>
> //
> // Shadow this PEIM to run from memory @@ -1126,19 +1126,18 @@
> InitializeUfsBlockIoPeim (
> }
>
> //
> - // Get Ufs Device's Lun Info by reading Configuration Descriptor.
> + // Check if 8 common luns are active and set corresponding bit mask.
> //
> - Status = UfsRwDeviceDesc (Private, TRUE, UfsConfigDesc, 0, 0, &Config,
> sizeof (UFS_CONFIG_DESC));
> - if (EFI_ERROR (Status)) {
> - DEBUG ((DEBUG_ERROR, "Ufs Get Configuration Descriptor Error, Status
> = %r\n", Status));
> - Controller++;
> - continue;
> - }
> -
> for (Index = 0; Index < UFS_PEIM_MAX_LUNS; Index++) {
> - if (Config.UnitDescConfParams[Index].LunEn != 0) {
> - Private->Luns.BitMask |= (BIT0 << Index);
> + Status = UfsRwDeviceDesc (Private, TRUE, UfsUnitDesc, (UINT8)Index, 0,
> &UnitDescriptor, sizeof (UFS_UNIT_DESC));
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "Fail to read UFS Unit Descriptor, Index = %X,
> Status = %r\n", Index, Status));
> + continue;
> + }
> +
> + if (UnitDescriptor.LunEn == 0x1) {
> DEBUG ((DEBUG_INFO, "Ufs %d Lun %d is enabled\n", Controller,
> Index));
> + Private->Luns.BitMask |= (BIT0 << Index);
> }
> }
>
> --
> 2.31.1.windows.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH v6 3/3] MdeModulePkg: Put off UFS HCS.DP checking to fix timing problem
2021-12-23 4:19 ` [PATCH v6 3/3] MdeModulePkg: Put off UFS HCS.DP checking to " VincentX Ke
@ 2021-12-23 5:44 ` Wu, Hao A
0 siblings, 0 replies; 7+ messages in thread
From: Wu, Hao A @ 2021-12-23 5:44 UTC (permalink / raw)
To: devel@edk2.groups.io, Ke, VincentX; +Cc: Ni, Ray, Chiu, Ian, Chu, Maggie
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Best Regards,
Hao Wu
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> VincentX Ke
> Sent: Thursday, December 23, 2021 12:19 PM
> To: devel@edk2.groups.io
> Cc: Ke, VincentX <vincentx.ke@intel.com>; Wu, Hao A
> <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Chiu, Ian
> <Ian.chiu@intel.com>; Chu, Maggie <maggie.chu@intel.com>
> Subject: [edk2-devel] [PATCH v6 3/3] MdeModulePkg: Put off UFS HCS.DP
> checking to fix timing problem
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3776
>
> Put off UFS HCS.DP (Device Attached) checking until UfsDeviceDetection() to
> fix timing problem.
>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Ian Chiu <Ian.chiu@intel.com>
> Cc: Maggie Chu <maggie.chu@intel.com>
> Signed-off-by: VincentX Ke <vincentx.ke@intel.com>
> ---
> MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c | 47 +++++++++------------
> 1 file changed, 19 insertions(+), 28 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
> b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
> index cffe8e02a7..864bf6928d 100644
> --- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
> +++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c
> @@ -1353,23 +1353,6 @@ UfsExecUicCommands (
> }
> }
>
> - //
> - // Check value of HCS.DP and make sure that there is a device attached to
> the Link.
> - //
> - Address = UfsHcBase + UFS_HC_STATUS_OFFSET;
> - Data = MmioRead32 (Address);
> - if ((Data & UFS_HC_HCS_DP) == 0) {
> - Address = UfsHcBase + UFS_HC_IS_OFFSET;
> - Status = UfsWaitMemSet (Address, UFS_HC_IS_ULSS, UFS_HC_IS_ULSS,
> UFS_TIMEOUT);
> - if (EFI_ERROR (Status)) {
> - return EFI_DEVICE_ERROR;
> - }
> -
> - return EFI_NOT_FOUND;
> - }
> -
> - DEBUG ((DEBUG_INFO, "UfsblockioPei: found a attached UFS device\n"));
> -
> return EFI_SUCCESS;
> }
>
> @@ -1443,6 +1426,8 @@ UfsDeviceDetection (
> )
> {
> UINTN Retry;
> + UINTN Address;
> + UINT32 Data;
> EFI_STATUS Status;
>
> //
> @@ -1451,22 +1436,28 @@ UfsDeviceDetection (
> //
> for (Retry = 0; Retry < 3; Retry++) {
> Status = UfsExecUicCommands (Private, UfsUicDmeLinkStartup, 0, 0, 0);
> - if (!EFI_ERROR (Status)) {
> - break;
> + if (EFI_ERROR (Status)) {
> + return EFI_DEVICE_ERROR;
> }
>
> - if (Status == EFI_NOT_FOUND) {
> - continue;
> + //
> + // Check value of HCS.DP and make sure that there is a device attached to
> the Link
> + //
> + Address = Private->UfsHcBase + UFS_HC_STATUS_OFFSET;
> + Data = MmioRead32 (Address);
> + if ((Data & UFS_HC_HCS_DP) == 0) {
> + Address = Private->UfsHcBase + UFS_HC_IS_OFFSET;
> + Status = UfsWaitMemSet (Address, UFS_HC_IS_ULSS, UFS_HC_IS_ULSS,
> UFS_TIMEOUT);
> + if (EFI_ERROR (Status)) {
> + return EFI_DEVICE_ERROR;
> + }
> + } else {
> + DEBUG ((DEBUG_INFO, "UfsblockioPei: found a attached UFS
> device\n"));
> + return EFI_SUCCESS;
> }
> -
> - return EFI_DEVICE_ERROR;
> }
>
> - if (Retry == 3) {
> - return EFI_NOT_FOUND;
> - }
> -
> - return EFI_SUCCESS;
> + return EFI_NOT_FOUND;
> }
>
> /**
> --
> 2.31.1.windows.1
>
>
>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-12-23 5:44 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-12-23 4:19 [PATCH v6 0/3] MdeModulePkg: Fix UfsBlockIoPei timing problem VincentX Ke
2021-12-23 4:19 ` [PATCH v6 1/3] MdeModulePkg: Replace with UFS_UNIT_DESC to fix timeout problem VincentX Ke
2021-12-23 5:44 ` Wu, Hao A
2021-12-23 4:19 ` [PATCH v6 2/3] MdeModulePkg: Refactoring UFS DME request and fix timing problem VincentX Ke
2021-12-23 5:42 ` Wu, Hao A
2021-12-23 4:19 ` [PATCH v6 3/3] MdeModulePkg: Put off UFS HCS.DP checking to " VincentX Ke
2021-12-23 5:44 ` [edk2-devel] " Wu, Hao A
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox