FreeLists / openbeosnetteam / [openbeosnetteam] Re: Realtek 8139 Driver

["Philippe Houdoin" <philippe.houdoin@xxxxxxx>]

Hi,

Niels, I've received your driver source.
I'm bit suprised how you handle the rx buffer! 
You know it's suppose to be a ring buffer, don't you?
According to Realtek datasheet, their chip will handle it that way. 
But I can't see where you handle this circular property in your driver 
code!?
You simply always increment the buffer tail address on 32bits boundary.
There should be some "modulo 64k"  magic somewhere!!!
;-)

Next, a packet could wraps around the ring buffer, something your 
append_packet() function don't care (yet !?).
I don't have any RTL8139 card to stress your code more, but I don't see 
how the reception could works right 
looking at current code.

I notice, too, that both NewOS rtl8139.c driver (which, unfortunatly 
for you, is no more 
present in our CVS and as NewOS source depot seem unavailable these 
days... bad luck) 
and NetBSD rtl81x9.c drivers disable chip interrupts by writing 0 to 
IMR register at start of the interrupt handler, 
and re-enable them at exit.
Maybe it's a good design to follow?

Next, it's may be a good idea to clear all interrupt status masks, even 
if you don't handle them yet.
Instead of building this isr_write value, simply *acknoledge* the whole 
interrupt status masks by writing it:

isr_contents = m_pcimodule->read_io_16( data->reg_base + ISR );
if (isr_contents) 
  m_pcimodule->write_io_16(data->reg_base + ISR, isr_contents);

Last, looking at NetBSD and NewOS drivers, they both seems to loop on 
reading the ISR and handling each interrupt case until there's no more. 
Maybe this chip don't raise all interrupt status flags at same time?

Okay, enough with my suggestions.

I'll create a new rtl8139 directoy to host your driver in the next 
hours or days, depends on son noise level :-p

-Philippe.

--
Fortune Cookie Says:

If anything can go wrong, it will.