public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Pedro Falcato" <pedro.falcato@gmail.com>
To: devel@edk2.groups.io, benny.lin@intel.com
Cc: Michael D Kinney <michael.d.kinney@intel.com>,
	Liming Gao <gaoliming@byosoft.com.cn>,
	 Zhiguang Liu <zhiguang.liu@intel.com>
Subject: Re: [edk2-devel] [PATCH 1/2] MdePkg: Support FDT library.
Date: Fri, 31 Mar 2023 00:19:03 +0100	[thread overview]
Message-ID: <CAKbZUD22bEKPcj6oBjMz8v_WpuRshtZ9HDRazJZ_G7GXZK1B9A@mail.gmail.com> (raw)
In-Reply-To: <ff3ca197ad830617e71e8db50b08c1b355cb89e8.1680186454.git.benny.lin@intel.com>

On Thu, Mar 30, 2023 at 6:13 PM Benny Lin <benny.lin@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.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Signed-off-by: Benny Lin <benny.lin@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?

> 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

> +
> +  @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?
> +#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?
> +
> +//
> +// 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.
> +
> +/**
> +  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.
> +
> +**/
> +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

> +
> +/**
> +  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).
> +
> +  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).

> 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.

-- 
Pedro

  parent reply	other threads:[~2023-03-30 23:19 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-30 16:52 [PATCH 0/2] Support FDT library benny.lin
2023-03-30 16:52 ` [PATCH 1/2] MdePkg: " Benny Lin
2023-03-30 17:01   ` Michael D Kinney
2023-03-30 23:19   ` Pedro Falcato [this message]
2023-04-12 16:59     ` [edk2-devel] " Benny Lin
2023-03-30 16:52 ` [PATCH 2/2] .pytool: " Benny Lin
2023-03-30 21:50 ` [edk2-devel] [PATCH 0/2] " Pedro Falcato
2023-03-30 22:59   ` Michael D Kinney
2023-03-30 23:26     ` Pedro Falcato
2023-03-30 23:32       ` Michael D Kinney
2023-03-31  2:35         ` Pedro Falcato
2023-03-31 11:39       ` Gerd Hoffmann
2023-03-31 12:18         ` Pedro Falcato
2023-03-31 12:17       ` Leif Lindholm
2023-03-31 12:12   ` Leif Lindholm
2023-04-01  1:29 ` Andrei Warkentin
2023-04-06 16:33   ` Sheng Lean Tan
2023-04-07 13:23     ` Pedro Falcato
2023-04-07 22:35       ` Andrei Warkentin
2023-04-07 23:04         ` Michael D Kinney
2023-04-11 13:19           ` Sheng Lean Tan
2023-04-11 16:07             ` Pedro Falcato
2023-04-11 17:00               ` Michael D Kinney

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAKbZUD22bEKPcj6oBjMz8v_WpuRshtZ9HDRazJZ_G7GXZK1B9A@mail.gmail.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox