public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH edk2-platforms 0/3] enable Secure96 GPIO LEDs on ACPI systems
@ 2019-04-25 12:32 Ard Biesheuvel
  2019-04-25 12:32 ` [PATCH edk2-platforms 1/3] Silicon/SynQuacer: describe 96boards LS connector GPIOs via ACPI Ard Biesheuvel
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Ard Biesheuvel @ 2019-04-25 12:32 UTC (permalink / raw)
  To: devel; +Cc: leif.lindholm, graeme.gregory, masahisa.kojima, Ard Biesheuvel

This series plumbs in the support for describing 96boards mezzanines
via ACPI. For now, only the GPIO LEDs are described: in the future,
I intend to wire up the I2C and SPI parts as well, but these are
currently untested and so not ready for review.

Note that this applies onto

Silicon/SynQuacer: add ACPI description of GPIO controller and power button

which I sent out earlier today.

Ard Biesheuvel (3):
  Silicon/SynQuacer: describe 96boards LS connector GPIOs via ACPI
  Platform/96Boards: add ACPI support to mezzanine/LS connector driver
  Platform/Secure96Dxe: add ACPI description of the GPIO LEDs

 Platform/96Boards/Include/Protocol/Mezzanine.h        |  21 ++++
 Platform/96Boards/LsConnectorDxe/LsConnectorDxe.c     |  35 ++++---
 Platform/96Boards/LsConnectorDxe/LsConnectorDxe.inf   |   1 +
 Platform/96Boards/Secure96Dxe/Secure96.asl            | 103 ++++++++++++++++++++
 Platform/96Boards/Secure96Dxe/Secure96Dxe.c           |  59 ++++++++++-
 Platform/96Boards/Secure96Dxe/Secure96Dxe.inf         |   1 +
 Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf |  14 +++
 Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl       |  20 ++++
 8 files changed, 238 insertions(+), 16 deletions(-)
 create mode 100644 Platform/96Boards/Secure96Dxe/Secure96.asl

-- 
2.20.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH edk2-platforms 1/3] Silicon/SynQuacer: describe 96boards LS connector GPIOs via ACPI
  2019-04-25 12:32 [PATCH edk2-platforms 0/3] enable Secure96 GPIO LEDs on ACPI systems Ard Biesheuvel
@ 2019-04-25 12:32 ` Ard Biesheuvel
  2019-04-25 12:32 ` [PATCH edk2-platforms 2/3] Platform/96Boards: add ACPI support to mezzanine/LS connector driver Ard Biesheuvel
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Ard Biesheuvel @ 2019-04-25 12:32 UTC (permalink / raw)
  To: devel; +Cc: leif.lindholm, graeme.gregory, masahisa.kojima, Ard Biesheuvel

Describe the 96boards LS connector GPIO resources via a new LS96
device object.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf | 14 ++++++++++++++
 Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl       | 20 ++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf b/Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf
index 6fbdf4d67a88..0fef8b9ca05b 100644
--- a/Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf
+++ b/Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf
@@ -40,6 +40,7 @@ [Packages]
   EmbeddedPkg/EmbeddedPkg.dec
   MdePkg/MdePkg.dec
   MdeModulePkg/MdeModulePkg.dec
+  Platform/96Boards/96Boards.dec
   Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/NetsecDxe.dec
   Silicon/Socionext/SynQuacer/SynQuacer.dec
 
@@ -63,3 +64,16 @@ [FixedPcd]
   gSynQuacerTokenSpaceGuid.PcdNetsecEepromBase
   gSynQuacerTokenSpaceGuid.PcdNetsecPhyAddress
   gSynQuacerTokenSpaceGuid.PcdPcie0PresenceDetectGpioPin
+
+  g96BoardsTokenSpaceGuid.PcdGpioPinA
+  g96BoardsTokenSpaceGuid.PcdGpioPinB
+  g96BoardsTokenSpaceGuid.PcdGpioPinC
+  g96BoardsTokenSpaceGuid.PcdGpioPinD
+  g96BoardsTokenSpaceGuid.PcdGpioPinE
+  g96BoardsTokenSpaceGuid.PcdGpioPinF
+  g96BoardsTokenSpaceGuid.PcdGpioPinG
+  g96BoardsTokenSpaceGuid.PcdGpioPinH
+  g96BoardsTokenSpaceGuid.PcdGpioPinI
+  g96BoardsTokenSpaceGuid.PcdGpioPinJ
+  g96BoardsTokenSpaceGuid.PcdGpioPinK
+  g96BoardsTokenSpaceGuid.PcdGpioPinL
diff --git a/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl b/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl
index acb77739ded6..0702edc06f74 100644
--- a/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl
+++ b/Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl
@@ -253,5 +253,25 @@ DefinitionBlock ("DsdtTable.aml", "DSDT", 1, "SNI", "SYNQUACR",
         Return (0xF)
       }
     }
+
+    Device (LS96) {
+      Name (GPIO, ResourceTemplate () {
+        GpioIo (Exclusive, PullDefault, 0, 0, IoRestrictionNone, "\\_SB.GPIO")
+        {
+          FixedPcdGet32 (PcdGpioPinA),
+          FixedPcdGet32 (PcdGpioPinB),
+          FixedPcdGet32 (PcdGpioPinC),
+          FixedPcdGet32 (PcdGpioPinD),
+          FixedPcdGet32 (PcdGpioPinE),
+          FixedPcdGet32 (PcdGpioPinF),
+          FixedPcdGet32 (PcdGpioPinG),
+          FixedPcdGet32 (PcdGpioPinH),
+          FixedPcdGet32 (PcdGpioPinI),
+          FixedPcdGet32 (PcdGpioPinJ),
+          FixedPcdGet32 (PcdGpioPinK),
+          FixedPcdGet32 (PcdGpioPinL),
+        }
+      })
+    }
   } // Scope (_SB)
 }
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH edk2-platforms 2/3] Platform/96Boards: add ACPI support to mezzanine/LS connector driver
  2019-04-25 12:32 [PATCH edk2-platforms 0/3] enable Secure96 GPIO LEDs on ACPI systems Ard Biesheuvel
  2019-04-25 12:32 ` [PATCH edk2-platforms 1/3] Silicon/SynQuacer: describe 96boards LS connector GPIOs via ACPI Ard Biesheuvel
