On Monday 20 December 2004 8:08 am, Alan Stern wrote: > > This and the next few patches centralize most of the functionality > required for creating, registering, and removing hcd and bus structures > into the core. They remove a lot of duplication among various HC drivers > and also fix a bug that affected all the drivers, causing them to call > their stop() routines when start() returned an error.
Thhis all seems mostly OK to me. I'm not going to have any time soon to really rework the bus glue abstraction as I'd mentioned; this looks to be the best whack at that we'll get in the near future, so there's no point in not merging updated versions of as424b as425b, and as426b. However: - I'd rather see each of the bus glue patches -- for PCI, and for each of the non-PCI drivers -- be a separate patch, cc'd the person responsible for that glue. Otherwise it's pretty certain that the relevant folk won't notice they need to review things (at least to the extent of sanity testing) ... especially when as425b has a title that doesn't even mention OMAP, PXA27x, SL811, SA-1111, or LH7A404 (to catch developers' eyes). - I'm wondering why usb_create_hcd() doesn't dev_set_drvdata() so that all the bus glue (PCI etc) needn't replicate that. And likewise, have usb_remove_hcd() clear it. - Probably usb_add_hcd() should drop the IRQ handler parameter; everything should use the standard filter. (All existing cases where something else is used are wrong.) - In usb_add_hcd(), that call to the driver reset() will eventually need reworking. I think the "usb-handoff" stuff is probably a better approach for the PC BIOS initialization problems, although should live in the USB subtree (not the PCI quirks.c file) and not have a different copy of that logic. - The second dev_info() in usb_add_hcd() shouldn't assume "pci mem"; it's not always PCI, though it would always be "iomem". Also, it'd be cleaner to have the buf and bufp variables local to that block of code. - The OMAP patch doesn't apply against the latest omap-ohci; here's a version I sanity-tested. (Applies against Greg's BK or against the current linux-omap tree.) - I noticed that the ohci-lh7a404.c built with a warning. Probably worth asking Marc Singer to test-drive your patch and resolve that. (It's not initializing quite right ...) - Huh, I noticed that most of the non-PCI OHCI chips claim to be using IO space accessors, not memory accessors. That's untrue; but it works because ARM doesn't have IO space. (This bug seems to have been cloned from the original ohci-sa1111 support.) - For the sl811, please don't switch to the driver model diagnostics. As with pretty much every driver using the platform bus, they're just annoying: there's no point in having every driver message preceded by _two_ copies of the driver name. (Especially for the sl811: if there were a need for more than one root hub port, the designer would have chosen a different controller, not used two of those!!) - The OHCI PPC bus glue will need to be updated for this. Unfortunately we can't realistically expect all the non-PCI bus glue to be re-tested with 2.6.11, except for cases where the relevant board handles an almost-stock kernel ... which isn't as routine with embedded hardware as we might like, it takes time to merge enough drivers to the mainline kernel. So I think there may be a few issues lurking with some of this stuff, especially for older hardware like sa1111, for a while yet. But it looks basically OK. - Dave ------------------------------------------------------- This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting Tool for open source databases. Create drag-&-drop reports. Save time by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc. Download a FREE copy at http://www.intelliview.com/go/osdn_nl _______________________________________________ [email protected] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
