[edk2-platforms: PATCH v2 0/2] Platforms/RPi3: Device Tree driver improvements

[edk2-platforms: PATCH v2 0/2] Platforms/RPi3: Device Tree driver improvements

[Pete Batard]

Changes from v1:
- Break down DEBUB output embellishments from the other code changes.
- Use GUIDs that are explicitly named after each platform/model rather than
  generic ones.
- Drop the use of a GUID table/index and use a single GUID variable instead.
- Add additional fallback in case the required internal Device Tree cannot
  be located (e.g. missing from the FDF).
- Ensures that all of the Device Tree selection and fallback can be set up
  in the .dec (FdtDxe reusability).
- Add premilinary Raspberry Pi 4 support (FdtDxe reusability).

Note: Since the changes required to support the Raspberry Pi 4 are a direct
byproduct of the changes required for multiple Device Tree support, I did
not feel the need to break them down into a separate patch.

Pete Batard (2):
  Platforms/RPi3: Improve debug output in FdtDxe
  Platforms/RPi3: Add multiple embedded Device Tree selection

 Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c   | 78 +++++++++++++++-----
 Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.inf |  5 +-
 Platform/RaspberryPi/RPi3/RPi3.dec                  |  8 +-
 Platform/RaspberryPi/RPi3/RPi3.fdf                  |  6 +-
 4 files changed, 77 insertions(+), 20 deletions(-)

-- 
2.21.0.windows.1

[edk2-platforms: PATCH v2 1/2] Platforms/RPi3: Improve debug output in FdtDxe

[Pete Batard]

Harmonize messages, avoid acronyms and provide human readable status codes.

Signed-off-by: Pete Batard <pete@akeo.ie>
---
 Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c | 24 ++++++++++----------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c b/Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c
index c84e5b2767e2..da99ef9535c8 100644
--- a/Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c
+++ b/Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c
@@ -383,12 +383,12 @@ FdtDxeInitialize (
      * Have FDT passed via config.txt.
      */
     FdtSize = fdt_totalsize (FdtImage);
-    DEBUG ((DEBUG_INFO, "DTB passed via config.txt of 0x%lx bytes\n", FdtSize));
+    DEBUG ((DEBUG_INFO, "Device Tree passed via config.txt (0x%lx bytes)\n", FdtSize));
     Status = EFI_SUCCESS;
   } else {
     Internal = TRUE;
-    DEBUG ((DEBUG_INFO, "No/bad FDT at %p (%a), trying internal DTB...\n",
-      FdtImage, fdt_strerror (Retval)));
+    DEBUG ((DEBUG_INFO, "No/Bad Device Tree found at address 0x%p (%a), "
+      "trying internal one...\n", FdtImage, fdt_strerror (Retval)));
     Status = GetSectionFromAnyFv (&gRaspberryPiFdtFileGuid, EFI_SECTION_RAW, 0,
                &FdtImage, &FdtSize);
     if (Status == EFI_SUCCESS) {
@@ -399,7 +399,7 @@ FdtDxeInitialize (
   }
 
   if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "Failed to locate device tree: %r\n", Status));
+    DEBUG ((DEBUG_ERROR, "Failed to locate Device Tree: %r\n", Status));
     return Status;
   }
 
@@ -410,7 +410,7 @@ FdtDxeInitialize (
   Status = gBS->AllocatePages (AllocateAnyPages, EfiBootServicesData,
                   EFI_SIZE_TO_PAGES (FdtSize), (EFI_PHYSICAL_ADDRESS*)&mFdtImage);
   if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "Failed to allocate new device tree: %r\n", Status));
+    DEBUG ((DEBUG_ERROR, "Failed to allocate Device Tree: %r\n", Status));
     return Status;
   }
 
@@ -419,27 +419,27 @@ FdtDxeInitialize (
 
   Status = SanitizePSCI ();
   if (EFI_ERROR (Status)) {
-    Print (L"Failed to sanitize PSCI (error %d)\n", Status);
+    Print (L"Failed to sanitize PSCI: %r\n", Status);
   }
 
   Status = CleanMemoryNodes ();
   if (EFI_ERROR (Status)) {
-    Print (L"Failed to clean memory nodes (error %d)\n", Status);
+    Print (L"Failed to clean memory nodes: %r\n", Status);
   }
 
   Status = CleanSimpleFramebuffer ();
   if (EFI_ERROR (Status)) {
-    Print (L"Failed to clean frame buffer (error %d)\n", Status);
+    Print (L"Failed to clean frame buffer: %r\n", Status);
   }
 
   Status = FixEthernetAliases ();
   if (EFI_ERROR (Status)) {
-    Print (L"Failed to fix ethernet aliases (error %d)\n", Status);
+    Print (L"Failed to fix ethernet aliases: %r\n", Status);
   }
 
   Status = UpdateMacAddress ();
   if (EFI_ERROR (Status)) {
-    Print (L"Failed to update MAC address (error %d)\n", Status);
+    Print (L"Failed to update MAC address: %r\n", Status);
   }
 
   if (Internal) {
@@ -448,11 +448,11 @@ FdtDxeInitialize (
      */
     Status = UpdateBootArgs ();
     if (EFI_ERROR (Status)) {
-      Print (L"Failed to update boot arguments (error %d)\n", Status);
+      Print (L"Failed to update boot arguments: %r\n", Status);
     }
   }
 
-  DEBUG ((DEBUG_INFO, "Installed FDT is at %p\n", mFdtImage));
+  DEBUG ((DEBUG_INFO, "Installed Device Tree at address 0x%p\n", mFdtImage));
   Status = gBS->InstallConfigurationTable (&gFdtTableGuid, mFdtImage);
   ASSERT_EFI_ERROR (Status);
 
-- 
2.21.0.windows.1

[edk2-platforms: PATCH v2 2/2] Platforms/RPi3: Add multiple embedded Device Tree selection

[Pete Batard]

The Raspberry Pi 3 platform currently has 2 different models, each with a
different Device Tree. Rather than embedding a single one, and requiring
users to manually provide the other, this patch ensures that we now embed
both and and serve the relevant one at runtime.

This patch also adds support for the Raspberry Pi 4 in FdtDxe.

Signed-off-by: Pete Batard <pete@akeo.ie>
---
 Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c   | 56 +++++++++++++++++---
 Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.inf |  5 +-
 Platform/RaspberryPi/RPi3/RPi3.dec                  |  8 ++-
 Platform/RaspberryPi/RPi3/RPi3.fdf                  |  6 ++-
 4 files changed, 66 insertions(+), 9 deletions(-)

diff --git a/Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c b/Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c
index da99ef9535c8..45ffe2e394a2 100644
--- a/Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c
+++ b/Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c
@@ -366,9 +366,11 @@ FdtDxeInitialize (
   )
 {
   EFI_STATUS Status;
+  EFI_GUID   *FdtGuid;
   VOID       *FdtImage;
   UINTN      FdtSize;
   INT32      Retval;
+  UINT32     BoardRevision;
   BOOLEAN    Internal;
 
   Status = gBS->LocateProtocol (&gRaspberryPiFirmwareProtocolGuid, NULL,
@@ -386,16 +388,58 @@ FdtDxeInitialize (
     DEBUG ((DEBUG_INFO, "Device Tree passed via config.txt (0x%lx bytes)\n", FdtSize));
     Status = EFI_SUCCESS;
   } else {
+    /*
+     * Use one of the embedded FDT's.
+     */
     Internal = TRUE;
     DEBUG ((DEBUG_INFO, "No/Bad Device Tree found at address 0x%p (%a), "
-      "trying internal one...\n", FdtImage, fdt_strerror (Retval)));
-    Status = GetSectionFromAnyFv (&gRaspberryPiFdtFileGuid, EFI_SECTION_RAW, 0,
-               &FdtImage, &FdtSize);
-    if (Status == EFI_SUCCESS) {
-      if (fdt_check_header (FdtImage) != 0) {
-        Status = EFI_INCOMPATIBLE_VERSION;
+      "looking up internal one...\n", FdtImage, fdt_strerror (Retval)));
+    /*
+     * Query the board revision to differentiate between models.
+     */
+    Status = mFwProtocol->GetModelRevision (&BoardRevision);
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_ERROR, "Failed to get board type: %r\n", Status));
+      DEBUG ((DEBUG_INFO, "Using default internal Device Tree\n"));
+      FdtGuid = &gRaspberryPiDefaultFdtGuid;
+    } else {
+      // www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/README.md
+      switch ((BoardRevision >> 4) & 0xFF) {
+      case 0x08:
+        DEBUG ((DEBUG_INFO, "Using Raspberry Pi 3 Model B internal Device Tree\n"));
+        FdtGuid = &gRaspberryPi3ModelBFdtGuid;
+        break;
+      case 0x0D:
+        DEBUG ((DEBUG_INFO, "Using Raspberry Pi 3 Model B+ internal Device Tree\n"));
+        FdtGuid = &gRaspberryPi3ModelBPlusFdtGuid;
+        break;
+      case 0x11:
+        DEBUG ((DEBUG_INFO, "Using Raspberry Pi 4 Model B internal Device Tree\n"));
+        FdtGuid = &gRaspberryPi4ModelBFdtGuid;
+        break;
+      default:
+        DEBUG ((DEBUG_INFO, "Using default internal Device Tree\n"));
+        FdtGuid = &gRaspberryPiDefaultFdtGuid;
+        break;
       }
     }
+    do {
+      Status = GetSectionFromAnyFv (FdtGuid, EFI_SECTION_RAW, 0, &FdtImage, &FdtSize);
+      if (Status == EFI_SUCCESS) {
+        if (fdt_check_header (FdtImage) != 0) {
+          Status = EFI_INCOMPATIBLE_VERSION;
+        }
+      }
+      // No retry needed if we are successful or are dealing with the default Fdt.
+      if ( (Status == EFI_SUCCESS) ||
+           (CompareGuid (FdtGuid, &gRaspberryPiDefaultFdtGuid)) )
+        break;
+      // Otherwise, try one more time with the default Fdt. An example of this
+      // is if we detected a non-default Fdt, that isn't included in the FDF.
+      DEBUG ((DEBUG_INFO, "Internal Device Tree was not found for this platform, "
+        "falling back to default...\n"));
+      FdtGuid = &gRaspberryPiDefaultFdtGuid;
+    } while (1);
   }
 
   if (EFI_ERROR (Status)) {
diff --git a/Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.inf b/Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.inf
index 5b0b1a09f374..570b05b62564 100644
--- a/Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.inf
+++ b/Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.inf
@@ -35,7 +35,10 @@ [LibraryClasses]
 
 [Guids]
   gFdtTableGuid
-  gRaspberryPiFdtFileGuid
+  gRaspberryPi3ModelBFdtGuid
+  gRaspberryPi3ModelBPlusFdtGuid
+  gRaspberryPi4ModelBFdtGuid
+  gRaspberryPiDefaultFdtGuid
 
 [Protocols]
   gRaspberryPiFirmwareProtocolGuid              ## CONSUMES
diff --git a/Platform/RaspberryPi/RPi3/RPi3.dec b/Platform/RaspberryPi/RPi3/RPi3.dec
index 22de439fde8f..d2a813417648 100644
--- a/Platform/RaspberryPi/RPi3/RPi3.dec
+++ b/Platform/RaspberryPi/RPi3/RPi3.dec
@@ -24,9 +24,15 @@ [Protocols]
 
 [Guids]
   gRaspberryPiTokenSpaceGuid = {0xCD7CC258, 0x31DB, 0x11E6, {0x9F, 0xD3, 0x63, 0xB0, 0xB8, 0xEE, 0xD6, 0xB5}}
-  gRaspberryPiFdtFileGuid = {0xDF5DA223, 0x1D27, 0x47C3, { 0x8D, 0x1B, 0x9A, 0x41, 0xB5, 0x5A, 0x18, 0xBC}}
   gRaspberryPiEventResetGuid = {0xCD7CC258, 0x31DB, 0x11E6, {0x9F, 0xD3, 0x63, 0xB4, 0xB4, 0xE4, 0xD4, 0xB4}}
   gConfigDxeFormSetGuid = {0xCD7CC258, 0x31DB, 0x22E6, {0x9F, 0x22, 0x63, 0xB0, 0xB8, 0xEE, 0xD6, 0xB5}}
+  # GUIDs used by FdtDxe to serve a Device Tree at runtime. Not all of these need to apply
+  # to the current platform or match an actual FDF binary, but they need to be defined.
+  gRaspberryPi3ModelBFdtGuid = { 0xDF5DA223, 0x1D27, 0x47C3, { 0x8D, 0x1B, 0x9A, 0x41, 0xB5, 0x5A, 0x18, 0xBC } }
+  gRaspberryPi3ModelBPlusFdtGuid = { 0x3D523012, 0x73FE, 0x40E5, { 0x89, 0x2E, 0x1A, 0x4D, 0xF6, 0x0F, 0x3C, 0x0C } }
+  gRaspberryPi4ModelBFdtGuid = { 0x80AB6833, 0xCAE4, 0x4CEE, { 0xB5, 0x9D, 0xEB, 0x20, 0x39, 0xB0, 0x55, 0x51 } }
+  # Default Fdt to serve if the hardware model can't be detected. Should match one of the above.
+  gRaspberryPiDefaultFdtGuid = {0xDF5DA223, 0x1D27, 0x47C3, { 0x8D, 0x1B, 0x9A, 0x41, 0xB5, 0x5A, 0x18, 0xBC}}
 
 [PcdsFixedAtBuild.common]
   gRaspberryPiTokenSpaceGuid.PcdFdtBaseAddress|0x10000|UINT32|0x00000001
diff --git a/Platform/RaspberryPi/RPi3/RPi3.fdf b/Platform/RaspberryPi/RPi3/RPi3.fdf
index c62d649834c7..17c0094ac1cf 100644
--- a/Platform/RaspberryPi/RPi3/RPi3.fdf
+++ b/Platform/RaspberryPi/RPi3/RPi3.fdf
@@ -300,11 +300,15 @@ [FV.FvMain]
   INF Platform/RaspberryPi/$(PLATFORM_NAME)/Drivers/LogoDxe/LogoDxe.inf
 
   #
-  # FDT (GUID matches gRaspberryPiFdtFileGuid in FdtDxe)
+  # Device Tree support (used by FdtDxe)
+  # GUIDs should match gRaspberryPi#####FdtGuid's from the .dec
   #
   FILE FREEFORM = DF5DA223-1D27-47C3-8D1B-9A41B55A18BC {
     SECTION RAW = Platform/RaspberryPi/$(PLATFORM_NAME)/DeviceTree/bcm2710-rpi-3-b.dtb
   }
+  FILE FREEFORM = 3D523012-73FE-40E5-892E-1A4DF60F3C0C {
+    SECTION RAW = Platform/RaspberryPi/$(PLATFORM_NAME)/DeviceTree/bcm2710-rpi-3-b-plus.dtb
+  }
 
 [FV.FVMAIN_COMPACT]
 FvAlignment        = 16
-- 
2.21.0.windows.1

Re: [edk2-platforms: PATCH v2 0/2] Platforms/RPi3: Device Tree driver improvements

[Leif Lindholm]

On Fri, Aug 16, 2019 at 11:00:07AM +0100, Pete Batard wrote:
> Changes from v1:
> - Break down DEBUB output embellishments from the other code changes.
> - Use GUIDs that are explicitly named after each platform/model rather than
>   generic ones.
> - Drop the use of a GUID table/index and use a single GUID variable instead.
> - Add additional fallback in case the required internal Device Tree cannot
>   be located (e.g. missing from the FDF).
> - Ensures that all of the Device Tree selection and fallback can be set up
>   in the .dec (FdtDxe reusability).
> - Add premilinary Raspberry Pi 4 support (FdtDxe reusability).
> 
> Note: Since the changes required to support the Raspberry Pi 4 are a direct
> byproduct of the changes required for multiple Device Tree support, I did
> not feel the need to break them down into a separate patch.

For the series,
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
Pushed as 591f0c39589c..7da906bfcd03.

Thanks!

> Pete Batard (2):
>   Platforms/RPi3: Improve debug output in FdtDxe
>   Platforms/RPi3: Add multiple embedded Device Tree selection
> 
>  Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c   | 78 +++++++++++++++-----
>  Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.inf |  5 +-
>  Platform/RaspberryPi/RPi3/RPi3.dec                  |  8 +-
>  Platform/RaspberryPi/RPi3/RPi3.fdf                  |  6 +-
>  4 files changed, 77 insertions(+), 20 deletions(-)
> 
> -- 
> 2.21.0.windows.1
>