Re: [edk2-devel] [PATCH 3/3] Platform/RaspberryPi: Add entry for user fan control

Mon, 17 Aug 2020 11:11:23 -0700 

Hi,
Thanks for taking a look at this, I will roll another version with the suggested changes. It seems my space '(' rules are broken :) 




On 8/17/20 6:10 AM, Pete Batard wrote:

More minor style issues:

On 2020.08.14 00:00, Jeremy Linton wrote:

Add a menu item that allows the user to enable GPIO based
fan control via SSDT. This should only be seen/enabled on RPI4
because that is what its been tested with. As of this commit
its currently limited to only operating on a single GPIO pin (19).

Given GPIO pin current limitations its likely that a bit of
additional circuitry is required to drive a fan, and the GPIO
high/low signal can only be used as a enable/disable signal. A
search for "rpi npn gpio fan" or similar should turn up some
hits for how to do this simply.

It appears there are a couple boards (fan SHIM) which operate this
way, and probably should have custom menu items/SSDT edits as
people acquire the boards and test them.

Cc: Leif Lindholm <l...@nuviainc.com>
Cc: Pete Batard <p...@akeo.ie>
Cc: Andrei Warkentin <awarken...@vmware.com>
Cc: Ard Biesheuvel <ard.biesheu...@arm.com>
Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahm...@arm.com>
Signed-off-by: Jeremy Linton <jeremy.lin...@arm.com>
---

  Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 55 
++++++++++++++++++++++

  .../RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf    |  3 ++
  .../RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni |  5 ++
  .../RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 17 +++++++
  Platform/RaspberryPi/Include/ConfigVars.h          |  4 ++
  Platform/RaspberryPi/RPi3/RPi3.dsc                 |  5 ++
  Platform/RaspberryPi/RPi4/RPi4.dsc                 |  8 ++++
  Platform/RaspberryPi/RaspberryPi.dec               |  1 +
  .../Bcm27xx/Include/IndustryStandard/Bcm2711.h     |  2 +
  9 files changed, 100 insertions(+)


diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c 
b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c

index af54136ade..f10347be64 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -15,6 +15,7 @@
  #include <Library/AcpiLib.h>

  #include <Library/DebugLib.h>

  #include <Library/DevicePathLib.h>

+#include <Library/DxeServicesLib.h>

  #include <Library/DxeServicesTableLib.h>

  #include <Library/GpioLib.h>

  #include <Library/HiiLib.h>

@@ -22,6 +23,7 @@
  #include <Library/NetLib.h>

  #include <Library/UefiBootServicesTableLib.h>

  #include <Library/UefiRuntimeServicesTableLib.h>

+#include <Protocol/AcpiTable.h>

  #include <Protocol/BcmGenetPlatformDevice.h>

  #include <Protocol/RpiFirmware.h>

  #include <ConfigVars.h>

@@ -246,6 +248,14 @@ SetupVariables (
      ASSERT_EFI_ERROR (Status);

    }


+  Size = sizeof (UINT32);

+  Status = gRT->GetVariable (L"FanOnGpio",

+                  &gConfigDxeFormSetGuid,

+                  NULL, &Size, &Var32);

+  if (EFI_ERROR (Status)) {

+    PcdSet32 (PcdFanOnGpio, PcdGet32 (PcdFanOnGpio));

+  }

+

    Size = sizeof(AssetTagVar);


