Hello,

On Wed Nov 22, 2023 at 11:23 PM CET, Kevin Hilman wrote:

Théo Lebrun [off-list ref] writes:

quoted

On Fri Nov 17, 2023 at 12:51 PM CET, Roger Quadros wrote:

quoted

On 17/11/2023 12:17, Théo Lebrun wrote:

quoted

On Thu Nov 16, 2023 at 10:44 PM CET, Roger Quadros wrote:

quoted

On 16/11/2023 20:56, Théo Lebrun wrote:

quoted

On Thu Nov 16, 2023 at 1:40 PM CET, Roger Quadros wrote:

quoted

On 15/11/2023 17:02, Théo Lebrun wrote:

quoted

On Wed Nov 15, 2023 at 12:37 PM CET, Roger Quadros wrote:

quoted

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

On my platform, when the device is attached to the PD it gets turned on.
That feels logical to me: if a driver is not RPM aware it "just works".
Are there platforms where RPM must get enabled for the attached
power-domains to be turned on?

The call chain that attaches & enables PD is platform_probe ->
dev_pm_domain_attach. That function takes a bool power_on which is
always true. In the genpd case, genpd_dev_pm_attach
calls __genpd_dev_pm_attach which does a genpd_power_on.

Things I've not accounted for:

 - ACPI looks like it does the same but I've not checked. It gets passed
   the power_on bool argument.

 - genpd_dev_pm_attach early returns with no error if there are multiple
   PM domains attached to the device. There are many platforms in the
   case according to some devicetree grepping. I can imagine a behavior
   difference where dev_pm_domain callpaths handle that differently in
   the RPM case. Is that what we are discussing?

quoted

quoted

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.

quoted

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.

I agree & I have a prototype, but that requires some work on the child
devices as from what I remember they are not eager to go to runtime
suspend (ie a driver might be holding a reference).

I feel this leans outside the scope of this patch series though.

Regards,

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel