Re: [platforms: PATCH 12/12] Marvell/Armada7k8k: Introduce NonDiscoverable device init routines

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

On Sat, Oct 20, 2018 at 03:57:41AM +0200, Marcin Wojtas wrote:
> To abstract the initialization required for non-discoverable devices, which
> is often platform specific (i.e., enabling VBUS gpios for USB), introduce
> a NonDiscoverableInitLib for use by the NonDiscoverable code, for which
> each platform can supply its own version.
> 
> Add VBUS enabling routines for supported platforms (Armada70x0Db,
> Armada80x0Db, Armada80x0McBin).

Please expand VBUS on first use in commit message.

> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
>  Silicon/Marvell/Marvell.dec                                                         |   1 +
>  Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc                                      |   3 +
>  Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc                                      |   3 +
>  Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.dsc                               |   3 +
>  Platform/Marvell/Armada70x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.inf     |  47 ++++++++
>  Platform/Marvell/Armada80x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.inf     |  48 +++++++++
>  Platform/SolidRun/Armada80x0McBin/NonDiscoverableInitLib/NonDiscoverableInitLib.inf |  48 +++++++++
>  Silicon/Marvell/Drivers/NonDiscoverableDxe/NonDiscoverableDxe.inf                   |   1 +
>  Silicon/Marvell/Include/Library/NonDiscoverableInitLib.h                            |  28 +++++
>  Platform/Marvell/Armada70x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.c       |  99 +++++++++++++++++
>  Platform/Marvell/Armada80x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.c       | 113 ++++++++++++++++++++
>  Platform/SolidRun/Armada80x0McBin/NonDiscoverableInitLib/NonDiscoverableInitLib.c   |  73 +++++++++++++
>  Silicon/Marvell/Drivers/NonDiscoverableDxe/NonDiscoverableDxe.c                     |   7 +-
>  13 files changed, 471 insertions(+), 3 deletions(-)
>  create mode 100644 Platform/Marvell/Armada70x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.inf
>  create mode 100644 Platform/Marvell/Armada80x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.inf
>  create mode 100644 Platform/SolidRun/Armada80x0McBin/NonDiscoverableInitLib/NonDiscoverableInitLib.inf
>  create mode 100644 Silicon/Marvell/Include/Library/NonDiscoverableInitLib.h
>  create mode 100644 Platform/Marvell/Armada70x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.c
>  create mode 100644 Platform/Marvell/Armada80x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.c
>  create mode 100644 Platform/SolidRun/Armada80x0McBin/NonDiscoverableInitLib/NonDiscoverableInitLib.c
> 
> diff --git a/Silicon/Marvell/Marvell.dec b/Silicon/Marvell/Marvell.dec
> index 7e1c37a..20a32ef 100644
> --- a/Silicon/Marvell/Marvell.dec
> +++ b/Silicon/Marvell/Marvell.dec
> @@ -63,6 +63,7 @@
>    ArmadaBoardDescLib|Include/Library/ArmadaBoardDescLib.h
>    ArmadaIcuLib|Include/Library/ArmadaIcuLib.h
>    ArmadaSoCDescLib|Include/Library/ArmadaSoCDescLib.h
> +  NonDiscoverableInitLib|Include/Library/NonDiscoverableInitLib.h
>    SampleAtResetLib|Include/Library/SampleAtResetLib.h
>  
>  [Protocols]
> diff --git a/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc b/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc
> index 31815e4..e8cd177 100644
> --- a/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc
> +++ b/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc
> @@ -48,6 +48,9 @@
>  
>  !include Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
>  
> +[LibraryClasses.common]
> +  NonDiscoverableInitLib|Platform/Marvell/Armada70x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.inf
> +
>  [Components.common]
>    Silicon/Marvell/Armada7k8k/DeviceTree/Armada70x0Db.inf
>  
> diff --git a/Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc b/Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc
> index 42f7bd3..8e8c2ba 100644
> --- a/Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc
> +++ b/Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc
> @@ -48,6 +48,9 @@
>  
>  !include Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
>  
> +[LibraryClasses.common]
> +  NonDiscoverableInitLib|Platform/Marvell/Armada80x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.inf
> +
>  [Components.common]
>    Silicon/Marvell/Armada7k8k/DeviceTree/Armada80x0Db.inf
>  
> diff --git a/Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.dsc b/Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.dsc
> index 077224d..d080136 100644
> --- a/Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.dsc
> +++ b/Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.dsc
> @@ -49,6 +49,9 @@
>  
>  !include Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
>  
> +[LibraryClasses.common]
> +  NonDiscoverableInitLib|Platform/SolidRun/Armada80x0McBin/NonDiscoverableInitLib/NonDiscoverableInitLib.inf
> +
>  [Components.common]
>    Silicon/Marvell/Armada7k8k/DeviceTree/Armada80x0McBin.inf
>  
> diff --git a/Platform/Marvell/Armada70x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.inf b/Platform/Marvell/Armada70x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.inf
> new file mode 100644
> index 0000000..bbe7ff8
> --- /dev/null
> +++ b/Platform/Marvell/Armada70x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.inf
> @@ -0,0 +1,47 @@
> +## @file
> +#
> +#  Copyright (c) 2017, 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                      = Armada70x0DbNonDiscoverableInitLib
> +  FILE_GUID                      = 151b04bc-9195-4380-b1fa-987130b450f0
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = NonDiscoverableInitLib
> +
> +#
> +# The following information is for reference only and not required by the build
> +# tools.
> +#
> +#  VALID_ARCHITECTURES           = AARCH64
> +#
> +
> +[Sources]
> +  NonDiscoverableInitLib.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  Silicon/Marvell/Marvell.dec
> +
> +[LibraryClasses]
> +  DebugLib
> +  IoLib
> +
> +[Protocols]
> +  gMarvellGpioProtocolGuid
> +
> +[Depex]
> +  gMarvellPlatformInitCompleteProtocolGuid
> diff --git a/Platform/Marvell/Armada80x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.inf b/Platform/Marvell/Armada80x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.inf
> new file mode 100644
> index 0000000..00b5ed5
> --- /dev/null
> +++ b/Platform/Marvell/Armada80x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.inf
> @@ -0,0 +1,48 @@
> +## @file
> +#
> +#  Copyright (c) 2017, Linaro Ltd. All rights reserved.<BR>
> +#  Copyright (c) 2018, Marvell International 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                      = Armada80x0DbNonDiscoverableInitLib
> +  FILE_GUID                      = 719c91d5-25aa-4366-988a-1642ae3a6734
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = NonDiscoverableInitLib
> +
> +#
> +# The following information is for reference only and not required by the build
> +# tools.
> +#
> +#  VALID_ARCHITECTURES           = AARCH64
> +#
> +
> +[Sources]
> +  NonDiscoverableInitLib.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  Silicon/Marvell/Marvell.dec
> +
> +[LibraryClasses]
> +  DebugLib
> +  IoLib
> +
> +[Protocols]
> +  gMarvellGpioProtocolGuid
> +
> +[Depex]
> +  gMarvellPlatformInitCompleteProtocolGuid
> diff --git a/Platform/SolidRun/Armada80x0McBin/NonDiscoverableInitLib/NonDiscoverableInitLib.inf b/Platform/SolidRun/Armada80x0McBin/NonDiscoverableInitLib/NonDiscoverableInitLib.inf
> new file mode 100644
> index 0000000..66720d2
> --- /dev/null
> +++ b/Platform/SolidRun/Armada80x0McBin/NonDiscoverableInitLib/NonDiscoverableInitLib.inf
> @@ -0,0 +1,48 @@
> +## @file
> +#
> +#  Copyright (c) 2017, Linaro Ltd. All rights reserved.<BR>
> +#  Copyright (c) 2018, Marvell International 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                      = Armada80x0McBinNonDiscoverableInitLib
> +  FILE_GUID                      = 470963c4-476e-4b85-a7c4-17868177f441
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = NonDiscoverableInitLib
> +
> +#
> +# The following information is for reference only and not required by the build
> +# tools.
> +#
> +#  VALID_ARCHITECTURES           = AARCH64
> +#
> +
> +[Sources]
> +  NonDiscoverableInitLib.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  Silicon/Marvell/Marvell.dec
> +
> +[LibraryClasses]
> +  DebugLib
> +  IoLib
> +
> +[Protocols]
> +  gMarvellGpioProtocolGuid
> +
> +[Depex]
> +  gMarvellPlatformInitCompleteProtocolGuid
> diff --git a/Silicon/Marvell/Drivers/NonDiscoverableDxe/NonDiscoverableDxe.inf b/Silicon/Marvell/Drivers/NonDiscoverableDxe/NonDiscoverableDxe.inf
> index 98e5b0c..3f9f7bc 100644
> --- a/Silicon/Marvell/Drivers/NonDiscoverableDxe/NonDiscoverableDxe.inf
> +++ b/Silicon/Marvell/Drivers/NonDiscoverableDxe/NonDiscoverableDxe.inf
> @@ -50,6 +50,7 @@
>  
>  [LibraryClasses]
>    NonDiscoverableDeviceRegistrationLib
> +  NonDiscoverableInitLib
>    UefiDriverEntryPoint
>  
>  [Protocols]
> diff --git a/Silicon/Marvell/Include/Library/NonDiscoverableInitLib.h b/Silicon/Marvell/Include/Library/NonDiscoverableInitLib.h
> new file mode 100644
> index 0000000..eaffce3
> --- /dev/null
> +++ b/Silicon/Marvell/Include/Library/NonDiscoverableInitLib.h
> @@ -0,0 +1,28 @@
> +/**
> +*
> +*  Copyright (c) 2017, Linaro Ltd. All rights reserved.
> +*  Copyright (c) 2018, Marvell International 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.
> +*
> +**/
> +
> +#ifndef __NON_DISCOVERABLE_INIT_LIB_H__
> +#define __NON_DISCOVERABLE_INIT_LIB_H__
> +
> +#include <Library/NonDiscoverableDeviceRegistrationLib.h>
> +
> +NON_DISCOVERABLE_DEVICE_INIT
> +EFIAPI
> +GetInitializerForType (

This needs some namespacing. Also not that clear from the name what it
does. I'll see if I can think of a better name from looking at point
of call.

> +  IN  NON_DISCOVERABLE_DEVICE_TYPE  Type,
> +  IN  UINTN                         Index
> +  );
> +
> +#endif
> diff --git a/Platform/Marvell/Armada70x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.c b/Platform/Marvell/Armada70x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.c
> new file mode 100644
> index 0000000..a93576e
> --- /dev/null
> +++ b/Platform/Marvell/Armada70x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.c
> @@ -0,0 +1,99 @@
> +/**
> +*
> +*  Copyright (c) 2017, Linaro Ltd. All rights reserved.
> +*  Copyright (c) 2018, Marvell International 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/DebugLib.h>
> +#include <Library/DevicePathLib.h>
> +#include <Library/IoLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/NonDiscoverableDeviceRegistrationLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +
> +#include <Protocol/MvGpio.h>
> +#include <Protocol/NonDiscoverableDevice.h>
> +
> +#define ARMADA_70x0_DB_IO_EXPANDER0       0
> +#define ARMADA_70x0_DB_VBUS0_PIN          0
> +#define ARMADA_70x0_DB_VBUS0_LIMIT_PIN    4
> +#define ARMADA_70x0_DB_VBUS1_PIN          1
> +#define ARMADA_70x0_DB_VBUS1_LIMIT_PIN    5

Please put the #defines in a local .h.

> +
> +STATIC CONST GPIO_PIN_DESC mArmada70x0DbVbusEn[] = {

Doesn't need Armada70x0Db, does need more than VbusEn.

> +  {
> +    ARMADA_70x0_DB_IO_EXPANDER0,
> +    ARMADA_70x0_DB_VBUS0_PIN,
> +    TRUE,
> +  },
> +  {
> +    ARMADA_70x0_DB_IO_EXPANDER0,
> +    ARMADA_70x0_DB_VBUS0_LIMIT_PIN,
> +    TRUE,
> +  },
> +  {
> +    ARMADA_70x0_DB_IO_EXPANDER0,
> +    ARMADA_70x0_DB_VBUS1_PIN,
> +    TRUE,
> +  },
> +  {
> +    ARMADA_70x0_DB_IO_EXPANDER0,
> +    ARMADA_70x0_DB_VBUS1_LIMIT_PIN,
> +    TRUE,
> +  },
> +};
> +
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +Armada70x0DbInitXhciVbus (

STATIC doesn't need Armada70x0Db prefix.

> +  IN  NON_DISCOVERABLE_DEVICE       *This
> +  )
> +{
> +  CONST GPIO_PIN_DESC     *VbusEnPinDesc;
> +  MARVELL_GPIO_PROTOCOL   *GpioProtocol;
> +  EFI_STATUS              Status;
> +  UINTN                   Index;
> +
> +  Status = MvGpioGetProtocol (GPIO_DRIVER_TYPE_PCA95XX, &GpioProtocol);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "%a: Unable to find GPIO protocol\n", __FUNCTION__));
> +    return Status;
> +  }
> +
> +  VbusEnPinDesc = mArmada70x0DbVbusEn;
> +  for (Index = 0; Index < ARRAY_SIZE (mArmada70x0DbVbusEn); Index++) {
> +    GpioProtocol->DirectionOutput (GpioProtocol,
> +                    VbusEnPinDesc->ControllerId,
> +                    VbusEnPinDesc->PinNumber,
> +                    VbusEnPinDesc->ActiveHigh);
> +    VbusEnPinDesc++;
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> +
> +NON_DISCOVERABLE_DEVICE_INIT
> +EFIAPI
> +GetInitializerForType (
> +  IN  NON_DISCOVERABLE_DEVICE_TYPE  Type,
> +  IN  UINTN                         Index
> +  )
> +{
> +  if (Type == NonDiscoverableDeviceTypeXhci) {
> +        return Armada70x0DbInitXhciVbus;
> +  }
> +
> +  return NULL;
> +}
> diff --git a/Platform/Marvell/Armada80x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.c b/Platform/Marvell/Armada80x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.c
> new file mode 100644
> index 0000000..05f6b89
> --- /dev/null
> +++ b/Platform/Marvell/Armada80x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.c
> @@ -0,0 +1,113 @@
> +/**
> +*
> +*  Copyright (c) 2017, Linaro Ltd. All rights reserved.
> +*  Copyright (c) 2018, Marvell International 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/DebugLib.h>
> +#include <Library/DevicePathLib.h>
> +#include <Library/IoLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/NonDiscoverableDeviceRegistrationLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +
> +#include <Protocol/MvGpio.h>
> +#include <Protocol/NonDiscoverableDevice.h>
> +
> +#define ARMADA_80x0_DB_IO_EXPANDER0       0
> +#define ARMADA_80x0_DB_VBUS0_PIN          0
> +#define ARMADA_80x0_DB_VBUS0_LIMIT_PIN    4
> +#define ARMADA_80x0_DB_VBUS1_PIN          1
> +#define ARMADA_80x0_DB_VBUS1_LIMIT_PIN    5
> +
> +#define ARMADA_80x0_DB_IO_EXPANDER1       1
> +#define ARMADA_80x0_DB_VBUS2_PIN          0
> +#define ARMADA_80x0_DB_VBUS2_LIMIT_PIN    4
> +

Local .h?

> +STATIC CONST GPIO_PIN_DESC mArmada80x0DbVbusEn[] = {

Name as above.

> +  {
> +    ARMADA_80x0_DB_IO_EXPANDER0,
> +    ARMADA_80x0_DB_VBUS0_PIN,
> +    TRUE,
> +  },
> +  {
> +    ARMADA_80x0_DB_IO_EXPANDER0,
> +    ARMADA_80x0_DB_VBUS0_LIMIT_PIN,
> +    TRUE,
> +  },
> +  {
> +    ARMADA_80x0_DB_IO_EXPANDER0,
> +    ARMADA_80x0_DB_VBUS1_PIN,
> +    TRUE,
> +  },
> +  {
> +    ARMADA_80x0_DB_IO_EXPANDER0,
> +    ARMADA_80x0_DB_VBUS1_LIMIT_PIN,
> +    TRUE,
> +  },
> +  {
> +    ARMADA_80x0_DB_IO_EXPANDER1,
> +    ARMADA_80x0_DB_VBUS2_PIN,
> +    TRUE,
> +  },
> +  {
> +    ARMADA_80x0_DB_IO_EXPANDER1,
> +    ARMADA_80x0_DB_VBUS2_LIMIT_PIN,
> +    TRUE,
> +  },
> +};
> +
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +Armada80x0DbInitXhciVbus (

STATIC doesn't need Armada80x0Db prefix.

> +  IN  NON_DISCOVERABLE_DEVICE       *This
> +  )
> +{
> +  CONST GPIO_PIN_DESC     *VbusEnPinDesc;
> +  MARVELL_GPIO_PROTOCOL   *GpioProtocol;
> +  EFI_STATUS              Status;
> +  UINTN                   Index;
> +
> +  Status = MvGpioGetProtocol (GPIO_DRIVER_TYPE_PCA95XX, &GpioProtocol);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "%a: Unable to find GPIO protocol\n", __FUNCTION__));
> +    return Status;
> +  }
> +
> +  VbusEnPinDesc = mArmada80x0DbVbusEn;
> +  for (Index = 0; Index < ARRAY_SIZE (mArmada80x0DbVbusEn); Index++) {
> +    GpioProtocol->DirectionOutput (GpioProtocol,
> +                    VbusEnPinDesc->ControllerId,
> +                    VbusEnPinDesc->PinNumber,
> +                    VbusEnPinDesc->ActiveHigh);
> +    VbusEnPinDesc++;
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> +
> +NON_DISCOVERABLE_DEVICE_INIT
> +EFIAPI
> +GetInitializerForType (
> +  IN  NON_DISCOVERABLE_DEVICE_TYPE  Type,
> +  IN  UINTN                         Index
> +  )
> +{
> +  if (Type == NonDiscoverableDeviceTypeXhci) {
> +        return Armada80x0DbInitXhciVbus;
> +  }
> +
> +  return NULL;
> +}
> diff --git a/Platform/SolidRun/Armada80x0McBin/NonDiscoverableInitLib/NonDiscoverableInitLib.c b/Platform/SolidRun/Armada80x0McBin/NonDiscoverableInitLib/NonDiscoverableInitLib.c
> new file mode 100644
> index 0000000..764b3b8
> --- /dev/null
> +++ b/Platform/SolidRun/Armada80x0McBin/NonDiscoverableInitLib/NonDiscoverableInitLib.c
> @@ -0,0 +1,73 @@
> +/**
> +*
> +*  Copyright (c) 2017, Linaro Ltd. All rights reserved.
> +*  Copyright (c) 2018, Marvell International 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/DebugLib.h>
> +#include <Library/DevicePathLib.h>
> +#include <Library/IoLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/NonDiscoverableDeviceRegistrationLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +
> +#include <Protocol/MvGpio.h>
> +#include <Protocol/NonDiscoverableDevice.h>
> +
> +#define ARMADA_80x0_CP0_CONTROLLER1_INDEX   2

Hmm, this doean't look Macchiato-specific. Could it be pulled in from
elsewhere?

> +#define ARMADA_80x0_MCBIN_VBUS0_PIN         15

Local .h?

> +
> +STATIC CONST GPIO_PIN_DESC mArmada80x0McBinVbusEn = {

Name as above.

> +  ARMADA_80x0_CP0_CONTROLLER1_INDEX,
> +  ARMADA_80x0_MCBIN_VBUS0_PIN,
> +  TRUE,
> +};
> +
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +Armada80x0McBinInitXhciVbus (

STATIC as above.

/
    Leif

> +  IN  NON_DISCOVERABLE_DEVICE       *This
> +  )
> +{
> +  MARVELL_GPIO_PROTOCOL   *GpioProtocol;
> +  EFI_STATUS              Status;
> +
> +  Status = MvGpioGetProtocol (GPIO_DRIVER_TYPE_SOC_CONTROLLER, &GpioProtocol);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "%a: Unable to find GPIO protocol\n", __FUNCTION__));
> +    return Status;
> +  }
> +
> +  GpioProtocol->DirectionOutput (GpioProtocol,
> +                  mArmada80x0McBinVbusEn.ControllerId,
> +                  mArmada80x0McBinVbusEn.PinNumber,
> +                  mArmada80x0McBinVbusEn.ActiveHigh);
> +
> +  return Status;
> +}
> +
> +NON_DISCOVERABLE_DEVICE_INIT
> +EFIAPI
> +GetInitializerForType (
> +  IN  NON_DISCOVERABLE_DEVICE_TYPE  Type,
> +  IN  UINTN                         Index
> +  )
> +{
> +  if (Type == NonDiscoverableDeviceTypeXhci) {
> +        return Armada80x0McBinInitXhciVbus;
> +  }
> +
> +  return NULL;
> +}
> diff --git a/Silicon/Marvell/Drivers/NonDiscoverableDxe/NonDiscoverableDxe.c b/Silicon/Marvell/Drivers/NonDiscoverableDxe/NonDiscoverableDxe.c
> index c5cf904..5de9b7b 100644
> --- a/Silicon/Marvell/Drivers/NonDiscoverableDxe/NonDiscoverableDxe.c
> +++ b/Silicon/Marvell/Drivers/NonDiscoverableDxe/NonDiscoverableDxe.c
> @@ -36,6 +36,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>  
>  #include <Library/DebugLib.h>
>  #include <Library/NonDiscoverableDeviceRegistrationLib.h>
> +#include <Library/NonDiscoverableInitLib.h>
>  #include <Library/UefiBootServicesTableLib.h>
>  
>  #include <Protocol/BoardDesc.h>
> @@ -57,7 +58,7 @@ NonDiscoverableInitXhci (
>      Status = RegisterNonDiscoverableMmioDevice (
>                       NonDiscoverableDeviceTypeXhci,
>                       Desc[i].SoC->XhciDmaType,
> -                     NULL,
> +                     GetInitializerForType (NonDiscoverableDeviceTypeXhci, i),
>                       NULL,
>                       1,
>                       Desc[i].SoC->XhciBaseAddress,
> @@ -86,7 +87,7 @@ NonDiscoverableInitAhci (
>      Status = RegisterNonDiscoverableMmioDevice (
>                       NonDiscoverableDeviceTypeAhci,
>                       Desc[i].SoC->AhciDmaType,
> -                     NULL,
> +                     GetInitializerForType (NonDiscoverableDeviceTypeAhci, i),
>                       NULL,
>                       1,
>                       Desc[i].SoC->AhciBaseAddress,
> @@ -115,7 +116,7 @@ NonDiscoverableInitSdhci (
>      Status = RegisterNonDiscoverableMmioDevice (
>                       NonDiscoverableDeviceTypeSdhci,
>                       Desc[i].SoC->SdMmcDmaType,
> -                     NULL,
> +                     GetInitializerForType (NonDiscoverableDeviceTypeSdhci, i),
>                       NULL,
>                       1,
>                       Desc[i].SoC->SdMmcBaseAddress,
> -- 
> 2.7.4
>