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

2013-07-17 Thread Eduardo Valentin
On 09-07-2013 12:14, R, Durgadoss wrote:
> Hi Eduardo,
> 
>> -Original Message-
>> From: Eduardo Valentin [mailto:eduardo.valen...@ti.com]
>> Sent: Tuesday, July 09, 2013 7:30 PM
>> To: linux...@vger.kernel.org; R, Durgadoss; amit.dan...@samsung.com
>> Cc: Zhang, Rui; Eduardo Valentin; linux-kernel@vger.kernel.org
>> Subject: [RFC PATCH 2/4] thermal: introduce device tree parser
>>
>> 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.
>>
>> Cc: Zhang Rui 
>> Cc: linux...@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Eduardo Valentin 
>> ---
> 
> I looked at the Documentation part of this. And it looks good.
> 
> At some places you are using ERANGE. Technically, this represents
> 'Math result not representable'. May be should be use EINVAL 
> itself ? I would leave it up to you ;)

I will be using EDOM in next version, tks

> 
> Thanks,
> Durga
> 
>>  .../devicetree/bindings/thermal/thermal.txt|  92 +
>>  drivers/thermal/Kconfig|  13 +
>>  drivers/thermal/Makefile   |   1 +
>>  drivers/thermal/thermal_dt.c   | 412 
>> +
>>  drivers/thermal/thermal_dt.h   |  44 +++
>>  include/linux/thermal.h|   3 +
>>  6 files changed, 565 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/thermal/thermal.txt
>>  create mode 100644 drivers/thermal/thermal_dt.c
>>  create mode 100644 drivers/thermal/thermal_dt.h
>>
>> diff --git a/Documentation/devicetree/bindings/thermal/thermal.txt
>> b/Documentation/devicetree/bindings/thermal/thermal.txt
>> new file mode 100644
>> index 000..2c25ab2
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/thermal/thermal.txt
>> @@ -0,0 +1,92 @@
>> +
>> +Thermal Framework Device Tree descriptor
>> +
>> +
>> +This file describes how to define a thermal structure using device tree.
>> +A thermal structure includes thermal zones and their components, such
>> +as name, governor, trip points, polling intervals and cooling devices
>> +binding descriptors. A binding descriptor may contain information on
>> +how to react, with a cooling stepped action or a weight on a fair share.
>> +
>> +
>> +trip
>> +
>> +
>> +The trip node is a node to describe a point in the temperature domain
>> +in which the system takes an action. This node describes just the point,
>> +not the action.
>> +
>> +A node describing a trip must contain:
>> +- temperature: the trip temperature level, in milliCelsius.
>> +- hysteresis: a (low) hysteresis value on 'temperature'. This is a relative
>> +value, in milliCelsius.
>> +- type: the trip type. Here is the type mapping:
>> +THERMAL_TRIP_ACTIVE = 0
>> +THERMAL_TRIP_PASSIVE = 1
>> +THERMAL_TRIP_HOT = 2
>> +THERMAL_TRIP_CRITICAL = 3
>> +
>> +**
>> +bind_param
>> +**
>> +
>> +The bind_param node is a node to describe how cooling devices get assigned
>> +to trip points of the zone. The cooling devices are expected to be loaded
>> +in the target system.
>> +
>> +A node describing a bind_param must contain:
>> +- cooling_device: A string with the cooling device name.
>> +- weight: The 'influence' of a particular cooling device on this zone.
>> + This is on a percentage scale. The sum of all these weights
>> + (for a particular zone) cannot exceed 100.
>> +- trip_mask: This is a bit mask that gives the binding relation between
>> +   this thermal zone and cdev, for a particular trip point.
>> +   If nth bit is set, then the cdev and thermal zone are bound
>> +   for trip point n.
>> +
>> +
>> +thermal_zone
>> +
>> +
>> +The thermal

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

2013-07-17 Thread Eduardo Valentin
On 09-07-2013 12:14, R, Durgadoss wrote:
 Hi Eduardo,
 
 -Original Message-
 From: Eduardo Valentin [mailto:eduardo.valen...@ti.com]
 Sent: Tuesday, July 09, 2013 7:30 PM
 To: linux...@vger.kernel.org; R, Durgadoss; amit.dan...@samsung.com
 Cc: Zhang, Rui; Eduardo Valentin; linux-kernel@vger.kernel.org
 Subject: [RFC PATCH 2/4] thermal: introduce device tree parser

 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.

 Cc: Zhang Rui rui.zh...@intel.com
 Cc: linux...@vger.kernel.org
 Cc: linux-kernel@vger.kernel.org
 Signed-off-by: Eduardo Valentin eduardo.valen...@ti.com
 ---
 
 I looked at the Documentation part of this. And it looks good.
 
 At some places you are using ERANGE. Technically, this represents
 'Math result not representable'. May be should be use EINVAL 
 itself ? I would leave it up to you ;)

I will be using EDOM in next version, tks

 
 Thanks,
 Durga
 
  .../devicetree/bindings/thermal/thermal.txt|  92 +
  drivers/thermal/Kconfig|  13 +
  drivers/thermal/Makefile   |   1 +
  drivers/thermal/thermal_dt.c   | 412 
 +
  drivers/thermal/thermal_dt.h   |  44 +++
  include/linux/thermal.h|   3 +
  6 files changed, 565 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/thermal/thermal.txt
  create mode 100644 drivers/thermal/thermal_dt.c
  create mode 100644 drivers/thermal/thermal_dt.h

 diff --git a/Documentation/devicetree/bindings/thermal/thermal.txt
 b/Documentation/devicetree/bindings/thermal/thermal.txt
 new file mode 100644
 index 000..2c25ab2
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/thermal/thermal.txt
 @@ -0,0 +1,92 @@
 +
 +Thermal Framework Device Tree descriptor
 +
 +
 +This file describes how to define a thermal structure using device tree.
 +A thermal structure includes thermal zones and their components, such
 +as name, governor, trip points, polling intervals and cooling devices
 +binding descriptors. A binding descriptor may contain information on
 +how to react, with a cooling stepped action or a weight on a fair share.
 +
 +
 +trip
 +
 +
 +The trip node is a node to describe a point in the temperature domain
 +in which the system takes an action. This node describes just the point,
 +not the action.
 +
 +A node describing a trip must contain:
 +- temperature: the trip temperature level, in milliCelsius.
 +- hysteresis: a (low) hysteresis value on 'temperature'. This is a relative
 +value, in milliCelsius.
 +- type: the trip type. Here is the type mapping:
 +THERMAL_TRIP_ACTIVE = 0
 +THERMAL_TRIP_PASSIVE = 1
 +THERMAL_TRIP_HOT = 2
 +THERMAL_TRIP_CRITICAL = 3
 +
 +**
 +bind_param
 +**
 +
 +The bind_param node is a node to describe how cooling devices get assigned
 +to trip points of the zone. The cooling devices are expected to be loaded
 +in the target system.
 +
 +A node describing a bind_param must contain:
 +- cooling_device: A string with the cooling device name.
 +- weight: The 'influence' of a particular cooling device on this zone.
 + This is on a percentage scale. The sum of all these weights
 + (for a particular zone) cannot exceed 100.
 +- trip_mask: This is a bit mask that gives the binding relation between
 +   this thermal zone and cdev, for a particular trip point.
 +   If nth bit is set, then the cdev and thermal zone are bound
 +   for trip point n.
 +
 +
 +thermal_zone
 +
 +
 +The thermal_zone node is the node containing all the required info
 +for describing a thermal zone, including its cdev bindings. The thermal_zone
 +node must contain, apart from its own properties, one node containing
 +trip nodes and one node containing all the zone bind parameters.
 +
 +Required properties:
 +- type: this is a string containing the zone type. Say 'cpu', 'core', 
 'mem', etc.
 +- mask: Bit string: If 'n'th bit is set, then trip point 'n' is writeable.
 +
 +- passive_delay: number of milliseconds to wait between polls when
 +performing passive cooling.
 +- polling_delay: number of milliseconds to wait between polls when checking
 +
 +- governor: A string containing the default governor for this zone.
 +
 +Below is an example:
 +thermal_zone {
 +type

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

2013-07-15 Thread Eduardo Valentin
On 15-07-2013 13:03, R, Durgadoss wrote:
>> -Original Message-
>> From: linux-pm-ow...@vger.kernel.org [mailto:linux-pm-
>> ow...@vger.kernel.org] On Behalf Of Eduardo Valentin
>> Sent: Monday, July 15, 2013 5:25 PM
>> To: Wei Ni
>> Cc: Eduardo Valentin; linux...@vger.kernel.org; R, Durgadoss;
>> amit.dan...@samsung.com; Zhang, Rui; linux-kernel@vger.kernel.org
>> Subject: Re: [RFC PATCH 2/4] thermal: introduce device tree parser
>>
>> 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@10{
>>>> +temperature = <10>; /* 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 = < 1 2>; //1 is lower value, 2 is upper value;
>>> trip1 = < 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, bu

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

2013-07-15 Thread R, Durgadoss
> -Original Message-
> From: linux-pm-ow...@vger.kernel.org [mailto:linux-pm-
> ow...@vger.kernel.org] On Behalf Of Eduardo Valentin
> Sent: Monday, July 15, 2013 5:25 PM
> To: Wei Ni
> Cc: Eduardo Valentin; linux...@vger.kernel.org; R, Durgadoss;
> amit.dan...@samsung.com; Zhang, Rui; linux-kernel@vger.kernel.org
> Subject: Re: [RFC PATCH 2/4] thermal: introduce device tree parser
> 
> 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@10{
> >> +temperature = <10>; /* 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 = < 1 2>; //1 is lower value, 2 is upper value;
> > trip1 = < 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?

okay, I see.. Two approaches I could think of:
1. Introduce two arrays (size = number of trips in the tz) named
upper/lower_limits[size] in the 'thermal_bind_params' structure.
This way we don't need any API change. We can slightly cha

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

2013-07-15 Thread Eduardo Valentin
On 10-07-2013 11:16, Stephen Warren wrote:
> On 07/10/2013 12:48 AM, 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@10{
>>> +temperature = <10>; /* 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;
>> }
> 
> You can always use the C pre-processor in DT now to define named constants:
> 
> include/dt-bindings//h
> 
>   #define THERMAL_PASSIVE_TRIP 0
>   #define THERMAL_ACTIVE_TRIP 1
> 
> *.dts:
> 
>   type = ;
> 
> Having a single 'property = value;' rather than n different Boolean
> property names seems better, irrespective of whether value is an integer
> or string; parsing and error-checking will be simpler.

agreed here. I will amend with the above suggestions. makes the dt file
much more readable. thanks Stephen and Wei.

> 
> 


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

Eduardo Valentin



signature.asc
Description: OpenPGP digital signature


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

2013-07-15 Thread Eduardo Valentin
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@10{
>> +temperature = <10>; /* 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 = < 1 2>; //1 is lower value, 2 is upper value;
> trip1 = < 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



signature.asc
Description: OpenPGP digital signature


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

2013-07-15 Thread Eduardo Valentin
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@10{
 +temperature = 10; /* 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



signature.asc
Description: OpenPGP digital signature


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

2013-07-15 Thread Eduardo Valentin
On 10-07-2013 11:16, Stephen Warren wrote:
 On 07/10/2013 12:48 AM, 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@10{
 +temperature = 10; /* 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;
 }
 
 You can always use the C pre-processor in DT now to define named constants:
 
 include/dt-bindings//h
 
   #define THERMAL_PASSIVE_TRIP 0
   #define THERMAL_ACTIVE_TRIP 1
 
 *.dts:
 
   type = THERMAL_PASSIVE_TRIP;
 
 Having a single 'property = value;' rather than n different Boolean
 property names seems better, irrespective of whether value is an integer
 or string; parsing and error-checking will be simpler.

agreed here. I will amend with the above suggestions. makes the dt file
much more readable. thanks Stephen and Wei.

 
 


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

Eduardo Valentin



signature.asc
Description: OpenPGP digital signature


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

2013-07-15 Thread R, Durgadoss
 -Original Message-
 From: linux-pm-ow...@vger.kernel.org [mailto:linux-pm-
 ow...@vger.kernel.org] On Behalf Of Eduardo Valentin
 Sent: Monday, July 15, 2013 5:25 PM
 To: Wei Ni
 Cc: Eduardo Valentin; linux...@vger.kernel.org; R, Durgadoss;
 amit.dan...@samsung.com; Zhang, Rui; linux-kernel@vger.kernel.org
 Subject: Re: [RFC PATCH 2/4] thermal: introduce device tree parser
 
 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@10{
  +temperature = 10; /* 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?

okay, I see.. Two approaches I could think of:
1. Introduce two arrays (size = number of trips in the tz) named
upper/lower_limits[size] in the 'thermal_bind_params' structure.
This way we don't need any API change. We can slightly change the
implementation inside '__bind' function in thermal_core.c to get this
working.

2. Pass 3 more parameters in the .match function:
.match(tz, cdev, trip, lower, upper). The platform layer
then determines whether there is a match; and if so,
provides sane values for lower and upper variables.

At this point of time, I think I prefer method 1 ;)
Let me know your thoughts.

Thanks,
Durga
 
 
 
 
  Thanks.
  Wei.
 
 
 
 
 
 --
 You have got to be excited about what you are doing. (L. Lamport)
 
 Eduardo Valentin

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2013-07-15 Thread Eduardo Valentin
On 15-07-2013 13:03, R, Durgadoss wrote:
 -Original Message-
 From: linux-pm-ow...@vger.kernel.org [mailto:linux-pm-
 ow...@vger.kernel.org] On Behalf Of Eduardo Valentin
 Sent: Monday, July 15, 2013 5:25 PM
 To: Wei Ni
 Cc: Eduardo Valentin; linux...@vger.kernel.org; R, Durgadoss;
 amit.dan...@samsung.com; Zhang, Rui; linux-kernel@vger.kernel.org
 Subject: Re: [RFC PATCH 2/4] thermal: introduce device tree parser

 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@10{
 +temperature = 10; /* 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?
 
 okay, I see.. Two approaches I could think of:
 1. Introduce two arrays (size = number of trips in the tz) named
 upper/lower_limits[size] in the 'thermal_bind_params' structure.
 This way we don't need any API change. We can slightly change the
 implementation inside '__bind' function in thermal_core.c to get this
 working.
 
 2. Pass 3 more parameters in the .match function:
 .match(tz, cdev, trip, lower, upper). The platform layer
 then determines whether there is a match; and if so,
 provides sane values for lower and upper variables.
 
 At this point of time, I think I prefer method 1 ;)
 Let me know your thoughts.
 

Yeah, I agree that (1) is likely to scale. I will cook something with it
for next version.

 Thanks,
 Durga




 Thanks.
 Wei.





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

 Eduardo Valentin
 
 
 


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

Eduardo Valentin



signature.asc
Description: OpenPGP digital signature


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

2013-07-10 Thread Stephen Warren
On 07/10/2013 12:48 AM, 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@10{
>> +temperature = <10>; /* 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;
> }

You can always use the C pre-processor in DT now to define named constants:

include/dt-bindings//h

#define THERMAL_PASSIVE_TRIP 0
#define THERMAL_ACTIVE_TRIP 1

*.dts:

type = ;

Having a single 'property = value;' rather than n different Boolean
property names seems better, irrespective of whether value is an integer
or string; parsing and error-checking will be simpler.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2013-07-10 Thread Wei Ni
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@10{
> +temperature = <10>; /* 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;
trip0 = < 1 2>; //1 is lower value, 2 is upper value;
trip1 = < 3 4>;
}


Thanks.
Wei.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2013-07-10 Thread Wei Ni
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@10{
 +temperature = 10; /* 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;
trip0 = alert 1 2; //1 is lower value, 2 is upper value;
trip1 = crit 3 4;
}


Thanks.
Wei.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2013-07-10 Thread Stephen Warren
On 07/10/2013 12:48 AM, 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@10{
 +temperature = 10; /* 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;
 }

You can always use the C pre-processor in DT now to define named constants:

include/dt-bindings//h

#define THERMAL_PASSIVE_TRIP 0
#define THERMAL_ACTIVE_TRIP 1

*.dts:

type = THERMAL_PASSIVE_TRIP;

Having a single 'property = value;' rather than n different Boolean
property names seems better, irrespective of whether value is an integer
or string; parsing and error-checking will be simpler.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2013-07-09 Thread R, Durgadoss
Hi Eduardo,

> -Original Message-
> From: Eduardo Valentin [mailto:eduardo.valen...@ti.com]
> Sent: Tuesday, July 09, 2013 7:30 PM
> To: linux...@vger.kernel.org; R, Durgadoss; amit.dan...@samsung.com
> Cc: Zhang, Rui; Eduardo Valentin; linux-kernel@vger.kernel.org
> Subject: [RFC PATCH 2/4] thermal: introduce device tree parser
> 
> 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.
> 
> Cc: Zhang Rui 
> Cc: linux...@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Eduardo Valentin 
> ---

I looked at the Documentation part of this. And it looks good.

At some places you are using ERANGE. Technically, this represents
'Math result not representable'. May be should be use EINVAL 
itself ? I would leave it up to you ;)

Thanks,
Durga

>  .../devicetree/bindings/thermal/thermal.txt|  92 +
>  drivers/thermal/Kconfig|  13 +
>  drivers/thermal/Makefile   |   1 +
>  drivers/thermal/thermal_dt.c   | 412 
> +
>  drivers/thermal/thermal_dt.h   |  44 +++
>  include/linux/thermal.h|   3 +
>  6 files changed, 565 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/thermal/thermal.txt
>  create mode 100644 drivers/thermal/thermal_dt.c
>  create mode 100644 drivers/thermal/thermal_dt.h
> 
> diff --git a/Documentation/devicetree/bindings/thermal/thermal.txt
> b/Documentation/devicetree/bindings/thermal/thermal.txt
> new file mode 100644
> index 000..2c25ab2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/thermal.txt
> @@ -0,0 +1,92 @@
> +
> +Thermal Framework Device Tree descriptor
> +
> +
> +This file describes how to define a thermal structure using device tree.
> +A thermal structure includes thermal zones and their components, such
> +as name, governor, trip points, polling intervals and cooling devices
> +binding descriptors. A binding descriptor may contain information on
> +how to react, with a cooling stepped action or a weight on a fair share.
> +
> +
> +trip
> +
> +
> +The trip node is a node to describe a point in the temperature domain
> +in which the system takes an action. This node describes just the point,
> +not the action.
> +
> +A node describing a trip must contain:
> +- temperature: the trip temperature level, in milliCelsius.
> +- hysteresis: a (low) hysteresis value on 'temperature'. This is a relative
> +value, in milliCelsius.
> +- type: the trip type. Here is the type mapping:
> + THERMAL_TRIP_ACTIVE = 0
> + THERMAL_TRIP_PASSIVE = 1
> + THERMAL_TRIP_HOT = 2
> + THERMAL_TRIP_CRITICAL = 3
> +
> +**
> +bind_param
> +**
> +
> +The bind_param node is a node to describe how cooling devices get assigned
> +to trip points of the zone. The cooling devices are expected to be loaded
> +in the target system.
> +
> +A node describing a bind_param must contain:
> +- cooling_device: A string with the cooling device name.
> +- weight: The 'influence' of a particular cooling device on this zone.
> + This is on a percentage scale. The sum of all these weights
> + (for a particular zone) cannot exceed 100.
> +- trip_mask: This is a bit mask that gives the binding relation between
> +   this thermal zone and cdev, for a particular trip point.
> +   If nth bit is set, then the cdev and thermal zone are bound
> +   for trip point n.
> +
> +
> +thermal_zone
> +
> +
> +The thermal_zone node is the node containing all the required info
> +for describing a thermal zone, including its cdev bindings. The thermal_zone
> +node must contain, apart from its own properties, one node containing
> +trip nodes and one node containing all the zone bind parameters.
> +
> +Required properties:
> +- type: this is a string containing the zone type. Say 'cpu', 'core', 'mem', 
> etc.
> +- mask: Bit string: If 'n'th bit is set, then trip point 'n' is writeable.
> +
> +- passiv

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

2013-07-09 Thread Eduardo Valentin
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.

Cc: Zhang Rui 
Cc: linux...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Eduardo Valentin 
---
 .../devicetree/bindings/thermal/thermal.txt|  92 +
 drivers/thermal/Kconfig|  13 +
 drivers/thermal/Makefile   |   1 +
 drivers/thermal/thermal_dt.c   | 412 +
 drivers/thermal/thermal_dt.h   |  44 +++
 include/linux/thermal.h|   3 +
 6 files changed, 565 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/thermal.txt
 create mode 100644 drivers/thermal/thermal_dt.c
 create mode 100644 drivers/thermal/thermal_dt.h

diff --git a/Documentation/devicetree/bindings/thermal/thermal.txt 
b/Documentation/devicetree/bindings/thermal/thermal.txt
new file mode 100644
index 000..2c25ab2
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/thermal.txt
@@ -0,0 +1,92 @@
+
+Thermal Framework Device Tree descriptor
+
+
+This file describes how to define a thermal structure using device tree.
+A thermal structure includes thermal zones and their components, such
+as name, governor, trip points, polling intervals and cooling devices
+binding descriptors. A binding descriptor may contain information on
+how to react, with a cooling stepped action or a weight on a fair share.
+
+
+trip
+
+
+The trip node is a node to describe a point in the temperature domain
+in which the system takes an action. This node describes just the point,
+not the action.
+
+A node describing a trip must contain:
+- temperature: the trip temperature level, in milliCelsius.
+- hysteresis: a (low) hysteresis value on 'temperature'. This is a relative
+value, in milliCelsius.
+- type: the trip type. Here is the type mapping:
+   THERMAL_TRIP_ACTIVE = 0
+   THERMAL_TRIP_PASSIVE = 1
+   THERMAL_TRIP_HOT = 2
+   THERMAL_TRIP_CRITICAL = 3
+
+**
+bind_param
+**
+
+The bind_param node is a node to describe how cooling devices get assigned
+to trip points of the zone. The cooling devices are expected to be loaded
+in the target system.
+
+A node describing a bind_param must contain:
+- cooling_device: A string with the cooling device name.
+- weight: The 'influence' of a particular cooling device on this zone.
+ This is on a percentage scale. The sum of all these weights
+ (for a particular zone) cannot exceed 100.
+- trip_mask: This is a bit mask that gives the binding relation between
+   this thermal zone and cdev, for a particular trip point.
+   If nth bit is set, then the cdev and thermal zone are bound
+   for trip point n.
+
+
+thermal_zone
+
+
+The thermal_zone node is the node containing all the required info
+for describing a thermal zone, including its cdev bindings. The thermal_zone
+node must contain, apart from its own properties, one node containing
+trip nodes and one node containing all the zone bind parameters.
+
+Required properties:
+- type: this is a string containing the zone type. Say 'cpu', 'core', 'mem', 
etc.
+- mask: Bit string: If 'n'th bit is set, then trip point 'n' is writeable.
+
+- passive_delay: number of milliseconds to wait between polls when
+   performing passive cooling.
+- polling_delay: number of milliseconds to wait between polls when checking
+
+- governor: A string containing the default governor for this zone.
+
+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@10{
+temperature = <10>; /* milliCelsius */
+hysteresis = <0>; /* milliCelsius */
+type = <1>;
+};
+crit@125000{
+temperature = <125000>; /* milliCelsius */
+hysteresis = <0>; /* milliCelsius */
+type = <3>;
+};
+};
+bind_params {
+action@0{
+

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

2013-07-09 Thread Eduardo Valentin
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.

Cc: Zhang Rui rui.zh...@intel.com
Cc: linux...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Eduardo Valentin eduardo.valen...@ti.com
---
 .../devicetree/bindings/thermal/thermal.txt|  92 +
 drivers/thermal/Kconfig|  13 +
 drivers/thermal/Makefile   |   1 +
 drivers/thermal/thermal_dt.c   | 412 +
 drivers/thermal/thermal_dt.h   |  44 +++
 include/linux/thermal.h|   3 +
 6 files changed, 565 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/thermal.txt
 create mode 100644 drivers/thermal/thermal_dt.c
 create mode 100644 drivers/thermal/thermal_dt.h

diff --git a/Documentation/devicetree/bindings/thermal/thermal.txt 
b/Documentation/devicetree/bindings/thermal/thermal.txt
new file mode 100644
index 000..2c25ab2
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/thermal.txt
@@ -0,0 +1,92 @@
+
+Thermal Framework Device Tree descriptor
+
+
+This file describes how to define a thermal structure using device tree.
+A thermal structure includes thermal zones and their components, such
+as name, governor, trip points, polling intervals and cooling devices
+binding descriptors. A binding descriptor may contain information on
+how to react, with a cooling stepped action or a weight on a fair share.
+
+
+trip
+
+
+The trip node is a node to describe a point in the temperature domain
+in which the system takes an action. This node describes just the point,
+not the action.
+
+A node describing a trip must contain:
+- temperature: the trip temperature level, in milliCelsius.
+- hysteresis: a (low) hysteresis value on 'temperature'. This is a relative
+value, in milliCelsius.
+- type: the trip type. Here is the type mapping:
+   THERMAL_TRIP_ACTIVE = 0
+   THERMAL_TRIP_PASSIVE = 1
+   THERMAL_TRIP_HOT = 2
+   THERMAL_TRIP_CRITICAL = 3
+
+**
+bind_param
+**
+
+The bind_param node is a node to describe how cooling devices get assigned
+to trip points of the zone. The cooling devices are expected to be loaded
+in the target system.
+
+A node describing a bind_param must contain:
+- cooling_device: A string with the cooling device name.
+- weight: The 'influence' of a particular cooling device on this zone.
+ This is on a percentage scale. The sum of all these weights
+ (for a particular zone) cannot exceed 100.
+- trip_mask: This is a bit mask that gives the binding relation between
+   this thermal zone and cdev, for a particular trip point.
+   If nth bit is set, then the cdev and thermal zone are bound
+   for trip point n.
+
+
+thermal_zone
+
+
+The thermal_zone node is the node containing all the required info
+for describing a thermal zone, including its cdev bindings. The thermal_zone
+node must contain, apart from its own properties, one node containing
+trip nodes and one node containing all the zone bind parameters.
+
+Required properties:
+- type: this is a string containing the zone type. Say 'cpu', 'core', 'mem', 
etc.
+- mask: Bit string: If 'n'th bit is set, then trip point 'n' is writeable.
+
+- passive_delay: number of milliseconds to wait between polls when
+   performing passive cooling.
+- polling_delay: number of milliseconds to wait between polls when checking
+
+- governor: A string containing the default governor for this zone.
+
+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@10{
+temperature = 10; /* milliCelsius */
+hysteresis = 0; /* milliCelsius */
+type = 1;
+};
+crit@125000{
+temperature = 125000; /* milliCelsius */
+hysteresis = 0; /* milliCelsius */
+type = 3;
+};
+};
+bind_params {
+action@0{
+  

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

2013-07-09 Thread R, Durgadoss
Hi Eduardo,

 -Original Message-
 From: Eduardo Valentin [mailto:eduardo.valen...@ti.com]
 Sent: Tuesday, July 09, 2013 7:30 PM
 To: linux...@vger.kernel.org; R, Durgadoss; amit.dan...@samsung.com
 Cc: Zhang, Rui; Eduardo Valentin; linux-kernel@vger.kernel.org
 Subject: [RFC PATCH 2/4] thermal: introduce device tree parser
 
 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.
 
 Cc: Zhang Rui rui.zh...@intel.com
 Cc: linux...@vger.kernel.org
 Cc: linux-kernel@vger.kernel.org
 Signed-off-by: Eduardo Valentin eduardo.valen...@ti.com
 ---

I looked at the Documentation part of this. And it looks good.

At some places you are using ERANGE. Technically, this represents
'Math result not representable'. May be should be use EINVAL 
itself ? I would leave it up to you ;)

Thanks,
Durga

  .../devicetree/bindings/thermal/thermal.txt|  92 +
  drivers/thermal/Kconfig|  13 +
  drivers/thermal/Makefile   |   1 +
  drivers/thermal/thermal_dt.c   | 412 
 +
  drivers/thermal/thermal_dt.h   |  44 +++
  include/linux/thermal.h|   3 +
  6 files changed, 565 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/thermal/thermal.txt
  create mode 100644 drivers/thermal/thermal_dt.c
  create mode 100644 drivers/thermal/thermal_dt.h
 
 diff --git a/Documentation/devicetree/bindings/thermal/thermal.txt
 b/Documentation/devicetree/bindings/thermal/thermal.txt
 new file mode 100644
 index 000..2c25ab2
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/thermal/thermal.txt
 @@ -0,0 +1,92 @@
 +
 +Thermal Framework Device Tree descriptor
 +
 +
 +This file describes how to define a thermal structure using device tree.
 +A thermal structure includes thermal zones and their components, such
 +as name, governor, trip points, polling intervals and cooling devices
 +binding descriptors. A binding descriptor may contain information on
 +how to react, with a cooling stepped action or a weight on a fair share.
 +
 +
 +trip
 +
 +
 +The trip node is a node to describe a point in the temperature domain
 +in which the system takes an action. This node describes just the point,
 +not the action.
 +
 +A node describing a trip must contain:
 +- temperature: the trip temperature level, in milliCelsius.
 +- hysteresis: a (low) hysteresis value on 'temperature'. This is a relative
 +value, in milliCelsius.
 +- type: the trip type. Here is the type mapping:
 + THERMAL_TRIP_ACTIVE = 0
 + THERMAL_TRIP_PASSIVE = 1
 + THERMAL_TRIP_HOT = 2
 + THERMAL_TRIP_CRITICAL = 3
 +
 +**
 +bind_param
 +**
 +
 +The bind_param node is a node to describe how cooling devices get assigned
 +to trip points of the zone. The cooling devices are expected to be loaded
 +in the target system.
 +
 +A node describing a bind_param must contain:
 +- cooling_device: A string with the cooling device name.
 +- weight: The 'influence' of a particular cooling device on this zone.
 + This is on a percentage scale. The sum of all these weights
 + (for a particular zone) cannot exceed 100.
 +- trip_mask: This is a bit mask that gives the binding relation between
 +   this thermal zone and cdev, for a particular trip point.
 +   If nth bit is set, then the cdev and thermal zone are bound
 +   for trip point n.
 +
 +
 +thermal_zone
 +
 +
 +The thermal_zone node is the node containing all the required info
 +for describing a thermal zone, including its cdev bindings. The thermal_zone
 +node must contain, apart from its own properties, one node containing
 +trip nodes and one node containing all the zone bind parameters.
 +
 +Required properties:
 +- type: this is a string containing the zone type. Say 'cpu', 'core', 'mem', 
 etc.
 +- mask: Bit string: If 'n'th bit is set, then trip point 'n' is writeable.
 +
 +- passive_delay: number of milliseconds to wait between polls when
 + performing passive cooling.
 +- polling_delay: number of milliseconds to wait between polls when checking
 +
 +- governor: A string containing the default governor for this zone.
 +
 +Below is an example:
 +thermal_zone {
 +type = CPU;
 +mask = 0x03; /* trips writability */
 +passive_delay