linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Thermal: Don't resolve THERMAL_NO_LIMIT to max_state.
@ 2013-03-26 13:29 Yuxuan Shui
  2013-04-12  1:38 ` Zhang Rui
  0 siblings, 1 reply; 2+ messages in thread
From: Yuxuan Shui @ 2013-03-26 13:29 UTC (permalink / raw)
  To: rui.zhang; +Cc: linux-pm, linux-kernel, Yuxuan Shui

max_state may change at runtime, for example, when loading/unloading
cpufreq policy.

Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>
---
 drivers/thermal/step_wise.c   | 11 ++++++++---
 drivers/thermal/thermal_sys.c |  5 ++---
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/thermal/step_wise.c b/drivers/thermal/step_wise.c
index 407cde3..2edb7e9 100644
--- a/drivers/thermal/step_wise.c
+++ b/drivers/thermal/step_wise.c
@@ -54,18 +54,23 @@ static unsigned long get_target_state(struct thermal_instance *instance,
 {
 	struct thermal_cooling_device *cdev = instance->cdev;
 	unsigned long cur_state;
+	unsigned long max_state;
+	int upper;
 
 	cdev->ops->get_cur_state(cdev, &cur_state);
+	cdev->ops->get_max_state(cdev, &max_state);
+
+	upper = instance->upper == THERMAL_NO_LIMIT ? max_state : instance->upper;
 
 	switch (trend) {
 	case THERMAL_TREND_RAISING:
 		if (throttle)
-			cur_state = cur_state < instance->upper ?
-				    (cur_state + 1) : instance->upper;
+			cur_state = cur_state < upper ?
+				    (cur_state + 1) : upper;
 		break;
 	case THERMAL_TREND_RAISE_FULL:
 		if (throttle)
-			cur_state = instance->upper;
+			cur_state = upper;
 		break;
 	case THERMAL_TREND_DROPPING:
 		if (cur_state == instance->lower) {
diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 5b7863a..f02e4d4 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -1134,11 +1134,10 @@ int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
 
 	cdev->ops->get_max_state(cdev, &max_state);
 
-	/* lower default 0, upper default max_state */
+	/* lower default 0 */
 	lower = lower == THERMAL_NO_LIMIT ? 0 : lower;
-	upper = upper == THERMAL_NO_LIMIT ? max_state : upper;
 
-	if (lower > upper || upper > max_state)
+	if (lower > upper || (upper != THERMAL_NO_LIMIT && upper > max_state))
 		return -EINVAL;
 
 	dev =
-- 
1.8.2


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] Thermal: Don't resolve THERMAL_NO_LIMIT to max_state.
  2013-03-26 13:29 [PATCH] Thermal: Don't resolve THERMAL_NO_LIMIT to max_state Yuxuan Shui
@ 2013-04-12  1:38 ` Zhang Rui
  0 siblings, 0 replies; 2+ messages in thread
From: Zhang Rui @ 2013-04-12  1:38 UTC (permalink / raw)
  To: Yuxuan Shui; +Cc: linux-pm, linux-kernel, Amit Kachhap, durga, eduardo

On Tue, 2013-03-26 at 21:29 +0800, Yuxuan Shui wrote:
> max_state may change at runtime, for example, when loading/unloading
> cpufreq policy.
> 
this seems to be a problem that we have not covered yet.

when loading/unloading the cpufreq policy, the cpufreq_frequency_table
will be changed as well, right?
what if we want to use p4 for trip point 2, and then there are only two
valid entries after reloading cpufreq policy?

To me, the real thing we missed here is a mechanism for dynamic
binding. 

thanks,
rui

> Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>
> ---
>  drivers/thermal/step_wise.c   | 11 ++++++++---
>  drivers/thermal/thermal_sys.c |  5 ++---
>  2 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/thermal/step_wise.c b/drivers/thermal/step_wise.c
> index 407cde3..2edb7e9 100644
> --- a/drivers/thermal/step_wise.c
> +++ b/drivers/thermal/step_wise.c
> @@ -54,18 +54,23 @@ static unsigned long get_target_state(struct thermal_instance *instance,
>  {
>  	struct thermal_cooling_device *cdev = instance->cdev;
>  	unsigned long cur_state;
> +	unsigned long max_state;
> +	int upper;
>  
>  	cdev->ops->get_cur_state(cdev, &cur_state);
> +	cdev->ops->get_max_state(cdev, &max_state);
> +
> +	upper = instance->upper == THERMAL_NO_LIMIT ? max_state : instance->upper;
>  
>  	switch (trend) {
>  	case THERMAL_TREND_RAISING:
>  		if (throttle)
> -			cur_state = cur_state < instance->upper ?
> -				    (cur_state + 1) : instance->upper;
> +			cur_state = cur_state < upper ?
> +				    (cur_state + 1) : upper;
>  		break;
>  	case THERMAL_TREND_RAISE_FULL:
>  		if (throttle)
> -			cur_state = instance->upper;
> +			cur_state = upper;
>  		break;
>  	case THERMAL_TREND_DROPPING:
>  		if (cur_state == instance->lower) {
> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> index 5b7863a..f02e4d4 100644
> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -1134,11 +1134,10 @@ int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
>  
>  	cdev->ops->get_max_state(cdev, &max_state);
>  
> -	/* lower default 0, upper default max_state */
> +	/* lower default 0 */
>  	lower = lower == THERMAL_NO_LIMIT ? 0 : lower;
> -	upper = upper == THERMAL_NO_LIMIT ? max_state : upper;
>  
> -	if (lower > upper || upper > max_state)
> +	if (lower > upper || (upper != THERMAL_NO_LIMIT && upper > max_state))
>  		return -EINVAL;
>  
>  	dev =



^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2013-04-12  1:38 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-26 13:29 [PATCH] Thermal: Don't resolve THERMAL_NO_LIMIT to max_state Yuxuan Shui
2013-04-12  1:38 ` Zhang Rui

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).