comments below:

On 02/09/16 12:41, Ryan Harkin wrote:

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 exception is the Stall() in the AutoNegotiate() function. This stall is waiting for the link to negotiate, which may require stalling until it is ready. In this instance, 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

---

EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c | 9 +++--- EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c | 43 +++++++++++++------------ 2 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c index 4de5204..79bee3f 100644 --- a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c +++ b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c @@ -307,8 +307,7 @@ SnpInitialize ( // Write the current configuration to the register MmioWrite32 (LAN9118_PMT_CTRL, PmConf);

• gBS->Stall (LAN9118_STALL);
• gBS->Stall (LAN9118_STALL);

• MemoryFence();

You removed two Stall() calls, but only added one MemoryFence(). Why?

Just kidding. :)

I won't try to review this patch in depth without actually knowing how to program the device, but the idea seems clear, from skimming the patch.

Acked-by: Laszlo Ersek lersek@redhat.com

Thanks Laszlo

// Configure GPIO and HW Status = ConfigureHardware (HW_CONF_USE_LEDS, Snp); @@ -431,7 +430,7 @@ SnpReset ( // Write the current configuration to the register MmioWrite32 (LAN9118_PMT_CTRL, PmConf);

• gBS->Stall (LAN9118_STALL);

• MemoryFence();

// Reactivate the LEDs Status = ConfigureHardware (HW_CONF_USE_LEDS, Snp); @@ -446,7 +445,7 @@ SnpReset ( HwConf |= HW_CFG_TX_FIFO_SIZE(gTxBuffer); // assign size chosen in SnpInitialize MmioWrite32 (LAN9118_HW_CFG, HwConf); // Write the conf

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); }

// Enable the receiver and transmitter and clear their contents @@ -701,7 +700,7 @@ SnpReceiveFilters ( // Write the options to the MAC_CSR // IndirectMACWrite32 (INDIRECT_MAC_INDEX_CR, MacCSRValue);

• gBS->Stall (LAN9118_STALL);

• MemoryFence();

// // If we have to retrieve something, start packet reception. diff --git a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c index 9531b0b..8398c10 100644 --- a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c +++ b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c @@ -236,7 +236,7 @@ IndirectEEPROMRead32 ( // Write to Eeprom command register MmioWrite32 (LAN9118_E2P_CMD, EepromCmd);

• gBS->Stall (LAN9118_STALL);

• MemoryFence();

// Wait until operation has completed while (MmioRead32 (LAN9118_E2P_CMD) & E2P_EPC_BUSY); @@ -284,7 +284,7 @@ IndirectEEPROMWrite32 ( // Write to Eeprom command register MmioWrite32 (LAN9118_E2P_CMD, EepromCmd);

• gBS->Stall (LAN9118_STALL);

• MemoryFence();

// Wait until operation has completed while (MmioRead32 (LAN9118_E2P_CMD) & E2P_EPC_BUSY); @@ -362,13 +362,13 @@ Lan9118Initialize ( if (((MmioRead32 (LAN9118_PMT_CTRL) & MPTCTRL_PM_MODE_MASK) >> 12) != 0) { DEBUG ((DEBUG_NET, "Waking from reduced power state.\n")); MmioWrite32 (LAN9118_BYTE_TEST, 0xFFFFFFFF);

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); }

// Check that device is active Timeout = 20; while ((MmioRead32 (LAN9118_PMT_CTRL) & MPTCTRL_READY) == 0 && --Timeout) {

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); } if (!Timeout) { return EFI_TIMEOUT;

@@ -377,7 +377,7 @@ Lan9118Initialize ( // Check that EEPROM isn't active Timeout = 20; while ((MmioRead32 (LAN9118_E2P_CMD) & E2P_EPC_BUSY) && --Timeout){

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); } if (!Timeout) { return EFI_TIMEOUT;

@@ -447,12 +447,12 @@ SoftReset ( // Write the configuration MmioWrite32 (LAN9118_HW_CFG, HwConf);

• gBS->Stall (LAN9118_STALL);

• MemoryFence();

// Wait for reset to complete while (MmioRead32 (LAN9118_HW_CFG) & HWCFG_SRST) {

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); ResetTime += 1;

// If time taken exceeds 100us, then there was an error condition @@ -500,7 +500,7 @@ PhySoftReset ( // Wait for completion while (MmioRead32 (LAN9118_PMT_CTRL) & MPTCTRL_PHY_RST) {

•  gBS->Stall (LAN9118_STALL);
  

•  MemoryFence();
  

  } // PHY Basic Control Register reset } else if (Flags & PHY_RESET_BCR) {

@@ -508,7 +508,7 @@ PhySoftReset ( // Wait for completion while (IndirectPHYRead32 (PHY_INDEX_BASIC_CTRL) & PHYCR_RESET) {

•  gBS->Stall (LAN9118_STALL);
  

•  MemoryFence();
  

  } }

@@ -542,7 +542,7 @@ ConfigureHardware ( // Write the configuration MmioWrite32 (LAN9118_GPIO_CFG, GpioConf);

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); }

