1. Patchew
  2. EDK2
  3. View series

[edk2-devel] [PATCH] MdeModulePkg/UfsPassThruDxe: Improve Error handling of Ufs Pass Thru driver

Purna Chandra Rao Bandaru posted 1 patch 3 years, 2 months ago
Failed in applying to current master (apply log)
MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c    | 26 +++++++++++++++++++++-----
MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 18 ++++++++++++------
2 files changed, 33 insertions(+), 11 deletions(-)
[edk2-devel] [PATCH] MdeModulePkg/UfsPassThruDxe: Improve Error handling of Ufs Pass Thru driver
Posted by Purna Chandra Rao Bandaru 3 years, 2 months ago 
From: Bandaru <purna.chandra.rao.bandaru@intel.com>

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

Following is the brief description of the changes
 1) There are cards that can take upto 600ms for Init and hence increase
    the time out for fDeviceInit polling loop.
 2) Add UFS host conctroller reset in the last retry of Link start up.
 3) Retry sending NOP OUT command upto 10 times

Signed-off-by: Bandaru <purna.chandra.rao.bandaru@intel.com>

Change-Id: I6c0dbc1c147487e51f0ed5f2425957ae089b0160
---
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c    | 26 +++++++++++++++++++++-----
 MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 18 ++++++++++++------
 2 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
index 9768c2e6fb..89048745be 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);
 
+  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;
+  }
 }
 
 /**
@@ -905,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.
   //
+  for (Index = 10; Index > 0; Index--) {
   Status = UfsExecNopCmds (Private);
   if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "Ufs Sending NOP IN command Error, Status = %r\n", 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;
   }
 
diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
index 0b1030ab47..4fa5689196 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,17 +1929,15 @@ 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;
-    }
+    if (!EFI_ERROR (Status)) {
 
     Status = UfsMmioRead32 (Private, UFS_HC_STATUS_OFFSET, &Data);
     if (EFI_ERROR (Status)) {
@@ -1960,6 +1958,14 @@ UfsDeviceDetection (
         }
       }
       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;
+      }
     }
   }
 
-- 
2.16.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#71609): https://edk2.groups.io/g/devel/message/71609
Mute This Topic: https://groups.io/mt/80560328/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MdeModulePkg/UfsPassThruDxe: Improve Error handling of Ufs Pass Thru driver
Posted by Laszlo Ersek 3 years, 2 months ago 
Jian, Hao, Ray -- any feedback on this?

Thanks
Laszlo

On 02/11/21 13:59, Purna Chandra Rao Bandaru wrote:
> From: Bandaru <purna.chandra.rao.bandaru@intel.com>
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=3217
> 
> Following is the brief description of the changes
>  1) There are cards that can take upto 600ms for Init and hence increase
>     the time out for fDeviceInit polling loop.
>  2) Add UFS host conctroller reset in the last retry of Link start up.
>  3) Retry sending NOP OUT command upto 10 times
> 
> Signed-off-by: Bandaru <purna.chandra.rao.bandaru@intel.com>
> 
> Change-Id: I6c0dbc1c147487e51f0ed5f2425957ae089b0160
> ---
>  MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c    | 26 +++++++++++++++++++++-----
>  MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 18 ++++++++++++------
>  2 files changed, 33 insertions(+), 11 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> index 9768c2e6fb..89048745be 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);
>  
> +  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;
> +  }
>  }
>  
>  /**
> @@ -905,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.
>    //
> +  for (Index = 10; Index > 0; Index--) {
>    Status = UfsExecNopCmds (Private);
>    if (EFI_ERROR (Status)) {
> -    DEBUG ((DEBUG_ERROR, "Ufs Sending NOP IN command Error, Status = %r\n", 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;
>    }
>  
> diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
> index 0b1030ab47..4fa5689196 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,17 +1929,15 @@ 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;
> -    }
> +    if (!EFI_ERROR (Status)) {
>  
>      Status = UfsMmioRead32 (Private, UFS_HC_STATUS_OFFSET, &Data);
>      if (EFI_ERROR (Status)) {
> @@ -1960,6 +1958,14 @@ UfsDeviceDetection (
>          }
>        }
>        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;
> +      }
>      }
>    }
>  
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#71744): https://edk2.groups.io/g/devel/message/71744
Mute This Topic: https://groups.io/mt/80560328/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MdeModulePkg/UfsPassThruDxe: Improve Error handling of Ufs Pass Thru driver
Posted by Wu, Hao A 3 years, 2 months ago 
> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Wednesday, February 17, 2021 10:11 PM
> To: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Ni, Ray <ray.ni@intel.com>
> Cc: devel@edk2.groups.io; Bandaru, Purna Chandra Rao
> <purna.chandra.rao.bandaru@intel.com>
> Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/UfsPassThruDxe: Improve
> Error handling of Ufs Pass Thru driver
> 
> Jian, Hao, Ray -- any feedback on this?


Hello Laszlo and Bandaru,

I will try to provide feedbacks for this patch once I am back to the office (early next week).
Sorry for the delayed response.

Best Regards,
Hao Wu


> 
> Thanks
> Laszlo
> 
> On 02/11/21 13:59, Purna Chandra Rao Bandaru wrote:
> > From: Bandaru <purna.chandra.rao.bandaru@intel.com>
> >
> > https://bugzilla.tianocore.org/show_bug.cgi?id=3217
> >
> > Following is the brief description of the changes
> >  1) There are cards that can take upto 600ms for Init and hence increase
> >     the time out for fDeviceInit polling loop.
> >  2) Add UFS host conctroller reset in the last retry of Link start up.
> >  3) Retry sending NOP OUT command upto 10 times
> >
> > Signed-off-by: Bandaru <purna.chandra.rao.bandaru@intel.com>
> >
> > Change-Id: I6c0dbc1c147487e51f0ed5f2425957ae089b0160
> > ---
> >  MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c    | 26
> +++++++++++++++++++++-----
> >  MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 18
> > ++++++++++++------
> >  2 files changed, 33 insertions(+), 11 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> > b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> > index 9768c2e6fb..89048745be 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);
> >
> > +  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;
> > +  }
> >  }
> >
> >  /**
> > @@ -905,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.
> >    //
> > +  for (Index = 10; Index > 0; Index--) {
> >    Status = UfsExecNopCmds (Private);
> >    if (EFI_ERROR (Status)) {
> > -    DEBUG ((DEBUG_ERROR, "Ufs Sending NOP IN command Error, Status
> = %r\n", 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;
> >    }
> >
> > diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
> > b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
> > index 0b1030ab47..4fa5689196 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,17 +1929,15 @@ 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;
> > -    }
> > +    if (!EFI_ERROR (Status)) {
> >
> >      Status = UfsMmioRead32 (Private, UFS_HC_STATUS_OFFSET, &Data);
> >      if (EFI_ERROR (Status)) {
> > @@ -1960,6 +1958,14 @@ UfsDeviceDetection (
> >          }
> >        }
> >        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;
> > +      }
> >      }
> >    }
> >
> >



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#71760): https://edk2.groups.io/g/devel/message/71760
Mute This Topic: https://groups.io/mt/80560328/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.