On Wed, Oct 16, 2019 at 01:36:07AM +0000, Chang, Abner (HPS SW/FW Technologist)
wrote:
> > -----Original Message-----
> > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> > Leif Lindholm
> > Sent: Wednesday, October 2, 2019 5:15 AM
> > To: devel@edk2.groups.io; Chen, Gilbert <gilbert.c...@hpe.com>
> > Subject: Re: [edk2-devel] [plaforms/devel-riscv-v2 PATCHv2 02/14]
> > Silicon/SiFive: Add library module of SiFive RISC-V cores
> >
> > On Thu, Sep 19, 2019 at 11:51:19AM +0800, Gilbert Chen wrote:
> > > Initial version of SiFive RISC-V core libraries. Library of each core
> > > creates processor core SMBIOS data hob for building SMBIOS records in
> > > DXE phase.
> >
> > So yes, this implementation needs to change.
> > These should all implement the same LibraryClass.
>
> No. It shouldn't be the same library class (If you were saying the
> same LibraryClass). RISC-V SoC could be the combination of different
> RISC-V cores, or even the cores from different vendors. This depends
> on how SoC vendor combine those IPs.
Ah, OK. Sorry, did not realise this aspect.
> Either U54 or E51 could be a standalone SoC, while U54MC is the
> combination of 4 x U54 core and one E51 core.
>
> U5MC under Platform/SiFive could be 1-8 U5 core and optionally
> support E5 core. This is the special case for U500 VC707 platform
> because the core number could be customized.
>
> > Also, U54 appears to be a simple superset of U51.
> U54 is a single core.
>
> >
> > What I would suggest is creating a
> > Silicon/SiFive/Library/SiFiveCoreInfoLib, which calls into a
> > SiFiveSoCCoreInfoLib in Silicon/SiFive/<SoC>/Library, providing the acual
> > SoC-
> > specific bits.
>
> Platform system firmware integrator just pull in the necessary core
> libraries from Silicon/{vendor} and invoke the function to create
> specific core bits.
>
> I think this implementation is well and flexible which has no need to change.
Oh, my criticism was that it was *too* flexible (with resulting code
overhead). I still feel the very small source differences, especially
between E51/U54, indicate that these could be implemented by the same
source file but different .inf files.
But this is not something that needs to be addressed before this patch
goes into -staging.
Regards,
Leif
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#49178): https://edk2.groups.io/g/devel/message/49178
Mute This Topic: https://groups.io/mt/34196349/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
