Nishanth,
This approach sounds good. The only down-side that I can see is that the driver needs to keep track of context. Personally, I prefer self-contained code whenever possible, but in this case your approach is better.
In order to do this properly, we should add to the base API in include/i2c.h something like:
void i2c_set_bus(uchar dev); uchar i2c_get_bus(void); void i2c_set_bus_speed(int speed); int i2c_get_bus_speed(void);
or merge the index and speed as you have in your code.
The drivers can then either choose to implement or not implement this feature, and we can add a parallel command tree based on 'i2c' as Wolfgang has suggested.
Thoughts?
Ben
On Tue, 2006-05-16 at 10:10 -0500, Menon, Nishanth wrote:
Hi Ben, Some reasons why I am against the approach of giving i2c bus along with the command itself:
- Cannot handle scenarios of HS controllers.
- cmd_i2c is a debug interface, once a bus is selected, we would like
to do read and write operations to a specific chip. To repeat the bus number all the time is a overhead. 3. To a smaller extent, older users need to be aware of the change involved. 4. a simpler and lesser intrusive implementation adding an additional command to select bus can achieve the same functionality. (I send a previous mail with this).
Attached is a patch to common/cmd_i2c.c that allows access to two I2C controllers on a board. Note that this doesn't actually change any
- importantly, common/cmd_i2c.c is a bad place to limit i2c busses..
each board/.. file should be given the functionality. Regards, Nishanth Menon