Messages in this thread | | | Date | Thu, 21 Apr 2011 16:27:14 +0300 | From | Roger Quadros <> | Subject | Re: [PATCH 1/2] usb: gadget: composite: Allow function drivers to defer setup responses |
| |
On 04/19/2011 05:14 PM, ext Alan Stern wrote: > On Tue, 19 Apr 2011, Roger Quadros wrote: > >> +/** >> + * usb_composite_setup_continue() - Continue the delayed setup transfer > > You're should say "control transfer", not "setup transfer". Control > transfers consist of a setup stage, an optional data stage, and a > status stage. The setup stage has already ended by the time the > function's setup handler is called.
Yes I will re-word the things and re-send the patches.
> >> + * @cdev: the composite device who's setup transfer was delayed >> + * >> + * This function must be called by the USB function driver to continue >> + * with the setup transfer's data/status phase in case it had requested to > > Data and Status are stages, not phases. See section 8.5.3 of the > USB-2.0 spec. > > You made these mistakes in a few different places throughout the patch. > >> + * delay the status phase. A USB function's setup handler (e.g. set_alt()) >> + * can request the composite framework to delay the setup request's status phase >> + * by returning USB_GADGET_DELAYED_STATUS. >> + */ >> +void usb_composite_setup_continue(struct usb_composite_dev *cdev) >> +{ >> + int value; >> + struct usb_request *req = cdev->req; >> + unsigned long flags; >> + >> + DBG(cdev, "%s\n", __func__); >> + spin_lock_irqsave(&cdev->lock, flags); >> + >> + if (cdev->delayed_status == 0) { >> + WARN(cdev, "%s: Unexpected call\n", __func__); >> + >> + } else if (--cdev->delayed_status == 0) { >> + DBG(cdev, "%s: Completing delayed status\n", __func__); >> + req->length = 0; >> + req->zero = 1; > > There's no reason to set this flag. It has no effect when req->length > is 0. > OK.
Thanks for review.
-- regards, -roger
| |