[PATCH v1 0/2] thermal: core: Simplifications related to governor handling

[PATCH v1 0/2] thermal: core: Simplifications related to governor handling

[Rafael J. Wysocki]

Hi All,

These two patches simplify the registration and unregistration of thermal
governors.

They are intended for 7.2.

Thanks!

[PATCH v1 1/2] thermal: core: Remove dead code from two functions

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Since thermal_register_governor() is only called by
thermal_register_governors() which runs before clearing
thermal_class_unavailable, thermal_tz_list is guaranteed to be
empty when it runs, so the code walking that list in it is
effectively dead.

The same observation applies to the code walking thermal_tz_list
in thermal_unregister_governor() which is also called only if
thermal_class_unavailable hasn't been cleared.

Remove the dead code from both functions mentioned above and rearrange
thermal_register_governor() to reduce indentation level.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/thermal_core.c |   61 +++++++----------------------------------
 1 file changed, 11 insertions(+), 50 deletions(-)

--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -119,59 +119,28 @@ static int thermal_set_governor(struct t
 
 int thermal_register_governor(struct thermal_governor *governor)
 {
-	int err;
-	const char *name;
-	struct thermal_zone_device *pos;
 
 	if (!governor)
 		return -EINVAL;
 
 	guard(mutex)(&thermal_governor_lock);
 
-	err = -EBUSY;
-	if (!__find_governor(governor->name)) {
-		bool match_default;
-
-		err = 0;
-		list_add(&governor->governor_list, &thermal_governor_list);
-		match_default = !strncmp(governor->name,
-					 DEFAULT_THERMAL_GOVERNOR,
-					 THERMAL_NAME_LENGTH);
-
-		if (!def_governor && match_default)
-			def_governor = governor;
-	}
-
-	guard(mutex)(&thermal_list_lock);
-
-	list_for_each_entry(pos, &thermal_tz_list, node) {
-		/*
-		 * only thermal zones with specified tz->tzp->governor_name
-		 * may run with tz->govenor unset
-		 */
-		if (pos->governor)
-			continue;
-
-		name = pos->tzp->governor_name;
-
-		if (!strncasecmp(name, governor->name, THERMAL_NAME_LENGTH)) {
-			int ret;
-
-			ret = thermal_set_governor(pos, governor);
-			if (ret)
-				dev_err(&pos->device,
-					"Failed to set governor %s for thermal zone %s: %d\n",
-					governor->name, pos->type, ret);
-		}
-	}
+	if (__find_governor(governor->name))
+		return -EBUSY;
 
-	return err;
+	list_add(&governor->governor_list, &thermal_governor_list);
+
+	if (strncmp(governor->name, DEFAULT_THERMAL_GOVERNOR, THERMAL_NAME_LENGTH))
+		return 0;
+
+	if (!def_governor)
+		def_governor = governor;
+
+	return 0;
 }
 
 void thermal_unregister_governor(struct thermal_governor *governor)
 {
-	struct thermal_zone_device *pos;
-
 	if (!governor)
 		return;
 
@@ -181,14 +150,6 @@ void thermal_unregister_governor(struct
 		return;
 
 	list_del(&governor->governor_list);
-
-	guard(mutex)(&thermal_list_lock);
-
-	list_for_each_entry(pos, &thermal_tz_list, node) {
-		if (!strncasecmp(pos->governor->name, governor->name,
-				 THERMAL_NAME_LENGTH))
-			thermal_set_governor(pos, NULL);
-	}
 }
 
 int thermal_zone_device_set_policy(struct thermal_zone_device *tz,

[PATCH v1 2/2] thermal: core: Simplify unregistration of governors

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Thermal governors are only unregistered in the thermal_init() error
path and they actually are only deleted from thermal_governor_list.

Put the entire code needed to do that to thermal_unregister_governors()
and rearrange thermal_init() to call that function also when
thermal_register_governors() returns an error.

This allows thermal_unregister_governor() to be dropped
and thermal_register_governor() that is only called by
thermal_register_governors() can be made static __init, so the
headers of these two functions can be dropped from thermal_core.h.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/thermal_core.c |   50 +++++++++++------------------------------
 drivers/thermal/thermal_core.h |    2 -
 2 files changed, 14 insertions(+), 38 deletions(-)

--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -117,14 +117,12 @@ static int thermal_set_governor(struct t
 	return ret;
 }
 
-int thermal_register_governor(struct thermal_governor *governor)
+static int __init thermal_register_governor(struct thermal_governor *governor)
 {
 
 	if (!governor)
 		return -EINVAL;
 
-	guard(mutex)(&thermal_governor_lock);
-
 	if (__find_governor(governor->name))
 		return -EBUSY;
 
@@ -139,19 +137,6 @@ int thermal_register_governor(struct the
 	return 0;
 }
 
-void thermal_unregister_governor(struct thermal_governor *governor)
-{
-	if (!governor)
-		return;
-
-	guard(mutex)(&thermal_governor_lock);
-
-	if (!__find_governor(governor->name))
-		return;
-
-	list_del(&governor->governor_list);
-}
-
 int thermal_zone_device_set_policy(struct thermal_zone_device *tz,
 				   char *policy)
 {
@@ -187,40 +172,34 @@ int thermal_build_list_of_policies(char
 
 static void __init thermal_unregister_governors(void)
 {
-	struct thermal_governor **governor;
+	struct thermal_governor *gov, *pos;
+
+	guard(mutex)(&thermal_governor_lock);
 
-	for_each_governor_table(governor)
-		thermal_unregister_governor(*governor);
+	list_for_each_entry_safe(gov, pos, &thermal_governor_list, governor_list)
+		list_del(&gov->governor_list);
 }
 
 static int __init thermal_register_governors(void)
 {
-	int ret = 0;
 	struct thermal_governor **governor;
 
+	guard(mutex)(&thermal_governor_lock);
+
 	for_each_governor_table(governor) {
+		int ret;
+
 		ret = thermal_register_governor(*governor);
 		if (ret) {
 			pr_err("Failed to register governor: '%s'",
 			       (*governor)->name);
-			break;
+			return ret;
 		}
 
-		pr_info("Registered thermal governor '%s'",
-			(*governor)->name);
-	}
-
-	if (ret) {
-		struct thermal_governor **gov;
-
-		for_each_governor_table(gov) {
-			if (gov == governor)
-				break;
-			thermal_unregister_governor(*gov);
-		}
+		pr_info("Registered thermal governor '%s'", (*governor)->name);
 	}
 
-	return ret;
+	return 0;
 }
 
 static int __thermal_zone_device_set_mode(struct thermal_zone_device *tz,
@@ -1872,7 +1851,7 @@ static int __init thermal_init(void)
 
 	result = thermal_register_governors();
 	if (result)
-		goto destroy_workqueue;
+		goto unregister_governors;
 
 	result = class_register(&thermal_class);
 	if (result)
@@ -1884,7 +1863,6 @@ static int __init thermal_init(void)
 
 unregister_governors:
 	thermal_unregister_governors();
-destroy_workqueue:
 	destroy_workqueue(thermal_wq);
 unregister_netlink:
 	thermal_netlink_exit();
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -258,8 +258,6 @@ struct thermal_instance {
 #define to_cooling_device(_dev)	\
 	container_of(_dev, struct thermal_cooling_device, device)
 
-int thermal_register_governor(struct thermal_governor *);
-void thermal_unregister_governor(struct thermal_governor *);
 int thermal_zone_device_set_policy(struct thermal_zone_device *, char *);
 int thermal_build_list_of_policies(char *buf);
 void __thermal_zone_device_update(struct thermal_zone_device *tz,