return EFI_SUCCESS; @@ -585,6 +585,7 @@ AutoNegotiate ( // Wait until it is up or until Time Out TimeOut = 2000; while ((IndirectPHYRead32 (PHY_INDEX_BASIC_STATUS) & PHYSTS_LINK_STS) == 0) {

•  MemoryFence();
   gBS->Stall (LAN9118_STALL);
   TimeOut--;
   if (!TimeOut) {
  

@@ -671,7 +672,7 @@ StopTx ( TxCfg = MmioRead32 (LAN9118_TX_CFG); TxCfg |= TXCFG_TXS_DUMP | TXCFG_TXD_DUMP; MmioWrite32 (LAN9118_TX_CFG, TxCfg);

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); }

// Check if already stopped @@ -690,7 +691,7 @@ StopTx ( if (TxCfg & TXCFG_TX_ON) { TxCfg |= TXCFG_STOP_TX; MmioWrite32 (LAN9118_TX_CFG, TxCfg);

•  gBS->Stall (LAN9118_STALL);
  

•  MemoryFence();
  

// Wait for Tx to finish transmitting while (MmioRead32 (LAN9118_TX_CFG) & TXCFG_STOP_TX); @@ -725,7 +726,7 @@ StopRx ( RxCfg = MmioRead32 (LAN9118_RX_CFG); RxCfg |= RXCFG_RX_DUMP; MmioWrite32 (LAN9118_RX_CFG, RxCfg);

• gBS->Stall (LAN9118_STALL);

• MemoryFence();

while (MmioRead32 (LAN9118_RX_CFG) & RXCFG_RX_DUMP); } @@ -751,28 +752,28 @@ StartTx ( TxCfg = MmioRead32 (LAN9118_TX_CFG); TxCfg |= TXCFG_TXS_DUMP | TXCFG_TXD_DUMP; MmioWrite32 (LAN9118_TX_CFG, TxCfg);

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); }

// Check if tx was started from MAC and enable if not if (Flags & START_TX_MAC) { MacCsr = IndirectMACRead32 (INDIRECT_MAC_INDEX_CR);

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); if ((MacCsr & MACCR_TX_EN) == 0) { MacCsr |= MACCR_TX_EN; IndirectMACWrite32 (INDIRECT_MAC_INDEX_CR, MacCsr);

•  gBS->Stall (LAN9118_STALL);
  

•  MemoryFence();
  

  } }

// Check if tx was started from TX_CFG and enable if not if (Flags & START_TX_CFG) { TxCfg = MmioRead32 (LAN9118_TX_CFG);

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); if ((TxCfg & TXCFG_TX_ON) == 0) { TxCfg |= TXCFG_TX_ON; MmioWrite32 (LAN9118_TX_CFG, TxCfg);

•  gBS->Stall (LAN9118_STALL);
  

•  MemoryFence();
  

  } }

@@ -802,14 +803,14 @@ StartRx ( RxCfg = MmioRead32 (LAN9118_RX_CFG); RxCfg |= RXCFG_RX_DUMP; MmioWrite32 (LAN9118_RX_CFG, RxCfg);

•  gBS->Stall (LAN9118_STALL);
  

•  MemoryFence();
  

while (MmioRead32 (LAN9118_RX_CFG) & RXCFG_RX_DUMP); } MacCsr |= MACCR_RX_EN; IndirectMACWrite32 (INDIRECT_MAC_INDEX_CR, MacCsr);

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); }

return EFI_SUCCESS; @@ -999,7 +1000,7 @@ ChangeFifoAllocation ( HwConf &= ~(0xF0000); HwConf |= ((TxFifoOption & 0xF) << 16); MmioWrite32 (LAN9118_HW_CFG, HwConf);

• gBS->Stall (LAN9118_STALL);

• MemoryFence();

return EFI_SUCCESS; }

On Tue, Feb 09, 2016 at 11:41:02AM +0000, Ryan Harkin wrote:

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 exception is the Stall() in the AutoNegotiate() function. This stall is waiting for the link to negotiate, which may require stalling until it is ready. In this instance, I kept the stall, but also added a MemoryFence().

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

So, this is clearly a fix, and I think it should go in. But as a word of warning, this could break on any platforms predating Cortex-A15/A7, with external system caches.

However, this is technically a bug in MemoryFence() on those platforms, and with this driver using hard-wired base addresses and stuff I don't expect a lot of users outside of the platforms you are looking after wourself.

/ Leif

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ryan Harkin ryan.harkin@linaro.org

---

EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c | 9 +++--- EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c | 43 +++++++++++++------------ 2 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c index 4de5204..79bee3f 100644 --- a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c +++ b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c @@ -307,8 +307,7 @@ SnpInitialize ( // Write the current configuration to the register MmioWrite32 (LAN9118_PMT_CTRL, PmConf);

• gBS->Stall (LAN9118_STALL);
• gBS->Stall (LAN9118_STALL);

• MemoryFence();

// Configure GPIO and HW Status = ConfigureHardware (HW_CONF_USE_LEDS, Snp); @@ -431,7 +430,7 @@ SnpReset ( // Write the current configuration to the register MmioWrite32 (LAN9118_PMT_CTRL, PmConf);

• gBS->Stall (LAN9118_STALL);

• MemoryFence();

// Reactivate the LEDs Status = ConfigureHardware (HW_CONF_USE_LEDS, Snp); @@ -446,7 +445,7 @@ SnpReset ( HwConf |= HW_CFG_TX_FIFO_SIZE(gTxBuffer); // assign size chosen in SnpInitialize MmioWrite32 (LAN9118_HW_CFG, HwConf); // Write the conf

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); }

// Enable the receiver and transmitter and clear their contents @@ -701,7 +700,7 @@ SnpReceiveFilters ( // Write the options to the MAC_CSR // IndirectMACWrite32 (INDIRECT_MAC_INDEX_CR, MacCSRValue);

• gBS->Stall (LAN9118_STALL);

• MemoryFence();

// // If we have to retrieve something, start packet reception. diff --git a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c index 9531b0b..8398c10 100644 --- a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c +++ b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c @@ -236,7 +236,7 @@ IndirectEEPROMRead32 ( // Write to Eeprom command register MmioWrite32 (LAN9118_E2P_CMD, EepromCmd);

• gBS->Stall (LAN9118_STALL);

• MemoryFence();

// Wait until operation has completed while (MmioRead32 (LAN9118_E2P_CMD) & E2P_EPC_BUSY); @@ -284,7 +284,7 @@ IndirectEEPROMWrite32 ( // Write to Eeprom command register MmioWrite32 (LAN9118_E2P_CMD, EepromCmd);

• gBS->Stall (LAN9118_STALL);

• MemoryFence();

// Wait until operation has completed while (MmioRead32 (LAN9118_E2P_CMD) & E2P_EPC_BUSY); @@ -362,13 +362,13 @@ Lan9118Initialize ( if (((MmioRead32 (LAN9118_PMT_CTRL) & MPTCTRL_PM_MODE_MASK) >> 12) != 0) { DEBUG ((DEBUG_NET, "Waking from reduced power state.\n")); MmioWrite32 (LAN9118_BYTE_TEST, 0xFFFFFFFF);

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); }

// Check that device is active Timeout = 20; while ((MmioRead32 (LAN9118_PMT_CTRL) & MPTCTRL_READY) == 0 && --Timeout) {

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); } if (!Timeout) { return EFI_TIMEOUT;

@@ -377,7 +377,7 @@ Lan9118Initialize ( // Check that EEPROM isn't active Timeout = 20; while ((MmioRead32 (LAN9118_E2P_CMD) & E2P_EPC_BUSY) && --Timeout){

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); } if (!Timeout) { return EFI_TIMEOUT;

@@ -447,12 +447,12 @@ SoftReset ( // Write the configuration MmioWrite32 (LAN9118_HW_CFG, HwConf);

• gBS->Stall (LAN9118_STALL);

• MemoryFence();

// Wait for reset to complete while (MmioRead32 (LAN9118_HW_CFG) & HWCFG_SRST) {

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); ResetTime += 1;

// If time taken exceeds 100us, then there was an error condition @@ -500,7 +500,7 @@ PhySoftReset ( // Wait for completion while (MmioRead32 (LAN9118_PMT_CTRL) & MPTCTRL_PHY_RST) {

•  gBS->Stall (LAN9118_STALL);
  

•  MemoryFence();
  

  } // PHY Basic Control Register reset } else if (Flags & PHY_RESET_BCR) {

@@ -508,7 +508,7 @@ PhySoftReset ( // Wait for completion while (IndirectPHYRead32 (PHY_INDEX_BASIC_CTRL) & PHYCR_RESET) {

•  gBS->Stall (LAN9118_STALL);
  

•  MemoryFence();
  

  } }

@@ -542,7 +542,7 @@ ConfigureHardware ( // Write the configuration MmioWrite32 (LAN9118_GPIO_CFG, GpioConf);

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); }

return EFI_SUCCESS; @@ -585,6 +585,7 @@ AutoNegotiate ( // Wait until it is up or until Time Out TimeOut = 2000; while ((IndirectPHYRead32 (PHY_INDEX_BASIC_STATUS) & PHYSTS_LINK_STS) == 0) {

•  MemoryFence();
   gBS->Stall (LAN9118_STALL);
   TimeOut--;
   if (!TimeOut) {
  

@@ -671,7 +672,7 @@ StopTx ( TxCfg = MmioRead32 (LAN9118_TX_CFG); TxCfg |= TXCFG_TXS_DUMP | TXCFG_TXD_DUMP; MmioWrite32 (LAN9118_TX_CFG, TxCfg);

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); }

// Check if already stopped @@ -690,7 +691,7 @@ StopTx ( if (TxCfg & TXCFG_TX_ON) { TxCfg |= TXCFG_STOP_TX; MmioWrite32 (LAN9118_TX_CFG, TxCfg);

•  gBS->Stall (LAN9118_STALL);
  

•  MemoryFence();
  

// Wait for Tx to finish transmitting while (MmioRead32 (LAN9118_TX_CFG) & TXCFG_STOP_TX); @@ -725,7 +726,7 @@ StopRx ( RxCfg = MmioRead32 (LAN9118_RX_CFG); RxCfg |= RXCFG_RX_DUMP; MmioWrite32 (LAN9118_RX_CFG, RxCfg);

• gBS->Stall (LAN9118_STALL);

• MemoryFence();

while (MmioRead32 (LAN9118_RX_CFG) & RXCFG_RX_DUMP); } @@ -751,28 +752,28 @@ StartTx ( TxCfg = MmioRead32 (LAN9118_TX_CFG); TxCfg |= TXCFG_TXS_DUMP | TXCFG_TXD_DUMP; MmioWrite32 (LAN9118_TX_CFG, TxCfg);

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); }

