From: Laszlo Ersek <lersek@redhat.com>
To: Leif Lindholm <leif.lindholm@linaro.org>
Cc: edk2-devel-01 <edk2-devel@ml01.01.org>,
Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: Re: [PATCH 4/5] ArmVirtPkg/QemuFwCfgLib: rebase lib instance to OvmfPkg/IndustryStandard
Date: Fri, 2 Dec 2016 12:35:01 +0100 [thread overview]
Message-ID: <a42880e6-4aaa-d323-2956-d1e261efc5d2@redhat.com> (raw)
In-Reply-To: <20161202110342.GS27069@bivouac.eciton.net>
On 12/02/16 12:03, Leif Lindholm wrote:
> 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).
Good point. But, in the next version, this #include directive will go
away anyway, because this file already includes
<Library/QemuFwCfgLib.h>, and that header file will get the new macros
in v2 (based on Jordan's feedback).
Thanks!
Laszlo
>
> 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
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>
next prev parent reply other threads:[~2016-12-02 11:35 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-01 17:56 [PATCH 0/5] OvmfPkg/QemuFwCfgLib: support the DMA-like interface Laszlo Ersek
2016-12-01 17:56 ` [PATCH 1/5] ArmVirtPkg/QemuFwCfgLib: remove superfluous InternalQemuFwCfgIsAvailable() Laszlo Ersek
2016-12-02 10:58 ` Leif Lindholm
2016-12-01 17:56 ` [PATCH 2/5] OvmfPkg/QemuFwCfgLib: move InternalQemuFwCfgIsAvailable() to lib instances Laszlo Ersek
2016-12-01 17:56 ` [PATCH 3/5] OvmfPkg/IndustryStandard: add QemuFwCfgDma.h Laszlo Ersek
2016-12-01 19:34 ` Jordan Justen
2016-12-01 20:48 ` Laszlo Ersek
2016-12-02 1:01 ` Jordan Justen
2016-12-02 10:05 ` Laszlo Ersek
2016-12-01 17:56 ` [PATCH 4/5] ArmVirtPkg/QemuFwCfgLib: rebase lib instance to OvmfPkg/IndustryStandard Laszlo Ersek
2016-12-02 11:03 ` Leif Lindholm
2016-12-02 11:35 ` Laszlo Ersek [this message]
2016-12-01 17:56 ` [PATCH 5/5] OvmfPkg/QemuFwCfgLib: support QEMU's DMA-like fw_cfg access method Laszlo Ersek
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=a42880e6-4aaa-d323-2956-d1e261efc5d2@redhat.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