[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 at intel.com>
> Cc: Liming Gao <gaoliming at byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu at intel.com>
> Signed-off-by: Paul G <paul.grimes at 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 at 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 (#64760): https://edk2.groups.io/g/devel/message/64760
Mute This Topic: https://groups.io/mt/76462757/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-