public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Pete Batard" <pete@akeo.ie>
To: Sunny Wang <Sunny.Wang@arm.com>, devel@edk2.groups.io
Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>,
	Sami Mujawar <sami.mujawar@arm.com>,
	Jeremy Linton <jeremy.linton@arm.com>,
	Ard Biesheuvel <ardb+tianocore@kernel.org>
Subject: Re: [PATCH v2 1/2] Platform/RaspberryPi: Dynamically build UARTs info in ACPI
Date: Wed, 24 Mar 2021 12:44:13 +0000	[thread overview]
Message-ID: <67010c6f-100d-812a-423d-ae30756873c4@akeo.ie> (raw)
In-Reply-To: <20210324094524.184-1-Sunny.Wang@arm.com>

Hi Sunny,

I understand that you are going to re-send these patches due to e-mail 
mangling, but let me add a few of notes that might help you as you are 
doing that.

First of all, your patch series was missing a cover letter (there should 
have been a [PATCH v2 0/2]), that doesn't get applied, but that 
describes the series or the changes applied between 2 revisions.

This isn't a big deal for a series that has only 2 patches, but it seems 
to indicate that you missed some steps from Laszlo's helpful guide on 
how to configure your environment to send patches to EDK2, which you can 
find at:

https://github.com/tianocore/tianocore.github.io/wiki/Laszlo%27s-unkempt-git-guide-for-edk2-contributors-and-maintainers

Especially, you should have run the command:

git config format.coverletter true

Now, with regards to patch mangling, as you may be able to see, what I 
got is not as bad as what you seem to have been getting. In fact, 
barring a few issues (which I am pointing out below) your patch is 
almost usable, even with the double line feeds I am seeing from my 
e-mail client (because that's actually how I currently receive about 
half the patches that are posted on this list anyway, so much so that I 
have had to craft a quick script to remove them).

There are however some issues, that actually prevent your patch from 
applying properly, and, while I'm at it, I also added a few comments 
about some extra lines you added, that you should be able to fix for 
your next submission:

On 2021.03.24 09:45, Sunny Wang wrote:
> Changes:
> 
>    1. Add code to ConfigDxe driver and AcpiTables module to dynamically
> 
>       build either Mini UART or PL011 UART info in ACPI. This fixes the
> 
>       issue discussed in https://github.com/pftf/RPi4/issues/118.
> 
>    2. Cleanup by moving duplicate Debug Port 2 table related defines and
> 
>       structures to a newly created header file (RpiDebugPort2Table.h).
> 
> 
> 
> Testing Done:
> 
>    - Booted to UEFI shell and use acpiview command to check the result of
> 
>      the different UART settings in config.txt (enabling either Mini UART
> 
>      or PL011) and SPCR, DBG2 tables and device BTH0 are dynamically
> 
>      changed as expected.
> 
> 
> 
> Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
> 
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> 
> Cc: Jeremy Linton <jeremy.linton@arm.com>
> 
> Cc: Pete Batard <pete@akeo.ie>
> 
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> 
> Signed-off-by: Sunny Wang <sunny.wang@arm.com>
> 
> ---
> 
>   .../RaspberryPi/AcpiTables/AcpiTables.inf     |   9 +-
> 
>   .../RaspberryPi/AcpiTables/Dbg2MiniUart.aslc  |  82 ++++++++
> 
>   .../AcpiTables/{Dbg2.aslc => Dbg2Pl011.aslc}  | 187 ++++++++---------
> 
>   .../RaspberryPi/AcpiTables/SpcrMiniUart.aslc  |  92 +++++++++
> 
>   .../AcpiTables/{Spcr.aslc => SpcrPl011.aslc}  | 188 +++++++++---------
> 
>   Platform/RaspberryPi/AcpiTables/Uart.asl      |  10 +-
> 
>   .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 107 +++++++++-
> 
>   .../IndustryStandard/RpiDebugPort2Table.h     |  34 ++++
> 
>   8 files changed, 497 insertions(+), 212 deletions(-)
> 
>   create mode 100644 Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc
> 
>   rename Platform/RaspberryPi/AcpiTables/{Dbg2.aslc => Dbg2Pl011.aslc} (72%)
> 
>   create mode 100644 Platform/RaspberryPi/AcpiTables/SpcrMiniUart.aslc
> 
>   rename Platform/RaspberryPi/AcpiTables/{Spcr.aslc => SpcrPl011.aslc} (87%)
> 
>   create mode 100644 Platform/RaspberryPi/Include/IndustryStandard/RpiDebugPort2Table.h
> 
> 
> 
> diff --git a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
> 
> index d3363a76a1..fc8e927138 100644
> 
> --- a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
> 
> +++ b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
> 
> @@ -2,7 +2,7 @@
> 
>   #
> 
>   #  ACPI table data and ASL sources required to boot the platform.
> 
>   #
> 
> -#  Copyright (c) 2019, ARM Limited. All rights reserved.
> 
> +#  Copyright (c) 2019-2021, ARM Limited. All rights reserved.
> 
>   #  Copyright (c) 2017, Andrey Warkentin <andrey.warkentin@gmail.com>
> 
>   #  Copyright (c) Microsoft Corporation. All rights reserved.
> 
>   #
> 
> @@ -28,12 +28,14 @@
> 
>     Emmc.asl
> 
>     Madt.aslc
> 
>     Fadt.aslc
> 
> -  Dbg2.aslc
> 
> +  Dbg2MiniUart.aslc
> 
> +  Dbg2Pl011.aslc
> 
>     Gtdt.aslc
> 
>     Iort.aslc
> 
>     Dsdt.asl
> 
>     Csrt.aslc
> 
> -  Spcr.aslc
> 
> +  SpcrMiniUart.aslc
> 
> +  SpcrPl011.aslc
> 
>     Pptt.aslc
> 
>     SsdtThermal.asl
> 
> 

For the patch to be applicable, there should have been an additional 
space on the line above.
In other words, is should be ">  " and not just "> ".

> 
> @@ -71,3 +73,4 @@
> 
> 

Same here, and for about 4-5 more instances further down, that I'm not 
going to document.

If using 'git format-patch' and 'git send-email' as documented in 
Laszlo's guide, I would expect these to go away.

> 
>   [BuildOptions]
> 
>     GCC:*_*_*_ASL_FLAGS       = -vw3133 -vw3150
> 
> +

This doesn't have to do with patch mangling, but you are adding an 
unwarranted extra line here. Can you please remove it from your next 
submission?

> 
> diff --git a/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc b/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc
> 
> new file mode 100644
> 
> index 0000000000..c83b978731
> 
> --- /dev/null
> 
> +++ b/Platform/RaspberryPi/AcpiTables/Dbg2MiniUart.aslc
> 
> @@ -0,0 +1,82 @@
> 
> +/** @file
> 
> + *
> 
> + *  Debug Port Table (DBG2)
> 
> + *
> 
> + *  Copyright (c) 2019, Pete Batard <pete@akeo.ie>
> 
> + *  Copyright (c) 2012-2021, ARM Limited. All rights reserved.
> 
> + *
> 
> + *  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> + *
> 
> + **/
> 
> +
> 
> +#include <IndustryStandard/Acpi.h>
> 
> +#include <IndustryStandard/Bcm2836.h>
> 
> +#include <IndustryStandard/RpiDebugPort2Table.h>
> 
> +#include <Library/AcpiLib.h>
> 
> +#include <Library/PcdLib.h>
> 
> +
> 
> +#include "AcpiTables.h"
> 
> +
> 
> +#pragma pack(1)
> 
> +
> 
> +#define RPI_UART_INTERFACE_TYPE                         EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_BCM2835_UART
> 
> +#define RPI_UART_BASE_ADDRESS                           BCM2836_MINI_UART_BASE_ADDRESS
> 
> +#define RPI_UART_LENGTH                                 BCM2836_MINI_UART_LENGTH
> 
> +//
> 
> +// RPI_UART_STR should match the value used Uart.asl
> 
> +//
> 
> +#define RPI_UART_STR                                    { '\\', '_', 'S', 'B', '.', 'G', 'D', 'V', '0', '.', 'U', 'R', 'T', 'M', 0x00 }
> 
> +
> 
> +#define DBG2_DEBUG_PORT_DDI(NumReg, SubType, UartBase, UartAddrLen, UartNameStr) {                                    \
> 
> +    {                                                                                                                 \
> 
> +      EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION,         /* UINT8     Revision */                        \
> 
> +      sizeof (DBG2_DEBUG_DEVICE_INFORMATION),                         /* UINT16    Length */                          \
> 
> +      NumReg,                                                         /* UINT8     NumberofGenericAddressRegisters */ \
> 
> +      RPI_DBG2_NAMESPACESTRING_FIELD_SIZE,                            /* UINT16    NameSpaceStringLength */           \
> 
> +      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, NameSpaceString),     /* UINT16    NameSpaceStringOffset */           \
> 
> +      0,                                                              /* UINT16    OemDataLength */                   \
> 
> +      0,                                                              /* UINT16    OemDataOffset */                   \
> 
> +      EFI_ACPI_DBG2_PORT_TYPE_SERIAL,                                 /* UINT16    Port Type */                       \
> 
> +      SubType,                                                        /* UINT16    Port Subtype */                    \
> 
> +      {EFI_ACPI_RESERVED_BYTE, EFI_ACPI_RESERVED_BYTE},               /* UINT8     Reserved[2] */                     \
> 
> +      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, BaseAddressRegister), /* UINT16    BaseAddressRegister Offset */      \
> 
> +      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, AddressSize)          /* UINT16    AddressSize Offset */              \
> 
> +    },                                                                                                                \
> 
> +    ARM_GAS32 (UartBase),                            /* EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE BaseAddressRegister */ \
> 
> +    UartAddrLen,                                     /* UINT32  AddressSize */                                        \
> 
> +    UartNameStr                                      /* UINT8   NameSpaceString[MAX_DBG2_NAME_LEN] */                 \
> 
> +  }
> 
> +
> 
> +
> 
> +STATIC DBG2_TABLE Dbg2 = {
> 
> +  {
> 
> +    ACPI_HEADER (
> 
> +      EFI_ACPI_6_3_DEBUG_PORT_2_TABLE_SIGNATURE,
> 
> +      DBG2_TABLE,
> 
> +      EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION
> 
> +    ),
> 
> +    OFFSET_OF (DBG2_TABLE, Dbg2DeviceInfo),
> 
> +    RPI_DBG2_NUM_DEBUG_PORTS                                          /* UINT32  NumberDbgDeviceInfo */
> 
> +  },
> 
> +  {
> 
> +    /*
> 
> +     * Kernel Debug Port
> 
> +     */
> 
> +    DBG2_DEBUG_PORT_DDI (
> 
> +      RPI_DBG2_NUMBER_OF_GENERIC_ADDRESS_REGISTERS,
> 
> +      RPI_UART_INTERFACE_TYPE,
> 
> +      RPI_UART_BASE_ADDRESS,
> 
> +      RPI_UART_LENGTH,
> 
> +      RPI_UART_STR
> 
> +    ),
> 
> +  }
> 
> +};
> 
> +
> 
> +#pragma pack()
> 
> +
> 
> +//
> 
> +// Reference the table being generated to prevent the optimizer from removing
> 
> +// the data structure from the executable
> 
> +//
> 
> +VOID* CONST ReferenceAcpiTable = &Dbg2;
> 
> +

Extra line added. Please remove it.

