Re: [edk2-devel] [PATCH 0/2] Support FDT library.

["Michael D Kinney" <michael.d.kinney@intel.com>]

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Pedro Falcato
> Sent: Thursday, March 30, 2023 2:50 PM
> To: devel@edk2.groups.io; Lin, Benny <benny.lin@intel.com>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang
> <zhiguang.liu@intel.com>; Sean Brogan <sean.brogan@microsoft.com>; Michael Kubacki <mikuback@linux.microsoft.com>
> Subject: Re: [edk2-devel] [PATCH 0/2] Support FDT library.
> 
> 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>
> > Cc: Sean Brogan <sean.brogan@microsoft.com>
> > Cc: Michael Kubacki <mikuback@linux.microsoft.com>
> > Signed-off-by: Benny Lin <benny.lin@intel.com>
> >
> > Benny Lin (2):
> >   MdePkg: Support FDT library.
> >   .pytool: Support FDT library.
> >
> >  .gitmodules                               |   3 +
> >  .pytool/CISettings.py                     |   2 +
> >  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 +
> >  19 files changed, 988 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
> >
> > --
> > 2.39.1.windows.1
> 
> There's already a copy of libfdt plus "FdtLib" at
> https://github.com/tianocore/edk2/tree/master/EmbeddedPkg/Library/FdtLib.
> Please figure out what to do with it.
> It's an old copy and has been accidentally uncrustify'd so it's
> probably a good idea to at least ditch that specific copy for a git
> submodule.

I have discussed this overlap with Leif. After this patch series is 
added, the EmbeddedPkg maintainers can convert that package to use
this lib and delete the duplicate sources.

> 
> Also, NAK to Yet Another libc Implementation (and not a particularly
> good one at that).

Please provide constructive feedback on what is not good about this specific libc
Implementation so that appropriate code updates could be made for this patch series.

This is following the same pattern as other libs that are consuming a submodule
that requires some amount of libc support.

Getting down to one libc wrapper would be great.  Do you want to provide a proposed
implementation?  That could be handled as a separate task.

> 
> --
> Pedro
> 
> 
> 
>