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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
