[edk2] [patch] MdeModulePkg/SdMmc: Add break to avoid dead loop when polling OCR Reg

Feng Tian posted 1 patch 7 years, 1 month ago
Failed in applying to current master (apply log)
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 9 ++++++++-
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c   | 8 ++++++++
MdeModulePkg/Bus/Sd/EmmcBlockIoPei/EmmcHci.c    | 9 ++++++++-
MdeModulePkg/Bus/Sd/SdBlockIoPei/SdHci.c        | 9 ++++++++-
4 files changed, 32 insertions(+), 3 deletions(-)
[edk2] [patch] MdeModulePkg/SdMmc: Add break to avoid dead loop when polling OCR Reg
Posted by Feng Tian 7 years, 1 month ago
At worst case, OCR register may always not set BIT31. It will cause
original code enter to dead loop. Adding a break for such case.

Cc: Hao Wu <hao.a.wu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Feng Tian <feng.tian@intel.com>
---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 9 ++++++++-
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c   | 8 ++++++++
 MdeModulePkg/Bus/Sd/EmmcBlockIoPei/EmmcHci.c    | 9 ++++++++-
 MdeModulePkg/Bus/Sd/SdBlockIoPei/SdHci.c        | 9 ++++++++-
 4 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
index 9dbec10..6653a87 100755
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
@@ -1109,6 +1109,7 @@ EmmcIdentification (
   EFI_SD_MMC_PASS_THRU_PROTOCOL  *PassThru;
   UINT32                         Ocr;
   UINT16                         Rca;
+  UINTN                          Retry;
 
   PciIo    = Private->PciIo;
   PassThru = &Private->PassThru;
@@ -1119,7 +1120,8 @@ EmmcIdentification (
     return Status;
   }
 
-  Ocr = 0;
+  Ocr   = 0;
+  Retry = 0;
   do {
     Status = EmmcGetOcr (PassThru, Slot, &Ocr);
     if (EFI_ERROR (Status)) {
@@ -1127,6 +1129,11 @@ EmmcIdentification (
       return Status;
     }
     Ocr |= BIT30;
+
+    if (Retry++ == 100) {
+      return EFI_DEVICE_ERROR;	
+    }
+    gBS->Stall(10 * 1000);
   } while ((Ocr & BIT31) == 0);
 
   Status = EmmcGetAllCid (PassThru, Slot);
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
index 9122848..1da0849 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
@@ -1017,6 +1017,7 @@ SdCardIdentification (
   UINT8                          PowerCtrl;
   UINT32                         PresentState;
   UINT8                          HostCtrl2;
+  UINTN                          Retry;
 
   PciIo    = Private->PciIo;
   PassThru = &Private->PassThru;
@@ -1097,12 +1098,19 @@ SdCardIdentification (
   //    Note here we only support the cards complied with SD physical
   //    layer simplified spec version 2.0 and version 3.0 and above.
   //
+  Ocr   = 0;
+  Retry = 0;
   do {
     Status = SdCardSendOpCond (PassThru, Slot, 0, Ocr, S18r, Xpc, TRUE, &Ocr);
     if (EFI_ERROR (Status)) {
       DEBUG ((DEBUG_ERROR, "SdCardIdentification: SdCardSendOpCond fails with %r Ocr %x, S18r %x, Xpc %x\n", Status, Ocr, S18r, Xpc));
       return EFI_DEVICE_ERROR;
     }
+
+    if (Retry++ == 100) {
+      return EFI_DEVICE_ERROR;	
+    }
+    gBS->Stall(10 * 1000);
   } while ((Ocr & BIT31) == 0);
 
   //
diff --git a/MdeModulePkg/Bus/Sd/EmmcBlockIoPei/EmmcHci.c b/MdeModulePkg/Bus/Sd/EmmcBlockIoPei/EmmcHci.c
index 2c0baca..5cb20a3 100644
--- a/MdeModulePkg/Bus/Sd/EmmcBlockIoPei/EmmcHci.c
+++ b/MdeModulePkg/Bus/Sd/EmmcBlockIoPei/EmmcHci.c
@@ -2827,6 +2827,7 @@ EmmcPeimIdentification (
   EFI_STATUS                     Status;
   UINT32                         Ocr;
   UINT32                         Rca;
+  UINTN                          Retry;
 
   Status = EmmcPeimReset (Slot);
   if (EFI_ERROR (Status)) {
@@ -2834,13 +2835,19 @@ EmmcPeimIdentification (
     return Status;
   }
 
-  Ocr = 0;
+  Ocr   = 0;
+  Retry = 0;
   do {
     Status = EmmcPeimGetOcr (Slot, &Ocr);
     if (EFI_ERROR (Status)) {
       DEBUG ((EFI_D_ERROR, "EmmcPeimIdentification: EmmcPeimGetOcr fails with %r\n", Status));
       return Status;
     }
+
+    if (Retry++ == 100) {
+      return EFI_DEVICE_ERROR;	
+    }
+    MicroSecondDelay (10 * 1000);
   } while ((Ocr & BIT31) == 0);
 
   Status = EmmcPeimGetAllCid (Slot);
diff --git a/MdeModulePkg/Bus/Sd/SdBlockIoPei/SdHci.c b/MdeModulePkg/Bus/Sd/SdBlockIoPei/SdHci.c
index 23e6563..81076ba 100644
--- a/MdeModulePkg/Bus/Sd/SdBlockIoPei/SdHci.c
+++ b/MdeModulePkg/Bus/Sd/SdBlockIoPei/SdHci.c
@@ -2754,7 +2754,7 @@ SdPeimIdentification (
   UINT32                         PresentState;
   UINT8                          HostCtrl2;
   SD_HC_SLOT_CAP                 Capability;
-
+  UINTN                          Retry;
   //
   // 1. Send Cmd0 to the device
   //
@@ -2842,12 +2842,19 @@ SdPeimIdentification (
   //    Note here we only support the cards complied with SD physical
   //    layer simplified spec version 2.0 and version 3.0 and above.
   //
+  Ocr   = 0;
+  Retry = 0;
   do {
     Status = SdPeimSendOpCond (Slot, 0, Ocr, S18r, Xpc, TRUE, &Ocr);
     if (EFI_ERROR (Status)) {
       DEBUG ((EFI_D_ERROR, "SdPeimIdentification: SdPeimSendOpCond fails with %r Ocr %x, S18r %x, Xpc %x\n", Status, Ocr, S18r, Xpc));
       return EFI_DEVICE_ERROR;
     }
+
+    if (Retry++ == 100) {
+      return EFI_DEVICE_ERROR;	
+    }
+    MicroSecondDelay (10 * 1000);
   } while ((Ocr & BIT31) == 0);
 
   //
-- 
2.7.1.windows.2

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [patch] MdeModulePkg/SdMmc: Add break to avoid dead loop when polling OCR Reg
Posted by Wu, Hao A 7 years, 1 month ago
> -----Original Message-----
> From: Tian, Feng
> Sent: Monday, March 13, 2017 11:24 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A
> Subject: [patch] MdeModulePkg/SdMmc: Add break to avoid dead loop when
> polling OCR Reg
> 
> At worst case, OCR register may always not set BIT31. It will cause
> original code enter to dead loop. Adding a break for such case.
> 
> Cc: Hao Wu <hao.a.wu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Feng Tian <feng.tian@intel.com>
> ---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 9 ++++++++-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c   | 8 ++++++++
>  MdeModulePkg/Bus/Sd/EmmcBlockIoPei/EmmcHci.c    | 9 ++++++++-
>  MdeModulePkg/Bus/Sd/SdBlockIoPei/SdHci.c        | 9 ++++++++-
>  4 files changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> index 9dbec10..6653a87 100755
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> @@ -1109,6 +1109,7 @@ EmmcIdentification (
>    EFI_SD_MMC_PASS_THRU_PROTOCOL  *PassThru;
>    UINT32                         Ocr;
>    UINT16                         Rca;
> +  UINTN                          Retry;
> 
>    PciIo    = Private->PciIo;
>    PassThru = &Private->PassThru;
> @@ -1119,7 +1120,8 @@ EmmcIdentification (
>      return Status;
>    }
> 
> -  Ocr = 0;
> +  Ocr   = 0;
> +  Retry = 0;
>    do {
>      Status = EmmcGetOcr (PassThru, Slot, &Ocr);
>      if (EFI_ERROR (Status)) {
> @@ -1127,6 +1129,11 @@ EmmcIdentification (
>        return Status;
>      }
>      Ocr |= BIT30;
> +
> +    if (Retry++ == 100) {

Hi Feng,

Maybe a debug message can be added here to indicate the type of error,
like other error handling logics in this function.

Apart from that, the patch is good to me.
Reviewed-by: Hao Wu <hao.a.wu@intel.com>

Best Regards,
Hao Wu

> +      return EFI_DEVICE_ERROR;
> +    }
> +    gBS->Stall(10 * 1000);
>    } while ((Ocr & BIT31) == 0);
> 
>    Status = EmmcGetAllCid (PassThru, Slot);
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> index 9122848..1da0849 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> @@ -1017,6 +1017,7 @@ SdCardIdentification (
>    UINT8                          PowerCtrl;
>    UINT32                         PresentState;
>    UINT8                          HostCtrl2;
> +  UINTN                          Retry;
> 
>    PciIo    = Private->PciIo;
>    PassThru = &Private->PassThru;
> @@ -1097,12 +1098,19 @@ SdCardIdentification (
>    //    Note here we only support the cards complied with SD physical
>    //    layer simplified spec version 2.0 and version 3.0 and above.
>    //
> +  Ocr   = 0;
> +  Retry = 0;
>    do {
>      Status = SdCardSendOpCond (PassThru, Slot, 0, Ocr, S18r, Xpc, TRUE, &Ocr);
>      if (EFI_ERROR (Status)) {
>        DEBUG ((DEBUG_ERROR, "SdCardIdentification: SdCardSendOpCond fails
> with %r Ocr %x, S18r %x, Xpc %x\n", Status, Ocr, S18r, Xpc));
>        return EFI_DEVICE_ERROR;
>      }
> +
> +    if (Retry++ == 100) {
> +      return EFI_DEVICE_ERROR;
> +    }
> +    gBS->Stall(10 * 1000);
>    } while ((Ocr & BIT31) == 0);
> 
>    //
> diff --git a/MdeModulePkg/Bus/Sd/EmmcBlockIoPei/EmmcHci.c
> b/MdeModulePkg/Bus/Sd/EmmcBlockIoPei/EmmcHci.c
> index 2c0baca..5cb20a3 100644
> --- a/MdeModulePkg/Bus/Sd/EmmcBlockIoPei/EmmcHci.c
> +++ b/MdeModulePkg/Bus/Sd/EmmcBlockIoPei/EmmcHci.c
> @@ -2827,6 +2827,7 @@ EmmcPeimIdentification (
>    EFI_STATUS                     Status;
>    UINT32                         Ocr;
>    UINT32                         Rca;
> +  UINTN                          Retry;
> 
>    Status = EmmcPeimReset (Slot);
>    if (EFI_ERROR (Status)) {
> @@ -2834,13 +2835,19 @@ EmmcPeimIdentification (
>      return Status;
>    }
> 
> -  Ocr = 0;
> +  Ocr   = 0;
> +  Retry = 0;
>    do {
>      Status = EmmcPeimGetOcr (Slot, &Ocr);
>      if (EFI_ERROR (Status)) {
>        DEBUG ((EFI_D_ERROR, "EmmcPeimIdentification: EmmcPeimGetOcr fails
> with %r\n", Status));
>        return Status;
>      }
> +
> +    if (Retry++ == 100) {
> +      return EFI_DEVICE_ERROR;
> +    }
> +    MicroSecondDelay (10 * 1000);
>    } while ((Ocr & BIT31) == 0);
> 
>    Status = EmmcPeimGetAllCid (Slot);
> diff --git a/MdeModulePkg/Bus/Sd/SdBlockIoPei/SdHci.c
> b/MdeModulePkg/Bus/Sd/SdBlockIoPei/SdHci.c
> index 23e6563..81076ba 100644
> --- a/MdeModulePkg/Bus/Sd/SdBlockIoPei/SdHci.c
> +++ b/MdeModulePkg/Bus/Sd/SdBlockIoPei/SdHci.c
> @@ -2754,7 +2754,7 @@ SdPeimIdentification (
>    UINT32                         PresentState;
>    UINT8                          HostCtrl2;
>    SD_HC_SLOT_CAP                 Capability;
> -
> +  UINTN                          Retry;
>    //
>    // 1. Send Cmd0 to the device
>    //
> @@ -2842,12 +2842,19 @@ SdPeimIdentification (
>    //    Note here we only support the cards complied with SD physical
>    //    layer simplified spec version 2.0 and version 3.0 and above.
>    //
> +  Ocr   = 0;
> +  Retry = 0;
>    do {
>      Status = SdPeimSendOpCond (Slot, 0, Ocr, S18r, Xpc, TRUE, &Ocr);
>      if (EFI_ERROR (Status)) {
>        DEBUG ((EFI_D_ERROR, "SdPeimIdentification: SdPeimSendOpCond fails
> with %r Ocr %x, S18r %x, Xpc %x\n", Status, Ocr, S18r, Xpc));
>        return EFI_DEVICE_ERROR;
>      }
> +
> +    if (Retry++ == 100) {
> +      return EFI_DEVICE_ERROR;
> +    }
> +    MicroSecondDelay (10 * 1000);
>    } while ((Ocr & BIT31) == 0);
> 
>    //
> --
> 2.7.1.windows.2

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