[PATCH 1/6] thermal/core: Remove the 'forced_passive' option

[PATCH 1/6] thermal/core: Remove the 'forced_passive' option

[Daniel Lezcano]

The code was reorganized in 2012 with the commit 0c01ebbfd3caf1.

The main change is a loop on the trip points array and a unconditional
call to the throttle() ops of the governors for each of them even if
the trip temperature is not reached yet.

With this change, the 'forced_passive' is no longer checked in the
thermal_zone_device_update() function but in the step wise governor's
throttle() callback.

As the force_passive does no belong to the trip point array, the
thermal_zone_device_update() can not compare with the specified
passive temperature, thus does not detect the passive limit has been
crossed. Consequently, throttle() is never called and the
'forced_passive' branch is unreached.

In addition, the default processor cooling device is not automatically
bound to the thermal zone if there is not passive trip point, thus the
'forced_passive' can not operate.

If there is an active trip point, then the throttle function will be
called to mitigate at this temperature and the 'forced_passive' will
override the mitigation of the active trip point in this case but with
the default cooling device bound to the thermal zone, so usually a
fan, and that is not a passive cooling effect.

Given the regression exists since more than 8 years, nobody complained
and at the best of my knowledge there is no bug open in
https://bugzilla.kernel.org, it is reasonable to say it is unused.

Remove the 'forced_passive' related code.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 .../driver-api/thermal/sysfs-api.rst          | 13 ---
 drivers/thermal/gov_step_wise.c               | 14 +---
 drivers/thermal/thermal_sysfs.c               | 80 -------------------
 include/linux/thermal.h                       |  4 -
 4 files changed, 3 insertions(+), 108 deletions(-)

diff --git a/Documentation/driver-api/thermal/sysfs-api.rst b/Documentation/driver-api/thermal/sysfs-api.rst
index e7520cb439ac..a4969c474cc3 100644
--- a/Documentation/driver-api/thermal/sysfs-api.rst
+++ b/Documentation/driver-api/thermal/sysfs-api.rst
@@ -520,19 +520,6 @@ available_policies
 
 	RW, Optional
 
-passive
-	Attribute is only present for zones in which the passive cooling
-	policy is not supported by native thermal driver. Default is zero
-	and can be set to a temperature (in millidegrees) to enable a
-	passive trip point for the zone. Activation is done by polling with
-	an interval of 1 second.
-
-	Unit: millidegrees Celsius
-
-	Valid values: 0 (disabled) or greater than 1000
-
-	RW, Optional
-
 emul_temp
 	Interface to set the emulated temperature method in thermal zone
 	(sensor). After setting this temperature, the thermal zone may pass
diff --git a/drivers/thermal/gov_step_wise.c b/drivers/thermal/gov_step_wise.c
index 2ae7198d3067..12acb12aac50 100644
--- a/drivers/thermal/gov_step_wise.c
+++ b/drivers/thermal/gov_step_wise.c
@@ -109,7 +109,7 @@ static void update_passive_instance(struct thermal_zone_device *tz,
 	 * If value is +1, activate a passive instance.
 	 * If value is -1, deactivate a passive instance.
 	 */
-	if (type == THERMAL_TRIP_PASSIVE || type == THERMAL_TRIPS_NONE)
+	if (type == THERMAL_TRIP_PASSIVE)
 		tz->passive += value;
 }
 
@@ -122,13 +122,8 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
 	bool throttle = false;
 	int old_target;
 
-	if (trip == THERMAL_TRIPS_NONE) {
-		trip_temp = tz->forced_passive;
-		trip_type = THERMAL_TRIPS_NONE;
-	} else {
-		tz->ops->get_trip_temp(tz, trip, &trip_temp);
-		tz->ops->get_trip_type(tz, trip, &trip_type);
-	}
+	tz->ops->get_trip_temp(tz, trip, &trip_temp);
+	tz->ops->get_trip_type(tz, trip, &trip_type);
 
 	trend = get_tz_trend(tz, trip);
 
@@ -189,9 +184,6 @@ static int step_wise_throttle(struct thermal_zone_device *tz, int trip)
 
 	thermal_zone_trip_update(tz, trip);
 
-	if (tz->forced_passive)
-		thermal_zone_trip_update(tz, THERMAL_TRIPS_NONE);
-
 	mutex_lock(&tz->lock);
 
 	list_for_each_entry(instance, &tz->thermal_instances, tz_node)
diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
index 0866e949339b..4e7f9e880d76 100644
--- a/drivers/thermal/thermal_sysfs.c
+++ b/drivers/thermal/thermal_sysfs.c
@@ -216,49 +216,6 @@ trip_point_hyst_show(struct device *dev, struct device_attribute *attr,
 	return ret ? ret : sprintf(buf, "%d\n", temperature);
 }
 
-static ssize_t
-passive_store(struct device *dev, struct device_attribute *attr,
-	      const char *buf, size_t count)
-{
-	struct thermal_zone_device *tz = to_thermal_zone(dev);
-	int state;
-
-	if (sscanf(buf, "%d\n", &state) != 1)
-		return -EINVAL;
-
-	/* sanity check: values below 1000 millicelcius don't make sense
-	 * and can cause the system to go into a thermal heart attack
-	 */
-	if (state && state < 1000)
-		return -EINVAL;
-
-	if (state && !tz->forced_passive) {
-		if (!tz->passive_delay)
-			tz->passive_delay = 1000;
-		thermal_zone_device_rebind_exception(tz, "Processor",
-						     sizeof("Processor"));
-	} else if (!state && tz->forced_passive) {
-		tz->passive_delay = 0;
-		thermal_zone_device_unbind_exception(tz, "Processor",
-						     sizeof("Processor"));
-	}
-
-	tz->forced_passive = state;
-
-	thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
-
-	return count;
-}
-
-static ssize_t
-passive_show(struct device *dev, struct device_attribute *attr,
-	     char *buf)
-{
-	struct thermal_zone_device *tz = to_thermal_zone(dev);
-
-	return sprintf(buf, "%d\n", tz->forced_passive);
-}
-
 static ssize_t
 policy_store(struct device *dev, struct device_attribute *attr,
 	     const char *buf, size_t count)
@@ -403,7 +360,6 @@ static DEVICE_ATTR_RW(sustainable_power);
 
 /* These thermal zone device attributes are created based on conditions */
 static DEVICE_ATTR_RW(mode);
-static DEVICE_ATTR_RW(passive);
 
 /* These attributes are unconditionally added to a thermal zone */
 static struct attribute *thermal_zone_dev_attrs[] = {
@@ -438,45 +394,9 @@ static const struct attribute_group thermal_zone_mode_attribute_group = {
 	.attrs = thermal_zone_mode_attrs,
 };
 
-/* We expose passive only if passive trips are present */
-static struct attribute *thermal_zone_passive_attrs[] = {
-	&dev_attr_passive.attr,
-	NULL,
-};
-
-static umode_t thermal_zone_passive_is_visible(struct kobject *kobj,
-					       struct attribute *attr,
-					       int attrno)
-{
-	struct device *dev = kobj_to_dev(kobj);
-	struct thermal_zone_device *tz;
-	enum thermal_trip_type trip_type;
-	int count, passive = 0;
-
-	tz = container_of(dev, struct thermal_zone_device, device);
-
-	for (count = 0; count < tz->trips && !passive; count++) {
-		tz->ops->get_trip_type(tz, count, &trip_type);
-
-		if (trip_type == THERMAL_TRIP_PASSIVE)
-			passive = 1;
-	}
-
-	if (!passive)
-		return attr->mode;
-
-	return 0;
-}
-
-static const struct attribute_group thermal_zone_passive_attribute_group = {
-	.attrs = thermal_zone_passive_attrs,
-	.is_visible = thermal_zone_passive_is_visible,
-};
-
 static const struct attribute_group *thermal_zone_attribute_groups[] = {
 	&thermal_zone_attribute_group,
 	&thermal_zone_mode_attribute_group,
-	&thermal_zone_passive_attribute_group,
 	/* This is not NULL terminated as we create the group dynamically */
 };
 
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index c80032322158..a57232a9a6f9 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -131,9 +131,6 @@ struct thermal_cooling_device {
 			trip point.
  * @prev_high_trip:	the above current temperature if you've crossed a
 			passive trip point.
- * @forced_passive:	If > 0, temperature at which to switch on all ACPI
- *			processor cooling devices.  Currently only used by the
- *			step-wise governor.
  * @need_update:	if equals 1, thermal_zone_device_update needs to be invoked.
  * @ops:	operations this &thermal_zone_device supports
  * @tzp:	thermal zone parameters
@@ -167,7 +164,6 @@ struct thermal_zone_device {
 	int passive;
 	int prev_low_trip;
 	int prev_high_trip;
-	unsigned int forced_passive;
 	atomic_t need_update;
 	struct thermal_zone_device_ops *ops;
 	struct thermal_zone_params *tzp;
-- 
2.25.1

[PATCH 2/6] thermal/core: Remove unused functions rebind/unbind exception

[Daniel Lezcano]

The functions thermal_zone_device_rebind_exception and
thermal_zone_device_unbind_exception are not used from anywhere.

Remove that code.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/thermal/thermal_core.c | 37 ----------------------------------
 drivers/thermal/thermal_core.h |  4 ----
 2 files changed, 41 deletions(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 567bc6f254c0..a0f0c33c8d9c 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -598,26 +598,6 @@ static void thermal_zone_device_check(struct work_struct *work)
 	thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
 }
 
-void thermal_zone_device_rebind_exception(struct thermal_zone_device *tz,
-					  const char *cdev_type, size_t size)
-{
-	struct thermal_cooling_device *cdev = NULL;
-
-	mutex_lock(&thermal_list_lock);
-	list_for_each_entry(cdev, &thermal_cdev_list, node) {
-		/* skip non matching cdevs */
-		if (strncmp(cdev_type, cdev->type, size))
-			continue;
-
-		/* re binding the exception matching the type pattern */
-		thermal_zone_bind_cooling_device(tz, THERMAL_TRIPS_NONE, cdev,
-						 THERMAL_NO_LIMIT,
-						 THERMAL_NO_LIMIT,
-						 THERMAL_WEIGHT_DEFAULT);
-	}
-	mutex_unlock(&thermal_list_lock);
-}
-
 int for_each_thermal_governor(int (*cb)(struct thermal_governor *, void *),
 			      void *data)
 {
@@ -685,23 +665,6 @@ struct thermal_zone_device *thermal_zone_get_by_id(int id)
 	return match;
 }
 
-void thermal_zone_device_unbind_exception(struct thermal_zone_device *tz,
-					  const char *cdev_type, size_t size)
-{
-	struct thermal_cooling_device *cdev = NULL;
-
-	mutex_lock(&thermal_list_lock);
-	list_for_each_entry(cdev, &thermal_cdev_list, node) {
-		/* skip non matching cdevs */
-		if (strncmp(cdev_type, cdev->type, size))
-			continue;
-		/* unbinding the exception matching the type pattern */
-		thermal_zone_unbind_cooling_device(tz, THERMAL_TRIPS_NONE,
-						   cdev);
-	}
-	mutex_unlock(&thermal_list_lock);
-}
-
 /*
  * Device management section: cooling devices, zones devices, and binding
  *
diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index 8df600fa7b79..e50c6b2909fe 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -118,10 +118,6 @@ struct thermal_instance {
 
 int thermal_register_governor(struct thermal_governor *);
 void thermal_unregister_governor(struct thermal_governor *);
-void thermal_zone_device_rebind_exception(struct thermal_zone_device *,
-					  const char *, size_t);
-void thermal_zone_device_unbind_exception(struct thermal_zone_device *,
-					  const char *, size_t);
 int thermal_zone_device_set_policy(struct thermal_zone_device *, char *);
 int thermal_build_list_of_policies(char *buf);
 
-- 
2.25.1

[PATCH 3/6] thermal/core: Remove pointless test with the THERMAL_TRIPS_NONE macro

[Daniel Lezcano]

The THERMAL_TRIPS_NONE is equal to -1, it is pointless to do a
conversion in this function.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/thermal/thermal_sysfs.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
index 4e7f9e880d76..345917a58f2f 100644
--- a/drivers/thermal/thermal_sysfs.c
+++ b/drivers/thermal/thermal_sysfs.c
@@ -875,10 +875,7 @@ trip_point_show(struct device *dev, struct device_attribute *attr, char *buf)
 	instance =
 	    container_of(attr, struct thermal_instance, attr);
 
-	if (instance->trip == THERMAL_TRIPS_NONE)
-		return sprintf(buf, "-1\n");
-	else
-		return sprintf(buf, "%d\n", instance->trip);
+	return sprintf(buf, "%d\n", instance->trip);
 }
 
 ssize_t
-- 
2.25.1

[PATCH 4/6] acpi/drivers/thermal: Remove TRIPS_NONE cooling device binding

[Daniel Lezcano]

The loop is here to create default cooling device binding on the
THERMAL_TRIPS_NONE number which is used to be the 'forced_passive'
feature. However, we removed all code dealing with that in the thermal
core, thus this binding does no longer make sense.

Remove it.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/acpi/thermal.c | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index b5e4bc9e3282..26a89ff80a0e 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -764,25 +764,6 @@ static int acpi_thermal_cooling_device_cb(struct thermal_zone_device *thermal,
 		}
 	}
 
-	for (i = 0; i < tz->devices.count; i++) {
-		handle = tz->devices.handles[i];
-		status = acpi_bus_get_device(handle, &dev);
-		if (ACPI_SUCCESS(status) && (dev == device)) {
-			if (bind)
-				result = thermal_zone_bind_cooling_device
-						(thermal, THERMAL_TRIPS_NONE,
-						 cdev, THERMAL_NO_LIMIT,
-						 THERMAL_NO_LIMIT,
-						 THERMAL_WEIGHT_DEFAULT);
-			else
-				result = thermal_zone_unbind_cooling_device
-						(thermal, THERMAL_TRIPS_NONE,
-						 cdev);
-			if (result)
-				goto failed;
-		}
-	}
-
 failed:
 	return result;
 }
-- 
2.25.1

[PATCH 5/6] thermal/core: Remove THERMAL_TRIPS_NONE test

[Daniel Lezcano]

The last site calling the thermal_zone_bind_cooling_device() function
with the THERMAL_TRIPS_NONE parameter was removed.

We can get rid of this test as no user of this function is calling
this function with this parameter.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/thermal/thermal_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index a0f0c33c8d9c..bcc2ea4f5482 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -710,7 +710,7 @@ int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
 	unsigned long max_state;
 	int result, ret;
 
-	if (trip >= tz->trips || (trip < 0 && trip != THERMAL_TRIPS_NONE))
+	if (trip >= tz->trips || trip < 0)
 		return -EINVAL;
 
 	list_for_each_entry(pos1, &thermal_tz_list, node) {
-- 
2.25.1

[PATCH 6/6] thermal/core: Remove unused macro THERMAL_TRIPS_NONE

[Daniel Lezcano]

The macro THERMAL_TRIPS_NONE is no longer used, remove it.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 include/linux/thermal.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index a57232a9a6f9..060a2160add4 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -17,7 +17,6 @@
 #include <linux/workqueue.h>
 #include <uapi/linux/thermal.h>
 
-#define THERMAL_TRIPS_NONE	-1
 #define THERMAL_MAX_TRIPS	12
 
 /* invalid cooling state */
-- 
2.25.1

Re: [PATCH 1/6] thermal/core: Remove the 'forced_passive' option

[Daniel Lezcano]

On 15/12/2020 00:38, Daniel Lezcano wrote:
> The code was reorganized in 2012 with the commit 0c01ebbfd3caf1.
> 
> The main change is a loop on the trip points array and a unconditional
> call to the throttle() ops of the governors for each of them even if
> the trip temperature is not reached yet.
> 
> With this change, the 'forced_passive' is no longer checked in the
> thermal_zone_device_update() function but in the step wise governor's
> throttle() callback.
> 
> As the force_passive does no belong to the trip point array, the
> thermal_zone_device_update() can not compare with the specified
> passive temperature, thus does not detect the passive limit has been
> crossed. Consequently, throttle() is never called and the
> 'forced_passive' branch is unreached.
> 
> In addition, the default processor cooling device is not automatically
> bound to the thermal zone if there is not passive trip point, thus the
> 'forced_passive' can not operate.
> 
> If there is an active trip point, then the throttle function will be
> called to mitigate at this temperature and the 'forced_passive' will
> override the mitigation of the active trip point in this case but with
> the default cooling device bound to the thermal zone, so usually a
> fan, and that is not a passive cooling effect.
> 
> Given the regression exists since more than 8 years, nobody complained
> and at the best of my knowledge there is no bug open in
> https://bugzilla.kernel.org, it is reasonable to say it is unused.
> 
> Remove the 'forced_passive' related code.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>

Is there any concern with this series ?


> ---
>  .../driver-api/thermal/sysfs-api.rst          | 13 ---
>  drivers/thermal/gov_step_wise.c               | 14 +---
>  drivers/thermal/thermal_sysfs.c               | 80 -------------------
>  include/linux/thermal.h                       |  4 -
>  4 files changed, 3 insertions(+), 108 deletions(-)
> 
> diff --git a/Documentation/driver-api/thermal/sysfs-api.rst b/Documentation/driver-api/thermal/sysfs-api.rst
> index e7520cb439ac..a4969c474cc3 100644
> --- a/Documentation/driver-api/thermal/sysfs-api.rst
> +++ b/Documentation/driver-api/thermal/sysfs-api.rst
> @@ -520,19 +520,6 @@ available_policies
>  
>  	RW, Optional
>  
> -passive
> -	Attribute is only present for zones in which the passive cooling
> -	policy is not supported by native thermal driver. Default is zero
> -	and can be set to a temperature (in millidegrees) to enable a
> -	passive trip point for the zone. Activation is done by polling with
> -	an interval of 1 second.
> -
> -	Unit: millidegrees Celsius
> -
> -	Valid values: 0 (disabled) or greater than 1000
> -
> -	RW, Optional
> -
>  emul_temp
>  	Interface to set the emulated temperature method in thermal zone
>  	(sensor). After setting this temperature, the thermal zone may pass
> diff --git a/drivers/thermal/gov_step_wise.c b/drivers/thermal/gov_step_wise.c
> index 2ae7198d3067..12acb12aac50 100644
> --- a/drivers/thermal/gov_step_wise.c
> +++ b/drivers/thermal/gov_step_wise.c
> @@ -109,7 +109,7 @@ static void update_passive_instance(struct thermal_zone_device *tz,
>  	 * If value is +1, activate a passive instance.
>  	 * If value is -1, deactivate a passive instance.
>  	 */
> -	if (type == THERMAL_TRIP_PASSIVE || type == THERMAL_TRIPS_NONE)
> +	if (type == THERMAL_TRIP_PASSIVE)
>  		tz->passive += value;
>  }
>  
> @@ -122,13 +122,8 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
>  	bool throttle = false;
>  	int old_target;
>  
> -	if (trip == THERMAL_TRIPS_NONE) {
> -		trip_temp = tz->forced_passive;
> -		trip_type = THERMAL_TRIPS_NONE;
> -	} else {
> -		tz->ops->get_trip_temp(tz, trip, &trip_temp);
> -		tz->ops->get_trip_type(tz, trip, &trip_type);
> -	}
> +	tz->ops->get_trip_temp(tz, trip, &trip_temp);
> +	tz->ops->get_trip_type(tz, trip, &trip_type);
>  
>  	trend = get_tz_trend(tz, trip);
>  
> @@ -189,9 +184,6 @@ static int step_wise_throttle(struct thermal_zone_device *tz, int trip)
>  
>  	thermal_zone_trip_update(tz, trip);
>  
> -	if (tz->forced_passive)
> -		thermal_zone_trip_update(tz, THERMAL_TRIPS_NONE);
> -
>  	mutex_lock(&tz->lock);
>  
>  	list_for_each_entry(instance, &tz->thermal_instances, tz_node)
> diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
> index 0866e949339b..4e7f9e880d76 100644
> --- a/drivers/thermal/thermal_sysfs.c
> +++ b/drivers/thermal/thermal_sysfs.c
> @@ -216,49 +216,6 @@ trip_point_hyst_show(struct device *dev, struct device_attribute *attr,
>  	return ret ? ret : sprintf(buf, "%d\n", temperature);
>  }
>  
> -static ssize_t
> -passive_store(struct device *dev, struct device_attribute *attr,
> -	      const char *buf, size_t count)
> -{
> -	struct thermal_zone_device *tz = to_thermal_zone(dev);
> -	int state;
> -
> -	if (sscanf(buf, "%d\n", &state) != 1)
> -		return -EINVAL;
> -
> -	/* sanity check: values below 1000 millicelcius don't make sense
> -	 * and can cause the system to go into a thermal heart attack
> -	 */
> -	if (state && state < 1000)
> -		return -EINVAL;
> -
> -	if (state && !tz->forced_passive) {
> -		if (!tz->passive_delay)
> -			tz->passive_delay = 1000;
> -		thermal_zone_device_rebind_exception(tz, "Processor",
> -						     sizeof("Processor"));
> -	} else if (!state && tz->forced_passive) {
> -		tz->passive_delay = 0;
> -		thermal_zone_device_unbind_exception(tz, "Processor",
> -						     sizeof("Processor"));
> -	}
> -
> -	tz->forced_passive = state;
> -
> -	thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
> -
> -	return count;
> -}
> -
> -static ssize_t
> -passive_show(struct device *dev, struct device_attribute *attr,
> -	     char *buf)
> -{
> -	struct thermal_zone_device *tz = to_thermal_zone(dev);
> -
> -	return sprintf(buf, "%d\n", tz->forced_passive);
> -}
> -
>  static ssize_t
>  policy_store(struct device *dev, struct device_attribute *attr,
>  	     const char *buf, size_t count)
> @@ -403,7 +360,6 @@ static DEVICE_ATTR_RW(sustainable_power);
>  
>  /* These thermal zone device attributes are created based on conditions */
>  static DEVICE_ATTR_RW(mode);
> -static DEVICE_ATTR_RW(passive);
>  
>  /* These attributes are unconditionally added to a thermal zone */
>  static struct attribute *thermal_zone_dev_attrs[] = {
> @@ -438,45 +394,9 @@ static const struct attribute_group thermal_zone_mode_attribute_group = {
>  	.attrs = thermal_zone_mode_attrs,
>  };
>  
> -/* We expose passive only if passive trips are present */
> -static struct attribute *thermal_zone_passive_attrs[] = {
> -	&dev_attr_passive.attr,
> -	NULL,
> -};
> -
> -static umode_t thermal_zone_passive_is_visible(struct kobject *kobj,
> -					       struct attribute *attr,
> -					       int attrno)
> -{
> -	struct device *dev = kobj_to_dev(kobj);
> -	struct thermal_zone_device *tz;
> -	enum thermal_trip_type trip_type;
> -	int count, passive = 0;
> -
> -	tz = container_of(dev, struct thermal_zone_device, device);
> -
> -	for (count = 0; count < tz->trips && !passive; count++) {
> -		tz->ops->get_trip_type(tz, count, &trip_type);
> -
> -		if (trip_type == THERMAL_TRIP_PASSIVE)
> -			passive = 1;
> -	}
> -
> -	if (!passive)
> -		return attr->mode;
> -
> -	return 0;
> -}
> -
> -static const struct attribute_group thermal_zone_passive_attribute_group = {
> -	.attrs = thermal_zone_passive_attrs,
> -	.is_visible = thermal_zone_passive_is_visible,
> -};
> -
>  static const struct attribute_group *thermal_zone_attribute_groups[] = {
>  	&thermal_zone_attribute_group,
>  	&thermal_zone_mode_attribute_group,
> -	&thermal_zone_passive_attribute_group,
>  	/* This is not NULL terminated as we create the group dynamically */
>  };
>  
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index c80032322158..a57232a9a6f9 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -131,9 +131,6 @@ struct thermal_cooling_device {
>  			trip point.
>   * @prev_high_trip:	the above current temperature if you've crossed a
>  			passive trip point.
> - * @forced_passive:	If > 0, temperature at which to switch on all ACPI
> - *			processor cooling devices.  Currently only used by the
> - *			step-wise governor.
>   * @need_update:	if equals 1, thermal_zone_device_update needs to be invoked.
>   * @ops:	operations this &thermal_zone_device supports
>   * @tzp:	thermal zone parameters
> @@ -167,7 +164,6 @@ struct thermal_zone_device {
>  	int passive;
>  	int prev_low_trip;
>  	int prev_high_trip;
> -	unsigned int forced_passive;
>  	atomic_t need_update;
>  	struct thermal_zone_device_ops *ops;
>  	struct thermal_zone_params *tzp;
> 


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH 4/6] acpi/drivers/thermal: Remove TRIPS_NONE cooling device binding

[Daniel Lezcano]

Hi Rui,


On 15/12/2020 00:38, Daniel Lezcano wrote:
> The loop is here to create default cooling device binding on the
> THERMAL_TRIPS_NONE number which is used to be the 'forced_passive'
> feature. However, we removed all code dealing with that in the thermal
> core, thus this binding does no longer make sense.
> 
> Remove it.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>

Are you fine with this change?

Thanks

  -- Daniel

> ---
>  drivers/acpi/thermal.c | 19 -------------------
>  1 file changed, 19 deletions(-)
> 
> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> index b5e4bc9e3282..26a89ff80a0e 100644
> --- a/drivers/acpi/thermal.c
> +++ b/drivers/acpi/thermal.c
> @@ -764,25 +764,6 @@ static int acpi_thermal_cooling_device_cb(struct thermal_zone_device *thermal,
>  		}
>  	}
>  
> -	for (i = 0; i < tz->devices.count; i++) {
> -		handle = tz->devices.handles[i];
> -		status = acpi_bus_get_device(handle, &dev);
> -		if (ACPI_SUCCESS(status) && (dev == device)) {
> -			if (bind)
> -				result = thermal_zone_bind_cooling_device
> -						(thermal, THERMAL_TRIPS_NONE,
> -						 cdev, THERMAL_NO_LIMIT,
> -						 THERMAL_NO_LIMIT,
> -						 THERMAL_WEIGHT_DEFAULT);
> -			else
> -				result = thermal_zone_unbind_cooling_device
> -						(thermal, THERMAL_TRIPS_NONE,
> -						 cdev);
> -			if (result)
> -				goto failed;
> -		}
> -	}
> -
>  failed:
>  	return result;
>  }
> 


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

RE: [PATCH 4/6] acpi/drivers/thermal: Remove TRIPS_NONE cooling device binding

[Zhang, Rui]

ACPI thermal driver binds the devices listed in _TZD method with THERMAL_TRIPS_NONE.
Now given that
1. THERMAL_TRIPS_NONE is removed from thermal framework
2. _TZP is rarely supported. I searched ~500 acpidumps from different platforms reported by end users in kernel Bugzilla, there is only one platform with _TZP implemented, and it was almost 10 years ago.

So, I think it is safe to remove this piece of code.

> -----Original Message-----
> From: Daniel Lezcano <daniel.lezcano@linaro.org>
> Sent: Tuesday, January 05, 2021 11:44 PM
> To: Zhang, Rui <rui.zhang@intel.com>
> Cc: mjg59@codon.org.uk; linux-pm@vger.kernel.org; linux-
> kernel@vger.kernel.org; amitk@kernel.org; thara.gopinath@linaro.org;
> Rafael J. Wysocki <rjw@rjwysocki.net>; Len Brown <lenb@kernel.org>; open
> list:ACPI THERMAL DRIVER <linux-acpi@vger.kernel.org>
> Subject: Re: [PATCH 4/6] acpi/drivers/thermal: Remove TRIPS_NONE cooling
> device binding
> Importance: High
> 
> Hi Rui,
> 
> 
> On 15/12/2020 00:38, Daniel Lezcano wrote:
> > The loop is here to create default cooling device binding on the
> > THERMAL_TRIPS_NONE number which is used to be the 'forced_passive'
> > feature. However, we removed all code dealing with that in the thermal
> > core, thus this binding does no longer make sense.
> >
> > Remove it.
> >
> > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>

Acked-by: Zhang Rui <rui.zhang@intel.com>

Thanks,
rui
> 
> Are you fine with this change?
> 
> Thanks
> 
>   -- Daniel
> 
> > ---
> >  drivers/acpi/thermal.c | 19 -------------------
> >  1 file changed, 19 deletions(-)
> >
> > diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c index
> > b5e4bc9e3282..26a89ff80a0e 100644
> > --- a/drivers/acpi/thermal.c
> > +++ b/drivers/acpi/thermal.c
> > @@ -764,25 +764,6 @@ static int acpi_thermal_cooling_device_cb(struct
> thermal_zone_device *thermal,
> >  		}
> >  	}
> >
> > -	for (i = 0; i < tz->devices.count; i++) {
> > -		handle = tz->devices.handles[i];
> > -		status = acpi_bus_get_device(handle, &dev);
> > -		if (ACPI_SUCCESS(status) && (dev == device)) {
> > -			if (bind)
> > -				result = thermal_zone_bind_cooling_device
> > -						(thermal,
> THERMAL_TRIPS_NONE,
> > -						 cdev, THERMAL_NO_LIMIT,
> > -						 THERMAL_NO_LIMIT,
> > -
> THERMAL_WEIGHT_DEFAULT);
> > -			else
> > -				result =
> thermal_zone_unbind_cooling_device
> > -						(thermal,
> THERMAL_TRIPS_NONE,
> > -						 cdev);
> > -			if (result)
> > -				goto failed;
> > -		}
> > -	}
> > -
> >  failed:
> >  	return result;
> >  }
> >
> 
> 
> --
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
> 
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-
> blog/> Blog

Re: [PATCH 4/6] acpi/drivers/thermal: Remove TRIPS_NONE cooling device binding

[Daniel Lezcano]

On 07/01/2021 06:10, Zhang, Rui wrote:
> ACPI thermal driver binds the devices listed in _TZD method with THERMAL_TRIPS_NONE.
> Now given that
> 1. THERMAL_TRIPS_NONE is removed from thermal framework
> 2. _TZP is rarely supported. I searched ~500 acpidumps from different platforms reported by end users in kernel Bugzilla, there is only one platform with _TZP implemented, and it was almost 10 years ago.
> 
> So, I think it is safe to remove this piece of code.

Thanks Rui for digging into



-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH 1/6] thermal/core: Remove the 'forced_passive' option

[Thara Gopinath]

On 12/14/20 6:38 PM, Daniel Lezcano wrote:
> The code was reorganized in 2012 with the commit 0c01ebbfd3caf1.
> 
> The main change is a loop on the trip points array and a unconditional
> call to the throttle() ops of the governors for each of them even if
> the trip temperature is not reached yet.
> 
> With this change, the 'forced_passive' is no longer checked in the
> thermal_zone_device_update() function but in the step wise governor's
> throttle() callback.
> 
> As the force_passive does no belong to the trip point array, the
> thermal_zone_device_update() can not compare with the specified
> passive temperature, thus does not detect the passive limit has been
> crossed. Consequently, throttle() is never called and the
> 'forced_passive' branch is unreached.
> 
> In addition, the default processor cooling device is not automatically
> bound to the thermal zone if there is not passive trip point, thus the
> 'forced_passive' can not operate.
> 
> If there is an active trip point, then the throttle function will be
> called to mitigate at this temperature and the 'forced_passive' will
> override the mitigation of the active trip point in this case but with
> the default cooling device bound to the thermal zone, so usually a
> fan, and that is not a passive cooling effect.
> 
> Given the regression exists since more than 8 years, nobody complained
> and at the best of my knowledge there is no bug open in
> https://bugzilla.kernel.org, it is reasonable to say it is unused.
> 
> Remove the 'forced_passive' related code.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>

