[PATCH v4 0/2] Support to tune governor in run time

[PATCH v4 0/2] Support to tune governor in run time

[Wei Ni]

This serie can support to turn governor for thermal zone in
run time.
Adds avaiable_policies attribute, so the user can get available
governor policies and change te governor to a new one.
Adds thermal_update_governor() function, so the thermal platform
driver can use it to update governor.

Changes from v1:
1. split avaiable_policies part to the "patch 1/2", and add document
for it.
2. add document for the thermal_update_governor.
Changes from v2:
1. fix the overflow issue in the avaiable_policies_show
Changes from v3:
1. remove the .start/.stop callback, because we doesn't have the real
case to support it now.

Wei Ni (2):
  thermal: add available policies attribute
  thermal: add interface to support tune governor in run-time

 Documentation/thermal/sysfs-api.txt |   10 ++++++
 drivers/thermal/thermal_core.c      |   66 +++++++++++++++++++++++++++++++----
 include/linux/thermal.h             |    1 +
 3 files changed, 70 insertions(+), 7 deletions(-)

-- 
1.7.9.5

[PATCH v4 1/2] thermal: add available policies attribute

[Wei Ni]

The Linux thermal framework support to change thermal governor
policy in userspace, but it can't show what available policies
supported.

This patch adds available_policies attribute to the thermal
framework, it can list the thermal governors which can be
used for a particular zone. This attribute is read only.

Signed-off-by: Wei Ni <wni@nvidia.com>
---
 Documentation/thermal/sysfs-api.txt |    6 ++++++
 drivers/thermal/thermal_core.c      |   28 ++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
index 87519cb..8459d45 100644
--- a/Documentation/thermal/sysfs-api.txt
+++ b/Documentation/thermal/sysfs-api.txt
@@ -172,6 +172,7 @@ Thermal zone device sys I/F, created once it's registered:
     |---temp:			Current temperature
     |---mode:			Working mode of the thermal zone
     |---policy:			Thermal governor used for this zone
+    |---available_policies	Available thermal governors for this zone
     |---trip_point_[0-*]_temp:	Trip point temperature
     |---trip_point_[0-*]_type:	Trip point type
     |---trip_point_[0-*]_hyst:	Hysteresis value for this trip point
@@ -238,6 +239,10 @@ policy
 	One of the various thermal governors used for a particular zone.
 	RW, Required
 
+available_policies
+	Available thermal governors which can be used for a particular zone.
+	RO, Required
+
 trip_point_[0-*]_temp
 	The temperature above which trip point will be fired.
 	Unit: millidegree Celsius
@@ -330,6 +335,7 @@ method, the sys I/F structure will be built like this:
     |---temp:			37000
     |---mode:			enabled
     |---policy:			step_wise
+    |---available_policies:	step_wise fair_share
     |---trip_point_0_temp:	100000
     |---trip_point_0_type:	critical
     |---trip_point_1_temp:	80000
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 338a88b..a91684b 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -761,6 +761,27 @@ policy_show(struct device *dev, struct device_attribute *devattr, char *buf)
 	return sprintf(buf, "%s\n", tz->governor->name);
 }
 
+static ssize_t
+available_policies_show(struct device *dev, struct device_attribute *devattr,
+			char *buf)
+{
+	struct thermal_governor *pos;
+	ssize_t count = 0;
+	ssize_t size = 0;
+
+	mutex_lock(&thermal_governor_lock);
+
+	list_for_each_entry(pos, &thermal_governor_list, governor_list) {
+		size = PAGE_SIZE - count;
+		count += scnprintf(buf + count, size, "%s ", pos->name);
+	}
+	count += scnprintf(buf + count, size, "\n");
+
+	mutex_unlock(&thermal_governor_lock);
+
+	return count;
+}
+
 #ifdef CONFIG_THERMAL_EMULATION
 static ssize_t
 emul_temp_store(struct device *dev, struct device_attribute *attr,
@@ -794,6 +815,7 @@ static DEVICE_ATTR(temp, 0444, temp_show, NULL);
 static DEVICE_ATTR(mode, 0644, mode_show, mode_store);
 static DEVICE_ATTR(passive, S_IRUGO | S_IWUSR, passive_show, passive_store);
 static DEVICE_ATTR(policy, S_IRUGO | S_IWUSR, policy_show, policy_store);
+static DEVICE_ATTR(available_policies, S_IRUGO, available_policies_show, NULL);
 
 /* sys I/F for cooling device */
 #define to_cooling_device(_dev)	\
@@ -1527,6 +1549,11 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
 	if (result)
 		goto unregister;
 
+	/* Create available_policies attribute */
+	result = device_create_file(&tz->device, &dev_attr_available_policies);
+	if (result)
+		goto unregister;
+
 	/* Update 'this' zone's governor information */
 	mutex_lock(&thermal_governor_lock);
 
@@ -1622,6 +1649,7 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz)
 	if (tz->ops->get_mode)
 		device_remove_file(&tz->device, &dev_attr_mode);
 	device_remove_file(&tz->device, &dev_attr_policy);
+	device_remove_file(&tz->device, &dev_attr_available_policies);
 	remove_trip_attrs(tz);
 	tz->governor = NULL;
 
-- 
1.7.9.5

[PATCH v4 2/2] thermal: add interface to support tune governor in run-time

[Wei Ni]

In the of-thermal driver, it register a thermal zone device
without governor, since the governers are not static, we
should be able to update to a new one in run-time, so I add
a new fucntion thermal_update_governor() to do it.

Change-Id: Ie4995d6404d6d4f16250958487f3e3b8f63fc4c6
Signed-off-by: Wei Ni <wni@nvidia.com>
---
 Documentation/thermal/sysfs-api.txt |    4 ++++
 drivers/thermal/thermal_core.c      |   38 ++++++++++++++++++++++++++++-------
 include/linux/thermal.h             |    1 +
 3 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
index 8459d45..bcd50b5 100644
--- a/Documentation/thermal/sysfs-api.txt
+++ b/Documentation/thermal/sysfs-api.txt
@@ -405,3 +405,7 @@ platform data is provided, this uses the step_wise throttling policy.
 This function serves as an arbitrator to set the state of a cooling
 device. It sets the cooling device to the deepest cooling state if
 possible.
+
+5.5:thermal_update_governor:
+This function update the thermal zone's governor to a new one.
+Returns 0 if success, an erro code otherwise.
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index a91684b..ef0fc40 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -130,6 +130,35 @@ exit:
 	return;
 }
 
+/**
+ * thermal_update_governor() - update the thermal zone device's governor
+ * to a new one.
+ * @tzd: pointer of thermal zone device, which need to update governor.
+ * @name: new governor name.
+ *
+ * Return: On success returns 0, an error code otherwise
+ */
+int thermal_update_governor(struct thermal_zone_device *tzd,
+			    const char *name)
+{
+	struct thermal_governor *new_gov;
+	int ret = 0;
+
+	mutex_lock(&thermal_governor_lock);
+
+	new_gov = __find_governor(name);
+	if (!new_gov) {
+		ret = -EINVAL;
+		goto exit;
+	}
+
+	tzd->governor = new_gov;
+
+exit:
+	mutex_unlock(&thermal_governor_lock);
+	return ret;
+}
+
 static int get_idr(struct idr *idr, struct mutex *lock, int *id)
 {
 	int ret;
@@ -734,22 +763,17 @@ policy_store(struct device *dev, struct device_attribute *attr,
 {
 	int ret = -EINVAL;
 	struct thermal_zone_device *tz = to_thermal_zone(dev);
-	struct thermal_governor *gov;
 	char name[THERMAL_NAME_LENGTH];
 
 	snprintf(name, sizeof(name), "%s", buf);
 
-	mutex_lock(&thermal_governor_lock);
-
-	gov = __find_governor(strim(name));
-	if (!gov)
+	ret = thermal_update_governor(tz, strim(name));
+	if (ret < 0)
 		goto exit;
 
-	tz->governor = gov;
 	ret = count;
 
 exit:
-	mutex_unlock(&thermal_governor_lock);
 	return ret;
 }
 
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index f7e11c7..715b3cd 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -293,6 +293,7 @@ struct thermal_instance *get_thermal_instance(struct thermal_zone_device *,
 		struct thermal_cooling_device *, int);
 void thermal_cdev_update(struct thermal_cooling_device *);
 void thermal_notify_framework(struct thermal_zone_device *, int);
+int thermal_update_governor(struct thermal_zone_device *, const char *);
 
 #ifdef CONFIG_NET
 extern int thermal_generate_netlink_event(struct thermal_zone_device *tz,
-- 
1.7.9.5

Re: [PATCH v4 2/2] thermal: add interface to support tune governor in run-time

[Wei Ni]

On 01/20/2014 05:55 PM, Wei Ni wrote:
> In the of-thermal driver, it register a thermal zone device
> without governor, since the governers are not static, we
> should be able to update to a new one in run-time, so I add
> a new fucntion thermal_update_governor() to do it.
> 
> Change-Id: Ie4995d6404d6d4f16250958487f3e3b8f63fc4c6

Oh, I need to remove change id.

> Signed-off-by: Wei Ni <wni@nvidia.com>
> ---
>  Documentation/thermal/sysfs-api.txt |    4 ++++
>  drivers/thermal/thermal_core.c      |   38 ++++++++++++++++++++++++++++-------
>  include/linux/thermal.h             |    1 +
>  3 files changed, 36 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
> index 8459d45..bcd50b5 100644
> --- a/Documentation/thermal/sysfs-api.txt
> +++ b/Documentation/thermal/sysfs-api.txt
> @@ -405,3 +405,7 @@ platform data is provided, this uses the step_wise throttling policy.
>  This function serves as an arbitrator to set the state of a cooling
>  device. It sets the cooling device to the deepest cooling state if
>  possible.
> +
> +5.5:thermal_update_governor:
> +This function update the thermal zone's governor to a new one.
> +Returns 0 if success, an erro code otherwise.
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index a91684b..ef0fc40 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -130,6 +130,35 @@ exit:
>  	return;
>  }
>  
> +/**
> + * thermal_update_governor() - update the thermal zone device's governor
> + * to a new one.
> + * @tzd: pointer of thermal zone device, which need to update governor.
> + * @name: new governor name.
> + *
> + * Return: On success returns 0, an error code otherwise
> + */
> +int thermal_update_governor(struct thermal_zone_device *tzd,
> +			    const char *name)
> +{
> +	struct thermal_governor *new_gov;
> +	int ret = 0;
> +
> +	mutex_lock(&thermal_governor_lock);
> +
> +	new_gov = __find_governor(name);
> +	if (!new_gov) {
> +		ret = -EINVAL;
> +		goto exit;
> +	}
> +
> +	tzd->governor = new_gov;
> +
> +exit:
> +	mutex_unlock(&thermal_governor_lock);
> +	return ret;
> +}
> +
>  static int get_idr(struct idr *idr, struct mutex *lock, int *id)
>  {
>  	int ret;
> @@ -734,22 +763,17 @@ policy_store(struct device *dev, struct device_attribute *attr,
>  {
>  	int ret = -EINVAL;
>  	struct thermal_zone_device *tz = to_thermal_zone(dev);
> -	struct thermal_governor *gov;
>  	char name[THERMAL_NAME_LENGTH];
>  
>  	snprintf(name, sizeof(name), "%s", buf);
>  
> -	mutex_lock(&thermal_governor_lock);
> -
> -	gov = __find_governor(strim(name));
> -	if (!gov)
> +	ret = thermal_update_governor(tz, strim(name));
> +	if (ret < 0)
>  		goto exit;
>  
> -	tz->governor = gov;
>  	ret = count;
>  
>  exit:
> -	mutex_unlock(&thermal_governor_lock);
>  	return ret;
>  }
>  
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index f7e11c7..715b3cd 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -293,6 +293,7 @@ struct thermal_instance *get_thermal_instance(struct thermal_zone_device *,
>  		struct thermal_cooling_device *, int);
>  void thermal_cdev_update(struct thermal_cooling_device *);
>  void thermal_notify_framework(struct thermal_zone_device *, int);
> +int thermal_update_governor(struct thermal_zone_device *, const char *);
>  
>  #ifdef CONFIG_NET
>  extern int thermal_generate_netlink_event(struct thermal_zone_device *tz,
> 


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

Re: [PATCH v4 2/2] thermal: add interface to support tune governor in run-time

[Eduardo Valentin]

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

On 20-01-2014 05:58, Wei Ni wrote:
> On 01/20/2014 05:55 PM, Wei Ni wrote:
>> In the of-thermal driver, it register a thermal zone device
>> without governor, since the governers are not static, we

s/governer/governor/g

>> should be able to update to a new one in run-time, so I add
>> a new fucntion thermal_update_governor() to do it.

s/fucntion/function/g

>>
>> Change-Id: Ie4995d6404d6d4f16250958487f3e3b8f63fc4c6
> 
> Oh, I need to remove change id.

Yup.

At first glance, I am not against having this API. The only request I
make is to merge this only if  we have at least one real use(r) of it.

> 
>> Signed-off-by: Wei Ni <wni@nvidia.com>
>> ---
>>  Documentation/thermal/sysfs-api.txt |    4 ++++
>>  drivers/thermal/thermal_core.c      |   38 ++++++++++++++++++++++++++++-------
>>  include/linux/thermal.h             |    1 +
>>  3 files changed, 36 insertions(+), 7 deletions(-)
>>
>> diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
>> index 8459d45..bcd50b5 100644
>> --- a/Documentation/thermal/sysfs-api.txt
>> +++ b/Documentation/thermal/sysfs-api.txt
>> @@ -405,3 +405,7 @@ platform data is provided, this uses the step_wise throttling policy.
>>  This function serves as an arbitrator to set the state of a cooling
>>  device. It sets the cooling device to the deepest cooling state if
>>  possible.
>> +
>> +5.5:thermal_update_governor:
>> +This function update the thermal zone's governor to a new one.
>> +Returns 0 if success, an erro code otherwise.
>> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
>> index a91684b..ef0fc40 100644
>> --- a/drivers/thermal/thermal_core.c
>> +++ b/drivers/thermal/thermal_core.c
>> @@ -130,6 +130,35 @@ exit:
>>  	return;
>>  }
>>  
>> +/**
>> + * thermal_update_governor() - update the thermal zone device's governor
>> + * to a new one.
>> + * @tzd: pointer of thermal zone device, which need to update governor.
>> + * @name: new governor name.
>> + *
>> + * Return: On success returns 0, an error code otherwise
>> + */
>> +int thermal_update_governor(struct thermal_zone_device *tzd,
>> +			    const char *name)
>> +{
>> +	struct thermal_governor *new_gov;
>> +	int ret = 0;
>> +
>> +	mutex_lock(&thermal_governor_lock);
>> +
>> +	new_gov = __find_governor(name);
>> +	if (!new_gov) {
>> +		ret = -EINVAL;
>> +		goto exit;
>> +	}
>> +
>> +	tzd->governor = new_gov;
>> +
>> +exit:
>> +	mutex_unlock(&thermal_governor_lock);
>> +	return ret;
>> +}
>> +
>>  static int get_idr(struct idr *idr, struct mutex *lock, int *id)
>>  {
>>  	int ret;
>> @@ -734,22 +763,17 @@ policy_store(struct device *dev, struct device_attribute *attr,
>>  {
>>  	int ret = -EINVAL;
>>  	struct thermal_zone_device *tz = to_thermal_zone(dev);
>> -	struct thermal_governor *gov;
>>  	char name[THERMAL_NAME_LENGTH];
>>  
>>  	snprintf(name, sizeof(name), "%s", buf);
>>  
>> -	mutex_lock(&thermal_governor_lock);
>> -
>> -	gov = __find_governor(strim(name));
>> -	if (!gov)
>> +	ret = thermal_update_governor(tz, strim(name));
>> +	if (ret < 0)
>>  		goto exit;
>>  
>> -	tz->governor = gov;
>>  	ret = count;
>>  
>>  exit:
>> -	mutex_unlock(&thermal_governor_lock);
>>  	return ret;
>>  }
>>  
>> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>> index f7e11c7..715b3cd 100644
>> --- a/include/linux/thermal.h
>> +++ b/include/linux/thermal.h
>> @@ -293,6 +293,7 @@ struct thermal_instance *get_thermal_instance(struct thermal_zone_device *,
>>  		struct thermal_cooling_device *, int);
>>  void thermal_cdev_update(struct thermal_cooling_device *);
>>  void thermal_notify_framework(struct thermal_zone_device *, int);
>> +int thermal_update_governor(struct thermal_zone_device *, const char *);
>>  
>>  #ifdef CONFIG_NET
>>  extern int thermal_generate_netlink_event(struct thermal_zone_device *tz,
>>
> 
> 
> -----------------------------------------------------------------------------------
> This email message is for the sole use of the intended recipient(s) and may contain
> confidential information.  Any unauthorized review, use, disclosure or distribution
> is prohibited.  If you are not the intended recipient, please contact the sender by
> reply email and destroy all copies of the original message.
> -----------------------------------------------------------------------------------
> 
> 


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

Re: [PATCH v4 0/2] Support to tune governor in run time

[Matthew Longnecker]

Wei,

On 1/20/2014 1:55 AM, Wei Ni wrote:
> This serie can support to turn governor for thermal zone in
> run time.
> Adds avaiable_policies attribute, so the user can get available
> governor policies and change te governor to a new one.
> Adds thermal_update_governor() function, so the thermal platform
> driver can use it to update governor.


I recommend the name thermal_select_governor rather than 
thermal_update_governor.

The name thermal_update_governor sounds like it should be similar to 
thermal_update_zone. But, the two are very different. One is used for 
switching to a new governor and the other is used to trigger thermal 
response for the existing zone.

-Matt

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

Re: [PATCH v4 1/2] thermal: add available policies attribute

[Zhang Rui]

On Mon, 2014-01-20 at 17:55 +0800, Wei Ni wrote:
> The Linux thermal framework support to change thermal governor
> policy in userspace, but it can't show what available policies
> supported.
> 
> This patch adds available_policies attribute to the thermal
> framework, it can list the thermal governors which can be
> used for a particular zone. This attribute is read only.
> 
> Signed-off-by: Wei Ni <wni@nvidia.com>

applied.

thanks,
rui
> ---
>  Documentation/thermal/sysfs-api.txt |    6 ++++++
>  drivers/thermal/thermal_core.c      |   28 ++++++++++++++++++++++++++++
>  2 files changed, 34 insertions(+)
> 
> diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
> index 87519cb..8459d45 100644
> --- a/Documentation/thermal/sysfs-api.txt
> +++ b/Documentation/thermal/sysfs-api.txt
> @@ -172,6 +172,7 @@ Thermal zone device sys I/F, created once it's registered:
>      |---temp:			Current temperature
>      |---mode:			Working mode of the thermal zone
>      |---policy:			Thermal governor used for this zone
> +    |---available_policies	Available thermal governors for this zone
>      |---trip_point_[0-*]_temp:	Trip point temperature
>      |---trip_point_[0-*]_type:	Trip point type
>      |---trip_point_[0-*]_hyst:	Hysteresis value for this trip point
> @@ -238,6 +239,10 @@ policy
>  	One of the various thermal governors used for a particular zone.
>  	RW, Required
>  
> +available_policies
> +	Available thermal governors which can be used for a particular zone.
> +	RO, Required
> +
>  trip_point_[0-*]_temp
>  	The temperature above which trip point will be fired.
>  	Unit: millidegree Celsius
> @@ -330,6 +335,7 @@ method, the sys I/F structure will be built like this:
>      |---temp:			37000
>      |---mode:			enabled
>      |---policy:			step_wise
> +    |---available_policies:	step_wise fair_share
>      |---trip_point_0_temp:	100000
>      |---trip_point_0_type:	critical
>      |---trip_point_1_temp:	80000
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 338a88b..a91684b 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -761,6 +761,27 @@ policy_show(struct device *dev, struct device_attribute *devattr, char *buf)
>  	return sprintf(buf, "%s\n", tz->governor->name);
>  }
>  
> +static ssize_t
> +available_policies_show(struct device *dev, struct device_attribute *devattr,
> +			char *buf)
> +{
> +	struct thermal_governor *pos;
> +	ssize_t count = 0;
> +	ssize_t size = 0;
> +
> +	mutex_lock(&thermal_governor_lock);
> +
> +	list_for_each_entry(pos, &thermal_governor_list, governor_list) {
> +		size = PAGE_SIZE - count;
> +		count += scnprintf(buf + count, size, "%s ", pos->name);
> +	}
> +	count += scnprintf(buf + count, size, "\n");
> +
> +	mutex_unlock(&thermal_governor_lock);
> +
> +	return count;
> +}
> +
>  #ifdef CONFIG_THERMAL_EMULATION
>  static ssize_t
>  emul_temp_store(struct device *dev, struct device_attribute *attr,
> @@ -794,6 +815,7 @@ static DEVICE_ATTR(temp, 0444, temp_show, NULL);
>  static DEVICE_ATTR(mode, 0644, mode_show, mode_store);
>  static DEVICE_ATTR(passive, S_IRUGO | S_IWUSR, passive_show, passive_store);
>  static DEVICE_ATTR(policy, S_IRUGO | S_IWUSR, policy_show, policy_store);
> +static DEVICE_ATTR(available_policies, S_IRUGO, available_policies_show, NULL);
>  
>  /* sys I/F for cooling device */
>  #define to_cooling_device(_dev)	\
> @@ -1527,6 +1549,11 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,
>  	if (result)
>  		goto unregister;
>  
> +	/* Create available_policies attribute */
> +	result = device_create_file(&tz->device, &dev_attr_available_policies);
> +	if (result)
> +		goto unregister;
> +
>  	/* Update 'this' zone's governor information */
>  	mutex_lock(&thermal_governor_lock);
>  
> @@ -1622,6 +1649,7 @@ void thermal_zone_device_unregister(struct thermal_zone_device *tz)
>  	if (tz->ops->get_mode)
>  		device_remove_file(&tz->device, &dev_attr_mode);
>  	device_remove_file(&tz->device, &dev_attr_policy);
> +	device_remove_file(&tz->device, &dev_attr_available_policies);
>  	remove_trip_attrs(tz);
>  	tz->governor = NULL;
>  

Re: [PATCH v4 2/2] thermal: add interface to support tune governor in run-time

[Zhang Rui]

On Mon, 2014-01-20 at 17:55 +0800, Wei Ni wrote:
> In the of-thermal driver, it register a thermal zone device
> without governor,

I think this would be fixed by
https://patchwork.kernel.org/patch/3730411/
right?


>  since the governers are not static, we
> should be able to update to a new one in run-time, so I add
> a new fucntion thermal_update_governor() to do it.
> 
With the patch mentioned above, the of thermal zones will run with the
def_governor.
do we still need to switch to a different governor at runtime?

thanks,
rui

> Change-Id: Ie4995d6404d6d4f16250958487f3e3b8f63fc4c6
> Signed-off-by: Wei Ni <wni@nvidia.com>
> ---
>  Documentation/thermal/sysfs-api.txt |    4 ++++
>  drivers/thermal/thermal_core.c      |   38 ++++++++++++++++++++++++++++-------
>  include/linux/thermal.h             |    1 +
>  3 files changed, 36 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
> index 8459d45..bcd50b5 100644
> --- a/Documentation/thermal/sysfs-api.txt
> +++ b/Documentation/thermal/sysfs-api.txt
> @@ -405,3 +405,7 @@ platform data is provided, this uses the step_wise throttling policy.
>  This function serves as an arbitrator to set the state of a cooling
>  device. It sets the cooling device to the deepest cooling state if
>  possible.
> +
> +5.5:thermal_update_governor:
> +This function update the thermal zone's governor to a new one.
> +Returns 0 if success, an erro code otherwise.
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index a91684b..ef0fc40 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -130,6 +130,35 @@ exit:
>  	return;
>  }
>  
> +/**
> + * thermal_update_governor() - update the thermal zone device's governor
> + * to a new one.
> + * @tzd: pointer of thermal zone device, which need to update governor.
> + * @name: new governor name.
> + *
> + * Return: On success returns 0, an error code otherwise
> + */
> +int thermal_update_governor(struct thermal_zone_device *tzd,
> +			    const char *name)
> +{
> +	struct thermal_governor *new_gov;
> +	int ret = 0;
> +
> +	mutex_lock(&thermal_governor_lock);
> +
> +	new_gov = __find_governor(name);
> +	if (!new_gov) {
> +		ret = -EINVAL;
> +		goto exit;
> +	}
> +
> +	tzd->governor = new_gov;
> +
> +exit:
> +	mutex_unlock(&thermal_governor_lock);
> +	return ret;
> +}
> +
>  static int get_idr(struct idr *idr, struct mutex *lock, int *id)
>  {
>  	int ret;
> @@ -734,22 +763,17 @@ policy_store(struct device *dev, struct device_attribute *attr,
>  {
>  	int ret = -EINVAL;
>  	struct thermal_zone_device *tz = to_thermal_zone(dev);
> -	struct thermal_governor *gov;
>  	char name[THERMAL_NAME_LENGTH];
>  
>  	snprintf(name, sizeof(name), "%s", buf);
>  
> -	mutex_lock(&thermal_governor_lock);
> -
> -	gov = __find_governor(strim(name));
> -	if (!gov)
> +	ret = thermal_update_governor(tz, strim(name));
> +	if (ret < 0)
>  		goto exit;
>  
> -	tz->governor = gov;
>  	ret = count;
>  
>  exit:
> -	mutex_unlock(&thermal_governor_lock);
>  	return ret;
>  }
>  
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index f7e11c7..715b3cd 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -293,6 +293,7 @@ struct thermal_instance *get_thermal_instance(struct thermal_zone_device *,
>  		struct thermal_cooling_device *, int);
>  void thermal_cdev_update(struct thermal_cooling_device *);
>  void thermal_notify_framework(struct thermal_zone_device *, int);
> +int thermal_update_governor(struct thermal_zone_device *, const char *);
>  
>  #ifdef CONFIG_NET
>  extern int thermal_generate_netlink_event(struct thermal_zone_device *tz,

Re: [PATCH v4 2/2] thermal: add interface to support tune governor in run-time

[Wei Ni]

On 02/27/2014 03:10 PM, Zhang Rui wrote:
> On Mon, 2014-01-20 at 17:55 +0800, Wei Ni wrote:
>> In the of-thermal driver, it register a thermal zone device
>> without governor,
> 
> I think this would be fixed by
> https://patchwork.kernel.org/patch/3730411/
> right?

Yes, I think so.

> 
> 
>>  since the governers are not static, we
>> should be able to update to a new one in run-time, so I add
>> a new fucntion thermal_update_governor() to do it.
>>
> With the patch mentioned above, the of thermal zones will run with the
> def_governor.
> do we still need to switch to a different governor at runtime?

In some case, the user will want to use other governor, not the
def_governor, then he can use this function. for example:
...
/* use of-thermal to register */
tzd = thermal_zone_of_sensor_register();
/* assume the "step_wise" is def_governor */
thermal_update_governor(tzd, "fair_share");
...

Thanks.
Wei.

> 
> thanks,
> rui
> 
>> Change-Id: Ie4995d6404d6d4f16250958487f3e3b8f63fc4c6
>> Signed-off-by: Wei Ni <wni@nvidia.com>
>> ---
>>  Documentation/thermal/sysfs-api.txt |    4 ++++
>>  drivers/thermal/thermal_core.c      |   38 ++++++++++++++++++++++++++++-------
>>  include/linux/thermal.h             |    1 +
>>  3 files changed, 36 insertions(+), 7 deletions(-)
>>
>> diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
>> index 8459d45..bcd50b5 100644
>> --- a/Documentation/thermal/sysfs-api.txt
>> +++ b/Documentation/thermal/sysfs-api.txt
>> @@ -405,3 +405,7 @@ platform data is provided, this uses the step_wise throttling policy.
>>  This function serves as an arbitrator to set the state of a cooling
>>  device. It sets the cooling device to the deepest cooling state if
>>  possible.
>> +
>> +5.5:thermal_update_governor:
>> +This function update the thermal zone's governor to a new one.
>> +Returns 0 if success, an erro code otherwise.
>> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
>> index a91684b..ef0fc40 100644
>> --- a/drivers/thermal/thermal_core.c
>> +++ b/drivers/thermal/thermal_core.c
>> @@ -130,6 +130,35 @@ exit:
>>  	return;
>>  }
>>  
>> +/**
>> + * thermal_update_governor() - update the thermal zone device's governor
>> + * to a new one.
>> + * @tzd: pointer of thermal zone device, which need to update governor.
>> + * @name: new governor name.
>> + *
>> + * Return: On success returns 0, an error code otherwise
>> + */
>> +int thermal_update_governor(struct thermal_zone_device *tzd,
>> +			    const char *name)
>> +{
>> +	struct thermal_governor *new_gov;
>> +	int ret = 0;
>> +
>> +	mutex_lock(&thermal_governor_lock);
>> +
>> +	new_gov = __find_governor(name);
>> +	if (!new_gov) {
>> +		ret = -EINVAL;
>> +		goto exit;
>> +	}
>> +
>> +	tzd->governor = new_gov;
>> +
>> +exit:
>> +	mutex_unlock(&thermal_governor_lock);
>> +	return ret;
>> +}
>> +
>>  static int get_idr(struct idr *idr, struct mutex *lock, int *id)
>>  {
>>  	int ret;
>> @@ -734,22 +763,17 @@ policy_store(struct device *dev, struct device_attribute *attr,
>>  {
>>  	int ret = -EINVAL;
>>  	struct thermal_zone_device *tz = to_thermal_zone(dev);
>> -	struct thermal_governor *gov;
>>  	char name[THERMAL_NAME_LENGTH];
>>  
>>  	snprintf(name, sizeof(name), "%s", buf);
>>  
>> -	mutex_lock(&thermal_governor_lock);
>> -
>> -	gov = __find_governor(strim(name));
>> -	if (!gov)
>> +	ret = thermal_update_governor(tz, strim(name));
>> +	if (ret < 0)
>>  		goto exit;
>>  
>> -	tz->governor = gov;
>>  	ret = count;
>>  
>>  exit:
>> -	mutex_unlock(&thermal_governor_lock);
>>  	return ret;
>>  }
>>  
>> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>> index f7e11c7..715b3cd 100644
>> --- a/include/linux/thermal.h
>> +++ b/include/linux/thermal.h
>> @@ -293,6 +293,7 @@ struct thermal_instance *get_thermal_instance(struct thermal_zone_device *,
>>  		struct thermal_cooling_device *, int);
>>  void thermal_cdev_update(struct thermal_cooling_device *);
>>  void thermal_notify_framework(struct thermal_zone_device *, int);
>> +int thermal_update_governor(struct thermal_zone_device *, const char *);
>>  
>>  #ifdef CONFIG_NET
>>  extern int thermal_generate_netlink_event(struct thermal_zone_device *tz,
> 
> 


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

Re: [PATCH v4 2/2] thermal: add interface to support tune governor in run-time

[Zhang Rui]

On Thu, 2014-02-27 at 16:00 +0800, Wei Ni wrote:
> On 02/27/2014 03:10 PM, Zhang Rui wrote:
> > On Mon, 2014-01-20 at 17:55 +0800, Wei Ni wrote:
> >> In the of-thermal driver, it register a thermal zone device
> >> without governor,
> > 
> > I think this would be fixed by
> > https://patchwork.kernel.org/patch/3730411/
> > right?
> 
> Yes, I think so.
> 
> > 
> > 
> >>  since the governers are not static, we
> >> should be able to update to a new one in run-time, so I add
> >> a new fucntion thermal_update_governor() to do it.
> >>
> > With the patch mentioned above, the of thermal zones will run with the
> > def_governor.
> > do we still need to switch to a different governor at runtime?
> 
> In some case, the user will want to use other governor, not the
> def_governor, then he can use this function. for example:
> ...
> /* use of-thermal to register */
> tzd = thermal_zone_of_sensor_register();
> /* assume the "step_wise" is def_governor */
> thermal_update_governor(tzd, "fair_share");
> ...
> 
yes, I know.
If you need the code above in any platform thermal driver, it would be
good to submit this patch together with that change, and I'll be glad to
take it. Or else, my concern is that we're introducing an API without
any real users.

thanks,
rui
> Thanks.
> Wei.
> 
> > 
> > thanks,
> > rui
> > 
> >> Change-Id: Ie4995d6404d6d4f16250958487f3e3b8f63fc4c6
> >> Signed-off-by: Wei Ni <wni@nvidia.com>
> >> ---
> >>  Documentation/thermal/sysfs-api.txt |    4 ++++
> >>  drivers/thermal/thermal_core.c      |   38 ++++++++++++++++++++++++++++-------
> >>  include/linux/thermal.h             |    1 +
> >>  3 files changed, 36 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
> >> index 8459d45..bcd50b5 100644
> >> --- a/Documentation/thermal/sysfs-api.txt
> >> +++ b/Documentation/thermal/sysfs-api.txt
> >> @@ -405,3 +405,7 @@ platform data is provided, this uses the step_wise throttling policy.
> >>  This function serves as an arbitrator to set the state of a cooling
> >>  device. It sets the cooling device to the deepest cooling state if
> >>  possible.
> >> +
> >> +5.5:thermal_update_governor:
> >> +This function update the thermal zone's governor to a new one.
> >> +Returns 0 if success, an erro code otherwise.
> >> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> >> index a91684b..ef0fc40 100644
> >> --- a/drivers/thermal/thermal_core.c
> >> +++ b/drivers/thermal/thermal_core.c
> >> @@ -130,6 +130,35 @@ exit:
> >>  	return;
> >>  }
> >>  
> >> +/**
> >> + * thermal_update_governor() - update the thermal zone device's governor
> >> + * to a new one.
> >> + * @tzd: pointer of thermal zone device, which need to update governor.
> >> + * @name: new governor name.
> >> + *
> >> + * Return: On success returns 0, an error code otherwise
> >> + */
> >> +int thermal_update_governor(struct thermal_zone_device *tzd,
> >> +			    const char *name)
> >> +{
> >> +	struct thermal_governor *new_gov;
> >> +	int ret = 0;
> >> +
> >> +	mutex_lock(&thermal_governor_lock);
> >> +
> >> +	new_gov = __find_governor(name);
> >> +	if (!new_gov) {
> >> +		ret = -EINVAL;
> >> +		goto exit;
> >> +	}
> >> +
> >> +	tzd->governor = new_gov;
> >> +
> >> +exit:
> >> +	mutex_unlock(&thermal_governor_lock);
> >> +	return ret;
> >> +}
> >> +
> >>  static int get_idr(struct idr *idr, struct mutex *lock, int *id)
> >>  {
> >>  	int ret;
> >> @@ -734,22 +763,17 @@ policy_store(struct device *dev, struct device_attribute *attr,
> >>  {
> >>  	int ret = -EINVAL;
> >>  	struct thermal_zone_device *tz = to_thermal_zone(dev);
> >> -	struct thermal_governor *gov;
> >>  	char name[THERMAL_NAME_LENGTH];
> >>  
> >>  	snprintf(name, sizeof(name), "%s", buf);
> >>  
> >> -	mutex_lock(&thermal_governor_lock);
> >> -
> >> -	gov = __find_governor(strim(name));
> >> -	if (!gov)
> >> +	ret = thermal_update_governor(tz, strim(name));
> >> +	if (ret < 0)
> >>  		goto exit;
> >>  
> >> -	tz->governor = gov;
> >>  	ret = count;
> >>  
> >>  exit:
> >> -	mutex_unlock(&thermal_governor_lock);
> >>  	return ret;
> >>  }
> >>  
> >> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> >> index f7e11c7..715b3cd 100644
> >> --- a/include/linux/thermal.h
> >> +++ b/include/linux/thermal.h
> >> @@ -293,6 +293,7 @@ struct thermal_instance *get_thermal_instance(struct thermal_zone_device *,
> >>  		struct thermal_cooling_device *, int);
> >>  void thermal_cdev_update(struct thermal_cooling_device *);
> >>  void thermal_notify_framework(struct thermal_zone_device *, int);
> >> +int thermal_update_governor(struct thermal_zone_device *, const char *);
> >>  
> >>  #ifdef CONFIG_NET
> >>  extern int thermal_generate_netlink_event(struct thermal_zone_device *tz,
> > 
> > 
> 
> 
> -----------------------------------------------------------------------------------
> This email message is for the sole use of the intended recipient(s) and may contain
> confidential information.  Any unauthorized review, use, disclosure or distribution
> is prohibited.  If you are not the intended recipient, please contact the sender by
> reply email and destroy all copies of the original message.
> -----------------------------------------------------------------------------------
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 2/2] thermal: add interface to support tune governor in run-time

[Wei Ni]

On 02/27/2014 04:06 PM, Zhang Rui wrote:
> On Thu, 2014-02-27 at 16:00 +0800, Wei Ni wrote:
>> On 02/27/2014 03:10 PM, Zhang Rui wrote:
>>> On Mon, 2014-01-20 at 17:55 +0800, Wei Ni wrote:
>>>> In the of-thermal driver, it register a thermal zone device
>>>> without governor,
>>>
>>> I think this would be fixed by
>>> https://patchwork.kernel.org/patch/3730411/
>>> right?
>>
>> Yes, I think so.
>>
>>>
>>>
>>>>  since the governers are not static, we
>>>> should be able to update to a new one in run-time, so I add
>>>> a new fucntion thermal_update_governor() to do it.
>>>>
>>> With the patch mentioned above, the of thermal zones will run with the
>>> def_governor.
>>> do we still need to switch to a different governor at runtime?
>>
>> In some case, the user will want to use other governor, not the
>> def_governor, then he can use this function. for example:
>> ...
>> /* use of-thermal to register */
>> tzd = thermal_zone_of_sensor_register();
>> /* assume the "step_wise" is def_governor */
>> thermal_update_governor(tzd, "fair_share");
>> ...
>>
> yes, I know.
> If you need the code above in any platform thermal driver, it would be
> good to submit this patch together with that change, and I'll be glad to
> take it. Or else, my concern is that we're introducing an API without
> any real users.

Sure, it's ok.

> 
> thanks,
> rui
>> Thanks.
>> Wei.
>>
>>>
>>> thanks,
>>> rui
>>>
>>>> Change-Id: Ie4995d6404d6d4f16250958487f3e3b8f63fc4c6
>>>> Signed-off-by: Wei Ni <wni@nvidia.com>
>>>> ---
>>>>  Documentation/thermal/sysfs-api.txt |    4 ++++
>>>>  drivers/thermal/thermal_core.c      |   38 ++++++++++++++++++++++++++++-------
>>>>  include/linux/thermal.h             |    1 +
>>>>  3 files changed, 36 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
>>>> index 8459d45..bcd50b5 100644
>>>> --- a/Documentation/thermal/sysfs-api.txt
>>>> +++ b/Documentation/thermal/sysfs-api.txt
>>>> @@ -405,3 +405,7 @@ platform data is provided, this uses the step_wise throttling policy.
>>>>  This function serves as an arbitrator to set the state of a cooling
>>>>  device. It sets the cooling device to the deepest cooling state if
>>>>  possible.
>>>> +
>>>> +5.5:thermal_update_governor:
>>>> +This function update the thermal zone's governor to a new one.
>>>> +Returns 0 if success, an erro code otherwise.
>>>> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
>>>> index a91684b..ef0fc40 100644
>>>> --- a/drivers/thermal/thermal_core.c
>>>> +++ b/drivers/thermal/thermal_core.c
>>>> @@ -130,6 +130,35 @@ exit:
>>>>  	return;
>>>>  }
>>>>  
>>>> +/**
>>>> + * thermal_update_governor() - update the thermal zone device's governor
>>>> + * to a new one.
>>>> + * @tzd: pointer of thermal zone device, which need to update governor.
>>>> + * @name: new governor name.
>>>> + *
>>>> + * Return: On success returns 0, an error code otherwise
>>>> + */
>>>> +int thermal_update_governor(struct thermal_zone_device *tzd,
>>>> +			    const char *name)
>>>> +{
>>>> +	struct thermal_governor *new_gov;
>>>> +	int ret = 0;
>>>> +
>>>> +	mutex_lock(&thermal_governor_lock);
>>>> +
>>>> +	new_gov = __find_governor(name);
>>>> +	if (!new_gov) {
>>>> +		ret = -EINVAL;
>>>> +		goto exit;
>>>> +	}
>>>> +
>>>> +	tzd->governor = new_gov;
>>>> +
>>>> +exit:
>>>> +	mutex_unlock(&thermal_governor_lock);
>>>> +	return ret;
>>>> +}
>>>> +
>>>>  static int get_idr(struct idr *idr, struct mutex *lock, int *id)
>>>>  {
>>>>  	int ret;
>>>> @@ -734,22 +763,17 @@ policy_store(struct device *dev, struct device_attribute *attr,
>>>>  {
>>>>  	int ret = -EINVAL;
>>>>  	struct thermal_zone_device *tz = to_thermal_zone(dev);
>>>> -	struct thermal_governor *gov;
>>>>  	char name[THERMAL_NAME_LENGTH];
>>>>  
>>>>  	snprintf(name, sizeof(name), "%s", buf);
>>>>  
>>>> -	mutex_lock(&thermal_governor_lock);
>>>> -
>>>> -	gov = __find_governor(strim(name));
>>>> -	if (!gov)
>>>> +	ret = thermal_update_governor(tz, strim(name));
>>>> +	if (ret < 0)
>>>>  		goto exit;
>>>>  
>>>> -	tz->governor = gov;
>>>>  	ret = count;
>>>>  
>>>>  exit:
>>>> -	mutex_unlock(&thermal_governor_lock);
>>>>  	return ret;
>>>>  }
>>>>  
>>>> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>>>> index f7e11c7..715b3cd 100644
>>>> --- a/include/linux/thermal.h
>>>> +++ b/include/linux/thermal.h
>>>> @@ -293,6 +293,7 @@ struct thermal_instance *get_thermal_instance(struct thermal_zone_device *,
>>>>  		struct thermal_cooling_device *, int);
>>>>  void thermal_cdev_update(struct thermal_cooling_device *);
>>>>  void thermal_notify_framework(struct thermal_zone_device *, int);
>>>> +int thermal_update_governor(struct thermal_zone_device *, const char *);
>>>>  
>>>>  #ifdef CONFIG_NET
>>>>  extern int thermal_generate_netlink_event(struct thermal_zone_device *tz,
>>>
>>>
>>
>>
>> -----------------------------------------------------------------------------------
>> This email message is for the sole use of the intended recipient(s) and may contain
>> confidential information.  Any unauthorized review, use, disclosure or distribution
>> is prohibited.  If you are not the intended recipient, please contact the sender by
>> reply email and destroy all copies of the original message.
>> -----------------------------------------------------------------------------------
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>