Hi,
On 1/11/24 01:46, Ard Biesheuvel wrote:
Hi Jeremy,
Thanks for the patches.
Thanks for looking at them!
On Thu, 11 Jan 2024 at 00:52, Jeremy Linton <jeremy.lin...@arm.com> wrote:
The rpi's config.txt controls which uart (pl011, or miniuart) is
selected as the console. TFA and edk2 follow its lead, but if the
miniuart is selected as the primary and the machine is booted in ACPI
mode the baud/etc is never configured for the pl011. The linux kernel
won't reconfigure it either as its listed as a "SBSA" uart, so it
simply won't work.
This re-enables BT on the pl011 in ACPI mode, and it somewhat starts
to work again.
Signed-off-by: Jeremy Linton <jeremy.lin...@arm.com>
---
.../DualSerialPortLib/DualSerialPortLib.c | 37 +++++++++++--------
1 file changed, 22 insertions(+), 15 deletions(-)
diff --git a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
index d2f983bf0a..79545d93d6 100644
--- a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
+++ b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
@@ -76,6 +76,8 @@ SerialPortInitialize (
EFI_PARITY_TYPE Parity;
UINT8 DataBits;
EFI_STOP_BITS_TYPE StopBits;
+ RETURN_STATUS Ret;
+ UINTN Timeout;
What is this for?
IIRC, its as you see, being used below to avoid an infinite loop when
the miniuart doesn't become ready. That shouldn't be possible with this
set, so its a safety change.
But IIRC, the infinite loop can be triggered by trying to enable the
miniuart early, when its not the console, before the config dxe is run
to turn the clock/regulator on. Which is what I was originally trying to
do in this patch by configuring both uarts unconditionally.
//
// First thing we need to do is determine which of PL011 or miniUART is
selected
@@ -85,23 +87,27 @@ SerialPortInitialize (
UsePl011UartSet = TRUE;
}
- if (UsePl011Uart) {
- BaudRate = FixedPcdGet64 (PcdUartDefaultBaudRate);
+ // always init the pl011 on the pi4, linux expects a SBSA uart to be at
115200
+ // this means we need to set the baud/etc even if we arn't using it as a
console
^ I should probably fix the spelling issues there too.
+ if ((UsePl011Uart) || (RPI_MODEL == 4)) {
ReceiveFifoDepth = 0; // Use default FIFO depth
+ BaudRate = FixedPcdGet64 (PcdUartDefaultBaudRate);
Shouldn't we hardcode 115200 here if !UsePl011Uart?
Probably? For spec compliance for sure.
Although the BT as I mentioned is flaky, it will scan and connect to
things like keyboards/mice and sorta work, although it may drop the
connections/etc. A2dp devices are a no-go. But, running the port faster
than 115200 seems to at least extend how long it works. So, presumably
part of the problem is that the BT wants to bump the baud rate and reset
the device, neither of which it can do in ACPI mode at the moment. And
for clarity it does some of this in DT mode too, so its not entirely
just baud rate and reset problems. I spent a fair bit of time a year+
back trying to get the BT to work reliably in just DT mode with mainline
on the miniuart as well as on the pl011 where it sorta works and moved
on to other things before solving the problem.
So, I guess I can hard-code it here, at least then we are spec compliant.
Parity = (EFI_PARITY_TYPE)FixedPcdGet8 (PcdUartDefaultParity);
DataBits = FixedPcdGet8 (PcdUartDefaultDataBits);
StopBits = (EFI_STOP_BITS_TYPE) FixedPcdGet8 (PcdUartDefaultStopBits);
- return PL011UartInitializePort (
- PL011_UART_REGISTER_BASE,
- PL011UartClockGetFreq(),
- &BaudRate,
- &ReceiveFifoDepth,
- &Parity,
- &DataBits,
- &StopBits
- );
- } else {
+ Ret = PL011UartInitializePort (
+ PL011_UART_REGISTER_BASE,
+ PL011UartClockGetFreq(),
+ &BaudRate,
+ &ReceiveFifoDepth,
+ &Parity,
+ &DataBits,
+ &StopBits
+ );
+ }
+
+ if (!UsePl011Uart) {
SerialRegisterBase = MINI_UART_REGISTER_BASE;
Divisor = SerialPortGetDivisor (PcdGet32 (PcdSerialBaudRate));
@@ -127,7 +133,8 @@ SerialPortInitialize (
// Wait for the serial port to be ready.
// Verify that both the transmit FIFO and the shift register are empty.
//
- while ((SerialPortReadRegister (SerialRegisterBase, R_UART_LSR) &
(B_UART_LSR_TEMT | B_UART_LSR_TXRDY)) != (B_UART_LSR_TEMT | B_UART_LSR_TXRDY));
+ Timeout = 1000;
+ while (((SerialPortReadRegister (SerialRegisterBase, R_UART_LSR) &
(B_UART_LSR_TEMT | B_UART_LSR_TXRDY)) != (B_UART_LSR_TEMT | B_UART_LSR_TXRDY)) &&
(Timeout--));
//
// Configure baud rate
@@ -158,9 +165,9 @@ SerialPortInitialize (
// Put Modem Control Register(MCR) into its reset state of 0x00.
//
SerialPortWriteRegister (SerialRegisterBase, R_UART_MCR, 0x00);
-
- return RETURN_SUCCESS;
+ Ret = RETURN_SUCCESS;
}
+ return Ret;
}
/**
--
2.43.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113621): https://edk2.groups.io/g/devel/message/113621
Mute This Topic: https://groups.io/mt/103652853/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
