Re: [PATCH edk2-platforms v2 5/7] Platform/NinetySixBoards: add a driver for the Secure96 mezzanine board

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

On Tue, Feb 20, 2018 at 05:49:42PM +0000, Ard Biesheuvel wrote:
> Add a driver that describes the Secure96 mezzanine board, and exposes
> both the information required to describe it to the OS using a DT overlay,
> and to describe it to UEFI itself.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  Platform/NinetySixBoards/Secure96Dxe/Secure96.dts    |  76 +++++++
>  Platform/NinetySixBoards/Secure96Dxe/Secure96.h      |  26 +++
>  Platform/NinetySixBoards/Secure96Dxe/Secure96Dxe.c   | 211 ++++++++++++++++++++
>  Platform/NinetySixBoards/Secure96Dxe/Secure96Dxe.inf |  67 +++++++
>  4 files changed, 380 insertions(+)
> 
> diff --git a/Platform/NinetySixBoards/Secure96Dxe/Secure96.dts b/Platform/NinetySixBoards/Secure96Dxe/Secure96.dts
> new file mode 100644
> index 000000000000..08f59978c06f
> --- /dev/null
> +++ b/Platform/NinetySixBoards/Secure96Dxe/Secure96.dts
> @@ -0,0 +1,76 @@
> +/** @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.
> + */
> +
> +#include "Secure96.h"
> +
> +#define GPIO_PARENT_PLACEHOLDER_PHANDLE     0x0

Since this is likely to be used as a template for future drivers
including DT overlays (not just mezzanine), could you over-comment a
bit? Could you say "32-bit value filled in by this driver's helper
function FixupOverlay() when applying overlay" (if that's correct)?

/
    Leif

> +
> +/dts-v1/;
> +/plugin/;
> +
> +/ {
> +    fragment@0 {
> +        target-path = "I2C_PARENT_PLACEHOLDER_STRING";
> +        __overlay__ {
> +            ATSHA204A_DT_NODENAME {
> +                compatible = "atmel,atsha204a";
> +                reg = <ATSHA204A_SLAVE_ADDRESS>;
> +            };
> +            ATECC508A_DT_NODENAME {
> +                compatible = "atmel,atecc508a";
> +                reg = <ATECC508A_SLAVE_ADDRESS>;
> +            };
> +        };
> +    };
> +
> +    fragment@1 {
> +        target-path = "SPI_PARENT_PLACEHOLDER_STRING";
> +        __overlay__ {
> +            INFINEON_SLB9670_DT_NODENAME {
> +                compatible = "infineon,slb9670";
> +                reg = <INFINEON_SLB9670_SPI_CS>;
> +                spi-max-frequency = <22500000>;
> +            };
> +        };
> +    };
> +
> +    fragment@2 {
> +        target-path = "/";
> +        __overlay__ {
> +            gpio-leds {
> +                compatible = "gpio-leds";
> +
> +                secure96-u1 {
> +                    gpios = <GPIO_PARENT_PLACEHOLDER_PHANDLE
> +                             FixedPcdGet32 (PcdGpioPinG)
> +                             FixedPcdGet32 (PcdGpioPolarity)>;
> +                };
> +                secure96-u2 {
> +                    gpios = <GPIO_PARENT_PLACEHOLDER_PHANDLE
> +                             FixedPcdGet32 (PcdGpioPinF)
> +                             FixedPcdGet32 (PcdGpioPolarity)>;
> +                };
> +                secure96-u3 {
> +                    gpios = <GPIO_PARENT_PLACEHOLDER_PHANDLE
> +                             FixedPcdGet32 (PcdGpioPinI)
> +                             FixedPcdGet32 (PcdGpioPolarity)>;
> +                };
> +                secure96-u4 {
> +                    gpios = <GPIO_PARENT_PLACEHOLDER_PHANDLE
> +                             FixedPcdGet32 (PcdGpioPinH)
> +                             FixedPcdGet32 (PcdGpioPolarity)>;
> +                };
> +            };
> +        };
> +    };
> +};
> diff --git a/Platform/NinetySixBoards/Secure96Dxe/Secure96.h b/Platform/NinetySixBoards/Secure96Dxe/Secure96.h
> new file mode 100644
> index 000000000000..84b8aed13d1e
> --- /dev/null
> +++ b/Platform/NinetySixBoards/Secure96Dxe/Secure96.h
> @@ -0,0 +1,26 @@
> +/** @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.
> +**/
> +
> +#ifndef _SECURE96_H_
> +#define _SECURE96_H_
> +
> +#define ATSHA204A_SLAVE_ADDRESS         0x60
> +#define ATSHA204A_DT_NODENAME           atsha204a@60
> +
> +#define ATECC508A_SLAVE_ADDRESS         0x51
> +#define ATECC508A_DT_NODENAME           atecc508a@51
> +
> +#define INFINEON_SLB9670_SPI_CS         0x0
> +#define INFINEON_SLB9670_DT_NODENAME    tpm@0
> +
> +#endif // _SECURE96_H_
> diff --git a/Platform/NinetySixBoards/Secure96Dxe/Secure96Dxe.c b/Platform/NinetySixBoards/Secure96Dxe/Secure96Dxe.c
> new file mode 100644
> index 000000000000..000f4b4abba4
> --- /dev/null
> +++ b/Platform/NinetySixBoards/Secure96Dxe/Secure96Dxe.c
> @@ -0,0 +1,211 @@
> +/** @file
> +  96boards Secure96 mezzanine board DXE driver.
> +
> +  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.
> +**/
> +
> +#include <PiDxe.h>
> +#include <libfdt.h>
> +#include <Library/BaseLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/DxeServicesLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiLib.h>
> +#include <Protocol/LsConnector.h>
> +#include <Protocol/Mezzanine.h>
> +
> +#include "Secure96.h"
> +
> +STATIC CONST UINT32 mI2cAtmelSha204aSlaveAddress[] = {
> +  ATSHA204A_SLAVE_ADDRESS,
> +
> +  //
> +  // The Atmel AtSha204a has an annoying 'wake' mode where it will only wake
> +  // up if SDA is held low for a certain amount of time. Attempting to access
> +  // a device at address 0x0 at 100 kHz should be sufficient to create this
> +  // wake condition, so add address 0x0 to the slave addresses.
> +  //
> +  0
> +};
> +
> +STATIC CONST EFI_I2C_DEVICE mI2c0Devices[] = {
> +  {
> +    &gAtSha204aI2cDeviceGuid,                     // DeviceGuid
> +    0,                                            // DeviceIndex
> +    0,                                            // HardwareRevision
> +    0,                                            // I2C bus configuration
> +    ARRAY_SIZE (mI2cAtmelSha204aSlaveAddress),    // SlaveAddressCount
> +    mI2cAtmelSha204aSlaveAddress                  // SlaveAddressArray
> +  }
> +};
> +
> +STATIC CONST CHAR8 mLedNodes[][46] = {
> +  "/fragment@2/__overlay__/gpio-leds/secure96-u1",
> +  "/fragment@2/__overlay__/gpio-leds/secure96-u2",
> +  "/fragment@2/__overlay__/gpio-leds/secure96-u3",
> +  "/fragment@2/__overlay__/gpio-leds/secure96-u4",
> +};
> +
> +STATIC
> +VOID
> +SetOverlayFragmentTarget (
> +  VOID            *Overlay,
> +  CONST CHAR8     *NodeName,
> +  CONST CHAR8     *Target
> +  )
> +{
> +  INT32       Node;
> +  INT32       Err;
> +
> +  Node = fdt_path_offset (Overlay, NodeName);
> +  ASSERT (Node > 0);
> +
> +  Err = fdt_setprop (Overlay, Node, "target-path", Target,
> +          AsciiStrLen (Target) + 1);
> +  if (Err) {
> +    DEBUG ((DEBUG_ERROR, "%a: fdt_setprop() failed - %a\n",
> +      __FUNCTION__, fdt_strerror (Err)));
> +  }
> +}
> +
> +STATIC
> +VOID
> +FixupOverlay (
> +  VOID        *Dtb,
> +  VOID        *Overlay
> +  )
> +{
> +  INT32       Node;
> +  UINT32      GpioPhandle;
> +  UINTN       Idx;
> +  UINT32      *GpioProp;
> +  INT32       Err;
> +
> +  //
> +  // Set the correct GPIO phandle in the LED nodes
> +  //
> +  Node = fdt_path_offset (Dtb, FixedPcdGetPtr (PcdGpioParent));
> +  ASSERT (Node > 0);
> +
> +  GpioPhandle = fdt_get_phandle (Dtb, Node);
> +  if (!GpioPhandle) {
> +    //
> +    // Node has no phandle yet -> create one
> +    //
> +    GpioPhandle = 1 + fdt_get_max_phandle (Dtb);
> +    ASSERT (GpioPhandle >= 1);
> +
> +    Err = fdt_setprop_u32 (Dtb, Node, "phandle", GpioPhandle);
> +    if (Err) {
> +      DEBUG ((DEBUG_ERROR,
> +        "%a: fdt_setprop_u32(.., .., \"phandle\", 0x%x) failed - %a\n",
> +        __FUNCTION__, GpioPhandle, fdt_strerror (Err)));
> +    }
> +  }
> +
> +  for (Idx = 0; Idx < ARRAY_SIZE (mLedNodes); Idx++) {
> +    Node = fdt_path_offset (Overlay, mLedNodes[Idx]);
> +    ASSERT (Node > 0);
> +
> +    GpioProp = fdt_getprop_w (Overlay, Node, "gpios", NULL);
> +    ASSERT (GpioProp != NULL);
> +
> +    *GpioProp = cpu_to_fdt32 (GpioPhandle);
> +  }
> +
> +  SetOverlayFragmentTarget (Overlay, "/fragment@0",
> +    FixedPcdGetPtr (PcdI2c0Parent));
> +
> +  SetOverlayFragmentTarget (Overlay, "/fragment@1",
> +    FixedPcdGetPtr (PcdSpiParent));
> +}
> +
> +/**
> +  Apply the mezzanine's DT overlay
> +
> +  @param[in]      This      Pointer to the MEZZANINE_PROTOCOL instance.
> +  @param[in,out]  Dtb       Pointer to the device tree blob
> +
> +  @return   EFI_SUCCESS     Operation succeeded.
> +  @return   other           An error has occurred.
> +**/
> +STATIC
> +EFI_STATUS
> +ApplyDeviceTreeOverlay (
> +  IN      MEZZANINE_PROTOCOL    *This,
> +  IN  OUT VOID                  *Dtb
> +  )
> +{
> +  VOID            *Overlay;
> +  UINTN           OverlaySize;
> +  EFI_STATUS      Status;
> +  INT32           Err;
> +
> +  //
> +  // Load the raw overlay DTB image from the raw section of this FFS file.
> +  //
> +  Status = GetSectionFromFv (&gEfiCallerIdGuid,
> +             EFI_SECTION_RAW, 0, &Overlay, &OverlaySize);
> +  ASSERT_EFI_ERROR (Status);
> +  if (EFI_ERROR (Status)) {
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  //
> +  // Fix up unresolved references in the overlay.
> +  //
> +  FixupOverlay (Dtb, Overlay);
> +
> +  //
> +  // Merge the overlay with the DTB
> +  //
> +  Err = fdt_overlay_apply (Dtb, Overlay);
> +  if (Err) {
> +    DEBUG ((DEBUG_ERROR, "%a: fdt_overlay_apply() failed - %a\n",
> +      __FUNCTION__, fdt_strerror (Err)));
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> +
> +STATIC MEZZANINE_PROTOCOL mMezzanine = {
> +  ApplyDeviceTreeOverlay,
> +  ARRAY_SIZE (mI2c0Devices),
> +  0,
> +  mI2c0Devices,
> +  NULL,
> +  NULL,
> +};
> +
> +EFI_STATUS
> +EFIAPI
> +Secure96DxeEntryPoint (
> +  IN EFI_HANDLE         ImageHandle,
> +  IN EFI_SYSTEM_TABLE   *SystemTable
> +  )
> +{
> +  EFI_STATUS              Status;
> +  LS_CONNECTOR_PROTOCOL   *LsConnector;
> +
> +  Status = gBS->LocateProtocol (&gNinetySixBoardsLsConnectorProtocolGuid, NULL,
> +                  (VOID **)&LsConnector);
> +  ASSERT_EFI_ERROR (Status);
> +
> +  if (LsConnector->MezzanineType != MezzanineSecure96) {
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  return gBS->InstallProtocolInterface (&ImageHandle,
> +                &gNinetySixBoardsMezzanineProtocolGuid,
> +                EFI_NATIVE_INTERFACE,
> +                &mMezzanine);
> +}
> diff --git a/Platform/NinetySixBoards/Secure96Dxe/Secure96Dxe.inf b/Platform/NinetySixBoards/Secure96Dxe/Secure96Dxe.inf
> new file mode 100644
> index 000000000000..e5686b9f5382
> --- /dev/null
> +++ b/Platform/NinetySixBoards/Secure96Dxe/Secure96Dxe.inf
> @@ -0,0 +1,67 @@
> +## @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                      = Secure96Dxe
> +  FILE_GUID                      = 31519ec4-65f1-4790-b223-aa9330dd75fd
> +  MODULE_TYPE                    = DXE_DRIVER
> +  VERSION_STRING                 = 1.0
> +  ENTRY_POINT                    = Secure96DxeEntryPoint
> +
> +[Sources]
> +  Secure96.dts
> +  Secure96.h
> +  Secure96Dxe.c
> +
> +[Packages]
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +  Platform/NinetySixBoards/NinetySixBoards.dec
> +  Silicon/Atmel/AtSha204a/AtSha204a.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  DebugLib
> +  DxeServicesLib
> +  FdtLib
> +  UefiBootServicesTableLib
> +  UefiDriverEntryPoint
> +  UefiLib
> +
> +[Protocols]
> +  gNinetySixBoardsLsConnectorProtocolGuid            ## CONSUMES
> +  gNinetySixBoardsMezzanineProtocolGuid              ## PRODUCES
> +
> +[Guids]
> +  gAtSha204aI2cDeviceGuid
> +  gFdtTableGuid
> +
> +[FixedPcd]
> +  gNinetySixBoardsTokenSpaceGuid.PcdGpioParent
> +  gNinetySixBoardsTokenSpaceGuid.PcdGpioPinF
> +  gNinetySixBoardsTokenSpaceGuid.PcdGpioPinG
> +  gNinetySixBoardsTokenSpaceGuid.PcdGpioPinH
> +  gNinetySixBoardsTokenSpaceGuid.PcdGpioPinI
> +  gNinetySixBoardsTokenSpaceGuid.PcdGpioPolarity
> +  gNinetySixBoardsTokenSpaceGuid.PcdI2c0Parent
> +  gNinetySixBoardsTokenSpaceGuid.PcdSpiParent
> +
> +[Depex]
> +  gNinetySixBoardsLsConnectorProtocolGuid
> +
> +[BuildOptions]
> +  # dtc emits lots of spurious warnings for overlays
> +  *_*_*_DTC_FLAGS = -q
> -- 
> 2.11.0
>