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

Reply via email to