On Wed, 5 May 2021 11:42:13 +0200
Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx> wrote:
> The pm_runtime_get_sync() internally increments the
> dev->power.usage_count without decrementing it, even on errors.
>
> On some places, this is ok, but on others the usage count
> ended being unbalanced on failures.
>
> Replace it by the new pm_runtime_resume_and_get(), introduced by:
> commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
> in order to properly decrement the usage counter, avoiding
> a potential PM usage counter leak.
>
> As a bonus, such function always return zero on success. So,
> some code can be simplified.
>
> Reviewed-by: Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx>
Might be nice to call out the two odd cases below.
> diff --git a/drivers/media/platform/exynos4-is/mipi-csis.c b/drivers/media/platform/exynos4-is/mipi-csis.c
> index 1aac167abb17..ebf39c856894 100644
> --- a/drivers/media/platform/exynos4-is/mipi-csis.c
> +++ b/drivers/media/platform/exynos4-is/mipi-csis.c
> @@ -494,7 +494,7 @@ static int s5pcsis_s_power(struct v4l2_subdev *sd, int on)
> struct device *dev = &state->pdev->dev;
>
> if (on)
> - return pm_runtime_get_sync(dev);
> + return pm_runtime_resume_and_get(dev);
>
> return pm_runtime_put_sync(dev);
> }
> @@ -509,11 +509,9 @@ static int s5pcsis_s_stream(struct v4l2_subdev *sd, int enable)
>
> if (enable) {
> s5pcsis_clear_counters(state);
> - ret = pm_runtime_get_sync(&state->pdev->dev);
> - if (ret && ret != 1) {
Perhaps add something to the description on this less common case?
> - pm_runtime_put_noidle(&state->pdev->dev);
> + ret = pm_runtime_resume_and_get(&state->pdev->dev);
> + if (ret < 0)
> return ret;
> - }
> }
>
> mutex_lock(&state->lock);
> @@ -535,7 +533,7 @@ static int s5pcsis_s_stream(struct v4l2_subdev *sd, int enable)
> if (!enable)
> pm_runtime_put(&state->pdev->dev);
>
> - return ret == 1 ? 0 : ret;
> + return ret;
> }
>
> static int s5pcsis_enum_mbus_code(struct v4l2_subdev *sd,
