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

[Lukasz Luba <lukasz.luba@arm.com>]

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>