[U-Boot] [PATCH V2 1/2] usb: dwc2: fix aligned buffer usage
The original aligned_buffer usage: a) Uselessly copied data into the aligned buffer even for IN transactions. Fix this my making the copy conditional. b) Always programmed the HW to transfer to/from the start of the aligned buffer. This worked fine for OUT transactions since the memcpy copied the OUT data to this location too. However, for large IN transactions, since the copy from the aligned buffer to the "client" buffer was deferred until after all chunks were transferred. it resulted in each chunk's transfer over-writing the data for the first transfer. Fix this by copying IN data as soon as it's received.
Signed-off-by: Stephen Warren swarren@wwwdotorg.org --- v2: Move all memcpy()s back into the per-chunk loop; we can't rely on maxpacket being a multiple of 8-bytes, so we need to copy each chunk to the aligned_buffer not aligned_buffer+done; the HW requires 8-byte alignment for the DMA buffer.
drivers/usb/host/dwc2.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index 5a1c44a8fb75..05d21b7948f5 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -795,7 +795,9 @@ int chunk_msg(struct usb_device *dev, unsigned long pipe, int *pid, int in, (*pid << DWC2_HCTSIZ_PID_OFFSET), &hc_regs->hctsiz);
- memcpy(aligned_buffer, (char *)buffer + done, len - done); + if (!in) + memcpy(aligned_buffer, (char *)buffer + done, len); + writel((uint32_t)aligned_buffer, &hc_regs->hcdma);
/* Set host channel enable after all other setup is complete. */ @@ -810,16 +812,16 @@ int chunk_msg(struct usb_device *dev, unsigned long pipe, int *pid, int in, break; }
- done += xfer_len; if (in) { - done -= sub; + xfer_len -= sub; + memcpy(buffer + done, aligned_buffer, xfer_len); if (sub) stop_transfer = 1; } - } while ((done < len) && !stop_transfer);
- if (done && in) - memcpy(buffer, aligned_buffer, done); + done += xfer_len; + + } while ((done < len) && !stop_transfer);
writel(0, &hc_regs->hcintmsk); writel(0xFFFFFFFF, &hc_regs->hcint);
Each USB transfer is split up into chunks that are held in an aligned buffer. This imposes a limit on the size of each chunk, but no limit on the total size of transferred data. Fix the logic in chunk_msg() not to reject large transfers, but simply take the size of the aligned buffer into account when calculating the chunk size.
Signed-off-by: Stephen Warren swarren@wwwdotorg.org --- v2: New patch.
drivers/usb/host/dwc2.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index 05d21b7948f5..e370d29ffc8e 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -756,24 +756,18 @@ int chunk_msg(struct usb_device *dev, unsigned long pipe, int *pid, int in, 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", - __func__, len, DWC2_DATA_BUF_SIZE); - dev->status = 0; - dev->act_len = 0; - return -EINVAL; - } - do { /* Initialize channel */ 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. */ if (xfer_len > CONFIG_DWC2_MAX_TRANSFER_SIZE) xfer_len = CONFIG_DWC2_MAX_TRANSFER_SIZE - max + 1; + if (xfer_len > DWC2_DATA_BUF_SIZE) + xfer_len = DWC2_DATA_BUF_SIZE - max + 1;
+ /* Make sure that xfer_len is a multiple of max packet size. */ if (xfer_len > 0) { num_packets = (xfer_len + max - 1) / max; if (num_packets > CONFIG_DWC2_MAX_PACKET_COUNT) {
On Sunday, March 08, 2015 at 06:08:13 PM, Stephen Warren wrote:
The original aligned_buffer usage: a) Uselessly copied data into the aligned buffer even for IN transactions. Fix this my making the copy conditional. b) Always programmed the HW to transfer to/from the start of the aligned buffer. This worked fine for OUT transactions since the memcpy copied the OUT data to this location too. However, for large IN transactions, since the copy from the aligned buffer to the "client" buffer was deferred until after all chunks were transferred. it resulted in each chunk's transfer over-writing the data for the first transfer. Fix this by copying IN data as soon as it's received.
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
On 03/09/2015 06:46 AM, Marek Vasut wrote:
On Sunday, March 08, 2015 at 06:08:13 PM, Stephen Warren wrote:
The original aligned_buffer usage: a) Uselessly copied data into the aligned buffer even for IN transactions. Fix this my making the copy conditional. b) Always programmed the HW to transfer to/from the start of the aligned buffer. This worked fine for OUT transactions since the memcpy copied the OUT data to this location too. However, for large IN transactions, since the copy from the aligned buffer to the "client" buffer was deferred until after all chunks were transferred. it resulted in each chunk's transfer over-writing the data for the first transfer. Fix this by copying IN data as soon as it's received.
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.
Yes, that makes sense at this point in the cycle.
BTW, I should have mentioned the result of these on the RPi, although they're much the same as with the earlier patch you posted to support
max_packet_size control transfers:
* I can enumerate a USB keyboard directly attached to the SoC, without intervening hub.
* If I enable USB keyboard support in U-Boot, and configure it to poll via the control EP or interrupt EP (I implemented the submit_interrupt function identically to submit_bulk), it will roughly work, except:
** The system usually hangs up very shortly after detecting the USB keyboard. Sometimes it lasts a long time though. This appears to be a hard hang; if I force the timer module to generate an IRQ, then typically I would see U-Boot spew the PC/SP/... when the IRQ fires. However, if the timer IRQ is scheduled to fire after the USB-related hang, nothing is printed. I need to try JTAG I guess.
** When polling via the interrupt EP, the character repeat is massively too fast; much faster than on e.g. NVIDIA Tegra Seaboard with the same keyboard polling option selected. It seems that USB IN transactions often repeat without actually writing to the IN data buffer, although I can't see any status in the registers indicating that the transaction was NAK'd/NYET'd/failed. I wonder if I need to manually delay transactions with this controller - do EHCI controllers hold off interrupt transactions automatically and only submit them to the bus at a low rate?
participants (2)
-
Marek Vasut -
Stephen Warren