Re: [PATCH edk2-platforms v2 6/7] Platform/NinetySixBoards: add core driver for LS connector and config

[Leif Lindholm <leif.lindholm@linaro.org>]

On Tue, Feb 20, 2018 at 05:49:43PM +0000, Ard Biesheuvel wrote:
> This adds a driver that manages the 96boards LS connector, i.e, it
> installs a HII page to configure the type of mezzanine that is installed
> in the slot, and it exposes this information via the LS connector protocol.
> It is also in charge of applying the overlay to the platform device tree
> at end of DXE.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  Platform/NinetySixBoards/Include/Guid/FormSet.h                    |  23 ++
>  Platform/NinetySixBoards/NinetySixBoards.dec                       |   3 +
>  Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsDxe.c   | 221 ++++++++++++++++++++
>  Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsDxe.h   |  32 +++
>  Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsDxe.inf |  57 +++++
>  Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsHii.uni |  27 +++
>  Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsHii.vfr |  51 +++++
>  7 files changed, 414 insertions(+)
> 
> diff --git a/Platform/NinetySixBoards/Include/Guid/FormSet.h b/Platform/NinetySixBoards/Include/Guid/FormSet.h
> new file mode 100644
> index 000000000000..db16657f0848
> --- /dev/null
> +++ b/Platform/NinetySixBoards/Include/Guid/FormSet.h
> @@ -0,0 +1,23 @@
> +/** @file
> +
> +  Copyright (c) 2018, Linaro 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.
> +
> +**/
> +
> +#ifndef __NINETY_SIX_BOARDS_FORMSET_H__
> +#define __NINETY_SIX_BOARDS_FORMSET_H__
> +
> +#define NINETY_SIX_BOARDS_FORMSET_GUID  \
> +  { 0x7500c9d2, 0x9203, 0x4a37, { 0x84, 0xbb, 0x92, 0xa9, 0xce, 0x34, 0x38, 0xbd } }
> +
> +extern EFI_GUID gNinetySixBoardsFormsetGuid;
> +
> +#endif // __NINETY_SIX_BOARDS_FORMSET_H__
> diff --git a/Platform/NinetySixBoards/NinetySixBoards.dec b/Platform/NinetySixBoards/NinetySixBoards.dec
> index 5c3fe43dbb24..e7d1705d47ff 100644
> --- a/Platform/NinetySixBoards/NinetySixBoards.dec
> +++ b/Platform/NinetySixBoards/NinetySixBoards.dec
> @@ -37,6 +37,9 @@ [Guids]
>    gNinetySixBoardsI2c1MasterGuid = { 0xcf64ac46, 0xd0be, 0x4a69, { 0x90, 0xa2, 0xf2, 0x82, 0x5b, 0x92, 0x25, 0x61 } }
>    gNinetySixBoardsSpiMasterGuid = { 0x9703fd99, 0xe638, 0x42b8, { 0xab, 0x81, 0x52, 0x61, 0x1b, 0xf7, 0xf7, 0x5d } }
>  
> +  # GUID for the HII configuration form
> +  gNinetySixBoardsFormsetGuid = { 0x7500c9d2, 0x9203, 0x4a37, { 0x84, 0xbb, 0x92, 0xa9, 0xce, 0x34, 0x38, 0xbd } }
> +
>  [PcdsFixedAtBuild]
>    # ASCII DT paths to the I2C parent nodes of the 96boards LS connector
>    gNinetySixBoardsTokenSpaceGuid.PcdI2c0Parent|""|VOID*|0x00000001
> diff --git a/Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsDxe.c b/Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsDxe.c
> new file mode 100644
> index 000000000000..6dc5f549e560
> --- /dev/null
> +++ b/Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsDxe.c
> @@ -0,0 +1,221 @@
> +/** @file
> +
> +  Copyright (c) 2018, Linaro, Ltd. 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 <Uefi.h>
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/DevicePathLib.h>
> +#include <Library/HiiLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiLib.h>
> +#include <Library/UefiRuntimeServicesTableLib.h>
> +#include <Protocol/LsConnector.h>
> +#include <Protocol/Mezzanine.h>
> +
> +#include "NinetySixBoardsDxe.h"
> +
> +extern  UINT8                     NinetySixBoardsHiiBin[];
> +extern  UINT8                     NinetySixBoardsDxeStrings[];
> +
> +typedef struct {
> +  VENDOR_DEVICE_PATH              VendorDevicePath;
> +  EFI_DEVICE_PATH_PROTOCOL        End;
> +} HII_VENDOR_DEVICE_PATH;
> +
> +STATIC HII_VENDOR_DEVICE_PATH     mNinetySixBoardsDxeVendorDevicePath = {
> +  {
> +    {
> +      HARDWARE_DEVICE_PATH,
> +      HW_VENDOR_DP,
> +      {
> +        (UINT8) (sizeof (VENDOR_DEVICE_PATH)),
> +        (UINT8) ((sizeof (VENDOR_DEVICE_PATH)) >> 8)
> +      }
> +    },
> +    NINETY_SIX_BOARDS_FORMSET_GUID
> +  },
> +  {
> +    END_DEVICE_PATH_TYPE,
> +    END_ENTIRE_DEVICE_PATH_SUBTYPE,
> +    {
> +      (UINT8) (END_DEVICE_PATH_LENGTH),
> +      (UINT8) ((END_DEVICE_PATH_LENGTH) >> 8)
> +    }
> +  }
> +};
> +
> +STATIC LS_CONNECTOR_PROTOCOL  mLsConnector;
> +STATIC EFI_EVENT              EndOfDxeEvent;
> +
> +STATIC
> +EFI_STATUS
> +InstallHiiPages (
> +  VOID
> +  )
> +{
> +  EFI_STATUS                      Status;
> +  EFI_HII_HANDLE                  HiiHandle;
> +  EFI_HANDLE                      DriverHandle;
> +
> +  DriverHandle = NULL;
> +  Status = gBS->InstallMultipleProtocolInterfaces (&DriverHandle,
> +                  &gEfiDevicePathProtocolGuid,
> +                  &mNinetySixBoardsDxeVendorDevicePath,
> +                  NULL);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  HiiHandle = HiiAddPackages (&gNinetySixBoardsFormsetGuid,
> +                              DriverHandle,
> +                              NinetySixBoardsDxeStrings,
> +                              NinetySixBoardsHiiBin,
> +                              NULL);
> +
> +  if (HiiHandle == NULL) {
> +    gBS->UninstallMultipleProtocolInterfaces (DriverHandle,
> +           &gEfiDevicePathProtocolGuid,
> +           &mNinetySixBoardsDxeVendorDevicePath,
> +           NULL);
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +  return EFI_SUCCESS;
> +}
> +
> +STATIC
> +VOID
> +EFIAPI
> +ApplyDeviceTreeOverlay (
> +  EFI_EVENT           Event,
> +  VOID                *Context
> +  )
> +{
> +  VOID                    *Dtb;
> +  MEZZANINE_PROTOCOL      *Mezzanine;
> +  EFI_STATUS              Status;
> +
> +  //
> +  // Find the DTB in the configuration table array. If it isn't there, just
> +  // bail without an error: we may be running on an ACPI platform even if
> +  // this driver does not support it [yet].
> +  //
> +  Status = EfiGetSystemConfigurationTable (&gFdtTableGuid, &Dtb);
> +  if (Status == EFI_NOT_FOUND) {
> +    return;
> +  }
> +  ASSERT_EFI_ERROR (Status);
> +
> +  Status = gBS->LocateProtocol (&gNinetySixBoardsMezzanineProtocolGuid, NULL,
> +                  (VOID **)&Mezzanine);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_INFO, "%a: no mezzanine driver active\n", __FUNCTION__));
> +    return;
> +  }
> +
> +  Status = Mezzanine->ApplyDeviceTreeOverlay (Mezzanine, Dtb);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_WARN, "%a: failed to apply DT overlay - %r\n", __FUNCTION__,
> +      Status));
> +  }
> +}
> +
> +/**
> +  The entry point for NinetySixBoardsDxe driver.
> +
> +  @param[in] ImageHandle     The image handle of the driver.
> +  @param[in] SystemTable     The system table.
> +
> +  @retval EFI_ALREADY_STARTED     The driver already exists in system.
> +  @retval EFI_OUT_OF_RESOURCES    Fail to execute entry point due to lack of
> +                                  resources.
> +  @retval EFI_SUCCES              All the related protocols are installed on
> +                                  the driver.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +EntryPoint (
> +  IN EFI_HANDLE                   ImageHandle,
> +  IN EFI_SYSTEM_TABLE             *SystemTable
> +  )
> +{
> +  EFI_STATUS                      Status;
> +  NINETY_SIX_BOARDS_CONFIG_DATA   ConfigData;
> +  UINTN                           BufferSize;
> +
> +  //
> +  // Get the current config settings from the EFI variable.
> +  //
> +  BufferSize = sizeof (ConfigData);
> +  Status = gRT->GetVariable (NINETY_SIX_BOARDS_CONFIG_VARIABLE_NAME,
> +                  &gNinetySixBoardsFormsetGuid, NULL, &BufferSize, &ConfigData);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_INFO, "%a: no config data found\n", __FUNCTION__));
> +    ConfigData.MezzanineType = MEZZANINE_NONE;
> +  }
> +
> +  if (!EFI_ERROR (Status) &&
> +      ConfigData.MezzanineType > MEZZANINE_MAX) {
> +    DEBUG ((DEBUG_WARN,
> +      "%a: invalid value for %s, defaulting to MEZZANINE_NONE\n",
> +      __FUNCTION__, NINETY_SIX_BOARDS_CONFIG_VARIABLE_NAME));
> +    ConfigData.MezzanineType = MEZZANINE_NONE;
> +    Status = EFI_INVALID_PARAMETER; // trigger setvar below
> +  }
> +
> +  //
> +  // Write the newly selected value back to the variable store.
> +  //
> +  if (EFI_ERROR (Status)) {
> +    ZeroMem (&ConfigData.Reserved, sizeof (ConfigData.Reserved));
> +    Status = gRT->SetVariable (NINETY_SIX_BOARDS_CONFIG_VARIABLE_NAME,
> +                    &gNinetySixBoardsFormsetGuid,
> +                    EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS,
> +                    sizeof (ConfigData), &ConfigData);
> +
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((DEBUG_ERROR, "%a: gRT->SetVariable () failed - %r\n",
> +        __FUNCTION__, Status));
> +      return Status;
> +    }
> +  }
> +
> +  switch (ConfigData.MezzanineType) {
> +  case MEZZANINE_SECURE96:
> +    mLsConnector.MezzanineType = MezzanineSecure96;
> +    break;
> +  default:
> +    mLsConnector.MezzanineType = MezzanineUnknown;
> +  }
> +
> +  Status = gBS->InstallProtocolInterface (&ImageHandle,
> +                  &gNinetySixBoardsLsConnectorProtocolGuid,
> +                  EFI_NATIVE_INTERFACE,
> +                  &mLsConnector);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  Status = gBS->CreateEventEx (
> +                  EVT_NOTIFY_SIGNAL,
> +                  TPL_NOTIFY,
> +                  ApplyDeviceTreeOverlay,
> +                  NULL,
> +                  &gEfiEndOfDxeEventGroupGuid,
> +                  &EndOfDxeEvent);
> +  ASSERT_EFI_ERROR (Status);
> +
> +  return InstallHiiPages ();
> +}
> diff --git a/Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsDxe.h b/Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsDxe.h
> new file mode 100644
> index 000000000000..e94a115a5929
> --- /dev/null
> +++ b/Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsDxe.h
> @@ -0,0 +1,32 @@
> +/** @file
> +
> +  Copyright (c) 2018, Linaro 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.
> +
> +**/
> +
> +#ifndef __NINETY_SIX_BOARDS_DXE_H__
> +#define __NINETY_SIX_BOARDS_DXE_H__
> +
> +#include <Guid/HiiPlatformSetupFormset.h>
> +#include <Guid/FormSet.h>
> +
> +#define MEZZANINE_NONE                            0x0
> +#define MEZZANINE_SECURE96                        0x1
> +#define MEZZANINE_MAX                             0x1

