Hi,
On Tue, Jul 23, 2013 at 10:06:15AM -0400, Alan Stern wrote:
> > > > @@ -148,6 +148,7 @@ static int __dwc3_gadget_ep0_queue(struct dwc3_ep *dep,
> > > >
> > > > direction = !dwc->ep0_expect_in;
> > > > dwc->delayed_status = false;
> > > > + usb_gadget_set_state(&dwc->gadget, USB_STATE_CONFIGURED);
> > >
> > > Isn't this overkill? Do you really want to call usb_gadget_set_state()
> > > every time the gadget driver queues a transfer on ep0?
> > >
> > > Or am I missing an important part of the context?
> >
> > heh, you're missing context, that will only be called when we had
> > delayed status flag set:
> >
> > | static int __dwc3_gadget_ep0_queue(struct dwc3_ep *dep,
> > | struct dwc3_request *req)
> > | {
> >
> > [ ... ]
> >
> > | /*
> > | * In case gadget driver asked us to delay the STATUS phase,
> > | * handle it here.
> > | */
> > | if (dwc->delayed_status) {
> > | unsigned direction;
> > |
> > | direction = !dwc->ep0_expect_in;
> > | dwc->delayed_status = false;
> > | usb_gadget_set_state(&dwc->gadget, USB_STATE_CONFIGURED);
>
> I see. Doesn't the mass-storage gadget also use delayed status when
> going into the _un_configured state?
no it doesn't, we bail out early if config number is zero, look at
composite.c and you'll see in case of configuration zero, we just make
sure to disable all endpoints and don't call ->set_alt():
| static void reset_config(struct usb_composite_dev *cdev)
| {
| struct usb_function *f;
|
| DBG(cdev, "reset config\n");
|
| list_for_each_entry(f, &cdev->config->functions, list) {
| if (f->disable)
| f->disable(f);
|
| bitmap_zero(f->endpoints, 32);
| }
| cdev->config = NULL;
| }
|
| static int set_config(struct usb_composite_dev *cdev,
| const struct usb_ctrlrequest *ctrl, unsigned number)
| {
| struct usb_gadget *gadget = cdev->gadget;
| struct usb_configuration *c = NULL;
| int result = -EINVAL;
| unsigned power = gadget_is_otg(gadget) ? 8 : 100;
| int tmp;
|
| if (number) {
| list_for_each_entry(c, &cdev->configs, list) {
| if (c->bConfigurationValue == number) {
| /*
| * We disable the FDs of the previous
| * configuration only if the new configuration
| * is a valid one
| */
| if (cdev->config)
| reset_config(cdev);
| result = 0;
| break;
| }
| }
| if (result < 0)
| goto done;
| } else { /* Zero configuration value - need to reset the config */
| if (cdev->config)
| reset_config(cdev);
| result = 0;
| }
|
| INFO(cdev, "%s config #%d: %s\n",
| usb_speed_string(gadget->speed),
| number, c ? c->label : "unconfigured");
|
| if (!c)
| goto done;
[ ... ]
| done:
| usb_gadget_vbus_draw(gadget, power);
| if (result >= 0 && cdev->delayed_status)
| result = USB_GADGET_DELAYED_STATUS;
| return result;
| }
|
| int
| composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
| {
| struct usb_composite_dev *cdev = get_gadget_data(gadget);
| struct usb_request *req = cdev->req;
| int value = -EOPNOTSUPP;
| int status = 0;
| u16 w_index = le16_to_cpu(ctrl->wIndex);
| u8 intf = w_index & 0xFF;
| u16 w_value = le16_to_cpu(ctrl->wValue);
| u16 w_length = le16_to_cpu(ctrl->wLength);
| struct usb_function *f = NULL;
| u8 endp;
[ ... ]
| switch (ctrl->bRequest) {
[ ... ]
| /* any number of configs can work */
| case USB_REQ_SET_CONFIGURATION:
| if (ctrl->bRequestType != 0)
| goto unknown;
| if (gadget_is_otg(gadget)) {
| if (gadget->a_hnp_support)
| DBG(cdev, "HNP available\n");
| else if (gadget->a_alt_hnp_support)
| DBG(cdev, "HNP on another port\n");
| else
| VDBG(cdev, "HNP inactive\n");
| }
| spin_lock(&cdev->lock);
| value = set_config(cdev, ctrl, w_value);
| spin_unlock(&cdev->lock);
| break;
[ ...]
| }
|
| /* respond with data transfer before status phase? */
| if (value >= 0 && value != USB_GADGET_DELAYED_STATUS) {
| req->length = value;
| req->zero = value < w_length;
| value = usb_ep_queue(gadget->ep0, req, GFP_ATOMIC);
| if (value < 0) {
| DBG(cdev, "ep_queue --> %d\n", value);
| req->status = 0;
| composite_setup_complete(gadget->ep0, req);
| }
| } else if (value == USB_GADGET_DELAYED_STATUS && w_length != 0) {
| WARN(cdev,
| "%s: Delayed status not supported for w_length != 0",
| __func__);
| }
|
| done:
| /* device either stalls (value < 0) or reports success */
| return value;
| }
--
balbi
Attachment:
signature.asc
Description: Digital signature
