
On Wed, 18 Feb 2009, Wolfgang Denk wrote:
Dear ksi@koi8.net,
In message Pine.LNX.4.64ksi.0902181112400.5002@home-gw.koi8.net you wrote:
Duplicating the source code (and thus the object code, too) to create additional instances of basicly the same driver seems to be the wrong approach to me.
It doesn't scale well, to say the least.
It is _NOT_ the same driver. Every bitbaged I2C driver is _DIFFERENT_. Only the _NAME_ is the same.
This is one way to implement it, but not the only one.
Just to give an example of a different implementation (without claiming that that would be a better one): we could provide an array of functions (instead of macros) for each such adapter, so we could use just a single instance of the driver which takes the address of the respective array as argument.
Yes, it is possible, but it is not the best approach. Most of those macros translate in 2 to 3 words. If you use functions you should add at very least 2 more words to it, one for a function call another for a function return. That is without array, just directly linked functions. If you are to use an array it adds another 2 words -- one for the array element (the pointer) itself and another one for the pointer to that array in i2c function code.
Linux uses functions for that but they are _INLINE_ ones in headers included into the driver file i.e. they are essentially macros.
That means that implementation is much worse than _EXISTING_ one. And out of decent and much worse one which one would you choose?
The soft_i2c.c is _NOT_ a driver _SOURCE_. It is a _TEMPLATE_ that makes a
This is your implementation. It is not the only possible implemen- tation. Please try and open your mind to discuss alternative possibilities as well.
No, it is NOT my implementation. It is _EXISTING_ driver in the main tree. I did _NOT_ change the driver, I just made several copies of it.
And, BTW, you can see something very similar in Linux kernel (i2c-algo-bit.c.)
I'm open to any alternative possibilities but I can not see anything better. That _EXISTING_ soft_i2c.c we have in the current tree is a little miracle that was there since the start of times so I can't see any reason to throw it away and reinvent the wheel.
And it can _NOT_ scale because it is impossible to make a generic driver _SOURCE_ for each and every hardware configuration imaginable. That existing
Impossible. Famous last words.
Well, it looks like it is you are Russian, not me :) It is well known Russian passion to scrap everything and design a Universal Server Of Everything from scratch :) I'm trying to avoid that...
soft_i2c.c makes _GENERATION_ of such a driver trivial. The only problem with it is it only makes _ONE_ such interface.
And it duplicates the source code for each additional instance that is needed.
And no, we are _NOT_ duplicating source code. Source code is _DIFFERENT_ for different adapters. We just creating several _INSTANCES_ using that template with _DIFFERENT_ parameters. And those instances are all different. The template itself does _NOT_ go into the final code.
Call it what you want, I call it duplication of code.
It might be a duplication of SOURCE TEXT, but not CODE.
Is this theory or did you perform code size measurements?
It is obvious. Furthermore, it doesn't make sence to count size difference here because it is miniscule -- how many I2C adapters do we have on a board?
It is obvious. Famous last words again.
Eh, do you think anyone really has time to make such comparisons to find out that size difference is zero?
The former does not require additional adapter struct member, hwadap_no. And, unlike the latter, it is self-contained, it doesn't require any external global variable to decide what to do. One can initialize all adapters by:
for (i = 0; i < NUM_I2C_ADAPTERS; i++) i2c_adap[i]->init(...);
Most probably we never *want* to initialize all adapters...
It is easier that way and wouldn't do any harm in most cases...
--- ****************************************************************** * KSI@home KOI8 Net < > The impossible we do immediately. * * Las Vegas NV, USA < > Miracles require 24-hour notice. * ******************************************************************