Re: [PATCHv9 02/20] thermal: introduce device tree parser

Eduardo Valentin
2013-11-25T15:47:17Z

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

On 25-11-2013 11:37, Mark Rutland wrote:
> On Tue, Nov 12, 2013 at 07:46:04PM +0000, Eduardo Valentin wrote:
>> This patch introduces a device tree bindings for
>> describing the hardware thermal behavior and limits.
>> Also a parser to read and interpret the data and feed
>> it in the thermal framework is presented.
>>
>> 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 tree nodes to use
>> this infrastructure.
>>
>> Note that, in order to be able to have control
>> on the sensor registration on the DT thermal zone,
>> it was required to allow changing the thermal zone
>> .get_temp callback. For this reason, this patch
>> also removes the 'const' modifier from the .ops
>> field of thermal zone devices.
>>
>> Cc: Zhang Rui <rui.zhang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
>> Signed-off-by: Eduardo Valentin <eduardo.valentin-l0cyMroinI0@public.gmane.org>
>> ---
>>
>> Hello all,
>>
>> Very minor changes from v8 to v9.
>>
>> Changelog:
>>
>> - Rephrase a couple of sentences in the binding document
>> - Fixed a couple of property types in the binding document
>> - Removed the constant macro definitions for trip type. This
>> change also affected the bindings posted on patches 09/14/15.
>>
>> .../devicetree/bindings/thermal/thermal.txt | 586 ++++++++++++++
>> drivers/thermal/Kconfig | 13 +
>> drivers/thermal/Makefile | 1 +
>> drivers/thermal/of-thermal.c | 849 +++++++++++++++++++++
>> drivers/thermal/thermal_core.c | 9 +-
>> drivers/thermal/thermal_core.h | 9 +
>> include/dt-bindings/thermal/thermal.h | 17 +
>> include/linux/thermal.h | 28 +-
>> 8 files changed, 1509 insertions(+), 3 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/thermal/thermal.txt
>> create mode 100644 drivers/thermal/of-thermal.c
>> create mode 100644 include/dt-bindings/thermal/thermal.h
>>
>> diff --git a/Documentation/devicetree/bindings/thermal/thermal.txt b/Documentation/devicetree/bindings/thermal/thermal.txt
>> new file mode 100644
>> index 0000000..59f5bd2
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/thermal/thermal.txt
>> @@ -0,0 +1,586 @@
>> +* Thermal Framework Device Tree descriptor
>> +
>> +This file describes a generic binding to provide a way of
>> +defining hardware thermal structure using device tree.
>> +A thermal structure includes thermal zones and their components,
>> +such as trip points, polling intervals, sensors and cooling devices
>> +binding descriptors.
>> +
>> +The target of device tree thermal descriptors is to describe only
>> +the hardware thermal aspects. The thermal device tree bindings are
>> +not about how the system must control or which algorithm or policy
>> +must be taken in place.
>> +
>> +There are five types of nodes involved to describe thermal bindings:
>> +- thermal sensors: devices which may be used to take temperature
>> + measurements.
>> +- cooling devices: devices which may be used to dissipate heat.
>> +- trip points: describe key temperatures at which cooling is recommended. The
>> + set of points should be chosen based on hardware limits.
>> +- cooling maps: used to describe links between trip points and cooling devices;
>> +- thermal zones: used to describe thermal data within the hardware;
>> +
>> +The following is a description of each of these node types.
>> +
>> +* Thermal sensor devices
>> +
>> +Thermal sensor devices are nodes providing temperature sensing capabilities on
>> +thermal zones. Typical devices are I2C ADC converters and bandgaps. These are
>> +nodes providing temperature data to thermal zones. Thermal sensor devices may
>> +control one or more internal sensors.
>> +
>> +Required property:
>> +- #thermal-sensor-cells: Used to provide sensor device specific information
>> + Type: unsigned while referring to it. Typically 0 on thermal sensor
>> + Size: one cell nodes with only one sensor, and at least 1 on nodes
>> + with several internal sensors, in order
>> + to identify uniquely the sensor instances within
>> + the IC. See thermal zone binding for more details
>> + on how consumers refer to sensor devices.
>> +
>> +* Cooling device nodes
>> +
>> +Cooling devices are nodes providing control on power dissipation. There
>> +are essentially two ways to provide control on power dissipation. First
>> +is by means of regulating device performance, which is known as passive
>> +cooling. A typical passive cooling is a CPU that has dynamic voltage and
>> +frequency scaling (DVFS), and uses lower frequencies as cooling states.
>> +Second is by means of activating devices in order to remove
>> +the dissipated heat, which is known as active cooling, e.g. regulating
>> +fan speeds. In both cases, cooling devices shall have a way to determine
>> +the state of cooling in which the device is.
>> +
>> +Required properties:
>> +- cooling-min-state: An integer indicating the smallest
>> + Type: unsigned cooling state accepted. Typically 0.
>> + Size: one cell
>> +
>> +- cooling-max-state: An integer indicating the largest
>> + Type: unsigned cooling state accepted.
>> + Size: one cell
>> +
>> +- #cooling-cells: Used to provide cooling device specific information
>> + Type: unsigned while referring to it. Must be at least 2, in order
>> + Size: one cell to specify minimum and maximum cooling state used
>> + in the reference. The first cell is the minimum
>> + cooling state requested and the second cell is
>> + the maximum cooling state requested in the reference.
>> + See Cooling device maps section below for more details
>> + on how consumers refer to cooling devices.
>
> This is the point I'm still most concerned with, as it provides a
> definite meaning to an otherwise abstract property. However, if you
> believe this is sane and unlikely to be problematic, then I am happy to
> leave the decision to you.
>
>> +
>> +* Trip points
>> +
>> +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.
>> +
>> +Required properties:
>> +- temperature: An integer indicating the trip temperature level,
>> + Type: signed in millicelsius.
>> + Size: one cell
>> +
>> +- hysteresis: A low hysteresis value on temperature property (above).
>> + Type: unsigned This is a relative value, in millicelsius.
>> + Size: one cell
>> +
>> +- type: a string containing the trip type. Supported values are:
>
> Get rid of the "Supported", that's a Linux detail.
>
> s/Supported/expected/ ?

OK.

>
> Given the discussion has otherwise boiled down to bits that I think can
> change later, I'm happy to give my ack:
>
> Acked-by: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
>

Thanks for your valuable contributions to this work. For sure it would
not be at the state it is without your punctual and objective review.

> I'll leave it to the others to fight against this if they still have
> concerns.
>
> Thanks,
> Mark.
>
>

--
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 --]