From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f68.google.com (mail-wr1-f68.google.com [209.85.221.68]) by mx.groups.io with SMTP id smtpd.web10.41125.1597662644611893934 for ; Mon, 17 Aug 2020 04:10:44 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@akeo-ie.20150623.gappssmtp.com header.s=20150623 header.b=Du+aKrCY; spf=none, err=permanent DNS error (domain: akeo.ie, ip: 209.85.221.68, mailfrom: pete@akeo.ie) Received: by mail-wr1-f68.google.com with SMTP id p20so14570754wrf.0 for ; Mon, 17 Aug 2020 04:10:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=akeo-ie.20150623.gappssmtp.com; s=20150623; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=dDW8HXEiO9ZwRBEalv7Yw029fCufz1Szpw1uQmh8vWM=; b=Du+aKrCYOpCkVikiTPx3CzZ1ChNXxn+csZBr0f+flhJZEbSIbLRPjxxm8Lo4u1VEWd bANLkkGP6PEvIAbpfmTMfmnJppNR5+h3/nYGmetKsa205Olpc1jCLTg/I83DmdsPqIa1 g4wBW4hwRoAIQsCUfp7P6avyPw5b0mKiol+fRc7G1UCrsoSEVvewxx54LOatdZ2kLKD2 5hckPH+NxWtu0LT3iGDHP6B0j7zo6zUZtpK39/HvQRwJdw9buFZSi/ti0Ru1wMlsjboz nNATjUcK7OD8qxhzZMfXeGggU2GOyl92DDmNgDJZcQQ3S4cNXlkbnzKx2ZyYddUbRpPl GS7g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=dDW8HXEiO9ZwRBEalv7Yw029fCufz1Szpw1uQmh8vWM=; b=mJP9zStmKXI5y2mM2IalpoHXh/unrQyHVVQbB49+tFnIoWYc+/URZEiqeM8xPljpHi /Vvmr+7aAlFKETDk17W2ktquVp5xQLCI5iE+1bsfj4+Uylmatb8mxhu4mDXDtkDrW2pc 8nTcfGjcj6h4KbHZnSJMrDup9o7upRSn/TO3yTsKqjiYApyzZoS7+ESD2yhOegK3TXwW ROlDvYiWBSU9F8wtykTcm0nlQLU0g9mm4TsEuSWwx4byfwFinvyB8K7h0DwvIIOkZf/P sdiLxIqY1wueSk9KH6DZoc7dwzNtc2jJwWRSR1a/0R/Kx4YpWkLs1GUwoPr0Pt8+VahN zmYg== X-Gm-Message-State: AOAM532NX3MramaS+W3MtBx/j5PoYXci+7c8VR+AEU0pVly+5/Gem/37 CMtKbiMeXuIOHXmlIgLbUcw5NA== X-Google-Smtp-Source: ABdhPJwb639e11YO44BdGSfDnkaudwgEaQhFNXPx2nRm7lAX6Zt1+/t+s7brF8PEuKb6mDAqbS0Z3g== X-Received: by 2002:a5d:4907:: with SMTP id x7mr15949686wrq.166.1597662643078; Mon, 17 Aug 2020 04:10:43 -0700 (PDT) Return-Path: Received: from [10.0.0.122] ([84.203.54.14]) by smtp.googlemail.com with ESMTPSA id n18sm31035723wrp.58.2020.08.17.04.10.41 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 17 Aug 2020 04:10:42 -0700 (PDT) Subject: Re: [PATCH 3/3] Platform/RaspberryPi: Add entry for user fan control To: Jeremy Linton , devel@edk2.groups.io Cc: Leif Lindholm , Andrei Warkentin , Ard Biesheuvel , Samer El-Haj-Mahmoud References: <20200813230056.40526-1-jeremy.linton@arm.com> <20200813230056.40526-4-jeremy.linton@arm.com> From: "Pete Batard" Message-ID: Date: Mon, 17 Aug 2020 12:10:41 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 MIME-Version: 1.0 In-Reply-To: <20200813230056.40526-4-jeremy.linton@arm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit 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 > Cc: Pete Batard > Cc: Andrei Warkentin > Cc: Ard Biesheuvel > Cc: Samer El-Haj-Mahmoud > Signed-off-by: Jeremy Linton > --- > 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 > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -22,6 +23,7 @@ > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -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 > > #include "ConfigDxeFormSetGuid.h" > > #include > > +#include > > > > // > > // 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