On Monday 03 January 2005 9:26 am, Olav Kongas wrote: > Hi, > > Here comes a new isp116x driver. I took David Brownell's > recently updated sl811 driver and ported it to isp116x > chips. It is considerably smaller and simpler than other > isp116x drivers for 2.6 kernel posted to this > list recently. The driver does not support iso transfers.
Cool! Smaller and simpler are good ... they tend to translate to "more reliable", and "easier to maintain". (Which for Linux means "more people can help".) Folk have been wanting isp116x support for a long time ... it looks like we're a lot closer! I take it you're getting this ready to submit for merging to the main kernel treee (2.6.11?), after you get some feedback from other users (and more testing)? Here are a couple review comments, from a quick read: - #define DRIVER_VERSION __DATE__ does suggest it's not quite ready for merging ... I tend to use "dev/" __TIME__", to be a bit more explicit (and highlight "wrong module" problems just a smidgeon better), since I often use dates as version codes and so __DATE__ gets ambiguous. - Please don't include "ohci.h". If there's stuff from there you want to use, just copy it; this isn't an OHCI chip. (The root port bits might even use the "hub.h" names...) - Please keep the drivers/usb/host/Kconfig entries in alphabetical order. - If you haven't run this with the "usbtest" driver, please try to do so (see http://www.linux-usb.org/usbtest for scripts etc). That does a reasonable job of forcing an HCD to use code paths that normal/successful operation won't hit; those are the ones where not acting like the "other" HCDs may cause trouble later. (Testing interrupt transfers involves a module option.) - Do you really need to put a 2 msec floor on periodic transfers, or is that just paranoia? - You can simplify the root port logic a lot by having a single jiffies counter. I think that's how UHCI does it now; EHCI does that, but it needs a per-port counter (since some silicon has so many ports that the USB spec says just one isn't enough). It's not polite to spin for 100 msec with IRQs blocked... - Some of the comments copied from the SL811 HCD are clearly wrong for the ISP 116x, like the CONFIG_PM ones. (When you're using USB_SUSPEND, the root hub and the controller really are going to be different.) - I didn't see any of the code I copied from Cypress there (for SL811 registers, and access to them), so there's no need to include their copyright. - Are there any reasons this shouldn't work for the ISP 1362, after some more tweaks? One complication there is that a driver for that chip should plan to support the gadget functionality, HCD functionality, or both (and OTG). That means factoring it can be a bit tricky. (I have some ideas along those lines, not quite ready to share, for TDI243 which has the same sort of configuration complexity.) This looks quite promising!! - Dave ------------------------------------------------------- The SF.Net email is sponsored by: Beat the post-holiday blues Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek. It's fun and FREE -- well, almost....http://www.thinkgeek.com/sfshirt _______________________________________________ [email protected] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
