On Fri, Nov 22, 2013 at 12:44 PM, Paul Zimmerman <Paul.Zimmerman@xxxxxxxxxxxx> wrote: >> From: linux-usb-owner@xxxxxxxxxxxxxxx [mailto:linux-usb-owner@xxxxxxxxxxxxxxx] On Behalf Of Dan Williams >> Sent: Friday, November 22, 2013 1:08 AM >> >> C_PORT_CONNECTION changes are now handled and cleared while a port is >> powered off. It is also specified to be cleared by both usb2 [1] and >> usb3 [2] hubs when the port is in the logical power-off state. >> >> C_PORT_ENABLE is cleared by usb2 [3] hubs in the logical power-off >> state, and the bit is reserved for usb3 [2] hubs. >> >> In usb_port_runtime_resume() it does not make sense to attempt to > > I guess you meant usb_port_runtime_suspend() here? > No, sorry, I rebased this patch while developing and did not refresh this commit log. There used to be a usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE); in usb_port_runtime_resume(), but that got deleted in Patch 1. However there is USB_PORT_FEAT_ENABLE vs USB_PORT_FEAT_C_ENABLE confusion in this commit log. >> re-disable the port after a resume / debounce failure, we want the port >> to be in enabled state regardless. >> >> [1]: USB2.0 Specification Section 11.24.2.7.2.1 >> [2]: USB3.0 Specification Section 10.16.2.6.2 >> [3]: USB2.0 Specification Section 11.24.2.7.2.2 >> >> Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx> >> --- >> drivers/usb/core/port.c | 2 -- >> 1 files changed, 0 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c >> index 48011263ce35..3dbd4d653b15 100644 >> --- a/drivers/usb/core/port.c >> +++ b/drivers/usb/core/port.c >> @@ -114,8 +114,6 @@ static int usb_port_runtime_suspend(struct device *dev) >> retval = usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_POWER); >> if (retval) >> clear_bit(port1, hub->poweroff_bits); >> - usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_CONNECTION); >> - usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE); > > This is not an attempt to re-disable the port, rather it is clearing the > status change bits. So you should reword your commit log to match. > > Are you sure it is a good idea to remove the clearing of the change bits > powered off, there may be some hubs out there which don't follow the spec > here? Even if the spec says they should be cleared when the port is > exactly. It seems safer to leave this as-is. Or have you seen it cause a > problem? They still get cleared by the resume path. Also USB_PORT_FEAT_C_CONNECTION continues to be handled by khubd, just not acted on while the port is powered off. This commit log is confusing, I'll clarify it to say that clearing them here is redundant. -- 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
