[edk2-devel] [PATCH 0/5] Platform/RaspberryPi: Various minor fixes

[edk2-devel] [PATCH 0/5] Platform/RaspberryPi: Various minor fixes

[Jeremy Linton]

This includes a change to always initialize the PL011 to the
configured baud (which should be 115200 for the SBSA UART), which
fixes linux's assumption that SBSA UARTs are pre-programmed for
115200. This in turn (re)enables the PL011 when the console is on the
miniuart per the config.txt file.

Also included is another spin with the DT/XHCI reset patch which puts
removal of the DT node that causes linux to reset the XHCI controller,
as well as an additional patch that updates the DT to match the PCIe
MMIO window we have programmed. This cures much of the problem with
the PCIe/XHCI configuration when booted in DT mode on linux.

There is also a few menu visibility/section tweaks to assure ACPI/DT
specific settings show up at the appropriate time.

As well as a minor fix to work around a bogus compiler warning.

Jeremy Linton (5):
  Platform/RaspberryPi/DualSerialPortLib: Always configure the pl011
  Silicon/Broadcom/BcmGenetDxe: Suppress some bogus compiler warnings
  Platform/RaspberryPi: Cleanup menu visibility
  Platform/RaspberryPi: Give the user control over the XHCI mailbox
  Platform/RaspberryPi: Update PCIe MMIO window for DT

 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 10 +++++
 .../Drivers/ConfigDxe/ConfigDxe.inf           |  1 +
 .../Drivers/ConfigDxe/ConfigDxeHii.uni        |  5 +++
 .../Drivers/ConfigDxe/ConfigDxeHii.vfr        | 21 +++++++++--
 Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c  | 28 ++++++++++++++
 .../RaspberryPi/Drivers/FdtDxe/FdtDxe.inf     |  1 +
 .../DualSerialPortLib/DualSerialPortLib.c     | 37 +++++++++++--------
 Platform/RaspberryPi/RPi3/RPi3.dsc            |  6 +++
 Platform/RaspberryPi/RPi4/RPi4.dsc            |  7 ++++
 Platform/RaspberryPi/RaspberryPi.dec          |  1 +
 .../Drivers/Net/BcmGenetDxe/GenericPhy.c      |  2 +
 .../Drivers/Net/BcmGenetDxe/SimpleNetwork.c   |  3 ++
 12 files changed, 104 insertions(+), 18 deletions(-)

--
2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113543): https://edk2.groups.io/g/devel/message/113543
Mute This Topic: https://groups.io/mt/103652852/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 1/5] Platform/RaspberryPi/DualSerialPortLib: Always configure the pl011

[Jeremy Linton]

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.linton@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;

   //
   // 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
+  if ((UsePl011Uart) || (RPI_MODEL == 4)) {
     ReceiveFifoDepth = 0;         // Use default FIFO depth
+    BaudRate = FixedPcdGet64 (PcdUartDefaultBaudRate);
     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 (#113544): https://edk2.groups.io/g/devel/message/113544
Mute This Topic: https://groups.io/mt/103652853/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 2/5] Silicon/Broadcom/BcmGenetDxe: Suppress some bogus compiler warnings

[Jeremy Linton]

Some recent GCC revisions will throw warnings about values being used
before being initialized. But in the case where the lack of initialization
is the result of the called function returning error status the EFI_ERROR()
macro/error seems to confuse the compiler about the fact that the value
is then never used.

So, while the code appears to be fine, lets just zero the variables
anyway to make the compiler happy.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenericPhy.c    | 2 ++
 Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenericPhy.c b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenericPhy.c
index 9e5d30fafd..2d5f70170e 100644
--- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenericPhy.c
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/GenericPhy.c
@@ -381,6 +381,8 @@ GenericPhyUpdateConfig (
   BOOLEAN             LinkUp;

   Status = GenericPhyGetLinkStatus (Phy);
+  Speed = 0;
+  Duplex = 0;
   LinkUp = EFI_ERROR (Status) ? FALSE : TRUE;

   if (Phy->LinkUp != LinkUp) {
diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
index 3b51a86d65..7a7c398b1f 100644
--- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
@@ -731,6 +731,9 @@ GenetSimpleNetworkReceive (
   UINT8               *Frame;
   UINTN               FrameLength;

+  DescIndex = 0;
+  FrameLength = 0;
+
   if (This == NULL || Buffer == NULL) {
     DEBUG ((DEBUG_ERROR, "%a: Invalid parameter (missing handle or buffer)\n",
       __FUNCTION__));
--
2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113545): https://edk2.groups.io/g/devel/message/113545
Mute This Topic: https://groups.io/mt/103652854/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 3/5] Platform/RaspberryPi: Cleanup menu visibility

[Jeremy Linton]

Lets allow some of these options to change when the
system is in ACPI+DT mode. Plus the fan temp should
be disabled when ACPI isn't enabled.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
index e8bf16312d..f668b7a553 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
@@ -196,7 +196,7 @@ formset
         endoneof;

 #if (RPI_MODEL == 4)
-        grayoutif NOT ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_ACPI;
+        grayoutif ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_DT;
           oneof varid = FanOnGpio.Enabled,
               prompt      = STRING_TOKEN(STR_ADVANCED_FANONGPIO_PROMPT),
               help        = STRING_TOKEN(STR_ADVANCED_FANONGPIO_HELP),
@@ -207,7 +207,7 @@ formset
           endoneof;
         endif;

-        grayoutif ideqval FanOnGpio.Enabled == 0;
+        grayoutif ideqval FanOnGpio.Enabled == 0 OR ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_DT;
           numeric varid = FanTemp.Value,
               prompt      = STRING_TOKEN(STR_ADVANCED_FANTEMP_PROMPT),
               help        = STRING_TOKEN(STR_ADVANCED_FANTEMP_HELP),
@@ -219,7 +219,7 @@ formset
         endif;

         suppressif ideqval XhciPci.Value == 2;
-          grayoutif NOT ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_ACPI;
+          grayoutif ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_DT;
             oneof varid = XhciPci.Value,
               prompt      = STRING_TOKEN(STR_ADVANCED_XHCIPCI_PROMPT),
               help        = STRING_TOKEN(STR_ADVANCED_XHCIPCI_HELP),
--
2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113546): https://edk2.groups.io/g/devel/message/113546
Mute This Topic: https://groups.io/mt/103652855/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 4/5] Platform/RaspberryPi: Give the user control over the XHCI mailbox

[Jeremy Linton]

Its a complete tossup whether removing the mailbox call after we have
set up the XHCI works for a given kernel+distro in DT mode. So lets
give users which want to try DT the option of flipping this on/off.

Users that don't want to have to deal with DT, can use ACPI.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c     | 10 ++++++++++
 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf   |  1 +
 .../Drivers/ConfigDxe/ConfigDxeHii.uni            |  5 +++++
 .../Drivers/ConfigDxe/ConfigDxeHii.vfr            | 15 +++++++++++++++
 Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c      |  4 ++++
 Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf    |  1 +
 Platform/RaspberryPi/RPi3/RPi3.dsc                |  6 ++++++
 Platform/RaspberryPi/RPi4/RPi4.dsc                |  7 +++++++
 Platform/RaspberryPi/RaspberryPi.dec              |  1 +
 9 files changed, 50 insertions(+)

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
index 3dcf2bac0d..2484787982 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -298,6 +298,16 @@ SetupVariables (
         Status = PcdSet32S (PcdXhciPci, 1);
         ASSERT_EFI_ERROR (Status);
       }
+
+      Size = sizeof (UINT32);
+      Status = gRT->GetVariable (L"XhciReload",
+                                 &gConfigDxeFormSetGuid,
+                                 NULL, &Size, &Var32);
+      if (EFI_ERROR (Status)) {
+        Status = PcdSet32S (PcdXhciReload, PcdGet32 (PcdXhciReload));
+        ASSERT_EFI_ERROR (Status);
+      }
+
     }
   } else {
     /*
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
index 6f6e8f42ac..475e645537 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
@@ -96,6 +96,7 @@
   gRaspberryPiTokenSpaceGuid.PcdUartInUse
   gRaspberryPiTokenSpaceGuid.PcdXhciPci
   gRaspberryPiTokenSpaceGuid.PcdMiniUartClockRate
+  gRaspberryPiTokenSpaceGuid.PcdXhciReload

 [Depex]
   gPcdProtocolGuid AND gRaspberryPiFirmwareProtocolGuid
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
index 5ec17072c3..8130638876 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
@@ -62,6 +62,11 @@
 #string STR_ADVANCED_XHCIPCI_XHCI     #language en-US "XHCI"
 #string STR_ADVANCED_XHCIPCI_PCIE     #language en-US "PCIe"

+#string STR_ADVANCED_XHCIRELOAD_PROMPT  #language en-US "DT Reload XHCI firmware"
+#string STR_ADVANCED_XHCIRELOAD_HELP    #language en-US "OS should reload XHCI firmware on reset"
+#string STR_ADVANCED_XHCIRELOAD_DISABLE #language en-US "Disabled"
+#string STR_ADVANCED_XHCIRELOAD_RELOAD  #language en-US "Reload"
+
 #string STR_ADVANCED_ASSET_TAG_PROMPT #language en-US "Asset Tag"
 #string STR_ADVANCED_ASSET_TAG_HELP   #language en-US "Set the system Asset Tag"

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
index f668b7a553..f13b70711d 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
@@ -61,6 +61,11 @@ formset
       name  = XhciPci,
       guid  = CONFIGDXE_FORM_SET_GUID;

+    efivarstore ADVANCED_XHCIPCI_VARSTORE_DATA,
+      attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
+      name  = XhciReload,
+      guid  = CONFIGDXE_FORM_SET_GUID;
+
     efivarstore SYSTEM_TABLE_MODE_VARSTORE_DATA,
       attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
       name  = SystemTableMode,
@@ -228,6 +233,16 @@ formset
               option text = STRING_TOKEN(STR_ADVANCED_XHCIPCI_PCIE), value = 1, flags = 0;
             endoneof;
           endif;
+
+          grayoutif ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_ACPI;
+            oneof varid = XhciReload.Value,
+              prompt      = STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_PROMPT),
+              help        = STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_HELP),
+              flags       = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
+              option text = STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_DISABLE), value = 0, flags = DEFAULT;
+              option text = STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_RELOAD), value = 1, flags = 0;
+            endoneof;
+          endif;
         endif;
 #endif
         string varid = AssetTag.AssetTag,
diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
index cbbc2ad30d..dd4fc0a05e 100644
--- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
+++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
@@ -375,6 +375,10 @@ SyncPcie (
     return EFI_NOT_FOUND;
   }

+  if (PcdGet32 (PcdXhciReload) != 1) {
+    return EFI_SUCCESS;
+  }
+
   /*
    * Now that we are always running without DMA translation, and with a 3G
    * limit, there shouldn't be a need to reset/reload the XHCI. The
diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
index 26f84e5940..d9fb6ee480 100644
--- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
+++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
@@ -47,3 +47,4 @@

 [Pcd]
   gRaspberryPiTokenSpaceGuid.PcdSystemTableMode
+  gRaspberryPiTokenSpaceGuid.PcdXhciReload
diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc
index a967cd5a20..f5361ab95e 100644
--- a/Platform/RaspberryPi/RPi3/RPi3.dsc
+++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
@@ -529,6 +529,12 @@
   #
   gRaspberryPiTokenSpaceGuid.PcdXhciPci|L"XhciPci"|gConfigDxeFormSetGuid|0x0|0

+  # DT contains XHCI quirk node (not valid on rpi3)
+  #
+  # 0  - DISABLED
+  #
+  gRaspberryPiTokenSpaceGuid.PcdXhciReload|L"XhciReload"|gConfigDxeFormSetGuid|0x0|0
+
   #
   # Common UEFI ones.
   #
diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
index 8c49c67ab2..4e91eb9aea 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -547,6 +547,13 @@
   #
   gRaspberryPiTokenSpaceGuid.PcdXhciPci|L"XhciPci"|gConfigDxeFormSetGuid|0x0|0

+  # DT contains XHCI quirk node
+  #
+  # 0  - No reload
+  # 1  - Yes, DT has Reload
+  #
+  gRaspberryPiTokenSpaceGuid.PcdXhciReload|L"XhciReload"|gConfigDxeFormSetGuid|0x0|0
+
   #
   # Common UEFI ones.
   #
diff --git a/Platform/RaspberryPi/RaspberryPi.dec b/Platform/RaspberryPi/RaspberryPi.dec
index 17b6061a05..6bd16a5ae9 100644
--- a/Platform/RaspberryPi/RaspberryPi.dec
+++ b/Platform/RaspberryPi/RaspberryPi.dec
@@ -73,3 +73,4 @@
   gRaspberryPiTokenSpaceGuid.PcdUartInUse|1|UINT32|0x00000021
   gRaspberryPiTokenSpaceGuid.PcdXhciPci|0|UINT32|0x00000022
   gRaspberryPiTokenSpaceGuid.PcdMiniUartClockRate|0|UINT32|0x00000023
+  gRaspberryPiTokenSpaceGuid.PcdXhciReload|0|UINT32|0x00000024
--
2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113547): https://edk2.groups.io/g/devel/message/113547
Mute This Topic: https://groups.io/mt/103652856/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 5/5] Platform/RaspberryPi: Update PCIe MMIO window for DT

[Jeremy Linton]

Since we are updating the DT memory map and telling it how
we have configured the PCIe, there isn't a reason for moving the
MMIO window. In fact this appears to fix OpenBSD+DT as well as
it makes the linux XHCI reset sequence happier.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c | 24 ++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
index dd4fc0a05e..e6942df6a3 100644
--- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
+++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
@@ -375,6 +375,30 @@ SyncPcie (
     return EFI_NOT_FOUND;
   }

+  // move the MMIO window too
+  DmaRanges[0] = cpu_to_fdt32 (0x02000000); //non prefech 32-bit
+  DmaRanges[1] = cpu_to_fdt32 (0x00000000); //bus addr @ 0x0f8000000
+  DmaRanges[2] = cpu_to_fdt32 (0xf8000000);
+  DmaRanges[3] = cpu_to_fdt32 (0x00000006); //cpu addr @ 0x600000000
+  DmaRanges[4] = cpu_to_fdt32 (0x00000000);
+  DmaRanges[5] = cpu_to_fdt32 (0x00000000);
+  DmaRanges[6] = cpu_to_fdt32 (0x04000000); // len = 0x4000 0000
+
+  DEBUG ((DEBUG_INFO, "%a: Updating PCIe ranges\n",  __func__));
+
+  /*
+   * Match dma-ranges with the EDK2+ACPI setup we are using.  This works
+   * around a failure in Linux and OpenBSD to reset the PCIe/XHCI correctly
+   * when in DT mode.
+   */
+  Retval = fdt_setprop (mFdtImage, Node, "ranges",
+                        DmaRanges,  sizeof DmaRanges);
+  if (Retval != 0) {
+    DEBUG ((DEBUG_ERROR, "%a: failed to locate PCIe MMIO 'ranges' property (%d)\n",
+      __func__, Retval));
+    return EFI_NOT_FOUND;
+  }
+
   if (PcdGet32 (PcdXhciReload) != 1) {
     return EFI_SUCCESS;
   }
--
2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113548): https://edk2.groups.io/g/devel/message/113548
Mute This Topic: https://groups.io/mt/103652857/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 1/5] Platform/RaspberryPi/DualSerialPortLib: Always configure the pl011

[Ard Biesheuvel]

Hi Jeremy,

Thanks for the patches.


On Thu, 11 Jan 2024 at 00:52, Jeremy Linton <jeremy.linton@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.linton@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?


>
>    //
>    // 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
> +  if ((UsePl011Uart) || (RPI_MODEL == 4)) {
>      ReceiveFifoDepth = 0;         // Use default FIFO depth
> +    BaudRate = FixedPcdGet64 (PcdUartDefaultBaudRate);

Shouldn't we hardcode 115200 here if !UsePl011Uart?

>      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 (#113589): https://edk2.groups.io/g/devel/message/113589
Mute This Topic: https://groups.io/mt/103652853/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 4/5] Platform/RaspberryPi: Give the user control over the XHCI mailbox

[Ard Biesheuvel]

On Thu, 11 Jan 2024 at 00:52, Jeremy Linton <jeremy.linton@arm.com> wrote:
>
> Its a complete tossup whether removing the mailbox call after we have
> set up the XHCI works for a given kernel+distro in DT mode. So lets
> give users which want to try DT the option of flipping this on/off.
>
> Users that don't want to have to deal with DT, can use ACPI.
>
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
>  .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c     | 10 ++++++++++
>  .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf   |  1 +
>  .../Drivers/ConfigDxe/ConfigDxeHii.uni            |  5 +++++
>  .../Drivers/ConfigDxe/ConfigDxeHii.vfr            | 15 +++++++++++++++
>  Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c      |  4 ++++
>  Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf    |  1 +
>  Platform/RaspberryPi/RPi3/RPi3.dsc                |  6 ++++++
>  Platform/RaspberryPi/RPi4/RPi4.dsc                |  7 +++++++
>  Platform/RaspberryPi/RaspberryPi.dec              |  1 +
>  9 files changed, 50 insertions(+)
>

This looks ok to me but it doesn't appear to apply on today's edk2-platforms.

> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> index 3dcf2bac0d..2484787982 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> @@ -298,6 +298,16 @@ SetupVariables (
>          Status = PcdSet32S (PcdXhciPci, 1);
>          ASSERT_EFI_ERROR (Status);
>        }
> +
> +      Size = sizeof (UINT32);
> +      Status = gRT->GetVariable (L"XhciReload",
> +                                 &gConfigDxeFormSetGuid,
> +                                 NULL, &Size, &Var32);
> +      if (EFI_ERROR (Status)) {
> +        Status = PcdSet32S (PcdXhciReload, PcdGet32 (PcdXhciReload));
> +        ASSERT_EFI_ERROR (Status);
> +      }
> +
>      }
>    } else {
>      /*
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> index 6f6e8f42ac..475e645537 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> @@ -96,6 +96,7 @@
>    gRaspberryPiTokenSpaceGuid.PcdUartInUse
>    gRaspberryPiTokenSpaceGuid.PcdXhciPci
>    gRaspberryPiTokenSpaceGuid.PcdMiniUartClockRate
> +  gRaspberryPiTokenSpaceGuid.PcdXhciReload
>
>  [Depex]
>    gPcdProtocolGuid AND gRaspberryPiFirmwareProtocolGuid
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> index 5ec17072c3..8130638876 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
> @@ -62,6 +62,11 @@
>  #string STR_ADVANCED_XHCIPCI_XHCI     #language en-US "XHCI"
>  #string STR_ADVANCED_XHCIPCI_PCIE     #language en-US "PCIe"
>
> +#string STR_ADVANCED_XHCIRELOAD_PROMPT  #language en-US "DT Reload XHCI firmware"
> +#string STR_ADVANCED_XHCIRELOAD_HELP    #language en-US "OS should reload XHCI firmware on reset"
> +#string STR_ADVANCED_XHCIRELOAD_DISABLE #language en-US "Disabled"
> +#string STR_ADVANCED_XHCIRELOAD_RELOAD  #language en-US "Reload"
> +
>  #string STR_ADVANCED_ASSET_TAG_PROMPT #language en-US "Asset Tag"
>  #string STR_ADVANCED_ASSET_TAG_HELP   #language en-US "Set the system Asset Tag"
>
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> index f668b7a553..f13b70711d 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
> @@ -61,6 +61,11 @@ formset
>        name  = XhciPci,
>        guid  = CONFIGDXE_FORM_SET_GUID;
>
> +    efivarstore ADVANCED_XHCIPCI_VARSTORE_DATA,
> +      attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
> +      name  = XhciReload,
> +      guid  = CONFIGDXE_FORM_SET_GUID;
> +
>      efivarstore SYSTEM_TABLE_MODE_VARSTORE_DATA,
>        attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
>        name  = SystemTableMode,
> @@ -228,6 +233,16 @@ formset
>                option text = STRING_TOKEN(STR_ADVANCED_XHCIPCI_PCIE), value = 1, flags = 0;
>              endoneof;
>            endif;
> +
> +          grayoutif ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_ACPI;
> +            oneof varid = XhciReload.Value,
> +              prompt      = STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_PROMPT),
> +              help        = STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_HELP),
> +              flags       = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
> +              option text = STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_DISABLE), value = 0, flags = DEFAULT;
> +              option text = STRING_TOKEN(STR_ADVANCED_XHCIRELOAD_RELOAD), value = 1, flags = 0;
> +            endoneof;
> +          endif;
>          endif;
>  #endif
>          string varid = AssetTag.AssetTag,
> diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> index cbbc2ad30d..dd4fc0a05e 100644
> --- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> +++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> @@ -375,6 +375,10 @@ SyncPcie (
>      return EFI_NOT_FOUND;
>    }
>
> +  if (PcdGet32 (PcdXhciReload) != 1) {
> +    return EFI_SUCCESS;
> +  }
> +
>    /*
>     * Now that we are always running without DMA translation, and with a 3G
>     * limit, there shouldn't be a need to reset/reload the XHCI. The
> diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
> index 26f84e5940..d9fb6ee480 100644
> --- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
> +++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf
> @@ -47,3 +47,4 @@
>
>  [Pcd]
>    gRaspberryPiTokenSpaceGuid.PcdSystemTableMode
> +  gRaspberryPiTokenSpaceGuid.PcdXhciReload
> diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc
> index a967cd5a20..f5361ab95e 100644
> --- a/Platform/RaspberryPi/RPi3/RPi3.dsc
> +++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
> @@ -529,6 +529,12 @@
>    #
>    gRaspberryPiTokenSpaceGuid.PcdXhciPci|L"XhciPci"|gConfigDxeFormSetGuid|0x0|0
>
> +  # DT contains XHCI quirk node (not valid on rpi3)
> +  #
> +  # 0  - DISABLED
> +  #
> +  gRaspberryPiTokenSpaceGuid.PcdXhciReload|L"XhciReload"|gConfigDxeFormSetGuid|0x0|0
> +
>    #
>    # Common UEFI ones.
>    #
> diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
> index 8c49c67ab2..4e91eb9aea 100644
> --- a/Platform/RaspberryPi/RPi4/RPi4.dsc
> +++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
> @@ -547,6 +547,13 @@
>    #
>    gRaspberryPiTokenSpaceGuid.PcdXhciPci|L"XhciPci"|gConfigDxeFormSetGuid|0x0|0
>
> +  # DT contains XHCI quirk node
> +  #
> +  # 0  - No reload
> +  # 1  - Yes, DT has Reload
> +  #
> +  gRaspberryPiTokenSpaceGuid.PcdXhciReload|L"XhciReload"|gConfigDxeFormSetGuid|0x0|0
> +
>    #
>    # Common UEFI ones.
>    #
> diff --git a/Platform/RaspberryPi/RaspberryPi.dec b/Platform/RaspberryPi/RaspberryPi.dec
> index 17b6061a05..6bd16a5ae9 100644
> --- a/Platform/RaspberryPi/RaspberryPi.dec
> +++ b/Platform/RaspberryPi/RaspberryPi.dec
> @@ -73,3 +73,4 @@
>    gRaspberryPiTokenSpaceGuid.PcdUartInUse|1|UINT32|0x00000021
>    gRaspberryPiTokenSpaceGuid.PcdXhciPci|0|UINT32|0x00000022
>    gRaspberryPiTokenSpaceGuid.PcdMiniUartClockRate|0|UINT32|0x00000023
> +  gRaspberryPiTokenSpaceGuid.PcdXhciReload|0|UINT32|0x00000024
> --
> 2.43.0
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113590): https://edk2.groups.io/g/devel/message/113590
Mute This Topic: https://groups.io/mt/103652856/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 5/5] Platform/RaspberryPi: Update PCIe MMIO window for DT

[Ard Biesheuvel]

On Thu, 11 Jan 2024 at 00:52, Jeremy Linton <jeremy.linton@arm.com> wrote:
>
> Since we are updating the DT memory map and telling it how
> we have configured the PCIe, there isn't a reason for moving the
> MMIO window. In fact this appears to fix OpenBSD+DT as well as
> it makes the linux XHCI reset sequence happier.
>
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
>  Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c | 24 ++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>
> diff --git a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> index dd4fc0a05e..e6942df6a3 100644
> --- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> +++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
> @@ -375,6 +375,30 @@ SyncPcie (
>      return EFI_NOT_FOUND;
>    }
>
> +  // move the MMIO window too
> +  DmaRanges[0] = cpu_to_fdt32 (0x02000000); //non prefech 32-bit
> +  DmaRanges[1] = cpu_to_fdt32 (0x00000000); //bus addr @ 0x0f8000000
> +  DmaRanges[2] = cpu_to_fdt32 (0xf8000000);
> +  DmaRanges[3] = cpu_to_fdt32 (0x00000006); //cpu addr @ 0x600000000
> +  DmaRanges[4] = cpu_to_fdt32 (0x00000000);
> +  DmaRanges[5] = cpu_to_fdt32 (0x00000000);
> +  DmaRanges[6] = cpu_to_fdt32 (0x04000000); // len = 0x4000 0000
> +

Is there any way we can use symbolic constants here?

> +  DEBUG ((DEBUG_INFO, "%a: Updating PCIe ranges\n",  __func__));
> +
> +  /*
> +   * Match dma-ranges with the EDK2+ACPI setup we are using.  This works
> +   * around a failure in Linux and OpenBSD to reset the PCIe/XHCI correctly
> +   * when in DT mode.
> +   */

Please sync the comment with the code.

> +  Retval = fdt_setprop (mFdtImage, Node, "ranges",
> +                        DmaRanges,  sizeof DmaRanges);
> +  if (Retval != 0) {
> +    DEBUG ((DEBUG_ERROR, "%a: failed to locate PCIe MMIO 'ranges' property (%d)\n",
> +      __func__, Retval));
> +    return EFI_NOT_FOUND;
> +  }
> +
>    if (PcdGet32 (PcdXhciReload) != 1) {
>      return EFI_SUCCESS;
>    }
> --
> 2.43.0
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113591): https://edk2.groups.io/g/devel/message/113591
Mute This Topic: https://groups.io/mt/103652857/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 1/5] Platform/RaspberryPi/DualSerialPortLib: Always configure the pl011

[Jeremy Linton]

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.linton@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.linton@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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-