Re: [RFT 0/2] TI compliance mode fixes
On 05/09/2013 07:31 PM, Sarah Sharp wrote: Hi Tony, Don, and Oliver, Can one of you double check these two patches resolve the resume from S3 issue on effected HP systems with the TI compliance mode quirk? Oliver has confirmed that the first patch works, but no one has tested the patch to disable D3cold for the xHCI host on those systems. Ying, can you double check that the code to disable D3cold looks sane? If these patches work on the systems, I will send them out next week to Greg. Thanks, Sarah Sharp Just a quick note. Both patches work correctly on 3.8. However, I'm experiencing a hang when returning from hibernate in 3.10-rc1. I will troubleshoot this tomorrow. -- 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: [RFT 0/2] TI compliance mode fixes
On 05/09/2013 07:31 PM, Sarah Sharp wrote: Hi Tony, Don, and Oliver, Can one of you double check these two patches resolve the resume from S3 issue on effected HP systems with the TI compliance mode quirk? Oliver has confirmed that the first patch works, but no one has tested the patch to disable D3cold for the xHCI host on those systems. Ying, can you double check that the code to disable D3cold looks sane? If these patches work on the systems, I will send them out next week to Greg. Thanks, Sarah Sharp Sarah, I'll be testing these patches in both the upstream and RHEL6 environments over the next couple days. Regards, Tony -- 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: [RFT] xhci - correct comp_mode_recovery_timer on return from hibernate
On 04/17/2013 08:27 PM, Sarah Sharp wrote: -- snip -- [Sarah reworked this patch to cover the case where the xHCI restore register operation fails, and (temp STS_SRE) is true (and we re-init the host, including re-init for the compliance mode), but hibernate is false. The original patch would have caused list corruption in this case.] Signed-off-by: Tony Camuso tcam...@redhat.com Acked-by: Don Zickus dzic...@redhat.com Signed-off-by: Sarah Sharp sarah.a.sh...@linux.intel.com --- Tony, will you test this version? Thanks! Sarah, With ioatdma blacklisted, I was able to test this patch on an hp z820, which is the system family on which the original problem was detected by hp. The amended patch works correctly. State is correctly restored from both hibernate and suspend. -- 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: 3.9-rc7 suspend failure
On Tue, Apr 23, 2013 at 04:18:00PM -0400, Tony Camuso wrote: It hangs in both suspend and resume with vanilla 3.9-rc7 and rc8. That should say, It hangs in resume-from-suspend -- 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: 3.9-rc7 suspend failure
On 04/26/2013 12:47 PM, Dave Jiang wrote: I have a feeling it's something else and the patch maybe have exposed it. A channel error occured and bit 12 is Completion Address Error. IOATDMA driver has no suspend/resume support. So what happens to the driver when a suspend and then resume happens? Is it possible that the DMA mapped completion address no longer exists after resume? What platform did this happen on? Dave, Platform is HP Z820 with Two 8-core Intel(R) Xeon(R) CPU E5-2660 0 @ 2.20GHz and HT enabled. Output of lspci and lsmod available at these links. http://people.redhat.com/tcamuso/xhci-logs/lspci.txt http://people.redhat.com/tcamuso/xhci-logs/lsmod.txt -- 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: 3.9-rc7 suspend failure
On 04/26/2013 01:46 PM, Dave Jiang wrote: On 04/26/2013 10:39 AM, Tony Camuso wrote: Platform is HP Z820 with Two 8-core Intel(R) Xeon(R) CPU E5-2660 0 @ 2.20GHz and HT enabled. Output of lspci and lsmod available at these links. http://people.redhat.com/tcamuso/xhci-logs/lspci.txt http://people.redhat.com/tcamuso/xhci-logs/lsmod.txt Is this S3 state? No, running at 1.2GHz # cat /sys/devices/system/cpu/cpu*/cpufreq/cpuinfo_cur_freq 120 120 120 120 120 120 120 120 120 120 120 120 120 120 120 120 120 120 120 120 120 120 120 120 120 120 120 120 120 120 120 120 -- 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: 3.9-rc7 suspend failure
On 04/26/2013 01:53 PM, Dave Jiang wrote: No I mean are you transitioning to S3 (standby, sleep, or suspend to RAM) state via suspend and then resumed? Sorry, I misunderstood the question. The Call Trace occurs near the end of resuming from S3 after executing the following command. # rtcwake --mode=mem --second=60 -- 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: 3.9-rc7 suspend failure
On 04/26/2013 02:12 PM, Dave Jiang wrote: On 04/26/2013 11:09 AM, Dave Jiang wrote: A different question. Is the system utilizing IOATDMA at all at the point of suspend/resume? Any RAID5/6 volumes? Or NET_DMA is on? Dave, I just booted with ioatdma blacklisted in /etc/modprobe.d/blacklist.conf With ioatdma no longer loaded, I no longer see the problem with resuming from suspend. Does ioatdma have suspend/resume support? -- 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: [RFT] xhci - correct comp_mode_recovery_timer on return from hibernate
On 04/23/2013 05:14 PM, Sarah Sharp wrote: On Tue, Apr 23, 2013 at 04:18:00PM -0400, Tony Camuso wrote: It hangs in both suspend and resume with vanilla 3.9-rc7 and rc8. Neither the new patch nor the old patch helps. It might be unrelated to USB then. Let's see what the bisect shows. It does appear to be unrelated to usb. Now we need to determine why this should cause problems with suspend/resume. - # git bisect bad 4dec23d7718e6f1f5e1773698d112025169e7d49 is the first bad commit commit 4dec23d7718e6f1f5e1773698d112025169e7d49 Author: Dave Jiang dave.ji...@intel.com Date: Thu Feb 7 14:38:32 2013 -0700 ioatdma: fix race between updating ioat-head and IOAT_COMPLETION_PENDING There is a race that can hit during __cleanup() when the ioat-head pointer is incremented during descriptor submission. The __cleanup() can clear the PENDING flag when it does not see any active descriptors. This causes new submitted descriptors to be ignored because the COMPLETION_PENDING flag is cleared. This was introduced when code was adapted from ioatdma v1 to ioatdma v2. For v2 and v3, IOAT_COMPLETION_PENDING flag will be abandoned and a new flag IOAT_CHAN_ACTIVE will be utilized. This flag will also be protected under the prep_lock when being modified in order to avoid the race. Signed-off-by: Dave Jiang dave.ji...@intel.com Reviewed-by: Dan Williams d...@fb.com Signed-off-by: Vinod Koul vinod.k...@intel.com :04 04 6a99537c215da68179b7431d165c4aa88c2f569d b339a473063ad04cd985399e43e2e11c9a2a7026 M drivers - Here is a Call Trace when resuming from suspend with the above patch. - [ 96.324588] ioatdma :40:04.0: ioat2_timer_event: Channel halted (1000) [ 96.324659] [ cut here ] [ 96.324661] kernel BUG at drivers/dma/ioat/dma_v2.c:317! [ 96.324665] invalid opcode: [#1] SMP [ 96.324709] Modules linked in: ipt_MASQUERADE nf_conntrack_netbios_ns nf_conntrack_broadcast ip6table_mangle ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 iptable_nat nf_nat_ipv4 nf_nat iptable_mangle nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack ebtable_filter ebtables ip6table_filter ip6_tables kvm_intel snd_hda_codec_hdmi coretemp snd_hda_codec_realtek kvm snd_hda_intel snd_hda_codec snd_hwdep snd_seq snd_seq_device snd_pcm snd_page_alloc snd_timer snd hp_wmi crc32c_intel sb_edac ioatdma sparse_keymap iTCO_wdt soundcore edac_core ghash_clmulni_intel rfkill e1000e iTCO_vendor_support mei microcode i2c_i801 lpc_ich pcspkr serio_raw dca mfd_core nouveau isci video mxm_wmi i2c_algo_bit drm_kms_helper ttm libsas drm mpt2sas firewire_ohci firewire_core ata_generic raid_class scsi_transport_sas i2c_core pata_acpi crc_itu_t wmi [ 96.324718] CPU 0 [ 96.324718] Pid: 0, comm: swapper/0 Not tainted 3.8.0-rc2+ #10 Hewlett-Packard HP Z820 Workstation/158B [ 96.324731] RIP: 0010:[a0275466] [a0275466] ioat2_timer_event+0x1b6/0x2c0 [ioatdma] [ 96.324733] RSP: 0018:8801a9c03df0 EFLAGS: 00010206 [ 96.324734] RAX: 0023 RBX: 8801a6880228 RCX: 00ae [ 96.324735] RDX: RSI: 0086 RDI: 0246 [ 96.324737] RBP: 8801a9c03e20 R08: 81e4b4e0 R09: 81e7faf0 [ 96.324738] R10: ba64 R11: 0004 R12: 1000 [ 96.324740] R13: 0100 R14: a02752b0 R15: 8801a6880228 [ 96.324742] FS: () GS:8801a9c0() knlGS: [ 96.324744] CS: 0010 DS: ES: CR0: 80050033 [ 96.324745] CR2: 7f8ce419 CR3: 01c0c000 CR4: 000407f0 [ 96.324747] DR0: DR1: DR2: [ 96.324748] DR3: DR6: 0ff0 DR7: 0400 [ 96.324751] Process swapper/0 (pid: 0, threadinfo 81c0, task 81c14440) [ 96.324751] Stack: [ 96.324758] 00019e050740 00019e050740 81e8c500 8801a6880290 [ 96.324763] 0100 a02752b0 8801a9c03e60 8106cb9a [ 96.324768] 8103d3d4 81e8c500 8801a6880290 a02752b0 [ 96.324769] Call Trace: [ 96.324774] IRQ [ 96.324780] [a02752b0] ? reshape_ring+0x330/0x330 [ioatdma] [ 96.324788] [8106cb9a] call_timer_fn+0x3a/0x120 [ 96.324794] [8103d3d4] ? native_apic_msr_eoi_write+0x14/0x20 [ 96.324801] [a02752b0] ? reshape_ring+0x330/0x330 [ioatdma] [ 96.324807] [81501ab0] ? cpufreq_p4_target+0x120/0x120 [ 96.324811] [8106e92e] run_timer_softirq+0x1fe/0x2b0 [ 96.324815] [81501ab0] ? cpufreq_p4_target+0x120/0x120
Re: [RFT] xhci - correct comp_mode_recovery_timer on return from hibernate
On 04/18/2013 05:12 PM, Sarah Sharp wrote: On Thu, Apr 18, 2013 at 02:45:40PM -0400, Tony Camuso wrote: Sarah, This patch works with RHEL6.4+ kernel, but hangs with 3.9-rc7. = snip = Is it hanging on suspend or on resume? 3.9.0-rc8 hangs in both suspend and resume on a hp z620 running F18, with or without the old and new patch. Meanwhile, 3.8.0 has the problem for which we originally devised this patch... ... BUT your revised patch fixes the problem in 3.8.0. If you like, I can bisect between 3.8.0 and 3.9.0-rc7 (where I first detected the new problem). Does S3 work on 3.9-rc7 with your original patch (without the new comp_timer_running variable)? Does S3 work on the effected system with vanilla 3.9-rc7? It hangs in both suspend and resume with vanilla 3.9-rc7 and rc8. Neither the new patch nor the old patch helps. The full log with CONFIG_USB_DEBUG and CONFIG_USB_XHCI_HCD_DEBUGGING would be helpful. The logs are large, so I've posted them here. http://people.redhat.com/tcamuso/xhci-logs/ Logs were created with 3.9.0-rc8 commit 824282ca7d250bd7c301f221c3cd902ce906d731 Merge: f83b293 3b5e50e Author: Linus Torvalds torva...@linux-foundation.org Date: Mon Apr 22 15:00:59 2013 -0700 -- 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: [RFT] xhci - correct comp_mode_recovery_timer on return from hibernate
On 04/18/2013 05:12 PM, Sarah Sharp wrote: On Thu, Apr 18, 2013 at 02:45:40PM -0400, Tony Camuso wrote: Sarah, This patch works with RHEL6.4+ kernel, but hangs with 3.9-rc7. I can attach the S3_Suspend_message.log, if you like, but here are the last few lines. Is it hanging on suspend or on resume? Does S3 work on 3.9-rc7 with your original patch (without the new comp_timer_running variable)? Does S3 work on the effected system with vanilla 3.9-rc7? Sarah, Just to let you know I haven't forgotten you ... I have been sidetracked by hardware or firmware problems. I need to try a different system. -- 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: [RFT] xhci - correct comp_mode_recovery_timer on return from hibernate
On 04/17/2013 08:27 PM, Sarah Sharp wrote: [Sarah reworked this patch to cover the case where the xHCI restore register operation fails, and (temp STS_SRE) is true (and we re-init the host, including re-init for the compliance mode), but hibernate is false. The original patch would have caused list corruption in this case.] Sarah, This patch works with RHEL6.4+ kernel, but hangs with 3.9-rc7. I can attach the S3_Suspend_message.log, if you like, but here are the last few lines. # tail /var/tmp/S3_Suspend_message.log Bridge firewalling registered device virbr0-nic entered promiscuous mode device virbr0-nic left promiscuous mode virbr0: port 1(virbr0-nic) entered disabled state Ebtables v2.0 registered SELinux: initialized (dev mqueue, type mqueue), uses transition SIDs SELinux: initialized (dev proc, type proc), uses genfs_contexts SELinux: initialized (dev mqueue, type mqueue), uses transition SIDs SELinux: initialized (dev proc, type proc), uses genfs_contexts usb 2-1.1: USB disconnect, device number 3 -- 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 v2] xhci - clarify compliance mode debug messages
There are no functional changes in this patch. However, because the
compliance mode timer can be deleted in more than one function, it
seemed expedient to include the function name in the debug strings.
Also limited the use of capitals to the first word in the compliance
mode debug messages, except after a function name where all words
start with lower case, in keeping with the style prevalent elsewhere
in xhci.c.
Signed-off-by: Tony Camuso tcam...@redhat.com
---
drivers/usb/host/xhci.c | 14 +-
1 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 53b8f89..5bb1a6a 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -417,9 +417,9 @@ static void compliance_mode_recovery(unsigned long arg)
* Compliance Mode Detected. Letting USB Core
* handle the Warm Reset
*/
- xhci_dbg(xhci, Compliance Mode Detected-Port %d!\n,
+ xhci_dbg(xhci, Compliance mode detected-port %d\n,
i + 1);
- xhci_dbg(xhci, Attempting Recovery routine!\n);
+ xhci_dbg(xhci, Attempting compliance mode recovery\n);
hcd = xhci-shared_hcd;
if (hcd-state == HC_STATE_SUSPENDED)
@@ -457,7 +457,7 @@ static void compliance_mode_recovery_timer_init(struct
xhci_hcd *xhci)
set_timer_slack(xhci-comp_mode_recovery_timer,
msecs_to_jiffies(COMP_MODE_RCVRY_MSECS));
add_timer(xhci-comp_mode_recovery_timer);
- xhci_dbg(xhci, Compliance Mode Recovery Timer Initialized.\n);
+ xhci_dbg(xhci, Compliance mode recovery timer initialized\n);
}
/*
@@ -733,8 +733,11 @@ void xhci_stop(struct usb_hcd *hcd)
/* Deleting Compliance Mode Recovery Timer */
if ((xhci-quirks XHCI_COMP_MODE_QUIRK)
- (!(xhci_all_ports_seen_u0(xhci
+ (!(xhci_all_ports_seen_u0(xhci {
del_timer_sync(xhci-comp_mode_recovery_timer);
+ xhci_dbg(xhci, %s: compliance mode recovery timer deleted\n,
+ __func__);
+ }
if (xhci-quirks XHCI_AMD_PLL_FIX)
usb_amd_dev_put();
@@ -930,7 +933,8 @@ int xhci_suspend(struct xhci_hcd *xhci)
if ((xhci-quirks XHCI_COMP_MODE_QUIRK)
(!(xhci_all_ports_seen_u0(xhci {
del_timer_sync(xhci-comp_mode_recovery_timer);
- xhci_dbg(xhci, Compliance Mode Recovery Timer Deleted!\n);
+ xhci_dbg(xhci, %s: compliance mode recovery timer deleted\n,
+ __func__);
}
/* step 5: remove core well power */
--
1.7.1
--
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] xhci - clarify compliance mode debug messages
On 04/03/2013 01:23 PM, Sarah Sharp wrote:
Hmm, actually, this patch doesn't apply any more. Sorry for not getting
to it sooner. Can you resend this against Greg's latest usb-next
branch?
Sarah Sharp
Yes, will do.
On Wed, Apr 03, 2013 at 10:21:56AM -0700, Sarah Sharp wrote:
Hi Tony,
Just a note that I'm queuing this patch to Greg for 3.10. I'm working
on a bug fix patch for 3.9 that includes turning off runtime PM, and
should fix the issue with the compliance timer causing list corruption
on resume.
Sarah Sharp
On Tue, Feb 19, 2013 at 08:59:35AM -0500, Tony Camuso wrote:
There are no functional changes in this patch. However, because the
compliance mode timer can be deleted in more than one function, it
seemed expedient to include the function name in the debug strings.
Also limited the use of capitals to the first word in the compliance
mode debug messages, except after a function name where all words
start with lower case, in keeping with the style prevalent elsewhere
in xhci.c.
Signed-off-by: Tony Camuso tcam...@redhat.com
---
drivers/usb/host/xhci.c | 12 +++-
1 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 267a7b1..757b645 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -416,9 +416,9 @@ static void compliance_mode_recovery(unsigned long arg)
* Compliance Mode Detected. Letting USB Core
* handle the Warm Reset
*/
- xhci_dbg(xhci, Compliance Mode Detected-Port %d!\n,
+ xhci_dbg(xhci, Compliance mode detected-port %d\n,
i + 1);
- xhci_dbg(xhci, Attempting Recovery routine!\n);
+ xhci_dbg(xhci, Attempting compliance mode recovery\n);
hcd = xhci-shared_hcd;
if (hcd-state == HC_STATE_SUSPENDED)
@@ -456,7 +456,7 @@ static void compliance_mode_recovery_timer_init(struct
xhci_hcd *xhci)
set_timer_slack(xhci-comp_mode_recovery_timer,
msecs_to_jiffies(COMP_MODE_RCVRY_MSECS));
add_timer(xhci-comp_mode_recovery_timer);
- xhci_dbg(xhci, Compliance Mode Recovery Timer Initialized.\n);
+ xhci_dbg(xhci, Compliance mode recovery timer initialized\n);
}
/*
@@ -929,7 +929,8 @@ int xhci_suspend(struct xhci_hcd *xhci)
if ((xhci-quirks XHCI_COMP_MODE_QUIRK)
!(xhci_all_ports_seen_u0(xhci))) {
del_timer_sync(xhci-comp_mode_recovery_timer);
- xhci_dbg(xhci, Compliance Mode Recovery Timer Deleted!\n);
+ xhci_dbg(xhci, %s: compliance mode recovery timer deleted\n,
+ __func__);
}
/* step 5: remove core well power */
@@ -992,7 +993,8 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
if ((xhci-quirks XHCI_COMP_MODE_QUIRK)
!(xhci_all_ports_seen_u0(xhci))) {
del_timer_sync(xhci-comp_mode_recovery_timer);
- xhci_dbg(xhci, Compliance Mode Recovery Timer
deleted!\n);
+ xhci_dbg(xhci, %s: compliance mode recovery timer
deleted\n,
+ __func__);
}
/* Let the USB core know _both_ roothubs lost power. */
--
1.7.1
--
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
--
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 v4] xhci - correct comp_mode_recovery_timer on return from hibernate
On 03/14/2013 05:42 PM, Alexis R. Cortes wrote: Hi Sarah, On 3/11/2013 5:20 PM, Sarah Sharp wrote: On Mon, Mar 11, 2013 at 05:33:26PM +, Cortes, Alexis wrote: Hi Sarah, Sorry for my delayed response, I was investigating this. By 'Inactive' state you mean the Compliance mode? since SS.Inactive and Compliance are not the same. Yes, I mean Compliance mode. When in D3hot or D3cold, the host must be able to transmit a PME whenever a device is plugged into the DS port. If a SS device is plugged into DS port and fails to make it to U0, the Port will land in Compliance or SS.Disabled. If Compliance, then there will be no PME notification. If it lands in SS.Disabled, the USB2 will be enabled and then a PME notification will be sent for USB2 connection. I just realized this. Then we definitely need to poll during runtime suspend, or disable runtime PM for the PCI device all together. Does this mean wake from S3 (system suspend) on device connect will be broken as well, if the port fails to make it to U0 and goes into Compliance mode? I believe so, since the timing issues caused by the hardware could make the port enter to Compliance, thus it will never reach U0. However I have never tested this scenario. Best Regards, Alexis Cortes. Alexis, Does this mean that systems having this chip should not use hibernate/suspend? -- 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 v4] xhci - correct comp_mode_recovery_timer on return from hibernate
On 03/22/2013 02:33 PM, Cortes, Alexis wrote: Hi Tony, Well, considering the circumstances, the only issue I see here is that the system won't be able to wake on a device connect if the port to which the device was connected enters in compliance mode (I might add that compliance mode is not a 100% of the times failure), however I haven't tested this scenario before. However I think that this problem doesn't worth to disable suspend/hibernate, but this is my opinion. I wonder if there's a way to re-work this scenario. Thanks, Alexis. Well, given that information, do you believe that the patch submitted in http://www.mail-archive.com/linux-usb@vger.kernel.org/msg15942.html is a good way to address the problem described in ... http://www.mail-archive.com/linux-usb@vger.kernel.org/msg14202.html ? Or perhaps the patch submitted in ... http://www.mail-archive.com/linux-usb@vger.kernel.org/msg14965.html Or maybe there is a better solution? -- 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 v4] xhci - correct comp_mode_recovery_timer on return from hibernate
On 02/27/2013 07:20 PM, Sarah Sharp wrote:
Basically, I'd like Tony to make his first patch work, rather than
pursuing moving the timer manipulation to xhci_bus_suspend/resume.
Not to add confusion, but here is a less intrusive patch that simply
checks to see if the Compliance Mode Recovery Timer already exists
before attempting to initialize it.
---
drivers/usb/host/xhci.c |5 +
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index f1f01a8..e51db78 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -445,6 +445,11 @@ static void compliance_mode_recovery(unsigned long arg)
*/
static void compliance_mode_recovery_timer_init(struct xhci_hcd *xhci)
{
+ if (timer_pending(xhci-comp_mode_recovery_timer)) {
+ xhci_dbg(xhci, Compliance Mode Recovery Timer already active.\n);
+ return;
+ }
+
xhci-port_status_u0 = 0;
init_timer(xhci-comp_mode_recovery_timer);
--
1.7.1
Re: [PATCH v4] xhci - correct comp_mode_recovery_timer on return from hibernate
On 02/26/2013 11:11 AM, Alan Stern wrote:
On Tue, 26 Feb 2013, Tony Camuso wrote:
Then should I continue to pursue relocating calls to init and delete
the compliance mode recovery timer to xhci_bus_suspend/resume?
I don't know. If you think the scheme will work then pursue it,
otherwise don't.
Alan,
I was able to make the scheme work by adding the following
immediately upon entering compliance_recovery_node_timer_init()
if (timer_pending(xhci-comp_mode_recovery_timer)) {
xhci_dbg(xhci, Compliance Mode Recovery Timer already
active.\n);
return;
}
However ...
I don't the bus level is the correct place to deal with this timer.
xhci_bus_suspend and xhci_bus_resume are called many times during boot,
and not always serially. Consider the following snippets taken from
boot with CONFIG_USB_DEBUG=y and CONFIG_USB_XHCI_HCD_DEBUGGING=y
and printk's at the entry of xhci_bus_suspend and xhci_bus_resume.
xhci_hcd :08:00.0: Finished xhci_init
xhci_hcd :08:00.0: Compliance Mode Recovery Timer Initialized.
:
hub 3-0:1.0: hub_suspend
usb usb3: bus auto-suspend, wakeup 1
*** xhci_bus_suspend: pci::08:00.0 usb-bus:4 slot_id:0 port:0
xhci_hcd :08:00.0: set port power, actual port 2 status = 0x2a0
xhci_hcd :08:00.0: Compliance Mode Recovery Timer Deleted!
*** xhci_bus_suspend: completed normaly
xhci_hcd :08:00.0: xhci_hub_status_data: stopping port polling.
:
hub 4-0:1.0: hub_suspend
usb usb4: bus auto-suspend, wakeup 1
*** xhci_bus_suspend: pci::08:00.0 usb-bus:4 slot_id:0 port:0
xhci_hcd :08:00.0: Compliance Mode Recovery Timer Deleted!
*** xhci_bus_suspend: completed normaly
xhci_hcd :08:00.0: xhci_hub_status_data: stopping port polling.
:
usb usb4: usb auto-resume
*** xhci_bus_resume: pci::08:00.0 usb-bus:4 slot_id:0 port:0
xhci_hcd :08:00.0: Compliance Mode Recovery Timer Initialized.
*** xhci_bus_resume: completed normaly
hub 4-0:1.0: hub_resume
:
usb usb4: usb auto-resume
*** xhci_bus_resume: pci::08:00.0 usb-bus:4 slot_id:0 port:0
xhci_hcd :08:00.0: Compliance Mode Recovery Timer Initialized.
*** xhci_bus_resume: completed normaly
hub 4-0:1.0: hub_resume
:
hub 4-0:1.0: state 7 ports 4 chg evt
usb usb3: usb auto-resume
*** xhci_bus_resume: pci::08:00.0 usb-bus:4 slot_id:0 port:0
*** xhci: Compliance Mode Recovery Timer already active.
*** xhci_bus_resume: completed normaly
hub 3-0:1.0: hub_resume
:
etc, etc,
As far as I know, xhci_bus_resume is never called without
xhci_bus_suspend being called first. Hence the timer should not be on
the list when xhci_bus_resume runs.
See above.
Presently, the timer is added in xchi_init() and xhci_resume(). Our problem
emerges when xhci_resume tries to add the timer after it has been restored
by resuming from hibernate.
In the new scheme, the timer should not be added by xhci_resume.
It wasn't, yet I still encountered the problem.
As an aside, we may be able to leave the code as it is now and just add the
code snippet at the top of this post.
That solution introduces the least churn and solves the problem.
--
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 v4] xhci - correct comp_mode_recovery_timer on return from hibernate
On 02/27/2013 07:20 PM, Sarah Sharp wrote: I would like be conservative here, and keep as much of the current code we have (with the compliance timer being manipulated in xhci_suspend and xhci_resume). That was the code that was submitted by TI, and we should stick as close to it as possible, without further information from Alex. Basically, I'd like Tony to make his first patch work, rather than pursuing moving the timer manipulation to xhci_bus_suspend/resume. Sarah, Here's the link to v4 of the patch in this thread ... http://www.mail-archive.com/linux-usb@vger.kernel.org/msg15160.html That version of the patch has been tested for suspend and hibernate and fixes the original problem. -- 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 v4] xhci - correct comp_mode_recovery_timer on return from hibernate
On 02/25/2013 05:15 PM, Alan Stern wrote: Probably because the buses are registered at boot but there aren't any devices plugged in. (Or maybe there are devices, but the system is too busy doing other things during boot to detect them for a while.) Since the buses are idle, they get suspended. Then should I continue to pursue relocating calls to init and delete the compliance mode recovery timer to xhci_bus_suspend/resume? On Mon, 25 Feb 2013, Tony Camuso wrote: Furthermore, xhci_bus_suspend is called before xhci_bus_resume, so an attempt is made to delete the compliance mode recovery timer, which does not yet exist. This produces a list_add corruption call trace a number of times during boot. That doesn't make sense. You are correct. It's when xhci_bus_resume is called that we get the list_add corruption. WARNING: at lib/list_debug.c:33 __list_add+0xbe/0xd0() Hardware name: HP Z820 Workstation list_add corruption. prev-next should be next (8801a9199028), but was (null). (prev=88032ef22c40). : : Call Trace: [8105676f] warn_slowpath_common+0x7f/0xc0 [81056866] warn_slowpath_fmt+0x46/0x50 [81080f07] ? down_trylock+0x37/0x50 [8127512e] __list_add+0xbe/0xd0 [810663ab] __internal_add_timer+0x9b/0x110 [81066441] internal_add_timer+0x21/0x50 [81067d4a] mod_timer+0x10a/0x210 [81067e68] add_timer+0x18/0x30 [a044b547] compliance_mode_recovery_timer_init+0x77/0xb0 [xhci_hcd] [813f6fff] hcd_bus_resume+0x9f/0x220 [81406d65] generic_resume+0x25/0x30 [813fd764] usb_resume_both+0x134/0x150 : : etc The timer must be added somewhere other than in xhci_bus_resume; otherwise it would never get activated in the current xhci-hcd. Wherever that other place is, it should still be in operation. Presently, the timer is added in xchi_init() and xhci_resume(). Our problem emerges when xhci_resume tries to add the timer after it has been restored by resuming from hibernate. -- 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 v4] xhci - correct comp_mode_recovery_timer on return from hibernate
On 02/21/2013 05:04 PM, Alan Stern wrote: Would it be simpler to delete the timer in xhci_bus_suspend() and restart it in xhci_bus_resume()? Then you wouldn't need to care about the difference between suspend and hibernation. Alan, I tried implementing this. There were some static functions in xhci.c that had to be exposed so they could be called by xhci_bus_suspend and xhci_bus_resume. Besides that, xchi_bus_suspend/resume are called a number of times during boot. I don't know why this is, but you may be able to shed some light on that. Furthermore, xhci_bus_suspend is called before xhci_bus_resume, so an attempt is made to delete the compliance mode recovery timer, which does not yet exist. This produces a list_add corruption call trace a number of times during boot. Ultimately, the system panics. Or do you need the timer to remain running even while the root hub and USB bus are suspended? Did not get far enough to determine this. -- 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] xhci - correct comp_mode_recovery_timer on return from hibernate
On 02/21/2013 10:46 AM, Alan Stern wrote: On Wed, 20 Feb 2013, Sarah Sharp wrote: Of course, in your case this doesn't matter. In the memory image, the timer is active. Hence it is still active when the system resumes, even though xhci_suspend _was_ called. Ah, I see now. So basically any memory changes in xhci_suspend will be wiped on resume, but any hardware state should be saved. I was initially concerned about the large number of memory writes in xhci_suspend (e.g. zeroing the command ring), but on resume from hibernate, xhci_resume basically ends up wiping the entire slate and re-initializing the host. It would be nice if xhci_suspend was passed a boolean to indicate it was being called as part of the hibernate process. That way we could skip a lot of unnecessary state saving. That's a good idea. Do you want to implement it? All it requires is to add an extra argument to the hc_driver.pci_suspend method, modify the PCI-based HCDs appropriately (there aren't many), and change the suspend-related pathways in core/hcd-pci.c. Alan Stern I have a new version of the patch incorporating Alan's explanation, but I'll wait to see how Sarah responds to Alan's suggestion. While Alan's is a more turbulent solution, it is a more correct one. -- 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 v4] xhci - correct comp_mode_recovery_timer on return from hibernate
Commit 71c731a2 (usb: host: xhci: Fix Compliance Mode on SN65LVPE502CP
Hardware) was a workaround for systems using the SN65LVPE502CP,
controller, but it introduced a bug in resume from hibernate.
The fix created a timer, comp_mode_recovery_timer, which is deleted from
a timer list when xhci_suspend() is called. However, the hibernate image,
including the timer list containing the comp_mode_recovery_timer, had
already been saved before the timer was deleted.
Upon resume from hibernate, the list containing the comp_mode_recovery_timer
is restored from the image saved to disk, and xhci_resume(), assuming that
the timer had been deleted by xhci_suspend(), makes a call to
compliance_mode_recoery_timer_init(), which creates a new instance of the
comp_mode_recovery_timer and attempts to place it into the same list in which
it is already active, thus corrupting the list during the list_add() call.
At this point, a call trace is emitted indicating the list corruption.
Soon afterward, the system locks up, the watchdog times out, and the
ensuing NMI crashes the system.
The problem did not occur when resuming from suspend. In suspend, the
image in RAM remains exactly as it was when xhci_suspend() deleted the
comp_mode_recovery_timer, so there is no problem when xhci_resume()
creates a new instance of this timer and places it in the still empty
list.
This patch avoids the problem by deleting the timer in xhci_resume()
when resuming from hibernate. Now xhci_resume() can safely make the
call to create a new instance of this timer, whether returning from
suspend or hibernate.
Thanks to Alan Stern for his help with understanding the problem.
Signed-off-by: Tony Camuso tcam...@redhat.com
Acked-by: Don Zickus dzic...@redhat.com
---
drivers/usb/host/xhci.c |9 -
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index f1f01a8..fe4f7ab 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -988,6 +988,13 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
/* If restore operation fails, re-initialize the HC during resume */
if ((temp STS_SRE) || hibernated) {
+
+ if ((xhci-quirks XHCI_COMP_MODE_QUIRK)
+ !(xhci_all_ports_seen_u0(xhci))) {
+ del_timer_sync(xhci-comp_mode_recovery_timer);
+ xhci_dbg(xhci, Compliance Mode Recovery Timer
deleted!\n);
+ }
+
/* 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);
@@ -1071,7 +1078,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
* to suffer the Compliance Mode issue again. It doesn't matter if
* ports have entered previously to U0 before system's suspension.
*/
- if (xhci-quirks XHCI_COMP_MODE_QUIRK)
+ if ((xhci-quirks XHCI_COMP_MODE_QUIRK) !hibernated)
compliance_mode_recovery_timer_init(xhci);
/* Re-enable port polling. */
--
1.7.1
--
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 v4] xhci - correct comp_mode_recovery_timer on return from hibernate
Sarah,
Here it is. Just finished testing it.
The differences between this v4 patch and the v1 patch is an explanation
that incorporates the knowledge shared by Alan Stern concerning what is
actually happening.
I have omitted the cosmetic changes suggested by Sergei. There may be
one cosmetic change I may want to make to clarify where the timer is
being deleted, suspend or resume, but I will submit that patch in the
thread I started for that purpose.
Regards,
Tony
On 02/21/2013 04:11 PM, Tony Camuso wrote:
Commit 71c731a2 (usb: host: xhci: Fix Compliance Mode on SN65LVPE502CP
Hardware) was a workaround for systems using the SN65LVPE502CP,
controller, but it introduced a bug in resume from hibernate.
The fix created a timer, comp_mode_recovery_timer, which is deleted from
a timer list when xhci_suspend() is called. However, the hibernate image,
including the timer list containing the comp_mode_recovery_timer, had
already been saved before the timer was deleted.
Upon resume from hibernate, the list containing the comp_mode_recovery_timer
is restored from the image saved to disk, and xhci_resume(), assuming that
the timer had been deleted by xhci_suspend(), makes a call to
compliance_mode_recoery_timer_init(), which creates a new instance of the
comp_mode_recovery_timer and attempts to place it into the same list in which
it is already active, thus corrupting the list during the list_add() call.
At this point, a call trace is emitted indicating the list corruption.
Soon afterward, the system locks up, the watchdog times out, and the
ensuing NMI crashes the system.
The problem did not occur when resuming from suspend. In suspend, the
image in RAM remains exactly as it was when xhci_suspend() deleted the
comp_mode_recovery_timer, so there is no problem when xhci_resume()
creates a new instance of this timer and places it in the still empty
list.
This patch avoids the problem by deleting the timer in xhci_resume()
when resuming from hibernate. Now xhci_resume() can safely make the
call to create a new instance of this timer, whether returning from
suspend or hibernate.
Thanks to Alan Stern for his help with understanding the problem.
Signed-off-by: Tony Camuso tcam...@redhat.com
Acked-by: Don Zickus dzic...@redhat.com
---
drivers/usb/host/xhci.c |9 -
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index f1f01a8..fe4f7ab 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -988,6 +988,13 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
/* If restore operation fails, re-initialize the HC during resume */
if ((temp STS_SRE) || hibernated) {
+
+ if ((xhci-quirks XHCI_COMP_MODE_QUIRK)
+ !(xhci_all_ports_seen_u0(xhci))) {
+ del_timer_sync(xhci-comp_mode_recovery_timer);
+ xhci_dbg(xhci, Compliance Mode Recovery Timer
deleted!\n);
+ }
+
/* 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);
@@ -1071,7 +1078,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
* to suffer the Compliance Mode issue again. It doesn't matter if
* ports have entered previously to U0 before system's suspension.
*/
- if (xhci-quirks XHCI_COMP_MODE_QUIRK)
+ if ((xhci-quirks XHCI_COMP_MODE_QUIRK) !hibernated)
compliance_mode_recovery_timer_init(xhci);
/* Re-enable port polling. */
--
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] xhci - clarify compliance mode debug messages
On 02/19/2013 04:48 PM, Sarah Sharp wrote: Hi Tony, Greg closed the usb-next tree for 3.9 two weeks ago. The bug fix patches will have to go in after the 3.9 merge window closes (approximately two weeks from now). Understood. Sorry for the lack of response, I was on vacation. I'll review your patch in the next few days. Thanks. The one thing I wanted to check was my understanding of the hibernate flow path. As you mentioned, I thought that xhci_suspend would be called in the hibernate path, but it's not. Perhaps there is a better function that's called in both the hibernate and suspend path where we can stop the compliance mode timer. I'll poke around and see if I can find something like that, unless, as Don indicated, Rafael can jump in and give us a tip. Regards, Tony -- 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] xhci - clarify compliance mode debug messages
On 02/20/2013 10:15 AM, Alan Stern wrote: On Tue, 19 Feb 2013, Sarah Sharp wrote: The one thing I wanted to check was my understanding of the hibernate flow path. As you mentioned, I thought that xhci_suspend would be called in the hibernate path, but it's not. Are you sure about that? AFAICT it should be called -- although it gets called during the poweroff phase of hibernation, not the freeze phase. core/hcd-pci.c: usb_hcd_pci_pm_ops.poweroff = hcd_pci_suspend core/hcd-pci.c: hcd_pci_suspend calls suspend_common core/hcd-pci.c: suspend_common calls hcd-driver-pci_suspend host/xhci-pci.c: xhci_pci_hc_driver.pci_suspend = xhci_pci_suspend host/xhci-pci.c: xhci_pci_suspend calls xhci_suspend We instrumented the code, and when invoking pm-hibernate, we did not flow through xhci_suspend(), where the timer is deleted. This should not be necessary. System was crashing on resume-from-hibernate. Not crashing with the patch. -- 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] xhci - clarify compliance mode debug messages
On 02/20/2013 12:35 PM, Alan Stern wrote: I bet it really was called, but you didn't realize it because of the way you monitored the output from the printks. You looked at the log buffer or the kernel log file after resuming, right? You didn't have a serial console attached to another machine, to capture the output as it occurred. Actually, we did have a serial console connected. This makes a difference, because output that occurs after the memory image is stored on disk will not be present in the image and hence will not be visible after you resume from hibernation. Of course, in your case this doesn't matter. In the memory image, the timer is active. Hence it is still active when the system resumes, even though xhci_suspend _was_ called. Could be because the printk's are not synchronous with execution. Upon resuming from hibernate, we'd see a the call trace, which indicated list_add() corruption where the compliance mode timer was added to the timer list in xhci. Don't know why we didn't crash right there. Maybe something needs to change in list_add(), too. However, we did not see this behavior when returning from suspend. If you want to handle hibernation correctly, you have to understand how it works. It is all described in excruciating detail in Documentation/power/devices.txt. Homework assignment. :) Resuming from hibernation is not like resuming from system suspend, for two important reasons: After hibernation, the system has gone through a complete reboot. The BIOS has probably made changes, and devices generally need to be completely reinitialized. The state a driver sees following hibernation is the state that existed during the freeze phase, which is different from what you get during system suspend (and also different from what you get during later phases of hibernation). Because we didn't see our printks in xhci_suspend(), we assumed, perhaps erroneously, that we never entered and the compliance mode timer was never deleted. We thought that the call trace with list_add() corruption was a smoking gun, and our patch did make the problem go away. We will have another look ... -- 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] xhci - clarify compliance mode debug messages
There are no functional changes in this patch. However, because the
compliance mode timer can be deleted in more than one function, it
seemed expedient to include the function name in the debug strings.
Also limited the use of capitals to the first word in the compliance
mode debug messages, except after a function name where all words
start with lower case, in keeping with the style prevalent elsewhere
in xhci.c.
Signed-off-by: Tony Camuso tcam...@redhat.com
---
drivers/usb/host/xhci.c | 12 +++-
1 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 267a7b1..757b645 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -416,9 +416,9 @@ static void compliance_mode_recovery(unsigned long arg)
* Compliance Mode Detected. Letting USB Core
* handle the Warm Reset
*/
- xhci_dbg(xhci, Compliance Mode Detected-Port %d!\n,
+ xhci_dbg(xhci, Compliance mode detected-port %d\n,
i + 1);
- xhci_dbg(xhci, Attempting Recovery routine!\n);
+ xhci_dbg(xhci, Attempting compliance mode recovery\n);
hcd = xhci-shared_hcd;
if (hcd-state == HC_STATE_SUSPENDED)
@@ -456,7 +456,7 @@ static void compliance_mode_recovery_timer_init(struct
xhci_hcd *xhci)
set_timer_slack(xhci-comp_mode_recovery_timer,
msecs_to_jiffies(COMP_MODE_RCVRY_MSECS));
add_timer(xhci-comp_mode_recovery_timer);
- xhci_dbg(xhci, Compliance Mode Recovery Timer Initialized.\n);
+ xhci_dbg(xhci, Compliance mode recovery timer initialized\n);
}
/*
@@ -929,7 +929,8 @@ int xhci_suspend(struct xhci_hcd *xhci)
if ((xhci-quirks XHCI_COMP_MODE_QUIRK)
!(xhci_all_ports_seen_u0(xhci))) {
del_timer_sync(xhci-comp_mode_recovery_timer);
- xhci_dbg(xhci, Compliance Mode Recovery Timer Deleted!\n);
+ xhci_dbg(xhci, %s: compliance mode recovery timer deleted\n,
+ __func__);
}
/* step 5: remove core well power */
@@ -992,7 +993,8 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
if ((xhci-quirks XHCI_COMP_MODE_QUIRK)
!(xhci_all_ports_seen_u0(xhci))) {
del_timer_sync(xhci-comp_mode_recovery_timer);
- xhci_dbg(xhci, Compliance Mode Recovery Timer
deleted!\n);
+ xhci_dbg(xhci, %s: compliance mode recovery timer
deleted\n,
+ __func__);
}
/* Let the USB core know _both_ roothubs lost power. */
--
1.7.1
--
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] xhci - clarify compliance mode debug messages
Sarah, This patch is pursuant to the patch at ... http://www.mail-archive.com/linux-usb@vger.kernel.org/msg14965.html, Message ID 1361209953-5195-1-git-send-email-tcam...@redhat.com ... and must be applied after that patch is applied. Can these patches be included in the next merge? Thanks, Tony -- 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 v3] xhci - correct comp_mode_recovery_timer on return from hibernate
Commit 71c731a2 (usb: host: xhci: Fix Compliance Mode on SN65LVPE502CP
Hardware) was a workaround for systems using the SN65LVPE502CP controller,
but it introduced a bug where resume from hibernate would add the
comp_mode_recovery_timer to the timer queue while it was already
active when saved to disk on hibernate. This caused list_add corruption
leading to a crash when resuming from hibernate.
The original patch erroneously assumed that the timer would be deleted
by a call to xhci_suspend() or xhci_stop(), but neither of these calls
is made during the hibernate sequence. When returning from hibernate,
the timer was still active, and the attempt to list_add the same timer
corrupted the list.
We can avoid this problem when resuming from hibernate by first deleting
the timer and then initializing it and adding it to the timer list.
Removed extraneous parenthesis from conditional statements of the
original commit.
Signed-off-by: Tony Camuso tcam...@redhat.com
Acked-by: Don Zickus dzic...@redhat.com
---
drivers/usb/host/xhci.c | 13 ++---
1 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index f1f01a8..267a7b1 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -732,7 +732,7 @@ void xhci_stop(struct usb_hcd *hcd)
/* Deleting Compliance Mode Recovery Timer */
if ((xhci-quirks XHCI_COMP_MODE_QUIRK)
- (!(xhci_all_ports_seen_u0(xhci
+ !(xhci_all_ports_seen_u0(xhci)))
del_timer_sync(xhci-comp_mode_recovery_timer);
if (xhci-quirks XHCI_AMD_PLL_FIX)
@@ -927,7 +927,7 @@ int xhci_suspend(struct xhci_hcd *xhci)
* is about to be suspended.
*/
if ((xhci-quirks XHCI_COMP_MODE_QUIRK)
- (!(xhci_all_ports_seen_u0(xhci {
+ !(xhci_all_ports_seen_u0(xhci))) {
del_timer_sync(xhci-comp_mode_recovery_timer);
xhci_dbg(xhci, Compliance Mode Recovery Timer Deleted!\n);
}
@@ -988,6 +988,13 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
/* If restore operation fails, re-initialize the HC during resume */
if ((temp STS_SRE) || hibernated) {
+
+ if ((xhci-quirks XHCI_COMP_MODE_QUIRK)
+ !(xhci_all_ports_seen_u0(xhci))) {
+ del_timer_sync(xhci-comp_mode_recovery_timer);
+ xhci_dbg(xhci, Compliance Mode Recovery Timer
deleted!\n);
+ }
+
/* 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);
@@ -1071,7 +1078,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
* to suffer the Compliance Mode issue again. It doesn't matter if
* ports have entered previously to U0 before system's suspension.
*/
- if (xhci-quirks XHCI_COMP_MODE_QUIRK)
+ if ((xhci-quirks XHCI_COMP_MODE_QUIRK) !hibernated)
compliance_mode_recovery_timer_init(xhci);
/* Re-enable port polling. */
--
1.7.1
--
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 v3] xhci - correct comp_mode_recovery_timer on return from hibernate
The difference between v2 and v3 of this patch was mostly cosmetic.
(!(xhci_all_ports_seen_u0(xhci))
... was changed to...
!(xhci_all_ports_seen_u0(xhci))
And ...
Compliance Mode Recovery Timer Deleted!\n
... was changed to ...
Compliance Mode Recovery Timer deleted!\n
On 02/18/2013 12:52 PM, Tony Camuso wrote:
Commit 71c731a2 (usb: host: xhci: Fix Compliance Mode on SN65LVPE502CP
Hardware) was a workaround for systems using the SN65LVPE502CP controller,
but it introduced a bug where resume from hibernate would add the
comp_mode_recovery_timer to the timer queue while it was already
active when saved to disk on hibernate. This caused list_add corruption
leading to a crash when resuming from hibernate.
The original patch erroneously assumed that the timer would be deleted
by a call to xhci_suspend() or xhci_stop(), but neither of these calls
is made during the hibernate sequence. When returning from hibernate,
the timer was still active, and the attempt to list_add the same timer
corrupted the list.
We can avoid this problem when resuming from hibernate by first deleting
the timer and then initializing it and adding it to the timer list.
Removed extraneous parenthesis from conditional statements of the
original commit.
Signed-off-by: Tony Camuso tcam...@redhat.com
Acked-by: Don Zickus dzic...@redhat.com
---
drivers/usb/host/xhci.c | 13 ++---
1 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index f1f01a8..267a7b1 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -732,7 +732,7 @@ void xhci_stop(struct usb_hcd *hcd)
/* Deleting Compliance Mode Recovery Timer */
if ((xhci-quirks XHCI_COMP_MODE_QUIRK)
- (!(xhci_all_ports_seen_u0(xhci
+ !(xhci_all_ports_seen_u0(xhci)))
del_timer_sync(xhci-comp_mode_recovery_timer);
if (xhci-quirks XHCI_AMD_PLL_FIX)
@@ -927,7 +927,7 @@ int xhci_suspend(struct xhci_hcd *xhci)
* is about to be suspended.
*/
if ((xhci-quirks XHCI_COMP_MODE_QUIRK)
- (!(xhci_all_ports_seen_u0(xhci {
+ !(xhci_all_ports_seen_u0(xhci))) {
del_timer_sync(xhci-comp_mode_recovery_timer);
xhci_dbg(xhci, Compliance Mode Recovery Timer Deleted!\n);
}
@@ -988,6 +988,13 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
/* If restore operation fails, re-initialize the HC during resume */
if ((temp STS_SRE) || hibernated) {
+
+ if ((xhci-quirks XHCI_COMP_MODE_QUIRK)
+ !(xhci_all_ports_seen_u0(xhci))) {
+ del_timer_sync(xhci-comp_mode_recovery_timer);
+ xhci_dbg(xhci, Compliance Mode Recovery Timer
deleted!\n);
+ }
+
/* 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);
@@ -1071,7 +1078,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
* to suffer the Compliance Mode issue again. It doesn't matter if
* ports have entered previously to U0 before system's suspension.
*/
- if (xhci-quirks XHCI_COMP_MODE_QUIRK)
+ if ((xhci-quirks XHCI_COMP_MODE_QUIRK) !hibernated)
compliance_mode_recovery_timer_init(xhci);
/* Re-enable port polling. */
--
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 v2] xhci - correct comp_mode_recovery_timer on return from hibernate
Sergei,
Thank you for your response.
On 02/16/2013 11:32 AM, Sergei Shtylyov wrote:
Commit 71c731a
Please also specify that commit's summary in parens (or however you like).
Yes, I will re-submit with an excerpt of the summary. It's rather long, so I
will just take the first paragraph, rather than all the details.
+
+if ((xhci-quirks XHCI_COMP_MODE_QUIRK)
+(!(xhci_all_ports_seen_u0(xhci {
You don't need () around !x.
This was a cut-and-paste from the same commit I mentioned above
(71c731a2).
However, I have no objection to changing it in both places in the
interests of cleaner style.
+del_timer_sync(xhci-comp_mode_recovery_timer);
+xhci_dbg(xhci, Compliance Mode Recovery Timer Deleted!\n);
Why all words capitalized?
Another cut-and-paste from what was already put there by the above commit.
I can understand why Compliance Mode Recovery Timer is capitalized, since
it could be considered a proper name, but Deleted at least should be
lower-cased.
+}
+
/* 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);
@@ -1071,7 +1078,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
* to suffer the Compliance Mode issue again. It doesn't matter if
* ports have entered previously to U0 before system's suspension.
*/
-if (xhci-quirks XHCI_COMP_MODE_QUIRK)
+if ((xhci-quirks XHCI_COMP_MODE_QUIRK) (!hibernated))
Again, no need for () around !x.
I own this one :)
I will change it.
Regards,
Tony
--
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] xhci - correct comp_mode_recovery_timer on return from hibernate
Commit 71c731a was a workaround for systems using the SN65LVPE502CP,
controller, but it introduced a bug where resume from hibernate would
add the comp_mode_recovery_timer to the timer queue while it was already
active when saved to disk on hibernate. This caused list_add corruption
leading to a crash when resuming from hibernate.
The original patch erroneously assumed that the timer would be deleted
by a call to xhci_suspend() or xhci_stop(), but neither of these calls
is made during the hibernate sequence. When returning from hibernate,
the timer was still active, and the attempt to list_add the same timer
corrupted the list.
We can avoid this problem when resuming from hibernate by first deleting
the timer and then initializing it and adding it to the timer list.
Signed-off-by: Tony Camuso tcam...@redhat.com
Acked-by: Don Zickus dzic...@redhat.com
---
drivers/usb/host/xhci.c |9 -
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index b53d756..1697c14 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -975,6 +975,13 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
/* If restore operation fails, re-initialize the HC during resume */
if ((temp STS_SRE) || hibernated) {
+
+ if ((xhci-quirks XHCI_COMP_MODE_QUIRK)
+ (!(xhci_all_ports_seen_u0(xhci {
+ del_timer_sync(xhci-comp_mode_recovery_timer);
+ xhci_dbg(xhci, Compliance Mode Recovery Timer
Deleted!\n);
+ }
+
/* 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);
@@ -1058,7 +1065,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
* to suffer the Compliance Mode issue again. It doesn't matter if
* ports have entered previously to U0 before system's suspension.
*/
- if (xhci-quirks XHCI_COMP_MODE_QUIRK)
+ if ((xhci-quirks XHCI_COMP_MODE_QUIRK) (!hibernated))
compliance_mode_recovery_timer_init(xhci);
return retval;
--
1.7.1
--
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
