Re: [PATCH] PM: runtime: Use pmruntime sync variant to put suppliers

[Rafael J. Wysocki]

On Thu, Oct 8, 2020 at 3:08 AM Stanimir Varbanov
<stanimir.varbanov@xxxxxxxxxx> wrote:
>
> Hi Rafael,
>
> On 10/7/20 5:37 PM, Rafael J. Wysocki wrote:
> > On Wed, Oct 7, 2020 at 2:20 AM Stanimir Varbanov
> > <stanimir.varbanov@xxxxxxxxxx> wrote:
> >>
> >> Calling pm_runtime_put_sync over a device with suppliers with device
> >> link flags PM_RUNTIME | RPM_ACTIVE it is observed that the supplier
> >> is not put (turned off) at the end, but instead put asynchronously.
> >
> > Yes, that's by design.
> >
> >> In some case This could lead to issues for the callers which expects
> >> that the pmruntime sync variants should also put the suppliers
> >> synchronously.
> >
> > Why would anyone expect that to happen?
>
> It is logical to me that when I call pm_runtime_put_sync the device and
> its suppliers are put synchronously. If I want to put device and its
> suppliers asynchronously I'd use pm_runtime_put. Is that wrong assumption?

The handling of suppliers is analogous to the handling of parents and
the parents are suspended asynchronously when a child suspends.

The difference between _put() and _put_sync() only applies to the
device passed in as the argument.

> >> Also the opposite rpm_get_suppliers is already using pmruntime _sync
> >> variant of the API.
> >
> > Yes, it does, because that is necessary.
> >
> >> Correct this by changing pmruntime_put to pmruntime_put_sync in
> >> rpm_put_suppliers.
> >
> > It is not a correction, but a change in behavior without good enough
> > rationale, as it stands.
>
> In my driver case I want to deal with a recovery of a crash in remote
> processor (the remote processor is used to control and program hardware
> blocks and also to communicate with host processor through shared
> memory). To restart the remote processor I have to disable clocks and
> turn off few power domains (one of the power domains is made a supplier
> of my main device) in order to complete the cold-boot.

PM-runtime doesn't guarantee you the behavior that you'd like to see here.

> The problem I'm facing with this design is that when I call
> runtime_put_sync (to disable device's clocks and turn off power domain)
> the clocks are disabled (part of pmruntime_suspend callback) but the
> pmdomain (the device supplier) is not turned synchronously. I workaround
> this by checking the supplier device via pm_runtime_active() until it
> becomes inactive and the continue with rest of the steps.

This is not a use case for PM-runtime at all.

PM-runtime is all about going low-power opportunistically, whereas you
want to enforce power down.

> From my point of view this check for supplier activity should be part of
> pmruntime API.

But the API is not what you should be using for this purpose in the first place.