diff mbox

[edk2,02/10] MdeModulePkg//ArpDxe: Retrieved SnpMode only after configuring Snp

Message ID 0877601216922E4B83A7129715B5DA2B9A5717850F@GEORGE.Emea.Arm.com
State New
Headers show

Commit Message

Olivier Martin Oct. 31, 2013, 12:35 a.m. UTC 
Hi Fu,

as you said according to the UEFI spec "all the fields in Snp.Mode structure must be discovered during driver initialization".

I sent an email to USWG and the Network subteam to ask for clarification about this statement a few months ago. And no one has replied so far. So my guess is everyone is confused by the statement of the UEFI spec!
My next step is to send an ECR to clarify the statement in the UEFI spec and make 'Snp->Initialize()' responsability to initialize the structure...
If USWG prefer the driver binding start function be responsible for the initialization then I would like to see this clarification in the UEFI spec.

Thanks,
Olivier

Comments

Laszlo Ersek Dec. 15, 2013, 11:04 a.m. UTC | #1 
On 10/31/13 01:35, Olivier Martin wrote:
> Hi Fu,
> 
> as you said according to the UEFI spec "all the fields in Snp.Mode
> structure must be discovered during driver initialization".
> 
> I sent an email to USWG and the Network subteam to ask for
> clarification about this statement a few months ago. And no one has
> replied so far. So my guess is everyone is confused by the statement
> of the UEFI spec!
> My next step is to send an ECR to clarify the statement in the UEFI
> spec and make 'Snp->Initialize()' responsability to initialize the
> structure...
> If USWG prefer the driver binding start function be responsible for
> the initialization then I would like to see this clarification in the
> UEFI spec.

This is a good question.

OvmfPkg/VirtioNetDxe does populate Snp.Mode in the driver binding start
function. But in order to do so, as it must publish the MAC address and
the media present status, it needs to bring up the card temporarily (and
then shut it down).

VirtioNetDriverBindingStart() [OvmfPkg/VirtioNetDxe/DriverBinding.c]
  VirtioNetSnpPopulate()
    VirtioNetGetFeatures()

I assume this would be possible in the LAN9118 / LAN91x drivers too
(although perhaps somewhat inconvenient).

BTW is this the most recent version of the patchset?

Thanks
Laszlo
diff mbox

Patch

diff --git a/MdeModulePkg/Universal/Network/ArpDxe/ArpDriver.c b/MdeModulePkg/Universal/Network/ArpDxe/ArpDriver.c
old mode 100644
new mode 100755
index 81ddd62..5cf717f
--- a/MdeModulePkg/Universal/Network/ArpDxe/ArpDriver.c
+++ b/MdeModulePkg/Universal/Network/ArpDxe/ArpDriver.c
@@ -103,22 +103,6 @@  ArpCreateService (
   }

   //
-  // Get the underlayer Snp mode data.
-  //
-  Status = ArpService->Mnp->GetModeData (ArpService->Mnp, NULL, &ArpService->SnpMode);
-  if ((Status != EFI_NOT_STARTED) && EFI_ERROR (Status)) {
-    goto ERROR_EXIT;
-  }
-
-  if (ArpService->SnpMode.IfType != NET_IFTYPE_ETHERNET) {
-    //
-    // Only support the ethernet.
-    //
-    Status = EFI_UNSUPPORTED;
-    goto ERROR_EXIT;
-  }
-
-  //
   // Set the Mnp config parameters.
   //
   ArpService->MnpConfigData.ReceivedQueueTimeoutValue = 0; @@ -141,6 +125,23 @@ ArpCreateService (