public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Michael Brown <mbrown@fensystems.co.uk>
To: devel@edk2.groups.io
Cc: ard.biesheuvel@linaro.org, leif.lindholm@linaro.org,
	pete@akeo.ie, Michael Brown <mbrown@fensystems.co.uk>
Subject: [edk2-platforms: PATCH v2 1/1] Platform/RPi3: Report device tree modification errors using Print()
Date: Fri,  9 Aug 2019 00:16:25 +0100	[thread overview]
Message-ID: <20190808231625.1730843-2-mbrown@fensystems.co.uk> (raw)
In-Reply-To: <20190806170719.GJ25813@bivouac.eciton.net>

Most functions in FdtDxe currently return VOID and report errors using
only DEBUG.  Update functions to return EFI_STATUS and report errors
using Print() so that errors are at least visible in non-debug builds.

Signed-off-by: Michael Brown <mbrown@fensystems.co.uk>
---
 .../RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c  | 102 ++++++++++++------
 1 file changed, 70 insertions(+), 32 deletions(-)

diff --git a/Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c b/Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c
index 57e4bee8da..c84e5b2767 100644
--- a/Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c
+++ b/Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c
@@ -13,6 +13,7 @@
 #include <Library/DxeServicesLib.h>
 #include <Library/MemoryAllocationLib.h>
 #include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiLib.h>
 #include <libfdt.h>
 
 #include <Protocol/RpiFirmware.h>
@@ -24,7 +25,7 @@ STATIC VOID                             *mFdtImage;
 STATIC RASPBERRY_PI_FIRMWARE_PROTOCOL   *mFwProtocol;
 
 STATIC
-VOID
+EFI_STATUS
 FixEthernetAliases (
   VOID
 )
@@ -36,6 +37,7 @@ FixEthernetAliases (
   UINTN         CopySize;
   CHAR8         *Copy;
   INTN          Retval;
+  EFI_STATUS    Status;
 
   //
   // Look up the 'ethernet[0]' aliases
@@ -43,14 +45,14 @@ FixEthernetAliases (
   Aliases = fdt_path_offset (mFdtImage, "/aliases");
   if (Aliases < 0) {
     DEBUG ((DEBUG_ERROR, "%a: failed to locate '/aliases'\n", __FUNCTION__));
-    return;
+    return EFI_NOT_FOUND;
   }
   Ethernet = fdt_getprop (mFdtImage, Aliases, "ethernet", NULL);
   Ethernet0 = fdt_getprop (mFdtImage, Aliases, "ethernet0", NULL);
   Alias = Ethernet ? Ethernet : Ethernet0;
   if (!Alias) {
     DEBUG ((DEBUG_ERROR, "%a: failed to locate 'ethernet[0]' alias\n", __FUNCTION__));
-    return;
+    return EFI_NOT_FOUND;
   }
 
   //
@@ -60,15 +62,17 @@ FixEthernetAliases (
   Copy = AllocateCopyPool (CopySize, Alias);
   if (!Copy) {
     DEBUG ((DEBUG_ERROR, "%a: failed to copy '%a'\n", __FUNCTION__, Alias));
-    return;
+    return EFI_OUT_OF_RESOURCES;
   }
 
   //
   // Create missing aliases
   //
+  Status = EFI_SUCCESS;
   if (!Ethernet) {
     Retval = fdt_setprop (mFdtImage, Aliases, "ethernet", Copy, CopySize);
     if (Retval != 0) {
+      Status = EFI_NOT_FOUND;
       DEBUG ((DEBUG_ERROR, "%a: failed to create 'ethernet' alias (%d)\n",
         __FUNCTION__, Retval));
     }
@@ -77,6 +81,7 @@ FixEthernetAliases (
   if (!Ethernet0) {
     Retval = fdt_setprop (mFdtImage, Aliases, "ethernet0", Copy, CopySize);
     if (Retval != 0) {
+      Status = EFI_NOT_FOUND;
       DEBUG ((DEBUG_ERROR, "%a: failed to create 'ethernet0' alias (%d)\n",
         __FUNCTION__, Retval));
     }
@@ -84,10 +89,11 @@ FixEthernetAliases (
   }
 
   FreePool (Copy);
+  return Status;
 }
 
 STATIC
-VOID
+EFI_STATUS
 UpdateMacAddress (
   VOID
   )
@@ -103,7 +109,7 @@ UpdateMacAddress (
   Node = fdt_path_offset (mFdtImage, "ethernet");
   if (Node < 0) {
     DEBUG ((DEBUG_ERROR, "%a: failed to locate 'ethernet' alias\n", __FUNCTION__));
-    return;
+    return EFI_NOT_FOUND;
   }
 
   //
@@ -112,7 +118,7 @@ UpdateMacAddress (
   Status = mFwProtocol->GetMacAddress (MacAddress);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "%a: failed to retrieve MAC address\n", __FUNCTION__));
-    return;
+    return Status;
   }
 
   Retval = fdt_setprop (mFdtImage, Node, "mac-address", MacAddress,
@@ -120,16 +126,17 @@ UpdateMacAddress (
   if (Retval != 0) {
     DEBUG ((DEBUG_ERROR, "%a: failed to create 'mac-address' property (%d)\n",
       __FUNCTION__, Retval));
-    return;
+    return EFI_NOT_FOUND;
   }
 
   DEBUG ((DEBUG_INFO, "%a: setting MAC address to %02x:%02x:%02x:%02x:%02x:%02x\n",
     __FUNCTION__, MacAddress[0], MacAddress[1], MacAddress[2], MacAddress[3],
     MacAddress[4], MacAddress[5]));
+  return EFI_SUCCESS;
 }
 
 STATIC
-VOID
+EFI_STATUS
 CleanMemoryNodes (
   VOID
   )
@@ -139,7 +146,7 @@ CleanMemoryNodes (
 
   Node = fdt_path_offset (mFdtImage, "/memory");
   if (Node < 0) {
-    return;
+    return EFI_SUCCESS;
   }
 
   /*
@@ -150,11 +157,14 @@ CleanMemoryNodes (
   Retval = fdt_del_node (mFdtImage, Node);
   if (Retval != 0) {
     DEBUG ((DEBUG_ERROR, "Failed to remove /memory\n"));
+    return EFI_NOT_FOUND;
   }
+
+  return EFI_SUCCESS;
 }
 
 STATIC
-VOID
+EFI_STATUS
 SanitizePSCI (
   VOID
   )
@@ -166,7 +176,7 @@ SanitizePSCI (
   Root = fdt_path_offset (mFdtImage, "/");
   ASSERT (Root >= 0);
   if (Root < 0) {
-    return;
+    return EFI_NOT_FOUND;
   }
 
   Node = fdt_path_offset (mFdtImage, "/psci");
@@ -177,41 +187,42 @@ SanitizePSCI (
   ASSERT (Node >= 0);
   if (Node < 0) {
     DEBUG ((DEBUG_ERROR, "Couldn't find/create /psci\n"));
-    return;
+    return EFI_NOT_FOUND;
   }
 
   Retval = fdt_setprop_string (mFdtImage, Node, "compatible", "arm,psci-1.0");
   if (Retval != 0) {
     DEBUG ((DEBUG_ERROR, "Couldn't set /psci compatible property\n"));
-    return;
+    return EFI_NOT_FOUND;
   }
 
   Retval = fdt_setprop_string (mFdtImage, Node, "method", "smc");
   if (Retval != 0) {
     DEBUG ((DEBUG_ERROR, "Couldn't set /psci method property\n"));
-    return;
+    return EFI_NOT_FOUND;
   }
 
   Root = fdt_path_offset (mFdtImage, "/cpus");
   if (Root < 0) {
     DEBUG ((DEBUG_ERROR, "No CPUs to update with PSCI enable-method?\n"));
-    return;
+    return EFI_NOT_FOUND;
   }
 
   Node = fdt_first_subnode (mFdtImage, Root);
   while (Node >= 0) {
     if (fdt_setprop_string (mFdtImage, Node, "enable-method", "psci") != 0) {
       DEBUG ((DEBUG_ERROR, "Failed to update enable-method for a CPU\n"));
-      return;
+      return EFI_NOT_FOUND;
     }
 
     fdt_delprop (mFdtImage, Node, "cpu-release-addr");
     Node = fdt_next_subnode (mFdtImage, Node);
   }
+  return EFI_SUCCESS;
 }
 
 STATIC
-VOID
+EFI_STATUS
 CleanSimpleFramebuffer (
   VOID
   )
@@ -225,7 +236,7 @@ CleanSimpleFramebuffer (
    */
   Node = fdt_path_offset (mFdtImage, "display0");
   if (Node < 0) {
-    return;
+    return EFI_SUCCESS;
   }
 
   /*
@@ -236,25 +247,28 @@ CleanSimpleFramebuffer (
   Retval = fdt_del_node (mFdtImage, Node);
   if (Retval != 0) {
     DEBUG ((DEBUG_ERROR, "Failed to remove display0\n"));
-    return;
+    return EFI_NOT_FOUND;
   }
 
   Node = fdt_path_offset (mFdtImage, "/aliases");
   if (Node < 0) {
     DEBUG ((DEBUG_ERROR, "Couldn't find /aliases to remove display0\n"));
-    return;
+    return EFI_NOT_FOUND;
   }
 
   Retval = fdt_delprop (mFdtImage, Node, "display0");
   if (Retval != 0) {
     DEBUG ((DEBUG_ERROR, "Failed to remove display0 alias\n"));
+    return EFI_NOT_FOUND;
   }
+
+  return EFI_SUCCESS;
 }
 
 #define MAX_CMDLINE_SIZE    512
 
 STATIC
-VOID
+EFI_STATUS
 UpdateBootArgs (
   VOID
   )
@@ -270,7 +284,7 @@ UpdateBootArgs (
   Node = fdt_path_offset (mFdtImage, "/chosen");
   if (Node < 0) {
     DEBUG ((DEBUG_ERROR, "%a: failed to locate /chosen node\n", __FUNCTION__));
-    return;
+    return EFI_NOT_FOUND;
   }
 
   //
@@ -282,7 +296,7 @@ UpdateBootArgs (
   CommandLine = AllocatePool (MAX_CMDLINE_SIZE) + 4;
   if (!CommandLine) {
     DEBUG ((DEBUG_ERROR, "%a: failed to allocate memory\n", __FUNCTION__));
-    return;
+    return EFI_OUT_OF_RESOURCES;
   }
 
   //
@@ -291,12 +305,12 @@ UpdateBootArgs (
   Status = mFwProtocol->GetCommandLine (MAX_CMDLINE_SIZE, CommandLine + 4);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "%a: failed to retrieve command line\n", __FUNCTION__));
-    return;
+    return Status;
   }
 
   if (AsciiStrLen (CommandLine + 4) == 0) {
     DEBUG ((DEBUG_INFO, "%a: empty command line received\n", __FUNCTION__));
-    return;
+    return EFI_SUCCESS;
   }
 
   CommandLine[3] = ' ';
@@ -331,6 +345,7 @@ UpdateBootArgs (
   DEBUG_CODE_END ();
 
   FreePool (CommandLine - 4);
+  return EFI_SUCCESS;
 }
 
 
@@ -402,16 +417,39 @@ FdtDxeInitialize (
   Retval = fdt_open_into (FdtImage, mFdtImage, FdtSize);
   ASSERT (Retval == 0);
 
-  SanitizePSCI ();
-  CleanMemoryNodes ();
-  CleanSimpleFramebuffer ();
-  FixEthernetAliases ();
-  UpdateMacAddress ();
+  Status = SanitizePSCI ();
+  if (EFI_ERROR (Status)) {
+    Print (L"Failed to sanitize PSCI (error %d)\n", Status);
+  }
+
+  Status = CleanMemoryNodes ();
+  if (EFI_ERROR (Status)) {
+    Print (L"Failed to clean memory nodes (error %d)\n", Status);
+  }
+
+  Status = CleanSimpleFramebuffer ();
+  if (EFI_ERROR (Status)) {
+    Print (L"Failed to clean frame buffer (error %d)\n", Status);
+  }
+
+  Status = FixEthernetAliases ();
+  if (EFI_ERROR (Status)) {
+    Print (L"Failed to fix ethernet aliases (error %d)\n", Status);
+  }
+
+  Status = UpdateMacAddress ();
+  if (EFI_ERROR (Status)) {
+    Print (L"Failed to update MAC address (error %d)\n", Status);
+  }
+
   if (Internal) {
     /*
      * A GPU-provided DTB already has the full command line.
      */
-    UpdateBootArgs ();
+    Status = UpdateBootArgs ();
+    if (EFI_ERROR (Status)) {
+      Print (L"Failed to update boot arguments (error %d)\n", Status);
+    }
   }
 
   DEBUG ((DEBUG_INFO, "Installed FDT is at %p\n", mFdtImage));
-- 
2.21.0


  parent reply	other threads:[~2019-08-08 23:16 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-19 17:29 [edk2-platforms: PATCH 0/1] Handle RPi device trees using "ethernet0" Michael Brown
2019-07-19 17:29 ` [edk2-platforms: PATCH 1/1] Platform/RPi3: Accept "ethernet" or "ethernet0" aliases in device tree Michael Brown
2019-07-23 10:34   ` Leif Lindholm
2019-07-23 11:00     ` Michael Brown
2019-07-23 11:17       ` Leif Lindholm
2019-07-24 14:31         ` [edk2-platforms: PATCHv2 0/1] Platform/RPi3: Provide "ethernet[0]" " Michael Brown
2019-07-24 14:31         ` [edk2-platforms: PATCHv2 1/1] " Michael Brown
2019-07-24 18:53           ` Leif Lindholm
2019-07-24 20:53             ` [edk2-devel] " Michael Brown
2019-07-24 21:43               ` Leif Lindholm
2019-07-25 10:27                 ` [edk2-platforms: PATCH 0/1] Report device tree modification errors using Print() Michael Brown
2019-07-25 10:27                 ` [edk2-platforms: PATCH 1/1] Platform/RPi3: " Michael Brown
2019-08-06 17:07                   ` Leif Lindholm
2019-08-08 23:16                     ` [edk2-platforms: PATCH v2 0/1] Platform/RPi3: Report device tree Michael Brown
2019-08-09 10:07                       ` Leif Lindholm
2019-08-08 23:16                     ` Michael Brown [this message]
2019-07-25 12:51                 ` [edk2-devel] [edk2-platforms: PATCHv2 1/1] Platform/RPi3: Provide "ethernet[0]" aliases in " Leif Lindholm
2019-07-23 11:19       ` [edk2-platforms: PATCH 1/1] Platform/RPi3: Accept "ethernet" or "ethernet0" " Pete Batard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190808231625.1730843-2-mbrown@fensystems.co.uk \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox