David Brownell wrote:
>
> Roman Weissgaerber wrote:
> >
> > Hi,
> >
> > I don't think this is the right way to handle suspend/resume.
> > So, I don't like that patch.
>
> I could perhaps try an alternate patch you provided. What is
> the issue you have with this one?
>
You disconnect all devices and reset the USB-subsystem.
This is like a shutdown of a computer at a suspend and a
boot at a resume.
Roman
> > As far as I can remember there was a patch made by Alan which
> > disables the HC-Interrupt and solves that problem, right.
> > (But Linus don't like it that way.)
>
> Alan's did not solve the problem when I tested it on my hardware.
> (See the note I posted on that topic yesterday.)
>
> It's possible that there are multiple problems in this space.
>
> > Now, why we just disable the interrupts by setting the MIE-Bit in
> > the HcInterruptDisable Register on suspend and activate the interrupts
> > by setting the MIE-Bit in the HcInterruptEnable Register on resume, again?
>
> Partly because when the chip came back after suspend, its
> registers acted just like they'd gotten a power-up reset.
> Tweaking interrupts couldn't restart it, or re-establish
> the various modes that were previously set up.
>
> There are other reasons too.
>
> - Dave
>
> >
> > Roman
> >
> > David Brownell wrote:
> > >
> > > Here's the patch I mentioned earlier, which avoids the Oops by
> > > making suspend/resume work again. Against pre4-2 with the patch
> > > I submitted for fewer magic #s and better debugging (that matters).
> > >
> > > For me, it's the difference between minor or major reinitialization of
> > > the USB subsystem over suspend/resume, and just having it work.
> > >
> > > The notable fixes:
> > >
> > > - On power management suspend, disconnect all devices on
> > > the controller's bus. Controller left in reset state.
> > >
> > > - On power management resume, reset and restart the controller.
> > > Otherwise it didn't seem to want to restart.
> > >
> > > There are a few other changes in here, and any relating to whitespace
> > > or debugging messages could of course be discarded ... though I'd leave
> > > the change splitting the "#endif break;" line into two, unless someone
> > > believes the code could really have been right as-was.
> > >
> > > I'd like to know how this works for other non-PMAC OHCI laptops.
> > > (PMAC has its own suspend/resume code, untouched by this patch.)
> > >
> > > - Dave
> > >
> > >
>--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> > > --- linux/drivers/usb/usb-ohci.c+patch Sat Apr 1 19:20:56 2000
> > > +++ linux/drivers/usb/usb-ohci.c Sun Apr 2 23:02:42 2000
> > > @@ -149,8 +149,8 @@
> > > printk ("%s stat:%d\n", i < len? "...": "", urb->status);
> > > }
> > > }
> > > -
> > > }
> > > +
> > > /* just for debugging; prints non-empty branches of the int ed tree inclusive
>iso eds*/
> > > void ep_print_int_eds (ohci_t * ohci, char * str) {
> > > int i, j;
> > > @@ -610,8 +610,8 @@
> > > }
> > > spin_unlock_irqrestore (&usb_ed_lock, flags);
> > >
> > > - if (cnt > 0) {
> > > - dev->wait = &wait;
> > > + if (cnt > 0) {
> > > + dev->wait = &wait;
> > > current->state = TASK_UNINTERRUPTIBLE;
> > > schedule_timeout (HZ / 10);
> > > remove_wait_queue (&op_wakeup, &wait);
> > > @@ -835,10 +835,12 @@
> > > }
> > > }
> > > }
> > > - for (i = int_branch; i < 32; i += interval)
>ohci->ohci_int_load[i] -= ed->int_load;
> > > + for (i = int_branch; i < 32; i += interval)
> > > + ohci->ohci_int_load[i] -= ed->int_load;
> > > #ifdef DEBUG
> > > ep_print_int_eds (ohci, "UNLINK_INT");
> > > -#endif break;
> > > +#endif
> > > + break;
> > >
> > > case ISO:
> > > if (ohci->ed_isotail == ed)
> > > @@ -1601,6 +1603,7 @@
> > > case RH_SET_CONFIGURATION: WR_RH_STAT (0x10000); OK (0);
> > >
> > > default:
> > > + dbg ("unsupported root hub command");
> > > status = TD_CC_STALL;
> > > }
> > >
> > > @@ -1637,7 +1640,7 @@
> > > * HC functions
> > > *-------------------------------------------------------------------------*/
> > >
> > > -/* reset the HC not the BUS */
> > > +/* reset the HC and BUS */
> > >
> > > static int hc_reset (ohci_t * ohci)
> > > {
> > > @@ -1685,7 +1688,7 @@
> > >
> > > static int hc_start (ohci_t * ohci)
> > > {
> > > - unsigned int mask;
> > > + __u32 mask;
> > > unsigned int fminterval;
> > > struct usb_device * usb_dev;
> > > struct ohci_device * dev;
> > > @@ -1704,35 +1707,35 @@
> > > writel (fminterval, &ohci->regs->fminterval);
> > > writel (0x628, &ohci->regs->lsthresh);
> > >
> > > - /* Choose the interrupts we care about now, others later on demand */
> > > - mask = OHCI_INTR_MIE | OHCI_INTR_UE | OHCI_INTR_WDH | OHCI_INTR_SO;
> > > -
> > > /* start controller operations */
> > > ohci->hc_control = OHCI_CONTROL_INIT | OHCI_USB_OPER;
> > > writel (ohci->hc_control, &ohci->regs->control);
> > >
> > > + /* Choose the interrupts we care about now, others later on demand */
> > > + mask = OHCI_INTR_MIE | OHCI_INTR_UE | OHCI_INTR_WDH | OHCI_INTR_SO;
> > > writel (mask, &ohci->regs->intrenable);
> > > writel (mask, &ohci->regs->intrstatus);
> > >
> > > -#ifdef OHCI_USE_NPS
> > > +#ifdef OHCI_USE_NPS
> > > writel ((readl(&ohci->regs->roothub.a) | RH_A_NPS) & ~RH_A_PSM,
> > > &ohci->regs->roothub.a);
> > > writel (RH_HS_LPSC, &ohci->regs->roothub.status);
> > > // POTPGT delay is bits 24-31, in 2 ms units.
> > > mdelay ((readl(&ohci->regs->roothub.a) >> 23) & 0x1fe);
> > > -#endif /* OHCI_USE_NPS */
> > > +#endif /* OHCI_USE_NPS */
> > >
> > > /* connect the virtual root hub */
> > > -
> > > + ohci->rh.devnum = 0;
> > > usb_dev = usb_alloc_dev (NULL, ohci->bus);
> > > - if (!usb_dev) return -1;
> > > + if (!usb_dev)
> > > + return -ENOMEM;
> > >
> > > dev = usb_to_ohci (usb_dev);
> > > ohci->bus->root_hub = usb_dev;
> > > usb_connect (usb_dev);
> > > if (usb_new_device (usb_dev) != 0) {
> > > usb_free_dev (usb_dev);
> > > - return -1;
> > > + return -ENODEV;
> > > }
> > >
> > > return 0;
> > > @@ -1755,7 +1758,7 @@
> > > return;
> > > }
> > >
> > > - dbg("Interrupt: %x frame: %x", ints, le16_to_cpu (ohci->hcca.frame_no));
> > > + // dbg("Interrupt: %x frame: %x", ints, le16_to_cpu
>(ohci->hcca.frame_no));
> > >
> > > if (ints & OHCI_INTR_UE) {
> > > ohci->disabled++;
> > > @@ -1835,8 +1838,9 @@
> > > dbg("USB HC release ohci");
> > >
> > > /* disconnect all devices */
> > > - if (ohci->bus->root_hub) usb_disconnect (&ohci->bus->root_hub);
> > > -
> > > + if (ohci->bus->root_hub)
> > > + usb_disconnect (&ohci->bus->root_hub);
> > > +
> > > hc_reset (ohci);
> > > writel (OHCI_USB_RESET, &ohci->regs->control);
> > > wait_ms (10);
> > > @@ -1872,6 +1876,7 @@
> > > #endif
> > > printk(KERN_INFO __FILE__ ": USB OHCI at membase 0x%lx, IRQ %s\n",
> > > (unsigned long) mem_base, bufp);
> > > + printk(KERN_INFO __FILE__ ": %s\n", dev->name);
> > >
> > > ohci = hc_alloc_ohci (mem_base);
> > > if (!ohci) {
> > > @@ -1890,7 +1895,7 @@
> > > wait_ms (10);
> > > usb_register_bus (ohci->bus);
> > >
> > > - if (request_irq (irq, hc_interrupt, SA_SHIRQ, "ohci-usb", ohci) == 0) {
> > > + if (request_irq (irq, hc_interrupt, SA_SHIRQ, "usb-ohci", ohci) == 0) {
> > > struct pm_dev *pmdev;
> > >
> > > ohci->irq = irq;
> > > @@ -1984,22 +1989,24 @@
> > > #endif /* CONFIG_PMAC_PBOOK */
> > >
> > > /*-------------------------------------------------------------------------*/
> > > -
> > > +
> > > static int handle_pm_event (struct pm_dev *dev, pm_request_t rqst, void *data)
> > > {
> > > ohci_t * ohci = (ohci_t*) dev->data;
> > > + int temp = 0;
> > > +
> > > if (ohci) {
> > > switch (rqst) {
> > > case PM_SUSPEND:
> > > - dbg("USB-Bus suspend: %p", ohci);
> > > - writel (ohci->hc_control = 0xFF, &ohci->regs->control);
> > > - wait_ms (10);
> > > + dbg("USB-Bus suspend: %p", ohci->regs);
> > > + if (ohci->bus->root_hub)
> > > + usb_disconnect (&ohci->bus->root_hub);
> > > + hc_reset (ohci);
> > > break;
> > > case PM_RESUME:
> > > - dbg("USB-Bus resume: %p", ohci);
> > > - writel (ohci->hc_control = 0x7F, &ohci->regs->control);
> > > - wait_ms (20);
> > > - writel (ohci->hc_control = 0xBF, &ohci->regs->control);
> > > + dbg("USB-Bus resume: %p", ohci->regs);
> > > + if ((temp = hc_reset (ohci)) < 0 || (temp = hc_start
>(ohci)) < 0)
> > > + err ("can't restart controller, %d", temp);
> > > break;
> > > }
> > > }
> > >
> > >
>--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: [EMAIL PROTECTED]
> > > For additional commands, e-mail: [EMAIL PROTECTED]
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, e-mail: [EMAIL PROTECTED]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
