From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Vasut Date: Sun, 13 Dec 2015 05:46:42 +0100 Subject: [U-Boot] [PATCH 2/6] usb: dwc2: Handle NAK during CONTROL DATA and STATUS stage In-Reply-To: <1449980278-19881-3-git-send-email-stefan.bruens@rwth-aachen.de> References: <1449980278-19881-1-git-send-email-stefan.bruens@rwth-aachen.de> <1449980278-19881-3-git-send-email-stefan.bruens@rwth-aachen.de> Message-ID: <201512130546.42529.marex@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Sunday, December 13, 2015 at 05:17:54 AM, 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. > > Signed-off-by: Stefan Br?ns > --- > drivers/usb/host/dwc2.c | 37 ++++++++++++++++++++++++------------- > 1 file changed, 24 insertions(+), 13 deletions(-) > > diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c > index 5ef6deb..db3acd4 100644 > --- a/drivers/usb/host/dwc2.c > +++ b/drivers/usb/host/dwc2.c > @@ -892,8 +892,8 @@ static int _submit_control_msg(struct dwc2_priv *priv, > struct usb_device *dev, { > int devnum = usb_pipedevice(pipe); > int pid, ret, act_len; > - /* For CONTROL endpoint pid should start with DATA1 */ > int status_direction; > + unsigned long timeout; > > if (devnum == priv->root_hub_devnum) { > dev->status = 0; > @@ -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 */ > pid = DWC2_HC_PID_DATA1; > - ret = chunk_msg(priv, dev, pipe, &pid, usb_pipein(pipe), buffer, > - len, false); > + act_len = 0; > + 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; > + } while (len && (ret == -EAGAIN)); I can't say I'm a big fan of these conditions ^ . Can you rework a bit so it's more obvious what the code does ? The condition in the while (cond) is really inobvious at a first glance. > if (ret) > return ret; > - act_len = dev->act_len; > - } /* End of DATA stage */ > - else > + status_direction = usb_pipeout(pipe); > + } else { > + /* No-data CONTROL always ends with an IN transaction */ > + status_direction = 1; > act_len = 0; > + } Otherwise: Acked-by: Marek Vasut