* [PATCH v2] ehci: fix EHCI host controller initialization sequence
@ 2021-01-09 14:47 Eugene Korenevsky
2021-01-09 16:31 ` Alan Stern
0 siblings, 1 reply; 2+ messages in thread
From: Eugene Korenevsky @ 2021-01-09 14:47 UTC (permalink / raw)
To: linux-usb; +Cc: Alan Stern, Greg Kroah-Hartman
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.
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=211095
Signed-off-by: Eugene Korenevsky <ekorenevsky@astralinux.ru>
---
drivers/usb/host/ehci-hcd.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index e358ae17d51e..490473af4906 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -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));
--
2.20.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v2] ehci: fix EHCI host controller initialization sequence
2021-01-09 14:47 [PATCH v2] ehci: fix EHCI host controller initialization sequence Eugene Korenevsky
@ 2021-01-09 16:31 ` Alan Stern
0 siblings, 0 replies; 2+ messages in thread
From: Alan Stern @ 2021-01-09 16:31 UTC (permalink / raw)
To: Eugene Korenevsky; +Cc: linux-usb, Greg Kroah-Hartman
On Sat, Jan 09, 2021 at 05:47:36PM +0300, Eugene Korenevsky wrote:
> 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.
>
> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=211095
> Signed-off-by: Eugene Korenevsky <ekorenevsky@astralinux.ru>
> ---
> drivers/usb/host/ehci-hcd.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> index e358ae17d51e..490473af4906 100644
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -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);
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.
> +
> + 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
> + return rc;
> + }
> +
> ehci->last_periodic_enable = ktime_get_real();
>
> temp = HC_VERSION(ehci, ehci_readl(ehci, &ehci->caps->hc_capbase));
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-01-09 16:32 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-09 14:47 [PATCH v2] ehci: fix EHCI host controller initialization sequence Eugene Korenevsky
2021-01-09 16:31 ` Alan Stern
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.