public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Ard Biesheuvel" <ard.biesheuvel@linaro.org>
To: tzy.way.ooi@intel.com
Cc: edk2-devel-groups-io <devel@edk2.groups.io>,
	Leif Lindholm <leif.lindholm@linaro.org>,
	 Michael D Kinney <michael.d.kinney@intel.com>,
	Loh Tien Hock <tien.hock.loh@intel.com>
Subject: Re: [PATCH v4 edk2-platforms 1/1] Silicon/DesignWare/Driver: DwEmacSnpDxe: Add DesignWare EMAC driver
Date: Fri, 17 May 2019 18:25:20 +0200	[thread overview]
Message-ID: <CAKv+Gu8Ka2MSu8XX8o=iDHe8VkJeBaWZr-gg69Qk=0=18jJqTQ@mail.gmail.com> (raw)
In-Reply-To: <CAKv+Gu-A1quvkUY+zHwDm6uyM6GepZNc5fP5x5gNe8GB_vT+0Q@mail.gmail.com>

(resending due to bounce)

On Thu, 16 May 2019 at 21:37, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> On Wed, 8 May 2019 at 11:22, <tzy.way.ooi@intel.com> wrote:
> >
> > From: Ooi Tzy Way <tzy.way.ooi@intel.com>
> >
> > Add driver support for the Ethernet MAC based on Synopsys DesignWare
> > 3504-0 Universal 10/100/1000 Ethernet MAC and KSZ9031 PHY
> >
>
> Are these discrete components? If so, I wonder whether we should use some
> kind of abstraction for the PHY operations.
>
> > Cc: Ard BieSheuvel<ard.biesheuvel@linaro.org>
> > Cc: Leif Lindholm<leif.lindholm@linaro.org>
> > Cc: Michael D Kinney<michael.d.kinney@intel.com>
> > Cc: Loh Tien Hock<tien.hock.loh@intel.com>
>
> Please use spaces before <
>
> >
> > Contributed-under: Tianocore Contribution Agreement 1.1
> > Signed-off-by: Ooi Tzy Way <tzy.way.ooi@intel.com>
> >
> > ---
> > v4:
> > - Describe the device with non-discoverable device protocol
> > - Expose the MMIO base address in the EFI ACPI address space descriptor
> > - Remove cache maintainence and use DmaLib for non-coherent DMA access
> > ---
> >  Silicon/DesignWare/DesignWare.dec                        |   34 +
> >  Silicon/DesignWare/Drivers/DwEmacSnpDxe/DwEmacSnpDxe.inf |   82 ++
> >  Silicon/DesignWare/Drivers/DwEmacSnpDxe/DwEmacSnpDxe.h   |  229 ++++
> >  Silicon/DesignWare/Drivers/DwEmacSnpDxe/EmacDxeUtil.h    |  392 ++++++
> >  Silicon/DesignWare/Drivers/DwEmacSnpDxe/PhyDxeUtil.h     |  341 ++++++
> >  Silicon/DesignWare/Drivers/DwEmacSnpDxe/ComponentName.c  |  327 +++++
> >  Silicon/DesignWare/Drivers/DwEmacSnpDxe/DriverBinding.c  |  450 +++++++
> >  Silicon/DesignWare/Drivers/DwEmacSnpDxe/DwEmacSnpDxe.c   | 1273 ++++++++++++++++++++
> >  Silicon/DesignWare/Drivers/DwEmacSnpDxe/EmacDxeUtil.c    |  684 +++++++++++
> >  Silicon/DesignWare/Drivers/DwEmacSnpDxe/PhyDxeUtil.c     |  621 ++++++++++
> >  10 files changed, 4433 insertions(+)
> >
> > diff --git a/Silicon/DesignWare/DesignWare.dec b/Silicon/DesignWare/DesignWare.dec
> > new file mode 100755
> > index 000000000000..bad81fb6a023
> > --- /dev/null
> > +++ b/Silicon/DesignWare/DesignWare.dec
> > @@ -0,0 +1,34 @@
> > +  #Copyright (c) 2011 - 2019, Intel Corporaton. All rights reserved.
> > +  #This program and the accompanying materials
> > +  #are licensed and made available under the terms and conditions of the BSD License
> > +  #which accompanies this distribution.  The full text of the license may be found at
> > +  #http://opensource.org/licenses/bsd-license.php
> > +  #THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > +  #WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > +
> > +[Defines]
> > +  DEC_SPECIFICATION              = 0x00010005
>
> Please use the most recent version for EDK2 specific file formats (Note that
> you can use, e.g.,  1.27 instead of 0x0001001B)
>
> > +  PACKAGE_NAME                   = DesignWarePkg
> > +  PACKAGE_GUID                   = 3b5936d8-c72d-412d-b4d9-3bf0dea73598
> > +  PACKAGE_VERSION                = 0.1
> > +
> > +################################################################################
> > +#
> > +# Include Section - list of Include Paths that are provided by this package.
> > +#                   Comments are used for Keywords and Module Types.
> > +#
> > +# Supported Module Types:
> > +#  BASE SEC PEI_CORE PEIM DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER DXE_SAL_DRIVER UEFI_DRIVER UEFI_APPLICATION
> > +#
> > +################################################################################
> > +
> > +[Guids.common]
> > +  gDesignWareTokenSpaceGuid = { 0x89cb1241, 0xd283, 0x4543, { 0x88, 0x9c, 0x6b, 0x62, 0x36, 0x1a, 0x95, 0x7a } }
> > +  gDwEmacNetNonDiscoverableDeviceGuid = { 0x401950CD, 0xF9CD, 0x4A65, { 0xAD, 0x8E, 0x84, 0x9F, 0x3B, 0xAF, 0x23, 0x04 } }
> > +
> > +
> > +[PcdsFixedAtBuild.common]
> > +  gDesignWareTokenSpaceGuid.PcdDwEmacDefaultMacAddress|0|UINT64|0x30000050
> > +  gDesignWareTokenSpaceGuid.PcdDwEmacDxeBaseAddress|0|UINT32|0x30000051
> > +
>
> The base address should not be specified via a PCD. This is what I was trying
> to explain when I told you to use the driver model: it is the *platform* that
> instantiates the non-discoverable device protocol, and this driver only binds
> to it using the driver model. The protocol should not be instantiated by this
> driver.
>
> > +
> > diff --git a/Silicon/DesignWare/Drivers/DwEmacSnpDxe/DwEmacSnpDxe.inf b/Silicon/DesignWare/Drivers/DwEmacSnpDxe/DwEmacSnpDxe.inf
> > new file mode 100755
> > index 000000000000..09e7f1e77f69
> > --- /dev/null
> > +++ b/Silicon/DesignWare/Drivers/DwEmacSnpDxe/DwEmacSnpDxe.inf
> > @@ -0,0 +1,82 @@
> > +#/** @file
> > +#  DW Emac Simple Networking Protocol Driver (SNP) DXE Driver
> > +#
> > +#  Copyright (c) 2011 - 2019, Intel Corporaton. All rights reserved.
> > +#  This program and the accompanying materials
> > +#  are licensed and made available under the terms and conditions of the BSD License
> > +#  which accompanies this distribution.  The full text of the license may be found at
> > +#  http://opensource.org/licenses/bsd-license.php
> > +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > +#
> > +#  The original software modules are licensed as follows:
> > +#
> > +#  Copyright (c) 2012-2014, ARM Limited. All rights reserved.
> > +#
> > +#  This program and the accompanying materials
> > +#  are licensed and made available under the terms and conditions of the BSD License
> > +#  which accompanies this distribution.  The full text of the license may be found at
> > +#  http://opensource.org/licenses/bsd-license.php
> > +#
> > +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > +#
> > +#**/
> > +[Defines]
> > +  INF_VERSION                    = 0x00010005
>
> Please use the latest version of the INF spec
>
> > +  BASE_NAME                      = DwEmacSnpDxe
> > +  FILE_GUID                      = 06f3315f-9fe6-4938-b83f-2c072af802ba
> > +  MODULE_TYPE                    = DXE_DRIVER
>
> This should be a UEFI driver not a DXE driver.
>
> > +  VERSION_STRING                 = 0.1
> > +  ENTRY_POINT                    = DwEmacSnpDxeEntry
> > +
> > +[Sources.common]
> > +  ComponentName.c
> > +  DriverBinding.c
> > +  DwEmacSnpDxe.c
> > +  EmacDxeUtil.c
> > +  PhyDxeUtil.c
> > +  DwEmacSnpDxe.h
> > +  EmacDxeUtil.h
> > +  PhyDxeUtil.h
> > +
> > +[Packages]
> > +  ArmPkg/ArmPkg.dec
>
> Why do you need ArmPkg?
>
> > +  EmbeddedPkg/EmbeddedPkg.dec
> > +  NetworkPkg/NetworkPkg.dec
> > +  MdeModulePkg/MdeModulePkg.dec
> > +  MdePkg/MdePkg.dec
> > +  Silicon/DesignWare/DesignWare.dec
> > +
> > +[LibraryClasses]
> > +  BaseLib
> > +  UefiLib
> > +  NetLib
> > +  UefiDriverEntryPoint
> > +  BaseMemoryLib
> > +  ArmLib
>
> Why do you need ArmLib?
>
> > +  IoLib
> > +  TimerLib
> > +  DevicePathLib
> > +  DebugLib
> > +  PrintLib
> > +  NonDiscoverableDeviceRegistrationLib
> > +  DmaLib
> > +
> > +[Protocols]
> > +  gEfiSimpleNetworkProtocolGuid
> > +  gEfiMetronomeArchProtocolGuid
> > +  gEfiPxeBaseCodeProtocolGuid
> > +  gEfiDevicePathProtocolGuid
> > +  gEfiDriverBindingProtocolGuid
> > +  gEdkiiNonDiscoverableDeviceProtocolGuid
> > +
> > +[Guids]
> > +  gDwEmacNetNonDiscoverableDeviceGuid
> > +
> > +[Pcd]
> > +  gDesignWareTokenSpaceGuid.PcdDwEmacDefaultMacAddress
> > +  gDesignWareTokenSpaceGuid.PcdDwEmacDxeBaseAddress
> > +
> > +[Depex]
> > +  TRUE
>
> You can drop this for a UEFI driver.
>
> > diff --git a/Silicon/DesignWare/Drivers/DwEmacSnpDxe/DwEmacSnpDxe.h b/Silicon/DesignWare/Drivers/DwEmacSnpDxe/DwEmacSnpDxe.h
> > new file mode 100755
> > index 000000000000..677fdad8bd5d
> > --- /dev/null
> > +++ b/Silicon/DesignWare/Drivers/DwEmacSnpDxe/DwEmacSnpDxe.h
> > @@ -0,0 +1,229 @@
> > +/** @file
> > +
> > +  Copyright (c) 2011 - 2019, Intel Corporaton. All rights reserved.
> > +  This program and the accompanying materials
> > +  are licensed and made available under the terms and conditions of the BSD License
> > +  which accompanies this distribution.  The full text of the license may be found at
> > +  http://opensource.org/licenses/bsd-license.php
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > +
> > +  The original software modules are licensed as follows:
> > +
> > +  Copyright (c) 2012-2014, ARM Limited. All rights reserved.
> > +
> > +  This program and the accompanying materials
> > +  are licensed and made available under the terms and conditions of the BSD License
> > +  which accompanies this distribution.  The full text of the license may be found at
> > +  http://opensource.org/licenses/bsd-license.php
> > +
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD 3 Clause LICENSE ON AN "AS IS" BASIS,
> > +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > +**/
> > +
> > +
> > +#ifndef __DWEMAC_SNP_DXE_H__
> > +#define __DWEMAC_SNP_DXE_H__
> > +
> > +#include <Uefi.h>
> > +#include <Uefi/UefiSpec.h>
> > +#include <Base.h>
>
>
> Do you really need all of the above?
>
> > +
> > +// Protocols used by this driver
> > +#include <Protocol/SimpleNetwork.h>
> > +#include <Protocol/ComponentName2.h>
> > +#include <Protocol/PxeBaseCode.h>
> > +#include <Protocol/DevicePath.h>
> > +#include <Protocol/NonDiscoverableDevice.h>
> > +
> > +// Libraries used by this driver
> > +#include <Library/UefiLib.h>
> > +#include <Library/DebugLib.h>
> > +#include <Library/UefiBootServicesTableLib.h>
> > +#include <Library/MemoryAllocationLib.h>
> > +#include <Library/IoLib.h>
> > +#include <Library/PcdLib.h>
> > +#include <Library/NetLib.h>
> > +#include <Library/DevicePathLib.h>
> > +#include <Library/ArmLib.h>
>
> Why do you need ArmLib.h?
>
> > +#include <Library/PrintLib.h>
> > +#include <Library/DmaLib.h>
> > +
> > +#include "PhyDxeUtil.h"
> > +#include "EmacDxeUtil.h"
> > +
> > +/*------------------------------------------------------------------------------
> > +  Information Structure
> > +------------------------------------------------------------------------------*/
> > +
> > +typedef struct {
> > +  MAC_ADDR_DEVICE_PATH      MacAddrDP;
> > +  EFI_DEVICE_PATH_PROTOCOL  End;
> > +} EFI_SIMPLE_NETWORK_DEVICE_PATH;
>
> Don't use a EFI_ prefix for driver local data structures.
>
> > +
> > +typedef struct {
> > +  // Driver signature
> > +  UINT32            Signature;
> > +  EFI_HANDLE        ControllerHandle;
> > +
> > +  // EFI SNP protocol instances
> > +  EFI_SIMPLE_NETWORK_PROTOCOL Snp;
> > +  EFI_SIMPLE_NETWORK_MODE SnpMode;
> > +
> > +  // EFI Snp statistics instance
> > +  EFI_NETWORK_STATISTICS Stats;
> > +
> > +  EMAC_DRIVER  MacDriver;
> > +  PHY_DRIVER   PhyDriver;
> > +
> > +  NON_DISCOVERABLE_DEVICE           *Dev;
> > +
> > +  EFI_LOCK                          Lock;
> > +
> > +  // Array of the recycled transmit buffer address
> > +  UINT64                 *RecycledTxBuf;
> > +
> > +  // The maximum number of recycled buffer pointers in RecycledTxBuf.
> > +  UINT32                 MaxRecycledTxBuf;
> > +
> > +  // Current number of recycled buffer pointers in RecycledTxBuf.
> > +  UINT32                 RecycledTxBufCount;
> > +
> > +  // For DmaUnmap
> > +  VOID                            *MappingTxdesc;
> > +  VOID                            *MappingRxdesc;
> > +  VOID                            *MappingTxbuf;
> > +  VOID                            *MappingRxbuf;
> > +
>
> Please clean up the indentation of the member names: use the same level for all.
>
> > +} EFI_SIMPLE_NETWORK_DRIVER;
> > +
>
> Don't use a EFI_ prefix for driver local data structures.
>
> > +extern EFI_COMPONENT_NAME_PROTOCOL    gSnpComponentName;
> > +extern EFI_COMPONENT_NAME2_PROTOCOL   gSnpComponentName2;
> > +
> > +
> > +#define SNP_DRIVER_SIGNATURE                    SIGNATURE_32('A', 'S', 'N', 'P')
> > +#define INSTANCE_FROM_SNP_THIS(a)               CR(a, EFI_SIMPLE_NETWORK_DRIVER, Snp, SNP_DRIVER_SIGNATURE)
> > +#define SNP_TX_BUFFER_INCREASEMENT              32
>
> INCREASE not INCREASEMENT
>
> > +#define SNP_MAX_TX_BUFFER_NUM                   65536
> > +
> > +/*---------------------------------------------------------------------------------------------------------------------
> > +
> > +  UEFI-Compliant functions for EFI_SIMPLE_NETWORK_PROTOCOL
> > +
> > +  Refer to the Simple Network Protocol section (21.1) in the UEFI 2.3.1 Specification for related definitions
> > +
> > +---------------------------------------------------------------------------------------------------------------------*/
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +SnpStart (
> > +  IN        EFI_SIMPLE_NETWORK_PROTOCOL* Snp
> > +  );
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +SnpStop (
> > +  IN        EFI_SIMPLE_NETWORK_PROTOCOL* Snp
> > +  );
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +SnpInitialize (
> > +  IN EFI_SIMPLE_NETWORK_PROTOCOL *Snp,
> > +  IN UINTN                       ExtraRxBufferSize OPTIONAL,
> > +  IN UINTN                       ExtraTxBufferSize OPTIONAL
> > +  );
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +SnpReset (
> > +  IN EFI_SIMPLE_NETWORK_PROTOCOL *Snp,
> > +  IN BOOLEAN                     ExtendedVerification
> > +  );
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +SnpShutdown (
> > +  IN        EFI_SIMPLE_NETWORK_PROTOCOL* Snp
> > +  );
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +SnpReceiveFilters (
> > +  IN  EFI_SIMPLE_NETWORK_PROTOCOL  *Snp,
> > +  IN  UINT32                       Enable,
> > +  IN  UINT32                       Disable,
> > +  IN  BOOLEAN                      ResetMCastFilter,
> > +  IN  UINTN                        MCastFilterCnt  OPTIONAL,
> > +  IN  EFI_MAC_ADDRESS              *MCastFilter  OPTIONAL
> > +  );
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +SnpStationAddress (
> > +  IN        EFI_SIMPLE_NETWORK_PROTOCOL *Snp,
> > +  IN        BOOLEAN                      Reset,
> > +  IN        EFI_MAC_ADDRESS             *NewMac
> > +);
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +SnpStatistics (
> > +  IN        EFI_SIMPLE_NETWORK_PROTOCOL *Snp,
> > +  IN        BOOLEAN                      Reset,
> > +  IN  OUT   UINTN                        *StatSize,
> > +      OUT   EFI_NETWORK_STATISTICS       *Statistics
> > +  );
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +SnpMcastIptoMac (
> > +  IN        EFI_SIMPLE_NETWORK_PROTOCOL  *Snp,
> > +  IN        BOOLEAN                      IsIpv6,
> > +  IN        EFI_IP_ADDRESS               *Ip,
> > +      OUT   EFI_MAC_ADDRESS              *McastMac
> > +  );
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +SnpNvData (
> > +  IN EFI_SIMPLE_NETWORK_PROTOCOL *Snp,
> > +  IN BOOLEAN                     ReadWrite,
> > +  IN UINTN                       Offset,
> > +  IN UINTN                       BufferSize,
> > +  IN OUT VOID                    *Buffer
> > +  );
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +SnpGetStatus (
> > +  IN  EFI_SIMPLE_NETWORK_PROTOCOL *Snp,
> > +  OUT UINT32                      *IrqStat  OPTIONAL,
> > +  OUT VOID                        **TxBuff  OPTIONAL
> > +  );
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +SnpTransmit (
> > +  IN  EFI_SIMPLE_NETWORK_PROTOCOL  *Snp,
> > +  IN  UINTN                        HdrSize,
> > +  IN  UINTN                        BuffSize,
> > +  IN  VOID*                        Data,
> > +  IN  EFI_MAC_ADDRESS              *SrcAddr  OPTIONAL,
> > +  IN  EFI_MAC_ADDRESS              *DstAddr  OPTIONAL,
> > +  IN  UINT16                       *Protocol OPTIONAL
> > +  );
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +SnpReceive (
> > +  IN        EFI_SIMPLE_NETWORK_PROTOCOL *Snp,
> > +      OUT   UINTN                       *HdrSize      OPTIONAL,
> > +  IN  OUT   UINTN                       *BuffSize,
> > +      OUT   VOID                        *Data,
> > +      OUT   EFI_MAC_ADDRESS             *SrcAddr      OPTIONAL,
> > +      OUT   EFI_MAC_ADDRESS             *DstAddr      OPTIONAL,
> > +      OUT   UINT16                      *Protocol     OPTIONAL
> > +  );
> > +
> > +#endif // __DWEMAC_SNP_DXE_H__
> > diff --git a/Silicon/DesignWare/Drivers/DwEmacSnpDxe/EmacDxeUtil.h b/Silicon/DesignWare/Drivers/DwEmacSnpDxe/EmacDxeUtil.h
> > new file mode 100755
> > index 000000000000..872923252e73
> > --- /dev/null
> > +++ b/Silicon/DesignWare/Drivers/DwEmacSnpDxe/EmacDxeUtil.h
> > @@ -0,0 +1,392 @@
> > +/** @file
> > +
> > +  Copyright (c) 2011 - 2019, Intel Corporaton. All rights reserved.
> > +  This program and the accompanying materials
> > +  are licensed and made available under the terms and conditions of the BSD License
> > +  which accompanies this distribution.  The full text of the license may be found at
> > +  http://opensource.org/licenses/bsd-license.php
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > +
> > +  The original software modules are licensed as follows:
> > +
> > +  Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
> > +  Copyright (c) 2011 - 2014, ARM Limited. All rights reserved.
> > +
> > +  This program and the accompanying materials
> > +  are licensed and made available under the terms and conditions of the BSD License
> > +  which accompanies this distribution.  The full text of the license may be found at
> > +  http://opensource.org/licenses/bsd-license.php
> > +
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD 3 Clause LICENSE ON AN "AS IS" BASIS,
> > +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > +**/
> > +
> > +
> > +#ifndef __EMAC_DXE_UTIL_H__
> > +#define __EMAC_DXE_UTIL_H__
> > +
> > +#include <libfdt.h>
>
> Why do you need this?
>
> > +#include <Library/BaseMemoryLib.h>
> > +#include <Library/UefiLib.h>
> > +#include <Library/DebugLib.h>
> > +#include <Library/IoLib.h>
> > +#include <Library/MemoryAllocationLib.h>
> > +#include <Library/PcdLib.h>
> > +#include <Library/PrintLib.h>
> > +#include <Library/TimerLib.h>
> > +#include <Library/NetLib.h>
> > +#include <Library/ArmLib.h>
>
> Why?
>
> > +
> > +// Protocols used by this driver
> > +#include <Protocol/SimpleNetwork.h>
> > +#include <Protocol/ComponentName2.h>
> > +#include <Protocol/PxeBaseCode.h>
> > +#include <Protocol/DevicePath.h>
> > +
> > +
> > +// Most common CRC32 Polynomial for little endian machines
> > +#define CRC_POLYNOMIAL         0xEDB88320
> > +#define HASH_TABLE_REG(n)      0x500 + (0x4 * n)
> > +#define RX_MAX_PACKET          1600
> > +
> > +#define CONFIG_ETH_BUFSIZE     2048
> > +#define CONFIG_TX_DESCR_NUM    10
> > +#define CONFIG_RX_DESCR_NUM    10
> > +#define TX_TOTAL_BUFSIZE       (CONFIG_ETH_BUFSIZE * CONFIG_TX_DESCR_NUM)
> > +#define RX_TOTAL_BUFSIZE       (CONFIG_ETH_BUFSIZE * CONFIG_RX_DESCR_NUM)
> > +
> > +// DMA status error bit
> > +#define RX_DMA_WRITE_DATA_TRANSFER_ERROR       0x0
> > +#define TX_DMA_READ_DATA_TRANSFER_ERROR        0x3
> > +#define RX_DMA_DESCRIPTOR_WRITE_ACCESS_ERROR   0x4
> > +#define TX_DMA_DESCRIPTOR_WRITE_ACCESS_ERROR   0x5
> > +#define RX_DMA_DESCRIPTOR_READ_ACCESS_ERROR    0x6
> > +#define TX_DMA_DESCRIPTOR_READ_ACCESS_ERROR    0x7
> > +
> > +// tx descriptor
> > +#define TDES0_OWN           BIT31
> > +#define TDES0_TXINT         BIT30
> > +#define TDES0_TXLAST        BIT29
> > +#define TDES0_TXFIRST       BIT28
> > +#define TDES0_TXCRCDIS      BIT27
> > +#define TDES0_TXRINGEND     BIT21
> > +#define TDES0_TXCHAIN       BIT20
> > +
> > +#define TDES1_SIZE1MASK     (0x1FFF << 0)
> > +#define TDES1_SIZE1SHFT     (0)
> > +#define TDES1_SIZE2MASK     (0x1FFF << 16)
> > +#define TDES1_SIZE2SHFT     (16)
> > +
> > +// rx descriptor
> > +#define RDES0_FL_MASK       0x3fff
> > +#define RDES0_FL_SHIFT      16
> > +#define RDES1_CHAINED       BIT14
> > +
> > +#define RDES0_CE            BIT1
> > +#define RDES0_DBE           BIT2
> > +#define RDES0_RE            BIT3
> > +#define RDES0_RWT           BIT4
> > +#define RDES0_LC            BIT6
> > +#define RDES0_GF            BIT7
> > +#define RDES0_OE            BIT11
> > +#define RDES0_LE            BIT12
> > +#define RDES0_SAF           BIT13
> > +#define RDES0_DE            BIT14
> > +#define RDES0_ES            BIT15
> > +#define RDES0_AFM           BIT30
> > +#define RDES0_OWN           BIT31
> > +
> > +
> > +// emac config phy interface setting
> > +#define PHY_INTERFACE_MODE_GMII      0
> > +#define PHY_INTERFACE_MODE_MII       1
> > +#define PHY_INTERFACE_MODE_RGMII     2
> > +#define PHY_INTERFACE_MODE_RMII      3
> > +
> > +// DW emac mask
> > +#define DW_EMAC_DMAGRP_BUS_MODE_SWR_SET_MSK             0x00000001
> > +#define DW_EMAC_DMAGRP_BUS_MODE_FB_SET_MSK              0x00010000
> > +#define DW_EMAC_DMAGRP_BUS_MODE_PBL_SET_MSK             0x00003f00
> > +#define DW_EMAC_DMAGRP_BUS_MODE_PR_SET_MSK              0x0000c000
> > +#define DW_EMAC_DMAGRP_OPERATION_MODE_FTF_SET_MSK       0x00100000
> > +#define DW_EMAC_DMAGRP_OPERATION_MODE_TSF_SET_MSK       0x00200000
> > +#define DW_EMAC_DMAGRP_INTERRUPT_ENABLE_TIE_SET_MSK     0x00000001
> > +#define DW_EMAC_DMAGRP_INTERRUPT_ENABLE_RIE_SET_MSK     0x00000040
> > +#define DW_EMAC_DMAGRP_INTERRUPT_ENABLE_NIE_SET_MSK     0x00010000
> > +#define DW_EMAC_DMAGRP_INTERRUPT_ENABLE_AIE_SET_MSK     0x00008000
> > +#define DW_EMAC_DMAGRP_INTERRUPT_ENABLE_FBE_SET_MSK     0x00002000
> > +#define DW_EMAC_DMAGRP_INTERRUPT_ENABLE_UNE_SET_MSK     0x00000020
> > +#define DW_EMAC_DMAGRP_INTERRUPT_ENABLE_TSE_SET_MSK     0x00000002
> > +#define DW_EMAC_DMAGRP_INTERRUPT_ENABLE_TUE_SET_MSK     0x00000004
> > +#define DW_EMAC_DMAGRP_INTERRUPT_ENABLE_TJE_SET_MSK     0x00000008
> > +#define DW_EMAC_DMAGRP_INTERRUPT_ENABLE_OVE_SET_MSK     0x00000010
> > +#define DW_EMAC_DMAGRP_INTERRUPT_ENABLE_RUE_SET_MSK     0x00000080
> > +#define DW_EMAC_DMAGRP_INTERRUPT_ENABLE_RSE_SET_MSK     0x00000100
> > +#define DW_EMAC_DMAGRP_INTERRUPT_ENABLE_RWE_SET_MSK     0x00000200
> > +#define DW_EMAC_DMAGRP_INTERRUPT_ENABLE_ETE_SET_MSK     0x00000400
> > +#define DW_EMAC_DMAGRP_INTERRUPT_ENABLE_ERE_SET_MSK     0x00004000
> > +#define DW_EMAC_DMAGRP_OPERATION_MODE_ST_SET_MSK        0x00002000
> > +#define DW_EMAC_DMAGRP_OPERATION_MODE_SR_SET_MSK        0x00000002
> > +#define DW_EMAC_GMACGRP_MAC_CONFIGURATION_RE_SET_MSK    0x00000004
> > +#define DW_EMAC_GMACGRP_MAC_CONFIGURATION_TE_SET_MSK    0x00000008
> > +#define DW_EMAC_GMACGRP_MAC_FRAME_FILTER_RESET          0x00000000
> > +#define DW_EMAC_GMACGRP_MAC_FRAME_FILTER_HMC_SET_MSK    0x00000004
> > +#define DW_EMAC_GMACGRP_MAC_FRAME_FILTER_DBF_SET_MSK    0x00000020
> > +#define DW_EMAC_GMACGRP_MAC_FRAME_FILTER_PR_SET_MSK     0x00000001
> > +#define DW_EMAC_GMACGRP_MAC_FRAME_FILTER_PM_SET_MSK     0x00000010
> > +#define DW_EMAC_DMAGRP_OPERATION_MODE_ST_CLR_MSK        0xffffdfff
> > +#define DW_EMAC_GMACGRP_MAC_CONFIGURATION_RE_CLR_MSK    0xfffffffb
> > +#define DW_EMAC_GMACGRP_MAC_CONFIGURATION_TE_CLR_MSK    0xfffffff7
> > +#define DW_EMAC_DMAGRP_OPERATION_MODE_SR_CLR_MSK        0xfffffffd
> > +#define DW_EMAC_DMAGRP_STATUS_NIS_SET_MSK               0x00010000
> > +#define DW_EMAC_DMAGRP_STATUS_RI_SET_MSK                0x00000040
> > +#define DW_EMAC_DMAGRP_STATUS_TI_SET_MSK                0x00000001
> > +#define DW_EMAC_DMAGRP_STATUS_TU_SET_MSK                0x00000004
> > +#define DW_EMAC_DMAGRP_STATUS_ERI_SET_MSK               0x00004000
> > +#define DW_EMAC_DMAGRP_STATUS_AIS_SET_MSK               0x00008000
> > +#define DW_EMAC_DMAGRP_STATUS_TPS_SET_MSK               0x00000002
> > +#define DW_EMAC_DMAGRP_STATUS_TJT_SET_MSK               0x00000008
> > +#define DW_EMAC_DMAGRP_STATUS_OVF_SET_MSK               0x00000010
> > +#define DW_EMAC_DMAGRP_STATUS_UNF_SET_MSK               0x00000020
> > +#define DW_EMAC_DMAGRP_STATUS_RU_SET_MSK                0x00000080
> > +#define DW_EMAC_DMAGRP_STATUS_RPS_SET_MSK               0x00000100
> > +#define DW_EMAC_DMAGRP_STATUS_RWT_SET_MSK               0x00000200
> > +#define DW_EMAC_DMAGRP_STATUS_ETI_SET_MSK               0x00000400
> > +#define DW_EMAC_DMAGRP_STATUS_FBI_SET_MSK               0x00002000
> > +#define DW_EMAC_GMACGRP_MAC_CONFIGURATION_PS_SET_MSK    0x00008000
> > +#define DW_EMAC_GMACGRP_MAC_CONFIGURATION_FES_SET_MSK   0x00004000
> > +#define DW_EMAC_GMACGRP_MAC_CONFIGURATION_DM_SET_MSK    0x00000800
> > +#define DW_EMAC_GMACGRP_MAC_CONFIGURATION_BE_SET_MSK    0x00200000
> > +#define DW_EMAC_GMACGRP_MAC_CONFIGURATION_DO_SET_MSK    0x00002000
> > +
> > +#define DW_EMAC_DMAGRP_BUS_MODE_SWR_GET(value) (((value) & 0x00000001) >> 0)
> > +#define DW_EMAC_DMAGRP_STATUS_EB_GET(value) (((value) & 0x03800000) >> 23)
> > +#define DW_EMAC_GMACGRP_GMII_ADDRESS_GB_GET(value) (((value) & 0x00000001) >> 0)
> > +#define DW_EMAC_GMACGRP_GMII_DATA_GD_GET(value) (((value) & 0x0000ffff) >> 0)
> > +#define DW_EMAC_DMAGRP_OPERATION_MODE_FTF_GET(value) (((value) & 0x00100000) >> 20)
> > +
> > +// DW emac registers offset
> > +//#define DWEMAC_OFST                                                                                                          ((UINT32)PcdGet32 (PcdDwEmacDxeBaseAddress))
> > +#define DW_EMAC_GMACGRP_MAC_CONFIGURATION_OFST                                         0x000
> > +#define DW_EMAC_GMACGRP_MAC_FRAME_FILTER_OFST                                          0x004
> > +#define DW_EMAC_GMACGRP_GMII_ADDRESS_OFST                                              0x010
> > +#define DW_EMAC_GMACGRP_GMII_DATA_OFST                                                 0x014
> > +#define DW_EMAC_GMACGRP_FLOW_CONTROL_OFST                                              0x018
> > +#define DW_EMAC_GMACGRP_VLAN_TAG_OFST                                                  0x01c
> > +#define DW_EMAC_GMACGRP_VERSION_OFST                                                   0x020
> > +#define DW_EMAC_GMACGRP_DEBUG_OFST                                                     0x024
> > +#define DW_EMAC_GMACGRP_LPI_CONTROL_STATUS_OFST                                        0x030
> > +#define DW_EMAC_GMACGRP_LPI_TIMERS_CONTROL_OFST                                        0x034
> > +#define DW_EMAC_GMACGRP_INTERRUPT_STATUS_OFST                                          0x038
> > +#define DW_EMAC_GMACGRP_INTERRUPT_MASK_OFST                                            0x03c
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS0_HIGH_OFST                                         0x040
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS0_LOW_OFST                                          0x044
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS1_HIGH_OFST                                         0x048
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS1_LOW_OFST                                          0x04c
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS2_HIGH_OFST                                         0x050
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS2_LOW_OFST                                          0x054
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS3_HIGH_OFST                                         0x058
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS3_LOW_OFST                                          0x05c
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS4_HIGH_OFST                                         0x060
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS4_LOW_OFST                                          0x064
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS5_HIGH_OFST                                         0x068
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS5_LOW_OFST                                          0x06c
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS6_HIGH_OFST                                         0x070
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS6_LOW_OFST                                          0x074
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS7_HIGH_OFST                                         0x078
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS7_LOW_OFST                                          0x07c
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS8_HIGH_OFST                                         0x080
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS8_LOW_OFST                                          0x084
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS9_HIGH_OFST                                         0x088
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS9_LOW_OFST                                          0x08c
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS10_HIGH_OFST                                        0x090
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS10_LOW_OFST                                         0x094
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS11_HIGH_OFST                                        0x098
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS11_LOW_OFST                                         0x09c
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS12_HIGH_OFST                                        0x0a0
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS12_LOW_OFST                                         0x0a4
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS13_HIGH_OFST                                        0x0a8
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS13_LOW_OFST                                         0x0ac
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS14_HIGH_OFST                                        0x0b0
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS14_LOW_OFST                                         0x0b4
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS15_HIGH_OFST                                        0x0b8
> > +#define DW_EMAC_GMACGRP_MAC_ADDRESS15_LOW_OFST                                         0x0bc
> > +#define DW_EMAC_GMACGRP_SGMII_RGMII_SMII_CONTROL_STATUS_OFST           0x0d8
> > +#define DW_EMAC_GMACGRP_WDOG_TIMEOUT_OFST                                              0x0dc
> > +#define DW_EMAC_GMACGRP_GENPIO_OFST                                                    0x0e0
> > +#define DW_EMAC_GMACGRP_MMC_CONTROL_OFST                                               0X100
> > +#define DW_EMAC_GMACGRP_MMC_RECEIVE_INTERRUPT_OFST                                     0x104
> > +#define DW_EMAC_GMACGRP_MMC_TRANSMIT_INTERRUPT_OFST                                    0x108
> > +#define DW_EMAC_GMACGRP_MMC_RECEIVE_INTERRUPT_MASK_OFST                                0x10c
> > +#define DW_EMAC_GMACGRP_MMC_TRANSMIT_INTERRUPT_MASK_OFST                               0x110
> > +#define DW_EMAC_GMACGRP_TXOCTETCOUNT_GB_OFST                                           0x114
> > +#define DW_EMAC_GMACGRP_TXFRAMECOUNT_GB_OFST                                           0x118
> > +#define DW_EMAC_GMACGRP_TXBROADCASTFRAMES_G_OFST                                       0x11c
> > +#define DW_EMAC_GMACGRP_TXMULTICASTFRAMES_G_OFST                                       0x120
> > +#define DW_EMAC_GMACGRP_TXUNICASTFRAMES_GB_OFST                                                0x13c
> > +#define DW_EMAC_GMACGRP_TXLATECOL_OFST                                                         0x158
> > +#define DW_EMAC_GMACGRP_TXEXESSCOL_OFST                                                                0x15c
> > +#define DW_EMAC_GMACGRP_TXFRAMECOUNT_G_OFST                                                    0x168
> > +#define DW_EMAC_GMACGRP_TXOVERSIZE_G_OFST                                                      0x178
> > +#define DW_EMAC_GMACGRP_RXFRAMECOUNT_GB_OFST                                           0x180
> > +#define DW_EMAC_GMACGRP_RXOCTETCOUNT_GB_OFST                                           0x184
> > +#define DW_EMAC_GMACGRP_RXBROADCASTFRAMES_G_OFST                                       0x18c
> > +#define DW_EMAC_GMACGRP_RXMULTICASTFRAMES_G_OFST                                       0x190
> > +#define DW_EMAC_GMACGRP_RXCRCERROR_OFST                                                                0x194
> > +#define DW_EMAC_GMACGRP_RXUNDERSIZE_G_OFST                                                     0x1a4
> > +#define DW_EMAC_GMACGRP_RXOVERSIZE_G_OFST                                                      0x1a8
> > +#define DW_EMAC_GMACGRP_RXUNICASTFRAMES_G_OFST                                         0x1c4
> > +#define DW_EMAC_DMAGRP_BUS_MODE_OFST                                                   0x1000
> > +#define DW_EMAC_DMAGRP_TRANSMIT_POLL_DEMAND_OFST                                       0x1004
> > +#define DW_EMAC_DMAGRP_RECEIVE_POLL_DEMAND_OFST                                        0x1008
> > +#define DW_EMAC_DMAGRP_RECEIVE_DESCRIPTOR_LIST_ADDRESS_OFST             0x100c
> > +#define DW_EMAC_DMAGRP_TRANSMIT_DESCRIPTOR_LIST_ADDRESS_OFST            0x1010
> > +#define DW_EMAC_DMAGRP_STATUS_OFST                                      0x1014
> > +#define DW_EMAC_DMAGRP_OPERATION_MODE_OFST                              0x1018
> > +#define DW_EMAC_DMAGRP_INTERRUPT_ENABLE_OFST                            0x101c
> > +#define DW_EMAC_DMAGRP_MISSED_FRAME_AND_BUFFER_OVERFLOW_COUNTER_OFST    0x1020
> > +#define DW_EMAC_DMAGRP_RECEIVE_INTERRUPT_WATCHDOG_TIMER_OFST            0x1024
> > +#define DW_EMAC_DMAGRP_AXI_BUS_MODE_OFST                                0x1028
> > +#define DW_EMAC_DMAGRP_AHB_OR_AXI_STATUS_OFST                           0x102c
> > +#define DW_EMAC_DMAGRP_CURRENT_HOST_TRANSMIT_DESCRIPTOR_OFST                   0x1048
> > +#define DW_EMAC_DMAGRP_CURRENT_HOST_RECEIVE_DESCRIPTOR_OFST                    0x104c
> > +#define DW_EMAC_DMAGRP_CURRENT_HOST_TRANSMIT_BUFFER_ADDRESS_OFST               0x1050
> > +#define DW_EMAC_DMAGRP_CURRENT_HOST_RECEIVE_BUFFER_ADDRESS_OFST                0x1054
> > +#define DW_EMAC_DMAGRP_HW_FEATURE_OFST                                         0x1058
> > +
> > +typedef struct {
> > +  UINT32 Tdes0;
> > +  UINT32 Tdes1;
> > +  UINT32 Addr;
> > +  UINT32 AddrNext;
> > +} DESIGNWARE_HW_DESCRIPTOR;
> > +
> > +
> > +typedef struct {
> > +  DESIGNWARE_HW_DESCRIPTOR  *TxdescRing[CONFIG_TX_DESCR_NUM];
> > +  DESIGNWARE_HW_DESCRIPTOR  *RxdescRing[CONFIG_RX_DESCR_NUM];
> > +  CHAR8                     *TxBuffer[TX_TOTAL_BUFSIZE];
> > +  CHAR8                     *RxBuffer[RX_TOTAL_BUFSIZE];
> > +  EFI_PHYSICAL_ADDRESS      TxdescRingMap[CONFIG_TX_DESCR_NUM];
> > +  EFI_PHYSICAL_ADDRESS      RxdescRingMap[CONFIG_RX_DESCR_NUM];
> > +  EFI_PHYSICAL_ADDRESS      TxBufferMap[TX_TOTAL_BUFSIZE];
> > +  EFI_PHYSICAL_ADDRESS      RxBufferMap[RX_TOTAL_BUFSIZE];
> > +  UINT32                   TxCurrentDescriptorNum;
> > +  UINT32                   TxNextDescriptorNum;
> > +  UINT32                   RxCurrentDescriptorNum;
> > +  UINT32                   RxNextDescriptorNum;
> > +} EMAC_DRIVER;
> > +
> > +VOID
> > +EFIAPI
> > +EmacChooseInterface (
> > +  IN UINT32 MacInterface
> > +  );
> > +
> > +VOID
> > +EFIAPI
> > +EmacSetMacAddress (
> > +  IN EFI_MAC_ADDRESS  *MacAddress,
> > +  IN UINT32           MacBaseAddress
> > +  );
> > +
> > +VOID
> > +EFIAPI
> > +EmacReadMacAddress (
> > +  OUT EFI_MAC_ADDRESS *MacAddress,
> > +  IN UINT32           MacBaseAddress
> > +  );
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +EmacDxeInitialization (
> > +  IN EMAC_DRIVER *EmacDriver,
> > +  IN UINT32      MacBaseAddress
> > +  );
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +EmacDmaInit (
> > +  IN EMAC_DRIVER *EmacDriver,
> > +  IN UINT32      MacBaseAddress
> > +  );
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +EmacSetupTxdesc (
> > +  IN EMAC_DRIVER *EmacDriver,
> > +  IN UINT32      MacBaseAddress
> > + );
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +EmacSetupRxdesc (
> > +  IN EMAC_DRIVER *EmacDriver,
> > +  IN UINT32      MacBaseAddress
> > +  );
> > +
> > +VOID
> > +EFIAPI
> > +EmacStartTransmission (
> > +  IN UINT32           MacBaseAddress
> > +  );
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +EmacRxFilters (
> > +  IN        UINT32           ReceiveFilterSetting,
> > +  IN        BOOLEAN          Reset,
> > +  IN        UINTN            NumMfilter             OPTIONAL,
> > +  IN        EFI_MAC_ADDRESS *Mfilter                OPTIONAL,
> > +  IN        UINT32           MacBaseAddress
> > +  );
> > +
> > +UINT32
> > +EFIAPI
> > +GenEtherCrc32 (
> > +  IN    EFI_MAC_ADDRESS *Mac,
> > +  IN    UINT32 AddrLen
> > +  );
> > +
> > +UINT8
> > +EFIAPI
> > +BitReverse (
> > +  UINT8 X
> > +  );
> > +
> > +VOID
> > +EFIAPI
> > +EmacStopTxRx (
> > +  IN          UINT32        MacBaseAddress
> > +  );
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +EmacDmaStart (
> > +  IN          UINT32        MacBaseAddress
> > +  );
> > +
> > +VOID
> > +EFIAPI
> > +EmacGetDmaStatus (
> > +  OUT   UINT32 *IrqStat  OPTIONAL,
> > +  IN    UINT32           MacBaseAddress
> > +  );
> > +
> > +VOID
> > +EFIAPI
> > +EmacGetStatistic (
> > +  IN EFI_NETWORK_STATISTICS* Stats,
> > +  IN UINT32                  MacBaseAddress
> > +  );
> > +
> > +VOID
> > +EFIAPI
> > +EmacConfigAdjust (
> > +  IN UINT32 Speed,
> > +  IN UINT32 Duplex,
> > +  IN UINT32 MacBaseAddress
> > +  );
> > +
> > +#endif // __EMAC_DXE_UTIL_H__
> > diff --git a/Silicon/DesignWare/Drivers/DwEmacSnpDxe/PhyDxeUtil.h b/Silicon/DesignWare/Drivers/DwEmacSnpDxe/PhyDxeUtil.h
> > new file mode 100755
> > index 000000000000..6fff3d61688a
> > --- /dev/null
> > +++ b/Silicon/DesignWare/Drivers/DwEmacSnpDxe/PhyDxeUtil.h
> > @@ -0,0 +1,341 @@
> > +/** @file
> > +
> > +  Copyright (c) 2011 - 2019, Intel Corporaton. All rights reserved.
> > +  This program and the accompanying materials
> > +  are licensed and made available under the terms and conditions of the BSD License
> > +  which accompanies this distribution.  The full text of the license may be found at
> > +  http://opensource.org/licenses/bsd-license.php
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > +
> > +  The original software modules are licensed as follows:
> > +
> > +  Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
> > +  Copyright (c) 2011 - 2014, ARM Limited. All rights reserved.
> > +
> > +  This program and the accompanying materials
> > +  are licensed and made available under the terms and conditions of the BSD License
> > +  which accompanies this distribution.  The full text of the license may be found at
> > +  http://opensource.org/licenses/bsd-license.php
> > +
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD 3 Clause LICENSE ON AN "AS IS" BASIS,
> > +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > +**/
> > +
> > +
> > +#ifndef __PHY_DXE_H__
> > +#define __PHY_DXE_H__
> > +
> > +#include <libfdt.h>
>
> Why?
>
> > +#include <Library/BaseMemoryLib.h>
> > +#include <Library/DebugLib.h>
> > +#include <Library/IoLib.h>
> > +#include <Library/MemoryAllocationLib.h>
> > +#include <Library/PcdLib.h>
> > +#include <Library/PrintLib.h>
> > +#include <Library/TimerLib.h>
> > +
>
> In general, please only include headers that your code actually uses.
>
> > +typedef struct {
> > +  UINT32 PhyAddr;
> > +  UINT32 PhyCurrentLink;
> > +  UINT32 PhyOldLink;
> > +} PHY_DRIVER;
> > +
> > +
> > +//
> > +// PHY Registers
> > +//
> > +#define PHY_BASIC_CTRL                     0
> > +#define PHY_BASIC_STATUS                   1
> > +#define PHY_ID1                            2
> > +#define PHY_ID2                            3
> > +#define PHY_AUTO_NEG_ADVERT                4
> > +#define PHY_AUTO_NEG_LINK_ABILITY          5
> > +#define PHY_AUTO_NEG_EXP                   6
> > +#define PHY_1000BASE_T_CONTROL            9
> > +#define PHY_1000BASE_T_STATUS             10
> > +#define PHY_MODE                           17
> > +#define PHY_SPECIAL_MODES                  18
> > +#define PHY_SPECIAL_CTLR                   27
> > +#define PHY_INT_SRC                        29
> > +#define PHY_INT_MASK                       30
> > +#define PHY_SPECIAL_PHY_CTLR               31
>
> Please fix the indentation
>
> > +
> > +// PHY control register bits
> > +#define PHYCTRL_COLL_TEST                       BIT7                  // Collision test enable
> > +#define PHYCTRL_DUPLEX_MODE                     BIT8                  // Set Duplex Mode
> > +#define PHYCTRL_RST_AUTO                        BIT9                  // Restart Auto-Negotiation of Link abilities
> > +#define PHYCTRL_PD                              BIT11                 // Power-Down switch
> > +#define PHYCTRL_AUTO_EN                         BIT12                 // Auto-Negotiation Enable
> > +#define PHYCTRL_SPEED_SEL                       BIT13                 // Link Speed Selection
> > +#define PHYCTRL_LOOPBK                          BIT14                 // Set loopback mode
> > +#define PHYCTRL_RESET                           BIT15                 // Do a PHY reset
> > +
>
> Please align indented part with the code below. In fact, could you please
> clean up the indentation in general?
>
> > +// PHY status register bits
> > +#define PHYSTS_EXT_CAP                        BIT0                  // Extended Capabilities Register capability
> > +#define PHYSTS_JABBER                         BIT1                  // Jabber condition detected
> > +#define PHYSTS_LINK_STS                       BIT2                  // Link Status
> > +#define PHYSTS_AUTO_CAP                       BIT3                  // Auto-Negotiation Capability
> > +#define PHYSTS_REMOTE_FAULT                   BIT4                  // Remote fault detected
> > +#define PHYSTS_AUTO_COMP                      BIT5                  // Auto-Negotiation Completed
> > +#define PHYSTS_10BASET_HDPLX                  BIT11                 // 10Mbps Half-Duplex ability
> > +#define PHYSTS_10BASET_FDPLX                  BIT12                 // 10Mbps Full-Duplex ability
> > +#define PHYSTS_100BASETX_HDPLX                BIT13                 // 100Mbps Half-Duplex ability
> > +#define PHYSTS_100BASETX_FDPLX                BIT14                 // 100Mbps Full-Duplex ability
> > +#define PHYSTS_100BASE_T4                     BIT15                 // Base T4 ability
> > +
> > +// PHY Auto-Negotiation advertisement
> > +#define PHYANA_SEL_MASK                       ((UINT32)0x1F)        // Link type selector
> > +#define PHYANA_10BASET                        BIT5                  // Advertise 10BASET capability
> > +#define PHYANA_10BASETFD                      BIT6                  // Advertise 10BASET Full duplex capability
> > +#define PHYANA_100BASETX                      BIT7                  // Advertise 100BASETX capability
> > +#define PHYANA_100BASETXFD                    BIT8                  // Advertise 100 BASETX Full duplex capability
> > +#define PHYANA_PAUSE_OP_MASK                  (3 << 10)             // Advertise PAUSE frame capability
> > +#define PHYANA_REMOTE_FAULT                   BIT13                 // Remote fault detected
> > +
> > +#define PHYLPA_SLCT                                  0x001f    // Same as advertise selector
> > +#define PHYLPA_10HALF                            0x0020        // Can do 10mbps half-duplex
> > +#define PHYLPA_1000XFULL                             0x0020    // Can do 1000BASE-X full-duplex
> > +#define PHYLPA_10FULL                            0x0040        // Can do 10mbps full-duplex
> > +#define PHYLPA_1000XHALF                             0x0040    // Can do 1000BASE-X half-duplex
> > +#define PHYLPA_100HALF                           0x0080        // Can do 100mbps half-duplex
> > +#define PHYLPA_1000XPAUSE                            0x0080    // Can do 1000BASE-X pause
> > +#define PHYLPA_100FULL                           0x0100        // Can do 100mbps full-duplex
> > +#define PHYLPA_1000XPAUSE_ASYM               0x0100    // Can do 1000BASE-X pause asym
> > +#define PHYLPA_100BASE4                                  0x0200        // Can do 100mbps 4k packets
> > +#define PHYLPA_PAUSE_CAP                             0x0400    // Can pause
> > +#define PHYLPA_PAUSE_ASYM                            0x0800    // Can pause asymetrically
> > +#define PHYLPA_RESV                                  0x1000    // Unused
> > +#define PHYLPA_RFAULT                            0x2000        // Link partner faulted
> > +#define PHYLPA_LPACK                             0x4000        // Link partner acked us
> > +#define PHYLPA_NPAGE                             0x8000        // Next page bit
> > +
> > +#define PHYLPA_DUPLEX                           (LPA_10FULL | LPA_100FULL)
> > +#define PHYLPA_100                                      (LPA_100FULL | LPA_100HALF | LPA_100BASE4)
> > +
> > +// 1000BASE-T Status register
> > +#define PHYLPA_1000FULL                                  0x0800        /* Link partner 1000BASE-T full duplex */
> > +#define PHYLPA_1000HALF                                  0x0400        /* Link partner 1000BASE-T half duplex */
> > +
> > +// 1000BASE-T Control register
> > +#define PHYADVERTISE_1000FULL                0x0200  /* Advertise 1000BASE-T full duplex */
> > +#define PHYADVERTISE_1000HALF                0x0100  /* Advertise 1000BASE-T half duplex */
> > +
> > +#define SPEED_1000                            1000
> > +#define SPEED_100                             100
> > +#define SPEED_10                              10
> > +
> > +#define DUPLEX_FULL                           1
> > +#define DUPLEX_HALF                           0
> > +
> > +// PHY Super Special control/status
> > +#define PHYSSCS_HCDSPEED_MASK                (7 << 2)              // Speed indication
> > +#define PHYSSCS_AUTODONE                      BIT12                 // Auto-Negotiation Done
> > +
> > +// Flags for PHY reset
> > +#define PHY_RESET_PMT                         BIT0
> > +#define PHY_RESET_BCR                         BIT1
> > +#define PHY_RESET_CHECK_LINK                  BIT2
> > +
> > +// Flags for auto negotiation
> > +#define AUTO_NEGOTIATE_COLLISION_TEST         BIT0
> > +#define AUTO_NEGOTIATE_ADVERTISE_ALL          BIT1
> > +
> > +
> > +// Micrel KSZ9031 Extended registers
> > +#define PHY_KSZ9031RN_CONTROL_PAD_SKEW_REG           4
> > +#define PHY_KSZ9031RN_RX_DATA_PAD_SKEW_REG           5
> > +#define PHY_KSZ9031RN_TX_DATA_PAD_SKEW_REG           6
> > +#define PHY_KSZ9031RN_CLK_PAD_SKEW_REG           8
> > +
> > +// Data operations
> > +#define PHY_KSZ9031_MOD_DATA_NO_POST_INC        0x1
> > +#define PHY_KSZ9031_MOD_DATA_POST_INC_RW        0x2
> > +#define PHY_KSZ9031_MOD_DATA_POST_INC_W                 0x3
> > +
> > +#define PHY_KSZ9031RN_MMD_CTRL_REG              0x0d
> > +#define PHY_KSZ9031RN_MMD_REGDATA_REG       0x0e
> > +
> > +#define PHY_KSZ9031RN_CLK_SKEW_CLR_MASK                    0x3FF
> > +#define PHY_KSZ9031RN_CONTROL_SKEW_CLR_MASK                0xFF
> > +#define PHY_KSZ9031RN_RX_DATA_SKEW_CLR_MASK                0xFF
> > +#define PHY_KSZ9031RN_TX_DATA_SKEW_CLR_MASK                0xFF
> > +
> > +#define PHY_KSZ9031RN_CLK_PAD_SKEW_VALUE                 0x3FC
> > +#define PHY_KSZ9031RN_CONTROL_PAD_SKEW_VALUE         0x70
> > +#define PHY_KSZ9031RN_RX_DATA_PAD_SKEW_VALUE         0x7777
> > +#define PHY_KSZ9031RN_TX_DATA_PAD_SKEW_VALUE         0x0
> > +
> > +
> > +#define PHY_KSZ9031RN_DEV_ADDR                  0x2
> > +
> > +// MMD Address 0h, Auto-Negotiation FLP burst transmit timing
> > +#define PHY_KSZ9031RN_MMD_DEV_ADDR_00       0x00
> > +#define PHY_KSZ9031RN_MMD_D0_FLP_LO_REG     3
> > +#define PHY_KSZ9031RN_MMD_D0_FLP_16MS_LO    0x1A80
> > +#define PHY_KSZ9031RN_MMD_D0_FLP_HI_REG     4
> > +#define PHY_KSZ9031RN_MMD_D0_FLP_16MS_HI    0x0006
> > +
> > +// HPS MII
> > +#define MII_BUSY                                   (1 << 0)
> > +#define MII_WRITE                                  (1 << 1)
> > +#define MII_CLKRANGE_60_100M               (0x0)
> > +#define MII_CLKRANGE_100_150M              (0x4)
> > +#define MII_CLKRANGE_20_35M                    (0x8)
> > +#define MII_CLKRANGE_35_60M                    (0xC)
> > +#define MII_CLKRANGE_150_250M              (0x10)
> > +#define MII_CLKRANGE_250_300M              (0x14)
> > +
> > +#define MIIADDRSHIFT                           (11)
> > +#define MIIREGSHIFT                                (6)
> > +#define MII_REGMSK                                 (0x1F << 6)
> > +#define MII_ADDRMSK                                (0x1F << 11)
> > +
> > +// Others
> > +#define PHY_INVALID_ID                      0xFFFF
> > +#define LINK_UP                                                1
> > +#define LINK_DOWN                           0
> > +#define PHY_TIMEOUT                         200000
> > +
>
> Please also clean up the indentation of the function parameters below.
>
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +PhyDxeInitialization (
> > +  IN PHY_DRIVER* PhyDriver,
> > +  IN UINT32      MacBaseAddress
> > +  );
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +PhyDetectDevice (
> > +  IN PHY_DRIVER* PhyDriver,
> > +  IN UINT32      MacBaseAddress
> > +  );
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +PhyConfig (
> > +  IN PHY_DRIVER* PhyDriver,
> > +  IN UINT32      MacBaseAddress
> > +  );
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +PhySoftReset (
> > +  IN PHY_DRIVER* PhyDriver,
> > +  IN UINT32      MacBaseAddress
> > +  );
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +PhyReadId (
> > +  IN UINT32  PhyAddr,
> > +  IN UINT32      MacBaseAddress
> > +  );
> > +
> > +VOID
> > +EFIAPI
> > +PhyConfigSkew (
> > +  IN PHY_DRIVER* PhyDriver,
> > +  IN UINT32      MacBaseAddress
> > +  );
> > +
> > +VOID
> > +EFIAPI
> > +PhyDisplayConfigSkew (
> > +  IN PHY_DRIVER* PhyDriver,
> > +  IN UINT32      MacBaseAddress
> > +  );
> > +
> > +VOID
> > +EFIAPI
> > +PhyConfigFlpBurstTiming (
> > +  IN PHY_DRIVER* PhyDriver,
> > +  IN UINT32      MacBaseAddress
> > +  );
> > +
> > +VOID
> > +EFIAPI
> > +PhyDisplayFlpBurstTiming (
> > +  IN PHY_DRIVER* PhyDriver,
> > +  IN UINT32      MacBaseAddress
> > +  );
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +PhyAutoNego (
> > +  IN PHY_DRIVER* PhyDriver,
> > +  IN UINT32      MacBaseAddress
> > +  );
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +PhyLinkAdjustEmacConfig (
> > +  IN PHY_DRIVER* PhyDriver,
> > +  IN UINT32      MacBaseAddress
> > +  );
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +PhyCheckLinkStatus (
> > +  IN PHY_DRIVER* PhyDriver,
> > +  IN UINT32      MacBaseAddress
> > +  );
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +PhyReadCapability (
> > +  IN PHY_DRIVER* PhyDriver,
> > +  IN UINT32* Speed,
> > +  IN UINT32* Duplex,
> > +  IN UINT32      MacBaseAddress
> > +  );
> > +
> > +VOID
> > +EFIAPI
> > +PhyDisplayAbility (
> > +  IN UINT32 Speed,
> > +  IN UINT32 Duplex
> > +  );
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +PhyRead (
> > +  IN  UINT32  Addr,
> > +  IN  UINT32  Reg,
> > +  OUT UINT32 *Data,
> > +  IN UINT32      MacBaseAddress
> > +  );
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +PhyWrite (
> > +  IN UINT32 Addr,
> > +  IN UINT32 Reg,
> > +  IN UINT32 Data,
> > +  IN UINT32      MacBaseAddress
> > +  );
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +Phy9031ExtendedWrite (
> > +  IN PHY_DRIVER* PhyDriver,
> > +  IN UINT32 Mode,
> > +  IN UINT32 DevAddr,
> > +  IN UINT32 Regnum,
> > +  IN UINT16 Val,
> > +  IN UINT32      MacBaseAddress
> > +  );
> > +
> > +UINT32
> > +EFIAPI
> > +Phy9031ExtendedRead (
> > +  IN PHY_DRIVER* PhyDriver,
> > +  IN UINT32 Mode,
> > +  IN UINT32 DevAddr,
> > +  IN UINT32 Regnum,
> > +  IN UINT32      MacBaseAddress
> > +  );
> > +
> > +
> > +#endif /* __PHY_DXE_H__ */
> > diff --git a/Silicon/DesignWare/Drivers/DwEmacSnpDxe/ComponentName.c b/Silicon/DesignWare/Drivers/DwEmacSnpDxe/ComponentName.c
> > new file mode 100755
> > index 000000000000..08e91fae396e
> > --- /dev/null
> > +++ b/Silicon/DesignWare/Drivers/DwEmacSnpDxe/ComponentName.c
> > @@ -0,0 +1,327 @@
> > +/** @file
> > +Copyright (c) 2011 - 2019, Intel Corporaton. All rights reserved.
> > +  This program and the accompanying materials
> > +  are licensed and made available under the terms and conditions of the BSD License
> > +  which accompanies this distribution.  The full text of the license may be found at
> > +  http://opensource.org/licenses/bsd-license.php
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > +
> > +  The original software modules are licensed as follows:
> > +
> > +  Copyright (c) 2006 - 2007, Intel Corporation. All rights reserved.<BR>
> > +  This program and the accompanying materials
> > +  are licensed and made available under the terms and conditions of the BSD License
> > +  which accompanies this distribution.  The full text of the license may be found at
> > +  http://opensource.org/licenses/bsd-license.php
> > +
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > +
> > +Module Name:
> > +
> > +  ComponentName.c
> > +
> > +Abstract:
> > +
> > +
> > +**/
> > +
> > +#include "DwEmacSnpDxe.h"
> > +
> > +//
> > +// EFI Component Name Functions
> > +//
> > +/**
> > +  Retrieves a Unicode string that is the user readable name of the driver.
> > +
> > +  This function retrieves the user readable name of a driver in the form of a
> > +  Unicode string. If the driver specified by This has a user readable name in
> > +  the language specified by Language, then a pointer to the driver name is
> > +  returned in DriverName, and EFI_SUCCESS is returned. If the driver specified
> > +  by This does not support the language specified by Language,
> > +  then EFI_UNSUPPORTED is returned.
> > +
> > +  @param  This[in]              A pointer to the EFI_COMPONENT_NAME2_PROTOCOL or
> > +                                EFI_COMPONENT_NAME_PROTOCOL instance.
> > +
> > +  @param  Language[in]          A pointer to a Null-terminated ASCII string
> > +                                array indicating the language. This is the
> > +                                language of the driver name that the caller is
> > +                                requesting, and it must match one of the
> > +                                languages specified in SupportedLanguages. The
> > +                                number of languages supported by a driver is up
> > +                                to the driver writer. Language is specified
> > +                                in RFC 4646 or ISO 639-2 language code format.
> > +
> > +  @param  DriverName[out]       A pointer to the Unicode string to return.
> > +                                This Unicode string is the name of the
> > +                                driver specified by This in the language
> > +                                specified by Language.
> > +
> > +  @retval EFI_SUCCESS           The Unicode string for the Driver specified by
> > +                                This and the language specified by Language was
> > +                                returned in DriverName.
> > +
> > +  @retval EFI_INVALID_PARAMETER Language is NULL.
> > +
> > +  @retval EFI_INVALID_PARAMETER DriverName is NULL.
> > +
> > +  @retval EFI_UNSUPPORTED       The driver specified by This does not support
> > +                                the language specified by Language.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +SnpGetDriverName (
> > +  IN  EFI_COMPONENT_NAME_PROTOCOL  *This,
> > +  IN  CHAR8                        *Language,
> > +  OUT CHAR16                       **DriverName
> > +  );
> > +
> > +
> > +/**
> > +  Retrieves a Unicode string that is the user readable name of the controller
> > +  that is being managed by a driver.
> > +
> > +  This function retrieves the user readable name of the controller specified by
> > +  ControllerHandle and ChildHandle in the form of a Unicode string. If the
> > +  driver specified by This has a user readable name in the language specified by
> > +  Language, then a pointer to the controller name is returned in ControllerName,
> > +  and EFI_SUCCESS is returned.  If the driver specified by This is not currently
> > +  managing the controller specified by ControllerHandle and ChildHandle,
> > +  then EFI_UNSUPPORTED is returned.  If the driver specified by This does not
> > +  support the language specified by Language, then EFI_UNSUPPORTED is returned.
> > +
> > +  @param  This[in]              A pointer to the EFI_COMPONENT_NAME2_PROTOCOL or
> > +                                EFI_COMPONENT_NAME_PROTOCOL instance.
> > +
> > +  @param  ControllerHandle[in]  The handle of a controller that the driver
> > +                                specified by This is managing.  This handle
> > +                                specifies the controller whose name is to be
> > +                                returned.
> > +
> > +  @param  ChildHandle[in]       The handle of the child controller to retrieve
> > +                                the name of.  This is an optional parameter that
> > +                                may be NULL.  It will be NULL for device
> > +                                drivers.  It will also be NULL for a bus drivers
> > +                                that wish to retrieve the name of the bus
> > +                                controller.  It will not be NULL for a bus
> > +                                driver that wishes to retrieve the name of a
> > +                                child controller.
> > +
> > +  @param  Language[in]          A pointer to a Null-terminated ASCII string
> > +                                array indicating the language.  This is the
> > +                                language of the driver name that the caller is
> > +                                requesting, and it must match one of the
> > +                                languages specified in SupportedLanguages. The
> > +                                number of languages supported by a driver is up
> > +                                to the driver writer. Language is specified in
> > +                                RFC 4646 or ISO 639-2 language code format.
> > +
> > +  @param  ControllerName[out]   A pointer to the Unicode string to return.
> > +                                This Unicode string is the name of the
> > +                                controller specified by ControllerHandle and
> > +                                ChildHandle in the language specified by
> > +                                Language from the point of view of the driver
> > +                                specified by This.
> > +
> > +  @retval EFI_SUCCESS           The Unicode string for the user readable name in
> > +                                the language specified by Language for the
> > +                                driver specified by This was returned in
> > +                                DriverName.
> > +
> > +  @retval EFI_INVALID_PARAMETER ControllerHandle is not a valid EFI_HANDLE.
> > +
> > +  @retval EFI_INVALID_PARAMETER ChildHandle is not NULL and it is not a valid
> > +                                EFI_HANDLE.
> > +
> > +  @retval EFI_INVALID_PARAMETER Language is NULL.
> > +
> > +  @retval EFI_INVALID_PARAMETER ControllerName is NULL.
> > +
> > +  @retval EFI_UNSUPPORTED       The driver specified by This is not currently
> > +                                managing the controller specified by
> > +                                ControllerHandle and ChildHandle.
> > +
> > +  @retval EFI_UNSUPPORTED       The driver specified by This does not support
> > +                                the language specified by Language.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +SnpGetControllerName (
> > +  IN  EFI_COMPONENT_NAME_PROTOCOL                     *This,
> > +  IN  EFI_HANDLE                                      ControllerHandle,
> > +  IN  EFI_HANDLE                                      ChildHandle        OPTIONAL,
> > +  IN  CHAR8                                           *Language,
> > +  OUT CHAR16                                          **ControllerName
> > +  );
> > +
> > +
> > +//
> > +// EFI Component Name Protocol
> > +//
> > +GLOBAL_REMOVE_IF_UNREFERENCED EFI_COMPONENT_NAME_PROTOCOL  gSnpComponentName = {
> > +  SnpGetDriverName,
> > +  SnpGetControllerName,
> > +  "eng"
> > +};
> > +
> > +//
> > +// EFI Component Name 2 Protocol
> > +//
> > +GLOBAL_REMOVE_IF_UNREFERENCED EFI_COMPONENT_NAME2_PROTOCOL gSnpComponentName2 = {
> > +  (EFI_COMPONENT_NAME2_GET_DRIVER_NAME) SnpGetDriverName,
> > +  (EFI_COMPONENT_NAME2_GET_CONTROLLER_NAME) SnpGetControllerName,
> > +  "en"
> > +};
> > +
> > +
> > +GLOBAL_REMOVE_IF_UNREFERENCED EFI_UNICODE_STRING_TABLE mSnpNameTable[] = {
> > +  {
> > +    "eng;en",
> > +    L"SNP DesignWare EMAC Driver"
> > +  },
> > +  {
> > +    NULL,
> > +    NULL
> > +  }
> > +};
> > +
> > +/**
> > +  Retrieves a Unicode string that is the user readable name of the driver.
> > +
> > +  This function retrieves the user readable name of a driver in the form of a
> > +  Unicode string. If the driver specified by This has a user readable name in
> > +  the language specified by Language, then a pointer to the driver name is
> > +  returned in DriverName, and EFI_SUCCESS is returned. If the driver specified
> > +  by This does not support the language specified by Language,
> > +  then EFI_UNSUPPORTED is returned.
> > +
> > +  @param  This[in]              A pointer to the EFI_COMPONENT_NAME2_PROTOCOL or
> > +                                EFI_COMPONENT_NAME_PROTOCOL instance.
> > +
> > +  @param  Language[in]          A pointer to a Null-terminated ASCII string
> > +                                array indicating the language. This is the
> > +                                language of the driver name that the caller is
> > +                                requesting, and it must match one of the
> > +                                languages specified in SupportedLanguages. The
> > +                                number of languages supported by a driver is up
> > +                                to the driver writer. Language is specified
> > +                                in RFC 4646 or ISO 639-2 language code format.
> > +
> > +  @param  DriverName[out]       A pointer to the Unicode string to return.
> > +                                This Unicode string is the name of the
> > +                                driver specified by This in the language
> > +                                specified by Language.
> > +
> > +  @retval EFI_SUCCESS           The Unicode string for the Driver specified by
> > +                                This and the language specified by Language was
> > +                                returned in DriverName.
> > +
> > +  @retval EFI_INVALID_PARAMETER Language is NULL.
> > +
> > +  @retval EFI_INVALID_PARAMETER DriverName is NULL.
> > +
> > +  @retval EFI_UNSUPPORTED       The driver specified by This does not support
> > +                                the language specified by Language.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +SnpGetDriverName (
> > +  IN  EFI_COMPONENT_NAME_PROTOCOL  *This,
> > +  IN  CHAR8                        *Language,
> > +  OUT CHAR16                       **DriverName
> > +  )
> > +{
> > +  return LookupUnicodeString2 (
> > +           Language,
> > +           This->SupportedLanguages,
> > +           mSnpNameTable,
> > +           DriverName,
> > +           (BOOLEAN)(This == &gSnpComponentName)
> > +           );
> > +}
> > +
> > +/**
> > +  Retrieves a Unicode string that is the user readable name of the controller
> > +  that is being managed by a driver.
> > +
> > +  This function retrieves the user readable name of the controller specified by
> > +  ControllerHandle and ChildHandle in the form of a Unicode string. If the
> > +  driver specified by This has a user readable name in the language specified by
> > +  Language, then a pointer to the controller name is returned in ControllerName,
> > +  and EFI_SUCCESS is returned.  If the driver specified by This is not currently
> > +  managing the controller specified by ControllerHandle and ChildHandle,
> > +  then EFI_UNSUPPORTED is returned.  If the driver specified by This does not
> > +  support the language specified by Language, then EFI_UNSUPPORTED is returned.
> > +
> > +  @param  This[in]              A pointer to the EFI_COMPONENT_NAME2_PROTOCOL or
> > +                                EFI_COMPONENT_NAME_PROTOCOL instance.
> > +
> > +  @param  ControllerHandle[in]  The handle of a controller that the driver
> > +                                specified by This is managing.  This handle
> > +                                specifies the controller whose name is to be
> > +                                returned.
> > +
> > +  @param  ChildHandle[in]       The handle of the child controller to retrieve
> > +                                the name of.  This is an optional parameter that
> > +                                may be NULL.  It will be NULL for device
> > +                                drivers.  It will also be NULL for a bus drivers
> > +                                that wish to retrieve the name of the bus
> > +                                controller.  It will not be NULL for a bus
> > +                                driver that wishes to retrieve the name of a
> > +                                child controller.
> > +
> > +  @param  Language[in]          A pointer to a Null-terminated ASCII string
> > +                                array indicating the language.  This is the
> > +                                language of the driver name that the caller is
> > +                                requesting, and it must match one of the
> > +                                languages specified in SupportedLanguages. The
> > +                                number of languages supported by a driver is up
> > +                                to the driver writer. Language is specified in
> > +                                RFC 4646 or ISO 639-2 language code format.
> > +
> > +  @param  ControllerName[out]   A pointer to the Unicode string to return.
> > +                                This Unicode string is the name of the
> > +                                controller specified by ControllerHandle and
> > +                                ChildHandle in the language specified by
> > +                                Language from the point of view of the driver
> > +                                specified by This.
> > +
> > +  @retval EFI_SUCCESS           The Unicode string for the user readable name in
> > +                                the language specified by Language for the
> > +                                driver specified by This was returned in
> > +                                DriverName.
> > +
> > +  @retval EFI_INVALID_PARAMETER ControllerHandle is not a valid EFI_HANDLE.
> > +
> > +  @retval EFI_INVALID_PARAMETER ChildHandle is not NULL and it is not a valid
> > +                                EFI_HANDLE.
> > +
> > +  @retval EFI_INVALID_PARAMETER Language is NULL.
> > +
> > +  @retval EFI_INVALID_PARAMETER ControllerName is NULL.
> > +
> > +  @retval EFI_UNSUPPORTED       The driver specified by This is not currently
> > +                                managing the controller specified by
> > +                                ControllerHandle and ChildHandle.
> > +
> > +  @retval EFI_UNSUPPORTED       The driver specified by This does not support
> > +                                the language specified by Language.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +SnpGetControllerName (
> > +  IN  EFI_COMPONENT_NAME_PROTOCOL                     *This,
> > +  IN  EFI_HANDLE                                      ControllerHandle,
> > +  IN  EFI_HANDLE                                      ChildHandle        OPTIONAL,
> > +  IN  CHAR8                                           *Language,
> > +  OUT CHAR16                                          **ControllerName
> > +  )
> > +{
> > +  return EFI_UNSUPPORTED;
> > +}
> > diff --git a/Silicon/DesignWare/Drivers/DwEmacSnpDxe/DriverBinding.c b/Silicon/DesignWare/Drivers/DwEmacSnpDxe/DriverBinding.c
> > new file mode 100755
> > index 000000000000..2b750c03d432
> > --- /dev/null
> > +++ b/Silicon/DesignWare/Drivers/DwEmacSnpDxe/DriverBinding.c
> > @@ -0,0 +1,450 @@
> > +/** @file
> > +
> > +  Copyright (c) 2011 - 2019, Intel Corporaton. All rights reserved.
> > +  This program and the accompanying materials
> > +  are licensed and made available under the terms and conditions of the BSD License
> > +  which accompanies this distribution.  The full text of the license may be found at
> > +  http://opensource.org/licenses/bsd-license.php
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > + **/
> > +
> > +#include "DwEmacSnpDxe.h"
> > +
> > +STATIC
> > +EFI_STATUS
> > +EFIAPI
> > +DriverSupported (
> > +  IN  EFI_DRIVER_BINDING_PROTOCOL *This,
> > +  IN  EFI_HANDLE                  Controller,
> > +  IN  EFI_DEVICE_PATH_PROTOCOL    *RemainingDevicePath
> > +  );
> > +
> > +STATIC
> > +EFI_STATUS
> > +EFIAPI
> > +DriverStart (
> > +  IN  EFI_DRIVER_BINDING_PROTOCOL *This,
> > +  IN  EFI_HANDLE                  Controller,
> > +  IN  EFI_DEVICE_PATH_PROTOCOL    *RemainingDevicePath
> > +  );
> > +
> > +STATIC
> > +EFI_STATUS
> > +EFIAPI
> > +DriverStop (
> > +  IN  EFI_DRIVER_BINDING_PROTOCOL *This,
> > +  IN  EFI_HANDLE                  Controller,
> > +  IN  UINTN                       NumberOfChildren,
> > +  IN  EFI_HANDLE                  *ChildHandleBuffer
> > +  );
> > +
> > +STATIC EFI_DRIVER_BINDING_PROTOCOL mDriverBinding = {
> > +  DriverSupported,
> > +  DriverStart,
> > +  DriverStop,
> > +  0xa,
> > +  NULL,
> > +  NULL
> > +};
> > +
> > +EFI_SIMPLE_NETWORK_DEVICE_PATH PathTemplate =  {
>
> Can this be STATIC?
>
> > +  {
> > +    {
> > +      MESSAGING_DEVICE_PATH, MSG_MAC_ADDR_DP,
> > +      { (UINT8) (sizeof(MAC_ADDR_DEVICE_PATH)), (UINT8) ((sizeof(MAC_ADDR_DEVICE_PATH)) >> 8) }
> > +    },
> > +    { { 0 } },
> > +    0
> > +  },
> > +  {
> > +    END_DEVICE_PATH_TYPE,
> > +    END_ENTIRE_DEVICE_PATH_SUBTYPE,
> > +    { sizeof(EFI_DEVICE_PATH_PROTOCOL), 0 }
> > +  }
> > +};
> > +
> > +STATIC EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR mDwEmacNetDesc[] = {
> > + {
> > +    ACPI_ADDRESS_SPACE_DESCRIPTOR,                    // Desc
> > +    sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) - 3,   // Len
> > +    ACPI_ADDRESS_SPACE_TYPE_MEM,                      // ResType
> > +    0,                                                // GenFlag
> > +    0,                                                // SpecificFlag
> > +    32,                                               // AddrSpaceGranularity
> > +    FixedPcdGet32 (PcdDwEmacDxeBaseAddress),          // AddrRangeMin
> > +    FixedPcdGet32 (PcdDwEmacDxeBaseAddress) +
> > +       SIZE_4KB - 1,                                     // AddrRangeMax
> > +    0,                                                // AddrTranslationOffset
> > +    SIZE_4KB,                                         // AddrLen
> > +  }, {
> > +    ACPI_ADDRESS_SPACE_DESCRIPTOR,                    // Desc
> > +    sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) - 3,   // Len
> > +    ACPI_ADDRESS_SPACE_TYPE_MEM,                      // ResType
> > +    0,                                                // GenFlag
> > +    0,                                                // SpecificFlag
> > +    32,                                               // AddrSpaceGranularity
> > +    FixedPcdGet32 (PcdDwEmacDefaultMacAddress),       // AddrRangeMin
> > +    FixedPcdGet32 (PcdDwEmacDefaultMacAddress),       // AddrRangeMax
> > +    0,                                                // AddrTranslationOffset
> > +    1,                                                // AddrLen
> > +  }, {
> > +    ACPI_END_TAG_DESCRIPTOR                           // Desc
> > +  }
> > +};
>
> This address space descriptor should be created by the platform, not by this
> driver.
>
> > +
> > +STATIC
> > +EFI_STATUS
> > +RegisterDevice (
> > +  IN  EFI_GUID                            *TypeGuid,
> > +  IN  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR   *Desc,
> > +  OUT EFI_HANDLE                          *Handle
> > +  )
> > +{
> > +  NON_DISCOVERABLE_DEVICE             *Device;
> > +  EFI_STATUS                          Status;
> > +
> > +  Device = (NON_DISCOVERABLE_DEVICE *)AllocateZeroPool (sizeof (*Device));
> > +  if (Device == NULL) {
> > +    return EFI_OUT_OF_RESOURCES;
> > +  }
> > +
> > +  Device->Type = TypeGuid;
> > +  Device->DmaType = NonDiscoverableDeviceDmaTypeNonCoherent;
> > +  Device->Resources = Desc;
> > +
> > +  Status = gBS->InstallMultipleProtocolInterfaces (Handle,
> > +                  &gEdkiiNonDiscoverableDeviceProtocolGuid, Device,
> > +                  NULL);
> > +  if (EFI_ERROR (Status)) {
> > +    goto FreeDevice;
> > +  }
> > +  return EFI_SUCCESS;
> > +
> > +  FreeDevice:
>
> Don't indent labels.
>
> > +  FreePool (Device);
> > +
> > +  return Status;
> > +}
> > +
> > +
> > +STATIC
> > +EFI_STATUS
> > +EFIAPI
> > +DriverSupported (
> > +  IN  EFI_DRIVER_BINDING_PROTOCOL *This,
> > +  IN  EFI_HANDLE                  Controller,
> > +  IN  EFI_DEVICE_PATH_PROTOCOL    *RemainingDevicePath
> > +  )
> > +{
> > +  NON_DISCOVERABLE_DEVICE    *Dev;
> > +  EFI_STATUS                 Status;
> > +
> > +  // Connect to the non-discoverable device
> > +  Status = gBS->OpenProtocol (Controller,
> > +                              &gEdkiiNonDiscoverableDeviceProtocolGuid,
> > +                              (VOID **) &Dev,
>
> Please drop the space after the cast
>
> > +                              This->DriverBindingHandle,
> > +                              Controller,
> > +                              EFI_OPEN_PROTOCOL_BY_DRIVER);
> > +  if (EFI_ERROR (Status)) {
> > +    return Status;
> > +  }
> > +
> > +  if (CompareGuid (Dev->Type, &gDwEmacNetNonDiscoverableDeviceGuid) &&
> > +      Dev->DmaType == NonDiscoverableDeviceDmaTypeNonCoherent) {
>
> I think we can drop this check: if a platform is integrated with cache coherent
> DMA and the cache coherent DmaLib is used, things should simply work as well.
>
> > +    Status = EFI_SUCCESS;
> > +  } else {
> > +    Status = EFI_UNSUPPORTED;
> > +  }
> > +
> > +  // Clean up.
> > +  gBS->CloseProtocol (Controller,
> > +                      &gEdkiiNonDiscoverableDeviceProtocolGuid,
> > +                      This->DriverBindingHandle,
> > +                      Controller);
> > +
> > +  return Status;
> > +}
> > +
> > +STATIC
> > +EFI_STATUS
> > +EFIAPI
> > +DriverStart (
> > +  IN  EFI_DRIVER_BINDING_PROTOCOL *This,
> > +  IN  EFI_HANDLE                  Controller,
> > +  IN  EFI_DEVICE_PATH_PROTOCOL    *RemainingDevicePath
> > +  )
> > +{
> > +  EFI_STATUS                       Status;
> > +  EFI_SIMPLE_NETWORK_DRIVER       *Snp;
> > +  EFI_SIMPLE_NETWORK_MODE         *SnpMode;
> > +  EFI_SIMPLE_NETWORK_DEVICE_PATH  *DevicePath;
> > +  UINT64                           DefaultMacAddress;
> > +  EFI_MAC_ADDRESS                 *SwapMacAddressPtr;
>
> Please indent the * above as if it were the first character of the variable
> name.
>
> > +  UINTN                           BufferSize;
> > +  UINTN                           BufferSizeBuf;
> > +
> > +  // Allocate Resources
> > +  Snp = AllocatePages (EFI_SIZE_TO_PAGES (sizeof (EFI_SIMPLE_NETWORK_DRIVER)));
> > +  if (Snp == NULL) {
> > +    return EFI_OUT_OF_RESOURCES;
> > +  }
> > +
> > +  BufferSize = EFI_PAGES_TO_SIZE ((sizeof (DESIGNWARE_HW_DESCRIPTOR))*10);
>
> Where does the '10' come from? Why not 9 or 11? And please make it a macro
> (and put spaces around '*')
>
> > +  BufferSizeBuf = EFI_PAGES_TO_SIZE ((sizeof (CHAR8))*10*2048);
>
> And these numbers? And what does BufferSizeBuf mean?
>
> > +
> > +  // DMA Allocte Buffer and Map for TxdescRing
>
> Allocate
>
> > +  Status = DmaAllocateBuffer (EfiBootServicesData, EFI_SIZE_TO_PAGES ((sizeof (DESIGNWARE_HW_DESCRIPTOR))*10), (VOID*)&Snp->MacDriver.TxdescRing);
>
> Please wrap overly long lines.
>
> > +  if (EFI_ERROR (Status)) {
> > +    DEBUG ((DEBUG_ERROR, "CreateDwEmacSnpDxe: DmaAllocateBuffer TxdescRing: %r\n", Status));
>
> Please use %a and __FUNCTION__ for the function name in the debug string (note
> that the function is not called CreateDwEmacSnpDxe() so this message is very
> misleading)
>
> > +    return Status;
> > +  }
> > +  Status = DmaMap (MapOperationBusMasterCommonBuffer, Snp->MacDriver.TxdescRing[0], &BufferSize, &Snp->MacDriver.TxdescRingMap[0], &Snp->MappingTxdesc);
>
> Why the [0]? How many descriptors are you mapping?
>
> > +  if (EFI_ERROR (Status)) {
> > +    DEBUG ((DEBUG_ERROR, "CreateDwEmacSnpDxe: DmaMap TxdescRing: %r\n", Status));
>
> Wrong function name - use %a and __FUNCTION__ instead.
>
> > +    return Status;
> > +  }
> > +
> > +  // DMA Allocte Buffer and Map for RxdescRing
> > +  Status = DmaAllocateBuffer (EfiBootServicesData, EFI_SIZE_TO_PAGES ((sizeof (DESIGNWARE_HW_DESCRIPTOR))*10), (VOID*)&Snp->MacDriver.RxdescRing);
> > +  if (EFI_ERROR (Status)) {
> > +    DEBUG ((DEBUG_ERROR, "CreateDwEmacSnpDxe: DmaAllocateBuffer RxdescRing: %r\n", Status));
> > +    return Status;
> > +  }
> > +  Status = DmaMap (MapOperationBusMasterCommonBuffer, Snp->MacDriver.RxdescRing[0], &BufferSize, &Snp->MacDriver.RxdescRingMap[0], &Snp->MappingRxdesc);
> > +  if (EFI_ERROR (Status)) {
> > +    DEBUG ((DEBUG_ERROR, "CreateDwEmacSnpDxe: DmaMap RxdescRing: %r\n", Status));
> > +    return Status;
> > +  }
> > +
>
> Same as above for rx
>
> > +  // DMA Allocte Buffer and Map for TxBuffer
> > +  Status = DmaAllocateBuffer (EfiBootServicesData, EFI_SIZE_TO_PAGES ((sizeof (CHAR8))*10*2048), (VOID*)&Snp->MacDriver.TxBuffer);
> > +  if (EFI_ERROR (Status)) {
> > +    DEBUG ((DEBUG_ERROR, "CreateDwEmacSnpDxe: DmaAllocateBuffer TxBuffer: %r\n", Status));
> > +    return Status;
> > +  }
> > +  Status = DmaMap (MapOperationBusMasterCommonBuffer, Snp->MacDriver.TxBuffer[0], &BufferSizeBuf, &Snp->MacDriver.TxBufferMap[0], &Snp->MappingTxbuf);
> > +  if (EFI_ERROR (Status)) {
> > +    DEBUG ((DEBUG_ERROR, "CreateDwEmacSnpDxe: DmaMap TxBuffer: %r\n", Status));
> > +    return Status;
> > +  }
> > +
>
> Why are you using common buffers for TX and RX buffers? This means you are
> copying data in and out of uncached buffers, which is terribly slow. Instead,
> you should use DmaMap() and DmaUnmap() to map ordinary buffers passed to the
> SNP layer - the DMA library will perform the required cache maintenance to
> ensure that the device and the CPU see the correct data.
>
> > +  // DMA Allocte Buffer and Map for RxBuffer
> > +  Status = DmaAllocateBuffer (EfiBootServicesData, EFI_SIZE_TO_PAGES ((sizeof (CHAR8))*10*2048), (VOID*)&Snp->MacDriver.RxBuffer);
> > +  if (EFI_ERROR (Status)) {
> > +    DEBUG ((DEBUG_ERROR, "CreateDwEmacSnpDxe: DmaAllocateBuffer RxBuffer: %r\n", Status));
> > +    return Status;
> > +  }
> > +  Status = DmaMap (MapOperationBusMasterWrite, Snp->MacDriver.RxBuffer[0], &BufferSizeBuf, &Snp->MacDriver.RxBufferMap[0], &Snp->MappingRxbuf);
> > +  if (EFI_ERROR (Status)) {
> > +    DEBUG ((DEBUG_ERROR, "CreateDwEmacSnpDxe: DmaMap RxBuffer: %r\n", Status));
> > +    return Status;
> > +  }
> > +
> > +
> > +  DevicePath = (EFI_SIMPLE_NETWORK_DEVICE_PATH*)AllocateCopyPool (sizeof (EFI_SIMPLE_NETWORK_DEVICE_PATH), &PathTemplate);
> > +  if (DevicePath == NULL) {
> > +    return EFI_OUT_OF_RESOURCES;
> > +  }
> > +
> > +  Status = gBS->OpenProtocol (Controller,
> > +                              &gEdkiiNonDiscoverableDeviceProtocolGuid,
> > +                              (VOID **)&Snp->Dev,
> > +                              This->DriverBindingHandle,
> > +                              Controller,
> > +                              EFI_OPEN_PROTOCOL_BY_DRIVER);
> > +
>
> Move this to the beginning, and handle any errors.
>
> > +  // Initialized signature (used by INSTANCE_FROM_SNP_THIS macro)
> > +  Snp->Signature = SNP_DRIVER_SIGNATURE;
> > +
> > +  EfiInitializeLock (&Snp->Lock, TPL_CALLBACK);
> > +
> > +  // Initialize pointers
> > +  SnpMode = &(Snp->SnpMode);
>
> No need for ()
>
> > +  Snp->Snp.Mode = SnpMode;
> > +
> > +  // Assign fields and func pointers
> > +  Snp->Snp.Revision = EFI_SIMPLE_NETWORK_PROTOCOL_REVISION;
> > +  Snp->Snp.WaitForPacket = NULL;
> > +  Snp->Snp.Initialize = SnpInitialize;
> > +  Snp->Snp.Start = SnpStart;
> > +  Snp->Snp.Stop = SnpStop;
> > +  Snp->Snp.Reset = SnpReset;
> > +  Snp->Snp.Shutdown = SnpShutdown;
> > +  Snp->Snp.ReceiveFilters = SnpReceiveFilters;
> > +  Snp->Snp.StationAddress = SnpStationAddress;
> > +  Snp->Snp.Statistics = SnpStatistics;
> > +  Snp->Snp.MCastIpToMac = SnpMcastIptoMac;
> > +  Snp->Snp.NvData = SnpNvData;
> > +  Snp->Snp.GetStatus = SnpGetStatus;
> > +  Snp->Snp.Transmit = SnpTransmit;
> > +  Snp->Snp.Receive = SnpReceive;
> > +
> > +  Snp->RecycledTxBuf = AllocatePool (sizeof (UINT64) * SNP_TX_BUFFER_INCREASEMENT);
> > +  if (Snp->RecycledTxBuf == NULL) {
> > +    Status = EFI_OUT_OF_RESOURCES;
> > +  }
> > +
> > +  Snp->MaxRecycledTxBuf    = SNP_TX_BUFFER_INCREASEMENT;
> > +  Snp->RecycledTxBufCount  = 0;
> > +
> > +  // Start completing simple network mode structure
> > +  SnpMode->State = EfiSimpleNetworkStopped;
> > +  SnpMode->HwAddressSize = NET_ETHER_ADDR_LEN; // HW address is 6 bytes
> > +  SnpMode->MediaHeaderSize = sizeof(ETHER_HEAD); // Not sure of this
>
> Please make sure this is correct. Also, use a space before (
>
> > +  SnpMode->MaxPacketSize = EFI_PAGE_SIZE; // Preamble + SOF + Ether Frame (with VLAN tag +4bytes)
> > +  SnpMode->NvRamSize = 0;           // No NVRAM with this device
> > +  SnpMode->NvRamAccessSize = 0; // No NVRAM with this device
> > +
> > +  // Update network mode information
> > +  SnpMode->ReceiveFilterMask = EFI_SIMPLE_NETWORK_RECEIVE_UNICAST     |
> > +                               EFI_SIMPLE_NETWORK_RECEIVE_MULTICAST   |
> > +                               EFI_SIMPLE_NETWORK_RECEIVE_BROADCAST   |
> > +                               EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS |
> > +                               EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS_MULTICAST;
> > +
> > +  // We do not intend to receive anything for the time being.
> > +  SnpMode->ReceiveFilterSetting = 0;
> > +
> > +  // EMAC has 64bit hash table, can filter 64 MCast MAC Addresses
> > +  SnpMode->MaxMCastFilterCount = MAX_MCAST_FILTER_CNT;
> > +  SnpMode->MCastFilterCount = 0;
> > +  ZeroMem (&SnpMode->MCastFilter, MAX_MCAST_FILTER_CNT * sizeof(EFI_MAC_ADDRESS));
> > +
> > +  // Set the interface type (1: Ethernet or 6: IEEE 802 Networks)
> > +  SnpMode->IfType = NET_IFTYPE_ETHERNET;
> > +
> > +  // Mac address is changeable as it is loaded from erasable memory
> > +  SnpMode->MacAddressChangeable = TRUE;
> > +
> > +  // Can only transmit one packet at a time
> > +  SnpMode->MultipleTxSupported = FALSE;
> > +
> > +  // MediaPresent checks for cable connection and partner link
> > +  SnpMode->MediaPresentSupported = TRUE;
> > +  SnpMode->MediaPresent = FALSE;
> > +
> > +  // Set broadcast address
> > +  SetMem (&SnpMode->BroadcastAddress, sizeof (EFI_MAC_ADDRESS), 0xFF);
> > +
> > +  //Set current address
> > +  DefaultMacAddress = Snp->Dev->Resources[1].AddrRangeMin;
> > +  // Swap PCD human readable form to correct endianess
> > +  SwapMacAddressPtr = (EFI_MAC_ADDRESS *) &DefaultMacAddress;
> > +  SnpMode->CurrentAddress.Addr[0] = SwapMacAddressPtr->Addr[5];
> > +  SnpMode->CurrentAddress.Addr[1] = SwapMacAddressPtr->Addr[4];
> > +  SnpMode->CurrentAddress.Addr[2] = SwapMacAddressPtr->Addr[3];
> > +  SnpMode->CurrentAddress.Addr[3] = SwapMacAddressPtr->Addr[2];
> > +  SnpMode->CurrentAddress.Addr[4] = SwapMacAddressPtr->Addr[1];
> > +  SnpMode->CurrentAddress.Addr[5] = SwapMacAddressPtr->Addr[0];
> > +
> > +  // Assign fields for device path
> > +  CopyMem (&DevicePath->MacAddrDP.MacAddress, &Snp->Snp.Mode->CurrentAddress, NET_ETHER_ADDR_LEN);
> > +  DevicePath->MacAddrDP.IfType = Snp->Snp.Mode->IfType;
> > +
> > +  Status = gBS->InstallMultipleProtocolInterfaces (
> > +                  &Controller,
> > +                  &gEfiSimpleNetworkProtocolGuid, &(Snp->Snp),
> > +                                 &gEfiDevicePathProtocolGuid, DevicePath,
> > +                                 NULL
> > +                  );
> > +
>
> Please fix the indentation
>
> > +  if (EFI_ERROR(Status)) {
> > +  gBS->CloseProtocol (Controller,
> > +                      &gEdkiiNonDiscoverableDeviceProtocolGuid,
> > +                                         This->DriverBindingHandle,
> > +                      Controller);
> > +
> > +  FreePages (Snp, EFI_SIZE_TO_PAGES (sizeof (EFI_SIMPLE_NETWORK_DRIVER)));
>
> Please fix the indentation
>
> > +  } else {
> > +    Snp->ControllerHandle = Controller;
> > +  }
> > +
> > +  return Status;
> > +}
> > +
> > +STATIC
> > +EFI_STATUS
> > +EFIAPI
> > +DriverStop (
> > +  IN  EFI_DRIVER_BINDING_PROTOCOL *This,
> > +  IN  EFI_HANDLE                  Controller,
> > +  IN  UINTN                       NumberOfChildren,
> > +  IN  EFI_HANDLE                  *ChildHandleBuffer
> > +  )
> > +{
> > +  EFI_STATUS Status;
> > +  EFI_SIMPLE_NETWORK_PROTOCOL *SnpProtocol;
> > +  EFI_SIMPLE_NETWORK_DRIVER   *Snp;
> > +
> > +  Status = gBS->HandleProtocol (
> > +                  Controller,
> > +                  &gEfiSimpleNetworkProtocolGuid,
> > +                  (VOID**)&SnpProtocol
>
> Space before ** please
>
> > +                );
> > +  if (EFI_ERROR (Status)) {
> > +    DEBUG ((DEBUG_ERROR, "DriverStop: HandleProtocol: %r\n", Status));
>
> Please use %a and __FUNCTION__
>
> > +    return Status;
> > +  }
> > +
> > +  Snp = INSTANCE_FROM_SNP_THIS(SnpProtocol);
> > +
> > +  Status = gBS->UninstallMultipleProtocolInterfaces (
> > +                  Controller,
> > +                  &gEfiSimpleNetworkProtocolGuid,
> > +                  &Snp->Snp,
> > +                  NULL);
> > +  if (EFI_ERROR (Status)) {
> > +    DEBUG ((DEBUG_ERROR, "DriverStop: UninstallMultipleProtocolInterfaces: %r\n",
> > +      Status));
> > +    return Status;
> > +  }
> > +
> > +  DmaUnmap(Snp->MappingTxdesc);
> > +  DmaUnmap(Snp->MappingRxdesc);
> > +  DmaUnmap(Snp->MappingTxbuf);
> > +  DmaUnmap(Snp->MappingRxbuf);
> > +
>
> Space before (
>
> > +  FreePool (Snp->RecycledTxBuf);
> > +  FreePages (Snp, EFI_SIZE_TO_PAGES (sizeof (EFI_SIMPLE_NETWORK_DRIVER)));
> > +
> > +  return Status;
> > +}
> > +
> > +
> > +/**
> > +   UEFI Driver Entry Point API
> > +   @param  ImageHandle       EFI_HANDLE.
> > +   @param  SystemTable       EFI_SYSTEM_TABLE.
> > +   @return EFI_SUCCESS       Success.
> > +   EFI_DEVICE_ERROR  Fail.
> > +**/
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +DwEmacSnpDxeEntry (
> > +  IN  EFI_HANDLE ImageHandle,
> > +  IN  EFI_SYSTEM_TABLE *SystemTable
> > +  )
> > +{
> > +  EFI_STATUS        Status;
> > +  EFI_HANDLE        Handle;
> > +
> > +  Handle = NULL;
> > +  Status = RegisterDevice (&gDwEmacNetNonDiscoverableDeviceGuid, mDwEmacNetDesc,
> > +             &Handle);
> > +  ASSERT_EFI_ERROR (Status);
> > +
>
> As i mentioned above, registering the device should be the responsibility of
> a platform driver, not the device driver
>
> > +  Status = EfiLibInstallDriverBindingComponentName2 (
> > +             ImageHandle,
> > +             SystemTable,
> > +             &mDriverBinding,
> > +             ImageHandle,
> > +             &gSnpComponentName,
> > +             &gSnpComponentName2
> > +             );
> > +
> > +  return Status;
> > +
> > +}
> > \ No newline at end of file
>
> Fix this please
>
> > diff --git a/Silicon/DesignWare/Drivers/DwEmacSnpDxe/DwEmacSnpDxe.c b/Silicon/DesignWare/Drivers/DwEmacSnpDxe/DwEmacSnpDxe.c
> > new file mode 100755
> > index 000000000000..cee1584c599e
> > --- /dev/null
> > +++ b/Silicon/DesignWare/Drivers/DwEmacSnpDxe/DwEmacSnpDxe.c
> > @@ -0,0 +1,1273 @@
> > +/** @file
> > +  DW EMAC SNP DXE driver
> > +
> > +  Copyright (c) 2011 - 2019, Intel Corporaton. All rights reserved.
> > +  This program and the accompanying materials
> > +  are licensed and made available under the terms and conditions of the BSD License
> > +  which accompanies this distribution.  The full text of the license may be found at
> > +  http://opensource.org/licenses/bsd-license.php
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > +
> > +  The original software modules are licensed as follows:
> > +
> > +  Copyright (c) 2012 - 2014, ARM Limited. All rights reserved.
> > +  Copyright (c) 2004 - 2010, Intel Corporation. All rights reserved.
> > +
> > +  This program and the accompanying materials
> > +  are licensed and made available under the terms and conditions of the BSD License
> > +  which accompanies this distribution.  The full text of the license may be found at
> > +  http://opensource.org/licenses/bsd-license.php
> > +
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD 3 Clause LICENSE ON AN "AS IS" BASIS,
> > +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > +**/
> > +
> > +#include "DwEmacSnpDxe.h"
> > +#include "EmacDxeUtil.h"
> > +#include "PhyDxeUtil.h"
> > +
> > +/**
> > +  Change the state of a network interface from "stopped" to "started."
> > +
> > +  This function starts a network interface. If the network interface successfully
> > +  starts, then EFI_SUCCESS will be returned.
> > +
> > +  @param  Snp                    A pointer to the EFI_SIMPLE_NETWORK_PROTOCOL instance.
> > +
> > +  @retval EFI_SUCCESS            The network interface was started.
> > +  @retval EFI_ALREADY_STARTED    The network interface is already in the started state.
> > +  @retval EFI_INVALID_PARAMETER  This parameter was NULL or did not point to a valid
> > +                                 EFI_SIMPLE_NETWORK_PROTOCOL structure.
> > +  @retval EFI_DEVICE_ERROR       The command could not be sent to the network interface.
> > +  @retval EFI_UNSUPPORTED        This function is not supported by the network interface.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +SnpStart (
> > +  IN  EFI_SIMPLE_NETWORK_PROTOCOL    *This
> > + )
> > +{
> > +  EFI_SIMPLE_NETWORK_DRIVER    *Snp;
>
> Blank line after variable declarations
>
> > +  DEBUG ((EFI_D_INFO,"SNP:DXE: %a ()\r\n", __FUNCTION__));
> > +
>
> Use DEBUG_ constants not EFI_D_ constants (applies everywhere)
>
> > +  // Check Snp instance
> > +  if (This == NULL) {
> > +    return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  Snp = INSTANCE_FROM_SNP_THIS(This);
> > +  // Check state
> > +  if ((Snp->SnpMode.State == EfiSimpleNetworkStarted)    ||
> > +      (Snp->SnpMode.State == EfiSimpleNetworkInitialized)  ) {
> > +    return EFI_ALREADY_STARTED;
> > +  }
> > +
> > +  // Change state
> > +  Snp->SnpMode.State = EfiSimpleNetworkStarted;
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +
> > +/**
> > +  Changes the state of a network interface from "started" to "stopped."
> > +
> > +  This function stops a network interface. This call is only valid if the network
> > +  interface is in the started state. If the network interface was successfully
> > +  stopped, then EFI_SUCCESS will be returned.
> > +
> > +  @param  Snp                     A pointer to the EFI_SIMPLE_NETWORK_PROTOCOL
> > +                                  instance.
> > +
> > +
> > +  @retval EFI_SUCCESS             The network interface was stopped.
> > +  @retval EFI_NOT_STARTED         The network interface has not been started.
> > +  @retval EFI_INVALID_PARAMETER   This parameter was NULL or did not point to a
> > +                                  valid EFI_SIMPLE_NETWORK_PROTOCOL structure.
> > +  @retval EFI_DEVICE_ERROR        The command could not be sent to the network
> > +                                  interface.
> > +  @retval EFI_UNSUPPORTED         This function is not supported by the network
> > +                                  interface.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +SnpStop (
> > +  IN        EFI_SIMPLE_NETWORK_PROTOCOL* This
> > +  )
> > +{
> > +  EFI_SIMPLE_NETWORK_DRIVER    *Snp;
> > +  UINT32        MacBase;
> > +  DEBUG ((EFI_D_INFO, "SNP:DXE: %a ()\r\n", __FUNCTION__));
> > +
>
> Same as above
>
> > +  // Check Snp Instance
> > +  if (This == NULL) {
> > +    return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  Snp = INSTANCE_FROM_SNP_THIS(This);
> > +  // Check state of the driver
> > +  if (Snp->SnpMode.State == EfiSimpleNetworkStopped) {
> > +    return EFI_NOT_STARTED;
> > +  }
> > +
> > +  //Get MAC base address
> > +  MacBase = (UINT32)Snp->Dev->Resources[0].AddrRangeMin;
> > +
>
> Can you add the base address to the driver struct directly, rather than
> do this triple dereference in every driver method? Also, a base address
> should be EFI_PHYSICAL_ADDRESS or UINTN, never UINT32.
>
> > +  // Stop the Tx and Rx
> > +  EmacStopTxRx (MacBase);
> > +  // Change the state
> > +  switch (Snp->SnpMode.State) {
> > +    case EfiSimpleNetworkStarted:
> > +    case EfiSimpleNetworkInitialized:
> > +      Snp->SnpMode.State = EfiSimpleNetworkStopped;
> > +      break;
> > +    default:
> > +      return EFI_DEVICE_ERROR;
> > +  }
> > +
> > +  // Put the device into a power saving mode ?
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +
> > +/**
> > +  Resets a network adapter and allocates the transmit and receive buffers
> > +  required by the network interface; optionally, also requests allocation of
> > +  additional transmit and receive buffers.
> > +
> > +  This function allocates the transmit and receive buffers required by the network
> > +  interface. If this allocation fails, then EFI_OUT_OF_RESOURCES is returned.
> > +  If the allocation succeeds and the network interface is successfully initialized,
> > +  then EFI_SUCCESS will be returned.
> > +
> > +  @param Snp                A pointer to the EFI_SIMPLE_NETWORK_PROTOCOL instance.
> > +
> > +  @param ExtraRxBufferSize  The size, in bytes, of the extra receive buffer space
> > +                            that the driver should allocate for the network interface.
> > +                            Some network interfaces will not be able to use the
> > +                            extra buffer, and the caller will not know if it is
> > +                            actually being used.
> > +  @param ExtraTxBufferSize  The size, in bytes, of the extra transmit buffer space
> > +                            that the driver should allocate for the network interface.
> > +                            Some network interfaces will not be able to use the
> > +                            extra buffer, and the caller will not know if it is
> > +                            actually being used.
> > +
> > +  @retval EFI_SUCCESS           The network interface was initialized.
> > +  @retval EFI_NOT_STARTED       The network interface has not been started.
> > +  @retval EFI_OUT_OF_RESOURCES  There was not enough memory for the transmit and
> > +                                receive buffers.
> > +  @retval EFI_INVALID_PARAMETER This parameter was NULL or did not point to a valid
> > +                                EFI_SIMPLE_NETWORK_PROTOCOL structure.
> > +  @retval EFI_DEVICE_ERROR      The command could not be sent to the network interface.
> > +  @retval EFI_UNSUPPORTED       The increased buffer size feature is not supported.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +SnpInitialize (
> > +  IN EFI_SIMPLE_NETWORK_PROTOCOL *This,
> > +  IN UINTN                       ExtraRxBufferSize OPTIONAL,
> > +  IN UINTN                       ExtraTxBufferSize OPTIONAL
> > +  )
> > +{
> > +  EFI_STATUS                  Status;
> > +  EFI_SIMPLE_NETWORK_DRIVER  *Snp;
> > +  UINT32                      MacBase;
> > +
> > +  DEBUG ((EFI_D_INFO, "SNP:DXE: %a ()\r\n", __FUNCTION__));
> > +
> > +  // Check Snp Instance
> > +  if (This == NULL) {
> > +    return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  Snp = INSTANCE_FROM_SNP_THIS (This);
> > +  // First check that driver has not already been initialized
> > +  if (Snp->SnpMode.State == EfiSimpleNetworkInitialized) {
> > +    return EFI_SUCCESS;
> > +  } else
>
> Put else and if on the same line
>
> > +  if (Snp->SnpMode.State == EfiSimpleNetworkStopped) {
> > +    return EFI_NOT_STARTED;
> > +  }
> > +
> > +  //Get MAC base address
> > +  MacBase = (UINT32)Snp->Dev->Resources[0].AddrRangeMin;
> > +
> > +  // Init PHY
> > +  Status = PhyDxeInitialization (&Snp->PhyDriver, MacBase);
> > +  if (EFI_ERROR (Status)) {
> > +    return EFI_DEVICE_ERROR;
> > +  }
> > +
> > +  // Init EMAC
> > +  Status = EmacDxeInitialization (&Snp->MacDriver, MacBase);
> > +  if (EFI_ERROR (Status)) {
> > +    return EFI_DEVICE_ERROR;
> > +  }
> > +
> > +  // Set MAC Address
> > +  EmacSetMacAddress (&Snp->SnpMode.CurrentAddress, MacBase);
> > +  EmacReadMacAddress(&Snp->SnpMode.CurrentAddress, MacBase);
>
> Space before (
>
> > +
> > +  // Init Link
> > +  Print (L"SNP:DXE: Auto-Negotiating Ethernet PHY Link ...\r\n");
>
> Don't use Print() for debug messages. This goes straight to the console
>
> > +  Status = PhyLinkAdjustEmacConfig (&Snp->PhyDriver, MacBase);
> > +  if (EFI_ERROR(Status)) {
>
> Space before (
>
> > +    Print (L"SNP:DXE: Link is Down - Network Cable is not plugged in?\r\n");
> > +    return EFI_DEVICE_ERROR;
> > +  }
> > +
> > +  EmacStartTransmission (MacBase);
> > +
> > +  // Declare the driver as initialized
> > +  Snp->SnpMode.State = EfiSimpleNetworkInitialized;
> > +
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +
> > +/**
> > +  Resets a network adapter and reinitializes it with the parameters that were
> > +  provided in the previous call to Initialize().
> > +
> > +  This function resets a network adapter and reinitializes it with the parameters
> > +  that were provided in the previous call to Initialize(). The transmit and
> > +  receive queues are emptied and all pending interrupts are cleared.
> > +  Receive filters, the station address, the statistics, and the multicast-IP-to-HW
> > +  MAC addresses are not reset by this call. If the network interface was
> > +  successfully reset, then EFI_SUCCESS will be returned. If the driver has not
> > +  been initialized, EFI_DEVICE_ERROR will be returned.
> > +
> > +  @param Snp                  A pointer to the EFI_SIMPLE_NETWORK_PROTOCOL instance.
> > +  @param ExtendedVerification Indicates that the driver may perform a more
> > +                              exhaustive verification operation of the device
> > +                              during reset.
> > +
> > +  @retval EFI_SUCCESS           The network interface was reset.
> > +  @retval EFI_NOT_STARTED       The network interface has not been started.
> > +  @retval EFI_INVALID_PARAMETER One or more of the parameters has an unsupported value.
> > +  @retval EFI_DEVICE_ERROR      The command could not be sent to the network interface.
> > +  @retval EFI_UNSUPPORTED       This function is not supported by the network interface.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +SnpReset (
> > +  IN EFI_SIMPLE_NETWORK_PROTOCOL *This,
> > +  IN BOOLEAN                     ExtendedVerification
> > +  )
> > +{
> > +  EFI_STATUS                  Status;
> > +  EFI_SIMPLE_NETWORK_DRIVER  *Snp;
> > +  UINT32                      MacBase;
> > +
> > +  Snp = INSTANCE_FROM_SNP_THIS (This);
> > +
> > +  DEBUG ((EFI_D_INFO, "SNP:DXE: %a ()\r\n", __FUNCTION__));
> > +
> > +  // Check Snp Instance
> > +  if (This == NULL) {
> > +    return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  // First check that driver has not already been initialized
> > +  if (Snp->SnpMode.State == EfiSimpleNetworkStarted) {
> > +    return EFI_DEVICE_ERROR;
> > +  } else if (Snp->SnpMode.State == EfiSimpleNetworkStopped) {
> > +    return EFI_NOT_STARTED;
> > +  }
> > +
> > +  // Get MAC base address
> > +  MacBase = (UINT32)Snp->Dev->Resources[0].AddrRangeMin;
> > +
> > +  // Initiate a PHY reset
> > +  Status = PhySoftReset (&Snp->PhyDriver, MacBase);
> > +  if (EFI_ERROR (Status)) {
> > +    Snp->SnpMode.State = EfiSimpleNetworkStopped;
> > +    return EFI_NOT_STARTED;
> > +  }
> > +
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +
> > +/**
> > +  Resets a network adapter and leaves it in a state that is safe for another
> > +  driver to initialize.
> > +
> > +  This function releases the memory buffers assigned in the Initialize() call.
> > +  Pending transmits and receives are lost, and interrupts are cleared and disabled.
> > +  After this call, only the Initialize() and Stop() calls may be used. If the
> > +  network interface was successfully shutdown, then EFI_SUCCESS will be returned.
> > +  If the driver has not been initialized, EFI_DEVICE_ERROR will be returned.
> > +
> > +  @param  This  A pointer to the EFI_SIMPLE_NETWORK_PROTOCOL instance.
> > +
> > +  @retval EFI_SUCCESS           The network interface was shutdown.
> > +  @retval EFI_NOT_STARTED       The network interface has not been started.
> > +  @retval EFI_INVALID_PARAMETER This parameter was NULL or did not point to a valid
> > +                                EFI_SIMPLE_NETWORK_PROTOCOL structure.
> > +  @retval EFI_DEVICE_ERROR      The command could not be sent to the network interface.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +SnpShutdown (
> > +  IN        EFI_SIMPLE_NETWORK_PROTOCOL*  This
> > +  )
> > +{
> > +  EFI_SIMPLE_NETWORK_DRIVER  *Snp;
> > +  UINT32                      MacBase;
> > +  DEBUG ((EFI_D_INFO, "SNP:DXE: %a ()\r\n", __FUNCTION__));
> > +
> > +  // Check Snp Instance
> > +  if (This == NULL) {
> > +    return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  Snp = INSTANCE_FROM_SNP_THIS (This);
> > +  // First check that driver has not already been initialized
> > +  if (Snp->SnpMode.State == EfiSimpleNetworkStarted) {
> > +    return EFI_DEVICE_ERROR;
> > +  } else if (Snp->SnpMode.State == EfiSimpleNetworkStopped) {
> > +    return EFI_NOT_STARTED;
> > +  }
> > +
> > +  //Get MAC base address
> > +  MacBase = (UINT32)Snp->Dev->Resources[0].AddrRangeMin;
> > +
> > +  EmacStopTxRx (MacBase);
> > +
> > +  Snp->SnpMode.State = EfiSimpleNetworkStopped;
> > +
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +
> > +/**
> > +  Manages the multicast receive filters of a network interface.
> > +
> > +  This function is used enable and disable the hardware and software receive
> > +  filters for the underlying network device.
> > +  The receive filter change is broken down into three steps:
> > +  * The filter mask bits that are set (ON) in the Enable parameter are added to
> > +    the current receive filter settings.
> > +  * The filter mask bits that are set (ON) in the Disable parameter are subtracted
> > +    from the updated receive filter settings.
> > +  * If the resulting receive filter setting is not supported by the hardware a
> > +    more liberal setting is selected.
> > +  If the same bits are set in the Enable and Disable parameters, then the bits
> > +  in the Disable parameter takes precedence.
> > +  If the ResetMCastFilter parameter is TRUE, then the multicast address list
> > +  filter is disabled (irregardless of what other multicast bits are set in the
> > +  Enable and Disable parameters). The SNP->Mode->MCastFilterCount field is set
> > +  to zero. The Snp->Mode->MCastFilter contents are undefined.
> > +  After enabling or disabling receive filter settings, software should verify
> > +  the new settings by checking the Snp->Mode->ReceiveFilterSettings,
> > +  Snp->Mode->MCastFilterCount and Snp->Mode->MCastFilter fields.
> > +  Note: Some network drivers and/or devices will automatically promote receive
> > +    filter settings if the requested setting can not be honored. For example, if
> > +    a request for four multicast addresses is made and the underlying hardware
> > +    only supports two multicast addresses the driver might set the promiscuous
> > +    or promiscuous multicast receive filters instead. The receiving software is
> > +    responsible for discarding any extra packets that get through the hardware
> > +    receive filters.
> > +    Note: Note: To disable all receive filter hardware, the network driver must
> > +      be Shutdown() and Stopped(). Calling ReceiveFilters() with Disable set to
> > +      Snp->Mode->ReceiveFilterSettings will make it so no more packets are
> > +      returned by the Receive() function, but the receive hardware may still be
> > +      moving packets into system memory before inspecting and discarding them.
> > +      Unexpected system errors, reboots and hangs can occur if an OS is loaded
> > +      and the network devices are not Shutdown() and Stopped().
> > +  If ResetMCastFilter is TRUE, then the multicast receive filter list on the
> > +  network interface will be reset to the default multicast receive filter list.
> > +  If ResetMCastFilter is FALSE, and this network interface allows the multicast
> > +  receive filter list to be modified, then the MCastFilterCnt and MCastFilter
> > +  are used to update the current multicast receive filter list. The modified
> > +  receive filter list settings can be found in the MCastFilter field of
> > +  EFI_SIMPLE_NETWORK_MODE. If the network interface does not allow the multicast
> > +  receive filter list to be modified, then EFI_INVALID_PARAMETER will be returned.
> > +  If the driver has not been initialized, EFI_DEVICE_ERROR will be returned.
> > +  If the receive filter mask and multicast receive filter list have been
> > +  successfully updated on the network interface, EFI_SUCCESS will be returned.
> > +
> > +  @param Snp              A pointer to the EFI_SIMPLE_NETWORK_PROTOCOL instance.
> > +  @param Enable           A bit mask of receive filters to enable on the network
> > +                          interface.
> > +  @param Disable          A bit mask of receive filters to disable on the network
> > +                          interface. For backward compatibility with EFI 1.1
> > +                          platforms, the EFI_SIMPLE_NETWORK_RECEIVE_MULTICAST bit
> > +                          must be set when the ResetMCastFilter parameter is TRUE.
> > +  @param ResetMCastFilter Set to TRUE to reset the contents of the multicast
> > +                          receive filters on the network interface to their
> > +                          default values.
> > +  @param MCastFilterCnt   Number of multicast HW MAC addresses in the new MCastFilter
> > +                          list. This value must be less than or equal to the
> > +                          MCastFilterCnt field of EFI_SIMPLE_NETWORK_MODE.
> > +                          This field is optional if ResetMCastFilter is TRUE.
> > +  @param MCastFilter      A pointer to a list of new multicast receive filter HW
> > +                          MAC addresses. This list will replace any existing
> > +                          multicast HW MAC address list. This field is optional
> > +                          if ResetMCastFilter is TRUE.
> > +
> > +  @retval EFI_SUCCESS            The multicast receive filter list was updated.
> > +  @retval EFI_NOT_STARTED        The network interface has not been started.
> > +  @retval EFI_INVALID_PARAMETER  One or more of the following conditions is TRUE:
> > +                                 * This is NULL
> > +                                 * There are bits set in Enable that are not set
> > +                                   in Snp->Mode->ReceiveFilterMask
> > +                                 * There are bits set in Disable that are not set
> > +                                   in Snp->Mode->ReceiveFilterMask
> > +                                 * Multicast is being enabled (the
> > +                                   EFI_SIMPLE_NETWORK_RECEIVE_MULTICAST bit is
> > +                                   set in Enable, it is not set in Disable, and
> > +                                   ResetMCastFilter is FALSE) and MCastFilterCount
> > +                                   is zero
> > +                                 * Multicast is being enabled and MCastFilterCount
> > +                                   is greater than Snp->Mode->MaxMCastFilterCount
> > +                                 * Multicast is being enabled and MCastFilter is NULL
> > +                                 * Multicast is being enabled and one or more of
> > +                                   the addresses in the MCastFilter list are not
> > +                                   valid multicast MAC addresses
> > +  @retval EFI_DEVICE_ERROR       One or more of the following conditions is TRUE:
> > +                                 * The network interface has been started but has
> > +                                   not been initialized
> > +                                 * An unexpected error was returned by the
> > +                                   underlying network driver or device
> > +  @retval EFI_UNSUPPORTED        This function is not supported by the network
> > +                                 interface.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +SnpReceiveFilters (
> > +  IN  EFI_SIMPLE_NETWORK_PROTOCOL  *This,
> > +  IN  UINT32                       Enable,
> > +  IN  UINT32                       Disable,
> > +  IN  BOOLEAN                      ResetMCastFilter,
> > +  IN  UINTN                        MCastFilterCnt  OPTIONAL,
> > +  IN  EFI_MAC_ADDRESS              *MCastFilter  OPTIONAL
> > +  )
> > +{
> > +  UINT32                      ReceiveFilterSetting;
> > +  EFI_SIMPLE_NETWORK_DRIVER  *Snp;
> > +  UINT32                      MacBase;
> > +
> > +  Snp = INSTANCE_FROM_SNP_THIS (This);
> > +
> > +  // Check Snp Instance
> > +  if (Snp == NULL) {
> > +    return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  // Check that driver was started and initialised
> > +  if (Snp->SnpMode.State == EfiSimpleNetworkStarted) {
> > +    return EFI_DEVICE_ERROR;
> > +  } else if (Snp->SnpMode.State == EfiSimpleNetworkStopped) {
> > +    return EFI_NOT_STARTED;
> > +  }
> > +
> > +   if ((Enable  & (~Snp->SnpMode.ReceiveFilterMask)) ||
> > +      (Disable & (~Snp->SnpMode.ReceiveFilterMask))    ) {
>
> Please clarify the voodoo bitwise boolean logic with a comment.
>
> > +    return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  //Get MAC base address
> > +  MacBase = (UINT32)Snp->Dev->Resources[0].AddrRangeMin;
> > +
> > +  ReceiveFilterSetting = (Snp->SnpMode.ReceiveFilterSetting | Enable) & (~Disable);
> > +
>
> Same here.
>
> > +  EmacRxFilters (ReceiveFilterSetting, ResetMCastFilter, MCastFilterCnt, MCastFilter, MacBase);
> > +
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +
> > +/**
> > +  Modifies or resets the current station address, if supported.
> > +
> > +  This function modifies or resets the current station address of a network
> > +  interface, if supported. If Reset is TRUE, then the current station address is
> > +  set to the network interface's permanent address. If Reset is FALSE, and the
> > +  network interface allows its station address to be modified, then the current
> > +  station address is changed to the address specified by New. If the network
> > +  interface does not allow its station address to be modified, then
> > +  EFI_INVALID_PARAMETER will be returned. If the station address is successfully
> > +  updated on the network interface, EFI_SUCCESS will be returned. If the driver
> > +  has not been initialized, EFI_DEVICE_ERROR will be returned.
> > +
> > +  @param Snp      A pointer to the EFI_SIMPLE_NETWORK_PROTOCOL instance.
> > +  @param Reset    Flag used to reset the station address to the network interface's
> > +                  permanent address.
> > +  @param NewMac   New station address to be used for the network interface.
> > +
> > +
> > +  @retval EFI_SUCCESS           The network interface's station address was updated.
> > +  @retval EFI_NOT_STARTED       The Simple Network Protocol interface has not been
> > +                                started by calling Start().
> > +  @retval EFI_INVALID_PARAMETER The New station address was not accepted by the NIC.
> > +  @retval EFI_INVALID_PARAMETER Reset is FALSE and New is NULL.
> > +  @retval EFI_DEVICE_ERROR      The Simple Network Protocol interface has not
> > +                                been initialized by calling Initialize().
> > +  @retval EFI_DEVICE_ERROR      An error occurred attempting to set the new
> > +                                station address.
> > +  @retval EFI_UNSUPPORTED       The NIC does not support changing the network
> > +                                interface's station address.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +SnpStationAddress (
> > +  IN        EFI_SIMPLE_NETWORK_PROTOCOL *This,
> > +  IN        BOOLEAN                      Reset,
> > +  IN        EFI_MAC_ADDRESS             *NewMac
> > +)
> > +{
> > +  return EFI_UNSUPPORTED;
>
> Further up, you set a property saying that the MAC address is changeble, no?
>
> > +}
> > +
> > +
> > +
> > +/**
> > +  Resets or collects the statistics on a network interface.
> > +
> > +  This function resets or collects the statistics on a network interface. If the
> > +  size of the statistics table specified by StatisticsSize is not big enough for
> > +  all the statistics that are collected by the network interface, then a partial
> > +  buffer of statistics is returned in StatisticsTable, StatisticsSize is set to
> > +  the size required to collect all the available statistics, and
> > +  EFI_BUFFER_TOO_SMALL is returned.
> > +  If StatisticsSize is big enough for all the statistics, then StatisticsTable
> > +  will be filled, StatisticsSize will be set to the size of the returned
> > +  StatisticsTable structure, and EFI_SUCCESS is returned.
> > +  If the driver has not been initialized, EFI_DEVICE_ERROR will be returned.
> > +  If Reset is FALSE, and both StatisticsSize and StatisticsTable are NULL, then
> > +  no operations will be performed, and EFI_SUCCESS will be returned.
> > +  If Reset is TRUE, then all of the supported statistics counters on this network
> > +  interface will be reset to zero.
> > +
> > +  @param Snp             A pointer to the EFI_SIMPLE_NETWORK_PROTOCOL instance.
> > +  @param Reset           Set to TRUE to reset the statistics for the network interface.
> > +  @param StatSize        On input the size, in bytes, of StatisticsTable. On output
> > +                         the size, in bytes, of the resulting table of statistics.
> > +  @param Statistics      A pointer to the EFI_NETWORK_STATISTICS structure that
> > +                         contains the statistics. Type EFI_NETWORK_STATISTICS is
> > +                         defined in "Related Definitions" below.
> > +
> > +  @retval EFI_SUCCESS           The requested operation succeeded.
> > +  @retval EFI_NOT_STARTED       The Simple Network Protocol interface has not been
> > +                                started by calling Start().
> > +  @retval EFI_BUFFER_TOO_SMALL  StatisticsSize is not NULL and StatisticsTable is
> > +                                NULL. The current buffer size that is needed to
> > +                                hold all the statistics is returned in StatisticsSize.
> > +  @retval EFI_BUFFER_TOO_SMALL  StatisticsSize is not NULL and StatisticsTable is
> > +                                not NULL. The current buffer size that is needed
> > +                                to hold all the statistics is returned in
> > +                                StatisticsSize. A partial set of statistics is
> > +                                returned in StatisticsTable.
> > +  @retval EFI_INVALID_PARAMETER StatisticsSize is NULL and StatisticsTable is not
> > +                                NULL.
> > +  @retval EFI_DEVICE_ERROR      The Simple Network Protocol interface has not
> > +                                been initialized by calling Initialize().
> > +  @retval EFI_DEVICE_ERROR      An error was encountered collecting statistics
> > +                                from the NIC.
> > +  @retval EFI_UNSUPPORTED       The NIC does not support collecting statistics
> > +                                from the network interface.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +SnpStatistics (
> > +  IN        EFI_SIMPLE_NETWORK_PROTOCOL  *This,
> > +  IN        BOOLEAN                      Reset,
> > +  IN  OUT   UINTN                        *StatSize,
> > +      OUT   EFI_NETWORK_STATISTICS       *Statistics
> > +  )
> > +{
> > +  EFI_SIMPLE_NETWORK_DRIVER  *Snp;
> > +  UINT32                      MacBase;
> > +
> > +  Snp = INSTANCE_FROM_SNP_THIS (This);
> > +
> > +  DEBUG ((EFI_D_INFO, "SNP:DXE: %a ()\r\n", __FUNCTION__));
> > +
> > +  // Check Snp instance
> > +  if (This == NULL) {
> > +    return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  // Check that driver was started and initialised
> > +  if (Snp->SnpMode.State == EfiSimpleNetworkStarted) {
> > +    return EFI_DEVICE_ERROR;
> > +  } else if (Snp->SnpMode.State == EfiSimpleNetworkStopped) {
> > +    return EFI_NOT_STARTED;
> > +  }
> > +
> > +  // Check pointless condition
>
> Wait what??
>
> > +  if ((!Reset) && (StatSize == NULL) && (Statistics == NULL)) {
> > +    return EFI_SUCCESS;
> > +  }
> > +
> > +  // Check the parameters
> > +  if ((StatSize == NULL) && (Statistics != NULL)) {
> > +    return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  // Do a reset if required
> > +  if (Reset) {
> > +    ZeroMem (&Snp->Stats, sizeof(EFI_NETWORK_STATISTICS));
> > +  }
> > +
> > +  // Check buffer size
> > +  if (*StatSize < sizeof(EFI_NETWORK_STATISTICS)) {
> > +    *StatSize = sizeof(EFI_NETWORK_STATISTICS);
> > +    return EFI_BUFFER_TOO_SMALL;
> > +  }
> > +
> > +  //Get MAC base address
> > +  MacBase = (UINT32)Snp->Dev->Resources[0].AddrRangeMin;
> > +
> > +  // read statistic counters
> > +  EmacGetStatistic (&Snp->Stats, MacBase);
> > +
> > +  // Fill in the statistics
> > +  CopyMem(&Statistics, &Snp->Stats, sizeof(EFI_NETWORK_STATISTICS));
>
> Space before (
>
> > +
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +
> > +/**
> > +  Converts a multicast IP address to a multicast HW MAC address.
> > +
> > +  This function converts a multicast IP address to a multicast HW MAC address
> > +  for all packet transactions. If the mapping is accepted, then EFI_SUCCESS will
> > +  be returned.
> > +
> > +  @param Snp         A pointer to the EFI_SIMPLE_NETWORK_PROTOCOL instance.
> > +  @param IsIpv6      Set to TRUE if the multicast IP address is IPv6 [RFC 2460].
> > +                     Set to FALSE if the multicast IP address is IPv4 [RFC 791].
> > +  @param Ip          The multicast IP address that is to be converted to a multicast
> > +                     HW MAC address.
> > +  @param McastMac    The multicast HW MAC address that is to be generated from IP.
> > +
> > +  @retval EFI_SUCCESS           The multicast IP address was mapped to the
> > +                                multicast HW MAC address.
> > +  @retval EFI_NOT_STARTED       The Simple Network Protocol interface has not
> > +                                been started by calling Start().
> > +  @retval EFI_INVALID_PARAMETER IP is NULL.
> > +  @retval EFI_INVALID_PARAMETER MAC is NULL.
> > +  @retval EFI_INVALID_PARAMETER IP does not point to a valid IPv4 or IPv6
> > +                                multicast address.
> > +  @retval EFI_DEVICE_ERROR      The Simple Network Protocol interface has not
> > +                                been initialized by calling Initialize().
> > +  @retval EFI_UNSUPPORTED       IPv6 is TRUE and the implementation does not
> > +                                support IPv6 multicast to MAC address conversion.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +SnpMcastIptoMac (
> > +  IN        EFI_SIMPLE_NETWORK_PROTOCOL  *This,
> > +  IN        BOOLEAN                      IsIpv6,
> > +  IN        EFI_IP_ADDRESS               *Ip,
> > +      OUT   EFI_MAC_ADDRESS              *McastMac
> > +  )
> > +{
> > +
> > +  DEBUG ((EFI_D_INFO, "SNP:DXE: %a ()\r\n", __FUNCTION__));
> > +
> > +  // Check Snp instance
> > +  if (This == NULL) {
> > +    return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  EFI_SIMPLE_NETWORK_DRIVER  *Snp;
> > +
> > +  Snp = INSTANCE_FROM_SNP_THIS (This);
> > +
> > +  // Check that driver was started and initialised
> > +  if (Snp->SnpMode.State == EfiSimpleNetworkStarted) {
> > +    return EFI_DEVICE_ERROR;
> > +  } else if (Snp->SnpMode.State == EfiSimpleNetworkStopped) {
> > +    return EFI_NOT_STARTED;
> > +  }
> > +
> > +  // Check parameters
> > +  if ((McastMac == NULL) || (Ip == NULL)) {
> > +    return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  // Make sure MAC address is empty
> > +  ZeroMem (McastMac, sizeof(EFI_MAC_ADDRESS));
> > +
> > +  // If we need ipv4 address
> > +  if (!IsIpv6) {
> > +    // Most significant 25 bits of a multicast HW address are set.
> > +    // 01-00-5E is the IPv4 Ethernet Multicast Address (see RFC 1112)
> > +    McastMac->Addr[0] = 0x01;
> > +    McastMac->Addr[1] = 0x00;
> > +    McastMac->Addr[2] = 0x5E;
> > +
> > +    // Lower 23 bits from ipv4 address
> > +    McastMac->Addr[3] = (Ip->v4.Addr[1] & 0x7F); // Clear the most significant bit (25th bit of MAC must be 0)
> > +    McastMac->Addr[4] = Ip->v4.Addr[2];
> > +    McastMac->Addr[5] = Ip->v4.Addr[3];
> > +  } else {
> > +    // Most significant 16 bits of multicast v6 HW address are set
> > +    // 33-33 is the IPv6 Ethernet Multicast Address (see RFC 2464)
> > +    McastMac->Addr[0] = 0x33;
> > +    McastMac->Addr[1] = 0x33;
> > +
> > +    // lower four octets are taken from ipv6 address
> > +    McastMac->Addr[2] = Ip->v6.Addr[8];
> > +    McastMac->Addr[3] = Ip->v6.Addr[9];
> > +    McastMac->Addr[4] = Ip->v6.Addr[10];
> > +    McastMac->Addr[5] = Ip->v6.Addr[11];
> > +  }
> > +
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +
> > +/**
> > +  Performs read and write operations on the NVRAM device attached to a network
> > +  interface.
> > +
> > +  This function performs read and write operations on the NVRAM device attached
> > +  to a network interface. If ReadWrite is TRUE, a read operation is performed.
> > +  If ReadWrite is FALSE, a write operation is performed. Offset specifies the
> > +  byte offset at which to start either operation. Offset must be a multiple of
> > +  NvRamAccessSize , and it must have a value between zero and NvRamSize.
> > +  BufferSize specifies the length of the read or write operation. BufferSize must
> > +  also be a multiple of NvRamAccessSize, and Offset + BufferSize must not exceed
> > +  NvRamSize.
> > +  If any of the above conditions is not met, then EFI_INVALID_PARAMETER will be
> > +  returned.
> > +  If all the conditions are met and the operation is "read," the NVRAM device
> > +  attached to the network interface will be read into Buffer and EFI_SUCCESS
> > +  will be returned. If this is a write operation, the contents of Buffer will be
> > +  used to update the contents of the NVRAM device attached to the network
> > +  interface and EFI_SUCCESS will be returned.
> > +
> > +  It does the basic checking on the input parameters and retrieves snp structure
> > +  and then calls the read_nvdata() call which does the actual reading
> > +
> > +  @param Snp        A pointer to the EFI_SIMPLE_NETWORK_PROTOCOL instance.
> > +  @param ReadWrite  TRUE for read operations, FALSE for write operations.
> > +  @param Offset     Byte offset in the NVRAM device at which to start the read or
> > +                    write operation. This must be a multiple of NvRamAccessSize
> > +                    and less than NvRamSize. (See EFI_SIMPLE_NETWORK_MODE)
> > +  @param BufferSize The number of bytes to read or write from the NVRAM device.
> > +                    This must also be a multiple of NvramAccessSize.
> > +  @param Buffer     A pointer to the data buffer.
> > +
> > +  @retval EFI_SUCCESS           The NVRAM access was performed.
> > +  @retval EFI_NOT_STARTED       The network interface has not been started.
> > +  @retval EFI_INVALID_PARAMETER One or more of the following conditions is TRUE:
> > +                                * The This parameter is NULL
> > +                                * The This parameter does not point to a valid
> > +                                  EFI_SIMPLE_NETWORK_PROTOCOL  structure
> > +                                * The Offset parameter is not a multiple of
> > +                                  EFI_SIMPLE_NETWORK_MODE.NvRamAccessSize
> > +                                * The Offset parameter is not less than
> > +                                  EFI_SIMPLE_NETWORK_MODE.NvRamSize
> > +                                * The BufferSize parameter is not a multiple of
> > +                                  EFI_SIMPLE_NETWORK_MODE.NvRamAccessSize
> > +                                * The Buffer parameter is NULL
> > +  @retval EFI_DEVICE_ERROR      The command could not be sent to the network
> > +                                interface.
> > +  @retval EFI_UNSUPPORTED       This function is not supported by the network
> > +                                interface.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +SnpNvData (
> > +  IN EFI_SIMPLE_NETWORK_PROTOCOL *This,
> > +  IN BOOLEAN                     ReadWrite,
> > +  IN UINTN                       Offset,
> > +  IN UINTN                       BufferSize,
> > +  IN OUT VOID                    *Buffer
> > +  )
> > +{
> > +  return EFI_UNSUPPORTED;
> > +}
> > +
> > +
> > +/**
> > +  Reads the current interrupt status and recycled transmit buffer status from a
> > +  network interface.
> > +
> > +  This function gets the current interrupt and recycled transmit buffer status
> > +  from the network interface. The interrupt status is returned as a bit mask in
> > +  InterruptStatus. If InterruptStatus is NULL, the interrupt status will not be
> > +  read. If TxBuf is not NULL, a recycled transmit buffer address will be retrieved.
> > +  If a recycled transmit buffer address is returned in TxBuf, then the buffer has
> > +  been successfully transmitted, and the status for that buffer is cleared. If
> > +  the status of the network interface is successfully collected, EFI_SUCCESS
> > +  will be returned. If the driver has not been initialized, EFI_DEVICE_ERROR will
> > +  be returned.
> > +
> > +  @param Snp             A pointer to the EFI_SIMPLE_NETWORK_PROTOCOL instance.
> > +  @param IrqStat         A pointer to the bit mask of the currently active
> > +                         interrupts (see "Related Definitions"). If this is NULL,
> > +                         the interrupt status will not be read from the device.
> > +                         If this is not NULL, the interrupt status will be read
> > +                         from the device. When the interrupt status is read, it
> > +                         will also be cleared. Clearing the transmit interrupt does
> > +                         not empty the recycled transmit buffer array.
> > +  @param TxBuff          Recycled transmit buffer address. The network interface
> > +                         will not transmit if its internal recycled transmit
> > +                         buffer array is full. Reading the transmit buffer does
> > +                         not clear the transmit interrupt. If this is NULL, then
> > +                         the transmit buffer status will not be read. If there
> > +                         are no transmit buffers to recycle and TxBuf is not NULL,
> > +                         TxBuf will be set to NULL.
> > +
> > +  @retval EFI_SUCCESS           The status of the network interface was retrieved.
> > +  @retval EFI_NOT_STARTED       The network interface has not been started.
> > +  @retval EFI_INVALID_PARAMETER This parameter was NULL or did not point to a valid
> > +                                EFI_SIMPLE_NETWORK_PROTOCOL structure.
> > +  @retval EFI_DEVICE_ERROR      The command could not be sent to the network
> > +                                interface.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +SnpGetStatus (
> > +  IN    EFI_SIMPLE_NETWORK_PROTOCOL *This,
> > +  OUT   UINT32                      *IrqStat  OPTIONAL,
> > +  OUT   VOID                        **TxBuff  OPTIONAL
> > +  )
> > +{
> > +  EFI_STATUS                 Status;
> > +  EFI_SIMPLE_NETWORK_DRIVER  *Snp;
> > +  UINT32                      MacBase;
> > +
> > +  Snp = INSTANCE_FROM_SNP_THIS (This);
> > +
> > +  // Check preliminaries
> > +  if (This == NULL) {
> > +    return EFI_INVALID_PARAMETER;
> > +  }
> > +  if (Snp->SnpMode.State != EfiSimpleNetworkInitialized) {
> > +    return EFI_NOT_STARTED;
> > +  }
> > +
> > +  // Get MAC base address
> > +  MacBase = (UINT32)Snp->Dev->Resources[0].AddrRangeMin;
> > +
> > +  // Update the media status
> > +  Status = PhyLinkAdjustEmacConfig (&Snp->PhyDriver, MacBase);
> > +  if (EFI_ERROR(Status)) {
> > +    Snp->SnpMode.MediaPresent = FALSE;
> > +  } else {
> > +    Snp->SnpMode.MediaPresent = TRUE;
> > +  }
> > +
> > +  // TxBuff
> > +  if (TxBuff != NULL) {
> > +    //
> > +    // Get a recycled buf from Snp->RecycledTxBuf
> > +    //
> > +    if (Snp->RecycledTxBufCount == 0) {
> > +      *TxBuff = NULL;
> > +    } else {
> > +      Snp->RecycledTxBufCount--;
> > +      *TxBuff = (VOID *) (UINTN) Snp->RecycledTxBuf[Snp->RecycledTxBufCount];
>
> No space after () casts
>
> > +    }
> > +  }
> > +
> > +  // Check DMA Irq status
> > +  EmacGetDmaStatus (IrqStat, MacBase);
> > +
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +
> > +/**
> > +  Places a packet in the transmit queue of a network interface.
> > +
> > +  This function places the packet specified by Header and Buffer on the transmit
> > +  queue. If HeaderSize is nonzero and HeaderSize is not equal to
> > +  This->Mode->MediaHeaderSize, then EFI_INVALID_PARAMETER will be returned. If
> > +  BufferSize is less than This->Mode->MediaHeaderSize, then EFI_BUFFER_TOO_SMALL
> > +  will be returned. If Buffer is NULL, then EFI_INVALID_PARAMETER will be
> > +  returned. If HeaderSize is nonzero and DestAddr or Protocol is NULL, then
> > +  EFI_INVALID_PARAMETER will be returned. If the transmit engine of the network
> > +  interface is busy, then EFI_NOT_READY will be returned. If this packet can be
> > +  accepted by the transmit engine of the network interface, the packet contents
> > +  specified by Buffer will be placed on the transmit queue of the network
> > +  interface, and EFI_SUCCESS will be returned. GetStatus() can be used to
> > +  determine when the packet has actually been transmitted. The contents of the
> > +  Buffer must not be modified until the packet has actually been transmitted.
> > +  The Transmit() function performs nonblocking I/O. A caller who wants to perform
> > +  blocking I/O, should call Transmit(), and then GetStatus() until the
> > +  transmitted buffer shows up in the recycled transmit buffer.
> > +  If the driver has not been initialized, EFI_DEVICE_ERROR will be returned.
> > +
> > +  @param Snp        A pointer to the EFI_SIMPLE_NETWORK_PROTOCOL instance.
> > +  @param HdrSize    The size, in bytes, of the media header to be filled in by the
> > +                    Transmit() function. If HeaderSize is nonzero, then it must
> > +                    be equal to This->Mode->MediaHeaderSize and the DestAddr and
> > +                    Protocol parameters must not be NULL.
> > +  @param BuffSize   The size, in bytes, of the entire packet (media header and
> > +                    data) to be transmitted through the network interface.
> > +  @param Data       A pointer to the packet (media header followed by data) to be
> > +                    transmitted. This parameter cannot be NULL. If HeaderSize is
> > +                    zero, then the media header in Buffer must already be filled
> > +                    in by the caller. If HeaderSize is nonzero, then the media
> > +                    header will be filled in by the Transmit() function.
> > +  @param SrcAddr    The source HW MAC address. If HeaderSize is zero, then this
> > +                    parameter is ignored. If HeaderSize is nonzero and SrcAddr
> > +                    is NULL, then This->Mode->CurrentAddress is used for the
> > +                    source HW MAC address.
> > +  @param DstAddr    The destination HW MAC address. If HeaderSize is zero, then
> > +                    this parameter is ignored.
> > +  @param Protocol   The type of header to build. If HeaderSize is zero, then this
> > +                    parameter is ignored. See RFC 1700, section "Ether Types,"
> > +                    for examples.
> > +
> > +  @retval EFI_SUCCESS           The packet was placed on the transmit queue.
> > +  @retval EFI_NOT_STARTED       The network interface has not been started.
> > +  @retval EFI_NOT_READY         The network interface is too busy to accept this
> > +                                transmit request.
> > +  @retval EFI_BUFFER_TOO_SMALL  The BufferSize parameter is too small.
> > +  @retval EFI_INVALID_PARAMETER One or more of the parameters has an unsupported
> > +                                value.
> > +  @retval EFI_DEVICE_ERROR      The command could not be sent to the network interface.
> > +  @retval EFI_UNSUPPORTED       This function is not supported by the network interface.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +SnpTransmit (
> > +  IN  EFI_SIMPLE_NETWORK_PROTOCOL  *This,
> > +  IN  UINTN                        HdrSize,
> > +  IN  UINTN                        BuffSize,
> > +  IN  VOID*                        Data,
> > +  IN  EFI_MAC_ADDRESS              *SrcAddr  OPTIONAL,
> > +  IN  EFI_MAC_ADDRESS              *DstAddr  OPTIONAL,
> > +  IN  UINT16                       *Protocol OPTIONAL
> > +  )
> > +{
> > +  EFI_SIMPLE_NETWORK_DRIVER *Snp;
> > +  UINT32                     DescNum;
> > +  DESIGNWARE_HW_DESCRIPTOR  *TxDescriptor;
> > +  UINT8*                     EthernetPacket;
> > +  UINT64                    *Tmp;
> > +  UINT32                     MacBase;
> > +
> > +  EthernetPacket = Data;
> > +
> > +  Snp = INSTANCE_FROM_SNP_THIS (This);
> > +
> > +  if (EFI_ERROR (EfiAcquireLockOrFail (&Snp->Lock))) {
> > +    return EFI_ACCESS_DENIED;
>
> This is not a documented return code for this method.
>
> > +  }
> > +
> > +  // Check preliminaries
> > +  if ((This == NULL) || (Data == NULL)) {
> > +    return EFI_INVALID_PARAMETER;
> > +  }
> > +  if (Snp->SnpMode.State != EfiSimpleNetworkInitialized) {
> > +    return EFI_NOT_STARTED;
> > +  }
> > +
> > +  Snp->MacDriver.TxCurrentDescriptorNum = Snp->MacDriver.TxNextDescriptorNum;
> > +  DescNum = Snp->MacDriver.TxCurrentDescriptorNum;
> > +
> > +  TxDescriptor = Snp->MacDriver.TxdescRing[0] + (DescNum);;
>
> Please check your array layout. Indexing from the *value* of element 0 is
> almost certainly wrong.
>
> > +
> > +  // Ensure header is correct size if non-zero
> > +  if (HdrSize) {
> > +    if (HdrSize != Snp->SnpMode.MediaHeaderSize) {
> > +      return EFI_INVALID_PARAMETER;
> > +    }
> > +
> > +    if ((DstAddr == NULL) || (Protocol == NULL)) {
> > +      return EFI_INVALID_PARAMETER;
> > +    }
> > +  }
> > +
> > +  // Ensure buffer size is valid
> > +  if (BuffSize < Snp->SnpMode.MediaHeaderSize) {
> > +    return EFI_BUFFER_TOO_SMALL;
> > +  }
> > +
> > +
> > +  if (HdrSize) {
> > +    EthernetPacket[0] = DstAddr->Addr[0];
> > +    EthernetPacket[1] = DstAddr->Addr[1];
> > +    EthernetPacket[2] = DstAddr->Addr[2];
> > +    EthernetPacket[3] = DstAddr->Addr[3];
> > +    EthernetPacket[4] = DstAddr->Addr[4];
> > +    EthernetPacket[5] = DstAddr->Addr[5];
> > +
> > +    EthernetPacket[6] = SrcAddr->Addr[0];
> > +    EthernetPacket[7] = SrcAddr->Addr[1];
> > +    EthernetPacket[8] = SrcAddr->Addr[2];
> > +    EthernetPacket[9] = SrcAddr->Addr[3];
> > +    EthernetPacket[10] = SrcAddr->Addr[4];
> > +    EthernetPacket[11] = SrcAddr->Addr[5];
> > +
> > +    EthernetPacket[13] = *Protocol & 0xFF;
> > +    EthernetPacket[12] = (*Protocol & 0xFF00) >> 8;
> > +  }
> > +
> > +  CopyMem((VOID*)(UINTN)TxDescriptor->Addr, EthernetPacket, BuffSize);
> > +
>
> I am aware that TX is usually not a hot path in UEFI, but we should still
> try to map a buffer directly rather than copy the contents into uncached
> memory. Also, put a space before (
>
>
> > +  TxDescriptor->Tdes1 = (BuffSize << TDES1_SIZE1SHFT) &
> > +                         TDES1_SIZE1MASK;
> > +
> > +  TxDescriptor->Tdes0 |= (TDES0_TXFIRST |
> > +                          TDES0_TXLAST |
> > +                          TDES0_OWN);
> > +
> > +  // Increase descriptor number
> > +  DescNum++;
> > +  if (DescNum >= CONFIG_TX_DESCR_NUM)
> > +    DescNum = 0;
> > +
>
> Always use { } with conditionals in EDK2 code
>
> > +  Snp->MacDriver.TxNextDescriptorNum = DescNum;
> > +
> > +  if ((Snp->MaxRecycledTxBuf + SNP_TX_BUFFER_INCREASEMENT) >= SNP_MAX_TX_BUFFER_NUM) {
> > +    return EFI_NOT_READY;
> > +  }
> > +
> > +  if (Snp->RecycledTxBufCount < Snp->MaxRecycledTxBuf) {
> > +    Snp->RecycledTxBuf[Snp->RecycledTxBufCount] = (UINT64) Data;
> > +    Snp->RecycledTxBufCount ++;
> > +  } else {
> > +    Tmp = AllocatePool (sizeof (UINT64) * (Snp->MaxRecycledTxBuf + SNP_TX_BUFFER_INCREASEMENT));
> > +    if (Tmp == NULL) {
> > +      return EFI_DEVICE_ERROR;
> > +    }
> > +    CopyMem (Tmp, Snp->RecycledTxBuf, sizeof (UINT64) * Snp->RecycledTxBufCount);
> > +    FreePool (Snp->RecycledTxBuf);
> > +    Snp->RecycledTxBuf    =  Tmp;
> > +    Snp->MaxRecycledTxBuf += SNP_TX_BUFFER_INCREASEMENT;
>
> Here you double the size of the TX recycle buffer array, but you never record
> 'Data' in it. Are you sure that is correct?
>
> > +  }
> > +
> > +  // Get MAC base address
> > +  MacBase = (UINT32)Snp->Dev->Resources[0].AddrRangeMin;
> > +
> > +  // Start the transmission
> > +  EmacDmaStart (MacBase);
> > +
> > +  EfiReleaseLock (&Snp->Lock);
> > +
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +
> > +/**
> > +  Receives a packet from a network interface.
> > +
> > +  This function retrieves one packet from the receive queue of a network interface.
> > +  If there are no packets on the receive queue, then EFI_NOT_READY will be
> > +  returned. If there is a packet on the receive queue, and the size of the packet
> > +  is smaller than BufferSize, then the contents of the packet will be placed in
> > +  Buffer, and BufferSize will be updated with the actual size of the packet.
> > +  In addition, if SrcAddr, DestAddr, and Protocol are not NULL, then these values
> > +  will be extracted from the media header and returned. EFI_SUCCESS will be
> > +  returned if a packet was successfully received.
> > +  If BufferSize is smaller than the received packet, then the size of the receive
> > +  packet will be placed in BufferSize and EFI_BUFFER_TOO_SMALL will be returned.
> > +  If the driver has not been initialized, EFI_DEVICE_ERROR will be returned.
> > +
> > +  @param Snp        A pointer to the EFI_SIMPLE_NETWORK_PROTOCOL instance.
> > +  @param HdrSize    The size, in bytes, of the media header received on the network
> > +                    interface. If this parameter is NULL, then the media header size
> > +                    will not be returned.
> > +  @param BuffSize   On entry, the size, in bytes, of Buffer. On exit, the size, in
> > +                    bytes, of the packet that was received on the network interface.
> > +  @param Data       A pointer to the data buffer to receive both the media
> > +                    header and the data.
> > +  @param SrcAddr    The source HW MAC address. If this parameter is NULL, the HW
> > +                    MAC source address will not be extracted from the media header.
> > +  @param DstAddr   The destination HW MAC address. If this parameter is NULL,
> > +                    the HW MAC destination address will not be extracted from
> > +                    the media header.
> > +  @param Protocol   The media header type. If this parameter is NULL, then the
> > +                    protocol will not be extracted from the media header. See
> > +                    RFC 1700 section "Ether Types" for examples.
> > +
> > +  @retval EFI_SUCCESS           The received data was stored in Buffer, and
> > +                                BufferSize has been updated to the number of
> > +                                bytes received.
> > +  @retval EFI_NOT_STARTED       The network interface has not been started.
> > +  @retval EFI_NOT_READY         No packets have been received on the network interface.
> > +  @retval EFI_BUFFER_TOO_SMALL  BufferSize is too small for the received packets.
> > +                                BufferSize has been updated to the required size.
> > +  @retval EFI_INVALID_PARAMETER One or more of the following conditions is TRUE:
> > +                                * The This parameter is NULL
> > +                                * The This parameter does not point to a valid
> > +                                  EFI_SIMPLE_NETWORK_PROTOCOL structure.
> > +                                * The BufferSize parameter is NULL
> > +                                * The Buffer parameter is NULL
> > +  @retval EFI_DEVICE_ERROR      The command could not be sent to the network interface.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +SnpReceive (
> > +  IN        EFI_SIMPLE_NETWORK_PROTOCOL *This,
> > +      OUT   UINTN                       *HdrSize      OPTIONAL,
> > +  IN  OUT   UINTN                       *BuffSize,
> > +      OUT   VOID                        *Data,
> > +      OUT   EFI_MAC_ADDRESS             *SrcAddr      OPTIONAL,
> > +      OUT   EFI_MAC_ADDRESS             *DstAddr      OPTIONAL,
> > +      OUT   UINT16                      *Protocol     OPTIONAL
> > +  )
> > +{
> > +  EFI_SIMPLE_NETWORK_DRIVER  *Snp;
> > +  EFI_MAC_ADDRESS Dst;
> > +  EFI_MAC_ADDRESS Src;
> > +  UINT32          Length, DesciptorStatus;
>
> DescriptorStatus.
>
> Also, please don't declare multiple variables on a single line.
>
>
> > +  UINT8*          RawData;
> > +  UINT32 DescNum;
> > +  DESIGNWARE_HW_DESCRIPTOR *RxDescriptor;
>
> Fix indentation please.
>
> > +
> > +  Snp = INSTANCE_FROM_SNP_THIS (This);
> > +
> > +  // Check preliminaries
> > +  if ((This == NULL) || (Data == NULL)) {
> > +    return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  if (Snp->SnpMode.State != EfiSimpleNetworkInitialized) {
> > +    return EFI_NOT_STARTED;
> > +  }
> > +
> > +  Snp = INSTANCE_FROM_SNP_THIS (This);
> > +
> > +  if (EFI_ERROR (EfiAcquireLockOrFail (&Snp->Lock))) {
> > +    return EFI_ACCESS_DENIED;
>
> Undocumented return code.
>
> > +  }
> > +
> > +  Snp->MacDriver.RxCurrentDescriptorNum = Snp->MacDriver.RxNextDescriptorNum;
> > +  DescNum = Snp->MacDriver.RxCurrentDescriptorNum;
> > +  RxDescriptor = Snp->MacDriver.RxdescRing[0]+(DescNum);
> > +
> > +  RawData = (UINT8*) Data;
> > +
> > +  DesciptorStatus = RxDescriptor->Tdes0;
> > +
> > +  if (DesciptorStatus & ((UINT32)RDES0_OWN)) {
> > +       goto ReleaseLock;
> > +  }
> > +
> > +
> > +  if (DesciptorStatus & RDES0_SAF) {
> > +    DEBUG ((EFI_D_WARN, "SNP:DXE: Rx Descritpor Status Error: Source Address Filter Fail\n"));
>
> Descritpor -> Descriptor
>
> > +    return EFI_DEVICE_ERROR;
> > +  }
> > +
> > +  if (DesciptorStatus & RDES0_AFM) {
> > +    DEBUG ((EFI_D_WARN, "SNP:DXE: Rx Descritpor Status Error: Destination Address Filter Fail\n"));
> > +    return EFI_DEVICE_ERROR;
> > +  }
> > +
> > +  if (DesciptorStatus & RDES0_ES) {
> > +    // Check for errors
> > +    if (DesciptorStatus & RDES0_RE) {
> > +      DEBUG ((EFI_D_WARN, "SNP:DXE: Rx Descritpor Status Error: Receive Error\n"));
> > +    }
> > +    if (DesciptorStatus & RDES0_DE) {
> > +      DEBUG ((EFI_D_WARN, "SNP:DXE: Rx Descritpor Status Error: Receive Error\n"));
> > +    }
> > +    if (DesciptorStatus & RDES0_RWT) {
> > +      DEBUG ((EFI_D_WARN, "SNP:DXE: Rx Descritpor Status Error: Watchdog Timeout\n"));
> > +    }
> > +    if (DesciptorStatus & RDES0_LC) {
> > +      DEBUG ((EFI_D_WARN, "SNP:DXE: Rx Descritpor Status Error: Late Collision\n"));
> > +    }
> > +    if (DesciptorStatus & RDES0_GF) {
> > +      DEBUG ((EFI_D_WARN, "SNP:DXE: Rx Descritpor Status Error: Giant Frame\n"));
> > +    }
> > +    if (DesciptorStatus & RDES0_OE) {
> > +      DEBUG ((EFI_D_WARN, "SNP:DXE: Rx Descritpor Status Error: Overflow Error\n"));
> > +    }
> > +    if (DesciptorStatus & RDES0_LE) {
> > +      DEBUG ((EFI_D_WARN, "SNP:DXE: Rx Descritpor Status Error:Length Error\n"));
> > +    }
> > +    if (DesciptorStatus & RDES0_DBE) {
> > +      DEBUG ((EFI_D_WARN, "SNP:DXE: Rx Descritpor Status Error: Dribble Bit Error\n"));
> > +    }
> > +
> > +    // Check descriptor error status
> > +    if (DesciptorStatus & RDES0_CE) {
> > +      DEBUG ((EFI_D_WARN, "SNP:DXE: Rx Descritpor Status Error: CRC Error\n"));
> > +    }
> > +    return EFI_DEVICE_ERROR;
> > +  }
> > +
> > +  Length = (DesciptorStatus >> RDES0_FL_SHIFT) & RDES0_FL_MASK;
> > +  if (!Length) {
> > +    DEBUG ((EFI_D_WARN, "SNP:DXE: Error: Invalid Frame Packet length \r\n"));
> > +    return EFI_NOT_READY;
> > +  }
> > +  // Check buffer size
> > +  if (*BuffSize < Length) {
> > +    DEBUG ((EFI_D_WARN, "SNP:DXE: Error: Buffer size is too small\n"));
> > +    return EFI_BUFFER_TOO_SMALL;
> > +  }
> > +  *BuffSize = Length;
> > +
> > +  if (HdrSize != NULL)
> > +    *HdrSize = Snp->SnpMode.MediaHeaderSize;
> > +
> > +  CopyMem (RawData, (VOID*)(UINTN) RxDescriptor->Addr, *BuffSize);
> > +
>
> This copy is much slower than necessary since the RX buffers have been
> allocated via the DmaLib API and mapped as common buffers. Please use
> normal allocations, and use directional mappings.
>
> > +  if (DstAddr != NULL) {
> > +    Dst.Addr[0] = RawData[0];
> > +    Dst.Addr[1] = RawData[1];
> > +    Dst.Addr[2] = RawData[2];
> > +    Dst.Addr[3] = RawData[3];
> > +    Dst.Addr[4] = RawData[4];
> > +    Dst.Addr[5] = RawData[5];
> > +    CopyMem (DstAddr, &Dst, NET_ETHER_ADDR_LEN);
> > +Print(L"received from source address %x %x\n\n", DstAddr, &Dst);
>
> Dont use print() in drivers
>
> > +  }
> > +
> > +  // Get the source address
> > +  if (SrcAddr != NULL) {
> > +    Src.Addr[0] = RawData[6];
> > +    Src.Addr[1] = RawData[7];
> > +    Src.Addr[2] = RawData[8];
> > +    Src.Addr[3] = RawData[9];
> > +    Src.Addr[4] = RawData[10];
> > +    Src.Addr[5] = RawData[11];
> > +Print(L"received from source address %x %x\n\n", SrcAddr, &Src);
>
> same here
>
> > +    CopyMem (SrcAddr,&Src, NET_ETHER_ADDR_LEN);
> > +  }
> > +
> > +  // Get the protocol
> > +  if (Protocol != NULL) {
> > +   *Protocol = NTOHS (RawData[12] | (RawData[13] >> 8) | (RawData[14] >> 16) | (RawData[15] >> 24));
> > +  }
> > +
> > +  RxDescriptor->Tdes0 |= (UINT32)RDES0_OWN;
> > +
> > +  DescNum++;
> > +
> > +  if (DescNum >= CONFIG_RX_DESCR_NUM)
> > +    DescNum = 0;
> > +
> > +  Snp->MacDriver.RxNextDescriptorNum = DescNum;
> > +
> > +  EfiReleaseLock (&Snp->Lock);
> > +
> > +  return EFI_SUCCESS;
> > +
> > +  ReleaseLock:
>
> Don't indent labels
>
> > +  EfiReleaseLock (&Snp->Lock);
> > +  return EFI_NOT_READY;
> > +
> > +}
> > +
> > diff --git a/Silicon/DesignWare/Drivers/DwEmacSnpDxe/EmacDxeUtil.c b/Silicon/DesignWare/Drivers/DwEmacSnpDxe/EmacDxeUtil.c
> > new file mode 100755
> > index 000000000000..a7f29f6cd0c7
> > --- /dev/null
> > +++ b/Silicon/DesignWare/Drivers/DwEmacSnpDxe/EmacDxeUtil.c
> > @@ -0,0 +1,684 @@
> > +/** @file
> > +
> > +  Copyright (c) 2011 - 2019, Intel Corporaton. All rights reserved.
> > +  This program and the accompanying materials
> > +  are licensed and made available under the terms and conditions of the BSD License
> > +  which accompanies this distribution.  The full text of the license may be found at
> > +  http://opensource.org/licenses/bsd-license.php
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > +
> > +  The original software modules are licensed as follows:
> > +
> > +  Copyright (c) 2012 - 2014, ARM Limited. All rights reserved.
> > +  Copyright (c) 2004 - 2010, Intel Corporation. All rights reserved.
> > +
> > +  This program and the accompanying materials
> > +  are licensed and made available under the terms and conditions of the BSD License
> > +  which accompanies this distribution.  The full text of the license may be found at
> > +  http://opensource.org/licenses/bsd-license.php
> > +
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD 3 Clause LICENSE ON AN "AS IS" BASIS,
> > +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > +**/
> > +
> > +#include "EmacDxeUtil.h"
> > +#include "PhyDxeUtil.h"
> > +
> > +VOID
> > +EFIAPI
> > +EmacSetMacAddress (
> > +  IN EFI_MAC_ADDRESS  *MacAddress,
> > +  IN UINT32            MacBaseAddress
> > +  )
> > +{
> > +  DEBUG ((EFI_D_INFO,"SNP:MAC: %a ()\r\n", __FUNCTION__));
> > +
> > +  // Note: This MAC_ADDR0 registers programming sequence cannot be swap:
> > +  // Must program HIGH Offset first before LOW Offset
> > +  // because synchronization is triggered when MAC Address0 Low Register are written.
> > +  MmioWrite32 (MacBaseAddress +DW_EMAC_GMACGRP_MAC_ADDRESS0_HIGH_OFST,
> > +               (UINT32)(MacAddress->Addr[4] & 0xFF) |
> > +                      ((MacAddress->Addr[5] & 0xFF) << 8)
> > +                    );
> > +  // MacAddress->Addr[0,1,2] is the 3 bytes OUI
> > +  MmioWrite32 (MacBaseAddress + DW_EMAC_GMACGRP_MAC_ADDRESS0_LOW_OFST,
> > +                       (MacAddress->Addr[0] & 0xFF) |
> > +                      ((MacAddress->Addr[1] & 0xFF) << 8) |
> > +                      ((MacAddress->Addr[2] & 0xFF) << 16) |
> > +                      ((MacAddress->Addr[3] & 0xFF) << 24)
> > +                    );
> > +
> > +  DEBUG ((EFI_D_INFO, "SNP:MAC: gmacgrp_mac_address0_low  = 0x%08X \r\n", MmioRead32 (MacBaseAddress + DW_EMAC_GMACGRP_MAC_ADDRESS0_LOW_OFST)));
> > +  DEBUG ((EFI_D_INFO, "SNP:MAC: gmacgrp_mac_address0_high = 0x%08X \r\n", MmioRead32 (MacBaseAddress +DW_EMAC_GMACGRP_MAC_ADDRESS0_HIGH_OFST)));
> > +}
> > +
> > +
> > +VOID
> > +EFIAPI
> > +EmacReadMacAddress (
> > +  OUT EFI_MAC_ADDRESS *MacAddress,
> > +  IN UINT32            MacBaseAddress
> > +  )
> > +{
> > +  UINT32          MacAddrHighValue;
> > +  UINT32          MacAddrLowValue;
> > +
> > +  DEBUG ((EFI_D_INFO, "SNP:MAC: %a ()\r\n", __FUNCTION__));
> > +
> > +  // Read the Mac Addr high register
> > +  MacAddrHighValue = (MmioRead32 (MacBaseAddress +DW_EMAC_GMACGRP_MAC_ADDRESS0_HIGH_OFST) & 0xFFFF);
> > +  // Read the Mac Addr low register
> > +  MacAddrLowValue = MmioRead32 (MacBaseAddress + DW_EMAC_GMACGRP_MAC_ADDRESS0_LOW_OFST);
> > +
> > +  SetMem (MacAddress, sizeof(*MacAddress), 0);
> > +  MacAddress->Addr[0] = (MacAddrLowValue & 0xFF);
> > +  MacAddress->Addr[1] = (MacAddrLowValue & 0xFF00) >> 8;
> > +  MacAddress->Addr[2] = (MacAddrLowValue & 0xFF0000) >> 16;
> > +  MacAddress->Addr[3] = (MacAddrLowValue & 0xFF000000) >> 24;
> > +  MacAddress->Addr[4] = (MacAddrHighValue & 0xFF);
> > +  MacAddress->Addr[5] = (MacAddrHighValue & 0xFF00) >> 8;
> > +
> > +  DEBUG ((EFI_D_INFO, "SNP:MAC: MAC Address = %02X:%02X:%02X:%02X:%02X:%02X\r\n",
> > +    MacAddress->Addr[0], MacAddress->Addr[1], MacAddress->Addr[2],
> > +    MacAddress->Addr[3], MacAddress->Addr[4], MacAddress->Addr[5]
> > +  ));
> > +
> > +}
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +EmacDxeInitialization (
> > +  IN EMAC_DRIVER* EmacDriver,
> > +  IN UINT32 MacBaseAddress
> > +  )
> > +{
> > +  DEBUG ((EFI_D_INFO, "SNP:MAC: %a ()\r\n", __FUNCTION__));
> > +
> > +  // Init EMAC DMA
> > +  EmacDmaInit (EmacDriver, MacBaseAddress);
> > +
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +EmacDmaInit (
> > +  IN   EMAC_DRIVER      *EmacDriver,
> > +  IN   UINT32           MacBaseAddress
> > +  )
> > +{
> > +  UINT32 DmaConf;
> > +  UINT32 DmaOpmode;
> > +  UINT32 InterruptEnable;
> > +
> > +  DEBUG ((EFI_D_INFO, "SNP:MAC: %a ()\r\n", __FUNCTION__));
> > +
> > +  // This section provides the instructions for initializing the DMA registers in the proper sequence. This
> > +  // initialization sequence can be done after the EMAC interface initialization has been completed. Perform
> > +  // the following steps to initialize the DMA:
> > +  // 1. Provide a software reset to reset all of the EMAC internal registers and logic. (DMA Register 0 (Bus
> > +  // Mode Register) – bit 0).
> > +
> > +  MmioOr32 (MacBaseAddress +
> > +             DW_EMAC_DMAGRP_BUS_MODE_OFST,
> > +             DW_EMAC_DMAGRP_BUS_MODE_SWR_SET_MSK);
> > +
> > +  // 2. Wait for the completion of the reset process (poll bit 0 of the DMA Register 0 (Bus Mode Register),
> > +  // which is only cleared after the reset operation is completed).
> > +  while (DW_EMAC_DMAGRP_BUS_MODE_SWR_GET (MmioRead32 (MacBaseAddress + DW_EMAC_DMAGRP_BUS_MODE_OFST)));
> > +
> > +  // 3. Poll the bits of Register 11 (AHB or AXI Status) to confirm that all previously initiated (before
> > +  // software reset) or ongoing transactions are complete.
> > +  // Note: If the application cannot poll the register after soft reset (because of performance reasons), then
> > +  // it is recommended that you continue with the next steps and check this register again (as
> > +  // mentioned in 12 on page 1-72) before triggering the DMA operations.†
> > +
> > +  // 4. Program the following fields to initialize the Bus Mode Register by setting values in DMA Register 0
> > +  // (Bus Mode Register):
> > +  // • Mixed Burst and AAL
> > +  // • Fixed burst or undefined burst
> > +  // • Burst length values and burst mode values
> > +  // • Descriptor Length (only valid if Ring Mode is used)
> > +  // • TX and RX DMA Arbitration scheme
> > +  // 5. Program the interface options in Register 10 (AXI Bus Mode Register). If fixed burst-length is enabled,
> > +  // then select the maximum burst-length possible on the bus (bits[7:1]).†
> > +
> > +  DmaConf = DW_EMAC_DMAGRP_BUS_MODE_FB_SET_MSK | DW_EMAC_DMAGRP_BUS_MODE_PBL_SET_MSK | DW_EMAC_DMAGRP_BUS_MODE_PR_SET_MSK;
> > +  MmioOr32 (MacBaseAddress +
> > +            DW_EMAC_DMAGRP_BUS_MODE_OFST,
> > +            DmaConf);
> > +
> > +  // 6. Create a proper descriptor chain for transmit and receive. In addition, ensure that the receive descriptors
> > +  // are owned by DMA (bit 31 of descriptor should be set). When OSF mode is used, at least two
> > +  // descriptors are required.
> > +  // 7. Make sure that your software creates three or more different transmit or receive descriptors in the
> > +  // chain before reusing any of the descriptors.
> > +  // 8. Initialize receive and transmit descriptor list address with the base address of the transmit and receive
> > +  // descriptor (Register 3 (Receive Descriptor List Address Register) and Register 4 (Transmit Descriptor
> > +  // List Address Register) respectively).
> > +
> > +  EmacSetupTxdesc (EmacDriver, MacBaseAddress);
> > +  EmacSetupRxdesc (EmacDriver, MacBaseAddress);
> > +
> > +  // 9. Program the following fields to initialize the mode of operation in Register 6 (Operation Mode
> > +  // Register):
> > +  // • Receive and Transmit Store And Forward†
> > +  // • Receive and Transmit Threshold Control (RTC and TTC)†
> > +  // • Hardware Flow Control enable†
> > +  // • Flow Control Activation and De-activation thresholds for MTL Receive and Transmit FIFO buffers
> > +  // (RFA and RFD)†
> > +  // • Error frame and undersized good frame forwarding enable†
> > +  // • OSF Mode†
> > +
> > +  DmaOpmode = DW_EMAC_DMAGRP_OPERATION_MODE_FTF_SET_MSK | DW_EMAC_DMAGRP_OPERATION_MODE_TSF_SET_MSK;
> > +  MmioOr32 (MacBaseAddress +
> > +            DW_EMAC_DMAGRP_OPERATION_MODE_OFST,
> > +            DmaOpmode);
> > +  //while (DW_EMAC_DMAGRP_OPERATION_MODE_FTF_GET (MmioRead32 (MacBaseAddress + DW_EMAC_DMAGRP_OPERATION_MODE_OFST)));
> > +
> > +  // 10.Clear the interrupt requests, by writing to those bits of the status register (interrupt bits only) that are
> > +  // set. For example, by writing 1 into bit 16, the normal interrupt summary clears this bit (DMA Register 5 (Status Register)).
> > +  MmioOr32 (MacBaseAddress +
> > +            DW_EMAC_DMAGRP_STATUS_OFST,
> > +            0x1FFFF);
> > +
> > +  // 11.Enable the interrupts by programming Register 7 (Interrupt Enable Register).
> > +  InterruptEnable = DW_EMAC_DMAGRP_INTERRUPT_ENABLE_TIE_SET_MSK |
> > +                    DW_EMAC_DMAGRP_INTERRUPT_ENABLE_RIE_SET_MSK |
> > +                    DW_EMAC_DMAGRP_INTERRUPT_ENABLE_NIE_SET_MSK |
> > +                    DW_EMAC_DMAGRP_INTERRUPT_ENABLE_AIE_SET_MSK |
> > +                    DW_EMAC_DMAGRP_INTERRUPT_ENABLE_FBE_SET_MSK |
> > +                    DW_EMAC_DMAGRP_INTERRUPT_ENABLE_UNE_SET_MSK |
> > +                    DW_EMAC_DMAGRP_INTERRUPT_ENABLE_TSE_SET_MSK |
> > +                    DW_EMAC_DMAGRP_INTERRUPT_ENABLE_TUE_SET_MSK |
> > +                    DW_EMAC_DMAGRP_INTERRUPT_ENABLE_TJE_SET_MSK |
> > +                    DW_EMAC_DMAGRP_INTERRUPT_ENABLE_OVE_SET_MSK |
> > +                    DW_EMAC_DMAGRP_INTERRUPT_ENABLE_RUE_SET_MSK |
> > +                    DW_EMAC_DMAGRP_INTERRUPT_ENABLE_RSE_SET_MSK |
> > +                    DW_EMAC_DMAGRP_INTERRUPT_ENABLE_RWE_SET_MSK |
> > +                    DW_EMAC_DMAGRP_INTERRUPT_ENABLE_ETE_SET_MSK |
> > +                    DW_EMAC_DMAGRP_INTERRUPT_ENABLE_ERE_SET_MSK;
> > +   MmioWrite32 (MacBaseAddress +
> > +                DW_EMAC_DMAGRP_INTERRUPT_ENABLE_OFST,
> > +                InterruptEnable);
> > +
> > +  // 12.Read Register 11 (AHB or AXI Status) to confirm that all previous transactions are complete.†
> > +  // Note: If any previous transaction is still in progress when you read the Register 11 (AHB or AXI
> > +  // Status), then it is strongly recommended to check the slave components addressed by the
> > +  // master interface.
> > +  if (MmioRead32 (MacBaseAddress + DW_EMAC_DMAGRP_AHB_OR_AXI_STATUS_OFST) != 0) {
> > +    DEBUG ((EFI_D_INFO, "SNP:MAC: Error! Previous AXI transaction is still in progress\r\n"));
> > +    //check the slave components addressed by the master interface
> > +    return EFI_DEVICE_ERROR;
> > +  }
> > +
> > +  DmaOpmode = DW_EMAC_DMAGRP_OPERATION_MODE_ST_SET_MSK | DW_EMAC_DMAGRP_OPERATION_MODE_SR_SET_MSK;
> > +  MmioOr32 (MacBaseAddress +
> > +            DW_EMAC_DMAGRP_OPERATION_MODE_OFST,
> > +            DmaOpmode);
> > +
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +EmacSetupTxdesc (
> > +  IN EMAC_DRIVER *EmacDriver,
> > +  IN UINT32 MacBaseAddress
> > + )
> > +{
> > +  INTN Index;
> > +  DESIGNWARE_HW_DESCRIPTOR *TxDescriptor;
> > +
> > +  for (Index = 0; Index < CONFIG_TX_DESCR_NUM; Index++) {
> > +       TxDescriptor = (VOID*)EmacDriver->TxdescRingMap[0]+(Index*16);;
> > +       TxDescriptor->Addr = (UINT32)(UINTN) EmacDriver->TxBufferMap[0] + (Index*2048);
> > +       TxDescriptor->AddrNext = (UINT32)(UINTN) EmacDriver->TxdescRingMap[0] + ((Index + 1)*16);
> > +       TxDescriptor->Tdes0 = TDES0_TXCHAIN;
> > +    TxDescriptor->Tdes1 = 0;
> > +  }
> > +
> > +  // Correcting the last pointer of the chain
> > +  TxDescriptor->AddrNext = (UINT32)(UINTN) EmacDriver->TxdescRingMap[0];
> > +
> > +  // Write the address of tx descriptor list
> > +  MmioWrite32(MacBaseAddress +
> > +              DW_EMAC_DMAGRP_TRANSMIT_DESCRIPTOR_LIST_ADDRESS_OFST,
> > +              (UINT32)(UINTN) EmacDriver->TxdescRingMap[0]);
> > +
> > +  // Initialize the descriptor number
> > +  EmacDriver->TxCurrentDescriptorNum = 0;
> > +  EmacDriver->TxNextDescriptorNum = 0;
> > +
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +EmacSetupRxdesc (
> > +  IN EMAC_DRIVER *EmacDriver,
> > +  IN UINT32 MacBaseAddress
> > +  )
> > +{
> > +  INTN                      Index;
> > +  DESIGNWARE_HW_DESCRIPTOR *RxDescriptor;
> > +
> > +  for (Index = 0; Index < CONFIG_RX_DESCR_NUM; Index++) {
> > +       RxDescriptor = (VOID*) (EmacDriver->RxdescRingMap[0] + (Index*16));
> > +    RxDescriptor->Addr = (UINT32)(UINTN) EmacDriver->RxBufferMap[0] + (Index*2048);
> > +    RxDescriptor->AddrNext = (UINT32)(UINTN) EmacDriver->RxdescRingMap[0] + ((Index + 1)*16);
> > +    RxDescriptor->Tdes0 = RDES0_OWN;
> > +    RxDescriptor->Tdes1 = RDES1_CHAINED | RX_MAX_PACKET;
> > +  }
> > +
> > +  // Correcting the last pointer of the chain
> > +  RxDescriptor->AddrNext = (UINT32)(UINTN) EmacDriver->RxdescRingMap[0];
> > +
> > +  // Write the address of tx descriptor list
> > +  MmioWrite32(MacBaseAddress +
> > +              DW_EMAC_DMAGRP_RECEIVE_DESCRIPTOR_LIST_ADDRESS_OFST,
> > +              (UINT32)(UINTN) EmacDriver->RxdescRingMap[0]);
> > +
> > +  // Initialize the descriptor number
> > +  EmacDriver->RxCurrentDescriptorNum = 0;
> > +  EmacDriver->RxNextDescriptorNum = 0;
> > +
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +
> > +VOID
> > +EFIAPI
> > +EmacStartTransmission (
> > +  IN UINT32 MacBaseAddress
> > +  )
> > +{
> > +  DEBUG ((EFI_D_INFO, "SNP:MAC: %a ()\r\n", __FUNCTION__));
> > +  MmioOr32 (MacBaseAddress +
> > +            DW_EMAC_GMACGRP_MAC_CONFIGURATION_OFST,
> > +            DW_EMAC_GMACGRP_MAC_CONFIGURATION_RE_SET_MSK |
> > +            DW_EMAC_GMACGRP_MAC_CONFIGURATION_TE_SET_MSK
> > +            );
> > +}
> > +
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +EmacRxFilters (
> > +  IN        UINT32 ReceiveFilterSetting,
> > +  IN        BOOLEAN Reset,
> > +  IN        UINTN NumMfilter          OPTIONAL,
> > +  IN        EFI_MAC_ADDRESS *Mfilter  OPTIONAL,
> > +  IN        UINT32 MacBaseAddress
> > +  )
> > + {
> > +  UINT32 MacFilter;
> > +  UINT32 Crc;
> > +  UINT32 Count;
> > +  UINT32 HashReg;
> > +  UINT32 HashBit;
> > +  UINT32 Reg;
> > +  UINT32 Val;
> > +
> > +  // If reset then clear the filter registers
> > +  if (Reset) {
> > +    for (Count = 0; Count < NumMfilter; Count++)
> > +    {
> > +      MmioWrite32 (MacBaseAddress + HASH_TABLE_REG(Count), 0x00000000);
> > +    }
> > +  }
> > +
> > +  // Set MacFilter to the reset value of the  DW_EMAC_GMACGRP_MAC_FRAME_FILTER register.
> > +  MacFilter =  DW_EMAC_GMACGRP_MAC_FRAME_FILTER_RESET;
> > +
> > +  if (ReceiveFilterSetting & EFI_SIMPLE_NETWORK_RECEIVE_UNICAST) {
> > +    //DEBUG ((EFI_D_INFO, "SNP:MAC: Enable Unicast Frame Reception\n"));
> > +  }
> > +
> > +  if (ReceiveFilterSetting & EFI_SIMPLE_NETWORK_RECEIVE_MULTICAST) {
> > +
> > +    //DEBUG ((EFI_D_INFO, "SNP:MAC: Enable Hash Multicast Frame Reception\n"));
> > +    MacFilter |=  DW_EMAC_GMACGRP_MAC_FRAME_FILTER_HMC_SET_MSK;
> > +
> > +    // Set the hash tables
> > +    if ((NumMfilter > 0) && (!Reset)) {
> > +      // Go through each filter address and set appropriate bits on hash table
> > +      for (Count = 0; Count < NumMfilter; Count++) {
> > +        // Generate a 32-bit CRC
> > +        Crc = GenEtherCrc32 (&Mfilter[Count], 6);
> > +        // reserve CRC + take upper 8 bit = take lower 8 bit and reverse it
> > +        Val = BitReverse(Crc & 0xff);
> > +        // The most significant bits determines the register to be used (Hash Table Register X),
> > +        // and the least significant five bits determine the bit within the register.
> > +        // For example, a hash value of 8b'10111111 selects Bit 31 of the Hash Table Register 5.
> > +        HashReg = (Val >> 5);
> > +        HashBit = (Val & 31);
> > +
> > +        Reg = MmioRead32(MacBaseAddress + HASH_TABLE_REG(HashReg));
>
> Space before '('  (applies everywhere)
>
> > +        // set 1 to HashBit of HashReg
> > +        // for example, set 1 to bit 31 to Reg 5 as in above example
> > +        Reg |= (1 << HashBit);
> > +        MmioWrite32(MacBaseAddress + HASH_TABLE_REG(HashReg), Reg);
> > +      }
> > +    }
> > +  }
> > +
> > +  if ((ReceiveFilterSetting & EFI_SIMPLE_NETWORK_RECEIVE_BROADCAST) == 0) {
> > +    MacFilter |=  DW_EMAC_GMACGRP_MAC_FRAME_FILTER_DBF_SET_MSK;
> > +  } else if (ReceiveFilterSetting & EFI_SIMPLE_NETWORK_RECEIVE_BROADCAST) {
> > +  }
> > +
> > +  if (ReceiveFilterSetting & EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS) {
> > +    MacFilter |=  DW_EMAC_GMACGRP_MAC_FRAME_FILTER_PR_SET_MSK;
> > +  }
> > +
> > +  if (ReceiveFilterSetting & EFI_SIMPLE_NETWORK_RECEIVE_PROMISCUOUS_MULTICAST) {
> > +    MacFilter |= ( DW_EMAC_GMACGRP_MAC_FRAME_FILTER_PM_SET_MSK);
> > +  }
> > +
> > +  // Set MacFilter to EMAC register
> > +  MmioWrite32 (MacBaseAddress +  DW_EMAC_GMACGRP_MAC_FRAME_FILTER_OFST, MacFilter);
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +
> > +UINT32
> > +EFIAPI
> > +GenEtherCrc32 (
> > +  IN    EFI_MAC_ADDRESS *Mac,
> > +  IN    UINT32 AddrLen
> > +  )
> > +{
> > +  INT32  Iter;
> > +  UINT32 Remainder;
> > +  UINT8  *Ptr;
> > +
> > +  Iter = 0;
> > +  Remainder = 0xFFFFFFFF;    // 0xFFFFFFFF is standard seed for Ethernet
> > +
> > +  // Convert Mac Address to array of bytes
> > +  Ptr = (UINT8*)Mac;
> > +
> > +  // Generate the Crc bit-by-bit (LSB first)
> > +  while (AddrLen--) {
> > +    Remainder ^= *Ptr++;
> > +    for (Iter = 0;Iter < 8;Iter++) {
> > +      // Check if exponent is set
> > +      if (Remainder & 1) {
> > +        Remainder = (Remainder >> 1) ^ CRC_POLYNOMIAL;
> > +      } else {
> > +        Remainder = (Remainder >> 1) ^ 0;
> > +      }
> > +    }
> > +  }
> > +
> > +  return (~Remainder);
> > +}
> > +
> > +
> > +STATIC CONST UINT8 NibbleTab[] = {
> > +    /* 0x0 0000 -> 0000 */  0x0,
> > +    /* 0x1 0001 -> 1000 */  0x8,
> > +    /* 0x2 0010 -> 0100 */  0x4,
> > +    /* 0x3 0011 -> 1100 */  0xc,
> > +    /* 0x4 0100 -> 0010 */  0x2,
> > +    /* 0x5 0101 -> 1010 */  0xa,
> > +    /* 0x6 0110 -> 0110 */  0x6,
> > +    /* 0x7 0111 -> 1110 */  0xe,
> > +    /* 0x8 1000 -> 0001 */  0x1,
> > +    /* 0x9 1001 -> 1001 */  0x9,
> > +    /* 0xa 1010 -> 0101 */  0x5,
> > +    /* 0xb 1011 -> 1101 */  0xd,
> > +    /* 0xc 1100 -> 0011 */  0x3,
> > +    /* 0xd 1101 -> 1011 */  0xb,
> > +    /* 0xe 1110 -> 0111 */  0x7,
> > +    /* 0xf 1111 -> 1111 */  0xf
> > +};
> > +
> > +UINT8
> > +EFIAPI
> > +BitReverse (
> > +  UINT8 X
> > +  )
> > +{
> > +  return (NibbleTab[X & 0xf] << 4) | NibbleTab[X >> 4];
> > +}
> > +
> > +
> > +VOID
> > +EFIAPI
> > +EmacStopTxRx (
> > +   IN    UINT32 MacBaseAddress
> > +  )
> > +{
> > +  DEBUG ((EFI_D_INFO, "SNP:MAC: %a ()\r\n", __FUNCTION__));
> > +
> > +  // Stop DMA TX
> > +  MmioAnd32 (MacBaseAddress +
> > +            DW_EMAC_DMAGRP_OPERATION_MODE_OFST,
> > +            DW_EMAC_DMAGRP_OPERATION_MODE_ST_CLR_MSK);
> > +
> > +  // Flush TX
> > +  MmioOr32 (MacBaseAddress +
> > +            DW_EMAC_DMAGRP_OPERATION_MODE_OFST,
> > +            DW_EMAC_DMAGRP_OPERATION_MODE_FTF_SET_MSK);
> > +
> > +  // Stop transmitters
> > +  MmioAnd32 (MacBaseAddress +
> > +            DW_EMAC_GMACGRP_MAC_CONFIGURATION_OFST,
> > +            DW_EMAC_GMACGRP_MAC_CONFIGURATION_RE_CLR_MSK &
> > +            DW_EMAC_GMACGRP_MAC_CONFIGURATION_TE_CLR_MSK);
> > +
> > +  // Stop DMA RX
> > +  MmioAnd32 (MacBaseAddress +
> > +            DW_EMAC_DMAGRP_OPERATION_MODE_OFST,
> > +            DW_EMAC_DMAGRP_OPERATION_MODE_SR_CLR_MSK);
> > +
> > +}
> > +
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +EmacDmaStart (
> > +  IN UINT32 MacBaseAddress
> > +  )
> > +{
> > +  // Start the transmission
> > +  MmioWrite32(MacBaseAddress +
> > +              DW_EMAC_DMAGRP_TRANSMIT_POLL_DEMAND_OFST,
> > +              0x1);
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +
> > +VOID
> > +EFIAPI
> > +EmacGetDmaStatus (
> > +  OUT   UINT32 *IrqStat  OPTIONAL,
> > +  IN    UINT32 MacBaseAddress
> > +  )
> > +{
> > +  UINT32  DmaStatus;
> > +  UINT32  ErrorBit;
> > +  UINT32  Mask = 0;
> > +
> > +  DmaStatus = MmioRead32 (MacBaseAddress +
> > +                           DW_EMAC_DMAGRP_STATUS_OFST);
> > +  if (DmaStatus & DW_EMAC_DMAGRP_STATUS_NIS_SET_MSK) {
> > +    Mask |= DW_EMAC_DMAGRP_STATUS_NIS_SET_MSK;
> > +    // Rx interrupt
> > +    if (DmaStatus & DW_EMAC_DMAGRP_STATUS_RI_SET_MSK) {
> > +      *IrqStat |= EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT;
> > +      Mask |= DW_EMAC_DMAGRP_STATUS_RI_SET_MSK;
> > +    } else {
> > +      *IrqStat &= ~EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT;
> > +    }
> > +    // Tx interrupt
> > +    if (DmaStatus & DW_EMAC_DMAGRP_STATUS_TI_SET_MSK) {
> > +      *IrqStat |= EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT;
> > +      Mask |= DW_EMAC_DMAGRP_STATUS_TI_SET_MSK;
> > +    } else {
> > +      *IrqStat &= ~EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT;
> > +    }
> > +    // Tx Buffer
> > +    if (DmaStatus & DW_EMAC_DMAGRP_STATUS_TU_SET_MSK){
> > +      Mask |= DW_EMAC_DMAGRP_STATUS_TU_SET_MSK;
> > +      }
> > +    // Early receive interrupt
> > +    if (DmaStatus & DW_EMAC_DMAGRP_STATUS_ERI_SET_MSK) {
> > +      Mask |= DW_EMAC_DMAGRP_STATUS_ERI_SET_MSK;
> > +    }
> > +  }
> > +  if (DmaStatus & DW_EMAC_DMAGRP_STATUS_AIS_SET_MSK) {
> > +    Mask |= DW_EMAC_DMAGRP_STATUS_AIS_SET_MSK;
> > +    // Transmit process stop
> > +    if (DmaStatus & DW_EMAC_DMAGRP_STATUS_TPS_SET_MSK) {
> > +      DEBUG ((EFI_D_INFO, "SNP:MAC: Transmit process stop\n"));
> > +      Mask |= DW_EMAC_DMAGRP_STATUS_TPS_SET_MSK;
> > +    }
> > +    // Transmit jabber timeout
> > +    if (DmaStatus & DW_EMAC_DMAGRP_STATUS_TJT_SET_MSK) {
> > +      DEBUG ((EFI_D_INFO, "SNP:MAC: Transmit jabber timeout\n"));
> > +      Mask |= DW_EMAC_DMAGRP_STATUS_TJT_SET_MSK;
> > +    }
> > +    // Receive FIFO overflow
> > +    if (DmaStatus & DW_EMAC_DMAGRP_STATUS_OVF_SET_MSK) {
> > +      DEBUG ((EFI_D_INFO, "SNP:MAC: Receive FIFO overflow\n"));
> > +      Mask |= DW_EMAC_DMAGRP_STATUS_OVF_SET_MSK;
> > +    }
> > +    // Transmit FIFO underflow
> > +    if (DmaStatus & DW_EMAC_DMAGRP_STATUS_UNF_SET_MSK) {
> > +      DEBUG ((EFI_D_INFO, "SNP:MAC: Receive FIFO underflow\n"));
> > +      Mask |= DW_EMAC_DMAGRP_STATUS_UNF_SET_MSK;
> > +    }
> > +    // Receive buffer unavailable
> > +    if (DmaStatus & DW_EMAC_DMAGRP_STATUS_RU_SET_MSK) {
> > +      //DEBUG ((EFI_D_INFO, "SNP:MAC: Receive buffer unavailable\n"));
> > +      Mask |= DW_EMAC_DMAGRP_STATUS_RU_SET_MSK;
> > +    }
> > +
> > +    // Receive process stop
> > +    if (DmaStatus & DW_EMAC_DMAGRP_STATUS_RPS_SET_MSK) {
> > +      DEBUG ((EFI_D_INFO, "SNP:MAC: Receive process stop\n"));
> > +      Mask |= DW_EMAC_DMAGRP_STATUS_RPS_SET_MSK;
> > +    }
> > +    // Receive watchdog timeout
> > +    if (DmaStatus & DW_EMAC_DMAGRP_STATUS_RWT_SET_MSK) {
> > +      DEBUG ((EFI_D_INFO, "SNP:MAC: Receive watchdog timeout\n"));
> > +      Mask |= DW_EMAC_DMAGRP_STATUS_RWT_SET_MSK;
> > +    }
> > +    // Early transmit interrupt
> > +    if (DmaStatus & DW_EMAC_DMAGRP_STATUS_ETI_SET_MSK) {
> > +      //DEBUG ((EFI_D_INFO, "SNP:MAC: Early transmit interrupt\n"));
> > +      Mask |= DW_EMAC_DMAGRP_STATUS_ETI_SET_MSK;
> > +    }
> > +    // Fatal bus error
> > +    if (DmaStatus & DW_EMAC_DMAGRP_STATUS_FBI_SET_MSK) {
> > +      DEBUG ((EFI_D_INFO, "SNP:MAC: Fatal bus error:\n"));
> > +      Mask |= DW_EMAC_DMAGRP_STATUS_FBI_SET_MSK;
> > +
> > +      ErrorBit = DW_EMAC_DMAGRP_STATUS_EB_GET (DmaStatus);
> > +      switch (ErrorBit) {
> > +        case RX_DMA_WRITE_DATA_TRANSFER_ERROR:
> > +          DEBUG ((EFI_D_INFO, "SNP:MAC: Rx Dma write data transfer error\n"));
> > +          break;
> > +        case TX_DMA_READ_DATA_TRANSFER_ERROR:
> > +          DEBUG ((EFI_D_INFO, "SNP:MAC: Tx Dma read data transfer error\n"));
> > +          break;
> > +        case RX_DMA_DESCRIPTOR_WRITE_ACCESS_ERROR:
> > +          DEBUG ((EFI_D_INFO, "SNP:MAC: Rx Dma descriptor write access error\n"));
> > +          break;
> > +        case RX_DMA_DESCRIPTOR_READ_ACCESS_ERROR:
> > +          DEBUG ((EFI_D_INFO, "SNP:MAC: Rx Dma descriptor read access error\n"));
> > +          break;
> > +        case TX_DMA_DESCRIPTOR_WRITE_ACCESS_ERROR:
> > +          DEBUG ((EFI_D_INFO, "SNP:MAC: Tx Dma descriptor write access error\n"));
> > +          break;
> > +        case TX_DMA_DESCRIPTOR_READ_ACCESS_ERROR:
> > +          DEBUG ((EFI_D_INFO, "SNP:MAC: Tx Dma descriptor read access error\n"));
> > +          break;
> > +        default:
> > +          DEBUG ((EFI_D_INFO, "SNP:MAC: Undefined error\n"));
> > +          break;
> > +      }
> > +    }
> > +  }
> > +  MmioOr32 (MacBaseAddress +
> > +            DW_EMAC_DMAGRP_STATUS_OFST,
> > +            Mask);
> > +}
> > +
> > +
> > +VOID
> > +EFIAPI
> > +EmacGetStatistic (
> > +  OUT EFI_NETWORK_STATISTICS* Statistic,
> > +  IN UINT32 MacBaseAddress
> > +  )
> > +{
> > +  EFI_NETWORK_STATISTICS* Stats;
> > +
> > +  DEBUG ((EFI_D_INFO, "SNP:MAC: %a ()\r\n", __FUNCTION__));
> > +
> > +  // Allocate Resources
> > +  Stats = AllocateZeroPool (sizeof (EFI_NETWORK_STATISTICS));
> > +  if (Stats == NULL) {
> > +    return;
> > +  }
> > +
> > +  Stats->RxTotalFrames     = MmioRead32 (MacBaseAddress + DW_EMAC_GMACGRP_RXFRAMECOUNT_GB_OFST);
> > +  Stats->RxUndersizeFrames = MmioRead32 (MacBaseAddress + DW_EMAC_GMACGRP_RXUNDERSIZE_G_OFST);
> > +  Stats->RxOversizeFrames  = MmioRead32 (MacBaseAddress + DW_EMAC_GMACGRP_RXOVERSIZE_G_OFST);
> > +  Stats->RxUnicastFrames   = MmioRead32 (MacBaseAddress + DW_EMAC_GMACGRP_RXUNICASTFRAMES_G_OFST);
> > +  Stats->RxBroadcastFrames = MmioRead32 (MacBaseAddress + DW_EMAC_GMACGRP_RXBROADCASTFRAMES_G_OFST);
> > +  Stats->RxMulticastFrames = MmioRead32 (MacBaseAddress + DW_EMAC_GMACGRP_RXMULTICASTFRAMES_G_OFST);
> > +  Stats->RxCrcErrorFrames  = MmioRead32 (MacBaseAddress + DW_EMAC_GMACGRP_RXCRCERROR_OFST);
> > +  Stats->RxTotalBytes      = MmioRead32 (MacBaseAddress + DW_EMAC_GMACGRP_RXOCTETCOUNT_GB_OFST);
> > +  Stats->RxGoodFrames      = Stats->RxUnicastFrames + Stats->RxBroadcastFrames + Stats->RxMulticastFrames;
> > +
> > +  Stats->TxTotalFrames     = MmioRead32 (MacBaseAddress + DW_EMAC_GMACGRP_TXFRAMECOUNT_GB_OFST);
> > +  Stats->TxGoodFrames      = MmioRead32 (MacBaseAddress + DW_EMAC_GMACGRP_TXFRAMECOUNT_G_OFST);
> > +  Stats->TxOversizeFrames  = MmioRead32 (MacBaseAddress + DW_EMAC_GMACGRP_TXOVERSIZE_G_OFST);
> > +  Stats->TxUnicastFrames   = MmioRead32 (MacBaseAddress + DW_EMAC_GMACGRP_TXUNICASTFRAMES_GB_OFST);
> > +  Stats->TxBroadcastFrames = MmioRead32 (MacBaseAddress + DW_EMAC_GMACGRP_TXBROADCASTFRAMES_G_OFST);
> > +  Stats->TxMulticastFrames = MmioRead32 (MacBaseAddress + DW_EMAC_GMACGRP_TXMULTICASTFRAMES_G_OFST);
> > +  Stats->TxTotalBytes      = MmioRead32 (MacBaseAddress + DW_EMAC_GMACGRP_TXOCTETCOUNT_GB_OFST);
> > +  Stats->Collisions        = MmioRead32 (MacBaseAddress + DW_EMAC_GMACGRP_TXLATECOL_OFST) +
> > +                             MmioRead32 (MacBaseAddress + DW_EMAC_GMACGRP_TXEXESSCOL_OFST);
> > +
> > +  // Fill in the statistics
> > +  CopyMem(Statistic, Stats, sizeof(EFI_NETWORK_STATISTICS));
> > +}
> > +
> > +
> > +VOID
> > +EFIAPI
> > +EmacConfigAdjust (
> > +  IN UINT32 Speed,
> > +  IN UINT32 Duplex,
> > +  IN UINT32 MacBaseAddress
> > +  )
> > +{
> > +  UINT32 Config;
> > +
> > +  Config =0;
> > +  if (Speed != SPEED_1000)
> > +   Config |= DW_EMAC_GMACGRP_MAC_CONFIGURATION_PS_SET_MSK;
> > +
> > +  if (Speed == SPEED_100)
> > +    Config |= DW_EMAC_GMACGRP_MAC_CONFIGURATION_FES_SET_MSK;
> > +
> > +  if (Duplex == DUPLEX_FULL)
> > +    Config |= DW_EMAC_GMACGRP_MAC_CONFIGURATION_DM_SET_MSK;
> > +
> > +  MmioOr32 (MacBaseAddress +
> > +            DW_EMAC_GMACGRP_MAC_CONFIGURATION_OFST,
> > +            DW_EMAC_GMACGRP_MAC_CONFIGURATION_BE_SET_MSK |
> > +            DW_EMAC_GMACGRP_MAC_CONFIGURATION_DO_SET_MSK |
> > +            Config);
> > +
> > +}
> > diff --git a/Silicon/DesignWare/Drivers/DwEmacSnpDxe/PhyDxeUtil.c b/Silicon/DesignWare/Drivers/DwEmacSnpDxe/PhyDxeUtil.c
> > new file mode 100755
> > index 000000000000..65a53b651814
> > --- /dev/null
> > +++ b/Silicon/DesignWare/Drivers/DwEmacSnpDxe/PhyDxeUtil.c
> > @@ -0,0 +1,621 @@
> > +/** @file
> > +
> > +  Copyright (c) 2011 - 2019, Intel Corporaton. All rights reserved.
> > +  This program and the accompanying materials
> > +  are licensed and made available under the terms and conditions of the BSD License
> > +  which accompanies this distribution.  The full text of the license may be found at
> > +  http://opensource.org/licenses/bsd-license.php
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > +
> > +  The original software modules are licensed as follows:
> > +
> > +  Copyright (c) 2012 - 2014, ARM Limited. All rights reserved.
> > +  Copyright (c) 2004 - 2010, Intel Corporation. All rights reserved.
> > +
> > +  This program and the accompanying materials
> > +  are licensed and made available under the terms and conditions of the BSD License
> > +  which accompanies this distribution.  The full text of the license may be found at
> > +  http://opensource.org/licenses/bsd-license.php
> > +
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD 3 Clause LICENSE ON AN "AS IS" BASIS,
> > +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > +**/
> > +
> > +
> > +#include "PhyDxeUtil.h"
> > +#include "EmacDxeUtil.h"
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +PhyDxeInitialization (
> > +  IN PHY_DRIVER* PhyDriver,
> > +  IN UINT32      MacBaseAddress
> > +  )
> > +{
> > +  DEBUG ((EFI_D_INFO, "SNP:PHY: %a ()\r\n", __FUNCTION__));
> > +
> > +  // initialize the phyaddr
> > +  PhyDriver->PhyAddr = 0;
> > +  PhyDriver->PhyCurrentLink = LINK_DOWN;
> > +  PhyDriver->PhyOldLink = LINK_DOWN;
> > +
> > +  if (PhyDetectDevice (PhyDriver, MacBaseAddress) != EFI_SUCCESS) {
>
> Use a status variable and EFI_ERROR()
>
> > +    return EFI_NOT_FOUND;
> > +  }
> > +
> > +  PhyConfig (PhyDriver, MacBaseAddress);
> > +
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +
> > +// PHY detect device
> > +EFI_STATUS
> > +EFIAPI
> > +PhyDetectDevice (
> > +  IN PHY_DRIVER* PhyDriver,
> > +  IN UINT32      MacBaseAddress
> > +  )
> > +{
> > +  UINT32 PhyAddr;
> > +
> > +  DEBUG ((EFI_D_INFO, "SNP:PHY: %a ()\r\n", __FUNCTION__));
> > +
> > +  for (PhyAddr = 0; PhyAddr < 32; PhyAddr++) {
> > +    if (PhyReadId (PhyAddr, MacBaseAddress) == EFI_SUCCESS) {
>
> Please use a Status variable and EFI_ERROR() here, and handle the error
> condition not the success condition.
>
> > +      PhyDriver->PhyAddr = PhyAddr;
> > +      return EFI_SUCCESS;
> > +    }
> > +  }
> > +  Print (L"SNP:PHY: Fail to detect Ethernet PHY!\r\n");
>
> Don't use print() in drivers
>
> > +  return EFI_NOT_FOUND;
> > +}
> > +
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +PhyConfig (
> > +  IN PHY_DRIVER* PhyDriver,
> > +  IN UINT32      MacBaseAddress
> > +  )
> > +{
> > +  EFI_STATUS Status;
> > +
> > +  DEBUG ((EFI_D_INFO, "SNP:PHY: %a ()\r\n", __FUNCTION__));
> > +
> > +  Status = PhySoftReset (PhyDriver, MacBaseAddress);
> > +  if (EFI_ERROR (Status)) {
> > +    return EFI_DEVICE_ERROR;
> > +  }
> > +
> > +  // Configure TX/RX Skew
> > +  PhyConfigSkew (PhyDriver, MacBaseAddress);
> > +
> > +  // Read back and display Skew settings
> > +  PhyDisplayConfigSkew (PhyDriver, MacBaseAddress);
> > +
> > +  // Configure AN FLP Burst Trasmit timing interval
> > +  PhyConfigFlpBurstTiming (PhyDriver, MacBaseAddress);
> > +  PhyDisplayFlpBurstTiming (PhyDriver, MacBaseAddress);
> > +
> > +  // Configure AN and Advertise
> > +  PhyAutoNego (PhyDriver,MacBaseAddress);
> > +
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +
> > +// Perform PHY software reset
> > +EFI_STATUS
> > +EFIAPI
> > +PhySoftReset (
> > +  IN PHY_DRIVER* PhyDriver,
> > +  IN UINT32      MacBaseAddress
> > +  )
> > +{
> > +  UINT32        TimeOut;
> > +  UINT32        Data32;
> > +  EFI_STATUS    Status;
> > +
> > +  DEBUG ((EFI_D_INFO, "SNP:PHY: %a ()\r\n", __FUNCTION__));
> > +
> > +  // PHY Basic Control Register reset
> > +  PhyWrite (PhyDriver->PhyAddr, PHY_BASIC_CTRL, PHYCTRL_RESET, MacBaseAddress);
> > +
> > +  // Wait for completion
> > +  TimeOut = 0;
> > +  do {
> > +    // Read PHY_BASIC_CTRL register from PHY
> > +    Status = PhyRead (PhyDriver->PhyAddr, PHY_BASIC_CTRL, &Data32, MacBaseAddress);
> > +    if (EFI_ERROR(Status)) {
> > +      return Status;
> > +    }
> > +    // Wait until PHYCTRL_RESET become zero
> > +    if ((Data32 & PHYCTRL_RESET) == 0) {
> > +      break;
> > +    }
> > +    MicroSecondDelay(1);
> > +  } while (TimeOut++ < PHY_TIMEOUT);
> > +  if (TimeOut >= PHY_TIMEOUT) {
> > +    DEBUG ((EFI_D_INFO, "SNP:PHY: ERROR! PhySoftReset timeout\n"));
> > +    return EFI_TIMEOUT;
> > +  }
> > +
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +
> > +// PHY read ID
> > +EFI_STATUS
> > +EFIAPI
> > +PhyReadId (
> > +  IN UINT32  PhyAddr,
> > +  IN UINT32  MacBaseAddress
> > +  )
> > +{
> > +  EFI_STATUS    Status;
> > +  UINT32        PhyId1;
> > +  UINT32        PhyId2;
> > +
> > +  //DEBUG ((EFI_D_INFO, "SNP:PHY: %a (0x%02X)\r\n", __FUNCTION__, PhyAddr));
> > +
> > +  Status = PhyRead (PhyAddr, PHY_ID1, &PhyId1, MacBaseAddress);
> > +  if (EFI_ERROR(Status)) return Status;
>
> Use proper { } and put the return statement on a separate line.
>
> > +  Status = PhyRead (PhyAddr, PHY_ID2, &PhyId2, MacBaseAddress);
> > +  if (EFI_ERROR(Status)) return Status;
>
> Same here
>
> > +
> > +  if (PhyId1 == PHY_INVALID_ID || PhyId2 == PHY_INVALID_ID) {
> > +    return EFI_NOT_FOUND;
> > +  }
> > +
> > +  DEBUG ((EFI_D_INFO, "SNP:PHY: Ethernet PHY detected. PHY_ID1=0x%04X, PHY_ID2=0x%04X, PHY_ADDR=0x%02X\r\n", PhyId1, PhyId2, PhyAddr));
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +
> > +VOID
> > +EFIAPI
> > +PhyConfigSkew (
> > +  IN PHY_DRIVER* PhyDriver,
> > +  IN UINT32      MacBaseAddress
> > +  )
> > +{
> > +  Phy9031ExtendedWrite(PhyDriver, PHY_KSZ9031_MOD_DATA_NO_POST_INC, PHY_KSZ9031RN_DEV_ADDR, PHY_KSZ9031RN_CONTROL_PAD_SKEW_REG, PHY_KSZ9031RN_CONTROL_PAD_SKEW_VALUE, MacBaseAddress);
> > +  Phy9031ExtendedWrite(PhyDriver, PHY_KSZ9031_MOD_DATA_NO_POST_INC, PHY_KSZ9031RN_DEV_ADDR, PHY_KSZ9031RN_CLK_PAD_SKEW_REG,     PHY_KSZ9031RN_CLK_PAD_SKEW_VALUE, MacBaseAddress);
> > +  Phy9031ExtendedWrite(PhyDriver, PHY_KSZ9031_MOD_DATA_NO_POST_INC, PHY_KSZ9031RN_DEV_ADDR, PHY_KSZ9031RN_RX_DATA_PAD_SKEW_REG, PHY_KSZ9031RN_RX_DATA_PAD_SKEW_VALUE, MacBaseAddress);
> > +  Phy9031ExtendedWrite(PhyDriver, PHY_KSZ9031_MOD_DATA_NO_POST_INC, PHY_KSZ9031RN_DEV_ADDR, PHY_KSZ9031RN_TX_DATA_PAD_SKEW_REG, PHY_KSZ9031RN_TX_DATA_PAD_SKEW_VALUE, MacBaseAddress);
> > +
>
> Wrap overly long lines
>
> > +}
> > +
> > +
> > +VOID
> > +EFIAPI
> > +PhyDisplayConfigSkew (
> > +  IN PHY_DRIVER* PhyDriver,
> > +  IN UINT32      MacBaseAddress
> > +  )
> > +{
> > +  // Display skew configuration
> > +  DEBUG ((EFI_D_INFO, "SNP:PHY: Control Signal Pad Skew = 0x%04X\r\n", Phy9031ExtendedRead(PhyDriver, PHY_KSZ9031_MOD_DATA_NO_POST_INC, PHY_KSZ9031RN_DEV_ADDR, PHY_KSZ9031RN_CONTROL_PAD_SKEW_REG, MacBaseAddress)));
> > +  DEBUG ((EFI_D_INFO, "SNP:PHY: RGMII Clock Pad Skew    = 0x%04X\r\n", Phy9031ExtendedRead(PhyDriver, PHY_KSZ9031_MOD_DATA_NO_POST_INC, PHY_KSZ9031RN_DEV_ADDR, PHY_KSZ9031RN_CLK_PAD_SKEW_REG, MacBaseAddress)));
> > +  DEBUG ((EFI_D_INFO, "SNP:PHY: RGMII RX Data Pad Skew  = 0x%04X\r\n", Phy9031ExtendedRead(PhyDriver, PHY_KSZ9031_MOD_DATA_NO_POST_INC, PHY_KSZ9031RN_DEV_ADDR, PHY_KSZ9031RN_RX_DATA_PAD_SKEW_REG, MacBaseAddress)));
> > +  DEBUG ((EFI_D_INFO, "SNP:PHY: RGMII TX Data Pad Skew  = 0x%04X\r\n", Phy9031ExtendedRead(PhyDriver, PHY_KSZ9031_MOD_DATA_NO_POST_INC, PHY_KSZ9031RN_DEV_ADDR, PHY_KSZ9031RN_TX_DATA_PAD_SKEW_REG, MacBaseAddress)));
>
> Same here
>
>
> > +}
> > +
> > +VOID
> > +EFIAPI
> > +PhyConfigFlpBurstTiming (
> > +  IN PHY_DRIVER* PhyDriver,
> > +  IN UINT32      MacBaseAddress
> > +  )
> > +{
> > +  Phy9031ExtendedWrite(PhyDriver, PHY_KSZ9031_MOD_DATA_NO_POST_INC, PHY_KSZ9031RN_MMD_DEV_ADDR_00, PHY_KSZ9031RN_MMD_D0_FLP_LO_REG, PHY_KSZ9031RN_MMD_D0_FLP_16MS_LO, MacBaseAddress);
> > +  Phy9031ExtendedWrite(PhyDriver, PHY_KSZ9031_MOD_DATA_NO_POST_INC, PHY_KSZ9031RN_MMD_DEV_ADDR_00, PHY_KSZ9031RN_MMD_D0_FLP_HI_REG, PHY_KSZ9031RN_MMD_D0_FLP_16MS_HI, MacBaseAddress);
>
>
> and here
>
> > +}
> > +
> > +VOID
> > +EFIAPI
> > +PhyDisplayFlpBurstTiming (
> > +  IN PHY_DRIVER* PhyDriver,
> > +  IN UINT32      MacBaseAddress
> > +  )
> > +{
> > +  // Display Auto-Negotiation FLP burst transmit timing
> > +  DEBUG ((EFI_D_INFO, "SNP:PHY: AN FLP Burst Transmit - LO = 0x%04X\r\n", Phy9031ExtendedRead(PhyDriver, PHY_KSZ9031_MOD_DATA_NO_POST_INC, PHY_KSZ9031RN_MMD_DEV_ADDR_00, PHY_KSZ9031RN_MMD_D0_FLP_LO_REG, MacBaseAddress)));
> > +  DEBUG ((EFI_D_INFO, "SNP:PHY: AN FLP Burst Transmit - HI = 0x%04X\r\n", Phy9031ExtendedRead(PhyDriver, PHY_KSZ9031_MOD_DATA_NO_POST_INC, PHY_KSZ9031RN_MMD_DEV_ADDR_00, PHY_KSZ9031RN_MMD_D0_FLP_HI_REG, MacBaseAddress)));
> > +}
> > +
> > +// Do auto-negotiation
> > +EFI_STATUS
> > +EFIAPI
> > +PhyAutoNego (
> > +  IN PHY_DRIVER* PhyDriver,
> > +  IN UINT32      MacBaseAddress
> > +  )
> > +{
> > +  EFI_STATUS    Status;
> > +  UINT32        PhyControl;
> > +  UINT32        PhyStatus;
> > +  UINT32        Features;
> > +
> > +  DEBUG ((EFI_D_INFO, "SNP:PHY: %a ()\r\n", __FUNCTION__));
> > +
> > +  // Read PHY Status
> > +  Status = PhyRead (PhyDriver->PhyAddr, PHY_BASIC_STATUS, &PhyStatus, MacBaseAddress);
> > +  if (EFI_ERROR(Status)) return Status;
> > +
> > +  // Check PHY Status if auto-negotiation is supported
> > +  if ((PhyStatus & PHYSTS_AUTO_CAP) == 0) {
> > +    DEBUG ((EFI_D_INFO, "SNP:PHY: Auto-negotiation is not supported.\n"));
> > +    return EFI_DEVICE_ERROR;
> > +  }
> > +
> > +  // Read PHY Auto-Nego Advertise capabilities register for 10/100 Base-T
> > +  Status = PhyRead (PhyDriver->PhyAddr, PHY_AUTO_NEG_ADVERT, &Features, MacBaseAddress);
> > +  if (EFI_ERROR(Status)) return Status;
> > +
> > +  // Set Advertise capabilities for 10Base-T/10Base-T full-duplex/100Base-T/100Base-T full-duplex
> > +  Features |= (PHYANA_10BASET | PHYANA_10BASETFD | PHYANA_100BASETX | PHYANA_100BASETXFD);
> > +  PhyWrite (PhyDriver->PhyAddr, PHY_AUTO_NEG_ADVERT, Features, MacBaseAddress);
> > +
> > +  // Read PHY Auto-Nego Advertise capabilities register for 1000 Base-T
> > +  Status = PhyRead (PhyDriver->PhyAddr, PHY_1000BASE_T_CONTROL, &Features, MacBaseAddress);
> > +  if (EFI_ERROR(Status)) return Status;
> > +
> > +  // Set Advertise capabilities for 1000 Base-T/1000 Base-T full-duplex
> > +  Features |= (PHYADVERTISE_1000FULL | PHYADVERTISE_1000HALF);
> > +  PhyWrite (PhyDriver->PhyAddr, PHY_1000BASE_T_CONTROL, Features, MacBaseAddress);
> > +
> > +  // Read control register
> > +  Status = PhyRead (PhyDriver->PhyAddr, PHY_BASIC_CTRL, &PhyControl, MacBaseAddress);
> > +  if (EFI_ERROR(Status)) return Status;
> > +
> > +  // Enable Auto-Negotiation
> > +  PhyControl |= PHYCTRL_AUTO_EN;
> > +  // Restart auto-negotiation
> > +  PhyControl |= PHYCTRL_RST_AUTO;
> > +  // Write this configuration
> > +  PhyWrite (PhyDriver->PhyAddr, PHY_BASIC_CTRL, PhyControl, MacBaseAddress);
> > +
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +PhyLinkAdjustEmacConfig (
> > +  IN PHY_DRIVER* PhyDriver,
> > +  IN UINT32      MacBaseAddress
> > +  )
> > +{
> > +  UINT32 Speed;
> > +  UINT32 Duplex;
> > +  EFI_STATUS Status;
> > +
> > +  //DEBUG ((EFI_D_INFO, "SNP:PHY: %a ()\r\n", __FUNCTION__));
> > +
> > +  Status = EFI_SUCCESS;
> > +  Speed = SPEED_10;
> > +  Duplex = DUPLEX_HALF;
> > +
> > +  if (PhyCheckLinkStatus (PhyDriver, MacBaseAddress) == EFI_SUCCESS) {
>
> Use a status variable and EFI_ERROR()
>
> > +    PhyDriver->PhyCurrentLink = LINK_UP;
> > +  } else {
> > +    PhyDriver->PhyCurrentLink = LINK_DOWN;
> > +  }
> > +
> > +  if (PhyDriver->PhyOldLink != PhyDriver->PhyCurrentLink) {
> > +    if (PhyDriver->PhyCurrentLink == LINK_UP) {
> > +      Print (L"SNP:PHY: Link is up - Network Cable is Plugged\r\n");
> > +      PhyReadCapability (PhyDriver, &Speed, &Duplex, MacBaseAddress);
> > +      EmacConfigAdjust (Speed, Duplex, MacBaseAddress);
> > +      Status = EFI_SUCCESS;
> > +    } else {
> > +      Print (L"SNP:PHY: Link is Down - Network Cable is Unplugged?\r\n");
> > +      Status = EFI_NOT_READY;
> > +    }
>
> Don't use print() in drivers.
>
> > +  } else if (PhyDriver->PhyCurrentLink == LINK_DOWN) {
> > +    Status = EFI_NOT_READY;
> > +  }
> > +
> > +  PhyDriver->PhyOldLink = PhyDriver->PhyCurrentLink;
> > +
> > +  return Status;
> > +}
> > +
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +PhyCheckLinkStatus (
> > +  IN PHY_DRIVER* PhyDriver,
> > +  IN UINT32      MacBaseAddress
> > +  )
> > +{
> > +  EFI_STATUS    Status;
> > +  UINT32        Data32;
> > +  UINTN         TimeOut;
> > +  UINT32        PhyBasicStatus;
> > +
> > +  //DEBUG ((EFI_D_INFO, "SNP:PHY: %a ()\r\n", __FUNCTION__));
> > +
> > +  // Get the PHY Status
> > +  Status = PhyRead (PhyDriver->PhyAddr, PHY_BASIC_STATUS, &PhyBasicStatus, MacBaseAddress);
> > +  if (EFI_ERROR(Status)) return Status;
> > +
> > +  // if Link is already up then dont need to proceed anymore
> > +  if (PhyBasicStatus & PHYSTS_LINK_STS) {
> > +    return EFI_SUCCESS;
> > +  }
> > +
> > +  // Wait until it is up or until Time Out
> > +  TimeOut = 0;
> > +  do {
> > +    // Read PHY_BASIC_STATUS register from PHY
> > +    Status = PhyRead (PhyDriver->PhyAddr, PHY_BASIC_STATUS, &Data32, MacBaseAddress);
> > +    if (EFI_ERROR(Status)) {
> > +      return Status;
> > +    }
> > +    // Wait until PHYSTS_LINK_STS become one
> > +    if (Data32 & PHYSTS_LINK_STS) {
> > +      // Link is up
> > +      break;
> > +    }
> > +    MicroSecondDelay(1);
> > +  } while (TimeOut++ < PHY_TIMEOUT);
> > +  if (TimeOut >= PHY_TIMEOUT) {
> > +    // Link is down
> > +    return EFI_TIMEOUT;
> > +  }
> > +
> > +  // Wait until autonego process has completed
> > +  TimeOut = 0;
> > +  do {
> > +    // Read PHY_BASIC_STATUS register from PHY
> > +    Status = PhyRead (PhyDriver->PhyAddr, PHY_BASIC_STATUS, &Data32, MacBaseAddress);
> > +    if (EFI_ERROR(Status)) {
>
> Space before (
>
> > +      return Status;
> > +    }
> > +    // Wait until PHYSTS_AUTO_COMP become one
> > +    if (Data32 & PHYSTS_AUTO_COMP) {
> > +      DEBUG ((EFI_D_INFO, "SNP:PHY: Auto Negotiation completed\r\n"));
> > +      break;
> > +    }
> > +    MicroSecondDelay(1);
> > +  } while (TimeOut++ < PHY_TIMEOUT);
> > +  if (TimeOut >= PHY_TIMEOUT) {
> > +    DEBUG ((EFI_D_INFO, "SNP:PHY: Error! Auto Negotiation timeout\n"));
> > +    return EFI_TIMEOUT;
> > +  }
> > +
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +PhyReadCapability (
> > +  IN PHY_DRIVER* PhyDriver,
> > +  IN UINT32* Speed,
> > +  IN UINT32* Duplex,
> > +  IN UINT32  MacBaseAddress
> > +  )
> > +{
> > +  EFI_STATUS    Status;
> > +  UINT32        PartnerAbilityGb;
> > +  UINT32        AdvertisingGb;
> > +  UINT32        CommonAbilityGb;
> > +  UINT32        PartnerAbility;
> > +  UINT32        Advertising;
> > +  UINT32        CommonAbility;
> > +
> > +  //DEBUG ((EFI_D_INFO, "SNP:PHY: %a ()\r\n", __FUNCTION__));
> > +
> > +  // For 1000 Base-T
> > +
> > +  Status = PhyRead (PhyDriver->PhyAddr, PHY_1000BASE_T_STATUS, &PartnerAbilityGb, MacBaseAddress);
> > +  if (EFI_ERROR(Status)) return Status;
> > +
> > +  Status = PhyRead (PhyDriver->PhyAddr, PHY_1000BASE_T_CONTROL, &AdvertisingGb, MacBaseAddress);
> > +  if (EFI_ERROR(Status)) return Status;
> > +
> > +  CommonAbilityGb = PartnerAbilityGb & (AdvertisingGb << 2);
> > +
> > +  // For 10/100 Base-T
> > +
> > +  Status = PhyRead (PhyDriver->PhyAddr, PHY_AUTO_NEG_LINK_ABILITY, &PartnerAbility, MacBaseAddress);
> > +  if (EFI_ERROR(Status)) return Status;
> > +
> > +  Status = PhyRead (PhyDriver->PhyAddr, PHY_AUTO_NEG_EXP, &Advertising, MacBaseAddress);
> > +  if (EFI_ERROR(Status)) return Status;
> > +
> > +  CommonAbility = PartnerAbility & Advertising;
> > +
> > +  // Determine the Speed and Duplex
> > +  if (PartnerAbilityGb & (PHYLPA_1000FULL | PHYLPA_1000HALF)) {
> > +    *Speed = SPEED_1000;
> > +    if (CommonAbilityGb & PHYLPA_1000FULL) {
> > +      *Duplex = DUPLEX_FULL;
> > +    }
> > +  } else if (CommonAbility & (PHYLPA_100FULL | PHYLPA_100HALF)) {
> > +    *Speed = SPEED_100;
> > +    if (CommonAbility & PHYLPA_100FULL) {
> > +      *Duplex = DUPLEX_FULL;
> > +    } else if (CommonAbility & PHYLPA_10FULL) {
> > +      *Duplex = DUPLEX_FULL;
> > +    }
> > +  }
> > +
> > +  PhyDisplayAbility (*Speed, *Duplex);
> > +
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +
> > +VOID
> > +EFIAPI
> > +PhyDisplayAbility (
> > +  IN UINT32 Speed,
> > +  IN UINT32 Duplex
> > +  )
> > +{
> > +  //DEBUG ((EFI_D_INFO, "SNP:PHY: %a ()\r\n", __FUNCTION__));
> > +
> > +  Print (L"SNP:PHY: ");
> > +  switch (Speed) {
> > +    case SPEED_1000:
> > +      Print (L"1 Gbps - ");
> > +      break;
> > +    case SPEED_100:
> > +      Print (L"100 Mbps - ");
> > +      break;
> > +    case SPEED_10:
> > +      Print (L"10 Mbps - ");
> > +      break;
> > +    default:
> > +      Print (L"Invalid link speed");
> > +      break;
> > +    }
> > +
> > +  switch (Duplex) {
> > +    case DUPLEX_FULL:
> > +      Print (L"Full Duplex\n");
> > +      break;
> > +    case DUPLEX_HALF:
> > +      Print (L"Half Duplex\n");
> > +      break;
> > +    default:
> > +      Print (L"Invalid duplex mode\n");
> > +      break;
> > +    }
>
> Don't use print() in drivers.
>
> > +}
> > +
> > +
> > +// Function to read from MII register (PHY Access)
> > +EFI_STATUS
> > +EFIAPI
> > +PhyRead (
> > +  IN  UINT32  Addr,
> > +  IN  UINT32  Reg,
> > +  OUT UINT32 *Data,
> > +  IN  UINT32  MacBaseAddress
> > +  )
> > +{
> > +  UINT32        MiiConfig;
> > +  UINT32        Count;
> > +
> > +  //DEBUG ((EFI_D_INFO, "SNP:PHY: %a ()\r\n", __FUNCTION__));
> > +
> > +  // Check it is a valid Reg
> > +  ASSERT(Reg < 31);
> > +
> > +  MiiConfig = ((Addr << MIIADDRSHIFT) & MII_ADDRMSK) |
> > +              ((Reg << MIIREGSHIFT) & MII_REGMSK)|
> > +               MII_CLKRANGE_150_250M |
> > +               MII_BUSY;
> > +
> > +  // write this config to register
> > +  MmioWrite32(MacBaseAddress + DW_EMAC_GMACGRP_GMII_ADDRESS_OFST, MiiConfig);
> > +
> > +  // Wait for busy bit to clear
> > +  Count = 0;
> > +  while (Count < 10000) {
> > +    if (!(DW_EMAC_GMACGRP_GMII_ADDRESS_GB_GET(MmioRead32(MacBaseAddress + DW_EMAC_GMACGRP_GMII_ADDRESS_OFST))))
> > +       {
> > +      *Data = DW_EMAC_GMACGRP_GMII_DATA_GD_GET(MmioRead32(MacBaseAddress + DW_EMAC_GMACGRP_GMII_DATA_OFST));
> > +      return EFI_SUCCESS;
> > +       }
>
> Please fix indentation
>
> > +    Count++;
> > +  };
> > +  DEBUG ((EFI_D_INFO, "SNP:PHY: MDIO busy bit timeout\r\n"));
> > +  return EFI_TIMEOUT;
> > +}
> > +
> > +
> > +// Function to write to the MII register (PHY Access)
> > +EFI_STATUS
> > +EFIAPI
> > +PhyWrite (
> > +  IN UINT32 Addr,
> > +  IN UINT32 Reg,
> > +  IN UINT32 Data,
> > +  IN UINT32 MacBaseAddress
> > +  )
> > +{
> > +  UINT32 MiiConfig;
> > +  UINT32 Count;
> > +
> > +  //DEBUG ((EFI_D_INFO, "SNP:PHY: %a ()\r\n", __FUNCTION__));
> > +
> > +  // Check it is a valid Reg
> > +  ASSERT(Reg < 31);
> > +
> > +  MiiConfig = ((Addr << MIIADDRSHIFT) & MII_ADDRMSK) |
> > +              ((Reg << MIIREGSHIFT) & MII_REGMSK)|
> > +               MII_WRITE |
> > +               MII_CLKRANGE_150_250M |
> > +               MII_BUSY;
> > +  // Write the desired value to the register first
> > +  MmioWrite32 (MacBaseAddress + DW_EMAC_GMACGRP_GMII_DATA_OFST, (Data & 0xFFFF));
> > +
> > +  // write this config to register
> > +  MmioWrite32(MacBaseAddress + DW_EMAC_GMACGRP_GMII_ADDRESS_OFST, MiiConfig);
>
> Space before (
>
> > +
> > +  // Wait for busy bit to clear
> > +  Count = 0;
> > +  while (Count < 1000) {
> > +      if (!(DW_EMAC_GMACGRP_GMII_ADDRESS_GB_GET(MmioRead32(MacBaseAddress + DW_EMAC_GMACGRP_GMII_ADDRESS_OFST))))
> > +        return EFI_SUCCESS;
>
> Use { }
>
> Should you add a MemoryFence() here?
>
> > +      Count++;
> > +    };
> > +
> > +  return EFI_TIMEOUT;
> > +}
> > +
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +Phy9031ExtendedWrite (
> > +  IN PHY_DRIVER* PhyDriver,
> > +  IN UINT32      Mode,
> > +  IN UINT32      DevAddr,
> > +  IN UINT32      Regnum,
> > +  IN UINT16      Val,
> > +  IN UINT32      MacBaseAddress
> > +  )
> > +{
> > +  //DEBUG ((EFI_D_INFO, "SNP:PHY: %a ()\r\n", __FUNCTION__));
> > +
> > +  PhyWrite(PhyDriver->PhyAddr, PHY_KSZ9031RN_MMD_CTRL_REG, DevAddr, MacBaseAddress);
> > +  PhyWrite(PhyDriver->PhyAddr, PHY_KSZ9031RN_MMD_REGDATA_REG, Regnum, MacBaseAddress);
> > +  PhyWrite(PhyDriver->PhyAddr, PHY_KSZ9031RN_MMD_CTRL_REG, (Mode << 14) | DevAddr, MacBaseAddress);
>
> Space before (
>
> > +  return PhyWrite(PhyDriver->PhyAddr, PHY_KSZ9031RN_MMD_REGDATA_REG, Val, MacBaseAddress);
> > +}
> > +
> > +
> > +UINT32
> > +EFIAPI
> > +Phy9031ExtendedRead (
> > +  IN PHY_DRIVER* PhyDriver,
> > +  IN UINT32      Mode,
> > +  IN UINT32      DevAddr,
> > +  IN UINT32      Regnum,
> > +  IN UINT32      MacBaseAddress
> > +  )
> > +{
> > +  EFI_STATUS    Status;
> > +  UINT32        Data32;
> > +
> > +  //DEBUG ((EFI_D_INFO, "SNP:PHY: %a ()\r\n", __FUNCTION__));
> > +
> > +  PhyWrite(PhyDriver->PhyAddr, PHY_KSZ9031RN_MMD_CTRL_REG, DevAddr, MacBaseAddress);
> > +  PhyWrite(PhyDriver->PhyAddr, PHY_KSZ9031RN_MMD_REGDATA_REG, Regnum, MacBaseAddress);
> > +  PhyWrite(PhyDriver->PhyAddr, PHY_KSZ9031RN_MMD_CTRL_REG, (Mode << 14) | DevAddr, MacBaseAddress);
> > +
>
> Space before (
>
> > +  Status = PhyRead (PhyDriver->PhyAddr, PHY_KSZ9031RN_MMD_REGDATA_REG, &Data32, MacBaseAddress);
> > +  if (EFI_ERROR(Status)) return 0;
>
> Use { } and put return on a separate line
>
> > +
> > +  return Data32;
> > +}
> > +
> > +
> > --
> > 2.17.1
> >

  reply	other threads:[~2019-05-17 16:25 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-08  9:22 [PATCH v4 edk2-platforms 1/1] Silicon/DesignWare/Driver: DwEmacSnpDxe: Add DesignWare EMAC driver tzy.way.ooi
2019-05-16 19:37 ` Ard Biesheuvel
2019-05-17 16:25   ` Ard Biesheuvel [this message]
  -- strict thread matches above, loose matches on Subject: below --
2019-05-08  9:52 tzy.way.ooi
2019-05-22  2:37 ` tzy.way.ooi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAKv+Gu8Ka2MSu8XX8o=iDHe8VkJeBaWZr-gg69Qk=0=18jJqTQ@mail.gmail.com' \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox