[PATCH v2 0/3] Hierarchical thermal zones

[PATCH v2 0/3] Hierarchical thermal zones

[Javi Merino]

This series adds the ability to create a hierarchy of thermal zones.
Thermal zones created via platform code or device tree can be set up
to calculate their temperature as the maximum of all its underlying
thermal zones.  This came up from discussions during LPC.

The first patch adds the basic support to thermal core.  Patch 2 adds
device tree support and patch 3 exports the hierarchy to sysfs.

Changes since v1:
  - Fix the prototype of thermal_zone_{add,del}_subtz() for
    !CONFIG_THERMAL as reported by the kbuild test robot

Javi Merino (3):
  thermal: Add support for hierarchical thermal zones
  thermal: of: parse stacked thermal zones from device tree
  thermal: show the sub thermal zones in sysfs

 .../devicetree/bindings/thermal/thermal.txt        | 154 +++++++++++++++++-
 Documentation/thermal/sysfs-api.txt                |  40 +++++
 drivers/thermal/of-thermal.c                       |  99 ++++++++++++
 drivers/thermal/thermal_core.c                     | 173 ++++++++++++++++++++-
 include/linux/thermal.h                            |  17 ++
 5 files changed, 475 insertions(+), 8 deletions(-)

-- 
1.9.1

[PATCH v2 1/3] thermal: Add support for hierarchical thermal zones

[Javi Merino]

Add the ability to stack thermal zones on top of each other, creating a
hierarchy of thermal zones.

Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Signed-off-by: Javi Merino <javi.merino@arm.com>
---
 Documentation/thermal/sysfs-api.txt |  35 +++++++++
 drivers/thermal/thermal_core.c      | 151 ++++++++++++++++++++++++++++++++++--
 include/linux/thermal.h             |  14 ++++
 3 files changed, 195 insertions(+), 5 deletions(-)

diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
index 10f062ea6bc2..5650b7eaaf7e 100644
--- a/Documentation/thermal/sysfs-api.txt
+++ b/Documentation/thermal/sysfs-api.txt
@@ -72,6 +72,41 @@ temperature) and throttle appropriate devices.
     It deletes the corresponding entry form /sys/class/thermal folder and
     unbind all the thermal cooling devices it uses.
 
+
+1.1.3 int thermal_zone_add_subtz(struct thermal_zone_device *tz,
+                                 struct thermal_zone_device *subtz)
+
+    Add subtz to the list of slaves of tz.  When calculating the
+    temperature of thermal zone tz, report the maximum of the slave
+    thermal zones.  This lets you create a hierarchy of thermal zones.
+    The hierarchy must be a Direct Acyclic Graph (DAG).  If a loop is
+    detected, thermal_zone_get_temp() will return -EDEADLK to prevent
+    the deadlock.  thermal_zone_add_subtz() does not affect subtz.
+
+    For example, if you have an SoC with a thermal sensor in each cpu
+    of the two cpus you could have a thermal zone to monitor each
+    sensor, let's call them cpu0_tz and cpu1_tz.  You could then have a
+    a SoC thermal zone (soc_tz) without a get_temp() op which can be
+    configured like this:
+
+    thermal_zone_add_subtz(soc_tz, cpu0_tz);
+    thermal_zone_add_subtz(soc_tz, cpu1_tz);
+
+    Now soc_tz's temperature is the maximum of cpu0_tz and cpu1_tz.
+    Furthermore, soc_tz could be combined with other thermal zones to
+    create a "device" thermal zone.
+
+
+1.1.4 int thermal_zone_del_subtz(struct thermal_zone_device *tz,
+                                 struct thermal_zone_device *subtz)
+
+    Delete subtz from the slave thermal zones of tz.  This basically
+    reverses what thermal_zone_add_subtz() does.  If tz still has
+    other slave thermal zones, its temperature would be the maximum of
+    the remaining slave thermal zones.  If tz no longer has slave
+    thermal zones, thermal_zone_get_temp() will return -EINVAL.
+
+
 1.2 thermal cooling device interface
 1.2.1 struct thermal_cooling_device *thermal_cooling_device_register(char *name,
 		void *devdata, struct thermal_cooling_device_ops *)
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index d9e525cc9c1c..a2ab482337f3 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -61,6 +61,11 @@ static DEFINE_MUTEX(thermal_governor_lock);
 
 static struct thermal_governor *def_governor;
 
+struct thermal_zone_link {
+	struct thermal_zone_device *slave;
+	struct list_head node;
+};
+
 static struct thermal_governor *__find_governor(const char *name)
 {
 	struct thermal_governor *pos;
@@ -465,6 +470,42 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, int trip)
 }
 
 /**
+ * get_subtz_temp() - get the maximum temperature of all the sub thermal zones
+ * @tz:	thermal zone pointer
+ * @temp: pointer in which to store the result
+ *
+ * Go through all the thermal zones listed in @tz slave_tzs and
+ * calculate the maximum temperature of all of them.  The maximum
+ * temperature is stored in @temp.
+ *
+ * Return: 0 on success, -EINVAL if there are no slave thermal zones,
+ * -E* if thermal_zone_get_temp() fails for any of the slave thermal
+ * zones.
+ */
+static int get_subtz_temp(struct thermal_zone_device *tz, int *temp)
+{
+	struct thermal_zone_link *link;
+	int max_temp = INT_MIN;
+
+	if (list_empty(&tz->slave_tzs))
+		return -EINVAL;
+
+	list_for_each_entry(link, &tz->slave_tzs, node) {
+		int this_temp, ret;
+
+		ret = thermal_zone_get_temp(link->slave, &this_temp);
+		if (ret)
+			return ret;
+
+		if (this_temp > max_temp)
+			max_temp = this_temp;
+	}
+
+	*temp = max_temp;
+	return 0;
+}
+
+/**
  * thermal_zone_get_temp() - returns the temperature of a thermal zone
  * @tz: a valid pointer to a struct thermal_zone_device
  * @temp: a valid pointer to where to store the resulting temperature.
@@ -481,11 +522,22 @@ int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp)
 	int crit_temp = INT_MAX;
 	enum thermal_trip_type type;
 
-	if (!tz || IS_ERR(tz) || !tz->ops->get_temp)
+	if (!tz || IS_ERR(tz))
 		goto exit;
 
+	/* Avoid loops */
+	if (tz->slave_tz_visited)
+		return -EDEADLK;
+
 	mutex_lock(&tz->lock);
 
+	tz->slave_tz_visited = true;
+
+	if (!tz->ops->get_temp) {
+		ret = get_subtz_temp(tz, temp);
+		goto unlock;
+	}
+
 	ret = tz->ops->get_temp(tz, temp);
 
 	if (IS_ENABLED(CONFIG_THERMAL_EMULATION) && tz->emul_temperature) {
@@ -506,7 +558,9 @@ int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp)
 		if (!ret && *temp < crit_temp)
 			*temp = tz->emul_temperature;
 	}
- 
+
+unlock:
+	tz->slave_tz_visited = false;
 	mutex_unlock(&tz->lock);
 exit:
 	return ret;
@@ -540,9 +594,6 @@ void thermal_zone_device_update(struct thermal_zone_device *tz)
 {
 	int count;
 
-	if (!tz->ops->get_temp)
-		return;
-
 	update_temperature(tz);
 
 	for (count = 0; count < tz->trips; count++)
@@ -1789,6 +1840,7 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
 	if (!tz)
 		return ERR_PTR(-ENOMEM);
 
+	INIT_LIST_HEAD(&tz->slave_tzs);
 	INIT_LIST_HEAD(&tz->thermal_instances);
 	idr_init(&tz->idr);
 	mutex_init(&tz->lock);
@@ -1921,6 +1973,7 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz)
 	const struct thermal_zone_params *tzp;
 	struct thermal_cooling_device *cdev;
 	struct thermal_zone_device *pos = NULL;
+	struct thermal_zone_link *link, *tmp;
 
 	if (!tz)
 		return;
@@ -1958,6 +2011,9 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz)
 
 	mutex_unlock(&thermal_list_lock);
 
+	list_for_each_entry_safe(link, tmp, &tz->slave_tzs, node)
+		thermal_zone_del_subtz(tz, link->slave);
+
 	thermal_zone_device_set_polling(tz, 0);
 
 	if (tz->type[0])
