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?
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
