Re: [PATCH edk2-platforms v2 5/7] Platform/RaspberryPi4: Register GENET platform device protocol

["Leif Lindholm" <leif@nuviainc.com>]

On Tue, May 12, 2020 at 09:55:10 +0200, Ard Biesheuvel wrote:
> Register an event at EndOfDxe to instantiate the EFI device path protocol
> with the GENET MAC address on a new handle, and install the
> BcmGenetPlatformDeviceProtocol on that handle. This protocol is used to
> pass platform information (GENET MAC address and register base address)
> to the GENET driver, which will consume this in its implementation of the
> UEFI driver model Supported/Start/Stop methods.
> 
> Co-authored-by: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
> Co-authored-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Co-authored-by: Andrei Warkentin <awarkentin@vmware.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> ---
>  Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf |  29 ++++--
>  Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c   | 108 ++++++++++++++++++--
>  2 files changed, 117 insertions(+), 20 deletions(-)
> 
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> index e47f3af69199..f20f3bcc1243 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> @@ -1,6 +1,8 @@
>  #/** @file
>  #
> -#  Copyright (c) 2019, ARM Limited. All rights reserved.
> +#  Component description file for the RasbperryPi DXE platform config driver.
> +#
> +#  Copyright (c) 2019 - 2020, ARM Limited. All rights reserved.
>  #  Copyright (c) 2018, Andrei Warkentin <andrey.warkentin@gmail.com>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -21,7 +23,6 @@ [Defines]
>  #
>  #  VALID_ARCHITECTURES           = AARCH64
>  #
> -
>  [Sources]
>    ConfigDxe.c
>    ConfigDxeFormSetGuid.h
> @@ -31,31 +32,36 @@ [Sources]
>  [Packages]
>    ArmPkg/ArmPkg.dec
>    ArmPlatformPkg/ArmPlatformPkg.dec
> -  MdePkg/MdePkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
>    MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +  NetworkPkg/NetworkPkg.dec
> +  Platform/RaspberryPi/RaspberryPi.dec
>    Silicon/Broadcom/Bcm27xx/Bcm27xx.dec
>    Silicon/Broadcom/Bcm283x/Bcm283x.dec
> -  Platform/RaspberryPi/RaspberryPi.dec
> -  EmbeddedPkg/EmbeddedPkg.dec
> +  Silicon/Broadcom/Drivers/Net/BcmNet.dec
>  
>  [LibraryClasses]
> +  AcpiLib
>    BaseLib
>    DebugLib
>    DxeServicesTableLib
> +  GpioLib
> +  HiiLib
> +  NetLib
>    PcdLib
>    UefiBootServicesTableLib
> -  UefiRuntimeServicesTableLib
>    UefiDriverEntryPoint
> -  HiiLib
> -  GpioLib
> -  AcpiLib
> +  UefiRuntimeServicesTableLib

The cleanup is good, but could we separate functional and
non-functional changes into separate patches?

>  
>  [Guids]
>    gConfigDxeFormSetGuid
> +  gEfiEndOfDxeEventGroupGuid
>  
>  [Protocols]
> -  gRaspberryPiFirmwareProtocolGuid      ## CONSUMES
> -  gRaspberryPiConfigAppliedProtocolGuid ## PRODUCES
> +  gBcmGenetPlatformDeviceProtocolGuid             ## PRODUCES
> +  gRaspberryPiFirmwareProtocolGuid                ## CONSUMES
> +  gRaspberryPiConfigAppliedProtocolGuid           ## PRODUCES
>  
>  [FixedPcd]
>    gRaspberryPiTokenSpaceGuid.PcdCpuLowSpeedMHz
> @@ -64,6 +70,7 @@ [FixedPcd]
>  
>  [Pcd]
>    gBcm27xxTokenSpaceGuid.PcdBcm27xxRegistersAddress
> +  gBcm27xxTokenSpaceGuid.PcdBcmGenetRegistersAddress
>    gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress
>    gRaspberryPiTokenSpaceGuid.PcdCpuClock
>    gRaspberryPiTokenSpaceGuid.PcdCustomCpuClock
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> index 00867879da20..47ca0a89a3d8 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> @@ -1,6 +1,6 @@
>  /** @file
>   *
> - *  Copyright (c) 2019, ARM Limited. All rights reserved.
> + *  Copyright (c) 2019 - 2020, ARM Limited. All rights reserved.
>   *  Copyright (c) 2018 - 2019, Andrei Warkentin <andrey.warkentin@gmail.com>
>   *
>   *  SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -8,19 +8,21 @@
>   **/
>  
>  #include <Uefi.h>
> +#include <IndustryStandard/Bcm2711.h>
> +#include <IndustryStandard/Bcm2836.h>
> +#include <IndustryStandard/Bcm2836Gpio.h>
> +#include <IndustryStandard/RpiMbox.h>
>  #include <Library/AcpiLib.h>
> -#include <Library/HiiLib.h>
>  #include <Library/DebugLib.h>
> +#include <Library/DevicePathLib.h>
>  #include <Library/DxeServicesTableLib.h>
> +#include <Library/GpioLib.h>
> +#include <Library/HiiLib.h>
>  #include <Library/IoLib.h>
> +#include <Library/NetLib.h>
>  #include <Library/UefiBootServicesTableLib.h>
>  #include <Library/UefiRuntimeServicesTableLib.h>
> -#include <Library/DevicePathLib.h>
> -#include <IndustryStandard/RpiMbox.h>
> -#include <IndustryStandard/Bcm2711.h>
> -#include <IndustryStandard/Bcm2836.h>
> -#include <IndustryStandard/Bcm2836Gpio.h>
> -#include <Library/GpioLib.h>
> +#include <Protocol/BcmGenetPlatformDevice.h>
>  #include <Protocol/RpiFirmware.h>
>  #include <ConfigVars.h>
>  #include "ConfigDxeFormSetGuid.h"
> @@ -34,6 +36,8 @@ STATIC RASPBERRY_PI_FIRMWARE_PROTOCOL *mFwProtocol;
>  STATIC UINT32 mModelFamily = 0;
>  STATIC UINT32 mModelInstalledMB = 0;
>  
> +STATIC EFI_MAC_ADDRESS  MacAddress;

