Re: [PATCH v2] usb: hub: Clear Port Reset Change during init/resume

[Sarah Sharp]

On Tue, Oct 15, 2013 at 05:45:00PM -0700, Julius Werner wrote:
> This patch adds the Port Reset Change flag to the set of bits that are
> preemptively cleared on init/resume of a hub. In theory this bit should
> never be set unexpectedly... in practice it can still happen if BIOS,
> SMM or ACPI code plays around with USB devices without cleaning up
> correctly. This is especially dangerous for XHCI root hubs, which don't
> generate any more Port Status Change Events until all change bits are
> cleared, so this is a good precaution to have (similar to how it's
> already done for the Warm Port Reset Change flag).

Did you run into an issue where port status change events weren't being
generated because the Port Reset flag was set?  I'm trying to figure out
if this addresses a real issue you hit (and thus should be queued for
stable), or if this is just a precaution.

I do agree this is a good fix.  ISTR that with some xHCI vendors, when
the host is reset, the hardware will drive a port reset down all ports.
That may cause the port reset and even warm port reset bits to be set.
I have a vague recollection that some implementations might not even
wait for the port reset to complete before saying the reset is done.  We
can't tell which hosts will and won't drive a reset, so we rely on the
USB core to clear those bits if they are set.

So perhaps instead of the BIOS/SMM/ACPI code leaving the ports in an
unclean state, the root cause of your issue is actually the call to
xhci_reset?  You can check the port status before that call to find out.

> Signed-off-by: Julius Werner <jwerner@xxxxxxxxxxxx>
> ---
>  drivers/usb/core/hub.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index e6b682c..c3dd64c 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -1130,6 +1130,11 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type)
>  			usb_clear_port_feature(hub->hdev, port1,
>  					USB_PORT_FEAT_C_ENABLE);
>  		}
> +		if (portchange & USB_PORT_STAT_C_RESET) {
> +			need_debounce_delay = true;
> +			usb_clear_port_feature(hub->hdev, port1,
> +					USB_PORT_FEAT_C_RESET);
> +		}
>  		if ((portchange & USB_PORT_STAT_C_BH_RESET) &&
>  				hub_is_superspeed(hub->hdev)) {
>  			need_debounce_delay = true;
> -- 
> 1.7.12.4
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/