Re: [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v1 01/22]: RiscVPkg: RISC-V processor package.

["Leif Lindholm" <leif.lindholm@linaro.org>]

On Mon, Sep 16, 2019 at 05:15:08AM +0000, Chang, Abner (HPS SW/FW Technologist) wrote:
> > -----Original Message-----
> > From: Leif Lindholm [mailto:leif.lindholm@linaro.org]
> > Sent: Thursday, September 5, 2019 1:51 AM
> > To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist)
> > <abner.chang@hpe.com>
> > Subject: Re: [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v1 01/22]:
> > RiscVPkg: RISC-V processor package.
> > 
> > Hi Abner,
> > 
> > On Wed, Sep 04, 2019 at 06:42:56PM +0800, Abner Chang wrote:
> > > - Add RiscVPkg package which provides RISC-V processor related drivers
> > and libraries.
> > > - Support RISC-V OpenSBI and RISC-V platforms
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > Signed-off-by: Abner Chang <abner.chang@hpe.com>
> > > ---
> > >  RiscVPkg/RiscVPkg.dec      |  57
> > +++++++++++++++++++++++++++++++++++++++++++++
> > >  RiscVPkg/RiscVPkg.uni      | Bin 0 -> 1718 bytes
> > >  RiscVPkg/RiscVPkgExtra.uni | Bin 0 -> 1374 bytes
> > >  3 files changed, 57 insertions(+)
> > >  create mode 100644 RiscVPkg/RiscVPkg.dec  create mode 100644
> > > RiscVPkg/RiscVPkg.uni  create mode 100644 RiscVPkg/RiscVPkgExtra.uni
> > >
> > > diff --git a/RiscVPkg/RiscVPkg.dec b/RiscVPkg/RiscVPkg.dec new file
> > > mode 100644 index 0000000..acf71fe
> > > --- /dev/null
> > > +++ b/RiscVPkg/RiscVPkg.dec
> > > @@ -0,0 +1,57 @@
> > > +## @file  RiscVPkg.dec
> > > +# This Package provides UEFI RISC-V modules and libraries.
> > > +#
> > > +# Copyright (c) 2016 - 2019, Hewlett Packard Enterprise Development
> > > +LP. All rights reserved.<BR> # # 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 #
> > > +INVALID URI REMOVED
> > 3A__opensource.org_li
> > > +censes_bsd-
> > 2Dlicense.php&d=DwIBAg&c=C5b8zRQO1miGmBeVZ2LFWg&r=_SN6FZBN
> > >
> > +4Vgi4Ulkskz6qU3NYRO03nHp9P7Z5q59A3E&m=1PSVwg69_Y8lpR9wdv1TN7
> > kg2brsZYR
> > > +sj5F_hpyPrv4&s=USJlvms7O9ZDAsM0U-
> > FGng8i0uJkAMNbDEp1S_C4p0A&e=
> > > +#
> > > +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> > > +BASIS, # WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> > EITHER EXPRESS OR IMPLIED.
> > > +#
> > > +##
> > > +
> > > +[Defines]
> > > +  DEC_SPECIFICATION              = 0x00010005
> > > +  PACKAGE_NAME                   = RiscVPkg
> > > +  PACKAGE_UNI_FILE               = RiscVPkg.uni
> > > +  PACKAGE_GUID                   = 993C7CAC-C87C-4F08-A2CF-AD3AABA859D1
> > > +  PACKAGE_VERSION                = 0.1
> > > +
> > > +[Includes]
> > > +  Include
> > > +  opensbi/include
> > > +  opensbi/lib/utils/libfdt
> > 
> > This one is something we need to sort out (together). Having multiple copies
> > of libfdt in the tree is not on.
> > 
> > I personally think we need a longer-term encapsulation of libfdt that doesn't
> > mess up the coding style. But until then, I would be much happier if you used
> > the half measure we have in EmbeddedPkg:
> > EmbeddedPkg/Library/FdtLib/ and EmbeddedPkg/Include/.
>
> We may not go this way due to everything is from OpenSBI and we
> don't want to maintain the difference to open source OpenSBI. Just
> take what OpenSBI provides.

If libfdt was a very quickly changing project, I might agree with you.
But it is not. It is a very simple piece of code that performs a small
set of operations on a very well defined structured encapsulation
format.

So please use the one from EmbeddedPkg. On the very unlikely
occurrence that you require functionality not provided by the version
in there, we can update it.

/
    Leif