@@ -1980,6 +2036,91 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz)
 EXPORT_SYMBOL_GPL(thermal_zone_device_unregister);
 
 /**
+ * thermal_zone_add_subtz() - add a sub thermal zone to a thermal zone
+ * @tz:	pointer to the master thermal zone
+ * @subtz: pointer to the slave thermal zone
+ *
+ * Add @subtz to the list of slave thermal zones of @tz.  If @tz
+ * doesn't provide a get_temp() op, its temperature will be calculated
+ * as a combination of the temperatures of its sub thermal zones.  See
+ * get_sub_tz_temp() for more information on how it's calculated.
+ *
+ * Return: 0 on success, -EINVAL if @tz or @subtz are not valid
+ * pointers, -EEXIST if the link already exists or -ENOMEM if we ran
+ * out of memory.
+ */
+int thermal_zone_add_subtz(struct thermal_zone_device *tz,
+			   struct thermal_zone_device *subtz)
+{
+	int ret;
+	struct thermal_zone_link *link;
+
+	if (IS_ERR_OR_NULL(tz) || IS_ERR_OR_NULL(subtz))
+		return -EINVAL;
+
+	mutex_lock(&tz->lock);
+
+	list_for_each_entry(link, &tz->slave_tzs, node) {
+		if (link->slave == subtz) {
+			ret = -EEXIST;
+			goto unlock;
+		}
+	}
+
+	link = kzalloc(sizeof(*link), GFP_KERNEL);
+	if (!link) {
+		ret = -ENOMEM;
+		goto unlock;
+	}
+
+	link->slave = subtz;
+	list_add_tail(&link->node, &tz->slave_tzs);
+
+	ret = 0;
+
+unlock:
+	mutex_unlock(&tz->lock);
+
+	return ret;
+}
+
+/**
+ * thermal_zone_del_subtz() - delete a sub thermal zone from its master thermal zone
+ * @tz:	pointer to the master thermal zone
+ * @subtz: pointer to the slave thermal zone
+ *
+ * Remove @subtz from the list of slave thermal zones of @tz.
+ *
+ * Return: 0 on success, -EINVAL if either thermal is invalid or if
+ * @subtz is not a slave of @tz.
+ */
+int thermal_zone_del_subtz(struct thermal_zone_device *tz,
+			   struct thermal_zone_device *subtz)
+{
+	int ret = -EINVAL;
+	struct thermal_zone_link *link;
+
+	if (IS_ERR_OR_NULL(tz) || IS_ERR_OR_NULL(subtz))
+		return -EINVAL;
+
+	mutex_lock(&tz->lock);
+
+	list_for_each_entry(link, &tz->slave_tzs, node) {
+		if (link->slave == subtz) {
+			list_del(&link->node);
+			kfree(link);
+
+			ret = 0;
+			break;
+		}
+	}
+
+	mutex_unlock(&tz->lock);
+
+	return ret;
+}
+
+/**
  * thermal_zone_get_zone_by_name() - search for a zone and returns its ref
  * @name: thermal zone name to fetch the temperature
  *
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 157d366e761b..01de95d16679 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -168,6 +168,8 @@ struct thermal_attr {
  * @ops:	operations this &thermal_zone_device supports
  * @tzp:	thermal zone parameters
  * @governor:	pointer to the governor for this thermal zone
+ * @slave_tzs:	list of thermal zones that are a slave of this thermal zone
+ * @slave_tz_visited: used to detect loops in stacked thermal zones
  * @governor_data:	private pointer for governor data
  * @thermal_instances:	list of &struct thermal_instance of this thermal zone
  * @idr:	&struct idr to generate unique id for this zone's cooling
@@ -195,6 +197,8 @@ struct thermal_zone_device {
 	struct thermal_zone_device_ops *ops;
 	struct thermal_zone_params *tzp;
 	struct thermal_governor *governor;
+	struct list_head slave_tzs;
+	bool slave_tz_visited;
 	void *governor_data;
 	struct list_head thermal_instances;
 	struct idr idr;
@@ -403,6 +407,10 @@ struct thermal_cooling_device *
 thermal_of_cooling_device_register(struct device_node *np, char *, void *,
 				   const struct thermal_cooling_device_ops *);
 void thermal_cooling_device_unregister(struct thermal_cooling_device *);
+int thermal_zone_add_subtz(struct thermal_zone_device *,
+			   struct thermal_zone_device *);
+int thermal_zone_del_subtz(struct thermal_zone_device *,
+			   struct thermal_zone_device *);
 struct thermal_zone_device *thermal_zone_get_zone_by_name(const char *name);
 int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp);
 
@@ -455,6 +463,12 @@ thermal_of_cooling_device_register(struct device_node *np,
 static inline void thermal_cooling_device_unregister(
 	struct thermal_cooling_device *cdev)
 { }
+static inline int thermal_zone_add_subtz(struct thermal_zone_device *tz,
+					 struct thermal_zone_device *subtz)
+{ return -ENODEV; }
+static inline int thermal_zone_del_subtz(struct thermal_zone_device *tz,
+					 struct thermal_zone_device *subtz)
+{ return -ENODEV; }
 static inline struct thermal_zone_device *thermal_zone_get_zone_by_name(
 		const char *name)
 { return ERR_PTR(-ENODEV); }
-- 
1.9.1

Re: [PATCH v2 1/3] thermal: Add support for hierarchical thermal zones

[Eduardo Valentin]

Javi,

First of all, thanks for taking the lead on implementing this.

It follows comments.

On Wed, Nov 04, 2015 at 05:37:40PM +0000, Javi Merino wrote:
> Add the ability to stack thermal zones on top of each other, creating a
> hierarchy of thermal zones.
> 
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> Signed-off-by: Javi Merino <javi.merino@arm.com>
> ---
>  Documentation/thermal/sysfs-api.txt |  35 +++++++++
>  drivers/thermal/thermal_core.c      | 151 ++++++++++++++++++++++++++++++++++--
>  include/linux/thermal.h             |  14 ++++
>  3 files changed, 195 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
> index 10f062ea6bc2..5650b7eaaf7e 100644
> --- a/Documentation/thermal/sysfs-api.txt
> +++ b/Documentation/thermal/sysfs-api.txt
> @@ -72,6 +72,41 @@ temperature) and throttle appropriate devices.
>      It deletes the corresponding entry form /sys/class/thermal folder and
>      unbind all the thermal cooling devices it uses.
>  
> +
> +1.1.3 int thermal_zone_add_subtz(struct thermal_zone_device *tz,
> +                                 struct thermal_zone_device *subtz)
> +
> +    Add subtz to the list of slaves of tz.  When calculating the
> +    temperature of thermal zone tz, report the maximum of the slave
> +    thermal zones.  This lets you create a hierarchy of thermal zones.
> +    The hierarchy must be a Direct Acyclic Graph (DAG).  If a loop is
> +    detected, thermal_zone_get_temp() will return -EDEADLK to prevent
> +    the deadlock.  thermal_zone_add_subtz() does not affect subtz.
> +
> +    For example, if you have an SoC with a thermal sensor in each cpu
> +    of the two cpus you could have a thermal zone to monitor each
> +    sensor, let's call them cpu0_tz and cpu1_tz.  You could then have a
> +    a SoC thermal zone (soc_tz) without a get_temp() op which can be
> +    configured like this:
> +
> +    thermal_zone_add_subtz(soc_tz, cpu0_tz);
> +    thermal_zone_add_subtz(soc_tz, cpu1_tz);
> +
> +    Now soc_tz's temperature is the maximum of cpu0_tz and cpu1_tz.
> +    Furthermore, soc_tz could be combined with other thermal zones to
> +    create a "device" thermal zone.
> +
> +
> +1.1.4 int thermal_zone_del_subtz(struct thermal_zone_device *tz,
> +                                 struct thermal_zone_device *subtz)
> +
> +    Delete subtz from the slave thermal zones of tz.  This basically
> +    reverses what thermal_zone_add_subtz() does.  If tz still has
> +    other slave thermal zones, its temperature would be the maximum of
> +    the remaining slave thermal zones.  If tz no longer has slave
> +    thermal zones, thermal_zone_get_temp() will return -EINVAL.
> +
> +
>  1.2 thermal cooling device interface
>  1.2.1 struct thermal_cooling_device *thermal_cooling_device_register(char *name,
>  		void *devdata, struct thermal_cooling_device_ops *)
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index d9e525cc9c1c..a2ab482337f3 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -61,6 +61,11 @@ static DEFINE_MUTEX(thermal_governor_lock);
>  
>  static struct thermal_governor *def_governor;
>  
> +struct thermal_zone_link {
> +	struct thermal_zone_device *slave;
> +	struct list_head node;
> +};
> +
>  static struct thermal_governor *__find_governor(const char *name)
>  {
>  	struct thermal_governor *pos;
> @@ -465,6 +470,42 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, int trip)
>  }
>  
>  /**
> + * get_subtz_temp() - get the maximum temperature of all the sub thermal zones
> + * @tz:	thermal zone pointer
> + * @temp: pointer in which to store the result
> + *
> + * Go through all the thermal zones listed in @tz slave_tzs and
> + * calculate the maximum temperature of all of them.  The maximum
> + * temperature is stored in @temp.
> + *
> + * Return: 0 on success, -EINVAL if there are no slave thermal zones,
> + * -E* if thermal_zone_get_temp() fails for any of the slave thermal
> + * zones.
> + */
> +static int get_subtz_temp(struct thermal_zone_device *tz, int *temp)
> +{
> +	struct thermal_zone_link *link;
> +	int max_temp = INT_MIN;
> +
> +	if (list_empty(&tz->slave_tzs))
> +		return -EINVAL;
> +
> +	list_for_each_entry(link, &tz->slave_tzs, node) {
> +		int this_temp, ret;
> +
> +		ret = thermal_zone_get_temp(link->slave, &this_temp);
> +		if (ret)
> +			return ret;
> +
> +		if (this_temp > max_temp)
> +			max_temp = this_temp;

I think we need a better design here. I do not like the fact that we are
limited to max temp operation. Remember the LPC discussion? I believe
the agreement was to have a set of aggregation functions. max temp is
definetly one of them. Also the aggregation functions would be sysfs
configurable.

> +	}
> +
> +	*temp = max_temp;
> +	return 0;
> +}
> +
> +/**
>   * thermal_zone_get_temp() - returns the temperature of a thermal zone
>   * @tz: a valid pointer to a struct thermal_zone_device
>   * @temp: a valid pointer to where to store the resulting temperature.
> @@ -481,11 +522,22 @@ int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp)
>  	int crit_temp = INT_MAX;
>  	enum thermal_trip_type type;
>  
> -	if (!tz || IS_ERR(tz) || !tz->ops->get_temp)
> +	if (!tz || IS_ERR(tz))
>  		goto exit;
>  
> +	/* Avoid loops */
> +	if (tz->slave_tz_visited)
> +		return -EDEADLK;
> +
>  	mutex_lock(&tz->lock);
>  
> +	tz->slave_tz_visited = true;
> +
> +	if (!tz->ops->get_temp) {
> +		ret = get_subtz_temp(tz, temp);
> +		goto unlock;
> +	}
> +
>  	ret = tz->ops->get_temp(tz, temp);
>  
>  	if (IS_ENABLED(CONFIG_THERMAL_EMULATION) && tz->emul_temperature) {
> @@ -506,7 +558,9 @@ int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp)
>  		if (!ret && *temp < crit_temp)
>  			*temp = tz->emul_temperature;
>  	}
> - 
> +
> +unlock:
> +	tz->slave_tz_visited = false;
>  	mutex_unlock(&tz->lock);
>  exit:
>  	return ret;
> @@ -540,9 +594,6 @@ void thermal_zone_device_update(struct thermal_zone_device *tz)
>  {
>  	int count;
>  
> -	if (!tz->ops->get_temp)
> -		return;
> -
>  	update_temperature(tz);
>  
>  	for (count = 0; count < tz->trips; count++)
> @@ -1789,6 +1840,7 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
>  	if (!tz)
>  		return ERR_PTR(-ENOMEM);
>  
> +	INIT_LIST_HEAD(&tz->slave_tzs);
>  	INIT_LIST_HEAD(&tz->thermal_instances);
>  	idr_init(&tz->idr);
>  	mutex_init(&tz->lock);
> @@ -1921,6 +1973,7 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz)
>  	const struct thermal_zone_params *tzp;
>  	struct thermal_cooling_device *cdev;
>  	struct thermal_zone_device *pos = NULL;
> +	struct thermal_zone_link *link, *tmp;
>  
>  	if (!tz)
>  		return;
> @@ -1958,6 +2011,9 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz)
>  
>  	mutex_unlock(&thermal_list_lock);
>  
> +	list_for_each_entry_safe(link, tmp, &tz->slave_tzs, node)
> +		thermal_zone_del_subtz(tz, link->slave);
> +
>  	thermal_zone_device_set_polling(tz, 0);
>  
>  	if (tz->type[0])
> @@ -1980,6 +2036,91 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz)
>  EXPORT_SYMBOL_GPL(thermal_zone_device_unregister);
>  
>  /**
> + * thermal_zone_add_subtz() - add a sub thermal zone to a thermal zone
> + * @tz:	pointer to the master thermal zone
> + * @subtz: pointer to the slave thermal zone
> + *
> + * Add @subtz to the list of slave thermal zones of @tz.  If @tz
> + * doesn't provide a get_temp() op, its temperature will be calculated
> + * as a combination of the temperatures of its sub thermal zones.  See
> + * get_sub_tz_temp() for more information on how it's calculated.
> + *
> + * Return: 0 on success, -EINVAL if @tz or @subtz are not valid
> + * pointers, -EEXIST if the link already exists or -ENOMEM if we ran
> + * out of memory.
> + */
> +int thermal_zone_add_subtz(struct thermal_zone_device *tz,
> +			   struct thermal_zone_device *subtz)
> +{
> +	int ret;
> +	struct thermal_zone_link *link;
> +
> +	if (IS_ERR_OR_NULL(tz) || IS_ERR_OR_NULL(subtz))
> +		return -EINVAL;
> +
> +	mutex_lock(&tz->lock);
> +
> +	list_for_each_entry(link, &tz->slave_tzs, node) {
> +		if (link->slave == subtz) {
> +			ret = -EEXIST;
> +			goto unlock;
> +		}
> +	}
> +
> +	link = kzalloc(sizeof(*link), GFP_KERNEL);
> +	if (!link) {
> +		ret = -ENOMEM;
> +		goto unlock;
> +	}
> +
> +	link->slave = subtz;
> +	list_add_tail(&link->node, &tz->slave_tzs);
> +
> +	ret = 0;
> +
> +unlock:
> +	mutex_unlock(&tz->lock);
> +
> +	return ret;
> +}
> +
> +/**
> + * thermal_zone_del_subtz() - delete a sub thermal zone from its master thermal zone
> + * @tz:	pointer to the master thermal zone
> + * @subtz: pointer to the slave thermal zone
> + *
> + * Remove @subtz from the list of slave thermal zones of @tz.
> + *
> + * Return: 0 on success, -EINVAL if either thermal is invalid or if
> + * @subtz is not a slave of @tz.
> + */
> +int thermal_zone_del_subtz(struct thermal_zone_device *tz,
> +			   struct thermal_zone_device *subtz)
> +{
> +	int ret = -EINVAL;
> +	struct thermal_zone_link *link;
> +
> +	if (IS_ERR_OR_NULL(tz) || IS_ERR_OR_NULL(subtz))
> +		return -EINVAL;
> +
> +	mutex_lock(&tz->lock);
> +
> +	list_for_each_entry(link, &tz->slave_tzs, node) {
> +		if (link->slave == subtz) {
> +			list_del(&link->node);
> +			kfree(link);
> +
> +			ret = 0;
> +			break;
> +		}
> +	}
> +
> +	mutex_unlock(&tz->lock);
> +
> +	return ret;
> +}
> +
> +/**
>   * thermal_zone_get_zone_by_name() - search for a zone and returns its ref
>   * @name: thermal zone name to fetch the temperature
>   *
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 157d366e761b..01de95d16679 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -168,6 +168,8 @@ struct thermal_attr {
>   * @ops:	operations this &thermal_zone_device supports
>   * @tzp:	thermal zone parameters
>   * @governor:	pointer to the governor for this thermal zone
> + * @slave_tzs:	list of thermal zones that are a slave of this thermal zone
> + * @slave_tz_visited: used to detect loops in stacked thermal zones
>   * @governor_data:	private pointer for governor data
>   * @thermal_instances:	list of &struct thermal_instance of this thermal zone
>   * @idr:	&struct idr to generate unique id for this zone's cooling
> @@ -195,6 +197,8 @@ struct thermal_zone_device {
>  	struct thermal_zone_device_ops *ops;
>  	struct thermal_zone_params *tzp;
>  	struct thermal_governor *governor;
> +	struct list_head slave_tzs;
> +	bool slave_tz_visited;
>  	void *governor_data;
>  	struct list_head thermal_instances;
>  	struct idr idr;
> @@ -403,6 +407,10 @@ struct thermal_cooling_device *
>  thermal_of_cooling_device_register(struct device_node *np, char *, void *,
>  				   const struct thermal_cooling_device_ops *);
>  void thermal_cooling_device_unregister(struct thermal_cooling_device *);
> +int thermal_zone_add_subtz(struct thermal_zone_device *,
> +			   struct thermal_zone_device *);
> +int thermal_zone_del_subtz(struct thermal_zone_device *,
> +			   struct thermal_zone_device *);
>  struct thermal_zone_device *thermal_zone_get_zone_by_name(const char *name);
>  int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp);
>  
> @@ -455,6 +463,12 @@ thermal_of_cooling_device_register(struct device_node *np,
>  static inline void thermal_cooling_device_unregister(
>  	struct thermal_cooling_device *cdev)
>  { }
> +static inline int thermal_zone_add_subtz(struct thermal_zone_device *tz,
> +					 struct thermal_zone_device *subtz)
> +{ return -ENODEV; }
> +static inline int thermal_zone_del_subtz(struct thermal_zone_device *tz,
> +					 struct thermal_zone_device *subtz)
> +{ return -ENODEV; }
>  static inline struct thermal_zone_device *thermal_zone_get_zone_by_name(
>  		const char *name)
>  { return ERR_PTR(-ENODEV); }
> -- 
> 1.9.1
> 

Re: [PATCH v2 1/3] thermal: Add support for hierarchical thermal zones

[Javi Merino]

On Wed, Nov 04, 2015 at 10:51:02AM -0800, Eduardo Valentin wrote:
> Javi,
> 
> First of all, thanks for taking the lead on implementing this.
> 
> It follows comments.
> 
> On Wed, Nov 04, 2015 at 05:37:40PM +0000, Javi Merino wrote:
> > Add the ability to stack thermal zones on top of each other, creating a
> > hierarchy of thermal zones.
> > 
> > Cc: Zhang Rui <rui.zhang@intel.com>
> > Cc: Eduardo Valentin <edubezval@gmail.com>
> > Signed-off-by: Javi Merino <javi.merino@arm.com>
> > ---
> >  Documentation/thermal/sysfs-api.txt |  35 +++++++++
> >  drivers/thermal/thermal_core.c      | 151 ++++++++++++++++++++++++++++++++++--
> >  include/linux/thermal.h             |  14 ++++
> >  3 files changed, 195 insertions(+), 5 deletions(-)
> > 
> > diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
> > index 10f062ea6bc2..5650b7eaaf7e 100644
> > --- a/Documentation/thermal/sysfs-api.txt
> > +++ b/Documentation/thermal/sysfs-api.txt
> > @@ -72,6 +72,41 @@ temperature) and throttle appropriate devices.
> >      It deletes the corresponding entry form /sys/class/thermal folder and
> >      unbind all the thermal cooling devices it uses.
> >  
> > +
> > +1.1.3 int thermal_zone_add_subtz(struct thermal_zone_device *tz,
> > +                                 struct thermal_zone_device *subtz)
> > +
> > +    Add subtz to the list of slaves of tz.  When calculating the
> > +    temperature of thermal zone tz, report the maximum of the slave
> > +    thermal zones.  This lets you create a hierarchy of thermal zones.
> > +    The hierarchy must be a Direct Acyclic Graph (DAG).  If a loop is
> > +    detected, thermal_zone_get_temp() will return -EDEADLK to prevent
> > +    the deadlock.  thermal_zone_add_subtz() does not affect subtz.
> > +
> > +    For example, if you have an SoC with a thermal sensor in each cpu
> > +    of the two cpus you could have a thermal zone to monitor each
> > +    sensor, let's call them cpu0_tz and cpu1_tz.  You could then have a
> > +    a SoC thermal zone (soc_tz) without a get_temp() op which can be
> > +    configured like this:
> > +
> > +    thermal_zone_add_subtz(soc_tz, cpu0_tz);
> > +    thermal_zone_add_subtz(soc_tz, cpu1_tz);
> > +
> > +    Now soc_tz's temperature is the maximum of cpu0_tz and cpu1_tz.
> > +    Furthermore, soc_tz could be combined with other thermal zones to
> > +    create a "device" thermal zone.
> > +
> > +
> > +1.1.4 int thermal_zone_del_subtz(struct thermal_zone_device *tz,
> > +                                 struct thermal_zone_device *subtz)
> > +
> > +    Delete subtz from the slave thermal zones of tz.  This basically
> > +    reverses what thermal_zone_add_subtz() does.  If tz still has
> > +    other slave thermal zones, its temperature would be the maximum of
> > +    the remaining slave thermal zones.  If tz no longer has slave
> > +    thermal zones, thermal_zone_get_temp() will return -EINVAL.
> > +
> > +
> >  1.2 thermal cooling device interface
> >  1.2.1 struct thermal_cooling_device *thermal_cooling_device_register(char *name,
> >  		void *devdata, struct thermal_cooling_device_ops *)
> > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> > index d9e525cc9c1c..a2ab482337f3 100644
> > --- a/drivers/thermal/thermal_core.c
> > +++ b/drivers/thermal/thermal_core.c
> > @@ -61,6 +61,11 @@ static DEFINE_MUTEX(thermal_governor_lock);
> >  
> >  static struct thermal_governor *def_governor;
> >  
> > +struct thermal_zone_link {
> > +	struct thermal_zone_device *slave;
> > +	struct list_head node;
> > +};
> > +
> >  static struct thermal_governor *__find_governor(const char *name)
> >  {
> >  	struct thermal_governor *pos;
> > @@ -465,6 +470,42 @@ static void handle_thermal_trip(struct thermal_zone_device *tz, int trip)
> >  }
> >  
> >  /**
> > + * get_subtz_temp() - get the maximum temperature of all the sub thermal zones
> > + * @tz:	thermal zone pointer
> > + * @temp: pointer in which to store the result
> > + *
> > + * Go through all the thermal zones listed in @tz slave_tzs and
> > + * calculate the maximum temperature of all of them.  The maximum
> > + * temperature is stored in @temp.
> > + *
> > + * Return: 0 on success, -EINVAL if there are no slave thermal zones,
> > + * -E* if thermal_zone_get_temp() fails for any of the slave thermal
> > + * zones.
> > + */
> > +static int get_subtz_temp(struct thermal_zone_device *tz, int *temp)
> > +{
> > +	struct thermal_zone_link *link;
> > +	int max_temp = INT_MIN;
> > +
> > +	if (list_empty(&tz->slave_tzs))
> > +		return -EINVAL;
> > +
> > +	list_for_each_entry(link, &tz->slave_tzs, node) {
> > +		int this_temp, ret;
> > +
> > +		ret = thermal_zone_get_temp(link->slave, &this_temp);
> > +		if (ret)
> > +			return ret;
> > +
> > +		if (this_temp > max_temp)
> > +			max_temp = this_temp;
> 
> I think we need a better design here. I do not like the fact that we are
> limited to max temp operation. Remember the LPC discussion? I believe
> the agreement was to have a set of aggregation functions. max temp is
> definetly one of them. Also the aggregation functions would be sysfs
> configurable.

I wasn't sure how to do this from device tree.  There's a coefficients
property but it's not really usable as it's defined as:

Z = c0 * x0 + c1 * x1 + ... + c(n-1) * x(n-1) + cn

Where all the c are integers.  As the x are temperatures, the result
can't be a temperature.  It could work if we said that we divide the
result byt the sum of the coefficients, making it a weighted sum.
something like:

Z = (c0 * x0 + c1 * x1 + ... + c(n-1) * x(n-1)) / (c0 + c1 + ... + c(n-1)) + cn

I guess we can skip the device tree specification for now and just let
userspace configure it using sysfs.  I'll add "weighted sum" as an
additional aggregation function for v3.

Cheers,
Javi

[PATCH v2 2/3] thermal: of: parse stacked thermal zones from device tree

[Javi Merino]

Let device tree set thermal zones in the thermal-sensors list of
phandles and set up the thermal zone hierarchy based on the information
present there.

Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Signed-off-by: Javi Merino <javi.merino@arm.com>
---
 .../devicetree/bindings/thermal/thermal.txt        | 154 ++++++++++++++++++++-
 drivers/thermal/of-thermal.c                       |  99 +++++++++++++
 2 files changed, 250 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>
+
+		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.
diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
index 42b7d4253b94..17e8ae5edd73 100644
--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c
@@ -166,6 +166,27 @@ of_thermal_get_trip_points(struct thermal_zone_device *tz)
 EXPORT_SYMBOL_GPL(of_thermal_get_trip_points);
 
 /**
+ * of_thermal_is_thermal_zone - check that a device node corresponds to a thermal zone
+ * @np:	the device node
+ *
+ * Valid thermal zone device nodes must provide the
+ * polling-delay-passive and polling-delay properties.  This function
+ * checks if @np is thermal-zone node.
+ *
+ * Return: true if @np is a valid device for a thermal zone.
+ */
+static bool of_thermal_is_thermal_zone(struct device_node *np)
+{
+	u32 out;
+
+	if ((of_property_read_u32(np, "polling-delay-passive", &out)) ||
+	    (of_property_read_u32(np, "polling-delay", &out)))
+		return false;
+
+	return true;
+}
+
+/**
  * of_thermal_set_emul_temp - function to set emulated temperature
  *
  * @tz:	pointer to a thermal zone
@@ -858,6 +879,81 @@ static inline void of_thermal_free_zone(struct __thermal_zone *tz)
 }
 
 /**
+ * link_stacked_thermal_zones - link thermal zones that are a superset of thermal zones
+ * @np:	device node for the root of the thermal zones
+ *
+ * A thermal zone can specify other thermal zones as its input using
+ * the thermal-sensors property of device tree.  This function parses
+ * all the thermal zones in dt and adds the thermal zones in their
+ * thermal-sensors as sub-thermalzones.
+ */
+static void link_stacked_thermal_zones(struct device_node *np)
+{
+	struct device_node *child;
+
+	for_each_child_of_node(np, child) {
+		int i, num_sensors;
+		struct thermal_zone_device *tz;
+
+		num_sensors = of_count_phandle_with_args(child,
+							 "thermal-sensors",
+							 "#thermal-sensor-cells");
+		if (num_sensors <= 0)
+			continue;
+
+		tz = thermal_zone_get_zone_by_name(child->name);
+		if (IS_ERR_OR_NULL(tz)) {
+			/*
+			 * If the tz is available we should have added
+			 * it in of_parse_thermal_zones()
+			 */
+			WARN(of_device_is_available(child),
+			     "Couldn't find thermal zone for %s\n",
+			     of_node_full_name(child));
+			continue;
+		}
+
+		for (i = 0; i < num_sensors; i++) {
+			struct of_phandle_args sensor_specs;
+			struct thermal_zone_device *subtz;
+			int ret;
+
+			ret = of_parse_phandle_with_args(child,
+							"thermal-sensors",
+							"#thermal-sensor-cells",
+							i, &sensor_specs);
+			if (ret) {
+				pr_warn("Failed to parse thermal-sensors of %s: %d\n",
+					of_node_full_name(child), ret);
+				of_node_put(sensor_specs.np);
+				continue;
+			}
+
+			if (!of_thermal_is_thermal_zone(sensor_specs.np)) {
+				of_node_put(sensor_specs.np);
+				continue;
+			}
+
+			subtz = thermal_zone_get_zone_by_name(
+				sensor_specs.np->name);
+			if (IS_ERR_OR_NULL(subtz)) {
+				pr_warn("Couldn't find thermal zone for %s, is it disabled?\n",
+					of_node_full_name(sensor_specs.np));
+				of_node_put(sensor_specs.np);
+				continue;
+			}
+
+			ret = thermal_zone_add_subtz(tz, subtz);
+			if (ret)
+				pr_warn("Failed to add thermal zone %s to %s: %d\n",
+					subtz->type, tz->type, ret);
+
+			of_node_put(sensor_specs.np);
+		}
+	}
+}
+
+/**
  * of_parse_thermal_zones - parse device tree thermal data
  *
  * Initialization function that can be called by machine initialization
@@ -936,6 +1032,9 @@ int __init of_parse_thermal_zones(void)
 			/* attempting to build remaining zones still */
 		}
 	}
