The PL011 can be a better choice for the serial console on the RPi4,
given that its baud clock is not derived from the CPU clock (which
may change under our feet unless we keep it fixed at a low rate), and
given the fact that the SBSA/SBBR specs that describe ARM specific
architectural requirements for ACPI only permit PL011 based UARTs to
begin with.
Therefore we add a new PL011_ENABLE build switch to tell the firmware
to use PL011 for all console serial I/O, including for TF-A, SPCR and
DBG2, as well as toggle the BlueTooth module to use the mini UART.
For the time being, the option is disabled by default because it
requires an overlay to be enabled in config.txt that pinmuxes the
PL011 TX/RX lines to the UART pins on the connector block.
Signed-off-by: Pete Batard <pete@akeo.ie>
---
Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc | 10 +++++
Platform/RaspberryPi/RPi4/AcpiTables/Spcr.aslc | 9 +++-
Platform/RaspberryPi/RPi4/AcpiTables/Uart.asl | 6 ++-
Platform/RaspberryPi/RPi4/RPi4.dsc | 23 +++++++++++
Platform/RaspberryPi/RPi4/RPi4.fdf | 4 ++
Platform/RaspberryPi/RPi4/Readme.md | 43 ++++++++++++++++----
6 files changed, 84 insertions(+), 11 deletions(-)
diff --git a/Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc b/Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc
index 589a5c2cbd71..e408eb02d027 100644
--- a/Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc
+++ b/Platform/RaspberryPi/RPi4/AcpiTables/Dbg2.aslc
@@ -22,6 +22,11 @@
#define RPI_DBG2_NUMBER_OF_GENERIC_ADDRESS_REGISTERS 1
#define RPI_DBG2_NAMESPACESTRING_FIELD_SIZE 10
+#ifdef PL011_ENABLE
+#define RPI_UART_BASE_ADDRESS FixedPcdGet64 (PcdSerialRegisterBase)
+#define RPI_UART_LENGTH 0x1000
+#define RPI_UART_STR { '\\', '_', 'S', 'B', '.', 'U', 'R', 'T', '0', 0x00 }
+#else
//
// When using the miniUART, PcdSerialRegisterBase points to the 8250 base address,
// which is offset by 0x40 from the actual Bcm2835 base address
@@ -32,6 +37,7 @@
// RPI_UART_STR should match the value used Uart.asl
//
#define RPI_UART_STR { '\\', '_', 'S', 'B', '.', 'U', 'R', 'T', 'M', 0x00 }
+#endif
typedef struct {
EFI_ACPI_DBG2_DEBUG_DEVICE_INFORMATION_STRUCT Dbg2Device;
@@ -83,7 +89,11 @@ STATIC DBG2_TABLE Dbg2 = {
*/
DBG2_DEBUG_PORT_DDI (
RPI_DBG2_NUMBER_OF_GENERIC_ADDRESS_REGISTERS,
+#ifdef PL011_ENABLE
+ EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_ARM_PL011_UART,
+#else
EFI_ACPI_DBG2_PORT_SUBTYPE_SERIAL_BCM2835_UART,
+#endif
RPI_UART_BASE_ADDRESS,
RPI_UART_LENGTH,
RPI_UART_STR
diff --git a/Platform/RaspberryPi/RPi4/AcpiTables/Spcr.aslc b/Platform/RaspberryPi/RPi4/AcpiTables/Spcr.aslc
index 65f48ceae688..e5c10c923626 100644
--- a/Platform/RaspberryPi/RPi4/AcpiTables/Spcr.aslc
+++ b/Platform/RaspberryPi/RPi4/AcpiTables/Spcr.aslc
@@ -18,12 +18,19 @@
#define RPI_UART_FLOW_CONTROL_NONE 0
+#ifdef PL011_ENABLE
+#define RPI_UART_INTERFACE_TYPE EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_ARM_PL011_UART
+#define RPI_UART_BASE_ADDRESS FixedPcdGet64 (PcdSerialRegisterBase)
+#define RPI_UART_INTERRUPT 0x99
+#else
+#define RPI_UART_INTERFACE_TYPE EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_BCM2835_UART
//
// When using the miniUART, PcdSerialRegisterBase points to the 8250 base address,
// which is offset by 0x40 from the actual Bcm2835 base address
//
#define RPI_UART_BASE_ADDRESS (FixedPcdGet64 (PcdSerialRegisterBase) - 0x40)
#define RPI_UART_INTERRUPT 0x7D
+#endif
STATIC EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
ACPI_HEADER (
@@ -32,7 +39,7 @@ STATIC EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION
),
// UINT8 InterfaceType;
- EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_BCM2835_UART,
+ RPI_UART_INTERFACE_TYPE,
// UINT8 Reserved1[3];
{
EFI_ACPI_RESERVED_BYTE,
diff --git a/Platform/RaspberryPi/RPi4/AcpiTables/Uart.asl b/Platform/RaspberryPi/RPi4/AcpiTables/Uart.asl
index 15149892f3b0..5b59f2dd3e16 100644
--- a/Platform/RaspberryPi/RPi4/AcpiTables/Uart.asl
+++ b/Platform/RaspberryPi/RPi4/AcpiTables/Uart.asl
@@ -108,7 +108,7 @@ Device(BTH0)
{
Name (RBUF, ResourceTemplate ()
{
- // BT UART: UART0 (PL011)
+ // BT UART: URT0 (PL011) or URTM (miniUART)
UARTSerialBus(
115200, // InitialBaudRate: in BPS
, // BitsPerByte: default to 8 bits
@@ -133,7 +133,11 @@ Device(BTH0)
// no flow control.
16, // ReceiveBufferSize
16, // TransmitBufferSize
+#ifdef PL011_ENABLE
+ "\\_SB.URTM", // ResourceSource:
+#else
"\\_SB.URT0", // ResourceSource:
+#endif
// UART bus controller name
, // ResourceSourceIndex: assumed to be 0
, // ResourceUsage: assumed to be
diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
index 1624ebda27d7..ccf5bd5b9ef3 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -38,6 +38,7 @@ [Defines]
DEFINE SECURE_BOOT_ENABLE = FALSE
DEFINE INCLUDE_TFTP_COMMAND = FALSE
DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x8000004F
+ DEFINE PL011_ENABLE = FALSE
################################################################################
#
@@ -116,10 +117,16 @@ [LibraryClasses.common]
ArmHvcLib|ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf
ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.inf
+!if $(PL011_ENABLE) == TRUE
+ PL011UartClockLib|ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.inf
+ PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
+ SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
+!else
PciCf8Lib|MdePkg/Library/BasePciCf8Lib/BasePciCf8Lib.inf
PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatformHookLibNull.inf
SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
+!endif
# Cryptographic libraries
IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
@@ -229,6 +236,12 @@ [BuildOptions]
GCC:*_*_AARCH64_DLINK_FLAGS = -Wl,--fix-cortex-a53-843419
GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG -DNDEBUG
+!if $(PL011_ENABLE) == TRUE
+ GCC:*_*_*_CC_FLAGS = -DPL011_ENABLE
+ GCC:*_*_*_ASLPP_FLAGS = -DPL011_ENABLE
+ GCC:*_*_*_ASLCC_FLAGS = -DPL011_ENABLE
+!endif
+
[BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
GCC:*_*_AARCH64_DLINK_FLAGS = -z common-page-size=0x10000
@@ -391,6 +404,13 @@ [PcdsFixedAtBuild.common]
gBcm27xxTokenSpaceGuid.PcdBcm27xxPciBusMmioLen|0x3ffffff
gBcm27xxTokenSpaceGuid.PcdBcm27xxPciCpuMmioAdr|0x600000000
+!if $(PL011_ENABLE) == TRUE
+ ## PL011 UART
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0xfe201000
+ gArmPlatformTokenSpaceGuid.PL011UartInteger|0
+ gArmPlatformTokenSpaceGuid.PL011UartFractional|0
+ gArmPlatformTokenSpaceGuid.PL011UartClkInHz|48000000
+!else
## NS16550 compatible UART
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0xfe215040
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|TRUE
@@ -398,7 +418,10 @@ [PcdsFixedAtBuild.common]
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|500000000
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialFifoControl|0x27
gEfiMdeModulePkgTokenSpaceGuid.PcdSerialExtendedTxFifoSize|8
+!endif
+
gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
+ gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth|0
#
# ARM General Interrupt Controller
diff --git a/Platform/RaspberryPi/RPi4/RPi4.fdf b/Platform/RaspberryPi/RPi4/RPi4.fdf
index 7a506bd2813b..50fe554ec9ec 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.fdf
+++ b/Platform/RaspberryPi/RPi4/RPi4.fdf
@@ -51,7 +51,11 @@ [FD.RPI_EFI]
# ATF primary boot image
#
0x00000000|0x00020000
+!if $(PL011_ENABLE) == TRUE
+FILE = Platform/RaspberryPi/$(PLATFORM_NAME)/TrustedFirmware/bl31_pl011.bin
+!else
FILE = Platform/RaspberryPi/$(PLATFORM_NAME)/TrustedFirmware/bl31_miniuart.bin
+!endif
#
# DTB.
diff --git a/Platform/RaspberryPi/RPi4/Readme.md b/Platform/RaspberryPi/RPi4/Readme.md
index 2e37646e043d..2ef38d1b2062 100644
--- a/Platform/RaspberryPi/RPi4/Readme.md
+++ b/Platform/RaspberryPi/RPi4/Readme.md
@@ -18,13 +18,21 @@ following __major__ limitations:
- USB is likely to work only in pre-OS phase at this stage (nonstandard ECAM,
missing ACPI tables).
-- Serial I/O from the OS may not work at all due to CPU throttling affecting
- the miniUART baudrate.
+- Serial I/O from the OS may not work due to CPU throttling affecting the
+ miniUART baudrate. This can be worked around by using the `PL011_ENABLE`
+ compilation option.
# Building
Build instructions from the top level edk2-platforms Readme.md apply.
+The following additional build options are also available:
+- `-D PL011_ENABLE=1`: Selects PL011 for the serial console instead of the
+ miniUART (default). This doesn't change the GPIO pinout for the UART but
+ can be useful if you find that the miniUART baud rate changes when the
+ OS throttles the CPU. Note that this requires one of `disable-bt.dtbo` or
+ `miniuart-bt.dtbo` overlays to have been applied (see below).
+
# Booting the firmware
1. Format a uSD card as FAT16 or FAT32
@@ -33,14 +41,31 @@ Build instructions from the top level edk2-platforms Readme.md apply.
- `bcm2711-rpi-4-b.dtb`
- `fixup4.dat`
- `start4.elf`
+ - `overlays/miniuart-bt.dbto` or `overlays/disable-bt.dtbo` (Optional)
4. Create a `config.txt` with the following content:
- ```
- arm_64bit=1
- enable_uart=1
- core_freq=250
- enable_gic=1
- armstub=RPI_EFI.fd
- ```
+ - For a firmware **without** the `PL011_ENABLE` build option:
+ ```
+ arm_64bit=1
+ enable_uart=1
+ core_freq=250
+ enable_gic=1
+ armstub=RPI_EFI.fd
+ disable_commandline_tags=1
+ ```
+ - For a firmware **with** the `PL011_ENABLE` build option:
+ ```
+ arm_64bit=1
+ enable_gic=1
+ armstub=RPI_EFI.fd
+ disable_commandline_tags=1
+ device_tree_address=0x20000
+ device_tree_end=0x30000
+ device_tree=bcm2711-rpi-4-b.dtb
+ dtoverlay=miniuart-bt
+ ```
+ The abobe also requires `miniuart-bt.dbto` to have been copied into an `overlays/`
+ directory on the uSD card. Alternatively, you may use `disable-bt` instead of
+ `miniuart-bt` if you don't require BlueTooth.
5. Insert the uSD card and power up the Pi.
# Notes
--
2.21.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#52346): https://edk2.groups.io/g/devel/message/52346
Mute This Topic: https://groups.io/mt/68791814/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On 12/18/19 12:41 PM, Pete Batard wrote:
> The PL011 can be a better choice for the serial console on the RPi4,
> given that its baud clock is not derived from the CPU clock (which
> may change under our feet unless we keep it fixed at a low rate), and
> given the fact that the SBSA/SBBR specs that describe ARM specific
> architectural requirements for ACPI only permit PL011 based UARTs to
> begin with.
>
> Therefore we add a new PL011_ENABLE build switch to tell the firmware
> to use PL011 for all console serial I/O, including for TF-A, SPCR and
> DBG2, as well as toggle the BlueTooth module to use the mini UART.
>
> For the time being, the option is disabled by default because it
> requires an overlay to be enabled in config.txt that pinmuxes the
> PL011 TX/RX lines to the UART pins on the connector block.
>
> Signed-off-by: Pete Batard <pete@akeo.ie>
> ---
[...]
> index 65f48ceae688..e5c10c923626 100644
> --- a/Platform/RaspberryPi/RPi4/AcpiTables/Spcr.aslc
> +++ b/Platform/RaspberryPi/RPi4/AcpiTables/Spcr.aslc
> @@ -18,12 +18,19 @@
>
> #define RPI_UART_FLOW_CONTROL_NONE 0
>
> +#ifdef PL011_ENABLE
> +#define RPI_UART_INTERFACE_TYPE EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_ARM_PL011_UART
> +#define RPI_UART_BASE_ADDRESS FixedPcdGet64 (PcdSerialRegisterBase)
See comment below.
> +#define RPI_UART_INTERRUPT 0x99
> +#else
> +#define RPI_UART_INTERFACE_TYPE EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_BCM2835_UART
> //
> // When using the miniUART, PcdSerialRegisterBase points to the 8250 base address,
> // which is offset by 0x40 from the actual Bcm2835 base address
> //
> #define RPI_UART_BASE_ADDRESS (FixedPcdGet64 (PcdSerialRegisterBase) - 0x40)
> #define RPI_UART_INTERRUPT 0x7D
> +#endif
>
> STATIC EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
> ACPI_HEADER (
> @@ -32,7 +39,7 @@ STATIC EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION
> ),
> // UINT8 InterfaceType;
> - EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_BCM2835_UART,
> + RPI_UART_INTERFACE_TYPE,
> // UINT8 Reserved1[3];
> {
> EFI_ACPI_RESERVED_BYTE,
> diff --git a/Platform/RaspberryPi/RPi4/AcpiTables/Uart.asl b/Platform/RaspberryPi/RPi4/AcpiTables/Uart.asl
> index 15149892f3b0..5b59f2dd3e16 100644
> --- a/Platform/RaspberryPi/RPi4/AcpiTables/Uart.asl
> +++ b/Platform/RaspberryPi/RPi4/AcpiTables/Uart.asl
> @@ -108,7 +108,7 @@ Device(BTH0)
> {
> Name (RBUF, ResourceTemplate ()
> {
> - // BT UART: UART0 (PL011)
> + // BT UART: URT0 (PL011) or URTM (miniUART)
> UARTSerialBus(
> 115200, // InitialBaudRate: in BPS
> , // BitsPerByte: default to 8 bits
> @@ -133,7 +133,11 @@ Device(BTH0)
> // no flow control.
> 16, // ReceiveBufferSize
> 16, // TransmitBufferSize
> +#ifdef PL011_ENABLE
> + "\\_SB.URTM", // ResourceSource:
> +#else
> "\\_SB.URT0", // ResourceSource:
> +#endif
> // UART bus controller name
> , // ResourceSourceIndex: assumed to be 0
> , // ResourceUsage: assumed to be
> diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
> index 1624ebda27d7..ccf5bd5b9ef3 100644
> --- a/Platform/RaspberryPi/RPi4/RPi4.dsc
> +++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
> @@ -38,6 +38,7 @@ [Defines]
> DEFINE SECURE_BOOT_ENABLE = FALSE
> DEFINE INCLUDE_TFTP_COMMAND = FALSE
> DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x8000004F
> + DEFINE PL011_ENABLE = FALSE
>
> ################################################################################
> #
> @@ -116,10 +117,16 @@ [LibraryClasses.common]
> ArmHvcLib|ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf
> ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.inf
>
> +!if $(PL011_ENABLE) == TRUE
> + PL011UartClockLib|ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.inf
> + PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
> + SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
> +!else
> PciCf8Lib|MdePkg/Library/BasePciCf8Lib/BasePciCf8Lib.inf
> PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
> PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatformHookLibNull.inf
> SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
> +!endif
>
> # Cryptographic libraries
> IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> @@ -229,6 +236,12 @@ [BuildOptions]
> GCC:*_*_AARCH64_DLINK_FLAGS = -Wl,--fix-cortex-a53-843419
> GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG -DNDEBUG
>
> +!if $(PL011_ENABLE) == TRUE
> + GCC:*_*_*_CC_FLAGS = -DPL011_ENABLE
> + GCC:*_*_*_ASLPP_FLAGS = -DPL011_ENABLE
> + GCC:*_*_*_ASLCC_FLAGS = -DPL011_ENABLE
> +!endif
> +
> [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
> GCC:*_*_AARCH64_DLINK_FLAGS = -z common-page-size=0x10000
>
> @@ -391,6 +404,13 @@ [PcdsFixedAtBuild.common]
> gBcm27xxTokenSpaceGuid.PcdBcm27xxPciBusMmioLen|0x3ffffff
> gBcm27xxTokenSpaceGuid.PcdBcm27xxPciCpuMmioAdr|0x600000000
>
> +!if $(PL011_ENABLE) == TRUE
> + ## PL011 UART
> + gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0xfe201000
Can we use relative to gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress
instead?
And instead of RPI_UART_BASE_ADDRESS():
#define BCM283X_UART_OFFSET 0x00201000
#define BCM283X_UART_BASE_ADDRESS (FixedPcdGet64
(PcdBcm283xRegistersAddress) \
+ BCM283X_UART_OFFSET)
> + gArmPlatformTokenSpaceGuid.PL011UartInteger|0
> + gArmPlatformTokenSpaceGuid.PL011UartFractional|0
> + gArmPlatformTokenSpaceGuid.PL011UartClkInHz|48000000
> +!else
> ## NS16550 compatible UART
> gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0xfe215040
> gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|TRUE
> @@ -398,7 +418,10 @@ [PcdsFixedAtBuild.common]
> gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|500000000
> gEfiMdeModulePkgTokenSpaceGuid.PcdSerialFifoControl|0x27
> gEfiMdeModulePkgTokenSpaceGuid.PcdSerialExtendedTxFifoSize|8
> +!endif
> +
> gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth|0
>
[...]
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#52361): https://edk2.groups.io/g/devel/message/52361
Mute This Topic: https://groups.io/mt/68791814/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On 2019.12.18 16:05, Philippe Mathieu-Daudé wrote:
> On 12/18/19 12:41 PM, Pete Batard wrote:
>> The PL011 can be a better choice for the serial console on the RPi4,
>> given that its baud clock is not derived from the CPU clock (which
>> may change under our feet unless we keep it fixed at a low rate), and
>> given the fact that the SBSA/SBBR specs that describe ARM specific
>> architectural requirements for ACPI only permit PL011 based UARTs to
>> begin with.
>>
>> Therefore we add a new PL011_ENABLE build switch to tell the firmware
>> to use PL011 for all console serial I/O, including for TF-A, SPCR and
>> DBG2, as well as toggle the BlueTooth module to use the mini UART.
>>
>> For the time being, the option is disabled by default because it
>> requires an overlay to be enabled in config.txt that pinmuxes the
>> PL011 TX/RX lines to the UART pins on the connector block.
>>
>> Signed-off-by: Pete Batard <pete@akeo.ie>
>> ---
> [...]
>> index 65f48ceae688..e5c10c923626 100644
>> --- a/Platform/RaspberryPi/RPi4/AcpiTables/Spcr.aslc
>> +++ b/Platform/RaspberryPi/RPi4/AcpiTables/Spcr.aslc
>> @@ -18,12 +18,19 @@
>> #define RPI_UART_FLOW_CONTROL_NONE 0
>> +#ifdef PL011_ENABLE
>> +#define RPI_UART_INTERFACE_TYPE
>> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_ARM_PL011_UART
>>
>> +#define RPI_UART_BASE_ADDRESS FixedPcdGet64
>> (PcdSerialRegisterBase)
>
> See comment below.
>
>> +#define RPI_UART_INTERRUPT 0x99
>> +#else
>> +#define RPI_UART_INTERFACE_TYPE
>> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_BCM2835_UART
>>
>> //
>> // When using the miniUART, PcdSerialRegisterBase points to the 8250
>> base address,
>> // which is offset by 0x40 from the actual Bcm2835 base address
>> //
>> #define RPI_UART_BASE_ADDRESS (FixedPcdGet64
>> (PcdSerialRegisterBase) - 0x40)
>> #define RPI_UART_INTERRUPT 0x7D
>> +#endif
>> STATIC EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
>> ACPI_HEADER (
>> @@ -32,7 +39,7 @@ STATIC
>> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
>> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION
>> ),
>> // UINT8 InterfaceType;
>> -
>> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_BCM2835_UART,
>>
>> + RPI_UART_INTERFACE_TYPE,
>> // UINT8 Reserved1[3];
>> {
>> EFI_ACPI_RESERVED_BYTE,
>> diff --git a/Platform/RaspberryPi/RPi4/AcpiTables/Uart.asl
>> b/Platform/RaspberryPi/RPi4/AcpiTables/Uart.asl
>> index 15149892f3b0..5b59f2dd3e16 100644
>> --- a/Platform/RaspberryPi/RPi4/AcpiTables/Uart.asl
>> +++ b/Platform/RaspberryPi/RPi4/AcpiTables/Uart.asl
>> @@ -108,7 +108,7 @@ Device(BTH0)
>> {
>> Name (RBUF, ResourceTemplate ()
>> {
>> - // BT UART: UART0 (PL011)
>> + // BT UART: URT0 (PL011) or URTM (miniUART)
>> UARTSerialBus(
>> 115200, // InitialBaudRate: in BPS
>> , // BitsPerByte: default to 8 bits
>> @@ -133,7 +133,11 @@ Device(BTH0)
>> // no flow control.
>> 16, // ReceiveBufferSize
>> 16, // TransmitBufferSize
>> +#ifdef PL011_ENABLE
>> + "\\_SB.URTM", // ResourceSource:
>> +#else
>> "\\_SB.URT0", // ResourceSource:
>> +#endif
>> // UART bus controller name
>> , // ResourceSourceIndex: assumed to be 0
>> , // ResourceUsage: assumed to be
>> diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc
>> b/Platform/RaspberryPi/RPi4/RPi4.dsc
>> index 1624ebda27d7..ccf5bd5b9ef3 100644
>> --- a/Platform/RaspberryPi/RPi4/RPi4.dsc
>> +++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
>> @@ -38,6 +38,7 @@ [Defines]
>> DEFINE SECURE_BOOT_ENABLE = FALSE
>> DEFINE INCLUDE_TFTP_COMMAND = FALSE
>> DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x8000004F
>> + DEFINE PL011_ENABLE = FALSE
>>
>> ################################################################################
>>
>> #
>> @@ -116,10 +117,16 @@ [LibraryClasses.common]
>> ArmHvcLib|ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf
>>
>> ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.inf
>>
>> +!if $(PL011_ENABLE) == TRUE
>> +
>> PL011UartClockLib|ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.inf
>>
>> + PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
>> +
>> SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
>>
>> +!else
>> PciCf8Lib|MdePkg/Library/BasePciCf8Lib/BasePciCf8Lib.inf
>> PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
>>
>> PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatformHookLibNull.inf
>>
>>
>> SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
>>
>> +!endif
>> # Cryptographic libraries
>> IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
>> @@ -229,6 +236,12 @@ [BuildOptions]
>> GCC:*_*_AARCH64_DLINK_FLAGS = -Wl,--fix-cortex-a53-843419
>> GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG -DNDEBUG
>> +!if $(PL011_ENABLE) == TRUE
>> + GCC:*_*_*_CC_FLAGS = -DPL011_ENABLE
>> + GCC:*_*_*_ASLPP_FLAGS = -DPL011_ENABLE
>> + GCC:*_*_*_ASLCC_FLAGS = -DPL011_ENABLE
>> +!endif
>> +
>> [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
>> GCC:*_*_AARCH64_DLINK_FLAGS = -z common-page-size=0x10000
>> @@ -391,6 +404,13 @@ [PcdsFixedAtBuild.common]
>> gBcm27xxTokenSpaceGuid.PcdBcm27xxPciBusMmioLen|0x3ffffff
>> gBcm27xxTokenSpaceGuid.PcdBcm27xxPciCpuMmioAdr|0x600000000
>> +!if $(PL011_ENABLE) == TRUE
>> + ## PL011 UART
>> + gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0xfe201000
>
> Can we use relative to gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress
> instead?
>
> And instead of RPI_UART_BASE_ADDRESS():
>
> #define BCM283X_UART_OFFSET 0x00201000
> #define BCM283X_UART_BASE_ADDRESS (FixedPcdGet64
> (PcdBcm283xRegistersAddress) \
>
> + BCM283X_UART_OFFSET)
That's not a bad suggestion but if we do that then I'd rather introduce
2 new BCM2836_MINI_UART_OFFSET and BCM2836_PL011_UART_OFFSET constants
in Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h and then
set BCM283X_UART_BASE_ADDRESS to PcdBcm283xRegistersAddress + one or the
other.
The whole point was to avoid adding offsets, which may or may not remain
constant over the evolution of the Bcm283x family and derivatives, into
individual sources, because then we way have to hunt these constants in
places we may not even remember they exist.
On the other hand, I wouldn't mind avoiding that confusing 0x40 offset
we need to substract when using the 8250 UART. So I think I'll go with
the BCM283X_UART_BASE_ADDRESS + offset route, after adding an extra
patch to the series that adds the UART offset constants to
Silicon/.../Bcm2836.h.
Regards,
/Pete
>
>
>> + gArmPlatformTokenSpaceGuid.PL011UartInteger|0
>> + gArmPlatformTokenSpaceGuid.PL011UartFractional|0
>> + gArmPlatformTokenSpaceGuid.PL011UartClkInHz|48000000
>> +!else
>> ## NS16550 compatible UART
>> gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0xfe215040
>> gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|TRUE
>> @@ -398,7 +418,10 @@ [PcdsFixedAtBuild.common]
>> gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|500000000
>> gEfiMdeModulePkgTokenSpaceGuid.PcdSerialFifoControl|0x27
>> gEfiMdeModulePkgTokenSpaceGuid.PcdSerialExtendedTxFifoSize|8
>> +!endif
>> +
>> gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
>> + gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth|0
> [...]
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#52366): https://edk2.groups.io/g/devel/message/52366
Mute This Topic: https://groups.io/mt/68791814/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On 12/18/19 5:59 PM, Pete Batard wrote:
> On 2019.12.18 16:05, Philippe Mathieu-Daudé wrote:
>> On 12/18/19 12:41 PM, Pete Batard wrote:
>>> The PL011 can be a better choice for the serial console on the RPi4,
>>> given that its baud clock is not derived from the CPU clock (which
>>> may change under our feet unless we keep it fixed at a low rate), and
>>> given the fact that the SBSA/SBBR specs that describe ARM specific
>>> architectural requirements for ACPI only permit PL011 based UARTs to
>>> begin with.
>>>
>>> Therefore we add a new PL011_ENABLE build switch to tell the firmware
>>> to use PL011 for all console serial I/O, including for TF-A, SPCR and
>>> DBG2, as well as toggle the BlueTooth module to use the mini UART.
>>>
>>> For the time being, the option is disabled by default because it
>>> requires an overlay to be enabled in config.txt that pinmuxes the
>>> PL011 TX/RX lines to the UART pins on the connector block.
>>>
>>> Signed-off-by: Pete Batard <pete@akeo.ie>
>>> ---
>> [...]
>>> index 65f48ceae688..e5c10c923626 100644
>>> --- a/Platform/RaspberryPi/RPi4/AcpiTables/Spcr.aslc
>>> +++ b/Platform/RaspberryPi/RPi4/AcpiTables/Spcr.aslc
>>> @@ -18,12 +18,19 @@
>>> #define RPI_UART_FLOW_CONTROL_NONE 0
>>> +#ifdef PL011_ENABLE
>>> +#define RPI_UART_INTERFACE_TYPE
>>> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_ARM_PL011_UART
>>>
>>> +#define RPI_UART_BASE_ADDRESS FixedPcdGet64
>>> (PcdSerialRegisterBase)
>>
>> See comment below.
>>
>>> +#define RPI_UART_INTERRUPT 0x99
>>> +#else
>>> +#define RPI_UART_INTERFACE_TYPE
>>> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_BCM2835_UART
>>>
>>> //
>>> // When using the miniUART, PcdSerialRegisterBase points to the
>>> 8250 base address,
>>> // which is offset by 0x40 from the actual Bcm2835 base address
>>> //
>>> #define RPI_UART_BASE_ADDRESS (FixedPcdGet64
>>> (PcdSerialRegisterBase) - 0x40)
>>> #define RPI_UART_INTERRUPT 0x7D
>>> +#endif
>>> STATIC EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
>>> ACPI_HEADER (
>>> @@ -32,7 +39,7 @@ STATIC
>>> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE Spcr = {
>>> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_REVISION
>>> ),
>>> // UINT8 InterfaceType;
>>> -
>>> EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_BCM2835_UART,
>>>
>>> + RPI_UART_INTERFACE_TYPE,
>>> // UINT8 Reserved1[3];
>>> {
>>> EFI_ACPI_RESERVED_BYTE,
>>> diff --git a/Platform/RaspberryPi/RPi4/AcpiTables/Uart.asl
>>> b/Platform/RaspberryPi/RPi4/AcpiTables/Uart.asl
>>> index 15149892f3b0..5b59f2dd3e16 100644
>>> --- a/Platform/RaspberryPi/RPi4/AcpiTables/Uart.asl
>>> +++ b/Platform/RaspberryPi/RPi4/AcpiTables/Uart.asl
>>> @@ -108,7 +108,7 @@ Device(BTH0)
>>> {
>>> Name (RBUF, ResourceTemplate ()
>>> {
>>> - // BT UART: UART0 (PL011)
>>> + // BT UART: URT0 (PL011) or URTM (miniUART)
>>> UARTSerialBus(
>>> 115200, // InitialBaudRate: in BPS
>>> , // BitsPerByte: default to 8 bits
>>> @@ -133,7 +133,11 @@ Device(BTH0)
>>> // no flow control.
>>> 16, // ReceiveBufferSize
>>> 16, // TransmitBufferSize
>>> +#ifdef PL011_ENABLE
>>> + "\\_SB.URTM", // ResourceSource:
>>> +#else
>>> "\\_SB.URT0", // ResourceSource:
>>> +#endif
>>> // UART bus controller name
>>> , // ResourceSourceIndex: assumed to be 0
>>> , // ResourceUsage: assumed to be
>>> diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc
>>> b/Platform/RaspberryPi/RPi4/RPi4.dsc
>>> index 1624ebda27d7..ccf5bd5b9ef3 100644
>>> --- a/Platform/RaspberryPi/RPi4/RPi4.dsc
>>> +++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
>>> @@ -38,6 +38,7 @@ [Defines]
>>> DEFINE SECURE_BOOT_ENABLE = FALSE
>>> DEFINE INCLUDE_TFTP_COMMAND = FALSE
>>> DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x8000004F
>>> + DEFINE PL011_ENABLE = FALSE
>>> ################################################################################
>>>
>>> #
>>> @@ -116,10 +117,16 @@ [LibraryClasses.common]
>>> ArmHvcLib|ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf
>>> ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.inf
>>>
>>> +!if $(PL011_ENABLE) == TRUE
>>> +
>>> PL011UartClockLib|ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.inf
>>>
>>> + PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
>>> +
>>> SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
>>>
>>> +!else
>>> PciCf8Lib|MdePkg/Library/BasePciCf8Lib/BasePciCf8Lib.inf
>>> PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
>>> PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatformHookLibNull.inf
>>>
>>> SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
>>>
>>> +!endif
>>> # Cryptographic libraries
>>> IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
>>> @@ -229,6 +236,12 @@ [BuildOptions]
>>> GCC:*_*_AARCH64_DLINK_FLAGS = -Wl,--fix-cortex-a53-843419
>>> GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG -DNDEBUG
>>> +!if $(PL011_ENABLE) == TRUE
>>> + GCC:*_*_*_CC_FLAGS = -DPL011_ENABLE
>>> + GCC:*_*_*_ASLPP_FLAGS = -DPL011_ENABLE
>>> + GCC:*_*_*_ASLCC_FLAGS = -DPL011_ENABLE
>>> +!endif
>>> +
>>> [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
>>> GCC:*_*_AARCH64_DLINK_FLAGS = -z common-page-size=0x10000
>>> @@ -391,6 +404,13 @@ [PcdsFixedAtBuild.common]
>>> gBcm27xxTokenSpaceGuid.PcdBcm27xxPciBusMmioLen|0x3ffffff
>>> gBcm27xxTokenSpaceGuid.PcdBcm27xxPciCpuMmioAdr|0x600000000
>>> +!if $(PL011_ENABLE) == TRUE
>>> + ## PL011 UART
>>> + gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0xfe201000
>>
>> Can we use relative to
>> gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress instead?
>>
>> And instead of RPI_UART_BASE_ADDRESS():
>>
>> #define BCM283X_UART_OFFSET 0x00201000
>> #define BCM283X_UART_BASE_ADDRESS (FixedPcdGet64
>> (PcdBcm283xRegistersAddress) \
>>
>> + BCM283X_UART_OFFSET)
>
> That's not a bad suggestion but if we do that then I'd rather introduce
> 2 new BCM2836_MINI_UART_OFFSET and BCM2836_PL011_UART_OFFSET constants
> in Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h and then
> set BCM283X_UART_BASE_ADDRESS to PcdBcm283xRegistersAddress + one or the
> other.
>
> The whole point was to avoid adding offsets, which may or may not remain
> constant over the evolution of the Bcm283x family and derivatives, into
> individual sources, because then we way have to hunt these constants in
> places we may not even remember they exist.
If the family evolves, we can handle the new case or copy/paste again.
I insist with this modularity because then with little work we can
support the RPi1, using:
gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress|0x20000000
> On the other hand, I wouldn't mind avoiding that confusing 0x40 offset
> we need to substract when using the 8250 UART. So I think I'll go with
> the BCM283X_UART_BASE_ADDRESS + offset route, after adding an extra
> patch to the series that adds the UART offset constants to
> Silicon/.../Bcm2836.h.
If you don't mind doing the work, I believe this is the best way.
Thanks!
Phil.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#52368): https://edk2.groups.io/g/devel/message/52368
Mute This Topic: https://groups.io/mt/68791814/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2026 Red Hat, Inc.