[PATCH 0/4] thermal: Introduce support for monitoring falling temperatures.

[PATCH 0/4] thermal: Introduce support for monitoring falling temperatures.

[Thara Gopinath]

Thermal framework today supports monitoring for rising temperatures and
subsequently initiating cooling action in case of a trip point being 
crossed. There are scenarios where a SoC needs some warming action to be
activated if the temperature falls below a cetain allowable limit.
Since warming action can be considered mirror opposite of cooling action,
most of the thermal framework can be re-used to achieve this.

To enable thermal framework to monitor falling temperature, a new parameter
is added to the thermal trip point binding in the device tree to indicate
the direction(rising/falling) of temperature monitoring. Thermal DT
driver is extended to capture this information from the device tree 
entries and to reflect it in the thermal framework as a new enum
variable in the thermal trip point structure.
As an initial attempt, step-wise governor is extended to support
bi-directional monitoring of temprature if a trip point is hit, depending
on the newly introduced enum variable. Finally thermal sysfs entries are
extended to indicate the trip point monitor direction.

Patch series introducing various resources that are used as warming devices
on Qualcomm sdm845:
https://lkml.org/lkml/2019/7/29/749 (already merged)

https://lkml.org/lkml/2019/9/10/727 (under review)


Thara Gopinath (4):
  dt-bindings: thermal: Introduce monitor-falling binding to thermal
    trip point description
  thermal: Thermal core and sysfs changes needed to support
    bi-directional monitoring of trip points.
  thermal: of-thermal: Extend thermal dt driver to support
    bi-directional monitoring of a thermal trip point.
  thermal: step_wise: Extend thermal step-wise governor to monitor
    falling temperature.

 .../devicetree/bindings/thermal/thermal.txt        |  8 +++
 drivers/thermal/of-thermal.c                       | 22 ++++++++
 drivers/thermal/step_wise.c                        | 59 +++++++++++++++------
 drivers/thermal/thermal_sysfs.c                    | 60 ++++++++++++++++++++--
 include/linux/thermal.h                            | 10 ++++
 include/uapi/linux/thermal.h                       |  2 +-
 6 files changed, 141 insertions(+), 20 deletions(-)

-- 
2.1.4

[PATCH 1/4] dt-bindings: thermal: Introduce monitor-falling parameter to thermal trip point binding

[Thara Gopinath]

Introduce a new binding parameter to thermal trip point description
to indicate whether the temperature level specified by the trip point
is monitored for a rise or fall in temperature.

Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
---
 Documentation/devicetree/bindings/thermal/thermal.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/thermal/thermal.txt b/Documentation/devicetree/bindings/thermal/thermal.txt
index ca14ba9..849a2a9 100644
--- a/Documentation/devicetree/bindings/thermal/thermal.txt
+++ b/Documentation/devicetree/bindings/thermal/thermal.txt
@@ -90,6 +90,14 @@ Required properties:
 	"critical":	Hardware not reliable.
   Type: string
 
+Optional property:
+- monitor-falling: 	Indicate whether the system action is kick
+  Type: boolean		started when the temperature falls below or rises
+			above the trip temperature level indicated in
+			"temperature".If true, the trip point is monitored
+			for falling temperature else the trip point is
+			monitored for rising temperature.
+
 * Cooling device maps
 
 The cooling device maps node is a node to describe how cooling devices
-- 
2.1.4

[PATCH 2/4] thermal: Thermal core and sysfs changes needed to support bi-directional monitoring of trip points.

[Thara Gopinath]

Thermal trip points can be defined to indicate whether a
temperature rise or a temperature fall is to be monitored. This
property can now be defined in the DT bindings for a trip point.
To support this following three changes are introduced to thermal
core and sysfs code.
1. Define a new variable in thermal_trip to capture the monitor
   rising/falling information from trip point DT bindings.
2. Define a new ops in thermal_zone_device_ops that can be populated
   to indicate whether a trip is being monitored for rising or falling
   temperature. If the ops is not populated or if the binding is missing
   in the DT, it is assumed that the trip is being monitored for rising
   temperature. (default behavior today)
3. Introduce sysfs entries for each trip point to read the direction of
   monitoring.

Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
---
 drivers/thermal/thermal_sysfs.c | 60 ++++++++++++++++++++++++++++++++++++++---
 include/linux/thermal.h         | 10 +++++++
 include/uapi/linux/thermal.h    |  2 +-
 3 files changed, 67 insertions(+), 5 deletions(-)

diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
index aa99edb..b4ef6be 100644
--- a/drivers/thermal/thermal_sysfs.c
+++ b/drivers/thermal/thermal_sysfs.c
@@ -216,6 +216,31 @@ trip_point_hyst_show(struct device *dev, struct device_attribute *attr,
 }
 
 static ssize_t
+trip_point_monitor_type_show(struct device *dev, struct device_attribute *attr,
+			     char *buf)
+{
+	struct thermal_zone_device *tz = to_thermal_zone(dev);
+	enum thermal_trip_monitor_type type;
+	int trip, result;
+
+	if (sscanf(attr->attr.name, "trip_point_%d_monitor_type", &trip) != 1)
+		return -EINVAL;
+
+	if (!tz->ops->get_trip_monitor_type)
+		goto exit;
+
+	result = tz->ops->get_trip_monitor_type(tz, trip, &type);
+	if (result)
+		return result;
+
+	if (type == THERMAL_TRIP_MONITOR_FALLING)
+		return sprintf(buf, "falling\n");
+
+exit:
+		return sprintf(buf, "rising\n");
+}
+
+static ssize_t
 passive_store(struct device *dev, struct device_attribute *attr,
 	      const char *buf, size_t count)
 {
@@ -520,10 +545,20 @@ static int create_trip_attrs(struct thermal_zone_device *tz, int mask)
 	if (!tz->trip_type_attrs)
 		return -ENOMEM;
 
+	tz->trip_monitor_type_attrs = kcalloc
+					(tz->trips,
+					sizeof(*tz->trip_monitor_type_attrs),
+					GFP_KERNEL);
+	if (!tz->trip_monitor_type_attrs) {
+		kfree(tz->trip_type_attrs);
+		return -ENOMEM;
+	}
+
 	tz->trip_temp_attrs = kcalloc(tz->trips, sizeof(*tz->trip_temp_attrs),
 				      GFP_KERNEL);
 	if (!tz->trip_temp_attrs) {
 		kfree(tz->trip_type_attrs);
+		kfree(tz->trip_monitor_type_attrs);
 		return -ENOMEM;
 	}
 
@@ -533,14 +568,16 @@ static int create_trip_attrs(struct thermal_zone_device *tz, int mask)
 					      GFP_KERNEL);
 		if (!tz->trip_hyst_attrs) {
 			kfree(tz->trip_type_attrs);
+			kfree(tz->trip_monitor_type_attrs);
 			kfree(tz->trip_temp_attrs);
 			return -ENOMEM;
 		}
 	}
 
-	attrs = kcalloc(tz->trips * 3 + 1, sizeof(*attrs), GFP_KERNEL);
+	attrs = kcalloc(tz->trips * 4 + 1, sizeof(*attrs), GFP_KERNEL);
 	if (!attrs) {
 		kfree(tz->trip_type_attrs);
+		kfree(tz->trip_monitor_type_attrs);
 		kfree(tz->trip_temp_attrs);
 		if (tz->ops->get_trip_hyst)
 			kfree(tz->trip_hyst_attrs);
@@ -559,6 +596,20 @@ static int create_trip_attrs(struct thermal_zone_device *tz, int mask)
 		tz->trip_type_attrs[indx].attr.show = trip_point_type_show;
 		attrs[indx] = &tz->trip_type_attrs[indx].attr.attr;
 
+		/* create trip monitor type attribute */
+		snprintf(tz->trip_monitor_type_attrs[indx].name,
+			 THERMAL_NAME_LENGTH, "trip_point_%d_monitor_type",
+			 indx);
+
+		sysfs_attr_init(&tz->trip_monitor_type_attrs[indx].attr.attr);
+		tz->trip_monitor_type_attrs[indx].attr.attr.name =
+				tz->trip_monitor_type_attrs[indx].name;
+		tz->trip_monitor_type_attrs[indx].attr.attr.mode = S_IRUGO;
+		tz->trip_monitor_type_attrs[indx].attr.show =
+				trip_point_monitor_type_show;
+		attrs[indx + tz->trips] =
+				&tz->trip_monitor_type_attrs[indx].attr.attr;
+
 		/* create trip temp attribute */
 		snprintf(tz->trip_temp_attrs[indx].name, THERMAL_NAME_LENGTH,
 			 "trip_point_%d_temp", indx);
@@ -574,7 +625,8 @@ static int create_trip_attrs(struct thermal_zone_device *tz, int mask)
 			tz->trip_temp_attrs[indx].attr.store =
 							trip_point_temp_store;
 		}
-		attrs[indx + tz->trips] = &tz->trip_temp_attrs[indx].attr.attr;
+		attrs[indx + tz->trips * 2] =
+				&tz->trip_temp_attrs[indx].attr.attr;
 
 		/* create Optional trip hyst attribute */
 		if (!tz->ops->get_trip_hyst)
@@ -592,10 +644,10 @@ static int create_trip_attrs(struct thermal_zone_device *tz, int mask)
 			tz->trip_hyst_attrs[indx].attr.store =
 					trip_point_hyst_store;
 		}
-		attrs[indx + tz->trips * 2] =
+		attrs[indx + tz->trips * 3] =
 					&tz->trip_hyst_attrs[indx].attr.attr;
 	}
-	attrs[tz->trips * 3] = NULL;
+	attrs[tz->trips * 4] = NULL;
 
 	tz->trips_attribute_group.attrs = attrs;
 
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index e45659c..1435176b 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -70,6 +70,11 @@ enum thermal_trip_type {
 	THERMAL_TRIP_CRITICAL,
 };
 
+enum thermal_trip_monitor_type {
+	THERMAL_TRIP_MONITOR_RISING = 0,
+	THERMAL_TRIP_MONITOR_FALLING
+};
+
 enum thermal_trend {
 	THERMAL_TREND_STABLE, /* temperature is stable */
 	THERMAL_TREND_RAISING, /* temperature is raising */
@@ -113,6 +118,8 @@ struct thermal_zone_device_ops {
 			  enum thermal_trend *);
 	int (*notify) (struct thermal_zone_device *, int,
 		       enum thermal_trip_type);
+	int (*get_trip_monitor_type)(struct thermal_zone_device *, int,
+				     enum thermal_trip_monitor_type *);
 };
 
 struct thermal_cooling_device_ops {
@@ -196,6 +203,7 @@ struct thermal_zone_device {
 	struct thermal_attr *trip_temp_attrs;
 	struct thermal_attr *trip_type_attrs;
 	struct thermal_attr *trip_hyst_attrs;
+	struct thermal_attr *trip_monitor_type_attrs;
 	void *devdata;
 	int trips;
 	unsigned long trips_disabled;	/* bitmap for disabled trips */
@@ -364,6 +372,7 @@ struct thermal_zone_of_device_ops {
  * @temperature: temperature value in miliCelsius
  * @hysteresis: relative hysteresis in miliCelsius
  * @type: trip point type
+ * @monitor_type: trip point monitor type.
  */
 
 struct thermal_trip {
@@ -371,6 +380,7 @@ struct thermal_trip {
 	int temperature;
 	int hysteresis;
 	enum thermal_trip_type type;
+	enum thermal_trip_monitor_type monitor_type;
 };
 
 /* Function declarations */
diff --git a/include/uapi/linux/thermal.h b/include/uapi/linux/thermal.h
index 9621837..c01492c 100644
--- a/include/uapi/linux/thermal.h
+++ b/include/uapi/linux/thermal.h
@@ -2,7 +2,7 @@
 #ifndef _UAPI_LINUX_THERMAL_H
 #define _UAPI_LINUX_THERMAL_H
 
-#define THERMAL_NAME_LENGTH	20
+#define THERMAL_NAME_LENGTH	30
 
 /* Adding event notification support elements */
 #define THERMAL_GENL_FAMILY_NAME                "thermal_event"
-- 
2.1.4

[PATCH 3/4] thermal: of-thermal: Extend thermal dt driver to support bi-directional monitoring of a thermal trip point.

[Thara Gopinath]

Introduce of_thermal_get_trip_monitor_type to return the direction
of monitoring of a thermal trip point. Also translate the DT information
regarding trip point monitor direction into the thermal framework.

Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
---
 drivers/thermal/of-thermal.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
index dc5093b..a5f6fdc 100644
--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c
@@ -377,6 +377,20 @@ static int of_thermal_set_trip_hyst(struct thermal_zone_device *tz, int trip,
 	return 0;
 }
 
+static int of_thermal_get_trip_monitor_type
+				(struct thermal_zone_device *tz, int trip,
+				 enum thermal_trip_monitor_type *type)
+{
+	struct __thermal_zone *data = tz->devdata;
+
+	if (trip >= data->ntrips || trip < 0)
+		return -EDOM;
+
+	*type = data->trips[trip].monitor_type;
+
+	return 0;
+}
+
 static int of_thermal_get_crit_temp(struct thermal_zone_device *tz,
 				    int *temp)
 {
@@ -401,6 +415,7 @@ static struct thermal_zone_device_ops of_thermal_ops = {
 	.set_trip_temp = of_thermal_set_trip_temp,
 	.get_trip_hyst = of_thermal_get_trip_hyst,
 	.set_trip_hyst = of_thermal_set_trip_hyst,
+	.get_trip_monitor_type = of_thermal_get_trip_monitor_type,
 	.get_crit_temp = of_thermal_get_crit_temp,
 
 	.bind = of_thermal_bind,
@@ -809,6 +824,7 @@ static int thermal_of_populate_trip(struct device_node *np,
 {
 	int prop;
 	int ret;
+	bool is_monitor_falling;
 
 	ret = of_property_read_u32(np, "temperature", &prop);
 	if (ret < 0) {
@@ -830,6 +846,12 @@ static int thermal_of_populate_trip(struct device_node *np,
 		return ret;
 	}
 
+	is_monitor_falling = of_property_read_bool(np, "monitor-falling");
+	if (is_monitor_falling)
+		trip->monitor_type = THERMAL_TRIP_MONITOR_FALLING;
+	else
+		trip->monitor_type = THERMAL_TRIP_MONITOR_RISING;
+
 	/* Required for cooling map matching */
 	trip->np = np;
 	of_node_get(np);
-- 
2.1.4

[PATCH 4/4] thermal: step_wise: Extend thermal step-wise governor to monitor falling temperature.

[Thara Gopinath]

>From the step wise governor point of view, the policy decisions
that has to taken on a thermal trip point that is defined to be monitored
for falling temprature is the mirror opposite of the decisions it has
to take on a trip point that is monitored for rising temperature.

Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
---
 drivers/thermal/step_wise.c | 59 +++++++++++++++++++++++++++++++++------------
 1 file changed, 44 insertions(+), 15 deletions(-)

diff --git a/drivers/thermal/step_wise.c b/drivers/thermal/step_wise.c
index 6e051cb..aa8e0a0 100644
--- a/drivers/thermal/step_wise.c
+++ b/drivers/thermal/step_wise.c
@@ -35,7 +35,8 @@
  *       deactivate the thermal instance
  */
 static unsigned long get_target_state(struct thermal_instance *instance,
-				enum thermal_trend trend, bool throttle)
+				enum thermal_trend trend, bool throttle,
+				enum thermal_trip_monitor_type type)
 {
 	struct thermal_cooling_device *cdev = instance->cdev;
 	unsigned long cur_state;
@@ -65,11 +66,21 @@ static unsigned long get_target_state(struct thermal_instance *instance,
 
 	switch (trend) {
 	case THERMAL_TREND_RAISING:
-		if (throttle) {
-			next_target = cur_state < instance->upper ?
-				    (cur_state + 1) : instance->upper;
-			if (next_target < instance->lower)
-				next_target = instance->lower;
+		if (type == THERMAL_TRIP_MONITOR_FALLING) {
+			if (cur_state <= instance->lower) {
+				if (!throttle)
+					next_target = THERMAL_NO_TARGET;
+			} else {
+				if (!throttle)
+					next_target = cur_state - 1;
+			}
+		} else {
+			if (throttle) {
+				next_target = cur_state < instance->upper ?
+					    (cur_state + 1) : instance->upper;
+				if (next_target < instance->lower)
+					next_target = instance->lower;
+			}
 		}
 		break;
 	case THERMAL_TREND_RAISE_FULL:
@@ -77,14 +88,23 @@ static unsigned long get_target_state(struct thermal_instance *instance,
 			next_target = instance->upper;
 		break;
 	case THERMAL_TREND_DROPPING:
-		if (cur_state <= instance->lower) {
-			if (!throttle)
-				next_target = THERMAL_NO_TARGET;
+		if (type == THERMAL_TRIP_MONITOR_FALLING) {
+			if (throttle) {
+				next_target = cur_state < instance->upper ?
+					(cur_state + 1) : instance->upper;
+				if (next_target < instance->lower)
+					next_target = instance->lower;
+			}
 		} else {
-			if (!throttle) {
-				next_target = cur_state - 1;
-				if (next_target > instance->upper)
-					next_target = instance->upper;
+			if (cur_state <= instance->lower) {
+				if (!throttle)
+					next_target = THERMAL_NO_TARGET;
+			} else {
+				if (!throttle) {
+					next_target = cur_state - 1;
+					if (next_target > instance->upper)
+						next_target = instance->upper;
+				}
 			}
 		}
 		break;
@@ -117,6 +137,8 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
 {
 	int trip_temp;
 	enum thermal_trip_type trip_type;
+	enum thermal_trip_monitor_type monitor_type =
+					THERMAL_TRIP_MONITOR_RISING;
 	enum thermal_trend trend;
 	struct thermal_instance *instance;
 	bool throttle = false;
@@ -130,9 +152,15 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
 		tz->ops->get_trip_type(tz, trip, &trip_type);
 	}
 
+	if (tz->ops->get_trip_monitor_type)
+		tz->ops->get_trip_monitor_type(tz, trip, &monitor_type);
+
 	trend = get_tz_trend(tz, trip);
 
-	if (tz->temperature >= trip_temp) {
+	if (((monitor_type == THERMAL_TRIP_MONITOR_RISING) &&
+	      (tz->temperature >= trip_temp)) ||
+	      ((monitor_type == THERMAL_TRIP_MONITOR_FALLING) &&
+	      (tz->temperature <= trip_temp))) {
 		throttle = true;
 		trace_thermal_zone_trip(tz, trip, trip_type);
 	}
@@ -147,7 +175,8 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
 			continue;
 
 		old_target = instance->target;
-		instance->target = get_target_state(instance, trend, throttle);
+		instance->target = get_target_state(instance, trend, throttle,
+						    monitor_type);
 		dev_dbg(&instance->cdev->device, "old_target=%d, target=%d\n",
 					old_target, (int)instance->target);
 
-- 
2.1.4

Re: [PATCH 1/4] dt-bindings: thermal: Introduce monitor-falling parameter to thermal trip point binding

[Rob Herring]

On Wed, Sep 18, 2019 at 10:18:20PM -0400, Thara Gopinath wrote:
> Introduce a new binding parameter to thermal trip point description
> to indicate whether the temperature level specified by the trip point
> is monitored for a rise or fall in temperature.

What if it is both?

When do you need this? Seems like you'd always want to monitor both 
directions to undo any action done on rising temp. Unless you want a 
hysteresis, but this doesn't seem like the best way to implement that.

> 
> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
> ---
>  Documentation/devicetree/bindings/thermal/thermal.txt | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/thermal/thermal.txt b/Documentation/devicetree/bindings/thermal/thermal.txt
> index ca14ba9..849a2a9 100644
> --- a/Documentation/devicetree/bindings/thermal/thermal.txt
> +++ b/Documentation/devicetree/bindings/thermal/thermal.txt
> @@ -90,6 +90,14 @@ Required properties:
>  	"critical":	Hardware not reliable.
>    Type: string
>  
> +Optional property:
> +- monitor-falling: 	Indicate whether the system action is kick
> +  Type: boolean		started when the temperature falls below or rises
> +			above the trip temperature level indicated in
> +			"temperature".If true, the trip point is monitored
> +			for falling temperature else the trip point is
> +			monitored for rising temperature.
> +
>  * Cooling device maps
>  
>  The cooling device maps node is a node to describe how cooling devices
> -- 
> 2.1.4
> 

Re: [PATCH 1/4] dt-bindings: thermal: Introduce monitor-falling parameter to thermal trip point binding

[Thara Gopinath]

Hi Rob,
Thanks for the review.

On 10/01/2019 06:09 PM, Rob Herring wrote:
> On Wed, Sep 18, 2019 at 10:18:20PM -0400, Thara Gopinath wrote:
>> Introduce a new binding parameter to thermal trip point description
>> to indicate whether the temperature level specified by the trip point
>> is monitored for a rise or fall in temperature.
> 
> What if it is both?
> 
> When do you need this? Seems like you'd always want to monitor both 
> directions to undo any action done on rising temp. Unless you want a 
> hysteresis, but this doesn't seem like the best way to implement that.
> 

The thermal framework is designed in such a manner that I cannot think
of a use case for both.
The framework takes care of removing the warming/cooling action when the
trip point is crossed in the opposite direction. It only needs an
indication on when to start implementing the
action.
For eg. When the temperature crosses/increases above 90 degree, the
framework will start the cooling action and will continue monitoring
till the temperature falls below 90 and the cooling action is removed.
Vice versa when the temperature decreases below say 5 degree, the
framework should  initiate the warming action and monitor till the
temperature rises above and remove the warming action.

So the trip point is really an indication of the temperature crossing a
threshold in the specified direction.

Now this parameter is needed to indicate whether the thermal framework
has to start implementing the warming/cooling action when the
temperature  rises above the trip point or falls below the trip point.

Till now the framework was always assuming that the cooling action had
to be implemented when temperature rises above the trip point.

-- 
Warm Regards
Thara

Re: [PATCH 4/4] thermal: step_wise: Extend thermal step-wise governor to monitor falling temperature.

[Ram Chandrasekar]

On 9/18/2019 8:18 PM, Thara Gopinath wrote:
>>From the step wise governor point of view, the policy decisions
> that has to taken on a thermal trip point that is defined to be monitored
> for falling temprature is the mirror opposite of the decisions it has
> to take on a trip point that is monitored for rising temperature.
> 
> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
> ---
>   drivers/thermal/step_wise.c | 59 +++++++++++++++++++++++++++++++++------------
>   1 file changed, 44 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/thermal/step_wise.c b/drivers/thermal/step_wise.c
> index 6e051cb..aa8e0a0 100644
> --- a/drivers/thermal/step_wise.c
> +++ b/drivers/thermal/step_wise.c
> @@ -35,7 +35,8 @@
>    *       deactivate the thermal instance
>    */
>   static unsigned long get_target_state(struct thermal_instance *instance,
> -				enum thermal_trend trend, bool throttle)
> +				enum thermal_trend trend, bool throttle,
> +				enum thermal_trip_monitor_type type)
>   {
>   	struct thermal_cooling_device *cdev = instance->cdev;
>   	unsigned long cur_state;
> @@ -65,11 +66,21 @@ static unsigned long get_target_state(struct thermal_instance *instance,
>   
>   	switch (trend) {
>   	case THERMAL_TREND_RAISING:
> -		if (throttle) {
> -			next_target = cur_state < instance->upper ?
> -				    (cur_state + 1) : instance->upper;
> -			if (next_target < instance->lower)
> -				next_target = instance->lower;
> +		if (type == THERMAL_TRIP_MONITOR_FALLING) {
> +			if (cur_state <= instance->lower) {
> +				if (!throttle)
> +					next_target = THERMAL_NO_TARGET;
> +			} else {
> +				if (!throttle)
> +					next_target = cur_state - 1;
> +			}
> +		} else {
> +			if (throttle) {
> +				next_target = cur_state < instance->upper ?
> +					    (cur_state + 1) : instance->upper;
> +				if (next_target < instance->lower)
> +					next_target = instance->lower;
> +			}
>   		}
>   		break;
>   	case THERMAL_TREND_RAISE_FULL:
> @@ -77,14 +88,23 @@ static unsigned long get_target_state(struct thermal_instance *instance,
>   			next_target = instance->upper;
>   		break;
>   	case THERMAL_TREND_DROPPING:
> -		if (cur_state <= instance->lower) {
> -			if (!throttle)
> -				next_target = THERMAL_NO_TARGET;
> +		if (type == THERMAL_TRIP_MONITOR_FALLING) {
> +			if (throttle) {
> +				next_target = cur_state < instance->upper ?
> +					(cur_state + 1) : instance->upper;
> +				if (next_target < instance->lower)
> +					next_target = instance->lower;
> +			}
>   		} else {
> -			if (!throttle) {
> -				next_target = cur_state - 1;
> -				if (next_target > instance->upper)
> -					next_target = instance->upper;
> +			if (cur_state <= instance->lower) {
> +				if (!throttle)
> +					next_target = THERMAL_NO_TARGET;
> +			} else {
> +				if (!throttle) {
> +					next_target = cur_state - 1;
> +					if (next_target > instance->upper)
> +						next_target = instance->upper;
> +				}
>   			}
>   		}
>   		break;
> @@ -117,6 +137,8 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
>   {
>   	int trip_temp;
>   	enum thermal_trip_type trip_type;
> +	enum thermal_trip_monitor_type monitor_type =
> +					THERMAL_TRIP_MONITOR_RISING;
>   	enum thermal_trend trend;
>   	struct thermal_instance *instance;
>   	bool throttle = false;
> @@ -130,9 +152,15 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
>   		tz->ops->get_trip_type(tz, trip, &trip_type);
>   	}
>   
> +	if (tz->ops->get_trip_monitor_type)
> +		tz->ops->get_trip_monitor_type(tz, trip, &monitor_type);
> +
>   	trend = get_tz_trend(tz, trip);
>   
> -	if (tz->temperature >= trip_temp) {
> +	if (((monitor_type == THERMAL_TRIP_MONITOR_RISING) &&
> +	      (tz->temperature >= trip_temp)) ||
> +	      ((monitor_type == THERMAL_TRIP_MONITOR_FALLING) &&
> +	      (tz->temperature <= trip_temp))) {
Governors monitoring warming devices need to have support for 
hysteresis. Assume a case where the device is in idle when the 
temperature goes below threshold and we trigger a mitigation. Even a 
minimal workload or even the processing of the threshold by the governor 
could warm the device and put the temperature above the threshold and we 
will have to remove any mitigation. To avoid this ping-pong, its best to 
add a hysteresis support.
>   		throttle = true;
>   		trace_thermal_zone_trip(tz, trip, trip_type);
>   	}
> @@ -147,7 +175,8 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
>   			continue;
>   
>   		old_target = instance->target;
> -		instance->target = get_target_state(instance, trend, throttle);
> +		instance->target = get_target_state(instance, trend, throttle,
> +						    monitor_type);
>   		dev_dbg(&instance->cdev->device, "old_target=%d, target=%d\n",
>   					old_target, (int)instance->target);
>   
> 

Re: [PATCH 1/4] dt-bindings: thermal: Introduce monitor-falling parameter to thermal trip point binding

[Amit Kucheria]

On Thu, Sep 19, 2019 at 7:48 AM Thara Gopinath
<thara.gopinath@linaro.org> wrote:
>
> Introduce a new binding parameter to thermal trip point description
> to indicate whether the temperature level specified by the trip point
> is monitored for a rise or fall in temperature.
>
> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
> ---
>  Documentation/devicetree/bindings/thermal/thermal.txt | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/thermal/thermal.txt b/Documentation/devicetree/bindings/thermal/thermal.txt
> index ca14ba9..849a2a9 100644
> --- a/Documentation/devicetree/bindings/thermal/thermal.txt
> +++ b/Documentation/devicetree/bindings/thermal/thermal.txt
> @@ -90,6 +90,14 @@ Required properties:
>         "critical":     Hardware not reliable.
>    Type: string
>
> +Optional property:
> +- monitor-falling:     Indicate whether the system action is kick

Stray space after :

> +  Type: boolean                started when the temperature falls below or rises

Unnecessary tab after boolean (I'll fix up the rest of the file in the
yaml conversion)

I suggest not making this boolean. Just use the property as a flag by
itself to denote a falling trip point. No need to deal with true/false
values.

Similarly, the sysfs file would show up only in case of a trip that
sets this flag and just contain a 1, for example.

> +                       above the trip temperature level indicated in
> +                       "temperature".If true, the trip point is monitored

Add space after full stop.


> +                       for falling temperature else the trip point is
> +                       monitored for rising temperature.
> +
>  * Cooling device maps
>
>  The cooling device maps node is a node to describe how cooling devices
> --
> 2.1.4
>