| Message ID | 1386592158-12086-1-git-send-email-vikas.sajjan@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Vikas, On Mon, Dec 9, 2013 at 5:59 PM, Vikas Sajjan <vikas.sajjan@linaro.org> wrote: few minor nits here. ;-) > Does warm reset while activating SuperSpeed HUBs if the hub activate type > is HUB_RESET_RESUME. > > When we do Suspend-to-RAM with (any one of the 16, 32, 64 Jetflash) transcend > USB 3.0 device connected on 3.0 port, during resume I noticed that the > XHCI controller has moved to sometimes RECOVERY, POLLING or INACTIVE STATE. > This behaviour is inconsistent and the connection with connected USB 3.0 device > on 3.0 port was LOST. > > Doing warm reset while activating SuperSpeed HUBs if the hub > activate type is HUB_RESET_RESUME, gets the connected device to the stable state. > > Reviewed at https://chromium-review.googlesource.com/#/c/177132/ > > Tested on exynos5420 and exynos5250 with Transcend Jetflash USB 3.0 device (8564:1000) > > rebased on Greg Kroah-Hartman's usb-next > git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git Above two lines may not be required in the commit message. > > Signed-off-by: Vikas Sajjan <vikas.sajjan@samsung.com> > --- > drivers/usb/core/hub.c | 41 +++++++++++++++++++++++++---------------- > 1 file changed, 25 insertions(+), 16 deletions(-) > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > index a7c04e2..d8432b0 100644 > --- a/drivers/usb/core/hub.c > +++ b/drivers/usb/core/hub.c > @@ -993,6 +993,21 @@ int usb_remove_device(struct usb_device *udev) > return 0; > } > > +#define PORT_RESET_TRIES 5 > +#define SET_ADDRESS_TRIES 2 > +#define GET_DESCRIPTOR_TRIES 2 > +#define SET_CONFIG_TRIES (2 * (use_both_schemes + 1)) > +#define USE_NEW_SCHEME(i) ((i) / 2 == (int)old_scheme_first) > + > +#define HUB_ROOT_RESET_TIME 50 /* times are in msec */ > +#define HUB_SHORT_RESET_TIME 10 > +#define HUB_BH_RESET_TIME 50 > +#define HUB_LONG_RESET_TIME 200 > +#define HUB_RESET_TIMEOUT 800 > + > +static int hub_port_reset(struct usb_hub *hub, int port1, > + struct usb_device *udev, unsigned int delay, bool warm); > + > enum hub_activation_type { > HUB_INIT, HUB_INIT2, HUB_INIT3, /* INITs must come first */ > HUB_POST_RESET, HUB_RESUME, HUB_RESET_RESUME, > @@ -1093,6 +1108,16 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type) > u16 portstatus, portchange; > > portstatus = portchange = 0; > + > + /* Some connected devices might be still in unknown state even Please take care of multiple line commenting style. > + * after reset-resume, a WARM_RESET gets the connected device > + * to the normal state. > + */ > + if (udev && hub_is_superspeed(hub->hdev) && > + type == HUB_RESET_RESUME) > + hub_port_reset(hub, port1, NULL, > + HUB_BH_RESET_TIME, true); > + > status = hub_port_status(hub, port1, &portstatus, &portchange); > if (udev || (portstatus & USB_PORT_STAT_CONNECTION)) > dev_dbg(hub->intfdev, > @@ -2510,22 +2535,6 @@ static unsigned hub_is_wusb(struct usb_hub *hub) > return hcd->wireless; > } > > - > -#define PORT_RESET_TRIES 5 > -#define SET_ADDRESS_TRIES 2 > -#define GET_DESCRIPTOR_TRIES 2 > -#define SET_CONFIG_TRIES (2 * (use_both_schemes + 1)) > -#define USE_NEW_SCHEME(i) ((i) / 2 == (int)old_scheme_first) > - > -#define HUB_ROOT_RESET_TIME 50 /* times are in msec */ > -#define HUB_SHORT_RESET_TIME 10 > -#define HUB_BH_RESET_TIME 50 > -#define HUB_LONG_RESET_TIME 200 > -#define HUB_RESET_TIMEOUT 800 > - > -static int hub_port_reset(struct usb_hub *hub, int port1, > - struct usb_device *udev, unsigned int delay, bool warm); > - > /* Is a USB 3.0 port in the Inactive or Complinance Mode state? > * Port worm reset is required to recover > */ > -- > 1.7.9.5 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 9 Dec 2013, Vikas Sajjan wrote: > Does warm reset while activating SuperSpeed HUBs if the hub activate type > is HUB_RESET_RESUME. > > When we do Suspend-to-RAM with (any one of the 16, 32, 64 Jetflash) transcend > USB 3.0 device connected on 3.0 port, during resume I noticed that the > XHCI controller has moved to sometimes RECOVERY, POLLING or INACTIVE STATE. > This behaviour is inconsistent and the connection with connected USB 3.0 device > on 3.0 port was LOST. > > Doing warm reset while activating SuperSpeed HUBs if the hub > activate type is HUB_RESET_RESUME, gets the connected device to the stable state. > > Reviewed at https://chromium-review.googlesource.com/#/c/177132/ > > Tested on exynos5420 and exynos5250 with Transcend Jetflash USB 3.0 device (8564:1000) > > rebased on Greg Kroah-Hartman's usb-next > git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git > > Signed-off-by: Vikas Sajjan <vikas.sajjan@samsung.com> > --- > drivers/usb/core/hub.c | 41 +++++++++++++++++++++++++---------------- > 1 file changed, 25 insertions(+), 16 deletions(-) > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > index a7c04e2..d8432b0 100644 > --- a/drivers/usb/core/hub.c > +++ b/drivers/usb/core/hub.c > @@ -1093,6 +1108,16 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type) > u16 portstatus, portchange; > > portstatus = portchange = 0; > + > + /* Some connected devices might be still in unknown state even > + * after reset-resume, a WARM_RESET gets the connected device > + * to the normal state. > + */ > + if (udev && hub_is_superspeed(hub->hdev) && > + type == HUB_RESET_RESUME) > + hub_port_reset(hub, port1, NULL, > + HUB_BH_RESET_TIME, true); Please don't do this all the time to every attached port. Do it only when it is really needed. Shouldn't you pass udev as the third argument? If not, please explain why not. Finally, I don't see why you put this in hub_activate(). Isn't it more closely connected with the reset-resume procedure for the child device? Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Dec 09, 2013 at 10:24:52AM -0500, Alan Stern wrote: > On Mon, 9 Dec 2013, Vikas Sajjan wrote: > > > Does warm reset while activating SuperSpeed HUBs if the hub activate type > > is HUB_RESET_RESUME. > > > > When we do Suspend-to-RAM with (any one of the 16, 32, 64 Jetflash) transcend > > USB 3.0 device connected on 3.0 port, during resume I noticed that the > > XHCI controller has moved to sometimes RECOVERY, POLLING or INACTIVE STATE. > > This behaviour is inconsistent and the connection with connected USB 3.0 device > > on 3.0 port was LOST. Does the device eventually re-connect on the USB port? Or is warm reset necessary to make the device connect? Does the xHCI register restore complete after resume from S3, or is power lost? I'm trying to figure out whether xhci_reset is called before your issue is triggered. > > Doing warm reset while activating SuperSpeed HUBs if the hub > > activate type is HUB_RESET_RESUME, gets the connected device to the stable state. > > > > Reviewed at https://chromium-review.googlesource.com/#/c/177132/ > > > > Tested on exynos5420 and exynos5250 with Transcend Jetflash USB 3.0 device (8564:1000) Is this issue specific to the particular USB device manufacturer (Transcend)? Does the same device lose connection on resume from S3 with other host controller vendors? Have you seen this issue when the USB 3.0 device is behind a USB 3.0 hub? I ask because this sounds like a low-level link training issue that's specific to the exynos host or USB device. I would rather track down which hardware is to blame than generically add a warm reset for all USB 3.0 devices. > > rebased on Greg Kroah-Hartman's usb-next > > git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git > > > > Signed-off-by: Vikas Sajjan <vikas.sajjan@samsung.com> > > --- > > drivers/usb/core/hub.c | 41 +++++++++++++++++++++++++---------------- > > 1 file changed, 25 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > > index a7c04e2..d8432b0 100644 > > --- a/drivers/usb/core/hub.c > > +++ b/drivers/usb/core/hub.c > > > @@ -1093,6 +1108,16 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type) > > u16 portstatus, portchange; > > > > portstatus = portchange = 0; > > + > > + /* Some connected devices might be still in unknown state even > > + * after reset-resume, a WARM_RESET gets the connected device > > + * to the normal state. > > + */ > > + if (udev && hub_is_superspeed(hub->hdev) && > > + type == HUB_RESET_RESUME) > > + hub_port_reset(hub, port1, NULL, > > + HUB_BH_RESET_TIME, true); > > Please don't do this all the time to every attached port. Do it only > when it is really needed. Agreed. Can we at least limit the warm reset to devices directly attached to roothubs? You can also change this code to get the port status and only do the warm reset if the port link state is USB_SS_PORT_LS_POLLING, USB_SS_PORT_LS_RX_DETECT, or USB_SS_PORT_LS_SS_INACTIVE. > Shouldn't you pass udev as the third argument? If not, please explain > why not. > > Finally, I don't see why you put this in hub_activate(). Isn't it more > closely connected with the reset-resume procedure for the child device? Sarah Sharp -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Sarah, On Mon, Dec 9, 2013 at 11:54 PM, Sarah Sharp <sarah.a.sharp@linux.intel.com> wrote: > On Mon, Dec 09, 2013 at 10:24:52AM -0500, Alan Stern wrote: >> On Mon, 9 Dec 2013, Vikas Sajjan wrote: >> >> > Does warm reset while activating SuperSpeed HUBs if the hub activate type >> > is HUB_RESET_RESUME. >> > >> > When we do Suspend-to-RAM with (any one of the 16, 32, 64 Jetflash) transcend >> > USB 3.0 device connected on 3.0 port, during resume I noticed that the >> > XHCI controller has moved to sometimes RECOVERY, POLLING or INACTIVE STATE. >> > This behaviour is inconsistent and the connection with connected USB 3.0 device >> > on 3.0 port was LOST. > > Does the device eventually re-connect on the USB port? Or is warm reset > necessary to make the device connect? Yes, warm reset was necesssary, without which the device was NOT reconnecting. > > Does the xHCI register restore complete after resume from S3, or is > power lost? I'm trying to figure out whether xhci_reset is called > before your issue is triggered. The reason why I came up with this solution is during xhci_resume(), it enters below condition and marks the reset_resume flag for the ROOT_HUB as 1 /* If restore operation fails, re-initialize the HC during resume */ if ((temp & STS_SRE) || hibernated) { /* Let the USB core know _both_ roothubs lost power. */ usb_root_hub_lost_power(xhci->main_hcd->self.root_hub); usb_root_hub_lost_power(xhci->shared_hcd->self.root_hub); > >> > Doing warm reset while activating SuperSpeed HUBs if the hub >> > activate type is HUB_RESET_RESUME, gets the connected device to the stable state. >> > >> > Reviewed at https://chromium-review.googlesource.com/#/c/177132/ >> > >> > Tested on exynos5420 and exynos5250 with Transcend Jetflash USB 3.0 device (8564:1000) > > Is this issue specific to the particular USB device manufacturer > (Transcend)? Does the same device lose connection on resume from S3 > with other host controller vendors? Have you seen this issue when the > USB 3.0 device is behind a USB 3.0 hub? This issue was specific to this paritcular make of Transcend. we saw this on our chromebook. I did try Suspend-to-RAM with the same device on Intel machine running Ubuntu. It had worked fine without any issue. Interestingly, if I connect with analyser, Suspend-to-RAM works fine and USB re-enumerates successfully. so connecting Suspend-to-RAM for debugging was not helping, as it works fine. I did put prints in multiple places to get port status, and i see that port is in sometimes RECOVERY, POLLING or In active STATE. The behaviour was inconsistent. > > I ask because this sounds like a low-level link training issue that's > specific to the exynos host or USB device. I would rather track down > which hardware is to blame than generically add a warm reset for all USB > 3.0 devices. > >> > rebased on Greg Kroah-Hartman's usb-next >> > git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git >> > >> > Signed-off-by: Vikas Sajjan <vikas.sajjan@samsung.com> >> > --- >> > drivers/usb/core/hub.c | 41 +++++++++++++++++++++++++---------------- >> > 1 file changed, 25 insertions(+), 16 deletions(-) >> > >> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c >> > index a7c04e2..d8432b0 100644 >> > --- a/drivers/usb/core/hub.c >> > +++ b/drivers/usb/core/hub.c >> >> > @@ -1093,6 +1108,16 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type) >> > u16 portstatus, portchange; >> > >> > portstatus = portchange = 0; >> > + >> > + /* Some connected devices might be still in unknown state even >> > + * after reset-resume, a WARM_RESET gets the connected device >> > + * to the normal state. >> > + */ >> > + if (udev && hub_is_superspeed(hub->hdev) && >> > + type == HUB_RESET_RESUME) >> > + hub_port_reset(hub, port1, NULL, >> > + HUB_BH_RESET_TIME, true); >> >> Please don't do this all the time to every attached port. Do it only >> when it is really needed. > > Agreed. Can we at least limit the warm reset to devices directly > attached to roothubs? You can also change this code to get the port > status and only do the warm reset if the port link state is > USB_SS_PORT_LS_POLLING, USB_SS_PORT_LS_RX_DETECT, or > USB_SS_PORT_LS_SS_INACTIVE. > >> Shouldn't you pass udev as the third argument? If not, please explain >> why not. >> >> Finally, I don't see why you put this in hub_activate(). Isn't it more >> closely connected with the reset-resume procedure for the child device? > > Sarah Sharp > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Alan, On Mon, Dec 9, 2013 at 8:54 PM, Alan Stern <stern@rowland.harvard.edu> wrote: > On Mon, 9 Dec 2013, Vikas Sajjan wrote: > >> Does warm reset while activating SuperSpeed HUBs if the hub activate type >> is HUB_RESET_RESUME. >> >> When we do Suspend-to-RAM with (any one of the 16, 32, 64 Jetflash) transcend >> USB 3.0 device connected on 3.0 port, during resume I noticed that the >> XHCI controller has moved to sometimes RECOVERY, POLLING or INACTIVE STATE. >> This behaviour is inconsistent and the connection with connected USB 3.0 device >> on 3.0 port was LOST. >> >> Doing warm reset while activating SuperSpeed HUBs if the hub >> activate type is HUB_RESET_RESUME, gets the connected device to the stable state. >> >> Reviewed at https://chromium-review.googlesource.com/#/c/177132/ >> >> Tested on exynos5420 and exynos5250 with Transcend Jetflash USB 3.0 device (8564:1000) >> >> rebased on Greg Kroah-Hartman's usb-next >> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git >> >> Signed-off-by: Vikas Sajjan <vikas.sajjan@samsung.com> >> --- >> drivers/usb/core/hub.c | 41 +++++++++++++++++++++++++---------------- >> 1 file changed, 25 insertions(+), 16 deletions(-) >> >> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c >> index a7c04e2..d8432b0 100644 >> --- a/drivers/usb/core/hub.c >> +++ b/drivers/usb/core/hub.c > >> @@ -1093,6 +1108,16 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type) >> u16 portstatus, portchange; >> >> portstatus = portchange = 0; >> + >> + /* Some connected devices might be still in unknown state even >> + * after reset-resume, a WARM_RESET gets the connected device >> + * to the normal state. >> + */ >> + if (udev && hub_is_superspeed(hub->hdev) && >> + type == HUB_RESET_RESUME) >> + hub_port_reset(hub, port1, NULL, >> + HUB_BH_RESET_TIME, true); > > Please don't do this all the time to every attached port. Do it only > when it is really needed. > > Shouldn't you pass udev as the third argument? If not, please explain > why not. yea, I have NOT tried passing udev as the third argument. > > Finally, I don't see why you put this in hub_activate(). Isn't it more > closely connected with the reset-resume procedure for the child device? I was trying to add a FIX in usb_port_resume(), but in our case we have CONFIG_USB_DEFAULT_PERSIST disabled. Interestingly, if I disable the CONFIG_USB_DEFAULT_PERSIST, then the function usb_port_resume() will never be called and transcend Jetflash device Suspend-to-RAM fails. In normal scenario (ie., CONFIG_USB_DEFAULT_PERSIST=y) the sequence is: =========================================================================== Step 1: For Root HUB : usb_resume_both() ---> usb_resume_device() -> generic_resume() --> hcd_bus_resume() --> xhci_bus_resume(). | |-->usb_resume_interface() ---> hub_reset_resume() --> xhci_update_hub_device() Step 2: For the Device connected usb_resume_both() ---> usb_resume_device() -> generic_resume()-->usb_port_resume()-->hub_port_logical_disconnect() --> hub_port_disable() --> hub_usb3_port_disable(). In our scenario (ie., CONFIG_USB_DEFAULT_PERSIST=N) the sequence is: =============================================================================== Step 1: For Root HUB usb_resume_both() ---> usb_resume_device() -> generic_resume() --> hcd_bus_resume() --> xhci_bus_resume(). | |-->usb_resume_interface() ---> hub_reset_resume() --> xhci_update_hub_device() Step 2 : Never occurs So Suspend-to-RAM fails. Hence i added a FIX in hub_reset_resume(). Let me know if I am wrong. > > Alan Stern > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 10 Dec 2013, Vikas Sajjan wrote: > > Finally, I don't see why you put this in hub_activate(). Isn't it more > > closely connected with the reset-resume procedure for the child device? > > > I was trying to add a FIX in usb_port_resume(), but in our case we > have CONFIG_USB_DEFAULT_PERSIST disabled. > > Interestingly, if I disable the CONFIG_USB_DEFAULT_PERSIST, then the > function usb_port_resume() will never be called and transcend Jetflash > device Suspend-to-RAM fails. I don't know what you mean by "fails". The system goes to sleep and then later on wakes up, doesn't it? Do you mean that the Jetflash device gets disconnected when the system wakes up? That's _supposed_ to happen under those circumstances. When hub_activate() sees HUB_RESET_RESUME, all child devices get disconnected except those where udev->persist_enabled is set. > In normal scenario (ie., CONFIG_USB_DEFAULT_PERSIST=y) the sequence is: > =========================================================================== > Step 1: For Root HUB : > usb_resume_both() ---> usb_resume_device() -> generic_resume() --> > hcd_bus_resume() --> xhci_bus_resume(). > | > |-->usb_resume_interface() ---> > hub_reset_resume() --> xhci_update_hub_device() > > Step 2: For the Device connected > usb_resume_both() ---> usb_resume_device() -> > generic_resume()-->usb_port_resume()-->hub_port_logical_disconnect() You lost me there. Why does usb_port_resume call hub_port_logical_disconnect? Does this happen because check_port_resume_type returns an error code? What are the values of the portchange and portstatus arguments to check_port_resume_type? > --> hub_port_disable() --> hub_usb3_port_disable(). > > > In our scenario (ie., CONFIG_USB_DEFAULT_PERSIST=N) the sequence is: > =============================================================================== > Step 1: For Root HUB > usb_resume_both() ---> usb_resume_device() -> generic_resume() --> > hcd_bus_resume() --> xhci_bus_resume(). > | > |-->usb_resume_interface() ---> > hub_reset_resume() --> xhci_update_hub_device() > > Step 2 : Never occurs That's exactly right. > So Suspend-to-RAM fails. No, it succeeds in behaving the way it is intended to behave. > Hence i added a FIX in hub_reset_resume(). > > Let me know if I am wrong. I can't tell at this point. It depends on the reason why hub_port_logical_disconnect got called. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> I don't know what you mean by "fails". The system goes to sleep and > then later on wakes up, doesn't it? > > Do you mean that the Jetflash device gets disconnected when the system > wakes up? That's _supposed_ to happen under those circumstances. > When hub_activate() sees HUB_RESET_RESUME, all child devices get > disconnected except those where udev->persist_enabled is set. This patch was written in response to the same bug as my "usb: hub: Use correct reset for wedged USB3 devices that are NOTATTACHED" submission. My patch only helps when the port gets stuck in Compliance Mode, but Vikas reports that he can sometimes see it stuck in Polling or Recovery states as well. The underlying issue is a deadlock in the USB 3.0 link training state machine when the host controller is unilaterally reset on resume (without driving a reset on the bus). The host port starts out back in Rx.Detect without remembering anything about its previous state, but the device is still in U3. The host detects Rx terminations, moves to Polling and starts sending LFPS link training packets, but the device doesn't expect those and interprets them as link problems (moving to Recovery). What happens next seems to be device specific, but apparently the device can end up in SS.Inactive while the host port gets stuck in Polling or Recovery (or some kind of livelock between those). This patch tries to warm reset all USB 3.0 ports on reset-resume (after xhci_reset() was called) that had devices connected to them before suspend. This seems to be the only way to ensure the devices' state machines get back to a well-defined state that the host can work with. I don't think this is a specific hardware bug, it's just an unfortunate design flaw that the USB 3.0 spec doesn't account for a root hub port being reset independently of its connected device. I think Sarah is correct that it could be limited to root hubs, though. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Dec 11, 2013 at 11:00:13AM -0800, Julius Werner wrote: > > I don't know what you mean by "fails". The system goes to sleep and > > then later on wakes up, doesn't it? > > > > Do you mean that the Jetflash device gets disconnected when the system > > wakes up? That's _supposed_ to happen under those circumstances. > > When hub_activate() sees HUB_RESET_RESUME, all child devices get > > disconnected except those where udev->persist_enabled is set. > > This patch was written in response to the same bug as my "usb: hub: > Use correct reset for wedged USB3 devices that are NOTATTACHED" > submission. My patch only helps when the port gets stuck in Compliance > Mode, but Vikas reports that he can sometimes see it stuck in Polling > or Recovery states as well. > > The underlying issue is a deadlock in the USB 3.0 link training state > machine when the host controller is unilaterally reset on resume > (without driving a reset on the bus). The xHCI spec requires that when the xHCI host is reset, a USB reset is driven down the USB 3.0 ports. If hot reset fails, the port may migrate to warm reset. See table 32 in the xHCI spec, in the definition of HCRST. It sounds like this host doesn't drive a USB reset down USB 3.0 ports at all on host controller reset? > The host port starts out back in > Rx.Detect without remembering anything about its previous state, but > the device is still in U3. The host detects Rx terminations, moves to > Polling and starts sending LFPS link training packets, but the device > doesn't expect those and interprets them as link problems (moving to > Recovery). What happens next seems to be device specific, but > apparently the device can end up in SS.Inactive while the host port > gets stuck in Polling or Recovery (or some kind of livelock between > those). > > This patch tries to warm reset all USB 3.0 ports on reset-resume > (after xhci_reset() was called) that had devices connected to them > before suspend. This seems to be the only way to ensure the devices' > state machines get back to a well-defined state that the host can work > with. I don't think this is a specific hardware bug, it's just an > unfortunate design flaw that the USB 3.0 spec doesn't account for a > root hub port being reset independently of its connected device. I > think Sarah is correct that it could be limited to root hubs, though. Sarah Sharp -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index a7c04e2..d8432b0 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -993,6 +993,21 @@ int usb_remove_device(struct usb_device *udev) return 0; } +#define PORT_RESET_TRIES 5 +#define SET_ADDRESS_TRIES 2 +#define GET_DESCRIPTOR_TRIES 2 +#define SET_CONFIG_TRIES (2 * (use_both_schemes + 1)) +#define USE_NEW_SCHEME(i) ((i) / 2 == (int)old_scheme_first) + +#define HUB_ROOT_RESET_TIME 50 /* times are in msec */ +#define HUB_SHORT_RESET_TIME 10 +#define HUB_BH_RESET_TIME 50 +#define HUB_LONG_RESET_TIME 200 +#define HUB_RESET_TIMEOUT 800 + +static int hub_port_reset(struct usb_hub *hub, int port1, + struct usb_device *udev, unsigned int delay, bool warm); + enum hub_activation_type { HUB_INIT, HUB_INIT2, HUB_INIT3, /* INITs must come first */ HUB_POST_RESET, HUB_RESUME, HUB_RESET_RESUME, @@ -1093,6 +1108,16 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type) u16 portstatus, portchange; portstatus = portchange = 0; + + /* Some connected devices might be still in unknown state even + * after reset-resume, a WARM_RESET gets the connected device + * to the normal state. + */ + if (udev && hub_is_superspeed(hub->hdev) && + type == HUB_RESET_RESUME) + hub_port_reset(hub, port1, NULL, + HUB_BH_RESET_TIME, true); + status = hub_port_status(hub, port1, &portstatus, &portchange); if (udev || (portstatus & USB_PORT_STAT_CONNECTION)) dev_dbg(hub->intfdev, @@ -2510,22 +2535,6 @@ static unsigned hub_is_wusb(struct usb_hub *hub) return hcd->wireless; } - -#define PORT_RESET_TRIES 5 -#define SET_ADDRESS_TRIES 2 -#define GET_DESCRIPTOR_TRIES 2 -#define SET_CONFIG_TRIES (2 * (use_both_schemes + 1)) -#define USE_NEW_SCHEME(i) ((i) / 2 == (int)old_scheme_first) - -#define HUB_ROOT_RESET_TIME 50 /* times are in msec */ -#define HUB_SHORT_RESET_TIME 10 -#define HUB_BH_RESET_TIME 50 -#define HUB_LONG_RESET_TIME 200 -#define HUB_RESET_TIMEOUT 800 - -static int hub_port_reset(struct usb_hub *hub, int port1, - struct usb_device *udev, unsigned int delay, bool warm); - /* Is a USB 3.0 port in the Inactive or Complinance Mode state? * Port worm reset is required to recover */
Does warm reset while activating SuperSpeed HUBs if the hub activate type is HUB_RESET_RESUME. When we do Suspend-to-RAM with (any one of the 16, 32, 64 Jetflash) transcend USB 3.0 device connected on 3.0 port, during resume I noticed that the XHCI controller has moved to sometimes RECOVERY, POLLING or INACTIVE STATE. This behaviour is inconsistent and the connection with connected USB 3.0 device on 3.0 port was LOST. Doing warm reset while activating SuperSpeed HUBs if the hub activate type is HUB_RESET_RESUME, gets the connected device to the stable state. Reviewed at https://chromium-review.googlesource.com/#/c/177132/ Tested on exynos5420 and exynos5250 with Transcend Jetflash USB 3.0 device (8564:1000) rebased on Greg Kroah-Hartman's usb-next git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git Signed-off-by: Vikas Sajjan <vikas.sajjan@samsung.com> --- drivers/usb/core/hub.c | 41 +++++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 16 deletions(-)