linux-kernel - Re: [PATCH v1] thermal/of: Introduce k-po, k-pu and k-i for a thermal zone

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c3f54e18-8683-8bd9-90fa-e3465cddf8e8@arm.com>
Date:   Tue, 25 Aug 2020 09:25:11 +0100
From:   Lukasz Luba <lukasz.luba@....com>
To:     Rob Herring <robh@...nel.org>
Cc:     Finley Xiao <finley.xiao@...k-chips.com>, heiko@...ech.de,
        rui.zhang@...el.com, daniel.lezcano@...aro.org,
        linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-pm@...r.kernel.org, devicetree@...r.kernel.org,
        huangtao@...k-chips.com, tony.xie@...k-chips.com, cl@...k-chips.com
Subject: Re: [PATCH v1] thermal/of: Introduce k-po, k-pu and k-i for a thermal
 zone

Hi Rob,

On 8/25/20 12:09 AM, Rob Herring wrote:
> On Tue, Aug 11, 2020 at 08:31:15PM +0800, Finley Xiao wrote:
>> The default value for k_pu is:
>>      2 * sustainable_power / (desired_temperature - switch_on_temp)
>> The default value for k_po is:
>>      sustainable_power / (desired_temperature - switch_on_temp)
>> The default value for k_i is 10.
>>
>> Even though these parameters of the PID controller can be changed
>> by the following sysfs files:
>>      /sys/class/thermal/thermal_zoneX/k_pu
>>      /sys/class/thermal/thermal_zoneX/k_po
>>      /sys/class/thermal/thermal_zoneX/k_i
>>
>> But it's still more convenient to change the default values by devicetree,
>> so introduce these three optional properties. If provided these properties,
>> they will be parsed and associated with the thermal zone via the thermal
>> zone parameters.
>>
>> Signed-off-by: Finley Xiao <finley.xiao@...k-chips.com>
>> ---
>>   Documentation/devicetree/bindings/thermal/thermal.txt | 14 ++++++++++++++
> 
> Bindings should be a separate file and this one is a DT schema now.
> 
>>   drivers/thermal/thermal_of.c                          |  7 +++++++
>>   2 files changed, 21 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/thermal/thermal.txt b/Documentation/devicetree/bindings/thermal/thermal.txt
>> index f78bec19ca35..ebe936b57ded 100644
>> --- a/Documentation/devicetree/bindings/thermal/thermal.txt
>> +++ b/Documentation/devicetree/bindings/thermal/thermal.txt
>> @@ -165,6 +165,20 @@ Optional property:
>>   			2000mW, while on a 10'' tablet is around
>>   			4500mW.
>>   
>> +- k-po:			Proportional parameter of the PID controller when
>> +			current temperature is above the target.
>> +  Type: signed
>> +  Size: one cell
>> +
>> +- k-pu:			Proportional parameter of the PID controller when
>> +			current temperature is below the target.
>> +  Type: signed
>> +  Size: one cell
>> +
>> +- k-i:			Integral parameter of the PID controller.
>> +  Type: signed
>> +  Size: one cell
> 
> What's PID?
> 
> I know nothing about the sysfs params, but the binding needs to stand on
> it's own and needs enough detail to educate me.
> 

Sorry for the delay, I missed that patch.
These parameters are the coefficients for the
Proportional-Integral-Derivative (PID) controller [1], which is the
core of the Intelligent Power Allocation (IPA) thermal governor.

Only IPA uses them, thus I don't think the governors parameters:
k-po, k-pu, k-i
should be part of the DeviceTree. I haven't seen such governors
tunnables in the DT, please point me if they exist somewhere.

Do you think Rob they might be specified in the DT?

Regards,
Lukasz

[1] https://en.wikipedia.org/wiki/PID_controller