Re: [RFC PATCH 2/4] thermal: introduce device tree parser

[Eduardo Valentin <eduardo.valentin@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 3656 bytes --]

On 10-07-2013 02:48, Wei Ni wrote:
> On 07/09/2013 10:00 PM, Eduardo Valentin wrote:
>> In order to be able to build thermal policies
>> based on generic sensors, like I2C device, that
>> can be places in different points on different boards,
>> there is a need to have a way to feed board dependent
>> data into the thermal framework.
>>
>> This patch introduces a thermal data parser for device
>> tree. The parsed data is used to build thermal zones
>> and thermal binding parameters. The output data
>> can then be used to deploy thermal policies.
>>
>> This patch adds also documentation regarding this
>> API and how to define define tree nodes to use
>> this infrastructure.
> 
> It looks good, with this infrastructure, we can add generic sensor
> driver into the thermal fw easily.
> 
> 
>> +
>> +Below is an example:
>> +thermal_zone {
>> +            type = "CPU";
>> +            mask = <0x03>; /* trips writability */
>> +            passive_delay = <250>; /* milliseconds */
>> +            polling_delay = <1000>; /* milliseconds */
>> +            governor = "step_wise";
>> +            trips {
>> +                    alert@100000{
>> +                            temperature = <100000>; /* milliCelsius */
>> +                            hysteresis = <0>; /* milliCelsius */
>> +                            type = <1>;
> 
> how about to use the trip type name directly, such as named as
> "passive-trip;", I think it's more readable. for example:
> trip0 {
> ....
> passive-trip;
> }
> trip1 {
> ....
> active-trip;
> }
> 
>> +                    };
>> +                    crit@125000{
>> +                            temperature = <125000>; /* milliCelsius */
>> +                            hysteresis = <0>; /* milliCelsius */
>> +                            type = <3>;
>> +                    };
>> +            };
>> +            bind_params {
>> +                    action@0{
>> +                            cooling_device = "thermal-cpufreq";
>> +                            weight = <100>; /* percentage */
>> +                            mask = <0x01>;
>> +                    };
>> +            };
>> +};
> 
> as we know, thermal_zone_bind_cooling_device() will set the upper/lower
> in the thermal_instance. In the default .bind function, it just set to
> THERMAL_NO_LIMIT, but for some platform, it need to set these
> upper/lower values for different cooling device and trips, how to pass
> these values in DT? how about to set:
> action@0 {
> ...
> mask = <0x03>; //or you can remove this property;

Well, this has been added accordingly to current API needs.

> trip0 = <&alert 1 2>; //1 is lower value, 2 is upper value;
> trip1 = <&crit 3 4>;

I suppose the first item in you 3-tuple is the trip point.

> }

Yeah, I also noticed that I was missing the upper and lower limits. But
unfortunately this is a limitation on the thermal FW API too!

If one passes a bind params, the structure which represents platform
info, then it won't be able to pass the upper and lower limits. But by
passing a .bind callback, then you have the opportunity to match it
using these two values.

I believe we would need to change the data structures and the API to
support upper and lower limits via platform representation. We could
simply use the .bind callback of the dt thermal zone, but I believe that
would abusing the API, assuming that .match is for platform binding.
Durga, what do you think?


> 
> 
> Thanks.
> Wei.
> 
> 
> 


-- 
You have got to be excited about what you are doing. (L. Lamport)

Eduardo Valentin


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 295 bytes --]