@ 2019-04-25 12:32 ` Ard Biesheuvel
  2019-04-25 12:32 ` [PATCH edk2-platforms 3/3] Platform/Secure96Dxe: add ACPI description of the GPIO LEDs Ard Biesheuvel
  2019-04-26 11:10 ` [PATCH edk2-platforms 0/3] enable Secure96 GPIO LEDs on ACPI systems Leif Lindholm
  3 siblings, 0 replies; 9+ messages in thread
From: Ard Biesheuvel @ 2019-04-25 12:32 UTC (permalink / raw)
  To: devel; +Cc: leif.lindholm, graeme.gregory, masahisa.kojima, Ard Biesheuvel

Make the LS connector mezzanine support code ACPI aware, and invoke
the appropriate hook in the driver code to install a SSDT instead of
a DT overlay when running on a system that is booting in ACPI mode.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Platform/96Boards/Include/Protocol/Mezzanine.h      | 21 ++++++++++++
 Platform/96Boards/LsConnectorDxe/LsConnectorDxe.c   | 35 ++++++++++++++------
 Platform/96Boards/LsConnectorDxe/LsConnectorDxe.inf |  1 +
 3 files changed, 46 insertions(+), 11 deletions(-)

diff --git a/Platform/96Boards/Include/Protocol/Mezzanine.h b/Platform/96Boards/Include/Protocol/Mezzanine.h
index 9847649d2ac3..97f43e9a920f 100644
--- a/Platform/96Boards/Include/Protocol/Mezzanine.h
+++ b/Platform/96Boards/Include/Protocol/Mezzanine.h
@@ -16,6 +16,7 @@
 #define _96BOARDS_MEZZANINE_H_
 
 #include <Pi/PiI2c.h>
+#include <Protocol/AcpiTable.h>
 #include <Protocol/SpiConfiguration.h>
 
 #define MEZZANINE_PROTOCOL_GUID \
@@ -39,12 +40,32 @@ EFI_STATUS
   IN  OUT VOID                  *Dtb
   );
 
+/**
+  Install the mezzanine's SSDT table
+
+  @param[in]      This      Pointer to the MEZZANINE_PROTOCOL instance.
+  @param[in]      Dtb       Pointer to the device tree blob
+
+  @return   EFI_SUCCESS     Operation succeeded.
+  @return   other           An error has occurred.
+**/
+typedef
+EFI_STATUS
+(EFIAPI *INSTALL_SSDT_TABLE) (
+  IN      MEZZANINE_PROTOCOL        *This,
+  IN      EFI_ACPI_TABLE_PROTOCOL   *AcpiProtocol
+  );
+
 struct _MEZZANINE_PROTOCOL {
   //
   // Get the device tree overlay for this mezzanine board
   //
   APPLY_DEVICE_TREE_OVERLAY   ApplyDeviceTreeOverlay;
   //
+  // Install the mezzanine's SSDT table
+  //
+  INSTALL_SSDT_TABLE          InstallSsdtTable;
+  //
   // The number of devices on LS connector I2C bus #0
   //
   UINT32                      I2c0NumDevices;
diff --git a/Platform/96Boards/LsConnectorDxe/LsConnectorDxe.c b/Platform/96Boards/LsConnectorDxe/LsConnectorDxe.c
index f19d95635056..27044c5da699 100644
--- a/Platform/96Boards/LsConnectorDxe/LsConnectorDxe.c
+++ b/Platform/96Boards/LsConnectorDxe/LsConnectorDxe.c
@@ -21,6 +21,7 @@
 #include <Library/UefiBootServicesTableLib.h>
 #include <Library/UefiLib.h>
 #include <Library/UefiRuntimeServicesTableLib.h>
+#include <Protocol/AcpiTable.h>
 #include <Protocol/LsConnector.h>
 #include <Protocol/Mezzanine.h>
 
@@ -97,7 +98,7 @@ InstallHiiPages (
 STATIC
 VOID
 EFIAPI
-ApplyDeviceTreeOverlay (
+PublishOsDescription (
   EFI_EVENT           Event,
   VOID                *Context
   )
@@ -105,11 +106,30 @@ ApplyDeviceTreeOverlay (
   VOID                    *Dtb;
   MEZZANINE_PROTOCOL      *Mezzanine;
   EFI_STATUS              Status;
+  EFI_ACPI_TABLE_PROTOCOL *AcpiProtocol;
+
+  Status = gBS->LocateProtocol (&g96BoardsMezzanineProtocolGuid, NULL,
+                  (VOID **)&Mezzanine);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_INFO, "%a: no mezzanine driver active\n", __FUNCTION__));
+    return;
+  }
+
+  Status = gBS->LocateProtocol (&gEfiAcpiTableProtocolGuid, NULL,
+                  (VOID **)&AcpiProtocol);
+  if (!EFI_ERROR (Status)) {
+    Status = Mezzanine->InstallSsdtTable (Mezzanine, AcpiProtocol);
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_WARN, "%a: failed to install SSDT table - %r\n",
+        __FUNCTION__, Status));
+    }
+    return;
+  }
 
   //
   // Find the DTB in the configuration table array. If it isn't there, just
-  // bail without an error: we may be running on an ACPI platform even if
-  // this driver does not support it [yet].
+  // bail without an error: the system may be able to proceed even without
+  // ACPI or DT description, so it isn't up to us to complain about this.
   //
   Status = EfiGetSystemConfigurationTable (&gFdtTableGuid, &Dtb);
   if (Status == EFI_NOT_FOUND) {
@@ -117,13 +137,6 @@ ApplyDeviceTreeOverlay (
   }
   ASSERT_EFI_ERROR (Status);
 
-  Status = gBS->LocateProtocol (&g96BoardsMezzanineProtocolGuid, NULL,
-                  (VOID **)&Mezzanine);
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_INFO, "%a: no mezzanine driver active\n", __FUNCTION__));
-    return;
-  }
-
   Status = Mezzanine->ApplyDeviceTreeOverlay (Mezzanine, Dtb);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_WARN, "%a: failed to apply DT overlay - %r\n", __FUNCTION__,
@@ -211,7 +224,7 @@ EntryPoint (
   Status = gBS->CreateEventEx (
                   EVT_NOTIFY_SIGNAL,
                   TPL_NOTIFY,
-                  ApplyDeviceTreeOverlay,
+                  PublishOsDescription,
                   NULL,
                   &gEfiEndOfDxeEventGroupGuid,
                   &EndOfDxeEvent);
diff --git a/Platform/96Boards/LsConnectorDxe/LsConnectorDxe.inf b/Platform/96Boards/LsConnectorDxe/LsConnectorDxe.inf
index 1bf528ceaa84..20b9637c1923 100644
--- a/Platform/96Boards/LsConnectorDxe/LsConnectorDxe.inf
+++ b/Platform/96Boards/LsConnectorDxe/LsConnectorDxe.inf
@@ -46,6 +46,7 @@ [LibraryClasses]
 [Protocols]
   g96BoardsLsConnectorProtocolGuid     ## PRODUCES
   g96BoardsMezzanineProtocolGuid       ## CONSUMES
+  gEfiAcpiTableProtocolGuid            ## SOMETIMES_CONSUMES
 
 [Guids]
   gEfiEndOfDxeEventGroupGuid
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH edk2-platforms 3/3] Platform/Secure96Dxe: add ACPI description of the GPIO LEDs
  2019-04-25 12:32 [PATCH edk2-platforms 0/3] enable Secure96 GPIO LEDs on ACPI systems Ard Biesheuvel
  2019-04-25 12:32 ` [PATCH edk2-platforms 1/3] Silicon/SynQuacer: describe 96boards LS connector GPIOs via ACPI Ard Biesheuvel
  2019-04-25 12:32 ` [PATCH edk2-platforms 2/3] Platform/96Boards: add ACPI support to mezzanine/LS connector driver Ard Biesheuvel
@ 2019-04-25 12:32 ` Ard Biesheuvel
  2019-04-26 11:16   ` Leif Lindholm
  2019-04-26 11:10 ` [PATCH edk2-platforms 0/3] enable Secure96 GPIO LEDs on ACPI systems Leif Lindholm
  3 siblings, 1 reply; 9+ messages in thread
From: Ard Biesheuvel @ 2019-04-25 12:32 UTC (permalink / raw)
  To: devel; +Cc: leif.lindholm, graeme.gregory, masahisa.kojima, Ard Biesheuvel

Wire up the new 96boards mezzanine SSDT loading support, and use it
to describe the four GPIO LEDs on the Secure96 mezzanine board.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Platform/96Boards/Secure96Dxe/Secure96.asl    | 103 ++++++++++++++++++++
 Platform/96Boards/Secure96Dxe/Secure96Dxe.c   |  59 ++++++++++-
 Platform/96Boards/Secure96Dxe/Secure96Dxe.inf |   1 +
 3 files changed, 158 insertions(+), 5 deletions(-)

diff --git a/Platform/96Boards/Secure96Dxe/Secure96.asl b/Platform/96Boards/Secure96Dxe/Secure96.asl
new file mode 100644
index 000000000000..bb9dac462a33
--- /dev/null
+++ b/Platform/96Boards/Secure96Dxe/Secure96.asl
@@ -0,0 +1,103 @@
+/** @file
+ * Copyright (c) 2019, Linaro Limited. All rights reserved.
+ *
+ * This program and the accompanying materials are licensed and made
+ * available under the terms and conditions of the BSD License which
+ * accompanies this distribution.  The full text of the license may be
+ * found at http://opensource.org/licenses/bsd-license.php
+ *
+ * THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
+ * IMPLIED.
+ */
+
+#include "Secure96.h"
+
+DefinitionBlock ("Secure96.aml", "SSDT", 2, "LINARO", "SECURE96", 1)
+{
+    External (\_SB.LS96.GPIO)
+
+    Scope (_SB)
+    {
+        Device (LD96)
+        {
+            Name (_HID, "PRP0001")  // _HID: Hardware ID
+            Name (_UID, 0x00)       // _UID: Unique ID
+            Name (_DSD, Package () {
+                ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+                Package () {
+                    Package () { "compatible", "gpio-leds" },
+                }
+            })
+
+            Method (_CRS)
+            {
+                Return (\_SB.LS96.GPIO)
+            }
+
+            Device (LDU1)
+            {
+                Name (_ADR, 0x1)
+                Name (_DSD, Package () {
+                    ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+                    Package () {
+                        Package () { "label", "secure96-u1" },
+                        Package () { "gpios",
+                            Package () {
+                                ^^LD96, 0, 6, FixedPcdGet32 (PcdGpioPolarity)
+                            },
+                        },
+                    }
+                })
+            }
+
+            Device (LDU2)
+            {
+                Name (_ADR, 0x2)
+                Name (_DSD, Package () {
+                    ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+                    Package () {
+                        Package () { "label", "secure96-u2" },
+                        Package () { "gpios",
+                            Package () {
+                                ^^LD96, 0, 5, FixedPcdGet32 (PcdGpioPolarity)
+                            },
+                        },
+                    }
+                })
+            }
+
+            Device (LDU3)
+            {
+                Name (_ADR, 0x3)
+                Name (_DSD, Package () {
+                    ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+                    Package () {
+                        Package () { "label", "secure96-u3" },
+                        Package () { "gpios",
+                            Package () {
+                                ^^LD96, 0, 8, FixedPcdGet32 (PcdGpioPolarity)
+                            },
+                        },
+                    }
+                })
+            }
+
+            Device (LDU4)
+            {
+                Name (_ADR, 0x4)
+                Name (_DSD, Package () {
+                    ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+                    Package () {
+                        Package () { "label", "secure96-u4" },
+                        Package () { "gpios",
+                            Package () {
+                                ^^LD96, 0, 7, FixedPcdGet32 (PcdGpioPolarity)
+                            },
+                        },
+                    }
+                })
+            }
+        }
+    }
+}
diff --git a/Platform/96Boards/Secure96Dxe/Secure96Dxe.c b/Platform/96Boards/Secure96Dxe/Secure96Dxe.c
index 6c48d7c0b024..68f8ec812b52 100644
--- a/Platform/96Boards/Secure96Dxe/Secure96Dxe.c
+++ b/Platform/96Boards/Secure96Dxe/Secure96Dxe.c
@@ -24,6 +24,8 @@
 
 #include "Secure96.h"
 
