Re: [PATCH] USB: core: Add warm reset while reset-resuming SuperSpeed HUBs
On Thu, Dec 12, 2013 at 11:05:04AM -0500, Alan Stern wrote: On Wed, 11 Dec 2013, Julius Werner wrote: ...although, the spec says that it does not wait for the port resets to complete. As far as I can see re-issuing a warm reset and waiting is the only way to guarantee the core times the recovery. Presumably the portstatus debounce in hub_activate() mitigates this, but that 100ms is less than a full reset timeout. It's definitely not just a timing issue for us. I can't reproduce all the same cases as Vikas, but when I attach a USB analyzer to the ones I do see the host controller doesn't even start sending a reset. 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? Oh, interesting, I hadn't seen that yet. So I guess the spec itself is fine if it were followed to the letter. I did some more tests about this on my Exynos machine: when I put a device to autosuspend (U3) and manually poke the xHC reset bit, I do see an automatic warm reset on the analyzer and the ports manage to retrain to U0. But after a system suspend/resume which calls xhci_reset() in the process, there is no reset on the wire. I also noticed that it doesn't drive a reset (even after manual poking) when there is no device connected on the other end of the analyzer. So this might be our problem: maybe these host controllers (Synopsys DesignWare) issue the spec-mandated warm reset only on ports where they think there is a device attached. But after a system suspend/resume (where the whole IP block on the SoC was powered down), the host controller cannot know that there is still a device with an active power session attached, and therefore doesn't drive the reset on its own. Ok, that makes some sense. I could see why host controllers wouldn't want to drive reset on an unconnected port. Even though this is a host controller bug, we still have to deal with it somehow. I guess we could move the code into xhci_plat_resume() and hide it behind a quirk to lessen the impact. But since reset_resume is not a common case for most host controllers, it's hard to say if this is DesignWare specific or a more widespread implementation mistake. I was going to suggest something along these lines too. This seems to be a bug in xHCI. Therefore the fix belongs in xhci-hcd, not in the hub driver. I agree. Is there a chance that the Synopsys DesignWare will be a PCI device instead of a platform device? If so, it would be better to put the code into xhci_resume instead of xhci_plat_resume. That also allows you to only issue the warm reset when the register restore state command fails, after the xhci_reset call. Also, I assume that other systems with the Synopsys DesignWare IP will experience this issue? I know of at least two other chipsets that will include that IP, and it would be good to find a way to trigger on the Synopsys IP, rather than off xHCI PCI vendor and device ID. Otherwise we'll be adding PCI IDs to the xHCI driver quirks for many many kernels to come. I'm actually leaning towards enabling the check for warm reset broadly. It seems like it wouldn't hurt to issue a warm reset on the USB 3.0 ports if they're in compliance, poll, or rx.detect. So, let's enable this broadly in xhci_resume, mark the patch for stable, but ask for the backport to be delayed until 3.13.3 is out, to allow for more testing. If anyone complains of xHCI behavior changes, we'll change the code to add a quirk. Sarah Sharp -- 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: [PATCH] USB: core: Add warm reset while reset-resuming SuperSpeed HUBs
Hi Sarah, On Fri, Dec 13, 2013 at 09:48:15AM -0800, Sarah Sharp wrote: On Thu, Dec 12, 2013 at 11:05:04AM -0500, Alan Stern wrote: On Wed, 11 Dec 2013, Julius Werner wrote: ...although, the spec says that it does not wait for the port resets to complete. As far as I can see re-issuing a warm reset and waiting is the only way to guarantee the core times the recovery. Presumably the portstatus debounce in hub_activate() mitigates this, but that 100ms is less than a full reset timeout. It's definitely not just a timing issue for us. I can't reproduce all the same cases as Vikas, but when I attach a USB analyzer to the ones I do see the host controller doesn't even start sending a reset. 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? Oh, interesting, I hadn't seen that yet. So I guess the spec itself is fine if it were followed to the letter. I did some more tests about this on my Exynos machine: when I put a device to autosuspend (U3) and manually poke the xHC reset bit, I do see an automatic warm reset on the analyzer and the ports manage to retrain to U0. But after a system suspend/resume which calls xhci_reset() in the process, there is no reset on the wire. I also noticed that it doesn't drive a reset (even after manual poking) when there is no device connected on the other end of the analyzer. So this might be our problem: maybe these host controllers (Synopsys DesignWare) issue the spec-mandated warm reset only on ports where they think there is a device attached. But after a system suspend/resume (where the whole IP block on the SoC was powered down), the host controller cannot know that there is still a device with an active power session attached, and therefore doesn't drive the reset on its own. Ok, that makes some sense. I could see why host controllers wouldn't want to drive reset on an unconnected port. Even though this is a host controller bug, we still have to deal with it somehow. I guess we could move the code into xhci_plat_resume() and hide it behind a quirk to lessen the impact. But since reset_resume is not a common case for most host controllers, it's hard to say if this is DesignWare specific or a more widespread implementation mistake. I was going to suggest something along these lines too. This seems to be a bug in xHCI. Therefore the fix belongs in xhci-hcd, not in the hub driver. I agree. Is there a chance that the Synopsys DesignWare will be a PCI device instead of a platform device? If so, it would be better to put the code into xhci_resume instead of xhci_plat_resume. That also allows DWC3 on Intel Baytrail and Merrifield is PCI device. Br, David -- 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: [PATCH] USB: core: Add warm reset while reset-resuming SuperSpeed HUBs
On Fri, Dec 13, 2013 at 9:48 AM, Sarah Sharp sarah.a.sh...@linux.intel.com wrote: On Thu, Dec 12, 2013 at 11:05:04AM -0500, Alan Stern wrote: On Wed, 11 Dec 2013, Julius Werner wrote: ...although, the spec says that it does not wait for the port resets to complete. As far as I can see re-issuing a warm reset and waiting is the only way to guarantee the core times the recovery. Presumably the portstatus debounce in hub_activate() mitigates this, but that 100ms is less than a full reset timeout. It's definitely not just a timing issue for us. I can't reproduce all the same cases as Vikas, but when I attach a USB analyzer to the ones I do see the host controller doesn't even start sending a reset. 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? Oh, interesting, I hadn't seen that yet. So I guess the spec itself is fine if it were followed to the letter. I did some more tests about this on my Exynos machine: when I put a device to autosuspend (U3) and manually poke the xHC reset bit, I do see an automatic warm reset on the analyzer and the ports manage to retrain to U0. But after a system suspend/resume which calls xhci_reset() in the process, there is no reset on the wire. I also noticed that it doesn't drive a reset (even after manual poking) when there is no device connected on the other end of the analyzer. So this might be our problem: maybe these host controllers (Synopsys DesignWare) issue the spec-mandated warm reset only on ports where they think there is a device attached. But after a system suspend/resume (where the whole IP block on the SoC was powered down), the host controller cannot know that there is still a device with an active power session attached, and therefore doesn't drive the reset on its own. Ok, that makes some sense. I could see why host controllers wouldn't want to drive reset on an unconnected port. Even though this is a host controller bug, we still have to deal with it somehow. I guess we could move the code into xhci_plat_resume() and hide it behind a quirk to lessen the impact. But since reset_resume is not a common case for most host controllers, it's hard to say if this is DesignWare specific or a more widespread implementation mistake. I was going to suggest something along these lines too. This seems to be a bug in xHCI. Therefore the fix belongs in xhci-hcd, not in the hub driver. I agree. Is there a chance that the Synopsys DesignWare will be a PCI device instead of a platform device? If so, it would be better to put the code into xhci_resume instead of xhci_plat_resume. That also allows you to only issue the warm reset when the register restore state command fails, after the xhci_reset call. Also, I assume that other systems with the Synopsys DesignWare IP will experience this issue? I know of at least two other chipsets that will include that IP, and it would be good to find a way to trigger on the Synopsys IP, rather than off xHCI PCI vendor and device ID. Otherwise we'll be adding PCI IDs to the xHCI driver quirks for many many kernels to come. I'm actually leaning towards enabling the check for warm reset broadly. It seems like it wouldn't hurt to issue a warm reset on the USB 3.0 ports if they're in compliance, poll, or rx.detect. So, let's enable this broadly in xhci_resume, mark the patch for stable, but ask for the backport to be delayed until 3.13.3 is out, to allow for more testing. If anyone complains of xHCI behavior changes, we'll change the code to add a quirk. Is there a clean way to make this per-port rather than globally at xhci_resume()? I am looking to hook into this for port power recovery as Tianyu's testing encountered warm reset required conditions at runtime_resume. I'm still on the hunt for a solid reproducer, but it indicates this is a more general quirk with power session recovery. -- 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: [PATCH] USB: core: Add warm reset while reset-resuming SuperSpeed HUBs
On Fri, 13 Dec 2013, Dan Williams wrote: I'm actually leaning towards enabling the check for warm reset broadly. It seems like it wouldn't hurt to issue a warm reset on the USB 3.0 ports if they're in compliance, poll, or rx.detect. So, let's enable this broadly in xhci_resume, mark the patch for stable, but ask for the backport to be delayed until 3.13.3 is out, to allow for more testing. If anyone complains of xHCI behavior changes, we'll change the code to add a quirk. Is there a clean way to make this per-port rather than globally at xhci_resume()? I am looking to hook into this for port power recovery as Tianyu's testing encountered warm reset required conditions at runtime_resume. I'm still on the hunt for a solid reproducer, but it indicates this is a more general quirk with power session recovery. There's no reason you can't do per-port testing inside xhci_resume (assuming you know what to test for) as well as putting a warm reset in the port-power handler of xhci_hub_control. Of course, doing simultaneous warm resets on multiple ports will use less time than resetting each port individually, in sequence. 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: [PATCH] USB: core: Add warm reset while reset-resuming SuperSpeed HUBs
On Fri, Dec 13, 2013 at 10:15 AM, Alan Stern st...@rowland.harvard.edu wrote: On Fri, 13 Dec 2013, Dan Williams wrote: I'm actually leaning towards enabling the check for warm reset broadly. It seems like it wouldn't hurt to issue a warm reset on the USB 3.0 ports if they're in compliance, poll, or rx.detect. So, let's enable this broadly in xhci_resume, mark the patch for stable, but ask for the backport to be delayed until 3.13.3 is out, to allow for more testing. If anyone complains of xHCI behavior changes, we'll change the code to add a quirk. Is there a clean way to make this per-port rather than globally at xhci_resume()? I am looking to hook into this for port power recovery as Tianyu's testing encountered warm reset required conditions at runtime_resume. I'm still on the hunt for a solid reproducer, but it indicates this is a more general quirk with power session recovery. There's no reason you can't do per-port testing inside xhci_resume (assuming you know what to test for) as well as putting a warm reset in the port-power handler of xhci_hub_control. I'm just uneasy putting the recovery there as we lose the context of why the port was powered-on. For example we don't want to pre-empt/duplicate a reset in xhci_hub_control() that is already specified in hub_events(). Of course, doing simultaneous warm resets on multiple ports will use less time than resetting each port individually, in sequence. For the hub resume case, yes. For pm_runtime_resume of an individual port I believe it needs to be synchronous. -- 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: [PATCH] USB: core: Add warm reset while reset-resuming SuperSpeed HUBs
On Fri, Dec 13, 2013 at 10:00:28AM -0800, David Cohen wrote: Hi Sarah, On Fri, Dec 13, 2013 at 09:48:15AM -0800, Sarah Sharp wrote: On Thu, Dec 12, 2013 at 11:05:04AM -0500, Alan Stern wrote: On Wed, 11 Dec 2013, Julius Werner wrote: ...although, the spec says that it does not wait for the port resets to complete. As far as I can see re-issuing a warm reset and waiting is the only way to guarantee the core times the recovery. Presumably the portstatus debounce in hub_activate() mitigates this, but that 100ms is less than a full reset timeout. It's definitely not just a timing issue for us. I can't reproduce all the same cases as Vikas, but when I attach a USB analyzer to the ones I do see the host controller doesn't even start sending a reset. 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? Oh, interesting, I hadn't seen that yet. So I guess the spec itself is fine if it were followed to the letter. I did some more tests about this on my Exynos machine: when I put a device to autosuspend (U3) and manually poke the xHC reset bit, I do see an automatic warm reset on the analyzer and the ports manage to retrain to U0. But after a system suspend/resume which calls xhci_reset() in the process, there is no reset on the wire. I also noticed that it doesn't drive a reset (even after manual poking) when there is no device connected on the other end of the analyzer. So this might be our problem: maybe these host controllers (Synopsys DesignWare) issue the spec-mandated warm reset only on ports where they think there is a device attached. But after a system suspend/resume (where the whole IP block on the SoC was powered down), the host controller cannot know that there is still a device with an active power session attached, and therefore doesn't drive the reset on its own. Ok, that makes some sense. I could see why host controllers wouldn't want to drive reset on an unconnected port. Even though this is a host controller bug, we still have to deal with it somehow. I guess we could move the code into xhci_plat_resume() and hide it behind a quirk to lessen the impact. But since reset_resume is not a common case for most host controllers, it's hard to say if this is DesignWare specific or a more widespread implementation mistake. I was going to suggest something along these lines too. This seems to be a bug in xHCI. Therefore the fix belongs in xhci-hcd, not in the hub driver. I agree. Is there a chance that the Synopsys DesignWare will be a PCI device instead of a platform device? If so, it would be better to put the code into xhci_resume instead of xhci_plat_resume. That also allows DWC3 on Intel Baytrail and Merrifield is PCI device. But it actually registers xHCI's platform device to probe it. So, nevermind. Br, David -- 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: [PATCH] USB: core: Add warm reset while reset-resuming SuperSpeed HUBs
On Wed, 11 Dec 2013, Julius Werner wrote: ...although, the spec says that it does not wait for the port resets to complete. As far as I can see re-issuing a warm reset and waiting is the only way to guarantee the core times the recovery. Presumably the portstatus debounce in hub_activate() mitigates this, but that 100ms is less than a full reset timeout. It's definitely not just a timing issue for us. I can't reproduce all the same cases as Vikas, but when I attach a USB analyzer to the ones I do see the host controller doesn't even start sending a reset. 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? Oh, interesting, I hadn't seen that yet. So I guess the spec itself is fine if it were followed to the letter. I did some more tests about this on my Exynos machine: when I put a device to autosuspend (U3) and manually poke the xHC reset bit, I do see an automatic warm reset on the analyzer and the ports manage to retrain to U0. But after a system suspend/resume which calls xhci_reset() in the process, there is no reset on the wire. I also noticed that it doesn't drive a reset (even after manual poking) when there is no device connected on the other end of the analyzer. So this might be our problem: maybe these host controllers (Synopsys DesignWare) issue the spec-mandated warm reset only on ports where they think there is a device attached. But after a system suspend/resume (where the whole IP block on the SoC was powered down), the host controller cannot know that there is still a device with an active power session attached, and therefore doesn't drive the reset on its own. Even though this is a host controller bug, we still have to deal with it somehow. I guess we could move the code into xhci_plat_resume() and hide it behind a quirk to lessen the impact. But since reset_resume is not a common case for most host controllers, it's hard to say if this is DesignWare specific or a more widespread implementation mistake. I was going to suggest something along these lines too. This seems to be a bug in xHCI. Therefore the fix belongs in xhci-hcd, not in the hub driver. 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: [PATCH] USB: core: Add warm reset while reset-resuming SuperSpeed HUBs
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-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: core: Add warm reset while reset-resuming SuperSpeed HUBs
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-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: core: Add warm reset while reset-resuming SuperSpeed HUBs
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-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: core: Add warm reset while reset-resuming SuperSpeed HUBs
On Wed, Dec 11, 2013 at 11:36 AM, Sarah Sharp
sarah.a.sh...@linux.intel.com wrote:
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?
...although, the spec says that it does not wait for the port resets
to complete. As far as I can see re-issuing a warm reset and waiting
is the only way to guarantee the core times the recovery. Presumably
the portstatus debounce in hub_activate() mitigates this, but that
100ms is less than a full reset timeout. I have something similar in
the port power rework patches [1], but I think something like the
following (untested) is more generic, it arranges for reset_resume to
start with a warm reset if necessary.
(also attached)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index a7c04e24ca48..30ce237569dd 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2783,8 +2783,14 @@ static int check_port_resume_type(struct
usb_device *udev,
struct usb_hub *hub, int port1,
int status, unsigned portchange, unsigned portstatus)
{
+ /* Did the port go SS.Inactive? Even if -persist_enabled is
cleared the
+* device won't come back until a warm reset completes
+*/
+ if (hub_port_warm_reset_required(hub, portstatus)) {
+ udev-reset_resume = 1;
+ udev-reset_resume_warm = 1;
/* Is the device still present? */
- if (status || port_is_suspended(hub, portstatus) ||
+ } else if (status || port_is_suspended(hub, portstatus) ||
!port_is_power_on(hub, portstatus) ||
!(portstatus USB_PORT_STAT_CONNECTION)) {
if (status = 0)
@@ -4022,7 +4028,8 @@ 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, false);
+ retval = hub_port_reset(hub, port1, udev, delay,
+ udev-reset_resume_warm);
if (retval 0) /* error or disconnect */
goto fail;
/* success, speed is known */
@@ -4730,7 +4737,8 @@ static void hub_events(void)
/* deal with port status changes */
for (i = 1; i = hdev-maxchild; i++) {
- if (test_bit(i, hub-busy_bits))
+ if (test_bit(i, hub-busy_bits) ||
+ test_bit(i, hub-delayed_change_bits))
continue;
connect_change = test_bit(i, hub-change_bits);
wakeup_change = test_and_clear_bit(i, hub-wakeup_bits);
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 7454865ad148..ff1b6fe4a0ff 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -572,6 +572,7 @@ struct usb_device {
unsigned do_remote_wakeup:1;
unsigned reset_resume:1;
+ unsigned reset_resume_warm:1;
unsigned port_is_suspended:1;
#endif
struct wusb_dev *wusb_dev;
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).
In testing the port power patches I see this particular device give up
on its superspeed connection if it sees too many link failures and
fallsback to
Re: [PATCH] USB: core: Add warm reset while reset-resuming SuperSpeed HUBs
On Wed, Dec 11, 2013 at 2:51 PM, Dan Williams dan.j.willi...@gmail.com wrote:
On Wed, Dec 11, 2013 at 11:36 AM, Sarah Sharp
sarah.a.sh...@linux.intel.com wrote:
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?
...although, the spec says that it does not wait for the port resets
to complete. As far as I can see re-issuing a warm reset and waiting
is the only way to guarantee the core times the recovery. Presumably
the portstatus debounce in hub_activate() mitigates this, but that
100ms is less than a full reset timeout. I have something similar in
the port power rework patches [1], but I think something like the
following (untested) is more generic, it arranges for reset_resume to
start with a warm reset if necessary.
(also attached)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index a7c04e24ca48..30ce237569dd 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2783,8 +2783,14 @@ static int check_port_resume_type(struct
usb_device *udev,
struct usb_hub *hub, int port1,
int status, unsigned portchange, unsigned portstatus)
{
+ /* Did the port go SS.Inactive? Even if -persist_enabled is
cleared the
+* device won't come back until a warm reset completes
+*/
+ if (hub_port_warm_reset_required(hub, portstatus)) {
+ udev-reset_resume = 1;
+ udev-reset_resume_warm = 1;
Also need to set 'status' to 0 here.
If it's truly just a case of waiting for port warm resets to complete
it might be better to inject additional debounce delay here, but the
spec seems to indicate that there is no way to know that escalated
warm resets are in progress. 4.19.5.1 says The Port Reset (PR) flag
shall be ‘1’ while Hot or Warm Reset is being executed. The Port Reset
Change (PRC) flag shall be set (‘1’) when the reset execution is
complete and PR transitions to ‘0’ but that is only if software
initiated the warm reset. When the warm reset was the result of an
HCRST we hit Note, the completion of the xHC reset process is not
gated by the Root Hub port reset process.
--
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: [PATCH] USB: core: Add warm reset while reset-resuming SuperSpeed HUBs
...although, the spec says that it does not wait for the port resets to complete. As far as I can see re-issuing a warm reset and waiting is the only way to guarantee the core times the recovery. Presumably the portstatus debounce in hub_activate() mitigates this, but that 100ms is less than a full reset timeout. It's definitely not just a timing issue for us. I can't reproduce all the same cases as Vikas, but when I attach a USB analyzer to the ones I do see the host controller doesn't even start sending a reset. 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? Oh, interesting, I hadn't seen that yet. So I guess the spec itself is fine if it were followed to the letter. I did some more tests about this on my Exynos machine: when I put a device to autosuspend (U3) and manually poke the xHC reset bit, I do see an automatic warm reset on the analyzer and the ports manage to retrain to U0. But after a system suspend/resume which calls xhci_reset() in the process, there is no reset on the wire. I also noticed that it doesn't drive a reset (even after manual poking) when there is no device connected on the other end of the analyzer. So this might be our problem: maybe these host controllers (Synopsys DesignWare) issue the spec-mandated warm reset only on ports where they think there is a device attached. But after a system suspend/resume (where the whole IP block on the SoC was powered down), the host controller cannot know that there is still a device with an active power session attached, and therefore doesn't drive the reset on its own. Even though this is a host controller bug, we still have to deal with it somehow. I guess we could move the code into xhci_plat_resume() and hide it behind a quirk to lessen the impact. But since reset_resume is not a common case for most host controllers, it's hard to say if this is DesignWare specific or a more widespread implementation mistake. -- 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
[PATCH] USB: core: Add warm reset while reset-resuming SuperSpeed HUBs
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.saj...@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_TIME50 /* times are in msec */
+#define HUB_SHORT_RESET_TIME 10
+#define HUB_BH_RESET_TIME 50
+#define HUB_LONG_RESET_TIME200
+#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_TIME50 /* times are in msec */
-#define HUB_SHORT_RESET_TIME 10
-#define HUB_BH_RESET_TIME 50
-#define HUB_LONG_RESET_TIME200
-#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-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: core: Add warm reset while reset-resuming SuperSpeed HUBs
Hi Vikas,
On Mon, Dec 9, 2013 at 5:59 PM, Vikas Sajjan vikas.saj...@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.saj...@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_TIME50 /* times are in msec */
+#define HUB_SHORT_RESET_TIME 10
+#define HUB_BH_RESET_TIME 50
+#define HUB_LONG_RESET_TIME200
+#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_TIME50 /* times are in msec */
-#define HUB_SHORT_RESET_TIME 10
-#define HUB_BH_RESET_TIME 50
-#define HUB_LONG_RESET_TIME200
-#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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Best Regards
Vivek Gautam
Samsung RD Institute, Bangalore
India
--
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: [PATCH] USB: core: Add warm reset while reset-resuming SuperSpeed HUBs
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.saj...@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-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: core: Add warm reset while reset-resuming SuperSpeed HUBs
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.saj...@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-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: core: Add warm reset while reset-resuming SuperSpeed HUBs
Hi Sarah,
On Mon, Dec 9, 2013 at 11:54 PM, Sarah Sharp
sarah.a.sh...@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.saj...@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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
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: [PATCH] USB: core: Add warm reset while reset-resuming SuperSpeed HUBs
Hi Alan, On Mon, Dec 9, 2013 at 8:54 PM, Alan Stern st...@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.saj...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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
