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

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

[ksi at koi8.net]

Signed-off-by: Sergey Kubushyn <ksi@koi8.net>      
---
diff --git a/common/devices.c b/common/devices.c
index 38f1bbc..073d89b 100644
--- a/common/devices.c
+++ b/common/devices.c
@@ -1,4 +1,8 @@
 /*
+ * Copyright (C) 2009 Sergey Kubushyn <ksi@koi8.net>
+ *
+ * Changes for multibus/multiadapter I2C support.
+ *
  * (C) Copyright 2000
  * Paolo Scaffardi, AIRVENT SAM s.p.a - RIMINI(ITALY), arsenio at tin.it
  *
@@ -30,7 +34,7 @@
 #ifdef CONFIG_LOGBUFFER
 #include <logbuff.h>
 #endif
-#if defined(CONFIG_HARD_I2C) || defined(CONFIG_SOFT_I2C)
+#if defined(CONFIG_HARD_I2C) || defined(CONFIG_SOFT_I2C) || defined(CONFIG_SYS_I2C_ADAPTERS)
 #include <i2c.h>
 #endif
 
@@ -215,9 +219,15 @@ int devices_init (void)
 	/* Initialize the list */
 	INIT_LIST_HEAD(&(devs.list));
 
+#ifdef CONFIG_NEW_I2C
+#ifdef CONFIG_SYS_I2C_ADAPTERS
+	i2c_init_all();
+#endif
+#else
 #if defined(CONFIG_HARD_I2C) || defined(CONFIG_SOFT_I2C)
 	i2c_init (CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
 #endif
+#endif
 #ifdef CONFIG_LCD
 	drv_lcd_init ();
 #endif
diff --git a/cpu/mpc8xx/video.c b/cpu/mpc8xx/video.c
index 4a59927..5abb4fc 100644
--- a/cpu/mpc8xx/video.c
+++ b/cpu/mpc8xx/video.c
@@ -809,7 +809,11 @@ static void video_encoder_init (void)
 
 	/* Initialize the I2C */
 	debug ("[VIDEO ENCODER] Initializing I2C bus...\n");
+#ifdef CONFIG_NEW_I2C
+	i2c_init_all();
+#else
 	i2c_init (CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
+#endif
 
 #ifdef CONFIG_FADS
 	/* Reset ADV7176 chip */
diff --git a/drivers/hwmon/adm1021.c b/drivers/hwmon/adm1021.c
index d753e9a..f6d38f1 100644
--- a/drivers/hwmon/adm1021.c
+++ b/drivers/hwmon/adm1021.c
@@ -174,7 +174,11 @@ dtt_init (void)
 	const char *const header = "DTT:   ";
 
 	/* switch to correct I2C bus */
+#ifdef CONFIG_NEW_I2C
+	i2c_set_bus_num(CONFIG_SYS_DTT_BUS_NUM);
+#else
 	I2C_SET_BUS(CONFIG_SYS_DTT_BUS_NUM);
+#endif
 
 	for (i = 0; i < sizeof(sensors); i++) {
 		if (_dtt_init(sensors[i]) != 0)
diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index 8119821..66d6865 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -156,8 +156,13 @@ int dtt_init (void)
 	int old_bus;
 
 	/* switch to correct I2C bus */
+#ifdef CONFIG_NEW_I2C
+	old_bus = i2c_get_bus_num();
+	i2c_set_bus_num(CONFIG_SYS_DTT_BUS_NUM);
+#else
 	old_bus = I2C_GET_BUS();
 	I2C_SET_BUS(CONFIG_SYS_DTT_BUS_NUM);
+#endif
 
 	for (i = 0; i < sizeof(sensors); i++) {
 	if (_dtt_init(sensors[i]) != 0)
@@ -167,7 +172,11 @@ int dtt_init (void)
 		dtt_get_temp(sensors[i]));
 	}
 	/* switch back to original I2C bus */
+#ifdef CONFIG_NEW_I2C
+	i2c_set_bus_num(old_bus);
+#else
 	I2C_SET_BUS(old_bus);
+#endif
 
 	return (0);
 } /* dtt_init() */
diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
index 9c74657..095c6ee 100644
--- a/drivers/i2c/Makefile
+++ b/drivers/i2c/Makefile
@@ -33,6 +33,8 @@ COBJS-$(CONFIG_DRIVER_OMAP24XX_I2C) += omap24xx_i2c.o
 COBJS-$(CONFIG_DRIVER_OMAP34XX_I2C) += omap24xx_i2c.o
 COBJS-$(CONFIG_SOFT_I2C) += soft_i2c.o
 COBJS-$(CONFIG_TSI108_I2C) += tsi108_i2c.o
+COBJS-$(CONFIG_SM502_I2C) += sm502_i2c.o
+COBJS-$(CONFIG_NEW_I2C) += i2c_core.o
 
 COBJS	:= $(COBJS-y)
 SRCS	:= $(COBJS:.o=.c)
diff --git a/drivers/i2c/fsl_i2c.c b/drivers/i2c/fsl_i2c.c
index ce646fd..f7998e3 100644
--- a/drivers/i2c/fsl_i2c.c
+++ b/drivers/i2c/fsl_i2c.c
@@ -1,4 +1,8 @@
 /*
+ * Copyright (c) 2009 Sergey Kubushyn <ksi@koi8.net>
+ *
+ * Changes for multibus/multiadapter I2C support.
+ *
  * Copyright 2006 Freescale Semiconductor, Inc.
  *
  * This program is free software; you can redistribute it and/or
@@ -18,7 +22,7 @@
 
 #include <common.h>
 
-#ifdef CONFIG_HARD_I2C
+#ifdef CONFIG_FSL_I2C
 
 #include <command.h>
 #include <i2c.h>		/* Functional interface */
@@ -31,24 +35,15 @@
 #define I2C_READ_BIT  1
 #define I2C_WRITE_BIT 0
 
-DECLARE_GLOBAL_DATA_PTR;
-
-/* Initialize the bus pointer to whatever one the SPD EEPROM is on.
- * Default is bus 0.  This is necessary because the DDR initialization
- * runs from ROM, and we can't switch buses because we can't modify
- * the global variables.
- */
-#ifndef CONFIG_SYS_SPD_BUS_NUM
-#define CONFIG_SYS_SPD_BUS_NUM 0
-#endif
-static unsigned int i2c_bus_num __attribute__ ((section (".data"))) = CONFIG_SYS_SPD_BUS_NUM;
+#define FSL_NAME(arg)	"fsl_i2c@" MK_NAME(arg)
+#define MK_NAME(arg)	#arg
 
-static unsigned int i2c_bus_speed[2] = {CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SPEED};
+DECLARE_GLOBAL_DATA_PTR;
 
 static const struct fsl_i2c *i2c_dev[2] = {
-	(struct fsl_i2c *) (CONFIG_SYS_IMMR + CONFIG_SYS_I2C_OFFSET),
-#ifdef CONFIG_SYS_I2C2_OFFSET
-	(struct fsl_i2c *) (CONFIG_SYS_IMMR + CONFIG_SYS_I2C2_OFFSET)
+	(struct fsl_i2c *) (CONFIG_SYS_IMMR + CONFIG_SYS_FSL_I2C_OFFSET),
+#ifdef CONFIG_SYS_FSL_I2C2_OFFSET
+	(struct fsl_i2c *) (CONFIG_SYS_IMMR + CONFIG_SYS_FSL_I2C2_OFFSET)
 #endif
 };
 
@@ -126,6 +121,8 @@ static const struct {
 #endif
 };
 
+i2c_adap_t	fsl_i2c_adap[];
+
 /**
  * Set the I2C bus speed for a given I2C device
  *
@@ -169,43 +166,29 @@ static unsigned int set_i2c_bus_speed(const struct fsl_i2c *dev,
 	return speed;
 }
 
-void
-i2c_init(int speed, int slaveadd)
+
+static void __i2c_init(int adap_no, int speed, int slaveadd)
 {
-	struct fsl_i2c *dev;
 	unsigned int temp;
 
-	dev = (struct fsl_i2c *) (CONFIG_SYS_IMMR + CONFIG_SYS_I2C_OFFSET);
-
-	writeb(0, &dev->cr);			/* stop I2C controller */
+	writeb(0, &i2c_dev[adap_no]->cr);	/* stop I2C controller */
 	udelay(5);				/* let it shutdown in peace */
-	temp = set_i2c_bus_speed(dev, gd->i2c1_clk, speed);
-	if (gd->flags & GD_FLG_RELOC)
-		i2c_bus_speed[0] = temp;
-	writeb(slaveadd << 1, &dev->adr);	/* write slave address */
-	writeb(0x0, &dev->sr);			/* clear status register */
-	writeb(I2C_CR_MEN, &dev->cr);		/* start I2C controller */
-
-#ifdef	CONFIG_SYS_I2C2_OFFSET
-	dev = (struct fsl_i2c *) (CONFIG_SYS_IMMR + CONFIG_SYS_I2C2_OFFSET);
-
-	writeb(0, &dev->cr);			/* stop I2C controller */
-	udelay(5);				/* let it shutdown in peace */
-	temp = set_i2c_bus_speed(dev, gd->i2c2_clk, speed);
-	if (gd->flags & GD_FLG_RELOC)
-		i2c_bus_speed[1] = temp;
-	writeb(slaveadd << 1, &dev->adr);	/* write slave address */
-	writeb(0x0, &dev->sr);			/* clear status register */
-	writeb(I2C_CR_MEN, &dev->cr);		/* start I2C controller */
-#endif
+	temp = set_i2c_bus_speed(i2c_dev[adap_no], gd->i2c1_clk, speed);
+	if (gd->flags & GD_FLG_RELOC) {
+		fsl_i2c_adap[adap_no].speed = temp;
+		fsl_i2c_adap[adap_no].slaveaddr = slaveadd;
+	}
+	writeb(slaveadd << 1, &i2c_dev[adap_no]->adr);	/* write slave address */
+	writeb(0x0, &i2c_dev[adap_no]->sr);			/* clear status register */
+	writeb(I2C_CR_MEN, &i2c_dev[adap_no]->cr);		/* start I2C controller */
 }
 
-static __inline__ int
-i2c_wait4bus(void)
+
+static __inline__ int i2c_wait4bus(int adap_no)
 {
 	unsigned long long timeval = get_ticks();
 
-	while (readb(&i2c_dev[i2c_bus_num]->sr) & I2C_SR_MBB) {
+	while (readb(&i2c_dev[adap_no]->sr) & I2C_SR_MBB) {
 		if ((get_ticks() - timeval) > usec2ticks(I2C_TIMEOUT))
 			return -1;
 	}
@@ -213,18 +196,18 @@ i2c_wait4bus(void)
 	return 0;
 }
 
-static __inline__ int
-i2c_wait(int write)
+
+static __inline__ int i2c_wait(int adap_no, int write)
 {
 	u32 csr;
 	unsigned long long timeval = get_ticks();
 
 	do {
-		csr = readb(&i2c_dev[i2c_bus_num]->sr);
+		csr = readb(&i2c_dev[adap_no]->sr);
 		if (!(csr & I2C_SR_MIF))
 			continue;
 
-		writeb(0x0, &i2c_dev[i2c_bus_num]->sr);
+		writeb(0x0, &i2c_dev[adap_no]->sr);
 
 		if (csr & I2C_SR_MAL) {
 			debug("i2c_wait: MAL\n");
@@ -248,85 +231,85 @@ i2c_wait(int write)
 	return -1;
 }
 
-static __inline__ int
-i2c_write_addr (u8 dev, u8 dir, int rsta)
+
+static __inline__ int i2c_write_addr (int adap_no, u8 dev, u8 dir, int rsta)
 {
 	writeb(I2C_CR_MEN | I2C_CR_MSTA | I2C_CR_MTX
 	       | (rsta ? I2C_CR_RSTA : 0),
-	       &i2c_dev[i2c_bus_num]->cr);
+	       &i2c_dev[adap_no]->cr);
 
-	writeb((dev << 1) | dir, &i2c_dev[i2c_bus_num]->dr);
+	writeb((dev << 1) | dir, &i2c_dev[adap_no]->dr);
 
-	if (i2c_wait(I2C_WRITE_BIT) < 0)
+	if (i2c_wait(adap_no, I2C_WRITE_BIT) < 0)
 		return 0;
 
 	return 1;
 }
 
-static __inline__ int
-__i2c_write(u8 *data, int length)
+
+static __inline__ int i2c_write_data(int adap_no, u8 *data, int length)
 {
 	int i;
 
 	writeb(I2C_CR_MEN | I2C_CR_MSTA | I2C_CR_MTX,
-	       &i2c_dev[i2c_bus_num]->cr);
+	       &i2c_dev[adap_no]->cr);
 
 	for (i = 0; i < length; i++) {
-		writeb(data[i], &i2c_dev[i2c_bus_num]->dr);
+		writeb(data[i], &i2c_dev[adap_no]->dr);
 
-		if (i2c_wait(I2C_WRITE_BIT) < 0)
+		if (i2c_wait(adap_no, I2C_WRITE_BIT) < 0)
 			break;
 	}
 
 	return i;
 }
 
-static __inline__ int
-__i2c_read(u8 *data, int length)
+
+static __inline__ int i2c_read_data(int adap_no, u8 *data, int length)
 {
 	int i;
 
 	writeb(I2C_CR_MEN | I2C_CR_MSTA | ((length == 1) ? I2C_CR_TXAK : 0),
-	       &i2c_dev[i2c_bus_num]->cr);
+	       &i2c_dev[adap_no]->cr);
 
 	/* dummy read */
-	readb(&i2c_dev[i2c_bus_num]->dr);
+	readb(&i2c_dev[adap_no]->dr);
 
 	for (i = 0; i < length; i++) {
-		if (i2c_wait(I2C_READ_BIT) < 0)
+		if (i2c_wait(adap_no, I2C_READ_BIT) < 0)
 			break;
 
 		/* Generate ack on last next to last byte */
 		if (i == length - 2)
 			writeb(I2C_CR_MEN | I2C_CR_MSTA | I2C_CR_TXAK,
-			       &i2c_dev[i2c_bus_num]->cr);
+			       &i2c_dev[adap_no]->cr);
 
 		/* Generate stop on last byte */
 		if (i == length - 1)
-			writeb(I2C_CR_MEN | I2C_CR_TXAK, &i2c_dev[i2c_bus_num]->cr);
+			writeb(I2C_CR_MEN | I2C_CR_TXAK, &i2c_dev[adap_no]->cr);
 
-		data[i] = readb(&i2c_dev[i2c_bus_num]->dr);
+		data[i] = readb(&i2c_dev[adap_no]->dr);
 	}
 
 	return i;
 }
 
