Re: [PATCH 22/51] iommu/arm-smmu: Switch to __pm_runtime_put_autosuspend()

[Pranjal Shrivastava <praan@google.com>]

On Fri, Oct 04, 2024 at 12:41:23PM +0300, Sakari Ailus wrote:
> pm_runtime_put_autosuspend() will soon be changed to include a call to
> pm_runtime_mark_last_busy(). This patch switches the current users to
> __pm_runtime_put_autosuspend() which will continue to have the
> functionality of old pm_runtime_put_autosuspend().
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/iommu/arm/arm-smmu/arm-smmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index 8321962b3714..cad02d5dc6d2 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -79,7 +79,7 @@ static inline int arm_smmu_rpm_get(struct arm_smmu_device *smmu)
>  static inline void arm_smmu_rpm_put(struct arm_smmu_device *smmu)
>  {
>  	if (pm_runtime_enabled(smmu->dev))
> -		pm_runtime_put_autosuspend(smmu->dev);
> +		__pm_runtime_put_autosuspend(smmu->dev);
>  }

Seems like a straightforward change as a result of [1].
Although, I had a few things to discuss:

1. The `rpm_resume` in drivers/base/power/runtime.c seems to call
`pm_runtime_mark_last_busy` in case the ->runtime_resume callback
returned successfully. In such a case, why would we want to move
`pm_runtime_mark_last_busy` within `pm_runtime_put_autosuspend` ?

2. In the arm-smmu driver, we seem to rely on the rpm_resume to call
`pm_runtime_mark_last_busy` as a part of the ->runtime_resume callback.
The only other case, where we might wanna `*mark_last_busy` is if we
want the autosuspend timer to be re-started in case of a failed suspend.
However, the `arm_smmu_runtime_suspend` doesn't return errno in any case
hence, I don't see any other case where we'd benefit from using
`mark_last_busy` in the arm-smmu driver. 

On the other hand, I don't see a problem with using it either :)
Any thoughts Will/Rob/Robin?

>  
>  static void arm_smmu_rpm_use_autosuspend(struct arm_smmu_device *smmu)
> -- 
> 2.39.5
> 

Apart from the above discussion, for this patch alone:
Reviewed-by: Pranjal Shrivastava <praan@google.com>

Thanks,
Pranjal

[1]
 https://lore.kernel.org/all/20240109133639.111210-1-sakari.ailus@linux.intel.com/