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
--
Best regards,
Lukasz Majewski
Samsung Poland R&D Center | Linux Platform Group
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
