On 12/12/2015 09:17 PM, Stefan Brüns wrote:
A function is allowed to return NAKs during the DATA stage to control data flow control. NAKs during the STATUS stage signal the function is still processing the request.
For my own education, do you have a link to the part of the spec that states that? I'd naively expect the control stage to give a NAK, but once a control transaction was accepted, the function would have to deal with it without NAKs? Still, I don't think this change would cause any issue either way.
diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c
@@ -907,26 +907,37 @@ static int _submit_control_msg(struct dwc2_priv *priv, struct usb_device *dev, if (ret) return ret;
- timeout = get_timer(0) + USB_TIMEOUT_MS(pipe); if (buffer) {
/* DATA stage */
I'd suggest putting that new comment right before the "timeout = ..." line, since that's the start of DATA stage processing.
If you're adding comments for the stages, perhaps add one at the start of the CONTROL stage too?
pid = DWC2_HC_PID_DATA1;
ret = chunk_msg(priv, dev, pipe, &pid, usb_pipein(pipe), buffer,len, false);
act_len = 0;
I don't think you need that assignment because...
do {if (get_timer(0) > timeout) {printf("Timeout during CONTROL DATA stage\n");return -ETIMEDOUT;}ret = chunk_msg(priv, dev, pipe, &pid, usb_pipein(pipe),buffer, len, false);act_len += dev->act_len;buffer += dev->act_len;len -= dev->act_len;
Shouldn't those all be = not += or -=-, just like in the original code? There's no chunking loop here, so the entire length either happens in one go or not at all.
pid = DWC2_HC_PID_DATA1;
- ret = chunk_msg(priv, dev, pipe, &pid, status_direction,
priv->status_buffer, 0, false);
- do {
ret = chunk_msg(priv, dev, pipe, &pid, status_direction,priv->status_buffer, 0, false);- } while (ret == -EAGAIN); if (ret) return ret;
Shouldn't that last loop have a timeout too?