diff mbox

[RESEND,07/16] Thermal: Introduce .get_trend() callback.

Message ID 1343182273-32096-8-git-send-email-rui.zhang@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Zhang Rui July 25, 2012, 2:11 a.m. UTC
tc1 and tc2 are used by OSPM to anticipate the temperature trends.
But they are ACPI platform specific concepts.

Introduce .get_trend() as a more general solution.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/acpi/thermal.c        |   33 +++++++++++++++++++++++++++++++++
 drivers/thermal/thermal_sys.c |   22 ++++++++++++++++++++--
 include/linux/thermal.h       |    9 +++++++++
 3 files changed, 62 insertions(+), 2 deletions(-)

Comments

Rafael Wysocki July 25, 2012, 8:19 p.m. UTC | #1
On Wednesday, July 25, 2012, Zhang Rui wrote:
> tc1 and tc2 are used by OSPM to anticipate the temperature trends.
> But they are ACPI platform specific concepts.
> 
> Introduce .get_trend() as a more general solution.
> 
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
>  drivers/acpi/thermal.c        |   33 +++++++++++++++++++++++++++++++++
>  drivers/thermal/thermal_sys.c |   22 ++++++++++++++++++++--
>  include/linux/thermal.h       |    9 +++++++++
>  3 files changed, 62 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> index 0154eac..01c92fd 100644
> --- a/drivers/acpi/thermal.c
> +++ b/drivers/acpi/thermal.c
> @@ -704,6 +704,38 @@ static int thermal_get_crit_temp(struct thermal_zone_device *thermal,
>  		return -EINVAL;
>  }
>  
> +static int thermal_get_trend(struct thermal_zone_device *thermal,
> +				int trip, enum thermal_trend *trend)
> +{
> +	struct acpi_thermal *tz = thermal->devdata;
> +	enum thermal_trip_type type;
> +	int i;
> +
> +	if (thermal_get_trip_type(thermal, trip, &type))
> +		return -EINVAL;
> +
> +	/* Only PASSIVE trip points need TREND */
> +	if (type != THERMAL_TRIP_PASSIVE)
> +		return -EINVAL;
> +
> +	/*
> +	 * tz->temperature has already been updated by generic thermal layer,
> +	 * before this callback being invoked
> +	 */
> +	i = (tz->trips.passive.tc1 * (tz->temperature - tz->last_temperature))
> +		+ (tz->trips.passive.tc2
> +		* (tz->temperature - tz->trips.passive.temperature));
> +
> +	if (i > 0)
> +		*trend = THERMAL_TREND_RAISING;
> +	else if (i < 0)
> +		*trend = THERMAL_TREND_DROPPING;
> +	else
> +		*trend = THERMAL_TREND_STABLE;
> +	return 0;
> +}
> +
> +
>  static int thermal_notify(struct thermal_zone_device *thermal, int trip,
>  			   enum thermal_trip_type trip_type)
>  {
> @@ -836,6 +868,7 @@ static const struct thermal_zone_device_ops acpi_thermal_zone_ops = {
>  	.get_trip_type = thermal_get_trip_type,
>  	.get_trip_temp = thermal_get_trip_temp,
>  	.get_crit_temp = thermal_get_crit_temp,
> +	.get_trend = thermal_get_trend,
>  	.notify = thermal_notify,
>  };
>  
> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> index 62b4279..d406524 100644
> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -699,6 +699,21 @@ thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
>  }
>  #endif
>  
> +static void thermal_get_trend(struct thermal_zone_device *tz,
> +		int trip, enum thermal_trend *trend)
> +{
> +	if (tz->ops->get_trend && !tz->ops->get_trend(tz, trip, trend))
> +			return;
> +
> +	if (tz->temperature > tz->last_temperature)
> +		*trend = THERMAL_TREND_RAISING;
> +	else if (tz->temperature < tz->last_temperature)
> +		*trend = THERMAL_TREND_DROPPING;
> +	else
> +		*trend = THERMAL_TREND_STABLE;
> +	return;
> +}
> +

