On Tue, Jul 21, 2020 at 11:03:03AM +0000, Peter Chen wrote: > On 20-07-09 10:50:04, Alan Stern wrote: > > > > We want to make the system simple and logical for users, not necessarily > > > > easy for hardware engineers. This means that wakeup events such as port > > > > connect, disconnect, and so on should be controlled by a single sysfs > > > > setting, for a single device. The most logical device for this purpose > > > > is the root hub, as I mentioned earlier in this discussion. > > > > > > > > As a result, the wakeup settings for the other components (non-core or > > > > PHY) should be based on the settings for the root hub. If the root hub > > > > is disabled for wakeup then the non-core hardware components shouldn't > > > > generate any wakeup requests, no matter what their power/wakeup files > > > > contain. And if the root hub is enabled for wakeup then the non-core > > > > hardware components should generate these requests, unless their own > > > > power/wakeup settings prevent them from doing so. > > > > > > > > From these conclusions, it logically follows that the default wakeup > > > > setting for the non-core components should be "enabled" -- unless those > > > > components are physically incapable of waking up the system. > > > > > > > > > > I agree with you that the default wakeup setting of core part for host > > > (xhci-plat.c) should be "enabled", but if for the dual-role controller, > > > the wakeup events like VBUS and ID do not related with roothub, we can't > > > set core part for controller (cdns3/core.c) for defaulted enabled, and > > > there is no thing like gadget bus's wakeup setting we could depend on. > > > > > > Besides, the non-core part (glue layer) somethings can't visit core > > > part, we had to depend on itself wakeup setting, but not the child's > > > wakeup setting. > > > > All right. Note that almost everything I wrote before was meant for the > > case of a host controller; it did not consider what should happen with a > > UDC or dual-role controller. > > > > It seems like the best answer will be to call device_init_wakeup for the > > core controller device, but not for the non-core devices. Or maybe call > > it for the non-core devices if the controller is host-only. > > > > Alan Stern > > Hi Alan, > > Since the controller's wakeup setting (PORT_WKCONN_E/PORT_WKDISC_E) > should be decided by roothub's wakeup setting, then, why controller's > wakeup setting could affect it at current code, does the controller > device may affect wakeup function at some situations? > > int xhci_suspend(struct xhci_hcd *xhci, bool do_wakeup) > int ehci_suspend(struct usb_hcd *hcd, bool do_wakeup) > > It suggests the user needing to set both controller and roothub's wakeup > enabled to let the wakeup function work. That's right. This is a tricky area. A decision was made a long time ago when the current wakeup mechanisms were being added into the kernel; we decided that in order for a wakeup signal to propagate from the originating device all the way up to the CPU, each of the intermediate devices (representing various interconnects) along the way would have to be enabled for wakeup. So for example, even if the root hub's power/wakeup file says "enabled", if the controller's or one of the non-core devices' power/wakeup file is "disabled" then the wakeup signal shouldn't go through. It would be like a situation where a device's interrupt flag is enabled and turned on, but the device can't send an IRQ signal because it isn't allowed to access the PCI bus. This is why I originally suggested that the wakeup setting for the non-core devices and the controller should default to "enabled". Alan Stern
