From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=45.249.212.35; helo=huawei.com; envelope-from=huangming23@huawei.com; receiver=edk2-devel@lists.01.org Received: from huawei.com (unknown [45.249.212.35]) (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 37AF522364890 for ; Tue, 30 Jan 2018 17:08:50 -0800 (PST) Received: from DGGEMS414-HUB.china.huawei.com (unknown [172.30.72.59]) by Forcepoint Email with ESMTP id 414DD7AEC3526; Wed, 31 Jan 2018 09:14:22 +0800 (CST) Received: from [127.0.0.1] (10.61.17.224) by DGGEMS414-HUB.china.huawei.com (10.3.19.214) with Microsoft SMTP Server id 14.3.361.1; Wed, 31 Jan 2018 09:14:13 +0800 To: Leif Lindholm References: <1516953650-57980-1-git-send-email-huangming23@huawei.com> <1516953650-57980-6-git-send-email-huangming23@huawei.com> <20180129195859.jkfhnxzpn5s5isgt@bivouac.eciton.net> <47d46308-ff52-af8e-15ca-4bf96828118b@huawei.com> <20180130132125.h3nwz4b3yjyeyeik@bivouac.eciton.net> CC: Ming Huang , , , , , , , , , , From: "Huangming (Mark)" Message-ID: Date: Wed, 31 Jan 2018 09:14:13 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <20180130132125.h3nwz4b3yjyeyeik@bivouac.eciton.net> X-Originating-IP: [10.61.17.224] X-CFilter-Loop: Reflected Subject: Re: [PATCH edk2-platforms v2 05/15] Hisilicon D03/D05: Add capsule upgrade support X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 31 Jan 2018 01:08:51 -0000 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit On 2018/1/30 21:21, Leif Lindholm wrote: > On Tue, Jan 30, 2018 at 08:48:27PM +0800, Huangming (Mark) wrote: >> >> >> On 2018/1/30 3:58, Leif Lindholm wrote: >>> A few style comments below. >>> >>> On Fri, Jan 26, 2018 at 04:00:40PM +0800, Ming Huang wrote: >>>> From: Jason Zhang >>>> >>>> This module support updating the boot CPU firmware only. >>>> >>>> Contributed-under: TianoCore Contribution Agreement 1.1 >>>> Signed-off-by: Jason Zhang >>>> Signed-off-by: Ming Huang >>>> Signed-off-by: Heyi Guo >>>> --- >>>> Platform/Hisilicon/D03/Capsule/SystemFirmwareUpdateConfig/SystemFirmwareUpdateConfig.ini | 45 +++++++ >>>> Platform/Hisilicon/D03/D03.dsc | 17 ++- >>>> Platform/Hisilicon/D03/D03.fdf | 70 +++++++++++ >>>> Platform/Hisilicon/D03/Drivers/SystemFirmwareDescriptor/SystemFirmwareDescriptor.aslc | 81 +++++++++++++ >>>> Platform/Hisilicon/D03/Drivers/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf | 50 ++++++++ >>>> Platform/Hisilicon/D03/Drivers/SystemFirmwareDescriptor/SystemFirmwareDescriptorPei.c | 70 +++++++++++ >>>> Platform/Hisilicon/D05/Capsule/SystemFirmwareUpdateConfig/SystemFirmwareUpdateConfig.ini | 45 +++++++ >>>> Platform/Hisilicon/D05/D05.dsc | 19 ++- >>>> Platform/Hisilicon/D05/D05.fdf | 70 +++++++++++ >>>> Platform/Hisilicon/D05/Drivers/SystemFirmwareDescriptor/SystemFirmwareDescriptor.aslc | 81 +++++++++++++ >>>> Platform/Hisilicon/D05/Drivers/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf | 50 ++++++++ >>>> Platform/Hisilicon/D05/Drivers/SystemFirmwareDescriptor/SystemFirmwareDescriptorPei.c | 70 +++++++++++ >>>> Silicon/Hisilicon/Hisilicon.dsc.inc | 11 +- >>>> Silicon/Hisilicon/Hisilicon.fdf.inc | 9 ++ >>>> Silicon/Hisilicon/Library/PlatformFlashAccessLib/PlatformFlashAccessLibDxe.c | 123 ++++++++++++++++++++ >>>> Silicon/Hisilicon/Library/PlatformFlashAccessLib/PlatformFlashAccessLibDxe.inf | 51 ++++++++ >>>> 16 files changed, 859 insertions(+), 3 deletions(-) >>>> >>> >>>> diff --git a/Platform/Hisilicon/D03/Drivers/SystemFirmwareDescriptor/SystemFirmwareDescriptor.aslc b/Platform/Hisilicon/D03/Drivers/SystemFirmwareDescriptor/SystemFirmwareDescriptor.aslc >>>> new file mode 100644 >>>> index 0000000..d9f4a00 >>>> --- /dev/null >>>> +++ b/Platform/Hisilicon/D03/Drivers/SystemFirmwareDescriptor/SystemFirmwareDescriptor.aslc >>>> @@ -0,0 +1,81 @@ >>>> +/** @file >>>> + System Firmware descriptor. >>>> + >>>> + Copyright (c) 2018, Hisilicon Limited. All rights reserved. >>>> + Copyright (c) 2018, Linaro Limited. All rights reserved. >>>> + Copyright (c) 2016, Intel Corporation. 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 >>>> + >>>> +#define PACKAGE_VERSION 0xFFFFFFFF >>>> +#define PACKAGE_VERSION_STRING L"Unknown" >>>> + >>>> +#define CURRENT_FIRMWARE_VERSION 0x00000002 >>>> +#define CURRENT_FIRMWARE_VERSION_STRING L"0x00000002" >>>> +#define LOWEST_SUPPORTED_FIRMWARE_VERSION 0x00000001 >>>> + >>>> +#define IMAGE_ID SIGNATURE_64('H','W','A', 'R', 'M', '_', 'F', 'd') >>>> +#define IMAGE_ID_STRING L"ARMPlatformFd" >>>> + >>>> +// PcdSystemFmpCapsuleImageTypeIdGuid >>>> +#define IMAGE_TYPE_ID_GUID { 0x44c850f2, 0x85ff, 0x4be5, { 0xbf, 0x34, 0xa5, 0x95, 0x28, 0xdf, 0x22, 0xd3 } } >>>> + >>>> +typedef struct { >>>> + EDKII_SYSTEM_FIRMWARE_IMAGE_DESCRIPTOR Descriptor; >>>> + // real string data >>>> + CHAR16 ImageIdNameStr[sizeof(IMAGE_ID_STRING) / sizeof(CHAR16)]; >>>> + CHAR16 VersionNameStr[sizeof(CURRENT_FIRMWARE_VERSION_STRING) / sizeof(CHAR16)]; >>>> + CHAR16 PackageVersionNameStr[sizeof(PACKAGE_VERSION_STRING) / sizeof(CHAR16)]; >>> >>> Use ARRAY_SIZE for the 3 above? >> >> If use ARRAY_SIZE, the three macro must change to array of CHAR16. > > What problem are you encountering when simply replacing > CHAR16 ImageIdNameStr[sizeof(IMAGE_ID_STRING) / sizeof(CHAR16)]; > with > CHAR16 ImageIdNameStr[ARRAY_SIZE (IMAGE_ID_STRING)]; > ? > In my mind, simply replacing can not be compiled, but actually it works. I will replace it as your comments. Thanks, Ming >> It is not necessary maybe.The same style is found in >> SystemFirmwareDescriptor.aslc from other platform,like >> AMD,Socionext. > > Patches welcome. > > ARRAY_SIZE was only added to MdePkg in October 2016. > Ideally, all code should be using this macro, but at least we can try > to make sure all _new_ code uses it, when we spot missed opportunities. > > / > Leif > > . > -- Best Regards, Ming