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

Reply via email to