public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Yao, Jiewen" <jiewen.yao@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
	"sami.mujawar@arm.com" <sami.mujawar@arm.com>
Cc: "Chang, Abner" <abner.chang@hpe.com>,
	Ard Biesheuvel <ardb+tianocore@kernel.org>,
	Leif Lindholm <leif@nuviainc.com>,
	"Justen, Jordan L" <jordan.l.justen@intel.com>,
	Gerd Hoffmann <kraxel@redhat.com>,
	"Schaefer, Daniel" <daniel.schaefer@hpe.com>,
	Sunil V L <sunilvl@ventanamicro.com>, nd <nd@arm.com>
Subject: Re: [edk2-devel] [PATCH V2 6/9] ArmVirtPkg/QemuFwCfgLib: Relocate QemuFwCfgLib to OvmfPkg
Date: Wed, 29 Sep 2021 12:26:21 +0000	[thread overview]
Message-ID: <PH0PR11MB4885A2F0AF25DB3CB12AD1F08CA99@PH0PR11MB4885.namprd11.prod.outlook.com> (raw)
In-Reply-To: <C91B76E1-8836-4CA8-8A02-67F180972B71@arm.com>

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.yao@intel.com>
> Cc: Chang, Abner <abner.chang@hpe.com>; devel@edk2.groups.io; Ard
> Biesheuvel <ardb+tianocore@kernel.org>; Leif Lindholm <leif@nuviainc.com>;
> Justen, Jordan L <jordan.l.justen@intel.com>; Gerd Hoffmann
> <kraxel@redhat.com>; Schaefer, Daniel <daniel.schaefer@hpe.com>; Sunil V L
> <sunilvl@ventanamicro.com>; nd <nd@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 https://edk2-
> 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.yao@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.Mujawar@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.chang@hpe.com>
>     >> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>     >> Cc: Leif Lindholm <leif@nuviainc.com>
>     >> Cc: Sami Mujawar <sami.mujawar@arm.com>
>     >> Cc: Jiewen Yao <jiewen.yao@intel.com>
>     >> Cc: Jordan Justen <jordan.l.justen@intel.com>
>     >> Cc: Gerd Hoffmann <kraxel@redhat.com>
>     >> Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
>     >> Cc: Sunil V L <sunilvl@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/VirtioMmioDevice
> Lib.inf
>     >> -  QemuFwCfgLib|ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
>     >> +
> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMMIO.inf
>     >>
> QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3LibNu
> ll.inf
>     >>
> QemuFwCfgSimpleParserLib|OvmfPkg/Library/QemuFwCfgSimpleParserLib/Qe
> muFwCfgSimpleParserLib.inf
>     >>
> QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQem
> 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/VirtioMmioDevice
> Lib.inf
>     >> -  QemuFwCfgLib|ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf
>     >> +
> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMMIO.inf
>     >>
> QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3LibNu
> ll.inf
>     >>
> QemuFwCfgSimpleParserLib|OvmfPkg/Library/QemuFwCfgSimpleParserLib/Qe
> muFwCfgSimpleParserLib.inf
>     >>
> QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQem
> 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));
>     >
> 
> 
> 
> 
> 


  reply	other threads:[~2021-09-29 12:26 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-28  8:30 [PATCH V2 0/9] Migrate ArmVirtPkg modules to OvmfPkg Abner Chang
2021-09-28  8:30 ` [PATCH V2 1/9] ArmVirtPkg/FdtClintDxe: Move FdtClientDxe to EmbeddedPkg Abner Chang
2021-09-28 23:09   ` Daniel Schaefer
2021-09-28  8:31 ` [PATCH V2 2/9] MdePkg: Add PcdPciIoTranslation PCD Abner Chang
2021-09-28 23:10   ` Daniel Schaefer
2021-09-29  0:51     ` Abner Chang
2021-09-29  1:10     ` 回复: [edk2-devel] " gaoliming
2021-09-28  8:31 ` [PATCH V2 3/9] ArmPkg: Use PcdPciIoTranslation PCD from MdePkg Abner Chang
2021-09-28 23:38   ` Daniel Schaefer
2021-09-28  8:31 ` [PATCH V2 4/9] ArmVirtPkg/FdtPciPcdProducerLib: Relocate PciPcdProducerLib to OvmfPkg Abner Chang
2021-09-28 23:16   ` Daniel Schaefer
     [not found]   ` <16A91FA9AD0B84BA.7073@groups.io>
2021-09-28 23:41     ` [edk2-devel] " Daniel Schaefer
2021-09-29  0:42       ` Abner Chang
2021-09-28  8:31 ` [PATCH V2 5/9] ArmVirtPkg/HighMemDxe: Relocate HighMemDxe " Abner Chang
2021-09-28 23:34   ` Daniel Schaefer
2021-09-28  8:31 ` [PATCH V2 6/9] ArmVirtPkg/QemuFwCfgLib: Relocate QemuFwCfgLib " Abner Chang
2021-09-28 11:43   ` [edk2-devel] " Gerd Hoffmann
2021-09-29  9:45   ` Sami Mujawar
2021-09-29 10:04     ` Yao, Jiewen
2021-09-29 11:33       ` Sami Mujawar
2021-09-29 12:26         ` Yao, Jiewen [this message]
2021-09-29 12:46           ` [edk2-devel] " Abner Chang
2021-09-29 12:57             ` Yao, Jiewen
2021-09-28  8:31 ` [PATCH V2 7/9] MdePkg: Add PcdPciMmio32(64)Translation PCDs Abner Chang
2021-09-28 23:36   ` Daniel Schaefer
2021-09-29  0:54     ` Abner Chang
2021-09-28  8:31 ` [PATCH V2 8/9] ArmVirtPkg/FdtPciHostBridgeLib: Relocate FdtPciHostBridgeLib to OvmfPkg/Fdt Abner Chang
2021-09-28 23:25   ` Daniel Schaefer
2021-09-28  8:31 ` [PATCH V2 9/9] ArmVirtPkg/VirtioFdtDxe: Relocate VirtioFdtDxe " Abner Chang
2021-09-28 23:36   ` Daniel Schaefer
2021-09-28 23:11 ` [PATCH V2 0/9] Migrate ArmVirtPkg modules to OvmfPkg Daniel Schaefer
2021-09-29  0:53   ` Abner Chang
2021-09-29  1:30     ` 回复: [edk2-devel] " gaoliming
2021-09-29  1:43       ` Abner Chang
2021-09-29  3:31         ` 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=PH0PR11MB4885A2F0AF25DB3CB12AD1F08CA99@PH0PR11MB4885.namprd11.prod.outlook.com \
    --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