> -----Original Message-----
> From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> Sent: Thursday, December 19, 2019 9:22 PM
> To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist)
> <abner.ch...@hpe.com>
> Cc: Chen, Gilbert <gilbert.c...@hpe.com>; Kinney, Michael D
> <michael.d.kin...@intel.com>
> Subject: Re: [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v3 03/39]
> RiscVPkg/opensbi: EDK2 RISC-V OpenSBI support
>
> On Thu, Dec 19, 2019 at 04:09:22 +0000, Abner Chang wrote:
> > > > diff --git a/RiscVPkg/Include/sbi/sbi.h
> > > > b/RiscVPkg/Include/sbi/sbi.h new file mode 100644 index
> > > > 0000000..04e7f18
> > > > --- /dev/null
> > > > +++ b/RiscVPkg/Include/sbi/sbi.h
> > > > @@ -0,0 +1,52 @@
> > > > +/** @file
> > > > + SBI inline function calls.
> > > > +
> > > > + Copyright (c) 2019, Hewlett Packard Enterprise Development LP.
> > > > + All rights reserved.<BR>
> > > > +
> > > > + SPDX-License-Identifier: BSD-2-Clause-Patent
> > > > +
> > > > +**/
> > > > +
> > > > +#ifndef EDK2_SBI_H_
> > > > +#define EDK2_SBI_H_
> > > > +
> > > > +#include <sbi/sbi_types.h> // Reference to header file wrapper
> > > > +#include <include/sbi/riscv_asm.h> // Reference to header file in
> > > > +opensbi
> > >
> > > This whole sbi situation is too much of a hornets' nest, and really
> > > does need to get sorted before we even push this to -staging.
> > > (But I do think is is one of the last things we need to address.)
> > >
> > > And I think the starting point to untangle it is to rename the
> > > wrapper include files in RiscVPkg/Include/sbi (and indeed the
> > > directory
> > > itself) to conform with the TianoCore standard.
> > >
> > > That means:
> > >
> > > RiscVPkg/Include/IndustryStandard/Sbi/Sbi.h
> > > RiscVPkg/Include/IndustryStandard/Sbi/SbiBits.h
> > > RiscVPkg/Include/IndustryStandard/Sbi/SbiTypes.h
> > >
> > > Unless all of the wrappers can be combined into a single
> > > RiscVPkg/Include/IndustryStandard/Sbi.h
> > >
> > > ...and then tested on a case-sensitive filesystem.
> > >
> > > For the record, doing that immediately results in the non-wrapper
> > > versions being pulled in and then the build failing.
> >
> > Leif,
> > We can't change the naming of those header files under
> > RiscVPkg/Include/sbi. We build opensbi source code in edk2 and those
> > source files refer to those header file. And we are not going to
> > change opensbi code as we discussed before.
> >
> > The only file we can change is sbi.h -> Sbi.h because this header file
> > is created for edk2 RISC-V related drivers.
> >
> > I think we can make it this way,
> > Rename sbi.h to RiscVOpensbi.h and move it to under
> > RiscVPkg/Include/IndustryStandard, no need "/Sbi" subdirectory.
>
> That sounds good to me.
>
> > Keep the naming of sbi_bits.h and sbi_types.h and still leave those
> > under RiscVPkg/Include/sbi.
> >
> > No naming changes in order to build opensbi source file. Can't even
> > change "/sbi" subdirectory to "/Sbi". sbi_bits.h and sbi_types.h are
> > the binding files for edk2 framework BTW.
>
> Sure, if we separate interface for EDK2 apps from binding, it becomes less
> bad. Plus sbi_bits.h needs to just go, but we'll fix that in opensbi before
> merging Risc-V to edk2 master.
I am not sure this one. That may take some time to fix that in opensbi. I would
rather merge our code to edk2 master first and deal with opensbi in parallel.
Some people they approach to me and ask for edk2 RISC-V port. Currently they
can just go to our private Github to retrieve code. More people will join edk2
RISC-V port if we can merge it to edk2 master earlier.
> I think the proper fix for sbi_types.h also involves a change to opensbi.
This has to ask opensbi to have binging mechanism for different frameworks,
however I agree with you in this part.
>
> But if you can
> - do those changes
> - move/rename SbiFirmwareContext.h to live with RiscVOpensbi.h
Hehe already. I know you :). I already moved this file to under
RiscVPkg/Include/IndustryStandard just a separate file.
> - add a (short) Readme.md to RiscVPkg/Include/sbi explaining what
> these files are and why
> I think that resolves my final reservations against bringing this set onto a
> staging branch.
sure
>
> Regards,
>
> Leif
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#52428): https://edk2.groups.io/g/devel/message/52428
Mute This Topic: https://groups.io/mt/38757517/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