Reviewed-by: Thara Gopinath <thara.gopinath@linaro.org>

> ---
>   .../driver-api/thermal/sysfs-api.rst          | 13 ---
>   drivers/thermal/gov_step_wise.c               | 14 +---
>   drivers/thermal/thermal_sysfs.c               | 80 -------------------
>   include/linux/thermal.h                       |  4 -
>   4 files changed, 3 insertions(+), 108 deletions(-)
> 
> diff --git a/Documentation/driver-api/thermal/sysfs-api.rst b/Documentation/driver-api/thermal/sysfs-api.rst
> index e7520cb439ac..a4969c474cc3 100644
> --- a/Documentation/driver-api/thermal/sysfs-api.rst
> +++ b/Documentation/driver-api/thermal/sysfs-api.rst
> @@ -520,19 +520,6 @@ available_policies
>   
>   	RW, Optional
>   
> -passive
> -	Attribute is only present for zones in which the passive cooling
> -	policy is not supported by native thermal driver. Default is zero
> -	and can be set to a temperature (in millidegrees) to enable a
> -	passive trip point for the zone. Activation is done by polling with
> -	an interval of 1 second.
> -
> -	Unit: millidegrees Celsius
> -
> -	Valid values: 0 (disabled) or greater than 1000
> -
> -	RW, Optional
> -
>   emul_temp
>   	Interface to set the emulated temperature method in thermal zone
>   	(sensor). After setting this temperature, the thermal zone may pass
> diff --git a/drivers/thermal/gov_step_wise.c b/drivers/thermal/gov_step_wise.c
> index 2ae7198d3067..12acb12aac50 100644
> --- a/drivers/thermal/gov_step_wise.c
> +++ b/drivers/thermal/gov_step_wise.c
> @@ -109,7 +109,7 @@ static void update_passive_instance(struct thermal_zone_device *tz,
>   	 * If value is +1, activate a passive instance.
>   	 * If value is -1, deactivate a passive instance.
>   	 */
> -	if (type == THERMAL_TRIP_PASSIVE || type == THERMAL_TRIPS_NONE)
> +	if (type == THERMAL_TRIP_PASSIVE)
>   		tz->passive += value;
>   }
>   
> @@ -122,13 +122,8 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
>   	bool throttle = false;
>   	int old_target;
>   
> -	if (trip == THERMAL_TRIPS_NONE) {
> -		trip_temp = tz->forced_passive;
> -		trip_type = THERMAL_TRIPS_NONE;
> -	} else {
> -		tz->ops->get_trip_temp(tz, trip, &trip_temp);
> -		tz->ops->get_trip_type(tz, trip, &trip_type);
> -	}
> +	tz->ops->get_trip_temp(tz, trip, &trip_temp);
> +	tz->ops->get_trip_type(tz, trip, &trip_type);
>   
>   	trend = get_tz_trend(tz, trip);
>   
> @@ -189,9 +184,6 @@ static int step_wise_throttle(struct thermal_zone_device *tz, int trip)
>   
>   	thermal_zone_trip_update(tz, trip);
>   
> -	if (tz->forced_passive)
> -		thermal_zone_trip_update(tz, THERMAL_TRIPS_NONE);
> -
>   	mutex_lock(&tz->lock);
>   
>   	list_for_each_entry(instance, &tz->thermal_instances, tz_node)
> diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
> index 0866e949339b..4e7f9e880d76 100644
> --- a/drivers/thermal/thermal_sysfs.c
> +++ b/drivers/thermal/thermal_sysfs.c
> @@ -216,49 +216,6 @@ trip_point_hyst_show(struct device *dev, struct device_attribute *attr,
>   	return ret ? ret : sprintf(buf, "%d\n", temperature);
>   }
>   
> -static ssize_t
> -passive_store(struct device *dev, struct device_attribute *attr,
> -	      const char *buf, size_t count)
> -{
> -	struct thermal_zone_device *tz = to_thermal_zone(dev);
> -	int state;
> -
> -	if (sscanf(buf, "%d\n", &state) != 1)
> -		return -EINVAL;
> -
> -	/* sanity check: values below 1000 millicelcius don't make sense
> -	 * and can cause the system to go into a thermal heart attack
> -	 */
> -	if (state && state < 1000)
> -		return -EINVAL;
> -
> -	if (state && !tz->forced_passive) {
> -		if (!tz->passive_delay)
> -			tz->passive_delay = 1000;
> -		thermal_zone_device_rebind_exception(tz, "Processor",
> -						     sizeof("Processor"));
> -	} else if (!state && tz->forced_passive) {
> -		tz->passive_delay = 0;
> -		thermal_zone_device_unbind_exception(tz, "Processor",
> -						     sizeof("Processor"));
> -	}
> -
> -	tz->forced_passive = state;
> -
> -	thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
> -
> -	return count;
> -}
> -
> -static ssize_t
> -passive_show(struct device *dev, struct device_attribute *attr,
> -	     char *buf)
> -{
> -	struct thermal_zone_device *tz = to_thermal_zone(dev);
> -
> -	return sprintf(buf, "%d\n", tz->forced_passive);
> -}
> -
>   static ssize_t
>   policy_store(struct device *dev, struct device_attribute *attr,
>   	     const char *buf, size_t count)
> @@ -403,7 +360,6 @@ static DEVICE_ATTR_RW(sustainable_power);
>   
>   /* These thermal zone device attributes are created based on conditions */
>   static DEVICE_ATTR_RW(mode);
> -static DEVICE_ATTR_RW(passive);
>   
>   /* These attributes are unconditionally added to a thermal zone */
>   static struct attribute *thermal_zone_dev_attrs[] = {
> @@ -438,45 +394,9 @@ static const struct attribute_group thermal_zone_mode_attribute_group = {
>   	.attrs = thermal_zone_mode_attrs,
>   };
>   
> -/* We expose passive only if passive trips are present */
> -static struct attribute *thermal_zone_passive_attrs[] = {
> -	&dev_attr_passive.attr,
> -	NULL,
> -};
> -
> -static umode_t thermal_zone_passive_is_visible(struct kobject *kobj,
> -					       struct attribute *attr,
> -					       int attrno)
> -{
> -	struct device *dev = kobj_to_dev(kobj);
> -	struct thermal_zone_device *tz;
> -	enum thermal_trip_type trip_type;
> -	int count, passive = 0;
> -
> -	tz = container_of(dev, struct thermal_zone_device, device);
> -
> -	for (count = 0; count < tz->trips && !passive; count++) {
> -		tz->ops->get_trip_type(tz, count, &trip_type);
> -
> -		if (trip_type == THERMAL_TRIP_PASSIVE)
> -			passive = 1;
> -	}
> -
> -	if (!passive)
> -		return attr->mode;
> -
> -	return 0;
> -}
> -
> -static const struct attribute_group thermal_zone_passive_attribute_group = {
> -	.attrs = thermal_zone_passive_attrs,
> -	.is_visible = thermal_zone_passive_is_visible,
> -};
> -
>   static const struct attribute_group *thermal_zone_attribute_groups[] = {
>   	&thermal_zone_attribute_group,
>   	&thermal_zone_mode_attribute_group,
> -	&thermal_zone_passive_attribute_group,
>   	/* This is not NULL terminated as we create the group dynamically */
>   };
>   
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index c80032322158..a57232a9a6f9 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -131,9 +131,6 @@ struct thermal_cooling_device {
>   			trip point.
>    * @prev_high_trip:	the above current temperature if you've crossed a
>   			passive trip point.
> - * @forced_passive:	If > 0, temperature at which to switch on all ACPI
> - *			processor cooling devices.  Currently only used by the
> - *			step-wise governor.
>    * @need_update:	if equals 1, thermal_zone_device_update needs to be invoked.
>    * @ops:	operations this &thermal_zone_device supports
>    * @tzp:	thermal zone parameters
> @@ -167,7 +164,6 @@ struct thermal_zone_device {
>   	int passive;
>   	int prev_low_trip;
>   	int prev_high_trip;
> -	unsigned int forced_passive;
>   	atomic_t need_update;
>   	struct thermal_zone_device_ops *ops;
>   	struct thermal_zone_params *tzp;
> 

-- 
Warm Regards
Thara

Re: [PATCH 2/6] thermal/core: Remove unused functions rebind/unbind exception

[Thara Gopinath]

On 12/14/20 6:38 PM, Daniel Lezcano wrote:
> The functions thermal_zone_device_rebind_exception and
> thermal_zone_device_unbind_exception are not used from anywhere.
> 
> Remove that code.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>

Reviewed-by: Thara Gopinath <thara.gopinath@linaro.org>

> ---
>   drivers/thermal/thermal_core.c | 37 ----------------------------------
>   drivers/thermal/thermal_core.h |  4 ----
>   2 files changed, 41 deletions(-)
> 
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 567bc6f254c0..a0f0c33c8d9c 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -598,26 +598,6 @@ static void thermal_zone_device_check(struct work_struct *work)
>   	thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
>   }
>   
> -void thermal_zone_device_rebind_exception(struct thermal_zone_device *tz,
> -					  const char *cdev_type, size_t size)
> -{
> -	struct thermal_cooling_device *cdev = NULL;
> -
> -	mutex_lock(&thermal_list_lock);
> -	list_for_each_entry(cdev, &thermal_cdev_list, node) {
> -		/* skip non matching cdevs */
> -		if (strncmp(cdev_type, cdev->type, size))
> -			continue;
> -
> -		/* re binding the exception matching the type pattern */
> -		thermal_zone_bind_cooling_device(tz, THERMAL_TRIPS_NONE, cdev,
> -						 THERMAL_NO_LIMIT,
> -						 THERMAL_NO_LIMIT,
> -						 THERMAL_WEIGHT_DEFAULT);
> -	}
> -	mutex_unlock(&thermal_list_lock);
> -}
> -
>   int for_each_thermal_governor(int (*cb)(struct thermal_governor *, void *),
>   			      void *data)
>   {
> @@ -685,23 +665,6 @@ struct thermal_zone_device *thermal_zone_get_by_id(int id)
>   	return match;
>   }
>   
> -void thermal_zone_device_unbind_exception(struct thermal_zone_device *tz,
> -					  const char *cdev_type, size_t size)
> -{
> -	struct thermal_cooling_device *cdev = NULL;
> -
> -	mutex_lock(&thermal_list_lock);
> -	list_for_each_entry(cdev, &thermal_cdev_list, node) {
> -		/* skip non matching cdevs */
> -		if (strncmp(cdev_type, cdev->type, size))
> -			continue;
> -		/* unbinding the exception matching the type pattern */
> -		thermal_zone_unbind_cooling_device(tz, THERMAL_TRIPS_NONE,
> -						   cdev);
> -	}
> -	mutex_unlock(&thermal_list_lock);
> -}
> -
>   /*
>    * Device management section: cooling devices, zones devices, and binding
>    *
> diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
> index 8df600fa7b79..e50c6b2909fe 100644
> --- a/drivers/thermal/thermal_core.h
> +++ b/drivers/thermal/thermal_core.h
> @@ -118,10 +118,6 @@ struct thermal_instance {
>   
>   int thermal_register_governor(struct thermal_governor *);
>   void thermal_unregister_governor(struct thermal_governor *);
> -void thermal_zone_device_rebind_exception(struct thermal_zone_device *,
> -					  const char *, size_t);
> -void thermal_zone_device_unbind_exception(struct thermal_zone_device *,
> -					  const char *, size_t);
>   int thermal_zone_device_set_policy(struct thermal_zone_device *, char *);
>   int thermal_build_list_of_policies(char *buf);
>   
> 

-- 
Warm Regards
Thara

Re: [PATCH 3/6] thermal/core: Remove pointless test with the THERMAL_TRIPS_NONE macro

[Thara Gopinath]

On 12/14/20 6:38 PM, Daniel Lezcano wrote:
> The THERMAL_TRIPS_NONE is equal to -1, it is pointless to do a
> conversion in this function.

It is pointless because you have removed force_passive and
will be removing THERMAL_TRIPS_NONE in the following patches.
Maybe that should be mentioned in the commit message.
Otherwise,

Reviewed-by: Thara Gopinath <thara.gopinath@linaro.org>

> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>   drivers/thermal/thermal_sysfs.c | 5 +----
>   1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
> index 4e7f9e880d76..345917a58f2f 100644
> --- a/drivers/thermal/thermal_sysfs.c
> +++ b/drivers/thermal/thermal_sysfs.c
> @@ -875,10 +875,7 @@ trip_point_show(struct device *dev, struct device_attribute *attr, char *buf)
>   	instance =
>   	    container_of(attr, struct thermal_instance, attr);
>   
> -	if (instance->trip == THERMAL_TRIPS_NONE)
> -		return sprintf(buf, "-1\n");
> -	else
> -		return sprintf(buf, "%d\n", instance->trip);
> +	return sprintf(buf, "%d\n", instance->trip);
>   }
>   
>   ssize_t
> 

-- 
Warm Regards
Thara

Re: [PATCH 5/6] thermal/core: Remove THERMAL_TRIPS_NONE test

[Thara Gopinath]

On 12/14/20 6:38 PM, Daniel Lezcano wrote:
> The last site calling the thermal_zone_bind_cooling_device() function
> with the THERMAL_TRIPS_NONE parameter was removed.
> 
> We can get rid of this test as no user of this function is calling
> this function with this parameter.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---

Reviewed-by: Thara Gopinath <thara.gopinath@linaro.org>

>   drivers/thermal/thermal_core.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index a0f0c33c8d9c..bcc2ea4f5482 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -710,7 +710,7 @@ int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,
>   	unsigned long max_state;
>   	int result, ret;
>   
> -	if (trip >= tz->trips || (trip < 0 && trip != THERMAL_TRIPS_NONE))
> +	if (trip >= tz->trips || trip < 0)
>   		return -EINVAL;
>   
>   	list_for_each_entry(pos1, &thermal_tz_list, node) {
> 

-- 
Warm Regards
Thara

Re: [PATCH 6/6] thermal/core: Remove unused macro THERMAL_TRIPS_NONE

[Thara Gopinath]

On 12/14/20 6:38 PM, Daniel Lezcano wrote:
> The macro THERMAL_TRIPS_NONE is no longer used, remove it.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---

Reviewed-by: Thara Gopinath <thara.gopinath@linaro.org>

>   include/linux/thermal.h | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index a57232a9a6f9..060a2160add4 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -17,7 +17,6 @@
>   #include <linux/workqueue.h>
>   #include <uapi/linux/thermal.h>
>   
> -#define THERMAL_TRIPS_NONE	-1
>   #define THERMAL_MAX_TRIPS	12
>   
>   /* invalid cooling state */
> 

-- 
Warm Regards
Thara