Re: Machine check in ohci_hub_status_data (powerpc)

On Wed, 18 Apr 2007, Johannes Berg wrote:

> Hi,
> 
> Here's something I just observed yesterday. At first I was blaming my
> suspend changes for powerpc since I can't make it happen without them,
> but I've gone back and forth over them and can't find the problem, so
> while I'm still not sure they aren't at fault I wanted some input from
> the USB side.
> 
> What happens is that apparently for whatever (legitimate or not) reason
> right before suspend the rh_timer in hcd.c gets scheduled but never runs
> before suspend. Then during resume, when IRQs are enabled, it gets to
> run, but the device hasn't been resumed yet.
> 
> So what happens then is that in rh_timer usb_hcd_poll_rh_status is
> called and calls (indirectly) ohci_hub_status_data which errors out in
> line 431:
> 
> 430:        /* init status */
> 431:        if (roothub_status (ohci) & (RH_HS_LPSC | RH_HS_OCIC))
> 432:                buf [0] = changed = 1;
> 
> for the obvious reason that roothub_status() can't actually access the
> registers yet. I suspect this can also happen on x86 but that just
> returns 0xffffffff on the read and not much more happens. Or powermac
> really is special because we have a hook to turn off the USB asic.
> 
> Now, since I mentioned a timer timing clearly has an influence :) But
> the odd thing is that it happens fairly consistently when I suspend from
> the console right after boot, while it never happens when suspending
> from within X (or even from the console when I'm logged in in X). Maybe
> suspend is so much slower from X that the timer fires before the machine
> is suspended or something.
> 
> It seems to me that the timer shouldn't be scheduled when we go into
> suspend, but I'll leave the proper fix up to somebody else. Unless of
> course my suspend changes do indeed have a bug somewhere (I sure hope
> not), unfortunately I haven't been able to trigger this on an unpatched
> kernel. But I haven't tried very hard either.

The comparable routine in uhci-hcd includes this code:

	if (!test_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags) || uhci->dead)
		goto done;

and likewise, ehci_hub_status_data() includes this:

	if (!HC_IS_RUNNING(hcd->state))
		return 0;

Something similar needs to be added in ohci-hub.c.  Does the patch below 
fix your problem?

Alan Stern



Index: usb-2.6/drivers/usb/host/ohci-hub.c
===================================================================
--- usb-2.6.orig/drivers/usb/host/ohci-hub.c
+++ usb-2.6/drivers/usb/host/ohci-hub.c
@@ -417,6 +417,8 @@ ohci_hub_status_data (struct usb_hcd *hc
 	unsigned long	flags;
 
 	spin_lock_irqsave (&ohci->lock, flags);
+	if (!test_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags))
+		goto done;
 
 	/* undocumented erratum seen on at least rev D */
 	if ((ohci->flags & OHCI_QUIRK_AMD756)


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel