Re: [PATCH] OHCI suspend/resume issue.

On Thu, 9 Aug 2007, Zephaniah E. Hull wrote:

> I'll try to keep this making sense, but I'm going to have to reply to
> things slightly out of order.

Thanks for the detailed reply.

> On Thu, Aug 09, 2007 at 11:27:02AM -0400, Alan Stern wrote:
> > On Thu, 9 Aug 2007, Zephaniah E. Hull wrote:
> > 
> > > OHCI isn't coming back on the OLPC after resume.
> > > 
> > > After a bit of testing, the problem seems to have come down to two points.
> > > 
> > > The first is that ohci_pci_resume is not forcing the root hub to be resumed
> > > properly, that's a fairly trivial and obviously correct patch.
> > 
> > Can you be more specific?  What exactly goes wrong?
> 
> The first problem is pretty noticeable, ohci_rh_resume never gets called.
> 
> The chain roughly goes usb_resume_both -> usb_resume_device ->
> generic_resume (some indirection here) -> hcd_bus_resume ->
> ohci_rh_resume (some indirection here too).
> 
> usb_resume_device has an interesting check, if udev->state !=
> USB_STATE_SUSPENED or if udev->reset_resume is false, then it silently
---------------------^

That's an &&, not an ||.  Unless somehow it got changed in your version 
of the code...

> decides not to pass things down the chain, without a failure. (Note that
> udev is usb_device, the joy of confusing naming.)
> 
> This is quite possibly a bug,

It certainly would be a bug if somebody changed it from "and" to "or".  
Is that the cause of your problem?

>  since a few lines down in the same
> function it checks for a quirk, USB_QUIRK_RESET_RESUME, and sets
> reset_resume to 1 if it's there.  This code has no impact, since it's
> never reached if reset_resume isn't already 1.

Go back and look at the routine again.  Maybe you just misread the 
first test.


> > > The second is trickier, ohci_rh_resume is getting called in a state that it
> > > thinks is an indication that it's coming back from a SUSPEND where it did not
> > > lose power.
> > 
> > You mean ohci->regs->control doesn't contain OHCI_USB_RESET in the 
> > appropriate bits?  What does it contain?  And why?
> 
> ohci->hc_control & OHCI_CTRL_HCFS == OHCI_USB_SUSPEND, and I honestly
> don't have the foggiest clue how or why it has that after coming back
> from the chip being powered off.
> 
> My best guess is that somewhere else in the resume path we're resetting
> it, but that's only a guess and I have no idea why anything would do
> that.

Nothing should.  You might want to add a debugging printk to
ohci_pci_resume() or prepare_for_handover() to see what the value is at
the start of the resume.  Maybe the firmware sets it incorrectly before 
the driver does anything.

> Infact, grepping through the source tree, the only thing setting
> OHCI_USB_SUSPEND is ohci_rh_suspend, which would have happened prior to
> turning off the device.
> 
> My guess is that something is blindly restoring from ohci->hc_control
> without first reading in the value.

Maybe, but nothing capable of doing that should get called before 
ohci_rh_resume().

Alan Stern


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel