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

["Pete Batard" <pete@akeo.ie>]

Nothing major, just whitespace/style and one minor capitalization issue:

On 2020.08.14 00:00, 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.
> 
> This commit also adds 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>
> ---
>   Platform/RaspberryPi/AcpiTables/Dsdt.asl | 31 +++++++++++++++++++++++++++++++
>   1 file changed, 31 insertions(+)
> 
> diff --git a/Platform/RaspberryPi/AcpiTables/Dsdt.asl b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
> index 353af2d876..a5c9567cdf 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)
> 
> +    {
> 
> +      Name(_HID, EISAID("PNP0C06"))

Space between EISAID and ("PNP0C06")

> 
> +      Name (_CCA, 0x0)
> 
> +
> 
> +      // all temps in are tenths of K (aka 2732 is the min temps in linux (aka 0C))

I'd use a capital 'L' in "Linux"

> 
> +      ThermalZone(TZ0) {

Space before '('

> 
> +        Method(_TMP, 0, Serialized) {

Space before '('
> 
> +          OperationRegion (TEMS, SystemMemory, 0xfd5d2200, 0x8)
> 
> +          Field (TEMS, DWordAcc, NoLock, Preserve) {
> 
> +            TMPS, 32
> 
> +          }
> 
> +          return (((419949 - ((TMPS & 0x3ff) * 487)) / 100) + 2732);
> 
> +        }
> 
> +        Method(_SCP, 3) { }              // receive cooling policy from OS
> 
> +
> 
> +        Method(_CRT) { return(3632) }    // (90K) Critical temp point (immediate power-off)
> 
> +        Method(_HOT) { return(3582) }    // (85K) HOT state where OS should hibernate
> 
> +        Method(_PSV) { return(3532) }    // (80K) Passive cooling (CPU throttling) trip point
> 
> +
> 
> +        // SSDT inserts _AC0/_AL0 @60C here, if a FAN is configured
> 
> +
> 
> +        Name(_TZP, 10)                   //The OSPM must poll this device every 1 seconds
> 
> +        Name(_PSL, Package(){ \_SB_.CPU0, \_SB_.CPU1, \_SB_.CPU2, \_SB_.CPU3})

6 x Space before '('

> 
> +      }
> 
> +    }
> 
>   #endif
> 
>   
> 
>     }
> 

Reviewed by: Pete Batard <pete@akeo.ie>