+
+	link_stacked_thermal_zones(np);
+
 	of_node_put(np);
 
 	return 0;
-- 
1.9.1

Re: [PATCH v2 2/3] thermal: of: parse stacked thermal zones from device tree

[Eduardo Valentin]

On Wed, Nov 04, 2015 at 05:37:41PM +0000, Javi Merino wrote:
> Let device tree set thermal zones in the thermal-sensors list of
> phandles and set up the thermal zone hierarchy based on the information
> present there.
> 
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> Signed-off-by: Javi Merino <javi.merino@arm.com>
> ---
>  .../devicetree/bindings/thermal/thermal.txt        | 154 ++++++++++++++++++++-
>  drivers/thermal/of-thermal.c                       |  99 +++++++++++++

Please split this patch.

>  2 files changed, 250 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>
> +

(no, I have not grepped the device tree)
Was there any other occurrence of such construction ?

It just looks awkward that one property holds two types of data. Is DT
strongly typed?

I would say, following the DT pattern, one would expect to have the
binding to allow one thermal zone to be written inside another thermal
zone, no?

Re: [PATCH v2 2/3] thermal: of: parse stacked thermal zones from device tree

[Javi Merino]

On Wed, Nov 04, 2015 at 10:56:34AM -0800, Eduardo Valentin wrote:
> On Wed, Nov 04, 2015 at 05:37:41PM +0000, Javi Merino wrote:
> > Let device tree set thermal zones in the thermal-sensors list of
> > phandles and set up the thermal zone hierarchy based on the information
> > present there.
> > 
> > Cc: Zhang Rui <rui.zhang@intel.com>
> > Cc: Eduardo Valentin <edubezval@gmail.com>
> > Signed-off-by: Javi Merino <javi.merino@arm.com>
> > ---
> >  .../devicetree/bindings/thermal/thermal.txt        | 154 ++++++++++++++++++++-
> >  drivers/thermal/of-thermal.c                       |  99 +++++++++++++
> 
> Please split this patch.

