Re: [PATCH v2 47/48] ARM: tegra: ventana: Support CPU voltage scaling and thermal throttling
From: Dmitry Osipenko
Date: Thu Dec 17 2020 - 14:02:03 EST
17.12.2020 21:28, Daniel Lezcano пишет:
> On 17/12/2020 19:06, Dmitry Osipenko wrote:
>> Enable CPU voltage scaling and thermal throttling on Tegra20 Ventana board.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@xxxxxxxxx>
>> ---
>> arch/arm/boot/dts/tegra20-ventana.dts | 40 ++++++++++++++++++++++++++-
>> 1 file changed, 39 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/tegra20-ventana.dts b/arch/arm/boot/dts/tegra20-ventana.dts
>> index 14ace2ef749c..c2d9f38960bc 100644
>> --- a/arch/arm/boot/dts/tegra20-ventana.dts
>> +++ b/arch/arm/boot/dts/tegra20-ventana.dts
>> @@ -2,8 +2,10 @@
>> /dts-v1/;
>>
>> #include <dt-bindings/input/input.h>
>> +#include <dt-bindings/thermal/thermal.h>
>> #include "tegra20.dtsi"
>> #include "tegra20-cpu-opp.dtsi"
>> +#include "tegra20-cpu-opp-microvolt.dtsi"
>>
>> / {
>> model = "NVIDIA Tegra20 Ventana evaluation board";
>> @@ -527,9 +529,10 @@ ldo_rtc {
>> };
>> };
>>
>> - temperature-sensor@4c {
>> + nct1008: temperature-sensor@4c {
>> compatible = "onnn,nct1008";
>> reg = <0x4c>;
>> + #thermal-sensor-cells = <1>;
>> };
>> };
>>
>> @@ -615,10 +618,13 @@ clk32k_in: clock@0 {
>>
>> cpus {
>> cpu0: cpu@0 {
>> + cpu-supply = <&vdd_cpu>;
>> operating-points-v2 = <&cpu0_opp_table>;
>> + #cooling-cells = <2>;
>> };
>>
>> cpu@1 {
>> + cpu-supply = <&vdd_cpu>;
>> operating-points-v2 = <&cpu0_opp_table>;
>> };
>> };
>> @@ -717,4 +723,36 @@ sound {
>> <&tegra_car TEGRA20_CLK_CDEV1>;
>> clock-names = "pll_a", "pll_a_out0", "mclk";
>> };
>> +
>> + thermal-zones {
>> + cpu-thermal {
>> + polling-delay-passive = <1000>; /* milliseconds */
>> + polling-delay = <5000>; /* milliseconds */
>> +
>> + thermal-sensors = <&nct1008 1>;
>> +
>> + trips {
>> + trip0: cpu-alert0 {
>> + /* start throttling at 50C */
>> + temperature = <50000>;
>> + hysteresis = <200>;
>
> Did you mean <2000> ?
The <200> is correct.
Please see this commit for example:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2b22393b27670b45a2c870bce3df6579efd9a86a
>> + type = "passive";
>> + };
>> +
>> + trip1: cpu-crit {
>> + /* shut down at 60C */
>> + temperature = <60000>;
>> + hysteresis = <2000>;
>
> I think you can drop the hysteresis here, when the critical temperature
> is reached, there is an emergency shutdown.
Yes, perhaps you're right. Hysteresis doesn't make sense for a critical
trip. I'll improve it in the next revision, thanks.
> 50°C and 60°C sound very low values, no ?
Tegra20 CPU is very cold in comparison to later generations, it should
stay under 50°C even at a full load.
>> + type = "critical";
>> + };
>> + };
>> +
>> + cooling-maps {
>> + map0 {
>> + trip = <&trip0>;
>> + cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
>
> You should add all CPUs here.
All CPU cores are coupled on Tegra in regards to CPUFreq, hence I think
it won't make any difference if secondary CPU cores will be added here,
isn't it?