> diff --git a/Platform/RaspberryPi/AcpiTables/Dbg2.aslc b/Platform/RaspberryPi/AcpiTables/Dbg2Pl011.aslc
> 
> similarity index 72%
> 
> rename from Platform/RaspberryPi/AcpiTables/Dbg2.aslc
> 
> rename to Platform/RaspberryPi/AcpiTables/Dbg2Pl011.aslc
> 
> index e3f2adae7e..dccfa24601 100644
> 
> --- a/Platform/RaspberryPi/AcpiTables/Dbg2.aslc
> 
> +++ b/Platform/RaspberryPi/AcpiTables/Dbg2Pl011.aslc
> 
> @@ -1,105 +1,82 @@
> 
> -/** @file
> 
> - *
> 
> - *  Debug Port Table (DBG2)
> 
> - *
> 
> - *  Copyright (c) 2019, Pete Batard <pete@akeo.ie>
> 
> - *  Copyright (c) 2012-2020, ARM Limited. All rights reserved.
> 
> - *
> 
> - *  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> - *
> 
> - **/
> 
> -
> 
> -#include <IndustryStandard/Acpi.h>
> 
> -#include <IndustryStandard/Bcm2836.h>
> 
> -#include <IndustryStandard/DebugPort2Table.h>
> 
> -#include <Library/AcpiLib.h>
> 
> -#include <Library/PcdLib.h>
> 
> -
> 
> -#include "AcpiTables.h"
> 
> -
> 
> -#pragma pack(1)
> 
> -
> 
> -#define RPI_DBG2_NUM_DEBUG_PORTS                        1
> 
> -#define RPI_DBG2_NUMBER_OF_GENERIC_ADDRESS_REGISTERS    1
> 
> -#define RPI_DBG2_NAMESPACESTRING_FIELD_SIZE             15
> 
> -
> 
> -#if (RPI_MODEL == 4)
> 
> -#define RPI_UART_INTERFACE_TYPE                         EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_PL011_UART
> 
> -#define RPI_UART_BASE_ADDRESS                           BCM2836_PL011_UART_BASE_ADDRESS
> 
> -#define RPI_UART_LENGTH                                 BCM2836_PL011_UART_LENGTH
> 
> -#define RPI_UART_STR                                    { '\\', '_', 'S', 'B', '.', 'G', 'D', 'V', '0', '.', 'U', 'R', 'T', '0', 0x00 }
> 
> -#else
> 
> -#define RPI_UART_INTERFACE_TYPE                         EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_BCM2835_UART
> 
> -#define RPI_UART_BASE_ADDRESS                           BCM2836_MINI_UART_BASE_ADDRESS
> 
> -#define RPI_UART_LENGTH                                 BCM2836_MINI_UART_LENGTH
> 
> -//
> 
> -// RPI_UART_STR should match the value used Uart.asl
> 
> -//
> 
> -#define RPI_UART_STR                                    { '\\', '_', 'S', 'B', '.', 'G', 'D', 'V', '0', '.', 'U', 'R', 'T', 'M', 0x00 }
> 
> -#endif
> 
> -
> 
> -typedef struct {
> 
> -  EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT         Dbg2Device;
> 
> -  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE                BaseAddressRegister;
> 
> -  UINT32                                                AddressSize;
> 
> -  UINT8                                                 NameSpaceString[RPI_DBG2_NAMESPACESTRING_FIELD_SIZE];
> 
> -} DBG2_DEBUG_DEVICE_INFORMATION;
> 
> -
> 
> -typedef struct {
> 
> -  EFI_ACPI_DEBUG_PORT_2_DESCRIPTION_TABLE               Description;
> 
> -  DBG2_DEBUG_DEVICE_INFORMATION                         Dbg2DeviceInfo[RPI_DBG2_NUM_DEBUG_PORTS];
> 
> -} DBG2_TABLE;
> 
> -
> 
> -
> 
> -#define DBG2_DEBUG_PORT_DDI(NumReg, SubType, UartBase, UartAddrLen, UartNameStr) {                                    \
> 
> -    {                                                                                                                 \
> 
> -      EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION,         /* UINT8     Revision */                        \
> 
> -      sizeof (DBG2_DEBUG_DEVICE_INFORMATION),                         /* UINT16    Length */                          \
> 
> -      NumReg,                                                         /* UINT8     NumberofGenericAddressRegisters */ \
> 
> -      RPI_DBG2_NAMESPACESTRING_FIELD_SIZE,                            /* UINT16    NameSpaceStringLength */           \
> 
> -      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, NameSpaceString),     /* UINT16    NameSpaceStringOffset */           \
> 
> -      0,                                                              /* UINT16    OemDataLength */                   \
> 
> -      0,                                                              /* UINT16    OemDataOffset */                   \
> 
> -      EFI_ACPI_DBG2_PORT_TYPE_SERIAL,                                 /* UINT16    Port Type */                       \
> 
> -      SubType,                                                        /* UINT16    Port Subtype */                    \
> 
> -      {EFI_ACPI_RESERVED_BYTE, EFI_ACPI_RESERVED_BYTE},               /* UINT8     Reserved[2] */                     \
> 
> -      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, BaseAddressRegister), /* UINT16    BaseAddressRegister Offset */      \
> 
> -      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, AddressSize)          /* UINT16    AddressSize Offset */              \
> 
> -    },                                                                                                                \
> 
> -    ARM_GAS32 (UartBase),                            /* EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE BaseAddressRegister */ \
> 
> -    UartAddrLen,                                     /* UINT32  AddressSize */                                        \
> 
> -    UartNameStr                                      /* UINT8   NameSpaceString[MAX_DBG2_NAME_LEN] */                 \
> 
> -  }
> 
> -
> 
> -
> 
> -STATIC DBG2_TABLE Dbg2 = {
> 
> -  {
> 
> -    ACPI_HEADER (
> 
> -      EFI_ACPI_6_3_DEBUG_PORT_2_TABLE_SIGNATURE,
> 
> -      DBG2_TABLE,
> 
> -      EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION
> 
> -    ),
> 
> -    OFFSET_OF (DBG2_TABLE, Dbg2DeviceInfo),
> 
> -    RPI_DBG2_NUM_DEBUG_PORTS                                          /* UINT32  NumberDbgDeviceInfo */
> 
> -  },
> 
> -  {
> 
> -    /*
> 
> -     * Kernel Debug Port
> 
> -     */
> 
> -    DBG2_DEBUG_PORT_DDI (
> 
> -      RPI_DBG2_NUMBER_OF_GENERIC_ADDRESS_REGISTERS,
> 
> -      RPI_UART_INTERFACE_TYPE,
> 
> -      RPI_UART_BASE_ADDRESS,
> 
> -      RPI_UART_LENGTH,
> 
> -      RPI_UART_STR
> 
> -    ),
> 
> -  }
> 
> -};
> 
> -
> 
> -#pragma pack()
> 
> -
> 
> -//
> 
> -// Reference the table being generated to prevent the optimizer from removing
> 
> -// the data structure from the executable
> 
> -//
> 
> -VOID* CONST ReferenceAcpiTable = &Dbg2;
> 
> +/** @file
> 
> + *
> 
> + *  Debug Port Table (DBG2)
> 
> + *
> 
> + *  Copyright (c) 2019, Pete Batard <pete@akeo.ie>
> 
> + *  Copyright (c) 2012-2021, ARM Limited. All rights reserved.
> 
> + *
> 
> + *  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> + *
> 
> + **/
> 
> +
> 
> +#include <IndustryStandard/Acpi.h>
> 
> +#include <IndustryStandard/Bcm2836.h>
> 
> +#include <IndustryStandard/RpiDebugPort2Table.h>
> 
> +#include <Library/AcpiLib.h>
> 
> +#include <Library/PcdLib.h>
> 
> +
> 
> +#include "AcpiTables.h"
> 
> +
> 
> +#pragma pack(1)
> 
> +
> 
> +#define RPI_UART_INTERFACE_TYPE                         EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_PL011_UART
> 
> +#define RPI_UART_BASE_ADDRESS                           BCM2836_PL011_UART_BASE_ADDRESS
> 
> +#define RPI_UART_LENGTH                                 BCM2836_PL011_UART_LENGTH
> 
> +//
> 
> +// RPI_UART_STR should match the value used Uart.asl
> 
> +//
> 
> +#define RPI_UART_STR                                    { '\\', '_', 'S', 'B', '.', 'G', 'D', 'V', '0', '.', 'U', 'R', 'T', '0', 0x00 }
> 
> +
> 
> +#define DBG2_DEBUG_PORT_DDI(NumReg, SubType, UartBase, UartAddrLen, UartNameStr) {                                    \
> 
> +    {                                                                                                                 \
> 
> +      EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION,         /* UINT8     Revision */                        \
> 
> +      sizeof (DBG2_DEBUG_DEVICE_INFORMATION),                         /* UINT16    Length */                          \
> 
> +      NumReg,                                                         /* UINT8     NumberofGenericAddressRegisters */ \
> 
> +      RPI_DBG2_NAMESPACESTRING_FIELD_SIZE,                            /* UINT16    NameSpaceStringLength */           \
> 
> +      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, NameSpaceString),     /* UINT16    NameSpaceStringOffset */           \
> 
> +      0,                                                              /* UINT16    OemDataLength */                   \
> 
> +      0,                                                              /* UINT16    OemDataOffset */                   \
> 
> +      EFI_ACPI_DBG2_PORT_TYPE_SERIAL,                                 /* UINT16    Port Type */                       \
> 
> +      SubType,                                                        /* UINT16    Port Subtype */                    \
> 
> +      {EFI_ACPI_RESERVED_BYTE, EFI_ACPI_RESERVED_BYTE},               /* UINT8     Reserved[2] */                     \
> 
> +      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, BaseAddressRegister), /* UINT16    BaseAddressRegister Offset */      \
> 
> +      OFFSET_OF (DBG2_DEBUG_DEVICE_INFORMATION, AddressSize)          /* UINT16    AddressSize Offset */              \
> 
> +    },                                                                                                                \
> 
> +    ARM_GAS32 (UartBase),                            /* EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE BaseAddressRegister */ \
> 
> +    UartAddrLen,                                     /* UINT32  AddressSize */                                        \
> 
> +    UartNameStr                                      /* UINT8   NameSpaceString[MAX_DBG2_NAME_LEN] */                 \
> 
> +  }
> 
> +
> 
> +
> 
> +STATIC DBG2_TABLE Dbg2 = {
> 
> +  {
> 
> +    ACPI_HEADER (
> 
> +      EFI_ACPI_6_3_DEBUG_PORT_2_TABLE_SIGNATURE,
> 
> +      DBG2_TABLE,
> 
> +      EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT_REVISION
> 
> +    ),
> 
> +    OFFSET_OF (DBG2_TABLE, Dbg2DeviceInfo),
> 
> +    RPI_DBG2_NUM_DEBUG_PORTS                                          /* UINT32  NumberDbgDeviceInfo */
> 
> +  },
> 
> +  {
> 
> +    /*
> 
> +     * Kernel Debug Port
> 
> +     */
> 
> +    DBG2_DEBUG_PORT_DDI (
> 
> +      RPI_DBG2_NUMBER_OF_GENERIC_ADDRESS_REGISTERS,
> 
> +      RPI_UART_INTERFACE_TYPE,
> 
> +      RPI_UART_BASE_ADDRESS,
> 
> +      RPI_UART_LENGTH,
> 
> +      RPI_UART_STR
> 
> +    ),
> 
> +  }
> 
> +};
> 
> +
> 
> +#pragma pack()
> 
> +
> 
> +//
> 
> +// Reference the table being generated to prevent the optimizer from removing
> 
> +// the data structure from the executable
> 
> +//
> 
> +VOID* CONST ReferenceAcpiTable = &Dbg2;
> 
> +

