EmbeddedPkg/Lan9118Dxe: use MemoryFence

When reviewing my LAN9118 driver PCD patch [1], Ard Biesheuvel noted
that most calls to gBS->Stall() in this driver seem to be used to
prevent timing issues between the device updating data and the host
reading the values.  And that replacing most of these calls with a
MemoryFence() would be more robust.

The only exceptions are the stalls that are enclosed inside retry loops:

 - in the AutoNegotiate() function.
   This stall is waiting for the link to negotiate, which may require
   stalling until it is ready.

 - in the Lan9118Initialize() function.
   These two stalls are waiting for devices and time out after a number
   of retries.

 - in the SoftReset() function.
   This stall is inside a loop where the comment states:
   "If time taken exceeds 100us, then there was an error condition"

In these instances, I kept the stall, but also added a MemoryFence().

[1] http://article.gmane.org/gmane.comp.bios.edk2.devel/7389

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ryan Harkin <ryan.harkin@linaro.org>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

diff --git a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c
index 4de520489930767525a48bbe794ef65cc005e383..79bee3f47cacde98ecf5e0c557ffef496ac3f222 100644 (file)
--- a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c
+++ b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c
@@ -307,8 +307,7 @@ SnpInitialize (
 \r
   // Write the current configuration to the register\r
   MmioWrite32 (LAN9118_PMT_CTRL, PmConf);\r
-  gBS->Stall (LAN9118_STALL);\r
-  gBS->Stall (LAN9118_STALL);\r
+  MemoryFence();\r
 \r
   // Configure GPIO and HW\r
   Status = ConfigureHardware (HW_CONF_USE_LEDS, Snp);\r
@@ -431,7 +430,7 @@ SnpReset (
 \r
   // Write the current configuration to the register\r
   MmioWrite32 (LAN9118_PMT_CTRL, PmConf);\r
-  gBS->Stall (LAN9118_STALL);\r
+  MemoryFence();\r
 \r
   // Reactivate the LEDs\r
   Status = ConfigureHardware (HW_CONF_USE_LEDS, Snp);\r
@@ -446,7 +445,7 @@ SnpReset (
     HwConf |= HW_CFG_TX_FIFO_SIZE(gTxBuffer);    // assign size chosen in SnpInitialize\r
 \r
     MmioWrite32 (LAN9118_HW_CFG, HwConf);        // Write the conf\r
-    gBS->Stall (LAN9118_STALL);\r
+    MemoryFence();\r
   }\r
 \r
   // Enable the receiver and transmitter and clear their contents\r
@@ -701,7 +700,7 @@ SnpReceiveFilters (
   // Write the options to the MAC_CSR\r
   //\r
   IndirectMACWrite32 (INDIRECT_MAC_INDEX_CR, MacCSRValue);\r
-  gBS->Stall (LAN9118_STALL);\r
+  MemoryFence();\r
 \r
   //\r
   // If we have to retrieve something, start packet reception.\r

diff --git a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c
index 9531b0ba2a463f6c9896a211b104f4d531ec004c..2ef1ecbb2887e0a1cec3b472b9b334ba60721ac7 100644 (file)
--- a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c
+++ b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c
@@ -236,7 +236,7 @@ IndirectEEPROMRead32 (
 \r
   // Write to Eeprom command register\r
   MmioWrite32 (LAN9118_E2P_CMD, EepromCmd);\r
-  gBS->Stall (LAN9118_STALL);\r
+  MemoryFence();\r
 \r
   // Wait until operation has completed\r
   while (MmioRead32 (LAN9118_E2P_CMD) & E2P_EPC_BUSY);\r
@@ -284,7 +284,7 @@ IndirectEEPROMWrite32 (
 \r
   // Write to Eeprom command register\r
   MmioWrite32 (LAN9118_E2P_CMD, EepromCmd);\r
-  gBS->Stall (LAN9118_STALL);\r
+  MemoryFence();\r
 \r
   // Wait until operation has completed\r
   while (MmioRead32 (LAN9118_E2P_CMD) & E2P_EPC_BUSY);\r
@@ -362,13 +362,14 @@ Lan9118Initialize (
   if (((MmioRead32 (LAN9118_PMT_CTRL) & MPTCTRL_PM_MODE_MASK) >> 12) != 0) {\r
     DEBUG ((DEBUG_NET, "Waking from reduced power state.\n"));\r
     MmioWrite32 (LAN9118_BYTE_TEST, 0xFFFFFFFF);\r
-    gBS->Stall (LAN9118_STALL);\r
+    MemoryFence();\r
   }\r
 \r
   // Check that device is active\r
   Timeout = 20;\r
   while ((MmioRead32 (LAN9118_PMT_CTRL) & MPTCTRL_READY) == 0 && --Timeout) {\r
     gBS->Stall (LAN9118_STALL);\r
+    MemoryFence();\r
   }\r
   if (!Timeout) {\r
     return EFI_TIMEOUT;\r
@@ -378,6 +379,7 @@ Lan9118Initialize (
   Timeout = 20;\r
   while ((MmioRead32 (LAN9118_E2P_CMD) & E2P_EPC_BUSY) && --Timeout){\r
     gBS->Stall (LAN9118_STALL);\r
+    MemoryFence();\r
   }\r
   if (!Timeout) {\r
     return EFI_TIMEOUT;\r
@@ -447,11 +449,12 @@ SoftReset (
 \r
   // Write the configuration\r
   MmioWrite32 (LAN9118_HW_CFG, HwConf);\r
-  gBS->Stall (LAN9118_STALL);\r
+  MemoryFence();\r
 \r
   // Wait for reset to complete\r
   while (MmioRead32 (LAN9118_HW_CFG) & HWCFG_SRST) {\r
 \r
+    MemoryFence();\r
     gBS->Stall (LAN9118_STALL);\r
     ResetTime += 1;\r
 \r
@@ -500,7 +503,7 @@ PhySoftReset (
 \r
     // Wait for completion\r
     while (MmioRead32 (LAN9118_PMT_CTRL) & MPTCTRL_PHY_RST) {\r
-      gBS->Stall (LAN9118_STALL);\r
+      MemoryFence();\r
     }\r
   // PHY Basic Control Register reset\r
   } else if (Flags & PHY_RESET_BCR) {\r
@@ -508,7 +511,7 @@ PhySoftReset (
 \r
     // Wait for completion\r
     while (IndirectPHYRead32 (PHY_INDEX_BASIC_CTRL) & PHYCR_RESET) {\r
-      gBS->Stall (LAN9118_STALL);\r
+      MemoryFence();\r
     }\r
   }\r
 \r
@@ -542,7 +545,7 @@ ConfigureHardware (
 \r
     // Write the configuration\r
     MmioWrite32 (LAN9118_GPIO_CFG, GpioConf);\r
-    gBS->Stall (LAN9118_STALL);\r
+    MemoryFence();\r
   }\r
 \r
   return EFI_SUCCESS;\r
@@ -585,6 +588,7 @@ AutoNegotiate (
     // Wait until it is up or until Time Out\r
     TimeOut = 2000;\r
     while ((IndirectPHYRead32 (PHY_INDEX_BASIC_STATUS) & PHYSTS_LINK_STS) == 0) {\r
+      MemoryFence();\r
       gBS->Stall (LAN9118_STALL);\r
       TimeOut--;\r
       if (!TimeOut) {\r
@@ -671,7 +675,7 @@ StopTx (
     TxCfg = MmioRead32 (LAN9118_TX_CFG);\r
     TxCfg |= TXCFG_TXS_DUMP | TXCFG_TXD_DUMP;\r
     MmioWrite32 (LAN9118_TX_CFG, TxCfg);\r
-    gBS->Stall (LAN9118_STALL);\r
+    MemoryFence();\r
   }\r
 \r
   // Check if already stopped\r
@@ -690,7 +694,7 @@ StopTx (
     if (TxCfg & TXCFG_TX_ON) {\r
       TxCfg |= TXCFG_STOP_TX;\r
       MmioWrite32 (LAN9118_TX_CFG, TxCfg);\r
-      gBS->Stall (LAN9118_STALL);\r
+      MemoryFence();\r
 \r
       // Wait for Tx to finish transmitting\r
       while (MmioRead32 (LAN9118_TX_CFG) & TXCFG_STOP_TX);\r
@@ -725,7 +729,7 @@ StopRx (
     RxCfg = MmioRead32 (LAN9118_RX_CFG);\r
     RxCfg |= RXCFG_RX_DUMP;\r
     MmioWrite32 (LAN9118_RX_CFG, RxCfg);\r
-    gBS->Stall (LAN9118_STALL);\r
+    MemoryFence();\r
 \r
     while (MmioRead32 (LAN9118_RX_CFG) & RXCFG_RX_DUMP);\r
   }\r
@@ -751,28 +755,28 @@ StartTx (
     TxCfg = MmioRead32 (LAN9118_TX_CFG);\r
     TxCfg |= TXCFG_TXS_DUMP | TXCFG_TXD_DUMP;\r
     MmioWrite32 (LAN9118_TX_CFG, TxCfg);\r
-    gBS->Stall (LAN9118_STALL);\r
+    MemoryFence();\r
   }\r
 \r
   // Check if tx was started from MAC and enable if not\r
   if (Flags & START_TX_MAC) {\r
     MacCsr = IndirectMACRead32 (INDIRECT_MAC_INDEX_CR);\r
-    gBS->Stall (LAN9118_STALL);\r
+    MemoryFence();\r
     if ((MacCsr & MACCR_TX_EN) == 0) {\r
       MacCsr |= MACCR_TX_EN;\r
       IndirectMACWrite32 (INDIRECT_MAC_INDEX_CR, MacCsr);\r
-      gBS->Stall (LAN9118_STALL);\r
+      MemoryFence();\r
     }\r
   }\r
 \r
   // Check if tx was started from TX_CFG and enable if not\r
   if (Flags & START_TX_CFG) {\r
     TxCfg = MmioRead32 (LAN9118_TX_CFG);\r
-    gBS->Stall (LAN9118_STALL);\r
+    MemoryFence();\r
     if ((TxCfg & TXCFG_TX_ON) == 0) {\r
       TxCfg |= TXCFG_TX_ON;\r
       MmioWrite32 (LAN9118_TX_CFG, TxCfg);\r
-      gBS->Stall (LAN9118_STALL);\r
+      MemoryFence();\r
     }\r
   }\r
 \r
@@ -802,14 +806,14 @@ StartRx (
       RxCfg = MmioRead32 (LAN9118_RX_CFG);\r
       RxCfg |= RXCFG_RX_DUMP;\r
       MmioWrite32 (LAN9118_RX_CFG, RxCfg);\r
-      gBS->Stall (LAN9118_STALL);\r
+      MemoryFence();\r
 \r
       while (MmioRead32 (LAN9118_RX_CFG) & RXCFG_RX_DUMP);\r
     }\r
 \r
     MacCsr |= MACCR_RX_EN;\r
     IndirectMACWrite32 (INDIRECT_MAC_INDEX_CR, MacCsr);\r
-    gBS->Stall (LAN9118_STALL);\r
+    MemoryFence();\r
   }\r
 \r
   return EFI_SUCCESS;\r
@@ -999,7 +1003,7 @@ ChangeFifoAllocation (
   HwConf &= ~(0xF0000);\r
   HwConf |= ((TxFifoOption & 0xF) << 16);\r
   MmioWrite32 (LAN9118_HW_CFG, HwConf);\r
-  gBS->Stall (LAN9118_STALL);\r
+  MemoryFence();\r
 \r
   return EFI_SUCCESS;\r
 }\r