All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/6] thermal/core: Remove the 'forced_passive' option
@ 2020-12-14 23:38 Daniel Lezcano
  2020-12-14 23:38 ` [PATCH 2/6] thermal/core: Remove unused functions rebind/unbind exception Daniel Lezcano
                   ` (7 more replies)
  0 siblings, 8 replies; 21+ messages in thread
From: Daniel Lezcano @ 2020-12-14 23:38 UTC (permalink / raw)
  To: daniel.lezcano, rui.zhang
  Cc: mjg59, linux-pm, linux-kernel, amitk, thara.gopinath,
	Jonathan Corbet, Viresh Kumar, open list:DOCUMENTATION

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


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

* [PATCH 2/6] thermal/core: Remove unused functions rebind/unbind exception
  2020-12-14 23:38 [PATCH 1/6] thermal/core: Remove the 'forced_passive' option Daniel Lezcano
@ 2020-12-14 23:38 ` Daniel Lezcano
  2021-01-11 23:15   ` Thara Gopinath
  2021-01-19 21:27   ` [thermal: thermal/next] " thermal-bot for Daniel Lezcano
  2020-12-14 23:38 ` [PATCH 3/6] thermal/core: Remove pointless test with the THERMAL_TRIPS_NONE macro Daniel Lezcano
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 21+ messages in thread
From: Daniel Lezcano @ 2020-12-14 23:38 UTC (permalink / raw)
  To: daniel.lezcano, rui.zhang
  Cc: mjg59, linux-pm, linux-kernel, amitk, thara.gopinath

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


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

* [PATCH 3/6] thermal/core: Remove pointless test with the THERMAL_TRIPS_NONE macro
  2020-12-14 23:38 [PATCH 1/6] thermal/core: Remove the 'forced_passive' option Daniel Lezcano
  2020-12-14 23:38 ` [PATCH 2/6] thermal/core: Remove unused functions rebind/unbind exception Daniel Lezcano
@ 2020-12-14 23:38 ` Daniel Lezcano
  2021-01-11 23:17   ` Thara Gopinath
  2021-01-19 21:27   ` [thermal: thermal/next] " thermal-bot for Daniel Lezcano
  2020-12-14 23:38 ` [PATCH 4/6] acpi/drivers/thermal: Remove TRIPS_NONE cooling device binding Daniel Lezcano
                   ` (5 subsequent siblings)
  7 siblings, 2 replies; 21+ messages in thread
From: Daniel Lezcano @ 2020-12-14 23:38 UTC (permalink / raw)
  To: daniel.lezcano, rui.zhang
  Cc: mjg59, linux-pm, linux-kernel, amitk, thara.gopinath

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


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

* [PATCH 4/6] acpi/drivers/thermal: Remove TRIPS_NONE cooling device binding
  2020-12-14 23:38 [PATCH 1/6] thermal/core: Remove the 'forced_passive' option Daniel Lezcano
  2020-12-14 23:38 ` [PATCH 2/6] thermal/core: Remove unused functions rebind/unbind exception Daniel Lezcano
  2020-12-14 23:38 ` [PATCH 3/6] thermal/core: Remove pointless test with the THERMAL_TRIPS_NONE macro Daniel Lezcano
@ 2020-12-14 23:38 ` Daniel Lezcano
  2021-01-05 15:44   ` Daniel Lezcano
  2021-01-19 21:27   ` [thermal: thermal/next] " thermal-bot for Daniel Lezcano
  2020-12-14 23:38 ` [PATCH 5/6] thermal/core: Remove THERMAL_TRIPS_NONE test Daniel Lezcano
                   ` (4 subsequent siblings)
  7 siblings, 2 replies; 21+ messages in thread
From: Daniel Lezcano @ 2020-12-14 23:38 UTC (permalink / raw)
  To: daniel.lezcano, rui.zhang
  Cc: mjg59, linux-pm, linux-kernel, amitk, thara.gopinath,
	Rafael J. Wysocki, Len Brown, open list:ACPI THERMAL DRIVER

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


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

* [PATCH 5/6] thermal/core: Remove THERMAL_TRIPS_NONE test
  2020-12-14 23:38 [PATCH 1/6] thermal/core: Remove the 'forced_passive' option Daniel Lezcano
                   ` (2 preceding siblings ...)
  2020-12-14 23:38 ` [PATCH 4/6] acpi/drivers/thermal: Remove TRIPS_NONE cooling device binding Daniel Lezcano
@ 2020-12-14 23:38 ` Daniel Lezcano
  2021-01-11 23:18   ` Thara Gopinath
  2021-01-19 21:27   ` [thermal: thermal/next] " thermal-bot for Daniel Lezcano
  2020-12-14 23:38 ` [PATCH 6/6] thermal/core: Remove unused macro THERMAL_TRIPS_NONE Daniel Lezcano
                   ` (3 subsequent siblings)
  7 siblings, 2 replies; 21+ messages in thread
From: Daniel Lezcano @ 2020-12-14 23:38 UTC (permalink / raw)
  To: daniel.lezcano, rui.zhang
  Cc: mjg59, linux-pm, linux-kernel, amitk, thara.gopinath

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


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

* [PATCH 6/6] thermal/core: Remove unused macro THERMAL_TRIPS_NONE
  2020-12-14 23:38 [PATCH 1/6] thermal/core: Remove the 'forced_passive' option Daniel Lezcano
                   ` (3 preceding siblings ...)
  2020-12-14 23:38 ` [PATCH 5/6] thermal/core: Remove THERMAL_TRIPS_NONE test Daniel Lezcano
@ 2020-12-14 23:38 ` Daniel Lezcano
  2021-01-11 23:18   ` Thara Gopinath
  2021-01-19 21:27   ` [thermal: thermal/next] " thermal-bot for Daniel Lezcano
  2020-12-22 17:05 ` [PATCH 1/6] thermal/core: Remove the 'forced_passive' option Daniel Lezcano
                   ` (2 subsequent siblings)
  7 siblings, 2 replies; 21+ messages in thread
From: Daniel Lezcano @ 2020-12-14 23:38 UTC (permalink / raw)
  To: daniel.lezcano, rui.zhang
  Cc: mjg59, linux-pm, linux-kernel, amitk, thara.gopinath

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


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

* Re: [PATCH 1/6] thermal/core: Remove the 'forced_passive' option
  2020-12-14 23:38 [PATCH 1/6] thermal/core: Remove the 'forced_passive' option Daniel Lezcano
                   ` (4 preceding siblings ...)
  2020-12-14 23:38 ` [PATCH 6/6] thermal/core: Remove unused macro THERMAL_TRIPS_NONE Daniel Lezcano
@ 2020-12-22 17:05 ` Daniel Lezcano
  2021-01-11 23:13 ` Thara Gopinath
  2021-01-19 21:27 ` [thermal: thermal/next] " thermal-bot for Daniel Lezcano
  7 siblings, 0 replies; 21+ messages in thread
From: Daniel Lezcano @ 2020-12-22 17:05 UTC (permalink / raw)
  To: rui.zhang
  Cc: mjg59, linux-pm, linux-kernel, amitk, thara.gopinath,
	Jonathan Corbet, Viresh Kumar, open list:DOCUMENTATION

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

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

* Re: [PATCH 4/6] acpi/drivers/thermal: Remove TRIPS_NONE cooling device binding
  2020-12-14 23:38 ` [PATCH 4/6] acpi/drivers/thermal: Remove TRIPS_NONE cooling device binding Daniel Lezcano
@ 2021-01-05 15:44   ` Daniel Lezcano
  2021-01-07  5:10     ` Zhang, Rui
  2021-01-19 21:27   ` [thermal: thermal/next] " thermal-bot for Daniel Lezcano
  1 sibling, 1 reply; 21+ messages in thread
From: Daniel Lezcano @ 2021-01-05 15:44 UTC (permalink / raw)
  To: rui.zhang
  Cc: mjg59, linux-pm, linux-kernel, amitk, thara.gopinath,
	Rafael J. Wysocki, Len Brown, open list:ACPI THERMAL DRIVER

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

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

* RE: [PATCH 4/6] acpi/drivers/thermal: Remove TRIPS_NONE cooling device binding
  2021-01-05 15:44   ` Daniel Lezcano
@ 2021-01-07  5:10     ` Zhang, Rui
  2021-01-07 16:46       ` Daniel Lezcano
  0 siblings, 1 reply; 21+ messages in thread
From: Zhang, Rui @ 2021-01-07  5:10 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: mjg59, linux-pm, linux-kernel, amitk, thara.gopinath,
	Rafael J. Wysocki, Len Brown, open list:ACPI THERMAL DRIVER

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

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

* Re: [PATCH 4/6] acpi/drivers/thermal: Remove TRIPS_NONE cooling device binding
  2021-01-07  5:10     ` Zhang, Rui
@ 2021-01-07 16:46       ` Daniel Lezcano
  0 siblings, 0 replies; 21+ messages in thread
From: Daniel Lezcano @ 2021-01-07 16:46 UTC (permalink / raw)
  To: Zhang, Rui
  Cc: mjg59, linux-pm, linux-kernel, amitk, thara.gopinath,
	Rafael J. Wysocki, Len Brown, open list:ACPI THERMAL DRIVER

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

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

* Re: [PATCH 1/6] thermal/core: Remove the 'forced_passive' option
  2020-12-14 23:38 [PATCH 1/6] thermal/core: Remove the 'forced_passive' option Daniel Lezcano
                   ` (5 preceding siblings ...)
  2020-12-22 17:05 ` [PATCH 1/6] thermal/core: Remove the 'forced_passive' option Daniel Lezcano
@ 2021-01-11 23:13 ` Thara Gopinath
  2021-01-19 21:27 ` [thermal: thermal/next] " thermal-bot for Daniel Lezcano
  7 siblings, 0 replies; 21+ messages in thread
From: Thara Gopinath @ 2021-01-11 23:13 UTC (permalink / raw)
  To: Daniel Lezcano, rui.zhang
  Cc: mjg59, linux-pm, linux-kernel, amitk, Jonathan Corbet,
	Viresh Kumar, open list:DOCUMENTATION



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

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

* Re: [PATCH 2/6] thermal/core: Remove unused functions rebind/unbind exception
  2020-12-14 23:38 ` [PATCH 2/6] thermal/core: Remove unused functions rebind/unbind exception Daniel Lezcano
@ 2021-01-11 23:15   ` Thara Gopinath
  2021-01-19 21:27   ` [thermal: thermal/next] " thermal-bot for Daniel Lezcano
  1 sibling, 0 replies; 21+ messages in thread
From: Thara Gopinath @ 2021-01-11 23:15 UTC (permalink / raw)
  To: Daniel Lezcano, rui.zhang; +Cc: mjg59, linux-pm, linux-kernel, amitk



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

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

* Re: [PATCH 3/6] thermal/core: Remove pointless test with the THERMAL_TRIPS_NONE macro
  2020-12-14 23:38 ` [PATCH 3/6] thermal/core: Remove pointless test with the THERMAL_TRIPS_NONE macro Daniel Lezcano
@ 2021-01-11 23:17   ` Thara Gopinath
  2021-01-19 21:27   ` [thermal: thermal/next] " thermal-bot for Daniel Lezcano
  1 sibling, 0 replies; 21+ messages in thread
From: Thara Gopinath @ 2021-01-11 23:17 UTC (permalink / raw)
  To: Daniel Lezcano, rui.zhang; +Cc: mjg59, linux-pm, linux-kernel, amitk



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

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

* Re: [PATCH 5/6] thermal/core: Remove THERMAL_TRIPS_NONE test
  2020-12-14 23:38 ` [PATCH 5/6] thermal/core: Remove THERMAL_TRIPS_NONE test Daniel Lezcano
@ 2021-01-11 23:18   ` Thara Gopinath
  2021-01-19 21:27   ` [thermal: thermal/next] " thermal-bot for Daniel Lezcano
  1 sibling, 0 replies; 21+ messages in thread
From: Thara Gopinath @ 2021-01-11 23:18 UTC (permalink / raw)
  To: Daniel Lezcano, rui.zhang; +Cc: mjg59, linux-pm, linux-kernel, amitk



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

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

* Re: [PATCH 6/6] thermal/core: Remove unused macro THERMAL_TRIPS_NONE
  2020-12-14 23:38 ` [PATCH 6/6] thermal/core: Remove unused macro THERMAL_TRIPS_NONE Daniel Lezcano
@ 2021-01-11 23:18   ` Thara Gopinath
  2021-01-19 21:27   ` [thermal: thermal/next] " thermal-bot for Daniel Lezcano
  1 sibling, 0 replies; 21+ messages in thread
From: Thara Gopinath @ 2021-01-11 23:18 UTC (permalink / raw)
  To: Daniel Lezcano, rui.zhang; +Cc: mjg59, linux-pm, linux-kernel, amitk



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

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

* [thermal: thermal/next] thermal/core: Remove unused macro THERMAL_TRIPS_NONE
  2020-12-14 23:38 ` [PATCH 6/6] thermal/core: Remove unused macro THERMAL_TRIPS_NONE Daniel Lezcano
  2021-01-11 23:18   ` Thara Gopinath
@ 2021-01-19 21:27   ` thermal-bot for Daniel Lezcano
  1 sibling, 0 replies; 21+ messages in thread
From: thermal-bot for Daniel Lezcano @ 2021-01-19 21:27 UTC (permalink / raw)
  To: linux-pm; +Cc: Daniel Lezcano, Thara Gopinath, rui.zhang, amitk

The following commit has been merged into the thermal/next branch of thermal:

Commit-ID:     2121496fdc5f2d93eda9743c4b487469f0042e3c
Gitweb:        https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git//2121496fdc5f2d93eda9743c4b487469f0042e3c
Author:        Daniel Lezcano <daniel.lezcano@linaro.org>
AuthorDate:    Tue, 15 Dec 2020 00:38:09 +01:00
Committer:     Daniel Lezcano <daniel.lezcano@linaro.org>
CommitterDate: Tue, 19 Jan 2021 22:23:32 +01:00

thermal/core: Remove unused macro THERMAL_TRIPS_NONE

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>
Link: https://lore.kernel.org/r/20201214233811.485669-6-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 a57232a..060a216 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 */

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

* [thermal: thermal/next] thermal/core: Remove unused functions rebind/unbind exception
  2020-12-14 23:38 ` [PATCH 2/6] thermal/core: Remove unused functions rebind/unbind exception Daniel Lezcano
  2021-01-11 23:15   ` Thara Gopinath
@ 2021-01-19 21:27   ` thermal-bot for Daniel Lezcano
  1 sibling, 0 replies; 21+ messages in thread
From: thermal-bot for Daniel Lezcano @ 2021-01-19 21:27 UTC (permalink / raw)
  To: linux-pm; +Cc: Daniel Lezcano, Thara Gopinath, rui.zhang, amitk

The following commit has been merged into the thermal/next branch of thermal:

Commit-ID:     a20b995b23e41190fb088e7dab4a2b956bf343ae
Gitweb:        https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git//a20b995b23e41190fb088e7dab4a2b956bf343ae
Author:        Daniel Lezcano <daniel.lezcano@linaro.org>
AuthorDate:    Tue, 15 Dec 2020 00:38:05 +01:00
Committer:     Daniel Lezcano <daniel.lezcano@linaro.org>
CommitterDate: Tue, 19 Jan 2021 22:23:04 +01:00

thermal/core: Remove unused functions rebind/unbind exception

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>
Link: https://lore.kernel.org/r/20201214233811.485669-2-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 567bc6f..a0f0c33 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 8df600f..e50c6b2 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);
 

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

* [thermal: thermal/next] acpi/drivers/thermal: Remove TRIPS_NONE cooling device binding
  2020-12-14 23:38 ` [PATCH 4/6] acpi/drivers/thermal: Remove TRIPS_NONE cooling device binding Daniel Lezcano
  2021-01-05 15:44   ` Daniel Lezcano
@ 2021-01-19 21:27   ` thermal-bot for Daniel Lezcano
  1 sibling, 0 replies; 21+ messages in thread
From: thermal-bot for Daniel Lezcano @ 2021-01-19 21:27 UTC (permalink / raw)
  To: linux-pm; +Cc: Daniel Lezcano, Zhang Rui, Thara Gopinath, amitk

The following commit has been merged into the thermal/next branch of thermal:

Commit-ID:     43bb4a9d658398151ad1b77340003ff81f4b8650
Gitweb:        https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git//43bb4a9d658398151ad1b77340003ff81f4b8650
Author:        Daniel Lezcano <daniel.lezcano@linaro.org>
AuthorDate:    Tue, 15 Dec 2020 00:38:07 +01:00
Committer:     Daniel Lezcano <daniel.lezcano@linaro.org>
CommitterDate: Tue, 19 Jan 2021 22:23:18 +01:00

acpi/drivers/thermal: Remove TRIPS_NONE cooling device binding

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>
Reviewed-by: Thara Gopinath <thara.gopinath@linaro.org>
Link: https://lore.kernel.org/r/20201214233811.485669-4-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 b5e4bc9..26a89ff 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;
 }

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

* [thermal: thermal/next] thermal/core: Remove pointless test with the THERMAL_TRIPS_NONE macro
  2020-12-14 23:38 ` [PATCH 3/6] thermal/core: Remove pointless test with the THERMAL_TRIPS_NONE macro Daniel Lezcano
  2021-01-11 23:17   ` Thara Gopinath
@ 2021-01-19 21:27   ` thermal-bot for Daniel Lezcano
  1 sibling, 0 replies; 21+ messages in thread
From: thermal-bot for Daniel Lezcano @ 2021-01-19 21:27 UTC (permalink / raw)
  To: linux-pm; +Cc: Daniel Lezcano, Thara Gopinath, rui.zhang, amitk

The following commit has been merged into the thermal/next branch of thermal:

Commit-ID:     53f04ca8153cc043cd2fa968ed451a85e8f70bd8
Gitweb:        https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git//53f04ca8153cc043cd2fa968ed451a85e8f70bd8
Author:        Daniel Lezcano <daniel.lezcano@linaro.org>
AuthorDate:    Tue, 15 Dec 2020 00:38:06 +01:00
Committer:     Daniel Lezcano <daniel.lezcano@linaro.org>
CommitterDate: Tue, 19 Jan 2021 22:23:11 +01:00

thermal/core: Remove pointless test with the THERMAL_TRIPS_NONE macro

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>
Reviewed-by: Thara Gopinath <thara.gopinath@linaro.org>
Link: https://lore.kernel.org/r/20201214233811.485669-3-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 4e7f9e8..345917a 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

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

* [thermal: thermal/next] thermal/core: Remove THERMAL_TRIPS_NONE test
  2020-12-14 23:38 ` [PATCH 5/6] thermal/core: Remove THERMAL_TRIPS_NONE test Daniel Lezcano
  2021-01-11 23:18   ` Thara Gopinath
@ 2021-01-19 21:27   ` thermal-bot for Daniel Lezcano
  1 sibling, 0 replies; 21+ messages in thread
From: thermal-bot for Daniel Lezcano @ 2021-01-19 21:27 UTC (permalink / raw)
  To: linux-pm; +Cc: Daniel Lezcano, Thara Gopinath, rui.zhang, amitk

The following commit has been merged into the thermal/next branch of thermal:

Commit-ID:     716072d065b62f5a63d81bee978fd234dc47a83b
Gitweb:        https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git//716072d065b62f5a63d81bee978fd234dc47a83b
Author:        Daniel Lezcano <daniel.lezcano@linaro.org>
AuthorDate:    Tue, 15 Dec 2020 00:38:08 +01:00
Committer:     Daniel Lezcano <daniel.lezcano@linaro.org>
CommitterDate: Tue, 19 Jan 2021 22:23:25 +01:00

thermal/core: Remove THERMAL_TRIPS_NONE test

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>
Link: https://lore.kernel.org/r/20201214233811.485669-5-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 a0f0c33..bcc2ea4 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) {

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

* [thermal: thermal/next] thermal/core: Remove the 'forced_passive' option
  2020-12-14 23:38 [PATCH 1/6] thermal/core: Remove the 'forced_passive' option Daniel Lezcano
                   ` (6 preceding siblings ...)
  2021-01-11 23:13 ` Thara Gopinath
@ 2021-01-19 21:27 ` thermal-bot for Daniel Lezcano
  7 siblings, 0 replies; 21+ messages in thread
From: thermal-bot for Daniel Lezcano @ 2021-01-19 21:27 UTC (permalink / raw)
  To: linux-pm; +Cc: Daniel Lezcano, Thara Gopinath, rui.zhang, amitk

The following commit has been merged into the thermal/next branch of thermal:

Commit-ID:     a7d6ba14efb778fc8c65c627a057d5dd29debd04
Gitweb:        https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git//a7d6ba14efb778fc8c65c627a057d5dd29debd04
Author:        Daniel Lezcano <daniel.lezcano@linaro.org>
AuthorDate:    Tue, 15 Dec 2020 00:38:04 +01:00
Committer:     Daniel Lezcano <daniel.lezcano@linaro.org>
CommitterDate: Tue, 19 Jan 2021 22:22:45 +01:00

thermal/core: Remove the 'forced_passive' option

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>
Link: https://lore.kernel.org/r/20201214233811.485669-1-daniel.lezcano@linaro.org
---
 Documentation/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 e7520cb..a4969c4 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 2ae7198..12acb12 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 0866e94..4e7f9e8 100644
--- a/drivers/thermal/thermal_sysfs.c
+++ b/drivers/thermal/thermal_sysfs.c
@@ -217,49 +217,6 @@ trip_point_hyst_show(struct device *dev, struct device_attribute *attr,
 }
 
 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 c800323..a57232a 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;

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

end of thread, other threads:[~2021-01-19 21:32 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-14 23:38 [PATCH 1/6] thermal/core: Remove the 'forced_passive' option Daniel Lezcano
2020-12-14 23:38 ` [PATCH 2/6] thermal/core: Remove unused functions rebind/unbind exception Daniel Lezcano
2021-01-11 23:15   ` Thara Gopinath
2021-01-19 21:27   ` [thermal: thermal/next] " thermal-bot for Daniel Lezcano
2020-12-14 23:38 ` [PATCH 3/6] thermal/core: Remove pointless test with the THERMAL_TRIPS_NONE macro Daniel Lezcano
2021-01-11 23:17   ` Thara Gopinath
2021-01-19 21:27   ` [thermal: thermal/next] " thermal-bot for Daniel Lezcano
2020-12-14 23:38 ` [PATCH 4/6] acpi/drivers/thermal: Remove TRIPS_NONE cooling device binding Daniel Lezcano
2021-01-05 15:44   ` Daniel Lezcano
2021-01-07  5:10     ` Zhang, Rui
2021-01-07 16:46       ` Daniel Lezcano
2021-01-19 21:27   ` [thermal: thermal/next] " thermal-bot for Daniel Lezcano
2020-12-14 23:38 ` [PATCH 5/6] thermal/core: Remove THERMAL_TRIPS_NONE test Daniel Lezcano
2021-01-11 23:18   ` Thara Gopinath
2021-01-19 21:27   ` [thermal: thermal/next] " thermal-bot for Daniel Lezcano
2020-12-14 23:38 ` [PATCH 6/6] thermal/core: Remove unused macro THERMAL_TRIPS_NONE Daniel Lezcano
2021-01-11 23:18   ` Thara Gopinath
2021-01-19 21:27   ` [thermal: thermal/next] " thermal-bot for Daniel Lezcano
2020-12-22 17:05 ` [PATCH 1/6] thermal/core: Remove the 'forced_passive' option Daniel Lezcano
2021-01-11 23:13 ` Thara Gopinath
2021-01-19 21:27 ` [thermal: thermal/next] " thermal-bot for Daniel Lezcano

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.