* Re: [PATCH v3 2/4] devicetree: bindings: let thermal-sensor point to other thermal zones
2015-11-25 15:09 ` [PATCH v3 2/4] devicetree: bindings: let thermal-sensor point to other thermal zones Javi Merino
@ 2015-11-25 17:54 ` Mark Rutland
2015-11-25 18:41 ` Javi Merino
2016-01-04 14:17 ` Sascha Hauer
2016-03-03 3:19 ` Eduardo Valentin
2 siblings, 1 reply; 8+ messages in thread
From: Mark Rutland @ 2015-11-25 17:54 UTC (permalink / raw)
To: Javi Merino
Cc: linux-pm, linux-kernel, rui.zang, edubezval, Zhang Rui,
Rob Herring, Pawel Moll, Ian Campbell, Kumar Gala, devicetree
On Wed, Nov 25, 2015 at 03:09:44PM +0000, Javi Merino wrote:
> The thermal-sensor property of the thermal zone node accepts phandles to
> thermal sensors. However, thermal zones can be created as an
> aggregation of other thermal zones. Extend the thermal-sensors property
> to allow phandles to other thermal zones. This patch also adds an
> example that showcases how a board thermal zone can be created from the
> aggregation of the cpu, gpu and lcd thermal zones.
>
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Javi Merino <javi.merino@arm.com>
> ---
>
> Notes:
> Hi devicetree,
>
> Is it ok to extend the definition of the thermal-sensors property like
> this? IOW are phandles strongly typed?
I think it's OK so long as each thermal zone has #theremal-sensor-cells
set explicitly, if used as a sensor, and we can agree on the semantics
of what it means for a thermal zone to be a sensor.
I don't really follow why you need the zone to be a sensor, and can't
simply refer to the sensor from two zones. Are you trying to imply an
ordering of trip points (e.g. that the sub-zones' trips should be taken
into account first)?
>
> .../devicetree/bindings/thermal/thermal.txt | 154 ++++++++++++++++++++-
> 1 file changed, 151 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/thermal/thermal.txt b/Documentation/devicetree/bindings/thermal/thermal.txt
> index 41b817f7b670..52b7e9ae3b4d 100644
> --- a/Documentation/devicetree/bindings/thermal/thermal.txt
> +++ b/Documentation/devicetree/bindings/thermal/thermal.txt
> @@ -145,9 +145,12 @@ Required properties:
> Size: one cell
>
> - thermal-sensors: A list of thermal sensor phandles and sensor specifier
> - Type: list of used while monitoring the thermal zone.
> - phandles + sensor
> - specifier
> + Type: list of used while monitoring the thermal zone. The phandles
> + phandles + sensor can point to thermal sensors or other thermal zone
> + specifier nodes. If it points to other thermal zone
> + nodes you should omit the sensor specifier
> + and set #thermal-sensor-cells to 0 for the
> + thermal zone.
The example misses #thermal-sensor-cells = <0> for each of the zones.
Can a zone normal have multiple sensors? If so, what is the aggregate
value if a zone is used as a sensor? Max? Min? Scaled by contribution
somehow?
Thanks,
Mark.
>
> - trips: A sub-node which is a container of only trip point nodes
> Type: sub-node required to describe the thermal zone.
> @@ -603,3 +606,148 @@ thermal-zones {
> The above example is a mix of previous examples, a sensor IP with several internal
> sensors used to monitor different zones, one of them is composed by several sensors and
> with different cooling devices.
> +
> +(e) Board thermal with stacked thermal zones
> +
> +Instead of setting up one thermal zone combining multiple thermal
> +zones and multiple trip points for each cooling device, we can create
> +a hierarchy of thermal zones.
> +
> +#include <dt-bindings/thermal/thermal.h>
> +
> +&i2c1 {
> + ...
> + /*
> + * An IC with several temperature sensor.
> + */
> + adc_dummy: sensor@0x50 {
> + ...
> + #thermal-sensor-cells = <1>; /* sensor internal ID */
> + };
> +};
> +
> +thermal-zones {
> +
> + cpu_thermal: cpu_thermal {
> + polling-delay-passive = <1000>; /* milliseconds */
> + polling-delay = <2500>; /* milliseconds */
> +
> + sustainable-power = <2500>;
> +
> + thermal-sensors = <&adc_dummy 0>
> +
> + trips {
> + cpu_trip: cpu-trip {
> + temperature = <60000>; /* millicelsius */
> + hysteresis = <2000>; /* millicelsius */
> + type = "passive";
> + };
> + };
> +
> + cooling-maps {
> + map0 {
> + trip = <&cpu_trip>;
> + cooling-device = <&cpu0 0 2>;
> + };
> + };
> + };
> +
> + gpu_thermal: gpu_thermal {
> + polling-delay-passive = <1000>; /* milliseconds */
> + polling-delay = <2500>; /* milliseconds */
> +
> + sustainable-power = <2500>;
> +
> + thermal-sensors = <&adc_dummy 2>
> +
> + trips {
> + gpu_trip: gpu-trip {
> + temperature = <55000>; /* millicelsius */
> + hysteresis = <2000>; /* millicelsius */
> + type = "passive";
> + }
> + };
> +
> + cooling-maps {
> + map0 {
> + trip = <&gpu_trip>;
> + cooling-device = <&gpu0 0 2>;
> + };
> + };
> + };
> +
> + lcd_thermal: lcd_thermal {
> + polling-delay-passive = <1000>; /* milliseconds */
> + polling-delay = <2500>; /* milliseconds */
> +
> + sustainable-power = <2500>;
> +
> + thermal-sensors = <&adc_dummy 1>
> +
> + trips {
> + lcd_trip: lcp-trip {
> + temperature = <53000>; /* millicelsius */
> + hysteresis = <2000>; /* millicelsius */
> + type = "passive";
> + };
> + };
> +
> + cooling-maps {
> + map0 {
> + trip = <&lcd_trip>;
> + cooling-device = <&lcd0 5 10>;
> + };
> + };
> + };
> +
> + board_thermal: board-thermal {
> + polling-delay-passive = <1000>; /* milliseconds */
> + polling-delay = <2500>; /* milliseconds */
> +
> + thermal-sensors = <&cpu_thermal &gpu_thermal &lcd_thermal>
> +
> + sustainable-power = <2500>;
> +
> + trips {
> + warm_trip: warm-trip {
> + temperature = <62000>; /* millicelsius */
> + hysteresis = <2000>; /* millicelsius */
> + type = "passive";
> + };
> + crit_trip: crit-trip {
> + temperature = <68000>; /* millicelsius */
> + hysteresis = <2000>; /* millicelsius */
> + type = "critical";
> + };
> + };
> +
> + cooling-maps {
> + map0 {
> + trip = <&warm_trip>;
> + cooling-device = <&cpu0 2 2>;
> + contribution = <55>;
> + };
> + map1 {
> + trip = <&warm_trip>;
> + cooling-device = <&gpu0 2 2>;
> + contribution = <20>;
> + };
> + map2 {
> + trip = <&lcd_trip>;
> + cooling-device = <&lcd0 7 10>;
> + contribution = <15>;
> + };
> + };
> + };
> +};
> +
> +The above example is a different take at example (d). We create one
> +thermal zone per sensor: cpu_thermal, gpu_thermal and lcd_thermal.
> +Each of which has its own trip point for each own cooling device. We
> +then define a board_thermal thermal zone that is a combination of all
> +the other thermal zones. If the board hits its warm_trip, then all
> +cooling devices are throttled.
> +
> +This example illustrates how we can throttle each device individually
> +if its too hot and at the same time have some control over the whole
> +system.
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v3 2/4] devicetree: bindings: let thermal-sensor point to other thermal zones
2015-11-25 17:54 ` Mark Rutland
@ 2015-11-25 18:41 ` Javi Merino
0 siblings, 0 replies; 8+ messages in thread
From: Javi Merino @ 2015-11-25 18:41 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
rui.zang-ral2JQCrhuEAvxtiuMwx3w, edubezval-Re5JQEeQqe8AvxtiuMwx3w,
Zhang Rui, Rob Herring, Pawel Moll, Ian Campbell, Kumar Gala,
devicetree-u79uwXL29TY76Z2rM5mHXA
On Wed, Nov 25, 2015 at 05:54:41PM +0000, Mark Rutland wrote:
> On Wed, Nov 25, 2015 at 03:09:44PM +0000, Javi Merino wrote:
> > The thermal-sensor property of the thermal zone node accepts phandles to
> > thermal sensors. However, thermal zones can be created as an
> > aggregation of other thermal zones. Extend the thermal-sensors property
> > to allow phandles to other thermal zones. This patch also adds an
> > example that showcases how a board thermal zone can be created from the
> > aggregation of the cpu, gpu and lcd thermal zones.
> >
> > Cc: Zhang Rui <rui.zhang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > Cc: Eduardo Valentin <edubezval-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
> > Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> > Cc: Ian Campbell <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
> > Cc: Kumar Gala <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> > Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Signed-off-by: Javi Merino <javi.merino-5wv7dgnIgG8@public.gmane.org>
> > ---
> >
> > Notes:
> > Hi devicetree,
> >
> > Is it ok to extend the definition of the thermal-sensors property like
> > this? IOW are phandles strongly typed?
>
> I think it's OK so long as each thermal zone has #thermal-sensor-cells
> set explicitly, if used as a sensor, and we can agree on the semantics
> of what it means for a thermal zone to be a sensor.
>
> I don't really follow why you need the zone to be a sensor, and can't
> simply refer to the sensor from two zones. Are you trying to imply an
> ordering of trip points (e.g. that the sub-zones' trips should be taken
> into account first)?
No, it doesn't affect the ordering of trip points.
This came out of a discussion at LPC. Currently thermal zones can
only have on thermal sensor associated with them. After some
discussion, Mike Turquette suggested that we could use an approach
similar to what it's done with power domains and stack them.
> > .../devicetree/bindings/thermal/thermal.txt | 154 ++++++++++++++++++++-
> > 1 file changed, 151 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/thermal/thermal.txt b/Documentation/devicetree/bindings/thermal/thermal.txt
> > index 41b817f7b670..52b7e9ae3b4d 100644
> > --- a/Documentation/devicetree/bindings/thermal/thermal.txt
> > +++ b/Documentation/devicetree/bindings/thermal/thermal.txt
> > @@ -145,9 +145,12 @@ Required properties:
> > Size: one cell
> >
> > - thermal-sensors: A list of thermal sensor phandles and sensor specifier
> > - Type: list of used while monitoring the thermal zone.
> > - phandles + sensor
> > - specifier
> > + Type: list of used while monitoring the thermal zone. The phandles
> > + phandles + sensor can point to thermal sensors or other thermal zone
> > + specifier nodes. If it points to other thermal zone
> > + nodes you should omit the sensor specifier
> > + and set #thermal-sensor-cells to 0 for the
> > + thermal zone.
>
> The example misses #thermal-sensor-cells = <0> for each of the zones.
You're right, I'll fix it for the next version
> Can a zone normal have multiple sensors? If so, what is the aggregate
> value if a zone is used as a sensor? Max? Min? Scaled by contribution
> somehow?
No, currently a thermal zone can only specify one sensor in its
thermal-sensors property
Cheers,
Javi
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/4] devicetree: bindings: let thermal-sensor point to other thermal zones
2015-11-25 15:09 ` [PATCH v3 2/4] devicetree: bindings: let thermal-sensor point to other thermal zones Javi Merino
2015-11-25 17:54 ` Mark Rutland
@ 2016-01-04 14:17 ` Sascha Hauer
2016-03-03 3:21 ` Eduardo Valentin
2016-03-03 3:19 ` Eduardo Valentin
2 siblings, 1 reply; 8+ messages in thread
From: Sascha Hauer @ 2016-01-04 14:17 UTC (permalink / raw)
To: Javi Merino
Cc: linux-pm, linux-kernel, rui.zang, edubezval, Zhang Rui,
Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
devicetree
On Wed, Nov 25, 2015 at 03:09:44PM +0000, Javi Merino wrote:
> The thermal-sensor property of the thermal zone node accepts phandles to
> thermal sensors. However, thermal zones can be created as an
> aggregation of other thermal zones. Extend the thermal-sensors property
> to allow phandles to other thermal zones. This patch also adds an
> example that showcases how a board thermal zone can be created from the
> aggregation of the cpu, gpu and lcd thermal zones.
>
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Javi Merino <javi.merino@arm.com>
> ---
>
> Notes:
> Hi devicetree,
>
> Is it ok to extend the definition of the thermal-sensors property like
> this? IOW are phandles strongly typed?
>
> .../devicetree/bindings/thermal/thermal.txt | 154 ++++++++++++++++++++-
> 1 file changed, 151 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/thermal/thermal.txt b/Documentation/devicetree/bindings/thermal/thermal.txt
> index 41b817f7b670..52b7e9ae3b4d 100644
> --- a/Documentation/devicetree/bindings/thermal/thermal.txt
> +++ b/Documentation/devicetree/bindings/thermal/thermal.txt
> @@ -145,9 +145,12 @@ Required properties:
> Size: one cell
>
> - thermal-sensors: A list of thermal sensor phandles and sensor specifier
> - Type: list of used while monitoring the thermal zone.
> - phandles + sensor
> - specifier
> + Type: list of used while monitoring the thermal zone. The phandles
> + phandles + sensor can point to thermal sensors or other thermal zone
> + specifier nodes. If it points to other thermal zone
> + nodes you should omit the sensor specifier
> + and set #thermal-sensor-cells to 0 for the
> + thermal zone.
>
> - trips: A sub-node which is a container of only trip point nodes
> Type: sub-node required to describe the thermal zone.
> @@ -603,3 +606,148 @@ thermal-zones {
> The above example is a mix of previous examples, a sensor IP with several internal
> sensors used to monitor different zones, one of them is composed by several sensors and
> with different cooling devices.
> +
> +(e) Board thermal with stacked thermal zones
> +
> +Instead of setting up one thermal zone combining multiple thermal
> +zones and multiple trip points for each cooling device, we can create
> +a hierarchy of thermal zones.
> +
> +#include <dt-bindings/thermal/thermal.h>
> +
> +&i2c1 {
> + ...
> + /*
> + * An IC with several temperature sensor.
> + */
> + adc_dummy: sensor@0x50 {
> + ...
> + #thermal-sensor-cells = <1>; /* sensor internal ID */
> + };
> +};
> +
> +thermal-zones {
> +
> + cpu_thermal: cpu_thermal {
> + polling-delay-passive = <1000>; /* milliseconds */
> + polling-delay = <2500>; /* milliseconds */
> +
> + sustainable-power = <2500>;
> +
> + thermal-sensors = <&adc_dummy 0>
> +
> + trips {
> + cpu_trip: cpu-trip {
> + temperature = <60000>; /* millicelsius */
> + hysteresis = <2000>; /* millicelsius */
> + type = "passive";
> + };
> + };
> +
> + cooling-maps {
> + map0 {
> + trip = <&cpu_trip>;
> + cooling-device = <&cpu0 0 2>;
> + };
> + };
> + };
> +
> + gpu_thermal: gpu_thermal {
> + polling-delay-passive = <1000>; /* milliseconds */
> + polling-delay = <2500>; /* milliseconds */
> +
> + sustainable-power = <2500>;
> +
> + thermal-sensors = <&adc_dummy 2>
> +
> + trips {
> + gpu_trip: gpu-trip {
> + temperature = <55000>; /* millicelsius */
> + hysteresis = <2000>; /* millicelsius */
> + type = "passive";
> + }
> + };
> +
> + cooling-maps {
> + map0 {
> + trip = <&gpu_trip>;
> + cooling-device = <&gpu0 0 2>;
> + };
> + };
> + };
> +
> + lcd_thermal: lcd_thermal {
> + polling-delay-passive = <1000>; /* milliseconds */
> + polling-delay = <2500>; /* milliseconds */
> +
> + sustainable-power = <2500>;
> +
> + thermal-sensors = <&adc_dummy 1>
> +
> + trips {
> + lcd_trip: lcp-trip {
> + temperature = <53000>; /* millicelsius */
> + hysteresis = <2000>; /* millicelsius */
> + type = "passive";
> + };
> + };
> +
> + cooling-maps {
> + map0 {
> + trip = <&lcd_trip>;
> + cooling-device = <&lcd0 5 10>;
> + };
> + };
> + };
> +
> + board_thermal: board-thermal {
> + polling-delay-passive = <1000>; /* milliseconds */
> + polling-delay = <2500>; /* milliseconds */
> +
> + thermal-sensors = <&cpu_thermal &gpu_thermal &lcd_thermal>
This seems inconsistent. Why can a thermal zone only have multiple
thermal sensors when they are thermal zones themselves? Either we assume
that one thermal zone has a single sensor or we assume that it can have
multiple sensors, but this should not depend on the zone being a sub zone
or not.
I think the thermal-sensors property should always point to one or
multiple sensors. I see no point in "This property either points to
exactly one sensor or multiple other thermal zones (from which we only
use the temperature)"
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v3 2/4] devicetree: bindings: let thermal-sensor point to other thermal zones
2016-01-04 14:17 ` Sascha Hauer
@ 2016-03-03 3:21 ` Eduardo Valentin
2016-03-21 11:55 ` Javi Merino
0 siblings, 1 reply; 8+ messages in thread
From: Eduardo Valentin @ 2016-03-03 3:21 UTC (permalink / raw)
To: Sascha Hauer
Cc: Javi Merino, linux-pm, linux-kernel, rui.zang, Zhang Rui,
Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
devicetree
On Mon, Jan 04, 2016 at 03:17:09PM +0100, Sascha Hauer wrote:
> On Wed, Nov 25, 2015 at 03:09:44PM +0000, Javi Merino wrote:
> > The thermal-sensor property of the thermal zone node accepts phandles to
> > thermal sensors. However, thermal zones can be created as an
> > +
> > + thermal-sensors = <&cpu_thermal &gpu_thermal &lcd_thermal>
>
> This seems inconsistent. Why can a thermal zone only have multiple
> thermal sensors when they are thermal zones themselves? Either we assume
> that one thermal zone has a single sensor or we assume that it can have
> multiple sensors, but this should not depend on the zone being a sub zone
> or not.
>
> I think the thermal-sensors property should always point to one or
> multiple sensors. I see no point in "This property either points to
> exactly one sensor or multiple other thermal zones (from which we only
> use the temperature)"
Agreed here. In fact, if we are going to allow thermal zones to be
treated as sensors, it means there should be no limits on what you put
of there, as long as all items have #thermal-sensor-cells. So, mixing
one (or more) regular sensors, with other thermal zones shall be
allowed, if we agree in this semantics.
>
> Sascha
>
>
> --
> Pengutronix e.K. | |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/4] devicetree: bindings: let thermal-sensor point to other thermal zones
2016-03-03 3:21 ` Eduardo Valentin
@ 2016-03-21 11:55 ` Javi Merino
2016-03-22 15:13 ` Eduardo Valentin
0 siblings, 1 reply; 8+ messages in thread
From: Javi Merino @ 2016-03-21 11:55 UTC (permalink / raw)
To: Eduardo Valentin
Cc: Sascha Hauer, linux-pm, linux-kernel, rui.zang, Zhang Rui,
Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
devicetree
On Wed, Mar 02, 2016 at 07:21:53PM -0800, Eduardo Valentin wrote:
> On Mon, Jan 04, 2016 at 03:17:09PM +0100, Sascha Hauer wrote:
> > On Wed, Nov 25, 2015 at 03:09:44PM +0000, Javi Merino wrote:
> > > The thermal-sensor property of the thermal zone node accepts phandles to
> > > thermal sensors. However, thermal zones can be created as an
> > > +
> > > + thermal-sensors = <&cpu_thermal &gpu_thermal &lcd_thermal>
> >
> > This seems inconsistent. Why can a thermal zone only have multiple
> > thermal sensors when they are thermal zones themselves? Either we assume
> > that one thermal zone has a single sensor or we assume that it can have
> > multiple sensors, but this should not depend on the zone being a sub zone
> > or not.
> >
> > I think the thermal-sensors property should always point to one or
> > multiple sensors. I see no point in "This property either points to
> > exactly one sensor or multiple other thermal zones (from which we only
> > use the temperature)"
>
>
> Agreed here. In fact, if we are going to allow thermal zones to be
> treated as sensors, it means there should be no limits on what you put
> of there, as long as all items have #thermal-sensor-cells. So, mixing
> one (or more) regular sensors, with other thermal zones shall be
> allowed, if we agree in this semantics.
Eduardo, thanks for the review. There doesn't seem to be much
interest in this and I currently have no time to work on it so I am
dropping this series for the time being. I'm happy for this to be
picked up by somebody else (or who knows, maybe I will be able to work
on it again in the future).
Javi
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/4] devicetree: bindings: let thermal-sensor point to other thermal zones
2016-03-21 11:55 ` Javi Merino
@ 2016-03-22 15:13 ` Eduardo Valentin
0 siblings, 0 replies; 8+ messages in thread
From: Eduardo Valentin @ 2016-03-22 15:13 UTC (permalink / raw)
To: Javi Merino
Cc: Sascha Hauer, linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
rui.zang-ral2JQCrhuEAvxtiuMwx3w, Zhang Rui, Rob Herring,
Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
devicetree-u79uwXL29TY76Z2rM5mHXA
On Mon, Mar 21, 2016 at 11:55:11AM +0000, Javi Merino wrote:
> On Wed, Mar 02, 2016 at 07:21:53PM -0800, Eduardo Valentin wrote:
> > On Mon, Jan 04, 2016 at 03:17:09PM +0100, Sascha Hauer wrote:
> > > On Wed, Nov 25, 2015 at 03:09:44PM +0000, Javi Merino wrote:
> > > > The thermal-sensor property of the thermal zone node accepts phandles to
> > > > thermal sensors. However, thermal zones can be created as an
> > > > +
> > > > + thermal-sensors = <&cpu_thermal &gpu_thermal &lcd_thermal>
> > >
> > > This seems inconsistent. Why can a thermal zone only have multiple
> > > thermal sensors when they are thermal zones themselves? Either we assume
> > > that one thermal zone has a single sensor or we assume that it can have
> > > multiple sensors, but this should not depend on the zone being a sub zone
> > > or not.
> > >
> > > I think the thermal-sensors property should always point to one or
> > > multiple sensors. I see no point in "This property either points to
> > > exactly one sensor or multiple other thermal zones (from which we only
> > > use the temperature)"
> >
> >
> > Agreed here. In fact, if we are going to allow thermal zones to be
> > treated as sensors, it means there should be no limits on what you put
> > of there, as long as all items have #thermal-sensor-cells. So, mixing
> > one (or more) regular sensors, with other thermal zones shall be
> > allowed, if we agree in this semantics.
>
> Eduardo, thanks for the review. There doesn't seem to be much
> interest in this and I currently have no time to work on it so I am
> dropping this series for the time being. I'm happy for this to be
> picked up by somebody else (or who knows, maybe I will be able to work
> on it again in the future).
Ok Javi, as this was one of the topics agreed for a change, I will take it over.
Thanks for your help.
>
> Javi
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/4] devicetree: bindings: let thermal-sensor point to other thermal zones
2015-11-25 15:09 ` [PATCH v3 2/4] devicetree: bindings: let thermal-sensor point to other thermal zones Javi Merino
2015-11-25 17:54 ` Mark Rutland
2016-01-04 14:17 ` Sascha Hauer
@ 2016-03-03 3:19 ` Eduardo Valentin
2 siblings, 0 replies; 8+ messages in thread
From: Eduardo Valentin @ 2016-03-03 3:19 UTC (permalink / raw)
To: Javi Merino
Cc: linux-pm, linux-kernel, rui.zang, Zhang Rui, Rob Herring,
Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, devicetree
On Wed, Nov 25, 2015 at 03:09:44PM +0000, Javi Merino wrote:
> The thermal-sensor property of the thermal zone node accepts phandles to
> thermal sensors. However, thermal zones can be created as an
> aggregation of other thermal zones. Extend the thermal-sensors property
> to allow phandles to other thermal zones. This patch also adds an
> example that showcases how a board thermal zone can be created from the
> aggregation of the cpu, gpu and lcd thermal zones.
>
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Javi Merino <javi.merino@arm.com>
> ---
>
> Notes:
> Hi devicetree,
>
> Is it ok to extend the definition of the thermal-sensors property like
> this? IOW are phandles strongly typed?
>
> .../devicetree/bindings/thermal/thermal.txt | 154 ++++++++++++++++++++-
> 1 file changed, 151 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/thermal/thermal.txt b/Documentation/devicetree/bindings/thermal/thermal.txt
> index 41b817f7b670..52b7e9ae3b4d 100644
> --- a/Documentation/devicetree/bindings/thermal/thermal.txt
> +++ b/Documentation/devicetree/bindings/thermal/thermal.txt
> @@ -145,9 +145,12 @@ Required properties:
> Size: one cell
>
> - thermal-sensors: A list of thermal sensor phandles and sensor specifier
> - Type: list of used while monitoring the thermal zone.
> - phandles + sensor
> - specifier
> + Type: list of used while monitoring the thermal zone. The phandles
> + phandles + sensor can point to thermal sensors or other thermal zone
> + specifier nodes. If it points to other thermal zone
> + nodes you should omit the sensor specifier
> + and set #thermal-sensor-cells to 0 for the
> + thermal zone.
>
> - trips: A sub-node which is a container of only trip point nodes
> Type: sub-node required to describe the thermal zone.
> @@ -603,3 +606,148 @@ thermal-zones {
> The above example is a mix of previous examples, a sensor IP with several internal
> sensors used to monitor different zones, one of them is composed by several sensors and
> with different cooling devices.
> +
> +(e) Board thermal with stacked thermal zones
> +
> +Instead of setting up one thermal zone combining multiple thermal
> +zones and multiple trip points for each cooling device, we can create
> +a hierarchy of thermal zones.
> +
> +#include <dt-bindings/thermal/thermal.h>
> +
> +&i2c1 {
> + ...
> + /*
> + * An IC with several temperature sensor.
> + */
> + adc_dummy: sensor@0x50 {
> + ...
> + #thermal-sensor-cells = <1>; /* sensor internal ID */
> + };
> +};
> +
> +thermal-zones {
> +
> + cpu_thermal: cpu_thermal {
> + polling-delay-passive = <1000>; /* milliseconds */
> + polling-delay = <2500>; /* milliseconds */
> +
> + sustainable-power = <2500>;
> +
> + thermal-sensors = <&adc_dummy 0>
> +
> + trips {
> + cpu_trip: cpu-trip {
> + temperature = <60000>; /* millicelsius */
> + hysteresis = <2000>; /* millicelsius */
> + type = "passive";
> + };
> + };
> +
> + cooling-maps {
> + map0 {
> + trip = <&cpu_trip>;
> + cooling-device = <&cpu0 0 2>;
> + };
> + };
> + };
> +
> + gpu_thermal: gpu_thermal {
> + polling-delay-passive = <1000>; /* milliseconds */
> + polling-delay = <2500>; /* milliseconds */
> +
> + sustainable-power = <2500>;
> +
> + thermal-sensors = <&adc_dummy 2>
> +
> + trips {
> + gpu_trip: gpu-trip {
> + temperature = <55000>; /* millicelsius */
> + hysteresis = <2000>; /* millicelsius */
> + type = "passive";
> + }
> + };
> +
> + cooling-maps {
> + map0 {
> + trip = <&gpu_trip>;
> + cooling-device = <&gpu0 0 2>;
> + };
> + };
> + };
> +
> + lcd_thermal: lcd_thermal {
> + polling-delay-passive = <1000>; /* milliseconds */
> + polling-delay = <2500>; /* milliseconds */
> +
> + sustainable-power = <2500>;
> +
> + thermal-sensors = <&adc_dummy 1>
> +
> + trips {
> + lcd_trip: lcp-trip {
> + temperature = <53000>; /* millicelsius */
> + hysteresis = <2000>; /* millicelsius */
> + type = "passive";
> + };
> + };
> +
> + cooling-maps {
> + map0 {
> + trip = <&lcd_trip>;
> + cooling-device = <&lcd0 5 10>;
> + };
> + };
> + };
> +
> + board_thermal: board-thermal {
> + polling-delay-passive = <1000>; /* milliseconds */
> + polling-delay = <2500>; /* milliseconds */
> +
> + thermal-sensors = <&cpu_thermal &gpu_thermal &lcd_thermal>
All the participating thermal zones need to have a #thermal-sensor-cells
entry (missing above).
> +
> + sustainable-power = <2500>;
> +
> + trips {
> + warm_trip: warm-trip {
> + temperature = <62000>; /* millicelsius */
> + hysteresis = <2000>; /* millicelsius */
> + type = "passive";
> + };
> + crit_trip: crit-trip {
> + temperature = <68000>; /* millicelsius */
> + hysteresis = <2000>; /* millicelsius */
> + type = "critical";
> + };
> + };
> +
> + cooling-maps {
> + map0 {
> + trip = <&warm_trip>;
> + cooling-device = <&cpu0 2 2>;
> + contribution = <55>;
> + };
> + map1 {
> + trip = <&warm_trip>;
> + cooling-device = <&gpu0 2 2>;
> + contribution = <20>;
> + };
> + map2 {
> + trip = <&lcd_trip>;
> + cooling-device = <&lcd0 7 10>;
> + contribution = <15>;
> + };
> + };
> + };
> +};
> +
> +The above example is a different take at example (d). We create one
> +thermal zone per sensor: cpu_thermal, gpu_thermal and lcd_thermal.
> +Each of which has its own trip point for each own cooling device. We
> +then define a board_thermal thermal zone that is a combination of all
> +the other thermal zones. If the board hits its warm_trip, then all
> +cooling devices are throttled.
> +
> +This example illustrates how we can throttle each device individually
> +if its too hot and at the same time have some control over the whole
> +system.
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread