On Thu, Jun 30, 2016 at 01:30:54PM -0700, Stephen Boyd wrote: > Quoting Peter Chen (2016-06-29 18:39:01) > > On Wed, Jun 29, 2016 at 05:43:30PM -0700, Stephen Boyd wrote: > > > Quoting Peter Chen (2016-06-28 23:46:00) > > > > On Sun, Jun 26, 2016 at 12:28:29AM -0700, Stephen Boyd wrote: > > > > > Sometimes the usb wrapper device is part of a power domain that > > > > > needs to stay on as long as the device is active. Let's get and > > > > > put the device in driver probe/remove so that we keep the power > > > > > domain powered as long as the device is attached. We can fine > > > > > tune this later to handle wakeup interrupts, etc. for finer grain > > > > > power management later, but this is necessary to make sure we can > > > > > keep accessing the device right now. > > > > > > > > Since some of the controllers work abnormal if we enables runtime > > > > pm unconditionally, so I use one system flag CI_HDRC_SUPPORTS_RUNTIME_PM > > > > for it. I can't understand why you can't access device without enable > > > > parent's runtime pm, the controller will not enter runtime suspend > > > > without that flag. > > > > > > Correct, the child device of ci_hdrc_msm will be able to do runtime PM > > > and keep the parent enabled if the CI_HDRC_SUPPORTS_RUNTIME_PM flag is > > > set. But even if that flag isn't set, the ci_hdrc_msm driver is calling > > > pm_runtime_enable() on the same device that it would be called on if the > > > CI_HDRC_SUPPORTS_RUNTIME_PM flag was set. That allows runtime PM > > > transition of child devices such as the usb ports (usb1-port1 for > > > example) to propagate up all the way to the ci_hdrc_msm device and > > > disable any attached power domains. > > > > Sorry, I can't get you. > > > > If the chipidea core's runtime is disabled, the port under the > > controller will not be in runtime suspended, only the bus will > > be in suspended due to USB core enables runtime PM by default. > > Hmm sorry, I was confused too. > > From what I can tell, if I don't call pm_runtime_set_active() on the > glue device, it will runtime suspend once I call pm_runtime_enable() on > it (which we do in ci_hdrc_mms_probe()). When we runtime suspend the > glue device, we turn off the power domain associated with it too. The > runtime pm enabled state of the core device doesn't seem to matter > either way here. So perhaps I should be calling pm_runtime_set_active() > before pm_runtime_enable() there instead of doing the get/put? It isn't > clear to me when we should be calling pm_runtime_get() vs. > pm_runtime_set_active() though. > > > > > > > > > Why don't we call runtime PM functions on the ci->dev for all cases of > > > ci->supports_runtime_pm? It seems like the glue drivers should be > > > managing their own device power states and the ci->dev should be managed > > > by core.c code. > > > > > > > This is current design. Chipidea core manages portsc.phcd and PHY's PM > > (through PHY's API), and glue layer manages its own clocks on the system > > bus for register visit (and data transfer if necessary). > > > > Sorry, I mean this code in core.c > > pm_runtime_set_active(&pdev->dev); > pm_runtime_enable(&pdev->dev); > pm_runtime_set_autosuspend_delay(&pdev->dev, 2000); > pm_runtime_mark_last_busy(ci->dev); > pm_runtime_use_autosuspend(&pdev->dev); > > which confused me. I thought pdev->dev was the glue device, but it's the > same as ci->dev, the core device. I get it now, but I'd like to change > all the calls there to use ci->dev to be clearer. Yes, please do it. Glue device is the parent for core device, and at core.c, only ci_hdrc_add_device and ci_get_platdata will touch glue device. -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
