On Wed, 15 Aug 2012, Chen Peter-B29397 wrote:

> Hi Alan,
> 
> One of my colleagues reports a problem that the enumeration will fail if
> the storage has some problems. I can re-produce this problem if I add a
> big delay (like 500ms) at handle_exception, at condition FSG_STATE_DISCONNECT,
> please see below code: 
> 
> 3050         case FSG_STATE_DISCONNECT:
> 3051                 for (i = 0; i < fsg->nluns; ++i)
> 3052                         fsg_lun_fsync_sub(fsg->luns + i);
> 3053                 mdelay(500);
> 3054                 do_set_config(fsg, 0);          // Unconfigured state
> 3055                 break;
> 
> When the problem occurs, the call sequence like below:
> 
> reset interrupt-> fsg->disconnect-> handle_exception
> reset interrupt-> fsg->disconnect
> ...
> udc setup package -> USB_REQ_SET_CONFIGURATION -> raise_exception(fsg, 
> FSG_STATE_CONFIG_CHANGE);
> 
> The reason why this problem occurs is that the first 
> fsg_lun_fsync_sub(fsg->luns + i) consumes too
> much times, the fsg->state is changed by raise_exception (2nd reset 
> interrupt), but the handle_exception
> (2nd reset interrupt) is not called after raise_exception(fsg, 
> FSG_STATE_CONFIG_CHANGE) is called.
> The fsg->state is still FSG_STATE_DISCONNECT when raise_exception(fsg, 
> FSG_STATE_CONFIG_CHANGE) is called,
> Since FSG_STATE_DISCONNECT > FSG_STATE_CONFIG_CHANGE, the SET_CONFIGURATION 
> will never be handled,
> the enumeration will be failed.

Yes, I see.  But that's the right thing to do, isn't it?  If the gadget
is so busy writing data to the backing storage that it can't carry out
a Set-Config request, then the configuration attempt _should_ fail.

> A workaround for this problem is:
> 
> 3050         case FSG_STATE_DISCONNECT:
> 3051                 if (fsg->config != 0)
> 3052                         for (i = 0; i < fsg->nluns; ++i)
> 3053                                 fsg_lun_fsync_sub(fsg->luns + i);
> 3054                 do_set_config(fsg, 0);          // Unconfigured state
> 3055                 break;
> 
> But it can't handle the problem that the reset occurs during the transfer due 
> to some timeout.
> (bus reset -> bus reset -> SET_CONFIGURATION)

Indeed.  Well, if the gadget is too slow to keep up with the host,
eventually it will stop working no matter what the driver does.  
However note that the second time it gets called, fsg_lun_fsync_sub()  
should run very quickly because all the dirty pages will already have
been written out.


A better solution would be to have separate gadget driver callbacks for
Reset and Disconnect.  Then we could do the fsg_lun_fsync_sub() only
during disconnect, not during reset.

Alternatively, we could keep the single existing Disconnect callback
and give the gadget driver a way to ask whether or not VBus power is
present.  Then we could do the fsg_lun_fsync_sub() only when there is
no power.

Either one of these would be a major change to the Gadget API, but they
seem like reasonable things to add.

Felipe, what do you think?

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to