Re: [U-Boot] [PATCH] Add I2C multibus support for OMAP2/3 boards

Tom Rix wrote:

From: Syed Mohammed Khasim khasim@ti.com

This was cherry-picked from

repo: http://www.beagleboard.org/u-boot-arm.git commit: 52eddcd07c2e7ad61d15bab2cf2d0d21466eaca2

In addition to adding multibus support, this patch also cleans up the register access. The register access has been changed from #defines to a structure.

Have you looked at my proposal I sent some hours before your patch?

http://lists.denx.de/pipermail/u-boot/2009-October/063556.html

Signed-off-by: Syed Mohammed Khasim khasim@ti.com Signed-off-by: Jason Kridner jkridner@beagleboard.org Signed-off-by: Tom Rix Tom.Rix@windriver.com

---

drivers/i2c/omap24xx_i2c.c | 220 ++++++++++++++++++++++------------- include/asm-arm/arch-omap24xx/i2c.h | 52 ++++++--- include/asm-arm/arch-omap3/i2c.h | 48 +++++--- include/configs/omap3_beagle.h | 1 + 4 files changed, 209 insertions(+), 112 deletions(-)

diff --git a/drivers/i2c/omap24xx_i2c.c b/drivers/i2c/omap24xx_i2c.c index 1a4c8c9..763d2f8 100644 --- a/drivers/i2c/omap24xx_i2c.c +++ b/drivers/i2c/omap24xx_i2c.c @@ -1,7 +1,7 @@ /*

• Basic I2C functions

• • Copyright (c) 2004 Texas Instruments

• • Copyright (c) 2004, 2009 Texas Instruments
  • This package is free software; you can redistribute it and/or
  • modify it under the terms of the license found in the file

@@ -25,10 +25,18 @@ #include <asm/arch/i2c.h> #include <asm/io.h>

+#ifdef CONFIG_OMAP34XX +#define I2C_NUM_IF 3 +#else +#define I2C_NUM_IF 2 +#endif

I prefer something like I2C_BUS_MAX for this. And move it to header file. Moving it OMAP2 and OMAP3 i2c.h headers will remove the #ifdef, too.

static void wait_for_bb (void); static u16 wait_for_pin (void); static void flush_fifo(void);

+static i2c_t *i2c = (i2c_t *)I2C_DEFAULT_BASE;

Looking at the other OMAP3 files, the recent syntax seems to be

static struct i2c *i2c_base = (struct i2c *)I2C_DEFAULT_BASE;

Just to be consistent.

void i2c_init (int speed, int slaveadd) { int psc, fsscll, fssclh; @@ -95,30 +103,30 @@ void i2c_init (int speed, int slaveadd) sclh = (unsigned int)fssclh; }

• writew(0x2, I2C_SYSC); /* for ES2 after soft reset */

• writew(0x2, &i2c->sysc); /* for ES2 after soft reset */ udelay(1000);

• writew(0x0, I2C_SYSC); /* will probably self clear but */

• writew(0x0, &i2c->sysc); /* will probably self clear but */

