[PATCH] proper bios handoff in ehci-hcd

[PATCH] proper bios handoff in ehci-hcd

[Gary_Lerhaupt]

Stuart Hayes here at Dell has identified this or/and mix-up in the ehci-hcd driver.  Because of this, ehci-hcd is not properly released by BIOSes supporting full 2.0 and port behavior can then become erratic.

This is broken in latest 2.4 and 2.6.

Gary Lerhaupt
Dell Linux Development
http://linux.dell.com

--- linux/drivers/usb/host/ehci-hcd.c.orig	2004-06-05 03:12:18.000000000 -0500
+++ linux/drivers/usb/host/ehci-hcd.c	2004-06-05 01:18:51.000000000 -0500
@@ -290,7 +290,7 @@ static int bios_handoff (struct ehci_hcd
 		int msec = 500;
 
 		/* request handoff to OS */
-		cap &= 1 << 24;
+		cap |= 1 << 24;
 		pci_write_config_dword (ehci->hcd.pdev, where, cap);
 
 		/* and wait a while for it to happen */

Re: [linux-usb-devel] [PATCH] proper bios handoff in ehci-hcd

[David Brownell]

Gary_Lerhaupt@Dell.com wrote:
> Stuart Hayes here at Dell has identified this or/and mix-up in the
> ehci-hcd driver.  Because of this, ehci-hcd is not properly released by
> BIOSes supporting full 2.0 and port behavior can then become erratic.

Good patch, it should be merged.  That handoff code actually
predates general availability of BIOSes handling _any_ EHCI
controllers, and your patch resolves a problem I'd seen on a
newish board but hadn't yet had time to track down (beyond
knowing that broken BIOS handoff was the issue).

Thanks to you and Stuart!

- Dave

Re: [PATCH] proper bios handoff in ehci-hcd

[Will Beers]

[-- Attachment #1: Type: text/plain, Size: 1563 bytes --]

> Stuart Hayes here at Dell has identified this or/and mix-up in the
> ehci-hcd driver.  Because of this, ehci-hcd is not properly released by
> BIOSes supporting full 2.0 and port behavior can then become erratic.

This change actually breaks USB altogether on an Asus P4P800, as I noticed when I updated to 2.6.8-rc1.

I get the following messages at boot:

-----
Jul 12 23:34:06 willdesktop kernel: ehci_hcd 0000:00:1d.7: BIOS handoff failed (104, 1010001)
Jul 12 23:34:06 willdesktop kernel: ehci_hcd 0000:00:1d.7: can't reset
Jul 12 23:34:06 willdesktop kernel: ehci_hcd 0000:00:1d.7: init 0000:00:1d.7 fail, -95
Jul 12 23:34:06 willdesktop kernel: ehci_hcd: probe of 0000:00:1d.7 failed with error -95
-----

I've seen a few other people with this problem, and reversing the change makes everything work perfectly again, maybe this was the cause of it all?  (sorry if I missed the fix before)

(included patch reverses it)

-Will

-----------------------------------------------------------------------------


diff -Nru a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
--- a/drivers/usb/host/ehci-hcd.c       2004-07-13 01:09:00.000000000 -0400
+++ b/drivers/usb/host/ehci-hcd.c       2004-07-13 01:08:32.000000000 -0400
@@ -293,7 +293,7 @@
                struct pci_dev *pdev = to_pci_dev(ehci->hcd.self.controller);
 
                /* request handoff to OS */
-               cap |= 1 << 24;
+               cap &= 1 << 24;
                pci_write_config_dword(pdev, where, cap);
 
                /* and wait a while for it to happen */

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/x-pkcs7-signature, Size: 3186 bytes --]

Re: [PATCH] proper bios handoff in ehci-hcd

[Will Beers]

