On 20-07-03 10:19:11, Alan Stern wrote: > On Fri, Jul 03, 2020 at 02:25:32PM +0800, Peter Chen wrote: > > After that, the user could enable controller as wakeup source > > for system suspend through /sys entry. > > > > Signed-off-by: Peter Chen <peter.chen@xxxxxxx> > > --- > > 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 cebe24ec80a5..bb5d73f0a796 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_set_wakeup_capable(hcd->self.controller, true); > > > In fact both this patch and the original code are wrong. It really should > be: > > device_init_wakeup(hcd->self.controller, true); > > This will add the wakeup entry in sysfs and set it to Enabled. This is > the appropriate behavior, as explained in the kerneldoc for > device_init_wakeup(). The reason is because the controller device doesn't > create any wakeup events on its own; it merely relays wakeup requests from > descendant devices (root hubs or USB devices). Hi Alan, At xhci-plat.c's system suspend API xhci_plat_suspend, it depends on power/wakeup value to determine whether the controller should enable port wakeup capabilities, and from the system level, whether the system supports USB wakeup or not is better to be determined by user, and is disabled by default. Yes, you are right. The wakeup events are from controller's descendant devices, and it is better to use roothub's wakeup capability to control portsc's wakeup setting. At controller driver, the meaning for wakeup setting is enabling wakeup interrupt for hardware signal events (dp/dm for USB2, and RX-detect for USB3), this hardware logic is the glue layer and designed by each vendors, without this logic, the USB controller can't be woken up due to the USBCMD.RS bit is cleared, and there is no standard EHCI or xHCI interrupt. The controller's wakeup setting is better to be disabled by default, and decided by user too. For me, either this patch or use roothub's wakeup capability to control portsc wakeup setting, both are OK. Mathias, what's your opinion? -- Thanks, Peter Chen
