| Message ID | 1413565804-13061-19-git-send-email-balbi@ti.com |
|---|---|
| State | New |
| Headers | show |
From: Felipe Balbi > According to Section 8.5.3.2 of the USB 2.0 specification, > a USB device must terminate a Data Phase with either a > short packet or a ZLP (if the previous transfer was > a multiple of wMaxPacketSize). > > For reference, here's what the USB 2.0 specification, section > 8.5.3.2 says: > > " > 8.5.3.2 Variable-length Data Stage > > A control pipe may have a variable-length data phase > in which the host requests more data than is contained > in the specified data structure. When all of the data > structure is returned to the host, the function should > indicate that the Data stage is ended by returning a > packet that is shorter than the MaxPacketSize for the > pipe. If the data structure is an exact multiple of > wMaxPacketSize for the pipe, the function will return > a zero-length packet to indicate the end of the Data > stage. > " If that is the same as my understanding of the USB3 spec then the requirement for a ZLP isn't unconditional. In particular if the data phase isn't variable length then a ZLP must not be added. David > Signed-off-by: Felipe Balbi <balbi@ti.com> > --- > drivers/usb/dwc3/ep0.c | 18 ++++++++++++------ > 1 file changed, 12 insertions(+), 6 deletions(-) > > diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c > index a47cc1e..0a34e71 100644 > --- a/drivers/usb/dwc3/ep0.c > +++ b/drivers/usb/dwc3/ep0.c > @@ -828,12 +828,18 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc, > > dwc3_ep0_stall_and_restart(dwc); > } else { > - /* > - * handle the case where we have to send a zero packet. This > - * seems to be case when req.length > maxpacket. Could it be? > - */ > - if (r) > - dwc3_gadget_giveback(ep0, r, 0); > + dwc3_gadget_giveback(ep0, r, 0); > + > + if (IS_ALIGNED(ur->length, ep0->endpoint.maxpacket)) { > + int ret; > + > + dwc->ep0_next_event = DWC3_EP0_COMPLETE; > + > + ret = dwc3_ep0_start_trans(dwc, epnum, > + dwc->ctrl_req_addr, 0, > + DWC3_TRBCTL_CONTROL_DATA); > + WARN_ON(ret < 0); > + } > } > } > > -- > 2.1.0.GIT > > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> From: linux-usb-owner@vger.kernel.org [mailto:linux-usb-owner@vger.kernel.org] On Behalf Of David Laight > Sent: Monday, October 20, 2014 2:48 AM > > From: Felipe Balbi > > According to Section 8.5.3.2 of the USB 2.0 specification, > > a USB device must terminate a Data Phase with either a > > short packet or a ZLP (if the previous transfer was > > a multiple of wMaxPacketSize). > > > > For reference, here's what the USB 2.0 specification, section > > 8.5.3.2 says: > > > > " > > 8.5.3.2 Variable-length Data Stage > > > > A control pipe may have a variable-length data phase > > in which the host requests more data than is contained > > in the specified data structure. When all of the data > > structure is returned to the host, the function should > > indicate that the Data stage is ended by returning a > > packet that is shorter than the MaxPacketSize for the > > pipe. If the data structure is an exact multiple of > > wMaxPacketSize for the pipe, the function will return > > a zero-length packet to indicate the end of the Data > > stage. > > " > > If that is the same as my understanding of the USB3 spec then the > requirement for a ZLP isn't unconditional. > > In particular if the data phase isn't variable length then a ZLP > must not be added. Also, in the USB 3.0 spec, the corresponding section has been modified a bit. The last sentence has been changed to this: "Note that if the amount of data in the data structure that is returned to the host *is less than the amount requested by the host* and is an exact multiple of maximum packet size then a control endpoint shall send a zero length DP to terminate the data stage." (emphasis mine) So I think you also need to take into account the wLength field of the request, and only send the ZLP if the amount of data returned is less than wLength.
On Mon, 20 Oct 2014, Paul Zimmerman wrote: > > From: linux-usb-owner@vger.kernel.org [mailto:linux-usb-owner@vger.kernel.org] On Behalf Of David Laight > > Sent: Monday, October 20, 2014 2:48 AM > > > > From: Felipe Balbi > > > According to Section 8.5.3.2 of the USB 2.0 specification, > > > a USB device must terminate a Data Phase with either a > > > short packet or a ZLP (if the previous transfer was > > > a multiple of wMaxPacketSize). > > > > > > For reference, here's what the USB 2.0 specification, section > > > 8.5.3.2 says: > > > > > > " > > > 8.5.3.2 Variable-length Data Stage > > > > > > A control pipe may have a variable-length data phase > > > in which the host requests more data than is contained > > > in the specified data structure. When all of the data > > > structure is returned to the host, the function should > > > indicate that the Data stage is ended by returning a > > > packet that is shorter than the MaxPacketSize for the > > > pipe. If the data structure is an exact multiple of > > > wMaxPacketSize for the pipe, the function will return > > > a zero-length packet to indicate the end of the Data > > > stage. > > > " > > > > If that is the same as my understanding of the USB3 spec then the > > requirement for a ZLP isn't unconditional. > > > > In particular if the data phase isn't variable length then a ZLP > > must not be added. > > Also, in the USB 3.0 spec, the corresponding section has been modified > a bit. The last sentence has been changed to this: > > "Note that if the amount of data in the data structure that is returned > to the host *is less than the amount requested by the host* and is an > exact multiple of maximum packet size then a control endpoint shall send > a zero length DP to terminate the data stage." (emphasis mine) > > So I think you also need to take into account the wLength field of the > request, and only send the ZLP if the amount of data returned is less > than wLength. That's right, and it's true for USB-2 as well. A ZLP is needed only in cases where the host otherwise wouldn't know the transfer is over, i.e., when the transfer length is a nonzero multiple of the maxpacket size and is smaller than wLength. It's not clear what a "variable length" control transfer is supposed to be. I guess it means that sometimes the device will send back less than wLength bytes of data. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Alan Stern > > > From: linux-usb-owner@vger.kernel.org [mailto:linux-usb-owner@vger.kernel.org] On Behalf Of David Laight > > > Sent: Monday, October 20, 2014 2:48 AM > > > > > > From: Felipe Balbi > > > > According to Section 8.5.3.2 of the USB 2.0 specification, > > > > a USB device must terminate a Data Phase with either a > > > > short packet or a ZLP (if the previous transfer was > > > > a multiple of wMaxPacketSize). > > > > > > > > For reference, here's what the USB 2.0 specification, section > > > > 8.5.3.2 says: > > > > > > > > " > > > > 8.5.3.2 Variable-length Data Stage > > > > > > > > A control pipe may have a variable-length data phase > > > > in which the host requests more data than is contained > > > > in the specified data structure. When all of the data > > > > structure is returned to the host, the function should > > > > indicate that the Data stage is ended by returning a > > > > packet that is shorter than the MaxPacketSize for the > > > > pipe. If the data structure is an exact multiple of > > > > wMaxPacketSize for the pipe, the function will return > > > > a zero-length packet to indicate the end of the Data > > > > stage. > > > > " > > > > > > If that is the same as my understanding of the USB3 spec then the > > > requirement for a ZLP isn't unconditional. > > > > > > In particular if the data phase isn't variable length then a ZLP > > > must not be added. > > > > Also, in the USB 3.0 spec, the corresponding section has been modified > > a bit. The last sentence has been changed to this: > > > > "Note that if the amount of data in the data structure that is returned > > to the host *is less than the amount requested by the host* and is an > > exact multiple of maximum packet size then a control endpoint shall send > > a zero length DP to terminate the data stage." (emphasis mine) > > > > So I think you also need to take into account the wLength field of the > > request, and only send the ZLP if the amount of data returned is less > > than wLength. > > That's right, and it's true for USB-2 as well. A ZLP is needed only in > cases where the host otherwise wouldn't know the transfer is over, > i.e., when the transfer length is a nonzero multiple of the maxpacket > size and is smaller than wLength. > > It's not clear what a "variable length" control transfer is supposed to > be. I guess it means that sometimes the device will send back less > than wLength bytes of data. I take it as being one where the amount of data returned isn't known by the host when it performs the 'read' request. So the response to a 'disk read' request is known and a ZLP isn't required (even though the transfer request is likely to be a multiple of the packet size). The length that matters is that of the buffer(s) supplied by the receiving driver. From the USB driver's point of view, only the 'application' (another driver) knows whether the next receive message is 'variable length', so the onus has to be on the 'application' sending the data to say whether it needs a ZLP. Has anyone fixed xhci to send ZLP yet ? David -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, > On Mon, 20 Oct 2014, Paul Zimmerman wrote: > > > > From: linux-usb-owner@vger.kernel.org [mailto:linux-usb- > owner@vger.kernel.org] On Behalf Of David Laight > > > Sent: Monday, October 20, 2014 2:48 AM > > > > > > From: Felipe Balbi > > > > According to Section 8.5.3.2 of the USB 2.0 specification, > > > > a USB device must terminate a Data Phase with either a > > > > short packet or a ZLP (if the previous transfer was > > > > a multiple of wMaxPacketSize). > > > > > > > > For reference, here's what the USB 2.0 specification, section > > > > 8.5.3.2 says: > > > > > > > > " > > > > 8.5.3.2 Variable-length Data Stage > > > > > > > > A control pipe may have a variable-length data phase > > > > in which the host requests more data than is contained > > > > in the specified data structure. When all of the data > > > > structure is returned to the host, the function should > > > > indicate that the Data stage is ended by returning a > > > > packet that is shorter than the MaxPacketSize for the > > > > pipe. If the data structure is an exact multiple of > > > > wMaxPacketSize for the pipe, the function will return > > > > a zero-length packet to indicate the end of the Data > > > > stage. > > > > " > > > > > > If that is the same as my understanding of the USB3 spec then the > > > requirement for a ZLP isn't unconditional. > > > > > > In particular if the data phase isn't variable length then a ZLP > > > must not be added. > > > > Also, in the USB 3.0 spec, the corresponding section has been > modified > > a bit. The last sentence has been changed to this: > > > > "Note that if the amount of data in the data structure that is > returned > > to the host *is less than the amount requested by the host* and is an > > exact multiple of maximum packet size then a control endpoint shall > send > > a zero length DP to terminate the data stage." (emphasis mine) > > > > So I think you also need to take into account the wLength field of > the > > request, and only send the ZLP if the amount of data returned is less > > than wLength. > > That's right, and it's true for USB-2 as well. A ZLP is needed only in > cases where the host otherwise wouldn't know the transfer is over, > i.e., when the transfer length is a nonzero multiple of the maxpacket > size and is smaller than wLength. Shall we use/check struct usb_request's zero flag for this? > > It's not clear what a "variable length" control transfer is supposed to > be. I guess it means that sometimes the device will send back less > than wLength bytes of data. > > Alan Stern > > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 22 Oct 2014, Anton Tikhomirov wrote: > > That's right, and it's true for USB-2 as well. A ZLP is needed only in > > cases where the host otherwise wouldn't know the transfer is over, > > i.e., when the transfer length is a nonzero multiple of the maxpacket > > size and is smaller than wLength. > > Shall we use/check struct usb_request's zero flag for this? Of course; we have to. There's no other way for the UDC driver to know whether the transfer is shorter than the host expects. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c index a47cc1e..0a34e71 100644 --- a/drivers/usb/dwc3/ep0.c +++ b/drivers/usb/dwc3/ep0.c @@ -828,12 +828,18 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc, dwc3_ep0_stall_and_restart(dwc); } else { - /* - * handle the case where we have to send a zero packet. This - * seems to be case when req.length > maxpacket. Could it be? - */ - if (r) - dwc3_gadget_giveback(ep0, r, 0); + dwc3_gadget_giveback(ep0, r, 0); + + if (IS_ALIGNED(ur->length, ep0->endpoint.maxpacket)) { + int ret; + + dwc->ep0_next_event = DWC3_EP0_COMPLETE; + + ret = dwc3_ep0_start_trans(dwc, epnum, + dwc->ctrl_req_addr, 0, + DWC3_TRBCTL_CONTROL_DATA); + WARN_ON(ret < 0); + } } }
According to Section 8.5.3.2 of the USB 2.0 specification, a USB device must terminate a Data Phase with either a short packet or a ZLP (if the previous transfer was a multiple of wMaxPacketSize). For reference, here's what the USB 2.0 specification, section 8.5.3.2 says: " 8.5.3.2 Variable-length Data Stage A control pipe may have a variable-length data phase in which the host requests more data than is contained in the specified data structure. When all of the data structure is returned to the host, the function should indicate that the Data stage is ended by returning a packet that is shorter than the MaxPacketSize for the pipe. If the data structure is an exact multiple of wMaxPacketSize for the pipe, the function will return a zero-length packet to indicate the end of the Data stage. " Signed-off-by: Felipe Balbi <balbi@ti.com> --- drivers/usb/dwc3/ep0.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)