Hello ksi,
ksi@koi8.net wrote:
On Fri, 13 Feb 2009, Heiko Schocher wrote:
ksi@koi8.net wrote:
Here is the second attempt for initial portion of multibus/multiadapter I2C support.
Can you please send your patches with some better commit messages. You only send your Signed-off-by, without any explanation. Please change this.
There is not much sense in extensive commit messages in this case, IMHO. It is not a bug fix or added feature at one particular place; it is a major rework. The only message I can give is something like "Changes for multiadapter/multibus I2C support..."
I'll add it to the second attempt that I will make later today.
This includes a set of common files, all drivers in drivers/i2c and all boards affected by these changes (config files, board files, and lib_xx files.)
There is an illustrative example of multiadapter multibus I2C config in MPC8548CDS.h config file (#if 0'd.) Definitions in that example are bogus so please don't expect it to work. It will compile though...
This set also includes big rework for soft_i2c.c that makes it template version that allows up to 4 bitbanged adapters. This number can be
Didn;t you try my suggestion? This is a really big define monster now, which I think, we can avoid, and without to change nearly all lines of the existing driver.
We can not avoid it. At least I can not see an easy way to do this. SOFT_I2C
Yes, we can. Look again deeper in my approach, this _is_ an easy way!
I also looked again in your changes in the fsl_i2c.c driver, and we can make this also simplier, by using cur_adap_nr->hwadapnr!
We have not to define for both hardware adapter each function in i2c_adap_t! For example:
You did: static void fsl_i2c1_init(int speed, int slaveadd) { __i2c_init(0, speed, slaveadd); }
instead we only need
i2c_init(cur_adap_nr->hwadapnr, speed, slaveadd);
with
i2c_adap_t fsl_i2c_adap[] = { { .init = i2c_init, [...] .hwadapnr = 0, .name = FSL_NAME(CONFIG_SYS_FSL_I2C_OFFSET) }, #ifdef CONFIG_SYS_FSL_I2C2_OFFSET { .init = i2c_init, [...] .hwadapnr = 1, .name = FSL_NAME(CONFIG_SYS_FSL_I2C2_OFFSET) }, #endif
Please change this driver also!
If I think more, we never even need to change the function parameters like you did for example for i2c_int ()! We can use at the beginning of every function who go in i2c_adap_t, the "cur_adap_nr->hwadapnr" and make the settings we need for this function... and wow we saved one function parameter.
is special. Those multiple e.g. MXC or OMAP3 adapters can be parameterized because different channels do only differ in their base address that can be made into a parameter. Software I2C is totally different because it has
Why is this different? If you change a base or the way to the pins?
totally different functions for different channels, there is nothing we can
Think about my explanation to the soft_i2c.c driver in previous EMail and above function.
It also works!
make into a parameter. All those I2C_SDA etc. are NOT DEFINES, they are MACROS. Every function for every channel is built of those macros that can
I know this in your approach, but we _don;t_ need this. We simply can make one "common" board function and switch in this function dependent on the "cur_adap_nr->hwadapnr" to the particular GPIO pin functions, wherever the are!
be absolutely different for each channel. They define NOT some PARAMETERS but function TEXT that will be compiled into executable code.
And this additional TEXT I save too! I think, you think too much in C++?
This is how it is done in Linux kernel (see e.g. drivers/hwmon/lm93.c.)
Thats no argument.
bye Heiko