On Wed, 2004-09-29 at 13:57 +0200, Duncan Sands wrote: > Hi David, > > > OK... if the firmware is found in /lib/firmware this loads it, then > > starts up the modem and monitors the status. If the firmware _isn't_ > > found it waits for modem_run as before. > > this races with modem_run: since the firmware loading is done in a separate > kernel thread, it is possible to do: > > (1) probe completes > (2) hotplug call - modem_run tries to grab the interface and fails > (3) firmware loading code fails to load the firmware - releases the interface > but too late.
Hmmm, true. That's how you had it in the first place though. Short of doing the first request_firmware() synchronously I'm not sure how we could fix that. Perhaps we could wait until the stage 1 firmware has arrived successfully, and _then_ try to claim the interface? > > It pokes at the modem first to see if it responds to a status request; > > if it _does_ then it knows the firmware is loaded, and doesn't try to > > load it again. If the initial status request fails, we load the > > firmware. > > Are you sure that is reliable? Not 100% sure, no. It's working fine on my modem but it would be nice to see it tested further. > > You were releasing interface #2 after loading the firmware -- I stopped > > doing that, because I think we want to keep it reserved to prevent the > > user from running modem_run. > > I did it exactly so the user can run modem_run! IIRC I set things up fairly > carefully to maintain backwards compatibility. But modem_run won't work if the firmware is already loaded _anyway_. In the case where we've already loaded the firmware, what's the benefit in letting them run modem_run? > Do you really need to set names according to the revision? After all, > user-space can get the version number from "version" in sysfs. Shouldn't > it be enough to use "stage1" and "stage2" and let user-space add a suffix > (here you may notice that I've forgotten how the firmware loader works - > something else to do tonight!). The firmware loader just gets the file with the name you asked for, from the /lib/firmware/ directory. Yes, we probably _could_ add extra smarts to it to make it work this kind of thing out for itself -- we could even use the firmware extraction tool and dynamically fetch the part we require... but I think it's best not to. > By the way, do revision 0 modems really have no altsetting 2 for interface 1? Oh, I think I got that the wrong way round. It was supposed to match this part of modem_run (which says endpoint in the comment but appears to mean altsetting): int ep = 1; /* Revision 0 modems require the use of the endpoint 2 */ if (revision == 0) ep = 2; if (pusb_set_interface(fdusb, 1, ep) < 0) { report(0, REPORT_ERROR, "pusb_set_interface"); return(-1); } -- dwmw2 ------------------------------------------------------- This SF.net email is sponsored by: IT Product Guide on ITManagersJournal Use IT products in your business? Tell us what you think of them. Give us Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more http://productguide.itmanagersjournal.com/guidepromo.tmpl _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel