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

[Felipe Balbi]

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.an...@st.com>

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 ?

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.

-- 
balbi

signature.asc
Description: Digital signature