Re: [RFC/RFT] OHCI: Fix for regression in 2.6.37-rc1 since commit 3df7169e7

Re: [RFC/RFT] OHCI: Fix for regression in 2.6.37-rc1 since commit 3df7169e7

[Alan Stern]

On Fri, 12 Nov 2010, [UTF-8] Pali Rohár wrote:

> Hello,
> 
> sorry for late. Can I test this patch (with usb shutdown patch) or is wrong?

It is wrong.  You can test it if you like, but it's not going to be 
accepted into the kernel.

Alan Stern

Re: [RFC/RFT] OHCI: Fix for regression in 2.6.37-rc1 since commit 3df7169e7

[Alan Stern]

On Sat, 11 Dec 2010, Larry Finger wrote:

> > I don't see anything wrong with any of this.  Where's the problem?
> 
> The problem is that I get thousands of the "unable to enumerate" messages in the
> logs. That PORT_OWNER does not seem to apply to this port. Everything seems to
> work, but the log spamming is intolerable.

Everything seems to work for the EHCI controller, the one you were 
looking at.  But the OHCI controller isn't working, so that's where you 
need to concentrate your attention.  ISTR before your computer broke 
you were trying to figure out why certain parts of the ohci-hcd 
initialization code weren't getting called.

> I find the USB structure on this box to be confusing. The output of lsusb is
> 
> Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
> Bus 001 Device 002: ID 04f2:b016 Chicony Electronics Co., Ltd VGA 30fps UVC Webcam
> Bus 001 Device 005: ID 07d1:3300 D-Link System
> Bus 002 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
> Bus 003 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
> Bus 004 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
> 
> There are 3 external USB ports, which are ports 3, 4, and 5 on the Bus 001 hub.

There's probably at least one internal port, where that Chicony webcam
is connected.  Unless it's using an external port?

> That is the one with iSerial of 1 0000:00:02.1. The other hubs do not seem to be
> connected to anything.

Bus 2 probably is not connected to anything.  My guess is that bus 3 is 
also connected to the three external ports and bus 4 is not wired up.

Alan Stern

Re: [RFC/RFT] OHCI: Fix for regression in 2.6.37-rc1 since commit 3df7169e7

[Larry Finger]

Alan,

The box with the problem has been repaired, and I am trying to debug it.

What I have learned so far:

The "unable to enumerate" message is triggered because PORT_OWNER is not set in
the status register.

So far, I have found that this bit is set in only 4 places:

1. set_owner()
2. store_companion()
3. ehci_hub_control()
4. ehci_relinquish_port()
5. check_reset_complete()

Of these, only #5 is called before the first enumeration message is logged, but
it is only called with arguments that never set the PORT_OWNED bit in the status.

At this point I'm at a loss. Could you please give me ideas on where I might look.

The logged data leading up to the first message are:

[    3.002762] hub 4-0:1.0: USB hub found
[    3.002780] hub 4-0:1.0: 5 ports detected
[    3.088286] ehci_hcd 0000:00:02.1: usb: Entered check_reset_complete with
port_status 0x1005
[    3.088433] ehci_hcd 0000:00:02.1: port 4 high speed
[    3.144061] usb 1-4: new high speed USB device using ehci_hcd and address 2
[    3.200286] ehci_hcd 0000:00:02.1: usb: Entered check_reset_complete with
port_status 0x1005
[    3.200437] ehci_hcd 0000:00:02.1: port 4 high speed
[    3.352298] ehci_hcd 0000:00:02.1: usb: Entered check_reset_complete with
port_status 0x1005
[    3.352304] ehci_hcd 0000:00:02.1: port 5 high speed
[    3.408071] usb 1-5: new high speed USB device using ehci_hcd and address 3
[    3.454449] PM: Starting manual resume from disk
[    3.464305] ehci_hcd 0000:00:02.1: usb: Entered check_reset_complete with
port_status 0x1005
[    3.464449] ehci_hcd 0000:00:02.1: port 5 high speed
[    3.600052] ehci_hcd 0000:00:04.1: usb: Entered check_reset_complete with
port_status 0x1001
[    3.600202] ehci_hcd 0000:00:04.1: port 5 full speed --> companion
[    3.600307] ehci_hcd 0000:00:04.1: Wrote 0x3001 to status reg
[    3.656089] hub 2-0:1.0: unable to enumerate USB device on port 5
[    3.656203] ehci_hcd 0000:00:04.1: In ehci_relinquish_port
[    3.656295] ehci_hcd 0000:00:04.1: In set_owner

Thanks,

Larry

Re: [RFC/RFT] OHCI: Fix for regression in 2.6.37-rc1 since commit 3df7169e7

[Alan Stern]

On Wed, 3 Nov 2010, Larry Finger wrote:

> On 11/03/2010 04:04 PM, Alan Stern wrote:
> > 
> > This patch is definitely wrong -- it defeats the entire purpose of 
> > rewriting the control register!  That write was added _specifically_ to 
> > put the controller into reset, i.e., to change the functional state.
> > 
> > Furthermore, the write does the same thing as ohci_hcd.c does in its
> > ohci_usb_reset() routine.  Regardless, ohci_run() does a complete host
> > controller reset, which should override anything done in pci-quirks.c.  
> > Perhaps _that_ routine needs some attention.
> 
> It is looking as if your patch is exposing a different bug. When I disable the
> write in pci_quirks leaving the controllers in OPER or RESUME mode, then
> ohci_run() is not called. I'm still trying to find out why.

