On Tue, 27 Jul 2004, Diego Dompe wrote:

> The driver registration logic was producing the kernel to get NULL
> pointer references.

See below.

> >     You moved the code that registers the LUN devices into
> >     fsg_bind().  This change looks correct; although there is
> >     a comment stating that the gadget doesn't get registered
> >     until after fsg_bind() returns, that comment is wrong.
> 
> My guest is that the code may have race conditions if the registration
> is done on the init function instead of fsg_bind. We probably have a
> hardware configuration that was the first on trigger the race condition.
> 
> This change avoid the race condition option, and also makes the code a
> bit smaller.

You're right.  If the gadget is plugged into a host when the driver is
registered, then there is a race between fsg_init() and other routines
that might try to use the LUNs.

> >     You added this line:
> >             gadget->dev.driver = &fsg_driver.driver;
> >     That is both wrong and unnecessary; it's the controller
> >     driver's responsibility to change gadget->dev, not the
> >     gadget driver's responsibility.  The gadget driver is not
> >     supposed to touch anything inside the struct usb_gadget.
> 
> I don't know quite good who is suppose to touch what yet...
> 
> If you remove that line and try to load the g_file_storage without
> parameters it give us:
[...]
> Which in better human language means that the ERROR macro is expanded to
> xprintk macro which turns into:
> dev_printk(level, %(f)->gadget->dev, fmt, ## args)
> 
> and since fsg->gadget->dev is not defined (yet?) this produces the NULL
> pointer.
> 
> Did you have other idea about how to solve this? When is the driver
> struct registered in the device struct? Could be another race condition
> trigger by our hardware?

That's a problem in the controller driver.  Which driver are you using?  
It should include something like this in its usb_gadget_register_driver() 
routine (this is copied from net2280.c):

        dev->gadget.dev.driver = &driver->driver;
        retval = driver->bind (&dev->gadget);

The first line sets the gadget controller's driver to the fsg driver
immediately before calling fsg_bind().

> We use the g_file_storage primary for several card readers, however we
> also like to preserve the small footprint. That's the reason we make
> that changes.

Okay.  Adding the write-protect and media change parts won't increase the 
memory size very much.


What do you think about the problem of stalling the bulk-out endpoint when 
the host sends too much data?  Maybe this hasn't caused you any 
difficulty yet.  The problem is that there's a race.  If we call 
fsg_set_halt() after the host has finished sending all its data, then the 
host won't see the stalled endpoint and so it won't clear the halt.  My 
feeling is that fsg should always use the "no-stall" option for data-out 
transfers, how do you feel?

Alan Stern



-------------------------------------------------------
This SF.Net email is sponsored by BEA Weblogic Workshop
FREE Java Enterprise J2EE developer tools!
Get your free copy of BEA WebLogic Workshop 8.1 today.
http://ads.osdn.com/?ad_id=4721&alloc_id=10040&op=click
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to