Same issue with extra line added.

> diff --git a/Platform/RaspberryPi/AcpiTables/SpcrMiniUart.aslc b/Platform/RaspberryPi/AcpiTables/SpcrMiniUart.aslc
> 
> new file mode 100644
> 
> index 0000000000..aaf5c5317e
> 
> --- /dev/null
> 
> +++ b/Platform/RaspberryPi/AcpiTables/SpcrMiniUart.aslc
> 
> @@ -0,0 +1,92 @@
> 
> +/** @file
> 
> +* SPCR Table
> 
> +*
> 
> +* Copyright (c) 2019 Pete Batard <pete@akeo.ie>
> 
> +* Copyright (c) 2014-2021, ARM Limited. All rights reserved.
> 
> +*
> 
> +* SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> +*
> 
> +**/
> 
> +
> 
> +#include <IndustryStandard/Acpi.h>
> 
> +#include <IndustryStandard/Bcm2836.h>
> 
> +#include <IndustryStandard/SerialPortConsoleRedirectionTable.h>
> 
> +#include <Library/AcpiLib.h>
> 
> +#include <Library/PcdLib.h>
> 
> +
> 
> +#include "AcpiTables.h"
> 
> +
> 
> +#define RPI_UART_FLOW_CONTROL_NONE           0
> 
> +
> 
> +#define RPI_UART_INTERFACE_TYPE              EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_BCM2835_UART
> 
> +#define RPI_UART_BASE_ADDRESS                BCM2836_MINI_UART_BASE_ADDRESS
> 
> +#define RPI_UART_INTERRUPT                   BCM2836_MINI_UART_INTERRUPT
> 
> +
> 
> +STATIC EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
> 
> +  ACPI_HEADER (
> 
> +    EFI_ACPI_6_3_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE,
> 
> +    EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE,
> 
> +    EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION
> 
> +  ),
> 
> +  // UINT8                                   InterfaceType;
> 
> +  RPI_UART_INTERFACE_TYPE,
> 
> +  // UINT8                                   Reserved1[3];
> 
> +  {
> 
> +    EFI_ACPI_RESERVED_BYTE,
> 
> +    EFI_ACPI_RESERVED_BYTE,
> 
> +    EFI_ACPI_RESERVED_BYTE
> 
> +  },
> 
> +  // EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE  BaseAddress;
> 
> +  ARM_GAS32 (RPI_UART_BASE_ADDRESS),
> 
> +  // UINT8                                   InterruptType;
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERRUPT_TYPE_GIC,
> 
> +  // UINT8                                   Irq;
> 
> +  0,                                         // Not used on ARM
> 
> +  // UINT32                                  GlobalSystemInterrupt;
> 
> +  RPI_UART_INTERRUPT,
> 
> +  // UINT8                                   BaudRate;
> 
> +#if (FixedPcdGet64 (PcdUartDefaultBaudRate) == 9600)
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_9600,
> 
> +#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 19200)
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_19200,
> 
> +#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 57600)
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_57600,
> 
> +#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 115200)
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_115200,
> 
> +#else
> 
> +#error Unsupported SPCR Baud Rate
> 
> +#endif
> 
> +  // UINT8                                   Parity;
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_PARITY_NO_PARITY,
> 
> +  // UINT8                                   StopBits;
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_STOP_BITS_1,
> 
> +  // UINT8                                   FlowControl;
> 
> +  RPI_UART_FLOW_CONTROL_NONE,
> 
> +  // UINT8                                   TerminalType;
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_TERMINAL_TYPE_VT_UTF8,
> 
> +  // UINT8                                   Reserved2;
> 
> +  EFI_ACPI_RESERVED_BYTE,
> 
> +  // UINT16                                  PciDeviceId;
> 
> +  0xFFFF,
> 
> +  // UINT16                                  PciVendorId;
> 
> +  0xFFFF,
> 
> +  // UINT8                                   PciBusNumber;
> 
> +  0x00,
> 
> +  // UINT8                                   PciDeviceNumber;
> 
> +  0x00,
> 
> +  // UINT8                                   PciFunctionNumber;
> 
> +  0x00,
> 
> +  // UINT32                                  PciFlags;
> 
> +  0x00000000,
> 
> +  // UINT8                                   PciSegment;
> 
> +  0x00,
> 
> +  // UINT32                                  Reserved3;
> 
> +  EFI_ACPI_RESERVED_DWORD
> 
> +};
> 
> +
> 
> +//
> 
> +// Reference the table being generated to prevent the optimizer from removing the
> 
> +// data structure from the executable
> 
> +//
> 
> +VOID* CONST ReferenceAcpiTable = &Spcr;
> 
> +

