This is the mail archive of the ecos-patches@sources.redhat.com 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: Bug fix in cl_8900a ethernet driver


On Fri, 2003-04-11 at 04:56, Jonathan Larmour wrote:
> Laurent Gonzalez wrote:
> > hi folks,
> > 
> > the file attached contains a patch to fix this bug.
> > 
> > In cs8900a_send, the cast from data to sdata is not safe. If data is not
> > aligned on a short boundary (even address), the 16bit read with the
> > pointer sdata, may return an unpredictible value or raise an alignement
> > error (depending the hardware).
> > The bug will happen if the variable odd_byte becomes true, and the next
> > packet has an aligned value for data. the first byte is joined with
> > saved_data, and the pointer data becomes odd.
> 
> Thanks for spotting this. This indeed looks like a problem, although I'm 
> slightly concerned that the patch is endian-specific.... but looking at 
> the driver, it looks to be endian-specific in its treatment of odd_byte 
> too. Nevertheless, I thought I may as well fix it just in case since even 
> if the CS8900 is only ever used with ARMs which it may be, ARMs have been 
> known to be big-endian.
> 
> However it's somewhat sub-optimal to _always_ resort to byte fiddling. 
> Most times the amount of data to be spent will be large enough that 
> optimising the aligned case is worth it. So I've created the attached 
> patch. Since I don't have cs8900 hardware, please let me know if it works 
> for you and I'll check it in!
> 
Your patch looks good. 
I agree that cs8900 seems to be mostly used with ARMs, AFAIK ARMs are
not known to be big-endian, it is implementation (or configuration)
dependant. All my eval boards do little endian access to their external
buses, that why my fix was endian specific. Thus, I only tested your
patch for a little-endian target, and it worked. I guess it will work
even for a big endian target.
It is a good thing to optimise the aligned case, but I don't thing that
the performance increase will be significative. The real bottle neck
here, is the memory access to the cs8900 that requires more than 100ns
for a single 16-bit io write. Even flash devices are faster !

-- 
GONZALEZ Laurent
Silicomp Research Institute
Tel: 04 76 41 66 98


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