ehci: fix EHCI host controller initialization sequence
Subsystems (2)
PATCH v2 1/1 ehci: fix EHCI host controller initialization sequence
According to EHCI spec, EHCI HC clears USBSTS.HCHalted whenever USBCMD.RS=1. However, it is a good practice to wait some time after setting USBCMD.RS (approximately 100ms) until USBSTS.HCHalted become zero.
Instead, previous version of ehci_run() wrote 1 to USBCMD.RS, issued read to USBCMD and waited for 5 ms.
That worked incorrectly at least for VirtualBox's EHCI virtual HC and caused accidental hangs.
--- drivers/usb/host/ehci-hcd.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)
drivers/usb/host/ehci-hcd.c +12 -2
@@ -574,6 +574,7 @@ static int ehci_run (struct usb_hcd *hcd)struct ehci_hcd *ehci =hcd_to_ehci (hcd);u32 temp;u32 hcc_params; +int rc; hcd->uses_new_polling =1 ;
@@ -629,9 +630,18 @@ static int ehci_run (struct usb_hcd *hcd)down_write (&ehci_cf_port_reset_rwsem); ehci->rh_state =EHCI_RH_RUNNING ;ehci_writel (ehci,FLAG_CF , &ehci->regs->configured_flag); -ehci_readl (ehci, &ehci->regs->command);/* unblock posted writes */ -msleep (5 ); + +/* Wait until HCD become operational */ + rc =ehci_handshake (ehci, &ehci->regs->status,STS_HALT ,0 ,100 *1000 ); +up_write (&ehci_cf_port_reset_rwsem); + +if (rc) { +ehci_err (ehci,"USB %x.%x, handshake error: %d\n" , + ((ehci->sbrn &0xf0 )>>4 ), (ehci->sbrn &0x0f ), rc); +return rc; + } + ehci->last_periodic_enable =ktime_get_real (); temp =HC_VERSION (ehci,ehci_readl (ehci, &ehci->caps->hc_capbase));
Signature · 1 line
2.20.1
Alan Stern
maintainerQuoted 28 lines · Eugene Korenevsky
According to EHCI spec, EHCI HC clears USBSTS.HCHalted whenever USBCMD.RS=1. However, it is a good practice to wait some time after setting USBCMD.RS (approximately 100ms) until USBSTS.HCHalted become zero.
Instead, previous version of
ehci_run()wrote 1 to USBCMD.RS, issued read to USBCMD and waited for 5 ms.
That worked incorrectly at least for VirtualBox's EHCI virtual HC and caused accidental hangs.Signed-off-by Eugene Korenevsky <[email protected]>--- drivers/usb/host/ehci-hcd.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)drivers/usb/host/ehci-hcd.c +1
-0@@ -574,6 +574,7 @@ static int ehci_run (struct usb_hcd *hcd)struct ehci_hcd *ehci =hcd_to_ehci (hcd);u32 temp;u32 hcc_params; +int rc; hcd->uses_new_polling =1 ;
drivers/usb/host/ehci-hcd.c static int ehci_run (struct usb_hcd *hcd) · line 637
Quoted 10 lines
drivers/usb/host/ehci-hcd.c +4
-2@@ -629,9 +630,18 @@ static int ehci_run (struct usb_hcd *hcd)down_write (&ehci_cf_port_reset_rwsem); ehci->rh_state =EHCI_RH_RUNNING ;ehci_writel (ehci,FLAG_CF , &ehci->regs->configured_flag); -ehci_readl (ehci, &ehci->regs->command);/* unblock posted writes */ -msleep (5 ); + +/* Wait until HCD become operational */ + rc =ehci_handshake (ehci, &ehci->regs->status,STS_HALT ,0 ,100 *1000 ); +up_write (&ehci_cf_port_reset_rwsem);
Are you sure you don't want to wait 5 ms before polling the STS_HALT flag? It would use less CPU time and should suffice in the majority of cases.
drivers/usb/host/ehci-hcd.c static int ehci_run (struct usb_hcd *hcd) · line 641
Quoted 4 lines
drivers/usb/host/ehci-hcd.c +4
-0+ +if (rc) { +ehci_err (ehci,"USB %x.%x, handshake error: %d\n" , + ((ehci->sbrn &0xf0 )>>4 ), (ehci->sbrn &0x0f ), rc);
How about something a little less generic than "handshake error"? Like "controller refused to start"?
Alan Stern
Quoted 9 lines
drivers/usb/host/ehci-hcd.c +3
-0+return rc; + } + ehci->last_periodic_enable =ktime_get_real (); temp =HC_VERSION (ehci,ehci_readl (ehci, &ehci->caps->hc_capbase));Signature · 1 line
2.20.1