Message-ID: <2523089.oKCvZdz31U@flatron>
Date:	Wed, 13 Nov 2013 17:57:22 +0100
From:	Tomasz Figa <tomasz.figa@...il.com>
To:	Eduardo Valentin <eduardo.valentin@...com>
Cc:	swarren@...dotorg.org, pawel.moll@....com, mark.rutland@....com,
	ian.campbell@...rix.com, rob.herring@...xeda.com,
	linux@...ck-us.net, rui.zhang@...el.com, wni@...dia.com,
	grant.likely@...aro.org, durgadoss.r@...el.com,
	linux-pm@...r.kernel.org, devicetree@...r.kernel.org,
	lm-sensors@...sensors.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCHv9 02/20] thermal: introduce device tree parser

Hi Eduardo,

On Tuesday 12 of November 2013 15:46:04 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.
[snip]
> 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 @@
[snip]
> +* 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

Could you explain (just in a reply) what a cooling state is and what are
the min and max values used for?

> +
> +- 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.
> +
> +* 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

What about replacing temperature and hysteresis with a single temperature
property that can be either one cell for 0 hysteresis or two cells to
specify lower and upper range of temperatures?

> +
> +- type:			a string containing the trip type. Supported values are:
> +	"active":	A trip point to enable active cooling
> +	"passive":	A trip point to enable passive cooling

The two above seem to be implying action, as opposed to the general
comment about trip points.

> +	"hot":		A trip point to notify emergency
> +	"critical":	Hardware not reliable.
> +  Type: string
> +
[snip]
> +* Examples
> +
> +Below are several examples on how to use thermal data descriptors
> +using device tree bindings:
> +
> +(a) - CPU thermal zone
> +
> +The CPU thermal zone example below describes how to setup one thermal zone
> +using one single sensor as temperature source and many cooling devices and
> +power dissipation control sources.
> +
> +#include <dt-bindings/thermal/thermal.h>
> +
> +cpus {
> +	/*
> +	 * Here is an example of describing a cooling device for a DVFS
> +	 * capable CPU. The CPU node describes its four OPPs.
> +	 * The cooling states possible are 0..3, and they are
> +	 * used as OPP indexes. The minimum cooling state is 0, which means
> +	 * all four OPPs can be available to the system. The maximum
> +	 * cooling state is 3, which means only the lowest OPPs (198MHz@...5V)
> +	 * can be available in the system.
> +	 */
> +	cpu0: cpu@0 {
> +		...
> +		operating-points = <
> +			/* kHz    uV */
> +			970000  1200000
> +			792000  1100000
> +			396000  950000
> +			198000  850000
> +		>;
> +		cooling-min-state = <0>;
> +		cooling-max-state = <3>;
> +		#cooling-cells = <2>; /* min followed by max */

I believe you don't need the min- and max-state properties here then. Your
thermal core can simply query the cpufreq driver (which would be a cooling
device here) about the range of states it supports.

> +	};
> +	...
> +};
> +
> +&i2c1 {
> +	...
> +	/*
> +	 * A simple fan controller which supports 10 speeds of operation
> +	 * (represented as 0-9).
> +	 */
> +	fan0: fan@...8 {
> +		...
> +		cooling-min-state = <0>;
> +		cooling-max-state = <9>;

This is similar. The fan driver will probaby know about available
fan speed levels and be able to report the range of states to thermal
core.

That's just for the binding. I will try to review the code when I find
a bit more time.

Best regards,
Tomasz

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

Powered by blists - more mailing lists