[-- Attachment #1: Type: text/plain, Size: 741 bytes --]

As Anders Johansson pointed out to me, the original patch is necessary - 
reversing it makes the loop

do {
                         msleep(10);
                         msec -= 10;
                         pci_read_config_dword 
(to_pci_dev(ehci->hcd.self.controller), where, &cap);
                 } while ((cap & (1 << 16)) && msec);

only execute once and makes the if block inaccessible.  Thanks for pointing 
that out...guess I was overly excited when it worked again and didn't think 
much into it.

I'm confused as to how bypassing all that via my (bad) patch allows it to 
work though.  I'd like to know more about this bit of code, if anyone have 
more general info on it, so I can keep trying to help find the culprit.

-Will




[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/x-pkcs7-signature, Size: 3186 bytes --]

Re: [linux-usb-devel] [PATCH] proper bios handoff in ehci-hcd

[Olaf Hering]

 On Tue, Jun 15, David Brownell wrote:

> Gary_Lerhaupt@Dell.com wrote:
> >Stuart Hayes here at Dell has identified this or/and mix-up in the
> >ehci-hcd driver.  Because of this, ehci-hcd is not properly released by
> >BIOSes supporting full 2.0 and port behavior can then become erratic.
> 
> Good patch, it should be merged.  That handoff code actually
> predates general availability of BIOSes handling _any_ EHCI
> controllers, and your patch resolves a problem I'd seen on a
> newish board but hadn't yet had time to track down (beyond
> knowing that broken BIOS handoff was the issue).

David,

there are 2 reports about breakage by this patch. One is on lkml, and
another one is in my bugzilla inbox. How can we fix that one? I assume
that handoff patch is correct.

  <6>NET: Registered protocol family 17
  <3>ehci_hcd 0000:00:1d.7: BIOS handoff failed (104, 1010001)
  <3>ehci_hcd 0000:00:1d.7: can't reset
  <3>ehci_hcd 0000:00:1d.7: init 0000:00:1d.7 fail, -95
  <4>ehci_hcd: probe of 0000:00:1d.7 failed with error -95

this is a FSC Amilo D7830 notebook, the guy on lkml has a  Asus P4P800 board.

-- 
USB is for mice, FireWire is for men!

sUse lINUX ag, nÜRNBERG

Re: [linux-usb-devel] [PATCH] proper bios handoff in ehci-hcd

[David Brownell]

Hi Olaf,

Olaf Hering wrote:
> 
> there are 2 reports about breakage by this patch. One is on lkml, and
> another one is in my bugzilla inbox. How can we fix that one? I assume
> that handoff patch is correct.

The only question I have about it right now is whether
it might not be more correct to use a _byte_ access to
set the "Host OS wants controller" flag.  It looks to me
like it does the right thing, per EHCI 1.0 section 5.1,
though maybe 500 msec is too short a period to wait.
See if 5000 msec helps.

The 0x01010001 flag is pretty clearly trouble, and
says that the BIOS hasn't reacted to the request.
Maybe it's polling at some rate slower than 2x/second
on those machines ... or maybe this is just a bios bug.

In this case, one could just look at byte 106 (104 + 2)
of pci config space later to see if it changed after
the 500 msec passed.

- Dave


>   <6>NET: Registered protocol family 17
>   <3>ehci_hcd 0000:00:1d.7: BIOS handoff failed (104, 1010001)
>   <3>ehci_hcd 0000:00:1d.7: can't reset
>   <3>ehci_hcd 0000:00:1d.7: init 0000:00:1d.7 fail, -95
>   <4>ehci_hcd: probe of 0000:00:1d.7 failed with error -95
> 
> this is a FSC Amilo D7830 notebook, the guy on lkml has a  Asus P4P800 board.
> 

Re: [linux-usb-devel] [PATCH] proper bios handoff in ehci-hcd

[Will Beers]

[-- Attachment #1: Type: text/plain, Size: 398 bytes --]

 > though maybe 500 msec is too short a period to wait.
 > See if 5000 msec helps.

I went all the way up to 20000 msec and it still didn't help.  I'm sure 
it's a bad idea, but removing that whole if-block below it makes it work 
(which is effectively what switching the and/or did).  I don't know enough 
about it to judge whether it's correct, but what exactly is it checking for 
there?

-Will

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/x-pkcs7-signature, Size: 3186 bytes --]

Re: [linux-usb-devel] [PATCH] proper bios handoff in ehci-hcd

[David Brownell]

Will Beers wrote:
>  > though maybe 500 msec is too short a period to wait.
>  > See if 5000 msec helps.
> 
> I went all the way up to 20000 msec and it still didn't help.  I'm sure 
> it's a bad idea, but removing that whole if-block below it makes it work 
> (which is effectively what switching the and/or did).  I don't know 
> enough about it to judge whether it's correct, but what exactly is it 
> checking for there?