As I said in the review of the previous version of this patch series, I think
that the code from the function above should go directly into
thermal_zone_device_passive().  The ugly pointer manipulations would be
avoidable in that case and the code would be cleaner overall in my opinion.

>  static void thermal_zone_device_set_polling(struct thermal_zone_device *tz,
>  					    int delay)
>  {
> @@ -733,6 +748,8 @@ static void thermal_zone_device_passive(struct thermal_zone_device *tz,
>  	if (temp >= trip_temp) {
>  		tz->passive = true;
>  
> +		thermal_get_trend(tz, trip, (enum thermal_trend *)&trend);
> +
>  		trend = (tz->tc1 * (temp - tz->last_temperature)) +
>  			(tz->tc2 * (temp - trip_temp));
>  
> @@ -1091,6 +1108,9 @@ void thermal_zone_device_update(struct thermal_zone_device *tz)
>  		goto leave;
>  	}
>  
> +	tz->last_temperature = tz->temperature;
> +	tz->temperature = temp;
> +
>  	for (count = 0; count < tz->trips; count++) {
>  		tz->ops->get_trip_type(tz, count, &trip_type);
>  		tz->ops->get_trip_temp(tz, count, &trip_temp);
> @@ -1150,8 +1170,6 @@ void thermal_zone_device_update(struct thermal_zone_device *tz)
>  		thermal_zone_device_passive(tz, temp, tz->forced_passive,
>  					    THERMAL_TRIPS_NONE);
>  
> -	tz->last_temperature = temp;
> -
>  leave:
>  	if (tz->passive)
>  		thermal_zone_device_set_polling(tz, tz->passive_delay);
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index a43b12c..a01e3e6 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -44,6 +44,12 @@ enum thermal_trip_type {
>  	THERMAL_TRIP_CRITICAL,
>  };
>  
> +enum thermal_trend {
> +	THERMAL_TREND_STABLE, /* temperature is stable */
> +	THERMAL_TREND_RAISING, /* temperature is raising */
> +	THERMAL_TREND_DROPPING, /* temperature is dropping */
> +};
> +
>  struct thermal_zone_device_ops {
>  	int (*bind) (struct thermal_zone_device *,
>  		     struct thermal_cooling_device *);
> @@ -65,6 +71,8 @@ struct thermal_zone_device_ops {
>  	int (*set_trip_hyst) (struct thermal_zone_device *, int,
>  			      unsigned long);
>  	int (*get_crit_temp) (struct thermal_zone_device *, unsigned long *);
> +	int (*get_trend) (struct thermal_zone_device *, int,
> +			  enum thermal_trend *);
>  	int (*notify) (struct thermal_zone_device *, int,
>  		       enum thermal_trip_type);
>  };
> @@ -111,6 +119,7 @@ struct thermal_zone_device {
>  	int tc2;
>  	int passive_delay;
>  	int polling_delay;
> +	int temperature;
>  	int last_temperature;
>  	bool passive;
>  	unsigned int forced_passive;
> 

Thanks,
Rafael
--
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
Zhang Rui July 26, 2012, 2:21 a.m. UTC | #2
On ?, 2012-07-25 at 22:19 +0200, Rafael J. Wysocki wrote:
> On Wednesday, July 25, 2012, Zhang Rui wrote:
> > tc1 and tc2 are used by OSPM to anticipate the temperature trends.
> > But they are ACPI platform specific concepts.
> > 
> > Introduce .get_trend() as a more general solution.
> > 
> > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > ---
> >  drivers/acpi/thermal.c        |   33 +++++++++++++++++++++++++++++++++
> >  drivers/thermal/thermal_sys.c |   22 ++++++++++++++++++++--
> >  include/linux/thermal.h       |    9 +++++++++
> >  3 files changed, 62 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> > index 0154eac..01c92fd 100644
> > --- a/drivers/acpi/thermal.c
> > +++ b/drivers/acpi/thermal.c
> > @@ -704,6 +704,38 @@ static int thermal_get_crit_temp(struct thermal_zone_device *thermal,
> >  		return -EINVAL;
> >  }
> >  
> > +static int thermal_get_trend(struct thermal_zone_device *thermal,
> > +				int trip, enum thermal_trend *trend)
> > +{
> > +	struct acpi_thermal *tz = thermal->devdata;
> > +	enum thermal_trip_type type;
> > +	int i;
> > +
> > +	if (thermal_get_trip_type(thermal, trip, &type))
> > +		return -EINVAL;
> > +
> > +	/* Only PASSIVE trip points need TREND */
> > +	if (type != THERMAL_TRIP_PASSIVE)
> > +		return -EINVAL;
> > +
> > +	/*
> > +	 * tz->temperature has already been updated by generic thermal layer,
> > +	 * before this callback being invoked
> > +	 */
> > +	i = (tz->trips.passive.tc1 * (tz->temperature - tz->last_temperature))
> > +		+ (tz->trips.passive.tc2
> > +		* (tz->temperature - tz->trips.passive.temperature));
> > +
> > +	if (i > 0)
> > +		*trend = THERMAL_TREND_RAISING;
> > +	else if (i < 0)
> > +		*trend = THERMAL_TREND_DROPPING;
> > +	else
> > +		*trend = THERMAL_TREND_STABLE;
> > +	return 0;
> > +}
> > +
> > +
> >  static int thermal_notify(struct thermal_zone_device *thermal, int trip,
> >  			   enum thermal_trip_type trip_type)
> >  {
> > @@ -836,6 +868,7 @@ static const struct thermal_zone_device_ops acpi_thermal_zone_ops = {
> >  	.get_trip_type = thermal_get_trip_type,
> >  	.get_trip_temp = thermal_get_trip_temp,
> >  	.get_crit_temp = thermal_get_crit_temp,
> > +	.get_trend = thermal_get_trend,
> >  	.notify = thermal_notify,
> >  };
> >  
> > diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> > index 62b4279..d406524 100644
> > --- a/drivers/thermal/thermal_sys.c
> > +++ b/drivers/thermal/thermal_sys.c
> > @@ -699,6 +699,21 @@ thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
> >  }
> >  #endif
> >  
> > +static void thermal_get_trend(struct thermal_zone_device *tz,
> > +		int trip, enum thermal_trend *trend)
> > +{
> > +	if (tz->ops->get_trend && !tz->ops->get_trend(tz, trip, trend))
> > +			return;
> > +
> > +	if (tz->temperature > tz->last_temperature)
> > +		*trend = THERMAL_TREND_RAISING;
> > +	else if (tz->temperature < tz->last_temperature)
> > +		*trend = THERMAL_TREND_DROPPING;
> > +	else
> > +		*trend = THERMAL_TREND_STABLE;
> > +	return;
> > +}
> > +
> 
> As I said in the review of the previous version of this patch series, I think
> that the code from the function above should go directly into
> thermal_zone_device_passive().  The ugly pointer manipulations would be
> avoidable in that case and the code would be cleaner overall in my opinion.
> 
agreed.

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
diff mbox

Patch

diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index 0154eac..01c92fd 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -704,6 +704,38 @@  static int thermal_get_crit_temp(struct thermal_zone_device *thermal,
 		return -EINVAL;
 }
 
+static int thermal_get_trend(struct thermal_zone_device *thermal,
+				int trip, enum thermal_trend *trend)
+{
+	struct acpi_thermal *tz = thermal->devdata;
+	enum thermal_trip_type type;
+	int i;
+
+	if (thermal_get_trip_type(thermal, trip, &type))
+		return -EINVAL;
+
+	/* Only PASSIVE trip points need TREND */
+	if (type != THERMAL_TRIP_PASSIVE)
+		return -EINVAL;
+
+	/*
+	 * tz->temperature has already been updated by generic thermal layer,
+	 * before this callback being invoked
+	 */
+	i = (tz->trips.passive.tc1 * (tz->temperature - tz->last_temperature))
+		+ (tz->trips.passive.tc2
+		* (tz->temperature - tz->trips.passive.temperature));
+
+	if (i > 0)
+		*trend = THERMAL_TREND_RAISING;
+	else if (i < 0)
+		*trend = THERMAL_TREND_DROPPING;
+	else
+		*trend = THERMAL_TREND_STABLE;
+	return 0;
+}
+
+
 static int thermal_notify(struct thermal_zone_device *thermal, int trip,
 			   enum thermal_trip_type trip_type)
 {
@@ -836,6 +868,7 @@  static const struct thermal_zone_device_ops acpi_thermal_zone_ops = {
 	.get_trip_type = thermal_get_trip_type,
 	.get_trip_temp = thermal_get_trip_temp,
 	.get_crit_temp = thermal_get_crit_temp,
+	.get_trend = thermal_get_trend,
 	.notify = thermal_notify,
 };
 
diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 62b4279..d406524 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -699,6 +699,21 @@  thermal_remove_hwmon_sysfs(struct thermal_zone_device *tz)
 }
 #endif
 
+static void thermal_get_trend(struct thermal_zone_device *tz,
+		int trip, enum thermal_trend *trend)
+{
+	if (tz->ops->get_trend && !tz->ops->get_trend(tz, trip, trend))
+			return;
+
+	if (tz->temperature > tz->last_temperature)
+		*trend = THERMAL_TREND_RAISING;
+	else if (tz->temperature < tz->last_temperature)
+		*trend = THERMAL_TREND_DROPPING;
+	else
+		*trend = THERMAL_TREND_STABLE;
+	return;
+}
+
 static void thermal_zone_device_set_polling(struct thermal_zone_device *tz,
 					    int delay)
 {
@@ -733,6 +748,8 @@  static void thermal_zone_device_passive(struct thermal_zone_device *tz,
 	if (temp >= trip_temp) {
 		tz->passive = true;
 
+		thermal_get_trend(tz, trip, (enum thermal_trend *)&trend);
+
 		trend = (tz->tc1 * (temp - tz->last_temperature)) +
 			(tz->tc2 * (temp - trip_temp));
 
@@ -1091,6 +1108,9 @@  void thermal_zone_device_update(struct thermal_zone_device *tz)
 		goto leave;
 	}
 
+	tz->last_temperature = tz->temperature;
+	tz->temperature = temp;
+
 	for (count = 0; count < tz->trips; count++) {
 		tz->ops->get_trip_type(tz, count, &trip_type);
 		tz->ops->get_trip_temp(tz, count, &trip_temp);
@@ -1150,8 +1170,6 @@  void thermal_zone_device_update(struct thermal_zone_device *tz)
 		thermal_zone_device_passive(tz, temp, tz->forced_passive,
 					    THERMAL_TRIPS_NONE);
 
-	tz->last_temperature = temp;
-
 leave:
 	if (tz->passive)
 		thermal_zone_device_set_polling(tz, tz->passive_delay);
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index a43b12c..a01e3e6 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -44,6 +44,12 @@  enum thermal_trip_type {
 	THERMAL_TRIP_CRITICAL,
 };
 
+enum thermal_trend {
+	THERMAL_TREND_STABLE, /* temperature is stable */
+	THERMAL_TREND_RAISING, /* temperature is raising */
+	THERMAL_TREND_DROPPING, /* temperature is dropping */
+};
+
 struct thermal_zone_device_ops {
 	int (*bind) (struct thermal_zone_device *,
 		     struct thermal_cooling_device *);
@@ -65,6 +71,8 @@  struct thermal_zone_device_ops {
 	int (*set_trip_hyst) (struct thermal_zone_device *, int,
 			      unsigned long);
 	int (*get_crit_temp) (struct thermal_zone_device *, unsigned long *);
+	int (*get_trend) (struct thermal_zone_device *, int,
+			  enum thermal_trend *);
 	int (*notify) (struct thermal_zone_device *, int,
 		       enum thermal_trip_type);
 };
@@ -111,6 +119,7 @@  struct thermal_zone_device {
 	int tc2;
 	int passive_delay;
 	int polling_delay;
+	int temperature;
 	int last_temperature;
 	bool passive;
 	unsigned int forced_passive;