Re: [PATCH 2/4] usb: dwc3: gadget: Stop TRB preparation after limit is reached

[Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2856 bytes --]

On Tue, Dec 30, 2014 at 08:11:13PM +0530, Amit Virdi wrote:
> >> The only reason why I have not been able to test these fixes on the
> >> latest is because the customized webcam gadget is not ported on the
> >> latest kernel. There have been a lot of changes in the video framework
> >> lately and that is not my area of expertise. So porting the customized
> >> webcam gadget to latest kernel and testing these fixes is not feasible
> >> for me.
> >
> > right, so while I can see that both patches are very minimal and likely
> > to be correct, I have no way of guaranteeing that. The only thing I can
> > do, is run the tests which I already run (none of which exercise the
> > cases you found though, other I'd have found them already) to make sure
> > your patches don't break what's already working.
> >
> 
> Okay
> 
> >> Having said all this, the fact remains that these are bugs in the dwc3
> >> driver from kernel v3.9 onwards. The log messages clearly depict this.
> >
> > no, it doesn't :-) your log snippet is so small that I cannot see how
> > the driver got to that point :-) The only thing I can see is that you
> > started two requests, one for 960000 bytes and another one for 2 bytes,
> > but I don't know how many TRBs we had available, nor do I see a Start
> > Transfer command, so I can't validate Start Transfer command parameters
> > were correct or not.
> >
> >> These bugs are obviously present till date.
> >
> > Right, the code is the same :-)
> >
> >> I ask you to give a more deeper thought on the whole situation and not
> >> undermine the importance of code review.
> >
> > heh
> >
> >>  - [Patch 2/4] fixes a bug that could have been found in the code review.
> >>  - [Patch 1/4] fixes a bug which was very very subtle but a deeper
> >> code review can certainly boost the confidence about the change made.
> >> list_is_last won't work when SG is used because, for the last request
> >> in the request_list, the TRB for the first sg entry modifies the
> >> list's next and prev pointers while moving the URB to req_queued.
> >>
> >> I can certainly provide the dwc3 specific kernel bootup logs, full
> >> regdump and any loglevel you want me to, if that helps
> >
> > Yeah, if you can provide those, then that'll help me verifying. Full
> > logs from boot to failure point with VERBOSE_DEBUG enabled (considering
> > you're not running on anything recent).
> >
> 
> Okay. I'll provide full verbose logs, along with the register dump,
> when I'm back to the office next week, for the working and non-working
> case, but how - as attachment or as inline?

Either way will do but I have a feeling mailing list attachment size
will bite you, so maybe it's best to make a tarball of both logs and
send that as attachment. Compressed text will be very small.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]