There are two flags in adjacent bytes of pci config space.
State transitions are shown in the spec [1] (simple state
diagrams), but basically your hardware started out in a
"BIOS owned" mode, and we want Linux to run it instead.
So we change (0,1) to (1,1), then BIOS should get an IRQ
before changing it to (1,0) and ignoring EHCI ... it's not.

Sounds to me like your BIOS may be broken.  But if you're
up for it, you could try using byte access to write that one
flag byte; I could also believe some hardware won't issue the
SMI interrupt without that.  There are also a lot of bits in
the next word, which might let you stomp on on the BIOS in
constructive useful ways.

- Dave

[1] http://www.usb.org/developers/docs/ at the very
     bottom of the page, for that part you won't need
     to know anything else about USB.

Re: [linux-usb-devel] [PATCH] proper bios handoff in ehci-hcd

[Will Beers]

[-- Attachment #1: Type: text/plain, Size: 858 bytes --]

 > Sounds to me like your BIOS may be broken.  But if you're
 > up for it, you could try using byte access to write that one

Changing the pci_read_config to a byte access fixes it, thanks!

(included a diff)

-Will


diff -Nru a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
--- a/drivers/usb/host/ehci-hcd.c       2004-07-13 01:09:00.000000000 -0400
+++ b/drivers/usb/host/ehci-hcd.c       2004-07-13 17:06:00.000000000 -0400
@@ -300,7 +300,7 @@
                 do {
                         msleep(10);
                         msec -= 10;
-                       pci_read_config_dword(pdev, where, &cap);
+                       pci_read_config_byte(pdev, where, &cap);
                 } while ((cap & (1 << 16)) && msec);
                 if (cap & (1 << 16)) {
                         ehci_err (ehci, "BIOS handoff failed (%d, %04x)\n",

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/x-pkcs7-signature, Size: 3186 bytes --]

Re: [linux-usb-devel] [PATCH] proper bios handoff in ehci-hcd

[David Brownell]

[-- Attachment #1: Type: text/plain, Size: 474 bytes --]

Will Beers wrote:
>  > Sounds to me like your BIOS may be broken.  But if you're
>  > up for it, you could try using byte access to write that one
> 
> Changing the pci_read_config to a byte access fixes it, thanks!

You're reading byte 0 not byte 2 of that field ... I meant
more like the attached patch to _write_ the flag (untested).


> -                       pci_read_config_dword(pdev, where, &cap);
> +                       pci_read_config_byte(pdev, where, &cap);

[-- Attachment #2: Diff --]
[-- Type: text/plain, Size: 407 bytes --]

--- 1.89/drivers/usb/host/ehci-hcd.c	Wed Jun 30 19:10:04 2004
+++ edited/drivers/usb/host/ehci-hcd.c	Tue Jul 13 14:33:41 2004
@@ -293,8 +293,7 @@
 		struct pci_dev *pdev = to_pci_dev(ehci->hcd.self.controller);
 
 		/* request handoff to OS */
-		cap |= 1 << 24;
-		pci_write_config_dword(pdev, where, cap);
+		pci_write_config_byte(pdev, where + 3, 1);
 
 		/* and wait a while for it to happen */
 		do {

Re: [linux-usb-devel] [PATCH] proper bios handoff in ehci-hcd

[Will Beers]

[-- Attachment #1: Type: text/plain, Size: 267 bytes --]

 > more like the attached patch to _write_ the flag (untested).

Alright, sorry for the misunderstanding - that patch doesn't fix it either, 
so it must just be a broken bios.  But regardless, I'll go read those docs 
and hopefully it'll make more sense then.

-Will

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/x-pkcs7-signature, Size: 3186 bytes --]

Re: [linux-usb-devel] [PATCH] proper bios handoff in ehci-hcd

[Will Beers]

[-- Attachment #1: Type: text/plain, Size: 104 bytes --]

Clarification - changing the write to use byte access didn't fix it, but 
changing the read did.

-Will

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/x-pkcs7-signature, Size: 3186 bytes --]