Théo Lebrun <theo.lebrun@xxxxxxxxxxx> writes: > Hi Roger, > > On Fri Nov 17, 2023 at 12:51 PM CET, Roger Quadros wrote: >> On 17/11/2023 12:17, Théo Lebrun wrote: >> > On Thu Nov 16, 2023 at 10:44 PM CET, Roger Quadros wrote: >> >> On 16/11/2023 20:56, Théo Lebrun wrote: >> >>> On Thu Nov 16, 2023 at 1:40 PM CET, Roger Quadros wrote: >> >>>> On 15/11/2023 17:02, Théo Lebrun wrote: >> >>>>> On Wed Nov 15, 2023 at 12:37 PM CET, Roger Quadros wrote: >> >>>>>> You might want to check suspend/resume ops in cdns3-plat and >> >>>>>> do something similar here. >> >>>>> >> >>>>> I'm unsure what you are referring to specifically in cdns3-plat? >> >>>> >> >>>> What I meant is, calling pm_runtime_get/put() from system suspend/resume >> >>>> hooks doesn't seem right. >> >>>> >> >>>> How about using something like pm_runtime_forbid(dev) on devices which >> >>>> loose USB context on runtime suspend e.g. J7200. >> >>>> So at probe we can get rid of the pm_runtime_get_sync() call. >> >>> >> >>> What is the goal of enabling PM runtime to then block (ie forbid) it in >> >>> its enabled state until system suspend? >> >> >> >> If USB controller retains context on runtime_suspend on some platforms >> >> then we don't want to forbid PM runtime. >> > >> > What's the point of runtime PM if nothing is done based on it? This is >> > the current behavior of the driver. The point is to signal to the power domain the device is in that it can power on/off. These IP blocks are (re)used on many different SoCs, so the driver should not make any assumptions about what power domain it is in (if any.) >> Even if driver doesn't have runtime_suspend/resume hooks, wouldn't >> the USB Power domain turn off if we enable runtime PM and allow runtime >> autosuspend and all children have runtime suspended? > > That cannot be the currently desired behavior as it would require a > runtime_resume implementation that restores this wrapper to its desired > state. Or, for this USB IP block to be in a power domain that has memory retention, in which case the power domain can still go off to save power, but not lose context. > It could however be something that could be implemented. It would be a > matter of enabling PM runtime and that is it in the probe. No need to > even init the hardware in the probe. Then the runtime_resume > implementation would call the new cdns_ti_init_hw. This is the way. > This is what the cdns3-imx wrapper is doing in a way, though what they > need is clocks rather than some registers to be written. > > That all feels like outside the scope of the current patch series > though. > > My suggestion for V2 would still therefore be to remove all PM runtime > as it has no impact. It could be added later down the road if cutting > the power-domain is a goal of yours. It may have no impact on the platform you are on, but it's very likely to have an impact on other platforms with this same IP. Kevin
