Dear Jean-Christophe PLAGNIOL-VILLARD,

In message <20090727200408.gc5...@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: w...@denx.de
If a train station is a place where a train stops,
                                           then what's a workstation?
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to