[haiku-bugs] Re: [Haiku] #7449: Check for invalid Frame List Sizes and enable Asynchronous Park Capability [gsoc2011]
- From: "gabriel.hartmann" <trac@xxxxxxxxxxxx>
- Date: Wed, 20 Apr 2011 04:34:24 -0000
#7449: Check for invalid Frame List Sizes and enable Asynchronous Park
Capability
[gsoc2011]
--------------------------------+----------------------------
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 gabriel.hartmann):
Replying to [comment:13 mmlr]:
> * 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.
Fixed.
> * 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).
Fixed.
> * There's a space missing after the comma in the trace.
Fixed.
> * You use "asyncParkCapable" like a boolean, so you should actually make
it one. According to coding style by adding a "(...) != 0" around the
statement.
Fixed.
> * 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).
Fixed. I kept the variable because it's clearer what's being checked for
in the if statement.
>
> 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.
I was asked to do this by Jérôme Duval as part of the Google Summer of
Code vetting process. A comment explaining why it's not necessary to
write any code, probably would have been frowned upon, although I don't
really know for sure. Feel free not to apply the patch if you think it
preferable to avoid executing the code.
--
Ticket URL: <http://dev.haiku-os.org/ticket/7449#comment:14>
Haiku <http://dev.haiku-os.org>
Haiku - the operating system.
Other related posts:
