This is the mail archive of the
ecos-discuss@sourceware.org
mailing list for the eCos project.
Re: Bugs in the at91 USB driver
- From: Andrew Lunn <andrew at lunn dot ch>
- To: Frank Pagliughi <fpagliughi at mindspring dot com>
- Cc: ecos-discuss at ecos dot sourceware dot org
- Date: Sun, 11 May 2008 14:26:02 +0200
- Subject: Re: [ECOS] Bugs in the at91 USB driver
- References: <48229002.8010108@mindspring.com>
On Thu, May 08, 2008 at 01:30:42AM -0400, Frank Pagliughi wrote:
> Hi,
>
> I was reading through the AT91 USB driver code and spotted two bugs, but
> am unsure what to do with them.
>
> 1. The "usbs_at91_control_setup_get_status()" function needs to queue up
> a value to send back to the host, but the function sets the endpoint
> buffer variables (pbegin, etc) to the address of a local variable,
> 'word', which disappears when the function returns (before the buffer is
> shipped to the host).
>
> I assume the return value can be placed into ep0's "control_buffer" for
> return to the host. Or a tx buffer can be declared and used for such
> purposes. Which would be better?
Or just make word static so it is not a stack variable.
> 2. The "usbs_state_notify()" function sends the wrong value to the state
> change callback function. Instead of sending a value of a
> "usbs_state_change" enumeration (like USBS_STATE_CHANGE_RESET) for the
> third parameter, it sends the new state (like USBS_STATE_POWERED).
>
> On one hand, this needs to be fixed so that class drivers and examples
> can work with the AT91 driver, but on the other hand, fixing it could
> break existing applications that were written to use the incorrect
> values being emitted by the driver.
Like you said, this needs to be fixed, it is wrong. It is actually not
too bad. If you compare usbs_state_change and USBS_STATE_*, most are
the same. So in many cases i think it will just work.
> It's especially confusing because the two enumerations are very
> close and the "state change" values are an eCos invention and not
> that well defined.
It is a shame the compiler did not emit a warning here. If the
USBS_STATE_* had been an enumeration, i think it would of done....
Please submit a patch.
Andrew
--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss