On 三, 2012-07-25 at 13:07 +0200, Rafael J. Wysocki wrote:

On Wednesday, July 25, 2012, Zhang Rui wrote:

quoted

On 二, 2012-07-24 at 11:27 +0200, Rafael J. Wysocki wrote:

quoted

On Tuesday, July 24, 2012, Zhang Rui wrote:

quoted

On 四, 2012-07-19 at 23:19 +0200, Rafael J. Wysocki wrote:

quoted

On Thursday, July 19, 2012, Zhang Rui wrote:

quoted

This function is used to update the cooling state of
all the cooling devices that are binded to an active trip point.

s/binded/bound/

got it.

quoted

quoted

This will be used for passive cooling as well, in the future patches.
as both active and passive cooling can share the same algorithm,
which is

1. if the temperature is higher than a trip point,
   a. if the trend is THERMAL_TREND_RAISING, use higher cooling
      state for this trip point
   b. if the trend is THERMAL_TREND_DROPPING, use lower cooling
      state for this trip point

2. if the temperature is lower than a trip point, use lower
   cooling state for this trip point.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/acpi/thermal.c        |    7 +++-
 drivers/thermal/thermal_sys.c |   91 +++++++++++++++++++++++++++++------------
 2 files changed, 71 insertions(+), 27 deletions(-)

diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index 73e335f..14c4879 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c

@@ -715,7 +715,12 @@ static int thermal_get_trend(struct thermal_zone_device *thermal,
 	if (thermal_get_trip_type(thermal, trip, &type))
 		return -EINVAL;
 
-	/* Only PASSIVE trip points need TREND */
+	if (type == THERMAL_TRIP_ACTIVE) {
+		/* aggressive active cooling */
+		*trend = THERMAL_TREND_RAISING;
+		return 0;

Please move that into thermal_zone_trip_update() directly, unless you
need it elsewhere.

IMO, I can say that ACPI thermal wants aggressive active cooling, as it
will never spin down the fan when the temperature is higher than the
trip point.

I meant the code organization, not the functionality. :-)

sure.

quoted

Since thermal_get_trend() is static, it is not used outside of this file,
so you can move the "if (type == THERMAL_TRIP_ACTIVE)" conditional from it
directly into the caller (unless there are more callers, which I'm not sure
about without reading the whole code again).

sorry I still do not get it.
the generic thermal layer is the caller of this callback.

I'm not talking about the callback, but of the static function with the
same name you've introduced into drivers/thermal/thermal_sys.c in the
previous patch. :-)

The only caller of this function seems to be thermal_zone_device_passive()
and my point is that it would be better to simply put the code from that
function into thermal_zone_device_passive() directly, unless there are
more callers added by the subsequent patches, which I'm not sure about.

hah, I understand.
yes, I can move thermal_sys.c thermal_get_trend() in to
thermal_trip_update.
and there will be no such confusion (two functions share the same name)
any more. :)

thanks,
rui

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html