[haiku-bugs] Re: [Haiku] #7449: Check for invalid Frame List Sizes and enable Asynchronous Park Capability [gsoc2011]

  • From: "mmlr" <trac@xxxxxxxxxxxx>
  • Date: Wed, 20 Apr 2011 02:30:43 -0000

#7449: Check for invalid Frame List Sizes and enable Asynchronous Park 
   Reporter:  gabriel.hartmann  |      Owner:  mmlr
       Type:  bug               |     Status:  new
   Priority:  normal            |  Milestone:  R1
  Component:  Drivers/USB       |    Version:  R1/Development
 Resolution:                    |   Keywords:  EHCI gsoc2011
 Blocked By:                    |   Blocking:
Has a Patch:  1                 |   Platform:  All

Comment (by mmlr):

 A few more remarks:
 * You should move the frame list size check to the top, i.e. right after
 reading it out. If you return early in that case there's no point in
 reading the capabilities and command registers.
 * The trace output is fatal, so it should actually be TRACE_ERROR so it is
 always logged (the same is true for the "didn't start" error case that is
 already there).
 * There's a space missing after the comma in the trace.
 * You use "asyncParkCapable" like a boolean, so you should actually make
 it one. According to coding style by adding a "(...) != 0" around the
 * Since you use "asyncParkCapable" only once you could simply dump the
 variable and move the expression into the if clause. If you keep it, then
 move it to where it is used, i.e. directly above the if (coding style,
 declare variables where they are used).

 A general remark, not trying to be negative or anything, but why the whole
 async park mode setup? I originally left it out because it is defined by
 the EHCI specs as defaulting to enabled with a count of 3 if supported by
 the controller. Since we always reset the controller during startup, it
 will reset to those defaults, making this setup pretty much superfluous to
 begin with. If it doesn't actually fix any misbehaving implementations out
 there I'd actually like to keep it out of there as it is just needless
 code. A comment could instead be added to explain the situation.

Ticket URL: <http://dev.haiku-os.org/ticket/7449#comment:13>
Haiku <http://dev.haiku-os.org>
Haiku - the operating system.

Other related posts: