<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div style="font-family: Consolas, Courier, monospace; font-size: 12pt; color: rgb(0, 0, 0);">
I believe that applies only to the Arasan integration, not MMC2.</div>
<div style="font-family: Consolas, Courier, monospace; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Consolas, Courier, monospace; font-size: 12pt; color: rgb(0, 0, 0);">
I'm trying to recollect why I thought this didn't matter  (or how it was getting mitigated), but i'm drawing a blank. I'm okay doing it.</div>
<div style="font-family: Consolas, Courier, monospace; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Consolas, Courier, monospace; font-size: 12pt; color: rgb(0, 0, 0);">
Reviewed-by: Andrei Warkentin <awarkentin@vmware.com></div>
<div id="appendonsend"></div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Jeremy Linton via groups.io <jeremy.linton=arm.com@groups.io><br>
<b>Sent:</b> Monday, December 14, 2020 5:23 PM<br>
<b>To:</b> devel@edk2.groups.io <devel@edk2.groups.io><br>
<b>Cc:</b> ard.biesheuvel@arm.com <ard.biesheuvel@arm.com>; leif@nuviainc.com <leif@nuviainc.com>; pete@akeo.ie <pete@akeo.ie>; andrey.warkentin@gmail.com <andrey.warkentin@gmail.com>; samer.el-haj-mahmoud@arm.com <samer.el-haj-mahmoud@arm.com>; Jeremy Linton
 <jeremy.linton@arm.com><br>
<b>Subject:</b> [edk2-devel] [PATCH 4/7] Platform/RaspberryPi/Arasan: Add write delay and voltage/clock config</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText">The uboot and linux drivers have notes that there is a clock domain crossing<br>
problem that happens with back to back writes to the sd controllers on the<br>
rpi. Its not clear if this is still applicable to the rpi4/emmc2 but<br>
it seems wise to add it.<br>
<br>
Futher, we need to assure that the card voltage is set to 3.3V, and<br>
we should try and follow some of the SDHCI docs when it comes to<br>
changing the clock.<br>
<br>
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com><br>
---<br>
 .../Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c    | 112 +++++++++++++++++----<br>
 .../Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.h    |   1 +<br>
 2 files changed, 93 insertions(+), 20 deletions(-)<br>
<br>
diff --git a/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c b/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c<br>
index 0cb7e85b38..a7b538a91a 100644<br>
--- a/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c<br>
+++ b/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c<br>
@@ -18,6 +18,56 @@ UINT32 LastExecutedCommand = (UINT32) -1;<br>
 STATIC RASPBERRY_PI_FIRMWARE_PROTOCOL *mFwProtocol;<br>
<br>
 STATIC UINTN MMCHS_BASE;<br>
<br>
 <br>
<br>
+STATIC<br>
<br>
+UINT32<br>
<br>
+EFIAPI<br>
<br>
+SdMmioWrite32 (<br>
<br>
+  IN      UINTN                     Address,<br>
<br>
+  IN      UINT32                    Value<br>
<br>
+  )<br>
<br>
+{<br>
<br>
+  UINT32 ret;<br>
<br>
+  ret = (UINT32)MmioWrite32 (Address, Value);<br>
<br>
+  // There is a bug about clock domain crossing on writes, delay to avoid it<br>
<br>
+  gBS->Stall (STALL_AFTER_REG_WRITE_US);<br>
<br>
+  return ret;<br>
<br>
+}<br>
<br>
+<br>
<br>
+STATIC<br>
<br>
+UINT32<br>
<br>
+EFIAPI<br>
<br>
+SdMmioOr32 (<br>
<br>
+  IN      UINTN                     Address,<br>
<br>
+  IN      UINT32                    OrData<br>
<br>
+  )<br>
<br>
+{<br>
<br>
+  return SdMmioWrite32 (Address, MmioRead32 (Address) | OrData);<br>
<br>
+}<br>
<br>
+<br>
<br>
+STATIC<br>
<br>
+UINT32<br>
<br>
+EFIAPI<br>
<br>
+SdMmioAnd32 (<br>
<br>
+  IN      UINTN                     Address,<br>
<br>
+  IN      UINT32                    AndData<br>
<br>
+  )<br>
<br>
+{<br>
<br>
+  return SdMmioWrite32 (Address, MmioRead32 (Address) & AndData);<br>
<br>
+}<br>
<br>
+<br>
<br>
+STATIC<br>
<br>
+UINT32<br>
<br>
+EFIAPI<br>
<br>
+SdMmioAndThenOr32 (<br>
<br>
+  IN      UINTN                     Address,<br>
<br>
+  IN      UINT32                    AndData,<br>
<br>
+  IN      UINT32                    OrData<br>
<br>
+  )<br>
<br>
+{<br>
<br>
+  return SdMmioWrite32 (Address, (MmioRead32 (Address) & AndData) | OrData);<br>
<br>
+}<br>
<br>
+<br>
<br>
+<br>
<br>
 /**<br>
<br>
    These SD commands are optional, according to the SD Spec<br>
<br>
 **/<br>
