On Thu, 19 Feb 2009, Wolfgang Denk wrote:
Dear ksi@koi8.net,
In message Pine.LNX.4.64ksi.0902191141090.18501@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@home KOI8 Net < > The impossible we do immediately. * * Las Vegas NV, USA < > Miracles require 24-hour notice. * ******************************************************************