Re: [PATCH] usb/dwc3: Correct cache sync issue in case of ep0 bounced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Fri, Aug 10, 2012 at 01:36:42PM +0530, Pratyush Anand wrote:
> On 8/10/2012 1:07 PM, Felipe Balbi wrote:
> >Hi,
> >
> >On Fri, Aug 10, 2012 at 12:54:52PM +0530, Pratyush Anand wrote:
> >>In case of ep0 out, if length is not aligned to maxpacket size then we
> >>use dwc->ep_bounce_addr for dma transfer and not request->dma. Since, we
> >>have alreday done memcpy from dwc->ep_bounce to request->buf, so we do
> >>not need to issue cache sync function. Infact, cache sync function will
> >>bring wrong data in request->buf from request->dma in this scenario.
> >>
> >>So, cache sync function must not be executed in case of ep0 bounced.
> >>
> >>Signed-off-by: Pratyush Anand <pratyush.anand@xxxxxx>
> >
> >this is very nice fix, but ...
> >
> >>---
> >>  drivers/usb/dwc3/ep0.c    |    1 -
> >>  drivers/usb/dwc3/gadget.c |    7 +++++--
> >>  2 files changed, 5 insertions(+), 3 deletions(-)
> >>
> >>diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
> >>index 7b99c5a..1e88de8 100644
> >>--- a/drivers/usb/dwc3/ep0.c
> >>+++ b/drivers/usb/dwc3/ep0.c
> >>@@ -804,7 +804,6 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,
> >>  		transferred = min_t(u32, ur->length,
> >>  				transfer_size - length);
> >>  		memcpy(ur->buf, dwc->ep0_bounce, transferred);
> >>-		dwc->ep0_bounced = false;
> >>  	} else {
> >>  		transferred = ur->length - length;
> >>  	}
> >>diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> >>index 800944f..4205a6c 100644
> >>--- a/drivers/usb/dwc3/gadget.c
> >>+++ b/drivers/usb/dwc3/gadget.c
> >>@@ -263,8 +263,11 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req,
> >>  	if (req->request.status == -EINPROGRESS)
> >>  		req->request.status = status;
> >>
> >>-	usb_gadget_unmap_request(&dwc->gadget, &req->request,
> >>-			req->direction);
> >>+	if (!dwc->ep0_bounced)
> >>+		usb_gadget_unmap_request(&dwc->gadget, &req->request,
> >>+				req->direction);
> >>+	else
> >>+		dwc->ep0_bounced = false;
> >
> >I'm not sure this will work always, though. What if we have unaligned
> >ep0 out PLUS some started transfers on non-ep0/1 and those complete
> >before the unaligned ep0out transfer ?
> 
> Yes , I think you are correct. In such scenario it might fail.
> 
> >
> >You will be preventing cache sync on the non-ep0out transfer. I think
> >you need to check for ep0, something like:
> >
> >if ((dep->number == 0 && !dwc->ep0_bounced) ||
> >	dep->number > 0)
> >	usb_gadget_unmap_request();
> >else
> >	dwc->ep0_bounced = false;
> >
> >or something similar.
> >
> 
> if (dep->number == 0 && dwc->ep0_bounced)
> 	dwc->ep0_bounced = false;
> else
> 	usb_gadget_unmap_request();
> 
> would be fine.

indeed. much simpler even :-)

-- 
balbi

Attachment: signature.asc
Description: Digital signature


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux