[U-Boot] [PATCH 1/5] usb: dwc2: unify waiting for transfer completion

Lift common code out of submit_bulk_msg() and submit_control_msg().
Signed-off-by: Stephen Warren swarren@wwwdotorg.org --- drivers/usb/host/dwc2.c | 167 +++++++++++++++++------------------------------- 1 file changed, 60 insertions(+), 107 deletions(-)
diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index e8142ac0922f..62111a754a1b 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -704,6 +704,36 @@ static int dwc_otg_submit_rh_msg(struct usb_device *dev, unsigned long pipe, return stat; }
+int wait_for_chhltd(uint32_t *sub, int *toggle) +{ + const uint32_t hcint_comp_hlt_ack = DWC2_HCINT_XFERCOMP | + DWC2_HCINT_CHHLTD | DWC2_HCINT_ACK; + struct dwc2_hc_regs *hc_regs = ®s->hc_regs[DWC2_HC_CHANNEL]; + int ret; + uint32_t hcint, hctsiz; + + ret = wait_for_bit(&hc_regs->hcint, DWC2_HCINT_CHHLTD, true); + if (ret) + return ret; + + hcint = readl(&hc_regs->hcint); + if (hcint != hcint_comp_hlt_ack) { + debug("%s: Error (HCINT=%08x)\n", __func__, hcint); + return -EINVAL; + } + + hctsiz = readl(&hc_regs->hctsiz); + *sub = (hctsiz & DWC2_HCTSIZ_XFERSIZE_MASK) >> + DWC2_HCTSIZ_XFERSIZE_OFFSET; + if (toggle) + *toggle = (hctsiz & DWC2_HCTSIZ_PID_MASK) >> + DWC2_HCTSIZ_PID_OFFSET; + + debug("%s: sub=%u toggle=%d\n", __func__, *sub, toggle ? *toggle : -1); + + return 0; +} + /* U-Boot USB transmission interface */ int submit_bulk_msg(struct usb_device *dev, unsigned long pipe, void *buffer, int len) @@ -712,13 +742,12 @@ int submit_bulk_msg(struct usb_device *dev, unsigned long pipe, void *buffer, int ep = usb_pipeendpoint(pipe); int max = usb_maxpacket(dev, pipe); int done = 0; - uint32_t hctsiz, sub, tmp; + int ret; + uint32_t sub; struct dwc2_hc_regs *hc_regs = ®s->hc_regs[DWC2_HC_CHANNEL]; - uint32_t hcint; uint32_t xfer_len; uint32_t num_packets; int stop_transfer = 0; - unsigned int timeout = 1000000;
if (devnum == root_hub_devnum) { dev->status = 0; @@ -771,50 +800,17 @@ int submit_bulk_msg(struct usb_device *dev, unsigned long pipe, void *buffer, (1 << DWC2_HCCHAR_MULTICNT_OFFSET) | DWC2_HCCHAR_CHEN);
- while (1) { - hcint = readl(&hc_regs->hcint); - - if (!(hcint & DWC2_HCINT_CHHLTD)) - continue; - - if (hcint & DWC2_HCINT_XFERCOMP) { - hctsiz = readl(&hc_regs->hctsiz); - done += xfer_len; - - sub = hctsiz & DWC2_HCTSIZ_XFERSIZE_MASK; - sub >>= DWC2_HCTSIZ_XFERSIZE_OFFSET; - - if (usb_pipein(pipe)) { - done -= sub; - if (hctsiz & DWC2_HCTSIZ_XFERSIZE_MASK) - stop_transfer = 1; - } - - tmp = hctsiz & DWC2_HCTSIZ_PID_MASK; - tmp >>= DWC2_HCTSIZ_PID_OFFSET; - if (tmp == DWC2_HC_PID_DATA1) { - bulk_data_toggle[devnum][ep] = - DWC2_HC_PID_DATA1; - } else { - bulk_data_toggle[devnum][ep] = - DWC2_HC_PID_DATA0; - } - break; - } - - if (hcint & DWC2_HCINT_STALL) { - puts("DWC OTG: Channel halted\n"); - bulk_data_toggle[devnum][ep] = - DWC2_HC_PID_DATA0; + ret = wait_for_chhltd(&sub, &bulk_data_toggle[devnum][ep]); + if (ret) { + stop_transfer = 1; + break; + }
+ done += xfer_len; + if (usb_pipein(pipe)) { + done -= sub; + if (sub) stop_transfer = 1; - break; - } - - if (!--timeout) { - printf("%s: Timeout!\n", __func__); - break; - } } }
@@ -838,11 +834,8 @@ int submit_control_msg(struct usb_device *dev, unsigned long pipe, void *buffer, int devnum = usb_pipedevice(pipe); int ep = usb_pipeendpoint(pipe); int max = usb_maxpacket(dev, pipe); - uint32_t hctsiz = 0, sub, tmp, ret; - uint32_t hcint; - const uint32_t hcint_comp_hlt_ack = DWC2_HCINT_XFERCOMP | - DWC2_HCINT_CHHLTD | DWC2_HCINT_ACK; - unsigned int timeout = 1000000; + int ret; + uint32_t sub;
/* For CONTROL endpoint pid should start with DATA1 */ int status_direction; @@ -878,14 +871,8 @@ int submit_control_msg(struct usb_device *dev, unsigned long pipe, void *buffer, DWC2_HCCHAR_CHEN | DWC2_HCCHAR_CHDIS, (1 << DWC2_HCCHAR_MULTICNT_OFFSET) | DWC2_HCCHAR_CHEN);
- ret = wait_for_bit(&hc_regs->hcint, DWC2_HCINT_CHHLTD, 1); - if (ret) - printf("%s: Timeout!\n", __func__); - - hcint = readl(&hc_regs->hcint); - - if (!(hcint & DWC2_HCINT_CHHLTD) || !(hcint & DWC2_HCINT_XFERCOMP)) { - printf("%s: Error (HCINT=%08x)\n", __func__, hcint); + ret = wait_for_chhltd(&sub, NULL); + if (ret) { dev->status = 0; dev->act_len = 0; return -EINVAL; @@ -917,47 +904,16 @@ int submit_control_msg(struct usb_device *dev, unsigned long pipe, void *buffer, (1 << DWC2_HCCHAR_MULTICNT_OFFSET) | DWC2_HCCHAR_CHEN);
- while (1) { - hcint = readl(&hc_regs->hcint); - if (!(hcint & DWC2_HCINT_CHHLTD)) - continue; - - if (hcint & DWC2_HCINT_XFERCOMP) { - hctsiz = readl(&hc_regs->hctsiz); - done = len; - - sub = hctsiz & DWC2_HCTSIZ_XFERSIZE_MASK; - sub >>= DWC2_HCTSIZ_XFERSIZE_OFFSET; - - if (usb_pipein(pipe)) - done -= sub; - } - - if (hcint & DWC2_HCINT_ACK) { - tmp = hctsiz & DWC2_HCTSIZ_PID_MASK; - tmp >>= DWC2_HCTSIZ_PID_OFFSET; - if (tmp == DWC2_HC_PID_DATA0) { - control_data_toggle[devnum][ep] = - DWC2_HC_PID_DATA0; - } else { - control_data_toggle[devnum][ep] = - DWC2_HC_PID_DATA1; - } - } - - if (hcint != hcint_comp_hlt_ack) { - printf("%s: Error (HCINT=%08x)\n", - __func__, hcint); - goto out; - } - - if (!--timeout) { - printf("%s: Timeout!\n", __func__); - goto out; - } - - break; + ret = wait_for_chhltd(&sub, &control_data_toggle[devnum][ep]); + if (ret) { + dev->status = 0; + dev->act_len = 0; + return -EINVAL; } + + done = len; + if (usb_pipein(pipe)) + done -= sub; } /* End of DATA stage */
/* STATUS stage */ @@ -980,20 +936,17 @@ int submit_control_msg(struct usb_device *dev, unsigned long pipe, void *buffer, DWC2_HCCHAR_CHEN | DWC2_HCCHAR_CHDIS, (1 << DWC2_HCCHAR_MULTICNT_OFFSET) | DWC2_HCCHAR_CHEN);
- while (1) { - hcint = readl(&hc_regs->hcint); - if (hcint & DWC2_HCINT_CHHLTD) - break; + ret = wait_for_chhltd(&sub, NULL); + if (ret) { + dev->status = 0; + dev->act_len = 0; + return -EINVAL; }
- if (hcint != hcint_comp_hlt_ack) - printf("%s: Error (HCINT=%08x)\n", __func__, hcint); - -out: dev->act_len = done; dev->status = 0;
- return done; + return 0; }
int submit_int_msg(struct usb_device *dev, unsigned long pipe, void *buffer,

Move the body of submit_bulk_msg() into new function chunk_msg(). This can be shared with submit_control_msg() to reduce code duplication, and allow control messages larger than maxpacket.
Signed-off-by: Stephen Warren swarren@wwwdotorg.org --- drivers/usb/host/dwc2.c | 55 +++++++++++++++++++++++++++++++++++-------------- 1 file changed, 39 insertions(+), 16 deletions(-)
diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index 62111a754a1b..fbe99304aafc 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -734,25 +734,30 @@ int wait_for_chhltd(uint32_t *sub, int *toggle) return 0; }
-/* U-Boot USB transmission interface */ -int submit_bulk_msg(struct usb_device *dev, unsigned long pipe, void *buffer, - int len) +static int dwc2_eptype[] = { + DWC2_HCCHAR_EPTYPE_ISOC, + DWC2_HCCHAR_EPTYPE_INTR, + DWC2_HCCHAR_EPTYPE_CONTROL, + DWC2_HCCHAR_EPTYPE_BULK, +}; + +int chunk_msg(struct usb_device *dev, unsigned long pipe, int *pid, int in, + void *buffer, int len) { + struct dwc2_hc_regs *hc_regs = ®s->hc_regs[DWC2_HC_CHANNEL]; int devnum = usb_pipedevice(pipe); int ep = usb_pipeendpoint(pipe); int max = usb_maxpacket(dev, pipe); + int eptype = dwc2_eptype[usb_pipetype(pipe)]; int done = 0; int ret; uint32_t sub; - struct dwc2_hc_regs *hc_regs = ®s->hc_regs[DWC2_HC_CHANNEL]; uint32_t xfer_len; uint32_t num_packets; int stop_transfer = 0;
- if (devnum == root_hub_devnum) { - dev->status = 0; - return -EINVAL; - } + debug("%s: msg: pipe %lx pid %d in %d len %d\n", __func__, pipe, *pid, + in, len);
if (len > DWC2_DATA_BUF_SIZE) { printf("%s: %d is more then available buffer size (%d)\n", @@ -764,8 +769,8 @@ int submit_bulk_msg(struct usb_device *dev, unsigned long pipe, void *buffer,
while ((done < len) && !stop_transfer) { /* Initialize channel */ - dwc_otg_hc_init(regs, DWC2_HC_CHANNEL, devnum, ep, - usb_pipein(pipe), DWC2_HCCHAR_EPTYPE_BULK, max); + dwc_otg_hc_init(regs, DWC2_HC_CHANNEL, devnum, ep, in, eptype, + max);
xfer_len = len - done; /* Make sure that xfer_len is a multiple of max packet size. */ @@ -782,13 +787,15 @@ int submit_bulk_msg(struct usb_device *dev, unsigned long pipe, void *buffer, num_packets = 1; }
- if (usb_pipein(pipe)) + if (in) xfer_len = num_packets * max;
+ debug("%s: chunk: pid %d xfer_len %u pkts %u\n", __func__, + *pid, xfer_len, num_packets); + writel((xfer_len << DWC2_HCTSIZ_XFERSIZE_OFFSET) | (num_packets << DWC2_HCTSIZ_PKTCNT_OFFSET) | - (bulk_data_toggle[devnum][ep] << - DWC2_HCTSIZ_PID_OFFSET), + (*pid << DWC2_HCTSIZ_PID_OFFSET), &hc_regs->hctsiz);
memcpy(aligned_buffer, (char *)buffer + done, len - done); @@ -800,21 +807,21 @@ int submit_bulk_msg(struct usb_device *dev, unsigned long pipe, void *buffer, (1 << DWC2_HCCHAR_MULTICNT_OFFSET) | DWC2_HCCHAR_CHEN);
- ret = wait_for_chhltd(&sub, &bulk_data_toggle[devnum][ep]); + ret = wait_for_chhltd(&sub, pid); if (ret) { stop_transfer = 1; break; }
done += xfer_len; - if (usb_pipein(pipe)) { + if (in) { done -= sub; if (sub) stop_transfer = 1; } }
- if (done && usb_pipein(pipe)) + if (done && in) memcpy(buffer, aligned_buffer, done);
writel(0, &hc_regs->hcintmsk); @@ -826,6 +833,22 @@ int submit_bulk_msg(struct usb_device *dev, unsigned long pipe, void *buffer, return 0; }
+/* U-Boot USB transmission interface */ +int submit_bulk_msg(struct usb_device *dev, unsigned long pipe, void *buffer, + int len) +{ + int devnum = usb_pipedevice(pipe); + int ep = usb_pipeendpoint(pipe); + + if (devnum == root_hub_devnum) { + dev->status = 0; + return -EINVAL; + } + + return chunk_msg(dev, pipe, &bulk_data_toggle[devnum][ep], + usb_pipein(pipe), buffer, len); +} + int submit_control_msg(struct usb_device *dev, unsigned long pipe, void *buffer, int len, struct devrequest *setup) {

This removes duplicated code.
Signed-off-by: Stephen Warren swarren@wwwdotorg.org --- drivers/usb/host/dwc2.c | 114 ++++++++---------------------------------------- 1 file changed, 19 insertions(+), 95 deletions(-)
diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index fbe99304aafc..189f6548f25f 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -767,7 +767,7 @@ int chunk_msg(struct usb_device *dev, unsigned long pipe, int *pid, int in, return -EINVAL; }
- while ((done < len) && !stop_transfer) { + do { /* Initialize channel */ dwc_otg_hc_init(regs, DWC2_HC_CHANNEL, devnum, ep, in, eptype, max); @@ -819,7 +819,7 @@ int chunk_msg(struct usb_device *dev, unsigned long pipe, int *pid, int in, if (sub) stop_transfer = 1; } - } + } while ((done < len) && !stop_transfer);
if (done && in) memcpy(buffer, aligned_buffer, done); @@ -852,14 +852,9 @@ int submit_bulk_msg(struct usb_device *dev, unsigned long pipe, void *buffer, int submit_control_msg(struct usb_device *dev, unsigned long pipe, void *buffer, int len, struct devrequest *setup) { - struct dwc2_hc_regs *hc_regs = ®s->hc_regs[DWC2_HC_CHANNEL]; - int done = 0; int devnum = usb_pipedevice(pipe); int ep = usb_pipeendpoint(pipe); - int max = usb_maxpacket(dev, pipe); - int ret; - uint32_t sub; - + int pid, ret, act_len; /* For CONTROL endpoint pid should start with DATA1 */ int status_direction;
@@ -869,75 +864,21 @@ int submit_control_msg(struct usb_device *dev, unsigned long pipe, void *buffer, return dwc_otg_submit_rh_msg(dev, pipe, buffer, len, setup); }
- if (len > DWC2_DATA_BUF_SIZE) { - printf("%s: %d is more then available buffer size(%d)\n", - __func__, len, DWC2_DATA_BUF_SIZE); - dev->status = 0; - dev->act_len = 0; - return -EINVAL; - } - - /* Initialize channel, OUT for setup buffer */ - dwc_otg_hc_init(regs, DWC2_HC_CHANNEL, devnum, ep, 0, - DWC2_HCCHAR_EPTYPE_CONTROL, max); - - /* SETUP stage */ - writel((8 << DWC2_HCTSIZ_XFERSIZE_OFFSET) | - (1 << DWC2_HCTSIZ_PKTCNT_OFFSET) | - (DWC2_HC_PID_SETUP << DWC2_HCTSIZ_PID_OFFSET), - &hc_regs->hctsiz); - - writel((uint32_t)setup, &hc_regs->hcdma); - - /* Set host channel enable after all other setup is complete. */ - clrsetbits_le32(&hc_regs->hcchar, DWC2_HCCHAR_MULTICNT_MASK | - DWC2_HCCHAR_CHEN | DWC2_HCCHAR_CHDIS, - (1 << DWC2_HCCHAR_MULTICNT_OFFSET) | DWC2_HCCHAR_CHEN); - - ret = wait_for_chhltd(&sub, NULL); - if (ret) { - dev->status = 0; - dev->act_len = 0; - return -EINVAL; - } - - /* Clear interrupts */ - writel(0, &hc_regs->hcintmsk); - writel(0xFFFFFFFF, &hc_regs->hcint); + pid = DWC2_HC_PID_SETUP; + ret = chunk_msg(dev, pipe, &pid, 0, setup, 8); + if (ret) + return ret;
if (buffer) { - /* DATA stage */ - dwc_otg_hc_init(regs, DWC2_HC_CHANNEL, devnum, ep, - usb_pipein(pipe), - DWC2_HCCHAR_EPTYPE_CONTROL, max); - - /* TODO: check if len < 64 */ control_data_toggle[devnum][ep] = DWC2_HC_PID_DATA1; - writel((len << DWC2_HCTSIZ_XFERSIZE_OFFSET) | - (1 << DWC2_HCTSIZ_PKTCNT_OFFSET) | - (control_data_toggle[devnum][ep] << - DWC2_HCTSIZ_PID_OFFSET), - &hc_regs->hctsiz); - - writel((uint32_t)buffer, &hc_regs->hcdma); - - /* Set host channel enable after all other setup is complete */ - clrsetbits_le32(&hc_regs->hcchar, DWC2_HCCHAR_MULTICNT_MASK | - DWC2_HCCHAR_CHEN | DWC2_HCCHAR_CHDIS, - (1 << DWC2_HCCHAR_MULTICNT_OFFSET) | - DWC2_HCCHAR_CHEN); - - ret = wait_for_chhltd(&sub, &control_data_toggle[devnum][ep]); - if (ret) { - dev->status = 0; - dev->act_len = 0; - return -EINVAL; - } - - done = len; - if (usb_pipein(pipe)) - done -= sub; + ret = chunk_msg(dev, pipe, &control_data_toggle[devnum][ep], + usb_pipein(pipe), buffer, len); + if (ret) + return ret; + act_len = dev->act_len; } /* End of DATA stage */ + else + act_len = 0;
/* STATUS stage */ if ((len == 0) || usb_pipeout(pipe)) @@ -945,29 +886,12 @@ int submit_control_msg(struct usb_device *dev, unsigned long pipe, void *buffer, else status_direction = 0;
- dwc_otg_hc_init(regs, DWC2_HC_CHANNEL, devnum, ep, - status_direction, DWC2_HCCHAR_EPTYPE_CONTROL, max); - - writel((1 << DWC2_HCTSIZ_PKTCNT_OFFSET) | - (DWC2_HC_PID_DATA1 << DWC2_HCTSIZ_PID_OFFSET), - &hc_regs->hctsiz); - - writel((uint32_t)status_buffer, &hc_regs->hcdma); - - /* Set host channel enable after all other setup is complete. */ - clrsetbits_le32(&hc_regs->hcchar, DWC2_HCCHAR_MULTICNT_MASK | - DWC2_HCCHAR_CHEN | DWC2_HCCHAR_CHDIS, - (1 << DWC2_HCCHAR_MULTICNT_OFFSET) | DWC2_HCCHAR_CHEN); - - ret = wait_for_chhltd(&sub, NULL); - if (ret) { - dev->status = 0; - dev->act_len = 0; - return -EINVAL; - } + pid = DWC2_HC_PID_DATA1; + ret = chunk_msg(dev, pipe, &pid, status_direction, status_buffer, 0); + if (ret) + return ret;
- dev->act_len = done; - dev->status = 0; + dev->act_len = act_len;
return 0; }

The control data toggle resets to DATA1 at the start of the data phase of every setup transaction. We don't need a global variable to store the value; we can just store it on the stack.
Signed-off-by: Stephen Warren swarren@wwwdotorg.org --- drivers/usb/host/dwc2.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index 189f6548f25f..e8a7e713c87f 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -27,7 +27,6 @@ DEFINE_ALIGN_BUFFER(uint8_t, status_buffer, DWC2_STATUS_BUF_SIZE, 8); #define MAX_DEVICE 16 #define MAX_ENDPOINT 16 static int bulk_data_toggle[MAX_DEVICE][MAX_ENDPOINT]; -static int control_data_toggle[MAX_DEVICE][MAX_ENDPOINT];
static int root_hub_devnum;
@@ -853,7 +852,6 @@ int submit_control_msg(struct usb_device *dev, unsigned long pipe, void *buffer, int len, struct devrequest *setup) { int devnum = usb_pipedevice(pipe); - int ep = usb_pipeendpoint(pipe); int pid, ret, act_len; /* For CONTROL endpoint pid should start with DATA1 */ int status_direction; @@ -870,9 +868,9 @@ int submit_control_msg(struct usb_device *dev, unsigned long pipe, void *buffer, return ret;
if (buffer) { - control_data_toggle[devnum][ep] = DWC2_HC_PID_DATA1; - ret = chunk_msg(dev, pipe, &control_data_toggle[devnum][ep], - usb_pipein(pipe), buffer, len); + pid = DWC2_HC_PID_DATA1; + ret = chunk_msg(dev, pipe, &pid, usb_pipein(pipe), buffer, + len); if (ret) return ret; act_len = dev->act_len; @@ -933,10 +931,8 @@ int usb_lowlevel_init(int index, enum usb_init_type init, void **controller) DWC2_HPRT0_PRTRST);
for (i = 0; i < MAX_DEVICE; i++) { - for (j = 0; j < MAX_ENDPOINT; j++) { - control_data_toggle[i][j] = DWC2_HC_PID_DATA1; + for (j = 0; j < MAX_ENDPOINT; j++) bulk_data_toggle[i][j] = DWC2_HC_PID_DATA0; - } }
return 0;

toggle is never NULL. Simplify the code by removing handling of when it is NULL.
Signed-off-by: Stephen Warren swarren@wwwdotorg.org --- drivers/usb/host/dwc2.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index e8a7e713c87f..5a1c44a8fb75 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -724,11 +724,9 @@ int wait_for_chhltd(uint32_t *sub, int *toggle) hctsiz = readl(&hc_regs->hctsiz); *sub = (hctsiz & DWC2_HCTSIZ_XFERSIZE_MASK) >> DWC2_HCTSIZ_XFERSIZE_OFFSET; - if (toggle) - *toggle = (hctsiz & DWC2_HCTSIZ_PID_MASK) >> - DWC2_HCTSIZ_PID_OFFSET; + *toggle = (hctsiz & DWC2_HCTSIZ_PID_MASK) >> DWC2_HCTSIZ_PID_OFFSET;
- debug("%s: sub=%u toggle=%d\n", __func__, *sub, toggle ? *toggle : -1); + debug("%s: sub=%u toggle=%d\n", __func__, *sub, *toggle);
return 0; }

On Sunday, March 08, 2015 at 06:48:51 AM, Stephen Warren wrote:
Lift common code out of submit_bulk_msg() and submit_control_msg().
Signed-off-by: Stephen Warren swarren@wwwdotorg.org
NICE!
Best regards, Marek Vasut

On Sunday, March 08, 2015 at 06:48:51 AM, Stephen Warren wrote:
Lift common code out of submit_bulk_msg() and submit_control_msg().
Signed-off-by: Stephen Warren swarren@wwwdotorg.org
Applied to u-boot-usb/topic/dwc2, thanks. I would like to see more testing from others if possible, otherwise this will go in after 2015.04 if you're fine with that.
Best regards, Marek Vasut
participants (2)
-
Marek Vasut
-
Stephen Warren