On Wed, 2 Feb 2005, David Brownell wrote: > 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.
Glad to have your approval! > 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). Makes sense. I'll split things up and CC: the maintainers. > - 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. Can't remember. I'm sure I thought about it while working on the patches; it probably just slipped my mind. > - 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.) Okay. This means removing the other handlers too, right? > - 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. I agree about reset(); it doesn't really do the right thing for the way it's used. The current implementation of the usb-handoff stuff isn't exactly what we want either. It should be less anxious to do a full reset; that's needed only when we can prove that the BIOS has interfered with the settings. Otherwise it's necessary only to prevent the device from interrupting or doing DMA. > - 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. Okay. That dev_info message is dicey anyway, because it tries to give useful information in a uniform format for a wide range of devices with varying requirements. I'll change the string to "iomem" and not worry any further. > - 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'll check it out. > - 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 ...) All right. This is the Marc Singer who contributes to the linux-arm list? > - 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.) I'm not sure what you mean. Their probe() functions call request_mem_region(). > - 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!!) Okay, I'll leave those messages unchanged. > - The OHCI PPC bus glue will need to be updated for this. Development doesn't stand still! Whenever a patch takes a while to be approved, more than likely new code will be added in the meantime that the patch also needs to address... > 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. It wouldn't be at all surprising. I can't even test-compile those drivers! Alan Stern ------------------------------------------------------- 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
