Re: Driver for via-velocity gigabit NIC

Marty Connor <[email protected]> Wed, 8 Mar 2006 09:00:18 -0500
Newsgroups gmane.network.etherboot.devel
Message-ID <[email protected]>
On Mar 8, 2006, at 8:41 AM, Timothy Legge wrote:
>> As much as I am glad to see a new driver, this troubles me.
>> The fact that the driver "works" is nice, but the fact that
>> you don't know why it was failing is not good.  I don't mean
>> to be overly harsh, but getting something to work is not
>> nearly enough in this case.  The next person who looks at
>> your driver and compares it to the Linux driver will wonder
>> why you changed what you changed.
>
> Considering that Etherboot and Linux differ a lot in the way  
> buffers are
> created and used that is not a problem.

I think it is a problem.  Not that there are differences, but that  
you don't seem to understand why things this fundamental work and  
don't work, and yet you're comfortable committing code and reviewing  
other people's drivers.

> The difference is that in this version, I changed from attempting  
> to use
> the method the the current Etherboot via-rhine driver uses for  
> alignment
> to the method I used in the etherboot r8169.  It was my original  
> method
> but I only used the other process when I ran into problems early in  
> the
> creation of the driver (over a year ago).  I like the current method
> better.  That being said, I fully plan to review how my current method
> differs from the method that did not work.  As you said in a previous
> email the problem is probably small and will be useful as a learning
> experience.

I am not convinced.  The "learning experience" should be done now,  
not later.
There's a lot about Etherboot I don't know, and when I don't, I ask  
for help from people who do.
What is troubling to me is not just that don't know why code works or  
doesn't work, but that you feel comfortable committing code you don't  
understand well.

>> Assuming the code worked in the Linux driver you were
>> porting, it should work in Etherboot.  If it does not, we
>> should know why.
>
> Completely irrelevant.

I say it is.

>   As above, this has to do with how the rings and
> buffers are aligned.

In what way?  How do they differ?  Are they mathematically equivalent  
ways of doing it?

>   Linux just depends on a lot of they lower level
> memory functionality to do the alignment.

Show the code.  Justify your choices.

>   I did not need to change the
> transmit and poll or anything other than the specific section that did
> at 64-byte alignment of the rings and buffers (in all probably less  
> than
> 30 lines of code).

Until you understand and can explain this code clearly, please do not  
commit it.

Marty




-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642