Same issue with unneeded extra line.

> 
> diff --git a/Platform/RaspberryPi/AcpiTables/Spcr.aslc b/Platform/RaspberryPi/AcpiTables/SpcrPl011.aslc
> 
> similarity index 87%
> 
> rename from Platform/RaspberryPi/AcpiTables/Spcr.aslc
> 
> rename to Platform/RaspberryPi/AcpiTables/SpcrPl011.aslc
> 
> index 07df3a718d..5a540adf08 100644
> 
> --- a/Platform/RaspberryPi/AcpiTables/Spcr.aslc
> 
> +++ b/Platform/RaspberryPi/AcpiTables/SpcrPl011.aslc
> 
> @@ -1,97 +1,91 @@
> 
> -/** @file
> 
> -* SPCR Table
> 
> -*
> 
> -* Copyright (c) 2019 Pete Batard <pete@akeo.ie>
> 
> -* Copyright (c) 2014-2016, ARM Limited. All rights reserved.
> 
> -*
> 
> -* SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> -*
> 
> -**/
> 
> -
> 
> -#include <IndustryStandard/Acpi.h>
> 
> -#include <IndustryStandard/Bcm2836.h>
> 
> -#include <IndustryStandard/SerialPortConsoleRedirectionTable.h>
> 
> -#include <Library/AcpiLib.h>
> 
> -#include <Library/PcdLib.h>
> 
> -
> 
> -#include "AcpiTables.h"
> 
> -
> 
> -#define RPI_UART_FLOW_CONTROL_NONE           0
> 
> -
> 
> -// Prefer PL011 serial output on the Raspberry Pi 4
> 
> -#if (RPI_MODEL == 4)
> 
> -#define RPI_UART_INTERFACE_TYPE              EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_ARM_PL011_UART
> 
> -#define RPI_UART_BASE_ADDRESS                BCM2836_PL011_UART_BASE_ADDRESS
> 
> -#define RPI_UART_INTERRUPT                   BCM2836_PL011_UART_INTERRUPT
> 
> -#else
> 
> -#define RPI_UART_INTERFACE_TYPE              EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_BCM2835_UART
> 
> -#define RPI_UART_BASE_ADDRESS                BCM2836_MINI_UART_BASE_ADDRESS
> 
> -#define RPI_UART_INTERRUPT                   BCM2836_MINI_UART_INTERRUPT
> 
> -#endif
> 
> -STATIC EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
> 
> -  ACPI_HEADER (
> 
> -    EFI_ACPI_6_3_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE,
> 
> -    EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE,
> 
> -    EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION
> 
> -  ),
> 
> -  // UINT8                                   InterfaceType;
> 
> -  RPI_UART_INTERFACE_TYPE,
> 
> -  // UINT8                                   Reserved1[3];
> 
> -  {
> 
> -    EFI_ACPI_RESERVED_BYTE,
> 
> -    EFI_ACPI_RESERVED_BYTE,
> 
> -    EFI_ACPI_RESERVED_BYTE
> 
> -  },
> 
> -  // EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE  BaseAddress;
> 
> -  ARM_GAS32 (RPI_UART_BASE_ADDRESS),
> 
> -  // UINT8                                   InterruptType;
> 
> -  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERRUPT_TYPE_GIC,
> 
> -  // UINT8                                   Irq;
> 
> -  0,                                         // Not used on ARM
> 
> -  // UINT32                                  GlobalSystemInterrupt;
> 
> -  RPI_UART_INTERRUPT,
> 
> -  // UINT8                                   BaudRate;
> 
> -#if (FixedPcdGet64 (PcdUartDefaultBaudRate) == 9600)
> 
> -  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_9600,
> 
> -#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 19200)
> 
> -  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_19200,
> 
> -#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 57600)
> 
> -  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_57600,
> 
> -#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 115200)
> 
> -  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_115200,
> 
> -#else
> 
> -#error Unsupported SPCR Baud Rate
> 
> -#endif
> 
> -  // UINT8                                   Parity;
> 
> -  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_PARITY_NO_PARITY,
> 
> -  // UINT8                                   StopBits;
> 
> -  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_STOP_BITS_1,
> 
> -  // UINT8                                   FlowControl;
> 
> -  RPI_UART_FLOW_CONTROL_NONE,
> 
> -  // UINT8                                   TerminalType;
> 
> -  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_TERMINAL_TYPE_VT_UTF8,
> 
> -  // UINT8                                   Reserved2;
> 
> -  EFI_ACPI_RESERVED_BYTE,
> 
> -  // UINT16                                  PciDeviceId;
> 
> -  0xFFFF,
> 
> -  // UINT16                                  PciVendorId;
> 
> -  0xFFFF,
> 
> -  // UINT8                                   PciBusNumber;
> 
> -  0x00,
> 
> -  // UINT8                                   PciDeviceNumber;
> 
> -  0x00,
> 
> -  // UINT8                                   PciFunctionNumber;
> 
> -  0x00,
> 
> -  // UINT32                                  PciFlags;
> 
> -  0x00000000,
> 
> -  // UINT8                                   PciSegment;
> 
> -  0x00,
> 
> -  // UINT32                                  Reserved3;
> 
> -  EFI_ACPI_RESERVED_DWORD
> 
> -};
> 
> -
> 
> -//
> 
> -// Reference the table being generated to prevent the optimizer from removing the
> 
> -// data structure from the executable
> 
> -//
> 
> -VOID* CONST ReferenceAcpiTable = &Spcr;
> 
> +/** @file
> 
> +* SPCR Table
> 
> +*
> 
> +* Copyright (c) 2019 Pete Batard <pete@akeo.ie>
> 
> +* Copyright (c) 2014-2021, ARM Limited. All rights reserved.
> 
> +*
> 
> +* SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> +*
> 
> +**/
> 
> +
> 
> +#include <IndustryStandard/Acpi.h>
> 
> +#include <IndustryStandard/Bcm2836.h>
> 
> +#include <IndustryStandard/SerialPortConsoleRedirectionTable.h>
> 
> +#include <Library/AcpiLib.h>
> 
> +#include <Library/PcdLib.h>
> 
> +
> 
> +#include "AcpiTables.h"
> 
> +
> 
> +#define RPI_UART_FLOW_CONTROL_NONE           0
> 
> +
> 
> +#define RPI_UART_INTERFACE_TYPE              EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_ARM_PL011_UART
> 
> +#define RPI_UART_BASE_ADDRESS                BCM2836_PL011_UART_BASE_ADDRESS
> 
> +#define RPI_UART_INTERRUPT                   BCM2836_PL011_UART_INTERRUPT
> 
> +
> 
> +STATIC EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
> 
> +  ACPI_HEADER (
> 
> +    EFI_ACPI_6_3_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE,
> 
> +    EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE,
> 
> +    EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION
> 
> +  ),
> 
> +  // UINT8                                   InterfaceType;
> 
> +  RPI_UART_INTERFACE_TYPE,
> 
> +  // UINT8                                   Reserved1[3];
> 
> +  {
> 
> +    EFI_ACPI_RESERVED_BYTE,
> 
> +    EFI_ACPI_RESERVED_BYTE,
> 
> +    EFI_ACPI_RESERVED_BYTE
> 
> +  },
> 
> +  // EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE  BaseAddress;
> 
> +  ARM_GAS32 (RPI_UART_BASE_ADDRESS),
> 
> +  // UINT8                                   InterruptType;
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERRUPT_TYPE_GIC,
> 
> +  // UINT8                                   Irq;
> 
> +  0,                                         // Not used on ARM
> 
> +  // UINT32                                  GlobalSystemInterrupt;
> 
> +  RPI_UART_INTERRUPT,
> 
> +  // UINT8                                   BaudRate;
> 
> +#if (FixedPcdGet64 (PcdUartDefaultBaudRate) == 9600)
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_9600,
> 
> +#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 19200)
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_19200,
> 
> +#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 57600)
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_57600,
> 
> +#elif (FixedPcdGet64 (PcdUartDefaultBaudRate) == 115200)
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_BAUD_RATE_115200,
> 
> +#else
> 
> +#error Unsupported SPCR Baud Rate
> 
> +#endif
> 
> +  // UINT8                                   Parity;
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_PARITY_NO_PARITY,
> 
> +  // UINT8                                   StopBits;
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_STOP_BITS_1,
> 
> +  // UINT8                                   FlowControl;
> 
> +  RPI_UART_FLOW_CONTROL_NONE,
> 
> +  // UINT8                                   TerminalType;
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_TERMINAL_TYPE_VT_UTF8,
> 
> +  // UINT8                                   Reserved2;
> 
> +  EFI_ACPI_RESERVED_BYTE,
> 
> +  // UINT16                                  PciDeviceId;
> 
> +  0xFFFF,
> 
> +  // UINT16                                  PciVendorId;
> 
> +  0xFFFF,
> 
> +  // UINT8                                   PciBusNumber;
> 
> +  0x00,
> 
> +  // UINT8                                   PciDeviceNumber;
> 
> +  0x00,
> 
> +  // UINT8                                   PciFunctionNumber;
> 
> +  0x00,
> 
> +  // UINT32                                  PciFlags;
> 
> +  0x00000000,
> 
> +  // UINT8                                   PciSegment;
> 
> +  0x00,
> 
> +  // UINT32                                  Reserved3;
> 
> +  EFI_ACPI_RESERVED_DWORD
> 
> +};
> 
> +
> 
> +//
> 
> +// Reference the table being generated to prevent the optimizer from removing the
> 
> +// data structure from the executable
> 
> +//
> 
> +VOID* CONST ReferenceAcpiTable = &Spcr;
> 
> diff --git a/Platform/RaspberryPi/AcpiTables/Uart.asl b/Platform/RaspberryPi/AcpiTables/Uart.asl
> 
> index 81ae6711af..8ce297078d 100644
> 
> --- a/Platform/RaspberryPi/AcpiTables/Uart.asl
> 
> +++ b/Platform/RaspberryPi/AcpiTables/Uart.asl
> 
> @@ -2,6 +2,7 @@
> 
>    *
> 
>    *  [DSDT] Serial devices (UART).
> 
>    *
> 
> + *  Copyright (c) 2021, ARM Limited. All rights reserved.
> 
>    *  Copyright (c) 2020, Pete Batard <pete@akeo.ie>
> 
>    *  Copyright (c) 2018, Andrey Warkentin <andrey.warkentin@gmail.com>
> 
>    *  Copyright (c) Microsoft Corporation. All rights reserved.
> 
> @@ -101,7 +102,10 @@ Device(BTH0)
> 
>     {
> 
>       Name (RBUF, ResourceTemplate ()
> 
>       {
> 
> -      // BT UART: URT0 (PL011) or URTM (miniUART)
> 
> +      //
> 
> +      // BT UART: ResourceSource will be dynamically updated to
> 
> +      // either URT0 (PL011) or URTM (miniUART) during boot
> 
> +      //
> 
>         UARTSerialBus(
> 
>           115200,        // InitialBaudRate: in BPS
> 
>           ,              // BitsPerByte: default to 8 bits
> 
> @@ -126,11 +130,7 @@ Device(BTH0)
> 
>                          //   no flow control.
> 
>           16,            // ReceiveBufferSize
> 
>           16,            // TransmitBufferSize
> 
> -#if (RPI_MODEL == 4)
> 
> -        "\\_SB.GDV0.URTM",  // ResourceSource:
> 
> -#else
> 
>           "\\_SB.GDV0.URT0",  // ResourceSource:
> 
> -#endif
> 
>                          //   UART bus controller name
> 
>           ,              // ResourceSourceIndex: assumed to be 0
> 
>           ,              // ResourceUsage: assumed to be
> 
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> 
> index 22f86d4d44..68ba14c846 100644
> 
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> 
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> 
> @@ -1,6 +1,6 @@
> 
>   /** @file
> 
>    *
> 
> - *  Copyright (c) 2019 - 2020, ARM Limited. All rights reserved.
> 
> + *  Copyright (c) 2019 - 2021, ARM Limited. All rights reserved.
> 
>    *  Copyright (c) 2018 - 2020, Andrei Warkentin <andrey.warkentin@gmail.com>
> 
>    *
> 
>    *  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -12,6 +12,9 @@
> 
>   #include <IndustryStandard/Bcm2836.h>
> 
>   #include <IndustryStandard/Bcm2836Gpio.h>
> 
>   #include <IndustryStandard/RpiMbox.h>
> 
> +#include <IndustryStandard/SerialPortConsoleRedirectionTable.h>
> 
> +#include <IndustryStandard/RpiDebugPort2Table.h>
> 
> +
> 
>   #include <Library/AcpiLib.h>
> 
>   #include <Library/DebugLib.h>
> 
>   #include <Library/DevicePathLib.h>
> 
> @@ -40,6 +43,8 @@ STATIC UINT32 mModelFamily = 0;
> 
>   STATIC UINT32 mModelInstalledMB = 0;
> 
> 
> 
>   STATIC EFI_MAC_ADDRESS  mMacAddress;
> 
> +BOOLEAN                 mUsePl011Uart;
> 
> +BOOLEAN                 mUseMiniUart;
> 
> 
> 
>   /*
> 
>    * The GUID inside Platform/RaspberryPi/RPi3/AcpiTables/AcpiTables.inf and
> 
> @@ -699,6 +704,71 @@ UpdateSdtNameOps (
> 
>     }
> 
>   }
> 
> 
> 
> +//
> 
> +// BTH0._HID.BCM2EA6
> 
> +//
> 
> +#define BTH0_HID_PATTERN_LEN 17
> 
> +
> 
> +//
> 
> +// \_SB.GDV0.URT
> 
> +//
> 
> +#define RESOURCE_SOURCE_PATTERN_LEN 13
> 
> +
> 
> +//
> 
> +// Scan the given namespace table (DSDT/SSDT) for AML NameOps
> 
> +// listed in the NameOpReplace structure. If one is found then
> 
> +// update the value in the table from the specified Pcd
> 
> +//
> 
> +// This allows us to have conditionals in AML controlled
> 
> +// by options in the BDS or detected during firmware bootstrap.
> 
> +// We could extend this concept for strings/etc but due to len
> 
> +// variations its probably easier to encode the strings
> 
> +// in the ASL and pick the correct one based off a variable.
> 
> +//
> 
> +STATIC
> 
> +VOID
> 
> +UpdateDevBth0 (
> 
> +  EFI_ACPI_DESCRIPTION_HEADER  *AcpiTable
> 
> +  )
> 
> +{
> 
> +  UINTN   Index;
> 
> +  CHAR8   Bth0HidPattern[BTH0_HID_PATTERN_LEN] = {0x42, 0x54, 0x48, 0x30, 0x08, 0x5F, 0X48, 0x49, 0x44, 0x0D, 0x42, 0x43, 0x4D, 0x32, 0x45, 0x41, 0x36};
> 
> +  CHAR8   ResourceSourcePattern[BTH0_HID_PATTERN_LEN] = {0x5C, 0x5F, 0x53, 0x42, 0x2E, 0x47, 0X44, 0x56, 0x30, 0x2E, 0x55, 0x52, 0x54};
> 
> +  BOOLEAN FoundBth0HidPattern;
> 
> +  UINT8   *SdtPtr;
> 
> +  UINT32  SdtSize;
> 
> +
> 
> +  FoundBth0HidPattern = FALSE;
> 
> +  SdtSize = AcpiTable->Length;
> 
> +
> 
> +  if (SdtSize > 0) {
> 
> +    SdtPtr = (UINT8 *)AcpiTable;
> 
> +    for (Index = 0; Index < (SdtSize - BTH0_HID_PATTERN_LEN); Index++) {
> 
> +      if (!FoundBth0HidPattern) {
> 
> +        if (CompareMem (SdtPtr + Index, Bth0HidPattern, BTH0_HID_PATTERN_LEN) == 0) {
> 
> +          FoundBth0HidPattern = TRUE;
> 
> +        }
> 
> +      } else {
> 
> +        if (CompareMem (SdtPtr + Index, ResourceSourcePattern, RESOURCE_SOURCE_PATTERN_LEN) == 0) {
> 
> +          if (mUsePl011Uart) {
> 
> +            //
> 
> +            // Since PL011 has been set as Primary UART, set the last char in
> 
> +            // ResourceSource string to 'M' (0x4D) so that Mini UART can be used as Secondary UART for BlueTooth.
> 
> +            //
> 
> +            SdtPtr[Index + RESOURCE_SOURCE_PATTERN_LEN] = 0x4D;
> 
> +          } else if (mUseMiniUart) {
> 
> +            //
> 
> +            // Since Mini UART has been set as Primary UART, set the last char in
> 
> +            // ResourceSource string to '0' (0x30) so that PL011 can be used as Secondary UART for BlueTooth.
> 
> +            //
> 
> +            SdtPtr[Index + RESOURCE_SOURCE_PATTERN_LEN] = 0x30;
> 
> +          }
> 
> +          break;
> 
> +        }
> 
> +      }
> 
> +    }
> 
> +  }
> 
> +}
> 
> 
> 
>   STATIC
> 
>   BOOLEAN
> 
> @@ -770,8 +840,14 @@ HandleDynamicNamespace (
> 
>   {
> 
>     UINTN Tables;
> 
> 
> 
> +  EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE *SpcrTable;
> 
> +  DBG2_TABLE                                     *Dbg2Table;
> 
> +
> 
>     switch (AcpiHeader->Signature) {
> 
>     case SIGNATURE_32 ('D', 'S', 'D', 'T'):
> 
> +    UpdateDevBth0 (AcpiHeader);
> 
> +    return TRUE;
> 
> +
> 
>     case SIGNATURE_32 ('S', 'S', 'D', 'T'):
> 
>       for (Tables = 0; SdtTables[Tables].OemTableId; Tables++) {
> 
>         if (AcpiHeader->OemTableId == SdtTables[Tables].OemTableId) {
> 
> @@ -779,14 +855,37 @@ HandleDynamicNamespace (
> 
>         }
> 
>       }
> 
>       DEBUG ((DEBUG_ERROR, "Found namespace table not in table list.\n"));
> 
> -
> 
>       return FALSE;
> 
> +
> 
>     case SIGNATURE_32 ('I', 'O', 'R', 'T'):
> 
>       // only enable the IORT on machines with >3G and no limit
> 
>       // to avoid problems with rhel/centos and other older OSs
> 
>       if (PcdGet32 (PcdRamLimitTo3GB) || !PcdGet32 (PcdRamMoreThan3GB)) {
> 
>         return FALSE;
> 
>       }
> 
> +    return TRUE;
> 
> +
> 
> +  case SIGNATURE_32 ('S', 'P', 'C', 'R'):
> 
> +    SpcrTable = (EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE *)AcpiHeader;
> 
> +    if (mUsePl011Uart && (SpcrTable->InterfaceType == EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_ARM_PL011_UART)) {
> 
> +      return TRUE;
> 
> +    } else if (mUseMiniUart && (SpcrTable->InterfaceType == EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_BCM2835_UART)) {
> 
> +      return TRUE;
> 
> +    } else {
> 
> +      return FALSE;
> 
> +    }
> 
> +    return TRUE;
> 
> +
> 
> +  case SIGNATURE_32 ('D', 'B', 'G', '2'):
> 
> +    Dbg2Table = (DBG2_TABLE *)AcpiHeader;
> 
> +    if (mUsePl011Uart && (Dbg2Table->Dbg2DeviceInfo[0].Dbg2Device.PortSubtype == EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_PL011_UART)) {
> 
> +      return TRUE;
> 
> +    } else if (mUseMiniUart && (Dbg2Table->Dbg2DeviceInfo[0].Dbg2Device.PortSubtype == EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_BCM2835_UART)) {
> 
> +      return TRUE;
> 
> +    } else {
> 
> +      return FALSE;
> 
> +    }
> 
> +    return TRUE;
> 
>     }
> 
> 
> 
>     return TRUE;
> 
> @@ -803,6 +902,9 @@ ConfigInitialize (
> 
>     EFI_STATUS                      Status;
> 
>     EFI_EVENT                       EndOfDxeEvent;
> 
> 
> 
> +  mUsePl011Uart = ((MmioRead32(GPIO_BASE_ADDRESS + 4) & 0x0003F000) == 0x00024000);
> 
> +  mUseMiniUart  = ((MmioRead32(GPIO_BASE_ADDRESS + 4) & 0x0003F000) == 0x00012000);
> 
> +
> 
>     Status = gBS->LocateProtocol (&gRaspberryPiFirmwareProtocolGuid,
> 
>                     NULL, (VOID**)&mFwProtocol);
> 
>     ASSERT_EFI_ERROR (Status);
> 
> @@ -859,3 +961,4 @@ ConfigInitialize (
> 
> 
> 
>     return EFI_SUCCESS;
> 
>   }
> 
> +
> 
> diff --git a/Platform/RaspberryPi/Include/IndustryStandard/RpiDebugPort2Table.h b/Platform/RaspberryPi/Include/IndustryStandard/RpiDebugPort2Table.h
> 
> new file mode 100644
> 
> index 0000000000..8f7452f97a
> 
> --- /dev/null
> 
> +++ b/Platform/RaspberryPi/Include/IndustryStandard/RpiDebugPort2Table.h
> 
> @@ -0,0 +1,34 @@
> 
> +/** @file
> 
> +
> 
> +  Copyright (c) 2021, ARM Limited. All rights reserved.
> 
> +
> 
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> +
> 
> + **/
> 
> +#ifndef __RPI_DEBUG_PORT_2_H__
> 
> +#define __RPI_DEBUG_PORT_2_H__
> 
> +
> 
> +#include <IndustryStandard/DebugPort2Table.h>
> 
> +
> 
> +#define RPI_DBG2_NUM_DEBUG_PORTS                        1
> 
> +#define RPI_DBG2_NUMBER_OF_GENERIC_ADDRESS_REGISTERS    1
> 
> +#define RPI_DBG2_NAMESPACESTRING_FIELD_SIZE             15
> 
> +
> 
> +#pragma pack(1)
> 
> +
> 
> +
> 
> +typedef struct {
> 
> +  EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT         Dbg2Device;
> 
> +  EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE                BaseAddressRegister;
> 
> +  UINT32                                                AddressSize;
> 
> +  UINT8                                                 NameSpaceString[RPI_DBG2_NAMESPACESTRING_FIELD_SIZE];
> 
> +} DBG2_DEBUG_DEVICE_INFORMATION;
> 
> +
> 
> +typedef struct {
> 
> +  EFI_ACPI_DEBUG_PORT_2_DESCRIPTION_TABLE               Description;
> 
> +  DBG2_DEBUG_DEVICE_INFORMATION                         Dbg2DeviceInfo[RPI_DBG2_NUM_DEBUG_PORTS];
> 
> +} DBG2_TABLE;
> 
> +
> 
> +#pragma pack()
> 
> +#endif  //__RPI_DEBUG_PORT_2_H__
> 
> +

And one last unneeded extra line that you can remove.

Hope this can help,

Regards,

/Pete

  parent reply	other threads:[~2021-03-24 12:44 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-24  9:45 [PATCH v2 1/2] Platform/RaspberryPi: Dynamically build UARTs info in ACPI Sunny Wang
2021-03-24  9:45 ` [PATCH v2 2/2] Platform/RaspberryPi: Enable Bluetooth and UART in Windows OS Sunny Wang
2021-03-24 19:50   ` Jeremy Linton
2021-03-26 14:33     ` [edk2-devel] " Mario Bălănică
2021-03-24 12:44 ` Pete Batard [this message]
2021-03-24 19:40 ` [PATCH v2 1/2] Platform/RaspberryPi: Dynamically build UARTs info in ACPI Jeremy Linton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=67010c6f-100d-812a-423d-ae30756873c4@akeo.ie \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox