Re: Controller's wakeup setting at usb_add_hcd
On Mon, 28 Oct 2013, Peter Chen wrote: Just like you said: The physical wakeup setting is controlled by both the controller driver and the platform driver. But currently, it is controlled (or affected) by hcd layer. It is _initialized_ by the HCD layer. The platform driver can always override this. It's easy; just call device_set_wakeup_enable(dev, 0) after usb_add_hcd(). In fact, your platform driver probably should remember the current wakeup setting when the HC is removed. The next time it is added you should change the wakeup setting to the remembered value. That way, if the user adjusts the setting and switches between host and device mode, the desired setting will persist. Yes, we can do that. But if there is no initializing for wakeup setting at usb_add_hcd, the controller driver needs to do nothing, No matter HC is removed or peripheral switches to HC, the wakeup setting will be totally decided by user, and the default setting is 0. Okay. I'll make a deal with you: You can remove the single device_wakeup_enable() call from usb_add_hcd() provided, in the same patch, you add device_wakeup_enable() calls to every HCD (except yours, obviously). Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Controller's wakeup setting at usb_add_hcd
On Mon, Oct 28, 2013 at 10:37:52AM -0400, Alan Stern wrote: On Mon, 28 Oct 2013, Peter Chen wrote: Just like you said: The physical wakeup setting is controlled by both the controller driver and the platform driver. But currently, it is controlled (or affected) by hcd layer. It is _initialized_ by the HCD layer. The platform driver can always override this. It's easy; just call device_set_wakeup_enable(dev, 0) after usb_add_hcd(). In fact, your platform driver probably should remember the current wakeup setting when the HC is removed. The next time it is added you should change the wakeup setting to the remembered value. That way, if the user adjusts the setting and switches between host and device mode, the desired setting will persist. Yes, we can do that. But if there is no initializing for wakeup setting at usb_add_hcd, the controller driver needs to do nothing, No matter HC is removed or peripheral switches to HC, the wakeup setting will be totally decided by user, and the default setting is 0. Okay. I'll make a deal with you: You can remove the single device_wakeup_enable() call from usb_add_hcd() provided, in the same patch, you add device_wakeup_enable() calls to every HCD (except yours, obviously). Thanks, Alan. -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Controller's wakeup setting at usb_add_hcd
On Thu, Oct 24, 2013 at 12:51:47PM -0400, Alan Stern wrote: On Thu, 24 Oct 2013, Peter Chen wrote: It is the same which the embedded world does, that is the physical wakeup setting is controlled by software flag. Since it is the controller's wakeup, why the controller driver does not go to control the software flag in general? The current problem for me is: usb_add_hcd initializes the controller's flag to 1, the controller driver needs to override it as 0. It will delete the hcd on the fly (call usb_remove_hcd) for otg controller due to plug in Micro B-to-A cable (ID 0-1), and later, the user may plug out this cable (ID 1-0), the hcd will be added again, at this time, the wakeup flag is set by hcd again. If I want to keep the wakeup flag as 0 by default, I need to clear it again. Yes. Just like you said: The physical wakeup setting is controlled by both the controller driver and the platform driver. But currently, it is controlled (or affected) by hcd layer. It is _initialized_ by the HCD layer. The platform driver can always override this. It's easy; just call device_set_wakeup_enable(dev, 0) after usb_add_hcd(). In fact, your platform driver probably should remember the current wakeup setting when the HC is removed. The next time it is added you should change the wakeup setting to the remembered value. That way, if the user adjusts the setting and switches between host and device mode, the desired setting will persist. Yes, we can do that. But if there is no initializing for wakeup setting at usb_add_hcd, the controller driver needs to do nothing, No matter HC is removed or peripheral switches to HC, the wakeup setting will be totally decided by user, and the default setting is 0. -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Controller's wakeup setting at usb_add_hcd
On Thu, 24 Oct 2013, Peter Chen wrote: If the HC is deleted and the UDC is added then the wakeup settings should be determined by the UDC, not by the HC. The next time the system goes to sleep, the UDC driver should be responsible for enabling or disabling wakeup. I agree both of your points, but after thinking more, I doubt that why hcd (or udc) needs to control its parent wakeup setting? Why not let controller driver decide it? Im not quite sure what you're asking. To start with, we are talking about two different wakeup settings: The software flag, which is controlled by userspace writing to the power/wakeup attribute or by the driver calling device_wakeup_enable(), device_wakeup_disable(), or device_set_wakeup_enable(); The physical wakeup setting in the hardware. In general, the controller driver does not control the software flag, except that the driver may initialize the flag. usb_add_hcd() always initializes the controller's flag to 1, but the driver can override it afterward. After it is initialized, the flag is controlled entirely by userspace. The physical wakeup setting is controlled by both the controller driver and the platform driver. For example, with a PCI-based EHCI controller, ehci-hub.c sets the various port wakeup flags and the PCI subsystem enables or disables PME. However this shouldn't matter, because these drivers should always tell the hardware to do what the software flag says. That is, if device_may_wakeup() is true then the hardware wakeup should be enabled; otherwise it should be disabled. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Controller's wakeup setting at usb_add_hcd
On Thu, Oct 24, 2013 at 10:25:25AM -0400, Alan Stern wrote: On Thu, 24 Oct 2013, Peter Chen wrote: If the HC is deleted and the UDC is added then the wakeup settings should be determined by the UDC, not by the HC. The next time the system goes to sleep, the UDC driver should be responsible for enabling or disabling wakeup. I agree both of your points, but after thinking more, I doubt that why hcd (or udc) needs to control its parent wakeup setting? Why not let controller driver decide it? Im not quite sure what you're asking. To start with, we are talking about two different wakeup settings: The software flag, which is controlled by userspace writing to the power/wakeup attribute or by the driver calling device_wakeup_enable(), device_wakeup_disable(), or device_set_wakeup_enable(); The physical wakeup setting in the hardware. In general, the controller driver does not control the software flag, except that the driver may initialize the flag. usb_add_hcd() always initializes the controller's flag to 1, but the driver can override it afterward. After it is initialized, the flag is controlled entirely by userspace. The physical wakeup setting is controlled by both the controller driver and the platform driver. For example, with a PCI-based EHCI controller, ehci-hub.c sets the various port wakeup flags and the PCI subsystem enables or disables PME. However this shouldn't matter, because these drivers should always tell the hardware to do what the software flag says. That is, if device_may_wakeup() is true then the hardware wakeup should be enabled; otherwise it should be disabled. It is the same which the embedded world does, that is the physical wakeup setting is controlled by software flag. Since it is the controller's wakeup, why the controller driver does not go to control the software flag in general? The current problem for me is: usb_add_hcd initializes the controller's flag to 1, the controller driver needs to override it as 0. It will delete the hcd on the fly (call usb_remove_hcd) for otg controller due to plug in Micro B-to-A cable (ID 0-1), and later, the user may plug out this cable (ID 1-0), the hcd will be added again, at this time, the wakeup flag is set by hcd again. If I want to keep the wakeup flag as 0 by default, I need to clear it again. Just like you said: The physical wakeup setting is controlled by both the controller driver and the platform driver. But currently, it is controlled (or affected) by hcd layer. -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Controller's wakeup setting at usb_add_hcd
On Thu, 24 Oct 2013, Peter Chen wrote: It is the same which the embedded world does, that is the physical wakeup setting is controlled by software flag. Since it is the controller's wakeup, why the controller driver does not go to control the software flag in general? The current problem for me is: usb_add_hcd initializes the controller's flag to 1, the controller driver needs to override it as 0. It will delete the hcd on the fly (call usb_remove_hcd) for otg controller due to plug in Micro B-to-A cable (ID 0-1), and later, the user may plug out this cable (ID 1-0), the hcd will be added again, at this time, the wakeup flag is set by hcd again. If I want to keep the wakeup flag as 0 by default, I need to clear it again. Yes. Just like you said: The physical wakeup setting is controlled by both the controller driver and the platform driver. But currently, it is controlled (or affected) by hcd layer. It is _initialized_ by the HCD layer. The platform driver can always override this. It's easy; just call device_set_wakeup_enable(dev, 0) after usb_add_hcd(). In fact, your platform driver probably should remember the current wakeup setting when the HC is removed. The next time it is added you should change the wakeup setting to the remembered value. That way, if the user adjusts the setting and switches between host and device mode, the desired setting will persist. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Controller's wakeup setting at usb_add_hcd
On Tue, 22 Oct 2013, Peter Chen wrote: Hi Alan, Currently, the controller's wakeup setting is enabled by default, is it ok let it be disabled by default, and let the user control it. Enable usb wakeup will cause more power consumption(some phy logic need to be on to detect wakeup) and unexpected usb wakeup may be considered as an unexpected behaviour. Isn't this the sort of thing that should be handled in userspace? If the user (or the distro) sets the appropriate wakeup value for the controller then the default doesn't matter. It's not possible for the kernel to guess what the user wants. For example, if there's a USB keyboard then probably wakeup should be enabled. But what if there's a mouse and no keyboard? The kernel can't tell. In the end, we have to let the user tell us. Meanwhile, the controller may not be dedicated for host, it may otg controller, the hcd is deleted when switches to device mode, the wakeup setting for controller is still enabled, it is not suitable. If the HC is deleted and the UDC is added then the wakeup settings should be determined by the UDC, not by the HC. The next time the system goes to sleep, the UDC driver should be responsible for enabling or disabling wakeup. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Controller's wakeup setting at usb_add_hcd
On Wed, Oct 23, 2013 at 10:55:08AM -0400, Alan Stern wrote: On Tue, 22 Oct 2013, Peter Chen wrote: Hi Alan, Currently, the controller's wakeup setting is enabled by default, is it ok let it be disabled by default, and let the user control it. Enable usb wakeup will cause more power consumption(some phy logic need to be on to detect wakeup) and unexpected usb wakeup may be considered as an unexpected behaviour. Isn't this the sort of thing that should be handled in userspace? If the user (or the distro) sets the appropriate wakeup value for the controller then the default doesn't matter. It's not possible for the kernel to guess what the user wants. For example, if there's a USB keyboard then probably wakeup should be enabled. But what if there's a mouse and no keyboard? The kernel can't tell. In the end, we have to let the user tell us. Meanwhile, the controller may not be dedicated for host, it may otg controller, the hcd is deleted when switches to device mode, the wakeup setting for controller is still enabled, it is not suitable. If the HC is deleted and the UDC is added then the wakeup settings should be determined by the UDC, not by the HC. The next time the system goes to sleep, the UDC driver should be responsible for enabling or disabling wakeup. I agree both of your points, but after thinking more, I doubt that why hcd (or udc) needs to control its parent wakeup setting? Why not let controller driver decide it? -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Controller's wakeup setting at usb_add_hcd
Hi Alan, Currently, the controller's wakeup setting is enabled by default, is it ok let it be disabled by default, and let the user control it. Enable usb wakeup will cause more power consumption(some phy logic need to be on to detect wakeup) and unexpected usb wakeup may be considered as an unexpected behaviour. Meanwhile, the controller may not be dedicated for host, it may otg controller, the hcd is deleted when switches to device mode, the wakeup setting for controller is still enabled, it is not suitable. -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
