[PATCH v2 00/12] thermal: core: Fixes and cleanups, mostly related to thermal zone init and exit

[PATCH v2 00/12] thermal: core: Fixes and cleanups, mostly related to thermal zone init and exit

[Rafael J. Wysocki]

Hi Everyone,

After posting the two series of thermal core patches for 6.13:

https://lore.kernel.org/linux-pm/4920970.GXAFRqVoOG@rjwysocki.net/

and

https://lore.kernel.org/linux-pm/6100907.lOV4Wx5bFT@rjwysocki.net/

before the 6.12 merge window, I have decided to reorder the changes included in
these series, so that fixes and more significant cleanups (for example, changing
they layout of data structures) go first, followed by the changes related to
using guards for locking, and the optimization involving sorted lists becomes
the last piece.

This series is the first part and the majority of patches in it come from the
second (RFC) series mentioned above.  Of course, they needed to be rebased to
be applied in the new order.  It is on top of 6.12-rc1 with

https://lore.kernel.org/linux-pm/12549318.O9o76ZdvQC@rjwysocki.net/

applied and it will be added to my thermal-core-testing branch.  It is in v2
to start with because all of the patches in it have already been posted in
some form.

The first 10 patches fix some potential issues related to thermal zone
initialization and exit (for example, user space may start to interact with
a thermal zone during its initialization before it's ready and system suspend
taking place at a wrong time may skip a new thermal zone so it is not suspended)
and do some cleanups related to that.  This concludes with the removal of the
need_update field from struct thermal_zone_device.

The last two patches move lists of thermal instances from thermal zones to
trip point descriptors and clean up some code on top of that.

Please refer to the individual patch changelogs for details.

Thanks!

[PATCH v2 01/12] thermal: core: Initialize thermal zones before registering them

[Rafael J. Wysocki]

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

Since user space can start interacting with a new thermal zone as soon
as device_register() called by thermal_zone_device_register_with_trips()
returns, it is better to initialize the thermal zone before calling
device_register() on it.

Fixes: d0df264fbd3c ("thermal/core: Remove pointless thermal_zone_device_reset() function")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

This is a new iteration of

https://lore.kernel.org/linux-pm/10527854.nUPlyArG6x@rjwysocki.net/

v1 -> v2: Fix typo in the subject

---
 drivers/thermal/thermal_core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -1465,6 +1465,7 @@ thermal_zone_device_register_with_trips(
 		thermal_zone_destroy_device_groups(tz);
 		goto remove_id;
 	}
+	thermal_zone_device_init(tz);
 	result = device_register(&tz->device);
 	if (result)
 		goto release_device;
@@ -1503,7 +1504,6 @@ thermal_zone_device_register_with_trips(
 
 	mutex_unlock(&thermal_list_lock);
 
-	thermal_zone_device_init(tz);
 	/* Update the new thermal zone and mark it as already updated. */
 	if (atomic_cmpxchg(&tz->need_update, 1, 0))
 		thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);

Re: [PATCH v2 01/12] thermal: core: Initialize thermal zones before registering them

[Lukasz Luba]

On 10/4/24 20:05, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Since user space can start interacting with a new thermal zone as soon
> as device_register() called by thermal_zone_device_register_with_trips()
> returns, it is better to initialize the thermal zone before calling
> device_register() on it.
> 
> Fixes: d0df264fbd3c ("thermal/core: Remove pointless thermal_zone_device_reset() function")
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> This is a new iteration of
> 
> https://lore.kernel.org/linux-pm/10527854.nUPlyArG6x@rjwysocki.net/
> 
> v1 -> v2: Fix typo in the subject
> 
> ---
>   drivers/thermal/thermal_core.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -1465,6 +1465,7 @@ thermal_zone_device_register_with_trips(
>   		thermal_zone_destroy_device_groups(tz);
>   		goto remove_id;
>   	}
> +	thermal_zone_device_init(tz);
>   	result = device_register(&tz->device);
>   	if (result)
>   		goto release_device;
> @@ -1503,7 +1504,6 @@ thermal_zone_device_register_with_trips(
>   
>   	mutex_unlock(&thermal_list_lock);
>   
> -	thermal_zone_device_init(tz);
>   	/* Update the new thermal zone and mark it as already updated. */
>   	if (atomic_cmpxchg(&tz->need_update, 1, 0))
>   		thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
> 
> 
> 

Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

[PATCH v2 02/12] thermal: core: Rearrange PM notification code

[Rafael J. Wysocki]

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

Move the code run for each thermal zone by the thermal PM notify
handler to separate functions.

This will help to make some subsequent changes look somewhat more
straightforward, among other things.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

This is a new iteration of

https://lore.kernel.org/linux-pm/4940614.GXAFRqVoOG@rjwysocki.net/

v1 -> v2: The thermal zone guard has not been defined yet, so use lock/unlock
          directly on the thermal zone lock and update the changelog accordingly.

---
 drivers/thermal/thermal_core.c |   88 +++++++++++++++++++++--------------------
 1 file changed, 46 insertions(+), 42 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -1675,6 +1675,48 @@ static void thermal_zone_device_resume(s
 	mutex_unlock(&tz->lock);
 }
 
+static void thermal_zone_pm_prepare(struct thermal_zone_device *tz)
+{
+	mutex_lock(&tz->lock);
+
+	if (tz->resuming) {
+		/*
+		 * thermal_zone_device_resume() queued up for this zone has not
+		 * acquired the lock yet, so release it to let the function run
+		 * and wait util it has done the work.
+		 */
+		mutex_unlock(&tz->lock);
+
+		wait_for_completion(&tz->resume);
+
+		mutex_lock(&tz->lock);
+	}
+
+	tz->suspended = true;
+
+	mutex_unlock(&tz->lock);
+}
+
+static void thermal_zone_pm_complete(struct thermal_zone_device *tz)
+{
+	mutex_lock(&tz->lock);
+
+	cancel_delayed_work(&tz->poll_queue);
+
+	reinit_completion(&tz->resume);
+	tz->resuming = true;
+
+	/*
+	 * Replace the work function with the resume one, which will restore the
+	 * original work function and schedule the polling work if needed.
+	 */
+	INIT_DELAYED_WORK(&tz->poll_queue, thermal_zone_device_resume);
+	/* Queue up the work without a delay. */
+	mod_delayed_work(system_freezable_power_efficient_wq, &tz->poll_queue, 0);
+
+	mutex_unlock(&tz->lock);
+}
+
 static int thermal_pm_notify(struct notifier_block *nb,
 			     unsigned long mode, void *_unused)
 {
@@ -1686,27 +1728,8 @@ static int thermal_pm_notify(struct noti
 	case PM_SUSPEND_PREPARE:
 		mutex_lock(&thermal_list_lock);
 
-		list_for_each_entry(tz, &thermal_tz_list, node) {
-			mutex_lock(&tz->lock);
-
-			if (tz->resuming) {
-				/*
-				 * thermal_zone_device_resume() queued up for
-				 * this zone has not acquired the lock yet, so
-				 * release it to let the function run and wait
-				 * util it has done the work.
-				 */
-				mutex_unlock(&tz->lock);
-
-				wait_for_completion(&tz->resume);
-
-				mutex_lock(&tz->lock);
-			}
-
-			tz->suspended = true;
-
-			mutex_unlock(&tz->lock);
-		}
+		list_for_each_entry(tz, &thermal_tz_list, node)
+			thermal_zone_pm_prepare(tz);
 
 		mutex_unlock(&thermal_list_lock);
 		break;
@@ -1715,27 +1738,8 @@ static int thermal_pm_notify(struct noti
 	case PM_POST_SUSPEND:
 		mutex_lock(&thermal_list_lock);
 
-		list_for_each_entry(tz, &thermal_tz_list, node) {
-			mutex_lock(&tz->lock);
-
-			cancel_delayed_work(&tz->poll_queue);
-
-			reinit_completion(&tz->resume);
-			tz->resuming = true;
-
-			/*
-			 * Replace the work function with the resume one, which
-			 * will restore the original work function and schedule
-			 * the polling work if needed.
-			 */
-			INIT_DELAYED_WORK(&tz->poll_queue,
-					  thermal_zone_device_resume);
-			/* Queue up the work without a delay. */
-			mod_delayed_work(system_freezable_power_efficient_wq,
-					 &tz->poll_queue, 0);
-
-			mutex_unlock(&tz->lock);
-		}
+		list_for_each_entry(tz, &thermal_tz_list, node)
+			thermal_zone_pm_complete(tz);
 
 		mutex_unlock(&thermal_list_lock);
 		break;

Re: [PATCH v2 02/12] thermal: core: Rearrange PM notification code

[Lukasz Luba]

On 10/4/24 20:09, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Move the code run for each thermal zone by the thermal PM notify
> handler to separate functions.
> 
> This will help to make some subsequent changes look somewhat more
> straightforward, among other things.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> This is a new iteration of
> 
> https://lore.kernel.org/linux-pm/4940614.GXAFRqVoOG@rjwysocki.net/
> 
> v1 -> v2: The thermal zone guard has not been defined yet, so use lock/unlock
>            directly on the thermal zone lock and update the changelog accordingly.
> 
> ---
>   drivers/thermal/thermal_core.c |   88 +++++++++++++++++++++--------------------
>   1 file changed, 46 insertions(+), 42 deletions(-)
> 
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -1675,6 +1675,48 @@ static void thermal_zone_device_resume(s
>   	mutex_unlock(&tz->lock);
>   }
>   
> +static void thermal_zone_pm_prepare(struct thermal_zone_device *tz)
> +{
> +	mutex_lock(&tz->lock);
> +
> +	if (tz->resuming) {
> +		/*
> +		 * thermal_zone_device_resume() queued up for this zone has not
> +		 * acquired the lock yet, so release it to let the function run
> +		 * and wait util it has done the work.
> +		 */
> +		mutex_unlock(&tz->lock);
> +
> +		wait_for_completion(&tz->resume);
> +
> +		mutex_lock(&tz->lock);
> +	}
> +
> +	tz->suspended = true;
> +
> +	mutex_unlock(&tz->lock);
> +}
> +
> +static void thermal_zone_pm_complete(struct thermal_zone_device *tz)
> +{
> +	mutex_lock(&tz->lock);
> +
> +	cancel_delayed_work(&tz->poll_queue);
> +
> +	reinit_completion(&tz->resume);
> +	tz->resuming = true;
> +
> +	/*
> +	 * Replace the work function with the resume one, which will restore the
> +	 * original work function and schedule the polling work if needed.
> +	 */
> +	INIT_DELAYED_WORK(&tz->poll_queue, thermal_zone_device_resume);
> +	/* Queue up the work without a delay. */
> +	mod_delayed_work(system_freezable_power_efficient_wq, &tz->poll_queue, 0);
> +
> +	mutex_unlock(&tz->lock);
> +}
> +
>   static int thermal_pm_notify(struct notifier_block *nb,
>   			     unsigned long mode, void *_unused)
>   {
> @@ -1686,27 +1728,8 @@ static int thermal_pm_notify(struct noti
>   	case PM_SUSPEND_PREPARE:
>   		mutex_lock(&thermal_list_lock);
>   
> -		list_for_each_entry(tz, &thermal_tz_list, node) {
> -			mutex_lock(&tz->lock);
> -
> -			if (tz->resuming) {
> -				/*
> -				 * thermal_zone_device_resume() queued up for
> -				 * this zone has not acquired the lock yet, so
> -				 * release it to let the function run and wait
> -				 * util it has done the work.
> -				 */
> -				mutex_unlock(&tz->lock);
> -
> -				wait_for_completion(&tz->resume);
> -
> -				mutex_lock(&tz->lock);
> -			}
> -
> -			tz->suspended = true;
> -
> -			mutex_unlock(&tz->lock);
> -		}
> +		list_for_each_entry(tz, &thermal_tz_list, node)
> +			thermal_zone_pm_prepare(tz);
>   
>   		mutex_unlock(&thermal_list_lock);
>   		break;
> @@ -1715,27 +1738,8 @@ static int thermal_pm_notify(struct noti
>   	case PM_POST_SUSPEND:
>   		mutex_lock(&thermal_list_lock);
>   
> -		list_for_each_entry(tz, &thermal_tz_list, node) {
> -			mutex_lock(&tz->lock);
> -
> -			cancel_delayed_work(&tz->poll_queue);
> -
> -			reinit_completion(&tz->resume);
> -			tz->resuming = true;
> -
> -			/*
> -			 * Replace the work function with the resume one, which
> -			 * will restore the original work function and schedule
> -			 * the polling work if needed.
> -			 */
> -			INIT_DELAYED_WORK(&tz->poll_queue,
> -					  thermal_zone_device_resume);
> -			/* Queue up the work without a delay. */
> -			mod_delayed_work(system_freezable_power_efficient_wq,
> -					 &tz->poll_queue, 0);
> -
> -			mutex_unlock(&tz->lock);
> -		}
> +		list_for_each_entry(tz, &thermal_tz_list, node)
> +			thermal_zone_pm_complete(tz);
>   
>   		mutex_unlock(&thermal_list_lock);
>   		break;
> 
> 
> 

Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

[PATCH v2 03/12] thermal: core: Represent suspend-related thermal zone flags as bits

[Rafael J. Wysocki]

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

Instead of using two separate fields in struct thermal_zone_device for
representing flags related to thermal zone suspend, represent them
explicitly as bits in one u8 "state" field.

Subsequently, that field will be used for addressing race conditions
related to thermal zone initialization and exit.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

This is a new iteration of

https://lore.kernel.org/linux-pm/2215885.irdbgypaU6@rjwysocki.net/

v1 -> v2: The thermal zone guard has not been introduced yet, so adjust for that.

---
 drivers/thermal/thermal_core.c |   11 +++++------
 drivers/thermal/thermal_core.h |   11 +++++++----
 2 files changed, 12 insertions(+), 10 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -547,7 +547,7 @@ void __thermal_zone_device_update(struct
 	int low = -INT_MAX, high = INT_MAX;
 	int temp, ret;
 
-	if (tz->suspended || tz->mode != THERMAL_DEVICE_ENABLED)
+	if (tz->state != TZ_STATE_READY || tz->mode != THERMAL_DEVICE_ENABLED)
 		return;
 
 	ret = __thermal_zone_get_temp(tz, &temp);
@@ -1662,7 +1662,7 @@ static void thermal_zone_device_resume(s
 
 	mutex_lock(&tz->lock);
 
-	tz->suspended = false;
+	tz->state &= ~(TZ_STATE_FLAG_SUSPENDED | TZ_STATE_FLAG_RESUMING);
 
 	thermal_debug_tz_resume(tz);
 	thermal_zone_device_init(tz);
@@ -1670,7 +1670,6 @@ static void thermal_zone_device_resume(s
 	__thermal_zone_device_update(tz, THERMAL_TZ_RESUME);
 
 	complete(&tz->resume);
-	tz->resuming = false;
 
 	mutex_unlock(&tz->lock);
 }
@@ -1679,7 +1678,7 @@ static void thermal_zone_pm_prepare(stru
 {
 	mutex_lock(&tz->lock);
 
-	if (tz->resuming) {
+	if (tz->state & TZ_STATE_FLAG_RESUMING) {
 		/*
 		 * thermal_zone_device_resume() queued up for this zone has not
 		 * acquired the lock yet, so release it to let the function run
@@ -1692,7 +1691,7 @@ static void thermal_zone_pm_prepare(stru
 		mutex_lock(&tz->lock);
 	}
 
-	tz->suspended = true;
+	tz->state |= TZ_STATE_FLAG_SUSPENDED;
 
 	mutex_unlock(&tz->lock);
 }
@@ -1704,7 +1703,7 @@ static void thermal_zone_pm_complete(str
 	cancel_delayed_work(&tz->poll_queue);
 
 	reinit_completion(&tz->resume);
-	tz->resuming = true;
+	tz->state |= TZ_STATE_FLAG_RESUMING;
 
 	/*
 	 * Replace the work function with the resume one, which will restore the
Index: linux-pm/drivers/thermal/thermal_core.h
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.h
+++ linux-pm/drivers/thermal/thermal_core.h
@@ -61,6 +61,11 @@ struct thermal_governor {
 	struct list_head	governor_list;
 };
 
+#define	TZ_STATE_FLAG_SUSPENDED	BIT(0)
+#define	TZ_STATE_FLAG_RESUMING	BIT(1)
+
+#define TZ_STATE_READY		0
+
 /**
  * struct thermal_zone_device - structure for a thermal zone
  * @id:		unique id number for each thermal zone
@@ -100,8 +105,7 @@ struct thermal_governor {
  * @node:	node in thermal_tz_list (in thermal_core.c)
  * @poll_queue:	delayed work for polling
  * @notify_event: Last notification event
- * @suspended: thermal zone suspend indicator
- * @resuming:	indicates whether or not thermal zone resume is in progress
+ * @state: 	current state of the thermal zone
  * @trips:	array of struct thermal_trip objects
  */
 struct thermal_zone_device {
@@ -134,8 +138,7 @@ struct thermal_zone_device {
 	struct list_head node;
 	struct delayed_work poll_queue;
 	enum thermal_notify_event notify_event;
-	bool suspended;
-	bool resuming;
+	u8 state;
 #ifdef CONFIG_THERMAL_DEBUGFS
 	struct thermal_debugfs *debugfs;
 #endif

Re: [PATCH v2 03/12] thermal: core: Represent suspend-related thermal zone flags as bits

[Lukasz Luba]

On 10/4/24 20:11, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Instead of using two separate fields in struct thermal_zone_device for
> representing flags related to thermal zone suspend, represent them
> explicitly as bits in one u8 "state" field.
> 
> Subsequently, that field will be used for addressing race conditions
> related to thermal zone initialization and exit.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> This is a new iteration of
> 
> https://lore.kernel.org/linux-pm/2215885.irdbgypaU6@rjwysocki.net/
> 
> v1 -> v2: The thermal zone guard has not been introduced yet, so adjust for that.
> 
> ---
>   drivers/thermal/thermal_core.c |   11 +++++------
>   drivers/thermal/thermal_core.h |   11 +++++++----
>   2 files changed, 12 insertions(+), 10 deletions(-)
> 
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -547,7 +547,7 @@ void __thermal_zone_device_update(struct
>   	int low = -INT_MAX, high = INT_MAX;
>   	int temp, ret;
>   
> -	if (tz->suspended || tz->mode != THERMAL_DEVICE_ENABLED)
> +	if (tz->state != TZ_STATE_READY || tz->mode != THERMAL_DEVICE_ENABLED)
>   		return;
>   
>   	ret = __thermal_zone_get_temp(tz, &temp);
> @@ -1662,7 +1662,7 @@ static void thermal_zone_device_resume(s
>   
>   	mutex_lock(&tz->lock);
>   
> -	tz->suspended = false;
> +	tz->state &= ~(TZ_STATE_FLAG_SUSPENDED | TZ_STATE_FLAG_RESUMING);

I see, we now clean both in on go...

>   
>   	thermal_debug_tz_resume(tz);
>   	thermal_zone_device_init(tz);
> @@ -1670,7 +1670,6 @@ static void thermal_zone_device_resume(s
>   	__thermal_zone_device_update(tz, THERMAL_TZ_RESUME);
>   
>   	complete(&tz->resume);
> -	tz->resuming = false;

so handled as well.

>   
>   	mutex_unlock(&tz->lock);
>   }
> @@ -1679,7 +1678,7 @@ static void thermal_zone_pm_prepare(stru
>   {
>   	mutex_lock(&tz->lock);
>   
> -	if (tz->resuming) {
> +	if (tz->state & TZ_STATE_FLAG_RESUMING) {
>   		/*
>   		 * thermal_zone_device_resume() queued up for this zone has not
>   		 * acquired the lock yet, so release it to let the function run
> @@ -1692,7 +1691,7 @@ static void thermal_zone_pm_prepare(stru
>   		mutex_lock(&tz->lock);
>   	}
>   
> -	tz->suspended = true;
> +	tz->state |= TZ_STATE_FLAG_SUSPENDED;
>   
>   	mutex_unlock(&tz->lock);
>   }
> @@ -1704,7 +1703,7 @@ static void thermal_zone_pm_complete(str
>   	cancel_delayed_work(&tz->poll_queue);
>   
>   	reinit_completion(&tz->resume);
> -	tz->resuming = true;
> +	tz->state |= TZ_STATE_FLAG_RESUMING;
>   
>   	/*
>   	 * Replace the work function with the resume one, which will restore the
> Index: linux-pm/drivers/thermal/thermal_core.h
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.h
> +++ linux-pm/drivers/thermal/thermal_core.h
> @@ -61,6 +61,11 @@ struct thermal_governor {
>   	struct list_head	governor_list;
>   };
>   
> +#define	TZ_STATE_FLAG_SUSPENDED	BIT(0)
> +#define	TZ_STATE_FLAG_RESUMING	BIT(1)
> +
> +#define TZ_STATE_READY		0
> +
>   /**
>    * struct thermal_zone_device - structure for a thermal zone
>    * @id:		unique id number for each thermal zone
> @@ -100,8 +105,7 @@ struct thermal_governor {
>    * @node:	node in thermal_tz_list (in thermal_core.c)
>    * @poll_queue:	delayed work for polling
>    * @notify_event: Last notification event
> - * @suspended: thermal zone suspend indicator
> - * @resuming:	indicates whether or not thermal zone resume is in progress
> + * @state: 	current state of the thermal zone
>    * @trips:	array of struct thermal_trip objects
>    */
>   struct thermal_zone_device {
> @@ -134,8 +138,7 @@ struct thermal_zone_device {
>   	struct list_head node;
>   	struct delayed_work poll_queue;
>   	enum thermal_notify_event notify_event;
> -	bool suspended;
> -	bool resuming;
> +	u8 state;
>   #ifdef CONFIG_THERMAL_DEBUGFS
>   	struct thermal_debugfs *debugfs;
>   #endif
> 
> 
> 

Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

[PATCH v2 04/12] thermal: core: Mark thermal zones as initializing to start with

[Rafael J. Wysocki]

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

After thermal_zone_device_register_with_trips() has called
device_register() and it has registered the new thermal zone device
with the driver core, user space may access its sysfs attributes and,
among other things, it may enable the thermal zone before it is ready.

To address this, introduce a new thermal zone state flag for
initialization and set it before calling device_register() in
thermal_zone_device_register_with_trips().  This causes
__thermal_zone_device_update() to return early until the new flag
is cleared.

To clear it when the thermal zone is ready, introduce a new
function called thermal_zone_init_complete() that will also invoke
__thermal_zone_device_update() after clearing that flag (both under the
thernal zone lock) and make thermal_zone_device_register_with_trips()
call the new function instead of checking need_update and calling
thermal_zone_device_update() when it is set.

After this change, if user space enables the thermal zone prematurely,
__thermal_zone_device_update() will return early for it until
thermal_zone_init_complete() is called.  In turn, if the thermal zone
is not enabled by user space before thermal_zone_init_complete() is
called, the __thermal_zone_device_update() call in it will return early
because the thermal zone has not been enabled yet, but that function
will be invoked again by thermal_zone_device_set_mode() when the thermal
zone is enabled and it will not return early this time.

The checking of need_update is not necessary any more because the
__thermal_zone_device_update() calls potentially triggered by cooling
device binding take place before calling thermal_zone_init_complete(),
so they all will return early, which means that
thermal_zone_init_complete() must call __thermal_zone_device_update()
in case the thermal zone is enabled prematurely by user space.

Fixes: 203d3d4aa482 ("the generic thermal sysfs driver")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

This is a new iteration of

https://lore.kernel.org/linux-pm/2973309.e9J7NaK4W3@rjwysocki.net/

v1 -> v2: Rebase and add a Fixes tag.

---
 drivers/thermal/thermal_core.c |   16 +++++++++++++---
 drivers/thermal/thermal_core.h |    1 +
 2 files changed, 14 insertions(+), 3 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -1332,6 +1332,16 @@ int thermal_zone_get_crit_temp(struct th
 }
 EXPORT_SYMBOL_GPL(thermal_zone_get_crit_temp);
 
+static void thermal_zone_init_complete(struct thermal_zone_device *tz)
+{
+	mutex_lock(&tz->lock);
+
+	tz->state &= ~TZ_STATE_FLAG_INIT;
+	__thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
+
+	mutex_unlock(&tz->lock);
+}
+
 /**
  * thermal_zone_device_register_with_trips() - register a new thermal zone device
  * @type:	the thermal zone device type
@@ -1451,6 +1461,8 @@ thermal_zone_device_register_with_trips(
 	tz->passive_delay_jiffies = msecs_to_jiffies(passive_delay);
 	tz->recheck_delay_jiffies = THERMAL_RECHECK_DELAY;
 
+	tz->state = TZ_STATE_FLAG_INIT;
+
 	/* sys I/F */
 	/* Add nodes that are always present via .groups */
 	result = thermal_zone_create_device_groups(tz);
@@ -1504,9 +1516,7 @@ thermal_zone_device_register_with_trips(
 
 	mutex_unlock(&thermal_list_lock);
 
-	/* Update the new thermal zone and mark it as already updated. */
-	if (atomic_cmpxchg(&tz->need_update, 1, 0))
-		thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
+	thermal_zone_init_complete(tz);
 
 	thermal_notify_tz_create(tz);
 
Index: linux-pm/drivers/thermal/thermal_core.h
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.h
+++ linux-pm/drivers/thermal/thermal_core.h
@@ -63,6 +63,7 @@ struct thermal_governor {
 
 #define	TZ_STATE_FLAG_SUSPENDED	BIT(0)
 #define	TZ_STATE_FLAG_RESUMING	BIT(1)
+#define	TZ_STATE_FLAG_INIT	BIT(2)
 
 #define TZ_STATE_READY		0
 

Re: [PATCH v2 04/12] thermal: core: Mark thermal zones as initializing to start with

[Lukasz Luba]

On 10/4/24 20:15, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> After thermal_zone_device_register_with_trips() has called
> device_register() and it has registered the new thermal zone device
> with the driver core, user space may access its sysfs attributes and,
> among other things, it may enable the thermal zone before it is ready.
> 
> To address this, introduce a new thermal zone state flag for
> initialization and set it before calling device_register() in
> thermal_zone_device_register_with_trips().  This causes
> __thermal_zone_device_update() to return early until the new flag
> is cleared.
> 
> To clear it when the thermal zone is ready, introduce a new
> function called thermal_zone_init_complete() that will also invoke
> __thermal_zone_device_update() after clearing that flag (both under the
> thernal zone lock) and make thermal_zone_device_register_with_trips()
> call the new function instead of checking need_update and calling
> thermal_zone_device_update() when it is set.
> 
> After this change, if user space enables the thermal zone prematurely,
> __thermal_zone_device_update() will return early for it until
> thermal_zone_init_complete() is called.  In turn, if the thermal zone
> is not enabled by user space before thermal_zone_init_complete() is
> called, the __thermal_zone_device_update() call in it will return early
> because the thermal zone has not been enabled yet, but that function
> will be invoked again by thermal_zone_device_set_mode() when the thermal
> zone is enabled and it will not return early this time.
> 
> The checking of need_update is not necessary any more because the
> __thermal_zone_device_update() calls potentially triggered by cooling
> device binding take place before calling thermal_zone_init_complete(),
> so they all will return early, which means that
> thermal_zone_init_complete() must call __thermal_zone_device_update()
> in case the thermal zone is enabled prematurely by user space.
> 
> Fixes: 203d3d4aa482 ("the generic thermal sysfs driver")
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> This is a new iteration of
> 
> https://lore.kernel.org/linux-pm/2973309.e9J7NaK4W3@rjwysocki.net/
> 
> v1 -> v2: Rebase and add a Fixes tag.
> 
> ---
>   drivers/thermal/thermal_core.c |   16 +++++++++++++---
>   drivers/thermal/thermal_core.h |    1 +
>   2 files changed, 14 insertions(+), 3 deletions(-)
> 
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -1332,6 +1332,16 @@ int thermal_zone_get_crit_temp(struct th
>   }
>   EXPORT_SYMBOL_GPL(thermal_zone_get_crit_temp);
>   
> +static void thermal_zone_init_complete(struct thermal_zone_device *tz)
> +{
> +	mutex_lock(&tz->lock);
> +
> +	tz->state &= ~TZ_STATE_FLAG_INIT;
> +	__thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
> +
> +	mutex_unlock(&tz->lock);
> +}
> +
>   /**
>    * thermal_zone_device_register_with_trips() - register a new thermal zone device
>    * @type:	the thermal zone device type
> @@ -1451,6 +1461,8 @@ thermal_zone_device_register_with_trips(
>   	tz->passive_delay_jiffies = msecs_to_jiffies(passive_delay);
>   	tz->recheck_delay_jiffies = THERMAL_RECHECK_DELAY;
>   
> +	tz->state = TZ_STATE_FLAG_INIT;
> +
>   	/* sys I/F */
>   	/* Add nodes that are always present via .groups */
>   	result = thermal_zone_create_device_groups(tz);
> @@ -1504,9 +1516,7 @@ thermal_zone_device_register_with_trips(
>   
>   	mutex_unlock(&thermal_list_lock);
>   
> -	/* Update the new thermal zone and mark it as already updated. */
> -	if (atomic_cmpxchg(&tz->need_update, 1, 0))
> -		thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
> +	thermal_zone_init_complete(tz);
>   
>   	thermal_notify_tz_create(tz);
>   
> Index: linux-pm/drivers/thermal/thermal_core.h
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.h
> +++ linux-pm/drivers/thermal/thermal_core.h
> @@ -63,6 +63,7 @@ struct thermal_governor {
>   
>   #define	TZ_STATE_FLAG_SUSPENDED	BIT(0)
>   #define	TZ_STATE_FLAG_RESUMING	BIT(1)
> +#define	TZ_STATE_FLAG_INIT	BIT(2)
>   
>   #define TZ_STATE_READY		0
>   
> 
> 
> 

Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

[PATCH v2 05/12] thermal: core: Fix race between zone registration and system suspend

[Rafael J. Wysocki]

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

If the registration of a thermal zone takes place at the time when
system suspend is started, thermal_pm_notify() can run before the new
thermal zone is added to thermal_tz_list and its "suspended" flag will
not be set.  Consequently, if __thermal_zone_device_update() is called
for that thermal zone, it will not return early as expected which may
cause some destructive interference with the system suspend or resume
flow to occur.

To avoid that, make thermal_zone_init_complete() introduced previously
set the "suspended" flag for new thermal zones if it runs during system
suspend or resume.

Fixes: 4e814173a8c4 ("thermal: core: Fix thermal zone suspend-resume synchronization")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

This is a new iteration of

https://lore.kernel.org/linux-pm/3335807.44csPzL39Z@rjwysocki.net/

v1 -> v2: Rebase and add a fixes tag.

---
 drivers/thermal/thermal_core.c |   18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -40,6 +40,8 @@ static DEFINE_MUTEX(thermal_governor_loc
 
 static struct thermal_governor *def_governor;
 
+static bool thermal_pm_suspended;
+
 /*
  * Governor section: set of functions to handle thermal governors
  *
@@ -1337,6 +1339,14 @@ static void thermal_zone_init_complete(s
 	mutex_lock(&tz->lock);
 
 	tz->state &= ~TZ_STATE_FLAG_INIT;
+	/*
+	 * If system suspend or resume is in progress at this point, the
+	 * new thermal zone needs to be marked as suspended because
+	 * thermal_pm_notify() has run already.
+	 */
+	if (thermal_pm_suspended)
+		tz->state |= TZ_STATE_FLAG_SUSPENDED;
+
 	__thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
 
 	mutex_unlock(&tz->lock);
@@ -1514,10 +1524,10 @@ thermal_zone_device_register_with_trips(
 	list_for_each_entry(cdev, &thermal_cdev_list, node)
 		thermal_zone_cdev_bind(tz, cdev);
 
-	mutex_unlock(&thermal_list_lock);
-
 	thermal_zone_init_complete(tz);
 
+	mutex_unlock(&thermal_list_lock);
+
 	thermal_notify_tz_create(tz);
 
 	thermal_debug_tz_add(tz);
@@ -1737,6 +1747,8 @@ static int thermal_pm_notify(struct noti
 	case PM_SUSPEND_PREPARE:
 		mutex_lock(&thermal_list_lock);
 
+		thermal_pm_suspended = true;
+
 		list_for_each_entry(tz, &thermal_tz_list, node)
 			thermal_zone_pm_prepare(tz);
 
@@ -1747,6 +1759,8 @@ static int thermal_pm_notify(struct noti
 	case PM_POST_SUSPEND:
 		mutex_lock(&thermal_list_lock);
 
+		thermal_pm_suspended = false;
+
 		list_for_each_entry(tz, &thermal_tz_list, node)
 			thermal_zone_pm_complete(tz);
 

Re: [PATCH v2 05/12] thermal: core: Fix race between zone registration and system suspend

[Lukasz Luba]

On 10/4/24 20:19, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> If the registration of a thermal zone takes place at the time when
> system suspend is started, thermal_pm_notify() can run before the new
> thermal zone is added to thermal_tz_list and its "suspended" flag will
> not be set.  Consequently, if __thermal_zone_device_update() is called
> for that thermal zone, it will not return early as expected which may
> cause some destructive interference with the system suspend or resume
> flow to occur.
> 
> To avoid that, make thermal_zone_init_complete() introduced previously
> set the "suspended" flag for new thermal zones if it runs during system
> suspend or resume.
> 
> Fixes: 4e814173a8c4 ("thermal: core: Fix thermal zone suspend-resume synchronization")
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> This is a new iteration of
> 
> https://lore.kernel.org/linux-pm/3335807.44csPzL39Z@rjwysocki.net/
> 
> v1 -> v2: Rebase and add a fixes tag.
> 
> ---
>   drivers/thermal/thermal_core.c |   18 ++++++++++++++++--
>   1 file changed, 16 insertions(+), 2 deletions(-)
> 
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -40,6 +40,8 @@ static DEFINE_MUTEX(thermal_governor_loc
>   
>   static struct thermal_governor *def_governor;
>   
> +static bool thermal_pm_suspended;
> +
>   /*
>    * Governor section: set of functions to handle thermal governors
>    *
> @@ -1337,6 +1339,14 @@ static void thermal_zone_init_complete(s
>   	mutex_lock(&tz->lock);
>   
>   	tz->state &= ~TZ_STATE_FLAG_INIT;
> +	/*
> +	 * If system suspend or resume is in progress at this point, the
> +	 * new thermal zone needs to be marked as suspended because
> +	 * thermal_pm_notify() has run already.
> +	 */
> +	if (thermal_pm_suspended)
> +		tz->state |= TZ_STATE_FLAG_SUSPENDED;
> +
>   	__thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
>   
>   	mutex_unlock(&tz->lock);
> @@ -1514,10 +1524,10 @@ thermal_zone_device_register_with_trips(
>   	list_for_each_entry(cdev, &thermal_cdev_list, node)
>   		thermal_zone_cdev_bind(tz, cdev);
>   
> -	mutex_unlock(&thermal_list_lock);
> -
>   	thermal_zone_init_complete(tz);
>   
> +	mutex_unlock(&thermal_list_lock);
> +
>   	thermal_notify_tz_create(tz);
>   
>   	thermal_debug_tz_add(tz);
> @@ -1737,6 +1747,8 @@ static int thermal_pm_notify(struct noti
>   	case PM_SUSPEND_PREPARE:
>   		mutex_lock(&thermal_list_lock);
>   
> +		thermal_pm_suspended = true;
> +
>   		list_for_each_entry(tz, &thermal_tz_list, node)
>   			thermal_zone_pm_prepare(tz);
>   
> @@ -1747,6 +1759,8 @@ static int thermal_pm_notify(struct noti
>   	case PM_POST_SUSPEND:
>   		mutex_lock(&thermal_list_lock);
>   
> +		thermal_pm_suspended = false;
> +
>   		list_for_each_entry(tz, &thermal_tz_list, node)
>   			thermal_zone_pm_complete(tz);
>   
> 
> 
> 

Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

[PATCH v2 06/12] thermal: core: Consolidate thermal zone locking during initialization

[Rafael J. Wysocki]

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

The part of thermal zone initialization carried out under
thermal_list_lock acquires the thermal zone lock and releases it
multiple times back and forth which is not really necessary.

Instead of doing this, make it acquire the thermal zone lock once after
acquiring thermal_list_lock and release it along with that lock.

For this purpose, move all of the code in question to
thermal_zone_init_complete() introduced previously and provide an
"unlocked" variant of thermal_zone_cdev_bind() to be invoked from
there.

Also notice that a thermal zone does not need to be added to
thermal_tz_list under its own lock, so make the new code acquire
the thermal zone lock after adding it to the list.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

This is a new iteration of

https://lore.kernel.org/linux-pm/10548633.nUPlyArG6x@rjwysocki.net/

v1 -> v2: Rebase, update the changelog.

---
 drivers/thermal/thermal_core.c |   39 ++++++++++++++++++++++-----------------
 1 file changed, 22 insertions(+), 17 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -933,16 +933,14 @@ void print_bind_err_msg(struct thermal_z
 		cdev->type, thermal_zone_trip_id(tz, trip), ret);
 }
 
-static void thermal_zone_cdev_bind(struct thermal_zone_device *tz,
-				   struct thermal_cooling_device *cdev)
+static void __thermal_zone_cdev_bind(struct thermal_zone_device *tz,
+				     struct thermal_cooling_device *cdev)
 {
 	struct thermal_trip_desc *td;
 
 	if (!tz->ops.should_bind)
 		return;
 
-	mutex_lock(&tz->lock);
-
 	for_each_trip_desc(tz, td) {
 		struct thermal_trip *trip = &td->trip;
 		struct cooling_spec c = {
@@ -959,6 +957,14 @@ static void thermal_zone_cdev_bind(struc
 		if (ret)
 			print_bind_err_msg(tz, trip, cdev, ret);
 	}
+}
+
+static void thermal_zone_cdev_bind(struct thermal_zone_device *tz,
+				   struct thermal_cooling_device *cdev)
+{
+	mutex_lock(&tz->lock);
+
+	__thermal_zone_cdev_bind(tz, cdev);
 
 	mutex_unlock(&tz->lock);
 }
@@ -1336,8 +1342,18 @@ EXPORT_SYMBOL_GPL(thermal_zone_get_crit_
 
 static void thermal_zone_init_complete(struct thermal_zone_device *tz)
 {
+	struct thermal_cooling_device *cdev;
+
+	mutex_lock(&thermal_list_lock);
+
+	list_add_tail(&tz->node, &thermal_tz_list);
+
 	mutex_lock(&tz->lock);
 
+	/* Bind cooling devices for this zone. */
+	list_for_each_entry(cdev, &thermal_cdev_list, node)
+		__thermal_zone_cdev_bind(tz, cdev);
+
 	tz->state &= ~TZ_STATE_FLAG_INIT;
 	/*
 	 * If system suspend or resume is in progress at this point, the
@@ -1350,6 +1366,8 @@ static void thermal_zone_init_complete(s
 	__thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
 
 	mutex_unlock(&tz->lock);
+
+	mutex_unlock(&thermal_list_lock);
 }
 
 /**
@@ -1386,7 +1404,6 @@ thermal_zone_device_register_with_trips(
 					unsigned int polling_delay)
 {
 	const struct thermal_trip *trip = trips;
-	struct thermal_cooling_device *cdev;
 	struct thermal_zone_device *tz;
 	struct thermal_trip_desc *td;
 	int id;
@@ -1514,20 +1531,8 @@ thermal_zone_device_register_with_trips(
 			goto unregister;
 	}
 
-	mutex_lock(&thermal_list_lock);
-
-	mutex_lock(&tz->lock);
-	list_add_tail(&tz->node, &thermal_tz_list);
-	mutex_unlock(&tz->lock);
-
-	/* Bind cooling devices for this zone */
-	list_for_each_entry(cdev, &thermal_cdev_list, node)
-		thermal_zone_cdev_bind(tz, cdev);
-
 	thermal_zone_init_complete(tz);
 
-	mutex_unlock(&thermal_list_lock);
-
 	thermal_notify_tz_create(tz);
 
 	thermal_debug_tz_add(tz);

Re: [PATCH v2 06/12] thermal: core: Consolidate thermal zone locking during initialization

[Lukasz Luba]

On 10/4/24 20:23, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> The part of thermal zone initialization carried out under
> thermal_list_lock acquires the thermal zone lock and releases it
> multiple times back and forth which is not really necessary.
> 
> Instead of doing this, make it acquire the thermal zone lock once after
> acquiring thermal_list_lock and release it along with that lock.
> 
> For this purpose, move all of the code in question to
> thermal_zone_init_complete() introduced previously and provide an
> "unlocked" variant of thermal_zone_cdev_bind() to be invoked from
> there.
> 
> Also notice that a thermal zone does not need to be added to
> thermal_tz_list under its own lock, so make the new code acquire
> the thermal zone lock after adding it to the list.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> This is a new iteration of
> 
> https://lore.kernel.org/linux-pm/10548633.nUPlyArG6x@rjwysocki.net/
> 
> v1 -> v2: Rebase, update the changelog.
> 
> ---
>   drivers/thermal/thermal_core.c |   39 ++++++++++++++++++++++-----------------
>   1 file changed, 22 insertions(+), 17 deletions(-)
> 
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -933,16 +933,14 @@ void print_bind_err_msg(struct thermal_z
>   		cdev->type, thermal_zone_trip_id(tz, trip), ret);
>   }
>   
> -static void thermal_zone_cdev_bind(struct thermal_zone_device *tz,
> -				   struct thermal_cooling_device *cdev)
> +static void __thermal_zone_cdev_bind(struct thermal_zone_device *tz,
> +				     struct thermal_cooling_device *cdev)
>   {
>   	struct thermal_trip_desc *td;
>   
>   	if (!tz->ops.should_bind)
>   		return;
>   
> -	mutex_lock(&tz->lock);
> -
>   	for_each_trip_desc(tz, td) {
>   		struct thermal_trip *trip = &td->trip;
>   		struct cooling_spec c = {
> @@ -959,6 +957,14 @@ static void thermal_zone_cdev_bind(struc
>   		if (ret)
>   			print_bind_err_msg(tz, trip, cdev, ret);
>   	}
> +}
> +
> +static void thermal_zone_cdev_bind(struct thermal_zone_device *tz,
> +				   struct thermal_cooling_device *cdev)
> +{
> +	mutex_lock(&tz->lock);
> +
> +	__thermal_zone_cdev_bind(tz, cdev);
>   
>   	mutex_unlock(&tz->lock);
>   }
> @@ -1336,8 +1342,18 @@ EXPORT_SYMBOL_GPL(thermal_zone_get_crit_
>   
>   static void thermal_zone_init_complete(struct thermal_zone_device *tz)
>   {
> +	struct thermal_cooling_device *cdev;
> +
> +	mutex_lock(&thermal_list_lock);
> +
> +	list_add_tail(&tz->node, &thermal_tz_list);
> +
>   	mutex_lock(&tz->lock);
>   
> +	/* Bind cooling devices for this zone. */
> +	list_for_each_entry(cdev, &thermal_cdev_list, node)
> +		__thermal_zone_cdev_bind(tz, cdev);
> +
>   	tz->state &= ~TZ_STATE_FLAG_INIT;
>   	/*
>   	 * If system suspend or resume is in progress at this point, the
> @@ -1350,6 +1366,8 @@ static void thermal_zone_init_complete(s
>   	__thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
>   
>   	mutex_unlock(&tz->lock);
> +
> +	mutex_unlock(&thermal_list_lock);
>   }
>   
>   /**
> @@ -1386,7 +1404,6 @@ thermal_zone_device_register_with_trips(
>   					unsigned int polling_delay)
>   {
>   	const struct thermal_trip *trip = trips;
> -	struct thermal_cooling_device *cdev;
>   	struct thermal_zone_device *tz;
>   	struct thermal_trip_desc *td;
>   	int id;
> @@ -1514,20 +1531,8 @@ thermal_zone_device_register_with_trips(
>   			goto unregister;
>   	}
>   
> -	mutex_lock(&thermal_list_lock);
> -
> -	mutex_lock(&tz->lock);
> -	list_add_tail(&tz->node, &thermal_tz_list);
> -	mutex_unlock(&tz->lock);
> -
> -	/* Bind cooling devices for this zone */
> -	list_for_each_entry(cdev, &thermal_cdev_list, node)
> -		thermal_zone_cdev_bind(tz, cdev);
> -
>   	thermal_zone_init_complete(tz);
>   
> -	mutex_unlock(&thermal_list_lock);
> -
>   	thermal_notify_tz_create(tz);
>   
>   	thermal_debug_tz_add(tz);
> 
> 
> 

Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

[PATCH v2 07/12] thermal: core: Mark thermal zones as exiting before unregistration

[Rafael J. Wysocki]

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

In analogy with a previous change in the thermal zone registration code
path, to ensure that __thermal_zone_device_update() will return early
for thermal zones that are going away, introduce a thermal zone state
flag representing the "exit" state and set it while deleting the thermal
zone from thermal_tz_list.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

This is a new iteration of

https://lore.kernel.org/linux-pm/1997536.PYKUYFuaPT@rjwysocki.net/

v1 -> v2: Rebase.

---
 drivers/thermal/thermal_core.c |    3 +++
 drivers/thermal/thermal_core.h |    1 +
 2 files changed, 4 insertions(+)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -1613,7 +1613,10 @@ void thermal_zone_device_unregister(stru
 	}
 
 	mutex_lock(&tz->lock);
+
+	tz->state |= TZ_STATE_FLAG_EXIT;
 	list_del(&tz->node);
+
 	mutex_unlock(&tz->lock);
 
 	/* Unbind all cdevs associated with 'this' thermal zone */
Index: linux-pm/drivers/thermal/thermal_core.h
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.h
+++ linux-pm/drivers/thermal/thermal_core.h
@@ -64,6 +64,7 @@ struct thermal_governor {
 #define	TZ_STATE_FLAG_SUSPENDED	BIT(0)
 #define	TZ_STATE_FLAG_RESUMING	BIT(1)
 #define	TZ_STATE_FLAG_INIT	BIT(2)
+#define	TZ_STATE_FLAG_EXIT	BIT(3)
 
 #define TZ_STATE_READY		0
 

Re: [PATCH v2 07/12] thermal: core: Mark thermal zones as exiting before unregistration

[Lukasz Luba]

On 10/4/24 20:26, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> In analogy with a previous change in the thermal zone registration code
> path, to ensure that __thermal_zone_device_update() will return early
> for thermal zones that are going away, introduce a thermal zone state
> flag representing the "exit" state and set it while deleting the thermal
> zone from thermal_tz_list.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> This is a new iteration of
> 
> https://lore.kernel.org/linux-pm/1997536.PYKUYFuaPT@rjwysocki.net/
> 
> v1 -> v2: Rebase.
> 
> ---
>   drivers/thermal/thermal_core.c |    3 +++
>   drivers/thermal/thermal_core.h |    1 +
>   2 files changed, 4 insertions(+)
> 
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -1613,7 +1613,10 @@ void thermal_zone_device_unregister(stru
>   	}
>   
>   	mutex_lock(&tz->lock);
> +
> +	tz->state |= TZ_STATE_FLAG_EXIT;
>   	list_del(&tz->node);
> +
>   	mutex_unlock(&tz->lock);
>   
>   	/* Unbind all cdevs associated with 'this' thermal zone */
> Index: linux-pm/drivers/thermal/thermal_core.h
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.h
> +++ linux-pm/drivers/thermal/thermal_core.h
> @@ -64,6 +64,7 @@ struct thermal_governor {
>   #define	TZ_STATE_FLAG_SUSPENDED	BIT(0)
>   #define	TZ_STATE_FLAG_RESUMING	BIT(1)
>   #define	TZ_STATE_FLAG_INIT	BIT(2)
> +#define	TZ_STATE_FLAG_EXIT	BIT(3)
>   
>   #define TZ_STATE_READY		0
>   
> 
> 
> 

Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

[PATCH v2 08/12] thermal: core: Consolidate thermal zone locking in the exit path

[Rafael J. Wysocki]

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

In analogy with a previous change in the thermal zone initialization
path, to avoid acquiring the thermal zone lock and releasing it multiple
times back and forth unnecessarily, move all of the code running under
thermal_list_lock in thermal_zone_device_unregister() into a new
function called thermal_zone_exit() and make the latter acquire the
thermal zone lock only once and release it along with thermal_list_lock.

For this purpose, provide an "unlocked" variant of
thermal_zone_cdev_unbind() to be called by thermal_zone_exit() under the
thermal zone lock.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

This is a new iteration of

https://lore.kernel.org/linux-pm/7729094.EvYhyI6sBW@rjwysocki.net/

v1 -> v2: Rebase, use list_del_init() for removing thermal zones from the
          global list so that the new list_emty() check doesn't blow up.

---
 drivers/thermal/thermal_core.c |   66 ++++++++++++++++++++++++-----------------
 1 file changed, 39 insertions(+), 27 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -1266,15 +1266,21 @@ unlock_list:
 }
 EXPORT_SYMBOL_GPL(thermal_cooling_device_update);
 
-static void thermal_zone_cdev_unbind(struct thermal_zone_device *tz,
-				     struct thermal_cooling_device *cdev)
+static void __thermal_zone_cdev_unbind(struct thermal_zone_device *tz,
+				       struct thermal_cooling_device *cdev)
 {
 	struct thermal_trip_desc *td;
 
-	mutex_lock(&tz->lock);
-
 	for_each_trip_desc(tz, td)
 		thermal_unbind_cdev_from_trip(tz, &td->trip, cdev);
+}
+
+static void thermal_zone_cdev_unbind(struct thermal_zone_device *tz,
+				     struct thermal_cooling_device *cdev)
+{
+	mutex_lock(&tz->lock);
+
+	__thermal_zone_cdev_unbind(tz, cdev);
 
 	mutex_unlock(&tz->lock);
 }
@@ -1588,43 +1594,49 @@ struct device *thermal_zone_device(struc
 }
 EXPORT_SYMBOL_GPL(thermal_zone_device);
 
-/**
- * thermal_zone_device_unregister - removes the registered thermal zone device
- * @tz: the thermal zone device to remove
- */
-void thermal_zone_device_unregister(struct thermal_zone_device *tz)
+static bool thermal_zone_exit(struct thermal_zone_device *tz)
 {
 	struct thermal_cooling_device *cdev;
-	struct thermal_zone_device *pos = NULL;
-
-	if (!tz)
-		return;
-
-	thermal_debug_tz_remove(tz);
+	bool ret = true;
 
 	mutex_lock(&thermal_list_lock);
-	list_for_each_entry(pos, &thermal_tz_list, node)
-		if (pos == tz)
-			break;
-	if (pos != tz) {
-		/* thermal zone device not found */
-		mutex_unlock(&thermal_list_lock);
-		return;
+
+	if (list_empty(&tz->node)) {
+		ret = false;
+		goto unlock;
 	}
 
 	mutex_lock(&tz->lock);
 
 	tz->state |= TZ_STATE_FLAG_EXIT;
-	list_del(&tz->node);
-
-	mutex_unlock(&tz->lock);
+	list_del_init(&tz->node);
 
-	/* Unbind all cdevs associated with 'this' thermal zone */
+	/* Unbind all cdevs associated with this thermal zone. */
 	list_for_each_entry(cdev, &thermal_cdev_list, node)
-		thermal_zone_cdev_unbind(tz, cdev);
+		__thermal_zone_cdev_unbind(tz, cdev);
+
+	mutex_unlock(&tz->lock);
 
+unlock:
 	mutex_unlock(&thermal_list_lock);
 
+	return ret;
+}
+
+/**
+ * thermal_zone_device_unregister - removes the registered thermal zone device
+ * @tz: the thermal zone device to remove
+ */
+void thermal_zone_device_unregister(struct thermal_zone_device *tz)
+{
+	if (!tz)
+		return;
+
+	thermal_debug_tz_remove(tz);
+
+	if (!thermal_zone_exit(tz))
+		return;
+
 	cancel_delayed_work_sync(&tz->poll_queue);
 
 	thermal_set_governor(tz, NULL);

Re: [PATCH v2 08/12] thermal: core: Consolidate thermal zone locking in the exit path

[Lukasz Luba]

On 10/4/24 20:30, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> In analogy with a previous change in the thermal zone initialization
> path, to avoid acquiring the thermal zone lock and releasing it multiple
> times back and forth unnecessarily, move all of the code running under
> thermal_list_lock in thermal_zone_device_unregister() into a new
> function called thermal_zone_exit() and make the latter acquire the
> thermal zone lock only once and release it along with thermal_list_lock.
> 
> For this purpose, provide an "unlocked" variant of
> thermal_zone_cdev_unbind() to be called by thermal_zone_exit() under the
> thermal zone lock.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> This is a new iteration of
> 
> https://lore.kernel.org/linux-pm/7729094.EvYhyI6sBW@rjwysocki.net/
> 
> v1 -> v2: Rebase, use list_del_init() for removing thermal zones from the
>            global list so that the new list_emty() check doesn't blow up.
> 
> ---
>   drivers/thermal/thermal_core.c |   66 ++++++++++++++++++++++++-----------------
>   1 file changed, 39 insertions(+), 27 deletions(-)
> 
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -1266,15 +1266,21 @@ unlock_list:
>   }
>   EXPORT_SYMBOL_GPL(thermal_cooling_device_update);
>   
> -static void thermal_zone_cdev_unbind(struct thermal_zone_device *tz,
> -				     struct thermal_cooling_device *cdev)
> +static void __thermal_zone_cdev_unbind(struct thermal_zone_device *tz,
> +				       struct thermal_cooling_device *cdev)
>   {
>   	struct thermal_trip_desc *td;
>   
> -	mutex_lock(&tz->lock);
> -
>   	for_each_trip_desc(tz, td)
>   		thermal_unbind_cdev_from_trip(tz, &td->trip, cdev);
> +}
> +
> +static void thermal_zone_cdev_unbind(struct thermal_zone_device *tz,
> +				     struct thermal_cooling_device *cdev)
> +{
> +	mutex_lock(&tz->lock);
> +
> +	__thermal_zone_cdev_unbind(tz, cdev);
>   
>   	mutex_unlock(&tz->lock);
>   }
> @@ -1588,43 +1594,49 @@ struct device *thermal_zone_device(struc
>   }
>   EXPORT_SYMBOL_GPL(thermal_zone_device);
>   
> -/**
> - * thermal_zone_device_unregister - removes the registered thermal zone device
> - * @tz: the thermal zone device to remove
> - */
> -void thermal_zone_device_unregister(struct thermal_zone_device *tz)
> +static bool thermal_zone_exit(struct thermal_zone_device *tz)
>   {
>   	struct thermal_cooling_device *cdev;
> -	struct thermal_zone_device *pos = NULL;
> -
> -	if (!tz)
> -		return;
> -
> -	thermal_debug_tz_remove(tz);
> +	bool ret = true;
>   
>   	mutex_lock(&thermal_list_lock);
> -	list_for_each_entry(pos, &thermal_tz_list, node)
> -		if (pos == tz)
> -			break;
> -	if (pos != tz) {
> -		/* thermal zone device not found */
> -		mutex_unlock(&thermal_list_lock);
> -		return;
> +
> +	if (list_empty(&tz->node)) {
> +		ret = false;
> +		goto unlock;
>   	}
>   
>   	mutex_lock(&tz->lock);
>   
>   	tz->state |= TZ_STATE_FLAG_EXIT;
> -	list_del(&tz->node);
> -
> -	mutex_unlock(&tz->lock);
> +	list_del_init(&tz->node);
>   
> -	/* Unbind all cdevs associated with 'this' thermal zone */
> +	/* Unbind all cdevs associated with this thermal zone. */
>   	list_for_each_entry(cdev, &thermal_cdev_list, node)
> -		thermal_zone_cdev_unbind(tz, cdev);
> +		__thermal_zone_cdev_unbind(tz, cdev);
> +
> +	mutex_unlock(&tz->lock);
>   
> +unlock:
>   	mutex_unlock(&thermal_list_lock);
>   
> +	return ret;
> +}
> +
> +/**
> + * thermal_zone_device_unregister - removes the registered thermal zone device
> + * @tz: the thermal zone device to remove
> + */
> +void thermal_zone_device_unregister(struct thermal_zone_device *tz)
> +{
> +	if (!tz)
> +		return;
> +
> +	thermal_debug_tz_remove(tz);
> +
> +	if (!thermal_zone_exit(tz))
> +		return;
> +
>   	cancel_delayed_work_sync(&tz->poll_queue);
>   
>   	thermal_set_governor(tz, NULL);
> 
> 
> 

Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

[PATCH v2 09/12] thermal: core: Update thermal zones after cooling device binding

[Rafael J. Wysocki]

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

If a new cooling device is registered and it is bound to at least one
trip point in a given thermal zone, that thermal zone needs to be
updated via __thermal_zone_device_update().

Instead of doing this with the help of the need_update atomic field in
struct thermal_zone_device, which is not particularly straightforward,
make __thermal_zone_cdev_bind() return a bool value indicating whether
or not the given thermal zone needs to be updated because a new cooling
device has been bound to it and update thermal_zone_cdev_bind() to
call __thermal_zone_device_update() when this value is "true".

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

This is a new iteration of

https://lore.kernel.org/linux-pm/3603909.iIbC2pHGDl@rjwysocki.net/

v1 -> v2: Rebase.

---
 drivers/thermal/thermal_core.c |   23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -933,13 +933,14 @@ void print_bind_err_msg(struct thermal_z
 		cdev->type, thermal_zone_trip_id(tz, trip), ret);
 }
 
-static void __thermal_zone_cdev_bind(struct thermal_zone_device *tz,
+static bool __thermal_zone_cdev_bind(struct thermal_zone_device *tz,
 				     struct thermal_cooling_device *cdev)
 {
 	struct thermal_trip_desc *td;
+	bool update_tz = false;
 
 	if (!tz->ops.should_bind)
-		return;
+		return false;
 
 	for_each_trip_desc(tz, td) {
 		struct thermal_trip *trip = &td->trip;
@@ -954,9 +955,15 @@ static void __thermal_zone_cdev_bind(str
 			continue;
 
 		ret = thermal_bind_cdev_to_trip(tz, trip, cdev, &c);
-		if (ret)
+		if (ret) {
 			print_bind_err_msg(tz, trip, cdev, ret);
+			continue;
+		}
+
+		update_tz = true;
 	}
+
+	return update_tz;
 }
 
 static void thermal_zone_cdev_bind(struct thermal_zone_device *tz,
@@ -964,7 +971,8 @@ static void thermal_zone_cdev_bind(struc
 {
 	mutex_lock(&tz->lock);
 
-	__thermal_zone_cdev_bind(tz, cdev);
+	if (__thermal_zone_cdev_bind(tz, cdev))
+		__thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
 
 	mutex_unlock(&tz->lock);
 }
@@ -991,7 +999,7 @@ __thermal_cooling_device_register(struct
 				  const struct thermal_cooling_device_ops *ops)
 {
 	struct thermal_cooling_device *cdev;
-	struct thermal_zone_device *pos = NULL;
+	struct thermal_zone_device *pos;
 	unsigned long current_state;
 	int id, ret;
 
@@ -1067,11 +1075,6 @@ __thermal_cooling_device_register(struct
 	list_for_each_entry(pos, &thermal_tz_list, node)
 		thermal_zone_cdev_bind(pos, cdev);
 
-	list_for_each_entry(pos, &thermal_tz_list, node)
-		if (atomic_cmpxchg(&pos->need_update, 1, 0))
-			thermal_zone_device_update(pos,
-						   THERMAL_EVENT_UNSPECIFIED);
-
 	mutex_unlock(&thermal_list_lock);
 
 	return cdev;

Re: [PATCH v2 09/12] thermal: core: Update thermal zones after cooling device binding

[Lukasz Luba]

On 10/4/24 20:33, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> If a new cooling device is registered and it is bound to at least one
> trip point in a given thermal zone, that thermal zone needs to be
> updated via __thermal_zone_device_update().
> 
> Instead of doing this with the help of the need_update atomic field in
> struct thermal_zone_device, which is not particularly straightforward,
> make __thermal_zone_cdev_bind() return a bool value indicating whether
> or not the given thermal zone needs to be updated because a new cooling
> device has been bound to it and update thermal_zone_cdev_bind() to
> call __thermal_zone_device_update() when this value is "true".
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> This is a new iteration of
> 
> https://lore.kernel.org/linux-pm/3603909.iIbC2pHGDl@rjwysocki.net/
> 
> v1 -> v2: Rebase.
> 
> ---
>   drivers/thermal/thermal_core.c |   23 +++++++++++++----------
>   1 file changed, 13 insertions(+), 10 deletions(-)
> 
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -933,13 +933,14 @@ void print_bind_err_msg(struct thermal_z
>   		cdev->type, thermal_zone_trip_id(tz, trip), ret);
>   }
>   
> -static void __thermal_zone_cdev_bind(struct thermal_zone_device *tz,
> +static bool __thermal_zone_cdev_bind(struct thermal_zone_device *tz,
>   				     struct thermal_cooling_device *cdev)
>   {
>   	struct thermal_trip_desc *td;
> +	bool update_tz = false;
>   
>   	if (!tz->ops.should_bind)
> -		return;
> +		return false;
>   
>   	for_each_trip_desc(tz, td) {
>   		struct thermal_trip *trip = &td->trip;
> @@ -954,9 +955,15 @@ static void __thermal_zone_cdev_bind(str
>   			continue;
>   
>   		ret = thermal_bind_cdev_to_trip(tz, trip, cdev, &c);
> -		if (ret)
> +		if (ret) {
>   			print_bind_err_msg(tz, trip, cdev, ret);
> +			continue;
> +		}
> +
> +		update_tz = true;
>   	}
> +
> +	return update_tz;
>   }
>   
>   static void thermal_zone_cdev_bind(struct thermal_zone_device *tz,
> @@ -964,7 +971,8 @@ static void thermal_zone_cdev_bind(struc
>   {
>   	mutex_lock(&tz->lock);
>   
> -	__thermal_zone_cdev_bind(tz, cdev);
> +	if (__thermal_zone_cdev_bind(tz, cdev))
> +		__thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
>   
>   	mutex_unlock(&tz->lock);
>   }
> @@ -991,7 +999,7 @@ __thermal_cooling_device_register(struct
>   				  const struct thermal_cooling_device_ops *ops)
>   {
>   	struct thermal_cooling_device *cdev;
> -	struct thermal_zone_device *pos = NULL;
> +	struct thermal_zone_device *pos;
>   	unsigned long current_state;
>   	int id, ret;
>   
> @@ -1067,11 +1075,6 @@ __thermal_cooling_device_register(struct
>   	list_for_each_entry(pos, &thermal_tz_list, node)
>   		thermal_zone_cdev_bind(pos, cdev);
>   
> -	list_for_each_entry(pos, &thermal_tz_list, node)
> -		if (atomic_cmpxchg(&pos->need_update, 1, 0))
> -			thermal_zone_device_update(pos,
> -						   THERMAL_EVENT_UNSPECIFIED);
> -
>   	mutex_unlock(&thermal_list_lock);
>   
>   	return cdev;
> 
> 
> 

Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

[PATCH v2 10/12] thermal: core: Drop need_update field from struct thermal_zone_device

[Rafael J. Wysocki]

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

After previous changes, the need_update field in struct thermal_zone_device
is only set and never read, so drop it.

No functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

This is a new iteration of

https://lore.kernel.org/linux-pm/3261209.5fSG56mABF@rjwysocki.net/

v1 -> v2: Rebase.

---
 drivers/thermal/thermal_core.c |    4 ----
 drivers/thermal/thermal_core.h |    2 --
 2 files changed, 6 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -840,7 +840,6 @@ static int thermal_bind_cdev_to_trip(str
 	if (!result) {
 		list_add_tail(&dev->tz_node, &tz->thermal_instances);
 		list_add_tail(&dev->cdev_node, &cdev->thermal_instances);
-		atomic_set(&tz->need_update, 1);
 
 		thermal_governor_update_tz(tz, THERMAL_TZ_BIND_CDEV);
 	}
@@ -1505,9 +1504,6 @@ thermal_zone_device_register_with_trips(
 	if (result)
 		goto remove_id;
 
-	/* A new thermal zone needs to be updated anyway. */
-	atomic_set(&tz->need_update, 1);
-
 	result = dev_set_name(&tz->device, "thermal_zone%d", tz->id);
 	if (result) {
 		thermal_zone_destroy_device_groups(tz);
Index: linux-pm/drivers/thermal/thermal_core.h
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.h
+++ linux-pm/drivers/thermal/thermal_core.h
@@ -95,7 +95,6 @@ struct thermal_governor {
 			trip point.
  * @prev_high_trip:	the above current temperature if you've crossed a
 			passive trip point.
- * @need_update:	if equals 1, thermal_zone_device_update needs to be invoked.
  * @ops:	operations this &thermal_zone_device supports
  * @tzp:	thermal zone parameters
  * @governor:	pointer to the governor for this thermal zone
@@ -129,7 +128,6 @@ struct thermal_zone_device {
 	int passive;
 	int prev_low_trip;
 	int prev_high_trip;
-	atomic_t need_update;
 	struct thermal_zone_device_ops ops;
 	struct thermal_zone_params *tzp;
 	struct thermal_governor *governor;

Re: [PATCH v2 10/12] thermal: core: Drop need_update field from struct thermal_zone_device

[Lukasz Luba]

On 10/4/24 20:35, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> After previous changes, the need_update field in struct thermal_zone_device
> is only set and never read, so drop it.
> 
> No functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> This is a new iteration of
> 
> https://lore.kernel.org/linux-pm/3261209.5fSG56mABF@rjwysocki.net/
> 
> v1 -> v2: Rebase.
> 
> ---
>   drivers/thermal/thermal_core.c |    4 ----
>   drivers/thermal/thermal_core.h |    2 --
>   2 files changed, 6 deletions(-)
> 
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -840,7 +840,6 @@ static int thermal_bind_cdev_to_trip(str
>   	if (!result) {
>   		list_add_tail(&dev->tz_node, &tz->thermal_instances);
>   		list_add_tail(&dev->cdev_node, &cdev->thermal_instances);
> -		atomic_set(&tz->need_update, 1);
>   
>   		thermal_governor_update_tz(tz, THERMAL_TZ_BIND_CDEV);
>   	}
> @@ -1505,9 +1504,6 @@ thermal_zone_device_register_with_trips(
>   	if (result)
>   		goto remove_id;
>   
> -	/* A new thermal zone needs to be updated anyway. */
> -	atomic_set(&tz->need_update, 1);
> -
>   	result = dev_set_name(&tz->device, "thermal_zone%d", tz->id);
>   	if (result) {
>   		thermal_zone_destroy_device_groups(tz);
> Index: linux-pm/drivers/thermal/thermal_core.h
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.h
> +++ linux-pm/drivers/thermal/thermal_core.h
> @@ -95,7 +95,6 @@ struct thermal_governor {
>   			trip point.
>    * @prev_high_trip:	the above current temperature if you've crossed a
>   			passive trip point.
> - * @need_update:	if equals 1, thermal_zone_device_update needs to be invoked.
>    * @ops:	operations this &thermal_zone_device supports
>    * @tzp:	thermal zone parameters
>    * @governor:	pointer to the governor for this thermal zone
> @@ -129,7 +128,6 @@ struct thermal_zone_device {
>   	int passive;
>   	int prev_low_trip;
>   	int prev_high_trip;
> -	atomic_t need_update;
>   	struct thermal_zone_device_ops ops;
>   	struct thermal_zone_params *tzp;
>   	struct thermal_governor *governor;
> 
> 
> 


Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

[PATCH v2 11/12] thermal: core: Move lists of thermal instances to trip descriptors

[Rafael J. Wysocki]

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

In almost all places where a thermal zone's list of thermal instances
is walked, there is a check to match a specific trip point and it is
walked in vain whenever there are no cooling devices associated with
the given trip.

To address this, store the lists of thermal instances in trip point
descriptors instead of storing them in thermal zones and adjust all
code using those lists accordingly.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

This is a new iteration of

https://lore.kernel.org/linux-pm/2196792.irdbgypaU6@rjwysocki.net/

v1 -> v2:
   * Rebase.
   * Retain power_actor_is_valid() in the Power allocator governor, but
     drop its first parameter which is not needed any more.

---
 drivers/thermal/gov_bang_bang.c       |   11 ++++-----
 drivers/thermal/gov_fair_share.c      |   16 ++++---------
 drivers/thermal/gov_power_allocator.c |   40 +++++++++++++++++-----------------
 drivers/thermal/gov_step_wise.c       |   16 ++++++-------
 drivers/thermal/thermal_core.c        |   33 ++++++++++++++++------------
 drivers/thermal/thermal_core.h        |    5 +---
 drivers/thermal/thermal_helpers.c     |    5 ++--
 7 files changed, 62 insertions(+), 64 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -490,7 +490,7 @@ static void thermal_zone_device_check(st
 
 static void thermal_zone_device_init(struct thermal_zone_device *tz)
 {
-	struct thermal_instance *pos;
+	struct thermal_trip_desc *td;
 
 	INIT_DELAYED_WORK(&tz->poll_queue, thermal_zone_device_check);
 
@@ -498,8 +498,12 @@ static void thermal_zone_device_init(str
 	tz->passive = 0;
 	tz->prev_low_trip = -INT_MAX;
 	tz->prev_high_trip = INT_MAX;
-	list_for_each_entry(pos, &tz->thermal_instances, tz_node)
-		pos->initialized = false;
+	for_each_trip_desc(tz, td) {
+		struct thermal_instance *instance;
+
+		list_for_each_entry(instance, &td->thermal_instances, trip_node)
+			instance->initialized = false;
+	}
 }
 
 static void thermal_governor_trip_crossed(struct thermal_governor *governor,
@@ -764,12 +768,12 @@ struct thermal_zone_device *thermal_zone
  * Return: 0 on success, the proper error value otherwise.
  */
 static int thermal_bind_cdev_to_trip(struct thermal_zone_device *tz,
-				     const struct thermal_trip *trip,
+				     struct thermal_trip *trip,
 				     struct thermal_cooling_device *cdev,
 				     struct cooling_spec *cool_spec)
 {
-	struct thermal_instance *dev;
-	struct thermal_instance *pos;
+	struct thermal_trip_desc *td = trip_to_trip_desc(trip);
+	struct thermal_instance *dev, *instance;
 	bool upper_no_limit;
 	int result;
 
@@ -832,13 +836,13 @@ static int thermal_bind_cdev_to_trip(str
 		goto remove_trip_file;
 
 	mutex_lock(&cdev->lock);
-	list_for_each_entry(pos, &tz->thermal_instances, tz_node)
-		if (pos->trip == trip && pos->cdev == cdev) {
+	list_for_each_entry(instance, &td->thermal_instances, trip_node)
+		if (instance->cdev == cdev) {
 			result = -EEXIST;
 			break;
 		}
 	if (!result) {
-		list_add_tail(&dev->tz_node, &tz->thermal_instances);
+		list_add_tail(&dev->trip_node, &td->thermal_instances);
 		list_add_tail(&dev->cdev_node, &cdev->thermal_instances);
 
 		thermal_governor_update_tz(tz, THERMAL_TZ_BIND_CDEV);
@@ -871,15 +875,16 @@ free_mem:
  * This function is usually called in the thermal zone device .unbind callback.
  */
 static void thermal_unbind_cdev_from_trip(struct thermal_zone_device *tz,
-					  const struct thermal_trip *trip,
+					  struct thermal_trip *trip,
 					  struct thermal_cooling_device *cdev)
 {
+	struct thermal_trip_desc *td = trip_to_trip_desc(trip);
 	struct thermal_instance *pos, *next;
 
 	mutex_lock(&cdev->lock);
-	list_for_each_entry_safe(pos, next, &tz->thermal_instances, tz_node) {
-		if (pos->trip == trip && pos->cdev == cdev) {
-			list_del(&pos->tz_node);
+	list_for_each_entry_safe(pos, next, &td->thermal_instances, trip_node) {
+		if (pos->cdev == cdev) {
+			list_del(&pos->trip_node);
 			list_del(&pos->cdev_node);
 
 			thermal_governor_update_tz(tz, THERMAL_TZ_UNBIND_CDEV);
@@ -1460,7 +1465,6 @@ thermal_zone_device_register_with_trips(
 		}
 	}
 
-	INIT_LIST_HEAD(&tz->thermal_instances);
 	INIT_LIST_HEAD(&tz->node);
 	ida_init(&tz->ida);
 	mutex_init(&tz->lock);
@@ -1484,6 +1488,7 @@ thermal_zone_device_register_with_trips(
 	tz->num_trips = num_trips;
 	for_each_trip_desc(tz, td) {
 		td->trip = *trip++;
+		INIT_LIST_HEAD(&td->thermal_instances);
 		/*
 		 * Mark all thresholds as invalid to start with even though
 		 * this only matters for the trips that start as invalid and
Index: linux-pm/drivers/thermal/thermal_core.h
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.h
+++ linux-pm/drivers/thermal/thermal_core.h
@@ -30,6 +30,7 @@ struct thermal_trip_desc {
 	struct thermal_trip trip;
 	struct thermal_trip_attrs trip_attrs;
 	struct list_head notify_list_node;
+	struct list_head thermal_instances;
 	int notify_temp;
 	int threshold;
 };
@@ -99,7 +100,6 @@ struct thermal_governor {
  * @tzp:	thermal zone parameters
  * @governor:	pointer to the governor for this thermal zone
  * @governor_data:	private pointer for governor data
- * @thermal_instances:	list of &struct thermal_instance of this thermal zone
  * @ida:	&struct ida to generate unique id for this zone's cooling
  *		devices
  * @lock:	lock to protect thermal_instances list
@@ -132,7 +132,6 @@ struct thermal_zone_device {
 	struct thermal_zone_params *tzp;
 	struct thermal_governor *governor;
 	void *governor_data;
-	struct list_head thermal_instances;
 	struct ida ida;
 	struct mutex lock;
 	struct list_head node;
@@ -229,7 +228,7 @@ struct thermal_instance {
 	struct device_attribute attr;
 	char weight_attr_name[THERMAL_NAME_LENGTH];
 	struct device_attribute weight_attr;
-	struct list_head tz_node; /* node in tz->thermal_instances */
+	struct list_head trip_node; /* node in trip->thermal_instances */
 	struct list_head cdev_node; /* node in cdev->thermal_instances */
 	unsigned int weight; /* The weight of the cooling device */
 	bool upper_no_limit;
Index: linux-pm/drivers/thermal/gov_bang_bang.c
===================================================================
--- linux-pm.orig/drivers/thermal/gov_bang_bang.c
+++ linux-pm/drivers/thermal/gov_bang_bang.c
@@ -67,6 +67,7 @@ static void bang_bang_control(struct the
 			      const struct thermal_trip *trip,
 			      bool crossed_up)
 {
+	const struct thermal_trip_desc *td = trip_to_trip_desc(trip);
 	struct thermal_instance *instance;
 
 	lockdep_assert_held(&tz->lock);
@@ -75,10 +76,8 @@ static void bang_bang_control(struct the
 		thermal_zone_trip_id(tz, trip), trip->temperature,
 		tz->temperature, trip->hysteresis);
 
-	list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
-		if (instance->trip == trip)
-			bang_bang_set_instance_target(instance, crossed_up);
-	}
+	list_for_each_entry(instance, &td->thermal_instances, trip_node)
+		bang_bang_set_instance_target(instance, crossed_up);
 }
 
 static void bang_bang_manage(struct thermal_zone_device *tz)
@@ -104,8 +103,8 @@ static void bang_bang_manage(struct ther
 		 * to the thermal zone temperature and the trip point threshold.
 		 */
 		turn_on = tz->temperature >= td->threshold;
-		list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
-			if (!instance->initialized && instance->trip == trip)
+		list_for_each_entry(instance, &td->thermal_instances, trip_node) {
+			if (!instance->initialized)
 				bang_bang_set_instance_target(instance, turn_on);
 		}
 	}
Index: linux-pm/drivers/thermal/gov_fair_share.c
===================================================================
--- linux-pm.orig/drivers/thermal/gov_fair_share.c
+++ linux-pm/drivers/thermal/gov_fair_share.c
@@ -44,7 +44,7 @@ static int get_trip_level(struct thermal
 /**
  * fair_share_throttle - throttles devices associated with the given zone
  * @tz: thermal_zone_device
- * @trip: trip point
+ * @td: trip point descriptor
  * @trip_level: number of trips crossed by the zone temperature
  *
  * Throttling Logic: This uses three parameters to calculate the new
@@ -61,29 +61,23 @@ static int get_trip_level(struct thermal
  * new_state of cooling device = P3 * P2 * P1
  */
 static void fair_share_throttle(struct thermal_zone_device *tz,
-				const struct thermal_trip *trip,
+				const struct thermal_trip_desc *td,
 				int trip_level)
 {
 	struct thermal_instance *instance;
 	int total_weight = 0;
 	int nr_instances = 0;
 
-	list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
-		if (instance->trip != trip)
-			continue;
-
+	list_for_each_entry(instance, &td->thermal_instances, trip_node) {
 		total_weight += instance->weight;
 		nr_instances++;
 	}
 
-	list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
+	list_for_each_entry(instance, &td->thermal_instances, trip_node) {
 		struct thermal_cooling_device *cdev = instance->cdev;
 		u64 dividend;
 		u32 divisor;
 
-		if (instance->trip != trip)
-			continue;
-
 		dividend = trip_level;
 		dividend *= cdev->max_state;
 		divisor = tz->num_trips;
@@ -116,7 +110,7 @@ static void fair_share_manage(struct the
 		    trip->type == THERMAL_TRIP_HOT)
 			continue;
 
-		fair_share_throttle(tz, trip, trip_level);
+		fair_share_throttle(tz, td, trip_level);
 	}
 }
 
Index: linux-pm/drivers/thermal/gov_power_allocator.c
===================================================================
--- linux-pm.orig/drivers/thermal/gov_power_allocator.c
+++ linux-pm/drivers/thermal/gov_power_allocator.c
@@ -97,11 +97,9 @@ struct power_allocator_params {
 	struct power_actor *power;
 };
 
-static bool power_actor_is_valid(struct power_allocator_params *params,
-				 struct thermal_instance *instance)
+static bool power_actor_is_valid(struct thermal_instance *instance)
 {
-	return (instance->trip == params->trip_max &&
-		 cdev_is_power_actor(instance->cdev));
+	return cdev_is_power_actor(instance->cdev);
 }
 
 /**
@@ -118,13 +116,14 @@ static bool power_actor_is_valid(struct
 static u32 estimate_sustainable_power(struct thermal_zone_device *tz)
 {
 	struct power_allocator_params *params = tz->governor_data;
+	const struct thermal_trip_desc *td = trip_to_trip_desc(params->trip_max);
 	struct thermal_cooling_device *cdev;
 	struct thermal_instance *instance;
 	u32 sustainable_power = 0;
 	u32 min_power;
 
-	list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
-		if (!power_actor_is_valid(params, instance))
+	list_for_each_entry(instance, &td->thermal_instances, trip_node) {
+		if (!power_actor_is_valid(instance))
 			continue;
 
 		cdev = instance->cdev;
@@ -400,6 +399,7 @@ static void divvy_up_power(struct power_
 static void allocate_power(struct thermal_zone_device *tz, int control_temp)
 {
 	struct power_allocator_params *params = tz->governor_data;
+	const struct thermal_trip_desc *td = trip_to_trip_desc(params->trip_max);
 	unsigned int num_actors = params->num_actors;
 	struct power_actor *power = params->power;
 	struct thermal_cooling_device *cdev;
@@ -417,10 +417,10 @@ static void allocate_power(struct therma
 	/* Clean all buffers for new power estimations */
 	memset(power, 0, params->buffer_size);
 
-	list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
+	list_for_each_entry(instance, &td->thermal_instances, trip_node) {
 		struct power_actor *pa = &power[i];
 
-		if (!power_actor_is_valid(params, instance))
+		if (!power_actor_is_valid(instance))
 			continue;
 
 		cdev = instance->cdev;
@@ -454,10 +454,10 @@ static void allocate_power(struct therma
 		       power_range);
 
 	i = 0;
-	list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
+	list_for_each_entry(instance, &td->thermal_instances, trip_node) {
 		struct power_actor *pa = &power[i];
 
-		if (!power_actor_is_valid(params, instance))
+		if (!power_actor_is_valid(instance))
 			continue;
 
 		power_actor_set_power(instance->cdev, instance,
@@ -538,12 +538,13 @@ static void reset_pid_controller(struct
 static void allow_maximum_power(struct thermal_zone_device *tz)
 {
 	struct power_allocator_params *params = tz->governor_data;
+	const struct thermal_trip_desc *td = trip_to_trip_desc(params->trip_max);
 	struct thermal_cooling_device *cdev;
 	struct thermal_instance *instance;
 	u32 req_power;
 
-	list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
-		if (!power_actor_is_valid(params, instance))
+	list_for_each_entry(instance, &td->thermal_instances, trip_node) {
+		if (!power_actor_is_valid(instance))
 			continue;
 
 		cdev = instance->cdev;
@@ -581,13 +582,11 @@ static void allow_maximum_power(struct t
 static int check_power_actors(struct thermal_zone_device *tz,
 			      struct power_allocator_params *params)
 {
+	const struct thermal_trip_desc *td = trip_to_trip_desc(params->trip_max);
 	struct thermal_instance *instance;
 	int ret = 0;
 
-	list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
-		if (instance->trip != params->trip_max)
-			continue;
-
+	list_for_each_entry(instance, &td->thermal_instances, trip_node) {
 		if (!cdev_is_power_actor(instance->cdev)) {
 			dev_warn(&tz->device, "power_allocator: %s is not a power actor\n",
 				 instance->cdev->type);
@@ -635,14 +634,15 @@ static void power_allocator_update_tz(st
 				      enum thermal_notify_event reason)
 {
 	struct power_allocator_params *params = tz->governor_data;
+	const struct thermal_trip_desc *td = trip_to_trip_desc(params->trip_max);
 	struct thermal_instance *instance;
 	int num_actors = 0;
 
 	switch (reason) {
 	case THERMAL_TZ_BIND_CDEV:
 	case THERMAL_TZ_UNBIND_CDEV:
-		list_for_each_entry(instance, &tz->thermal_instances, tz_node)
-			if (power_actor_is_valid(params, instance))
+		list_for_each_entry(instance, &td->thermal_instances, trip_node)
+			if (power_actor_is_valid(instance))
 				num_actors++;
 
 		if (num_actors == params->num_actors)
@@ -652,8 +652,8 @@ static void power_allocator_update_tz(st
 		break;
 	case THERMAL_INSTANCE_WEIGHT_CHANGED:
 		params->total_weight = 0;
-		list_for_each_entry(instance, &tz->thermal_instances, tz_node)
-			if (power_actor_is_valid(params, instance))
+		list_for_each_entry(instance, &td->thermal_instances, trip_node)
+			if (power_actor_is_valid(instance))
 				params->total_weight += instance->weight;
 		break;
 	default:
Index: linux-pm/drivers/thermal/gov_step_wise.c
===================================================================
--- linux-pm.orig/drivers/thermal/gov_step_wise.c
+++ linux-pm/drivers/thermal/gov_step_wise.c
@@ -66,9 +66,10 @@ static unsigned long get_target_state(st
 }
 
 static void thermal_zone_trip_update(struct thermal_zone_device *tz,
-				     const struct thermal_trip *trip,
+				     const struct thermal_trip_desc *td,
 				     int trip_threshold)
 {
+	const struct thermal_trip *trip = &td->trip;
 	enum thermal_trend trend = get_tz_trend(tz, trip);
 	int trip_id = thermal_zone_trip_id(tz, trip);
 	struct thermal_instance *instance;
@@ -82,12 +83,9 @@ static void thermal_zone_trip_update(str
 	dev_dbg(&tz->device, "Trip%d[type=%d,temp=%d]:trend=%d,throttle=%d\n",
 		trip_id, trip->type, trip_threshold, trend, throttle);
 
-	list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
+	list_for_each_entry(instance, &td->thermal_instances, trip_node) {
 		int old_target;
 
-		if (instance->trip != trip)
-			continue;
-
 		old_target = instance->target;
 		instance->target = get_target_state(instance, trend, throttle);
 
@@ -127,11 +125,13 @@ static void step_wise_manage(struct ther
 		    trip->type == THERMAL_TRIP_HOT)
 			continue;
 
-		thermal_zone_trip_update(tz, trip, td->threshold);
+		thermal_zone_trip_update(tz, td, td->threshold);
 	}
 
-	list_for_each_entry(instance, &tz->thermal_instances, tz_node)
-		thermal_cdev_update(instance->cdev);
+	for_each_trip_desc(tz, td) {
+		list_for_each_entry(instance, &td->thermal_instances, trip_node)
+			thermal_cdev_update(instance->cdev);
+	}
 }
 
 static struct thermal_governor thermal_gov_step_wise = {
Index: linux-pm/drivers/thermal/thermal_helpers.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_helpers.c
+++ linux-pm/drivers/thermal/thermal_helpers.c
@@ -43,10 +43,11 @@ static bool thermal_instance_present(str
 				     struct thermal_cooling_device *cdev,
 				     const struct thermal_trip *trip)
 {
+	const struct thermal_trip_desc *td = trip_to_trip_desc(trip);
 	struct thermal_instance *ti;
 
-	list_for_each_entry(ti, &tz->thermal_instances, tz_node) {
-		if (ti->trip == trip && ti->cdev == cdev)
+	list_for_each_entry(ti, &td->thermal_instances, trip_node) {
+		if (ti->cdev == cdev)
 			return true;
 	}
 

Re: [PATCH v2 11/12] thermal: core: Move lists of thermal instances to trip descriptors

[Lukasz Luba]

On 10/4/24 20:39, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> In almost all places where a thermal zone's list of thermal instances
> is walked, there is a check to match a specific trip point and it is
> walked in vain whenever there are no cooling devices associated with
> the given trip.
> 
> To address this, store the lists of thermal instances in trip point
> descriptors instead of storing them in thermal zones and adjust all
> code using those lists accordingly.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> This is a new iteration of
> 
> https://lore.kernel.org/linux-pm/2196792.irdbgypaU6@rjwysocki.net/
> 
> v1 -> v2:
>     * Rebase.
>     * Retain power_actor_is_valid() in the Power allocator governor, but
>       drop its first parameter which is not needed any more.
> 
> ---
>   drivers/thermal/gov_bang_bang.c       |   11 ++++-----
>   drivers/thermal/gov_fair_share.c      |   16 ++++---------
>   drivers/thermal/gov_power_allocator.c |   40 +++++++++++++++++-----------------
>   drivers/thermal/gov_step_wise.c       |   16 ++++++-------
>   drivers/thermal/thermal_core.c        |   33 ++++++++++++++++------------
>   drivers/thermal/thermal_core.h        |    5 +---
>   drivers/thermal/thermal_helpers.c     |    5 ++--
>   7 files changed, 62 insertions(+), 64 deletions(-)
> 
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -490,7 +490,7 @@ static void thermal_zone_device_check(st
>   
>   static void thermal_zone_device_init(struct thermal_zone_device *tz)
>   {
> -	struct thermal_instance *pos;
> +	struct thermal_trip_desc *td;
>   
>   	INIT_DELAYED_WORK(&tz->poll_queue, thermal_zone_device_check);
>   
> @@ -498,8 +498,12 @@ static void thermal_zone_device_init(str
>   	tz->passive = 0;
>   	tz->prev_low_trip = -INT_MAX;
>   	tz->prev_high_trip = INT_MAX;
> -	list_for_each_entry(pos, &tz->thermal_instances, tz_node)
> -		pos->initialized = false;
> +	for_each_trip_desc(tz, td) {
> +		struct thermal_instance *instance;
> +
> +		list_for_each_entry(instance, &td->thermal_instances, trip_node)
> +			instance->initialized = false;
> +	}
>   }
>   
>   static void thermal_governor_trip_crossed(struct thermal_governor *governor,
> @@ -764,12 +768,12 @@ struct thermal_zone_device *thermal_zone
>    * Return: 0 on success, the proper error value otherwise.
>    */
>   static int thermal_bind_cdev_to_trip(struct thermal_zone_device *tz,
> -				     const struct thermal_trip *trip,
> +				     struct thermal_trip *trip,
>   				     struct thermal_cooling_device *cdev,
>   				     struct cooling_spec *cool_spec)
>   {
> -	struct thermal_instance *dev;
> -	struct thermal_instance *pos;
> +	struct thermal_trip_desc *td = trip_to_trip_desc(trip);
> +	struct thermal_instance *dev, *instance;
>   	bool upper_no_limit;
>   	int result;
>   
> @@ -832,13 +836,13 @@ static int thermal_bind_cdev_to_trip(str
>   		goto remove_trip_file;
>   
>   	mutex_lock(&cdev->lock);
> -	list_for_each_entry(pos, &tz->thermal_instances, tz_node)
> -		if (pos->trip == trip && pos->cdev == cdev) {
> +	list_for_each_entry(instance, &td->thermal_instances, trip_node)
> +		if (instance->cdev == cdev) {
>   			result = -EEXIST;
>   			break;
>   		}
>   	if (!result) {
> -		list_add_tail(&dev->tz_node, &tz->thermal_instances);
> +		list_add_tail(&dev->trip_node, &td->thermal_instances);
>   		list_add_tail(&dev->cdev_node, &cdev->thermal_instances);
>   
>   		thermal_governor_update_tz(tz, THERMAL_TZ_BIND_CDEV);
> @@ -871,15 +875,16 @@ free_mem:
>    * This function is usually called in the thermal zone device .unbind callback.
>    */
>   static void thermal_unbind_cdev_from_trip(struct thermal_zone_device *tz,
> -					  const struct thermal_trip *trip,
> +					  struct thermal_trip *trip,
>   					  struct thermal_cooling_device *cdev)
>   {
> +	struct thermal_trip_desc *td = trip_to_trip_desc(trip);
>   	struct thermal_instance *pos, *next;
>   
>   	mutex_lock(&cdev->lock);
> -	list_for_each_entry_safe(pos, next, &tz->thermal_instances, tz_node) {
> -		if (pos->trip == trip && pos->cdev == cdev) {
> -			list_del(&pos->tz_node);
> +	list_for_each_entry_safe(pos, next, &td->thermal_instances, trip_node) {
> +		if (pos->cdev == cdev) {
> +			list_del(&pos->trip_node);
>   			list_del(&pos->cdev_node);
>   
>   			thermal_governor_update_tz(tz, THERMAL_TZ_UNBIND_CDEV);
> @@ -1460,7 +1465,6 @@ thermal_zone_device_register_with_trips(
>   		}
>   	}
>   
> -	INIT_LIST_HEAD(&tz->thermal_instances);
>   	INIT_LIST_HEAD(&tz->node);
>   	ida_init(&tz->ida);
>   	mutex_init(&tz->lock);
> @@ -1484,6 +1488,7 @@ thermal_zone_device_register_with_trips(
>   	tz->num_trips = num_trips;
>   	for_each_trip_desc(tz, td) {
>   		td->trip = *trip++;
> +		INIT_LIST_HEAD(&td->thermal_instances);
>   		/*
>   		 * Mark all thresholds as invalid to start with even though
>   		 * this only matters for the trips that start as invalid and
> Index: linux-pm/drivers/thermal/thermal_core.h
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.h
> +++ linux-pm/drivers/thermal/thermal_core.h
> @@ -30,6 +30,7 @@ struct thermal_trip_desc {
>   	struct thermal_trip trip;
>   	struct thermal_trip_attrs trip_attrs;
>   	struct list_head notify_list_node;
> +	struct list_head thermal_instances;
>   	int notify_temp;
>   	int threshold;
>   };
> @@ -99,7 +100,6 @@ struct thermal_governor {
>    * @tzp:	thermal zone parameters
>    * @governor:	pointer to the governor for this thermal zone
>    * @governor_data:	private pointer for governor data
> - * @thermal_instances:	list of &struct thermal_instance of this thermal zone
>    * @ida:	&struct ida to generate unique id for this zone's cooling
>    *		devices
>    * @lock:	lock to protect thermal_instances list
> @@ -132,7 +132,6 @@ struct thermal_zone_device {
>   	struct thermal_zone_params *tzp;
>   	struct thermal_governor *governor;
>   	void *governor_data;
> -	struct list_head thermal_instances;
>   	struct ida ida;
>   	struct mutex lock;
>   	struct list_head node;
> @@ -229,7 +228,7 @@ struct thermal_instance {
>   	struct device_attribute attr;
>   	char weight_attr_name[THERMAL_NAME_LENGTH];
>   	struct device_attribute weight_attr;
> -	struct list_head tz_node; /* node in tz->thermal_instances */
> +	struct list_head trip_node; /* node in trip->thermal_instances */
>   	struct list_head cdev_node; /* node in cdev->thermal_instances */
>   	unsigned int weight; /* The weight of the cooling device */
>   	bool upper_no_limit;
> Index: linux-pm/drivers/thermal/gov_bang_bang.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/gov_bang_bang.c
> +++ linux-pm/drivers/thermal/gov_bang_bang.c
> @@ -67,6 +67,7 @@ static void bang_bang_control(struct the
>   			      const struct thermal_trip *trip,
>   			      bool crossed_up)
>   {
> +	const struct thermal_trip_desc *td = trip_to_trip_desc(trip);
>   	struct thermal_instance *instance;
>   
>   	lockdep_assert_held(&tz->lock);
> @@ -75,10 +76,8 @@ static void bang_bang_control(struct the
>   		thermal_zone_trip_id(tz, trip), trip->temperature,
>   		tz->temperature, trip->hysteresis);
>   
> -	list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
> -		if (instance->trip == trip)
> -			bang_bang_set_instance_target(instance, crossed_up);
> -	}
> +	list_for_each_entry(instance, &td->thermal_instances, trip_node)
> +		bang_bang_set_instance_target(instance, crossed_up);
>   }
>   
>   static void bang_bang_manage(struct thermal_zone_device *tz)
> @@ -104,8 +103,8 @@ static void bang_bang_manage(struct ther
>   		 * to the thermal zone temperature and the trip point threshold.
>   		 */
>   		turn_on = tz->temperature >= td->threshold;
> -		list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
> -			if (!instance->initialized && instance->trip == trip)
> +		list_for_each_entry(instance, &td->thermal_instances, trip_node) {
> +			if (!instance->initialized)
>   				bang_bang_set_instance_target(instance, turn_on);
>   		}
>   	}
> Index: linux-pm/drivers/thermal/gov_fair_share.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/gov_fair_share.c
> +++ linux-pm/drivers/thermal/gov_fair_share.c
> @@ -44,7 +44,7 @@ static int get_trip_level(struct thermal
>   /**
>    * fair_share_throttle - throttles devices associated with the given zone
>    * @tz: thermal_zone_device
> - * @trip: trip point
> + * @td: trip point descriptor
>    * @trip_level: number of trips crossed by the zone temperature
>    *
>    * Throttling Logic: This uses three parameters to calculate the new
> @@ -61,29 +61,23 @@ static int get_trip_level(struct thermal
>    * new_state of cooling device = P3 * P2 * P1
>    */
>   static void fair_share_throttle(struct thermal_zone_device *tz,
> -				const struct thermal_trip *trip,
> +				const struct thermal_trip_desc *td,
>   				int trip_level)
>   {
>   	struct thermal_instance *instance;
>   	int total_weight = 0;
>   	int nr_instances = 0;
>   
> -	list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
> -		if (instance->trip != trip)
> -			continue;
> -
> +	list_for_each_entry(instance, &td->thermal_instances, trip_node) {
>   		total_weight += instance->weight;
>   		nr_instances++;
>   	}
>   
> -	list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
> +	list_for_each_entry(instance, &td->thermal_instances, trip_node) {
>   		struct thermal_cooling_device *cdev = instance->cdev;
>   		u64 dividend;
>   		u32 divisor;
>   
> -		if (instance->trip != trip)
> -			continue;
> -
>   		dividend = trip_level;
>   		dividend *= cdev->max_state;
>   		divisor = tz->num_trips;
> @@ -116,7 +110,7 @@ static void fair_share_manage(struct the
>   		    trip->type == THERMAL_TRIP_HOT)
>   			continue;
>   
> -		fair_share_throttle(tz, trip, trip_level);
> +		fair_share_throttle(tz, td, trip_level);
>   	}
>   }
>   
> Index: linux-pm/drivers/thermal/gov_power_allocator.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/gov_power_allocator.c
> +++ linux-pm/drivers/thermal/gov_power_allocator.c
> @@ -97,11 +97,9 @@ struct power_allocator_params {
>   	struct power_actor *power;
>   };
>   
> -static bool power_actor_is_valid(struct power_allocator_params *params,
> -				 struct thermal_instance *instance)
> +static bool power_actor_is_valid(struct thermal_instance *instance)
>   {
> -	return (instance->trip == params->trip_max &&
> -		 cdev_is_power_actor(instance->cdev));
> +	return cdev_is_power_actor(instance->cdev);
>   }
>   
>   /**
> @@ -118,13 +116,14 @@ static bool power_actor_is_valid(struct
>   static u32 estimate_sustainable_power(struct thermal_zone_device *tz)
>   {
>   	struct power_allocator_params *params = tz->governor_data;
> +	const struct thermal_trip_desc *td = trip_to_trip_desc(params->trip_max);
>   	struct thermal_cooling_device *cdev;
>   	struct thermal_instance *instance;
>   	u32 sustainable_power = 0;
>   	u32 min_power;
>   
> -	list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
> -		if (!power_actor_is_valid(params, instance))
> +	list_for_each_entry(instance, &td->thermal_instances, trip_node) {
> +		if (!power_actor_is_valid(instance))
>   			continue;
>   
>   		cdev = instance->cdev;
> @@ -400,6 +399,7 @@ static void divvy_up_power(struct power_
>   static void allocate_power(struct thermal_zone_device *tz, int control_temp)
>   {
>   	struct power_allocator_params *params = tz->governor_data;
> +	const struct thermal_trip_desc *td = trip_to_trip_desc(params->trip_max);
>   	unsigned int num_actors = params->num_actors;
>   	struct power_actor *power = params->power;
>   	struct thermal_cooling_device *cdev;
> @@ -417,10 +417,10 @@ static void allocate_power(struct therma
>   	/* Clean all buffers for new power estimations */
>   	memset(power, 0, params->buffer_size);
>   
> -	list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
> +	list_for_each_entry(instance, &td->thermal_instances, trip_node) {
>   		struct power_actor *pa = &power[i];
>   
> -		if (!power_actor_is_valid(params, instance))
> +		if (!power_actor_is_valid(instance))
>   			continue;
>   
>   		cdev = instance->cdev;
> @@ -454,10 +454,10 @@ static void allocate_power(struct therma
>   		       power_range);
>   
>   	i = 0;
> -	list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
> +	list_for_each_entry(instance, &td->thermal_instances, trip_node) {
>   		struct power_actor *pa = &power[i];
>   
> -		if (!power_actor_is_valid(params, instance))
> +		if (!power_actor_is_valid(instance))
>   			continue;
>   
>   		power_actor_set_power(instance->cdev, instance,
> @@ -538,12 +538,13 @@ static void reset_pid_controller(struct
>   static void allow_maximum_power(struct thermal_zone_device *tz)
>   {
>   	struct power_allocator_params *params = tz->governor_data;
> +	const struct thermal_trip_desc *td = trip_to_trip_desc(params->trip_max);
>   	struct thermal_cooling_device *cdev;
>   	struct thermal_instance *instance;
>   	u32 req_power;
>   
> -	list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
> -		if (!power_actor_is_valid(params, instance))
> +	list_for_each_entry(instance, &td->thermal_instances, trip_node) {
> +		if (!power_actor_is_valid(instance))
>   			continue;
>   
>   		cdev = instance->cdev;
> @@ -581,13 +582,11 @@ static void allow_maximum_power(struct t
>   static int check_power_actors(struct thermal_zone_device *tz,
>   			      struct power_allocator_params *params)
>   {
> +	const struct thermal_trip_desc *td = trip_to_trip_desc(params->trip_max);
>   	struct thermal_instance *instance;
>   	int ret = 0;
>   
> -	list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
> -		if (instance->trip != params->trip_max)
> -			continue;
> -
> +	list_for_each_entry(instance, &td->thermal_instances, trip_node) {
>   		if (!cdev_is_power_actor(instance->cdev)) {
>   			dev_warn(&tz->device, "power_allocator: %s is not a power actor\n",
>   				 instance->cdev->type);
> @@ -635,14 +634,15 @@ static void power_allocator_update_tz(st
>   				      enum thermal_notify_event reason)
>   {
>   	struct power_allocator_params *params = tz->governor_data;
> +	const struct thermal_trip_desc *td = trip_to_trip_desc(params->trip_max);
>   	struct thermal_instance *instance;
>   	int num_actors = 0;
>   
>   	switch (reason) {
>   	case THERMAL_TZ_BIND_CDEV:
>   	case THERMAL_TZ_UNBIND_CDEV:
> -		list_for_each_entry(instance, &tz->thermal_instances, tz_node)
> -			if (power_actor_is_valid(params, instance))
> +		list_for_each_entry(instance, &td->thermal_instances, trip_node)
> +			if (power_actor_is_valid(instance))
>   				num_actors++;
>   
>   		if (num_actors == params->num_actors)
> @@ -652,8 +652,8 @@ static void power_allocator_update_tz(st
>   		break;
>   	case THERMAL_INSTANCE_WEIGHT_CHANGED:
>   		params->total_weight = 0;
> -		list_for_each_entry(instance, &tz->thermal_instances, tz_node)
> -			if (power_actor_is_valid(params, instance))
> +		list_for_each_entry(instance, &td->thermal_instances, trip_node)
> +			if (power_actor_is_valid(instance))
>   				params->total_weight += instance->weight;
>   		break;
>   	default:
> Index: linux-pm/drivers/thermal/gov_step_wise.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/gov_step_wise.c
> +++ linux-pm/drivers/thermal/gov_step_wise.c
> @@ -66,9 +66,10 @@ static unsigned long get_target_state(st
>   }
>   
>   static void thermal_zone_trip_update(struct thermal_zone_device *tz,
> -				     const struct thermal_trip *trip,
> +				     const struct thermal_trip_desc *td,
>   				     int trip_threshold)
>   {
> +	const struct thermal_trip *trip = &td->trip;
>   	enum thermal_trend trend = get_tz_trend(tz, trip);
>   	int trip_id = thermal_zone_trip_id(tz, trip);
>   	struct thermal_instance *instance;
> @@ -82,12 +83,9 @@ static void thermal_zone_trip_update(str
>   	dev_dbg(&tz->device, "Trip%d[type=%d,temp=%d]:trend=%d,throttle=%d\n",
>   		trip_id, trip->type, trip_threshold, trend, throttle);
>   
> -	list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
> +	list_for_each_entry(instance, &td->thermal_instances, trip_node) {
>   		int old_target;
>   
> -		if (instance->trip != trip)
> -			continue;
> -
>   		old_target = instance->target;
>   		instance->target = get_target_state(instance, trend, throttle);
>   
> @@ -127,11 +125,13 @@ static void step_wise_manage(struct ther
>   		    trip->type == THERMAL_TRIP_HOT)
>   			continue;
>   
> -		thermal_zone_trip_update(tz, trip, td->threshold);
> +		thermal_zone_trip_update(tz, td, td->threshold);
>   	}
>   
> -	list_for_each_entry(instance, &tz->thermal_instances, tz_node)
> -		thermal_cdev_update(instance->cdev);
> +	for_each_trip_desc(tz, td) {
> +		list_for_each_entry(instance, &td->thermal_instances, trip_node)
> +			thermal_cdev_update(instance->cdev);
> +	}
>   }
>   
>   static struct thermal_governor thermal_gov_step_wise = {
> Index: linux-pm/drivers/thermal/thermal_helpers.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_helpers.c
> +++ linux-pm/drivers/thermal/thermal_helpers.c
> @@ -43,10 +43,11 @@ static bool thermal_instance_present(str
>   				     struct thermal_cooling_device *cdev,
>   				     const struct thermal_trip *trip)
>   {
> +	const struct thermal_trip_desc *td = trip_to_trip_desc(trip);
>   	struct thermal_instance *ti;
>   
> -	list_for_each_entry(ti, &tz->thermal_instances, tz_node) {
> -		if (ti->trip == trip && ti->cdev == cdev)
> +	list_for_each_entry(ti, &td->thermal_instances, trip_node) {
> +		if (ti->cdev == cdev)
>   			return true;
>   	}
>   
> 
> 
> 

Indeed, it looks better to not loop-and-continue to
figure out the needed trip and instances.

Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

[PATCH v2 12/12] thermal: core: Pass trip descriptors to trip bind/unbind functions

[Rafael J. Wysocki]

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

The code is somewhat cleaner if struct thermal_trip_desc pointers are
passed to thermal_bind_cdev_to_trip(), thermal_unbind_cdev_from_trip(),
and print_bind_err_msg() instead of struct thermal_trip pointers, so
modify it accordingly.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

This is a new iteration of

https://lore.kernel.org/linux-pm/2954063.e9J7NaK4W3@rjwysocki.net/

v1 -> v2: Rebase and drop the leftover Subject: field from the preamble.

---
 drivers/thermal/thermal_core.c |   27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -757,9 +757,9 @@ struct thermal_zone_device *thermal_zone
 /**
  * thermal_bind_cdev_to_trip - bind a cooling device to a thermal zone
  * @tz:		pointer to struct thermal_zone_device
- * @trip:	trip point the cooling devices is associated with in this zone.
+ * @td:		descriptor of the trip point to bind @cdev to
  * @cdev:	pointer to struct thermal_cooling_device
- * @cool_spec:	cooling specification for @trip and @cdev
+ * @cool_spec:	cooling specification for the trip point and @cdev
  *
  * This interface function bind a thermal cooling device to the certain trip
  * point of a thermal zone device.
@@ -768,11 +768,10 @@ struct thermal_zone_device *thermal_zone
  * Return: 0 on success, the proper error value otherwise.
  */
 static int thermal_bind_cdev_to_trip(struct thermal_zone_device *tz,
-				     struct thermal_trip *trip,
+				     struct thermal_trip_desc *td,
 				     struct thermal_cooling_device *cdev,
 				     struct cooling_spec *cool_spec)
 {
-	struct thermal_trip_desc *td = trip_to_trip_desc(trip);
 	struct thermal_instance *dev, *instance;
 	bool upper_no_limit;
 	int result;
@@ -796,7 +795,7 @@ static int thermal_bind_cdev_to_trip(str
 		return -ENOMEM;
 
 	dev->cdev = cdev;
-	dev->trip = trip;
+	dev->trip = &td->trip;
 	dev->upper = cool_spec->upper;
 	dev->upper_no_limit = upper_no_limit;
 	dev->lower = cool_spec->lower;
@@ -867,7 +866,7 @@ free_mem:
 /**
  * thermal_unbind_cdev_from_trip - unbind a cooling device from a thermal zone.
  * @tz:		pointer to a struct thermal_zone_device.
- * @trip:	trip point the cooling devices is associated with in this zone.
+ * @td:		descriptor of the trip point to unbind @cdev from
  * @cdev:	pointer to a struct thermal_cooling_device.
  *
  * This interface function unbind a thermal cooling device from the certain
@@ -875,10 +874,9 @@ free_mem:
  * This function is usually called in the thermal zone device .unbind callback.
  */
 static void thermal_unbind_cdev_from_trip(struct thermal_zone_device *tz,
-					  struct thermal_trip *trip,
+					  struct thermal_trip_desc *td,
 					  struct thermal_cooling_device *cdev)
 {
-	struct thermal_trip_desc *td = trip_to_trip_desc(trip);
 	struct thermal_instance *pos, *next;
 
 	mutex_lock(&cdev->lock);
@@ -930,11 +928,11 @@ static struct class *thermal_class;
 
 static inline
 void print_bind_err_msg(struct thermal_zone_device *tz,
-			const struct thermal_trip *trip,
+			const struct thermal_trip_desc *td,
 			struct thermal_cooling_device *cdev, int ret)
 {
 	dev_err(&tz->device, "binding cdev %s to trip %d failed: %d\n",
-		cdev->type, thermal_zone_trip_id(tz, trip), ret);
+		cdev->type, thermal_zone_trip_id(tz, &td->trip), ret);
 }
 
 static bool __thermal_zone_cdev_bind(struct thermal_zone_device *tz,
@@ -947,7 +945,6 @@ static bool __thermal_zone_cdev_bind(str
 		return false;
 
 	for_each_trip_desc(tz, td) {
-		struct thermal_trip *trip = &td->trip;
 		struct cooling_spec c = {
 			.upper = THERMAL_NO_LIMIT,
 			.lower = THERMAL_NO_LIMIT,
@@ -955,12 +952,12 @@ static bool __thermal_zone_cdev_bind(str
 		};
 		int ret;
 
-		if (!tz->ops.should_bind(tz, trip, cdev, &c))
+		if (!tz->ops.should_bind(tz, &td->trip, cdev, &c))
 			continue;
 
-		ret = thermal_bind_cdev_to_trip(tz, trip, cdev, &c);
+		ret = thermal_bind_cdev_to_trip(tz, td, cdev, &c);
 		if (ret) {
-			print_bind_err_msg(tz, trip, cdev, ret);
+			print_bind_err_msg(tz, td, cdev, ret);
 			continue;
 		}
 
@@ -1279,7 +1276,7 @@ static void __thermal_zone_cdev_unbind(s
 	struct thermal_trip_desc *td;
 
 	for_each_trip_desc(tz, td)
-		thermal_unbind_cdev_from_trip(tz, &td->trip, cdev);
+		thermal_unbind_cdev_from_trip(tz, td, cdev);
 }
 
 static void thermal_zone_cdev_unbind(struct thermal_zone_device *tz,

Re: [PATCH v2 12/12] thermal: core: Pass trip descriptors to trip bind/unbind functions

[Lukasz Luba]

On 10/4/24 20:42, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> The code is somewhat cleaner if struct thermal_trip_desc pointers are
> passed to thermal_bind_cdev_to_trip(), thermal_unbind_cdev_from_trip(),
> and print_bind_err_msg() instead of struct thermal_trip pointers, so
> modify it accordingly.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> This is a new iteration of
> 
> https://lore.kernel.org/linux-pm/2954063.e9J7NaK4W3@rjwysocki.net/
> 
> v1 -> v2: Rebase and drop the leftover Subject: field from the preamble.
> 
> ---
>   drivers/thermal/thermal_core.c |   27 ++++++++++++---------------
>   1 file changed, 12 insertions(+), 15 deletions(-)
> 
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -757,9 +757,9 @@ struct thermal_zone_device *thermal_zone
>   /**
>    * thermal_bind_cdev_to_trip - bind a cooling device to a thermal zone
>    * @tz:		pointer to struct thermal_zone_device
> - * @trip:	trip point the cooling devices is associated with in this zone.
> + * @td:		descriptor of the trip point to bind @cdev to
>    * @cdev:	pointer to struct thermal_cooling_device
> - * @cool_spec:	cooling specification for @trip and @cdev
> + * @cool_spec:	cooling specification for the trip point and @cdev
>    *
>    * This interface function bind a thermal cooling device to the certain trip
>    * point of a thermal zone device.
> @@ -768,11 +768,10 @@ struct thermal_zone_device *thermal_zone
>    * Return: 0 on success, the proper error value otherwise.
>    */
>   static int thermal_bind_cdev_to_trip(struct thermal_zone_device *tz,
> -				     struct thermal_trip *trip,
> +				     struct thermal_trip_desc *td,
>   				     struct thermal_cooling_device *cdev,
>   				     struct cooling_spec *cool_spec)
>   {
> -	struct thermal_trip_desc *td = trip_to_trip_desc(trip);
>   	struct thermal_instance *dev, *instance;
>   	bool upper_no_limit;
>   	int result;
> @@ -796,7 +795,7 @@ static int thermal_bind_cdev_to_trip(str
>   		return -ENOMEM;
>   
>   	dev->cdev = cdev;
> -	dev->trip = trip;
> +	dev->trip = &td->trip;
>   	dev->upper = cool_spec->upper;
>   	dev->upper_no_limit = upper_no_limit;
>   	dev->lower = cool_spec->lower;
> @@ -867,7 +866,7 @@ free_mem:
>   /**
>    * thermal_unbind_cdev_from_trip - unbind a cooling device from a thermal zone.
>    * @tz:		pointer to a struct thermal_zone_device.
> - * @trip:	trip point the cooling devices is associated with in this zone.
> + * @td:		descriptor of the trip point to unbind @cdev from
>    * @cdev:	pointer to a struct thermal_cooling_device.
>    *
>    * This interface function unbind a thermal cooling device from the certain
> @@ -875,10 +874,9 @@ free_mem:
>    * This function is usually called in the thermal zone device .unbind callback.
>    */
>   static void thermal_unbind_cdev_from_trip(struct thermal_zone_device *tz,
> -					  struct thermal_trip *trip,
> +					  struct thermal_trip_desc *td,
>   					  struct thermal_cooling_device *cdev)
>   {
> -	struct thermal_trip_desc *td = trip_to_trip_desc(trip);
>   	struct thermal_instance *pos, *next;
>   
>   	mutex_lock(&cdev->lock);
> @@ -930,11 +928,11 @@ static struct class *thermal_class;
>   
>   static inline
>   void print_bind_err_msg(struct thermal_zone_device *tz,
> -			const struct thermal_trip *trip,
> +			const struct thermal_trip_desc *td,
>   			struct thermal_cooling_device *cdev, int ret)
>   {
>   	dev_err(&tz->device, "binding cdev %s to trip %d failed: %d\n",
> -		cdev->type, thermal_zone_trip_id(tz, trip), ret);
> +		cdev->type, thermal_zone_trip_id(tz, &td->trip), ret);
>   }
>   
>   static bool __thermal_zone_cdev_bind(struct thermal_zone_device *tz,
> @@ -947,7 +945,6 @@ static bool __thermal_zone_cdev_bind(str
>   		return false;
>   
>   	for_each_trip_desc(tz, td) {
> -		struct thermal_trip *trip = &td->trip;
>   		struct cooling_spec c = {
>   			.upper = THERMAL_NO_LIMIT,
>   			.lower = THERMAL_NO_LIMIT,
> @@ -955,12 +952,12 @@ static bool __thermal_zone_cdev_bind(str
>   		};
>   		int ret;
>   
> -		if (!tz->ops.should_bind(tz, trip, cdev, &c))
> +		if (!tz->ops.should_bind(tz, &td->trip, cdev, &c))
>   			continue;
>   
> -		ret = thermal_bind_cdev_to_trip(tz, trip, cdev, &c);
> +		ret = thermal_bind_cdev_to_trip(tz, td, cdev, &c);
>   		if (ret) {
> -			print_bind_err_msg(tz, trip, cdev, ret);
> +			print_bind_err_msg(tz, td, cdev, ret);
>   			continue;
>   		}
>   
> @@ -1279,7 +1276,7 @@ static void __thermal_zone_cdev_unbind(s
>   	struct thermal_trip_desc *td;
>   
>   	for_each_trip_desc(tz, td)
> -		thermal_unbind_cdev_from_trip(tz, &td->trip, cdev);
> +		thermal_unbind_cdev_from_trip(tz, td, cdev);
>   }
>   
>   static void thermal_zone_cdev_unbind(struct thermal_zone_device *tz,
> 
> 
> 


Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

Re: [PATCH v2 00/12] thermal: core: Fixes and cleanups, mostly related to thermal zone init and exit

[Rafael J. Wysocki]

On Fri, Oct 4, 2024 at 10:11 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> Hi Everyone,
>
> After posting the two series of thermal core patches for 6.13:
>
> https://lore.kernel.org/linux-pm/4920970.GXAFRqVoOG@rjwysocki.net/
>
> and
>
> https://lore.kernel.org/linux-pm/6100907.lOV4Wx5bFT@rjwysocki.net/
>
> before the 6.12 merge window, I have decided to reorder the changes included in
> these series, so that fixes and more significant cleanups (for example, changing
> they layout of data structures) go first, followed by the changes related to
> using guards for locking, and the optimization involving sorted lists becomes
> the last piece.
>
> This series is the first part and the majority of patches in it come from the
> second (RFC) series mentioned above.  Of course, they needed to be rebased to
> be applied in the new order.  It is on top of 6.12-rc1 with
>
> https://lore.kernel.org/linux-pm/12549318.O9o76ZdvQC@rjwysocki.net/
>
> applied and it will be added to my thermal-core-testing branch.  It is in v2
> to start with because all of the patches in it have already been posted in
> some form.
>
> The first 10 patches fix some potential issues related to thermal zone
> initialization and exit (for example, user space may start to interact with
> a thermal zone during its initialization before it's ready and system suspend
> taking place at a wrong time may skip a new thermal zone so it is not suspended)
> and do some cleanups related to that.  This concludes with the removal of the
> need_update field from struct thermal_zone_device.
>
> The last two patches move lists of thermal instances from thermal zones to
> trip point descriptors and clean up some code on top of that.
>
> Please refer to the individual patch changelogs for details.

This material is now present in the thermal-core-testing and
thermal-core-experimental branches in linux-pm.git.

Re: [PATCH v2 00/12] thermal: core: Fixes and cleanups, mostly related to thermal zone init and exit

[Rafael J. Wysocki]

On Fri, Oct 11, 2024 at 8:50 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Fri, Oct 4, 2024 at 10:11 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> >
> > Hi Everyone,
> >
> > After posting the two series of thermal core patches for 6.13:
> >
> > https://lore.kernel.org/linux-pm/4920970.GXAFRqVoOG@rjwysocki.net/
> >
> > and
> >
> > https://lore.kernel.org/linux-pm/6100907.lOV4Wx5bFT@rjwysocki.net/
> >
> > before the 6.12 merge window, I have decided to reorder the changes included in
> > these series, so that fixes and more significant cleanups (for example, changing
> > they layout of data structures) go first, followed by the changes related to
> > using guards for locking, and the optimization involving sorted lists becomes
> > the last piece.
> >
> > This series is the first part and the majority of patches in it come from the
> > second (RFC) series mentioned above.  Of course, they needed to be rebased to
> > be applied in the new order.  It is on top of 6.12-rc1 with
> >
> > https://lore.kernel.org/linux-pm/12549318.O9o76ZdvQC@rjwysocki.net/
> >
> > applied and it will be added to my thermal-core-testing branch.  It is in v2
> > to start with because all of the patches in it have already been posted in
> > some form.
> >
> > The first 10 patches fix some potential issues related to thermal zone
> > initialization and exit (for example, user space may start to interact with
> > a thermal zone during its initialization before it's ready and system suspend
> > taking place at a wrong time may skip a new thermal zone so it is not suspended)
> > and do some cleanups related to that.  This concludes with the removal of the
> > need_update field from struct thermal_zone_device.
> >
> > The last two patches move lists of thermal instances from thermal zones to
> > trip point descriptors and clean up some code on top of that.
> >
> > Please refer to the individual patch changelogs for details.
>
> This material is now present in the thermal-core-testing and
> thermal-core-experimental branches in linux-pm.git.

I gather that it is not controversial and it has been around for quite
a while, and it was discussed during the PM+TC session at the LPC, so
I've just applied it for 6.13.

Re: [PATCH v2 00/12] thermal: core: Fixes and cleanups, mostly related to thermal zone init and exit

[Lukasz Luba]

Hi Rafael,

On 10/21/24 12:05, Rafael J. Wysocki wrote:
> On Fri, Oct 11, 2024 at 8:50 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>>
>> On Fri, Oct 4, 2024 at 10:11 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>>>
>>> Hi Everyone,
>>>
>>> After posting the two series of thermal core patches for 6.13:
>>>
>>> https://lore.kernel.org/linux-pm/4920970.GXAFRqVoOG@rjwysocki.net/
>>>
>>> and
>>>
>>> https://lore.kernel.org/linux-pm/6100907.lOV4Wx5bFT@rjwysocki.net/
>>>
>>> before the 6.12 merge window, I have decided to reorder the changes included in
>>> these series, so that fixes and more significant cleanups (for example, changing
>>> they layout of data structures) go first, followed by the changes related to
>>> using guards for locking, and the optimization involving sorted lists becomes
>>> the last piece.
>>>
>>> This series is the first part and the majority of patches in it come from the
>>> second (RFC) series mentioned above.  Of course, they needed to be rebased to
>>> be applied in the new order.  It is on top of 6.12-rc1 with
>>>
>>> https://lore.kernel.org/linux-pm/12549318.O9o76ZdvQC@rjwysocki.net/
>>>
>>> applied and it will be added to my thermal-core-testing branch.  It is in v2
>>> to start with because all of the patches in it have already been posted in
>>> some form.
>>>
>>> The first 10 patches fix some potential issues related to thermal zone
>>> initialization and exit (for example, user space may start to interact with
>>> a thermal zone during its initialization before it's ready and system suspend
>>> taking place at a wrong time may skip a new thermal zone so it is not suspended)
>>> and do some cleanups related to that.  This concludes with the removal of the
>>> need_update field from struct thermal_zone_device.
>>>
>>> The last two patches move lists of thermal instances from thermal zones to
>>> trip point descriptors and clean up some code on top of that.
>>>
>>> Please refer to the individual patch changelogs for details.
>>
>> This material is now present in the thermal-core-testing and
>> thermal-core-experimental branches in linux-pm.git.
> 
> I gather that it is not controversial and it has been around for quite
> a while, and it was discussed during the PM+TC session at the LPC, so
> I've just applied it for 6.13.

I hope it wasn't too late. The patch set looks good and I have
added my reviewed tags.

Regards,
Lukasz

Re: [PATCH v2 00/12] thermal: core: Fixes and cleanups, mostly related to thermal zone init and exit

[Rafael J. Wysocki]

Hi Lukasz,

On Tue, Oct 22, 2024 at 12:44 AM Lukasz Luba <lukasz.luba@arm.com> wrote:
>
> Hi Rafael,
>
> On 10/21/24 12:05, Rafael J. Wysocki wrote:
> > On Fri, Oct 11, 2024 at 8:50 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
> >>
> >> On Fri, Oct 4, 2024 at 10:11 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> >>>
> >>> Hi Everyone,
> >>>
> >>> After posting the two series of thermal core patches for 6.13:
> >>>
> >>> https://lore.kernel.org/linux-pm/4920970.GXAFRqVoOG@rjwysocki.net/
> >>>
> >>> and
> >>>
> >>> https://lore.kernel.org/linux-pm/6100907.lOV4Wx5bFT@rjwysocki.net/
> >>>
> >>> before the 6.12 merge window, I have decided to reorder the changes included in
> >>> these series, so that fixes and more significant cleanups (for example, changing
> >>> they layout of data structures) go first, followed by the changes related to
> >>> using guards for locking, and the optimization involving sorted lists becomes
> >>> the last piece.
> >>>
> >>> This series is the first part and the majority of patches in it come from the
> >>> second (RFC) series mentioned above.  Of course, they needed to be rebased to
> >>> be applied in the new order.  It is on top of 6.12-rc1 with
> >>>
> >>> https://lore.kernel.org/linux-pm/12549318.O9o76ZdvQC@rjwysocki.net/
> >>>
> >>> applied and it will be added to my thermal-core-testing branch.  It is in v2
> >>> to start with because all of the patches in it have already been posted in
> >>> some form.
> >>>
> >>> The first 10 patches fix some potential issues related to thermal zone
> >>> initialization and exit (for example, user space may start to interact with
> >>> a thermal zone during its initialization before it's ready and system suspend
> >>> taking place at a wrong time may skip a new thermal zone so it is not suspended)
> >>> and do some cleanups related to that.  This concludes with the removal of the
> >>> need_update field from struct thermal_zone_device.
> >>>
> >>> The last two patches move lists of thermal instances from thermal zones to
> >>> trip point descriptors and clean up some code on top of that.
> >>>
> >>> Please refer to the individual patch changelogs for details.
> >>
> >> This material is now present in the thermal-core-testing and
> >> thermal-core-experimental branches in linux-pm.git.
> >
> > I gather that it is not controversial and it has been around for quite
> > a while, and it was discussed during the PM+TC session at the LPC, so
> > I've just applied it for 6.13.
>
> I hope it wasn't too late. The patch set looks good and I have
> added my reviewed tags.

No, it wasn't, thank you!