On Thu, Aug 25, 2011 at 04:43:16PM +0800, Andiry Xu wrote:
> Current waiting time for warm(BH) reset in hub_port_warm_reset() is too
> short for xHC host to complete the warm reset and report a BH reset
> change.
>
> This patch extends the waiting time for warm reset and merges the function
> into hub_port_reset(), so it can handle both cold reset and warm reset.
>
> This fixes the issue that driver fails to clear BH reset change and port is
> "dead".
>
> Signed-off-by: Andiry Xu <andiry.xu@xxxxxxx>
> ---
>
> Sarah,
>
> Please help review and test if this solves your issue.
Thanks Andiry, I'll try to test this today.
Sarah
> ---
> drivers/usb/core/hub.c | 171 ++++++++++++++++++++++++-----------------------
> 1 files changed, 87 insertions(+), 84 deletions(-)
>
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 338f91f..7ef9013 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -2025,11 +2025,12 @@ static unsigned hub_is_wusb(struct usb_hub *hub)
>
> #define HUB_ROOT_RESET_TIME 50 /* times are in msec */
> #define HUB_SHORT_RESET_TIME 10
> +#define HUB_BH_RESET_TIME 100
> #define HUB_LONG_RESET_TIME 200
> #define HUB_RESET_TIMEOUT 500
>
> static int hub_port_wait_reset(struct usb_hub *hub, int port1,
> - struct usb_device *udev, unsigned int delay)
> + struct usb_device *udev, unsigned int delay, bool warm)
> {
> int delay_time, ret;
> u16 portstatus;
> @@ -2046,28 +2047,43 @@ static int hub_port_wait_reset(struct usb_hub *hub, int port1,
> if (ret < 0)
> return ret;
>
> - /* Device went away? */
> - if (!(portstatus & USB_PORT_STAT_CONNECTION))
> - return -ENOTCONN;
> -
> - /* bomb out completely if the connection bounced */
> - if ((portchange & USB_PORT_STAT_C_CONNECTION))
> - return -ENOTCONN;
> -
> - /* if we`ve finished resetting, then break out of the loop */
> - if (!(portstatus & USB_PORT_STAT_RESET) &&
> - (portstatus & USB_PORT_STAT_ENABLE)) {
> - if (hub_is_wusb(hub))
> - udev->speed = USB_SPEED_WIRELESS;
> - else if (hub_is_superspeed(hub->hdev))
> - udev->speed = USB_SPEED_SUPER;
> - else if (portstatus & USB_PORT_STAT_HIGH_SPEED)
> - udev->speed = USB_SPEED_HIGH;
> - else if (portstatus & USB_PORT_STAT_LOW_SPEED)
> - udev->speed = USB_SPEED_LOW;
> - else
> - udev->speed = USB_SPEED_FULL;
> - return 0;
> + if (!warm) {
> + /* Device went away? */
> + if (!(portstatus & USB_PORT_STAT_CONNECTION))
> + return -ENOTCONN;
> +
> + /* bomb out completely if the connection bounced */
> + if ((portchange & USB_PORT_STAT_C_CONNECTION))
> + return -ENOTCONN;
> +
> + /* if we`ve finished resetting, then break out of
> + * the loop
> + */
> + if (!(portstatus & USB_PORT_STAT_RESET) &&
> + (portstatus & USB_PORT_STAT_ENABLE)) {
> + if (hub_is_wusb(hub))
> + udev->speed = USB_SPEED_WIRELESS;
> + else if (hub_is_superspeed(hub->hdev))
> + udev->speed = USB_SPEED_SUPER;
> + else if (portstatus & USB_PORT_STAT_HIGH_SPEED)
> + udev->speed = USB_SPEED_HIGH;
> + else if (portstatus & USB_PORT_STAT_LOW_SPEED)
> + udev->speed = USB_SPEED_LOW;
> + else
> + udev->speed = USB_SPEED_FULL;
> + return 0;
> + }
> + } else {
> + if (portchange & USB_PORT_STAT_C_BH_RESET) {
> + clear_port_feature(hub->hdev, port1,
> + USB_PORT_FEAT_C_BH_PORT_RESET);
> +
> + if (portchange & USB_PORT_STAT_C_LINK_STATE)
> + clear_port_feature(hub->hdev, port1,
> + USB_PORT_FEAT_C_PORT_LINK_STATE);
> +
> + return 0;
> + }
> }
>
> /* switch to the long delay after two short delay failures */
> @@ -2075,35 +2091,49 @@ static int hub_port_wait_reset(struct usb_hub *hub, int port1,
> delay = HUB_LONG_RESET_TIME;
>
> dev_dbg (hub->intfdev,
> - "port %d not reset yet, waiting %dms\n",
> - port1, delay);
> + "port %d not %sreset yet, waiting %dms\n",
> + port1, warm ? "warm " : "", delay);
> }
>
> return -EBUSY;
> }
>
> +/* Handle port reset and port warm(BH) reset (for USB3 protocol ports) */
> static int hub_port_reset(struct usb_hub *hub, int port1,
> - struct usb_device *udev, unsigned int delay)
> + struct usb_device *udev, unsigned int delay, bool warm)
> {
> int i, status;
> struct usb_hcd *hcd;
>
> - hcd = bus_to_hcd(udev->bus);
> - /* Block EHCI CF initialization during the port reset.
> - * Some companion controllers don't like it when they mix.
> - */
> - down_read(&ehci_cf_port_reset_rwsem);
> + if (!warm) {
> + /* Block EHCI CF initialization during the port reset.
> + * Some companion controllers don't like it when they mix.
> + */
> + down_read(&ehci_cf_port_reset_rwsem);
> + } else {
> + if (!hub_is_superspeed(hub->hdev)) {
> + dev_err(hub->intfdev, "only USB3 hub support "
> + "warm reset\n");
> + return -EINVAL;
> + }
> + }
>
> /* Reset the port */
> for (i = 0; i < PORT_RESET_TRIES; i++) {
> - status = set_port_feature(hub->hdev,
> - port1, USB_PORT_FEAT_RESET);
> - if (status)
> + if (!warm)
> + status = set_port_feature(hub->hdev,
> + port1, USB_PORT_FEAT_RESET);
> + else
> + status = set_port_feature(hub->hdev,
> + port1, USB_PORT_FEAT_BH_PORT_RESET);
> +
> + if (status) {
> dev_err(hub->intfdev,
> - "cannot reset port %d (err = %d)\n",
> - port1, status);
> - else {
> - status = hub_port_wait_reset(hub, port1, udev, delay);
> + "cannot %sreset port %d (err = %d)\n",
> + warm ? "warm " : "", port1, status);
> + } else {
> + status = hub_port_wait_reset(hub, port1, udev, delay,
> + warm);
> if (status && status != -ENOTCONN)
> dev_dbg(hub->intfdev,
> "port_wait_reset: err = %d\n",
> @@ -2113,9 +2143,13 @@ static int hub_port_reset(struct usb_hub *hub, int port1,
> /* return on disconnect or reset */
> switch (status) {
> case 0:
> + if (warm)
> + goto clear_reset_change;
> +
> /* TRSTRCY = 10 ms; plus some extra */
> msleep(10 + 40);
> update_devnum(udev, 0);
> + hcd = bus_to_hcd(udev->bus);
> if (hcd->driver->reset_device) {
> status = hcd->driver->reset_device(hcd, udev);
> if (status < 0) {
> @@ -2127,18 +2161,20 @@ static int hub_port_reset(struct usb_hub *hub, int port1,
> /* FALL THROUGH */
> case -ENOTCONN:
> case -ENODEV:
> +clear_reset_change:
> clear_port_feature(hub->hdev,
> port1, USB_PORT_FEAT_C_RESET);
> /* FIXME need disconnect() for NOTATTACHED device */
> - usb_set_device_state(udev, status
> - ? USB_STATE_NOTATTACHED
> - : USB_STATE_DEFAULT);
> + if (!warm)
> + usb_set_device_state(udev, status
> + ? USB_STATE_NOTATTACHED
> + : USB_STATE_DEFAULT);
> goto done;
> }
>
> dev_dbg (hub->intfdev,
> - "port %d not enabled, trying reset again...\n",
> - port1);
> + "port %d not enabled, trying %sreset again...\n",
> + port1, warm ? "warm " : "");
> delay = HUB_LONG_RESET_TIME;
> }
>
> @@ -2146,45 +2182,11 @@ static int hub_port_reset(struct usb_hub *hub, int port1,
> "Cannot enable port %i. Maybe the USB cable is bad?\n",
> port1);
>
> - done:
> - up_read(&ehci_cf_port_reset_rwsem);
> - return status;
> -}
> -
> -/* Warm reset a USB3 protocol port */
> -static int hub_port_warm_reset(struct usb_hub *hub, int port)
> -{
> - int ret;
> - u16 portstatus, portchange;
> -
> - if (!hub_is_superspeed(hub->hdev)) {
> - dev_err(hub->intfdev, "only USB3 hub support warm reset\n");
> - return -EINVAL;
> - }
> -
> - /* Warm reset the port */
> - ret = set_port_feature(hub->hdev,
> - port, USB_PORT_FEAT_BH_PORT_RESET);
> - if (ret) {
> - dev_err(hub->intfdev, "cannot warm reset port %d\n", port);
> - return ret;
> - }
> -
> - msleep(20);
> - ret = hub_port_status(hub, port, &portstatus, &portchange);
> -
> - if (portchange & USB_PORT_STAT_C_RESET)
> - clear_port_feature(hub->hdev, port, USB_PORT_FEAT_C_RESET);
> -
> - if (portchange & USB_PORT_STAT_C_BH_RESET)
> - clear_port_feature(hub->hdev, port,
> - USB_PORT_FEAT_C_BH_PORT_RESET);
> -
> - if (portchange & USB_PORT_STAT_C_LINK_STATE)
> - clear_port_feature(hub->hdev, port,
> - USB_PORT_FEAT_C_PORT_LINK_STATE);
> +done:
> + if (!warm)
> + up_read(&ehci_cf_port_reset_rwsem);
>
> - return ret;
> + return status;
> }
>
> /* Check if a port is power on */
> @@ -2814,7 +2816,7 @@ hub_port_init (struct usb_hub *hub, struct usb_device *udev, int port1,
>
> /* Reset the device; full speed may morph to high speed */
> /* FIXME a USB 2.0 device may morph into SuperSpeed on reset. */
> - retval = hub_port_reset(hub, port1, udev, delay);
> + retval = hub_port_reset(hub, port1, udev, delay, false);
> if (retval < 0) /* error or disconnect */
> goto fail;
> /* success, speed is known */
> @@ -2944,7 +2946,7 @@ hub_port_init (struct usb_hub *hub, struct usb_device *udev, int port1,
> buf->bMaxPacketSize0;
> kfree(buf);
>
> - retval = hub_port_reset(hub, port1, udev, delay);
> + retval = hub_port_reset(hub, port1, udev, delay, false);
> if (retval < 0) /* error or disconnect */
> goto fail;
> if (oldspeed != udev->speed) {
> @@ -3565,7 +3567,8 @@ static void hub_events(void)
> (portstatus & USB_PORT_STAT_LINK_STATE)
> == USB_SS_PORT_LS_SS_INACTIVE) {
> dev_dbg(hub_dev, "warm reset port %d\n", i);
> - hub_port_warm_reset(hub, i);
> + hub_port_reset(hub, i, NULL,
> + HUB_BH_RESET_TIME, true);
> }
>
> if (connect_change)
> --
> 1.7.1
>
>
>
--
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