-int
-i2c_read(u8 dev, uint addr, int alen, u8 *data, int length)
+
+static int __i2c_read(int adap_no, u8 dev, uint addr, int alen, u8 *data, int length)
 {
 	int i = -1; /* signal error */
 	u8 *a = (u8*)&addr;
 
-	if (i2c_wait4bus() >= 0
-	    && i2c_write_addr(dev, I2C_WRITE_BIT, 0) != 0
-	    && __i2c_write(&a[4 - alen], alen) == alen)
+	if (i2c_wait4bus(adap_no) >= 0
+	    && i2c_write_addr(adap_no, dev, I2C_WRITE_BIT, 0) != 0
+	    && i2c_write_data(adap_no, &a[4 - alen], alen) == alen)
 		i = 0; /* No error so far */
 
 	if (length
-	    && i2c_write_addr(dev, I2C_READ_BIT, 1) != 0)
-		i = __i2c_read(data, length);
+	    && i2c_write_addr(adap_no, dev, I2C_READ_BIT, 1) != 0)
+		i = i2c_read_data(adap_no, data, length);
 
-	writeb(I2C_CR_MEN, &i2c_dev[i2c_bus_num]->cr);
+	writeb(I2C_CR_MEN, &i2c_dev[adap_no]->cr);
 
 	if (i == length)
 	    return 0;
@@ -334,19 +317,19 @@ i2c_read(u8 dev, uint addr, int alen, u8 *data, int length)
 	return -1;
 }
 
-int
-i2c_write(u8 dev, uint addr, int alen, u8 *data, int length)
+
+static int __i2c_write(int adap_no, u8 dev, uint addr, int alen, u8 *data, int length)
 {
 	int i = -1; /* signal error */
 	u8 *a = (u8*)&addr;
 
-	if (i2c_wait4bus() >= 0
-	    && i2c_write_addr(dev, I2C_WRITE_BIT, 0) != 0
-	    && __i2c_write(&a[4 - alen], alen) == alen) {
-		i = __i2c_write(data, length);
+	if (i2c_wait4bus(adap_no) >= 0
+	    && i2c_write_addr(adap_no, dev, I2C_WRITE_BIT, 0) != 0
+	    && i2c_write_data(adap_no, &a[4 - alen], alen) == alen) {
+		i = i2c_write_data(adap_no, data, length);
 	}
 
-	writeb(I2C_CR_MEN, &i2c_dev[i2c_bus_num]->cr);
+	writeb(I2C_CR_MEN, &i2c_dev[adap_no]->cr);
 
 	if (i == length)
 	    return 0;
@@ -354,54 +337,125 @@ i2c_write(u8 dev, uint addr, int alen, u8 *data, int length)
 	return -1;
 }
 
-int
-i2c_probe(uchar chip)
+
+static int __i2c_probe(int adap_no, uchar chip)
 {
 	/* For unknow reason the controller will ACK when
 	 * probing for a slave with the same address, so skip
 	 * it.
 	 */
-	if (chip == (readb(&i2c_dev[i2c_bus_num]->adr) >> 1))
+	if (chip == (readb(&i2c_dev[adap_no]->adr) >> 1))
 		return -1;
 
-	return i2c_read(chip, 0, 0, NULL, 0);
+	return __i2c_read(adap_no, chip, 0, 0, NULL, 0);
 }
 
-int i2c_set_bus_num(unsigned int bus)
+
+static int __i2c_set_bus_speed(int adap_no, unsigned int speed)
 {
-#ifdef CONFIG_SYS_I2C2_OFFSET
-	if (bus > 1) {
-#else
-	if (bus > 0) {
-#endif
-		return -1;
-	}
+	unsigned int i2c_clk = (adap_no == 1) ? gd->i2c2_clk : gd->i2c1_clk;
 
-	i2c_bus_num = bus;
+	writeb(0, &i2c_dev[adap_no]->cr);		/* stop controller */
+	fsl_i2c_adap[adap_no].speed =
+		set_i2c_bus_speed(i2c_dev[adap_no], i2c_clk, speed);
+	writeb(I2C_CR_MEN, &i2c_dev[adap_no]->cr);	/* start controller */
 
 	return 0;
 }
 
-int i2c_set_bus_speed(unsigned int speed)
+
+/* Wrappers for the first controller */
+static void fsl_i2c1_init(int speed, int slaveadd)
+{
+	__i2c_init(0, speed, slaveadd);
+}
+
+static int fsl_i2c1_read(u8 dev, uint addr, int alen, u8 *data, int length)
+{
+	return(__i2c_read(0, dev, addr, alen, data, length));
+}
+
+static int fsl_i2c1_write(u8 dev, uint addr, int alen, u8 *data, int length)
 {
-	unsigned int i2c_clk = (i2c_bus_num == 1) ? gd->i2c2_clk : gd->i2c1_clk;
+	return(__i2c_write(0, dev, addr, alen, data, length));
+}
 
-	writeb(0, &i2c_dev[i2c_bus_num]->cr);		/* stop controller */
-	i2c_bus_speed[i2c_bus_num] =
-		set_i2c_bus_speed(i2c_dev[i2c_bus_num], i2c_clk, speed);
-	writeb(I2C_CR_MEN, &i2c_dev[i2c_bus_num]->cr);	/* start controller */
+static int fsl_i2c1_probe(uchar chip)
+{
+	return(__i2c_probe(0, chip));
+}
 
-	return 0;
+static unsigned int fsl_i2c1_set_bus_speed(unsigned int speed)
+{
+	return(__i2c_set_bus_speed(0, speed));
+}
+
+static unsigned int fsl_i2c1_get_bus_speed(void)
+{
+	return(fsl_i2c_adap[0].speed);
+}
+
+
+/* Second controller */
+#ifdef CONFIG_SYS_FSL_I2C2_OFFSET
+static void fsl_i2c2_init(int speed, int slaveadd)
+{
+	__i2c_init(1, speed, slaveadd);
+}
+
+static int fsl_i2c2_read(u8 dev, uint addr, int alen, u8 *data, int length)
+{
+	return(__i2c_read(1, dev, addr, alen, data, length));
 }
 
-unsigned int i2c_get_bus_num(void)
+static int fsl_i2c2_write(u8 dev, uint addr, int alen, u8 *data, int length)
 {
-	return i2c_bus_num;
+	return(__i2c_write(1, dev, addr, alen, data, length));
 }
 
-unsigned int i2c_get_bus_speed(void)
+static int fsl_i2c2_probe(uchar chip)
 {
-	return i2c_bus_speed[i2c_bus_num];
+	return(__i2c_probe(1, chip));
 }
 
-#endif /* CONFIG_HARD_I2C */
+static unsigned int fsl_i2c2_set_bus_speed(unsigned int speed)
+{
+	return(__i2c_set_bus_speed(1, speed));
+}
+
+static unsigned int fsl_i2c2_get_bus_speed(void)
+{
+	return(fsl_i2c_adap[1].speed);
+}
+#endif
+
+i2c_adap_t	fsl_i2c_adap[2] = {
+	{
+		.init		=	fsl_i2c1_init,
+		.probe		=	fsl_i2c1_probe,
+		.read		=	fsl_i2c1_read,
+		.write		=	fsl_i2c1_write,
+		.set_bus_speed	=	fsl_i2c1_set_bus_speed,
+		.get_bus_speed	=	fsl_i2c1_get_bus_speed,
+		.speed		=	CONFIG_SYS_FSL_I2C_SPEED,
+		.slaveaddr	=	CONFIG_SYS_FSL_I2C_SLAVE,
+		.init_done	=	0,
+		.name		=	FSL_NAME(CONFIG_SYS_FSL_I2C_OFFSET)
+	},
+#ifdef CONFIG_SYS_FSL_I2C2_OFFSET
+	{
+		.init		=	fsl_i2c2_init,
+		.probe		=	fsl_i2c2_probe,
+		.read		=	fsl_i2c2_read,
+		.write		=	fsl_i2c2_write,
+		.set_bus_speed	=	fsl_i2c2_set_bus_speed,
+		.get_bus_speed	=	fsl_i2c2_get_bus_speed,
+		.speed		=	CONFIG_SYS_FSL_I2C2_SPEED,
+		.slaveaddr	=	CONFIG_SYS_FSL_I2C2_SLAVE,
+		.init_done	=	0,
+		.name		=	FSL_NAME(CONFIG_SYS_FSL_I2C2_OFFSET)
+	}
+#endif
+};
+
+#endif /* CONFIG_FSL_I2C */

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

[Heiko Schocher]

Hello ksi,

ksi at koi8.net wrote:
[...]
> diff --git a/drivers/i2c/fsl_i2c.c b/drivers/i2c/fsl_i2c.c
> index ce646fd..f7998e3 100644
> --- a/drivers/i2c/fsl_i2c.c
> +++ b/drivers/i2c/fsl_i2c.c
> @@ -1,4 +1,8 @@
>  /*
> + * Copyright (c) 2009 Sergey Kubushyn <ksi@koi8.net>
> + *
> + * Changes for multibus/multiadapter I2C support.
> + *
>   * Copyright 2006 Freescale Semiconductor, Inc.
>   *
>   * This program is free software; you can redistribute it and/or
> @@ -18,7 +22,7 @@
>  
>  #include <common.h>
>  
> -#ifdef CONFIG_HARD_I2C
> +#ifdef CONFIG_FSL_I2C
>  
>  #include <command.h>
>  #include <i2c.h>		/* Functional interface */
> @@ -31,24 +35,15 @@
>  #define I2C_READ_BIT  1
>  #define I2C_WRITE_BIT 0
>  
> -DECLARE_GLOBAL_DATA_PTR;
> -
> -/* Initialize the bus pointer to whatever one the SPD EEPROM is on.
> - * Default is bus 0.  This is necessary because the DDR initialization
> - * runs from ROM, and we can't switch buses because we can't modify
> - * the global variables.
> - */
> -#ifndef CONFIG_SYS_SPD_BUS_NUM
> -#define CONFIG_SYS_SPD_BUS_NUM 0
> -#endif
> -static unsigned int i2c_bus_num __attribute__ ((section (".data"))) = CONFIG_SYS_SPD_BUS_NUM;
> +#define FSL_NAME(arg)	"fsl_i2c@" MK_NAME(arg)
> +#define MK_NAME(arg)	#arg
>  
> -static unsigned int i2c_bus_speed[2] = {CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SPEED};
> +DECLARE_GLOBAL_DATA_PTR;
>  
>  static const struct fsl_i2c *i2c_dev[2] = {
> -	(struct fsl_i2c *) (CONFIG_SYS_IMMR + CONFIG_SYS_I2C_OFFSET),
> -#ifdef CONFIG_SYS_I2C2_OFFSET
> -	(struct fsl_i2c *) (CONFIG_SYS_IMMR + CONFIG_SYS_I2C2_OFFSET)
> +	(struct fsl_i2c *) (CONFIG_SYS_IMMR + CONFIG_SYS_FSL_I2C_OFFSET),
> +#ifdef CONFIG_SYS_FSL_I2C2_OFFSET
> +	(struct fsl_i2c *) (CONFIG_SYS_IMMR + CONFIG_SYS_FSL_I2C2_OFFSET)
>  #endif
>  };
>  
> @@ -126,6 +121,8 @@ static const struct {
>  #endif
>  };
>  
> +i2c_adap_t	fsl_i2c_adap[];
> +
>  /**
>   * Set the I2C bus speed for a given I2C device
>   *
> @@ -169,43 +166,29 @@ static unsigned int set_i2c_bus_speed(const struct fsl_i2c *dev,
>  	return speed;
>  }
>  
> -void
> -i2c_init(int speed, int slaveadd)
> +
> +static void __i2c_init(int adap_no, int speed, int slaveadd)
>  {
> -	struct fsl_i2c *dev;
>  	unsigned int temp;
>  
> -	dev = (struct fsl_i2c *) (CONFIG_SYS_IMMR + CONFIG_SYS_I2C_OFFSET);
> -
> -	writeb(0, &dev->cr);			/* stop I2C controller */
> +	writeb(0, &i2c_dev[adap_no]->cr);	/* stop I2C controller */
>   

Why do you here substitute a "dev->" with "&i2c_dev[adap_no]"?
Why not dev =  &i2c_dev[adap_no] and you can let the dev-> unchanged.

>  	udelay(5);				/* let it shutdown in peace */
> -	temp = set_i2c_bus_speed(dev, gd->i2c1_clk, speed);
> -	if (gd->flags & GD_FLG_RELOC)
> -		i2c_bus_speed[0] = temp;
> -	writeb(slaveadd << 1, &dev->adr);	/* write slave address */
> -	writeb(0x0, &dev->sr);			/* clear status register */
> -	writeb(I2C_CR_MEN, &dev->cr);		/* start I2C controller */
> -
> -#ifdef	CONFIG_SYS_I2C2_OFFSET
> -	dev = (struct fsl_i2c *) (CONFIG_SYS_IMMR + CONFIG_SYS_I2C2_OFFSET);
> -
> -	writeb(0, &dev->cr);			/* stop I2C controller */
> -	udelay(5);				/* let it shutdown in peace */
> -	temp = set_i2c_bus_speed(dev, gd->i2c2_clk, speed);
> -	if (gd->flags & GD_FLG_RELOC)
> -		i2c_bus_speed[1] = temp;
> -	writeb(slaveadd << 1, &dev->adr);	/* write slave address */
> -	writeb(0x0, &dev->sr);			/* clear status register */
> -	writeb(I2C_CR_MEN, &dev->cr);		/* start I2C controller */
> -#endif
> +	temp = set_i2c_bus_speed(i2c_dev[adap_no], gd->i2c1_clk, speed);
> +	if (gd->flags & GD_FLG_RELOC) {
> +		fsl_i2c_adap[adap_no].speed = temp;
> +		fsl_i2c_adap[adap_no].slaveaddr = slaveadd;
> +	}
> +	writeb(slaveadd << 1, &i2c_dev[adap_no]->adr);	/* write slave address */
> +	writeb(0x0, &i2c_dev[adap_no]->sr);			/* clear status register */
> +	writeb(I2C_CR_MEN, &i2c_dev[adap_no]->cr);		/* start I2C controller */
>  }
>  
bye
Heiko

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

[U-Boot] PATCH 2/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:
> [...]
> > diff --git a/drivers/i2c/fsl_i2c.c b/drivers/i2c/fsl_i2c.c
> > index ce646fd..f7998e3 100644
> > --- a/drivers/i2c/fsl_i2c.c
> > +++ b/drivers/i2c/fsl_i2c.c
> > @@ -1,4 +1,8 @@
> >  /*
> > + * Copyright (c) 2009 Sergey Kubushyn <ksi@koi8.net>
> > + *
> > + * Changes for multibus/multiadapter I2C support.
> > + *
> >   * Copyright 2006 Freescale Semiconductor, Inc.
> >   *
> >   * This program is free software; you can redistribute it and/or
> > @@ -18,7 +22,7 @@
> >  
> >  #include <common.h>
> >  
> > -#ifdef CONFIG_HARD_I2C
> > +#ifdef CONFIG_FSL_I2C
> >  
> >  #include <command.h>
> >  #include <i2c.h>		/* Functional interface */
> > @@ -31,24 +35,15 @@
> >  #define I2C_READ_BIT  1
> >  #define I2C_WRITE_BIT 0
> >  
> > -DECLARE_GLOBAL_DATA_PTR;
> > -
> > -/* Initialize the bus pointer to whatever one the SPD EEPROM is on.
> > - * Default is bus 0.  This is necessary because the DDR initialization
> > - * runs from ROM, and we can't switch buses because we can't modify
> > - * the global variables.
> > - */
> > -#ifndef CONFIG_SYS_SPD_BUS_NUM
> > -#define CONFIG_SYS_SPD_BUS_NUM 0
> > -#endif
> > -static unsigned int i2c_bus_num __attribute__ ((section (".data"))) = CONFIG_SYS_SPD_BUS_NUM;
> > +#define FSL_NAME(arg)	"fsl_i2c@" MK_NAME(arg)
> > +#define MK_NAME(arg)	#arg
> >  
> > -static unsigned int i2c_bus_speed[2] = {CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SPEED};
> > +DECLARE_GLOBAL_DATA_PTR;
> >  
> >  static const struct fsl_i2c *i2c_dev[2] = {
> > -	(struct fsl_i2c *) (CONFIG_SYS_IMMR + CONFIG_SYS_I2C_OFFSET),
> > -#ifdef CONFIG_SYS_I2C2_OFFSET
> > -	(struct fsl_i2c *) (CONFIG_SYS_IMMR + CONFIG_SYS_I2C2_OFFSET)
> > +	(struct fsl_i2c *) (CONFIG_SYS_IMMR + CONFIG_SYS_FSL_I2C_OFFSET),
> > +#ifdef CONFIG_SYS_FSL_I2C2_OFFSET
> > +	(struct fsl_i2c *) (CONFIG_SYS_IMMR + CONFIG_SYS_FSL_I2C2_OFFSET)
> >  #endif
> >  };
> >  
> > @@ -126,6 +121,8 @@ static const struct {
> >  #endif
> >  };
> >  
> > +i2c_adap_t	fsl_i2c_adap[];
> > +
> >  /**
> >   * Set the I2C bus speed for a given I2C device
> >   *
> > @@ -169,43 +166,29 @@ static unsigned int set_i2c_bus_speed(const struct fsl_i2c *dev,
> >  	return speed;
> >  }
> >  
> > -void
> > -i2c_init(int speed, int slaveadd)
> > +
> > +static void __i2c_init(int adap_no, int speed, int slaveadd)
> >  {
> > -	struct fsl_i2c *dev;
> >  	unsigned int temp;
> >  
> > -	dev = (struct fsl_i2c *) (CONFIG_SYS_IMMR + CONFIG_SYS_I2C_OFFSET);
> > -
> > -	writeb(0, &dev->cr);			/* stop I2C controller */
> > +	writeb(0, &i2c_dev[adap_no]->cr);	/* stop I2C controller */
> >   
> 
> Why do you here substitute a "dev->" with "&i2c_dev[adap_no]"?
> Why not dev =  &i2c_dev[adap_no] and you can let the dev-> unchanged.

This eliminates one variable, dev. Another reason is uniformity through the
entire driver source.

Anyway, keeping that dev won't make any difference other than making the
patch a tad shorter...

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

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

[Wolfgang Denk]

Dear ksi at koi8.net,

In message <Pine.LNX.4.64ksi.0902091353360.32084@home-gw.koi8.net> you wrote:
>
> > Why do you here substitute a "dev->" with "&i2c_dev[adap_no]"?
> > Why not dev =  &i2c_dev[adap_no] and you can let the dev-> unchanged.
> 
> This eliminates one variable, dev. Another reason is uniformity through the
> entire driver source.

But it makes the code harder to read.

Maybe we should rather change the other places in the driver to do the
same?

Note that it is NOT a goal for optimizations to save variables -
variables are extremely cheap, as they are just ameans to make the
code readable. The compiler will know much better than we how to
allocate memory or registers to hold such data.

> Anyway, keeping that dev won't make any difference other than making the
> patch a tad shorter...

... and the code better readable.

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
Writing a book is like washing an elephant: there's no good place  to
begin  or  end,  and  it's  hard to keep track of what you've already
covered.

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

[ksi at koi8.net]

On Tue, 10 Feb 2009, Wolfgang Denk wrote:

> Dear ksi at koi8.net,
> 
> In message <Pine.LNX.4.64ksi.0902091353360.32084@home-gw.koi8.net> you wrote:
> >
> > > Why do you here substitute a "dev->" with "&i2c_dev[adap_no]"?
> > > Why not dev =  &i2c_dev[adap_no] and you can let the dev-> unchanged.
> > 
> > This eliminates one variable, dev. Another reason is uniformity through the
> > entire driver source.
> 
> But it makes the code harder to read.
> 
> Maybe we should rather change the other places in the driver to do the
> same?
> 
> Note that it is NOT a goal for optimizations to save variables -
> variables are extremely cheap, as they are just ameans to make the
> code readable. The compiler will know much better than we how to
> allocate memory or registers to hold such data.
> 
> > Anyway, keeping that dev won't make any difference other than making the
> > patch a tad shorter...
> 
> ... and the code better readable.

OK, I can keep that dev, no problems. I'm not sure it makes code any more
readable though... And it is just a couple of places.

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