Re: [U-Boot] [PATCH 17/24] mxc_i2c: add bus recovery support

On 6/24/2012 1:51 AM, Heiko Schocher wrote:

Hello Troy,

On 22.06.2012 06:12, Troy Kisky wrote:

...

Toggling the scl line 9 clocks is the standard way of returning a locked up bus to idle condition.

Signed-off-by: Troy Kiskytroy.kisky@boundarydevices.com

drivers/i2c/mxc_i2c.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)

diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c index ec05798..339bb6f 100644 --- a/drivers/i2c/mxc_i2c.c +++ b/drivers/i2c/mxc_i2c.c @@ -246,6 +246,8 @@ static int i2c_init_transfer_(struct mxc_i2c_regs *i2c_regs, return 0; }

+static void toggle_i2c(void *i2c_regs);

• static int i2c_init_transfer(struct mxc_i2c_regs *i2c_regs, uchar chip, uint addr, int alen) {

@@ -264,6 +266,7 @@ static int i2c_init_transfer(struct mxc_i2c_regs *i2c_regs, if (ret != -ERESTART) writeb(0,&i2c_regs->i2cr); /* Disable controller */ udelay(100);

•    toggle_i2c(i2c_regs);
  }
  printf("%s: give up i2c_regs=%p\n", __func__, i2c_regs);
  return ret;
  

@@ -381,6 +384,29 @@ void *get_base(void) #endif }

+static struct i2c_parms *i2c_get_parms(void *base) +{

• int i = 0;
• struct i2c_parms *p = g_parms;
• while (i< ARRAY_SIZE(g_parms)) {

•    if (p->base == base)
  

•        return p;
  

•    p++;
  

•    i++;
  

• }
• printf("Invalid I2C base: %p\n", base);
• return NULL;

+}

+static void toggle_i2c(void *base) +{

• struct i2c_parms *p = i2c_get_parms(base);
• if (!p)

•    return;
  

• if (p->toggle_fn)

•    p->toggle_fn(p->toggle_data);
  

+}

• int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len) { return bus_i2c_read(get_base(), chip, addr, alen, buf, len);

Hmm.. why you cannot use the CONFIG_SYS_I2C_INIT_BOARD and i2c_init_board()

The fsl_i2c.c file uses CONFIG_SYS_I2C_INIT_BOARD to call the function.

I could add similar code to mxc_i2c.c, (adding a bus number parameter), but I do prefer the way I implemented it. Why should the bus recovery be limited to i2c_init?

for unblocking the i2c bus? And where is the function, which really toggles the SCL pin, as you described in the commit message?

Your right, my commit message is misleading, I'll update. The actual toggling function is in a later patch. Thanks for the review

Troy