public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] PATCH 3/8 Multi-adapter multi-bus I2C
@ 2009-02-07  1:05 ksi at koi8.net
  2009-02-09 12:21 ` Heiko Schocher
  0 siblings, 1 reply; 4+ messages in thread
From: ksi at koi8.net @ 2009-02-07  1:05 UTC (permalink / raw)
  To: u-boot

Signed-off-by: Sergey Kubushyn <ksi@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@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; \
 }
 
+
 /*-----------------------------------------------------------------------
  * STOP: Low -> High on SDA while SCL is High
  */
-static void send_stop(void)
-{
-	I2C_SOFT_DECLARATIONS	/* intentional without ';' */
-
-	I2C_SCL(0);
-	I2C_DELAY;
-	I2C_SDA(0);
-	I2C_ACTIVE;
-	I2C_DELAY;
-	I2C_SCL(1);
-	I2C_DELAY;
-	I2C_SDA(1);
-	I2C_DELAY;
-	I2C_TRISTATE;
+#define I2C_SOFT_SEND_STOP(n) \
+static void send_stop##n(void) \
+{ \
+	I2C_SOFT_DECLARATIONS##n \
+	I2C_SCL##n(0); \
+	I2C_DELAY##n; \
+	I2C_SDA##n(0); \
+	I2C_ACTIVE##n; \
+	I2C_DELAY##n; \
+	I2C_SCL##n(1); \
+	I2C_DELAY##n; \
+	I2C_SDA##n(1); \
+	I2C_DELAY##n; \
+	I2C_TRISTATE##n; \
 }
 
 
 /*-----------------------------------------------------------------------
  * ack should be I2C_ACK or I2C_NOACK
  */
-static void send_ack(int ack)
-{
-	I2C_SOFT_DECLARATIONS	/* intentional without ';' */
-
-	I2C_SCL(0);
-	I2C_DELAY;
-	I2C_ACTIVE;
-	I2C_SDA(ack);
-	I2C_DELAY;
-	I2C_SCL(1);
-	I2C_DELAY;
-	I2C_DELAY;
-	I2C_SCL(0);
-	I2C_DELAY;
+#define I2C_SOFT_SEND_ACK(n) \
+static void send_ack##n(int ack) \
+{ \
+	I2C_SOFT_DECLARATIONS##n \
+	I2C_SCL##n(0); \
+	I2C_DELAY##n; \
+	I2C_ACTIVE##n; \
+	I2C_SDA##n(ack); \
+	I2C_DELAY##n; \
+	I2C_SCL##n(1); \
+	I2C_DELAY##n; \
+	I2C_DELAY##n; \
+	I2C_SCL##n(0); \
+	I2C_DELAY##n; \
 }
 
 
 /*-----------------------------------------------------------------------
- * Send 8 bits and look for an acknowledgement.
- */
-static int write_byte(uchar data)
-{
-	I2C_SOFT_DECLARATIONS	/* intentional without ';' */
-	int j;
-	int nack;
-
-	I2C_ACTIVE;
-	for(j = 0; j < 8; j++) {
-		I2C_SCL(0);
-		I2C_DELAY;
-		I2C_SDA(data & 0x80);
-		I2C_DELAY;
-		I2C_SCL(1);
-		I2C_DELAY;
-		I2C_DELAY;
-
-		data <<= 1;
-	}
-
-	/*
-	 * Look for an <ACK>(negative logic) and return it.
-	 */
-	I2C_SCL(0);
-	I2C_DELAY;
-	I2C_SDA(1);
-	I2C_TRISTATE;
-	I2C_DELAY;
-	I2C_SCL(1);
-	I2C_DELAY;
-	I2C_DELAY;
-	nack = I2C_READ;
-	I2C_SCL(0);
-	I2C_DELAY;
-	I2C_ACTIVE;
-
-	return(nack);	/* not a nack is an ack */
-}
-
-#if defined(CONFIG_I2C_MULTI_BUS)
-/*
- * Functions for multiple I2C bus handling
+ * 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.
  */
-unsigned int i2c_get_bus_num(void)
-{
-	return i2c_bus_num;
+#define I2C_SOFT_SEND_RESET(n) \
+static void send_reset##n(void) \
+{ \
+	I2C_SOFT_DECLARATIONS##n \
+	int j; \
+	I2C_SCL##n(1); \
+	I2C_SDA##n(1); \
+	I2C_INIT##n; \
+	I2C_TRISTATE##n; \
+	for(j = 0; j < 9; j++) { \
+		I2C_SCL##n(0); \
+		I2C_DELAY##n; \
+		I2C_DELAY##n; \
+		I2C_SCL##n(1); \
+		I2C_DELAY##n; \
+		I2C_DELAY##n; \
+	} \
+	send_stop##n(); \
+	I2C_TRISTATE##n; \
 }
 
-int i2c_set_bus_num(unsigned int bus)
-{
-#if defined(CONFIG_I2C_MUX)
-	if (bus < CONFIG_SYS_MAX_I2C_BUS) {
-		i2c_bus_num = bus;
-	} else {
-		int	ret;
-
-		ret = i2x_mux_select_mux(bus);
-		if (ret == 0)
-			i2c_bus_num = bus;
-		else
-			return ret;
-	}
-#else
-	if (bus >= CONFIG_SYS_MAX_I2C_BUS)
-		return -1;
-	i2c_bus_num = bus;
-#endif
-	return 0;
-}
 
-/* TODO: add 100/400k switching */
-unsigned int i2c_get_bus_speed(void)
-{
-	return CONFIG_SYS_I2C_SPEED;
+/*-----------------------------------------------------------------------
+ * Send 8 bits and look for an acknowledgement.
+ */
+#define I2C_SOFT_WRITE_BYTE(n) \
+static int write_byte##n(uchar data) \
+{ \
+	I2C_SOFT_DECLARATIONS##n \
+	int j; \
+	int nack; \
+	I2C_ACTIVE##n; \
+	for(j = 0; j < 8; j++) { \
+		I2C_SCL##n(0); \
+		I2C_DELAY##n; \
+		I2C_SDA##n(data & 0x80); \
+		I2C_DELAY##n; \
+		I2C_SCL##n(1); \
+		I2C_DELAY##n; \
+		I2C_DELAY##n; \
+		data <<= 1; \
+	} \
+	I2C_SCL##n(0); \
+	I2C_DELAY##n; \
+	I2C_SDA##n(1); \
+	I2C_TRISTATE##n; \
+	I2C_DELAY##n; \
+	I2C_SCL##n(1); \
+	I2C_DELAY##n; \
+	I2C_DELAY##n; \
+	nack = I2C_READ##n; \
+	I2C_SCL##n(0); \
+	I2C_DELAY##n; \
+	I2C_ACTIVE##n; \
+	return(nack); \
 }
 
-int i2c_set_bus_speed(unsigned int speed)
-{
-	if (speed != CONFIG_SYS_I2C_SPEED)
-		return -1;
-
-	return 0;
-}
-#endif
 
 /*-----------------------------------------------------------------------
  * if ack == I2C_ACK, ACK the byte so can continue reading, else
  * send I2C_NOACK to end the read.
  */
-static uchar read_byte(int ack)
-{
-	I2C_SOFT_DECLARATIONS	/* intentional without ';' */
-	int  data;
-	int  j;
-
-	/*
-	 * Read 8 bits, MSB first.
-	 */
-	I2C_TRISTATE;
-	I2C_SDA(1);
-	data = 0;
-	for(j = 0; j < 8; j++) {
-		I2C_SCL(0);
-		I2C_DELAY;
-		I2C_SCL(1);
-		I2C_DELAY;
-		data <<= 1;
-		data |= I2C_READ;
-		I2C_DELAY;
-	}
-	send_ack(ack);
-
-	return(data);
+#define I2C_SOFT_READ_BYTE(n) \
+static uchar read_byte##n(int ack) \
+{ \
+	I2C_SOFT_DECLARATIONS##n \
+	int  data; \
+	int  j; \
+	I2C_TRISTATE##n; \
+	I2C_SDA##n(1); \
+	data = 0; \
+	for(j = 0; j < 8; j++) { \
+		I2C_SCL##n(0); \
+		I2C_DELAY##n; \
+		I2C_SCL##n(1); \
+		I2C_DELAY##n; \
+		data <<= 1; \
+		data |= I2C_READ##n; \
+		I2C_DELAY##n; \
+	} \
+	send_ack##n(ack); \
+	return(data); \
 }
 
-/*=====================================================================*/
-/*                         Public Functions                            */
-/*=====================================================================*/
 
-/*-----------------------------------------------------------------------
+/*============================================================*/
+/*                         I2C Ops                            */
+/*============================================================*/
+
+/*--------------------------------------------------------------
  * Initialization
  */
-void i2c_init (int speed, int slaveaddr)
-{
-#if defined(CONFIG_SYS_I2C_INIT_BOARD)
-	/* call board specific i2c bus reset routine before accessing the   */
-	/* environment, which might be in a chip on that bus. For details   */
-	/* about this problem see doc/I2C_Edge_Conditions.                  */
-	i2c_init_board();
+
+#ifdef CONFIG_SYS_I2C_INIT_BOARD
+#define I2C_SOFT_INIT_ADAPTER(n) \
+static void soft_i2c_init##n(int speed, int slaveaddr) \
+{ \
+	/* call board specific i2c bus reset routine before accessing the   */ \
+	/* environment, which might be in a chip on that bus. For details   */ \
+	/* about this problem see doc/I2C_Edge_Conditions.                  */ \
+	SOFT_I2C_INIT_BOARD; \
+}
 #else
-	/*
-	 * WARNING: Do NOT save speed in a static variable: if the
-	 * I2C routines are called before RAM is initialized (to read
-	 * the DIMM SPD, for instance), RAM won't be usable and your
-	 * system will crash.
-	 */
-	send_reset ();
-#endif
+#define I2C_SOFT_INIT_ADAPTER(n) \
+static void soft_i2c_init##n(int speed, int slaveaddr) \
+{ \
+	/* WARNING: Do NOT save speed in a static variable: if the	*/ \
+	/* I2C routines are called before RAM is initialized (to read	*/ \
+	/* the DIMM SPD, for instance), RAM won't be usable and your	*/ \
+	/* system will crash. */ \
+	send_reset##n(); \
 }
+#endif
+
 
 /*-----------------------------------------------------------------------
  * Probe to see if a chip is present.  Also good for checking for the
  * completion of EEPROM writes since the chip stops responding until
  * the write completes (typically 10mSec).
  */
-int i2c_probe(uchar addr)
-{
-	int rc;
-
-	/*
-	 * perform 1 byte write transaction with just address byte
-	 * (fake write)
-	 */
-	send_start();
-	rc = write_byte ((addr << 1) | 0);
-	send_stop();
-
-	return (rc ? 1 : 0);
+#define I2C_SOFT_PROBE(n) \
+static int soft_i2c_probe##n(uchar addr) \
+{ \
+	int rc; \
+	/* perform 1 byte write transaction with just address byte */ \
+	/* (fake write) */ \
+	send_start##n(); \
+	rc = write_byte##n((addr << 1) | 0); \
+	send_stop##n(); \
+	return (rc ? 1 : 0); \
 }
 
+
 /*-----------------------------------------------------------------------
  * Read bytes
  */
-int  i2c_read(uchar chip, uint addr, int alen, uchar *buffer, int len)
-{
-	int shift;
-	PRINTD("i2c_read: chip %02X addr %02X alen %d buffer %p len %d\n",
-		chip, addr, alen, buffer, len);
+#ifdef CONFIG_SOFT_I2C_READ_REPEATED_START
+#define RRSS_STOP	0
+#else
+#define RRSS_STOP	1
+#endif
 
 #ifdef CONFIG_SYS_I2C_EEPROM_ADDR_OVERFLOW
-	/*
-	 * EEPROM chips that implement "address overflow" are ones
-	 * like Catalyst 24WC04/08/16 which has 9/10/11 bits of
-	 * address and the extra bits end up in the "chip address"
-	 * bit slots. This makes a 24WC08 (1Kbyte) chip look like
-	 * four 256 byte chips.
-	 *
-	 * Note that we consider the length of the address field to
-	 * still be one byte because the extra address bits are
-	 * hidden in the chip address.
-	 */
-	chip |= ((addr >> (alen * 8)) & CONFIG_SYS_I2C_EEPROM_ADDR_OVERFLOW);
-
-	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); \
+			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.
-	 */
-	send_start();
-	if(alen > 0) {
-		if(write_byte(chip << 1)) {	/* write cycle */
-			send_stop();
-			PRINTD("i2c_read, no chip responded %02X\n", chip);
-			return(1);
-		}
-		shift = (alen-1) * 8;
-		while(alen-- > 0) {
-			if(write_byte(addr >> shift)) {
-				PRINTD("i2c_read, address not <ACK>ed\n");
-				return(1);
-			}
-			shift -= 8;
-		}
-
-		/* Some I2C chips need a stop/start sequence here,
-		 * other chips don't work with a full stop and need
-		 * only a start.  Default behaviour is to send the
-		 * stop/start sequence.
-		 */
-#ifdef CONFIG_SOFT_I2C_READ_REPEATED_START
-		send_start();
 #else
-		send_stop();
-		send_start();
+#define	DO_EEAD_OVF
 #endif
-	}
-	/*
-	 * Send the chip address again, this time for a read cycle.
-	 * Then read the data.  On the last byte, we do a NACK instead
-	 * of an ACK(len == 0) to terminate the read.
-	 */
-	write_byte((chip << 1) | 1);	/* read cycle */
-	while(len-- > 0) {
-		*buffer++ = read_byte(len == 0);
-	}
-	send_stop();
-	return(0);
+
+#define I2C_SOFT_READ(n) \
+static int soft_i2c_read##n(uchar chip, uint addr, int alen, uchar *buffer, int len) \
+{ \
+	int shift; \
+	PRINTD("%s: chip %02X addr %02X alen %d buffer %p len %d\n", \
+		__FUNCTION__, chip, addr, alen, buffer, len); \
+	/* EEPROM chips that implement "address overflow" are ones	*/ \
+	/* like Catalyst 24WC04/08/16 which has 9/10/11 bits of		*/ \
+	/* address and the extra bits end up in the "chip address"	*/ \
+	/* bit slots. This makes a 24WC08 (1Kbyte) chip look like	*/ \
+	/* four 256 byte chips. */ \
+	/* */ \
+	/* Note that we consider the length of the address field to	*/ \
+	/* still be one byte because the extra address bits are		*/ \
+	/* hidden in the chip address. */ \
+	DO_EEAD_OVF \
+	/* 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. */ \
+	send_start##n(); \
+	if(alen > 0) { \
+		if(write_byte##n(chip << 1)) {	/* write cycle */ \
+			send_stop##n(); \
+			PRINTD("%s, no chip responded %02X\n", __FUNCTION__, chip); \
+			return(1); \
+		} \
+		shift = (alen-1) * 8; \
+		while(alen-- > 0) { \
+			if(write_byte##n(addr >> shift)) { \
+				PRINTD("%s, address not <ACK>ed\n", __FUNCTION__); \
+				return(1); \
+			} \
+			shift -= 8; \
+		} \
+		if(RRSS_STOP) { \
+			send_stop##n();	/* reportedly some chips need a full stop */ \
+		} \
+		send_start##n(); \
+	} \
+	/* Send the chip address again, this time for a read cycle.	*/ \
+	/* Then read the data.  On the last byte, we do a NACK instead	*/ \
+	/* of an ACK(len == 0) to terminate the read. */ \
+	write_byte##n((chip << 1) | 1);	/* read cycle */ \
+	while(len-- > 0) { \
+		*buffer++ = read_byte##n(len == 0); \
+	} \
+	send_stop##n(); \
+	return(0); \
 }
 
+
 /*-----------------------------------------------------------------------
  * Write bytes
  */
-int  i2c_write(uchar chip, uint addr, int alen, uchar *buffer, int len)
-{
-	int shift, failures = 0;
-
-	PRINTD("i2c_write: chip %02X addr %02X alen %d buffer %p len %d\n",
-		chip, addr, alen, buffer, len);
-
-	send_start();
-	if(write_byte(chip << 1)) {	/* write cycle */
-		send_stop();
-		PRINTD("i2c_write, no chip responded %02X\n", chip);
-		return(1);
-	}
-	shift = (alen-1) * 8;
-	while(alen-- > 0) {
-		if(write_byte(addr >> shift)) {
-			PRINTD("i2c_write, address not <ACK>ed\n");
-			return(1);
-		}
-		shift -= 8;
-	}
-
-	while(len-- > 0) {
-		if(write_byte(*buffer++)) {
-			failures++;
-		}
-	}
-	send_stop();
-	return(failures);
+#define I2C_SOFT_WRITE(n) \
+static int  soft_i2c_write##n(uchar chip, uint addr, int alen, uchar *buffer, int len) \
+{ \
+	int shift, failures = 0; \
+	PRINTD("%s: chip %02X addr %02X alen %d buffer %p len %d\n", \
+		__FUNCTION__, chip, addr, alen, buffer, len); \
+	send_start##n(); \
+	if(write_byte##n(chip << 1)) {	/* write cycle */ \
+		send_stop##n(); \
+		PRINTD("%s, no chip responded %02X\n", __FUNCTION__, chip); \
+		return(1); \
+	} \
+	shift = (alen-1) * 8; \
+	while(alen-- > 0) { \
+		if(write_byte##n(addr >> shift)) { \
+			PRINTD("%s, address not <ACK>ed\n", __FUNCTION__); \
+			return(1); \
+		} \
+		shift -= 8; \
+	} \
+	while(len-- > 0) { \
+		if(write_byte##n(*buffer++)) { \
+			failures++; \
+		} \
+	} \
+	send_stop##n(); \
+	return(failures); \
+}
+
+
+#define I2C_SOFT_GET_BUS_SPEED(n) \
+static unsigned int soft_i2c_get_bus_speed##n(void) \
+{ \
+	return CONFIG_SYS_SOFT_I2C##n##_SPEED; \
+}
+
+
+#define I2C_SOFT_SET_BUS_SPEED(n) \
+static unsigned int soft_i2c_set_bus_speed##n(unsigned int speed) \
+{ \
+	if (speed != CONFIG_SYS_SOFT_I2C##n##_SPEED) \
+		return -1; \
+	return 0; \
 }
+
+
+/*============================================================*/
+/*                  Here comes the real stuff                 */
+/*============================================================*/
+#if defined(I2C_SOFT_DECLARATIONS)
+I2C_SOFT_SEND_START()
+I2C_SOFT_SEND_STOP()
+I2C_SOFT_SEND_ACK()
+#if !defined(CONFIG_SYS_I2C_INIT_BOARD)
+I2C_SOFT_SEND_RESET()
+#endif
+I2C_SOFT_WRITE_BYTE()
+I2C_SOFT_READ_BYTE()
+I2C_SOFT_INIT_ADAPTER()
+I2C_SOFT_PROBE()
+I2C_SOFT_READ()
+I2C_SOFT_WRITE()
+I2C_SOFT_GET_BUS_SPEED()
+I2C_SOFT_SET_BUS_SPEED()
+#elif defined(I2C_SOFT_DECLARATIONS0)
+I2C_SOFT_SEND_START(0)
+I2C_SOFT_SEND_STOP(0)
+I2C_SOFT_SEND_ACK(0)
+#if !defined(CONFIG_SYS_I2C_INIT_BOARD)
+I2C_SOFT_SEND_RESET(0)
+#endif
+I2C_SOFT_WRITE_BYTE(0)
+I2C_SOFT_READ_BYTE(0)
+I2C_SOFT_INIT_ADAPTER(0)
+I2C_SOFT_PROBE(0)
+I2C_SOFT_READ(0)
+I2C_SOFT_WRITE(0)
+I2C_SOFT_GET_BUS_SPEED(0)
+I2C_SOFT_SET_BUS_SPEED(0)
+#endif
+
+#if defined(I2C_SOFT_DECLARATIONS1)
+I2C_SOFT_SEND_START(1)
+I2C_SOFT_SEND_STOP(1)
+I2C_SOFT_SEND_ACK(1)
+#if !defined(CONFIG_SYS_I2C_INIT_BOARD)
+I2C_SOFT_SEND_RESET(1)
+#endif
+I2C_SOFT_WRITE_BYTE(1)
+I2C_SOFT_READ_BYTE(1)
+I2C_SOFT_INIT_ADAPTER(1)
+I2C_SOFT_PROBE(1)
+I2C_SOFT_READ(1)
+I2C_SOFT_WRITE(1)
+I2C_SOFT_GET_BUS_SPEED(1)
+I2C_SOFT_SET_BUS_SPEED(1)
+#endif
+
+#if defined(I2C_SOFT_DECLARATIONS2)
+I2C_SOFT_SEND_START(2)
+I2C_SOFT_SEND_STOP(2)
+I2C_SOFT_SEND_ACK(2)
+#if !defined(CONFIG_SYS_I2C_INIT_BOARD)
+I2C_SOFT_SEND_RESET(2)
+#endif
+I2C_SOFT_WRITE_BYTE(2)
+I2C_SOFT_READ_BYTE(2)
+I2C_SOFT_INIT_ADAPTER(2)
+I2C_SOFT_PROBE(2)
+I2C_SOFT_READ(2)
+I2C_SOFT_WRITE(2)
+I2C_SOFT_GET_BUS_SPEED(2)
+I2C_SOFT_SET_BUS_SPEED(2)
+#endif
+
+#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
+
+i2c_adap_t	soft_i2c_adap[] = {
+	{
+#if defined(I2C_SOFT_DECLARATIONS)
+		.init		=	soft_i2c_init,
+		.probe		=	soft_i2c_probe,
+		.read		=	soft_i2c_read,
+		.write		=	soft_i2c_write,
+		.set_bus_speed	=	soft_i2c_set_bus_speed,
+		.get_bus_speed	=	soft_i2c_get_bus_speed,
+		.speed		=	CONFIG_SYS_SOFT_I2C_SPEED,
+		.slaveaddr	=	CONFIG_SYS_SOFT_I2C_SLAVE,
+		.init_done	=	0,
+		.name		=	"soft-i2c"
+#elif defined(I2C_SOFT_DECLARATIONS0)
+		.init		=	soft_i2c_init0,
+		.probe		=	soft_i2c_probe0,
+		.read		=	soft_i2c_read0,
+		.write		=	soft_i2c_write0,
+		.set_bus_speed	=	soft_i2c_set_bus_speed0,
+		.get_bus_speed	=	soft_i2c_get_bus_speed0,
+		.speed		=	CONFIG_SYS_SOFT_I2C0_SPEED,
+		.slaveaddr	=	CONFIG_SYS_SOFT_I2C0_SLAVE,
+		.init_done	=	0,
+		.name		=	"soft-i2c#0"
+#endif
+	},
+#if defined(I2C_SOFT_DECLARATIONS1)
+	{
+		.init		=	soft_i2c_init1,
+		.probe		=	soft_i2c_probe1,
+		.read		=	soft_i2c_read1,
+		.write		=	soft_i2c_write1,
+		.set_bus_speed	=	soft_i2c_set_bus_speed1,
+		.get_bus_speed	=	soft_i2c_get_bus_speed1,
+		.speed		=	CONFIG_SYS_SOFT_I2C1_SPEED,
+		.slaveaddr	=	CONFIG_SYS_SOFT_I2C1_SLAVE,
+		.init_done	=	0,
+		.name		=	"soft-i2c#1"
+	},
+#endif
+#if defined(I2C_SOFT_DECLARATIONS2)
+	{
+		.init		=	soft_i2c_init2,
+		.probe		=	soft_i2c_probe2,
+		.read		=	soft_i2c_read2,
+		.write		=	soft_i2c_write2,
+		.set_bus_speed	=	soft_i2c_set_bus_speed2,
+		.get_bus_speed	=	soft_i2c_get_bus_speed2,
+		.speed		=	CONFIG_SYS_SOFT_I2C2_SPEED,
+		.slaveaddr	=	CONFIG_SYS_SOFT_I2C2_SLAVE,
+		.init_done	=	0,
+		.name		=	"soft-i2c#2"
+	},
+#endif
+#if defined(I2C_SOFT_DECLARATIONS3)
+	{
+		.init		=	soft_i2c_init3,
+		.probe		=	soft_i2c_probe3,
+		.read		=	soft_i2c_read3,
+		.write		=	soft_i2c_write3,
+		.set_bus_speed	=	soft_i2c_set_bus_speed3,
+		.get_bus_speed	=	soft_i2c_get_bus_speed3,
+		.speed		=	CONFIG_SYS_SOFT_I2C3_SPEED,
+		.slaveaddr	=	CONFIG_SYS_SOFT_I2C3_SLAVE,
+		.init_done	=	0,
+		.name		=	"soft-i2c#3"
+	},
+#endif
+};

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [U-Boot] PATCH 3/8 Multi-adapter multi-bus I2C
  2009-02-07  1:05 [U-Boot] PATCH 3/8 Multi-adapter multi-bus I2C ksi at koi8.net
@ 2009-02-09 12:21 ` Heiko Schocher
  2009-02-09 22:25   ` ksi at koi8.net
  0 siblings, 1 reply; 4+ messages in thread
From: Heiko Schocher @ 2009-02-09 12:21 UTC (permalink / raw)
  To: u-boot

Hello ksi,

ksi at koi8.net wrote:
> Signed-off-by: Sergey Kubushyn <ksi@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@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.

> +			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)

> +i2c_adap_t	soft_i2c_adap[] = {
> +	{
> +#if defined(I2C_SOFT_DECLARATIONS)
> +		.init		=	soft_i2c_init,
> +		.probe		=	soft_i2c_probe,
> +		.read		=	soft_i2c_read,
> +		.write		=	soft_i2c_write,
> +		.set_bus_speed	=	soft_i2c_set_bus_speed,
> +		.get_bus_speed	=	soft_i2c_get_bus_speed,
> +		.speed		=	CONFIG_SYS_SOFT_I2C_SPEED,
> +		.slaveaddr	=	CONFIG_SYS_SOFT_I2C_SLAVE,
> +		.init_done	=	0,
> +		.name		=	"soft-i2c"
> +#elif defined(I2C_SOFT_DECLARATIONS0)
> +		.init		=	soft_i2c_init0,
> +		.probe		=	soft_i2c_probe0,
> +		.read		=	soft_i2c_read0,
> +		.write		=	soft_i2c_write0,
> +		.set_bus_speed	=	soft_i2c_set_bus_speed0,
> +		.get_bus_speed	=	soft_i2c_get_bus_speed0,
> +		.speed		=	CONFIG_SYS_SOFT_I2C0_SPEED,
> +		.slaveaddr	=	CONFIG_SYS_SOFT_I2C0_SLAVE,
> +		.init_done	=	0,
> +		.name		=	"soft-i2c#0"
> +#endif
> +	},
> +#if defined(I2C_SOFT_DECLARATIONS1)
> +	{
> +		.init		=	soft_i2c_init1,
> +		.probe		=	soft_i2c_probe1,
> +		.read		=	soft_i2c_read1,
> +		.write		=	soft_i2c_write1,
> +		.set_bus_speed	=	soft_i2c_set_bus_speed1,
> +		.get_bus_speed	=	soft_i2c_get_bus_speed1,
> +		.speed		=	CONFIG_SYS_SOFT_I2C1_SPEED,
> +		.slaveaddr	=	CONFIG_SYS_SOFT_I2C1_SLAVE,
> +		.init_done	=	0,
> +		.name		=	"soft-i2c#1"
> +	},
> +#endif
> +#if defined(I2C_SOFT_DECLARATIONS2)
> +	{
> +		.init		=	soft_i2c_init2,
> +		.probe		=	soft_i2c_probe2,
> +		.read		=	soft_i2c_read2,
> +		.write		=	soft_i2c_write2,
> +		.set_bus_speed	=	soft_i2c_set_bus_speed2,
> +		.get_bus_speed	=	soft_i2c_get_bus_speed2,
> +		.speed		=	CONFIG_SYS_SOFT_I2C2_SPEED,
> +		.slaveaddr	=	CONFIG_SYS_SOFT_I2C2_SLAVE,
> +		.init_done	=	0,
> +		.name		=	"soft-i2c#2"
> +	},
> +#endif
> +#if defined(I2C_SOFT_DECLARATIONS3)
> +	{
> +		.init		=	soft_i2c_init3,
> +		.probe		=	soft_i2c_probe3,
> +		.read		=	soft_i2c_read3,
> +		.write		=	soft_i2c_write3,
> +		.set_bus_speed	=	soft_i2c_set_bus_speed3,
> +		.get_bus_speed	=	soft_i2c_get_bus_speed3,
> +		.speed		=	CONFIG_SYS_SOFT_I2C3_SPEED,
> +		.slaveaddr	=	CONFIG_SYS_SOFT_I2C3_SLAVE,
> +		.init_done	=	0,
> +		.name		=	"soft-i2c#3"
> +	},
> +#endif
> +};
>   

bye
Heiko

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [U-Boot] PATCH 3/8 Multi-adapter multi-bus I2C
  2009-02-09 12:21 ` Heiko Schocher
@ 2009-02-09 22:25   ` ksi at koi8.net
  2009-02-10  8:20     ` Heiko Schocher
  0 siblings, 1 reply; 4+ messages in thread
From: ksi at koi8.net @ 2009-02-09 22:25 UTC (permalink / raw)
  To: u-boot

On Mon, 9 Feb 2009, Heiko Schocher wrote:

> Hello ksi,
> 
> ksi at koi8.net wrote:
> > Signed-off-by: Sergey Kubushyn <ksi@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@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.   *
******************************************************************

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [U-Boot] PATCH 3/8 Multi-adapter multi-bus I2C
  2009-02-09 22:25   ` ksi at koi8.net
@ 2009-02-10  8:20     ` Heiko Schocher
  0 siblings, 0 replies; 4+ messages in thread
From: Heiko Schocher @ 2009-02-10  8:20 UTC (permalink / raw)
  To: u-boot

Hello ksi,

ksi at koi8.net wrote:
> On Mon, 9 Feb 2009, Heiko Schocher wrote:
> 
>> Hello ksi,
>>
>> ksi at koi8.net wrote:
>>> Signed-off-by: Sergey Kubushyn <ksi@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@koi8.net>
>>> + *

[...]

>>> +#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.

Obvious.

> 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...

For example soft_i2c_read():

static int soft_i2c_read(uchar chip, uint addr, int alen, uchar *buffer, int len)
{
[...]
	send_start();
	if(alen > 0) {
		if(write_byte(chip << 1)) {	/* write cycle */
[...]
}

This is now a real function you can make a pointer for

i2c_adap_t	soft_i2c_adap[] = {
{
	.read		=	soft_i2c_read,
}
}

In soft_i2c_read() there are calls for example for send_start().

static void send_start(void)
{
[...]
	I2C_DELAY;
	I2C_SDA(1);
	I2C_ACTIVE;
	I2C_DELAY;
}

and in this function, there are the calls for I2C_SDA(), I2C_ACTIVE,... which
are look as I described. So where is the problem?

And we have not to change aprox. all lines of code from this driver!

> 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.

So, I think, on my suggestion to.

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

Why should my functions not 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...

Hmm.. I am not a CPP Guru, but it should work without these "define monster".

I look for a little time to try out my suggestion.

bye
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2009-02-10  8:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-02-07  1:05 [U-Boot] PATCH 3/8 Multi-adapter multi-bus I2C ksi at koi8.net
2009-02-09 12:21 ` Heiko Schocher
2009-02-09 22:25   ` ksi at koi8.net
2009-02-10  8:20     ` Heiko Schocher

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox