MdeModulePkg/SdMmcPciHcDxe: Send SEND_STATUS at lower frequency

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

To avoid stability issues on some designs the driver
will now send SEND_STATUS at previous, lower, frequency
when upgrading the bus timing.

Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Marcin Wojtas <mw@semihalf.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Cc: Liming Gao <liming.gao@intel.com>

Signed-off-by: Mateusz Albecki <mateusz.albecki@intel.com>
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>

MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c

@@ -558,6 +558,43 @@ EmmcTuningClkForHs200 (
   return EFI_DEVICE_ERROR;
 }
 
+/**
+  Check the SWITCH operation status.
+
+  @param[in] PassThru  A pointer to the EFI_SD_MMC_PASS_THRU_PROTOCOL instance.
+  @param[in] Slot      The slot number on which command should be sent.
+  @param[in] Rca       The relative device address.
+
+  @retval EFI_SUCCESS  The SWITCH finished siccessfully.
+  @retval others       The SWITCH failed.
+**/
+EFI_STATUS
+EmmcCheckSwitchStatus (
+  IN EFI_SD_MMC_PASS_THRU_PROTOCOL  *PassThru,
+  IN UINT8                          Slot,
+  IN UINT16                         Rca
+  )
+{
+  EFI_STATUS  Status;
+  UINT32      DevStatus;
+
+  Status = EmmcSendStatus (PassThru, Slot, Rca, &DevStatus);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "EmmcCheckSwitchStatus: Send status fails with %r\n", Status));
+    return Status;
+  }
+
+  //
+  // Check the switch operation is really successful or not.
+  //
+  if ((DevStatus & BIT7) != 0) {
+    DEBUG ((DEBUG_ERROR, "EmmcCheckSwitchStatus: The switch operation fails as DevStatus is 0x%08x\n", DevStatus));
+    return EFI_DEVICE_ERROR;
+  }
+
+  return EFI_SUCCESS;
+}
+
 /**
   Switch the bus width to specified width.
 
@@ -591,7 +628,6 @@ EmmcSwitchBusWidth (
   UINT8               Index;
   UINT8               Value;
   UINT8               CmdSet;
-  UINT32              DevStatus;
 
   //
   // Write Byte, the Value field is written into the byte pointed by Index.
@@ -617,18 +653,10 @@ EmmcSwitchBusWidth (
     return Status;
   }
 
-  Status = EmmcSendStatus (PassThru, Slot, Rca, &DevStatus);
+  Status = EmmcCheckSwitchStatus (PassThru, Slot, Rca);
   if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "EmmcSwitchBusWidth: Send status fails with %r\n", Status));
     return Status;
   }
-  //
-  // Check the switch operation is really successful or not.
-  //
-  if ((DevStatus & BIT7) != 0) {
-    DEBUG ((DEBUG_ERROR, "EmmcSwitchBusWidth: The switch operation fails as DevStatus is 0x%08x\n", DevStatus));
-    return EFI_DEVICE_ERROR;
-  }
 
   Status = SdMmcHcSetBusWidth (PciIo, Slot, BusWidth);
 
@@ -669,9 +697,9 @@ EmmcSwitchBusTiming (
   UINT8                     Index;
   UINT8                     Value;
   UINT8                     CmdSet;
-  UINT32                    DevStatus;
   SD_MMC_HC_PRIVATE_DATA    *Private;
   UINT8                     HostCtrl1;
+  BOOLEAN                   DelaySendStatus;
 
   Private = SD_MMC_HC_PRIVATE_FROM_THIS (PassThru);
   //
@@ -695,7 +723,7 @@ EmmcSwitchBusTiming (
       Value = 0;
       break;
     default:
-      DEBUG ((DEBUG_ERROR, "EmmcSwitchBusTiming: Unsupported BusTiming(%d\n)", BusTiming));
+      DEBUG ((DEBUG_ERROR, "EmmcSwitchBusTiming: Unsupported BusTiming(%d)\n", BusTiming));
       return EFI_INVALID_PARAMETER;
   }
 
@@ -724,6 +752,26 @@ EmmcSwitchBusTiming (
     return Status;
   }
 
+  //
+  // For cases when we switch bus timing to higher mode from current we want to
+  // send SEND_STATUS at current, lower, frequency then the target frequency to avoid
+  // stability issues. It has been observed that some designs are unable to process the
+  // SEND_STATUS at higher frequency during switch to HS200 @200MHz irrespective of the number of retries
+  // and only running the clock tuning is able to make them work at target frequency.
+  //
+  // For cases when we are downgrading the frequency and current high frequency is invalid
+  // we have to first change the frequency to target frequency and then send the SEND_STATUS.
+  //
+  if (Private->Slot[Slot].CurrentFreq < (ClockFreq * 1000)) {
+    Status = EmmcCheckSwitchStatus (PassThru, Slot, Rca);
+    if (EFI_ERROR (Status)) {
+      return Status;
+    }
+    DelaySendStatus = FALSE;
+  } else {
+    DelaySendStatus = TRUE;
+  }
+
   //
   // Convert the clock freq unit from MHz to KHz.
   //
@@ -732,17 +780,11 @@ EmmcSwitchBusTiming (
     return Status;
   }
 
-  Status = EmmcSendStatus (PassThru, Slot, Rca, &DevStatus);
+  if (DelaySendStatus) {
-  if (EFI_ERROR (Status)) {
+    Status = EmmcCheckSwitchStatus (PassThru, Slot, Rca);
-    DEBUG ((DEBUG_ERROR, "EmmcSwitchBusTiming: Send status fails with %r\n", Status));
+    if (EFI_ERROR (Status)) {
-    return Status;
+      return Status;
-  }
+    }
-  //
-  // Check the switch operation is really successful or not.
-  //
-  if ((DevStatus & BIT7) != 0) {
-    DEBUG ((DEBUG_ERROR, "EmmcSwitchBusTiming: The switch operation fails as DevStatus is 0x%08x\n", DevStatus));
-    return EFI_DEVICE_ERROR;
   }
 
   return Status;

MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c

@@ -28,7 +28,7 @@ EFI_DRIVER_BINDING_PROTOCOL gSdMmcPciHcDriverBinding = {
   NULL
 };
 
-#define SLOT_INIT_TEMPLATE {0, UnknownSlot, 0, 0, 0, \
+#define SLOT_INIT_TEMPLATE {0, UnknownSlot, 0, 0, 0, 0, \
                                {EDKII_SD_MMC_BUS_WIDTH_IGNORE,\
                                EDKII_SD_MMC_CLOCK_FREQ_IGNORE,\
                                {EDKII_SD_MMC_DRIVER_STRENGTH_IGNORE}}}

MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h

@@ -83,6 +83,7 @@ typedef struct {
   BOOLEAN                            MediaPresent;
   BOOLEAN                            Initialized;
   SD_MMC_CARD_TYPE                   CardType;
+  UINT64                             CurrentFreq;
   EDKII_SD_MMC_OPERATING_PARAMETERS  OperatingParameters;
 } SD_MMC_HC_SLOT;
 

MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c

@@ -931,6 +931,8 @@ SdMmcHcClockSupply (
     }
   }
 
+  Private->Slot[Slot].CurrentFreq = ClockFreq;
+
   return Status;
 }