Re: [PATCH] xhci: Hard reset controller on shutdown.

[wenxiong]

On 2017-10-21 10:04, Alan Stern wrote:

On Fri, 20 Oct 2017, Brian King wrote:


On 10/19/2017 12:04 PM, Alan Stern wrote:
> On Thu, 19 Oct 2017, Mathias Nyman wrote:
>
>> Current shutdown routine just forces the host controller to stop, it clears 
the
>> run bit and polls the "halted" status for 16ms. Apparently we don't see the 
halted
>> bit within 16ms.
>>
>> Spec say that the correct way to stop is to first command all transfer rings 
to stop,
>> then stop the command ring, and after that stop the host controller.
>>
>> If we just bluntly stop the host (as we do) spec say (xhci 5.4.1.1) it 
should stop
>> anyway within 16ms, but undefined behavior may occur.
>>
>> So the options in xHCI are down to:
>> 1. just clear the run bit and ignore checking any status.
>> - really fast shutdown routine for xhci
>> 2. clear run bit and increase status polling time, see if we get rid of 
error message.
>> - can get rid of error message but no actual change, well, we would know 
if host stopped
>> 3. properly stop all transfer rings and command ring, and then stop host.
>> - cleanest and slowest way, do we care about this? everything should be 
reset after shutdown.
>
> Or you could use a sledgehammer approach, and do a hardware reset of
> the controller chip.
>
> Alan Stern
>

Sort of like this? With this patch I can successfully kexec boot 
kernels and my
USB devices show up every time. Without it, I see xhci init failures 
consistently

on the kexec booted kernels and the xhci load fails.

8<

Fixes kexec boot. Without a hard reset, some USB chips will fail to
initialize in a kexec booted kernel.

Signed-off-by: Brian King 
---

Index: linux.git/drivers/usb/host/xhci.c
===
--- linux.git.orig/drivers/usb/host/xhci.c
+++ linux.git/drivers/usb/host/xhci.c
@@ -729,6 +729,7 @@ static void xhci_shutdown(struct usb_hcd
 	/* Yet another workaround for spurious wakeups at shutdown with HSW 
*/

if (xhci->quirks & XHCI_SPURIOUS_WAKEUP)
pci_set_power_state(to_pci_dev(hcd->self.sysdev), PCI_D3hot);
+   pci_reset_function_locked(to_pci_dev(hcd->self.sysdev));
 }

 #ifdef CONFIG_PM


The decision is up to Mathias.  However, I suspect this code won't work
very well on systems where the xHCI controller is not on a PCI bus (if
any systems like that exist).

Also, there doesn't seem to be much point in setting the power state to
D3hot immediately before resetting the controller.

Alan Stern


Hi Mathias and Alan,

I have tested the patch with difference hardware and difference Linux 
distro(including upstream kernel) this week.


With this patch, USB devices are always showed up when doing kexec and 
fixes several issues opened by customers.


Please let us know if you have any suggestion for the patch.

Thanks for your help!
Wendy

--
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: "USB Host halt failed, -110" error when rebooting system

[wenxiong]

On 2017-10-19 12:04, Alan Stern wrote:

On Thu, 19 Oct 2017, Mathias Nyman wrote:

Current shutdown routine just forces the host controller to stop, it 
clears the
run bit and polls the "halted" status for 16ms. Apparently we don't 
see the halted

bit within 16ms.

Spec say that the correct way to stop is to first command all transfer 
rings to stop,

then stop the command ring, and after that stop the host controller.

If we just bluntly stop the host (as we do) spec say (xhci 5.4.1.1) it 
should stop

anyway within 16ms, but undefined behavior may occur.

So the options in xHCI are down to:
1. just clear the run bit and ignore checking any status.
- really fast shutdown routine for xhci
2. clear run bit and increase status polling time, see if we get rid 
of error message.
- can get rid of error message but no actual change, well, we 
would know if host stopped
3. properly stop all transfer rings and command ring, and then stop 
host.
- cleanest and slowest way, do we care about this? everything 
should be reset after shutdown.


Or you could use a sledgehammer approach, and do a hardware reset of
the controller chip.

Alan Stern


Thanks for all your feedback!!
We will re-submit the patch with hardware reset.

Thanks,
Wendy

--
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: "USB Host halt failed, -110" error when rebooting system

[wenxiong]

From: Wen Xiong 


We saw "Host halt failed, -110" error when rebooting system/
shutdowning system/kexec constantly. 

This patch called usb_disconnect() before calling xhci_halt(). 
usb_disconnect()disconnect the parent and all of its children,
clean up hardware state and make sure that hardware is ready
to be halted down.

Rebooting.
[18648.996035] sd 0:2:1:0: [sdb] Synchronizing SCSI cache
[18678.831197] mpt3sas_cm1: sending message unit reset !!
[18678.832774] mpt3sas_cm1: message unit reset: SUCCESS
[18683.900798] mpt3sas_cm0: sending message unit reset !!
[18683.902370] mpt3sas_cm0: message unit reset: SUCCESS
[18693.921103] xhci_hcd 0005:01:00.0: Host halt failed, -110
[18693.924483] reboot: Restarting system
[18861.282906007,5] OPAL: Reboot request...

Signed-off-by: Wen Xiong 

Thanks,
Wendy

---
 drivers/usb/host/xhci.c |7 +++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index ee198ea..67fdb0f 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -709,10 +709,17 @@ static void xhci_stop(struct usb_hcd *hcd)
 static void xhci_shutdown(struct usb_hcd *hcd)
 {
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+   struct usb_device *rhdev = hcd->self.root_hub;
+
+   dev_info(hcd->self.controller, "remove, state %x\n", hcd->state);
 
if (xhci->quirks & XHCI_SPURIOUS_REBOOT)
usb_disable_xhci_ports(to_pci_dev(hcd->self.sysdev));
 
+   mutex_lock(_bus_idr_lock);
+   usb_disconnect();
+   mutex_unlock(_bus_idr_lock);
+
spin_lock_irq(>lock);
xhci_halt(xhci);
/* Workaround for spurious wakeups at shutdown with HSW */
-- 
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