[edk2-devel] [edk2-staging/RISC-V-V2 PATCH v3 29/39] RiscVPlatformPkg/RealTimeClockLibNull: Null instance of RTC lib.

[Leif Lindholm]

On Fri, Nov 22, 2019 at 16:05:07 +0000, Abner Chang wrote:
> > > > Sure, but we also don't *need* to add a new implementation for this
> > > > - RiscVPkg can still use the EmbeddedPkg one.
> > > >
> > > > (And if we did, it should probably be in MdeModulePkg.)
> > >
> > > I think we had similar discussion about this before. My comment was
> > > RiscVPkg as a processor package should not have dependence with
> > > EmbeddedPkg.
> > 
> > This is not RiscVPkg though, this is RiscVPlatformPkg.
> > And also, it does not appear to be used there anyway?
>
> Same comments from me for RiscVPlatformPkg. I don't see any reasons
> to have dependence with EmbeddedPkg in RiscVPlatformPkg as
> RiscVPlatformPkg is a generic RISC-V platform modules . Platform
> such as U540 could choice which RTC instance it needs.

I don't think there is any particular inherent aspect about
EmbeddedPkg being more evil or unreliable than MdeModulePkg. If
anything, it suffers from poor naming. (Basically, it was the staging
area for bringing a bunch of !x86 stuff into the tree, and the first
platform port was to an embedded board...)

But more importantly, RealTimeClockLib is only used by
EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf, so
it's a bit difficult to argue that EmbeddedPkg is an unsuitable source
for the library.

We will at some point do an overhaul of the directory tree, so getting
hung up on current package names isn't a worthwhile investment.

The only real exceptions to this are MdePkg and MdeModulePkg, which
should not depend on any other packages. And I tend to argue even
about that.

> > Certainly I can still build RiscVPlatformPkg.dsc if I delete that
> > library mapping.
>
> We can remove this one and create a null one in MdePkg which is akin
> to the null instance of TimerLib.

I have no issue with that as such.
I also don't see a value.

A RealTimeClockLibNull is only useful for enabling compilation of
incomplete platforms (and as such, if it was included in MdeModulePkg,
it should probably have unconditional ASSERTs added to all functions).

Whereas VirtualRealTimeClockLib can ameliorate the situation of not
having a persistent RTC in the system.

But for the purpose of this set, please just drop this patch and any
references to this module.

/
    Leif

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#51196): https://edk2.groups.io/g/devel/message/51196
Mute This Topic: https://groups.io/mt/38757551/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-