[U-Boot] PATCH 3/8 Multi-adapter multi-bus I2C

[ksi at koi8.net]

On Mon, 9 Feb 2009, Heiko Schocher wrote:

> Hello ksi,
> 
> ksi at koi8.net wrote:
> > Signed-off-by: Sergey Kubushyn <ksi at koi8.net>      
> > ---
> > diff --git a/drivers/i2c/soft_i2c.c b/drivers/i2c/soft_i2c.c
> > index da6cec1..f0c1771 100644
> > --- a/drivers/i2c/soft_i2c.c
> > +++ b/drivers/i2c/soft_i2c.c
> > @@ -1,4 +1,8 @@
> >  /*
> > + * Copyright (c) 2009 Sergey Kubushyn <ksi at koi8.net>
> > + *
> > + * Changes for multibus/multiadapter I2C support.
> > + *
> >   * (C) Copyright 2001, 2002
> >   * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
> >   *
> > @@ -72,375 +76,493 @@ DECLARE_GLOBAL_DATA_PTR;
> >  #define PRINTD(fmt,args...)
> >  #endif
> >  
> > -#if defined(CONFIG_I2C_MULTI_BUS)
> > -static unsigned int i2c_bus_num __attribute__ ((section (".data"))) = 0;
> > -#endif /* CONFIG_I2C_MULTI_BUS */
> > -
> >  /*-----------------------------------------------------------------------
> >   * Local functions
> >   */
> > -#if !defined(CONFIG_SYS_I2C_INIT_BOARD)
> > -static void  send_reset	(void);
> > +#ifndef I2C_INIT
> > +#define I2C_INIT	do {} while(0)
> >  #endif
> > -static void  send_start	(void);
> > -static void  send_stop	(void);
> > -static void  send_ack	(int);
> > -static int   write_byte	(uchar byte);
> > -static uchar read_byte	(int);
> > -
> > -#if !defined(CONFIG_SYS_I2C_INIT_BOARD)
> > -/*-----------------------------------------------------------------------
> > - * Send a reset sequence consisting of 9 clocks with the data signal high
> > - * to clock any confused device back into an idle state.  Also send a
> > - * <stop> at the end of the sequence for belts & suspenders.
> > - */
> > -static void send_reset(void)
> > -{
> > -	I2C_SOFT_DECLARATIONS	/* intentional without ';' */
> > -	int j;
> > -
> > -	I2C_SCL(1);
> > -	I2C_SDA(1);
> > -#ifdef	I2C_INIT
> > -	I2C_INIT;
> > +#ifndef I2C_INIT0
> > +#define I2C_INIT0	do {} while(0)
> >  #endif
> > -	I2C_TRISTATE;
> > -	for(j = 0; j < 9; j++) {
> > -		I2C_SCL(0);
> > -		I2C_DELAY;
> > -		I2C_DELAY;
> > -		I2C_SCL(1);
> > -		I2C_DELAY;
> > -		I2C_DELAY;
> > -	}
> > -	send_stop();
> > -	I2C_TRISTATE;
> > -}
> > +#ifndef I2C_INIT1
> > +#define I2C_INIT1	do {} while(0)
> > +#endif
> > +#ifndef I2C_INIT2
> > +#define I2C_INIT2	do {} while(0)
> >  #endif
> > +#ifndef I2C_INIT3
> > +#define I2C_INIT3	do {} while(0)
> > +#endif
> > +
> >  
> >  /*-----------------------------------------------------------------------
> >   * START: High -> Low on SDA while SCL is High
> >   */
> > -static void send_start(void)
> > -{
> > -	I2C_SOFT_DECLARATIONS	/* intentional without ';' */
> > -
> > -	I2C_DELAY;
> > -	I2C_SDA(1);
> > -	I2C_ACTIVE;
> > -	I2C_DELAY;
> > -	I2C_SCL(1);
> > -	I2C_DELAY;
> > -	I2C_SDA(0);
> > -	I2C_DELAY;
> > +#define I2C_SOFT_SEND_START(n) \
> > +static void send_start##n(void) \
> > +{ \
> > +	I2C_SOFT_DECLARATIONS##n \
> > +	I2C_DELAY##n; \
> > +	I2C_SDA##n(1); \
> > +	I2C_ACTIVE##n; \
> > +	I2C_DELAY##n; \
> > +	I2C_SCL##n(1); \
> > +	I2C_DELAY##n; \
> > +	I2C_SDA##n(0); \
> > +	I2C_DELAY##n; \
> >  }
> >   
> [...]
> > -	PRINTD("i2c_read: fix addr_overflow: chip %02X addr %02X\n",
> > -		chip, addr);
> > -#endif
> > +#define	DO_EEAD_OVF	chip |= ((addr >> (alen * 8)) & CONFIG_SYS_I2C_EEPROM_ADDR_OVERFLOW); \
> >   
> 
> Line too long.

No problems, fixed.

> > +			PRINTD("%s: fix addr_overflow: chip %02X addr %02X\n", \
> > +				__FUNCTION__, chip, addr);
> >  
> > -	/*
> > -	 * Do the addressing portion of a write cycle to set the
> > -	 * chip's address pointer.  If the address length is zero,
> > -	 * don't do the normal write cycle to set the address pointer,
> > -	 * there is no address pointer in this chip.
> >   
> [...]
> > +#if defined(I2C_SOFT_DECLARATIONS3)
> > +I2C_SOFT_SEND_START(3)
> > +I2C_SOFT_SEND_STOP(3)
> > +I2C_SOFT_SEND_ACK(3)
> > +#if !defined(CONFIG_SYS_I2C_INIT_BOARD)
> > +I2C_SOFT_SEND_RESET(3)
> > +#endif
> > +I2C_SOFT_WRITE_BYTE(3)
> > +I2C_SOFT_READ_BYTE(3)
> > +I2C_SOFT_INIT_ADAPTER(3)
> > +I2C_SOFT_PROBE(3)
> > +I2C_SOFT_READ(3)
> > +I2C_SOFT_WRITE(3)
> > +I2C_SOFT_GET_BUS_SPEED(3)
> > +I2C_SOFT_SET_BUS_SPEED(3)
> > +#endif
> > +
> >   
> 
> Hmm... are this lots of defines really necessary? Couldn't we add
> something like
> 
> int    hw_adapnr; /* hardware adapter number */
> 
> to the i2c_adap_t struct, and have an pointer (cur_i2c_adap?) to the
> current used i2c_adap_t? Then you have where you need it, your
> hw_adapnr and need not all of this defines.
> 
> For example you need in the config for MPC8548CDS.h just this define:
> 
> #define I2C_SDA(bit)	(printf("HW adap: %d sda: %d", cur_i2c_adap->hw_adapnr, bit))
> 
> and not  I2C_SDA(bit) and I2C_SDA1(bit)

Eh, those are _NOT_ defines, those are _INSTANCES_.

First of all, you need real functions to make pointers to them at compile
time.

Second, SOFT_I2C is special; it is _NOT_ possible to make generic
paratemerized functions. Each, e.g., I2C_SDA is different and those config
file defines are _MACROS_, not defines. One can have one I2C_SOFT adapter
made of a couple of on-SoC GPIOs and another one constructed of, e.g.,
unused GPIOs from PCI bridge or whatever. That means that _ALL_ those I2C_*
macros will be totally different for those 2 adapters thus making 2 sets of
access functions that have absolutely nothing in common. You can not
parameterize this...

I agree that those 4 #ifdef'ed instances are not the prettiest and 4 is an
arbitrary number but I'm not THAT good in CPP trickery to come up with a
generic template that would be good for arbitrary number of instances if it
can be done at all...

And that template allows for using existing SOFT_I2C macros in existing
config files without any changes to them.

Also let's not forget that all those function sets are instantiated at
_COMPILE_ time so they can be run from ROM.

I would like to hear suggestions on that from real CPP gurus. That would've
made the code prettier and I would've learned new neat tricks...

---
******************************************************************
*  KSI at home    KOI8 Net  < >  The impossible we do immediately.  *
*  Las Vegas   NV, USA   < >  Miracles require 24-hour notice.   *
******************************************************************