This is the mail archive of the ecos-discuss@sourceware.org mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Bugs in the at91 USB driver


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]