[PATCH 0/9] Platform/RaspberryPi: Utilize SPI flash for EFI variables

[PATCH 0/9] Platform/RaspberryPi: Utilize SPI flash for EFI variables

[Jeremy Linton]

The RPi4 has a SPI flash with unused capacity. This set detects if
that capacity is sufficient for a UEFI variable store and utilizes
it as such. This fixes a long list of problems, and along the way likely
also fixes a random boot failure caused by the FaultTolerantWriteDxe
garbage collecting, and erasing the flash volume header which is being
used to return information about the underlying variable storage capacity.

This set was dependent on an earlier, mostly ignored set of changes to
move the GPIO/etc devices into their own SSDT and disable them. Because
of that, the two sets have been merged.

Why is that? Because the SPI flash is mux'ed with the PWM used to play
audio out the 3.5mm audio jack on this device. This causes a long list
of problems we must try and avoid, starting with the fact that the pins
need to be controlled by the uefi runtime service. The other problem is
obviously that any time a variable is updated, if the user is utilizing
the 3.5mm audio they will hear clicks and pops. Turns out that behavior
isn't unique to this patch set because the low level boot/etc exhibits this
when running in a TFA+uboot/edk2 environment. A fairly small tweak to TFA
fixes the majority of this, and the remaining runtime problems caused
by this patch actually are very slight and generally not noticeable unless
one goes looking for them. OTOH, we revert to the earlier non persisted
variable store if the firmware is running in a DT only mode, or the
user enables the ACPI GPIO block.


Jeremy Linton (9):
  Platform/RaspberryPi: Cleanup menu visibility
  Platform/RaspberryPi: Give the user control over the XHCI mailbox
  Platform/RaspberryPi: Move GPIO/SPI/I2C to SSDT
  Platform/RaspberryPi: Add menu item to enable/disable GPIO
  Platform/RaspberryPi: Add constants for controlling SPI
  Platform/RaspberryPi: Add mailbox cmd to control audio amp
  Platform/RaspberryPi: Add SPI/GPIO to memory map
  Platform/RaspberryPi: Allow pin function selection at runtime
  Platform/RaspberryPi: Add SPI flash variable store.

 Platform/RaspberryPi/AcpiTables/AcpiTables.inf     |   1 +
 Platform/RaspberryPi/AcpiTables/Dsdt.asl           |   7 -
 Platform/RaspberryPi/AcpiTables/GpuDevs.asl        | 125 ----
 Platform/RaspberryPi/AcpiTables/SsdtGpio.asl       | 157 +++++
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c |  47 ++
 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf    |   2 +
 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni |  10 +
 .../RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr |  36 +-
 Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c       |  10 +-
 Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.inf     |   1 +
 .../Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c        |  62 +-
 .../Drivers/VarBlockServiceDxe/FvbInfo.c           |   8 +-
 .../Drivers/VarBlockServiceDxe/VarBlockService.c   | 654 ++++++++++++++++++++-
 .../Drivers/VarBlockServiceDxe/VarBlockService.h   |  10 +
 .../VarBlockServiceDxe/VarBlockServiceDxe.c        |  38 +-
 .../VarBlockServiceDxe/VarBlockServiceDxe.inf      |   6 +
 Platform/RaspberryPi/Include/ConfigVars.h          |   4 +
 .../RaspberryPi/Include/IndustryStandard/RpiMbox.h |   1 +
 .../RaspberryPi/Include/Protocol/RpiFirmware.h     |   7 +
 Platform/RaspberryPi/RPi3/RPi3.dsc                 |  12 +
 Platform/RaspberryPi/RPi4/RPi4.dsc                 |  14 +
 Platform/RaspberryPi/RaspberryPi.dec               |   2 +
 .../Bcm283x/Include/IndustryStandard/Bcm2836.h     |  34 ++
 Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h |   6 +
 Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c |  16 +-
 25 files changed, 1107 insertions(+), 163 deletions(-)
 create mode 100644 Platform/RaspberryPi/AcpiTables/SsdtGpio.asl

-- 
2.13.7

[PATCH 1/9] 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 df45d7b1fe..830c72eac6 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),
@@ -208,7 +208,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),
@@ -220,7 +220,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.13.7

[PATCH 2/9] 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>
---
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c      | 10 ++++++++++
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf    |  1 +
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni |  5 +++++
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 15 +++++++++++++++
 Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c            | 10 +++++++---
 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, 53 insertions(+), 3 deletions(-)

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
index 6b69934ef3..8979b3da4c 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -297,6 +297,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 e6e22ad82e..ff182e831d 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
@@ -95,6 +95,7 @@
   gRaspberryPiTokenSpaceGuid.PcdFanTemp
   gRaspberryPiTokenSpaceGuid.PcdUartInUse
   gRaspberryPiTokenSpaceGuid.PcdXhciPci
+  gRaspberryPiTokenSpaceGuid.PcdXhciReload
 
 [Depex]
   gPcdProtocolGuid AND gRaspberryPiFirmwareProtocolGuid
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
index e25bb83cd3..e40b0914f8 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
@@ -63,6 +63,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 830c72eac6..61171cf658 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,
@@ -229,6 +234,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 a4816d4295..a808b1bf8c 100644
--- a/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
+++ b/Platform/RaspberryPi/Drivers/FdtDxe/FdtDxe.c
@@ -352,8 +352,8 @@ SyncPcie (
   }
 
   // non translated 32-bit DMA window with a limit of 0xc0000000
-  DmaRanges[0] = cpu_to_fdt32 (0x02000000); 
-  DmaRanges[1] = cpu_to_fdt32 (0x00000000); 
+  DmaRanges[0] = cpu_to_fdt32 (0x02000000);
+  DmaRanges[1] = cpu_to_fdt32 (0x00000000);
   DmaRanges[2] = cpu_to_fdt32 (0x00000000);
   DmaRanges[3] = cpu_to_fdt32 (0x00000000);
   DmaRanges[4] = cpu_to_fdt32 (0x00000000);
@@ -362,7 +362,7 @@ SyncPcie (
 
   DEBUG ((DEBUG_INFO, "%a: Updating PCIe dma-ranges\n",  __FUNCTION__));
 
-  /* 
+  /*
    * 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.
@@ -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 6ab5d1ae6d..9b00327002 100644
--- a/Platform/RaspberryPi/RPi3/RPi3.dsc
+++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
@@ -526,6 +526,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 44ed60ab2f..6de4407749 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -544,6 +544,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 797be59274..c50ebdcf77 100644
--- a/Platform/RaspberryPi/RaspberryPi.dec
+++ b/Platform/RaspberryPi/RaspberryPi.dec
@@ -72,3 +72,4 @@
   gRaspberryPiTokenSpaceGuid.PcdMmcEnableDma|0|UINT32|0x0000001F
   gRaspberryPiTokenSpaceGuid.PcdUartInUse|1|UINT32|0x00000021
   gRaspberryPiTokenSpaceGuid.PcdXhciPci|0|UINT32|0x00000022
+  gRaspberryPiTokenSpaceGuid.PcdXhciReload|0|UINT32|0x00000023
-- 
2.13.7

[PATCH 3/9] Platform/RaspberryPi: Move GPIO/SPI/I2C to SSDT

[Jeremy Linton]

The UEFI firmware uses the GPIO port for the fan and
real soon now the runtime SPI variable store. As such
we need to be able to either isolate those devices from
the OS or we risk clashing with OS's that reconfigure
the GPIO pins. Ideally we would just rip this out
and use _DSM() or just individual device power
on/off methods to adjust the GPIO pins when needed.

For now, lets leave it since windows at least knows
about it. In the future we will decide whether the
firmware is controlling something (SPI!) based on
whether the user has enabled the GPIO block.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 Platform/RaspberryPi/AcpiTables/AcpiTables.inf     |   1 +
 Platform/RaspberryPi/AcpiTables/Dsdt.asl           |   7 -
 Platform/RaspberryPi/AcpiTables/GpuDevs.asl        | 125 ----------------
 Platform/RaspberryPi/AcpiTables/SsdtGpio.asl       | 157 +++++++++++++++++++++
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c |   6 +
 5 files changed, 164 insertions(+), 132 deletions(-)
 create mode 100644 Platform/RaspberryPi/AcpiTables/SsdtGpio.asl

diff --git a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
index 1170d8b112..9c7041cda9 100644
--- a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
+++ b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
@@ -41,6 +41,7 @@
   SsdtThermal.asl
   Xhci.asl
   Pci.asl
+  SsdtGpio.asl
 
 [Packages]
   ArmPkg/ArmPkg.dec
diff --git a/Platform/RaspberryPi/AcpiTables/Dsdt.asl b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
index 124372f880..358d2b3f54 100644
--- a/Platform/RaspberryPi/AcpiTables/Dsdt.asl
+++ b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
@@ -21,13 +21,6 @@
 
 #include "AcpiTables.h"
 
-#define BCM_ALT0 0x4
-#define BCM_ALT1 0x5
-#define BCM_ALT2 0x6
-#define BCM_ALT3 0x7
-#define BCM_ALT4 0x3
-#define BCM_ALT5 0x2
-
 //
 // The ASL compiler does not support argument arithmetic in functions
 // like QWordMemory (). So we need to instantiate dummy qword regions
diff --git a/Platform/RaspberryPi/AcpiTables/GpuDevs.asl b/Platform/RaspberryPi/AcpiTables/GpuDevs.asl
index c69e565795..f41f2670b3 100644
--- a/Platform/RaspberryPi/AcpiTables/GpuDevs.asl
+++ b/Platform/RaspberryPi/AcpiTables/GpuDevs.asl
@@ -395,56 +395,6 @@ Device (VCSM)
   }
 }
 
-// Description: GPIO
-Device (GPI0)
-{
-  Name (_HID, "BCM2845")
-  Name (_CID, "BCM2845")
-  Name (_UID, 0x0)
-  Name (_CCA, 0x0)
-  Method (_STA)
-  {
-    Return(0xf)
-  }
-  Name (RBUF, ResourceTemplate ()
-  {
-    MEMORY32FIXED (ReadWrite, 0, GPIO_LENGTH, RMEM)
-    Interrupt (ResourceConsumer, Level, ActiveHigh, Shared)
-    {
-      BCM2386_GPIO_INTERRUPT0, BCM2386_GPIO_INTERRUPT1,
-      BCM2386_GPIO_INTERRUPT2, BCM2386_GPIO_INTERRUPT3
-    }
-  })
-  Method (_CRS, 0x0, Serialized)
-  {
-    MEMORY32SETBASE (RBUF, RMEM, RBAS, GPIO_OFFSET)
-    Return (^RBUF)
-  }
-}
-
-// Description: I2C
-Device (I2C1)
-{
-  Name (_HID, "BCM2841")
-  Name (_CID, "BCM2841")
-  Name (_UID, 0x1)
-  Name (_CCA, 0x0)
-  Method (_STA)
-  {
-    Return(0xf)
-  }
-  Name (RBUF, ResourceTemplate ()
-  {
-    MEMORY32FIXED (ReadWrite, 0, BCM2836_I2C1_LENGTH, RMEM)
-    Interrupt (ResourceConsumer, Level, ActiveHigh, Shared) { BCM2836_I2C1_INTERRUPT }
-    PinFunction (Exclusive, PullUp, BCM_ALT0, "\\_SB.GDV0.GPI0", 0, ResourceConsumer, , ) { 2, 3 }
-  })
-  Method (_CRS, 0x0, Serialized)
-  {
-    MEMORY32SETBASE (RBUF, RMEM, RBAS, BCM2836_I2C1_OFFSET)
-    Return (^RBUF)
-  }
-}
 
 // I2C2 is the HDMI DDC connection
 Device (I2C2)
@@ -470,81 +420,6 @@ Device (I2C2)
   }
 }
 
-// SPI
-Device (SPI0)
-{
-  Name (_HID, "BCM2838")
-  Name (_CID, "BCM2838")
-  Name (_UID, 0x0)
-  Name (_CCA, 0x0)
-  Method (_STA)
-  {
-    Return (0xf)
-  }
-  Name (RBUF, ResourceTemplate ()
-  {
-    MEMORY32FIXED (ReadWrite, 0, BCM2836_SPI0_LENGTH, RMEM)
-    Interrupt (ResourceConsumer, Level, ActiveHigh, Shared) { BCM2836_SPI0_INTERRUPT }
-    PinFunction (Exclusive, PullDown, BCM_ALT0, "\\_SB.GDV0.GPI0", 0, ResourceConsumer, , ) { 9, 10, 11 } // MISO, MOSI, SCLK
-    PinFunction (Exclusive, PullUp, BCM_ALT0, "\\_SB.GDV0.GPI0", 0, ResourceConsumer, , ) { 8 } // CE0
-    PinFunction (Exclusive, PullUp, BCM_ALT0, "\\_SB.GDV0.GPI0", 0, ResourceConsumer, , ) { 7 } // CE1
-  })
-
-  Method (_CRS, 0x0, Serialized)
-  {
-    MEMORY32SETBASE (RBUF, RMEM, RBAS, BCM2836_SPI0_OFFSET)
-    Return (^RBUF)
-  }
-}
-
-Device (SPI1)
-{
-  Name (_HID, "BCM2839")
-  Name (_CID, "BCM2839")
-  Name (_UID, 0x1)
-  Name (_CCA, 0x0)
-  Name (_DEP, Package() { \_SB.GDV0.RPIQ })
-  Method (_STA)
-  {
-    Return (0xf)
-  }
-  Name (RBUF, ResourceTemplate ()
-  {
-    MEMORY32FIXED (ReadWrite, 0, BCM2836_SPI1_LENGTH, RMEM)
-    Interrupt (ResourceConsumer, Level, ActiveHigh, Shared,) { BCM2836_SPI1_INTERRUPT }
-    PinFunction (Exclusive, PullDown, BCM_ALT4, "\\_SB.GDV0.GPI0", 0, ResourceConsumer, , ) { 19, 20, 21 } // MISO, MOSI, SCLK
-    PinFunction (Exclusive, PullDown, BCM_ALT4, "\\_SB.GDV0.GPI0", 0, ResourceConsumer, , ) { 16 } // CE2
-  })
-
-  Method (_CRS, 0x0, Serialized)
-  {
-    MEMORY32SETBASE (RBUF, RMEM, RBAS, BCM2836_SPI1_OFFSET)
-    Return (^RBUF)
-  }
-}
-
-// SPI2 has no pins on GPIO header
-// Device (SPI2)
-// {
-//   Name (_HID, "BCM2839")
-//   Name (_CID, "BCM2839")
-//   Name (_UID, 0x2)
-//   Name (_CCA, 0x0)
-//   Name (_DEP, Package() { \_SB.GDV0.RPIQ })
-//   Method (_STA)
-//   {
-//     Return (0xf)     // Disabled
-//   }
-//   Method (_CRS, 0x0, Serialized)
-//   {
-//     Name (RBUF, ResourceTemplate ()
-//     {
-//       MEMORY32FIXED (ReadWrite, BCM2836_SPI2_BASE_ADDRESS, BCM2836_SPI2_LENGTH, RMEM)
-//       Interrupt (ResourceConsumer, Level, ActiveHigh, Shared,) { BCM2836_SPI2_INTERRUPT }
-//     })
-//     Return (RBUF)
-//   }
-// }
 
 // PWM Driver
 Device (PWM0)
diff --git a/Platform/RaspberryPi/AcpiTables/SsdtGpio.asl b/Platform/RaspberryPi/AcpiTables/SsdtGpio.asl
new file mode 100644
index 0000000000..b01392e427
--- /dev/null
+++ b/Platform/RaspberryPi/AcpiTables/SsdtGpio.asl
@@ -0,0 +1,157 @@
+/** @file
+ *
+ *  Secondary System Description Table (SSDT) for the GPIO port
+ *
+ *  Copyright (c) 2021, Arm Ltd. All rights reserved.
+ *
+ *  SPDX-License-Identifier: BSD-2-Clause-Patent
+ *
+ **/
+
+#include <IndustryStandard/Acpi.h>
+#include <IndustryStandard/Bcm2711.h>
+#include <IndustryStandard/Bcm2836.h>
+#include <IndustryStandard/Bcm2836Gpio.h>
+
+#include "AcpiTables.h"
+
+#define BCM_ALT0 0x4
+#define BCM_ALT1 0x5
+#define BCM_ALT2 0x6
+#define BCM_ALT3 0x7
+#define BCM_ALT4 0x3
+#define BCM_ALT5 0x2
+
+DefinitionBlock (__FILE__, "SSDT", 5, "RPIFDN", "RPI3GPIO", 2)
+{
+  External (\_SB_.GDV0, DeviceObj)
+  External (\_SB_.GDV0.RPIQ, DeviceObj)
+  Scope (\_SB_.GDV0)
+  {
+    // Description: GPIO
+    Device (GPI0)
+    {
+      Name (_HID, "BCM2845")
+      Name (_CID, "BCM2845")
+      Name (_UID, 0x0)
+      Name (_CCA, 0x0)
+      Method (_STA)
+      {
+        Return(0xf)
+      }
+      Name (RBUF, ResourceTemplate ()
+      {
+        MEMORY32FIXED (ReadWrite, 0, GPIO_LENGTH, RMEM)
+        Interrupt (ResourceConsumer, Level, ActiveHigh, Shared)
+        {
+          BCM2386_GPIO_INTERRUPT0, BCM2386_GPIO_INTERRUPT1,
+          BCM2386_GPIO_INTERRUPT2, BCM2386_GPIO_INTERRUPT3
+        }
+      })
+      Method (_CRS, 0x0, Serialized)
+      {
+        MEMORY32SETBASE (RBUF, RMEM, RBAS, GPIO_OFFSET)
+        Return (^RBUF)
+      }
+    }
+
+    // SPI
+    Device (SPI0)
+    {
+      Name (_HID, "BCM2838")
+      Name (_CID, "BCM2838")
+      Name (_UID, 0x0)
+      Name (_CCA, 0x0)
+      Method (_STA)
+      {
+        Return (0xf)
+      }
+      Name (RBUF, ResourceTemplate ()
+      {
+        MEMORY32FIXED (ReadWrite, 0, BCM2836_SPI0_LENGTH, RMEM)
+        Interrupt (ResourceConsumer, Level, ActiveHigh, Shared) { BCM2836_SPI0_INTERRUPT }
+        PinFunction (Exclusive, PullDown, BCM_ALT0, "\\_SB.GDV0.GPI0", 0, ResourceConsumer, , ) { 9, 10, 11 } // MISO, MOSI, SCLK
+        PinFunction (Exclusive, PullUp, BCM_ALT0, "\\_SB.GDV0.GPI0", 0, ResourceConsumer, , ) { 8 } // CE0
+        PinFunction (Exclusive, PullUp, BCM_ALT0, "\\_SB.GDV0.GPI0", 0, ResourceConsumer, , ) { 7 } // CE1
+      })
+
+      Method (_CRS, 0x0, Serialized)
+      {
+        MEMORY32SETBASE (RBUF, RMEM, RBAS, BCM2836_SPI0_OFFSET)
+        Return (^RBUF)
+      }
+    }
+
+    Device (SPI1)
+    {
+      Name (_HID, "BCM2839")
+      Name (_CID, "BCM2839")
+      Name (_UID, 0x1)
+      Name (_CCA, 0x0)
+      Name (_DEP, Package() { \_SB.GDV0.RPIQ })
+      Method (_STA)
+      {
+        Return (0xf)
+      }
+      Name (RBUF, ResourceTemplate ()
+      {
+        MEMORY32FIXED (ReadWrite, 0, BCM2836_SPI1_LENGTH, RMEM)
+        Interrupt (ResourceConsumer, Level, ActiveHigh, Shared,) { BCM2836_SPI1_INTERRUPT }
+        PinFunction (Exclusive, PullDown, BCM_ALT4, "\\_SB_.GDV0.GPI0", 0, ResourceConsumer, , ) { 19, 20, 21 } // MISO, MOSI, SCLK
+        PinFunction (Exclusive, PullDown, BCM_ALT4, "\\_SB_.GDV0.GPI0", 0, ResourceConsumer, , ) { 16 } // CE2
+      })
+
+      Method (_CRS, 0x0, Serialized)
+      {
+        MEMORY32SETBASE (RBUF, RMEM, RBAS, BCM2836_SPI1_OFFSET)
+        Return (^RBUF)
+      }
+    }
+
+  // SPI2 has no pins on GPIO header
+  // Device (SPI2)
+  // {
+  //   Name (_HID, "BCM2839")
+  //   Name (_CID, "BCM2839")
+  //   Name (_UID, 0x2)
+  //   Name (_CCA, 0x0)
+  //   Name (_DEP, Package() { \_SB.GDV0.RPIQ })
+  //   Method (_STA)
+  //   {
+  //     Return (0xf)     // Disabled
+  //   }
+  //   Method (_CRS, 0x0, Serialized)
+  //   {
+  //     Name (RBUF, ResourceTemplate ()
+  //     {
+  //       MEMORY32FIXED (ReadWrite, BCM2836_SPI2_BASE_ADDRESS, BCM2836_SPI2_LENGTH, RMEM)
+  //       Interrupt (ResourceConsumer, Level, ActiveHigh, Shared,) { BCM2836_SPI2_INTERRUPT }
+  //     })
+  //     Return (RBUF)
+  //   }
+  // }
+
+    Device (I2C1)
+    {
+      Name (_HID, "BCM2841")
+      Name (_CID, "BCM2841")
+      Name (_UID, 0x1)
+      Name (_CCA, 0x0)
+      Method (_STA)
+      {
+        Return(0xf)
+      }
+      Name (RBUF, ResourceTemplate ()
+      {
+        MEMORY32FIXED (ReadWrite, 0, BCM2836_I2C1_LENGTH, RMEM)
+        Interrupt (ResourceConsumer, Level, ActiveHigh, Shared) { BCM2836_I2C1_INTERRUPT }
+        PinFunction (Exclusive, PullUp, BCM_ALT0, "\\_SB_.GDV0.GPI0", 0, ResourceConsumer, , ) { 2, 3 }
+      })
+      Method (_CRS, 0x0, Serialized)
+      {
+        MEMORY32SETBASE (RBUF, RMEM, RBAS, BCM2836_I2C1_OFFSET)
+        Return (^RBUF)
+      }
+    }
+  }
+}
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
index 8979b3da4c..f32d75ad34 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -838,6 +838,12 @@ STATIC CONST NAMESPACE_TABLES SdtTables[] = {
     NULL
   },
 #endif
+  {
+    SIGNATURE_64 ('R', 'P', 'I', '3', 'G', 'P', 'I', 'O'),
+    0,
+    0,
+    NULL
+  },
   { // DSDT
     SIGNATURE_64 ('R', 'P', 'I', 0, 0, 0, 0, 0),
     0,
-- 
2.13.7

[PATCH 4/9] Platform/RaspberryPi: Add menu item to enable/disable GPIO

[Jeremy Linton]

Now that the GPIO devices are in their own SSDT lets add
a menu item for the rpi4 to enable/disable it. For the
rpi3 the SSDT is always exported.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c      | 17 ++++++++++++++++-
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf    |  1 +
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni |  5 +++++
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 15 +++++++++++++++
 Platform/RaspberryPi/Include/ConfigVars.h               |  4 ++++
 Platform/RaspberryPi/RPi3/RPi3.dsc                      |  6 ++++++
 Platform/RaspberryPi/RPi4/RPi4.dsc                      |  7 +++++++
 Platform/RaspberryPi/RaspberryPi.dec                    |  1 +
 8 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
index f32d75ad34..3d058cb40d 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -308,12 +308,27 @@ SetupVariables (
       }
 
     }
+
+    Size = sizeof (UINT32);
+    Status = gRT->GetVariable (L"EnableGpio",
+                               &gConfigDxeFormSetGuid,
+                               NULL, &Size, &Var32);
+    if (EFI_ERROR (Status)) {
+      Status = PcdSet32S (PcdEnableGpio, PcdGet32 (PcdEnableGpio));
+      ASSERT_EFI_ERROR (Status);
+    }
+
   } else {
     /*
      * Disable PCIe and XHCI
      */
     Status = PcdSet32S (PcdXhciPci, 0);
     ASSERT_EFI_ERROR (Status);
+    /*
+     * Enable GPIO
+     */
+    Status = PcdSet32S (PcdEnableGpio, 1);
+    ASSERT_EFI_ERROR (Status);
   }
 
   Size = sizeof (AssetTagVar);
@@ -840,7 +855,7 @@ STATIC CONST NAMESPACE_TABLES SdtTables[] = {
 #endif
   {
     SIGNATURE_64 ('R', 'P', 'I', '3', 'G', 'P', 'I', 'O'),
-    0,
+    PcdToken(PcdEnableGpio),
     0,
     NULL
   },
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
index ff182e831d..1cba4a2a22 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
@@ -96,6 +96,7 @@
   gRaspberryPiTokenSpaceGuid.PcdUartInUse
   gRaspberryPiTokenSpaceGuid.PcdXhciPci
   gRaspberryPiTokenSpaceGuid.PcdXhciReload
+  gRaspberryPiTokenSpaceGuid.PcdEnableGpio
 
 [Depex]
   gPcdProtocolGuid AND gRaspberryPiFirmwareProtocolGuid
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
index e40b0914f8..a036fe9edb 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
@@ -68,6 +68,11 @@
 #string STR_ADVANCED_XHCIRELOAD_DISABLE #language en-US "Disabled"
 #string STR_ADVANCED_XHCIRELOAD_RELOAD  #language en-US "Reload"
 
+#string STR_ADVANCED_ENABLEGPIO_PROMPT  #language en-US "Export GPIO devices to OS"
+#string STR_ADVANCED_ENABLEGPIO_HELP    #language en-US "OS can see the GPIO device and some low level SPI and I2C interfaces"
+#string STR_ADVANCED_ENABLEGPIO_DISABLE #language en-US "Disabled"
+#string STR_ADVANCED_ENABLEGPIO_ENABLE  #language en-US "Enable"
+
 #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 61171cf658..24324bb81c 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
@@ -66,6 +66,11 @@ formset
       name  = XhciReload,
       guid  = CONFIGDXE_FORM_SET_GUID;
 
+    efivarstore ADVANCED_ENABLEGPIO_VARSTORE_DATA,
+      attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
+      name  = EnableGpio,
+      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,
@@ -245,6 +250,16 @@ formset
             endoneof;
           endif;
         endif;
+
+        grayoutif ideqval SystemTableMode.Mode == SYSTEM_TABLE_MODE_DT;
+          oneof varid = EnableGpio.Value,
+            prompt      = STRING_TOKEN(STR_ADVANCED_ENABLEGPIO_PROMPT),
+            help        = STRING_TOKEN(STR_ADVANCED_ENABLEGPIO_HELP),
+            flags       = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED,
+            option text = STRING_TOKEN(STR_ADVANCED_ENABLEGPIO_DISABLE), value = 0, flags = DEFAULT;
+            option text = STRING_TOKEN(STR_ADVANCED_ENABLEGPIO_ENABLE), value = 1, flags = 0;
+          endoneof;
+        endif;
 #endif
         string varid = AssetTag.AssetTag,
             prompt  = STRING_TOKEN(STR_ADVANCED_ASSET_TAG_PROMPT),
diff --git a/Platform/RaspberryPi/Include/ConfigVars.h b/Platform/RaspberryPi/Include/ConfigVars.h
index a5b32b5284..43a39891d4 100644
--- a/Platform/RaspberryPi/Include/ConfigVars.h
+++ b/Platform/RaspberryPi/Include/ConfigVars.h
@@ -81,6 +81,10 @@ typedef struct {
 } ADVANCED_XHCIPCI_VARSTORE_DATA;
 
 typedef struct {
+  UINT32 Value;
+} ADVANCED_ENABLEGPIO_VARSTORE_DATA;
+
+typedef struct {
 #define SYSTEM_TABLE_MODE_ACPI 0
 #define SYSTEM_TABLE_MODE_BOTH 1
 #define SYSTEM_TABLE_MODE_DT   2
diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc
index 9b00327002..9db4d93735 100644
--- a/Platform/RaspberryPi/RPi3/RPi3.dsc
+++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
@@ -532,6 +532,12 @@
   #
   gRaspberryPiTokenSpaceGuid.PcdXhciReload|L"XhciReload"|gConfigDxeFormSetGuid|0x0|0
 
+  # Export GPIO block to OS
+  #
+  # 1  - Yes (for legacy reasons)
+  #
+  gRaspberryPiTokenSpaceGuid.PcdEnableGpio|L"EnableGpio"|gConfigDxeFormSetGuid|0x0|1
+
   #
   # Common UEFI ones.
   #
diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
index 6de4407749..c918af6dab 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -551,6 +551,13 @@
   #
   gRaspberryPiTokenSpaceGuid.PcdXhciReload|L"XhciReload"|gConfigDxeFormSetGuid|0x0|0
 
+  # Export GPIO block to OS
+  #
+  # 0  - No
+  # 1  - Yes
+  #
+  gRaspberryPiTokenSpaceGuid.PcdEnableGpio|L"EnableGpio"|gConfigDxeFormSetGuid|0x0|0
+
   #
   # Common UEFI ones.
   #
diff --git a/Platform/RaspberryPi/RaspberryPi.dec b/Platform/RaspberryPi/RaspberryPi.dec
index c50ebdcf77..97709f9b94 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.PcdXhciReload|0|UINT32|0x00000023
+  gRaspberryPiTokenSpaceGuid.PcdEnableGpio|0|UINT32|0x00000024
-- 
2.13.7

[PATCH 5/9] Platform/RaspberryPi: Add constants for controlling SPI

[Jeremy Linton]

Add the #defines needed to access the SPI interface
documented in the BCM2711 Peripheral guide chapter 8.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 .../Bcm283x/Include/IndustryStandard/Bcm2836.h     | 34 ++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h b/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h
index a930c64af3..55a446a86c 100644
--- a/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h
+++ b/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h
@@ -109,6 +109,40 @@
 #define BCM2836_SPI2_LENGTH                                 0x00000040
 #define BCM2836_SPI2_BASE_ADDRESS                           (BCM2836_SOC_REGISTERS + BCM2836_SPI2_OFFSET)
 
+/* SPI register offsets */
+#define BCM2835_SPI_CS                                      0x00
+#define BCM2835_SPI_FIFO                                    0x04
+#define BCM2835_SPI_CLK                                     0x08
+#define BCM2835_SPI_DLEN                                    0x0c
+#define BCM2835_SPI_LTOH                                    0x10
+#define BCM2835_SPI_DC                                      0x14
+
+/* Bitfields in CS */
+#define BCM2835_SPI_CS_LEN_LONG                             0x02000000
+#define BCM2835_SPI_CS_DMA_LEN                              0x01000000
+#define BCM2835_SPI_CS_CSPOL2                               0x00800000
+#define BCM2835_SPI_CS_CSPOL1                               0x00400000
+#define BCM2835_SPI_CS_CSPOL0                               0x00200000
+#define BCM2835_SPI_CS_RXF                                  0x00100000
+#define BCM2835_SPI_CS_RXR                                  0x00080000
+#define BCM2835_SPI_CS_TXD                                  0x00040000
+#define BCM2835_SPI_CS_RXD                                  0x00020000
+#define BCM2835_SPI_CS_DONE                                 0x00010000
+#define BCM2835_SPI_CS_LEN                                  0x00002000
+#define BCM2835_SPI_CS_REN                                  0x00001000
+#define BCM2835_SPI_CS_ADCS                                 0x00000800
+#define BCM2835_SPI_CS_INTR                                 0x00000400
+#define BCM2835_SPI_CS_INTD                                 0x00000200
+#define BCM2835_SPI_CS_DMAEN                                0x00000100
+#define BCM2835_SPI_CS_TA                                   0x00000080
+#define BCM2835_SPI_CS_CSPOL                                0x00000040
+#define BCM2835_SPI_CS_CLEAR_RX                             0x00000020
+#define BCM2835_SPI_CS_CLEAR_TX                             0x00000010
+#define BCM2835_SPI_CS_CPOL                                 0x00000008
+#define BCM2835_SPI_CS_CPHA                                 0x00000004
+#define BCM2835_SPI_CS_CS_10                                0x00000002
+#define BCM2835_SPI_CS_CS_01                                0x00000001
+
 /* dma constants */
 #define BCM2836_DMA0_OFFSET                                 0x00007000
 #define BCM2836_DMA0_BASE_ADDRESS                           (BCM2836_SOC_REGISTERS + BCM2836_DMA0_OFFSET)
-- 
2.13.7

[PATCH 6/9] Platform/RaspberryPi: Add mailbox cmd to control audio amp

[Jeremy Linton]

The lower level firmware can enable/disable a LDO audio
amp, which allows us to mute/unmute audio output while
the firmware is running.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 .../Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c        | 62 +++++++++++++++++++++-
 .../RaspberryPi/Include/IndustryStandard/RpiMbox.h |  1 +
 .../RaspberryPi/Include/Protocol/RpiFirmware.h     |  7 +++
 3 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
index d38ffbc7d3..03c1e1708f 100644
--- a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
+++ b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.c
@@ -1532,6 +1532,65 @@ RpiFirmwareNotifyGpioSetCfg (
   return Status;
 }
 
+#pragma pack()
+typedef struct {
+  UINT32                       State;
+} RPI_FW_SET_LDO_REG_TAG;
+
+typedef struct {
+  RPI_FW_BUFFER_HEAD           BufferHead;
+  RPI_FW_TAG_HEAD              TagHead;
+  RPI_FW_SET_LDO_REG_TAG       TagBody;
+  UINT32                       EndTag;
+} RPI_FW_SET_LDO_REG_CMD;
+#pragma pack()
+
+
+STATIC
+EFI_STATUS
+EFIAPI
+RpiFirmwareSetLdoRegState (
+  IN UINTN State
+  )
+{
+  RPI_FW_SET_LDO_REG_CMD       *Cmd;
+  EFI_STATUS                   Status;
+  UINT32                       Result;
+
+  if (!AcquireSpinLockOrFail (&mMailboxLock)) {
+    DEBUG ((DEBUG_ERROR, "%a: failed to acquire spinlock\n", __FUNCTION__));
+    return EFI_DEVICE_ERROR;
+  }
+
+  DEBUG ((DEBUG_ERROR, "%a: set LDO\n", __FUNCTION__));
+  Cmd = mDmaBuffer;
+  ZeroMem (Cmd, sizeof (*Cmd));
+
+  Cmd->BufferHead.BufferSize  = sizeof (*Cmd);
+  Cmd->BufferHead.Response    = 0;
+  Cmd->TagHead.TagId          = RPI_MBOX_SET_LDO_REGULATOR;
+  Cmd->TagHead.TagSize        = sizeof (Cmd->TagBody);
+  Cmd->TagBody.State          = State;
+
+  Cmd->TagHead.TagValueSize   = 0;
+  Cmd->EndTag                 = 0;
+
+  Status = MailboxTransaction (Cmd->BufferHead.BufferSize, RPI_MBOX_VC_CHANNEL, &Result);
+
+  DEBUG ((DEBUG_ERROR, "%a: set LDO Done\n", __FUNCTION__));
+  if (EFI_ERROR (Status) ||
+      Cmd->BufferHead.Response != RPI_MBOX_RESP_SUCCESS) {
+    DEBUG ((DEBUG_ERROR,
+      "%a: mailbox  transaction error: Status == %r, Response == 0x%x\n",
+      __FUNCTION__, Status, Cmd->BufferHead.Response));
+  }
+
+  ReleaseSpinLock (&mMailboxLock);
+
+  return Status;
+}
+
+
 STATIC RASPBERRY_PI_FIRMWARE_PROTOCOL mRpiFirmwareProtocol = {
   RpiFirmwareSetPowerState,
   RpiFirmwareGetMacAddress,
@@ -1557,7 +1616,8 @@ STATIC RASPBERRY_PI_FIRMWARE_PROTOCOL mRpiFirmwareProtocol = {
   RpiFirmwareNotifyXhciReset,
   RpiFirmwareGetCurrentClockState,
   RpiFirmwareSetClockState,
-  RpiFirmwareNotifyGpioSetCfg
+  RpiFirmwareNotifyGpioSetCfg,
+  RpiFirmwareSetLdoRegState
 };
 
 /**
diff --git a/Platform/RaspberryPi/Include/IndustryStandard/RpiMbox.h b/Platform/RaspberryPi/Include/IndustryStandard/RpiMbox.h
index 551c2b82e5..f36aaafaf8 100644
--- a/Platform/RaspberryPi/Include/IndustryStandard/RpiMbox.h
+++ b/Platform/RaspberryPi/Include/IndustryStandard/RpiMbox.h
@@ -92,6 +92,7 @@
 #define RPI_MBOX_NOTIFY_REBOOT                                0x00030048
 #define RPI_MBOX_GET_POE_HAT_VAL                              0x00030049
 #define RPI_MBOX_SET_POE_HAT_VAL                              0x00030050
+#define RPI_MBOX_SET_LDO_REGULATOR                            0x00030056
 #define RPI_MBOX_NOTIFY_XHCI_RESET                            0x00030058
 
 #define RPI_MBOX_SET_CLOCK_STATE                              0x00038001
diff --git a/Platform/RaspberryPi/Include/Protocol/RpiFirmware.h b/Platform/RaspberryPi/Include/Protocol/RpiFirmware.h
index c48bb6e434..175894e37a 100644
--- a/Platform/RaspberryPi/Include/Protocol/RpiFirmware.h
+++ b/Platform/RaspberryPi/Include/Protocol/RpiFirmware.h
@@ -171,6 +171,12 @@ EFI_STATUS
   UINTN State
   );
 
+typedef
+EFI_STATUS
+(EFIAPI *SET_LDO_REGULATOR) (
+  UINTN State
+  );
+
 typedef struct {
   SET_POWER_STATE        SetPowerState;
   GET_MAC_ADDRESS        GetMacAddress;
@@ -197,6 +203,7 @@ typedef struct {
   GET_CLOCK_STATE        GetClockState;
   SET_CLOCK_STATE        SetClockState;
   GPIO_SET_CFG           SetGpioConfig;
+  SET_LDO_REGULATOR      SetLdoRegState;
 } RASPBERRY_PI_FIRMWARE_PROTOCOL;
 
 extern EFI_GUID gRaspberryPiFirmwareProtocolGuid;
-- 
2.13.7

[PATCH 7/9] Platform/RaspberryPi: Add SPI/GPIO to memory map

[Jeremy Linton]

A large reason for using the SPI flash on this platform is that
it can be updated without OS interference at rutime. In order for
that to happen we need both the SPI, as well as the GPIO
which is used to change the pinmux from the PWM device to SPI added
to the UEFI memory map as being used by the runtime service.

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

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
index 3d058cb40d..ccdeddf3c0 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -503,6 +503,22 @@ ApplyVariables (
     DEBUG ((DEBUG_INFO, "Current CPU speed is %u MHz\n", Rate / FREQ_1_MHZ));
   }
 
+  if (mModelFamily == 4) {
+    Status = gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo, BCM2836_SPI0_BASE_ADDRESS,
+                                  SIZE_4KB, EFI_MEMORY_UC | EFI_MEMORY_RUNTIME);
+    ASSERT_EFI_ERROR (Status);
+    Status = gDS->SetMemorySpaceAttributes (BCM2836_SPI0_BASE_ADDRESS,
+                                            SIZE_4KB, EFI_MEMORY_UC|EFI_MEMORY_RUNTIME);
+
+    Status = gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo, GPIO_BASE_ADDRESS,
+                                  SIZE_4KB, EFI_MEMORY_UC | EFI_MEMORY_RUNTIME);
+    ASSERT_EFI_ERROR (Status);
+    Status = gDS->SetMemorySpaceAttributes (GPIO_BASE_ADDRESS,
+                                            SIZE_4KB, EFI_MEMORY_UC|EFI_MEMORY_RUNTIME);
+
+    ASSERT_EFI_ERROR (Status);
+  }
+
   if (mModelFamily >= 4 && PcdGet32 (PcdRamMoreThan3GB) != 0 &&
       PcdGet32 (PcdRamLimitTo3GB) == 0) {
     UINT64 SystemMemorySize;
-- 
2.13.7

[PATCH 8/9] Platform/RaspberryPi: Allow pin function selection at runtime

[Jeremy Linton]

Update GpioLib slightly so that we can change the GPIO pin
muxing at runtime. For the moment only the GpioPinFuncGet/Set()
routines are used at runtime, and only by the Variable service.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h |  6 ++++++
 Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c | 16 ++++++++++++++--
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h b/Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h
index 1f7d2204e0..79765be4fb 100644
--- a/Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h
+++ b/Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h
@@ -45,4 +45,10 @@ GpioSetPull (
   IN  UINTN   Pud
 );
 
+VOID
+GpioSetupRuntime (
+  VOID
+);
+
+
 #endif /* __GPIO_LIB__ */
diff --git a/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c b/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c
index eaf53e5369..fc1f928e6b 100644
--- a/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c
+++ b/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c
@@ -15,10 +15,22 @@
 #include <Library/DebugLib.h>
 #include <Library/IoLib.h>
 #include <Library/GpioLib.h>
+#include <Library/UefiRuntimeLib.h>
 #include <Library/TimerLib.h>
 #include <IndustryStandard/Bcm2836.h>
 #include <IndustryStandard/Bcm2836Gpio.h>
 
+
+STATIC EFI_PHYSICAL_ADDRESS GpioGfpSel0 = GPIO_GPFSEL0;
+
+VOID
+GpioSetupRuntime (
+  VOID
+  )
+{
+  EfiConvertPointer (0x0, (VOID**)&GpioGfpSel0);
+}
+
 STATIC
 VOID
 GpioFSELModify (
@@ -30,7 +42,7 @@ GpioFSELModify (
   UINT32 Val;
   EFI_PHYSICAL_ADDRESS Reg;
 
-  Reg = RegIndex * sizeof (UINT32) + GPIO_GPFSEL0;
+  Reg = RegIndex * sizeof (UINT32) + GpioGfpSel0;
 
   ASSERT (Reg <= GPIO_GPFSEL5);
   ASSERT ((~ModifyMask & FunctionMask) == 0);
@@ -77,7 +89,7 @@ GpioPinFuncGet (
 
   RegIndex = Pin / 10;
   SelIndex = Pin % 10;
-  Reg = RegIndex * sizeof (UINT32) + GPIO_GPFSEL0;
+  Reg = RegIndex * sizeof (UINT32) + GpioGfpSel0;
 
   Val = MmioRead32 (Reg);
   Val >>= SelIndex * GPIO_FSEL_BITS_PER_PIN;
-- 
2.13.7

Re: [edk2-devel] [PATCH 0/9] Platform/RaspberryPi: Utilize SPI flash for EFI variables

[Ard Biesheuvel]

On Thu, 2 Dec 2021 at 17:52, Jeremy Linton <jeremy.linton@arm.com> wrote:
>
> The RPi4 has a SPI flash with unused capacity. This set detects if
> that capacity is sufficient for a UEFI variable store and utilizes
> it as such. This fixes a long list of problems, and along the way likely
> also fixes a random boot failure caused by the FaultTolerantWriteDxe
> garbage collecting, and erasing the flash volume header which is being
> used to return information about the underlying variable storage capacity.
>
> This set was dependent on an earlier, mostly ignored set of changes to
> move the GPIO/etc devices into their own SSDT and disable them. Because
> of that, the two sets have been merged.
>
> Why is that? Because the SPI flash is mux'ed with the PWM used to play
> audio out the 3.5mm audio jack on this device. This causes a long list
> of problems we must try and avoid, starting with the fact that the pins
> need to be controlled by the uefi runtime service. The other problem is
> obviously that any time a variable is updated, if the user is utilizing
> the 3.5mm audio they will hear clicks and pops. Turns out that behavior
> isn't unique to this patch set because the low level boot/etc exhibits this
> when running in a TFA+uboot/edk2 environment. A fairly small tweak to TFA
> fixes the majority of this, and the remaining runtime problems caused
> by this patch actually are very slight and generally not noticeable unless
> one goes looking for them. OTOH, we revert to the earlier non persisted
> variable store if the firmware is running in a DT only mode, or the
> user enables the ACPI GPIO block.
>
>
> Jeremy Linton (9):
>   Platform/RaspberryPi: Cleanup menu visibility
>   Platform/RaspberryPi: Give the user control over the XHCI mailbox
>   Platform/RaspberryPi: Move GPIO/SPI/I2C to SSDT
>   Platform/RaspberryPi: Add menu item to enable/disable GPIO
>   Platform/RaspberryPi: Add constants for controlling SPI
>   Platform/RaspberryPi: Add mailbox cmd to control audio amp
>   Platform/RaspberryPi: Add SPI/GPIO to memory map
>   Platform/RaspberryPi: Allow pin function selection at runtime
>   Platform/RaspberryPi: Add SPI flash variable store.
>

Very nice!

I am having trouble applying these patches, though. Could you please
resend without the random whitespace changes?

Re: [edk2-devel] [PATCH 0/9] Platform/RaspberryPi: Utilize SPI flash for EFI variables

[Ard Biesheuvel]

On Thu, 2 Dec 2021 at 18:03, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Thu, 2 Dec 2021 at 17:52, Jeremy Linton <jeremy.linton@arm.com> wrote:
> >
> > The RPi4 has a SPI flash with unused capacity. This set detects if
> > that capacity is sufficient for a UEFI variable store and utilizes
> > it as such. This fixes a long list of problems, and along the way likely
> > also fixes a random boot failure caused by the FaultTolerantWriteDxe
> > garbage collecting, and erasing the flash volume header which is being
> > used to return information about the underlying variable storage capacity.
> >
> > This set was dependent on an earlier, mostly ignored set of changes to
> > move the GPIO/etc devices into their own SSDT and disable them. Because
> > of that, the two sets have been merged.
> >
> > Why is that? Because the SPI flash is mux'ed with the PWM used to play
> > audio out the 3.5mm audio jack on this device. This causes a long list
> > of problems we must try and avoid, starting with the fact that the pins
> > need to be controlled by the uefi runtime service. The other problem is
> > obviously that any time a variable is updated, if the user is utilizing
> > the 3.5mm audio they will hear clicks and pops. Turns out that behavior
> > isn't unique to this patch set because the low level boot/etc exhibits this
> > when running in a TFA+uboot/edk2 environment. A fairly small tweak to TFA
> > fixes the majority of this, and the remaining runtime problems caused
> > by this patch actually are very slight and generally not noticeable unless
> > one goes looking for them. OTOH, we revert to the earlier non persisted
> > variable store if the firmware is running in a DT only mode, or the
> > user enables the ACPI GPIO block.
> >
> >
> > Jeremy Linton (9):
> >   Platform/RaspberryPi: Cleanup menu visibility
> >   Platform/RaspberryPi: Give the user control over the XHCI mailbox
> >   Platform/RaspberryPi: Move GPIO/SPI/I2C to SSDT
> >   Platform/RaspberryPi: Add menu item to enable/disable GPIO
> >   Platform/RaspberryPi: Add constants for controlling SPI
> >   Platform/RaspberryPi: Add mailbox cmd to control audio amp
> >   Platform/RaspberryPi: Add SPI/GPIO to memory map
> >   Platform/RaspberryPi: Allow pin function selection at runtime
> >   Platform/RaspberryPi: Add SPI flash variable store.
> >
>
> Very nice!
>
> I am having trouble applying these patches, though. Could you please
> resend without the random whitespace changes?

It appears only 2/9 is affected, the remaining ones apply cleanly,
with the exception of 9/9, which seems to be missing entirely. Could
you please resend that one as well?

Re: [edk2-devel] [PATCH 0/9] Platform/RaspberryPi: Utilize SPI flash for EFI variables

[Jeremy Linton]

On 12/2/21 11:09, Ard Biesheuvel wrote:
> On Thu, 2 Dec 2021 at 18:03, Ard Biesheuvel <ardb@kernel.org> wrote:
>>
>> On Thu, 2 Dec 2021 at 17:52, Jeremy Linton <jeremy.linton@arm.com> wrote:
>>>
>>> The RPi4 has a SPI flash with unused capacity. This set detects if
>>> that capacity is sufficient for a UEFI variable store and utilizes
>>> it as such. This fixes a long list of problems, and along the way likely
>>> also fixes a random boot failure caused by the FaultTolerantWriteDxe
>>> garbage collecting, and erasing the flash volume header which is being
>>> used to return information about the underlying variable storage capacity.
>>>
>>> This set was dependent on an earlier, mostly ignored set of changes to
>>> move the GPIO/etc devices into their own SSDT and disable them. Because
>>> of that, the two sets have been merged.
>>>
>>> Why is that? Because the SPI flash is mux'ed with the PWM used to play
>>> audio out the 3.5mm audio jack on this device. This causes a long list
>>> of problems we must try and avoid, starting with the fact that the pins
>>> need to be controlled by the uefi runtime service. The other problem is
>>> obviously that any time a variable is updated, if the user is utilizing
>>> the 3.5mm audio they will hear clicks and pops. Turns out that behavior
>>> isn't unique to this patch set because the low level boot/etc exhibits this
>>> when running in a TFA+uboot/edk2 environment. A fairly small tweak to TFA
>>> fixes the majority of this, and the remaining runtime problems caused
>>> by this patch actually are very slight and generally not noticeable unless
>>> one goes looking for them. OTOH, we revert to the earlier non persisted
>>> variable store if the firmware is running in a DT only mode, or the
>>> user enables the ACPI GPIO block.
>>>
>>>
>>> Jeremy Linton (9):
>>>    Platform/RaspberryPi: Cleanup menu visibility
>>>    Platform/RaspberryPi: Give the user control over the XHCI mailbox
>>>    Platform/RaspberryPi: Move GPIO/SPI/I2C to SSDT
>>>    Platform/RaspberryPi: Add menu item to enable/disable GPIO
>>>    Platform/RaspberryPi: Add constants for controlling SPI
>>>    Platform/RaspberryPi: Add mailbox cmd to control audio amp
>>>    Platform/RaspberryPi: Add SPI/GPIO to memory map
>>>    Platform/RaspberryPi: Allow pin function selection at runtime
>>>    Platform/RaspberryPi: Add SPI flash variable store.
>>>
>>
>> Very nice!
>>
>> I am having trouble applying these patches, though. Could you please
>> resend without the random whitespace changes?
> 
> It appears only 2/9 is affected, the remaining ones apply cleanly,
> with the exception of 9/9, which seems to be missing entirely. Could
> you please resend that one as well?
> 

Yah, so it looks like 2/9 picked up a couple tab/trailing whitespace 
cleanups when I ran it through the edk2 checkpatch right before sending 
it. I can kill those, although, that doesn't really explain why it 
doesn't apply. Both the copies I got look ok, although the groups.io 
version changed the content-type from text/plain/7bit to 
text/plain/quoted-printable, which is disturbing if it decided to quote 
parts of the patch.

9/9 though seems to be my side, because it got rejected due to UTF-8 
whitespace on the last line of the patch not being 7-bit.

My mail process on this ML seems to continue to be a pain, and i'm not 
entirely sure why its special (I post patches elseswhere with a lot less 
trouble).

Re: [edk2-devel] [PATCH 0/9] Platform/RaspberryPi: Utilize SPI flash for EFI variables

[Jeremy Linton]

Hi,


On 12/2/21 11:09, Ard Biesheuvel wrote:
> On Thu, 2 Dec 2021 at 18:03, Ard Biesheuvel <ardb@kernel.org> wrote:
>>
>> On Thu, 2 Dec 2021 at 17:52, Jeremy Linton <jeremy.linton@arm.com> wrote:
>>>
>>> The RPi4 has a SPI flash with unused capacity. This set detects if
>>> that capacity is sufficient for a UEFI variable store and utilizes
>>> it as such. This fixes a long list of problems, and along the way likely
>>> also fixes a random boot failure caused by the FaultTolerantWriteDxe
>>> garbage collecting, and erasing the flash volume header which is being
>>> used to return information about the underlying variable storage capacity.
>>>
>>> This set was dependent on an earlier, mostly ignored set of changes to
>>> move the GPIO/etc devices into their own SSDT and disable them. Because
>>> of that, the two sets have been merged.
>>>
>>> Why is that? Because the SPI flash is mux'ed with the PWM used to play
>>> audio out the 3.5mm audio jack on this device. This causes a long list
>>> of problems we must try and avoid, starting with the fact that the pins
>>> need to be controlled by the uefi runtime service. The other problem is
>>> obviously that any time a variable is updated, if the user is utilizing
>>> the 3.5mm audio they will hear clicks and pops. Turns out that behavior
>>> isn't unique to this patch set because the low level boot/etc exhibits this
>>> when running in a TFA+uboot/edk2 environment. A fairly small tweak to TFA
>>> fixes the majority of this, and the remaining runtime problems caused
>>> by this patch actually are very slight and generally not noticeable unless
>>> one goes looking for them. OTOH, we revert to the earlier non persisted
>>> variable store if the firmware is running in a DT only mode, or the
>>> user enables the ACPI GPIO block.
>>>
>>>
>>> Jeremy Linton (9):
>>>    Platform/RaspberryPi: Cleanup menu visibility
>>>    Platform/RaspberryPi: Give the user control over the XHCI mailbox
>>>    Platform/RaspberryPi: Move GPIO/SPI/I2C to SSDT
>>>    Platform/RaspberryPi: Add menu item to enable/disable GPIO
>>>    Platform/RaspberryPi: Add constants for controlling SPI
>>>    Platform/RaspberryPi: Add mailbox cmd to control audio amp
>>>    Platform/RaspberryPi: Add SPI/GPIO to memory map
>>>    Platform/RaspberryPi: Allow pin function selection at runtime
>>>    Platform/RaspberryPi: Add SPI flash variable store.
>>>
>>
>> Very nice!
>>
>> I am having trouble applying these patches, though. Could you please
>> resend without the random whitespace changes?
> 
> It appears only 2/9 is affected, the remaining ones apply cleanly,
> with the exception of 9/9, which seems to be missing entirely. Could
> you please resend that one as well?
> 

Hi,

So, 2/2 was probably me too, I resent it as well with the same subject 
but of course the email thread id isn't right.

Re: [edk2-devel] [PATCH 0/9] Platform/RaspberryPi: Utilize SPI flash for EFI variables

[Ard Biesheuvel]

On Thu, 2 Dec 2021 at 18:55, Jeremy Linton <jeremy.linton@arm.com> wrote:
>
> Hi,
>
>
> On 12/2/21 11:09, Ard Biesheuvel wrote:
> > On Thu, 2 Dec 2021 at 18:03, Ard Biesheuvel <ardb@kernel.org> wrote:
> >>
> >> On Thu, 2 Dec 2021 at 17:52, Jeremy Linton <jeremy.linton@arm.com> wrote:
> >>>
> >>> The RPi4 has a SPI flash with unused capacity. This set detects if
> >>> that capacity is sufficient for a UEFI variable store and utilizes
> >>> it as such. This fixes a long list of problems, and along the way likely
> >>> also fixes a random boot failure caused by the FaultTolerantWriteDxe
> >>> garbage collecting, and erasing the flash volume header which is being
> >>> used to return information about the underlying variable storage capacity.
> >>>
> >>> This set was dependent on an earlier, mostly ignored set of changes to
> >>> move the GPIO/etc devices into their own SSDT and disable them. Because
> >>> of that, the two sets have been merged.
> >>>
> >>> Why is that? Because the SPI flash is mux'ed with the PWM used to play
> >>> audio out the 3.5mm audio jack on this device. This causes a long list
> >>> of problems we must try and avoid, starting with the fact that the pins
> >>> need to be controlled by the uefi runtime service. The other problem is
> >>> obviously that any time a variable is updated, if the user is utilizing
> >>> the 3.5mm audio they will hear clicks and pops. Turns out that behavior
> >>> isn't unique to this patch set because the low level boot/etc exhibits this
> >>> when running in a TFA+uboot/edk2 environment. A fairly small tweak to TFA
> >>> fixes the majority of this, and the remaining runtime problems caused
> >>> by this patch actually are very slight and generally not noticeable unless
> >>> one goes looking for them. OTOH, we revert to the earlier non persisted
> >>> variable store if the firmware is running in a DT only mode, or the
> >>> user enables the ACPI GPIO block.
> >>>
> >>>
> >>> Jeremy Linton (9):
> >>>    Platform/RaspberryPi: Cleanup menu visibility
> >>>    Platform/RaspberryPi: Give the user control over the XHCI mailbox
> >>>    Platform/RaspberryPi: Move GPIO/SPI/I2C to SSDT
> >>>    Platform/RaspberryPi: Add menu item to enable/disable GPIO
> >>>    Platform/RaspberryPi: Add constants for controlling SPI
> >>>    Platform/RaspberryPi: Add mailbox cmd to control audio amp
> >>>    Platform/RaspberryPi: Add SPI/GPIO to memory map
> >>>    Platform/RaspberryPi: Allow pin function selection at runtime
> >>>    Platform/RaspberryPi: Add SPI flash variable store.
> >>>
> >>
> >> Very nice!
> >>
> >> I am having trouble applying these patches, though. Could you please
> >> resend without the random whitespace changes?
> >
> > It appears only 2/9 is affected, the remaining ones apply cleanly,
> > with the exception of 9/9, which seems to be missing entirely. Could
> > you please resend that one as well?
> >
>
> Hi,
>
> So, 2/2 was probably me too, I resent it as well with the same subject
> but of course the email thread id isn't right.

Thanks

I gave this a spin, and Boot#### variables created by the Debian
installer persisted as expected, so

Tested-by: Ard Biesheuvel <ardb@kernel.org>

Before I merge this, though, could you elaborate on how playing with
the SPI flash like this is not going to brick my RPi4? Also, others,
please chime in as well.

-- 
Ard.

Re: [edk2-devel] [PATCH 0/9] Platform/RaspberryPi: Utilize SPI flash for EFI variables

[Jeremy Linton]

Hi,

On 12/3/21 12:12, Ard Biesheuvel wrote:
> On Thu, 2 Dec 2021 at 18:55, Jeremy Linton <jeremy.linton@arm.com> wrote:
>>
>> Hi,
>>
>>
>> On 12/2/21 11:09, Ard Biesheuvel wrote:
>>> On Thu, 2 Dec 2021 at 18:03, Ard Biesheuvel <ardb@kernel.org> wrote:
>>>>
>>>> On Thu, 2 Dec 2021 at 17:52, Jeremy Linton <jeremy.linton@arm.com> wrote:
>>>>>
>>>>> The RPi4 has a SPI flash with unused capacity. This set detects if
>>>>> that capacity is sufficient for a UEFI variable store and utilizes
>>>>> it as such. This fixes a long list of problems, and along the way likely
>>>>> also fixes a random boot failure caused by the FaultTolerantWriteDxe
>>>>> garbage collecting, and erasing the flash volume header which is being
>>>>> used to return information about the underlying variable storage capacity.
>>>>>
>>>>> This set was dependent on an earlier, mostly ignored set of changes to
>>>>> move the GPIO/etc devices into their own SSDT and disable them. Because
>>>>> of that, the two sets have been merged.
>>>>>
>>>>> Why is that? Because the SPI flash is mux'ed with the PWM used to play
>>>>> audio out the 3.5mm audio jack on this device. This causes a long list
>>>>> of problems we must try and avoid, starting with the fact that the pins
>>>>> need to be controlled by the uefi runtime service. The other problem is
>>>>> obviously that any time a variable is updated, if the user is utilizing
>>>>> the 3.5mm audio they will hear clicks and pops. Turns out that behavior
>>>>> isn't unique to this patch set because the low level boot/etc exhibits this
>>>>> when running in a TFA+uboot/edk2 environment. A fairly small tweak to TFA
>>>>> fixes the majority of this, and the remaining runtime problems caused
>>>>> by this patch actually are very slight and generally not noticeable unless
>>>>> one goes looking for them. OTOH, we revert to the earlier non persisted
>>>>> variable store if the firmware is running in a DT only mode, or the
>>>>> user enables the ACPI GPIO block.
>>>>>
>>>>>
>>>>> Jeremy Linton (9):
>>>>>     Platform/RaspberryPi: Cleanup menu visibility
>>>>>     Platform/RaspberryPi: Give the user control over the XHCI mailbox
>>>>>     Platform/RaspberryPi: Move GPIO/SPI/I2C to SSDT
>>>>>     Platform/RaspberryPi: Add menu item to enable/disable GPIO
>>>>>     Platform/RaspberryPi: Add constants for controlling SPI
>>>>>     Platform/RaspberryPi: Add mailbox cmd to control audio amp
>>>>>     Platform/RaspberryPi: Add SPI/GPIO to memory map
>>>>>     Platform/RaspberryPi: Allow pin function selection at runtime
>>>>>     Platform/RaspberryPi: Add SPI flash variable store.
>>>>>
>>>>
>>>> Very nice!
>>>>
>>>> I am having trouble applying these patches, though. Could you please
>>>> resend without the random whitespace changes?
>>>
>>> It appears only 2/9 is affected, the remaining ones apply cleanly,
>>> with the exception of 9/9, which seems to be missing entirely. Could
>>> you please resend that one as well?
>>>
>>
>> Hi,
>>
>> So, 2/2 was probably me too, I resent it as well with the same subject
>> but of course the email thread id isn't right.
> 
> Thanks
> 
> I gave this a spin, and Boot#### variables created by the Debian
> installer persisted as expected, so
> 
> Tested-by: Ard Biesheuvel <ardb@kernel.org>
> 
> Before I merge this, though, could you elaborate on how playing with
> the SPI flash like this is not going to brick my RPi4? Also, others,
> please chime in as well.
> 


First though, in the constant tweaking of patches, I noticed that 6/9 
"Add mailbox command to control audio amp" should probably have the LDO 
state DEBUG_ERROR's removed/reduced (I just removed them). NBD either 
way I guess.

So, back to how you won't permanently brick your rpi. Bricking it seems 
a lot harder than random SPI corruption, which I managed to achieve a 
few times while developing this set. More than once I corrupted it 
sufficiently to keep the low level bootloader from running. In those 
cases the rpi foundation's 
https://www.raspberrypi.com/news/raspberry-pi-imager-imaging-utility/ 
imaging tool has an option "Bootloader EEPOM configuration", which 
creates an SD image that the SoC will prefer to boot from over the SPI 
flash. That utility erases the entire flash and writes the latest 
bootloader image to it. The whole process takes a few seconds if one 
keeps the recovery disk handy.

So, I think we are good if someone decides to run that utility to 
upgrade their "EEPROM", or we have bugs that corrupt it. My larger worry 
is that we create upgrade problems with the EFI firmware itself, but I 
don't see any evidence of that happening yet, we just need to be careful 
about how we initialize new variables to avoid a situation where the 
user has to use that utility to reset the EFI variable portion of the flash.

The other issue is that the rpi foundation hasn't made any guarantees 
that this space will remain available in the future, which this code 
should deal with as is, by reverting to the previous behavior. If/when 
they do that we can trim some of their fat, or ask them politely to 
create a reduced feature version for us (say by removing nvme boot/etc), 
or simply keep using the older versions until we find legitimate 
problems with them.