public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Leif Lindholm" <leif.lindholm@linaro.org>
To: devel@edk2.groups.io, abner.chang@hpe.com
Cc: "Chen, Gilbert" <gilbert.chen@hpe.com>
Subject: Re: [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v3 29/39] RiscVPlatformPkg/RealTimeClockLibNull: Null instance of RTC lib.
Date: Fri, 22 Nov 2019 16:32:11 +0000	[thread overview]
Message-ID: <20191122163210.GI7359@bivouac.eciton.net> (raw)
In-Reply-To: <CS1PR8401MB11925F17130502BCC5BEA9DDFF490@CS1PR8401MB1192.NAMPRD84.PROD.OUTLOOK.COM>

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

  reply	other threads:[~2019-11-22 16:32 UTC|newest]

Thread overview: 77+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-28  1:58 [edk2-staging/RISC-V-V2 PATCH v3 00/39] RISC-V EDK2 Port on Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 01/39] RiscVPkg: RISC-V processor package Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 02/39] RiscVPkg/Include: Add header files of RISC-V CPU package Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 03/39] RiscVPkg/opensbi: EDK2 RISC-V OpenSBI support Abner Chang
2019-11-21 16:24   ` Leif Lindholm
2019-12-19  4:09     ` [edk2-devel] " Abner Chang
2019-12-19 13:21       ` Leif Lindholm
2019-12-19 14:48         ` Abner Chang
     [not found]         ` <15E1CD20DD2FE7F1.29030@groups.io>
2019-12-20  3:04           ` Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 04/39] MdePkg: RISC-V RV64 binding in MdePkg Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 05/39] MdePkg/Include: RISC-V definitions Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 06/39] MdeModulePkg/CapsuleRuntimeDxe: Add RISCV64 arch Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 07/39] MdePkg/BaseLib: BaseLib for RISC-V RV64 Processor Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 08/39] MdePkg/BaseCacheMaintenanceLib: RISC-V cache maintenance implementation Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 09/39] MdePkg/BaseIoLibIntrinsic: RISC-V I/O intrinsic functions Abner Chang
2019-11-21 16:36   ` Leif Lindholm
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 10/39] MdePkg/BasePeCoff: Add RISC-V PE/Coff related code Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 11/39] MdePkg/BaseCpuLib: RISC-V Base CPU library implementation Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 12/39] MdePkg/BaseSynchronizationLib: RISC-V cache related code Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 13/39] MdeModulePkg/Logo Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 14/39] NetworkPkg Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 15/39] RiscVPkg/Library: RISC-V CPU library Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 16/39] RiscVPkg/Library: Add RISC-V exception library Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 17/39] RiscVPkg/Library: Add RISC-V timer library Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 18/39] RiscVPkg/Library: Add EDK2 RISC-V OpenSBI library Abner Chang
2019-11-21 16:48   ` [edk2-devel] " Leif Lindholm
2019-12-19  6:00     ` Abner Chang
2019-12-20 15:04       ` Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 19/39] RiscVPkg/Library: RISC-V platform level DxeIPL libraries Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 20/39] MdeModulePkg/DxeIplPeim : RISC-V platform level DxeIPL Abner Chang
2019-10-28  1:58 ` [edk2-staging/RISC-V-V2 PATCH v3 21/39] RiscVPkg/PeiServicesTablePointerLibOpenSbi: RISC-V PEI Service Table Pointer library Abner Chang
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 22/39] RiscVPlatformPkg/RiscVPlatformTempMemoryInit: RISC-V Platform Temporary Memory library Abner Chang
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 23/39] RiscVPkg/CpuDxe: Add RISC-V CPU DXE driver Abner Chang
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 24/39] BaseTools: BaseTools changes for RISC-V platform Abner Chang
2019-11-21 16:55   ` Leif Lindholm
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 25/39] BaseTools/Scripts Abner Chang
2019-11-19 18:17   ` [edk2-devel] " Mark Salter
2019-11-20  8:23     ` Abner Chang
2019-11-21  7:40       ` Abner Chang
2019-11-21 16:56   ` Leif Lindholm
2019-11-22  3:49     ` Abner Chang
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 26/39] RiscVPkg/SmbiosDxe: Generic SMBIOS DXE driver for RISC-V platforms Abner Chang
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 27/39] RiscVPkg/RiscVOpensbLlib: Add submodule opensbi Abner Chang
2019-11-21 17:00   ` Leif Lindholm
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 28/39] RiscVPlatformPkg/FirmwareContextProcessorSpecificLib:Add FirmwareContextProcessorSpecificLib module Abner Chang
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 29/39] RiscVPlatformPkg/RealTimeClockLibNull: Null instance of RTC lib Abner Chang
2019-11-21 17:02   ` Leif Lindholm
2019-11-22 12:28     ` Abner Chang
2019-11-22 14:08       ` Leif Lindholm
2019-11-22 14:19         ` Abner Chang
2019-11-22 14:55           ` Leif Lindholm
2019-11-22 16:05             ` [edk2-devel] " Abner Chang
2019-11-22 16:32               ` Leif Lindholm [this message]
2019-11-23  7:38                 ` Abner Chang
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 30/39] RiscVPlatformPkg/OpensbiPlatformLibNull: NULL instance of RiscVOpensbiPlatformLib Abner Chang
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 31/39] RiscVPlatformPkg/PlatformMemoryTestLibNull: NULL instance of PlatformMemoryTestLib Abner Chang
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 32/39] RiscVPlatformPkg/PlatformUpdateProgressLibNull: NULL instance of PlatformUpdateProgressLib Abner Chang
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 33/39] RiscVPlatformPkg/PlatformBootManagerLib: Platform Boot Manager library Abner Chang
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 34/39] RiscVPkg/RiscVPlatformTimerLibNull: NULL instance of RISC-V platform timer library Abner Chang
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 35/39] RiscVPlatformPkg/SecMain: RISC-V SecMain module Abner Chang
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 36/39] RiscVPlatformPkg: Add RiscVPlatformPkg Abner Chang
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 37/39] edk2-staging:RISC-V-V2: Add RiscVEdk2Readme.md Abner Chang
2019-11-21 17:09   ` Leif Lindholm
2019-11-22  3:57     ` [edk2-devel] " Abner Chang
2019-11-22  9:34       ` Leif Lindholm
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 38/39] edk2-staging: Revise Maintainers.txt for RISC-V packages Abner Chang
2019-10-29  1:05   ` [edk2-devel] " Laszlo Ersek
2019-10-29  1:31     ` Abner Chang
2019-10-28  1:59 ` [edk2-staging/RISC-V-V2 PATCH v3 39/39] edk2-staging: Revise Readme.md Abner Chang
2019-10-29  1:07   ` [edk2-devel] " Laszlo Ersek
2019-10-29  1:35     ` Abner Chang
2019-10-29  2:01     ` Leif Lindholm
2019-10-31  8:20       ` Laszlo Ersek
2019-11-21 17:15   ` Leif Lindholm
2019-11-22  2:05     ` Abner Chang
2019-11-22  9:42       ` Leif Lindholm
2019-11-22 11:46         ` Abner Chang

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=20191122163210.GI7359@bivouac.eciton.net \
    --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