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

ksi at koi8.net ksi at koi8.net
Thu Feb 19 23:26:44 CET 2009


On Thu, 19 Feb 2009, Wolfgang Denk wrote:

> Dear ksi at koi8.net,
> 
> In message <Pine.LNX.4.64ksi.0902191141090.18501 at home-gw.koi8.net> you wrote:
> > 
> > > in board config file ... OK, we are worser against your approach, because
> > > we have for all I2C_SDA, I2C_SCL accesses + 1 switch, but I don;t think
> > > this is such a problem.
> > 
> > First of all, you are using an external global variable for object methods.
> > That is a VERY BAD practice and I can't even imagine a use case that would
> > justify this.
> 
> We are pretty pragmatic here. If it solves a problem efficiently, we
> use even global variables.

But not in object methods... And it must serve some purpose. I'm a sinner
myself and I have to confess to even using goto's sometimes but it must have
some reason...

> > That means you'll have to rewrite the entire U-Boot. 99% of the boards have
> > only one bus so they did not switch busses. That means they never called
> > that i2c_set_bus_num() relying on i2c_init() in libxxx/board.c instead.
> 
> I cannot follow your argument.
> 
> Yes, the status quo is as you describe, it relies on  i2c_init()  and
> is   simple-minded   and  does  not  support  an  arbitry  number  of
> arbitrarily complex I2C bus trees and multiplexors and expanders  and
> what  else.  But  it  was  sufficient  for the first 10 years and 500
> boards of U-Boot development.
> 
> Now we are discussion a major redesign, so what is the big problem of
> changing this part? "rewrite the entire U-Boot"? Please stay serious.
> Compared to the other changes you suggest, this is  not  that  big  a
> part.

No, my changes are limited. Look, somebody must initialize an adapter. As
for now it is done with a single i2c_init() usually in libxxx/board.c. Then
the entire code assumes adapter is already initialized and just issues
i2c_read/write() as it see fits. 99% of this code is written on assumption
that there is only one I2C bus so it doesn't use i2c_set_bus_num() or
whatever, it just fires up i2c_read() and that's it.

This would perfectly work with my changes without modifying that code -- the
only bus is bus number 0 so there is nothing wrong with not setting the bus
for each I2C access; it is already at that only bus.

Now, if we have adapter initialization moved to i2c_set_bus() all that code
will cease to work because i2c_set_bus() is never called thus adapter will
never be initialized and all those i2c_read() and friends will fail.

That means that we should read through each and every board's code to find
where i2c functions are used and add i2c_set_bus() calls as needed. That is
not INSTEAD of that big rewrite, that is _IN ADDITION_ to it. That is a very
sizeable chunk of additional changes.

I DID think of adding adapter initialization to i2c_set_bus() initially but
then it turned out it generated more problems than it solved (and it solved
none) so I dropped that idea.

> > Sorry guys, I do not have THAT much free time that my employer would let me
> > to spend on this.
> 
> Well, you at least have some commercial motivation to spend time  for
> this code and discussion, while for me it's all my "free" time (and I
> better  don't  tell you what my wife says of my interpretation "free"
> here).

Eh, you won't believe what constitutes my "free" time and for how long ahead
that "free" time is planned out. I don't think mere mortals live that
long... :)

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


More information about the U-Boot mailing list