> > + if (iso){
>
> Use "if (iso) {" instead ... one espace before such brackets.
I did not entirely succeed in suppressing my own coding style in these
patches; I relied heavily on linux-c-mode, but there are a number fo
things it will not change/enforce (like spaces instead of tabs; it
will realign, but leave the whitespace as spaces, etc).
> > + /* for now, we don't accelerate iso completions ... so spin
> > + a while. */
> > +
>
> "a while" is not as useful as "until the only reference is from udev->ep_*[]".
The comment was preexisting.
> Comment style should be
>
> /* just one line */
>
> or
>
> /* line 1
> * line 2..n-1
> */
linux-c-mode made that change, I assumed it was correct.
> ... and you should never add end-of-line whitespace.
I never do. Either linux-c-mode did or gmail did (or both).
> > + while(iso->refcount>1){
>
> Use "while () {" not "while(){" ... it's not a function call.
> In fact,
>
> while (iso->refcount > 1) {
>
> ... you're not including whitespace which you _should_ include,
> and (elsewhere) are adding whitespace you should not include.
I accept this in the context of the kernel coding style. Given that
this is all entirely mechanical and mandatory, I'm surprised
linux-c-mode does not handle it for me.
> > - if (stream->refcount == 1) {
> > + /* don't free on last descriptor; free when endpoint disable
> > + finally releases last refcount. Although it is technically
> > + broken for an endpoint driver to submit its streaming
> > + descriptors such that a new one appears after the old one
> > + ends, it is only punishing the users to insist on breaking
> > + these drivers when it's not necessary to do so. This saves
> > + substantial overhead in that case.*/
>
> The comment should be corrected, both for content and style. In this
> case I'd suggest
>
> /* if refcount == 1 this is just from udev->ep_{in,out}[];
> * we can at least avoid reallocating this stream's memory, and
> * the previously budgeted bandwidth may still be available.
> */
Except that your comment is incorrect in both content and intent. The
whole point is that the bandwidth *is* still there and *will* be
available, by design. There is no 'may'. There is no 'should'. It
is invariant. More than that, the ehci_iso_stream struct does not
exist for its own sake; it is the handle being used by the budget to
ID the stream. I don't care about its memory, the important part is
that it is a persistent handle.
Monty
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