Is your OHCI controller attached via PCI?  If it is, I don't see how 
ohci_pci_start() can avoid calling ohci_run().

Alan Stern

Re: [RFC/RFT] OHCI: Fix for regression in 2.6.37-rc1 since commit 3df7169e7

[Larry Finger]

On 11/03/2010 04:04 PM, Alan Stern wrote:
> 
> This patch is definitely wrong -- it defeats the entire purpose of 
> rewriting the control register!  That write was added _specifically_ to 
> put the controller into reset, i.e., to change the functional state.
> 
> Furthermore, the write does the same thing as ohci_hcd.c does in its
> ohci_usb_reset() routine.  Regardless, ohci_run() does a complete host
> controller reset, which should override anything done in pci-quirks.c.  
> Perhaps _that_ routine needs some attention.

It is looking as if your patch is exposing a different bug. When I disable the
write in pci_quirks leaving the controllers in OPER or RESUME mode, then
ohci_run() is not called. I'm still trying to find out why.

Larry

Re: [RFC/RFT] OHCI: Fix for regression in 2.6.37-rc1 since commit 3df7169e7

[Alan Stern]

On Wed, 3 Nov 2010, Larry Finger wrote:

> Following commit 3df7169e73fc1d71a39cffeacc969f6840cdf52b, my logs are spammed
> with messages of the form "hub 2-0:1.0: unable to enumerate USB device on
> port 5".
> 
> Experimentation has shown that if the functional state bits of the control
> register are preserved in the fixup, then the messages no longer appear.
> 
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> ---
> 
> Pali,
> 
> Is it possible for you to test this patch to ensure that it does not interfere
> with the fix for the shutdown problem.
> 
> Thanks,
> 
> Larry
> ---
> 
> Index: linux-realtek/drivers/usb/host/pci-quirks.c
> ===================================================================
> --- linux-realtek.orig/drivers/usb/host/pci-quirks.c
> +++ linux-realtek/drivers/usb/host/pci-quirks.c
> @@ -35,6 +35,7 @@
>  #define OHCI_INTRENABLE		0x10
>  #define OHCI_INTRDISABLE	0x14
>  #define OHCI_OCR		(1 << 3)	/* ownership change request */
> +#define OHCI_CTRL_HCFS		(3 << 6)	/* functional state */
>  #define OHCI_CTRL_RWC		(1 << 9)	/* remote wakeup connected */
>  #define OHCI_CTRL_IR		(1 << 8)	/* interrupt routing */
>  #define OHCI_INTR_OC		(1 << 30)	/* ownership change */
> @@ -184,7 +185,7 @@ static void __devinit quirk_usb_handoff_
>  #ifdef __hppa__
>  #define	OHCI_CTRL_MASK		(OHCI_CTRL_RWC | OHCI_CTRL_IR)
>  #else
> -#define	OHCI_CTRL_MASK		OHCI_CTRL_RWC
> +#define	OHCI_CTRL_MASK		(OHCI_CTRL_RWC | OHCI_CTRL_HCFS)
> 
>  	if (control & OHCI_CTRL_IR) {
>  		int wait_time = 500; /* arbitrary; 5 seconds */

This patch is definitely wrong -- it defeats the entire purpose of 
rewriting the control register!  That write was added _specifically_ to 
put the controller into reset, i.e., to change the functional state.

Furthermore, the write does the same thing as ohci_hcd.c does in its
ohci_usb_reset() routine.  Regardless, ohci_run() does a complete host
controller reset, which should override anything done in pci-quirks.c.  
Perhaps _that_ routine needs some attention.

Alan Stern

[RFC/RFT] OHCI: Fix for regression in 2.6.37-rc1 since commit 3df7169e7

[Larry Finger]

Following commit 3df7169e73fc1d71a39cffeacc969f6840cdf52b, my logs are spammed
with messages of the form "hub 2-0:1.0: unable to enumerate USB device on
port 5".

Experimentation has shown that if the functional state bits of the control
register are preserved in the fixup, then the messages no longer appear.

Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---

Pali,

Is it possible for you to test this patch to ensure that it does not interfere
with the fix for the shutdown problem.

Thanks,

Larry
---

Index: linux-realtek/drivers/usb/host/pci-quirks.c
===================================================================
--- linux-realtek.orig/drivers/usb/host/pci-quirks.c
+++ linux-realtek/drivers/usb/host/pci-quirks.c
@@ -35,6 +35,7 @@
 #define OHCI_INTRENABLE		0x10
 #define OHCI_INTRDISABLE	0x14
 #define OHCI_OCR		(1 << 3)	/* ownership change request */
+#define OHCI_CTRL_HCFS		(3 << 6)	/* functional state */
 #define OHCI_CTRL_RWC		(1 << 9)	/* remote wakeup connected */
 #define OHCI_CTRL_IR		(1 << 8)	/* interrupt routing */
 #define OHCI_INTR_OC		(1 << 30)	/* ownership change */
@@ -184,7 +185,7 @@ static void __devinit quirk_usb_handoff_
 #ifdef __hppa__
 #define	OHCI_CTRL_MASK		(OHCI_CTRL_RWC | OHCI_CTRL_IR)
 #else
-#define	OHCI_CTRL_MASK		OHCI_CTRL_RWC
+#define	OHCI_CTRL_MASK		(OHCI_CTRL_RWC | OHCI_CTRL_HCFS)

 	if (control & OHCI_CTRL_IR) {
 		int wait_time = 500; /* arbitrary; 5 seconds */