Subject: Re: [PATCH 5/8] smc91x: add SMC91X_IO_SHIFT* macros and make SMC_IO_SHIFT a variable

From: Eric Miao <[email-masked]>
Date: Fri, 20 Jun 2008 17:19:23 +0800

Sascha Hauer wrote:
> Your numbers look like a very bad overall performance. I remember we got
> nearly 100Mbit/s on a pxa system with the smsc connected in 32bit mode.
> While this project was performance critical I'm often in the situation
> that I just expect the smsc driver to work without looking at the
> ifdeffery in the header file.
> Shouldn't it be possible to make access macros optional for those who
> want performance and use functions as the default case?
> 

This isn't a serious performance test. I just want to give myself a
rough feeling of the degrade level in a casual way.

Using a centralized function will be the ultimate solution to handle
difference between platforms, that requires another bunch of patches.
And there are also some exceptions like mainstone, which uses its own
SMC_outw() to workaround something buggy in hardware.

To have this driver supporting so many platforms at run-time is really
a head-scratching work, and may eventually introduce some trade-offs :-/

> 
>> So I'm thinking that the overhead may not be so significant as expected,
>> 1. control register accesses are rare compared to data register
>> 2. data register access is usually fixed at one address and enclosed in
>>    a loop, which the compiler may well optimize
>>
>>> And this is very important to have the lowest overhead possible with 
>>> this chip that can do 100mbps on platforms with a CPU clock almost as 
>>> slow.
>>>
>> Indeed, the overhead will be magnified on a system with slow CPU clock,
>> maybe I should spend some time to have a test also. However, arguably,
>> the smc91x chips are usually used as a debug ethernet on most (if not
>> all) platforms, I don't think a serious design will deploy such a chip
>> for performance critical application, though.
> 
> Not anymore probably, but we seem to tun into the same problem with the
> current smc911x driver aswell

Indeed.

> Sascha