回复: [edk2-devel] [PATCH v1 1/1] MdePkg: Correcting EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT definition


gaoliming
 

Paul:
Laszlo also agrees current patch. Some clean up may be done later. I will merge this patch first.

Thanks
Liming

-----邮件原件-----
发件人: Grimes, Paul <Paul.Grimes@...>
发送时间: 2020年8月29日 2:40
收件人: Laszlo Ersek <lersek@...>; devel@edk2.groups.io
抄送: Michael D Kinney <michael.d.kinney@...>; Liming Gao
<gaoliming@...>; Zhiguang Liu <zhiguang.liu@...>; Grimes,
Paul <Paul.Grimes@...>
主题: RE: [edk2-devel] [PATCH v1 1/1] MdePkg: Correcting
EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT definition

[AMD Public Use]

Thanks for the feedback, Lazlo. I agree with your point on the optimal
format for these #defines. I think it would be best to submit the current
patch as is given that the same feedback could (should?) be applied to various
other #defines in the file,
eg:
EFI_ACPI_DMA_BUS_MASTER_MASK 0x04, which only applies to Bit 2
and
EFI_ACPI_IRQ_POLARITY_MASK 0x08, ... Bit 3 and
EFI_ACPI_IRQ_MODE 0x01, ... bit 0

IMO if these defines were to be updated for clarity, it should probably be done
for the whole file in a separate commit.

Thanks,
Paul

-----Original Message-----
From: Laszlo Ersek <lersek@...>
Sent: Friday, August 28, 2020 10:06 AM
To: devel@edk2.groups.io; Grimes, Paul <Paul.Grimes@...>
Cc: Michael D Kinney <michael.d.kinney@...>; Liming Gao
<gaoliming@...>; Zhiguang Liu <zhiguang.liu@...>
Subject: Re: [edk2-devel] [PATCH v1 1/1] MdePkg: Correcting
EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT definition

[CAUTION: External Email]

On 08/27/20 22:40, Paul wrote:
In Acpi10.h, EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT is defined as 0x10,
but
should be 0x02 per the ACPI Specification.

REF:https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2F
bugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2937&amp;data=02%7C01
%7Cp
aul.grimes%40amd.com%7C82b28bb6544a4612fc1108d84b749dc6%7C3dd8
961fe488
4e608e11a82d994e183d%7C0%7C0%7C637342311528396385&amp;sdata=
7vHYIHHaJU
4yrXzAWtv5xTf%2BQfclAUBusz278%2F6I%2BRY%3D&amp;reserved=0

Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Signed-off-by: Paul G <paul.grimes@...>
---
MdePkg/Include/IndustryStandard/Acpi10.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdePkg/Include/IndustryStandard/Acpi10.h
b/MdePkg/Include/IndustryStandard/Acpi10.h
index fa06eefbb6e6..adeb5ae8c219 100644
--- a/MdePkg/Include/IndustryStandard/Acpi10.h
+++ b/MdePkg/Include/IndustryStandard/Acpi10.h
@@ -358,7 +358,7 @@ typedef struct {
#define EFI_ACPI_DMA_TRANSFER_TYPE_MASK
0x03

#define EFI_ACPI_DMA_TRANSFER_TYPE_8_BIT 0x00

#define EFI_ACPI_DMA_TRANSFER_TYPE_8_BIT_AND_16_BIT
0x01

-#define EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT 0x10

+#define EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT 0x02



//

// IO Information
Good catch. The ACPI spec text was likely cut n' pasted into the edk2 source,
and then prefixed with "0x". The spec says,

"""
Bits [1:0] DMA transfer type preference, _SIZ
00 8-bit only
01 8- and 16-bit
10 16-bit only
11 Reserved
"""

but that's in binary, not in hexadecimal.

In fact, the leading zero on *all four* macros (including
EFI_ACPI_DMA_TRANSFER_TYPE_MASK) is misleading. In hex, the leading
zero in the current macros stands for bits [7:4], which are completely
irrelevant for the _SIZ bit-field in the DMA Descriptor. So optimally we'd have

#define EFI_ACPI_DMA_TRANSFER_TYPE_MASK 0x3
#define EFI_ACPI_DMA_TRANSFER_TYPE_8_BIT 0x0
#define EFI_ACPI_DMA_TRANSFER_TYPE_8_BIT_AND_16_BIT 0x1
#define EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT 0x2

But I agree the current patch is OK too:

Reviewed-by: Laszlo Ersek <lersek@...>

I also agree it's a bugfix and should be merged now.

Thanks
Laszlo

Join devel@edk2.groups.io to automatically receive all group messages.