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?
Note that ohci_pci_resume() isn't _supposed_ to force the root hub to
be resumed. ohci_rh_resume() runs at a later time, after
ohci_pci_resume().
It would be appropriate to detect loss of VBUS power in
ohci_pci_resume() and set the controller's state accordingly, as the
comment in that routine indicates. But I don't know the best way of
doing so.
> 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?
> I've patched it, and hopefully there won't be any false positives, but I don't
> have another machine to do suspend/resume testing on, so while it works for
> me,
> I can't guarantee that it won't cause problems for others.
>
> In any case, USB 1.1 devices directly plugged into the machine didn't work
> after resume before this, and do now.
>
> Signed-off-by: Zephaniah E. Hull <[EMAIL PROTECTED]>
>
> diff --git a/drivers/usb/host/ohci-hub.c b/drivers/usb/host/ohci-hub.c
> index bb9cc59..3660dcc 100644
> --- a/drivers/usb/host/ohci-hub.c
> +++ b/drivers/usb/host/ohci-hub.c
> @@ -165,6 +165,12 @@ static int ohci_rh_resume (struct ohci_hcd *ohci)
> }
> } else switch (ohci->hc_control & OHCI_CTRL_HCFS) {
> case OHCI_USB_SUSPEND:
> + /* FIXME: we should try to detect loss of VBUS better. */
> + if (!autostopped) {
> + ohci_dbg (ohci, "resume root hub -- lost power\n");
> + status = -EBUSY;
> + break;
> + }
This is definitely not the right thing to do. We should be in the
default (reset) case, not here. The correct approach is to find out
why we aren't and fix it up.
And this certainly _will_ generate false positives on other machines.
> ohci->hc_control &= ~(OHCI_CTRL_HCFS|OHCI_SCHED_ENABLES);
> ohci->hc_control |= OHCI_USB_RESUME;
> ohci_writel (ohci, ohci->hc_control, &ohci->regs->control);
> diff --git a/drivers/usb/host/ohci-pci.c b/drivers/usb/host/ohci-pci.c
> index 48de318..ae1ecb2 100644
> --- a/drivers/usb/host/ohci-pci.c
> +++ b/drivers/usb/host/ohci-pci.c
> @@ -282,7 +282,9 @@ static int ohci_pci_resume (struct usb_hcd *hcd)
> prepare_for_handover(hcd);
>
> /* Force the PM core to resume the root hub */
> - hcd_to_bus(hcd)->root_hub->dev.power.prev_state.event = PM_EVENT_ON;
> + usb_root_hub_lost_power(hcd->self.root_hub);
> +
> + hcd->state = HC_STATE_SUSPENDED;
Again, the wrong thing to do. For one thing, there's no reason to call
usb_root_hub_lost_power() here since it will get called later on in
ohci_rh_resume(). For another, there's no reason to set hcd->state to
HC_STATE_SUSPENDED, since usb_hcd_pci_resume() in
drivers/usb/core/hcd-pci.c has already verified that it has that value.
> return 0;
> }
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/
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
