Hi Heiko,
#if defined(CONFIG_I2C_MULTI_BUS) static unsigned int i2c_bus_num __attribute__ ((section (".data"))) = 0; +const char *soft_i2c_name[CONFIG_SYS_MAX_I2C_BUS] = {
- NULL,
- NULL,
- NULL,
- NULL,
- "soft_i2c_4",
- "soft_i2c_5",
- NULL,
+};
For what do you need "soft_i2c_name"? I see no usage of this in your patchset? Also why the "NULL" for 0-3 and 6 positions?
And what is, if CONFIG_SYS_MAX_I2C_BUS is < 7 ?
Indeed this can be removed - it is not needed (for now).
+#if defined(CONFIG_I2C_MULTI_BUS) +/* Handle multiple I2C buses instances */ +int get_multi_scl_pin(void) +{
- switch (I2C_GET_BUS()) {
- case I2C_4:
return CONFIG_SOFT_I2C_I2C4_SCL;- case I2C_5:
return CONFIG_SOFT_I2C_I2C5_SCL;- };
- return 0;
+}
+int get_multi_sda_pin(void) +{
- switch (I2C_GET_BUS()) {
- case I2C_4:
return CONFIG_SOFT_I2C_I2C4_SDA;- case I2C_5:
return CONFIG_SOFT_I2C_I2C5_SDA;- };
- return 0;
+}
+int multi_i2c_init(void) +{
- return 0;
+} +#endif /* CONFIG_I2C_MULTI_BUS */
Again, what is with busnr = 0-3 or 6?
This is not needed in the i2c soft file. You can define this functions board specific ... so, no change in the driver is needed ... please move this to board specific code.
Please consider, that get_multi_{sda|scl}_pin can be used by other boards. Those are written in a generic way (by calling I2C_GET_BUS()).
What here I'm trying to avoid is the code duplication for each board (e.g. Samsung's GONI, Universal, Trats, Origen ... etc). I can agree, that now only I2C_{4|5} are defined (since for now Samsung is using I2C_4 and I2C_5).
But other cases can be also defined.
What I see even more important is a definition of (at <i2c.h>):
+#if (defined(CONFIG_SOFT_I2C)&& defined(CONFIG_I2C_MULTI_BUS)) +enum { + I2C_0, + I2C_1, + I2C_2, + I2C_3, + I2C_4, + I2C_5, + I2C_6 +};
since this will organize the order of multiple (soft) I2C devices.
Imagine that 2 PMICs are on the board (I2C_4 and I2C_5). I need to distinct those (when calling I2C_SET|GET_BUS) And then support for another I2C device (e.g. I2C_2) at other subsystem is provided. Then I can: 1. Add common definition of I2C_X (as I've proposed) to <i2c.h> 2. Add #define I2C_X on the ./include/configs/{e.g. trats}.h board.
For second approach used I need to duplicate the code for other targets (goni, universal, origen) when needed and I cannot avoid that someone else will define other names -> like #define MINE_I2C_X on his/her ./include/configs/{board}.h