// Check if tx was started from MAC and enable if not if (Flags & START_TX_MAC) { MacCsr = IndirectMACRead32 (INDIRECT_MAC_INDEX_CR);

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); if ((MacCsr & MACCR_TX_EN) == 0) { MacCsr |= MACCR_TX_EN; IndirectMACWrite32 (INDIRECT_MAC_INDEX_CR, MacCsr);

•  gBS->Stall (LAN9118_STALL);
  

•  MemoryFence();
  

  } }

// Check if tx was started from TX_CFG and enable if not if (Flags & START_TX_CFG) { TxCfg = MmioRead32 (LAN9118_TX_CFG);

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); if ((TxCfg & TXCFG_TX_ON) == 0) { TxCfg |= TXCFG_TX_ON; MmioWrite32 (LAN9118_TX_CFG, TxCfg);

•  gBS->Stall (LAN9118_STALL);
  

•  MemoryFence();
  

  } }

@@ -802,14 +803,14 @@ StartRx ( RxCfg = MmioRead32 (LAN9118_RX_CFG); RxCfg |= RXCFG_RX_DUMP; MmioWrite32 (LAN9118_RX_CFG, RxCfg);

•  gBS->Stall (LAN9118_STALL);
  

•  MemoryFence();
  

while (MmioRead32 (LAN9118_RX_CFG) & RXCFG_RX_DUMP); } MacCsr |= MACCR_RX_EN; IndirectMACWrite32 (INDIRECT_MAC_INDEX_CR, MacCsr);

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); }

return EFI_SUCCESS; @@ -999,7 +1000,7 @@ ChangeFifoAllocation ( HwConf &= ~(0xF0000); HwConf |= ((TxFifoOption & 0xF) << 16); MmioWrite32 (LAN9118_HW_CFG, HwConf);

• gBS->Stall (LAN9118_STALL);

• MemoryFence();

return EFI_SUCCESS; } -- 2.1.4

On 9 February 2016 at 14:25, Ard Biesheuvel ard.biesheuvel@linaro.org wrote:

On 9 February 2016 at 12:41, Ryan Harkin ryan.harkin@linaro.org wrote:

...

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 exception is the Stall() in the AutoNegotiate() function. This stall is waiting for the link to negotiate, which may require stalling until it is ready. In this instance, 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

OK, not so fast. As I remarked in #linaro-enterprise as well, there are a few instances where we may need both the MemoryFence() *and* a small delay (please see below)

The problem here is that I don't have access to h/w specs, so I'd prefer to err on the side of caution.

...

---

EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c | 9 +++--- EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c | 43 +++++++++++++------------ 2 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c index 4de5204..79bee3f 100644 --- a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c +++ b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c @@ -307,8 +307,7 @@ SnpInitialize (

// Write the current configuration to the register MmioWrite32 (LAN9118_PMT_CTRL, PmConf);

• gBS->Stall (LAN9118_STALL);
• gBS->Stall (LAN9118_STALL);

• MemoryFence();

  // Configure GPIO and HW Status = ConfigureHardware (HW_CONF_USE_LEDS, Snp);

@@ -431,7 +430,7 @@ SnpReset (

// Write the current configuration to the register MmioWrite32 (LAN9118_PMT_CTRL, PmConf);

• gBS->Stall (LAN9118_STALL);

• MemoryFence();

  // Reactivate the LEDs Status = ConfigureHardware (HW_CONF_USE_LEDS, Snp);

@@ -446,7 +445,7 @@ SnpReset ( HwConf |= HW_CFG_TX_FIFO_SIZE(gTxBuffer); // assign size chosen in SnpInitialize

 MmioWrite32 (LAN9118_HW_CFG, HwConf);        // Write the conf

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); }

  // Enable the receiver and transmitter and clear their contents

@@ -701,7 +700,7 @@ SnpReceiveFilters ( // Write the options to the MAC_CSR // IndirectMACWrite32 (INDIRECT_MAC_INDEX_CR, MacCSRValue);

• gBS->Stall (LAN9118_STALL);

• MemoryFence();

  // // If we have to retrieve something, start packet reception.

diff --git a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c index 9531b0b..8398c10 100644 --- a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c +++ b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c @@ -236,7 +236,7 @@ IndirectEEPROMRead32 (

// Write to Eeprom command register MmioWrite32 (LAN9118_E2P_CMD, EepromCmd);

• gBS->Stall (LAN9118_STALL);

• MemoryFence();

  // Wait until operation has completed while (MmioRead32 (LAN9118_E2P_CMD) & E2P_EPC_BUSY);

@@ -284,7 +284,7 @@ IndirectEEPROMWrite32 (

// Write to Eeprom command register MmioWrite32 (LAN9118_E2P_CMD, EepromCmd);

• gBS->Stall (LAN9118_STALL);

• MemoryFence();

  // Wait until operation has completed while (MmioRead32 (LAN9118_E2P_CMD) & E2P_EPC_BUSY);

@@ -362,13 +362,13 @@ Lan9118Initialize ( if (((MmioRead32 (LAN9118_PMT_CTRL) & MPTCTRL_PM_MODE_MASK) >> 12) != 0) { DEBUG ((DEBUG_NET, "Waking from reduced power state.\n")); MmioWrite32 (LAN9118_BYTE_TEST, 0xFFFFFFFF);

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); }

  // Check that device is active Timeout = 20; while ((MmioRead32 (LAN9118_PMT_CTRL) & MPTCTRL_READY) == 0 && --Timeout) {

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); }

Here we may have woken it from a reduced power state.

...

if (!Timeout) { return EFI_TIMEOUT; @@ -377,7 +377,7 @@ Lan9118Initialize ( // Check that EEPROM isn't active Timeout = 20; while ((MmioRead32 (LAN9118_E2P_CMD) & E2P_EPC_BUSY) && --Timeout){

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); } if (!Timeout) { return EFI_TIMEOUT;

@@ -447,12 +447,12 @@ SoftReset (

// Write the configuration MmioWrite32 (LAN9118_HW_CFG, HwConf);

• gBS->Stall (LAN9118_STALL);

• MemoryFence();

  // Wait for reset to complete while (MmioRead32 (LAN9118_HW_CFG) & HWCFG_SRST) {

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); ResetTime += 1;

  // If time taken exceeds 100us, then there was an error condition

@@ -500,7 +500,7 @@ PhySoftReset (

 // Wait for completion
 while (MmioRead32 (LAN9118_PMT_CTRL) & MPTCTRL_PHY_RST) {

•  gBS->Stall (LAN9118_STALL);
  

•  MemoryFence();
  

  }

This may take 100 microseconds, as per the comment, so having a Stall() here is reasonable, although it makes sense to use a constant here.

...

// PHY Basic Control Register reset } else if (Flags & PHY_RESET_BCR) { @@ -508,7 +508,7 @@ PhySoftReset (

 // Wait for completion
 while (IndirectPHYRead32 (PHY_INDEX_BASIC_CTRL) & PHYCR_RESET) {

•  gBS->Stall (LAN9118_STALL);
  

•  MemoryFence();
  

Another loop, but in this case, it is reasonable to hammer on the actual MMIO register rather than on the generic timer counter register to check whether the delay has expired, so this Stall() can be removed.

...

 }

}

@@ -542,7 +542,7 @@ ConfigureHardware (

 // Write the configuration
 MmioWrite32 (LAN9118_GPIO_CFG, GpioConf);

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); }

  return EFI_SUCCESS;

@@ -585,6 +585,7 @@ AutoNegotiate ( // Wait until it is up or until Time Out TimeOut = 2000; while ((IndirectPHYRead32 (PHY_INDEX_BASIC_STATUS) & PHYSTS_LINK_STS) == 0) {

•  MemoryFence();
   gBS->Stall (LAN9118_STALL);
   TimeOut--;
   if (!TimeOut) {
  

@@ -671,7 +672,7 @@ StopTx ( TxCfg = MmioRead32 (LAN9118_TX_CFG); TxCfg |= TXCFG_TXS_DUMP | TXCFG_TXD_DUMP; MmioWrite32 (LAN9118_TX_CFG, TxCfg);

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); }

  // Check if already stopped

@@ -690,7 +691,7 @@ StopTx ( if (TxCfg & TXCFG_TX_ON) { TxCfg |= TXCFG_STOP_TX; MmioWrite32 (LAN9118_TX_CFG, TxCfg);

•  gBS->Stall (LAN9118_STALL);
  

•  MemoryFence();
  
   // Wait for Tx to finish transmitting
   while (MmioRead32 (LAN9118_TX_CFG) & TXCFG_STOP_TX);
  

@@ -725,7 +726,7 @@ StopRx ( RxCfg = MmioRead32 (LAN9118_RX_CFG); RxCfg |= RXCFG_RX_DUMP; MmioWrite32 (LAN9118_RX_CFG, RxCfg);

• gBS->Stall (LAN9118_STALL);

• MemoryFence();

  while (MmioRead32 (LAN9118_RX_CFG) & RXCFG_RX_DUMP);

This needs a MemoryFence() as well

...

} @@ -751,28 +752,28 @@ StartTx ( TxCfg = MmioRead32 (LAN9118_TX_CFG); TxCfg |= TXCFG_TXS_DUMP | TXCFG_TXD_DUMP; MmioWrite32 (LAN9118_TX_CFG, TxCfg);

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); }

  // Check if tx was started from MAC and enable if not if (Flags & START_TX_MAC) { MacCsr = IndirectMACRead32 (INDIRECT_MAC_INDEX_CR);

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); if ((MacCsr & MACCR_TX_EN) == 0) { MacCsr |= MACCR_TX_EN; IndirectMACWrite32 (INDIRECT_MAC_INDEX_CR, MacCsr);

•  gBS->Stall (LAN9118_STALL);
  

•  MemoryFence();
  

  } }

  // Check if tx was started from TX_CFG and enable if not if (Flags & START_TX_CFG) { TxCfg = MmioRead32 (LAN9118_TX_CFG);

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); if ((TxCfg & TXCFG_TX_ON) == 0) { TxCfg |= TXCFG_TX_ON; MmioWrite32 (LAN9118_TX_CFG, TxCfg);

•  gBS->Stall (LAN9118_STALL);
  

•  MemoryFence();
  

  } }

@@ -802,14 +803,14 @@ StartRx ( RxCfg = MmioRead32 (LAN9118_RX_CFG); RxCfg |= RXCFG_RX_DUMP; MmioWrite32 (LAN9118_RX_CFG, RxCfg);

•  gBS->Stall (LAN9118_STALL);
  

•  MemoryFence();
  
   while (MmioRead32 (LAN9118_RX_CFG) & RXCFG_RX_DUMP);
  

... and here

...

 }

 MacCsr |= MACCR_RX_EN;
 IndirectMACWrite32 (INDIRECT_MAC_INDEX_CR, MacCsr);

• gBS->Stall (LAN9118_STALL);

• MemoryFence(); }

  return EFI_SUCCESS;

@@ -999,7 +1000,7 @@ ChangeFifoAllocation ( HwConf &= ~(0xF0000); HwConf |= ((TxFifoOption & 0xF) << 16); MmioWrite32 (LAN9118_HW_CFG, HwConf);

• gBS->Stall (LAN9118_STALL);

• MemoryFence();

  return EFI_SUCCESS;

}

2.1.4

On 02/09/16 12:41, Ryan Harkin wrote:

Add a PCD for the link negotiation timeout so the platform can over-ride the default value.

When the ARM Juno Development Platform uses the "EFI Network" option with then LAN9118 driver, it fails to boot the first time and so the board drops back to Shell again:

Warning: LAN9118 Driver in stopped state Link timeout in auto-negotiation. Lan9118: Auto Negociation not supported. EhcExecTransfer: transfer failed with 2 EhcControlTransfer: error - Device Error, transfer - 2 Buffer: EFI Hard Drive Booting EFI Misc Device Booting EFI Misc Device 1 Booting EFI Hard Drive Booting EFI Network Warning: LAN9118 Driver not initialized Link timeout in auto-negotiation. Lan9118: Auto Negociation not supported. Booting EFI Internal Shell

Exiting Shell drops the user back to the Intel BDS UI. Selecting "Continue" then succeeds in booting from the EFI Network:

Booting EFI Misc Device Booting EFI Misc Device 1 Booting EFI Hard Drive Booting EFI Network ..MnpFreeTxBuf: Duplicated recycle report from SNP. MnpFreeTxBuf: Duplicated recycle report from SNP. [snip repeated errors]

Discussion on the edk2-devel mailing list [1] prompted Laszlo Ersek to suggest the time taken for the NIC to negotiate was causing a problem. He suggested the solution contained in this patch to provide a PCD configurable by the platform.

Setting the PCD to a larger value works for Juno R0, R1 and R2.

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

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ryan Harkin ryan.harkin@linaro.org

---

EmbeddedPkg/EmbeddedPkg.dec | 1 + EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.inf | 1 + EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.h | 2 -- EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c | 2 +- 4 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec index f557527..2ede469 100644 --- a/EmbeddedPkg/EmbeddedPkg.dec +++ b/EmbeddedPkg/EmbeddedPkg.dec @@ -145,6 +145,7 @@ [PcdsFixedAtBuild.common] # LAN9118 Ethernet Driver PCDs gEmbeddedTokenSpaceGuid.PcdLan9118DxeBaseAddress|0x0|UINT32|0x00000025 gEmbeddedTokenSpaceGuid.PcdLan9118DefaultMacAddress|0x0|UINT64|0x00000026

• gEmbeddedTokenSpaceGuid.PcdLan9118DefaultNegotiationTimeout|2|UINT32|0x00000027

# # Android FastBoot diff --git a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.inf b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.inf index 9e5f98b..3c2246f 100644 --- a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.inf +++ b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.inf @@ -51,6 +51,7 @@ [Protocols] [FixedPcd] gEmbeddedTokenSpaceGuid.PcdLan9118DxeBaseAddress gEmbeddedTokenSpaceGuid.PcdLan9118DefaultMacAddress

• gEmbeddedTokenSpaceGuid.PcdLan9118DefaultNegotiationTimeout

[Depex] TRUE diff --git a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.h b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.h index cc883e8..4d73f40 100644 --- a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.h +++ b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.h @@ -38,8 +38,6 @@ #include "Lan9118DxeUtil.h" #include "Lan9118DxeHw.h" -#define LAN9118_STALL 2

