Hi,
Janusz Dziedzic <janusz.dziedzic@xxxxxxxxx> writes:
> On 7 April 2017 at 13:36, Felipe Balbi <felipe.balbi@xxxxxxxxxxxxxxx> wrote:
>> Just like we did for all other endpoint types, let's rely on a chained
>> TRB pointing to ep0_bounce_addr in order to align transfer size. This
>> will make the code simpler.
>>
>> Signed-off-by: Felipe Balbi <felipe.balbi@xxxxxxxxxxxxxxx>
>> ---
>> drivers/usb/dwc3/core.h | 6 +----
>> drivers/usb/dwc3/ep0.c | 65 +++++++++++++----------------------------------
>> drivers/usb/dwc3/gadget.c | 50 ++++++++----------------------------
>> 3 files changed, 29 insertions(+), 92 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>> index 2a2673767ccd..e1958f6237af 100644
>> --- a/drivers/usb/dwc3/core.h
>> +++ b/drivers/usb/dwc3/core.h
>> @@ -43,7 +43,7 @@
>> #define DWC3_PULL_UP_TIMEOUT 500 /* ms */
>> #define DWC3_ZLP_BUF_SIZE 1024 /* size of a superspeed bulk */
>> #define DWC3_BOUNCE_SIZE 1024 /* size of a superspeed bulk */
>> -#define DWC3_EP0_BOUNCE_SIZE 512
>> +#define DWC3_EP0_SETUP_SIZE 512
>> #define DWC3_ENDPOINTS_NUM 32
>> #define DWC3_XHCI_RESOURCES_NUM 2
>>
>> @@ -763,12 +763,10 @@ struct dwc3_scratchpad_array {
>> * struct dwc3 - representation of our controller
>> * @drd_work - workqueue used for role swapping
>> * @ep0_trb: trb which is used for the ctrl_req
>> - * @ep0_bounce: bounce buffer for ep0
>> * @zlp_buf: used when request->zero is set
>> * @setup_buf: used while precessing STD USB requests
>> * @ep0_trb: dma address of ep0_trb
>> * @ep0_usb_req: dummy req used while handling STD USB requests
>> - * @ep0_bounce_addr: dma address of ep0_bounce
>> * @scratch_addr: dma address of scratchbuf
>> * @ep0_in_setup: one control transfer is completed and enter setup phase
>> * @lock: for synchronizing
>> @@ -866,13 +864,11 @@ struct dwc3 {
>> struct work_struct drd_work;
>> struct dwc3_trb *ep0_trb;
>> void *bounce;
>> - void *ep0_bounce;
>> void *zlp_buf;
>> void *scratchbuf;
>> u8 *setup_buf;
>> dma_addr_t ep0_trb_addr;
>> dma_addr_t bounce_addr;
>> - dma_addr_t ep0_bounce_addr;
>> dma_addr_t scratch_addr;
>> struct dwc3_request ep0_usb_req;
>> struct completion ep0_in_setup;
>> diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
>> index 3ba2309c837f..04249243e4d3 100644
>> --- a/drivers/usb/dwc3/ep0.c
>> +++ b/drivers/usb/dwc3/ep0.c
>> @@ -836,7 +836,6 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,
>> struct usb_request *ur;
>> struct dwc3_trb *trb;
>> struct dwc3_ep *ep0;
>> - unsigned transfer_size = 0;
>> unsigned maxp;
>> unsigned remaining_ur_length;
>> void *buf;
>> @@ -849,9 +848,7 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,
>> ep0 = dwc->eps[0];
>>
>> dwc->ep0_next_event = DWC3_EP0_NRDY_STATUS;
>> -
>> trb = dwc->ep0_trb;
>> -
>> trace_dwc3_complete_trb(ep0, trb);
>>
>> r = next_request(&ep0->pending_list);
>> @@ -872,39 +869,17 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,
>> remaining_ur_length = ur->length;
>>
>> length = trb->size & DWC3_TRB_SIZE_MASK;
>> -
>> maxp = ep0->endpoint.maxpacket;
>> + transferred = ur->length - length;
>> + ur->actual += transferred;
>>
>> if (dwc->ep0_bounced) {
>> - /*
>> - * Handle the first TRB before handling the bounce buffer if
>> - * the request length is greater than the bounce buffer size
>> - */
>> - if (ur->length > DWC3_EP0_BOUNCE_SIZE) {
>> - transfer_size = ALIGN(ur->length - maxp, maxp);
>> - transferred = transfer_size - length;
>> - buf = (u8 *)buf + transferred;
>> - ur->actual += transferred;
>> - remaining_ur_length -= transferred;
>> -
>> - trb++;
>> - length = trb->size & DWC3_TRB_SIZE_MASK;
>> -
>> - ep0->trb_enqueue = 0;
>> - }
>> -
>> - transfer_size = roundup((ur->length - transfer_size),
>> - maxp);
>> -
>> - transferred = min_t(u32, remaining_ur_length,
>> - transfer_size - length);
>> - memcpy(buf, dwc->ep0_bounce, transferred);
>> - } else {
>> - transferred = ur->length - length;
>> + trb++;
>> + trb->ctrl &= ~DWC3_TRB_CTRL_HWO;
>> + ep0->trb_enqueue = 0;
>> + dwc->ep0_bounced = false;
>> }
>>
>> - ur->actual += transferred;
>> -
>> if ((epnum & 1) && ur->actual < ur->length) {
>> /* for some reason we did not get everything out */
>>
>> @@ -1006,8 +981,8 @@ static void __dwc3_ep0_do_control_data(struct dwc3 *dwc,
>> ret = dwc3_ep0_start_trans(dep);
>> } else if (!IS_ALIGNED(req->request.length, dep->endpoint.maxpacket)
>> && (dep->number == 0)) {
>> - u32 transfer_size = 0;
>> u32 maxpacket;
>> + u32 rem;
>>
>> ret = usb_gadget_map_request_by_dev(dwc->sysdev,
>> &req->request, dep->number);
>> @@ -1015,23 +990,19 @@ static void __dwc3_ep0_do_control_data(struct dwc3 *dwc,
>> return;
>>
>> maxpacket = dep->endpoint.maxpacket;
>> -
>> - if (req->request.length > DWC3_EP0_BOUNCE_SIZE) {
>> - transfer_size = ALIGN(req->request.length - maxpacket,
>> - maxpacket);
>> - dwc3_ep0_prepare_one_trb(dep, req->request.dma,
>> - transfer_size,
>> - DWC3_TRBCTL_CONTROL_DATA,
>> - true);
>> - }
>> -
>> - transfer_size = roundup((req->request.length - transfer_size),
>> - maxpacket);
>> -
>> + rem = req->request.length % maxpacket;
>> dwc->ep0_bounced = true;
>>
>> - dwc3_ep0_prepare_one_trb(dep, dwc->ep0_bounce_addr,
>> - transfer_size, DWC3_TRBCTL_CONTROL_DATA,
>> + /* prepare normal TRB */
>> + dwc3_ep0_prepare_one_trb(dep, req->request.dma,
>> + req->request.length,
>> + DWC3_TRBCTL_CONTROL_DATA,
>> + true);
>> +
>> + /* Now prepare one extra TRB to align transfer size */
>> + dwc3_ep0_prepare_one_trb(dep, dwc->bounce_addr,
>> + maxpacket - rem,
>> + DWC3_TRBCTL_CONTROL_DATA,
>> false);
>> ret = dwc3_ep0_start_trans(dep);
>> } else {
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 4787d6f449fa..4dc80729ae11 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -171,7 +171,6 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req,
>> int status)
>> {
>> struct dwc3 *dwc = dep->dwc;
>> - unsigned int unmap_after_complete = false;
>>
>> req->started = false;
>> list_del(&req->list);
>> @@ -181,19 +180,8 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req,
>> if (req->request.status == -EINPROGRESS)
>> req->request.status = status;
>>
>> - /*
>> - * NOTICE we don't want to unmap before calling ->complete() if we're
>> - * dealing with a bounced ep0 request. If we unmap it here, we would end
>> - * up overwritting the contents of req->buf and this could confuse the
>> - * gadget driver.
>> - */
>> - if (dwc->ep0_bounced && dep->number <= 1) {
>> - dwc->ep0_bounced = false;
>> - unmap_after_complete = true;
>> - } else {
>> - usb_gadget_unmap_request_by_dev(dwc->sysdev,
>> - &req->request, req->direction);
>> - }
>> + usb_gadget_unmap_request_by_dev(dwc->sysdev,
>> + &req->request, req->direction);
>>
>> trace_dwc3_gadget_giveback(req);
>>
>> @@ -201,10 +189,6 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req,
>> usb_gadget_giveback_request(&dep->endpoint, &req->request);
>> spin_lock(&dwc->lock);
>>
>> - if (unmap_after_complete)
>> - usb_gadget_unmap_request_by_dev(dwc->sysdev,
>> - &req->request, req->direction);
>> -
>> if (dep->number > 1)
>> pm_runtime_put(dwc->dev);
>> }
>> @@ -3153,32 +3137,23 @@ int dwc3_gadget_init(struct dwc3 *dwc)
>> goto err0;
>> }
>>
>> - dwc->setup_buf = kzalloc(DWC3_EP0_BOUNCE_SIZE, GFP_KERNEL);
>> + dwc->setup_buf = kzalloc(DWC3_EP0_SETUP_SIZE, GFP_KERNEL);
>> if (!dwc->setup_buf) {
>> ret = -ENOMEM;
>> goto err1;
>> }
>>
>> - dwc->ep0_bounce = dma_alloc_coherent(dwc->sysdev,
>> - DWC3_EP0_BOUNCE_SIZE, &dwc->ep0_bounce_addr,
>> - GFP_KERNEL);
>> - if (!dwc->ep0_bounce) {
>> - dev_err(dwc->dev, "failed to allocate ep0 bounce buffer\n");
>> - ret = -ENOMEM;
>> - goto err2;
>> - }
>> -
>> dwc->zlp_buf = kzalloc(DWC3_ZLP_BUF_SIZE, GFP_KERNEL);
>> if (!dwc->zlp_buf) {
>> ret = -ENOMEM;
>> - goto err3;
>> + goto err2;
>> }
>>
>> dwc->bounce = dma_alloc_coherent(dwc->sysdev, DWC3_BOUNCE_SIZE,
>> &dwc->bounce_addr, GFP_KERNEL);
>> if (!dwc->bounce) {
>> ret = -ENOMEM;
>> - goto err4;
>> + goto err3;
>> }
>>
>> init_completion(&dwc->ep0_in_setup);
>> @@ -3218,7 +3193,7 @@ int dwc3_gadget_init(struct dwc3 *dwc)
>>
>> ret = dwc3_gadget_init_endpoints(dwc, dwc->num_eps);
>> if (ret)
>> - goto err5;
>> + goto err4;
>>
>> ret = usb_add_gadget_udc(dwc->dev, &dwc->gadget);
>> if (ret) {
>> @@ -3228,16 +3203,14 @@ int dwc3_gadget_init(struct dwc3 *dwc)
>>
>> return 0;
>> err5:
>> - dma_free_coherent(dwc->sysdev, DWC3_BOUNCE_SIZE, dwc->bounce,
>> - dwc->bounce_addr);
>> + dwc3_gadget_free_endpoints(dwc);
>>
>> err4:
>> - kfree(dwc->zlp_buf);
>> + dma_free_coherent(dwc->sysdev, DWC3_BOUNCE_SIZE, dwc->bounce,
>> + dwc->bounce_addr);
>>
>> err3:
>> - dwc3_gadget_free_endpoints(dwc);
>> - dma_free_coherent(dwc->sysdev, DWC3_EP0_BOUNCE_SIZE,
>> - dwc->ep0_bounce, dwc->ep0_bounce_addr);
>> + kfree(dwc->zlp_buf);
>>
>> err2:
>> kfree(dwc->setup_buf);
>> @@ -3260,9 +3233,6 @@ void dwc3_gadget_exit(struct dwc3 *dwc)
>>
>> dma_free_coherent(dwc->sysdev, DWC3_BOUNCE_SIZE, dwc->bounce,
>> dwc->bounce_addr);
>> - dma_free_coherent(dwc->sysdev, DWC3_EP0_BOUNCE_SIZE,
>> - dwc->ep0_bounce, dwc->ep0_bounce_addr);
>> -
>> kfree(dwc->setup_buf);
>> kfree(dwc->zlp_buf);
>>
>> --
>> 2.11.0.295.gd7dffce1ce
>>
>
> Thanks for patches.
> Tested this series in my env where have problems with ep0 bounced
> buffers (rndis and delayed dma unmap).
> Works perfect.
Good, thanks for testing :-) No more memcpy() needed :-)
--
balbi
Attachment:
signature.asc
Description: PGP signature