• if (readw (I2C_CON) & I2C_CON_EN) {

• writew (0, I2C_CON);
  

• udelay (50000);
  

• if (readw(&i2c->con) & I2C_CON_EN) {

• writew(0, &i2c->con);
  

• udelay(50000);
  

udelay: Are this formatting changes? You are correct, a lot of formatting improvements are needed here. But they should go to an other patch.

• writew(psc, I2C_PSC);
• writew(scll, I2C_SCLL);
• writew(sclh, I2C_SCLH);

• writew(psc, &i2c->psc);

• writew(scll, &i2c->scll);

• writew(sclh, &i2c->sclh);

  /* own address */

• writew (slaveadd, I2C_OA);
• writew (I2C_CON_EN, I2C_CON);

• writew(slaveadd, &i2c->oa);

• writew(I2C_CON_EN, &i2c->con);

  /* have to enable intrrupts or OMAP i2c module doesn't work */

• writew (I2C_IE_XRDY_IE | I2C_IE_RRDY_IE | I2C_IE_ARDY_IE |

• I2C_IE_NACK_IE | I2C_IE_AL_IE, I2C_IE);
  

• udelay (1000);

• writew(I2C_IE_XRDY_IE | I2C_IE_RRDY_IE | I2C_IE_ARDY_IE |

• I2C_IE_NACK_IE | I2C_IE_AL_IE, &i2c->ie);
  

• udelay(1000);

Formatting change?

flush_fifo();

• writew (0xFFFF, I2C_STAT);
• writew (0, I2C_CNT);

• writew(0xFFFF, &i2c->stat);
• writew(0, &i2c->cnt);

}

static int i2c_read_byte (u8 devaddr, u8 regoffset, u8 * value) @@ -130,19 +138,19 @@ static int i2c_read_byte (u8 devaddr, u8 regoffset, u8 * value) wait_for_bb ();

/* one byte only */

• writew (1, I2C_CNT);

• writew(1, &i2c->cnt); /* set slave address */

• writew (devaddr, I2C_SA);

• writew(devaddr, &i2c->sa); /* no stop bit needed here */

• writew (I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_TRX, I2C_CON);

• writew(I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_TRX, &i2c->con);

  status = wait_for_pin ();

  if (status & I2C_STAT_XRDY) { /* Important: have to use byte access */

• writeb (regoffset, I2C_DATA);
  

• udelay (20000);
  

• if (readw (I2C_STAT) & I2C_STAT_NACK) {
  

• writeb(regoffset, &i2c->data);
  

• udelay(20000);
  

Formatting change?

• if (readw(&i2c->stat) & I2C_STAT_NACK) {
  i2c_error = 1;
  

  } } else {

@@ -151,28 +159,28 @@ static int i2c_read_byte (u8 devaddr, u8 regoffset, u8 * value)

if (!i2c_error) { /* free bus, otherwise we can't use a combined transction */

• writew (0, I2C_CON);
  

• while (readw (I2C_STAT) || (readw (I2C_CON) & I2C_CON_MST)) {
  

• writew(0, &i2c->con);
  

• while (readw(&i2c->stat) || (readw(&i2c->con) & I2C_CON_MST)) {
  udelay (10000);
  /* Have to clear pending interrupt to clear I2C_STAT */
  

• 	writew (0xFFFF, I2C_STAT);
  

• 	writew(0xFFFF, &i2c->stat);
  

  }

  wait_for_bb (); /* set slave address */

• writew (devaddr, I2C_SA);
  

• writew(devaddr, &i2c->sa);
  

  /* read one byte from slave */

• writew (1, I2C_CNT);
  

• writew(1, &i2c->cnt);
  

  /* need stop bit here */

• writew (I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_STP,
  

• 	I2C_CON);
  

• writew(I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_STP,
  

• 	&i2c->con);
  
  

  status = wait_for_pin (); if (status & I2C_STAT_RRDY) {

#if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX)

• 	*value = readb (I2C_DATA);
  

• 	*value = readb(&i2c->data);
  

#else

• 	*value = readw (I2C_DATA);
  

• 	*value = readw(&i2c->data);
  

#endif udelay (20000); } else { @@ -180,17 +188,17 @@ static int i2c_read_byte (u8 devaddr, u8 regoffset, u8 * value) }

if (!i2c_error) {

• 	writew (I2C_CON_EN, I2C_CON);
  

• 	while (readw (I2C_STAT)
  

• 	       || (readw (I2C_CON) & I2C_CON_MST)) {
  

• 	writew(I2C_CON_EN, &i2c->con);
  

• 	while (readw(&i2c->stat)
  

• 	       || (readw(&i2c->con) & I2C_CON_MST)) {
  	udelay (10000);
  

• 		writew (0xFFFF, I2C_STAT);
  

• 		writew(0xFFFF, &i2c->stat);
  }
  

  } } flush_fifo();

• writew (0xFFFF, I2C_STAT);
• writew (0, I2C_CNT);

• writew(0xFFFF, &i2c->stat);
• writew(0, &i2c->cnt); return i2c_error;

}

@@ -203,12 +211,12 @@ static int i2c_write_byte (u8 devaddr, u8 regoffset, u8 value) wait_for_bb ();

/* two bytes */

• writew (2, I2C_CNT);

• writew(2, &i2c->cnt); /* set slave address */

• writew (devaddr, I2C_SA);

• writew(devaddr, &i2c->sa); /* stop bit needed here */

• writew (I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_TRX |

• I2C_CON_STP, I2C_CON);
  

• writew(I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_TRX |

• I2C_CON_STP, &i2c->con);
  
  

  /* wait until state change */ status = wait_for_pin ();

@@ -216,24 +224,24 @@ static int i2c_write_byte (u8 devaddr, u8 regoffset, u8 value) if (status & I2C_STAT_XRDY) { #if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX) /* send out 1 byte */

• writeb (regoffset, I2C_DATA);
  

• writew (I2C_STAT_XRDY, I2C_STAT);
  

• writeb(regoffset, &i2c->data);
  

• writew(I2C_STAT_XRDY, &i2c->stat);
  
  

  status = wait_for_pin (); if ((status & I2C_STAT_XRDY)) { /* send out next 1 byte */

• 	writeb (value, I2C_DATA);
  

• 	writew (I2C_STAT_XRDY, I2C_STAT);
  

• 	writeb(value, &i2c->data);
  

• 	writew(I2C_STAT_XRDY, &i2c->stat);
  

  } else { i2c_error = 1; }

#else /* send out two bytes */

• writew ((value << 8) + regoffset, I2C_DATA);
  

• writew((value << 8) + regoffset, &i2c->data);
  

#endif /* must have enough delay to allow BB bit to go low */

• udelay (50000);
  

• if (readw (I2C_STAT) & I2C_STAT_NACK) {
  

• udelay(50000);
  

• if (readw(&i2c->stat) & I2C_STAT_NACK) {
  i2c_error = 1;
  

  } } else {

@@ -243,18 +251,18 @@ static int i2c_write_byte (u8 devaddr, u8 regoffset, u8 value) if (!i2c_error) { int eout = 200;

• writew (I2C_CON_EN, I2C_CON);
  

• while ((stat = readw (I2C_STAT)) || (readw (I2C_CON) & I2C_CON_MST)) {
  

• 	udelay (1000);
  

• writew(I2C_CON_EN, &i2c->con);
  

• while ((stat = readw(&i2c->stat)) || (readw(&i2c->con) & I2C_CON_MST)) {
  

• 	udelay(1000);
  

Formatting change?

	/* have to read to clear intrrupt */

• 	writew (0xFFFF, I2C_STAT);
  

• 	writew(0xFFFF, &i2c->stat);
  if(--eout == 0) /* better leave with error than hang */
  	break;
  

  } } flush_fifo();

• writew (0xFFFF, I2C_STAT);
• writew (0, I2C_CNT);

• writew(0xFFFF, &i2c->stat);
• writew(0, &i2c->cnt); return i2c_error;

}

@@ -265,14 +273,14 @@ static void flush_fifo(void) * you get a bus error */ while(1){

• stat = readw(I2C_STAT);
  

• stat = readw(&i2c->stat);
  

  if(stat == I2C_STAT_RRDY){

#if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX)

• 	readb(I2C_DATA);
  

• 	readb(&i2c->data);
  

#else

• 	readw(I2C_DATA);
  

• 	readw(&i2c->data);
  

#endif

• 	writew(I2C_STAT_RRDY,I2C_STAT);
  

• 	writew(I2C_STAT_RRDY, &i2c->stat);
  udelay(1000);
  

  }else break;

@@ -283,7 +291,7 @@ int i2c_probe (uchar chip) { int res = 1; /* default = fail */

• if (chip == readw (I2C_OA)) {

• if (chip == readw(&i2c->oa)) { return res; }

@@ -291,27 +299,27 @@ int i2c_probe (uchar chip) wait_for_bb ();

/* try to read one byte */

• writew (1, I2C_CNT);

• writew(1, &i2c->cnt); /* set slave address */

• writew (chip, I2C_SA);

• writew(chip, &i2c->sa); /* stop bit needed here */

• writew (I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_STP, I2C_CON);

• writew(I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_STP, &i2c->con); /* enough delay for the NACK bit set */

• udelay (50000);

• udelay(50000);

Formatting change?

• if (!(readw (I2C_STAT) & I2C_STAT_NACK)) {

• if (!(readw(&i2c->stat) & I2C_STAT_NACK)) { res = 0; /* success case */ flush_fifo();

• writew(0xFFFF, I2C_STAT);
  

• writew(0xFFFF, &i2c->stat);
  

  } else {

• writew(0xFFFF, I2C_STAT);	 /* failue, clear sources*/
  

• writew (readw (I2C_CON) | I2C_CON_STP, I2C_CON); /* finish up xfer */
  

• writew(0xFFFF, &i2c->stat);	 /* failue, clear sources*/
  

• writew(readw(&i2c->con) | I2C_CON_STP, &i2c->con); /* finish up xfer */
  

  udelay(20000); wait_for_bb (); } flush_fifo();

• writew (0, I2C_CNT); /* don't allow any more data in...we don't want it.*/
• writew(0xFFFF, I2C_STAT);

• writew(0, &i2c->cnt); /* don't allow any more data in...we don't want it.*/
• writew(0xFFFF, &i2c->stat); return res;

}

@@ -345,18 +353,18 @@ int i2c_write (uchar chip, uint addr, int alen, uchar * buffer, int len) int i;

if (alen > 1) {

• printf ("I2C read: addr len %d not supported\n", alen);
  

• printf ("I2C write: addr len %d not supported\n", alen);
  

  return 1; }

  if (addr + len > 256) {

• printf ("I2C read: address out of range\n");
  

• printf ("I2C write: address out of range\n");
  

Here and above, typo change?

return 1;

}

for (i = 0; i < len; i++) { if (i2c_write_byte (chip, addr + i, buffer[i])) {

• 	printf ("I2C read: I/O error\n");
  

• 	printf ("I2C write: I/O error\n");
  

Typo change?

	i2c_init (CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
	return 1;
}

@@ -370,17 +378,17 @@ static void wait_for_bb (void) int timeout = 10; u16 stat;

• writew(0xFFFF, I2C_STAT); /* clear current interruts...*/
• while ((stat = readw (I2C_STAT) & I2C_STAT_BB) && timeout--) {

• writew (stat, I2C_STAT);
  

• udelay (50000);
  

Formatting change?

• writew(0xFFFF, &i2c->stat); /* clear current interruts...*/

• while ((stat = readw(&i2c->stat) & I2C_STAT_BB) && timeout--) {

• writew(stat, &i2c->stat);
  

• udelay(50000);
  

  }

  if (timeout <= 0) { printf ("timed out in wait_for_bb: I2C_STAT=%x\n",

• 	readw (I2C_STAT));
  

• 	readw(&i2c->stat));
  

  }

• writew(0xFFFF, I2C_STAT); /* clear delayed stuff*/

• writew(0xFFFF, &i2c->stat); /* clear delayed stuff*/

}

static u16 wait_for_pin (void) @@ -390,7 +398,7 @@ static u16 wait_for_pin (void)

do { udelay (1000);

• status = readw (I2C_STAT);
  

• status = readw(&i2c->stat);
  

  } while ( !(status & (I2C_STAT_ROVR | I2C_STAT_XUDF | I2C_STAT_XRDY | I2C_STAT_RRDY | I2C_STAT_ARDY | I2C_STAT_NACK |

@@ -398,8 +406,58 @@ static u16 wait_for_pin (void)

if (timeout <= 0) { printf ("timed out in wait_for_pin: I2C_STAT=%x\n",

• 	readw (I2C_STAT));
  

• 	writew(0xFFFF, I2C_STAT);
  

• 	readw(&i2c->stat));
  

• 	writew(0xFFFF, &i2c->stat);
  

} return status; }

+int i2c_set_bus_num(unsigned int bus)

Do we need an extern declaration in i2c.h for this? To be able to call it from somewhere else without warning?

+{

• if ((bus < 0) || (bus >= I2C_NUM_IF)) {

As mentioned above, I'd like something like bus max here.

• printf("Bad bus ID-%d\n", bus);
  

• return -1;
  

• }

+#if defined(CONFIG_OMAP34XX)

• if (bus == 2)

• i2c = (i2c_t *)I2C_BASE3;
  

• else

+#endif

• if (bus == 1)

• i2c = (i2c_t *)I2C_BASE2;
  

• else

• i2c = (i2c_t *)I2C_BASE1;
  

• return 0;

+}

I would remove the following three functions as they are not needed at the moment (i.e. without command line interface).

+unsigned int i2c_get_bus_num(void) +{

• if (i2c == (i2c_t *)I2C_BASE1)

• return 0;
  

• else
• if (i2c == (i2c_t *)I2C_BASE2)

• return 1;
  

+#if defined(CONFIG_OMAP34XX)

• else
• if (i2c == (i2c_t *)I2C_BASE3)

• return 2;
  

+#endif

• else

• return 0xFFFFFFFF;
  

+}

+/*

• • To be Implemented
• */

+int i2c_set_bus_speed(unsigned int speed) +{

• return 0;

+}

+unsigned int i2c_get_bus_speed(void) +{

• return 0;

+}

diff --git a/include/asm-arm/arch-omap24xx/i2c.h b/include/asm-arm/arch-omap24xx/i2c.h index 44db7a2..832a0e1 100644 --- a/include/asm-arm/arch-omap24xx/i2c.h +++ b/include/asm-arm/arch-omap24xx/i2c.h

Ah, thanks for pointing to this. I missed this.

@@ -23,24 +23,44 @@ #ifndef _OMAP24XX_I2C_H_ #define _OMAP24XX_I2C_H_

-#define I2C_BASE 0x48070000 +#define I2C_BASE1 0x48070000 #define I2C_BASE2 0x48072000 /* nothing hooked up on h4 */

I wonder why arch-omap24xx/i2c.h has no I2C_DEFAULT_BASE (used above now).

-#define I2C_REV (I2C_BASE + 0x00) -#define I2C_IE (I2C_BASE + 0x04) -#define I2C_STAT (I2C_BASE + 0x08) -#define I2C_IV (I2C_BASE + 0x0c) -#define I2C_BUF (I2C_BASE + 0x14) -#define I2C_CNT (I2C_BASE + 0x18) -#define I2C_DATA (I2C_BASE + 0x1c) -#define I2C_SYSC (I2C_BASE + 0x20) -#define I2C_CON (I2C_BASE + 0x24) -#define I2C_OA (I2C_BASE + 0x28) -#define I2C_SA (I2C_BASE + 0x2c) -#define I2C_PSC (I2C_BASE + 0x30) -#define I2C_SCLL (I2C_BASE + 0x34) -#define I2C_SCLH (I2C_BASE + 0x38) -#define I2C_SYSTEST (I2C_BASE + 0x3c) +#define I2C_DEFAULT_BASE I2C_BASE1

+typedef struct i2c {

Seems the recent syntax is without typedef.

• unsigned short rev; /* 0x00 */
• unsigned short pad_rev; /* 0x02 */

Looking at

include/asm-arm/arch-omap3/cpu.h

the current style is to use resX for padding. Like done below with res1 already. We should stay consistent here.

• unsigned short ie; /* 0x04 */
• unsigned short pad_ie; /* 0x06 */
• unsigned short stat; /* 0x08 */
• unsigned short pad_stat; /* 0x0a */
• unsigned short iv; /* 0x0c */
• unsigned short pad_iv; /* 0x0e */
• unsigned int res1; /* 0x10 */

unsigned short resX[3] instead?

• unsigned short buf; /* 0x14 */
• unsigned short pad_buf; /* 0x16 */
• unsigned short cnt; /* 0x18 */
• unsigned short pad_cnt; /* 0x1a */
• unsigned short data; /* 0x1c */
• unsigned short pad_data; /* 0x1e */
• unsigned short sysc; /* 0x20 */
• unsigned short pad_sysc; /* 0x22 */
• unsigned short con; /* 0x24 */
• unsigned short pad_con; /* 0x26 */
• unsigned short oa; /* 0x28 */
• unsigned short pad_oa; /* 0x2a */
• unsigned short sa; /* 0x2c */
• unsigned short pad_sa; /* 0x2e */
• unsigned short psc; /* 0x30 */
• unsigned short pad_psc; /* 0x32 */
• unsigned short scll; /* 0x34 */
• unsigned short pad_scll; /* 0x36 */
• unsigned short sclh; /* 0x38 */
• unsigned short pad_sclh; /* 0x3a */
• unsigned short systest; /* 0x3c */
• unsigned short pad_systest; /* 0x3e */

+} i2c_t;

/* I2C masks */

diff --git a/include/asm-arm/arch-omap3/i2c.h b/include/asm-arm/arch-omap3/i2c.h index 8b339cc..3da1fcf 100644 --- a/include/asm-arm/arch-omap3/i2c.h +++ b/include/asm-arm/arch-omap3/i2c.h @@ -25,21 +25,39 @@

#define I2C_DEFAULT_BASE I2C_BASE1

-#define I2C_REV (I2C_DEFAULT_BASE + 0x00) -#define I2C_IE (I2C_DEFAULT_BASE + 0x04) -#define I2C_STAT (I2C_DEFAULT_BASE + 0x08) -#define I2C_IV (I2C_DEFAULT_BASE + 0x0c) -#define I2C_BUF (I2C_DEFAULT_BASE + 0x14) -#define I2C_CNT (I2C_DEFAULT_BASE + 0x18) -#define I2C_DATA (I2C_DEFAULT_BASE + 0x1c) -#define I2C_SYSC (I2C_DEFAULT_BASE + 0x20) -#define I2C_CON (I2C_DEFAULT_BASE + 0x24) -#define I2C_OA (I2C_DEFAULT_BASE + 0x28) -#define I2C_SA (I2C_DEFAULT_BASE + 0x2c) -#define I2C_PSC (I2C_DEFAULT_BASE + 0x30) -#define I2C_SCLL (I2C_DEFAULT_BASE + 0x34) -#define I2C_SCLH (I2C_DEFAULT_BASE + 0x38) -#define I2C_SYSTEST (I2C_DEFAULT_BASE + 0x3c) +typedef struct i2c {

Comments from above apply here, too.

• unsigned short rev; /* 0x00 */
• unsigned short pad_rev; /* 0x02 */
• unsigned short ie; /* 0x04 */
• unsigned short pad_ie; /* 0x06 */
• unsigned short stat; /* 0x08 */
• unsigned short pad_stat; /* 0x0a */
• unsigned short iv; /* 0x0c */
• unsigned short pad_iv; /* 0x0e */
• unsigned int res1; /* 0x10 */
• unsigned short buf; /* 0x14 */
• unsigned short pad_buf; /* 0x16 */
• unsigned short cnt; /* 0x18 */
• unsigned short pad_cnt; /* 0x1a */
• unsigned short data; /* 0x1c */
• unsigned short pad_data; /* 0x1e */
• unsigned short sysc; /* 0x20 */
• unsigned short pad_sysc; /* 0x22 */
• unsigned short con; /* 0x24 */
• unsigned short pad_con; /* 0x26 */
• unsigned short oa; /* 0x28 */
• unsigned short pad_oa; /* 0x2a */
• unsigned short sa; /* 0x2c */
• unsigned short pad_sa; /* 0x2e */
• unsigned short psc; /* 0x30 */
• unsigned short pad_psc; /* 0x32 */
• unsigned short scll; /* 0x34 */
• unsigned short pad_scll; /* 0x36 */
• unsigned short sclh; /* 0x38 */
• unsigned short pad_sclh; /* 0x3a */
• unsigned short systest; /* 0x3c */
• unsigned short pad_systest; /* 0x3e */

+} i2c_t;

/* I2C masks */

diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h index 19a5ec9..d5a0d49 100644 --- a/include/configs/omap3_beagle.h +++ b/include/configs/omap3_beagle.h @@ -128,6 +128,7 @@ #define CONFIG_SYS_I2C_BUS 0 #define CONFIG_SYS_I2C_BUS_SELECT 1 #define CONFIG_DRIVER_OMAP34XX_I2C 1 +#define CONFIG_I2C_MULTI_BUS 1

Not needed.

While all above are only style questions, what's about the main functionality topic:

Do we have to call i2c_init() for bus 1 and 2 if switching to it? If yes, who does it? For bus 0 it's already in the code. I'd like that the user doesn't have to care about it. Therefore, I added

static unsigned int bus_initialized[3] = {0, 0, 0}; static unsigned int current_bus = 0;

void i2c_init (int speed, int slaveadd) { ... bus_initialized[current_bus] = 1; }

int i2c_set_bus_num(unsigned int bus) { ... current_bus = bus;

if(!bus_initialized[current_bus]) i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE); ... }

Do you like to test my patch in attachment?

It is an extension of

http://lists.denx.de/pipermail/u-boot/2009-October/063556.html

to cover arch-omap24xx/i2c.h, too.

Compile tested for omap3_beagle and omap2420h4.

Best regards

Dirk

Subject: [PATCH] OMAP2/3: I2C: Add support for second and third bus From: Dirk Behme dirk.behme@googlemail.com

Add support to use second and third I2C bus, too.

Bus 0 is still the default, but by calling i2c_set_bus_num(1/2) before doing I2C accesses, code can switch to bus 1 and 2, too. Don't forget to switch back afterwards, then.

Signed-off-by: Dirk Behme dirk.behme@googlemail.com ---

Based on Hugo Vincent's patch

http://lists.denx.de/pipermail/u-boot/2009-June/055029.html

(this patch only contains the multibus support)

drivers/i2c/omap24xx_i2c.c | 166 +++++++++++++++++++++--------------- include/asm-arm/arch-omap24xx/i2c.h | 55 ++++++++--- include/asm-arm/arch-omap3/i2c.h | 50 +++++++--- 3 files changed, 172 insertions(+), 99 deletions(-)

Index: u-boot-main/drivers/i2c/omap24xx_i2c.c =================================================================== --- u-boot-main.orig/drivers/i2c/omap24xx_i2c.c +++ u-boot-main/drivers/i2c/omap24xx_i2c.c @@ -29,6 +29,11 @@ static void wait_for_bb (void); static u16 wait_for_pin (void); static void flush_fifo(void);

+static struct i2c *i2c_base = (struct i2c *)I2C_DEFAULT_BASE; + +static unsigned int bus_initialized[3] = {0, 0, 0}; +static unsigned int current_bus = 0; + void i2c_init (int speed, int slaveadd) { int psc, fsscll, fssclh; @@ -95,30 +100,32 @@ void i2c_init (int speed, int slaveadd) sclh = (unsigned int)fssclh; }

- writew(0x2, I2C_SYSC); /* for ES2 after soft reset */ + writew(0x2, &i2c_base->sysc); /* for ES2 after soft reset */ udelay(1000); - writew(0x0, I2C_SYSC); /* will probably self clear but */ + writew(0x0, &i2c_base->sysc); /* will probably self clear but */

- if (readw (I2C_CON) & I2C_CON_EN) { - writew (0, I2C_CON); + if (readw (&i2c_base->con) & I2C_CON_EN) { + writew (0, &i2c_base->con); udelay (50000); }

- writew(psc, I2C_PSC); - writew(scll, I2C_SCLL); - writew(sclh, I2C_SCLH); + writew(psc, &i2c_base->psc); + writew(scll, &i2c_base->scll); + writew(sclh, &i2c_base->sclh);

/* own address */ - writew (slaveadd, I2C_OA); - writew (I2C_CON_EN, I2C_CON); + writew (slaveadd, &i2c_base->oa); + writew (I2C_CON_EN, &i2c_base->con);

/* have to enable intrrupts or OMAP i2c module doesn't work */ writew (I2C_IE_XRDY_IE | I2C_IE_RRDY_IE | I2C_IE_ARDY_IE | - I2C_IE_NACK_IE | I2C_IE_AL_IE, I2C_IE); + I2C_IE_NACK_IE | I2C_IE_AL_IE, &i2c_base->ie); udelay (1000); flush_fifo(); - writew (0xFFFF, I2C_STAT); - writew (0, I2C_CNT); + writew (0xFFFF, &i2c_base->stat); + writew (0, &i2c_base->cnt); + + bus_initialized[current_bus] = 1; }

static int i2c_read_byte (u8 devaddr, u8 regoffset, u8 * value) @@ -130,19 +137,19 @@ static int i2c_read_byte (u8 devaddr, u8 wait_for_bb ();

/* one byte only */ - writew (1, I2C_CNT); + writew (1, &i2c_base->cnt); /* set slave address */ - writew (devaddr, I2C_SA); + writew (devaddr, &i2c_base->sa); /* no stop bit needed here */ - writew (I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_TRX, I2C_CON); + writew (I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_TRX, &i2c_base->con);

status = wait_for_pin ();

if (status & I2C_STAT_XRDY) { /* Important: have to use byte access */ - writeb (regoffset, I2C_DATA); + writeb (regoffset, &i2c_base->data); udelay (20000); - if (readw (I2C_STAT) & I2C_STAT_NACK) { + if (readw (&i2c_base->stat) & I2C_STAT_NACK) { i2c_error = 1; } } else { @@ -151,28 +158,28 @@ static int i2c_read_byte (u8 devaddr, u8

if (!i2c_error) { /* free bus, otherwise we can't use a combined transction */ - writew (0, I2C_CON); - while (readw (I2C_STAT) || (readw (I2C_CON) & I2C_CON_MST)) { + writew (0, &i2c_base->con); + while (readw (&i2c_base->stat) || (readw (&i2c_base->con) & I2C_CON_MST)) { udelay (10000); /* Have to clear pending interrupt to clear I2C_STAT */ - writew (0xFFFF, I2C_STAT); + writew (0xFFFF, &i2c_base->stat); }

wait_for_bb (); /* set slave address */ - writew (devaddr, I2C_SA); + writew (devaddr, &i2c_base->sa); /* read one byte from slave */ - writew (1, I2C_CNT); + writew (1, &i2c_base->cnt); /* need stop bit here */ writew (I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_STP, - I2C_CON); + &i2c_base->con);

status = wait_for_pin (); if (status & I2C_STAT_RRDY) { #if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX) - *value = readb (I2C_DATA); + *value = readb (&i2c_base->data); #else - *value = readw (I2C_DATA); + *value = readw (&i2c_base->data); #endif udelay (20000); } else { @@ -180,17 +187,17 @@ static int i2c_read_byte (u8 devaddr, u8 }

if (!i2c_error) { - writew (I2C_CON_EN, I2C_CON); - while (readw (I2C_STAT) - || (readw (I2C_CON) & I2C_CON_MST)) { + writew (I2C_CON_EN, &i2c_base->con); + while (readw (&i2c_base->stat) + || (readw (&i2c_base->con) & I2C_CON_MST)) { udelay (10000); - writew (0xFFFF, I2C_STAT); + writew (0xFFFF, &i2c_base->stat); } } } flush_fifo(); - writew (0xFFFF, I2C_STAT); - writew (0, I2C_CNT); + writew (0xFFFF, &i2c_base->stat); + writew (0, &i2c_base->cnt); return i2c_error; }

@@ -203,12 +210,12 @@ static int i2c_write_byte (u8 devaddr, u wait_for_bb ();

/* two bytes */ - writew (2, I2C_CNT); + writew (2, &i2c_base->cnt); /* set slave address */ - writew (devaddr, I2C_SA); + writew (devaddr, &i2c_base->sa); /* stop bit needed here */ writew (I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_TRX | - I2C_CON_STP, I2C_CON); + I2C_CON_STP, &i2c_base->con);

/* wait until state change */ status = wait_for_pin (); @@ -216,24 +223,24 @@ static int i2c_write_byte (u8 devaddr, u if (status & I2C_STAT_XRDY) { #if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX) /* send out 1 byte */ - writeb (regoffset, I2C_DATA); - writew (I2C_STAT_XRDY, I2C_STAT); + writeb (regoffset, &i2c_base->data); + writew (I2C_STAT_XRDY, &i2c_base->stat);

status = wait_for_pin (); if ((status & I2C_STAT_XRDY)) { /* send out next 1 byte */ - writeb (value, I2C_DATA); - writew (I2C_STAT_XRDY, I2C_STAT); + writeb (value, &i2c_base->data); + writew (I2C_STAT_XRDY, &i2c_base->stat); } else { i2c_error = 1; } #else /* send out two bytes */ - writew ((value << 8) + regoffset, I2C_DATA); + writew ((value << 8) + regoffset, &i2c_base->data); #endif /* must have enough delay to allow BB bit to go low */ udelay (50000); - if (readw (I2C_STAT) & I2C_STAT_NACK) { + if (readw (&i2c_base->stat) & I2C_STAT_NACK) { i2c_error = 1; } } else { @@ -243,18 +250,18 @@ static int i2c_write_byte (u8 devaddr, u if (!i2c_error) { int eout = 200;

- writew (I2C_CON_EN, I2C_CON); - while ((stat = readw (I2C_STAT)) || (readw (I2C_CON) & I2C_CON_MST)) { + writew (I2C_CON_EN, &i2c_base->con); + while ((stat = readw (&i2c_base->stat)) || (readw (&i2c_base->con) & I2C_CON_MST)) { udelay (1000); /* have to read to clear intrrupt */ - writew (0xFFFF, I2C_STAT); + writew (0xFFFF, &i2c_base->stat); if(--eout == 0) /* better leave with error than hang */ break; } } flush_fifo(); - writew (0xFFFF, I2C_STAT); - writew (0, I2C_CNT); + writew (0xFFFF, &i2c_base->stat); + writew (0, &i2c_base->cnt); return i2c_error; }

@@ -265,14 +272,14 @@ static void flush_fifo(void) * you get a bus error */ while(1){ - stat = readw(I2C_STAT); + stat = readw(&i2c_base->stat); if(stat == I2C_STAT_RRDY){ #if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX) - readb(I2C_DATA); + readb(&i2c_base->data); #else - readw(I2C_DATA); + readw(&i2c_base->data); #endif - writew(I2C_STAT_RRDY,I2C_STAT); + writew(I2C_STAT_RRDY,&i2c_base->stat); udelay(1000); }else break; @@ -283,7 +290,7 @@ int i2c_probe (uchar chip) { int res = 1; /* default = fail */

- if (chip == readw (I2C_OA)) { + if (chip == readw (&i2c_base->oa)) { return res; }

@@ -291,27 +298,27 @@ int i2c_probe (uchar chip) wait_for_bb ();

/* try to read one byte */ - writew (1, I2C_CNT); + writew (1, &i2c_base->cnt); /* set slave address */ - writew (chip, I2C_SA); + writew (chip, &i2c_base->sa); /* stop bit needed here */ - writew (I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_STP, I2C_CON); + writew (I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_STP, &i2c_base->con); /* enough delay for the NACK bit set */ udelay (50000);

- if (!(readw (I2C_STAT) & I2C_STAT_NACK)) { + if (!(readw (&i2c_base->stat) & I2C_STAT_NACK)) { res = 0; /* success case */ flush_fifo(); - writew(0xFFFF, I2C_STAT); + writew(0xFFFF, &i2c_base->stat); } else { - writew(0xFFFF, I2C_STAT); /* failue, clear sources*/ - writew (readw (I2C_CON) | I2C_CON_STP, I2C_CON); /* finish up xfer */ + writew(0xFFFF, &i2c_base->stat); /* failue, clear sources*/ + writew (readw (&i2c_base->con) | I2C_CON_STP, &i2c_base->con); /* finish up xfer */ udelay(20000); wait_for_bb (); } flush_fifo(); - writew (0, I2C_CNT); /* don't allow any more data in...we don't want it.*/ - writew(0xFFFF, I2C_STAT); + writew (0, &i2c_base->cnt); /* don't allow any more data in...we don't want it.*/ + writew(0xFFFF, &i2c_base->stat); return res; }

@@ -370,17 +377,17 @@ static void wait_for_bb (void) int timeout = 10; u16 stat;

- writew(0xFFFF, I2C_STAT); /* clear current interruts...*/ - while ((stat = readw (I2C_STAT) & I2C_STAT_BB) && timeout--) { - writew (stat, I2C_STAT); + writew(0xFFFF, &i2c_base->stat); /* clear current interruts...*/ + while ((stat = readw (&i2c_base->stat) & I2C_STAT_BB) && timeout--) { + writew (stat, &i2c_base->stat); udelay (50000); }

if (timeout <= 0) { printf ("timed out in wait_for_bb: I2C_STAT=%x\n", - readw (I2C_STAT)); + readw (&i2c_base->stat)); } - writew(0xFFFF, I2C_STAT); /* clear delayed stuff*/ + writew(0xFFFF, &i2c_base->stat); /* clear delayed stuff*/ }

static u16 wait_for_pin (void) @@ -390,7 +397,7 @@ static u16 wait_for_pin (void)

do { udelay (1000); - status = readw (I2C_STAT); + status = readw (&i2c_base->stat); } while ( !(status & (I2C_STAT_ROVR | I2C_STAT_XUDF | I2C_STAT_XRDY | I2C_STAT_RRDY | I2C_STAT_ARDY | I2C_STAT_NACK | @@ -398,8 +405,33 @@ static u16 wait_for_pin (void)

if (timeout <= 0) { printf ("timed out in wait_for_pin: I2C_STAT=%x\n", - readw (I2C_STAT)); - writew(0xFFFF, I2C_STAT); + readw (&i2c_base->stat)); + writew(0xFFFF, &i2c_base->stat); } return status; } + +int i2c_set_bus_num(unsigned int bus) +{ + if ((bus < 0) || (bus >= I2C_BUS_MAX)) { + printf("Bad bus: %d\n", bus); + return -1; + } + +#ifdef CONFIG_OMAP34XX + if (bus == 2) + i2c_base = (struct i2c *)I2C_BASE3; + else +#endif + if (bus == 1) + i2c_base = (struct i2c *)I2C_BASE2; + else + i2c_base = (struct i2c *)I2C_BASE1; + + current_bus = bus; + + if(!bus_initialized[current_bus]) + i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE); + + return 0; +} Index: u-boot-main/include/asm-arm/arch-omap3/i2c.h =================================================================== --- u-boot-main.orig/include/asm-arm/arch-omap3/i2c.h +++ u-boot-main/include/asm-arm/arch-omap3/i2c.h @@ -25,21 +25,39 @@

#define I2C_DEFAULT_BASE I2C_BASE1

-#define I2C_REV (I2C_DEFAULT_BASE + 0x00) -#define I2C_IE (I2C_DEFAULT_BASE + 0x04) -#define I2C_STAT (I2C_DEFAULT_BASE + 0x08) -#define I2C_IV (I2C_DEFAULT_BASE + 0x0c) -#define I2C_BUF (I2C_DEFAULT_BASE + 0x14) -#define I2C_CNT (I2C_DEFAULT_BASE + 0x18) -#define I2C_DATA (I2C_DEFAULT_BASE + 0x1c) -#define I2C_SYSC (I2C_DEFAULT_BASE + 0x20) -#define I2C_CON (I2C_DEFAULT_BASE + 0x24) -#define I2C_OA (I2C_DEFAULT_BASE + 0x28) -#define I2C_SA (I2C_DEFAULT_BASE + 0x2c) -#define I2C_PSC (I2C_DEFAULT_BASE + 0x30) -#define I2C_SCLL (I2C_DEFAULT_BASE + 0x34) -#define I2C_SCLH (I2C_DEFAULT_BASE + 0x38) -#define I2C_SYSTEST (I2C_DEFAULT_BASE + 0x3c) +struct i2c { + unsigned short rev; /* 0x00 */ + unsigned short res1; + unsigned short ie; /* 0x04 */ + unsigned short res2; + unsigned short stat; /* 0x08 */ + unsigned short res3; + unsigned short iv; /* 0x0C */ + unsigned short res4[3]; + unsigned short buf; /* 0x14 */ + unsigned short res5; + unsigned short cnt; /* 0x18 */ + unsigned short res6; + unsigned short data; /* 0x1C */ + unsigned short res7; + unsigned short sysc; /* 0x20 */ + unsigned short res8; + unsigned short con; /* 0x24 */ + unsigned short res9; + unsigned short oa; /* 0x28 */ + unsigned short res10; + unsigned short sa; /* 0x2C */ + unsigned short res11; + unsigned short psc; /* 0x30 */ + unsigned short res12; + unsigned short scll; /* 0x34 */ + unsigned short res13; + unsigned short sclh; /* 0x38 */ + unsigned short res14; + unsigned short systest; /* 0x3c */ +}; + +#define I2C_BUS_MAX 3

/* I2C masks */

@@ -181,4 +199,6 @@ #define I2C_PSC_MAX 0x0f #define I2C_PSC_MIN 0x00

+extern int i2c_set_bus_num(unsigned int); + #endif /* _I2C_H_ */ Index: u-boot-main/include/asm-arm/arch-omap24xx/i2c.h =================================================================== --- u-boot-main.orig/include/asm-arm/arch-omap24xx/i2c.h +++ u-boot-main/include/asm-arm/arch-omap24xx/i2c.h @@ -6,7 +6,7 @@ * project. * * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License as + * moify it under the terms of the GNU General Public License as * published by the Free Software Foundation; either version 2 of * the License, or (at your option) any later version. * @@ -23,24 +23,44 @@ #ifndef _OMAP24XX_I2C_H_ #define _OMAP24XX_I2C_H_

-#define I2C_BASE 0x48070000 +#define I2C_BASE1 0x48070000 #define I2C_BASE2 0x48072000 /* nothing hooked up on h4 */

-#define I2C_REV (I2C_BASE + 0x00) -#define I2C_IE (I2C_BASE + 0x04) -#define I2C_STAT (I2C_BASE + 0x08) -#define I2C_IV (I2C_BASE + 0x0c) -#define I2C_BUF (I2C_BASE + 0x14) -#define I2C_CNT (I2C_BASE + 0x18) -#define I2C_DATA (I2C_BASE + 0x1c) -#define I2C_SYSC (I2C_BASE + 0x20) -#define I2C_CON (I2C_BASE + 0x24) -#define I2C_OA (I2C_BASE + 0x28) -#define I2C_SA (I2C_BASE + 0x2c) -#define I2C_PSC (I2C_BASE + 0x30) -#define I2C_SCLL (I2C_BASE + 0x34) -#define I2C_SCLH (I2C_BASE + 0x38) -#define I2C_SYSTEST (I2C_BASE + 0x3c) +#define I2C_DEFAULT_BASE I2C_BASE1 + +struct i2c { + unsigned short rev; /* 0x00 */ + unsigned short res1; + unsigned short ie; /* 0x04 */ + unsigned short res2; + unsigned short stat; /* 0x08 */ + unsigned short res3; + unsigned short iv; /* 0x0C */ + unsigned short res4[3]; + unsigned short buf; /* 0x14 */ + unsigned short res5; + unsigned short cnt; /* 0x18 */ + unsigned short res6; + unsigned short data; /* 0x1C */ + unsigned short res7; + unsigned short sysc; /* 0x20 */ + unsigned short res8; + unsigned short con; /* 0x24 */ + unsigned short res9; + unsigned short oa; /* 0x28 */ + unsigned short res10; + unsigned short sa; /* 0x2C */ + unsigned short res11; + unsigned short psc; /* 0x30 */ + unsigned short res12; + unsigned short scll; /* 0x34 */ + unsigned short res13; + unsigned short sclh; /* 0x38 */ + unsigned short res14; + unsigned short systest; /* 0x3c */ +}; + +#define I2C_BUS_MAX 2

/* I2C masks */

@@ -147,5 +167,6 @@ #define I2C_PSC_MAX 0x0f #define I2C_PSC_MIN 0x00

+extern int i2c_set_bus_num(unsigned int);

#endif