Hi, Pedro, Thanks for your feedback. Sorry for the late response. Please find below my comment.
-----Original Message----- From: Pedro Falcato <pedro.falc...@gmail.com> Sent: Friday, March 31, 2023 7:19 AM To: devel@edk2.groups.io; Lin, Benny <benny....@intel.com> Cc: Kinney, Michael D <michael.d.kin...@intel.com>; Gao, Liming <gaolim...@byosoft.com.cn>; Liu, Zhiguang <zhiguang....@intel.com> Subject: Re: [edk2-devel] [PATCH 1/2] MdePkg: Support FDT library. On Thu, Mar 30, 2023 at 6:13 PM Benny Lin <benny....@intel.com> wrote: >> >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4392 >> Add FDT support in EDK2 by submodule 3rd party libfdt >> (https://github.com/devicetree-org/pylibfdt/tree/main/libfdt) >> >> Cc: Michael D Kinney <michael.d.kin...@intel.com> >> Cc: Liming Gao <gaolim...@byosoft.com.cn> >> Cc: Zhiguang Liu <zhiguang....@intel.com> >> Signed-off-by: Benny Lin <benny....@intel.com> >> --- >> .gitmodules | 3 + >> MdePkg/Include/Library/FdtLib.h | 300 ++++++++++++++++++++++ >> MdePkg/Library/BaseFdtLib/BaseFdtLib.inf | 62 +++++ >> MdePkg/Library/BaseFdtLib/BaseFdtLib.uni | 14 + >> MdePkg/Library/BaseFdtLib/FdtLib.c | 284 ++++++++++++++++++++ >> MdePkg/Library/BaseFdtLib/LibFdtSupport.h | 102 ++++++++ >> MdePkg/Library/BaseFdtLib/LibFdtWrapper.c | 138 ++++++++++ >> MdePkg/Library/BaseFdtLib/libfdt | 1 + >> MdePkg/Library/BaseFdtLib/limits.h | 10 + >> MdePkg/Library/BaseFdtLib/stdbool.h | 10 + >> MdePkg/Library/BaseFdtLib/stddef.h | 10 + >> MdePkg/Library/BaseFdtLib/stdint.h | 10 + >> MdePkg/Library/BaseFdtLib/stdlib.h | 10 + >> MdePkg/Library/BaseFdtLib/string.h | 10 + >> MdePkg/MdePkg.ci.yaml | 17 +- >> MdePkg/MdePkg.dec | 4 + >> MdePkg/MdePkg.dsc | 2 + >> ReadMe.rst | 1 + >> 18 files changed, 986 insertions(+), 2 deletions(-) create mode >> 100644 MdePkg/Include/Library/FdtLib.h create mode 100644 >> MdePkg/Library/BaseFdtLib/BaseFdtLib.inf >> create mode 100644 MdePkg/Library/BaseFdtLib/BaseFdtLib.uni >> create mode 100644 MdePkg/Library/BaseFdtLib/FdtLib.c >> create mode 100644 MdePkg/Library/BaseFdtLib/LibFdtSupport.h >> create mode 100644 MdePkg/Library/BaseFdtLib/LibFdtWrapper.c >> create mode 160000 MdePkg/Library/BaseFdtLib/libfdt create mode >> 100644 MdePkg/Library/BaseFdtLib/limits.h >> create mode 100644 MdePkg/Library/BaseFdtLib/stdbool.h >> create mode 100644 MdePkg/Library/BaseFdtLib/stddef.h >> create mode 100644 MdePkg/Library/BaseFdtLib/stdint.h >> create mode 100644 MdePkg/Library/BaseFdtLib/stdlib.h >> create mode 100644 MdePkg/Library/BaseFdtLib/string.h >> >> diff --git a/.gitmodules b/.gitmodules index 8011a88d9d..5da342e90c >> 100644 >> --- a/.gitmodules >> +++ b/.gitmodules >> @@ -23,3 +23,6 @@ >> [submodule "UnitTestFrameworkPkg/Library/GoogleTestLib/googletest"] >> path = UnitTestFrameworkPkg/Library/GoogleTestLib/googletest >> url = https://github.com/google/googletest.git >> +[submodule "MdePkg/Library/BaseFdtLib/libfdt"] >> + path = MdePkg/Library/BaseFdtLib/libfdt >> + url = https://github.com/devicetree-org/pylibfdt.git >> diff --git a/MdePkg/Include/Library/FdtLib.h >> b/MdePkg/Include/Library/FdtLib.h new file mode 100644 index >> 0000000000..bcb097b77e >> --- /dev/null >> +++ b/MdePkg/Include/Library/FdtLib.h >> @@ -0,0 +1,300 @@ >> +/** @file >> + Flattened Device Tree Library. >> + >> + Copyright (c) 2023, Intel Corporation. All rights reserved.<BR> >> + SPDX-License-Identifier: BSD-2-Clause-Patent >> + >> +**/ >> + >> +#ifndef FDT_LIB_H_ >> +#define FDT_LIB_H_ >> + >> +/// >> +/// Flattened Device Tree definition >> +/// >> +typedef struct { >> + UINT32 Magic; /* magic word FDT_MAGIC */ >> + UINT32 TotalSize; /* total size of DT block */ >> + UINT32 OffsetDtStruct; /* offset to structure */ >> + UINT32 OffsetDtStrings; /* offset to strings */ >> + UINT32 OffsetMemRsvmap; /* offset to memory reserve map */ >> + UINT32 Version; /* format version */ >> + UINT32 LastCompVersion; /* last compatible version */ >> + >> + /* version 2 fields below */ >> + UINT32 BootCpuidPhys; /* Which physical CPU id we're >> + booting on */ >> + /* version 3 fields below */ >> + UINT32 SizeDtStrings; /* size of the strings block */ >> + >> + /* version 17 fields below */ >> + UINT32 SizeDtStruct; /* size of the structure block */ >> +} FDT_HEADER; >> + >> +typedef struct { >> + UINT64 Address; >> + UINT64 Size; >> +} FDT_RESERVE_ENTRY; >> + >> +typedef struct { >> + UINT32 Tag; >> + CHAR8 Name[]; >> +} FDT_NODE_HEADER; >> + >> +typedef struct { >> + UINT32 Tag; >> + UINT32 Length; >> + UINT32 NameOffset; >> + CHAR8 Data[]; >> +} FDT_PROPERTY; >> + >> +#define FDT_GET_HEADER(Fdt, Field) FDT32_TO_CPU(((CONST FDT_HEADER >> +*)(Fdt))->Field) >> + >> +#define FDT_MAGIC(Fdt) (FDT_GET_HEADER(Fdt, Magic)) >> +#define FDT_TOTAL_SIZE(Fdt) (FDT_GET_HEADER(Fdt, TotalSize)) >> +#define FDT_OFFSET_DT_STRUCT(Fdt) (FDT_GET_HEADER(Fdt, OffsetDtStruct)) >> +#define FDT_OFFSET_DT_STRINGS(Fdt) (FDT_GET_HEADER(Fdt, >> +OffsetDtStrings)) #define FDT_OFFSET_MEM_RSVMAP(Fdt) (FDT_GET_HEADER(Fdt, >> OffsetMemRsvmap)) >> +#define FDT_VERSION(Fdt) (FDT_GET_HEADER(Fdt, Version)) >> +#define FDT_LAST_COMP_VERSION(Fdt) (FDT_GET_HEADER(Fdt, LastCompVersion)) >> +#define FDT_BOOT_CPUID_PHYS(Fdt) (FDT_GET_HEADER(Fdt, BootCpuidPhys)) >> +#define FDT_SIZE_DT_STRINGS(Fdt) (FDT_GET_HEADER(Fdt, SizeDtStrings)) >> +#define FDT_SIZE_DT_STRUCT(Fdt) (FDT_GET_HEADER(Fdt, SizeDtStruct)) >> + >> +/** >> + Create a empty Flattened Device Tree. >> + >> + @param[in] Buffer The pointer to allocate a pool for FDT blob. >> + @param[in] BufferSize The BufferSize to the pool size. >> + >> + @return Zero for successfully, otherwise failed. >> + >> +**/ >> +RETURN_STATUS >> +EFIAPI >> +FdtCreateEmptyTree ( >> + IN VOID *Buffer, >> + IN UINTN BufferSize >> + ); >> + >> +/** >> + Returns a offset of next node from the given node. >> + >> + @param[in] Fdt The pointer to FDT blob. >> + @param[in] Offset The offset to previous node. >> + @param[in] Depth The depth to the level of tree hierarchy. >> + >> + @return The offset to next node offset. >> + >> +**/ >> +INT32 >> +EFIAPI >> +FdtNextNode ( >> + IN CONST VOID *Fdt, >> + IN INT32 Offset, >> + IN INT32 *Depth >> + ); >> + >> +/** >> + Returns a offset of first node under the given node. >> + >> + @param[in] Fdt The pointer to FDT blob. >> + @param[in] Offset The offset to previous node. >> + >> + @return The offset to next node offset. >> + >> +**/ >> +INT32 >> +EFIAPI >> +FdtFirstSubnode ( >> + IN CONST VOID *Fdt, >> + IN INT32 Offset >> + ); >> + >> +/** >> + Returns a offset of next node from the given node. >> + >> + @param[in] Fdt The pointer to FDT blob. >> + @param[in] Offset The offset to previous node. >> + >> + @return The offset to next node offset. >> + >> +**/ >> +INT32 >> +EFIAPI >> +FdtNextSubnode ( >> + IN CONST VOID *Fdt, >> + IN INT32 Offset >> + ); >> + >> +/** >> + Returns a offset of first node which includes the given name. >> + >> + @param[in] Fdt The pointer to FDT blob. >> + @param[in] ParentOffset The offset to the node which start find under. >> + @param[in] Name The name to search the node with the name. >> + @param[in] NameLength The length of the name to check only. >> + >> + @return The offset to node offset with given node name. >> + >> +**/ >> +INT32 >> +EFIAPI >> +FdtSubnodeOffsetNameLen ( >> + IN CONST VOID *Fdt, >> + IN INT32 ParentOffset, >> + IN CONST CHAR8 *Name, >> + IN INT32 NameLength >> + ); >> + >> +/** >> + Returns a offset of first node which includes the given property name and >> value. >> + >> + @param[in] Fdt The pointer to FDT blob. >> + @param[in] StartOffset The offset to the starting node to find. >> + @param[in] PropertyName The property name to search the node including >> the named property. >> + @param[in] PropertyValue The property value to check the same property >> value. >> + @param[in] PropertyLength The length of the value in PropertValue. >> + >> + @return The offset to node offset with given property. >> + >> +**/ >> +INT32 >> +EFIAPI >> +FdtNodeOffsetByPropValue ( >> + IN CONST VOID *Fdt, >> + IN INT32 StartOffset, >> + IN CONST CHAR8 *PropertyName, >> + IN CONST VOID *PropertyValue, >> + IN INT32 PropertyLength >> + ); >> + >> +/** >> + Returns a property with the given name from the given node. >> + >> + @param[in] Fdt The pointer to FDT blob. >> + @param[in] NodeOffset The offset to the given node. >> + @param[in] Name The name to the property which need be searched >> + @param[in] Length The length to the size of the property found. >> + >> + @return The property to the structure of the found property. >> + >> +**/ >> +CONST FDT_PROPERTY * >> +EFIAPI >> +FdtGetProperty ( >> + IN CONST VOID *Fdt, >> + IN INT32 NodeOffset, >> + IN CONST CHAR8 *Name, >> + IN INT32 *Length >> + ); >> + >> +/** >> + Returns a offset of first property in the given node. >> + >> + @param[in] Fdt The pointer to FDT blob. >> + @param[in] Offset The offset to the node which need be searched. >> + >> + @return The offset to first property offset in the given node. >> + >> +**/ >> +INT32 >> +EFIAPI >> +FdtFirstPropertyOffset ( >> + IN CONST VOID *Fdt, >> + IN INT32 NodeOffset >> + ); >> + >> +/** >> + Returns a offset of next property from the given property. >> + >> + @param[in] Fdt The pointer to FDT blob. >> + @param[in] Offset The offset to previous property. >> + >> + @return The offset to next property offset. >> + >> +**/ >> +INT32 >> +EFIAPI >> +FdtNextPropertyOffset ( >> + IN CONST VOID *Fdt, >> + IN INT32 Offset >> + ); >> + >> +/** >> + Returns a property from the given offset of the property. >> + >> + @param[in] Fdt The pointer to FDT blob. >> + @param[in] Offset The offset to the given offset of the property. >> + @param[in] Length The length to the size of the property found. >> + >> + @return The property to the structure of the given property offset. >> + >> +**/ >> +CONST FDT_PROPERTY * >> +EFIAPI >> +FdtGetPropertyByOffset ( >> + IN CONST VOID *Fdt, >> + IN INT32 Offset, >> + IN INT32 *Length >> + ); >> + >> +/** >> + Returns a string by the given string offset. >> + >> + @param[in] Fdt The pointer to FDT blob. >> + @param[in] StrOffset The offset to the location in the strings block >> of FDT. >> + @param[in] Length The length to the size of string which need be >> retrieved. >> + >> + @return The string to the given string offset. >> + >> +**/ >> +CONST CHAR8 * >> +EFIAPI >> +FdtGetString ( >> + IN CONST VOID *Fdt, >> + IN INT32 StrOffset, >> + IN INT32 *Length OPTIONAL >> + ); >> + >> +/** >> + Add a new node to the FDT. >> + >> + @param[in] Fdt The pointer to FDT blob. >> + @param[in] ParentOffset The offset to the node offset which want to add >> in. >> + @param[in] Name The name to name the node. >> + >> + @return The offset to the new node. >> + >> +**/ >> +INT32 >> +EFIAPI >> +FdtAddSubnode ( >> + IN VOID *Fdt, >> + IN INT32 ParentOffset, >> + IN CONST CHAR8 *Name >> + ); >> + >> +/** >> + Add or modify a porperty in the given node. >> + >> + @param[in] Fdt The pointer to FDT blob. >> + @param[in] NodeOffset The offset to the node offset which want to add >> in. >> + @param[in] Name The name to name the property. >> + @param[in] Value The value to the property value. >> + @param[in] Length The length to the size of the property. >> + >> + @return Zero for successfully, otherwise failed. >> + >> +**/ >> +INT32 >> +EFIAPI >> +FdtSetProp ( >> + IN VOID *Fdt, >> + IN INT32 NodeOffset, >> + IN CONST CHAR8 *Name, >> + IN CONST VOID *Value, >> + IN UINT32 Length >> + ); >> + >> +#endif /* FDT_LIB_H_ */ >What's the point of all these wrappers if we end up immediately redirecting to >libfdt? The purpose of all wrappers is used for following up edk2 style. >> diff --git a/MdePkg/Library/BaseFdtLib/BaseFdtLib.inf >> b/MdePkg/Library/BaseFdtLib/BaseFdtLib.inf >> new file mode 100644 >> index 0000000000..730e568ff6 >> --- /dev/null >> +++ b/MdePkg/Library/BaseFdtLib/BaseFdtLib.inf >> @@ -0,0 +1,62 @@ >> +## @file >> +# Flattened Device Tree Library. >> +# >> +# Copyright (c) 2023, Intel Corporation. All rights reserved.<BR> # # >> +SPDX-License-Identifier: BSD-2-Clause-Patent # # ## >> + >> +[Defines] >> + INF_VERSION = 0x0001001B >> + BASE_NAME = BaseFdtLib >> + MODULE_UNI_FILE = BaseFdtLib.uni >> + FILE_GUID = C64DCB01-B037-4FF6-9CF3-E8CEC206DE04 >> + MODULE_TYPE = BASE >> + VERSION_STRING = 1.0 >> + LIBRARY_CLASS = FdtLib >> + >> + DEFINE FDT_LIB_PATH = libfdt/libfdt >> + >> +# >> +# VALID_ARCHITECTURES = IA32 X64 >> +# >> + >> +[Sources] >> + FdtLib.c >> + LibFdtWrapper.c >> + # header Wrapper files >> + limits.h >> + stdbool.h >> + stddef.h >> + stdint.h >> + stdlib.h >> + string.h >> + >> + $(FDT_LIB_PATH)/fdt.c >> + $(FDT_LIB_PATH)/fdt.h >> + $(FDT_LIB_PATH)/fdt_addresses.c >> + $(FDT_LIB_PATH)/fdt_check.c >> + $(FDT_LIB_PATH)/fdt_empty_tree.c >> + $(FDT_LIB_PATH)/fdt_overlay.c >> + $(FDT_LIB_PATH)/fdt_ro.c >> + $(FDT_LIB_PATH)/fdt_rw.c >> + $(FDT_LIB_PATH)/fdt_strerror.c >> + $(FDT_LIB_PATH)/fdt_sw.c >> + $(FDT_LIB_PATH)/fdt_wip.c >> + $(FDT_LIB_PATH)/libfdt.h >> + $(FDT_LIB_PATH)/libfdt_env.h >> + $(FDT_LIB_PATH)/libfdt_internal.h >> + >> +[Packages] >> + MdePkg/MdePkg.dec >> + >> +[LibraryClasses] >> + BaseLib >> + BaseMemoryLib >> + >> +[BuildOptions] >> + MSFT:*_*_IA32_CC_FLAGS = /wd4146 /wd4245 >> + MSFT:*_*_X64_CC_FLAGS = /wd4146 /wd4244 /wd4245 /wd4267 >> + >> diff --git a/MdePkg/Library/BaseFdtLib/BaseFdtLib.uni >> b/MdePkg/Library/BaseFdtLib/BaseFdtLib.uni >> new file mode 100644 >> index 0000000000..3f7e45ea6f >> --- /dev/null >> +++ b/MdePkg/Library/BaseFdtLib/BaseFdtLib.uni >> @@ -0,0 +1,14 @@ >> +// /** @file >> +// Flattened Device Tree Library. >> +// >> +// Copyright (c) 2023, Intel Corporation. All rights reserved.<BR> // >> +// SPDX-License-Identifier: BSD-2-Clause-Patent // // **/ >> + >> + >> +#string STR_MODULE_ABSTRACT #language en-US "Instance of FDT >> Library" >> + >> +#string STR_MODULE_DESCRIPTION #language en-US "This module >> provides FDT Library implementation." >> + >> diff --git a/MdePkg/Library/BaseFdtLib/FdtLib.c >> b/MdePkg/Library/BaseFdtLib/FdtLib.c >> new file mode 100644 >> index 0000000000..ba9a284e58 >> --- /dev/null >> +++ b/MdePkg/Library/BaseFdtLib/FdtLib.c >> @@ -0,0 +1,284 @@ >> +/** @file >> + Flattened Device Tree Library. >> + >> + Copyright (c) 2023, Intel Corporation. All rights reserved.<BR> >> + SPDX-License-Identifier: BSD-2-Clause-Patent >> + >> +**/ >> + >> +#include <libfdt/libfdt/libfdt.h> >> + >> +/** >> + Create a empty Flattened Device Tree. >> + >> + @param[in] Buffer The pointer to allocate a pool for FDT blob. >> + @param[in] BufferSize The BufferSize to the pool size. >> + >> + @return Zero for successfully, otherwise failed. >> + >> +**/ >> +INT32 >> +EFIAPI >> +FdtCreateEmptyTree ( >> + IN VOID *Buffer, >> + IN UINT32 BufferSize >> + ) >> +{ >> + return fdt_create_empty_tree (Buffer, (int)BufferSize); } >> + >> +/** >> + Returns a offset of next node from the given node. >> + >> + @param[in] Fdt The pointer to FDT blob. >> + @param[in] Offset The offset to previous node. >> + @param[in] Depth The depth to the level of tree hierarchy. >> + >> + @return The offset to next node offset. >> + >> +**/ >> +INT32 >> +EFIAPI >> +FdtNextNode ( >> + IN CONST VOID *Fdt, >> + IN INT32 Offset, >> + IN INT32 *Depth >> + ) >> +{ >> + return fdt_next_node (Fdt, Offset, Depth); } >> + >> +/** >> + Returns a offset of first node under the given node. >> + >> + @param[in] Fdt The pointer to FDT blob. >> + @param[in] Offset The offset to previous node. >> + >> + @return The offset to next node offset. >> + >> +**/ >> +INT32 >> +EFIAPI >> +FdtFirstSubnode ( >> + IN CONST VOID *Fdt, >> + IN INT32 Offset >> + ) >> +{ >> + return fdt_first_subnode (Fdt, Offset); } >> + >> +/** >> + Returns a offset of next node from the given node. >> + >> + @param[in] Fdt The pointer to FDT blob. >> + @param[in] Offset The offset to previous node. >> + >> + @return The offset to next node offset. >> + >> +**/ >> +INT32 >> +EFIAPI >> +FdtNextSubnode ( >> + IN CONST VOID *Fdt, >> + IN INT32 Offset >> + ) >> +{ >> + return fdt_next_subnode (Fdt, Offset); } >> + >> +/** >> + Returns a offset of first node which includes the given name. >> + >> + @param[in] Fdt The pointer to FDT blob. >> + @param[in] ParentOffset The offset to the node which start find under. >> + @param[in] Name The name to search the node with the name. >> + @param[in] NameLength The length of the name to check only. >> + >> + @return The offset to node offset with given node name. >> + >> +**/ >> +INT32 >> +EFIAPI >> +FdtSubnodeOffsetNameLen ( >> + IN CONST VOID *Fdt, >> + IN INT32 ParentOffset, >> + IN CONST CHAR8 *Name, >> + IN INT32 NameLength >> + ) >> +{ >> + return fdt_subnode_offset_namelen (Fdt, ParentOffset, Name, >> +NameLength); } >> + >> +/** >> + Returns a offset of first node which includes the given property name and >> value. >> + >> + @param[in] Fdt The pointer to FDT blob. >> + @param[in] StartOffset The offset to the starting node to find. >> + @param[in] PropertyName The property name to search the node including >> the named property. >> + @param[in] PropertyValue The property value to check the same property >> value. >> + @param[in] PropertyLength The length of the value in PropertValue. >> + >> + @return The offset to node offset with given property. >> + >> +**/ >> +INT32 >> +EFIAPI >> +FdtNodeOffsetByPropValue ( >> + IN CONST VOID *Fdt, >> + IN INT32 StartOffset, >> + IN CONST CHAR8 *PropertyName, >> + IN CONST VOID *PropertyValue, >> + IN INT32 PropertyLength >> + ) >> +{ >> + return fdt_node_offset_by_prop_value (Fdt, StartOffset, >> +PropertyName, PropertyValue, PropertyLength); } >> + >> +/** >> + Returns a property with the given name from the given node. >> + >> + @param[in] Fdt The pointer to FDT blob. >> + @param[in] NodeOffset The offset to the given node. >> + @param[in] Name The name to the property which need be searched >> + @param[in] Length The length to the size of the property found. >> + >> + @return The property to the structure of the found property. >> + >> +**/ >> +CONST struct fdt_property * >> +EFIAPI >> +FdtGetProperty ( >> + IN CONST VOID *Fdt, >> + IN INT32 NodeOffset, >> + IN CONST CHAR8 *Name, >> + IN INT32 *Length >> + ) >> +{ >> + return fdt_get_property (Fdt, NodeOffset, Name, Length); } >> + >> +/** >> + Returns a offset of first property in the given node. >> + >> + @param[in] Fdt The pointer to FDT blob. >> + @param[in] NodeOffset The offset to the node which need be searched. >> + >> + @return The offset to first property offset in the given node. >> + >> +**/ >> +INT32 >> +EFIAPI >> +FdtFirstPropertyOffset ( >> + IN CONST VOID *Fdt, >> + IN INT32 NodeOffset >> + ) >> +{ >> + return fdt_first_property_offset (Fdt, NodeOffset); } >> + >> +/** >> + Returns a offset of next property from the given property. >> + >> + @param[in] Fdt The pointer to FDT blob. >> + @param[in] Offset The offset to previous property. >> + >> + @return The offset to next property offset. >> + >> +**/ >> +INT32 >> +EFIAPI >> +FdtNextPropertyOffset ( >> + IN CONST VOID *Fdt, >> + IN INT32 Offset >> + ) >> +{ >> + return fdt_next_property_offset (Fdt, Offset); } >> + >> +/** >> + Returns a property from the given offset of the property. >> + >> + @param[in] Fdt The pointer to FDT blob. >> + @param[in] Offset The offset to the given offset of the property. >> + @param[in] Length The length to the size of the property found. >> + >> + @return The property to the structure of the given property offset. >> + >> +**/ >> +CONST struct fdt_property * >> +EFIAPI >> +FdtGetPropertyByOffset ( >> + IN CONST VOID *Fdt, >> + IN INT32 Offset, >> + IN INT32 *Length >> + ) >> +{ >> + return fdt_get_property_by_offset (Fdt, Offset, Length); } >> + >> +/** >> + Returns a string by the given string offset. >> + >> + @param[in] Fdt The pointer to FDT blob. >> + @param[in] StrOffset The offset to the location in the strings block >> of FDT. >> + @param[in] Length The length to the size of string which need be >> retrieved. >> + >> + @return The string to the given string offset. >> + >> +**/ >> +CONST CHAR8 * >> +EFIAPI >> +FdtGetString ( >> + IN CONST VOID *Fdt, >> + IN INT32 StrOffset, >> + IN INT32 *Length OPTIONAL >> + ) >> +{ >> + return fdt_get_string (Fdt, StrOffset, Length); } >> + >> +/** >> + Add a new node to the FDT. >> + >> + @param[in] Fdt The pointer to FDT blob. >> + @param[in] ParentOffset The offset to the node offset which want to add >> in. >> + @param[in] Name The name to name the node. >> + >> + @return The offset to the new node. >> + >> +**/ >> +INT32 >> +EFIAPI >> +FdtAddSubnode ( >> + IN VOID *Fdt, >> + IN INT32 ParentOffset, >> + IN CONST CHAR8 *Name >> + ) >> +{ >> + return fdt_add_subnode (Fdt, ParentOffset, Name); } >> + >> +/** >> + Add or modify a porperty in the given node. >*property Corrected in Patch V2. >> + >> + @param[in] Fdt The pointer to FDT blob. >> + @param[in] NodeOffset The offset to the node offset which want to add >> in. >> + @param[in] Name The name to name the property. >> + @param[in] Value The value to the property value. >> + @param[in] Length The length to the size of the property. >> + >> + @return Zero for successfully, otherwise failed. >> + >> +**/ >> +INT32 >> +EFIAPI >> +FdtSetProp ( >> + IN VOID *Fdt, >> + IN INT32 NodeOffset, >> + IN CONST CHAR8 *Name, >> + IN CONST VOID *Value, >> + IN UINT32 Length >> + ) >> +{ >> + return fdt_setprop (Fdt, NodeOffset, Name, Value, (int)Length); } >> diff --git a/MdePkg/Library/BaseFdtLib/LibFdtSupport.h >> b/MdePkg/Library/BaseFdtLib/LibFdtSupport.h >> new file mode 100644 >> index 0000000000..58b0bb403e >> --- /dev/null >> +++ b/MdePkg/Library/BaseFdtLib/LibFdtSupport.h >> @@ -0,0 +1,102 @@ >> +/** @file >> + Root include file of C runtime library to support building the >> +third-party >> + libfdt library. >> + >> + Copyright (c) 2023, Intel Corporation. All rights reserved.<BR> >> + SPDX-License-Identifier: BSD-2-Clause-Patent >> + >> +**/ >> + >> +#ifndef FDT_LIB_SUPPORT_H_ >> +#define FDT_LIB_SUPPORT_H_ >> + >> +#include <Base.h> >> +#include <Library/BaseLib.h> >> +#include <Library/BaseMemoryLib.h> >> + >> +typedef UINT8 uint8_t; >> +typedef UINT16 uint16_t; >> +typedef INT32 int32_t; >> +typedef UINT32 uint32_t; >> +typedef UINT64 uint64_t; >> +typedef UINTN uintptr_t; >> +typedef UINTN size_t; >> +typedef BOOLEAN bool; >> + >> +#define true (1 == 1) >> +#define false (1 == 0) >> + >> +// >> +// Definitions for global constants used by libfdt library routines >> +// #ifndef INT_MAX >> +#define INT_MAX 0x7FFFFFFF /* Maximum (signed) int value */ >> +#endif >Why the ifndef here? Same with below. >> +#define INT32_MAX 0x7FFFFFFF /* Maximum (signed) int32 value */ >> +#define UINT32_MAX 0xFFFFFFFF /* Maximum unsigned int32 value */ >> +#define MAX_STRING_SIZE 0x1000 >Why the arbitrary MAX_STRING_SIZE here? Because there're many standard c wrappers, the sequence of include paths will Influence each other. At first, I made a mistake with wrong include path so I got build failed with lacked definitions. However, I forgot to check those definitions when corrected include path. Remove no use definitions and #ifndef in Patch V2. >> + >> +// >> +// Function prototypes of libfdt Library routines // void * >> +memset ( >> + void *, >> + int, >> + size_t >> + ); >> + >> +int >> +memcmp ( >> + const void *, >> + const void *, >> + size_t >> + ); >> + >> +int >> +strcmp ( >> + const char *, >> + const char * >> + ); >> + >> +char * >> +strchr ( >> + const char *, >> + int >> + ); >> + >> +char * >> +strrchr ( >> + const char *, >> + int >> + ); >> + >> +unsigned long >> +strtoul ( >> + const char *, >> + char **, >> + int >> + ); >> + >> +char * >> +strcpy ( >> + char *strDest, >> + const char *strSource >> + ); >> + >> +// >> +// Macros that directly map functions to BaseLib, BaseMemoryLib, and >> +DebugLib functions // >> +#define memcpy(dest, source, count) CopyMem(dest,source, >> (UINTN)(count)) >> +#define memset(dest, ch, count) SetMem(dest, >> (UINTN)(count),(UINT8)(ch)) >> +#define memchr(buf, ch, count) ScanMem8(buf, >> (UINTN)(count),(UINT8)ch) >> +#define memcmp(buf1, buf2, count) (int)(CompareMem(buf1, buf2, >> (UINTN)(count))) >> +#define memmove(dest, source, count) CopyMem(dest, source, >> (UINTN)(count)) >> +#define strlen(str) (size_t)(AsciiStrLen(str)) >> +#define strnlen(str, count) (size_t)(AsciiStrnLenS(str, >> count)) >> +#define strncpy(strDest, strSource, count) AsciiStrnCpyS(strDest, >> MAX_STRING_SIZE, strSource, (UINTN)count) >> +#define strcat(strDest, strSource) AsciiStrCatS(strDest, >> MAX_STRING_SIZE, strSource) >> +#define strcmp(string1, string2, count) (int)(AsciiStrCmp(string1, >> string2)) >> +#define strncmp(string1, string2, count) (int)(AsciiStrnCmp(string1, >> string2, (UINTN)(count))) >> + >> +#endif /* FDT_LIB_SUPPORT_H_ */ >> diff --git a/MdePkg/Library/BaseFdtLib/LibFdtWrapper.c >> b/MdePkg/Library/BaseFdtLib/LibFdtWrapper.c >> new file mode 100644 >> index 0000000000..3f1cc69dc6 >> --- /dev/null >> +++ b/MdePkg/Library/BaseFdtLib/LibFdtWrapper.c >> @@ -0,0 +1,138 @@ >> +/** @file >> + Root include file of C runtime library to support building the >> +third-party >> + libfdt library. >> + >> + Copyright (c) 2023, Intel Corporation. All rights reserved.<BR> >> + SPDX-License-Identifier: BSD-2-Clause-Patent >> + >> +**/ >> + >> +#include "LibFdtSupport.h" >> +#include <Library/DebugLib.h> >Why are we pulling DebugLib? I don't see where this should be needed. Yes, we do not need. Forgot removal after debugging. Remove it in Patch V2. >> + >> +/** >> + Returns the first occurrence of a Null-terminated ASCII character >> + in a Null-terminated ASCII string. >> + >> + This function scans the contents of the ASCII string specified by s >> + and returns the first occurrence of c. If c is not found in s, then >> + NULL is returned. If the length of c is zero, then s is returned. >> + >> + @param s The pointer to a Null-terminated ASCII string. >> + @param c The pointer to a Null-terminated ASCII character >> to search for. >> + >> + @retval NULL If the c does not appear in s. >> + @retval others If there is a match return the first occurrence >> of c. >> + If the length of c is zero,return s. >The doxygen for strchr and strrchr is very wrong and is not ISO C (nor GNU C >nor any other brand of C) compliant. Please refer to a >Linux/macOS/BSD >manpage or to an ISO C standard draft. Remove doxygen in Patch V2. >> + >> +**/ >> +char * >> +strchr ( >> + const char *s, >> + int c >> + ) >> +{ >> + char pattern[2]; >> + >> + pattern[0] = (CHAR8)c; >> + pattern[1] = 0; >> + return AsciiStrStr (s, pattern); >> +} >> + >> +/** >> + Returns the last occurrence of a Null-terminated ASCII character >> + in a Null-terminated ASCII string. >> + >> + This function scans the contents of the ASCII string specified by s >> + and returns the last occurrence of c. If c is not found in s, then >> + NULL is returned. If the length of c is zero, then s is returned. >> + >> + @param s The pointer to a Null-terminated ASCII string. >> + @param c The pointer to a Null-terminated ASCII character >> to search for. >> + >> + @retval NULL If the c does not appear in s. >> + @retval others If there is a match return the last occurrence of >> c. >> + If the length of c is zero,return s. >> + >> +**/ >> +char * >> +strrchr ( >> + const char *s, >> + int c >> + ) >> +{ >> + char pattern[2]; >> + CONST CHAR8 *LastMatch; >> + CONST CHAR8 *StringTmp; >> + CONST CHAR8 *SearchString; >> + >> + pattern[0] = (CHAR8)c; >> + pattern[1] = 0; >> + SearchString = pattern; >> + >> + // >> + // Return NULL if both strings are less long than >> + PcdMaximumAsciiStringLength // if ((AsciiStrSize (s) == 0) || >> + (AsciiStrSize (SearchString) == 0)) { >> + return NULL; >> + } >> + >> + if (*SearchString == '\0') { >> + return (CHAR8 *)s; >> + } >> + >> + LastMatch = NULL; >> + do { >> + StringTmp = AsciiStrStr (s, SearchString); >> + >> + if (StringTmp == NULL) { >> + break; >> + } >> + >> + LastMatch = StringTmp; >> + s = StringTmp + 1; >> + } while (s != NULL); >> + >> + return (CHAR8 *)LastMatch; >> +} >Better implementations I wrote very quickly, feel free to EFI-it, public >domain: >https://gist.github.com/heatd/c1192f3f05148a6271be37fb3160b143 In my understanding, I should use those functions provided by edk2 to implement. Looks like we do not need so I integrated your implementations from libclib. When integrated libclib functions to wrapper file, I can build successfully with MSVC. However, I also test with integrated whole changes of libclib. Build failed occurred. I will keep checking. >> + >> +/** >> + Convert a Null-terminated Ascii decimal or hexadecimal string to a value >> of type UINTN. >> + >> + This function outputs a value of type UINTN by interpreting the >> + contents of the Ascii string specified by String as a decimal or >> hexadecimal number. >> + >> + @param nptr Pointer to a Null-terminated Ascii >> string. >> + @param endptr Pointer to character that stops scan. >> + @param base Pointer to decimal or hexadecimal. >> + >> + @retval MAX_UINTN If nptr is NULL. >> + If endptr is NULL. >> + If base is not 10 or 16. >> + @retval others Value is translated from String. >> + >> +**/ >> +unsigned long >> +strtoul ( >> + const char *nptr, >> + char **endptr, >> + int base >> + ) >> +{ >> + RETURN_STATUS Status; >> + UINTN ReturnValue; >There's no equivalence between UINTN and unsigned long on x64 Windows (where >they use LLP64, so 32-bit longs, while UINTN should be 64->bit). Keep unsigned long in Patch V2. >> + >> + if (base == 10) { >> + Status = AsciiStrDecimalToUintnS (nptr, endptr, &ReturnValue); } >> + else if (base == 16) { >> + Status = AsciiStrHexToUintnS (nptr, endptr, &ReturnValue); } >> + else { >> + Status = RETURN_INVALID_PARAMETER; } >> + >> + if (RETURN_ERROR (Status)) { >> + return MAX_UINTN; >> + } >> + >> + return (unsigned long)ReturnValue; >> +} >Note that this is not ISO C compliant, but seems to work in the libfdt case >(as far as I can see, only a single strtoul call with base 10). This will be replaced with libclib functions. >> diff --git a/MdePkg/Library/BaseFdtLib/libfdt >> b/MdePkg/Library/BaseFdtLib/libfdt >> new file mode 160000 >> index 0000000000..cfff805481 >> --- /dev/null >> +++ b/MdePkg/Library/BaseFdtLib/libfdt >> @@ -0,0 +1 @@ >> +Subproject commit cfff805481bdea27f900c32698171286542b8d3c >> diff --git a/MdePkg/Library/BaseFdtLib/limits.h >> b/MdePkg/Library/BaseFdtLib/limits.h >> new file mode 100644 >> index 0000000000..f6cf8d5702 >> --- /dev/null >> +++ b/MdePkg/Library/BaseFdtLib/limits.h >> @@ -0,0 +1,10 @@ >> +/** @file >> + Include file to support building the third-party libfdt library. >> + >> +Copyright (c) 2023, Intel Corporation. All rights reserved.<BR> >> +SPDX-License-Identifier: BSD-2-Clause-Patent >> + >> +**/ >> + >> +#include <LibFdtSupport.h> >> + >> diff --git a/MdePkg/Library/BaseFdtLib/stdbool.h >> b/MdePkg/Library/BaseFdtLib/stdbool.h >> new file mode 100644 >> index 0000000000..f6cf8d5702 >> --- /dev/null >> +++ b/MdePkg/Library/BaseFdtLib/stdbool.h >> @@ -0,0 +1,10 @@ >> +/** @file >> + Include file to support building the third-party libfdt library. >> + >> +Copyright (c) 2023, Intel Corporation. All rights reserved.<BR> >> +SPDX-License-Identifier: BSD-2-Clause-Patent >> + >> +**/ >> + >> +#include <LibFdtSupport.h> >If we want to be rigorous, strictly speaking, we should not pollute the >namespace. stdbool.h should only include the stdbool.h stuff (see the ISO >C >spec), and the same applies for the other headers. >Although I *guess* it doesn't matter much here. I agree with you and I think libclib is better for edk2. >-- >Pedro Benny -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102896): https://edk2.groups.io/g/devel/message/102896 Mute This Topic: https://groups.io/mt/97955739/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
