On Mon, May 05, 2014 at 01:34:10PM +0530, Amit Virdi wrote:
> [Re-sending, as previously it was HTML formatted and rejected by the server]
>
> Dear Felipe,
>
> On Fri, Feb 28, 2014 at 1:19 AM, Felipe Balbi <balbi@xxxxxx> wrote:
> >
> > by setting IOC always, we can recycle TRBs a
> > lot sooner at the expense of some increased
> > CPU load.
> >
> > The extra load seems to be quite minimal on
> > OMAP5 devices (instead of 1 IRQ for one MSC
> > transfer, we get
> > CONFIG_USB_GADGET_STORAGE_NUM_BUFFERS).
> >
> > Signed-off-by: Felipe Balbi <balbi@xxxxxx>
> > ---
> > drivers/usb/dwc3/gadget.c | 9 +++------
> > 1 file changed, 3 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > index 2da0a5a..9e878d9 100644
> > --- a/drivers/usb/dwc3/gadget.c
> > +++ b/drivers/usb/dwc3/gadget.c
> > @@ -771,9 +771,6 @@ static void dwc3_prepare_one_trb(struct dwc3_ep *dep,
> > trb->ctrl = DWC3_TRBCTL_ISOCHRONOUS_FIRST;
> > else
> > trb->ctrl = DWC3_TRBCTL_ISOCHRONOUS;
> > -
> > - if (!req->request.no_interrupt && !chain)
> > - trb->ctrl |= DWC3_TRB_CTRL_IOC;
> > break;
> >
> > case USB_ENDPOINT_XFER_BULK:
> > @@ -788,6 +785,9 @@ static void dwc3_prepare_one_trb(struct dwc3_ep *dep,
> > BUG();
> > }
> >
> > + if (!req->request.no_interrupt && !chain)
> > + trb->ctrl |= DWC3_TRB_CTRL_IOC;
> > +
> > if (usb_endpoint_xfer_isoc(dep->endpoint.desc)) {
> > trb->ctrl |= DWC3_TRB_CTRL_ISP_IMI;
> > trb->ctrl |= DWC3_TRB_CTRL_CSP;
>
> As a result of this patch, IOC bit for TRBs corresponding to
> Bulk/Interrupt/Isoc
> transfers is set. However, we are still not enabling XferInProgress event
> for Bulk
> and Interrupt EPs. It means enabling IOC is as good as not enabling it.
> This is
> because as per the DWC3 spec:
>
> When IOC is set in a TRB, and once the transfer for this buffer
> is completed, the core will issue XferInProgress event with IOC
> bit set in the event's status. This indicates the buffer is
> available for software to reuse or release.
>
> If we are not enabling the XferInProgress event for interrupt EP but
> setting its
> IOC flag, the overall effect of this is nothing positive. If my
> understanding is
> correct the only intention to enable IOC should be to process the buffer
> asap
> which does not happen until we enable XferInProgress event.
>
> Here's some testing I did to discover it:
> I queued 8 TRBs for interrupt EP in the request_queue. So, while
> preparing the TRBs, the DWC3 driver sets the ctrl fields as :
> TRB0 - IOC
> TRB1 - IOC
> TRB2 - IOC
> TRB3 - IOC
> TRB4 - IOC
> TRB5 - IOC
> TRB6 - IOC
> TRB7 - IOC, LST
>
> Now, since XferInProgress event is not enabled, so the core issues only one
> XferComplete event after it processes all the eight trbs. Am I missing
> anything here?
>
> I understand that enabling XferInProgress event for bulk/interrupt
> transfers will completely
> change the design of the dwc3 driver and hence is not the viable solution
> to the issue here.
just send a patch enabling XferInProgress.. I haven't gotten to it yet.
If you beat me to it, more power for you ;-)
--
balbi
Attachment:
signature.asc
Description: Digital signature
