[PATCH v1 0/1] Fix for define error in Acpi10.h

[PATCH v1 0/1] Fix for define error in Acpi10.h

[paul.grimes]

This change will update the EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT to
the correct value, according to the ACPI Specification.

REF:https://github.com/paulce06/edk2/tree/implement_acpi10.h_fix

Paul (1):
  MdePkg: Correcting EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT definition

 MdePkg/Include/IndustryStandard/Acpi10.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.21.0

[PATCH v1 1/1] MdePkg: Correcting EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT definition

[Paul]

In Acpi10.h, EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT is defined as 0x10,
but should be 0x02 per the ACPI Specification.

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2937

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Signed-off-by: Paul G <paul.grimes@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
-- 
2.21.0

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

[gaoliming]

Paul:
  This is a clear issue. Thanks for your reporting it. I would like to catch
the fix into this stable tag 202008. 
  Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

Thanks
Liming
> -----邮件原件-----
> 发件人: bounce+27952+64705+4905953+8761045@groups.io
> <bounce+27952+64705+4905953+8761045@groups.io> 代表 Paul
> 发送时间: 2020年8月28日 4:41
> 收件人: devel@edk2.groups.io
> 抄送: Michael D Kinney <michael.d.kinney@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>; Zhiguang Liu <zhiguang.liu@intel.com>
> 主题: [edk2-devel] [PATCH v1 1/1] MdePkg: Correcting
> EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT definition
> 
> In Acpi10.h, EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT is defined as 0x10,
> but should be 0x02 per the ACPI Specification.
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2937
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Signed-off-by: Paul G <paul.grimes@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
> 
> 
> --
> 2.21.0
> 
> 
> 

Re: [PATCH v1 1/1] MdePkg: Correcting EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT definition

[Zhiguang Liu]

Reviewed-by: Zhiguang Liu <zhiguang.liu@intel.com>

> -----Original Message-----
> From: Paul <Paul.Grimes@amd.com>
> Sent: Friday, August 28, 2020 4:41 AM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>
> Subject: [PATCH v1 1/1] MdePkg: Correcting
> EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT definition
> 
> In Acpi10.h, EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT is defined as 0x10,
> but should be 0x02 per the ACPI Specification.
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2937
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Signed-off-by: Paul G <paul.grimes@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
> 
> --
> 2.21.0

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

[Andrew Fish]

[-- Attachment #1: Type: text/plain, Size: 2403 bytes --]

Liming,

Sounds like a good thing to get into the stable tag.

Thanks,

Andrew Fish

> On Aug 27, 2020, at 5:57 PM, gaoliming <gaoliming@byosoft.com.cn> wrote:
> 
> Paul:
>  This is a clear issue. Thanks for your reporting it. I would like to catch
> the fix into this stable tag 202008. 
>  Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn <mailto:gaoliming@byosoft.com.cn>>
> 
> Thanks
> Liming
>> -----邮件原件-----
>> 发件人: bounce+27952+64705+4905953+8761045@groups.io <mailto:bounce+27952+64705+4905953+8761045@groups.io>
>> <bounce+27952+64705+4905953+8761045@groups.io <mailto:bounce+27952+64705+4905953+8761045@groups.io>> 代表 Paul
>> 发送时间: 2020年8月28日 4:41
>> 收件人: devel@edk2.groups.io <mailto:devel@edk2.groups.io>
>> 抄送: Michael D Kinney <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>>; Liming Gao
>> <gaoliming@byosoft.com.cn <mailto:gaoliming@byosoft.com.cn>>; Zhiguang Liu <zhiguang.liu@intel.com <mailto:zhiguang.liu@intel.com>>
>> 主题: [edk2-devel] [PATCH v1 1/1] MdePkg: Correcting
>> EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT definition
>> 
>> In Acpi10.h, EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT is defined as 0x10,
>> but should be 0x02 per the ACPI Specification.
>> 
>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2937
>> 
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
>> Signed-off-by: Paul G <paul.grimes@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
>> 
>> 
>> --
>> 2.21.0
>> 
>> 
>> 


[-- Attachment #2: Type: text/html, Size: 10134 bytes --]

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

[Laszlo Ersek]

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://bugzilla.tianocore.org/show_bug.cgi?id=2937
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Signed-off-by: Paul G <paul.grimes@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 <lersek@redhat.com>

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

Thanks
Laszlo

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

[Paul]

[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@redhat.com> 
Sent: Friday, August 28, 2020 10:06 AM
To: devel@edk2.groups.io; Grimes, Paul <Paul.Grimes@amd.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Zhiguang Liu <zhiguang.liu@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&amp;data=02%7C01%7Cp
> aul.grimes%40amd.com%7C82b28bb6544a4612fc1108d84b749dc6%7C3dd8961fe488
> 4e608e11a82d994e183d%7C0%7C0%7C637342311528396385&amp;sdata=7vHYIHHaJU
> 4yrXzAWtv5xTf%2BQfclAUBusz278%2F6I%2BRY%3D&amp;reserved=0
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Signed-off-by: Paul G <paul.grimes@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 <lersek@redhat.com>

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

Thanks
Laszlo

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

[Paul]

[AMD Public Use]

Thanks, Liming.  I'll proceed with the commit.

Thanks,
Paul

-----Original Message-----
From: gaoliming <gaoliming@byosoft.com.cn> 
Sent: Thursday, August 27, 2020 5:58 PM
To: devel@edk2.groups.io; Grimes, Paul <Paul.Grimes@amd.com>; 'Leif Lindholm' <leif@nuviainc.com>; 'Laszlo Ersek' <lersek@redhat.com>; afish@apple.com; 'Michael D Kinney' <michael.d.kinney@intel.com>
Cc: 'Michael D Kinney' <michael.d.kinney@intel.com>; 'Zhiguang Liu' <zhiguang.liu@intel.com>
Subject: 回复: [edk2-devel] [PATCH v1 1/1] MdePkg: Correcting EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT definition

[CAUTION: External Email]

Paul:
  This is a clear issue. Thanks for your reporting it. I would like to catch the fix into this stable tag 202008.
  Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

Thanks
Liming
> -----邮件原件-----
> 发件人: bounce+27952+64705+4905953+8761045@groups.io
> <bounce+27952+64705+4905953+8761045@groups.io> 代表 Paul
> 发送时间: 2020年8月28日 4:41
> 收件人: devel@edk2.groups.io
> 抄送: Michael D Kinney <michael.d.kinney@intel.com>; Liming Gao 
> <gaoliming@byosoft.com.cn>; Zhiguang Liu <zhiguang.liu@intel.com>
> 主题: [edk2-devel] [PATCH v1 1/1] MdePkg: Correcting 
> EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT definition
>
> 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%7Cd9425d4573104c573b5208d84aed6531%7C3dd8961fe488
> 4e608e11a82d994e183d%7C0%7C0%7C637341730767115023&amp;sdata=lVaJUuzXpm
> 91Ezi8phhdMxts8l4x9vTly0NIQsLV8%2FA%3D&amp;reserved=0
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Signed-off-by: Paul G <paul.grimes@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
>
>
> --
> 2.21.0
>
>
> 

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

[Paul]

[AMD Public Use]

* I'm new with this process.  I believe the next steps are in your hands?
Please let me know if you need anything from me to proceed. 😊

Thanks,
Paul

-----Original Message-----
From: Grimes, Paul <Paul.Grimes@amd.com> 
Sent: Friday, August 28, 2020 12:09 PM
To: gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io; 'Leif Lindholm' <leif@nuviainc.com>; 'Laszlo Ersek' <lersek@redhat.com>; afish@apple.com; 'Michael D Kinney' <michael.d.kinney@intel.com>
Cc: 'Michael D Kinney' <michael.d.kinney@intel.com>; 'Zhiguang Liu' <zhiguang.liu@intel.com>; Grimes, Paul <Paul.Grimes@amd.com>
Subject: RE: [edk2-devel] [PATCH v1 1/1] MdePkg: Correcting EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT definition

[AMD Public Use]

Thanks, Liming.  I'll proceed with the commit.

Thanks,
Paul

-----Original Message-----
From: gaoliming <gaoliming@byosoft.com.cn>
Sent: Thursday, August 27, 2020 5:58 PM
To: devel@edk2.groups.io; Grimes, Paul <Paul.Grimes@amd.com>; 'Leif Lindholm' <leif@nuviainc.com>; 'Laszlo Ersek' <lersek@redhat.com>; afish@apple.com; 'Michael D Kinney' <michael.d.kinney@intel.com>
Cc: 'Michael D Kinney' <michael.d.kinney@intel.com>; 'Zhiguang Liu' <zhiguang.liu@intel.com>
Subject: 回复: [edk2-devel] [PATCH v1 1/1] MdePkg: Correcting EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT definition

[CAUTION: External Email]

Paul:
  This is a clear issue. Thanks for your reporting it. I would like to catch the fix into this stable tag 202008.
  Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

Thanks
Liming
> -----邮件原件-----
> 发件人: bounce+27952+64705+4905953+8761045@groups.io
> <bounce+27952+64705+4905953+8761045@groups.io> 代表 Paul
> 发送时间: 2020年8月28日 4:41
> 收件人: devel@edk2.groups.io
> 抄送: Michael D Kinney <michael.d.kinney@intel.com>; Liming Gao 
> <gaoliming@byosoft.com.cn>; Zhiguang Liu <zhiguang.liu@intel.com>
> 主题: [edk2-devel] [PATCH v1 1/1] MdePkg: Correcting 
> EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT definition
>
> 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%7Cd9425d4573104c573b5208d84aed6531%7C3dd8961fe488
> 4e608e11a82d994e183d%7C0%7C0%7C637341730767115023&amp;sdata=lVaJUuzXpm
> 91Ezi8phhdMxts8l4x9vTly0NIQsLV8%2FA%3D&amp;reserved=0
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Signed-off-by: Paul G <paul.grimes@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
>
>
> --
> 2.21.0
>
>
> 

回复: [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@amd.com>
> 发送时间: 2020年8月29日 2:40
> 收件人: Laszlo Ersek <lersek@redhat.com>; devel@edk2.groups.io
> 抄送: Michael D Kinney <michael.d.kinney@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>; Zhiguang Liu <zhiguang.liu@intel.com>; Grimes,
> Paul <Paul.Grimes@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 <lersek@redhat.com>
> Sent: Friday, August 28, 2020 10:06 AM
> To: devel@edk2.groups.io; Grimes, Paul <Paul.Grimes@amd.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>; Zhiguang Liu <zhiguang.liu@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&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@intel.com>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> > Signed-off-by: Paul G <paul.grimes@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 <lersek@redhat.com>
> 
> I also agree it's a bugfix and should be merged now.
> 
> Thanks
> Laszlo

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

[gaoliming]

Merge @ 5ffcbc46908a2037ae3260d3cfcc103e4a6a48c0

But, I make a mistake. I miss Zhiguang and Laszlo reviewed-by for this patch. Sorry for it. 

Thanks
Liming
> -----邮件原件-----
> 发件人: bounce+27952+64779+4905953+8761045@groups.io
> <bounce+27952+64779+4905953+8761045@groups.io> 代表 Paul
> 发送时间: 2020年8月29日 3:24
> 收件人: gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io; 'Leif
> Lindholm' <leif@nuviainc.com>; 'Laszlo Ersek' <lersek@redhat.com>;
> afish@apple.com; 'Michael D Kinney' <michael.d.kinney@intel.com>
> 抄送: 'Michael D Kinney' <michael.d.kinney@intel.com>; 'Zhiguang Liu'
> <zhiguang.liu@intel.com>; Grimes, Paul <Paul.Grimes@amd.com>
> 主题: Re: [edk2-devel] [PATCH v1 1/1] MdePkg: Correcting
> EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT definition
> 
> [AMD Public Use]
> 
> * I'm new with this process.  I believe the next steps are in your hands?
> Please let me know if you need anything from me to proceed. 😊
> 
> Thanks,
> Paul
> 
> -----Original Message-----
> From: Grimes, Paul <Paul.Grimes@amd.com>
> Sent: Friday, August 28, 2020 12:09 PM
> To: gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io; 'Leif
> Lindholm' <leif@nuviainc.com>; 'Laszlo Ersek' <lersek@redhat.com>;
> afish@apple.com; 'Michael D Kinney' <michael.d.kinney@intel.com>
> Cc: 'Michael D Kinney' <michael.d.kinney@intel.com>; 'Zhiguang Liu'
> <zhiguang.liu@intel.com>; Grimes, Paul <Paul.Grimes@amd.com>
> Subject: RE: [edk2-devel] [PATCH v1 1/1] MdePkg: Correcting
> EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT definition
> 
> [AMD Public Use]
> 
> Thanks, Liming.  I'll proceed with the commit.
> 
> Thanks,
> Paul
> 
> -----Original Message-----
> From: gaoliming <gaoliming@byosoft.com.cn>
> Sent: Thursday, August 27, 2020 5:58 PM
> To: devel@edk2.groups.io; Grimes, Paul <Paul.Grimes@amd.com>; 'Leif
> Lindholm' <leif@nuviainc.com>; 'Laszlo Ersek' <lersek@redhat.com>;
> afish@apple.com; 'Michael D Kinney' <michael.d.kinney@intel.com>
> Cc: 'Michael D Kinney' <michael.d.kinney@intel.com>; 'Zhiguang Liu'
> <zhiguang.liu@intel.com>
> Subject: 回复: [edk2-devel] [PATCH v1 1/1] MdePkg: Correcting
> EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT definition
> 
> [CAUTION: External Email]
> 
> Paul:
>   This is a clear issue. Thanks for your reporting it. I would like to catch the fix
> into this stable tag 202008.
>   Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
> 
> Thanks
> Liming
> > -----邮件原件-----
> > 发件人: bounce+27952+64705+4905953+8761045@groups.io
> > <bounce+27952+64705+4905953+8761045@groups.io> 代表 Paul
> > 发送时间: 2020年8月28日 4:41
> > 收件人: devel@edk2.groups.io
> > 抄送: Michael D Kinney <michael.d.kinney@intel.com>; Liming Gao
> > <gaoliming@byosoft.com.cn>; Zhiguang Liu <zhiguang.liu@intel.com>
> > 主题: [edk2-devel] [PATCH v1 1/1] MdePkg: Correcting
> > EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT definition
> >
> > 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%7Cd9425d4573104c573b5208d84aed6531%7C3dd8
> 961fe488
> >
> 4e608e11a82d994e183d%7C0%7C0%7C637341730767115023&amp;sdata=l
> VaJUuzXpm
> > 91Ezi8phhdMxts8l4x9vTly0NIQsLV8%2FA%3D&amp;reserved=0
> >
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> > Signed-off-by: Paul G <paul.grimes@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
> >
> >
> > --
> > 2.21.0
> >
> >
> >
> 
> 
> 

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

[Laszlo Ersek]

On 08/28/20 20:39, Grimes, Paul wrote:
> [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.    

Sure, I'm OK with the patch as posted.
Laszlo

> 
> Thanks,
> Paul
> 
> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com> 
> Sent: Friday, August 28, 2020 10:06 AM
> To: devel@edk2.groups.io; Grimes, Paul <Paul.Grimes@amd.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Zhiguang Liu <zhiguang.liu@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&amp;data=02%7C01%7Cp
>> aul.grimes%40amd.com%7C82b28bb6544a4612fc1108d84b749dc6%7C3dd8961fe488
>> 4e608e11a82d994e183d%7C0%7C0%7C637342311528396385&amp;sdata=7vHYIHHaJU
>> 4yrXzAWtv5xTf%2BQfclAUBusz278%2F6I%2BRY%3D&amp;reserved=0
>>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
>> Signed-off-by: Paul G <paul.grimes@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 <lersek@redhat.com>
> 
> I also agree it's a bugfix and should be merged now.
> 
> Thanks
> Laszlo
> 

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

[Laszlo Ersek]

Hi Paul,

meta:

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://bugzilla.tianocore.org/show_bug.cgi?id=2937
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Signed-off-by: Paul G <paul.grimes@amd.com>
> ---
>  MdePkg/Include/IndustryStandard/Acpi10.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

please consider setting the "user.name" git config knob to your full
name. We now have:

commit 5ffcbc46908a2037ae3260d3cfcc103e4a6a48c0
Author: Paul <paul.grimes@amd.com>
Date:   Fri Aug 28 04:40:51 2020 +0800

    MdePkg: Correcting EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT definition

and I like to be friendly :) but I think the Author field should state,
in general, the full name, not just the first name.


Similary, the Signed-off-by tag at the end of the commit message should
carry the full name too. Please see section "Developer Certificate of
Origin" in "ReadMe.rst":

"""
Signed-off-by: Developer Name developer@example.org

where ``Developer Name`` is the contributor's real name, and the email
address is one the developer is reachable through at the time of
contributing.
"""

It's quite obvious from the email address that "Paul G" stands for Paul
Grimes, but still spell it out.


(These requests are for the future, of course; the present patch has
been merged.)

Thanks!
Laszlo

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

[Paul]

[AMD Public Use]

Hi Lazlo,

Thanks for the feedback.  Noted, I'll spell it out in the future.  'Last initial' was habit from a team I used to work with.

Thanks,
Paul


-----Original Message-----
From: Laszlo Ersek <lersek@redhat.com> 
Sent: Wednesday, September 2, 2020 1:58 AM
To: devel@edk2.groups.io; Grimes, Paul <Paul.Grimes@amd.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Zhiguang Liu <zhiguang.liu@intel.com>
Subject: Re: [edk2-devel] [PATCH v1 1/1] MdePkg: Correcting EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT definition

[CAUTION: External Email]

Hi Paul,

meta:

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%7C51cbad882a354dd9f33108d84f1e5a56%7C3dd8961fe488
> 4e608e11a82d994e183d%7C0%7C0%7C637346339077687090&amp;sdata=WT8dJXxJhK
> LhXU4qGObyYo3KN91WBs3%2FTesgkYdzssA%3D&amp;reserved=0
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Signed-off-by: Paul G <paul.grimes@amd.com>
> ---
>  MdePkg/Include/IndustryStandard/Acpi10.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

please consider setting the "user.name" git config knob to your full name. We now have:

commit 5ffcbc46908a2037ae3260d3cfcc103e4a6a48c0
Author: Paul <paul.grimes@amd.com>
Date:   Fri Aug 28 04:40:51 2020 +0800

    MdePkg: Correcting EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT definition

and I like to be friendly :) but I think the Author field should state, in general, the full name, not just the first name.


Similary, the Signed-off-by tag at the end of the commit message should carry the full name too. Please see section "Developer Certificate of Origin" in "ReadMe.rst":

"""
Signed-off-by: Developer Name developer@example.org

where ``Developer Name`` is the contributor's real name, and the email address is one the developer is reachable through at the time of contributing.
"""

It's quite obvious from the email address that "Paul G" stands for Paul Grimes, but still spell it out.


(These requests are for the future, of course; the present patch has been merged.)

Thanks!
Laszlo

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

[Philippe Mathieu-Daudé]

On 9/2/20 5:12 PM, Paul wrote:
> [AMD Public Use]
> 
> Hi Lazlo,
> 
> Thanks for the feedback.  Noted, I'll spell it out in the future.  'Last initial' was habit from a team I used to work with.
> 
> Thanks,
> Paul
> 
> 
> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com> 
> Sent: Wednesday, September 2, 2020 1:58 AM
> To: devel@edk2.groups.io; Grimes, Paul <Paul.Grimes@amd.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Zhiguang Liu <zhiguang.liu@intel.com>
> Subject: Re: [edk2-devel] [PATCH v1 1/1] MdePkg: Correcting EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT definition
> 
> [CAUTION: External Email]
> 
> Hi Paul,
> 
> meta:
> 
> 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%7C51cbad882a354dd9f33108d84f1e5a56%7C3dd8961fe488
>> 4e608e11a82d994e183d%7C0%7C0%7C637346339077687090&amp;sdata=WT8dJXxJhK
>> LhXU4qGObyYo3KN91WBs3%2FTesgkYdzssA%3D&amp;reserved=0
>>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
>> Signed-off-by: Paul G <paul.grimes@amd.com>
>> ---
>>  MdePkg/Include/IndustryStandard/Acpi10.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> please consider setting the "user.name" git config knob to your full name. We now have:
> 
> commit 5ffcbc46908a2037ae3260d3cfcc103e4a6a48c0
> Author: Paul <paul.grimes@amd.com>
> Date:   Fri Aug 28 04:40:51 2020 +0800
> 
>     MdePkg: Correcting EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT definition
> 
> and I like to be friendly :) but I think the Author field should state, in general, the full name, not just the first name.
> 
> 
> Similary, the Signed-off-by tag at the end of the commit message should carry the full name too. Please see section "Developer Certificate of Origin" in "ReadMe.rst":
> 
> """
> Signed-off-by: Developer Name developer@example.org
> 
> where ``Developer Name`` is the contributor's real name, and the email address is one the developer is reachable through at the time of contributing.
> """
> 
> It's quite obvious from the email address that "Paul G" stands for Paul Grimes, but still spell it out.

I just notice this and was going to make a comment :)

We can not fix the S-o-b tag, but Paul can fix his author name by
adding his entry in the .mailmap file:

-- >8 --
diff --git a/.mailmap b/.mailmap
index ba246ff6cd80..f3a4a5718e67 100644
--- a/.mailmap
+++ b/.mailmap
@@ -52,6 +52,7 @@ Michael Kubacki <michael.a.kubacki@intel.com>
 Michael Kubacki <michael.a.kubacki@intel.com> </o=Intel/ou=External
(FYDIBOHF25SPDLT)/cn=Recipients/cn=3c8b0226e75f4ab08d20c151cb7a8a72>
 Ming Tan <ming.tan@intel.com>
 Nikolai Saoukh <nms@otdel-1.org>
+Paul Grimes <paul.grimes@amd.com>
 Philippe Mathieu-Daudé <philmd@redhat.com>
 Ray Ni <ray.ni@intel.com>
 Ray Ni <ray.ni@intel.com> <C:/Program Files
(x86)/Git/O=Intel/OU=Pacifica02/cn=Recipients/cn=rni2>
---

Thanks,

Phil.

> 
> 
> (These requests are for the future, of course; the present patch has been merged.)
> 
> Thanks!
> Laszlo
> 
> 
> 

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

[Laszlo Ersek]

On 09/07/20 15:58, Philippe Mathieu-Daudé wrote:
> On 9/2/20 5:12 PM, Paul wrote:
>> [AMD Public Use]
>>
>> Hi Lazlo,
>>
>> Thanks for the feedback.  Noted, I'll spell it out in the future.  'Last initial' was habit from a team I used to work with.
>>
>> Thanks,
>> Paul
>>
>>
>> -----Original Message-----
>> From: Laszlo Ersek <lersek@redhat.com> 
>> Sent: Wednesday, September 2, 2020 1:58 AM
>> To: devel@edk2.groups.io; Grimes, Paul <Paul.Grimes@amd.com>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Zhiguang Liu <zhiguang.liu@intel.com>
>> Subject: Re: [edk2-devel] [PATCH v1 1/1] MdePkg: Correcting EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT definition
>>
>> [CAUTION: External Email]
>>
>> Hi Paul,
>>
>> meta:
>>
>> 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%7C51cbad882a354dd9f33108d84f1e5a56%7C3dd8961fe488
>>> 4e608e11a82d994e183d%7C0%7C0%7C637346339077687090&amp;sdata=WT8dJXxJhK
>>> LhXU4qGObyYo3KN91WBs3%2FTesgkYdzssA%3D&amp;reserved=0
>>>
>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>>> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
>>> Signed-off-by: Paul G <paul.grimes@amd.com>
>>> ---
>>>  MdePkg/Include/IndustryStandard/Acpi10.h | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> please consider setting the "user.name" git config knob to your full name. We now have:
>>
>> commit 5ffcbc46908a2037ae3260d3cfcc103e4a6a48c0
>> Author: Paul <paul.grimes@amd.com>
>> Date:   Fri Aug 28 04:40:51 2020 +0800
>>
>>     MdePkg: Correcting EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT definition
>>
>> and I like to be friendly :) but I think the Author field should state, in general, the full name, not just the first name.
>>
>>
>> Similary, the Signed-off-by tag at the end of the commit message should carry the full name too. Please see section "Developer Certificate of Origin" in "ReadMe.rst":
>>
>> """
>> Signed-off-by: Developer Name developer@example.org
>>
>> where ``Developer Name`` is the contributor's real name, and the email address is one the developer is reachable through at the time of contributing.
>> """
>>
>> It's quite obvious from the email address that "Paul G" stands for Paul Grimes, but still spell it out.
> 
> I just notice this and was going to make a comment :)
> 
> We can not fix the S-o-b tag, but Paul can fix his author name by
> adding his entry in the .mailmap file:
> 
> -- >8 --
> diff --git a/.mailmap b/.mailmap
> index ba246ff6cd80..f3a4a5718e67 100644
> --- a/.mailmap
> +++ b/.mailmap
> @@ -52,6 +52,7 @@ Michael Kubacki <michael.a.kubacki@intel.com>
>  Michael Kubacki <michael.a.kubacki@intel.com> </o=Intel/ou=External
> (FYDIBOHF25SPDLT)/cn=Recipients/cn=3c8b0226e75f4ab08d20c151cb7a8a72>
>  Ming Tan <ming.tan@intel.com>
>  Nikolai Saoukh <nms@otdel-1.org>
> +Paul Grimes <paul.grimes@amd.com>
>  Philippe Mathieu-Daudé <philmd@redhat.com>
>  Ray Ni <ray.ni@intel.com>
>  Ray Ni <ray.ni@intel.com> <C:/Program Files
> (x86)/Git/O=Intel/OU=Pacifica02/cn=Recipients/cn=rni2>
> ---
> 

Right, please see here:

[edk2-devel] [PATCH 18/22] .mailmap: add entry for Paul Grimes
https://edk2.groups.io/g/devel/message/65108
http://mid.mail-archive.com/20200907193102.30535-19-lersek@redhat.com

Thanks!
Laszlo