Into documentation and code?  I prefer it when the documentation is
updated with the code but I can split it if that's what you prefer.

> >  2 files changed, 250 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>
> > +
> 
> (no, I have not grepped the device tree)
> Was there any other occurrence of such construction ?
> 
> It just looks awkward that one property holds two types of data. Is DT
> strongly typed?
>
> I would say, following the DT pattern, one would expect to have the
> binding to allow one thermal zone to be written inside another thermal
> zone, no?

That would not allow us to have a thermal zone be a child of multiple
thermal zones.  For example, a thermal sensor could be half-way
through a GPU and a core and you would want to use that sensor's
thermal zone as input for both the GPU and the core thermal zones.

Cheers,
Javi

[PATCH v2 3/3] thermal: show the sub thermal zones in sysfs

[Javi Merino]

When a thermal zone is created that has sub thermal zones via
devicetree or via thermal_zone_add_subtz() expose that relationship in
sysfs.

Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Signed-off-by: Javi Merino <javi.merino@arm.com>
---
 Documentation/thermal/sysfs-api.txt |  5 +++++
 drivers/thermal/thermal_core.c      | 22 ++++++++++++++++++++++
 include/linux/thermal.h             |  3 +++
 3 files changed, 30 insertions(+)

diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
index 5650b7eaaf7e..53972973792a 100644
--- a/Documentation/thermal/sysfs-api.txt
+++ b/Documentation/thermal/sysfs-api.txt
@@ -220,6 +220,7 @@ Thermal zone device sys I/F, created once it's registered:
     |---trip_point_[0-*]_type:	Trip point type
     |---trip_point_[0-*]_hyst:	Hysteresis value for this trip point
     |---emul_temp:		Emulated temperature set node
