[U-Boot] [PATCH] Add I2C multibus support for OMAP2/3 boards
Dirk Behme
dirk.behme at googlemail.com
Sun Nov 1 09:24:01 CET 2009
Tom Rix wrote:
> From: Syed Mohammed Khasim <khasim at 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 at ti.com>
> Signed-off-by: Jason Kridner <jkridner at beagleboard.org>
> Signed-off-by: Tom Rix <Tom.Rix at 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
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: omap3_i2c_multibus_patch.txt
Url: http://lists.denx.de/pipermail/u-boot/attachments/20091101/62cb74ab/attachment.txt
More information about the U-Boot
mailing list