<br>
@@ -175,7 +225,9 @@ SoftReset (<br>
   IN UINT32 Mask<br>
<br>
   )<br>
<br>
 {<br>
<br>
-  MmioOr32 (MMCHS_SYSCTL, Mask);<br>
<br>
+  DEBUG ((DEBUG_MMCHOST_SD, "SoftReset with mask 0x%x\n", Mask));<br>
<br>
+<br>
<br>
+  SdMmioOr32 (MMCHS_SYSCTL, Mask);<br>
<br>
   if (PollRegisterWithMask (MMCHS_SYSCTL, Mask, 0) == EFI_TIMEOUT) {<br>
<br>
     DEBUG ((DEBUG_ERROR, "Failed to SoftReset with mask 0x%x\n", Mask));<br>
<br>
     return EFI_TIMEOUT;<br>
<br>
@@ -326,29 +378,29 @@ MMCSendCommand (<br>
   }<br>
<br>
 <br>
<br>
   if (IsAppCmd && MmcCmd == ACMD22) {<br>
<br>
-    MmioWrite32 (MMCHS_BLK, 4);<br>
<br>
+    SdMmioWrite32 (MMCHS_BLK, 4);<br>
<br>
   } else if (IsAppCmd && MmcCmd == ACMD51) {<br>
<br>
-    MmioWrite32 (MMCHS_BLK, 8);<br>
<br>
+    SdMmioWrite32 (MMCHS_BLK, 8);<br>
<br>
   } else if (!IsAppCmd && MmcCmd == CMD6) {<br>
<br>
-    MmioWrite32 (MMCHS_BLK, 64);<br>
<br>
+    SdMmioWrite32 (MMCHS_BLK, 64);<br>
<br>
   } else if (IsADTCCmd) {<br>
<br>
-    MmioWrite32 (MMCHS_BLK, BLEN_512BYTES);<br>
<br>
+    SdMmioWrite32 (MMCHS_BLK, BLEN_512BYTES);<br>
<br>
   }<br>
<br>
 <br>
<br>
   // Set Data timeout counter value to max value.<br>
<br>
-  MmioAndThenOr32 (MMCHS_SYSCTL, (UINT32) ~DTO_MASK, DTO_VAL);<br>
<br>
+  SdMmioAndThenOr32 (MMCHS_SYSCTL, (UINT32) ~DTO_MASK, DTO_VAL);<br>
<br>
 <br>
<br>
   //<br>
<br>
   // Clear Interrupt Status Register, but not the Card Inserted bit<br>
<br>
   // to avoid messing with card detection logic.<br>
<br>
   //<br>
<br>
-  MmioWrite32 (MMCHS_INT_STAT, ALL_EN & ~(CARD_INS));<br>
<br>
+  SdMmioWrite32 (MMCHS_INT_STAT, ALL_EN & ~(CARD_INS));<br>
<br>
 <br>
<br>
   // Set command argument register<br>
<br>
-  MmioWrite32 (MMCHS_ARG, Argument);<br>
<br>
+  SdMmioWrite32 (MMCHS_ARG, Argument);<br>
<br>
 <br>
<br>
   // Send the command<br>
<br>
-  MmioWrite32 (MMCHS_CMD, MmcCmd);<br>
<br>
+  SdMmioWrite32 (MMCHS_CMD, MmcCmd);<br>
<br>
 <br>
<br>
   // Check for the command status.<br>
<br>
   while (RetryCount < MAX_RETRY_COUNT) {<br>
<br>
@@ -373,7 +425,7 @@ MMCSendCommand (<br>
 <br>
<br>
     // Check if command is completed.<br>
<br>
     if ((MmcStatus & CC) == CC) {<br>
<br>
-      MmioWrite32 (MMCHS_INT_STAT, CC);<br>
<br>
+      SdMmioWrite32 (MMCHS_INT_STAT, CC);<br>
<br>
       break;<br>
<br>
     }<br>
<br>
 <br>
<br>
@@ -428,6 +480,21 @@ MMCNotifyState (<br>
         return Status;<br>
<br>
       }<br>
<br>
 <br>
<br>
+      DEBUG ((DEBUG_MMCHOST_SD, "ArasanMMCHost: CAP %X CAPH %X\n", MmioRead32(MMCHS_CAPA),MmioRead32(MMCHS_CUR_CAPA)));<br>
<br>
+<br>
<br>
+      // Lets switch to card detect test mode.<br>
<br>
+      SdMmioOr32 (MMCHS_HCTL, BIT7|BIT6);<br>
<br>
+<br>
<br>
+      // set card voltage<br>
<br>
+      SdMmioAnd32 (MMCHS_HCTL, ~SDBP_ON);<br>
<br>
+      SdMmioAndThenOr32 (MMCHS_HCTL, (UINT32) ~SDBP_MASK, SDVS_3_3_V);<br>
<br>
+      SdMmioOr32 (MMCHS_HCTL, SDBP_ON);<br>
<br>
+<br>
<br>
+      DEBUG ((DEBUG_MMCHOST_SD, "ArasanMMCHost: AC12 %X HCTL %X\n", MmioRead32(MMCHS_AC12),MmioRead32(MMCHS_HCTL)));<br>
<br>
+<br>
<br>
+      // First turn off the clock<br>
<br>
+      SdMmioAnd32 (MMCHS_SYSCTL, ~CEN);<br>
<br>
+<br>
<br>
       // Attempt to set the clock to 400Khz which is the expected initialization speed<br>
<br>
       Status = CalculateClockFrequencyDivisor (400000, &Divisor, NULL);<br>
<br>
       if (EFI_ERROR (Status)) {<br>
<br>
@@ -436,10 +503,15 @@ MMCNotifyState (<br>
       }<br>
<br>
 <br>
<br>
       // Set Data Timeout Counter value, set clock frequency, enable internal clock<br>
<br>
-      MmioOr32 (MMCHS_SYSCTL, DTO_VAL | Divisor | CEN | ICS | ICE);<br>
<br>
+      SdMmioOr32 (MMCHS_SYSCTL, DTO_VAL | Divisor | CEN | ICS | ICE);<br>
<br>
+      SdMmioOr32 (MMCHS_HCTL, SDBP_ON);<br>
<br>
+      // wait for ICS<br>
<br>
+      while ((MmioRead32 (MMCHS_SYSCTL) & ICS_MASK) != ICS);<br>
<br>
+<br>
<br>
+      DEBUG ((DEBUG_MMCHOST_SD, "ArasanMMCHost: AC12 %X HCTL %X\n", MmioRead32(MMCHS_AC12),MmioRead32(MMCHS_HCTL)));<br>
<br>
 <br>
<br>
       // Enable interrupts<br>
<br>
-      MmioWrite32 (MMCHS_IE, ALL_EN);<br>
<br>
+      SdMmioWrite32 (MMCHS_IE, ALL_EN);<br>
<br>
     }<br>
<br>
     break;<br>
<br>
   case MmcIdleState:<br>
<br>
@@ -452,7 +524,7 @@ MMCNotifyState (<br>
     ClockFrequency = 25000000;<br>
<br>
 <br>
<br>
     // First turn off the clock<br>
<br>
-    MmioAnd32 (MMCHS_SYSCTL, ~CEN);<br>
<br>
+    SdMmioAnd32 (MMCHS_SYSCTL, ~CEN);<br>
<br>
 <br>
<br>
     Status = CalculateClockFrequencyDivisor (ClockFrequency, &Divisor, NULL);<br>
<br>
     if (EFI_ERROR (Status)) {<br>
<br>
@@ -462,13 +534,13 @@ MMCNotifyState (<br>
     }<br>
<br>
 <br>
<br>
     // Setup new divisor<br>
<br>
-    MmioAndThenOr32 (MMCHS_SYSCTL, (UINT32) ~CLKD_MASK, Divisor);<br>
<br>
+    SdMmioAndThenOr32 (MMCHS_SYSCTL, (UINT32) ~CLKD_MASK, Divisor);<br>
<br>
 <br>
<br>
     // Wait for the clock to stabilise<br>
<br>
     while ((MmioRead32 (MMCHS_SYSCTL) & ICS_MASK) != ICS);<br>
<br>
 <br>
<br>
     // Set Data Timeout Counter value, set clock frequency, enable internal clock<br>
<br>
-    MmioOr32 (MMCHS_SYSCTL, CEN);<br>
<br>
+    SdMmioOr32 (MMCHS_SYSCTL, CEN);<br>
<br>
     break;<br>
<br>
   case MmcTransferState:<br>
<br>
     break;<br>
<br>
@@ -635,7 +707,7 @@ MMCReadBlockData (<br>
     while (RetryCount < MAX_RETRY_COUNT) {<br>
<br>
       MmcStatus = MmioRead32 (MMCHS_INT_STAT);<br>
<br>
       if ((MmcStatus & BRR) != 0) {<br>
<br>
-        MmioWrite32 (MMCHS_INT_STAT, BRR);<br>
<br>
+        SdMmioWrite32 (MMCHS_INT_STAT, BRR);<br>
<br>
         /*<br>
<br>
          * Data is ready.<br>
<br>
          */<br>
<br>
@@ -662,7 +734,7 @@ MMCReadBlockData (<br>
     gBS->Stall (STALL_AFTER_READ_US);<br>
<br>
   }<br>
<br>
 <br>
<br>
-  MmioWrite32 (MMCHS_INT_STAT, BRR);<br>
<br>
+  SdMmioWrite32 (MMCHS_INT_STAT, BRR);<br>
<br>
   return EFI_SUCCESS;<br>
<br>
 }<br>
<br>
 <br>
<br>
@@ -699,13 +771,13 @@ MMCWriteBlockData (<br>
     while (RetryCount < MAX_RETRY_COUNT) {<br>
<br>
       MmcStatus = MmioRead32 (MMCHS_INT_STAT);<br>
<br>
       if ((MmcStatus & BWR) != 0) {<br>
<br>
-        MmioWrite32 (MMCHS_INT_STAT, BWR);<br>
<br>
+        SdMmioWrite32 (MMCHS_INT_STAT, BWR);<br>
<br>
         /*<br>
<br>
          * Can write data.<br>
<br>
          */<br>
<br>
         mFwProtocol->SetLed (TRUE);<br>
<br>
         for (Count = 0; Count < BlockLen; Count += 4, Buffer++) {<br>
<br>
-          MmioWrite32 (MMCHS_DATA, *Buffer);<br>
<br>
+          SdMmioWrite32 (MMCHS_DATA, *Buffer);<br>
<br>
         }<br>
<br>
 <br>
<br>
         mFwProtocol->SetLed (FALSE);<br>
<br>
@@ -726,7 +798,7 @@ MMCWriteBlockData (<br>
     gBS->Stall (STALL_AFTER_WRITE_US);<br>
<br>
   }<br>
<br>
 <br>
<br>
-  MmioWrite32 (MMCHS_INT_STAT, BWR);<br>
<br>
+  SdMmioWrite32 (MMCHS_INT_STAT, BWR);<br>
<br>
   return EFI_SUCCESS;<br>
<br>
 }<br>
<br>
 <br>
<br>
diff --git a/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.h b/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.h<br>
index 6cd600f738..e94606cc5b 100644<br>
--- a/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.h<br>
+++ b/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.h<br>
@@ -37,6 +37,7 @@<br>
 #define STALL_AFTER_REC_RESP_US (50)<br>
<br>
 #define STALL_AFTER_WRITE_US (200)<br>
<br>
 #define STALL_AFTER_READ_US (20)<br>
<br>
+#define STALL_AFTER_REG_WRITE_US (10)<br>
<br>
 #define STALL_AFTER_RETRY_US (20)<br>
<br>
 <br>
<br>
 #define MAX_DIVISOR_VALUE 1023<br>
<br>
-- <br>
2.13.7<br>
<br>
<br>
<br>
-=-=-=-=-=-=<br>
Groups.io Links: You receive all messages sent to this group.<br>
View/Reply Online (#68816): <a href="https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F68816&amp;data=04%7C01%7Cawarkentin%40vmware.com%7C121bf8f78f3b437be80708d8a0875a08%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637435850504293478%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=6UBUDOHEs8MHLhK7%2B9DkL4NAdxzwttiJRvE88aqD%2BHw%3D&amp;reserved=0">
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F68816&amp;data=04%7C01%7Cawarkentin%40vmware.com%7C121bf8f78f3b437be80708d8a0875a08%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637435850504293478%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=6UBUDOHEs8MHLhK7%2B9DkL4NAdxzwttiJRvE88aqD%2BHw%3D&amp;reserved=0</a><br>
Mute This Topic: <a href="https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.io%2Fmt%2F78964893%2F4387333&amp;data=04%7C01%7Cawarkentin%40vmware.com%7C121bf8f78f3b437be80708d8a0875a08%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637435850504293478%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=OVqu67c28Qqnk25HasY%2BxkEC0KgOBQfxZWJxGZ0nJlk%3D&amp;reserved=0">
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.io%2Fmt%2F78964893%2F4387333&amp;data=04%7C01%7Cawarkentin%40vmware.com%7C121bf8f78f3b437be80708d8a0875a08%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637435850504293478%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=OVqu67c28Qqnk25HasY%2BxkEC0KgOBQfxZWJxGZ0nJlk%3D&amp;reserved=0</a><br>
Group Owner: devel+owner@edk2.groups.io<br>
Unsubscribe: <a href="https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Funsub&amp;data=04%7C01%7Cawarkentin%40vmware.com%7C121bf8f78f3b437be80708d8a0875a08%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637435850504293478%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=aXFzE1B1xX6JQeXymAEmWa2Z99welypR6HTHXrv1%2B7E%3D&amp;reserved=0">
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Funsub&amp;data=04%7C01%7Cawarkentin%40vmware.com%7C121bf8f78f3b437be80708d8a0875a08%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637435850504293478%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=aXFzE1B1xX6JQeXymAEmWa2Z99welypR6HTHXrv1%2B7E%3D&amp;reserved=0</a>
 [awarkentin@vmware.com]<br>
-=-=-=-=-=-=<br>
<br>
<br>
</div>
</span></font></div>
</body>
</html>


 <div width="1" style="color:white;clear:both">_._,_._,_</div> <hr> Groups.io Links:<p>   You receive all messages sent to this group.    <p> <a target="_blank" href="https://edk2.groups.io/g/devel/message/68877">View/Reply Online (#68877)</a> |    |  <a target="_blank" href="https://groups.io/mt/78964893/1813853">Mute This Topic</a>  | <a href="https://edk2.groups.io/g/devel/post">New Topic</a><br>    <a href="https://edk2.groups.io/g/devel/editsub/1813853">Your Subscription</a> | <a href="mailto:devel+owner@edk2.groups.io">Contact Group Owner</a> |  <a href="https://edk2.groups.io/g/devel/unsub">Unsubscribe</a>  [edk2-devel-archive@redhat.com]<br> <div width="1" style="color:white;clear:both">_._,_._,_</div>