Re: [PATCH 4/5] ArmVirtPkg/QemuFwCfgLib: rebase lib instance to OvmfPkg/IndustryStandard

[Leif Lindholm <leif.lindholm@linaro.org>]

On Thu, Dec 01, 2016 at 06:56:32PM +0100, Laszlo Ersek wrote:
> where "QemuFwCfgDma.h" was added in the previous patch.
> 
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>

This is nice cleanup.
One bit of bikeshedding below, address or ignore - regardless:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c | 24 +++-----------------
>  1 file changed, 3 insertions(+), 21 deletions(-)
> 
> diff --git a/ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c b/ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
> index 2fd8d9050566..62a85dff328e 100644
> --- a/ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
> +++ b/ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
> @@ -25,6 +25,8 @@
>  
>  #include <Protocol/FdtClient.h>
>  
> +#include <IndustryStandard/QemuFwCfgDma.h>
> +

So, I forget if we have official guidelines on this, but instinctively
I would put IndustryStandard before Library (alphabetically).

Regards,

Leif

>  STATIC UINTN mFwCfgSelectorAddress;
>  STATIC UINTN mFwCfgDataAddress;
>  STATIC UINTN mFwCfgDmaAddress;
> @@ -53,26 +55,6 @@ STATIC READ_BYTES_FUNCTION DmaReadBytes;
>  //
>  STATIC READ_BYTES_FUNCTION *InternalQemuFwCfgReadBytes = MmioReadBytes;
>  
> -//
> -// Communication structure for DmaReadBytes(). All fields are encoded in big
> -// endian.
> -//
> -#pragma pack (1)
> -typedef struct {
> -  UINT32 Control;
> -  UINT32 Length;
> -  UINT64 Address;
> -} FW_CFG_DMA_ACCESS;
> -#pragma pack ()
> -
> -//
> -// Macros for the FW_CFG_DMA_ACCESS.Control bitmap (in native encoding).
> -//
> -#define FW_CFG_DMA_CTL_ERROR  BIT0
> -#define FW_CFG_DMA_CTL_READ   BIT1
> -#define FW_CFG_DMA_CTL_SKIP   BIT2
> -#define FW_CFG_DMA_CTL_SELECT BIT3
> -
>  
>  /**
>    Returns a boolean indicating if the firmware configuration interface
> @@ -183,7 +165,7 @@ QemuFwCfgInitialize (
>  
>          QemuFwCfgSelectItem (QemuFwCfgItemInterfaceVersion);
>          Features = QemuFwCfgRead32 ();
> -        if ((Features & BIT1) != 0) {
> +        if ((Features & FW_CFG_F_DMA) != 0) {
>            mFwCfgDmaAddress = FwCfgDmaAddress;
>            InternalQemuFwCfgReadBytes = DmaReadBytes;
>          }
> -- 
> 2.9.2
> 
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel