2016-03-11 07:08:42

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [BUG] Device unbound in a resumed state

Hi,


Could be related (the same?) with [0].

I have a driver (hwrng/exynos-rng) which in probe does:
pm_runtime_set_autosuspend_delay(&pdev->dev, EXYNOS_AUTOSUSPEND_DELAY);
pm_runtime_use_autosuspend(&pdev->dev);
pm_runtime_enable(&pdev->dev);

and in remove:
pm_runtime_disable(&pdev->dev)

Just before unbinding in __device_release_driver() the device is resumed
but unfortunately not suspended later. I mean the
__device_release_driver()->pm_runtime_put_sync() does not trigger
runtime suspend.

This leads to leaving the device in active state (e.g. clocks enabled).

It does not happen after removal of autosuspend. Also runtime suspend
happens after very fast unbind-bind.


Best regards,
Krzysztof

[0] PM regression with commit 5de85b9d57ab PM runtime re-init in v4.5-rc1
http://comments.gmane.org/gmane.linux.power-management.general/70690


2016-03-11 18:46:24

by Alan Stern

[permalink] [raw]
Subject: Re: [BUG] Device unbound in a resumed state

On Fri, 11 Mar 2016, Krzysztof Kozlowski wrote:

> Hi,
>
>
> Could be related (the same?) with [0].
>
> I have a driver (hwrng/exynos-rng) which in probe does:
> pm_runtime_set_autosuspend_delay(&pdev->dev, EXYNOS_AUTOSUSPEND_DELAY);
> pm_runtime_use_autosuspend(&pdev->dev);
> pm_runtime_enable(&pdev->dev);
>
> and in remove:
> pm_runtime_disable(&pdev->dev)

But not pm_runtime_dont_use_autosuspend()?

Why disable runtime PM if you want the runtime-PM methods to put the
device into a low-power state?

> Just before unbinding in __device_release_driver() the device is resumed
> but unfortunately not suspended later. I mean the
> __device_release_driver()->pm_runtime_put_sync() does not trigger
> runtime suspend.

Because autosuspend is still in use at this point.

> This leads to leaving the device in active state (e.g. clocks enabled).
>
> It does not happen after removal of autosuspend. Also runtime suspend
> happens after very fast unbind-bind.

Overall it sounds like the system is behaving the way it is supposed
to.

But maybe we should make pm_runtime_use_autosuspend() call
pm_runtime_mark_last_busy(), to avoid the unbind - bind - immediate
autosuspend behavior.

Alan Stern

2016-03-12 05:25:07

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [BUG] Device unbound in a resumed state

2016-03-12 3:46 GMT+09:00 Alan Stern <[email protected]>:
> On Fri, 11 Mar 2016, Krzysztof Kozlowski wrote:
>
>> Hi,
>>
>>
>> Could be related (the same?) with [0].
>>
>> I have a driver (hwrng/exynos-rng) which in probe does:
>> pm_runtime_set_autosuspend_delay(&pdev->dev, EXYNOS_AUTOSUSPEND_DELAY);
>> pm_runtime_use_autosuspend(&pdev->dev);
>> pm_runtime_enable(&pdev->dev);
>>
>> and in remove:
>> pm_runtime_disable(&pdev->dev)
>
> But not pm_runtime_dont_use_autosuspend()?

Ahh, that one is missing. Thanks!

>
> Why disable runtime PM if you want the runtime-PM methods to put the
> device into a low-power state?

I don't want to play with runtime PM anymore at this time. I am
removing the device so I am cleaning what was done in probe. Without
the pm_runtime_disable() the next probe of device will trigger error
of unbalanced enable:
https://lkml.org/lkml/2016/3/11/59

>
>> Just before unbinding in __device_release_driver() the device is resumed
>> but unfortunately not suspended later. I mean the
>> __device_release_driver()->pm_runtime_put_sync() does not trigger
>> runtime suspend.
>
> Because autosuspend is still in use at this point.
>
>> This leads to leaving the device in active state (e.g. clocks enabled).
>>
>> It does not happen after removal of autosuspend. Also runtime suspend
>> happens after very fast unbind-bind.
>
> Overall it sounds like the system is behaving the way it is supposed
> to.
>
> But maybe we should make pm_runtime_use_autosuspend() call
> pm_runtime_mark_last_busy(), to avoid the unbind - bind - immediate
> autosuspend behavior.

The need of disabling autosuspend was not quite obvious to me and I
did not find information about it in documentation. However now it
makes sense... I'll send a patch updating the docs.

Thank you for feedback!

Best regards,
Krzysztof