Paul: Laszlo also agrees current patch. Some clean up may be done later. I will merge this patch first.
Thanks Liming > -----邮件原件----- > 发件人: Grimes, Paul <paul.gri...@amd.com> > 发送时间: 2020年8月29日 2:40 > 收件人: Laszlo Ersek <ler...@redhat.com>; devel@edk2.groups.io > 抄送: Michael D Kinney <michael.d.kin...@intel.com>; Liming Gao > <gaolim...@byosoft.com.cn>; Zhiguang Liu <zhiguang....@intel.com>; Grimes, > Paul <paul.gri...@amd.com> > 主题: 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 <ler...@redhat.com> > Sent: Friday, August 28, 2020 10:06 AM > To: devel@edk2.groups.io; Grimes, Paul <paul.gri...@amd.com> > Cc: Michael D Kinney <michael.d.kin...@intel.com>; Liming Gao > <gaolim...@byosoft.com.cn>; Zhiguang Liu <zhiguang....@intel.com> > 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&data=02%7C01 > %7Cp > > > aul.grimes%40amd.com%7C82b28bb6544a4612fc1108d84b749dc6%7C3dd8 > 961fe488 > > > 4e608e11a82d994e183d%7C0%7C0%7C637342311528396385&sdata= > 7vHYIHHaJU > > 4yrXzAWtv5xTf%2BQfclAUBusz278%2F6I%2BRY%3D&reserved=0 > > > > Cc: Michael D Kinney <michael.d.kin...@intel.com> > > Cc: Liming Gao <gaolim...@byosoft.com.cn> > > Cc: Zhiguang Liu <zhiguang....@intel.com> > > Signed-off-by: Paul G <paul.gri...@amd.com> > > --- > > 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 <ler...@redhat.com> > > I also agree it's a bugfix and should be merged now. > > Thanks > Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#64800): https://edk2.groups.io/g/devel/message/64800 Mute This Topic: https://groups.io/mt/76506912/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
