Re: [PATCH] OHCI suspend/resume issue.

On Thu, Aug 09, 2007 at 01:00:09PM -0400, Alan Stern wrote:
> 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...

Urgh, I definitely need some sleep, yes, it's a &&.
> 
> > 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.

Correct.

Which, from the debugging statements from previous failed runs, we have
something odder.

reset_resume == 0, udev->state == USB_STATE_CONFIGURED.

This is an even more bizarre state then I thought.
> 
> 
> > > > 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.

Perhaps, we seem to have a lot of very odd state at the time that we try
to resume the OHCI root hub.
> 
> > 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().

Agreed, but I'm definitely at the end of my ability to dig through the
kernel, I'll take another crack at it after I've slept.

Zephaniah E. Hull.

-- 
	  1024D/E65A7801 Zephaniah E. Hull <[email protected]>
	   92ED 94E4 B1E6 3624 226D  5727 4453 008B E65A 7801
	    CCs of replies from mailing lists are requested.

-------------------------------------------------------------------------
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

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFGu0m2RFMAi+ZaeAERAuBZAJ45HOISUKyQvPyvURbRjH/kHziNDgCdGR0L
WEYvA0exIjauMAlEYdcKVJc=
=5kI7
-----END PGP SIGNATURE-----