m-prefix on global variable?

/
    Leif

> +
>  /*
>   * The GUID inside Platform/RaspberryPi/RPi3/AcpiTables/AcpiTables.inf and
>   * Platform/RaspberryPi/RPi4/AcpiTables/AcpiTables.inf _must_ match below.
> @@ -47,6 +51,18 @@ typedef struct {
>    EFI_DEVICE_PATH_PROTOCOL End;
>  } HII_VENDOR_DEVICE_PATH;
>  
> +#pragma pack (1)
> +typedef struct {
> +  MAC_ADDR_DEVICE_PATH            MacAddrDP;
> +  EFI_DEVICE_PATH_PROTOCOL        End;
> +} GENET_DEVICE_PATH;
> +
> +typedef struct {
> +  GENET_DEVICE_PATH                   DevicePath;
> +  BCM_GENET_PLATFORM_DEVICE_PROTOCOL  PlatformDevice;
> +} GENET_DEVICE;
> +#pragma pack ()
> +
>  STATIC HII_VENDOR_DEVICE_PATH mVendorDevicePath = {
>    {
>      {
> @@ -69,6 +85,65 @@ STATIC HII_VENDOR_DEVICE_PATH mVendorDevicePath = {
>    }
>  };
>  
> +STATIC GENET_DEVICE mGenetDevice = {
> +  {
> +    {
> +      {
> +        MESSAGING_DEVICE_PATH,
> +        MSG_MAC_ADDR_DP,
> +        {
> +          (UINT8)(sizeof (MAC_ADDR_DEVICE_PATH)),
> +          (UINT8)((sizeof (MAC_ADDR_DEVICE_PATH)) >> 8)
> +        }
> +      },
> +      {{ 0 }},
> +      NET_IFTYPE_ETHERNET
> +    },
> +    {
> +      END_DEVICE_PATH_TYPE,
> +      END_ENTIRE_DEVICE_PATH_SUBTYPE,
> +      {
> +        sizeof (EFI_DEVICE_PATH_PROTOCOL),
> +        0
> +      }
> +    }
> +  },
> +  {
> +    GENET_BASE_ADDRESS,
> +    {{ 0 }}
> +  }
> +};
> +
> +
> +STATIC
> +VOID
> +EFIAPI
> +RegisterDevices (
> +  EFI_EVENT           Event,
> +  VOID                *Context
> +  )
> +{
> +  EFI_HANDLE  Handle;
> +  EFI_STATUS  Status;
> +
> +  if (mModelFamily == 4) {
> +    DEBUG ((DEBUG_INFO, "GENET: MAC address %02X:%02X:%02X:%02X:%02X:%02X\n",
> +            MacAddress.Addr[0], MacAddress.Addr[1], MacAddress.Addr[2],
> +            MacAddress.Addr[3], MacAddress.Addr[4], MacAddress.Addr[5]));
> +
> +    CopyMem (&mGenetDevice.DevicePath.MacAddrDP.MacAddress, MacAddress.Addr,
> +      NET_ETHER_ADDR_LEN);
> +    CopyMem (&mGenetDevice.PlatformDevice.MacAddress, MacAddress.Addr,
> +      NET_ETHER_ADDR_LEN);
> +
> +    Handle = NULL;
> +    Status = gBS->InstallMultipleProtocolInterfaces (&Handle,
> +                    &gEfiDevicePathProtocolGuid,          &mGenetDevice.DevicePath,
> +                    &gBcmGenetPlatformDeviceProtocolGuid, &mGenetDevice.PlatformDevice,
> +                    NULL);
> +    ASSERT_EFI_ERROR (Status);
> +  }
> +}
>  
>  STATIC EFI_STATUS
>  InstallHiiPages (
> @@ -242,6 +317,16 @@ SetupVariables (
>      PcdSet32 (PcdDisplayEnableSShot, PcdGet32 (PcdDisplayEnableSShot));
>    }
>  
> +  if (mModelFamily == 4) {
> +    //
> +    // Get the MAC address from the firmware.
> +    //
> +    Status = mFwProtocol->GetMacAddress (MacAddress.Addr);
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((DEBUG_WARN, "%a: failed to retrieve MAC address\n", __FUNCTION__));
> +    }
> +  }
> +
>    return EFI_SUCCESS;
>  }
>  
> @@ -448,7 +533,8 @@ ConfigInitialize (
>    IN EFI_SYSTEM_TABLE *SystemTable
>    )
>  {
> -  EFI_STATUS Status;
> +  EFI_STATUS                      Status;
> +  EFI_EVENT                       EndOfDxeEvent;
>  
>    Status = gBS->LocateProtocol (&gRaspberryPiFirmwareProtocolGuid,
>                    NULL, (VOID**)&mFwProtocol);
> @@ -494,5 +580,9 @@ ConfigInitialize (
>       ASSERT_EFI_ERROR (Status);
>    }
>  
> +  Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, TPL_NOTIFY, RegisterDevices,
> +                  NULL, &gEfiEndOfDxeEventGroupGuid, &EndOfDxeEvent);
> +  ASSERT_EFI_ERROR (Status);
> +
>    return EFI_SUCCESS;
>  }
> -- 
> 2.17.1
>