#define LAN9118_DEFAULT_MAC_ADDRL 0x00F70200 #define LAN9118_DEFAULT_MAC_ADDRH 0x00009040 diff --git a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c index 8398c10..66d2b4b 100644 --- a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c +++ b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c @@ -586,7 +586,7 @@ AutoNegotiate ( TimeOut = 2000; while ((IndirectPHYRead32 (PHY_INDEX_BASIC_STATUS) & PHYSTS_LINK_STS) == 0) { MemoryFence();

•  gBS->Stall (LAN9118_STALL);
  

•  gBS->Stall ( FixedPcdGet32 (PcdLan9118DefaultNegotiationTimeout) );
   TimeOut--;
   if (!TimeOut) {
     DEBUG ((EFI_D_ERROR, "Link timeout in auto-negotiation.\n"));
  
  

The whitespace *within* the Stall() parens should be removed when committing this patch. I recall this style from Xen, but it's not the edk2 coding style.

Reviewed-by: Laszlo Ersek lersek@redhat.com

On 9 February 2016 at 12:41, Ryan Harkin ryan.harkin@linaro.org wrote:

Add a PCD for the link negotiation timeout so the platform can over-ride the default value.

When the ARM Juno Development Platform uses the "EFI Network" option with then LAN9118 driver, it fails to boot the first time and so the board drops back to Shell again:

Warning: LAN9118 Driver in stopped state Link timeout in auto-negotiation. Lan9118: Auto Negociation not supported. EhcExecTransfer: transfer failed with 2 EhcControlTransfer: error - Device Error, transfer - 2 Buffer: EFI Hard Drive Booting EFI Misc Device Booting EFI Misc Device 1 Booting EFI Hard Drive Booting EFI Network Warning: LAN9118 Driver not initialized Link timeout in auto-negotiation. Lan9118: Auto Negociation not supported. Booting EFI Internal Shell

Exiting Shell drops the user back to the Intel BDS UI. Selecting "Continue" then succeeds in booting from the EFI Network:

Booting EFI Misc Device Booting EFI Misc Device 1 Booting EFI Hard Drive Booting EFI Network ..MnpFreeTxBuf: Duplicated recycle report from SNP. MnpFreeTxBuf: Duplicated recycle report from SNP. [snip repeated errors]

Discussion on the edk2-devel mailing list [1] prompted Laszlo Ersek to suggest the time taken for the NIC to negotiate was causing a problem. He suggested the solution contained in this patch to provide a PCD configurable by the platform.

Setting the PCD to a larger value works for Juno R0, R1 and R2.

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

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ryan Harkin ryan.harkin@linaro.org

Reviewed-by: Ard Biesheuvel ard.biesheuvel@linaro.org

---

EmbeddedPkg/EmbeddedPkg.dec | 1 + EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.inf | 1 + EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.h | 2 -- EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c | 2 +- 4 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec index f557527..2ede469 100644 --- a/EmbeddedPkg/EmbeddedPkg.dec +++ b/EmbeddedPkg/EmbeddedPkg.dec @@ -145,6 +145,7 @@ [PcdsFixedAtBuild.common] # LAN9118 Ethernet Driver PCDs gEmbeddedTokenSpaceGuid.PcdLan9118DxeBaseAddress|0x0|UINT32|0x00000025 gEmbeddedTokenSpaceGuid.PcdLan9118DefaultMacAddress|0x0|UINT64|0x00000026

• gEmbeddedTokenSpaceGuid.PcdLan9118DefaultNegotiationTimeout|2|UINT32|0x00000027

  # # Android FastBoot

diff --git a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.inf b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.inf index 9e5f98b..3c2246f 100644 --- a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.inf +++ b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.inf @@ -51,6 +51,7 @@ [Protocols] [FixedPcd] gEmbeddedTokenSpaceGuid.PcdLan9118DxeBaseAddress gEmbeddedTokenSpaceGuid.PcdLan9118DefaultMacAddress

• gEmbeddedTokenSpaceGuid.PcdLan9118DefaultNegotiationTimeout

[Depex] TRUE diff --git a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.h b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.h index cc883e8..4d73f40 100644 --- a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.h +++ b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.h @@ -38,8 +38,6 @@ #include "Lan9118DxeUtil.h" #include "Lan9118DxeHw.h"

-#define LAN9118_STALL 2

#define LAN9118_DEFAULT_MAC_ADDRL 0x00F70200 #define LAN9118_DEFAULT_MAC_ADDRH 0x00009040

diff --git a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c index 8398c10..66d2b4b 100644 --- a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c +++ b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c @@ -586,7 +586,7 @@ AutoNegotiate ( TimeOut = 2000; while ((IndirectPHYRead32 (PHY_INDEX_BASIC_STATUS) & PHYSTS_LINK_STS) == 0) { MemoryFence();

•  gBS->Stall (LAN9118_STALL);
  

•  gBS->Stall ( FixedPcdGet32 (PcdLan9118DefaultNegotiationTimeout) );
   TimeOut--;
   if (!TimeOut) {
     DEBUG ((EFI_D_ERROR, "Link timeout in auto-negotiation.\n"));
  

-- 2.1.4

On 02/09/16 12:41, Ryan Harkin wrote:

This patch makes a few minor DEBUG output changes:

• Fix typo in DEBUG output: Negociation->Negotiation

• Change DEBUG occurrences of "Lan9118" to "LAN9118" to make grepping the log output easier.

• Change the warning that auto-negotiation is not supported when AutoNegotiate() returns an error. The function already reports if the feature is supported or not and can also return an error for other reasons.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ryan Harkin ryan.harkin@linaro.org

---

EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c index 79bee3f..d0bf7be 100644 --- a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c +++ b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c @@ -142,7 +142,7 @@ Lan9118DxeEntry ( // Power up the device so we can find the MAC address Status = Lan9118Initialize (Snp); if (EFI_ERROR (Status)) {

• DEBUG ((EFI_D_ERROR, "Lan9118: Error initialising hardware\n"));

• DEBUG ((EFI_D_ERROR, "LAN9118: Error initialising hardware\n")); return EFI_DEVICE_ERROR; }

@@ -342,7 +342,7 @@ SnpInitialize ( // Do auto-negotiation if supported Status = AutoNegotiate (AUTO_NEGOTIATE_ADVERTISE_ALL, Snp); if (EFI_ERROR(Status)) {

• DEBUG ((EFI_D_WARN, "Lan9118: Auto Negociation not supported.\n"));

• DEBUG ((EFI_D_WARN, "LAN9118: Auto Negotiation failed.\n")); }

// Configure flow control depending on speed capabilities @@ -767,7 +767,7 @@ SnpStationAddress ( New = (EFI_MAC_ADDRESS *) PermAddr; Lan9118SetMacAddress ((EFI_MAC_ADDRESS *) PermAddr, Snp); } else {

•  DEBUG ((EFI_D_ERROR, "Lan9118: Warning: No valid MAC address in EEPROM, using fallback\n"));
  

•  DEBUG ((EFI_D_ERROR, "LAN9118: Warning: No valid MAC address in EEPROM, using fallback\n"));
   New = (EFI_MAC_ADDRESS*) (FixedPcdGet64 (PcdLan9118DefaultMacAddress));
  

  } } else {

Reviewed-by: Laszlo Ersek lersek@redhat.com

On 9 February 2016 at 12:41, Ryan Harkin ryan.harkin@linaro.org wrote:

The variable TimeOut is actually a retry, not a timeout, so I renamed the variable accordingly.

This patch makes no functional change.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ryan Harkin ryan.harkin@linaro.org

Reviewed-by: Ard Biesheuvel ard.biesheuvel@linaro.org

---

EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c index 66d2b4b..f8f4a78 100644 --- a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c +++ b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118DxeUtil.c @@ -355,7 +355,7 @@ Lan9118Initialize ( IN EFI_SIMPLE_NETWORK_PROTOCOL *Snp ) {

• UINTN Timeout;

• UINTN Retries; UINT64 DefaultMacAddress;

  // Attempt to wake-up the device if it is in a lower power state

@@ -366,20 +366,20 @@ Lan9118Initialize ( }

// Check that device is active

• Timeout = 20;
• while ((MmioRead32 (LAN9118_PMT_CTRL) & MPTCTRL_READY) == 0 && --Timeout) {

• Retries = 20;
• while ((MmioRead32 (LAN9118_PMT_CTRL) & MPTCTRL_READY) == 0 && --Retries) { MemoryFence(); }

• if (!Timeout) {

• if (!Retries) { return EFI_TIMEOUT; }

  // Check that EEPROM isn't active

• Timeout = 20;
• while ((MmioRead32 (LAN9118_E2P_CMD) & E2P_EPC_BUSY) && --Timeout){

• Retries = 20;
• while ((MmioRead32 (LAN9118_E2P_CMD) & E2P_EPC_BUSY) && --Retries){ MemoryFence(); }

• if (!Timeout) {

• if (!Retries) { return EFI_TIMEOUT; }

@@ -571,7 +571,7 @@ AutoNegotiate ( UINT32 PhyControl; UINT32 PhyStatus; UINT32 Features;

• UINT32 TimeOut;

• UINT32 Retries;

  // First check that auto-negotiation is supported PhyStatus = IndirectPHYRead32 (PHY_INDEX_BASIC_STATUS);

@@ -583,12 +583,12 @@ AutoNegotiate ( // Check that link is up first if ((PhyStatus & PHYSTS_LINK_STS) == 0) { // Wait until it is up or until Time Out

• TimeOut = 2000;

• Retries = 2000; while ((IndirectPHYRead32 (PHY_INDEX_BASIC_STATUS) & PHYSTS_LINK_STS) == 0) { MemoryFence(); gBS->Stall ( FixedPcdGet32 (PcdLan9118DefaultNegotiationTimeout) );

•  TimeOut--;
  

•  if (!TimeOut) {
  

•  Retries--;
  

•  if (!Retries) {
     DEBUG ((EFI_D_ERROR, "Link timeout in auto-negotiation.\n"));
     return EFI_TIMEOUT;
   }
  

-- 2.1.4