Re: [PATCH edk2-platforms v2 03/43] Silicon/Hisilicon/Acpi: Move some macro to PlatformArch.h

[Ming <ming.huang@linaro.org>]

在 8/17/2018 9:23 PM, Leif Lindholm 写道:
> 
> On Tue, Aug 14, 2018 at 04:08:23PM +0800, Ming Huang wrote:
>> ARM_ACPI_HEADER is used by a unify module in HwPkg,
>> so move some macro to PlatformArch.h for unify D0x.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ming Huang <ming.huang@linaro.org>
>> ---
>>  Silicon/Hisilicon/Hi1610/Hi1610AcpiTables/Hi1610Platform.h | 27 +++-----------------
>>  Silicon/Hisilicon/Hi1610/Include/PlatformArch.h            | 24 +++++++++++++++++
>>  Silicon/Hisilicon/Hi1616/D05AcpiTables/Hi1616Platform.h    | 24 +----------------
>>  Silicon/Hisilicon/Hi1616/Include/PlatformArch.h            | 23 +++++++++++++++++
>>  4 files changed, 51 insertions(+), 47 deletions(-)
>>
>> diff --git a/Silicon/Hisilicon/Hi1610/Hi1610AcpiTables/Hi1610Platform.h b/Silicon/Hisilicon/Hi1610/Hi1610AcpiTables/Hi1610Platform.h
>> index 5a95b02055..28546bea99 100644
>> --- a/Silicon/Hisilicon/Hi1610/Hi1610AcpiTables/Hi1610Platform.h
>> +++ b/Silicon/Hisilicon/Hi1610/Hi1610AcpiTables/Hi1610Platform.h
>> @@ -1,8 +1,8 @@
>>  /** @file
>>  *
>>  *  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
>> -*  Copyright (c) 2015, Hisilicon Limited. All rights reserved.
>> -*  Copyright (c) 2015, Linaro Limited. All rights reserved.
>> +*  Copyright (c) 2015-2018, Hisilicon Limited. All rights reserved.
>> +*  Copyright (c) 2015-2018, Linaro Limited. All rights reserved.
>>  *
>>  *  This program and the accompanying materials
>>  *  are licensed and made available under the terms and conditions of the BSD License
>> @@ -20,28 +20,7 @@
>>  #ifndef _HI1610_PLATFORM_H_
>>  #define _HI1610_PLATFORM_H_
>>  
>> -//
>> -// ACPI table information used to initialize tables.
>> -//
>> -#define EFI_ACPI_ARM_OEM_ID           'H','I','S','I',' ',' '   // OEMID 6 bytes long
>> -#define EFI_ACPI_ARM_OEM_TABLE_ID     SIGNATURE_64('H','I','P','0','6',' ',' ',' ') // OEM table id 8 bytes long
>> -#define EFI_ACPI_ARM_OEM_REVISION     0x00000000
>> -#define EFI_ACPI_ARM_CREATOR_ID       SIGNATURE_32('I','N','T','L')
>> -#define EFI_ACPI_ARM_CREATOR_REVISION 0x20151124
>> -
>> -// A macro to initialise the common header part of EFI ACPI tables as defined by
>> -// EFI_ACPI_DESCRIPTION_HEADER structure.
>> -#define ARM_ACPI_HEADER(Signature, Type, Revision) {              \
>> -    Signature,                      /* UINT32  Signature */       \
>> -    sizeof (Type),                  /* UINT32  Length */          \
>> -    Revision,                       /* UINT8   Revision */        \
>> -    0,                              /* UINT8   Checksum */        \
>> -    { EFI_ACPI_ARM_OEM_ID },        /* UINT8   OemId[6] */        \
>> -    EFI_ACPI_ARM_OEM_TABLE_ID,      /* UINT64  OemTableId */      \
>> -    EFI_ACPI_ARM_OEM_REVISION,      /* UINT32  OemRevision */     \
>> -    EFI_ACPI_ARM_CREATOR_ID,        /* UINT32  CreatorId */       \
>> -    EFI_ACPI_ARM_CREATOR_REVISION   /* UINT32  CreatorRevision */ \
>> -  }
>> +#include <../Include/PlatformArch.h>
> 
> I pointed out in review of v1: no relative include paths.
> 
> Cleanest way of resolving this is by adding a
> Silicon/Hisilicon/Hi1610/Hi1610.dec, much like
> Platform/Hisilicon/D03/D03.dec, and add a dependency on that in the
> [Packages] section of
> Silicon/Hisilicon/Hi1610/Hi1610AcpiTables/AcpiTablesHi1610.inf.
> 
>>  
>>  #define HI1610_WATCHDOG_COUNT  2
>>  
>> diff --git a/Silicon/Hisilicon/Hi1610/Include/PlatformArch.h b/Silicon/Hisilicon/Hi1610/Include/PlatformArch.h
>> index 45995c5893..8e5913447c 100644
>> --- a/Silicon/Hisilicon/Hi1610/Include/PlatformArch.h
>> +++ b/Silicon/Hisilicon/Hi1610/Include/PlatformArch.h
>> @@ -31,5 +31,29 @@
>>  
>>  #define S1_BASE               0x40000000000
>>  
>> +
>> +//
>> +// ACPI table information used to initialize tables.
>> +//
>> +#define EFI_ACPI_ARM_OEM_ID           'H','I','S','I',' ',' '   // OEMID 6 bytes long
>> +#define EFI_ACPI_ARM_OEM_TABLE_ID     SIGNATURE_64 ('H','I','P','0','6',' ',' ',' ') // OEM table id 8 bytes long
>> +#define EFI_ACPI_ARM_OEM_REVISION     0x00000000
>> +#define EFI_ACPI_ARM_CREATOR_ID       SIGNATURE_32 ('I','N','T','L')
>> +#define EFI_ACPI_ARM_CREATOR_REVISION 0x20151124
>> +
>> +// A macro to initialise the common header part of EFI ACPI tables as defined by
>> +// EFI_ACPI_DESCRIPTION_HEADER structure.
>> +#define ARM_ACPI_HEADER(Signature, Type, Revision) {            \
>> +  Signature,                      /* UINT32  Signature */       \
>> +  sizeof (Type),                  /* UINT32  Length */          \
>> +  Revision,                       /* UINT8   Revision */        \
>> +  0,                              /* UINT8   Checksum */        \
>> +  { EFI_ACPI_ARM_OEM_ID },        /* UINT8   OemId[6] */        \
>> +  EFI_ACPI_ARM_OEM_TABLE_ID,      /* UINT64  OemTableId */      \
>> +  EFI_ACPI_ARM_OEM_REVISION,      /* UINT32  OemRevision */     \
>> +  EFI_ACPI_ARM_CREATOR_ID,        /* UINT32  CreatorId */       \
>> +  EFI_ACPI_ARM_CREATOR_REVISION   /* UINT32  CreatorRevision */ \
>> +  }
>> +
>>  #endif
>>  
>> diff --git a/Silicon/Hisilicon/Hi1616/D05AcpiTables/Hi1616Platform.h b/Silicon/Hisilicon/Hi1616/D05AcpiTables/Hi1616Platform.h
>> index ad73aa2668..04f9e34a14 100644
>> --- a/Silicon/Hisilicon/Hi1616/D05AcpiTables/Hi1616Platform.h
>> +++ b/Silicon/Hisilicon/Hi1616/D05AcpiTables/Hi1616Platform.h
>> @@ -21,29 +21,7 @@
>>  #define _HI1610_PLATFORM_H_
>>  
>>  #include <IndustryStandard/Acpi.h>
>> -
>> -//
>> -// ACPI table information used to initialize tables.
>> -//
>> -#define EFI_ACPI_ARM_OEM_ID           'H','I','S','I',' ',' '   // OEMID 6 bytes long
>> -#define EFI_ACPI_ARM_OEM_TABLE_ID     SIGNATURE_64('H','I','P','0','7',' ',' ',' ') // OEM table id 8 bytes long
>> -#define EFI_ACPI_ARM_OEM_REVISION     0x00000000
>> -#define EFI_ACPI_ARM_CREATOR_ID       SIGNATURE_32('I','N','T','L')
>> -#define EFI_ACPI_ARM_CREATOR_REVISION 0x20151124
>> -
>> -// A macro to initialise the common header part of EFI ACPI tables as defined by
>> -// EFI_ACPI_DESCRIPTION_HEADER structure.
>> -#define ARM_ACPI_HEADER(Signature, Type, Revision) {              \
>> -    Signature,                      /* UINT32  Signature */       \
>> -    sizeof (Type),                  /* UINT32  Length */          \
>> -    Revision,                       /* UINT8   Revision */        \
>> -    0,                              /* UINT8   Checksum */        \
>> -    { EFI_ACPI_ARM_OEM_ID },        /* UINT8   OemId[6] */        \
>> -    EFI_ACPI_ARM_OEM_TABLE_ID,      /* UINT64  OemTableId */      \
>> -    EFI_ACPI_ARM_OEM_REVISION,      /* UINT32  OemRevision */     \
>> -    EFI_ACPI_ARM_CREATOR_ID,        /* UINT32  CreatorId */       \
>> -    EFI_ACPI_ARM_CREATOR_REVISION   /* UINT32  CreatorRevision */ \
>> -  }
>> +#include <../Include/PlatformArch.h>
> 
> Same comment, no relative includes.
> But here a Silicon/Hisilicon/Hi1616/Hi1616.dec.

Apology for miss the comments in v2 and thank for providing the way.
Modify it in v4.

> 
> /
>     Leif
> 
>>  
>>  #define HI1616_WATCHDOG_COUNT  2
>>  #define HI1616_GIC_STRUCTURE_COUNT  64
>> diff --git a/Silicon/Hisilicon/Hi1616/Include/PlatformArch.h b/Silicon/Hisilicon/Hi1616/Include/PlatformArch.h
>> index bf0994a5e9..24167d1633 100644
>> --- a/Silicon/Hisilicon/Hi1616/Include/PlatformArch.h
>> +++ b/Silicon/Hisilicon/Hi1616/Include/PlatformArch.h
>> @@ -31,5 +31,28 @@
>>  
>>  #define S1_BASE               0x40000000000
>>  
>> +//
>> +// ACPI table information used to initialize tables.
>> +//
>> +#define EFI_ACPI_ARM_OEM_ID           'H','I','S','I',' ',' '   // OEMID 6 bytes long
>> +#define EFI_ACPI_ARM_OEM_TABLE_ID     SIGNATURE_64 ('H','I','P','0','7',' ',' ',' ') // OEM table id 8 bytes long
>> +#define EFI_ACPI_ARM_OEM_REVISION     0x00000000
>> +#define EFI_ACPI_ARM_CREATOR_ID       SIGNATURE_32 ('I','N','T','L')
>> +#define EFI_ACPI_ARM_CREATOR_REVISION 0x20151124
>> +
>> +// A macro to initialise the common header part of EFI ACPI tables as defined by
>> +// EFI_ACPI_DESCRIPTION_HEADER structure.
>> +#define ARM_ACPI_HEADER(Signature, Type, Revision) {            \
>> +  Signature,                      /* UINT32  Signature */       \
>> +  sizeof (Type),                  /* UINT32  Length */          \
>> +  Revision,                       /* UINT8   Revision */        \
>> +  0,                              /* UINT8   Checksum */        \
>> +  { EFI_ACPI_ARM_OEM_ID },        /* UINT8   OemId[6] */        \
>> +  EFI_ACPI_ARM_OEM_TABLE_ID,      /* UINT64  OemTableId */      \
>> +  EFI_ACPI_ARM_OEM_REVISION,      /* UINT32  OemRevision */     \
>> +  EFI_ACPI_ARM_CREATOR_ID,        /* UINT32  CreatorId */       \
>> +  EFI_ACPI_ARM_CREATOR_REVISION   /* UINT32  CreatorRevision */ \
>> +  }
>> +
>>  #endif
>>  
>> -- 
>> 2.17.0
>>