Messages in this thread | | | From | "Rafael J. Wysocki" <> | Date | Thu, 5 Nov 2020 18:42:20 +0100 | Subject | Re: [PATCH] PM: runtime: Use pmruntime sync variant to put suppliers |
| |
On Thu, Oct 8, 2020 at 3:08 AM Stanimir Varbanov <stanimir.varbanov@linaro.org> 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@linaro.org> 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.
| |