On 02/08/16 18:59, Ryan Harkin wrote: > Add a PCD for the default 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 Laszo 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 2000 seems to work 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/Drivers/Lan9118Dxe/Lan9118Dxe.h | 2 +- > EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.inf | 1 + > EmbeddedPkg/EmbeddedPkg.dec | 1 + > 3 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.h > b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.h > index cc883e8..e5318da 100644 > --- a/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.h > +++ b/EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.h > @@ -38,7 +38,7 @@ > #include "Lan9118DxeUtil.h" > #include "Lan9118DxeHw.h" > > -#define LAN9118_STALL 2 > +#define LAN9118_STALL (FixedPcdGet64 > (PcdLan9118DefaultNegotiationTimeout)) > > #define LAN9118_DEFAULT_MAC_ADDRL 0x00F70200 > #define LAN9118_DEFAULT_MAC_ADDRH 0x00009040 > 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/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec > index f557527..338bdd0 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|UINT64|0x00000027 > > # > # Android FastBoot >
(1) There's a serious problem with this patch. You made a typo in my name. :) (Thanks for the credit though :)) (2) I noticed LAN9118_STALL is used in a bunch of places in this driver. Since those all use the same macro, I think it makes sense to set the macro from a PCD for all of them too. So that's good. (3) gBS->Stall() takes a UINTN. I propose to make the PCD UINT32, in order to match 32-bit builds better. (4) 2000 looks like a pretty big value, which may not be appropriate for all uses of LAN9118_STALL. OTOH that value is only mentioned in the commit message, it's not a code change. So that's fine too I think. Thanks! Laszlo "sneaky name" Ersek _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
