Hi Jiewen, It should be QemuFwCfgLibMmio instead of QemuFwCfgLibMMIO, right?
Abner > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Yao, Jiewen > Sent: Wednesday, September 29, 2021 8:26 PM > To: devel@edk2.groups.io; sami.muja...@arm.com > Cc: Chang, Abner (HPS SW/FW Technologist) <abner.ch...@hpe.com>; Ard > Biesheuvel <ardb+tianoc...@kernel.org>; Leif Lindholm > <l...@nuviainc.com>; Justen, Jordan L <jordan.l.jus...@intel.com>; Gerd > Hoffmann <kra...@redhat.com>; Schaefer, Daniel > <daniel.schae...@hpe.com>; Sunil V L <suni...@ventanamicro.com>; nd > <n...@arm.com> > Subject: Re: [edk2-devel] [PATCH V2 6/9] ArmVirtPkg/QemuFwCfgLib: > Relocate QemuFwCfgLib to OvmfPkg > > Thank you Sami. > > We have a clear naming rule for EDKII project during development. > But I don’t know where it is documented. Maybe a good addition to the doc > you point out. > > To summarize what I know: > > 1) Library name: [<Phase>]<ClassName>Lib[<InstanceName>] > 2) Driver Name: <DriverName><Phase> > > For the example you point out, I see no problem, because "XenIoMmioLib" is > the class name. So XXXMmioLib is correct. > > In this case, the class name is "QemuFwCfgLib", "MMIO" is the instance > name. We should use QemuFwCfgLibMmio. > > Thank you > Yao Jiewen > > > > > -----Original Message----- > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sami > > Mujawar > > Sent: Wednesday, September 29, 2021 7:33 PM > > To: Yao, Jiewen <jiewen....@intel.com> > > Cc: Chang, Abner <abner.ch...@hpe.com>; devel@edk2.groups.io; Ard > > Biesheuvel <ardb+tianoc...@kernel.org>; Leif Lindholm > <l...@nuviainc.com>; > > Justen, Jordan L <jordan.l.jus...@intel.com>; Gerd Hoffmann > > <kra...@redhat.com>; Schaefer, Daniel <daniel.schae...@hpe.com>; Sunil > V L > > <suni...@ventanamicro.com>; nd <n...@arm.com> > > Subject: Re: [edk2-devel] [PATCH V2 6/9] ArmVirtPkg/QemuFwCfgLib: > Relocate > > QemuFwCfgLib to OvmfPkg > > > > Hi Jiewen, > > > > Thank you for clarifying the library naming convention. > > I could not find any references/examples as such in > INVALID URI REMOVED > __;!!NpxR!zMRKC5N87Eb6j69O6lJZ1hVaDVHrnj_21pwNznZx6ZAtLm0Z948atj > NrWepfxQ0$ > > docs.gitbook.io/edk-ii-c-coding-standards- > > specification/v/release%2F2.20/4_naming_conventions/42_file_names > and > > therefore had suggested following the file naming as done for Xen. > > > > Regards, > > > > Sami Mujawar > > > > On 29/09/2021, 11:04, "Yao, Jiewen" <jiewen....@intel.com> wrote: > > > > hi > > I think the original name is correct. > > > > The naming convention is : <LibClassName>Lib<InstanceName> > > > > thank you! > > Yao, Jiewen > > > > > > > 在 2021年9月29日，下午5:45，Sami Mujawar > > <sami.muja...@arm.com> 写道： > > > > > > Hi Abner, > > > > > > Thank you for this patch. > > > > > > I have a minor suggestion marked inline as [SAMI]. > > > > > > Regards, > > > > > > Sami Mujawar > > > > > > > > >> On 28/09/2021 09:31 AM, Abner Chang wrote: > > >> Relocate QemuFwCfgLib to OvmfPkg/Library/QemuFwCfgLib and > rename > > >> it to QemuFwCfgLibMMIO, this library is leverage by both ARM and > > >> RISC-V archs. > > >> > > >> Signed-off-by: Abner Chang <abner.ch...@hpe.com> > > >> Cc: Ard Biesheuvel <ardb+tianoc...@kernel.org> > > >> Cc: Leif Lindholm <l...@nuviainc.com> > > >> Cc: Sami Mujawar <sami.muja...@arm.com> > > >> Cc: Jiewen Yao <jiewen....@intel.com> > > >> Cc: Jordan Justen <jordan.l.jus...@intel.com> > > >> Cc: Gerd Hoffmann <kra...@redhat.com> > > >> Cc: Daniel Schaefer <daniel.schae...@hpe.com> > > >> Cc: Sunil V L <suni...@ventanamicro.com> > > >> --- > > >> ArmVirtPkg/ArmVirtQemu.dsc | 2 +- > > >> ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 +- > > >> .../Library/QemuFwCfgLib/QemuFwCfgLibMMIO.inf | 5 ++--- > > >> .../Library/QemuFwCfgLib/QemuFwCfgLibMMIO.c | 7 > > ++++--- > > >> 4 files changed, 8 insertions(+), 8 deletions(-) > > >> rename ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf => > > OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMMIO.inf (87%) > > >> rename ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c => > > OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMMIO.c (93%) > > > [SAMI] Is it possible to rename QemuFwCfgLibMMIO.[c|inf] to > > QemuFwCfgMmioLib.[c|inf], please? This would then follow a pattern > similar to > > OvmfPkg\Library\XenIoMmioLib\XenIoMmioLib.[c|inf]. > > >> > > >> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc > b/ArmVirtPkg/ArmVirtQemu.dsc > > >> index 07f9699c79..6c949fd559 100644 > > >> --- a/ArmVirtPkg/ArmVirtQemu.dsc > > >> +++ b/ArmVirtPkg/ArmVirtQemu.dsc > > >> @@ -59,7 +59,7 @@ > > >> # Virtio Support > > >> VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf > > >> > > > VirtioMmioDeviceLib|OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDev > ice > > Lib.inf > > >> - > QemuFwCfgLib|ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf > > >> + > > > QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMMIO.inf > > >> > > > QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3Li > bNu > > ll.inf > > >> > > > QemuFwCfgSimpleParserLib|OvmfPkg/Library/QemuFwCfgSimpleParserLib/ > Qe > > muFwCfgSimpleParserLib.inf > > >> > > > QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/Generic > Qem > > uLoadImageLib.inf > > >> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc > > b/ArmVirtPkg/ArmVirtQemuKernel.dsc > > >> index cf7a2b4463..64035a948d 100644 > > >> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc > > >> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc > > >> @@ -57,7 +57,7 @@ > > >> # Virtio Support > > >> VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf > > >> > > > VirtioMmioDeviceLib|OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDev > ice > > Lib.inf > > >> - > QemuFwCfgLib|ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf > > >> + > > > QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMMIO.inf > > >> > > > QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3Li > bNu > > ll.inf > > >> > > > QemuFwCfgSimpleParserLib|OvmfPkg/Library/QemuFwCfgSimpleParserLib/ > Qe > > muFwCfgSimpleParserLib.inf > > >> > > > QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/Generic > Qem > > uLoadImageLib.inf > > >> diff --git a/ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf > > b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMMIO.inf > > >> similarity index 87% > > >> rename from ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf > > >> rename to OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMMIO.inf > > >> index f3cc827907..8101fac03f 100644 > > >> --- a/ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf > > >> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMMIO.inf > > >> @@ -23,17 +23,16 @@ > > >> # The following information is for reference only and not required > > by > the > > build > > >> # tools. > > >> # > > >> -# VALID_ARCHITECTURES = ARM AARCH64 > > >> +# VALID_ARCHITECTURES = ARM AARCH64 RISCV64 > > >> # > > >> [Sources] > > >> - QemuFwCfgLib.c > > >> + QemuFwCfgLibMMIO.c > > >> [Packages] > > >> MdePkg/MdePkg.dec > > >> OvmfPkg/OvmfPkg.dec > > >> EmbeddedPkg/EmbeddedPkg.dec > > >> - ArmVirtPkg/ArmVirtPkg.dec > > >> [LibraryClasses] > > >> BaseLib > > >> diff --git a/ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c > > b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMMIO.c > > >> similarity index 93% > > >> rename from ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c > > >> rename to OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMMIO.c > > >> index e2ac4108d1..b953f2eb6c 100644 > > >> --- a/ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c > > >> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMMIO.c > > >> @@ -4,6 +4,7 @@ > > >> Copyright (C) 2013 - 2014, Red Hat, Inc. > > >> Copyright (c) 2011 - 2013, Intel Corporation. All rights > > reserved.<BR> > > >> + (C) Copyright 2021 Hewlett Packard Enterprise Development > LP<BR> > > >> SPDX-License-Identifier: BSD-2-Clause-Patent > > >> **/ > > >> @@ -239,7 +240,7 @@ MmioReadBytes ( > > >> UINT8 *Ptr; > > >> UINT8 *End; > > >> -#ifdef MDE_CPU_AARCH64 > > >> +#if defined(MDE_CPU_AARCH64) || defined(MDE_CPU_RISCV64) > > >> Left = Size & 7; > > >> #else > > >> Left = Size & 3; > > >> @@ -249,7 +250,7 @@ MmioReadBytes ( > > >> Ptr = Buffer; > > >> End = Ptr + Size; > > >> -#ifdef MDE_CPU_AARCH64 > > >> +#if defined(MDE_CPU_AARCH64) || defined(MDE_CPU_RISCV64) > > >> while (Ptr < End) { > > >> *(UINT64 *)Ptr = MmioRead64 (mFwCfgDataAddress); > > >> Ptr += 8; > > >> @@ -322,7 +323,7 @@ DmaTransferBytes ( > > >> // > > >> // This will fire off the transfer. > > >> // > > >> -#ifdef MDE_CPU_AARCH64 > > >> +#if defined(MDE_CPU_AARCH64) || defined(MDE_CPU_RISCV64) > > >> MmioWrite64 (mFwCfgDmaAddress, SwapBytes64 > ((UINT64)&Access)); > > >> #else > > >> MmioWrite32 ((UINT32)(mFwCfgDmaAddress + 4), SwapBytes32 > > ((UINT32)&Access)); > > > > > > > > > > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#81282): https://edk2.groups.io/g/devel/message/81282 Mute This Topic: https://groups.io/mt/85920584/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
