| Message ID | 20200813033741.13982-6-peter.chen@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: host: xhci: some changes for xhci-plat.c | expand |
On 20-08-13 11:37:39, Peter Chen wrote: > It initializes the controller wakeup setting as default enabled > unless the user changes it, whether the controller responds > the wakeup event depends on roothub's wakeup setting since the > wakeup occurs at the bus not the controller itself. > > With this change, the controller uses this driver could have > wakeup capability due to device_may_wakeup(dev) at xhci_plat_suspend > is default true; without this change, this value is always false. > > Reviewed-by: Jun Li <jun.li@nxp.com> > Suggested-by: Alan Stern <stern@rowland.harvard.edu> > Signed-off-by: Peter Chen <peter.chen@nxp.com> > --- > drivers/usb/host/xhci-plat.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c > index 14ff65a387e8..0ef28b2caea3 100644 > --- a/drivers/usb/host/xhci-plat.c > +++ b/drivers/usb/host/xhci-plat.c > @@ -287,7 +287,7 @@ static int xhci_plat_probe(struct platform_device *pdev) > *priv = *priv_match; > } > > - device_wakeup_enable(hcd->self.controller); > + device_init_wakeup(hcd->self.controller, true); Hi Alan, We may can't set the wakeup enabled as default value since it will affect the parent's wakeup setting. See function: dpm_propagate_wakeup_to_parent at drivers/base/power/main.c. The parent's wakeup_path is set as true during the system system routine, then the power domain for the parent will not be off (See genpd_finish_suspend) after system suspend, it does not meet design expectation. The expectation is the device's power domain is only on if the device as wakeup source. Even with host-only use case, the thing is the same. If wakeup enabled as default, the related power domain will be on during the system suspend no matter the wakeup is really wanted. Do you agree I set the wakeup default value as disabled?
On Fri, Aug 14, 2020 at 06:25:04AM +0000, Peter Chen wrote: > Hi Alan, > > We may can't set the wakeup enabled as default value since it will > affect the parent's wakeup setting. See function: > dpm_propagate_wakeup_to_parent at drivers/base/power/main.c. > The parent's wakeup_path is set as true during the system > system routine, then the power domain for the parent will not be > off (See genpd_finish_suspend) after system suspend, it does not > meet design expectation. The expectation is the device's power > domain is only on if the device as wakeup source. > > Even with host-only use case, the thing is the same. If wakeup enabled > as default, the related power domain will be on during the system > suspend no matter the wakeup is really wanted. > > Do you agree I set the wakeup default value as disabled? Yes, I guess so. There doesn't seem to be any other way to handle this. What we really should have is a setting which means "Leave wakeup turned off unless it is enabled for a child device". Maybe something like that can be added in the future. Alan Stern
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c index 14ff65a387e8..0ef28b2caea3 100644 --- a/drivers/usb/host/xhci-plat.c +++ b/drivers/usb/host/xhci-plat.c @@ -287,7 +287,7 @@ static int xhci_plat_probe(struct platform_device *pdev) *priv = *priv_match; } - device_wakeup_enable(hcd->self.controller); + device_init_wakeup(hcd->self.controller, true); xhci->main_hcd = hcd; xhci->shared_hcd = __usb_create_hcd(driver, sysdev, &pdev->dev,