[Linaro-uefi,1/2] Drivers/Xhci: implement firmware download driver for Renesas PD720202

Message ID 1472555890-29179-2-git-send-email-ard.biesheuvel@linaro.org
State Superseded
Headers show

Commit Message

Ard Biesheuvel Aug. 30, 2016, 11:18 a.m.
This implements a PCI driver for the Renesas XHCI PD720202 PCIe
controller, which in some cases requires a device firmware to be
uploaded by the system firmware to function properly.

By registering a UEFI driver with high dispatch priority, this driver
should be able to access the device early, install the firmware, and
then decline the driver dispatch so that the device should be fully
functional when the real XHCI driver gets to see it.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Drivers/Xhci/RenesasFirmwarePD720202/RenesasFirmwarePD720202.c   | 365 ++++++++++++++++++++
 Drivers/Xhci/RenesasFirmwarePD720202/RenesasFirmwarePD720202.inf |  43 +++
 OpenPlatformPkg.dec                                              |   3 +
 3 files changed, 411 insertions(+)

Comments

Leif Lindholm Aug. 30, 2016, 1:32 p.m. | #1
On Tue, Aug 30, 2016 at 12:18:09PM +0100, Ard Biesheuvel wrote:
> This implements a PCI driver for the Renesas XHCI PD720202 PCIe
> controller, which in some cases requires a device firmware to be
> uploaded by the system firmware to function properly.
> 
> By registering a UEFI driver with high dispatch priority, this driver
> should be able to access the device early, install the firmware, and

Do change the "should be" after we've tested it :)

> then decline the driver dispatch so that the device should be fully
> functional when the real XHCI driver gets to see it.

Good stuff!
Minor comments below.

> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  Drivers/Xhci/RenesasFirmwarePD720202/RenesasFirmwarePD720202.c   | 365 ++++++++++++++++++++
>  Drivers/Xhci/RenesasFirmwarePD720202/RenesasFirmwarePD720202.inf |  43 +++
>  OpenPlatformPkg.dec                                              |   3 +
>  3 files changed, 411 insertions(+)
> 
> diff --git a/Drivers/Xhci/RenesasFirmwarePD720202/RenesasFirmwarePD720202.c b/Drivers/Xhci/RenesasFirmwarePD720202/RenesasFirmwarePD720202.c
> new file mode 100644
> index 000000000000..e1ff5d12c9bf
> --- /dev/null
> +++ b/Drivers/Xhci/RenesasFirmwarePD720202/RenesasFirmwarePD720202.c
> @@ -0,0 +1,365 @@
> +/**  @file
> +  Implementation of driver entry point and driver binding protocol.
> +
> +Copyright (c) 2016, 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 <PiDxe.h>
> +
> +#include <Library/DebugLib.h>
> +#include <Library/DxeServicesLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiLib.h>
> +
> +#include <IndustryStandard/Pci22.h>
> +
> +#include <Protocol/PciIo.h>
> +
> +#define PCI_VENDOR_ID_RENESAS               0x1912
> +#define PCI_DEVICE_ID_PD720202              0x15
> +
> +#define PCI_FW_CTL_STAT_REG                 0xF4
> +#define PCI_FW_CTL_DATA_REG                 0xF5
> +#define PCI_FW_DATA0                        0xF8
> +#define PCI_FW_DATA1                        0xFC
> +
> +#define PCI_FW_CTL_STAT_REG_FW_DL_ENABLE    (1U << 0)
> +#define PCI_FW_CTL_STAT_REG_RESULT_CODE     (7U << 4)
> +#define PCI_FW_CTL_STAT_REG_RESULT_OK       (1U << 4)
> +#define PCI_FW_CTL_STAT_REG_SET_DATA0       (1U << 0)
> +#define PCI_FW_CTL_STAT_REG_SET_DATA1       (1U << 1)
> +
> +STATIC CONST UINT32   *mFirmwareImage;
> +STATIC UINTN          mFirmwareImageSize;
> +
> +STATIC
> +UINT8
> +ReadCtlStatVal (
> +  IN EFI_PCI_IO_PROTOCOL     *PciIo,
> +  IN UINTN                   Offset
> +  )
> +{
> +  UINT8       CtlStatVal;
> +  EFI_STATUS  Status;
> +
> +  Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint8, Offset, 1, &CtlStatVal);
> +  ASSERT_EFI_ERROR (Status);
> +
> +  return CtlStatVal;
> +}
> +
> +STATIC
> +VOID
> +WriteCtlStatVal (
> +  IN EFI_PCI_IO_PROTOCOL     *PciIo,
> +  IN UINTN                   Offset,
> +  IN UINT8                   CtlStatVal
> +  )
> +{
> +  EFI_STATUS  Status;
> +
> +  Status = PciIo->Pci.Write (PciIo, EfiPciIoWidthUint8, Offset, 1, &CtlStatVal);
> +  ASSERT_EFI_ERROR (Status);
> +}
> +
> +STATIC
> +VOID
> +WriteDataVal (
> +  IN EFI_PCI_IO_PROTOCOL     *PciIo,
> +  IN UINTN                   Offset,
> +  IN UINT32                  DataVal
> +  )
> +{
> +  EFI_STATUS  Status;
> +
> +  Status = PciIo->Pci.Write (PciIo, EfiPciIoWidthUint32, Offset, 1, &DataVal);
> +  ASSERT_EFI_ERROR (Status);
> +}
> +
> +STATIC
> +BOOLEAN
> +WaitReadCtlStatVal (
> +  IN EFI_PCI_IO_PROTOCOL     *PciIo,
> +  IN UINTN                   Offset,
> +  IN UINT8                   Mask,
> +  IN UINT8                   Val
> +  )
> +{
> +  UINTN Timeout;
> +
> +  for (Timeout = 0; (ReadCtlStatVal (PciIo, Offset) & Mask) != Val; Timeout++) {
> +    if (Timeout > 1000) {
> +      DEBUG ((EFI_D_ERROR,
> +        "%a: Timeout waiting for reg [+0x%x] & 0x%x to become 0x%x\n",
> +        __FUNCTION__, Offset, Mask, Val));
> +      return FALSE;
> +    }
> +    gBS->Stall (10);
> +  }
> +  return TRUE;
> +}
> +
> +STATIC
> +VOID
> +DownloadPD720202Firmware (
> +  IN EFI_PCI_IO_PROTOCOL     *PciIo
> +  )
> +{
> +  UINTN   Idx;
> +
> +  Idx = 0;
> +
> +  // 1. Set "FW Download Enable" to '1b'.
> +  WriteCtlStatVal (PciIo, PCI_FW_CTL_STAT_REG,
> +    PCI_FW_CTL_STAT_REG_FW_DL_ENABLE);
> +
> +  // 2. Read "Set DATA0" and confirm it is '0b'.
> +  if (!WaitReadCtlStatVal (PciIo, PCI_FW_CTL_DATA_REG,
> +         PCI_FW_CTL_STAT_REG_SET_DATA0, 0)) {
> +    return;
> +  }
> +
> +  // 3. Write FW data to "DATA0".
> +  WriteDataVal (PciIo, PCI_FW_DATA0, mFirmwareImage[Idx++]);
> +
> +  // 4. Read "Set DATA1" and confirm it is '0b'.
> +  if (!WaitReadCtlStatVal (PciIo, PCI_FW_CTL_DATA_REG,
> +         PCI_FW_CTL_STAT_REG_SET_DATA1, 0)) {
> +    return;
> +  }
> +
> +  // 5. Write FW data to "DATA1".
> +  WriteDataVal (PciIo, PCI_FW_DATA1, mFirmwareImage[Idx++]);
> +
> +  // 6. Set "Set DATA0" & "Set DATA1" to '1b'.
> +  WriteCtlStatVal (PciIo, PCI_FW_CTL_DATA_REG,
> +    PCI_FW_CTL_STAT_REG_SET_DATA0 | PCI_FW_CTL_STAT_REG_SET_DATA1);
> +
> +  while (Idx < mFirmwareImageSize / sizeof(UINT32)) {
> +
> +    // 7. Read "Set DATA0" and confirm it is '0b'.
> +    if (!WaitReadCtlStatVal (PciIo, PCI_FW_CTL_DATA_REG,
> +           PCI_FW_CTL_STAT_REG_SET_DATA0, 0)) {
> +      return;
> +    }
> +
> +    // 8. Write FW data to"DATA0". Set "Set DATA0" to '1b'.
> +    WriteDataVal (PciIo, PCI_FW_DATA0, mFirmwareImage[Idx++]);
> +    WriteCtlStatVal (PciIo, PCI_FW_CTL_DATA_REG, PCI_FW_CTL_STAT_REG_SET_DATA0);
> +
> +    // 9. Read "Set DATA1" and confirm it is '0b'.
> +    if (!WaitReadCtlStatVal (PciIo, PCI_FW_CTL_DATA_REG,
> +           PCI_FW_CTL_STAT_REG_SET_DATA1, 0)) {
> +      return;
> +    }
> +
> +    // 10. Write FW data to"DATA1". Set "Set DATA1" to '1b'.
> +    WriteDataVal (PciIo, PCI_FW_DATA1, mFirmwareImage[Idx++]);
> +    WriteCtlStatVal (PciIo, PCI_FW_CTL_DATA_REG, PCI_FW_CTL_STAT_REG_SET_DATA1);
> +
> +    // 11. Return to step 7 and repeat the sequence from step 7 to step 10.
> +  }
> +
> +  // 12. After writing the last data of FW, the System Software must set "FW Download Enable" to '0b'.
> +  WriteCtlStatVal (PciIo, PCI_FW_CTL_STAT_REG, 0);
> +
> +  // 13. Read "Result Code" and confirm it is '001b'. 
> +  if (WaitReadCtlStatVal (PciIo, PCI_FW_CTL_STAT_REG,
> +        PCI_FW_CTL_STAT_REG_RESULT_CODE, PCI_FW_CTL_STAT_REG_RESULT_OK)) {
> +    DEBUG ((EFI_D_INFO, "%a: Renesas PD720202 firmware download successful\n",
> +      __FUNCTION__));
> +  } else {
> +    DEBUG ((EFI_D_ERROR, "%a: Renesas PD720202 firmware download FAILED\n",
> +      __FUNCTION__));
> +  }
> +}
> +
> +/**
> +  Test to see if this driver supports ControllerHandle. This service
> +  is called by the EFI boot service ConnectController(). In
> +  order to make drivers as small as possible, there are a few calling
> +  restrictions for this service. ConnectController() must
> +  follow these calling restrictions. If any other agent wishes to call
> +  Supported() it must also follow these calling restrictions.
> +
> +   @param  This                   Protocol instance pointer.
> +   @param  ControllerHandle       Handle of device to test.
> +   @param  RemainingDevicePath    Optional parameter use to pick a specific child
> +                                                         device to start.
> +
> +   @retval EFI_SUCCESS            This driver supports this device.
> +   @retval EFI_ALREADY_STARTED    This driver is already running on this device.
> +   @retval other                  This driver does not support this device.
> +
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +RenesasPD720202DriverSupported (
> +    IN EFI_DRIVER_BINDING_PROTOCOL    *This,
> +    IN EFI_HANDLE                     Controller,
> +    IN EFI_DEVICE_PATH_PROTOCOL       *RemainingDevicePath
> +    )
> +{
> +  EFI_STATUS              Status;
> +  EFI_PCI_IO_PROTOCOL     *PciIo;
> +  UINT32                  PciID;
> +  UINT8                   CtlStatVal;
> +
> +  //
> +  // Check for the PCI IO Protocol
> +  //
> +  Status = gBS->OpenProtocol (Controller, &gEfiPciIoProtocolGuid,
> +                  (VOID **)&PciIo, This->DriverBindingHandle, Controller,
> +                  EFI_OPEN_PROTOCOL_BY_DRIVER);
> +
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint32, PCI_VENDOR_ID_OFFSET,
> +                        1, &PciID);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((EFI_D_ERROR,
> +      "%a: Pci->Pci.Read() of vendor/device id failed (Status == %r)\n",
> +      __FUNCTION__, Status));
> +    goto CloseProtocol;
> +  }
> +
> +  if ((PciID & 0xffff) != PCI_VENDOR_ID_RENESAS ||
> +      (PciID >> 16) != PCI_DEVICE_ID_PD720202) {
> +    DEBUG ((EFI_D_INFO, "%a: ignoring unsupported PCI device\n", __FUNCTION__));
> +    goto CloseProtocol;
> +  }
> +
> +  //
> +  // We have found a device of the type we are looking for. Check if the
> +  // firmware needs to be loaded.
> +  //
> +  Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint8, PCI_FW_CTL_STAT_REG,
> +                        1, &CtlStatVal);
> +  if (!EFI_ERROR (Status) &&
> +      (CtlStatVal & PCI_FW_CTL_STAT_REG_RESULT_CODE) == 0) {

Could we have a RESULT_INVALID define?

> +    //
> +    // Firmware download required
> +    //
> +    DEBUG ((EFI_D_INFO, "%a: downloading firmware\n", __FUNCTION__));
> +    DownloadPD720202Firmware (PciIo);
> +  }
> +
> +CloseProtocol:
> +  gBS->CloseProtocol (Controller, &gEfiPciIoProtocolGuid,
> +         This->DriverBindingHandle, Controller);
> +
> +  //
> +  // Always return unsupported: we are not interested in driving the device,
> +  // only in having the opportunity to install the firmware before the real
> +  // driver attaches to it.
> +  //
> +  return EFI_UNSUPPORTED;
> +}
> +
> +/**
> +  Start this driver on Controller by opening PciIo and DevicePath protocols.
> +  Initialize PXE structures, create a copy of the Controller Device Path with the
> +  NIC's MAC address appended to it, install the NetworkInterfaceIdentifier protocol

Ah, I see what you did there. Be nice to adjust the comment before
commit though.

> +  on the newly created Device Path.
> +
> +  @param [in] This                    Protocol instance pointer.
> +  @param [in] Controller              Handle of device to work with.
> +  @param [in] RemainingDevicePath     Not used, always produce all possible children.
> +
> +  @retval EFI_SUCCESS                 This driver is added to Controller.
> +  @retval other                       This driver does not support this device.
> +
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +RenesasPD720202DriverStart (
> +    IN EFI_DRIVER_BINDING_PROTOCOL  *This,
> +    IN EFI_HANDLE                   Controller,
> +    IN EFI_DEVICE_PATH_PROTOCOL     *RemainingDevicePath
> +    )
> +{
> +  //
> +  // We are not interested in driving the device, we only poke the firmware
> +  // in the .Supported() callback.
> +  //
> +  ASSERT (FALSE);
> +  return EFI_INVALID_PARAMETER;
> +}
> +
> +/**
> +  Stop this driver on Controller
> +
> +  @param [in] This                    Protocol instance pointer.
> +  @param [in] Controller              Handle of device to stop driver on.
> +  @param [in] NumberOfChildren        How many children need to be stopped.
> +  @param [in] ChildHandleBuffer       Not used.
> +
> +  @retval EFI_SUCCESS                 This driver is removed Controller.
> +  @retval EFI_DEVICE_ERROR            The device could not be stopped due to a device error.
> +  @retval other                       This driver was not removed from this device.
> +
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +RenesasPD720202DriverStop (
> +    IN  EFI_DRIVER_BINDING_PROTOCOL *This,
> +    IN  EFI_HANDLE                  Controller,
> +    IN  UINTN                       NumberOfChildren,
> +    IN  EFI_HANDLE                  *ChildHandleBuffer
> +    )
> +{

So, I was going to point out that since we never Start the driver, it
is invalid to call Stop (and we should put a comment to that effct
in). However, I'm not convinced EFI_INVALID_PARAMETER is a valid thing
to return here: "If ControllerHandle is stopped, then EFI_SUCCESS is
returned."

(We may still want to assert, in order to find funny stuff going on.

> +  ASSERT (FALSE);
> +  return EFI_INVALID_PARAMETER;
> +}
> +
> +//
> +// Simple Network Protocol Driver Global Variables

*cough*

> +//
> +STATIC EFI_DRIVER_BINDING_PROTOCOL RenesasPD720202DriverBinding = {
> +  RenesasPD720202DriverSupported,
> +  RenesasPD720202DriverStart,
> +  RenesasPD720202DriverStop,
> +  0xfffffff0,

Would it be worth adding a define for this one?
If not, a comment pointing out the party trick might be in order:
// Version values of 0xfffffff0-0xffffffff are reserved for
// platform/OEM specific drivers. Protocol instances with higher
// 'Version' drivers will be used before lower 'Version' ones.
// XhciDxe uses version 0x30, so this driver will be called in
// preference, and XhciDxe will be invoked after
// RenesasPD720202DriverSupported returns EFI_UNSUPPORTED.

> +  NULL,
> +  NULL
> +};
> +
> +EFI_STATUS
> +EFIAPI
> +InitializeRenesasPD720202Driver (
> +    IN EFI_HANDLE       ImageHandle,
> +    IN EFI_SYSTEM_TABLE *SystemTable
> +    )
> +{
> +  EFI_STATUS    Status;
> +
> +  //
> +  // First, try to locate the firmware image. If it is missing, there is no
> +  // point in proceeding.
> +  //
> +  Status = GetSectionFromAnyFv (&gRenesasFirmwarePD720202ImageId,
> +    EFI_SECTION_RAW, 0, (VOID **) (VOID **)&mFirmwareImage,
> +    &mFirmwareImageSize);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((EFI_D_ERROR, "%a: could not locate PD720202 firmware image\n",
> +      __FUNCTION__));
> +    return Status;
> +  }
> +
> +  return EfiLibInstallDriverBinding (ImageHandle, SystemTable,
> +        &RenesasPD720202DriverBinding, NULL);
> +}
> diff --git a/Drivers/Xhci/RenesasFirmwarePD720202/RenesasFirmwarePD720202.inf b/Drivers/Xhci/RenesasFirmwarePD720202/RenesasFirmwarePD720202.inf
> new file mode 100644
> index 000000000000..f999bf7b1e60
> --- /dev/null
> +++ b/Drivers/Xhci/RenesasFirmwarePD720202/RenesasFirmwarePD720202.inf
> @@ -0,0 +1,43 @@
> +##  <at> file
> +# Component description file for Reneses PD720202 firmware download driver
> +#
> +# Copyright (c) 2016, 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                    = 0x00010005
> +  BASE_NAME                      = RenesasFirmwarePD720202
> +  FILE_GUID                      = 5979ebfe-d53c-4150-a972-568231583969
> +  MODULE_TYPE                    = UEFI_DRIVER
> +  VERSION_STRING                 = 1.0
> +  ENTRY_POINT                    = InitializeRenesasPD720202Driver
> +
> +[Sources]
> +  RenesasFirmwarePD720202.c
> +
> +[Packages]
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +  OpenPlatformPkg/OpenPlatformPkg.dec
> +
> +[LibraryClasses]
> +  DebugLib
> +  DxeServicesLib
> +  UefiBootServicesTableLib
> +  UefiLib
> +  UefiDriverEntryPoint
> +
> +[Protocols]
> +  gEfiPciIoProtocolGuid
> +
> +[Guids]
> +  gRenesasFirmwarePD720202ImageId
> diff --git a/OpenPlatformPkg.dec b/OpenPlatformPkg.dec
> index af38488aa80b..2db143d637b3 100644
> --- a/OpenPlatformPkg.dec
> +++ b/OpenPlatformPkg.dec
> @@ -31,6 +31,9 @@
>    ## Vendor GUID for RAM disk device path
>    gRamDiskGuid = {0x6ED4DD0, 0xFF78, 0x11D3, {0xBD, 0xC4, 0x00, 0xA0, 0xC9, 0x40, 0x53, 0xD1}}
>  
> +  ## GUID for RenesasFirmwarePD720202 firmware image
> +  gRenesasFirmwarePD720202ImageId = {0xA059EBC4, 0xD73D, 0x4279, {0x81,0xBF,0xE4,0xA8,0x93,0x08,0xB9,0x23}}
> +
>  [LibraryClasses]
>  
>  [PcdsFixedAtBuild]
> -- 
> 2.7.4
>
Ard Biesheuvel Aug. 30, 2016, 2 p.m. | #2
On 30 August 2016 at 14:32, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Tue, Aug 30, 2016 at 12:18:09PM +0100, Ard Biesheuvel wrote:
>> This implements a PCI driver for the Renesas XHCI PD720202 PCIe
>> controller, which in some cases requires a device firmware to be
>> uploaded by the system firmware to function properly.
>>
>> By registering a UEFI driver with high dispatch priority, this driver
>> should be able to access the device early, install the firmware, and
>
> Do change the "should be" after we've tested it :)
>

Sure :-)

>> then decline the driver dispatch so that the device should be fully
>> functional when the real XHCI driver gets to see it.
>
> Good stuff!
> Minor comments below.
>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  Drivers/Xhci/RenesasFirmwarePD720202/RenesasFirmwarePD720202.c   | 365 ++++++++++++++++++++
>>  Drivers/Xhci/RenesasFirmwarePD720202/RenesasFirmwarePD720202.inf |  43 +++
>>  OpenPlatformPkg.dec                                              |   3 +
>>  3 files changed, 411 insertions(+)
>>
>> diff --git a/Drivers/Xhci/RenesasFirmwarePD720202/RenesasFirmwarePD720202.c b/Drivers/Xhci/RenesasFirmwarePD720202/RenesasFirmwarePD720202.c
>> new file mode 100644
>> index 000000000000..e1ff5d12c9bf
>> --- /dev/null
>> +++ b/Drivers/Xhci/RenesasFirmwarePD720202/RenesasFirmwarePD720202.c
>> @@ -0,0 +1,365 @@
>> +/**  @file
>> +  Implementation of driver entry point and driver binding protocol.
>> +
>> +Copyright (c) 2016, 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 <PiDxe.h>
>> +
>> +#include <Library/DebugLib.h>
>> +#include <Library/DxeServicesLib.h>
>> +#include <Library/UefiBootServicesTableLib.h>
>> +#include <Library/UefiLib.h>
>> +
>> +#include <IndustryStandard/Pci22.h>
>> +
>> +#include <Protocol/PciIo.h>
>> +
>> +#define PCI_VENDOR_ID_RENESAS               0x1912
>> +#define PCI_DEVICE_ID_PD720202              0x15
>> +
>> +#define PCI_FW_CTL_STAT_REG                 0xF4
>> +#define PCI_FW_CTL_DATA_REG                 0xF5
>> +#define PCI_FW_DATA0                        0xF8
>> +#define PCI_FW_DATA1                        0xFC
>> +
>> +#define PCI_FW_CTL_STAT_REG_FW_DL_ENABLE    (1U << 0)
>> +#define PCI_FW_CTL_STAT_REG_RESULT_CODE     (7U << 4)
>> +#define PCI_FW_CTL_STAT_REG_RESULT_OK       (1U << 4)
>> +#define PCI_FW_CTL_STAT_REG_SET_DATA0       (1U << 0)
>> +#define PCI_FW_CTL_STAT_REG_SET_DATA1       (1U << 1)
>> +
>> +STATIC CONST UINT32   *mFirmwareImage;
>> +STATIC UINTN          mFirmwareImageSize;
>> +
>> +STATIC
>> +UINT8
>> +ReadCtlStatVal (
>> +  IN EFI_PCI_IO_PROTOCOL     *PciIo,
>> +  IN UINTN                   Offset
>> +  )
>> +{
>> +  UINT8       CtlStatVal;
>> +  EFI_STATUS  Status;
>> +
>> +  Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint8, Offset, 1, &CtlStatVal);
>> +  ASSERT_EFI_ERROR (Status);
>> +
>> +  return CtlStatVal;
>> +}
>> +
>> +STATIC
>> +VOID
>> +WriteCtlStatVal (
>> +  IN EFI_PCI_IO_PROTOCOL     *PciIo,
>> +  IN UINTN                   Offset,
>> +  IN UINT8                   CtlStatVal
>> +  )
>> +{
>> +  EFI_STATUS  Status;
>> +
>> +  Status = PciIo->Pci.Write (PciIo, EfiPciIoWidthUint8, Offset, 1, &CtlStatVal);
>> +  ASSERT_EFI_ERROR (Status);
>> +}
>> +
>> +STATIC
>> +VOID
>> +WriteDataVal (
>> +  IN EFI_PCI_IO_PROTOCOL     *PciIo,
>> +  IN UINTN                   Offset,
>> +  IN UINT32                  DataVal
>> +  )
>> +{
>> +  EFI_STATUS  Status;
>> +
>> +  Status = PciIo->Pci.Write (PciIo, EfiPciIoWidthUint32, Offset, 1, &DataVal);
>> +  ASSERT_EFI_ERROR (Status);
>> +}
>> +
>> +STATIC
>> +BOOLEAN
>> +WaitReadCtlStatVal (
>> +  IN EFI_PCI_IO_PROTOCOL     *PciIo,
>> +  IN UINTN                   Offset,
>> +  IN UINT8                   Mask,
>> +  IN UINT8                   Val
>> +  )
>> +{
>> +  UINTN Timeout;
>> +
>> +  for (Timeout = 0; (ReadCtlStatVal (PciIo, Offset) & Mask) != Val; Timeout++) {
>> +    if (Timeout > 1000) {
>> +      DEBUG ((EFI_D_ERROR,
>> +        "%a: Timeout waiting for reg [+0x%x] & 0x%x to become 0x%x\n",
>> +        __FUNCTION__, Offset, Mask, Val));
>> +      return FALSE;
>> +    }
>> +    gBS->Stall (10);
>> +  }
>> +  return TRUE;
>> +}
>> +
>> +STATIC
>> +VOID
>> +DownloadPD720202Firmware (
>> +  IN EFI_PCI_IO_PROTOCOL     *PciIo
>> +  )
>> +{
>> +  UINTN   Idx;
>> +
>> +  Idx = 0;
>> +
>> +  // 1. Set "FW Download Enable" to '1b'.
>> +  WriteCtlStatVal (PciIo, PCI_FW_CTL_STAT_REG,
>> +    PCI_FW_CTL_STAT_REG_FW_DL_ENABLE);
>> +
>> +  // 2. Read "Set DATA0" and confirm it is '0b'.
>> +  if (!WaitReadCtlStatVal (PciIo, PCI_FW_CTL_DATA_REG,
>> +         PCI_FW_CTL_STAT_REG_SET_DATA0, 0)) {
>> +    return;
>> +  }
>> +
>> +  // 3. Write FW data to "DATA0".
>> +  WriteDataVal (PciIo, PCI_FW_DATA0, mFirmwareImage[Idx++]);
>> +
>> +  // 4. Read "Set DATA1" and confirm it is '0b'.
>> +  if (!WaitReadCtlStatVal (PciIo, PCI_FW_CTL_DATA_REG,
>> +         PCI_FW_CTL_STAT_REG_SET_DATA1, 0)) {
>> +    return;
>> +  }
>> +
>> +  // 5. Write FW data to "DATA1".
>> +  WriteDataVal (PciIo, PCI_FW_DATA1, mFirmwareImage[Idx++]);
>> +
>> +  // 6. Set "Set DATA0" & "Set DATA1" to '1b'.
>> +  WriteCtlStatVal (PciIo, PCI_FW_CTL_DATA_REG,
>> +    PCI_FW_CTL_STAT_REG_SET_DATA0 | PCI_FW_CTL_STAT_REG_SET_DATA1);
>> +
>> +  while (Idx < mFirmwareImageSize / sizeof(UINT32)) {
>> +
>> +    // 7. Read "Set DATA0" and confirm it is '0b'.
>> +    if (!WaitReadCtlStatVal (PciIo, PCI_FW_CTL_DATA_REG,
>> +           PCI_FW_CTL_STAT_REG_SET_DATA0, 0)) {
>> +      return;
>> +    }
>> +
>> +    // 8. Write FW data to"DATA0". Set "Set DATA0" to '1b'.
>> +    WriteDataVal (PciIo, PCI_FW_DATA0, mFirmwareImage[Idx++]);
>> +    WriteCtlStatVal (PciIo, PCI_FW_CTL_DATA_REG, PCI_FW_CTL_STAT_REG_SET_DATA0);
>> +
>> +    // 9. Read "Set DATA1" and confirm it is '0b'.
>> +    if (!WaitReadCtlStatVal (PciIo, PCI_FW_CTL_DATA_REG,
>> +           PCI_FW_CTL_STAT_REG_SET_DATA1, 0)) {
>> +      return;
>> +    }
>> +
>> +    // 10. Write FW data to"DATA1". Set "Set DATA1" to '1b'.
>> +    WriteDataVal (PciIo, PCI_FW_DATA1, mFirmwareImage[Idx++]);
>> +    WriteCtlStatVal (PciIo, PCI_FW_CTL_DATA_REG, PCI_FW_CTL_STAT_REG_SET_DATA1);
>> +
>> +    // 11. Return to step 7 and repeat the sequence from step 7 to step 10.
>> +  }
>> +
>> +  // 12. After writing the last data of FW, the System Software must set "FW Download Enable" to '0b'.
>> +  WriteCtlStatVal (PciIo, PCI_FW_CTL_STAT_REG, 0);
>> +
>> +  // 13. Read "Result Code" and confirm it is '001b'.
>> +  if (WaitReadCtlStatVal (PciIo, PCI_FW_CTL_STAT_REG,
>> +        PCI_FW_CTL_STAT_REG_RESULT_CODE, PCI_FW_CTL_STAT_REG_RESULT_OK)) {
>> +    DEBUG ((EFI_D_INFO, "%a: Renesas PD720202 firmware download successful\n",
>> +      __FUNCTION__));
>> +  } else {
>> +    DEBUG ((EFI_D_ERROR, "%a: Renesas PD720202 firmware download FAILED\n",
>> +      __FUNCTION__));
>> +  }
>> +}
>> +
>> +/**
>> +  Test to see if this driver supports ControllerHandle. This service
>> +  is called by the EFI boot service ConnectController(). In
>> +  order to make drivers as small as possible, there are a few calling
>> +  restrictions for this service. ConnectController() must
>> +  follow these calling restrictions. If any other agent wishes to call
>> +  Supported() it must also follow these calling restrictions.
>> +
>> +   @param  This                   Protocol instance pointer.
>> +   @param  ControllerHandle       Handle of device to test.
>> +   @param  RemainingDevicePath    Optional parameter use to pick a specific child
>> +                                                         device to start.
>> +
>> +   @retval EFI_SUCCESS            This driver supports this device.
>> +   @retval EFI_ALREADY_STARTED    This driver is already running on this device.
>> +   @retval other                  This driver does not support this device.
>> +
>> +**/
>> +STATIC
>> +EFI_STATUS
>> +EFIAPI
>> +RenesasPD720202DriverSupported (
>> +    IN EFI_DRIVER_BINDING_PROTOCOL    *This,
>> +    IN EFI_HANDLE                     Controller,
>> +    IN EFI_DEVICE_PATH_PROTOCOL       *RemainingDevicePath
>> +    )
>> +{
>> +  EFI_STATUS              Status;
>> +  EFI_PCI_IO_PROTOCOL     *PciIo;
>> +  UINT32                  PciID;
>> +  UINT8                   CtlStatVal;
>> +
>> +  //
>> +  // Check for the PCI IO Protocol
>> +  //
>> +  Status = gBS->OpenProtocol (Controller, &gEfiPciIoProtocolGuid,
>> +                  (VOID **)&PciIo, This->DriverBindingHandle, Controller,
>> +                  EFI_OPEN_PROTOCOL_BY_DRIVER);
>> +
>> +  if (EFI_ERROR (Status)) {
>> +    return Status;
>> +  }
>> +
>> +  Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint32, PCI_VENDOR_ID_OFFSET,
>> +                        1, &PciID);
>> +  if (EFI_ERROR (Status)) {
>> +    DEBUG ((EFI_D_ERROR,
>> +      "%a: Pci->Pci.Read() of vendor/device id failed (Status == %r)\n",
>> +      __FUNCTION__, Status));
>> +    goto CloseProtocol;
>> +  }
>> +
>> +  if ((PciID & 0xffff) != PCI_VENDOR_ID_RENESAS ||
>> +      (PciID >> 16) != PCI_DEVICE_ID_PD720202) {
>> +    DEBUG ((EFI_D_INFO, "%a: ignoring unsupported PCI device\n", __FUNCTION__));
>> +    goto CloseProtocol;
>> +  }
>> +
>> +  //
>> +  // We have found a device of the type we are looking for. Check if the
>> +  // firmware needs to be loaded.
>> +  //
>> +  Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint8, PCI_FW_CTL_STAT_REG,
>> +                        1, &CtlStatVal);
>> +  if (!EFI_ERROR (Status) &&
>> +      (CtlStatVal & PCI_FW_CTL_STAT_REG_RESULT_CODE) == 0) {
>
> Could we have a RESULT_INVALID define?
>

Will do

>> +    //
>> +    // Firmware download required
>> +    //
>> +    DEBUG ((EFI_D_INFO, "%a: downloading firmware\n", __FUNCTION__));
>> +    DownloadPD720202Firmware (PciIo);
>> +  }
>> +
>> +CloseProtocol:
>> +  gBS->CloseProtocol (Controller, &gEfiPciIoProtocolGuid,
>> +         This->DriverBindingHandle, Controller);
>> +
>> +  //
>> +  // Always return unsupported: we are not interested in driving the device,
>> +  // only in having the opportunity to install the firmware before the real
>> +  // driver attaches to it.
>> +  //
>> +  return EFI_UNSUPPORTED;
>> +}
>> +
>> +/**
>> +  Start this driver on Controller by opening PciIo and DevicePath protocols.
>> +  Initialize PXE structures, create a copy of the Controller Device Path with the
>> +  NIC's MAC address appended to it, install the NetworkInterfaceIdentifier protocol
>
> Ah, I see what you did there. Be nice to adjust the comment before
> commit though.
>

Ah, I thought i got rid of the boilerplate comments, but apparently not

>> +  on the newly created Device Path.
>> +
>> +  @param [in] This                    Protocol instance pointer.
>> +  @param [in] Controller              Handle of device to work with.
>> +  @param [in] RemainingDevicePath     Not used, always produce all possible children.
>> +
>> +  @retval EFI_SUCCESS                 This driver is added to Controller.
>> +  @retval other                       This driver does not support this device.
>> +
>> +**/
>> +STATIC
>> +EFI_STATUS
>> +EFIAPI
>> +RenesasPD720202DriverStart (
>> +    IN EFI_DRIVER_BINDING_PROTOCOL  *This,
>> +    IN EFI_HANDLE                   Controller,
>> +    IN EFI_DEVICE_PATH_PROTOCOL     *RemainingDevicePath
>> +    )
>> +{
>> +  //
>> +  // We are not interested in driving the device, we only poke the firmware
>> +  // in the .Supported() callback.
>> +  //
>> +  ASSERT (FALSE);
>> +  return EFI_INVALID_PARAMETER;
>> +}
>> +
>> +/**
>> +  Stop this driver on Controller
>> +
>> +  @param [in] This                    Protocol instance pointer.
>> +  @param [in] Controller              Handle of device to stop driver on.
>> +  @param [in] NumberOfChildren        How many children need to be stopped.
>> +  @param [in] ChildHandleBuffer       Not used.
>> +
>> +  @retval EFI_SUCCESS                 This driver is removed Controller.
>> +  @retval EFI_DEVICE_ERROR            The device could not be stopped due to a device error.
>> +  @retval other                       This driver was not removed from this device.
>> +
>> +**/
>> +STATIC
>> +EFI_STATUS
>> +EFIAPI
>> +RenesasPD720202DriverStop (
>> +    IN  EFI_DRIVER_BINDING_PROTOCOL *This,
>> +    IN  EFI_HANDLE                  Controller,
>> +    IN  UINTN                       NumberOfChildren,
>> +    IN  EFI_HANDLE                  *ChildHandleBuffer
>> +    )
>> +{
>
> So, I was going to point out that since we never Start the driver, it
> is invalid to call Stop (and we should put a comment to that effct
> in). However, I'm not convinced EFI_INVALID_PARAMETER is a valid thing
> to return here: "If ControllerHandle is stopped, then EFI_SUCCESS is
> returned."
>
> (We may still want to assert, in order to find funny stuff going on.
>

Frankly, I don't think it makes a whole lot of difference what we
return here, since if it is being called, weird stuff is going on
anyway. So let's simply return EFI_SUCCESS instead.

>> +  ASSERT (FALSE);
>> +  return EFI_INVALID_PARAMETER;
>> +}
>> +
>> +//
>> +// Simple Network Protocol Driver Global Variables
>
> *cough*
>
>> +//
>> +STATIC EFI_DRIVER_BINDING_PROTOCOL RenesasPD720202DriverBinding = {
>> +  RenesasPD720202DriverSupported,
>> +  RenesasPD720202DriverStart,
>> +  RenesasPD720202DriverStop,
>> +  0xfffffff0,
>
> Would it be worth adding a define for this one?
> If not, a comment pointing out the party trick might be in order:
> // Version values of 0xfffffff0-0xffffffff are reserved for
> // platform/OEM specific drivers. Protocol instances with higher
> // 'Version' drivers will be used before lower 'Version' ones.
> // XhciDxe uses version 0x30, so this driver will be called in
> // preference, and XhciDxe will be invoked after
> // RenesasPD720202DriverSupported returns EFI_UNSUPPORTED.
>

Yes, let me add that once we're a bit more confident that it actually
works as expected

Thanks,
Ard.

Patch

diff --git a/Drivers/Xhci/RenesasFirmwarePD720202/RenesasFirmwarePD720202.c b/Drivers/Xhci/RenesasFirmwarePD720202/RenesasFirmwarePD720202.c
new file mode 100644
index 000000000000..e1ff5d12c9bf
--- /dev/null
+++ b/Drivers/Xhci/RenesasFirmwarePD720202/RenesasFirmwarePD720202.c
@@ -0,0 +1,365 @@ 
+/**  @file
+  Implementation of driver entry point and driver binding protocol.
+
+Copyright (c) 2016, 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 <PiDxe.h>
+
+#include <Library/DebugLib.h>
+#include <Library/DxeServicesLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiLib.h>
+
+#include <IndustryStandard/Pci22.h>
+
+#include <Protocol/PciIo.h>
+
+#define PCI_VENDOR_ID_RENESAS               0x1912
+#define PCI_DEVICE_ID_PD720202              0x15
+
+#define PCI_FW_CTL_STAT_REG                 0xF4
+#define PCI_FW_CTL_DATA_REG                 0xF5
+#define PCI_FW_DATA0                        0xF8
+#define PCI_FW_DATA1                        0xFC
+
+#define PCI_FW_CTL_STAT_REG_FW_DL_ENABLE    (1U << 0)
+#define PCI_FW_CTL_STAT_REG_RESULT_CODE     (7U << 4)
+#define PCI_FW_CTL_STAT_REG_RESULT_OK       (1U << 4)
+#define PCI_FW_CTL_STAT_REG_SET_DATA0       (1U << 0)
+#define PCI_FW_CTL_STAT_REG_SET_DATA1       (1U << 1)
+
+STATIC CONST UINT32   *mFirmwareImage;
+STATIC UINTN          mFirmwareImageSize;
+
+STATIC
+UINT8
+ReadCtlStatVal (
+  IN EFI_PCI_IO_PROTOCOL     *PciIo,
+  IN UINTN                   Offset
+  )
+{
+  UINT8       CtlStatVal;
+  EFI_STATUS  Status;
+
+  Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint8, Offset, 1, &CtlStatVal);
+  ASSERT_EFI_ERROR (Status);
+
+  return CtlStatVal;
+}
+
+STATIC
+VOID
+WriteCtlStatVal (
+  IN EFI_PCI_IO_PROTOCOL     *PciIo,
+  IN UINTN                   Offset,
+  IN UINT8                   CtlStatVal
+  )
+{
+  EFI_STATUS  Status;
+
+  Status = PciIo->Pci.Write (PciIo, EfiPciIoWidthUint8, Offset, 1, &CtlStatVal);
+  ASSERT_EFI_ERROR (Status);
+}
+
+STATIC
+VOID
+WriteDataVal (
+  IN EFI_PCI_IO_PROTOCOL     *PciIo,
+  IN UINTN                   Offset,
+  IN UINT32                  DataVal
+  )
+{
+  EFI_STATUS  Status;
+
+  Status = PciIo->Pci.Write (PciIo, EfiPciIoWidthUint32, Offset, 1, &DataVal);
+  ASSERT_EFI_ERROR (Status);
+}
+
+STATIC
+BOOLEAN
+WaitReadCtlStatVal (
+  IN EFI_PCI_IO_PROTOCOL     *PciIo,
+  IN UINTN                   Offset,
+  IN UINT8                   Mask,
+  IN UINT8                   Val
+  )
+{
+  UINTN Timeout;
+
+  for (Timeout = 0; (ReadCtlStatVal (PciIo, Offset) & Mask) != Val; Timeout++) {
+    if (Timeout > 1000) {
+      DEBUG ((EFI_D_ERROR,
+        "%a: Timeout waiting for reg [+0x%x] & 0x%x to become 0x%x\n",
+        __FUNCTION__, Offset, Mask, Val));
+      return FALSE;
+    }
+    gBS->Stall (10);
+  }
+  return TRUE;
+}
+
+STATIC
+VOID
+DownloadPD720202Firmware (
+  IN EFI_PCI_IO_PROTOCOL     *PciIo
+  )
+{
+  UINTN   Idx;
+
+  Idx = 0;
+
+  // 1. Set "FW Download Enable" to '1b'.
+  WriteCtlStatVal (PciIo, PCI_FW_CTL_STAT_REG,
+    PCI_FW_CTL_STAT_REG_FW_DL_ENABLE);
+
+  // 2. Read "Set DATA0" and confirm it is '0b'.
+  if (!WaitReadCtlStatVal (PciIo, PCI_FW_CTL_DATA_REG,
+         PCI_FW_CTL_STAT_REG_SET_DATA0, 0)) {
+    return;
+  }
+
+  // 3. Write FW data to "DATA0".
+  WriteDataVal (PciIo, PCI_FW_DATA0, mFirmwareImage[Idx++]);
+
+  // 4. Read "Set DATA1" and confirm it is '0b'.
+  if (!WaitReadCtlStatVal (PciIo, PCI_FW_CTL_DATA_REG,
+         PCI_FW_CTL_STAT_REG_SET_DATA1, 0)) {
+    return;
+  }
+
+  // 5. Write FW data to "DATA1".
+  WriteDataVal (PciIo, PCI_FW_DATA1, mFirmwareImage[Idx++]);
+
+  // 6. Set "Set DATA0" & "Set DATA1" to '1b'.
+  WriteCtlStatVal (PciIo, PCI_FW_CTL_DATA_REG,
+    PCI_FW_CTL_STAT_REG_SET_DATA0 | PCI_FW_CTL_STAT_REG_SET_DATA1);
+
+  while (Idx < mFirmwareImageSize / sizeof(UINT32)) {
+
+    // 7. Read "Set DATA0" and confirm it is '0b'.
+    if (!WaitReadCtlStatVal (PciIo, PCI_FW_CTL_DATA_REG,
+           PCI_FW_CTL_STAT_REG_SET_DATA0, 0)) {
+      return;
+    }
+
+    // 8. Write FW data to"DATA0". Set "Set DATA0" to '1b'.
+    WriteDataVal (PciIo, PCI_FW_DATA0, mFirmwareImage[Idx++]);
+    WriteCtlStatVal (PciIo, PCI_FW_CTL_DATA_REG, PCI_FW_CTL_STAT_REG_SET_DATA0);
+
+    // 9. Read "Set DATA1" and confirm it is '0b'.
+    if (!WaitReadCtlStatVal (PciIo, PCI_FW_CTL_DATA_REG,
+           PCI_FW_CTL_STAT_REG_SET_DATA1, 0)) {
+      return;
+    }
+
+    // 10. Write FW data to"DATA1". Set "Set DATA1" to '1b'.
+    WriteDataVal (PciIo, PCI_FW_DATA1, mFirmwareImage[Idx++]);
+    WriteCtlStatVal (PciIo, PCI_FW_CTL_DATA_REG, PCI_FW_CTL_STAT_REG_SET_DATA1);
+
+    // 11. Return to step 7 and repeat the sequence from step 7 to step 10.
+  }
+
+  // 12. After writing the last data of FW, the System Software must set "FW Download Enable" to '0b'.
+  WriteCtlStatVal (PciIo, PCI_FW_CTL_STAT_REG, 0);
+
+  // 13. Read "Result Code" and confirm it is '001b'. 
+  if (WaitReadCtlStatVal (PciIo, PCI_FW_CTL_STAT_REG,
+        PCI_FW_CTL_STAT_REG_RESULT_CODE, PCI_FW_CTL_STAT_REG_RESULT_OK)) {
+    DEBUG ((EFI_D_INFO, "%a: Renesas PD720202 firmware download successful\n",
+      __FUNCTION__));
+  } else {
+    DEBUG ((EFI_D_ERROR, "%a: Renesas PD720202 firmware download FAILED\n",
+      __FUNCTION__));
+  }
+}
+
+/**
+  Test to see if this driver supports ControllerHandle. This service
+  is called by the EFI boot service ConnectController(). In
+  order to make drivers as small as possible, there are a few calling
+  restrictions for this service. ConnectController() must
+  follow these calling restrictions. If any other agent wishes to call
+  Supported() it must also follow these calling restrictions.
+
+   @param  This                   Protocol instance pointer.
+   @param  ControllerHandle       Handle of device to test.
+   @param  RemainingDevicePath    Optional parameter use to pick a specific child
+                                                         device to start.
+
+   @retval EFI_SUCCESS            This driver supports this device.
+   @retval EFI_ALREADY_STARTED    This driver is already running on this device.
+   @retval other                  This driver does not support this device.
+
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+RenesasPD720202DriverSupported (
+    IN EFI_DRIVER_BINDING_PROTOCOL    *This,
+    IN EFI_HANDLE                     Controller,
+    IN EFI_DEVICE_PATH_PROTOCOL       *RemainingDevicePath
+    )
+{
+  EFI_STATUS              Status;
+  EFI_PCI_IO_PROTOCOL     *PciIo;
+  UINT32                  PciID;
+  UINT8                   CtlStatVal;
+
+  //
+  // Check for the PCI IO Protocol
+  //
+  Status = gBS->OpenProtocol (Controller, &gEfiPciIoProtocolGuid,
+                  (VOID **)&PciIo, This->DriverBindingHandle, Controller,
+                  EFI_OPEN_PROTOCOL_BY_DRIVER);
+
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint32, PCI_VENDOR_ID_OFFSET,
+                        1, &PciID);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((EFI_D_ERROR,
+      "%a: Pci->Pci.Read() of vendor/device id failed (Status == %r)\n",
+      __FUNCTION__, Status));
+    goto CloseProtocol;
+  }
+
+  if ((PciID & 0xffff) != PCI_VENDOR_ID_RENESAS ||
+      (PciID >> 16) != PCI_DEVICE_ID_PD720202) {
+    DEBUG ((EFI_D_INFO, "%a: ignoring unsupported PCI device\n", __FUNCTION__));
+    goto CloseProtocol;
+  }
+
+  //
+  // We have found a device of the type we are looking for. Check if the
+  // firmware needs to be loaded.
+  //
+  Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint8, PCI_FW_CTL_STAT_REG,
+                        1, &CtlStatVal);
+  if (!EFI_ERROR (Status) &&
+      (CtlStatVal & PCI_FW_CTL_STAT_REG_RESULT_CODE) == 0) {
+    //
+    // Firmware download required
+    //
+    DEBUG ((EFI_D_INFO, "%a: downloading firmware\n", __FUNCTION__));
+    DownloadPD720202Firmware (PciIo);
+  }
+
+CloseProtocol:
+  gBS->CloseProtocol (Controller, &gEfiPciIoProtocolGuid,
+         This->DriverBindingHandle, Controller);
+
+  //
+  // Always return unsupported: we are not interested in driving the device,
+  // only in having the opportunity to install the firmware before the real
+  // driver attaches to it.
+  //
+  return EFI_UNSUPPORTED;
+}
+
+/**
+  Start this driver on Controller by opening PciIo and DevicePath protocols.
+  Initialize PXE structures, create a copy of the Controller Device Path with the
+  NIC's MAC address appended to it, install the NetworkInterfaceIdentifier protocol
+  on the newly created Device Path.
+
+  @param [in] This                    Protocol instance pointer.
+  @param [in] Controller              Handle of device to work with.
+  @param [in] RemainingDevicePath     Not used, always produce all possible children.
+
+  @retval EFI_SUCCESS                 This driver is added to Controller.
+  @retval other                       This driver does not support this device.
+
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+RenesasPD720202DriverStart (
+    IN EFI_DRIVER_BINDING_PROTOCOL  *This,
+    IN EFI_HANDLE                   Controller,
+    IN EFI_DEVICE_PATH_PROTOCOL     *RemainingDevicePath
+    )
+{
+  //
+  // We are not interested in driving the device, we only poke the firmware
+  // in the .Supported() callback.
+  //
+  ASSERT (FALSE);
+  return EFI_INVALID_PARAMETER;
+}
+
+/**
+  Stop this driver on Controller
+
+  @param [in] This                    Protocol instance pointer.
+  @param [in] Controller              Handle of device to stop driver on.
+  @param [in] NumberOfChildren        How many children need to be stopped.
+  @param [in] ChildHandleBuffer       Not used.
+
+  @retval EFI_SUCCESS                 This driver is removed Controller.
+  @retval EFI_DEVICE_ERROR            The device could not be stopped due to a device error.
+  @retval other                       This driver was not removed from this device.
+
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+RenesasPD720202DriverStop (
+    IN  EFI_DRIVER_BINDING_PROTOCOL *This,
+    IN  EFI_HANDLE                  Controller,
+    IN  UINTN                       NumberOfChildren,
+    IN  EFI_HANDLE                  *ChildHandleBuffer
+    )
+{
+  ASSERT (FALSE);
+  return EFI_INVALID_PARAMETER;
+}
+
+//
+// Simple Network Protocol Driver Global Variables
+//
+STATIC EFI_DRIVER_BINDING_PROTOCOL RenesasPD720202DriverBinding = {
+  RenesasPD720202DriverSupported,
+  RenesasPD720202DriverStart,
+  RenesasPD720202DriverStop,
+  0xfffffff0,
+  NULL,
+  NULL
+};
+
+EFI_STATUS
+EFIAPI
+InitializeRenesasPD720202Driver (
+    IN EFI_HANDLE       ImageHandle,
+    IN EFI_SYSTEM_TABLE *SystemTable
+    )
+{
+  EFI_STATUS    Status;
+
+  //
+  // First, try to locate the firmware image. If it is missing, there is no
+  // point in proceeding.
+  //
+  Status = GetSectionFromAnyFv (&gRenesasFirmwarePD720202ImageId,
+    EFI_SECTION_RAW, 0, (VOID **) (VOID **)&mFirmwareImage,
+    &mFirmwareImageSize);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((EFI_D_ERROR, "%a: could not locate PD720202 firmware image\n",
+      __FUNCTION__));
+    return Status;
+  }
+
+  return EfiLibInstallDriverBinding (ImageHandle, SystemTable,
+        &RenesasPD720202DriverBinding, NULL);
+}
diff --git a/Drivers/Xhci/RenesasFirmwarePD720202/RenesasFirmwarePD720202.inf b/Drivers/Xhci/RenesasFirmwarePD720202/RenesasFirmwarePD720202.inf
new file mode 100644
index 000000000000..f999bf7b1e60
--- /dev/null
+++ b/Drivers/Xhci/RenesasFirmwarePD720202/RenesasFirmwarePD720202.inf
@@ -0,0 +1,43 @@ 
+##  <at> file
+# Component description file for Reneses PD720202 firmware download driver
+#
+# Copyright (c) 2016, 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                    = 0x00010005
+  BASE_NAME                      = RenesasFirmwarePD720202
+  FILE_GUID                      = 5979ebfe-d53c-4150-a972-568231583969
+  MODULE_TYPE                    = UEFI_DRIVER
+  VERSION_STRING                 = 1.0
+  ENTRY_POINT                    = InitializeRenesasPD720202Driver
+
+[Sources]
+  RenesasFirmwarePD720202.c
+
+[Packages]
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+  OpenPlatformPkg/OpenPlatformPkg.dec
+
+[LibraryClasses]
+  DebugLib
+  DxeServicesLib
+  UefiBootServicesTableLib
+  UefiLib
+  UefiDriverEntryPoint
+
+[Protocols]
+  gEfiPciIoProtocolGuid
+
+[Guids]
+  gRenesasFirmwarePD720202ImageId
diff --git a/OpenPlatformPkg.dec b/OpenPlatformPkg.dec
index af38488aa80b..2db143d637b3 100644
--- a/OpenPlatformPkg.dec
+++ b/OpenPlatformPkg.dec
@@ -31,6 +31,9 @@ 
   ## Vendor GUID for RAM disk device path
   gRamDiskGuid = {0x6ED4DD0, 0xFF78, 0x11D3, {0xBD, 0xC4, 0x00, 0xA0, 0xC9, 0x40, 0x53, 0xD1}}
 
+  ## GUID for RenesasFirmwarePD720202 firmware image
+  gRenesasFirmwarePD720202ImageId = {0xA059EBC4, 0xD73D, 0x4279, {0x81,0xBF,0xE4,0xA8,0x93,0x08,0xB9,0x23}}
+
 [LibraryClasses]
 
 [PcdsFixedAtBuild]