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

[ksi at koi8.net]

On Wed, 18 Feb 2009, Heiko Schocher wrote:

> Hello ksi,
> 
> ksi at koi8.net wrote:
> > On Wed, 18 Feb 2009, Wolfgang Denk wrote:
> > 
> >> Dear ksi at koi8.net,
> >>
> >> In message <Pine.LNX.4.64ksi.0902171456520.30777 at home-gw.koi8.net> you wrote:
> [...]
> >>> OK, this is not about using multiple I2C busses before relocation and using
> >>> them right now for any of existing boards (some of which are actually dead
> >>> or vaporware.)
> >> ...
> >>
> >> We agree that there are cases where multiple busses are needed. But do
> >> we need such complexity before relocation?
> > 
> > It is not before relocation. I don't see a reason to do this before
> > relocation either. But if we want to have such a possibility after
> > relocation we also need a mechanism to do this.
> 
> But, if this current pointer is writeable, it is also usable before relocation!

Once again -- it is not the pointer itself that should be writeable, it is
what it is pointing to.

> > And it is not all that complex, I can't understand why you think it is.
> 
> We actually mix things:
> 
> - complex: There we (Wolfgang and I) talk about your implentation
>   of the bitbang driver
> 
> not about your i2c-core ...

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.

> > As for now we have a set of regular i2c functions (i2c_init/i2c_read/etc.)
> > in each and every i2c driver. Those functions are exported so when we pick
> > up a driver (with e.g. CONFIG_HARD_I2C that metastasize through the entire
> > U-Boot source choosing different drivers for different platforms) those
> > functions get called where i2c_read() etc. are used.
> > 
> > I make all those functions static so they are not exported and add a simple
> > exported structure with pointers to those functions. Then there is one
> > wrapper, i2c_core.c that holds global bus number and exports that usual set
> > of i2c_read() and friends. Those functions in turn just play a dispatcher
> > role calling appropriate functions from an array of those driver structures
> > using current bus number as an index into that array. There is nothing more
> > complex than that.
> 
> Perfectly.
> 
> > The i2c_set_bus_number() is a bit more complex than just changing that
> > global variable to accomodate multiplexed busses but not rocket science
> > either -- if the current bus is multiplexed, it switches off all the muxes
> > in the path starting with the farthest one (if it is multihop, but I doubt
> > we'll see such a beast so it will be just one chip) and turn on the muxes
> > for the new bus (if it is multiplexed) starting with the closest one. The
> > companion i2c_get_bus_num() just returns that global variable.
> 
> Ok.
> 
> > What is that complex with such a design?
> 
> Nothing. And there is not even more complexity, when we add a
> current pointer, or? We just can use this current pointer, to make
> driver code more simpler by using this current pointer and hwadapnr ...

It will NOT make code any simpler. And it will NOT make it any smaller.

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.

Your current pointer adds additional member to the adapter structure and lot
of complexity for such an easy task.

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 ===

Is it any bigger or more complex than using additional structure members,
accessing those structure and doing other work? And it does NOT require
anything to be writeable to call a function from appropriate adapter.

What complexity are you talking about, guys?

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