Revert "EmbeddedPkg/Lan9118Dxe: use MemoryFence"

Commit a4626006bb ("EmbeddedPkg/Lan9118Dxe: use MemoryFence")
replaced some stalls with memory fences, on the presumption that these
were erroneously being used to order memory accesses. However, this was
not the case.

LAN9118 devices require a timing delay between state-changing
reads/writes and subsequent reads, as updates to the register file are
asynchronous and the effects of state-changes are not immediately
visible to subsequent reads.

This delay cannot be ensured through the use of memory barriers, which
only enforce observable ordering, and not timing. Thus, converting these
stalls to memory fences was erroneous, and may result in stale values
being read.

This reverts commit a4626006bb.

Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Ryan Harkin <ryan.harkin@linaro.org>
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
This commit is contained in:
Mark Rutland 2016-05-06 18:19:06 +01:00 committed by Ard Biesheuvel
parent d1ec2b2f78
commit 28f52b9fae
2 changed files with 23 additions and 26 deletions

View File

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

View File

@ -236,7 +236,7 @@ IndirectEEPROMRead32 (
// Write to Eeprom command register
MmioWrite32 (LAN9118_E2P_CMD, EepromCmd);
MemoryFence();
gBS->Stall (LAN9118_STALL);
// 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);
MemoryFence();
gBS->Stall (LAN9118_STALL);
// Wait until operation has completed
while (MmioRead32 (LAN9118_E2P_CMD) & E2P_EPC_BUSY);
@ -362,14 +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);
MemoryFence();
gBS->Stall (LAN9118_STALL);
}
// Check that device is active
Retries = 20;
while ((MmioRead32 (LAN9118_PMT_CTRL) & MPTCTRL_READY) == 0 && --Retries) {
gBS->Stall (LAN9118_STALL);
MemoryFence();
}
if (!Retries) {
return EFI_TIMEOUT;
@ -379,7 +378,6 @@ Lan9118Initialize (
Retries = 20;
while ((MmioRead32 (LAN9118_E2P_CMD) & E2P_EPC_BUSY) && --Retries){
gBS->Stall (LAN9118_STALL);
MemoryFence();
}
if (!Retries) {
return EFI_TIMEOUT;
@ -449,12 +447,11 @@ SoftReset (
// Write the configuration
MmioWrite32 (LAN9118_HW_CFG, HwConf);
MemoryFence();
gBS->Stall (LAN9118_STALL);
// Wait for reset to complete
while (MmioRead32 (LAN9118_HW_CFG) & HWCFG_SRST) {
MemoryFence();
gBS->Stall (LAN9118_STALL);
ResetTime += 1;
@ -503,7 +500,7 @@ PhySoftReset (
// Wait for completion
while (MmioRead32 (LAN9118_PMT_CTRL) & MPTCTRL_PHY_RST) {
MemoryFence();
gBS->Stall (LAN9118_STALL);
}
// PHY Basic Control Register reset
} else if (Flags & PHY_RESET_BCR) {
@ -511,7 +508,7 @@ PhySoftReset (
// Wait for completion
while (IndirectPHYRead32 (PHY_INDEX_BASIC_CTRL) & PHYCR_RESET) {
MemoryFence();
gBS->Stall (LAN9118_STALL);
}
}
@ -545,7 +542,7 @@ ConfigureHardware (
// Write the configuration
MmioWrite32 (LAN9118_GPIO_CFG, GpioConf);
MemoryFence();
gBS->Stall (LAN9118_STALL);
}
return EFI_SUCCESS;
@ -588,7 +585,6 @@ AutoNegotiate (
// Wait until it is up or until Time Out
Retries = FixedPcdGet32 (PcdLan9118DefaultNegotiationTimeout) / LAN9118_STALL;
while ((IndirectPHYRead32 (PHY_INDEX_BASIC_STATUS) & PHYSTS_LINK_STS) == 0) {
MemoryFence();
gBS->Stall (LAN9118_STALL);
Retries--;
if (!Retries) {
@ -675,7 +671,7 @@ StopTx (
TxCfg = MmioRead32 (LAN9118_TX_CFG);
TxCfg |= TXCFG_TXS_DUMP | TXCFG_TXD_DUMP;
MmioWrite32 (LAN9118_TX_CFG, TxCfg);
MemoryFence();
gBS->Stall (LAN9118_STALL);
}
// Check if already stopped
@ -694,7 +690,7 @@ StopTx (
if (TxCfg & TXCFG_TX_ON) {
TxCfg |= TXCFG_STOP_TX;
MmioWrite32 (LAN9118_TX_CFG, TxCfg);
MemoryFence();
gBS->Stall (LAN9118_STALL);
// Wait for Tx to finish transmitting
while (MmioRead32 (LAN9118_TX_CFG) & TXCFG_STOP_TX);
@ -729,7 +725,7 @@ StopRx (
RxCfg = MmioRead32 (LAN9118_RX_CFG);
RxCfg |= RXCFG_RX_DUMP;
MmioWrite32 (LAN9118_RX_CFG, RxCfg);
MemoryFence();
gBS->Stall (LAN9118_STALL);
while (MmioRead32 (LAN9118_RX_CFG) & RXCFG_RX_DUMP);
}
@ -755,28 +751,28 @@ StartTx (
TxCfg = MmioRead32 (LAN9118_TX_CFG);
TxCfg |= TXCFG_TXS_DUMP | TXCFG_TXD_DUMP;
MmioWrite32 (LAN9118_TX_CFG, TxCfg);
MemoryFence();
gBS->Stall (LAN9118_STALL);
}
// Check if tx was started from MAC and enable if not
if (Flags & START_TX_MAC) {
MacCsr = IndirectMACRead32 (INDIRECT_MAC_INDEX_CR);
MemoryFence();
gBS->Stall (LAN9118_STALL);
if ((MacCsr & MACCR_TX_EN) == 0) {
MacCsr |= MACCR_TX_EN;
IndirectMACWrite32 (INDIRECT_MAC_INDEX_CR, MacCsr);
MemoryFence();
gBS->Stall (LAN9118_STALL);
}
}
// Check if tx was started from TX_CFG and enable if not
if (Flags & START_TX_CFG) {
TxCfg = MmioRead32 (LAN9118_TX_CFG);
MemoryFence();
gBS->Stall (LAN9118_STALL);
if ((TxCfg & TXCFG_TX_ON) == 0) {
TxCfg |= TXCFG_TX_ON;
MmioWrite32 (LAN9118_TX_CFG, TxCfg);
MemoryFence();
gBS->Stall (LAN9118_STALL);
}
}
@ -806,14 +802,14 @@ StartRx (
RxCfg = MmioRead32 (LAN9118_RX_CFG);
RxCfg |= RXCFG_RX_DUMP;
MmioWrite32 (LAN9118_RX_CFG, RxCfg);
MemoryFence();
gBS->Stall (LAN9118_STALL);
while (MmioRead32 (LAN9118_RX_CFG) & RXCFG_RX_DUMP);
}
MacCsr |= MACCR_RX_EN;
IndirectMACWrite32 (INDIRECT_MAC_INDEX_CR, MacCsr);
MemoryFence();
gBS->Stall (LAN9118_STALL);
}
return EFI_SUCCESS;
@ -1003,7 +999,7 @@ ChangeFifoAllocation (
HwConf &= ~(0xF0000);
HwConf |= ((TxFifoOption & 0xF) << 16);
MmioWrite32 (LAN9118_HW_CFG, HwConf);
MemoryFence();
gBS->Stall (LAN9118_STALL);
return EFI_SUCCESS;
}