public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Pedro Falcato" <pedro.falcato@gmail.com>
To: "Kinney, Michael D" <michael.d.kinney@intel.com>
Cc: "devel@edk2.groups.io" <devel@edk2.groups.io>,
	"Lin, Benny" <benny.lin@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>,
	 Leif Lindholm <llindhol@qti.qualcomm.com>
Subject: Re: [edk2-devel] [PATCH 0/2] Support FDT library.
Date: Fri, 31 Mar 2023 00:26:37 +0100	[thread overview]
Message-ID: <CAKbZUD2u_tG5uV_+T4WwgBwss2cX89fBD+mA34BkJA8TAZqvKg@mail.gmail.com> (raw)
In-Reply-To: <CO1PR11MB492953805811ECD0831E5842D28E9@CO1PR11MB4929.namprd11.prod.outlook.com>

On Thu, Mar 30, 2023 at 11:59 PM Kinney, Michael D
<michael.d.kinney@intel.com> wrote:
>
>
>
> > -----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.
Ok, SGTM.
>
> >
> > 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.

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

I would like it if we could stop contributing to that problem. We very
much *know* there is a problem with both libc fragments and compiler
intrinsic fragments all over edk2.
A proper, correct C standard library is not trivial to implement
(hence the multiple problems I did find from a quick read of the
patch).
We also have a whole libc implementation in edk2-libc that seems to be
permanently gathering dust until Intel touches it for Python purposes
from time to time. So between crypto, libfdt, etc, could we try to
unify things here a bit?

Thanks,
Pedro

  reply	other threads:[~2023-03-30 23:26 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   ` [edk2-devel] " Pedro Falcato
2023-04-12 16:59     ` 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 [this message]
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=CAKbZUD2u_tG5uV_+T4WwgBwss2cX89fBD+mA34BkJA8TAZqvKg@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