[U-Boot] [PATCH] 7/12 Multiadapter/multibus I2C, drivers part 4

[ksi at koi8.net]

On Wed, 18 Feb 2009, Wolfgang Denk wrote:

> Dear ksi at koi8.net,
> 
> In message <Pine.LNX.4.64ksi.0902180945100.5002 at home-gw.koi8.net> you wrote:
> > 
> > OK, for bitbang driver it is just a source file size reduction. We can
> > simply duplicate (triplicate etc.) that file for more than one adapter. What
> > I did makes CPP make that duplication instead of us. But I can simply do it
> > manually. It will make soft_i2c.c 4 times bigger (for 4 adapters) and the
> > resulting object code will remain exactly the same, byte-to-byte.
> > 
> > The soft_i2c.c source file makes ONE adapter. If we want 2 adapters we
> > should have 2 such files. If we want 4 adapters there should be 4 such
> > files. It is that simple.
> 
> 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.

The soft_i2c.c is _NOT_ a driver _SOURCE_. It is a _TEMPLATE_ that makes a
source from set of macros that come from an external file (in our case it is
board config file.) And that is a beauty of this driver -- one doesn't have
to write his own driver for each and every board; he just defines a set of
basic operations and CPP takes it from there.

And it can _NOT_ scale because it is impossible to make a generic driver
_SOURCE_ for each and every hardware configuration imaginable. That existing
soft_i2c.c makes _GENERATION_ of such a driver trivial. The only problem
with it is it only makes _ONE_ such interface.

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.

> > For the functions that can be parameterized I'm adding a simple wrapper
> > function that simply passes additional argument to that parameterizable
> > function and calls it. That is in no way worse, or bigger, or more complex
> > than accessing some pointer, then getting adapter number, then branching to
> > the appropriate function. The wrapper is simpler and smaller. Codewise it is
> > even smaller if all arguments fit into registers because when you are
> > preparing that actual function call from a wrapper function ALL arguments
> > are already loaded into appropriate registers; you only have to load one
> > additional register with an immediate constant (channel number) and perform
> > a branch.
> 
> 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?

> > Look what I do:
> > 
> > === Cut ===
> > static int _i2c_probe(chip, adap_no)
> > {
> > ...
> > }
> > 
> > static int xx_i2c_probe1(chip)
> > {
> > 	return _i2c_probe(chip, 0);
> > }
> > 
> > static int xx_i2c_probe2(chip)
> > {
> > 	return _i2c_probe(chip, 1);
> > }
> > === Cut ===
> 
> Looks like overhead to me.

Sure it is. There is no way how you can avoid overhead without writing a
totally custom U-Boot for each and every board. But it is no worse than
this:

=== Cut ===
static int _i2c_probe(chip)
{
	adap_no = ADAP(i2c_cur_bus)->hwadap_no;	
	....
}

xx_i2c_probe1 = xx_i2c_probe2 = _i2c_probe;
=== Cut ===

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(...);

and there is no Catch 22 with changing current bus number to initialize
adapter that requires that adapter must be initialized to change the current
bus number.

As a rule of thumb, member functions should _NEVER_ use any global
variables. It is up to higher layer to work on those if they are required.

---
******************************************************************
*  KSI at home    KOI8 Net  < >  The impossible we do immediately.  *
*  Las Vegas   NV, USA   < >  Miracles require 24-hour notice.   *
******************************************************************