Space before '('




    Status = gRT->GetVariable(L"AssetTag",


Space before '('



@@ -368,6 +378,7 @@ ApplyVariables (
    UINT32 CpuClock = PcdGet32 (PcdCpuClock);

    UINT32 CustomCpuClock = PcdGet32 (PcdCustomCpuClock);

    UINT32 Rate = 0;

+  UINT32 FanOnGpio = PcdGet32 (PcdFanOnGpio);


    switch (CpuClock) {

    case CHIPSET_CPU_CLOCK_LOW:

@@ -565,8 +576,49 @@ ApplyVariables (
      GpioPinFuncSet (23, GPIO_FSEL_INPUT);

      GpioPinFuncSet (24, GPIO_FSEL_INPUT);

    }

+

+  if (FanOnGpio) {

+    DEBUG ((DEBUG_INFO, "Fan enabled on GPIO %d\n", FanOnGpio));

+    GpioPinFuncSet(FanOnGpio, GPIO_FSEL_OUTPUT);


Space before '('


+  }

  }


+EFI_STATUS

+FindInstallSsdt(UINT64 OemTableId)


Space before '('



+{

+  EFI_ACPI_TABLE_PROTOCOL         *AcpiTable;

+  UINTN                           Index;

+  EFI_ACPI_DESCRIPTION_HEADER     *Ssdt;

+  UINTN                           SsdtSize;

+  EFI_STATUS                      Status;

+  UINTN                           TableKey;

+

+

+  Status = gBS->LocateProtocol (&gEfiAcpiTableProtocolGuid, NULL,

+                                (VOID **)&AcpiTable);

+  if (EFI_ERROR (Status)) {

+    return Status;

+  }

+

+  for (Index = 0; !EFI_ERROR(Status); Index++) {


Space before '('




+    Status = GetSectionFromFv (&gEfiCallerIdGuid, EFI_SECTION_RAW, 
Index,


+                               (VOID **)&Ssdt, &SsdtSize);

+    if (Ssdt->OemTableId == OemTableId)

+        break;


Indentation should be 2 spaces after 'if'



+    SsdtSize = 0;

+  }

+

+  if (SsdtSize > 0) {

+    Status = AcpiTable->InstallAcpiTable (AcpiTable, Ssdt, SsdtSize,

+                                          &TableKey);

+    if (EFI_ERROR (Status)) {

+      DEBUG ((DEBUG_WARN, "%a: failed to install SSDT table %r\n",

+              __FUNCTION__, Status));

+    }

+  }

+

+  return Status;

+}


  EFI_STATUS

  EFIAPI

@@ -620,6 +672,9 @@ ConfigInitialize (
        PcdGet32 (PcdSystemTableMode) == SYSTEM_TABLE_MODE_BOTH) {

       Status = LocateAndInstallAcpiFromFv (&mAcpiTableFile);

       ASSERT_EFI_ERROR (Status);

+     if (PcdGet32 (PcdFanOnGpio)) {


+         FindInstallSsdt(SIGNATURE_64 ('R', 'P', 'I', 'T', 'H', 'F', 
'A', 'N'));


Space before '(' and indentation should be 2 spaces after 'if'



+     }

    }



    Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, TPL_NOTIFY, 
RegisterDevices,



diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf 
b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf

index cdce35bc74..fe3a01a570 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
@@ -28,6 +28,7 @@
    ConfigDxeFormSetGuid.h

    ConfigDxeHii.vfr

    ConfigDxeHii.uni

+  SsdtThermal.asl

    XhciQuirk.c


  [Packages]

@@ -46,6 +47,7 @@
    AcpiLib

    BaseLib

    DebugLib

+  DxeServicesLib

    DxeServicesTableLib

    GpioLib

    HiiLib

@@ -89,6 +91,7 @@
    gRaspberryPiTokenSpaceGuid.PcdSystemTableMode

    gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB

    gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB

+  gRaspberryPiTokenSpaceGuid.PcdFanOnGpio


  [Depex]

    gPcdProtocolGuid AND gRaspberryPiFirmwareProtocolGuid


diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni 
b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni

index 03763710a1..491d022fff 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
@@ -48,6 +48,11 @@

  #string STR_ADVANCED_SYSTAB_BOTH     #language en-US "ACPI + 
Devicetree"


  #string STR_ADVANCED_SYSTAB_DT       #language en-US "Devicetree"


+#string STR_ADVANCED_FANONGPIO_PROMPT #language en-US "ACPI fan control"


+#string STR_ADVANCED_FANONGPIO_HELP   #language en-US "Cycle a fan 
via GPIO-19 if temp exceeds 60C"


+#string STR_ADVANCED_FANONGPIO_OFF    #language en-US "Disabled"

+#string STR_ADVANCED_FANONGPIO_ON     #language en-US "Enabled"

+

  #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 d5615d7af0..0a5e4163e8 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
@@ -10,6 +10,7 @@
  #include <Guid/HiiPlatformSetupFormset.h>

  #include "ConfigDxeFormSetGuid.h"

  #include <ConfigVars.h>

+#include <IndustryStandard/Bcm2711.h>


  //

  // EFI Variable attributes

@@ -45,6 +46,11 @@ formset
        name  = RamLimitTo3GB,

        guid  = CONFIGDXE_FORM_SET_GUID;


+    efivarstore ADVANCED_FAN_ON_GPIO_VARSTORE_DATA,


+      attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | 
EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,


+      name  = FanOnGpio,

+      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,

@@ -174,6 +180,17 @@ formset

              option text = STRING_TOKEN(STR_ADVANCED_SYSTAB_DT), 
value = SYSTEM_TABLE_MODE_DT, flags = DEFAULT;


          endoneof;


+#if (RPI_MODEL == 4)


+        grayoutif NOT ideqval SystemTableMode.Mode == 
SYSTEM_TABLE_MODE_ACPI;


+          oneof varid = FanOnGpio.Enabled,

+              prompt      = STRING_TOKEN(STR_ADVANCED_FANONGPIO_PROMPT),

+              help        = STRING_TOKEN(STR_ADVANCED_FANONGPIO_HELP),


+              flags       = NUMERIC_SIZE_4 | INTERACTIVE | 
RESET_REQUIRED,



+              option text = STRING_TOKEN(STR_ADVANCED_FANONGPIO_OFF), 
value = 0, flags = DEFAULT;



+              option text = STRING_TOKEN(STR_ADVANCED_FANONGPIO_ON), 
value = GPIO_FAN_PIN, flags = 0;


+          endoneof;

+        endif;

+#endif

          string varid = AssetTag.AssetTag,

              prompt  = STRING_TOKEN(STR_ADVANCED_ASSET_TAG_PROMPT),

              help    = STRING_TOKEN(STR_ADVANCED_ASSET_TAG_HELP),


diff --git a/Platform/RaspberryPi/Include/ConfigVars.h 
b/Platform/RaspberryPi/Include/ConfigVars.h

index b1689b004d..1a40469bfa 100644
--- a/Platform/RaspberryPi/Include/ConfigVars.h
+++ b/Platform/RaspberryPi/Include/ConfigVars.h
@@ -69,6 +69,10 @@ typedef struct {
  } ADVANCED_RAM_LIMIT_TO_3GB_VARSTORE_DATA;


  typedef struct {

+  UINT32 Enabled;

+} ADVANCED_FAN_ON_GPIO_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 0998d8366c..cef8932ca2 100644
--- a/Platform/RaspberryPi/RPi3/RPi3.dsc
+++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
@@ -499,6 +499,11 @@

    
gRaspberryPiTokenSpaceGuid.PcdSystemTableMode|L"SystemTableMode"|gConfigDxeFormSetGuid|0x0|1 




    #

+  # Enable a fan in the ACPI thermal zone on GPIO pin #

+  #

+

gRaspberryPiTokenSpaceGuid.PcdFanOnGpio|L"FanOnGpio"|gConfigDxeFormSetGuid|0x0|0 



+

+  #

    # Common UEFI ones.

    #



diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc 
b/Platform/RaspberryPi/RPi4/RPi4.dsc

index baa7e63483..9d0eaf10a1 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -510,6 +510,14 @@

    
gRaspberryPiTokenSpaceGuid.PcdSystemTableMode|L"SystemTableMode"|gConfigDxeFormSetGuid|0x0|0 




    #

+  # Enable a fan in the ACPI thermal zone on GPIO pin #

+  #

+  # 0  - DISABLED

+  # 19 - Enabled on pin 19

+  #

+

gRaspberryPiTokenSpaceGuid.PcdFanOnGpio|L"FanOnGpio"|gConfigDxeFormSetGuid|0x0|0 



+

+  #

    # Common UEFI ones.

    #



diff --git a/Platform/RaspberryPi/RaspberryPi.dec 
b/Platform/RaspberryPi/RaspberryPi.dec

index c71177a2f7..a73650f2c3 100644
--- a/Platform/RaspberryPi/RaspberryPi.dec
+++ b/Platform/RaspberryPi/RaspberryPi.dec
@@ -66,3 +66,4 @@
    gRaspberryPiTokenSpaceGuid.PcdSystemTableMode|1|UINT32|0x0000001B

    gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB|0|UINT32|0x00000019

    gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|0|UINT32|0x0000001A

+  gRaspberryPiTokenSpaceGuid.PcdFanOnGpio|0|UINT32|0x0000001C


diff --git 
a/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h 
b/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h

index e9c81cafa1..7d9ea5d35c 100644
--- a/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h
+++ b/Silicon/Broadcom/Bcm27xx/Include/IndustryStandard/Bcm2711.h
@@ -86,4 +86,6 @@

  #define GENET_BASE_ADDRESS         FixedPcdGet64 
(PcdBcmGenetRegistersAddress)


  #define GENET_LENGTH               0x00010000


+#define GPIO_FAN_PIN               19 // fan shim uses GPIO 18

+

  #endif /* BCM2711_H__ */



Reviewed-by: Pete Batard <p...@akeo.ie>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64329): https://edk2.groups.io/g/devel/message/64329
Mute This Topic: https://groups.io/mt/76178276/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-























					Reply via email to