Re: [PATCH v3 edk2-platforms 2/5] Platform/ARM/Sgi: Add ACPI tables for SGI-Clark.Ares platform

[Leif Lindholm <leif.lindholm@linaro.org>]

On Mon, Nov 12, 2018 at 05:11:53PM +0530, Chandni Cherukuri wrote:
> Added ACPI tables and GUID name 'gSgiClarkAresAcpiTablesFileGuid'
> for SGI-Clark.Ares platform.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Signed-off-by: Chandni Cherukuri <chandni.cherukuri@arm.com>
> ---
>  Platform/ARM/SgiPkg/SgiPlatform.dec                     |   1 +
>  Platform/ARM/SgiPkg/SgiPlatform.dsc                     |   1 +
>  Platform/ARM/SgiPkg/SgiPlatform.fdf                     |   1 +
>  Platform/ARM/SgiPkg/AcpiTables/SgiClarkA/AcpiTables.inf |  58 +++++++
>  Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf |   1 +
>  Platform/ARM/SgiPkg/AcpiTables/SgiClarkA/Dbg2.aslc      |  90 +++++++++++
>  Platform/ARM/SgiPkg/AcpiTables/SgiClarkA/Dsdt.asl       | 116 +++++++++++++
>  Platform/ARM/SgiPkg/AcpiTables/SgiClarkA/Fadt.aslc      |  87 ++++++++++
>  Platform/ARM/SgiPkg/AcpiTables/SgiClarkA/Gtdt.aslc      | 152 +++++++++++++++++
>  Platform/ARM/SgiPkg/AcpiTables/SgiClarkA/Iort.aslc      | 106 ++++++++++++
>  Platform/ARM/SgiPkg/AcpiTables/SgiClarkA/Madt.aslc      | 171 ++++++++++++++++++++
>  Platform/ARM/SgiPkg/AcpiTables/SgiClarkA/Mcfg.aslc      |  61 +++++++
>  Platform/ARM/SgiPkg/AcpiTables/SgiClarkA/Spcr.aslc      |  77 +++++++++
>  Platform/ARM/SgiPkg/AcpiTables/SgiClarkA/Ssdt.asl       |  95 +++++++++++

Again, if SgiClarkA is a configuration, I would prefer the
configuration as a subdirectory, not merged with the platform name.

But I would also add that I would hope the intent is not to duplicate
everything into a separate "B" directory if/when a new configuration
turns up? Surely 98% of the content would remain unchanged.

If so, can we just drop the A from the directory name?

>  14 files changed, 1017 insertions(+)
> 
> diff --git a/Platform/ARM/SgiPkg/AcpiTables/SgiClarkA/Dbg2.aslc b/Platform/ARM/SgiPkg/AcpiTables/SgiClarkA/Dbg2.aslc
> new file mode 100644
> index 0000000..3970461
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/AcpiTables/SgiClarkA/Dbg2.aslc
> @@ -0,0 +1,90 @@
> +/** @file
> +*  Debug Port Table 2 (DBG2)
> +*
> +*  Copyright (c) 2018, ARM Limited. All rights reserved.
> +*
> +*  This program and the accompanying materials are licensed and made available
> +*  under the terms and conditions of the BSD License which accompanies this
> +*  distribution. The full text of the license may be found at
> +*  http://opensource.org/licenses/bsd-license.php
> +*
> +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +*
> +**/
> +
> +#include "SgiAcpiHeader.h"
> +#include <Library/AcpiLib.h>
> +#include <IndustryStandard/Acpi62.h>
> +#include <IndustryStandard/DebugPort2Table.h>
> +
> +#define SGI_DBG2_NUM_DEBUG_PORTS           1
> +#define SGI_DBG2_NUM_GAS                   1
> +#define SGI_DBG2_NS_STR_LENGTH             8
> +#define SGI_PL011_REGISTER_SPACE           0x1000
> +
> +#define NAME_STR_UART1     {'C', 'O', 'M', '1', '\0', '\0', '\0', '\0'}

Here was the thing I was referring to earlier today:

Is this used the same as what is now being fixed in
https://lists.01.org/pipermail/edk2-devel/2018-November/032523.html ?

If so, please update here and get it right from the start.

/
    Leif