Would this be simpler as an enum with a permanent final element called
MaxMezzanineType? This seems like the common EDK2 (and even UEFI)
pattern.

> +
> +#define NINETY_SIX_BOARDS_CONFIG_VARIABLE_NAME    L"NinetySixBoardsConfig"
> +
> +typedef struct {
> +  UINT8         MezzanineType;
> +  UINT8         Reserved[7];
> +} NINETY_SIX_BOARDS_CONFIG_DATA;
> +
> +#endif // __NINETY_SIX_BOARDS_DXE_H__
> diff --git a/Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsDxe.inf b/Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsDxe.inf
> new file mode 100644
> index 000000000000..5c6f863e8db4
> --- /dev/null
> +++ b/Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsDxe.inf
> @@ -0,0 +1,57 @@
> +## @file
> +#
> +#  Copyright (c) 2018, Linaro, Ltd. 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.
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION               = 0x0001001A
> +  BASE_NAME                 = NinetySixBoardsDxe
> +  FILE_GUID                 = 3f68e889-cb77-4efc-bc84-afa0a64ad26e
> +  MODULE_TYPE               = DXE_DRIVER
> +  VERSION_STRING            = 1.0
> +  ENTRY_POINT               = EntryPoint
> +
> +[Sources]
> +  NinetySixBoardsDxe.c
> +  NinetySixBoardsDxe.h
> +  NinetySixBoardsHii.vfr
> +  NinetySixBoardsHii.uni
> +
> +[Packages]
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  Platform/NinetySixBoards/NinetySixBoards.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  BaseMemoryLib
> +  DebugLib
> +  HiiLib
> +  UefiBootServicesTableLib
> +  UefiDriverEntryPoint
> +  UefiLib
> +  UefiRuntimeServicesTableLib
> +
> +[Protocols]
> +  gNinetySixBoardsLsConnectorProtocolGuid     ## PRODUCES
> +  gNinetySixBoardsMezzanineProtocolGuid       ## CONSUMES
> +
> +[Guids]
> +  gEfiEndOfDxeEventGroupGuid
> +  gFdtTableGuid
> +  gNinetySixBoardsFormsetGuid
> +
> +[Depex]
> +  gEfiVariableArchProtocolGuid        AND
> +  gEfiVariableWriteArchProtocolGuid
> diff --git a/Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsHii.uni b/Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsHii.uni
> new file mode 100644
> index 000000000000..23f4c5859207
> --- /dev/null
> +++ b/Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsHii.uni
> @@ -0,0 +1,27 @@
> +/** @file
> +
> +  Copyright (c) 2018, Linaro, Ltd. 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.
> +
> +**/
> +
> +#langdef en-US  "English"
> +
> +#string STR_FORM_SET_TITLE             #language en-US "96boards Mezzanine options"
> +#string STR_FORM_SET_TITLE_HELP        #language en-US "Configure the installed 96boards mezzanine"
> +
> +#string STR_MAIN_FORM_TITLE            #language en-US "96boards Mezzanine options"
> +#string STR_NULL_STRING                #language en-US ""
> +
> +#string STR_MEZZANINE_SELECT_PROMPT    #language en-US "96boards mezzanine"
> +#string STR_MEZZANINE_SELECT_HELP      #language en-US "The type of mezzanine board plugged into the 96boards LS connector"
> +
> +#string STR_MEZZANINE_NONE             #language en-US "None/Unknown"
> +#string STR_MEZZANINE_SECURE96         #language en-US "Secure96"
> diff --git a/Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsHii.vfr b/Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsHii.vfr
> new file mode 100644
> index 000000000000..7e78c9cfa22f
> --- /dev/null
> +++ b/Platform/NinetySixBoards/NinetySixBoardsDxe/NinetySixBoardsHii.vfr
> @@ -0,0 +1,51 @@
> +/** @file
> +
> +  Copyright (c) 2018, Linaro, Ltd. 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 "NinetySixBoardsDxe.h"
> +
> +//
> +// EFI Variable attributes
> +//
> +#define EFI_VARIABLE_NON_VOLATILE       0x00000001
> +#define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x00000002
> +#define EFI_VARIABLE_RUNTIME_ACCESS     0x00000004
> +#define EFI_VARIABLE_READ_ONLY          0x00000008

Hmm, that jogged my memory - I need to follow up on
https://www.mail-archive.com/edk2-devel@lists.01.org/msg34156.html

... and I would appreciate an R-b on
https://www.mail-archive.com/edk2-devel@lists.01.org/msg34154.html

:)

I would prefer if we could get this one in with a
#include <Uefi/UefiMultiPhase.h>
instead.

/
    Leif

> +
> +formset
> +  guid      = NINETY_SIX_BOARDS_FORMSET_GUID,
> +  title     = STRING_TOKEN(STR_FORM_SET_TITLE),
> +  help      = STRING_TOKEN(STR_FORM_SET_TITLE_HELP),
> +  classguid = EFI_HII_PLATFORM_SETUP_FORMSET_GUID,
> +
> +  efivarstore NINETY_SIX_BOARDS_CONFIG_DATA,
> +    attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_NON_VOLATILE,  // EFI variable attributes
> +    name  = NinetySixBoardsConfig,
> +    guid  = NINETY_SIX_BOARDS_FORMSET_GUID;
> +
> +  form formid = 0x1000,
> +    title  = STRING_TOKEN(STR_MAIN_FORM_TITLE);
> +
> +    oneof varid = NinetySixBoardsConfig.MezzanineType,
> +        prompt      = STRING_TOKEN(STR_MEZZANINE_SELECT_PROMPT),
> +        help        = STRING_TOKEN(STR_MEZZANINE_SELECT_HELP),
> +        flags       = NUMERIC_SIZE_1 | INTERACTIVE | RESET_REQUIRED,
> +        option text = STRING_TOKEN(STR_MEZZANINE_NONE), value = MEZZANINE_NONE, flags = DEFAULT;
> +        option text = STRING_TOKEN(STR_MEZZANINE_SECURE96), value = MEZZANINE_SECURE96, flags = 0;
> +    endoneof;
> +
> +    subtitle text = STRING_TOKEN(STR_NULL_STRING);
> +
> +  endform;
> +
> +endformset;
> -- 
> 2.11.0
>