Re: [PATCH 3/6] usb: cdns3-ti: add suspend/resume procedures for J7200

[Théo Lebrun]

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.
>
> 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.

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 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.

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com