From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web12.1928.1616614835383264992 for ; Wed, 24 Mar 2021 12:40:35 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: jeremy.linton@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id AAD4B1529; Wed, 24 Mar 2021 12:40:24 -0700 (PDT) Received: from [192.168.122.166] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 218FE3F718; Wed, 24 Mar 2021 12:40:24 -0700 (PDT) Subject: Re: [PATCH v2 1/2] Platform/RaspberryPi: Dynamically build UARTs info in ACPI To: Sunny Wang , devel@edk2.groups.io Cc: Samer El-Haj-Mahmoud , Sami Mujawar , Pete Batard , Ard Biesheuvel References: <20210324094524.184-1-Sunny.Wang@arm.com> From: "Jeremy Linton" Message-ID: <560ff2cd-0a4f-e034-1994-91ec493ce310@arm.com> Date: Wed, 24 Mar 2021 14:40:23 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.0 MIME-Version: 1.0 In-Reply-To: <20210324094524.184-1-Sunny.Wang@arm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Hi, Thanks for working on this! Comments inline: On 3/24/21 4:45 AM, 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 > Cc: Sami Mujawar > Cc: Jeremy Linton > Cc: Pete Batard > Cc: Ard Biesheuvel > Signed-off-by: Sunny Wang > --- > .../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 > # 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 > > @@ -71,3 +73,4 @@ > > [BuildOptions] > GCC:*_*_*_ASL_FLAGS = -vw3133 -vw3150 > + > 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 > + * Copyright (c) 2012-2021, ARM Limited. All rights reserved. > + * > + * SPDX-License-Identifier: BSD-2-Clause-Patent > + * > + **/ > + > +#include > +#include > +#include > +#include > +#include > + > +#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; > + > 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 > - * Copyright (c) 2012-2020, ARM Limited. All rights reserved. > - * > - * SPDX-License-Identifier: BSD-2-Clause-Patent > - * > - **/ > - > -#include > -#include > -#include > -#include > -#include > - > -#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 > + * Copyright (c) 2012-2021, ARM Limited. All rights reserved. > + * > + * SPDX-License-Identifier: BSD-2-Clause-Patent > + * > + **/ > + > +#include > +#include > +#include > +#include > +#include > + > +#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; > + > 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 > +* Copyright (c) 2014-2021, ARM Limited. All rights reserved. > +* > +* SPDX-License-Identifier: BSD-2-Clause-Patent > +* > +**/ > + > +#include > +#include > +#include > +#include > +#include > + > +#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; > + > 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 > -* Copyright (c) 2014-2016, ARM Limited. All rights reserved. > -* > -* SPDX-License-Identifier: BSD-2-Clause-Patent > -* > -**/ > - > -#include > -#include > -#include > -#include > -#include > - > -#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 > +* Copyright (c) 2014-2021, ARM Limited. All rights reserved. > +* > +* SPDX-License-Identifier: BSD-2-Clause-Patent > +* > +**/ > + > +#include > +#include > +#include > +#include > +#include > + > +#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 > * Copyright (c) 2018, Andrey Warkentin > * 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 > * > * SPDX-License-Identifier: BSD-2-Clause-Patent > @@ -12,6 +12,9 @@ > #include > #include > #include > +#include > +#include > + > #include > #include > #include > @@ -40,6 +43,8 @@ STATIC UINT32 mModelFamily = 0; > STATIC UINT32 mModelInstalledMB = 0; > > STATIC EFI_MAC_ADDRESS mMacAddress; > +BOOLEAN mUsePl011Uart; > +BOOLEAN mUseMiniUart; Turning this into a PCD will make more sense later.. > > /* > * 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; > + } > + } > + } > + } > +} So, IMHO its better to extend the existing table driven nameop replacement scheme than create a compeltely custom function here. Part of that is because we will want to potentially apply some further user configuration to the DSDT (say customizing the _PSV value), and some of it is my own bias to keep this generic. I think there are at least two ways to pull this off, the first is to use a new NameOp integer tied to mUse???Uart to select the correct string (similarly to how the emmc is selecting a differing Package()) in AML. The other choice is to extend the existing UpdateSsdtNameOps to suport strings. I personally would pick the first choice since it seems easier. > > 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; > + See comment above about prefering a common DSDT/SSDT path. > 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; This else/return, return is odd since the second return should be redundant yet has a differing return. > + > + 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; Same here: So that said, it might be worth considering if the DBG2/SPCR OemId's are diffrent that these table could be handled by the same logic handing enabling/disabling the SSDT's since each table is enabled by a single true/false value. > } > > 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); Is there a case where we don't want at least one of these set? Thanks again for working on this. > + > 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 > + > +#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__ > + > -- > 2.31.0.windows.1 >