+#define SECURE96_SSDT_OEM_TABLE_ID SIGNATURE_64('S','E','C','U','R','E','9','6')
+
 STATIC CONST UINT32 mI2cAtmelSha204aSlaveAddress[] = {
   ATSHA204A_SLAVE_ADDRESS,
 
@@ -148,15 +150,20 @@ ApplyDeviceTreeOverlay (
   UINTN           OverlaySize;
   EFI_STATUS      Status;
   INT32           Err;
+  UINTN           Index;
 
   //
   // Load the raw overlay DTB image from the raw section of this FFS file.
   //
-  Status = GetSectionFromFv (&gEfiCallerIdGuid,
-             EFI_SECTION_RAW, 0, &Overlay, &OverlaySize);
-  ASSERT_EFI_ERROR (Status);
-  if (EFI_ERROR (Status)) {
-    return EFI_NOT_FOUND;
+  for (Index = 0;; Index++) {
+    Status = GetSectionFromFv (&gEfiCallerIdGuid,
+               EFI_SECTION_RAW, Index, &Overlay, &OverlaySize);
+    if (EFI_ERROR (Status)) {
+      return EFI_NOT_FOUND;
+    }
+    if (!fdt_check_header (Overlay)) {
+      break;
+    }
   }
 
   //
@@ -177,8 +184,50 @@ ApplyDeviceTreeOverlay (
   return EFI_SUCCESS;
 }
 
+/**
+  Install the mezzanine's SSDT table
+
+  @param[in]      This      Pointer to the MEZZANINE_PROTOCOL instance.
+  @param[in]      Dtb       Pointer to the device tree blob
+
+  @return   EFI_SUCCESS     Operation succeeded.
+  @return   other           An error has occurred.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+InstallSsdtTable (
+  IN      MEZZANINE_PROTOCOL        *This,
+  IN      EFI_ACPI_TABLE_PROTOCOL   *AcpiProtocol
+  )
+{
+  EFI_ACPI_DESCRIPTION_HEADER   *Ssdt;
+  UINTN                         SsdtSize;
+  EFI_STATUS                    Status;
+  UINTN                         Index;
+  UINTN                         TableKey;
+
+  //
+  // Load SSDT table from the raw section of this FFS file.
+  //
+  for (Index = 0;; Index++) {
+    Status = GetSectionFromFv (&gEfiCallerIdGuid, EFI_SECTION_RAW, Index,
+               (VOID **)&Ssdt, &SsdtSize);
+    if (EFI_ERROR (Status)) {
+      return EFI_NOT_FOUND;
+    }
+    if (SsdtSize >= sizeof (EFI_ACPI_DESCRIPTION_HEADER) &&
+        Ssdt->OemTableId == SECURE96_SSDT_OEM_TABLE_ID) {
+      break;
+    }
+  }
+  return AcpiProtocol->InstallAcpiTable (AcpiProtocol, Ssdt, SsdtSize,
+                         &TableKey);
+}
+
 STATIC MEZZANINE_PROTOCOL mMezzanine = {
   ApplyDeviceTreeOverlay,
+  InstallSsdtTable,
   ARRAY_SIZE (mI2c0Devices),
   0,
   mI2c0Devices,
diff --git a/Platform/96Boards/Secure96Dxe/Secure96Dxe.inf b/Platform/96Boards/Secure96Dxe/Secure96Dxe.inf
index 72dbf1314c15..ce4c8b5f8fa5 100644
--- a/Platform/96Boards/Secure96Dxe/Secure96Dxe.inf
+++ b/Platform/96Boards/Secure96Dxe/Secure96Dxe.inf
@@ -21,6 +21,7 @@ [Defines]
   ENTRY_POINT                    = Secure96DxeEntryPoint
 
 [Sources]
+  Secure96.asl
   Secure96.dts
   Secure96.h
   Secure96Dxe.c
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH edk2-platforms 0/3] enable Secure96 GPIO LEDs on ACPI systems
  2019-04-25 12:32 [PATCH edk2-platforms 0/3] enable Secure96 GPIO LEDs on ACPI systems Ard Biesheuvel
                   ` (2 preceding siblings ...)
  2019-04-25 12:32 ` [PATCH edk2-platforms 3/3] Platform/Secure96Dxe: add ACPI description of the GPIO LEDs Ard Biesheuvel
@ 2019-04-26 11:10 ` Leif Lindholm
  3 siblings, 0 replies; 9+ messages in thread
From: Leif Lindholm @ 2019-04-26 11:10 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: devel, graeme.gregory, masahisa.kojima

On Thu, Apr 25, 2019 at 02:32:51PM +0200, Ard Biesheuvel wrote:
> This series plumbs in the support for describing 96boards mezzanines
> via ACPI. For now, only the GPIO LEDs are described: in the future,
> I intend to wire up the I2C and SPI parts as well, but these are
> currently untested and so not ready for review.
> 
> Note that this applies onto
> 
> Silicon/SynQuacer: add ACPI description of GPIO controller and power button
> 
> which I sent out earlier today.

For the series:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

I do have a separate comment on 3/3, but the series is not gated on that.

> Ard Biesheuvel (3):
>   Silicon/SynQuacer: describe 96boards LS connector GPIOs via ACPI
>   Platform/96Boards: add ACPI support to mezzanine/LS connector driver
>   Platform/Secure96Dxe: add ACPI description of the GPIO LEDs
> 
>  Platform/96Boards/Include/Protocol/Mezzanine.h        |  21 ++++
>  Platform/96Boards/LsConnectorDxe/LsConnectorDxe.c     |  35 ++++---
>  Platform/96Boards/LsConnectorDxe/LsConnectorDxe.inf   |   1 +
>  Platform/96Boards/Secure96Dxe/Secure96.asl            | 103 ++++++++++++++++++++
>  Platform/96Boards/Secure96Dxe/Secure96Dxe.c           |  59 ++++++++++-
>  Platform/96Boards/Secure96Dxe/Secure96Dxe.inf         |   1 +
>  Silicon/Socionext/SynQuacer/AcpiTables/AcpiTables.inf |  14 +++
>  Silicon/Socionext/SynQuacer/AcpiTables/Dsdt.asl       |  20 ++++
>  8 files changed, 238 insertions(+), 16 deletions(-)
>  create mode 100644 Platform/96Boards/Secure96Dxe/Secure96.asl
> 
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH edk2-platforms 3/3] Platform/Secure96Dxe: add ACPI description of the GPIO LEDs
  2019-04-25 12:32 ` [PATCH edk2-platforms 3/3] Platform/Secure96Dxe: add ACPI description of the GPIO LEDs Ard Biesheuvel
@ 2019-04-26 11:16   ` Leif Lindholm
  2019-04-26 11:56     ` Ard Biesheuvel
  0 siblings, 1 reply; 9+ messages in thread
From: Leif Lindholm @ 2019-04-26 11:16 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: devel, graeme.gregory, masahisa.kojima

On Thu, Apr 25, 2019 at 02:32:54PM +0200, Ard Biesheuvel wrote:
> Wire up the new 96boards mezzanine SSDT loading support, and use it
> to describe the four GPIO LEDs on the Secure96 mezzanine board.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  Platform/96Boards/Secure96Dxe/Secure96.asl    | 103 ++++++++++++++++++++
>  Platform/96Boards/Secure96Dxe/Secure96Dxe.c   |  59 ++++++++++-
>  Platform/96Boards/Secure96Dxe/Secure96Dxe.inf |   1 +
>  3 files changed, 158 insertions(+), 5 deletions(-)
> 
> diff --git a/Platform/96Boards/Secure96Dxe/Secure96.asl b/Platform/96Boards/Secure96Dxe/Secure96.asl
> new file mode 100644
> index 000000000000..bb9dac462a33
> --- /dev/null
> +++ b/Platform/96Boards/Secure96Dxe/Secure96.asl
> @@ -0,0 +1,103 @@
> +/** @file
> + * Copyright (c) 2019, Linaro Limited. All rights reserved.
> + *
> + * This program and the accompanying materials are licensed and made
> + * available under the terms and conditions of the BSD License which
> + * accompanies this distribution.  The full text of the license may be
> + * found at http://opensource.org/licenses/bsd-license.php
> + *
> + * THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
> + * IMPLIED.
> + */
> +
> +#include "Secure96.h"
> +
> +DefinitionBlock ("Secure96.aml", "SSDT", 2, "LINARO", "SECURE96", 1)
> +{
> +    External (\_SB.LS96.GPIO)
> +
> +    Scope (_SB)
> +    {
> +        Device (LD96)
> +        {
> +            Name (_HID, "PRP0001")  // _HID: Hardware ID
> +            Name (_UID, 0x00)       // _UID: Unique ID
> +            Name (_DSD, Package () {
> +                ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),

Surely the above stanza is the most repeated bit of boiler plate in
the entire tree? I count 207 instances before this goes in.
Can we stick a #define in some common header for this and reuse?
ACPI_DSD_UUID?

As I said, not required for this patch, but would be worth addressing.

/
    Leif

> +                Package () {
> +                    Package () { "compatible", "gpio-leds" },
> +                }
> +            })
> +
> +            Method (_CRS)
> +            {
> +                Return (\_SB.LS96.GPIO)
> +            }
> +
> +            Device (LDU1)
> +            {
> +                Name (_ADR, 0x1)
> +                Name (_DSD, Package () {
> +                    ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> +                    Package () {
> +                        Package () { "label", "secure96-u1" },
> +                        Package () { "gpios",
> +                            Package () {
> +                                ^^LD96, 0, 6, FixedPcdGet32 (PcdGpioPolarity)
> +                            },
> +                        },
> +                    }
> +                })
> +            }
> +
> +            Device (LDU2)
> +            {
> +                Name (_ADR, 0x2)
> +                Name (_DSD, Package () {
> +                    ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> +                    Package () {
> +                        Package () { "label", "secure96-u2" },
> +                        Package () { "gpios",
> +                            Package () {
> +                                ^^LD96, 0, 5, FixedPcdGet32 (PcdGpioPolarity)
> +                            },
> +                        },
> +                    }
> +                })
> +            }
> +
> +            Device (LDU3)
> +            {
> +                Name (_ADR, 0x3)
> +                Name (_DSD, Package () {
> +                    ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> +                    Package () {
> +                        Package () { "label", "secure96-u3" },
> +                        Package () { "gpios",
> +                            Package () {
> +                                ^^LD96, 0, 8, FixedPcdGet32 (PcdGpioPolarity)
> +                            },
> +                        },
> +                    }
> +                })
> +            }
> +
> +            Device (LDU4)
> +            {
> +                Name (_ADR, 0x4)
> +                Name (_DSD, Package () {
> +                    ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> +                    Package () {
> +                        Package () { "label", "secure96-u4" },
> +                        Package () { "gpios",
> +                            Package () {
> +                                ^^LD96, 0, 7, FixedPcdGet32 (PcdGpioPolarity)
> +                            },
> +                        },
> +                    }
> +                })
> +            }
> +        }
> +    }
> +}
> diff --git a/Platform/96Boards/Secure96Dxe/Secure96Dxe.c b/Platform/96Boards/Secure96Dxe/Secure96Dxe.c
> index 6c48d7c0b024..68f8ec812b52 100644
> --- a/Platform/96Boards/Secure96Dxe/Secure96Dxe.c
> +++ b/Platform/96Boards/Secure96Dxe/Secure96Dxe.c
> @@ -24,6 +24,8 @@
>  
>  #include "Secure96.h"
>  
> +#define SECURE96_SSDT_OEM_TABLE_ID SIGNATURE_64('S','E','C','U','R','E','9','6')
> +
>  STATIC CONST UINT32 mI2cAtmelSha204aSlaveAddress[] = {
>    ATSHA204A_SLAVE_ADDRESS,
>  
> @@ -148,15 +150,20 @@ ApplyDeviceTreeOverlay (
>    UINTN           OverlaySize;
>    EFI_STATUS      Status;
>    INT32           Err;
> +  UINTN           Index;
>  
>    //
>    // Load the raw overlay DTB image from the raw section of this FFS file.
>    //
> -  Status = GetSectionFromFv (&gEfiCallerIdGuid,
> -             EFI_SECTION_RAW, 0, &Overlay, &OverlaySize);
> -  ASSERT_EFI_ERROR (Status);
> -  if (EFI_ERROR (Status)) {
> -    return EFI_NOT_FOUND;
> +  for (Index = 0;; Index++) {
> +    Status = GetSectionFromFv (&gEfiCallerIdGuid,
> +               EFI_SECTION_RAW, Index, &Overlay, &OverlaySize);
> +    if (EFI_ERROR (Status)) {
> +      return EFI_NOT_FOUND;
> +    }
> +    if (!fdt_check_header (Overlay)) {
> +      break;
> +    }
>    }
>  
>    //
> @@ -177,8 +184,50 @@ ApplyDeviceTreeOverlay (
>    return EFI_SUCCESS;
>  }
>  
> +/**
> +  Install the mezzanine's SSDT table
> +
> +  @param[in]      This      Pointer to the MEZZANINE_PROTOCOL instance.
> +  @param[in]      Dtb       Pointer to the device tree blob
> +
> +  @return   EFI_SUCCESS     Operation succeeded.
> +  @return   other           An error has occurred.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +InstallSsdtTable (
> +  IN      MEZZANINE_PROTOCOL        *This,
> +  IN      EFI_ACPI_TABLE_PROTOCOL   *AcpiProtocol
> +  )
> +{
> +  EFI_ACPI_DESCRIPTION_HEADER   *Ssdt;
> +  UINTN                         SsdtSize;
> +  EFI_STATUS                    Status;
> +  UINTN                         Index;
> +  UINTN                         TableKey;
> +
> +  //
> +  // Load SSDT table from the raw section of this FFS file.
> +  //
> +  for (Index = 0;; Index++) {
> +    Status = GetSectionFromFv (&gEfiCallerIdGuid, EFI_SECTION_RAW, Index,
> +               (VOID **)&Ssdt, &SsdtSize);
> +    if (EFI_ERROR (Status)) {
> +      return EFI_NOT_FOUND;
> +    }
> +    if (SsdtSize >= sizeof (EFI_ACPI_DESCRIPTION_HEADER) &&
> +        Ssdt->OemTableId == SECURE96_SSDT_OEM_TABLE_ID) {
> +      break;
> +    }
> +  }
> +  return AcpiProtocol->InstallAcpiTable (AcpiProtocol, Ssdt, SsdtSize,
> +                         &TableKey);
> +}
> +
>  STATIC MEZZANINE_PROTOCOL mMezzanine = {
>    ApplyDeviceTreeOverlay,
> +  InstallSsdtTable,
>    ARRAY_SIZE (mI2c0Devices),
>    0,
>    mI2c0Devices,
> diff --git a/Platform/96Boards/Secure96Dxe/Secure96Dxe.inf b/Platform/96Boards/Secure96Dxe/Secure96Dxe.inf
> index 72dbf1314c15..ce4c8b5f8fa5 100644
> --- a/Platform/96Boards/Secure96Dxe/Secure96Dxe.inf
> +++ b/Platform/96Boards/Secure96Dxe/Secure96Dxe.inf
> @@ -21,6 +21,7 @@ [Defines]
>    ENTRY_POINT                    = Secure96DxeEntryPoint
>  
>  [Sources]
> +  Secure96.asl
>    Secure96.dts
>    Secure96.h
>    Secure96Dxe.c
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH edk2-platforms 3/3] Platform/Secure96Dxe: add ACPI description of the GPIO LEDs
  2019-04-26 11:16   ` Leif Lindholm
@ 2019-04-26 11:56     ` Ard Biesheuvel
  2019-04-26 14:17       ` Leif Lindholm
  0 siblings, 1 reply; 9+ messages in thread
From: Ard Biesheuvel @ 2019-04-26 11:56 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: edk2-devel-groups-io, Graeme Gregory, Masahisa Kojima

On Fri, 26 Apr 2019 at 13:16, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Thu, Apr 25, 2019 at 02:32:54PM +0200, Ard Biesheuvel wrote:
> > Wire up the new 96boards mezzanine SSDT loading support, and use it
> > to describe the four GPIO LEDs on the Secure96 mezzanine board.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> >  Platform/96Boards/Secure96Dxe/Secure96.asl    | 103 ++++++++++++++++++++
> >  Platform/96Boards/Secure96Dxe/Secure96Dxe.c   |  59 ++++++++++-
> >  Platform/96Boards/Secure96Dxe/Secure96Dxe.inf |   1 +
> >  3 files changed, 158 insertions(+), 5 deletions(-)
> >
> > diff --git a/Platform/96Boards/Secure96Dxe/Secure96.asl b/Platform/96Boards/Secure96Dxe/Secure96.asl
> > new file mode 100644
> > index 000000000000..bb9dac462a33
> > --- /dev/null
> > +++ b/Platform/96Boards/Secure96Dxe/Secure96.asl
> > @@ -0,0 +1,103 @@
> > +/** @file
> > + * Copyright (c) 2019, Linaro Limited. All rights reserved.
> > + *
> > + * This program and the accompanying materials are licensed and made
> > + * available under the terms and conditions of the BSD License which
> > + * accompanies this distribution.  The full text of the license may be
> > + * found at http://opensource.org/licenses/bsd-license.php
> > + *
> > + * THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > + * WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
> > + * IMPLIED.
> > + */
> > +
> > +#include "Secure96.h"
> > +
> > +DefinitionBlock ("Secure96.aml", "SSDT", 2, "LINARO", "SECURE96", 1)
> > +{
> > +    External (\_SB.LS96.GPIO)
> > +
> > +    Scope (_SB)
> > +    {
> > +        Device (LD96)
> > +        {
> > +            Name (_HID, "PRP0001")  // _HID: Hardware ID
> > +            Name (_UID, 0x00)       // _UID: Unique ID
> > +            Name (_DSD, Package () {
> > +                ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>
> Surely the above stanza is the most repeated bit of boiler plate in
> the entire tree? I count 207 instances before this goes in.
> Can we stick a #define in some common header for this and reuse?
> ACPI_DSD_UUID?
>
> As I said, not required for this patch, but would be worth addressing.
>

We should add the GUID to MdePkg, given that it occurs in the ACPI spec.


> > +                Package () {
> > +                    Package () { "compatible", "gpio-leds" },
> > +                }
> > +            })
> > +
> > +            Method (_CRS)
> > +            {
> > +                Return (\_SB.LS96.GPIO)
> > +            }
> > +
> > +            Device (LDU1)
> > +            {
> > +                Name (_ADR, 0x1)
> > +                Name (_DSD, Package () {
> > +                    ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> > +                    Package () {
> > +                        Package () { "label", "secure96-u1" },
> > +                        Package () { "gpios",
> > +                            Package () {
> > +                                ^^LD96, 0, 6, FixedPcdGet32 (PcdGpioPolarity)
> > +                            },
> > +                        },
> > +                    }
> > +                })
> > +            }
> > +
> > +            Device (LDU2)
> > +            {
> > +                Name (_ADR, 0x2)
> > +                Name (_DSD, Package () {
> > +                    ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> > +                    Package () {
> > +                        Package () { "label", "secure96-u2" },
> > +                        Package () { "gpios",
> > +                            Package () {
> > +                                ^^LD96, 0, 5, FixedPcdGet32 (PcdGpioPolarity)
> > +                            },
> > +                        },
> > +                    }
> > +                })
> > +            }
> > +
> > +            Device (LDU3)
> > +            {
> > +                Name (_ADR, 0x3)
> > +                Name (_DSD, Package () {
> > +                    ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> > +                    Package () {
> > +                        Package () { "label", "secure96-u3" },
> > +                        Package () { "gpios",
> > +                            Package () {
> > +                                ^^LD96, 0, 8, FixedPcdGet32 (PcdGpioPolarity)
> > +                            },
> > +                        },
> > +                    }
> > +                })
> > +            }
> > +
> > +            Device (LDU4)
> > +            {
> > +                Name (_ADR, 0x4)
> > +                Name (_DSD, Package () {
> > +                    ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> > +                    Package () {
> > +                        Package () { "label", "secure96-u4" },
> > +                        Package () { "gpios",
> > +                            Package () {
> > +                                ^^LD96, 0, 7, FixedPcdGet32 (PcdGpioPolarity)
> > +                            },
> > +                        },
> > +                    }
> > +                })
> > +            }
> > +        }
> > +    }
> > +}
> > diff --git a/Platform/96Boards/Secure96Dxe/Secure96Dxe.c b/Platform/96Boards/Secure96Dxe/Secure96Dxe.c
> > index 6c48d7c0b024..68f8ec812b52 100644
> > --- a/Platform/96Boards/Secure96Dxe/Secure96Dxe.c
> > +++ b/Platform/96Boards/Secure96Dxe/Secure96Dxe.c
> > @@ -24,6 +24,8 @@
> >
> >  #include "Secure96.h"
> >
> > +#define SECURE96_SSDT_OEM_TABLE_ID SIGNATURE_64('S','E','C','U','R','E','9','6')
> > +
> >  STATIC CONST UINT32 mI2cAtmelSha204aSlaveAddress[] = {
> >    ATSHA204A_SLAVE_ADDRESS,
> >
> > @@ -148,15 +150,20 @@ ApplyDeviceTreeOverlay (
> >    UINTN           OverlaySize;
> >    EFI_STATUS      Status;
> >    INT32           Err;
> > +  UINTN           Index;
> >
> >    //
> >    // Load the raw overlay DTB image from the raw section of this FFS file.
> >    //
> > -  Status = GetSectionFromFv (&gEfiCallerIdGuid,
> > -             EFI_SECTION_RAW, 0, &Overlay, &OverlaySize);
> > -  ASSERT_EFI_ERROR (Status);
> > -  if (EFI_ERROR (Status)) {
> > -    return EFI_NOT_FOUND;
> > +  for (Index = 0;; Index++) {
> > +    Status = GetSectionFromFv (&gEfiCallerIdGuid,
> > +               EFI_SECTION_RAW, Index, &Overlay, &OverlaySize);
> > +    if (EFI_ERROR (Status)) {
> > +      return EFI_NOT_FOUND;
> > +    }
> > +    if (!fdt_check_header (Overlay)) {
> > +      break;
> > +    }
> >    }
> >
> >    //
> > @@ -177,8 +184,50 @@ ApplyDeviceTreeOverlay (
> >    return EFI_SUCCESS;
> >  }
> >
> > +/**
> > +  Install the mezzanine's SSDT table
> > +
> > +  @param[in]      This      Pointer to the MEZZANINE_PROTOCOL instance.
> > +  @param[in]      Dtb       Pointer to the device tree blob
> > +
> > +  @return   EFI_SUCCESS     Operation succeeded.
> > +  @return   other           An error has occurred.
> > +**/
> > +STATIC
> > +EFI_STATUS
> > +EFIAPI
> > +InstallSsdtTable (
> > +  IN      MEZZANINE_PROTOCOL        *This,
> > +  IN      EFI_ACPI_TABLE_PROTOCOL   *AcpiProtocol
> > +  )
> > +{
> > +  EFI_ACPI_DESCRIPTION_HEADER   *Ssdt;
> > +  UINTN                         SsdtSize;
> > +  EFI_STATUS                    Status;
> > +  UINTN                         Index;
> > +  UINTN                         TableKey;
> > +
> > +  //
> > +  // Load SSDT table from the raw section of this FFS file.
> > +  //
> > +  for (Index = 0;; Index++) {
> > +    Status = GetSectionFromFv (&gEfiCallerIdGuid, EFI_SECTION_RAW, Index,
> > +               (VOID **)&Ssdt, &SsdtSize);
> > +    if (EFI_ERROR (Status)) {
> > +      return EFI_NOT_FOUND;
> > +    }
> > +    if (SsdtSize >= sizeof (EFI_ACPI_DESCRIPTION_HEADER) &&
> > +        Ssdt->OemTableId == SECURE96_SSDT_OEM_TABLE_ID) {
> > +      break;
> > +    }
> > +  }
> > +  return AcpiProtocol->InstallAcpiTable (AcpiProtocol, Ssdt, SsdtSize,
> > +                         &TableKey);
> > +}
> > +
> >  STATIC MEZZANINE_PROTOCOL mMezzanine = {
> >    ApplyDeviceTreeOverlay,
> > +  InstallSsdtTable,
> >    ARRAY_SIZE (mI2c0Devices),
> >    0,
> >    mI2c0Devices,
> > diff --git a/Platform/96Boards/Secure96Dxe/Secure96Dxe.inf b/Platform/96Boards/Secure96Dxe/Secure96Dxe.inf
> > index 72dbf1314c15..ce4c8b5f8fa5 100644
> > --- a/Platform/96Boards/Secure96Dxe/Secure96Dxe.inf
> > +++ b/Platform/96Boards/Secure96Dxe/Secure96Dxe.inf
> > @@ -21,6 +21,7 @@ [Defines]
> >    ENTRY_POINT                    = Secure96DxeEntryPoint
> >
> >  [Sources]
> > +  Secure96.asl
> >    Secure96.dts
> >    Secure96.h
> >    Secure96Dxe.c
> > --
> > 2.20.1
> >

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH edk2-platforms 3/3] Platform/Secure96Dxe: add ACPI description of the GPIO LEDs
  2019-04-26 11:56     ` Ard Biesheuvel
@ 2019-04-26 14:17       ` Leif Lindholm
  2019-04-26 14:18         ` Ard Biesheuvel
  0 siblings, 1 reply; 9+ messages in thread
From: Leif Lindholm @ 2019-04-26 14:17 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel-groups-io, Graeme Gregory, Masahisa Kojima

On Fri, Apr 26, 2019 at 01:56:54PM +0200, Ard Biesheuvel wrote:
> On Fri, 26 Apr 2019 at 13:16, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> >
> > On Thu, Apr 25, 2019 at 02:32:54PM +0200, Ard Biesheuvel wrote:
> > > Wire up the new 96boards mezzanine SSDT loading support, and use it
> > > to describe the four GPIO LEDs on the Secure96 mezzanine board.
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > ---
> > >  Platform/96Boards/Secure96Dxe/Secure96.asl    | 103 ++++++++++++++++++++
> > >  Platform/96Boards/Secure96Dxe/Secure96Dxe.c   |  59 ++++++++++-
> > >  Platform/96Boards/Secure96Dxe/Secure96Dxe.inf |   1 +
> > >  3 files changed, 158 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/Platform/96Boards/Secure96Dxe/Secure96.asl b/Platform/96Boards/Secure96Dxe/Secure96.asl
> > > new file mode 100644
> > > index 000000000000..bb9dac462a33
> > > --- /dev/null
> > > +++ b/Platform/96Boards/Secure96Dxe/Secure96.asl
> > > @@ -0,0 +1,103 @@
> > > +/** @file
> > > + * Copyright (c) 2019, Linaro Limited. All rights reserved.
> > > + *
> > > + * This program and the accompanying materials are licensed and made
> > > + * available under the terms and conditions of the BSD License which
> > > + * accompanies this distribution.  The full text of the license may be
> > > + * found at http://opensource.org/licenses/bsd-license.php
> > > + *
> > > + * THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > > + * WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
> > > + * IMPLIED.
> > > + */
> > > +
> > > +#include "Secure96.h"
> > > +
> > > +DefinitionBlock ("Secure96.aml", "SSDT", 2, "LINARO", "SECURE96", 1)
> > > +{
> > > +    External (\_SB.LS96.GPIO)
> > > +
> > > +    Scope (_SB)
> > > +    {
> > > +        Device (LD96)
> > > +        {
> > > +            Name (_HID, "PRP0001")  // _HID: Hardware ID
> > > +            Name (_UID, 0x00)       // _UID: Unique ID
> > > +            Name (_DSD, Package () {
> > > +                ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> >
> > Surely the above stanza is the most repeated bit of boiler plate in
> > the entire tree? I count 207 instances before this goes in.
> > Can we stick a #define in some common header for this and reuse?
> > ACPI_DSD_UUID?
> >
> > As I said, not required for this patch, but would be worth addressing.
> >
> 
> We should add the GUID to MdePkg, given that it occurs in the ACPI spec.

Yeah. What form would be practically consumable by .asl?

/
    Leif

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH edk2-platforms 3/3] Platform/Secure96Dxe: add ACPI description of the GPIO LEDs
  2019-04-26 14:17       ` Leif Lindholm
@ 2019-04-26 14:18         ` Ard Biesheuvel
  0 siblings, 0 replies; 9+ messages in thread
From: Ard Biesheuvel @ 2019-04-26 14:18 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: edk2-devel-groups-io, Graeme Gregory, Masahisa Kojima

On Fri, 26 Apr 2019 at 16:17, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Fri, Apr 26, 2019 at 01:56:54PM +0200, Ard Biesheuvel wrote:
> > On Fri, 26 Apr 2019 at 13:16, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > >
> > > On Thu, Apr 25, 2019 at 02:32:54PM +0200, Ard Biesheuvel wrote:
> > > > Wire up the new 96boards mezzanine SSDT loading support, and use it
> > > > to describe the four GPIO LEDs on the Secure96 mezzanine board.
> > > >
> > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > > ---
> > > >  Platform/96Boards/Secure96Dxe/Secure96.asl    | 103 ++++++++++++++++++++
> > > >  Platform/96Boards/Secure96Dxe/Secure96Dxe.c   |  59 ++++++++++-
> > > >  Platform/96Boards/Secure96Dxe/Secure96Dxe.inf |   1 +
> > > >  3 files changed, 158 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/Platform/96Boards/Secure96Dxe/Secure96.asl b/Platform/96Boards/Secure96Dxe/Secure96.asl
> > > > new file mode 100644
> > > > index 000000000000..bb9dac462a33
> > > > --- /dev/null
> > > > +++ b/Platform/96Boards/Secure96Dxe/Secure96.asl
> > > > @@ -0,0 +1,103 @@
> > > > +/** @file
> > > > + * Copyright (c) 2019, Linaro Limited. All rights reserved.
> > > > + *
> > > > + * This program and the accompanying materials are licensed and made
> > > > + * available under the terms and conditions of the BSD License which
> > > > + * accompanies this distribution.  The full text of the license may be
> > > > + * found at http://opensource.org/licenses/bsd-license.php
> > > > + *
> > > > + * THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > > > + * WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
> > > > + * IMPLIED.
> > > > + */
> > > > +
> > > > +#include "Secure96.h"
> > > > +
> > > > +DefinitionBlock ("Secure96.aml", "SSDT", 2, "LINARO", "SECURE96", 1)
> > > > +{
> > > > +    External (\_SB.LS96.GPIO)
> > > > +
> > > > +    Scope (_SB)
> > > > +    {
> > > > +        Device (LD96)
> > > > +        {
> > > > +            Name (_HID, "PRP0001")  // _HID: Hardware ID
> > > > +            Name (_UID, 0x00)       // _UID: Unique ID
> > > > +            Name (_DSD, Package () {
> > > > +                ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> > >
> > > Surely the above stanza is the most repeated bit of boiler plate in
> > > the entire tree? I count 207 instances before this goes in.
> > > Can we stick a #define in some common header for this and reuse?
> > > ACPI_DSD_UUID?
> > >
> > > As I said, not required for this patch, but would be worth addressing.
> > >
> >
> > We should add the GUID to MdePkg, given that it occurs in the ACPI spec.
>
> Yeah. What form would be practically consumable by .asl?
>

I think only a CPP macro including the "" is workable in practice.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-04-26 14:18 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-04-25 12:32 [PATCH edk2-platforms 0/3] enable Secure96 GPIO LEDs on ACPI systems Ard Biesheuvel
2019-04-25 12:32 ` [PATCH edk2-platforms 1/3] Silicon/SynQuacer: describe 96boards LS connector GPIOs via ACPI Ard Biesheuvel
2019-04-25 12:32 ` [PATCH edk2-platforms 2/3] Platform/96Boards: add ACPI support to mezzanine/LS connector driver Ard Biesheuvel
2019-04-25 12:32 ` [PATCH edk2-platforms 3/3] Platform/Secure96Dxe: add ACPI description of the GPIO LEDs Ard Biesheuvel
2019-04-26 11:16   ` Leif Lindholm
2019-04-26 11:56     ` Ard Biesheuvel
2019-04-26 14:17       ` Leif Lindholm
2019-04-26 14:18         ` Ard Biesheuvel
2019-04-26 11:10 ` [PATCH edk2-platforms 0/3] enable Secure96 GPIO LEDs on ACPI systems Leif Lindholm

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox