[PATCH 1/3] MdeModulePkg/UfsPassThruDxe: Improve Device initialization polling Loop

[PATCH 1/3] MdeModulePkg/UfsPassThruDxe: Improve Device initialization polling Loop

[Purna Chandra Rao Bandaru]

https://bugzilla.tianocore.org/show_bug.cgi?id=3217

Current Ufs Pass thru driver polls for 5us and return success even when
the timeout occurs.
There are cards that can take upto 600ms for Init and hence increased
the time out for fDeviceInit polling loop.

Signed-off-by: Bandaru <purna.chandra.rao.bandaru@intel.com>
Cc: Mateusz Albecki <mateusz.albecki@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>

Change-Id: I6cb063b43bdf37790db8e60c3919153cd2f3c086
---
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
index 9768c2e6fb..8859578af3 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.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>
   Copyright (c) Microsoft Corporation.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -749,7 +749,7 @@ UfsFinishDeviceInitialization (
 {
   EFI_STATUS  Status;
   UINT8  DeviceInitStatus;
-  UINT8  Timeout;
+  UINT16 Timeout;
 
   DeviceInitStatus = 0xFF;
 
@@ -761,17 +761,23 @@ UfsFinishDeviceInitialization (
     return Status;
   }
 
-  Timeout = 5;
+  Timeout = 6000; //There are cards that can take upto 600ms.
   do {
+    MicroSecondDelay (100); //Give 100 us and then start polling.
     Status = UfsReadFlag (Private, UfsFlagDevInit, &DeviceInitStatus);
     if (EFI_ERROR (Status)) {
       return Status;
     }
-    MicroSecondDelay (1);
     Timeout--;
   } while (DeviceInitStatus != 0 && Timeout != 0);
 
-  return EFI_SUCCESS;
+  if (Timeout == 0) {
+    DEBUG ((DEBUG_ERROR, "UfsFinishDeviceInitialization DeviceInitStatus=%x EFI_TIMEOUT \n", DeviceInitStatus));
+    return EFI_TIMEOUT;
+  } else {
+    DEBUG ((DEBUG_INFO, "UfsFinishDeviceInitialization Timeout left=%x EFI_SUCCESS \n", Timeout));
+    return EFI_SUCCESS;
+  }
 }
 
 /**
-- 
2.16.2.windows.1

[PATCH 2/3] MdeModulePkg/UfsPassThruDxe: Improve Error handling of Ufs Pass Thru driver

[Purna Chandra Rao Bandaru]

https://bugzilla.tianocore.org/show_bug.cgi?id=3217

Add UFS host conctroller reset in the last retry of the Link start up
to allow scope for recovery of the Link.

Signed-off-by: Bandaru <purna.chandra.rao.bandaru@intel.com>
Cc: Mateusz Albecki <mateusz.albecki@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>

Change-Id: Iecc87be51c557ec07890e2d700d6912ddf16adbd
---
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 45 +++++++++++++++++++++++++--------------------
 1 file changed, 25 insertions(+), 20 deletions(-)

diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
index 0b1030ab47..e1416696d4 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
@@ -2,7 +2,7 @@
   UfsPassThruDxe driver is used to produce EFI_EXT_SCSI_PASS_THRU protocol interface
   for upper layer application to execute UFS-supported SCSI cmds.
 
-  Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2014 - 2021, Intel Corporation. All rights reserved.<BR>
   Copyright (c) Microsoft Corporation.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -1929,37 +1929,42 @@ UfsDeviceDetection (
 
   //
   // Start UFS device detection.
-  // Try up to 3 times for establishing data link with device.
+  // Try up to 4 times for establishing data link with device.
   //
-  for (Retry = 0; Retry < 3; Retry++) {
+  for (Retry = 0; Retry < 4; Retry++) {
     LinkStartupCommand.Opcode = UfsUicDmeLinkStartup;
     LinkStartupCommand.Arg1 = 0;
     LinkStartupCommand.Arg2 = 0;
     LinkStartupCommand.Arg3 = 0;
     Status = UfsExecUicCommands (Private, &LinkStartupCommand);
-    if (EFI_ERROR (Status)) {
-      return EFI_DEVICE_ERROR;
-    }
-
-    Status = UfsMmioRead32 (Private, UFS_HC_STATUS_OFFSET, &Data);
-    if (EFI_ERROR (Status)) {
-      return EFI_DEVICE_ERROR;
-    }
-
-    if ((Data & UFS_HC_HCS_DP) == 0) {
-      Status = UfsWaitMemSet (Private, UFS_HC_IS_OFFSET, UFS_HC_IS_ULSS, UFS_HC_IS_ULSS, UFS_TIMEOUT);
+    if (!EFI_ERROR (Status)) {
+      Status = UfsMmioRead32 (Private, UFS_HC_STATUS_OFFSET, &Data);
       if (EFI_ERROR (Status)) {
         return EFI_DEVICE_ERROR;
       }
-    } else {
-      if (mUfsHcPlatform != NULL && mUfsHcPlatform->Callback != NULL) {
-        Status = mUfsHcPlatform->Callback (Private->Handle, EdkiiUfsHcPostLinkStartup, &Private->UfsHcDriverInterface);
+
+      if ((Data & UFS_HC_HCS_DP) == 0) {
+        Status = UfsWaitMemSet (Private, UFS_HC_IS_OFFSET, UFS_HC_IS_ULSS, UFS_HC_IS_ULSS, UFS_TIMEOUT);
         if (EFI_ERROR (Status)) {
-          DEBUG ((DEBUG_ERROR, "Failure from platform driver during EdkiiUfsHcPostLinkStartup, Status = %r\n", Status));
-          return Status;
+          return EFI_DEVICE_ERROR;
+        }
+      } else {
+        if (mUfsHcPlatform != NULL && mUfsHcPlatform->Callback != NULL) {
+          Status = mUfsHcPlatform->Callback (Private->Handle, EdkiiUfsHcPostLinkStartup, &Private->UfsHcDriverInterface);
+          if (EFI_ERROR (Status)) {
+            DEBUG ((DEBUG_ERROR, "Failure from platform driver during EdkiiUfsHcPostLinkStartup, Status = %r\n", Status));
+            return Status;
+          }
         }
+        return EFI_SUCCESS;
+      }
+    }
+    if (Retry == 2) {
+      Status = UfsEnableHostController (Private);
+      if (EFI_ERROR (Status)) {
+        DEBUG ((DEBUG_ERROR, "UfsDeviceDetection: Enable Host Controller Fails, Status = %r\n", Status));
+        return Status;
       }
-      return EFI_SUCCESS;
     }
   }
 
-- 
2.16.2.windows.1

[PATCH 3/3] MdeModulePkg/UfsPassThruDxe: Improve UFS device Readiness check.

[Purna Chandra Rao Bandaru]

https://bugzilla.tianocore.org/show_bug.cgi?id=3217

Retry sending NOP OUT command upto 10 times. This to give extra time for
UFS device to respond if was busy.

Signed-off-by: Bandaru <purna.chandra.rao.bandaru@intel.com>
Cc: Mateusz Albecki <mateusz.albecki@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>

Change-Id: I46054ea9ee34ad295fce58dff1756241fd22e17c
---
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
index 8859578af3..c4c2b03e46 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
@@ -911,9 +911,19 @@ UfsPassThruDriverBindingStart (
   // At the end of the UFS Interconnect Layer initialization on both host and device side,
   // the host shall send a NOP OUT UPIU to verify that the device UTP Layer is ready.
   //
-  Status = UfsExecNopCmds (Private);
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "Ufs Sending NOP IN command Error, Status = %r\n", Status));
+  for (Index = 10; Index > 0; Index--) {
+    Status = UfsExecNopCmds (Private);
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_ERROR, "Ufs Sending NOP IN command Error, Index = %x Status = %r\n", Index, Status));
+      MicroSecondDelay (100); //100 us
+      continue;
+    } else {
+      DEBUG ((DEBUG_INFO, "Ufs Sent NOP OUT successfully and received NOP IN, Status = %r\n", Status));
+      break;
+    }
+  }
+  if (!Index) {
+    DEBUG ((DEBUG_INFO, "NOP OUT failed all the 10 times Status = %r\n", Status));
     goto Error;
   }
 
-- 
2.16.2.windows.1

Re: [edk2-devel] [PATCH 1/3] MdeModulePkg/UfsPassThruDxe: Improve Device initialization polling Loop

[Laszlo Ersek]

Bandaru,

can you tell us please why you (apparently) refuse to subscribe to this
list -- in spite of the two invites that I have sent you from the
groups.io moderation WebUI?

You are creating unnecessary work for the moderators. They need to
approve your messages one by one, until you actually subscribe to the
list. It's only then that moderators can add you to the permanent
permit-list (i.e., permanently unmoderate you).

I've stopped approving your messages because you silently ignored
(apparently!) both invites that I generated for you. The latest of those
was sent on Feb 17. And that doesn't take into account any other invites
that other moderators may have sent you.

You are abusing the moderation system. Stop it. You are clearly a
recurring contributor, so subscribe already.

It's bad that I'm telling you this here, on the list, under your patch
set. However I also included a similarly detailed explanation/request
when I rejected your pending messages on Feb 17 (just before I would
send you my 2nd invite). You (apparently) completely ignored the
contents of that rejection message, you wouldn't subscribe, you have
just kept posting new patches since. I can't fathom your behavior.

In the same timeframe, I did the same to another contributor (rejected
their pending messages with a detailed explanation / request, and sent
them a 2nd invite too), and they *have* since subscribed. And now I
un-moderated them as well, ~5 minutes ago.

Do you actually *read* the emails that you receive? Or is your spam
filter mistrained?

Laszlo

On 02/22/21 18:02, Purna Chandra Rao Bandaru wrote:
> https://bugzilla.tianocore.org/show_bug.cgi?id=3217
> 
> Current Ufs Pass thru driver polls for 5us and return success even when
> the timeout occurs.
> There are cards that can take upto 600ms for Init and hence increased
> the time out for fDeviceInit polling loop.
> 
> Signed-off-by: Bandaru <purna.chandra.rao.bandaru@intel.com>
> Cc: Mateusz Albecki <mateusz.albecki@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> 
> Change-Id: I6cb063b43bdf37790db8e60c3919153cd2f3c086
> ---
>  MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> index 9768c2e6fb..8859578af3 100644
> --- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> +++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.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>
>    Copyright (c) Microsoft Corporation.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>  
> @@ -749,7 +749,7 @@ UfsFinishDeviceInitialization (
>  {
>    EFI_STATUS  Status;
>    UINT8  DeviceInitStatus;
> -  UINT8  Timeout;
> +  UINT16 Timeout;
>  
>    DeviceInitStatus = 0xFF;
>  
> @@ -761,17 +761,23 @@ UfsFinishDeviceInitialization (
>      return Status;
>    }
>  
> -  Timeout = 5;
> +  Timeout = 6000; //There are cards that can take upto 600ms.
>    do {
> +    MicroSecondDelay (100); //Give 100 us and then start polling.
>      Status = UfsReadFlag (Private, UfsFlagDevInit, &DeviceInitStatus);
>      if (EFI_ERROR (Status)) {
>        return Status;
>      }
> -    MicroSecondDelay (1);
>      Timeout--;
>    } while (DeviceInitStatus != 0 && Timeout != 0);
>  
> -  return EFI_SUCCESS;
> +  if (Timeout == 0) {
> +    DEBUG ((DEBUG_ERROR, "UfsFinishDeviceInitialization DeviceInitStatus=%x EFI_TIMEOUT \n", DeviceInitStatus));
> +    return EFI_TIMEOUT;
> +  } else {
> +    DEBUG ((DEBUG_INFO, "UfsFinishDeviceInitialization Timeout left=%x EFI_SUCCESS \n", Timeout));
> +    return EFI_SUCCESS;
> +  }
>  }
>  
>  /**
> 

Re: [edk2-devel] [PATCH 1/3] MdeModulePkg/UfsPassThruDxe: Improve Device initialization polling Loop

[purna.chandra.rao.bandaru]

Hi Laszlo

1) First of all Sorry for the inconvenience caused and I accept there was a mistake at my end as the email rules routed this email to one of the folders. 
2) Thank you for mentioning that this email was sent with group name and hence I have searched and have joined the  group. 
3) Also sorry for not knowing that moderator need to approve one by one due to this. 
I was only referring following as I was new to contribute, probably I missing more resources. Anyways, fine I will sort out this.
https://wiki.ith.intel.com/display/ITSFID/Downstream+Dev+Process
https://wiki.ith.intel.com/display/ITSFID/EDK+II+Development+Process

Thanks
~Purna

-----Original Message-----
From: Laszlo Ersek <lersek@redhat.com> 
Sent: Tuesday, February 23, 2021 11:20 PM
To: devel@edk2.groups.io; Bandaru, Purna Chandra Rao <purna.chandra.rao.bandaru@intel.com>
Cc: Albecki, Mateusz <mateusz.albecki@intel.com>; Ni, Ray <ray.ni@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Leif Lindholm (Nuvia address) <leif@nuviainc.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Andrew Fish <afish@apple.com>
Subject: Re: [edk2-devel] [PATCH 1/3] MdeModulePkg/UfsPassThruDxe: Improve Device initialization polling Loop

Bandaru,

can you tell us please why you (apparently) refuse to subscribe to this list -- in spite of the two invites that I have sent you from the groups.io moderation WebUI?

You are creating unnecessary work for the moderators. They need to approve your messages one by one, until you actually subscribe to the list. It's only then that moderators can add you to the permanent permit-list (i.e., permanently unmoderate you).

I've stopped approving your messages because you silently ignored
(apparently!) both invites that I generated for you. The latest of those was sent on Feb 17. And that doesn't take into account any other invites that other moderators may have sent you.

You are abusing the moderation system. Stop it. You are clearly a recurring contributor, so subscribe already.

It's bad that I'm telling you this here, on the list, under your patch set. However I also included a similarly detailed explanation/request when I rejected your pending messages on Feb 17 (just before I would send you my 2nd invite). You (apparently) completely ignored the contents of that rejection message, you wouldn't subscribe, you have just kept posting new patches since. I can't fathom your behavior.

In the same timeframe, I did the same to another contributor (rejected their pending messages with a detailed explanation / request, and sent them a 2nd invite too), and they *have* since subscribed. And now I un-moderated them as well, ~5 minutes ago.

Do you actually *read* the emails that you receive? Or is your spam filter mistrained?

Laszlo

On 02/22/21 18:02, Purna Chandra Rao Bandaru wrote:
> https://bugzilla.tianocore.org/show_bug.cgi?id=3217
> 
> Current Ufs Pass thru driver polls for 5us and return success even 
> when the timeout occurs.
> There are cards that can take upto 600ms for Init and hence increased 
> the time out for fDeviceInit polling loop.
> 
> Signed-off-by: Bandaru <purna.chandra.rao.bandaru@intel.com>
> Cc: Mateusz Albecki <mateusz.albecki@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> 
> Change-Id: I6cb063b43bdf37790db8e60c3919153cd2f3c086
> ---
>  MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c | 16 
> +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c 
> b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> index 9768c2e6fb..8859578af3 100644
> --- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> +++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.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>
>    Copyright (c) Microsoft Corporation.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>  
> @@ -749,7 +749,7 @@ UfsFinishDeviceInitialization (  {
>    EFI_STATUS  Status;
>    UINT8  DeviceInitStatus;
> -  UINT8  Timeout;
> +  UINT16 Timeout;
>  
>    DeviceInitStatus = 0xFF;
>  
> @@ -761,17 +761,23 @@ UfsFinishDeviceInitialization (
>      return Status;
>    }
>  
> -  Timeout = 5;
> +  Timeout = 6000; //There are cards that can take upto 600ms.
>    do {
> +    MicroSecondDelay (100); //Give 100 us and then start polling.
>      Status = UfsReadFlag (Private, UfsFlagDevInit, &DeviceInitStatus);
>      if (EFI_ERROR (Status)) {
>        return Status;
>      }
> -    MicroSecondDelay (1);
>      Timeout--;
>    } while (DeviceInitStatus != 0 && Timeout != 0);
>  
> -  return EFI_SUCCESS;
> +  if (Timeout == 0) {
> +    DEBUG ((DEBUG_ERROR, "UfsFinishDeviceInitialization DeviceInitStatus=%x EFI_TIMEOUT \n", DeviceInitStatus));
> +    return EFI_TIMEOUT;
> +  } else {
> +    DEBUG ((DEBUG_INFO, "UfsFinishDeviceInitialization Timeout left=%x EFI_SUCCESS \n", Timeout));
> +    return EFI_SUCCESS;
> +  }
>  }
>  
>  /**
>