From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 7C93A21A6F106 for ; Tue, 18 Apr 2017 19:51:45 -0700 (PDT) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga105.fm.intel.com with ESMTP; 18 Apr 2017 19:51:44 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.37,219,1488873600"; d="scan'208";a="958541539" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by orsmga003.jf.intel.com with ESMTP; 18 Apr 2017 19:51:44 -0700 Received: from fmsmsx122.amr.corp.intel.com (10.18.125.37) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 18 Apr 2017 19:51:43 -0700 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by fmsmsx122.amr.corp.intel.com (10.18.125.37) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 18 Apr 2017 19:51:43 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.178]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.193]) with mapi id 14.03.0319.002; Wed, 19 Apr 2017 10:51:40 +0800 From: "Ni, Ruiyu" To: Ard Biesheuvel , "edk2-devel@lists.01.org" , "leif.lindholm@linaro.org" Thread-Topic: [edk2] [PATCH] OptionRomPkg: add firmware loader driver for Renesas PD72020x Thread-Index: AQHSuGy/JQ+SNVlZLUK1fQsRQ7RUpaHL/idA Date: Wed, 19 Apr 2017 02:51:40 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5B93532E@SHSMSX104.ccr.corp.intel.com> References: <20170418175334.17765-1-ard.biesheuvel@linaro.org> In-Reply-To: <20170418175334.17765-1-ard.biesheuvel@linaro.org> Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH] OptionRomPkg: add firmware loader driver for Renesas PD72020x X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 19 Apr 2017 02:51:45 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Ard, My understanding is the new driver downloads firmware into the XHCI control= ler in Supported(). The generic XHCI driver can be started successfully only after the firmware= is downloaded. But what if the XHCI driver Supported()/Start() is called before your new d= river's Supported()? I see a dependency but your code doesn't guarantee the dependency. Thanks/Ray > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Ard Biesheuvel > Sent: Wednesday, April 19, 2017 1:54 AM > To: edk2-devel@lists.01.org; leif.lindholm@linaro.org; Ni, Ruiyu > > Cc: Ard Biesheuvel > Subject: [edk2] [PATCH] OptionRomPkg: add firmware loader driver for > Renesas PD72020x >=20 > Some versions of the Renesas PD72020x XHCI controller can be integrated > into platforms without any non-volatile storage for the controller's firm= ware, > in which case it is the job of the system firmware to upload a firmware i= mage > before attempting to attach the generic XHCI driver to it. >=20 > So implement a driver that does just this: it will upload the bundled fir= mware > image (accessible via its FFS GUID) in the Supported() method of the driv= er > binding protocol, and subsequently decline to bind to the device, which > allows the real XHCI driver to attach to it instead. >=20 > The firmware itself is not part of this patch: it is up to the integrator= to > incorporate it into a firmware volume under the associated GUID > (gRenesasFirmwarePD720202ImageId) >=20 > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel > --- > OptionRomPkg/OptionRomPkg.dec | 3 + > OptionRomPkg/OptionRomPkg.dsc | 5 + > OptionRomPkg/RenesasFirmwarePD720202/RenesasFirmwarePD720202.c | > 372 ++++++++++++++++++++ > OptionRomPkg/RenesasFirmwarePD720202/RenesasFirmwarePD720202.inf > | 43 +++ > 4 files changed, 423 insertions(+) >=20 > diff --git a/OptionRomPkg/OptionRomPkg.dec > b/OptionRomPkg/OptionRomPkg.dec index ea4d57b996ae..dc98ff58e374 > 100644 > --- a/OptionRomPkg/OptionRomPkg.dec > +++ b/OptionRomPkg/OptionRomPkg.dec > @@ -36,6 +36,9 @@ [LibraryClasses] > [Guids] > gOptionRomPkgTokenSpaceGuid =3D { 0x1e43298f, 0x3478, 0x41a7, { 0xb5, > 0x77, 0x86, 0x6, 0x46, 0x35, 0xc7, 0x28 } } >=20 > + ## GUID for RenesasFirmwarePD720202 firmware image > + gRenesasFirmwarePD720202ImageId =3D {0xA059EBC4, 0xD73D, 0x4279, > + {0x81,0xBF,0xE4,0xA8,0x93,0x08,0xB9,0x23}} > + > [PcdsFeatureFlag] >=20 > gOptionRomPkgTokenSpaceGuid.PcdSupportScsiPassThru|TRUE|BOOLEAN| > 0x00010001 >=20 > gOptionRomPkgTokenSpaceGuid.PcdSupportExtScsiPassThru|TRUE|BOOLEA > N|0x00010002 > diff --git a/OptionRomPkg/OptionRomPkg.dsc > b/OptionRomPkg/OptionRomPkg.dsc index 33655567109b..07c26117251d > 100644 > --- a/OptionRomPkg/OptionRomPkg.dsc > +++ b/OptionRomPkg/OptionRomPkg.dsc > @@ -115,5 +115,10 @@ [Components] > OptionRomPkg/Bus/Usb/UsbNetworking/Ax88772/Ax88772.inf > OptionRomPkg/Bus/Usb/UsbNetworking/Ax88772b/Ax88772b.inf >=20 > + > OptionRomPkg/RenesasFirmwarePD720202/RenesasFirmwarePD720202.inf { > + > + DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf > + } > + > [Components.IA32, Components.X64, Components.IPF] > OptionRomPkg/Application/BltLibSample/BltLibSample.inf > diff --git > a/OptionRomPkg/RenesasFirmwarePD720202/RenesasFirmwarePD720202.c > b/OptionRomPkg/RenesasFirmwarePD720202/RenesasFirmwarePD720202.c > new file mode 100644 > index 000000000000..c7d3f1820840 > --- /dev/null > +++ > b/OptionRomPkg/RenesasFirmwarePD720202/RenesasFirmwarePD720202.c > @@ -0,0 +1,372 @@ > +/** @file > + Implementation of driver entry point and driver binding protocol. > + > +Copyright (c) 2016 - 2017, 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 > + > +#include > +#include > +#include > +#include > + > +#include > + > +#include > + > +#define PCI_VENDOR_ID_RENESAS 0x1912 > +#define PCI_DEVICE_ID_PD720201 0x14 > +#define PCI_DEVICE_ID_PD720202 0x15 > + > +#define PCI_FW_CTL_STAT_REG 0xF4 > +#define PCI_FW_CTL_DATA_REG 0xF5 > +#define PCI_EXT_ROM_CTL_REG 0xF6 > +#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_INVALID (0) > +#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) > + > +#define PCI_EXT_ROM_CTL_REG_ROM_EXISTS (1U << 15) > + > +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 =3D 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 =3D 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 =3D 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 =3D 0; (ReadCtlStatVal (PciIo, Offset) & Mask) !=3D 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 =3D 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 speci= fic > 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 devi= ce. > + > +**/ > +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 =3D gBS->OpenProtocol (Controller, &gEfiPciIoProtocolGuid, > + (VOID **)&PciIo, This->DriverBindingHandle, Controller= , > + EFI_OPEN_PROTOCOL_BY_DRIVER); > + > + if (EFI_ERROR (Status)) { > + return Status; > + } > + > + Status =3D 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 =3D=3D %r)= \n", > + __FUNCTION__, Status)); > + goto CloseProtocol; > + } > + > + if ((PciID & 0xffff) !=3D PCI_VENDOR_ID_RENESAS || > + ((PciID >> 16) !=3D PCI_DEVICE_ID_PD720201 && > + (PciID >> 16) !=3D PCI_DEVICE_ID_PD720202)) { > + DEBUG ((EFI_D_INFO, "%a: ignoring unsupported PCI device > 0x%04x:0x%04x\n", > + __FUNCTION__, PciID & 0xffff, PciID >> 16)); > + goto CloseProtocol; > + } > + > + Status =3D PciIo->Pci.Read (PciIo, EfiPciIoWidthUint8, > PCI_FW_CTL_STAT_REG, > + 1, &CtlStatVal); if (!EFI_ERROR (Status) && > + (CtlStatVal & PCI_FW_CTL_STAT_REG_RESULT_CODE) =3D=3D > PCI_FW_CTL_STAT_REG_RESULT_INVALID) { > + // > + // 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. Not used. > + > + @param [in] This Protocol instance pointer. > + @param [in] Controller Handle of device to work with. > + @param [in] RemainingDevicePath Not used, always produce all possi= ble > 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. Not used. > + > + @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 du= e to > a device error. > + @retval other This driver was not removed from t= his 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_SUCCESS; > +} > + > +// > +// UEFI Driver Model entry point > +// > +STATIC EFI_DRIVER_BINDING_PROTOCOL RenesasPD720202DriverBinding =3D > { > + RenesasPD720202DriverSupported, > + RenesasPD720202DriverStart, > + RenesasPD720202DriverStop, > + > + // Version values of 0xfffffff0-0xffffffff are reserved for > +platform/OEM > + // specific drivers. Protocol instances with higher 'Version' > +properties > + // 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. > + 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 =3D 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__)); > + ASSERT_EFI_ERROR (Status); > + return Status; > + } > + > + return EfiLibInstallDriverBinding (ImageHandle, SystemTable, > + &RenesasPD720202DriverBinding, NULL); } > diff --git > a/OptionRomPkg/RenesasFirmwarePD720202/RenesasFirmwarePD720202.in > f > b/OptionRomPkg/RenesasFirmwarePD720202/RenesasFirmwarePD720202.in > f > new file mode 100644 > index 000000000000..7115ec1ba7e8 > --- /dev/null > +++ > b/OptionRomPkg/RenesasFirmwarePD720202/RenesasFirmwarePD720202.in > f > @@ -0,0 +1,43 @@ > +## @file > +# Component description file for Reneses PD720202 firmware download > +driver # # Copyright (c) 2016 - 2017, 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. > +# > +## > + > +[Defines] > + INF_VERSION =3D 0x00010005 > + BASE_NAME =3D RenesasFirmwarePD720202 > + FILE_GUID =3D 5979ebfe-d53c-4150-a972-56823158396= 9 > + MODULE_TYPE =3D UEFI_DRIVER > + VERSION_STRING =3D 1.0 > + ENTRY_POINT =3D InitializeRenesasPD720202Driver > + > +[Sources] > + RenesasFirmwarePD720202.c > + > +[Packages] > + MdeModulePkg/MdeModulePkg.dec > + MdePkg/MdePkg.dec > + OptionRomPkg/OptionRomPkg.dec > + > +[LibraryClasses] > + DebugLib > + DxeServicesLib > + UefiBootServicesTableLib > + UefiLib > + UefiDriverEntryPoint > + > +[Protocols] > + gEfiPciIoProtocolGuid > + > +[Guids] > + gRenesasFirmwarePD720202ImageId > -- > 2.9.3 >=20 > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel