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

Pranjal Shrivastava
2024-10-10T15:56:35Z

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]