[edk2-devel] [PATCH v4 5/6] Platform/RaspberryPi4: Allow the user to set Temp
Ard Biesheuvel
ard.biesheuvel at arm.com
Mon Aug 31 19:05:32 UTC 2020
On 8/31/20 8:57 PM, Pete Batard wrote:
> One remark about using PcdSet32S () as opposed to PcdSet32 (), since we
> just went through an exercise making sure that we switched to the secure
> version of these calls.
>
> On 2020.08.31 18:25, Jeremy Linton wrote:
>> Now that we have the ability to enable an AML fan object,
>> allow the user to select the temperature at which the
>> fan cycles on.
>>
>> Cc: Leif Lindholm <leif at nuviainc.com>
>> Cc: Pete Batard <pete at akeo.ie>
>> Cc: Andrei Warkentin <awarkentin at vmware.com>
>> Cc: Ard Biesheuvel <ard.biesheuvel at arm.com>
>> Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud at arm.com>
>> Signed-off-by: Jeremy Linton <jeremy.linton at arm.com>
>> ---
>> Platform/RaspberryPi/AcpiTables/SsdtThermal.asl | 9 +++++----
>> Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 11
>> ++++++++++-
>> Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 1 +
>> Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni | 5 ++++-
>> Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 16
>> ++++++++++++++++
>> Platform/RaspberryPi/Include/ConfigVars.h | 4 ++++
>> Platform/RaspberryPi/RPi3/RPi3.dsc | 1 +
>> Platform/RaspberryPi/RPi4/RPi4.dsc | 1 +
>> Platform/RaspberryPi/RaspberryPi.dec | 1 +
>> 9 files changed, 43 insertions(+), 6 deletions(-)
>>
>> diff --git a/Platform/RaspberryPi/AcpiTables/SsdtThermal.asl
>> b/Platform/RaspberryPi/AcpiTables/SsdtThermal.asl
>> index ee028173e1..acfa4699bb 100644
>> --- a/Platform/RaspberryPi/AcpiTables/SsdtThermal.asl
>> +++ b/Platform/RaspberryPi/AcpiTables/SsdtThermal.asl
>> @@ -23,6 +23,7 @@ DefinitionBlock (__FILE__, "SSDT", 5, "RPIFDN",
>> "RPITHFAN", 2)
>> {
>> // Define a NameOp we will modify during InstallTable
>> Name (GIOP, 0x2) //08 47 49 4f 50 0a 02 (value must be >1)
>> + Name (FTMP, 0x2)
>> // Describe a fan
>> PowerResource (PFAN, 0, 0) {
>> OperationRegion (GPIO, SystemMemory, GPIO_BASE_ADDRESS, 0x1000)
>> @@ -68,9 +69,9 @@ DefinitionBlock (__FILE__, "SSDT", 5, "RPIFDN",
>> "RPITHFAN", 2)
>> // merge in an active cooling point.
>> Scope (\_SB_.EC00.TZ00)
>> {
>> - Method (_AC0) { Return (3332) } // (60C) active cooling
>> trip point,
>> - // if this is lower than
>> PSV then we
>> - // prefer active cooling
>> - Name (_AL0, Package () { \_SB_.EC00.FAN0 }) // the fan used for
>> AC0 above
>> + Method (_AC0) { Return ( (FTMP * 10) + 2732) } // (60C) active
>> cooling trip point,
>> + // if this is
>> lower than PSV then we
>> + // prefer active
>> cooling
>> + Name (_AL0, Package () { \_SB_.EC00.FAN0 }) // the fan used
>> for AC0 above
>> }
>> }
>> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
>> b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
>> index d58cbbdfe7..e8f964a329 100644
>> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
>> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
>> @@ -256,8 +256,16 @@ SetupVariables (
>> PcdSet32 (PcdFanOnGpio, PcdGet32 (PcdFanOnGpio));
>>
>> }
>>
>>
>> - Size = sizeof(AssetTagVar);
>>
>> + Size = sizeof (UINT32);
>>
>> + Status = gRT->GetVariable (L"FanTemp",
>>
>> + &gConfigDxeFormSetGuid,
>>
>> + NULL, &Size, &Var32);
>>
>> + if (EFI_ERROR (Status)) {
>>
>> + PcdSet32 (PcdFanTemp, PcdGet32 (PcdFanTemp));
>
>
> This should be replaced with:
>
> + Status = PcdSet32S (PcdFanTemp, PcdGet32 (PcdFanTemp));
> + ASSERT_EFI_ERROR (Status);
>
Agreed.
> Rather than re-spin this patch set yet again, and since the rest of the
> series looks good, I'm hoping this change can be applied during
> integration.
>
That should not be a problem. If nothing else comes up, I will merge
these tomorrow.
>>
>> + }
>>
>> +
>>
>>
>> + Size = sizeof (AssetTagVar);
>>
>> Status = gRT->GetVariable(L"AssetTag",
>>
>> &gConfigDxeFormSetGuid,
>>
>> NULL, &Size, AssetTagVar);
>>
>> @@ -697,6 +705,7 @@ VerifyUpdateTable(
>>
>> STATIC AML_NAME_OP_REPLACE SsdtNameOpReplace[] = {
>>
>> {"GIOP", PcdToken(PcdFanOnGpio)},
>>
>> + {"FTMP", PcdToken(PcdFanTemp)},
>>
>> {}
>>
>> };
>>
>>
>> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
>> b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
>> index 321e402e65..544e3b3e10 100644
>> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
>> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
>> @@ -91,6 +91,7 @@
>> gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB
>>
>> gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB
>>
>> gRaspberryPiTokenSpaceGuid.PcdFanOnGpio
>>
>> + gRaspberryPiTokenSpaceGuid.PcdFanTemp
>>
>>
>> [Depex]
>>
>> gPcdProtocolGuid AND gRaspberryPiFirmwareProtocolGuid
>>
>> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
>> b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
>> index e2d1bb4b39..2afe8f32ae 100644
>> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
>> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni
>> @@ -49,11 +49,14 @@
>> #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 if temp exceeds 60C"
>>
>> +#string STR_ADVANCED_FANONGPIO_HELP #language en-US "Cycle a fan
>> via GPIO at given temperature"
>>
>> #string STR_ADVANCED_FANONGPIO_OFF #language en-US "Disabled"
>>
>> #string STR_ADVANCED_FANONGPIO_18 #language en-US "Fan
>> Shim/GPIO-18"
>>
>> #string STR_ADVANCED_FANONGPIO_19 #language en-US "GPIO-19"
>>
>>
>> +#string STR_ADVANCED_FANTEMP_PROMPT #language en-US "ACPI fan
>> temperature"
>>
>> +#string STR_ADVANCED_FANTEMP_HELP #language en-US "Cycle a fan at C"
>>
>> +
>>
>> #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 94332caab3..de5e43471a 100644
>> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
>> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr
>> @@ -51,6 +51,11 @@ formset
>> name = FanOnGpio,
>>
>> guid = CONFIGDXE_FORM_SET_GUID;
>>
>>
>> + efivarstore ADVANCED_FANTEMP_VARSTORE_DATA,
>>
>> + attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS |
>> EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
>>
>> + name = FanTemp,
>>
>> + 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,
>>
>> @@ -191,6 +196,17 @@ formset
>> option text = STRING_TOKEN(STR_ADVANCED_FANONGPIO_19),
>> value = 19, flags = 0;
>>
>> endoneof;
>>
>> endif;
>>
>> +
>>
>> + grayoutif ideqval FanOnGpio.Enabled == 0;
>>
>> + numeric varid = FanTemp.Value,
>>
>> + prompt = STRING_TOKEN(STR_ADVANCED_FANTEMP_PROMPT),
>>
>> + help = STRING_TOKEN(STR_ADVANCED_FANTEMP_HELP),
>>
>> + flags = DISPLAY_UINT_DEC | NUMERIC_SIZE_4 |
>> INTERACTIVE | RESET_REQUIRED,
>>
>> + minimum = 50,
>>
>> + maximum = 80,
>>
>> + default = 60,
>>
>> + endnumeric;
>>
>> + 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 1a40469bfa..8094d4ef9a 100644
>> --- a/Platform/RaspberryPi/Include/ConfigVars.h
>> +++ b/Platform/RaspberryPi/Include/ConfigVars.h
>> @@ -73,6 +73,10 @@ typedef struct {
>> } ADVANCED_FAN_ON_GPIO_VARSTORE_DATA;
>>
>>
>> typedef struct {
>>
>> + UINT32 Value;
>>
>> +} ADVANCED_FANTEMP_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 cef8932ca2..484a46ffba 100644
>> --- a/Platform/RaspberryPi/RPi3/RPi3.dsc
>> +++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
>> @@ -502,6 +502,7 @@
>> # Enable a fan in the ACPI thermal zone on GPIO pin #
>>
>> #
>>
>>
>> gRaspberryPiTokenSpaceGuid.PcdFanOnGpio|L"FanOnGpio"|gConfigDxeFormSetGuid|0x0|0
>>
>>
>> +
>> gRaspberryPiTokenSpaceGuid.PcdFanTemp|L"FanTemp"|gConfigDxeFormSetGuid|0x0|0
>>
>>
>>
>> #
>>
>> # Common UEFI ones.
>>
>> diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc
>> b/Platform/RaspberryPi/RPi4/RPi4.dsc
>> index 9d0eaf10a1..823c9fc007 100644
>> --- a/Platform/RaspberryPi/RPi4/RPi4.dsc
>> +++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
>> @@ -516,6 +516,7 @@
>> # 19 - Enabled on pin 19
>>
>> #
>>
>>
>> gRaspberryPiTokenSpaceGuid.PcdFanOnGpio|L"FanOnGpio"|gConfigDxeFormSetGuid|0x0|0
>>
>>
>> +
>> gRaspberryPiTokenSpaceGuid.PcdFanTemp|L"FanTemp"|gConfigDxeFormSetGuid|0x0|60
>>
>>
>>
>> #
>>
>> # Common UEFI ones.
>>
>> diff --git a/Platform/RaspberryPi/RaspberryPi.dec
>> b/Platform/RaspberryPi/RaspberryPi.dec
>> index a73650f2c3..c64c61930e 100644
>> --- a/Platform/RaspberryPi/RaspberryPi.dec
>> +++ b/Platform/RaspberryPi/RaspberryPi.dec
>> @@ -67,3 +67,4 @@
>> gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB|0|UINT32|0x00000019
>>
>> gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|0|UINT32|0x0000001A
>>
>> gRaspberryPiTokenSpaceGuid.PcdFanOnGpio|0|UINT32|0x0000001C
>>
>> + gRaspberryPiTokenSpaceGuid.PcdFanTemp|0|UINT32|0x0000001D
>>
>
> With the suggested change applied:
> Reviewed-by: Pete Batard <pete at akeo.ie>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#64859): https://edk2.groups.io/g/devel/message/64859
Mute This Topic: https://groups.io/mt/76538749/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list