+    |---subtz[0-*]              Slave thermal zones of this thermal zone
     |---sustainable_power:      Sustainable dissipatable power
     |---k_po:                   Proportional term during temperature overshoot
     |---k_pu:                   Proportional term during temperature undershoot
@@ -355,6 +356,10 @@ emul_temp
 	  because userland can easily disable the thermal policy by simply
 	  flooding this sysfs node with low temperature values.
 
+subtz[0-*]
+	sysfs link to the slave thermal zone device.
+        RO, Optional
+
 sustainable_power
 	An estimate of the sustained power that can be dissipated by
 	the thermal zone. Used by the power allocator governor. For
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index a2ab482337f3..af72b704af36 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -62,6 +62,8 @@ static DEFINE_MUTEX(thermal_governor_lock);
 static struct thermal_governor *def_governor;
 
 struct thermal_zone_link {
+	int id;
+	char name[THERMAL_NAME_LENGTH];
 	struct thermal_zone_device *slave;
 	struct list_head node;
 };
@@ -2076,6 +2078,23 @@ int thermal_zone_add_subtz(struct thermal_zone_device *tz,
 	link->slave = subtz;
 	list_add_tail(&link->node, &tz->slave_tzs);
 
+	ret = get_idr(&tz->link_idr, NULL, &link->id);
+	if (ret) {
+		/*
+		 * Even if we couldn't create the symlink in sysfs,
+		 * we've linked the thermal zones, so return success
+		 */
+		ret = 0;
+		goto unlock;
+	}
+
+	snprintf(link->name, ARRAY_SIZE(link->name), "subtz%d", link->id);
+	ret = sysfs_create_link(&tz->device.kobj, &subtz->device.kobj,
+				link->name);
+	if (ret)
+		pr_warn("Failed to create symlink to sub thermal zone: %d\n",
+			ret);
+
 	ret = 0;
 
 unlock:
@@ -2107,6 +2126,9 @@ int thermal_zone_del_subtz(struct thermal_zone_device *tz,
 
 	list_for_each_entry(link, &tz->slave_tzs, node) {
 		if (link->slave == subtz) {
+			sysfs_remove_link(&tz->device.kobj, link->name);
+			release_idr(&tz->link_idr, NULL, link->id);
+
 			list_del(&link->node);
 			kfree(link);
 
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 01de95d16679..e1fcac52a7a5 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -174,6 +174,8 @@ struct thermal_attr {
  * @thermal_instances:	list of &struct thermal_instance of this thermal zone
  * @idr:	&struct idr to generate unique id for this zone's cooling
  *		devices
+ * @link_idr:	&struct idr to generate unique ids for this zone's links to
+ *		other thermal zones
  * @lock:	lock to protect thermal_instances list
  * @node:	node in thermal_tz_list (in thermal_core.c)
  * @poll_queue:	delayed work for polling
@@ -202,6 +204,7 @@ struct thermal_zone_device {
 	void *governor_data;
 	struct list_head thermal_instances;
 	struct idr idr;
+	struct idr link_idr;
 	struct mutex lock;
 	struct list_head node;
 	struct delayed_work poll_queue;
-- 
1.9.1