On 9 February 2016 at 11:56, Laszlo Ersek <[email protected]> wrote: > 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 <[email protected]> >> --- >> 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. >
Good point, I've removed it locally. I'll repost when the feedback is gathered. > Reviewed-by: Laszlo Ersek <[email protected]> Thanks :) _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel

