From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.byosoft.com.cn (mail.byosoft.com.cn [58.240.74.243]) by mx.groups.io with SMTP id smtpd.web10.24066.1598749347794355313 for ; Sat, 29 Aug 2020 18:02:28 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=none, err=permanent DNS error (domain: byosoft.com.cn, ip: 58.240.74.243, mailfrom: gaoliming@byosoft.com.cn) Received: from DESKTOPS6D0PVI ([116.233.155.155]) (envelope-sender ) by 192.168.6.13 with ESMTP for ; Sun, 30 Aug 2020 09:02:19 +0800 X-WM-Sender: gaoliming@byosoft.com.cn X-WM-AuthFlag: YES X-WM-AuthUser: gaoliming@byosoft.com.cn From: "gaoliming" To: "'Grimes, Paul'" , "'Laszlo Ersek'" , Cc: "'Michael D Kinney'" , "'Zhiguang Liu'" References: <20200827204051.777-1-Paul.Grimes@amd.com> <20200827204051.777-2-Paul.Grimes@amd.com> <12ee54f0-976f-239c-81c7-3c6e71cc8892@redhat.com> In-Reply-To: Subject: =?UTF-8?B?5Zue5aSNOiBbZWRrMi1kZXZlbF0gW1BBVENIIHYxIDEvMV0gTWRlUGtnOiBDb3JyZWN0aW5nIEVGSV9BQ1BJX0RNQV9UUkFOU0ZFUl9UWVBFXzE2X0JJVCBkZWZpbml0aW9u?= Date: Sun, 30 Aug 2020 09:02:23 +0800 Message-ID: <001001d67e69$385ad0f0$a91072d0$@byosoft.com.cn> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQGb1UMkjnvmTu4AY07SymYuBKSjewHNNytCAdhq0i4CdIDcyamUVn8g Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Content-Language: zh-cn Paul: Laszlo also agrees current patch. Some clean up may be done later. I = will merge this patch first.=20 Thanks Liming > -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > =E5=8F=91=E4=BB=B6=E4=BA=BA: Grimes, Paul > =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: = 2020=E5=B9=B48=E6=9C=8829=E6=97=A5 2:40 > =E6=94=B6=E4=BB=B6=E4=BA=BA: Laszlo Ersek ; = devel@edk2.groups.io > =E6=8A=84=E9=80=81: Michael D Kinney ; = Liming Gao > ; Zhiguang Liu ; = Grimes, > Paul > =E4=B8=BB=E9=A2=98: RE: [edk2-devel] [PATCH v1 1/1] MdePkg: Correcting > EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT definition >=20 > [AMD Public Use] >=20 > 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 >=20 > IMO if these defines were to be updated for clarity, it should = probably be done > for the whole file in a separate commit. >=20 > Thanks, > Paul >=20 > -----Original Message----- > From: Laszlo Ersek > Sent: Friday, August 28, 2020 10:06 AM > To: devel@edk2.groups.io; Grimes, Paul > Cc: Michael D Kinney ; Liming Gao > ; Zhiguang Liu > Subject: Re: [edk2-devel] [PATCH v1 1/1] MdePkg: Correcting > EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT definition >=20 > [CAUTION: External Email] >=20 > 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=3Dhttps%3A%2F%2F > > > bugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2937&data=3D02%7C01 > %7Cp > > > aul.grimes%40amd.com%7C82b28bb6544a4612fc1108d84b749dc6%7C3dd8 > 961fe488 > > > 4e608e11a82d994e183d%7C0%7C0%7C637342311528396385&sdata=3D > 7vHYIHHaJU > > 4yrXzAWtv5xTf%2BQfclAUBusz278%2F6I%2BRY%3D&reserved=3D0 > > > > Cc: Michael D Kinney > > Cc: Liming Gao > > Cc: Zhiguang Liu > > Signed-off-by: Paul G > > --- > > 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 > > >=20 > Good catch. The ACPI spec text was likely cut n' pasted into the edk2 = source, > and then prefixed with "0x". The spec says, >=20 > """ > Bits [1:0] DMA transfer type preference, _SIZ > 00 8-bit only > 01 8- and 16-bit > 10 16-bit only > 11 Reserved > """ >=20 > but that's in binary, not in hexadecimal. >=20 > 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 >=20 > #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 >=20 > But I agree the current patch is OK too: >=20 > Reviewed-by: Laszlo Ersek >=20 > I also agree it's a bugfix and should be merged now. >=20 > Thanks > Laszlo