Hi !
The new ohci-hcd driver in 2.6 doesn't work for Power Management on Apple laptops at least. I've studied the code, but since I'm not very familiar with the whole new hcd architecture, I'm sure I'm still missing a lot.
This is with 2.6.0-test3-latest? I don't think PM itself is currently expected to work ... much less any particular driver.
There were a few small updates to that code last week. But the last time I checked there were still PM bugs in the PCI code (Pavel had a not-ready-to-merge patch). Plus, recent PM merges broke the APM support (it relies on the device_suspend routine, which no longer does anyting). So every platform where I've been able to use power management is currently broken -- no fault of USB.
The big problem with the current code is that it will still access the chip after suspend. (Or during the suspend operation, it would be better to properly stop all activities before writing to the control register to suspend the chip).
But how does it access that? There was a patch not long ago to prevent the root hub timers from doing anything during suspend. (I'm not sure that got merged.) Other code _should_ be refusing to touch the hardware when it's suspended.
- The root hub timer. I hacked the get status function to return 0 when ohci->sleeping is set, though that would need proper synchronisation on SMP etc...That helped not locking up 100% of the time during the suspend process, but I'm still having the problem waking the machine up (memory corruption). I also reject root hub control urbs
This sounds like what that root hub timer patch was doing. Actually the ohci->sleeping flag should go away; it's duplicating information in hcd->state. (Same thing with the ohci->disabled flag.)
- It seems that we don't accept to link urbs (ohci_urb_enqueue returns with an error), however, ohci_urb_dequeue may do things that end up touching the queue head registers. Those registers should probably be shadowed some way.
I don't think shadowing will work very well, since when the controller comes out of suspend it will expect those registers to still be valid. Better to prevent dequeue when suspended.
The last time this came up, Alan Stern suggested that USB drivers should unlink their URBs as part of suspend. I'm not entirely sure I like that solution; we didn't need it in 2.4, so it "shouldn't" be needed now. But there was also the comment that such changes should wait for some driver model changes to the suspend()/resume() calls. (Which don't yet seem to have been merged.)
- I haven't yet figured out other code path leading to touching chip registers outside of the above, I'm not too sure about the ED stuffs.
In normal operation, I'd only expect it from: IRQs, the root hub timer, and urb enqueue/dequeue. But things like checking the frame number (does any driver even use that call?) would too.
Any suggestion ? Right now, my fix to the root hub isn't enough it seems but I haven't yet spotted what else is touching the chip.
The proper sequence of operations would probably be something like (similar to what we have today) for S3 (and S2 if we ever use that)
- Set ohci->sleeping
No, this duplicate state should vanish.
- Disable all IRQs but SF. From this point, no "link" operation is accepted (URBs or EDs) and root hub stops polling. Unlinks are still allowed - Wait a few frames so pending unlinks and done queue can be flushed
Implementing unlinks and processing completed TDs requires a few more IRQs enabled than that, though! And all this shutdown should be managed through the driver model -- as much as necessary.
We could certainly arrange that urb submissions not be accepted for any device unless the driver model power_state allows.
- Mark the chip as suspended. This could be setting ohci->sleeping to 2 or using another flag. This one really means DO NOT TOUCH THE HW again (we could have wrappers on read/writel catching bugs here
That's the intent of the current code, but it seems as if a root hub patch (usb/core/hcd.c) still needs merging.
eventually but that would be ugly). At this point, unlinks will be done on shadow of the list head registers only. Make sure we sychronize with a pending unlink on another CPU
We shouldn't need to modify the unlink logic.
- Really suspend the chip (write to control etc...) - Do the remaining bits ... (PCI PM mode, PowerMac specific shutdown code etc....)
Do that seem correct to you ?
It doesn't seem like it uses the driver model suspend/resume logic, which is where a lot of this work should be done. For example, all the children of the OHCI hardware (root hub, and devices connected to it) can do their cleanup in driver model suspend calls before the OHCI PM gets called ... and resumed after it comes back.
In the case of S4 (suspend-to-disk), I beleive we have no much choice but doing a full unplug/replug of all devices though I'm not sure how to trigger that.
That should be almost identical to D3cold resume. Until the recent PM putbacks, that was working for me. OHCI does have specific code already in place to notice that the controller comes back in reset, not suspend state -- and to cleanly disconnect all devices. (Then, normal enumeration handles the rest.)
- Dave
Ben.
------------------------------------------------------- This SF.net email is sponsored by: VM Ware With VMware you can run multiple operating systems on a single machine. WITHOUT REBOOTING! Mix Linux / Windows / Novell virtual machines at the same time. Free trial click here:http://www.vmware.com/wl/offer/358/0 _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
