Re: [PATCH v3 1/5] Platform/RaspberryPi4: Add a basic thermal zone

["Jeremy Linton" <jeremy.linton@arm.com>]

On 8/31/20 10:33 AM, Ard Biesheuvel wrote:
> On 8/31/20 5:13 PM, Jeremy Linton wrote:
>> Hi,
>>
>> On 8/31/20 8:15 AM, Pete Batard wrote:
>>> One general, non-blocking comment below:
>>>
>>> On 2020.08.28 23:02, Jeremy Linton wrote:
>>>> Rather than exporting the temp sensor or mailbox
>>>> in ACPI land we can wrap them in AML and use the default
>>>> ACPI drivers provided by the OS. This enables the use of
>>>> "sensors" in linux to report the SOC temp.
>>>>
>>>> As a first pass add a basic passive cooling ACPI thermalzone
>>>> with trip points for passive cooling (throttling) handled
>>>> by the vc firmware, hibernate and critical shutdown. The
>>>> vc apparently kicks in at ~80C, so the hibernate and critical
>>>> set points are set at +5 and +10 of that. In the future
>>>> CPPC should be able to monitor the thermal throttling.
>>>>
>>>> Cc: Leif Lindholm <leif@nuviainc.com>
>>>> Cc: Pete Batard <pete@akeo.ie>
>>>> Cc: Andrei Warkentin <awarkentin@vmware.com>
>>>> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>>>> Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
>>>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>>>> Reviewed-by: Pete Batard <@pbatard>
>>>> ---
>>>>   Platform/RaspberryPi/AcpiTables/Dsdt.asl           | 31 
>>>> ++++++++++++++++++++++
>>>>   .../Bcm27xx/Include/IndustryStandard/Bcm2711.h     |  2 ++
>>>>   2 files changed, 33 insertions(+)
>>>>
>>>> diff --git a/Platform/RaspberryPi/AcpiTables/Dsdt.asl 
>>>> b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
>>>> index 353af2d876..73067aefd2 100644
>>>> --- a/Platform/RaspberryPi/AcpiTables/Dsdt.asl
>>>> +++ b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
>>>> @@ -252,6 +252,37 @@ DefinitionBlock ("Dsdt.aml", "DSDT", 5, 
>>>> "RPIFDN", "RPI", 2)
>>>>           }
>>>>
>>>>         })
>>>>
>>>>       }
>>>>
>>>> +
>>>>
>>>> +    // Define a simple thermal zone. The idea here is we compute 
>>>> the SOC temp
>>>>
>>>> +    // via a register we can read, and give it to the OS. This 
>>>> enables basic
>>>>
>>>> +    // reports from the "sensors" utility, and the OS can then poll 
>>>> and take
>>>>
>>>> +    // actions if that temp exceeds any of the given thresholds.
>>>>
>>>> +    Device (EC0)
>>>
>>> Just going to point out that all the other ACPI devices we seem to 
>>> define have 4 characters, so I'm not sure if we're breaking a 
>>> convention by introducing a 3 character one here...
>>
>> Well not an ACPI spec convention, because it seems the spec examples 
>> are mostly 3 characters. EDK2 OTOH, seems to be largely 4 but there 
>> are a fair number of 3 character device/etc methods around
>>
>> I'm not sure it matters, unless there is a edk2 convention I'm unaware 
>> of.
>>
> 
> I don't think such a convention exists, although I never tried the MS 
> iasl compiler, and the RPi asl code (which was contributed by MS) 
> suspiciously uses 4 letter identifiers everywhere.
> 
> Samer, any clue?
> 

I have another patch to allow the user to set the temp trip point which 
IIRC Andrei had asked about months ago when I originally mentioned that 
I had hacked up a thermal zone.

I might as well roll a v4 with that, the whitespace, and "0"'s appended 
to EC0 and TZ0 just in case... I'm sanity checking right now that it 
still works.