[U-Boot] [PATCH v2 2/2] arm nomadik: add i2c

[Wolfgang Denk <wd@denx.de>]

Dear Jean-Christophe PLAGNIOL-VILLARD,

In message <20090727200408.GC5259@game.jcrosoft.org> you wrote:
>
> > @@ -110,6 +110,22 @@
> >  #define CONFIG_PL01x_PORTS	{ (void *)CFG_SERIAL0, (void *)CFG_SERIAL1 }
> >  #define CONFIG_PL011_CLOCK	48000000
> >  
> > +/* i2c, for the port extenders (uses gpio.c in board directory) */
> > +#ifndef __ASSEMBLY__
> > +#include <asm/arch/gpio.h>
> I really do not like to include file from the config
> and it will be difficult to clean up when moving to Kconfig

But this is what many other boards are doing as well:

	$ grep '#[ 	]*include' include/configs/* | wc -l
	591

> > +#define CONFIG_CMD_I2C
> > +#define CONFIG_SOFT_I2C
> > +#define CONFIG_SYS_I2C_SPEED	400000
> > +#define __SDA			63
> > +#define __SCL			62
> > +#define I2C_SDA(x)		nmk_gpio_set(__SDA, x)
> > +#define I2C_SCL(x)		nmk_gpio_set(__SCL, x)
> > +#define I2C_READ		(nmk_gpio_get(__SDA)!=0)
> > +#define I2C_ACTIVE		nmk_gpio_dir(__SDA, 1)
> > +#define I2C_TRISTATE		nmk_gpio_dir(__SDA, 0)
> > +#define I2C_DELAY     (udelay(2))
> it will better to define a couple of generic function where we implement it
> as this

But that's how the CONFIG_SOFT_I2C interface is currently designed and
used by all other boards, too.

See for example here:

	$ grep I2C_ include/configs/TQM885D.h 
	#define CONFIG_SYS_I2C_SPEED            93000   /* 93 kHz is supposed to work   */
	#define CONFIG_SYS_I2C_SLAVE            0xFE
	#define I2C_INIT        (immr->im_cpm.cp_pbdir |=  PB_SCL)
	#define I2C_ACTIVE      (immr->im_cpm.cp_pbdir |=  PB_SDA)
	#define I2C_TRISTATE    (immr->im_cpm.cp_pbdir &= ~PB_SDA)
	#define I2C_READ        ((immr->im_cpm.cp_pbdat & PB_SDA) != 0)
	#define I2C_SDA(bit)    if(bit) immr->im_cpm.cp_pbdat |=  PB_SDA; \
	#define I2C_SCL(bit)    if(bit) immr->im_cpm.cp_pbdat |=  PB_SCL; \
	#define I2C_DELAY       udelay(2)       /* 1/4 I2C clock duration */
	#define CONFIG_SYS_I2C_EEPROM_ADDR      0x50            /* EEPROM AT24C??       */
	#define CONFIG_SYS_I2C_EEPROM_ADDR_LEN 2                /* two byte address     */
	# define CONFIG_SYS_I2C_RTC_ADDR 0x68

We should not attempt to change the design of the soft-I2C driver at
this point in a release cycle.

If you care about these things, you should have posted such comments
when Heiko asked for comments more than 4 months ago, see 
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/56416

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If a train station is a place where a train stops,
                                           then what's a workstation?