On Wed, May 02, 2012 at 02:35:38PM +0530, Pratyush Anand wrote:
> On 5/2/2012 12:04 PM, Felipe Balbi wrote:
> >On Tue, May 01, 2012 at 12:18:25PM +0530, Pratyush Anand wrote:
> >>If transfer not ready is received before ep queue is called for control
> >>out transfer then pending flag is set. Handling of this case was not
> >>correct.
> >>TRB size must be max packet for all control out transfer.
> >>dma sync function should also be called before start transfer.
> >>
> >>Signed-off-by: Pratyush Anand<pratyush.anand@xxxxxx>
> >>---
> >> drivers/usb/dwc3/ep0.c | 26 +++++++++++++++++++++++---
> >> 1 files changed, 23 insertions(+), 3 deletions(-)
> >>
> >>diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
> >>index c6c9b8a..ad4062a 100644
> >>--- a/drivers/usb/dwc3/ep0.c
> >>+++ b/drivers/usb/dwc3/ep0.c
> >>@@ -150,9 +150,29 @@ static int __dwc3_gadget_ep0_queue(struct dwc3_ep *dep,
> >> return 0;
> >> }
> >>
> >>- ret = dwc3_ep0_start_trans(dwc, direction,
> >>- req->request.dma, req->request.length,
> >>- DWC3_TRBCTL_CONTROL_DATA);
> >>+ dwc3_map_buffer_to_dma(req);
> >>+
> >>+ if (direction)
> >>+ ret = dwc3_ep0_start_trans(dwc, direction,
> >>+ req->request.dma, req->request.length,
> >>+ DWC3_TRBCTL_CONTROL_DATA);
> >>+ else {
> >>+ WARN_ON(req->request.length> dep->endpoint.maxpacket);
> >>+
> >>+ dwc->ep0_bounced = true;
> >
> >this introduces a bug. Not all cases will need bouncing, only wen
> >request.length isn't aligned to wMaxPacketSize. Also, instead of adding
>
> Ok.
>
> >all this huge amount of code just to change one simple parameter, you
> >could have something like:
> >
> >unsigned transfer_size = req->request.length;
> >
> >if (transfer_size % dep->endpoint.maxpacket)
> > transfer_size += transfer_size % dep->endpoint.maxpacket;
>
> will this not make transfer_size *= 2?
> Probably it is not intended.
> We want to keep transfer_size to maxpacket in case of direction=0;
no no, we want to make transfer_size aligned to wMaxPacketSize, so if
req->request.length == 1025, we want transfer_size = 2048
> So, I think we need to take care of both the case.
>
> 1. if (direction == 0) && (transfer_size % dep->endpoint.maxpacket),
> then use bounced.
indeed.
> 2. if (transfer_size == dep->endpoint.maxpacket) then use request.dma.
correct.
> >ret = dwc3_ep0_start_trans(dwc, direction,
> > req->request.dma,
> > direction ? req->request.length : transfer_size,
> > DWC3_TRBCTL_CONTROL_DATA);
> >
> >you would be adding much less code. also, you missed braces on the if ()
> >branch.
>
> I thought its single line instruction. will take care.
Take a look at coding style, when you have if..else conditionals, if one
of the branches has braces, all of them should have ;-)
--
balbi
Attachment:
signature.asc
Description: Digital signature
