Re: [PATCH edk2-platforms v1 04/14] Hisilicon D03/D05: Add capsule upgrade support

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 23 January 2018 at 08:53, Huangming (Mark) <huangming23@huawei.com> wrote:
>
>
> On 2018/1/20 18:50, Ard Biesheuvel wrote:
>> On 18 January 2018 at 15:01, Ming Huang <heyi.guo@linaro.org> wrote:
>>> From: Jason Zhang <zhangjinsong2@huawei.com>
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Jason Zhang <zhangjinsong2@huawei.com>
>>> Signed-off-by: Ming Huang <huangming23@huawei.com>
>>> Signed-off-by: Heyi Guo <heyi.guo@linaro.org>
>>> ---
>>>  Platform/Hisilicon/D03/Capsule/SystemFirmwareUpdateConfig/SystemFirmwareUpdateConfig.ini |  45 +++++++++
>>>  Platform/Hisilicon/D03/D03.dsc                                                           |  17 +++-
>>>  Platform/Hisilicon/D03/D03.fdf                                                           |  70 +++++++++++++
>>>  Platform/Hisilicon/D05/Capsule/SystemFirmwareUpdateConfig/SystemFirmwareUpdateConfig.ini |  45 +++++++++
>>>  Platform/Hisilicon/D05/D05.dsc                                                           |  19 +++-
>>>  Platform/Hisilicon/D05/D05.fdf                                                           |  70 +++++++++++++
>>>  Silicon/Hisilicon/Drivers/SystemFirmwareDescriptor/SystemFirmwareDescriptor.aslc         |  81 +++++++++++++++
>>>  Silicon/Hisilicon/Drivers/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf          |  50 +++++++++
>>>  Silicon/Hisilicon/Drivers/SystemFirmwareDescriptor/SystemFirmwareDescriptorPei.c         |  70 +++++++++++++
>>>  Silicon/Hisilicon/Hisilicon.dsc.inc                                                      |  11 +-
>>>  Silicon/Hisilicon/Hisilicon.fdf.inc                                                      |   9 ++
>>>  Silicon/Hisilicon/Library/PlatformFlashAccessLib/PlatformFlashAccessLibDxe.c             | 106 ++++++++++++++++++++
>>>  Silicon/Hisilicon/Library/PlatformFlashAccessLib/PlatformFlashAccessLibDxe.inf           |  51 ++++++++++
>>>  13 files changed, 641 insertions(+), 3 deletions(-)
>>>
>>
>> Excellent!! Very happy to see this added.
>>
...
>>> diff --git a/Silicon/Hisilicon/Library/PlatformFlashAccessLib/PlatformFlashAccessLibDxe.c b/Silicon/Hisilicon/Library/PlatformFlashAccessLib/PlatformFlashAccessLibDxe.c
>>> new file mode 100644
>>> index 0000000..db5725d
>>> --- /dev/null
>>> +++ b/Silicon/Hisilicon/Library/PlatformFlashAccessLib/PlatformFlashAccessLibDxe.c
>>> @@ -0,0 +1,106 @@
>>> +/** @file
>>> +  Platform Flash Access library.
>>> +
>>> +  Copyright (c) 2018, Hisilicon Limited. All rights reserved.
>>> +  Copyright (c) 2018, Linaro Limited. All rights reserved.
>>> +  Copyright (c) 2016, Intel Corporation. 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 <Library/BaseLib.h>
>>> +#include <Library/BaseMemoryLib.h>
>>> +#include <Library/DebugLib.h>
>>> +#include <Library/PcdLib.h>
>>> +#include <Library/PlatformFlashAccessLib.h>
>>> +#include <Library/UefiBootServicesTableLib.h>
>>> +#include <Protocol/HisiSpiFlashProtocol.h>
>>> +
>>> +STATIC EFI_PHYSICAL_ADDRESS     mInternalFdAddress;
>>> +STATIC EFI_PHYSICAL_ADDRESS     mSFCMEM0BaseAddress;
>>> +
>>> +HISI_SPI_FLASH_PROTOCOL         *mSpiProtocol;
>>
>> STATIC
>>
>
> I agree with all of your comments except this one.
> PerformFlashWrite is called in SystemFirmwareUpdateDxe module,
> so STATIC can not be added.
>

I meant mSpiProtocol not PerformFlashWrite


>>> +
>>> +/**
>>> +  Perform flash write opreation.
>>> +
>>> +  @param[in] FirmwareType      The type of firmware.
>>> +  @param[in] FlashAddress      The address of flash device to be accessed.
>>> +  @param[in] FlashAddressType  The type of flash device address.
>>> +  @param[in] Buffer            The pointer to the data buffer.
>>> +  @param[in] Length            The length of data buffer in bytes.
>>> +
>>> +  @retval EFI_SUCCESS           The operation returns successfully.
>>> +  @retval EFI_WRITE_PROTECTED   The flash device is read only.
>>> +  @retval EFI_UNSUPPORTED       The flash device access is unsupported.
>>> +  @retval EFI_INVALID_PARAMETER The input parameter is not valid.
>>> +**/
>>> +EFI_STATUS
>>> +EFIAPI
>>> +PerformFlashWrite (
>>> +  IN PLATFORM_FIRMWARE_TYPE       FirmwareType,
>>> +  IN EFI_PHYSICAL_ADDRESS         FlashAddress,
>>> +  IN FLASH_ADDRESS_TYPE           FlashAddressType,
>>> +  IN VOID                         *Buffer,
>>> +  IN UINTN                        Length
>>> +  )
>>> +{
>>> +  UINT32               RomAddress;
>>> +  EFI_STATUS           Status;
>>> +
>>> +  DEBUG ((DEBUG_INFO, "PerformFlashWrite - 0x%x(%x) - 0x%x\n", (UINTN)FlashAddress, (UINTN)FlashAddressType, Length));
>>> +
>>
>> Line length
>>
>>> +  if (FlashAddressType == FlashAddressTypeAbsoluteAddress) {
>>> +    FlashAddress = FlashAddress - mInternalFdAddress;
>>> +  }
>>> +
>>> +  RomAddress = (UINT32)FlashAddress + (mInternalFdAddress - mSFCMEM0BaseAddress);
>>> +
>>> +  DEBUG ((DEBUG_INFO, "Erase and Write Flash Start\n"));
>>> +
>>> +  Status = mSpiProtocol->EraseWrite (mSpiProtocol, (UINT32) RomAddress, (UINT8 *)Buffer, (UINT32) Length);
>>
>> Line length
>>
>>> +  if (EFI_ERROR (Status)) {
>>> +    DEBUG ((DEBUG_ERROR, "Erase and Write Status = %r \n", Status));
>>> +  }
>>> +
>>> +  return Status;
>>> +}
>>> +
>>> +/**
>>> +  Platform Flash Access Lib Constructor.
>>> +
>>> +  @param[in]  ImageHandle       The firmware allocated handle for the EFI image.
>>> +  @param[in]  SystemTable       A pointer to the EFI System Table.
>>> +
>>> +  @retval EFI_SUCCESS  Constructor returns successfully.
>>> +**/
>>> +EFI_STATUS
>>> +EFIAPI
>>> +PerformFlashAccessLibConstructor (
>>> +  IN EFI_HANDLE                         ImageHandle,
>>> +  IN EFI_SYSTEM_TABLE                   *SystemTable
>>> +  )
>>> +{
>>> +  EFI_STATUS  Status;
>>> +
>>> +  mInternalFdAddress = (EFI_PHYSICAL_ADDRESS)(UINTN) PcdGet64 (PcdFdBaseAddress);
>>> +
>>> +  mSFCMEM0BaseAddress = (EFI_PHYSICAL_ADDRESS)(UINTN) PcdGet64 (PcdSFCMEM0BaseAddress);
>>> +
>>
>> Drop the (UINTN) cast, EFI_PHYSICAL_ADDRESS is always 64 bits.
>>
>>> +  DEBUG ((DEBUG_INFO, "PcdFlashAreaBaseAddress - 0x%x, PcdSFCMEM0BaseAddress - 0x%x \n", mInternalFdAddress, mSFCMEM0BaseAddress));
>>> +
>>> +  Status = gBS->LocateProtocol (&gHisiSpiFlashProtocolGuid, NULL, (VOID **)&mSpiProtocol);
>>> +  if (EFI_ERROR (Status)) {
>>> +    DEBUG ((DEBUG_ERROR, "LocateProtocol gHisiSpiFlashProtocolGuid Status = %r \n", Status));
>>> +  }
>>> +
>>
>> Line length
>>
>>> +  return Status;
>>> +}
>>> diff --git a/Silicon/Hisilicon/Library/PlatformFlashAccessLib/PlatformFlashAccessLibDxe.inf b/Silicon/Hisilicon/Library/PlatformFlashAccessLib/PlatformFlashAccessLibDxe.inf
>>> new file mode 100644
>>> index 0000000..f4533ac
>>> --- /dev/null
>>> +++ b/Silicon/Hisilicon/Library/PlatformFlashAccessLib/PlatformFlashAccessLibDxe.inf
>>> @@ -0,0 +1,51 @@
>>> +## @file
>>> +#  Platform Flash Access library.
>>> +#
>>> +#  Copyright (c) 2018, Hisilicon Limited. All rights reserved.
>>> +#  Copyright (c) 2018, Linaro Limited. All rights reserved.
>>> +#  Copyright (c) 2016, Intel Corporation. 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
>>
>> 0x0000001A
>>
>>> +  BASE_NAME                      = PlatformFlashAccessLibDxe
>>> +  FILE_GUID                      = 9168384A-5F66-4CF7-AEB6-845BDEBD3012
>>
>> Use a fresh GUID
>>
>>> +  MODULE_TYPE                    = DXE_DRIVER
>>> +  VERSION_STRING                 = 1.0
>>> +  LIBRARY_CLASS                  = PlatformFlashAccessLib|DXE_DRIVER
>>> +  CONSTRUCTOR                    = PerformFlashAccessLibConstructor
>>> +
>>> +[Sources]
>>> +  PlatformFlashAccessLibDxe.c
>>> +
>>> +[Packages]
>>> +  ArmPkg/ArmPkg.dec
>>> +  MdeModulePkg/MdeModulePkg.dec
>>> +  MdePkg/MdePkg.dec
>>> +  SignedCapsulePkg/SignedCapsulePkg.dec
>>> +  Silicon/Hisilicon/HisiPkg.dec
>>> +
>>> +[LibraryClasses]
>>> +  BaseMemoryLib
>>> +  DebugLib
>>> +  PcdLib
>>> +  UefiBootServicesTableLib
>>> +
>>> +[Protocols]
>>> +  gHisiSpiFlashProtocolGuid
>>> +
>>> +[FixedPcd]
>>> +  gArmTokenSpaceGuid.PcdFdBaseAddress
>>> +  gHisiTokenSpaceGuid.PcdSFCMEM0BaseAddress
>>> +
>>> +[Depex]
>>> +  gHisiSpiFlashProtocolGuid
>>> --
>>> 1.9.1
>>>
>>
>> .
>>
>
> --
> Best Regards,
>
> Ming
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel