On Tue, 16 Dec 2008, Timur Tabi wrote:
Wolfgang Denk wrote:
Hm... what exactly is broken with the concept of having a "current device" or a "current bus"? We use it elasewhere, too (like for selection IDE or S-ATA devices and such), and so far I am not aware of fundamental issues because of that.
I think it's a kludge because you have to set the current device before you can access it. It seems ridiculous that you have to do this:
i2c_set_bus_num(x) i2c_write(...)
when you could do this:
i2c_write(x, ...)
Only if you have more than one I2C bus that 95% of supported boards don't have (95% is a wild guess; in reality, methinks, it is even closer to 99%.) That means you should only do this for less than 5% of existing boards.
Adding a parameter would require you to make changes for 100% of boards.
common/cmd_i2c.c is a single file, common for all platforms so it is a special case.
Sounds to me like you haven't really looked at the U-Boot code.
There are
plenty of places where one function does I2C operations, then calls another function that does its own.
Really? Where exactly does such a thing happen?
Well, I can't find a real example, but here's something close. Take a look at function pib_init() in mpc8568mds.c. This function needs to talk to the 2nd I2C bus. It has no idea who called it, so it has no idea what the current I2C bus is. Therefore, it has to save and restore it.
Now consider the code that calls pib_init(). Technically, this code cannot know that pib_init() does I2C operations. So it doesn't know that pib_init() could change the current I2C bus. So it wouldn't know to save and restore what it needs.
This situation could happen anywhere.
No, that is not an issue. That pib_init() does not happen out of a blue, it is YOU who call it in $(BOARD).c file so you MUST know what you are doing.
I tend to call this a bug that needs to be fixed, if there is ssuch code.
It's not a bug. Function X does I2C operations, and function Y also does I2C operations, but on a different device. If function X calls function Y, then Y needs to save and restore the current bus number. You will never get rid of this requirement as long as you have the concept of a current I2C bus number.
So you're still going to need i2c_get_bus_num() and i2c_set_bus_num(), and you're still going to have code like this:
bus = i2c_get_bus_num(); i2c_set_bus_num(x); i2c_write(...) i2c_set_bus_num(bus);
This statement is probably correct: I don't share your point of view here, either.
In that case, I have no interest in working on this new I2C design. You guys obviously have everything under control, so good luck.
-- Timur Tabi Linux Kernel Developer @ Freescale
--- ****************************************************************** * KSI@home KOI8 Net < > The impossible we do immediately. * * Las Vegas NV, USA < > Miracles require 24-hour notice. * ******************************************************************