[edk2-platforms][PATCH v2 0/3] Further RPi4 support groundwork

[edk2-platforms][PATCH v2 0/3] Further RPi4 support groundwork

[Pete Batard]

Changes from v1 (https://edk2.groups.io/g/devel/message/51334):
- Moved RNG related changes into their own series
- Fix a whitespace issue in the last patch

Andrei Warkentin (2):
  Platform/RPi/MmcDxe: Factorize SCR call and clean up MMC init
  Platform/RPi/MmcDxe: Improve MMC driver stability

Samer El-Haj-Mahmoud (1):
  Platform/RPi: Set SD routing according to model

 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c      | 137 ++++++++++++++------
 Platform/RaspberryPi/Drivers/MmcDxe/Mmc.c               |   7 +-
 Platform/RaspberryPi/Drivers/MmcDxe/MmcIdentification.c | 105 +++++++++------
 3 files changed, 164 insertions(+), 85 deletions(-)

-- 
2.21.0.windows.1

[edk2-platforms][PATCH v2 1/3] Platform/RPi/MmcDxe: Factorize SCR call and clean up MMC init

[Pete Batard]

From: Andrei Warkentin <andrey.warkentin@gmail.com>

This is mostly a maintainability improvement for the
InitializeSdMmcDevice () call achieved by factorizing the
code related to SCR execution into a new SdExecuteScr () call.

Signed-off-by: Pete Batard <pete@akeo.ie>
---
 Platform/RaspberryPi/Drivers/MmcDxe/MmcIdentification.c | 105 ++++++++++++--------
 1 file changed, 62 insertions(+), 43 deletions(-)

diff --git a/Platform/RaspberryPi/Drivers/MmcDxe/MmcIdentification.c b/Platform/RaspberryPi/Drivers/MmcDxe/MmcIdentification.c
index 4ee5c5ca6fb2..34a97e954220 100644
--- a/Platform/RaspberryPi/Drivers/MmcDxe/MmcIdentification.c
+++ b/Platform/RaspberryPi/Drivers/MmcDxe/MmcIdentification.c
@@ -1,5 +1,6 @@
 /** @file
  *
+ *  Copyright (c) 2019, Andrei Warkentin <andrey.warkentin@gmail.com>
  *  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
  *
  *  SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -567,6 +568,48 @@ SdSetSpeed (
   return EFI_SUCCESS;
 }
 
+STATIC
+EFI_STATUS
+SdExecuteScr (
+  IN  MMC_HOST_INSTANCE *MmcHostInstance,
+  OUT SCR *Scr
+  )
+{
+  EFI_STATUS Status;
+  UINT32 Response[4];
+  EFI_MMC_HOST_PROTOCOL *MmcHost = MmcHostInstance->MmcHost;
+
+  Status = MmcHost->SendCommand (MmcHost, MMC_CMD55,
+                      MmcHostInstance->CardInfo.RCA << 16);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "%a (MMC_CMD55): Error and Status = %r\n", __FUNCTION__, Status));
+    return Status;
+  }
+  Status = MmcHost->ReceiveResponse (MmcHost, MMC_RESPONSE_TYPE_R1, Response);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "%a (MMC_CMD55): Error and Status = %r\n", __FUNCTION__, Status));
+    return Status;
+  }
+  if ((Response[0] & MMC_STATUS_APP_CMD) == 0) {
+    return EFI_SUCCESS;
+  }
+
+  /* SCR */
+  Status = MmcHost->SendCommand (MmcHost, MMC_ACMD51, 0);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "%a(MMC_ACMD51): Error and Status = %r\n", __func__, Status));
+    return Status;
+  }
+
+  Status = MmcHost->ReadBlockData (MmcHost, 0, 8, (VOID *) Scr);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "%a(MMC_ACMD51): ReadBlockData Error and Status = %r\n", __func__, Status));
+    return Status;
+  }
+
+  return EFI_SUCCESS;
+}
+
 STATIC
 EFI_STATUS
 InitializeSdMmcDevice (
@@ -574,7 +617,6 @@ InitializeSdMmcDevice (
   )
 {
   UINT32        Response[4];
-  UINT32        Buffer[128];
   UINTN         BlockSize;
   UINTN         CardSize;
   UINTN         NumBlocks;
@@ -621,58 +663,35 @@ InitializeSdMmcDevice (
     return Status;
   }
 
-  Status = MmcHost->SendCommand (MmcHost, MMC_CMD55,
-                      MmcHostInstance->CardInfo.RCA << 16);
+  Status = SdExecuteScr (MmcHostInstance, &Scr);
   if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "%a (MMC_CMD55): Error and Status = %r\n", __FUNCTION__, Status));
-    return Status;
-  }
-  Status = MmcHost->ReceiveResponse (MmcHost, MMC_RESPONSE_TYPE_R1, Response);
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "%a (MMC_CMD55): Error and Status = %r\n", __FUNCTION__, Status));
-    return Status;
-  }
-  if ((Response[0] & MMC_STATUS_APP_CMD) == 0) {
-    return EFI_SUCCESS;
+     return Status;
   }
 
-  /* SCR */
-  Status = MmcHost->SendCommand (MmcHost, MMC_ACMD51, 0);
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "%a(MMC_ACMD51): Error and Status = %r\n", __func__, Status));
-    return Status;
-  } else {
-    Status = MmcHost->ReadBlockData (MmcHost, 0, 8, Buffer);
-    if (EFI_ERROR (Status)) {
-      DEBUG ((DEBUG_ERROR, "%a(MMC_ACMD51): ReadBlockData Error and Status = %r\n", __func__, Status));
-      return Status;
-    }
-    CopyMem (&Scr, Buffer, 8);
-    if (Scr.SD_SPEC == 2) {
-      if (Scr.SD_SPEC3 == 1) {
-        if (Scr.SD_SPEC4 == 1) {
-          DEBUG ((DEBUG_INFO, "Found SD Card for Spec Version 4.xx\n"));
-        } else {
-          DEBUG ((DEBUG_INFO, "Found SD Card for Spec Version 3.0x\n"));
-        }
+  if (Scr.SD_SPEC == 2) {
+    if (Scr.SD_SPEC3 == 1) {
+      if (Scr.SD_SPEC4 == 1) {
+        DEBUG ((DEBUG_INFO, "Found SD Card for Spec Version 4.xx\n"));
       } else {
-        if (Scr.SD_SPEC4 == 0) {
-          DEBUG ((DEBUG_INFO, "Found SD Card for Spec Version 2.0\n"));
-        } else {
-          DEBUG ((DEBUG_ERROR, "Found invalid SD Card\n"));
-        }
+        DEBUG ((DEBUG_INFO, "Found SD Card for Spec Version 3.0x\n"));
       }
     } else {
-      if ((Scr.SD_SPEC3 == 0) && (Scr.SD_SPEC4 == 0)) {
-        if (Scr.SD_SPEC == 1) {
-          DEBUG ((DEBUG_INFO, "Found SD Card for Spec Version 1.10\n"));
-        } else {
-          DEBUG ((DEBUG_INFO, "Found SD Card for Spec Version 1.0\n"));
-        }
+      if (Scr.SD_SPEC4 == 0) {
+        DEBUG ((DEBUG_INFO, "Found SD Card for Spec Version 2.0\n"));
       } else {
         DEBUG ((DEBUG_ERROR, "Found invalid SD Card\n"));
       }
     }
+  } else {
+    if ((Scr.SD_SPEC3 == 0) && (Scr.SD_SPEC4 == 0)) {
+      if (Scr.SD_SPEC == 1) {
+        DEBUG ((DEBUG_INFO, "Found SD Card for Spec Version 1.10\n"));
+      } else {
+        DEBUG ((DEBUG_INFO, "Found SD Card for Spec Version 1.0\n"));
+      }
+    } else {
+      DEBUG ((DEBUG_ERROR, "Found invalid SD Card\n"));
+    }
   }
 
   Status = SdSetSpeed (MmcHostInstance, CccSwitch);
-- 
2.21.0.windows.1

[edk2-platforms][PATCH v2 2/3] Platform/RPi/MmcDxe: Improve MMC driver stability

[Pete Batard]

From: Andrei Warkentin <andrey.warkentin@gmail.com>

For reasons that aren't entirely understood, SCR reads can randomly
fail on the Raspberry Pi 4. It can be on the first boot or during
subsequent reboots.

To alleviate this, and improve the overall behavior of the code,
we modify CheckCardsCallback () to retry InitializeMmcDevice ()
in case of failure.

Signed-off-by: Pete Batard <pete@akeo.ie>
---
 Platform/RaspberryPi/Drivers/MmcDxe/Mmc.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Platform/RaspberryPi/Drivers/MmcDxe/Mmc.c b/Platform/RaspberryPi/Drivers/MmcDxe/Mmc.c
index c3c7279e4707..f6c4cc7bc3a3 100644
--- a/Platform/RaspberryPi/Drivers/MmcDxe/Mmc.c
+++ b/Platform/RaspberryPi/Drivers/MmcDxe/Mmc.c
@@ -2,6 +2,7 @@
  *
  *  Main file of the MMC Dxe driver. The driver entrypoint is defined into this file.
  *
+ *  Copyright (c) 2019, Andrei Warkentin <andrey.warkentin@gmail.com>
  *  Copyright (c) 2011-2013, ARM Limited. All rights reserved.
  *
  *  SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -367,7 +368,11 @@ CheckCardsCallback (
       MmcHostInstance->Initialized = !MmcHostInstance->Initialized;
 
       if (MmcHostInstance->BlockIo.Media->MediaPresent) {
-        InitializeMmcDevice (MmcHostInstance);
+        Status = InitializeMmcDevice (MmcHostInstance);
+        if (EFI_ERROR (Status)) {
+          MmcHostInstance->Initialized = !MmcHostInstance->Initialized;
+          continue;
+        }
       }
 
       Status = gBS->ReinstallProtocolInterface (
-- 
2.21.0.windows.1

[edk2-platforms][PATCH v2 3/3] Platform/RPi: Set SD routing according to model

[Pete Batard]

From: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>

The Raspberry Pi 4 has a new SD controller. As a result we must
handle SD routing according to the model, which we perform in
the Config driver by using the GetModelFamily () call that was
recently introduced.

Signed-off-by: Pete Batard <pete@akeo.ie>
---
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 137 ++++++++++++++------
 1 file changed, 96 insertions(+), 41 deletions(-)

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
index 98e58a560ed4..f92ac709a3d8 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -1,6 +1,7 @@
 /** @file
  *
- *  Copyright (c) 2018, Andrei Warkentin <andrey.warkentin@gmail.com>
+ *  Copyright (c) 2019, ARM Limited. All rights reserved.
+ *  Copyright (c) 2018 - 2019, Andrei Warkentin <andrey.warkentin@gmail.com>
  *
  *  SPDX-License-Identifier: BSD-2-Clause-Patent
  *
@@ -9,10 +10,12 @@
 #include <Uefi.h>
 #include <Library/HiiLib.h>
 #include <Library/DebugLib.h>
+#include <Library/IoLib.h>
 #include <Library/UefiBootServicesTableLib.h>
 #include <Library/UefiRuntimeServicesTableLib.h>
 #include <Library/DevicePathLib.h>
 #include <IndustryStandard/RpiMbox.h>
+#include <IndustryStandard/Bcm2836.h>
 #include <IndustryStandard/Bcm2836Gpio.h>
 #include <Library/GpioLib.h>
 #include <Protocol/RpiFirmware.h>
@@ -212,6 +215,7 @@ ApplyVariables (
   UINT32 CpuClock = PcdGet32 (PcdCpuClock);
   UINT32 CustomCpuClock = PcdGet32 (PcdCustomCpuClock);
   UINT32 Rate = 0;
+  UINT32 ModelFamily = 0;
 
   if (CpuClock != 0) {
     if (CpuClock == 2) {
@@ -245,51 +249,102 @@ ApplyVariables (
     DEBUG ((DEBUG_INFO, "Current CPU speed is %uHz\n", Rate));
   }
 
-  /*
-   * Switching two groups around, so disable both first.
-   *
-   * No, I've not seen a problem, but having a group be
-   * routed to two sets of pins seems like asking for trouble.
-   */
-  GpioPinFuncSet (34, GPIO_FSEL_INPUT);
-  GpioPinFuncSet (35, GPIO_FSEL_INPUT);
-  GpioPinFuncSet (36, GPIO_FSEL_INPUT);
-  GpioPinFuncSet (37, GPIO_FSEL_INPUT);
-  GpioPinFuncSet (38, GPIO_FSEL_INPUT);
-  GpioPinFuncSet (39, GPIO_FSEL_INPUT);
-  GpioPinFuncSet (48, GPIO_FSEL_INPUT);
-  GpioPinFuncSet (49, GPIO_FSEL_INPUT);
-  GpioPinFuncSet (50, GPIO_FSEL_INPUT);
-  GpioPinFuncSet (51, GPIO_FSEL_INPUT);
-  GpioPinFuncSet (52, GPIO_FSEL_INPUT);
-  GpioPinFuncSet (53, GPIO_FSEL_INPUT);
-  if (PcdGet32 (PcdSdIsArasan)) {
-    DEBUG ((DEBUG_INFO, "Routing SD to Arasan\n"));
-    Gpio48Group = GPIO_FSEL_ALT3;
+  Status = mFwProtocol->GetModelFamily (&ModelFamily);
+  if (Status != EFI_SUCCESS) {
+    DEBUG ((DEBUG_ERROR, "Couldn't get the Raspberry Pi model family: %r\n", Status));
+  } else {
+    DEBUG ((DEBUG_INFO, "Current Raspberry Pi model family is 0x%x\n", ModelFamily));
+  }
+
+
+  if (ModelFamily == 3) {
     /*
-     * Route SDIO to SdHost.
+     * Pi 3: either Arasan or SdHost goes to SD card.
+     *
+     * Switching two groups around, so disable both first.
+     *
+     * No, I've not seen a problem, but having a group be
+     * routed to two sets of pins seems like asking for trouble.
      */
-    Gpio34Group = GPIO_FSEL_ALT0;
-  } else {
-    DEBUG ((DEBUG_INFO, "Routing SD to SdHost\n"));
-    Gpio48Group = GPIO_FSEL_ALT0;
+    GpioPinFuncSet (34, GPIO_FSEL_INPUT);
+    GpioPinFuncSet (35, GPIO_FSEL_INPUT);
+    GpioPinFuncSet (36, GPIO_FSEL_INPUT);
+    GpioPinFuncSet (37, GPIO_FSEL_INPUT);
+    GpioPinFuncSet (38, GPIO_FSEL_INPUT);
+    GpioPinFuncSet (39, GPIO_FSEL_INPUT);
+    GpioPinFuncSet (48, GPIO_FSEL_INPUT);
+    GpioPinFuncSet (49, GPIO_FSEL_INPUT);
+    GpioPinFuncSet (50, GPIO_FSEL_INPUT);
+    GpioPinFuncSet (51, GPIO_FSEL_INPUT);
+    GpioPinFuncSet (52, GPIO_FSEL_INPUT);
+    GpioPinFuncSet (53, GPIO_FSEL_INPUT);
+
+    if (PcdGet32 (PcdSdIsArasan)) {
+      DEBUG ((DEBUG_INFO, "Routing SD to Arasan\n"));
+      Gpio48Group = GPIO_FSEL_ALT3;
+      /*
+       * Route SDIO to SdHost.
+       */
+      Gpio34Group = GPIO_FSEL_ALT0;
+    } else {
+      DEBUG ((DEBUG_INFO, "Routing SD to SdHost\n"));
+      Gpio48Group = GPIO_FSEL_ALT0;
+      /*
+       * Route SDIO to Arasan.
+       */
+      Gpio34Group = GPIO_FSEL_ALT3;
+    }
+    GpioPinFuncSet (34, Gpio34Group);
+    GpioPinFuncSet (35, Gpio34Group);
+    GpioPinFuncSet (36, Gpio34Group);
+    GpioPinFuncSet (37, Gpio34Group);
+    GpioPinFuncSet (38, Gpio34Group);
+    GpioPinFuncSet (39, Gpio34Group);
+    GpioPinFuncSet (48, Gpio48Group);
+    GpioPinFuncSet (49, Gpio48Group);
+    GpioPinFuncSet (50, Gpio48Group);
+    GpioPinFuncSet (51, Gpio48Group);
+    GpioPinFuncSet (52, Gpio48Group);
+    GpioPinFuncSet (53, Gpio48Group);
+
+  } else if (ModelFamily == 4) {
     /*
-     * Route SDIO to Arasan.
+     * Pi 4: either Arasan or eMMC2 goes to SD card.
      */
-    Gpio34Group = GPIO_FSEL_ALT3;
+    if (PcdGet32 (PcdSdIsArasan)) {
+      /*
+       * WiFi disabled.
+       */
+      GpioPinFuncSet (34, GPIO_FSEL_INPUT);
+      GpioPinFuncSet (35, GPIO_FSEL_INPUT);
+      GpioPinFuncSet (36, GPIO_FSEL_INPUT);
+      GpioPinFuncSet (37, GPIO_FSEL_INPUT);
+      GpioPinFuncSet (38, GPIO_FSEL_INPUT);
+      GpioPinFuncSet (39, GPIO_FSEL_INPUT);
+      /*
+       * SD card pins go to Arasan.
+       */
+      MmioWrite32((GPIO_BASE_ADDRESS + 0xD0),
+                  MmioRead32(GPIO_BASE_ADDRESS + 0xD0) | 0x2);
+    } else {
+      /*
+       * SD card pins back to eMMC2.
+       */
+      MmioWrite32((GPIO_BASE_ADDRESS + 0xD0),
+                  MmioRead32(GPIO_BASE_ADDRESS + 0xD0) & ~0x2);
+      /*
+       * WiFi back to Arasan.
+       */
+      GpioPinFuncSet (34, GPIO_FSEL_ALT3);
+      GpioPinFuncSet (35, GPIO_FSEL_ALT3);
+      GpioPinFuncSet (36, GPIO_FSEL_ALT3);
+      GpioPinFuncSet (37, GPIO_FSEL_ALT3);
+      GpioPinFuncSet (38, GPIO_FSEL_ALT3);
+      GpioPinFuncSet (39, GPIO_FSEL_ALT3);
+    }
+  } else {
+    DEBUG ((DEBUG_ERROR, "Model Family %d not supported...\n", ModelFamily));
   }
-  GpioPinFuncSet (34, Gpio34Group);
-  GpioPinFuncSet (35, Gpio34Group);
-  GpioPinFuncSet (36, Gpio34Group);
-  GpioPinFuncSet (37, Gpio34Group);
-  GpioPinFuncSet (38, Gpio34Group);
-  GpioPinFuncSet (39, Gpio34Group);
-  GpioPinFuncSet (48, Gpio48Group);
-  GpioPinFuncSet (49, Gpio48Group);
-  GpioPinFuncSet (50, Gpio48Group);
-  GpioPinFuncSet (51, Gpio48Group);
-  GpioPinFuncSet (52, Gpio48Group);
-  GpioPinFuncSet (53, Gpio48Group);
 
   /*
    * JTAG pin    JTAG sig    GPIO      Mode    Header pin
-- 
2.21.0.windows.1

Re: [edk2-platforms][PATCH v2 0/3] Further RPi4 support groundwork

[Ard Biesheuvel]

On Mon, 2 Dec 2019 at 17:16, Pete Batard <pete@akeo.ie> wrote:
>
> Changes from v1 (https://edk2.groups.io/g/devel/message/51334):
> - Moved RNG related changes into their own series
> - Fix a whitespace issue in the last patch
>
> Andrei Warkentin (2):
>   Platform/RPi/MmcDxe: Factorize SCR call and clean up MMC init
>   Platform/RPi/MmcDxe: Improve MMC driver stability
>
> Samer El-Haj-Mahmoud (1):
>   Platform/RPi: Set SD routing according to model
>

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Pushed as d6b7f67e62da..0e6e3fc4af67

Thanks,



>  Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c      | 137 ++++++++++++++------
>  Platform/RaspberryPi/Drivers/MmcDxe/Mmc.c               |   7 +-
>  Platform/RaspberryPi/Drivers/MmcDxe/MmcIdentification.c | 105 +++++++++------
>  3 files changed, 164 insertions(+), 85 deletions(-)
>
> --
> 2.21.0.windows.1
>