Skip to contentSkip to conversation

mxc_i2c: fix i2c_imx_stop

PATCH Superseded Troy Kisky 7 participants 55 messages Jun 22, 2012 – Jun 26, 2012

Subsystems (5)

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Troy Kisky

author
#0 lore raw

PATCH 1/24 mxc_i2c: fix i2c_imx_stop

Instead of clearing 2 bits, all the other
bits were set because '|=' was used instead
of '&='.

---
 drivers/i2c/mxc_i2c.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Note:

All patches in the series are based on the i2c/master branch even though only 1-18 will be applied there.

drivers/i2c/mxc_i2c.c +1 -1
index fc68062..c0c45fd 100644
@@ -264,7 +264,7 @@ void i2c_imx_stop(void)
 
 	/* Stop I2C transaction */
 	temp = readb(&i2c_regs->i2cr);
-	temp |= ~(I2CR_MSTA | I2CR_MTX);
+	temp &= ~(I2CR_MSTA | I2CR_MTX);
 	writeb(temp, &i2c_regs->i2cr);
 
 	i2c_imx_bus_busy(0);
Signature · 1 line
1.7.9.5
author
#1 lore raw

PATCH 2/24 mxc_i2c: remove ifdef of CONFIG_HARD_I2C

This is always selected when CONFIG_I2C_MXC is
selected, so it adds no value.

---
 drivers/i2c/mxc_i2c.c |    6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)
drivers/i2c/mxc_i2c.c +1 -5
index c0c45fd..0b46c9c 100644
@@ -31,12 +31,9 @@
  */
 
 #include <common.h>
-#include <asm/io.h>
-
-#if defined(CONFIG_HARD_I2C)
-
 #include <asm/arch/clock.h>
 #include <asm/arch/imx-regs.h>
+#include <asm/io.h>
 #include <i2c.h>
 
 struct mxc_i2c_regs {
@@ -446,4 +443,3 @@ int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len)
 
 	return ret;
 }
-#endif /* CONFIG_HARD_I2C */
Signature · 1 line
1.7.9.5
#2 lore raw

PATCH 2/24 mxc_i2c: remove ifdef of CONFIG_HARD_I2C

Dear Troy Kisky,

Quoted 4 lines

This is always selected when CONFIG_I2C_MXC is
selected, so it adds no value.

Quoted 23 lines
---
 drivers/i2c/mxc_i2c.c |    6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)
drivers/i2c/mxc_i2c.c +1 -4
index c0c45fd..0b46c9c 100644
@@ -31,12 +31,9 @@
  */
 
 #include <common.h>
-#include <asm/io.h>
-
-#if defined(CONFIG_HARD_I2C)
-
 #include <asm/arch/clock.h>
 #include <asm/arch/imx-regs.h>
+#include <asm/io.h>
 #include <i2c.h>
 
 struct mxc_i2c_regs {

drivers/i2c/mxc_i2c.c int i2c_write(uchar chip, uint addr, int alen, uchar · line 449

Quoted 5 lines
drivers/i2c/mxc_i2c.c +0 -1
@@ -446,4 +443,3 @@ int i2c_write(uchar chip, uint addr, int alen, uchar
 *buf, int len)
 
 	return ret;
 }
-#endif /* CONFIG_HARD_I2C */

Best regards,
Marek Vasut

author
#3 lore raw

PATCH 3/24 mxc_i2c: create tx_byte function

In reply to Troy Kisky

Use tx_byte function instead of having 3 copies
of the code.

---
 drivers/i2c/mxc_i2c.c |   72 +++++++++++++++----------------------------------
 1 file changed, 21 insertions(+), 51 deletions(-)
drivers/i2c/mxc_i2c.c +21 -51
index 0b46c9c..0fd508a 100644
@@ -33,6 +33,7 @@
 #include <common.h>
 #include <asm/arch/clock.h>
 #include <asm/arch/imx-regs.h>
+#include <asm/errno.h>
 #include <asm/io.h>
 #include <i2c.h>
 
@@ -207,17 +208,21 @@ int i2c_imx_trx_complete(void)
 		udelay(1);
 	}
 
-	return 1;
+	return -ETIMEDOUT;
 }
 
-/*
- * Check if the transaction was ACKed
- */
-int i2c_imx_acked(void)
+static int tx_byte(struct mxc_i2c_regs *i2c_regs, u8 byte)
 {
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
+	unsigned ret;
 
-	return readb(&i2c_regs->i2sr) & I2SR_RX_NO_AK;
+	writeb(byte, &i2c_regs->i2dr);
+	ret = i2c_imx_trx_complete();
+	if (ret < 0)
+		return ret;
+	ret = readb(&i2c_regs->i2sr);
+	if (ret & I2SR_RX_NO_AK)
+		return -ENODEV;
+	return 0;
 }
 
 /*
@@ -271,30 +276,6 @@ void i2c_imx_stop(void)
 }
 
 /*
- * Set chip address and access mode
- *
- * read = 1: READ access
- * read = 0: WRITE access
- */
-int i2c_imx_set_chip_addr(uchar chip, int read)
-{
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
-	int ret;
-
-	writeb((chip << 1) | read, &i2c_regs->i2dr);
-
-	ret = i2c_imx_trx_complete();
-	if (ret)
-		return ret;
-
-	ret = i2c_imx_acked();
-	if (ret)
-		return ret;
-
-	return ret;
-}
-
-/*
  * Write register address
  */
 int i2c_imx_set_reg_addr(uint addr, int alen)
@@ -303,14 +284,8 @@ int i2c_imx_set_reg_addr(uint addr, int alen)
 	int ret = 0;
 
 	while (alen--) {
-		writeb((addr >> (alen * 8)) & 0xff, &i2c_regs->i2dr);
-
-		ret = i2c_imx_trx_complete();
-		if (ret)
-			break;
-
-		ret = i2c_imx_acked();
-		if (ret)
+		ret =  tx_byte(i2c_regs, (addr >> (alen * 8)) & 0xff);
+		if (ret < 0)
 			break;
 	}
 
@@ -322,13 +297,14 @@ int i2c_imx_set_reg_addr(uint addr, int alen)
  */
 int i2c_probe(uchar chip)
 {
+	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
 	int ret;
 
 	ret = i2c_imx_start();
 	if (ret)
 		return ret;
 
-	ret = i2c_imx_set_chip_addr(chip, 0);
+	ret = tx_byte(i2c_regs, chip << 1);
 	if (ret)
 		return ret;
 
@@ -352,7 +328,7 @@ int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
 		return ret;
 
 	/* write slave address */
-	ret = i2c_imx_set_chip_addr(chip, 0);
+	ret = tx_byte(i2c_regs, chip << 1);
 	if (ret)
 		return ret;
 
@@ -364,7 +340,7 @@ int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
 	temp |= I2CR_RSTA;
 	writeb(temp, &i2c_regs->i2cr);
 
-	ret = i2c_imx_set_chip_addr(chip, 1);
+	ret = tx_byte(i2c_regs, (chip << 1) | 1);
 	if (ret)
 		return ret;
 
@@ -419,7 +395,7 @@ int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len)
 		return ret;
 
 	/* write slave address */
-	ret = i2c_imx_set_chip_addr(chip, 0);
+	ret = tx_byte(i2c_regs, chip << 1);
 	if (ret)
 		return ret;
 
@@ -428,14 +404,8 @@ int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len)
 		return ret;
 
 	for (i = 0; i < len; i++) {
-		writeb(buf[i], &i2c_regs->i2dr);
-
-		ret = i2c_imx_trx_complete();
-		if (ret)
-			return ret;
-
-		ret = i2c_imx_acked();
-		if (ret)
+		ret =  tx_byte(i2c_regs, buf[i]);
+		if (ret < 0)
 			return ret;
 	}
 
Signature · 1 line
1.7.9.5
#4 lore raw

PATCH 3/24 mxc_i2c: create tx_byte function

Dear Troy Kisky,

Quoted 124 lines

Use tx_byte function instead of having 3 copies
of the code.

---
 drivers/i2c/mxc_i2c.c |   72

+++++++++++++++---------------------------------- 1 file changed, 21 insertions(+), 51 deletions(-)

drivers/i2c/mxc_i2c.c +17 -41
index 0b46c9c..0fd508a 100644
@@ -33,6 +33,7 @@
 #include <common.h>
 #include <asm/arch/clock.h>
 #include <asm/arch/imx-regs.h>
+#include <asm/errno.h>
 #include <asm/io.h>
 #include <i2c.h>
 
@@ -207,17 +208,21 @@ int i2c_imx_trx_complete(void)
 		udelay(1);
 	}
 
-	return 1;
+	return -ETIMEDOUT;
 }
 
-/*
- * Check if the transaction was ACKed
- */
-int i2c_imx_acked(void)
+static int tx_byte(struct mxc_i2c_regs *i2c_regs, u8 byte)
 {
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
+	unsigned ret;
 
-	return readb(&i2c_regs->i2sr) & I2SR_RX_NO_AK;
+	writeb(byte, &i2c_regs->i2dr);
+	ret = i2c_imx_trx_complete();
+	if (ret < 0)
+		return ret;
+	ret = readb(&i2c_regs->i2sr);
+	if (ret & I2SR_RX_NO_AK)
+		return -ENODEV;
+	return 0;
 }
 
 /*
@@ -271,30 +276,6 @@ void i2c_imx_stop(void)
 }
 
 /*
- * Set chip address and access mode
- *
- * read = 1: READ access
- * read = 0: WRITE access
- */
-int i2c_imx_set_chip_addr(uchar chip, int read)
-{
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
-	int ret;
-
-	writeb((chip << 1) | read, &i2c_regs->i2dr);
-
-	ret = i2c_imx_trx_complete();
-	if (ret)
-		return ret;
-
-	ret = i2c_imx_acked();
-	if (ret)
-		return ret;
-
-	return ret;
-}
-
-/*
  * Write register address
  */
 int i2c_imx_set_reg_addr(uint addr, int alen)
@@ -303,14 +284,8 @@ int i2c_imx_set_reg_addr(uint addr, int alen)
 	int ret = 0;
 
 	while (alen--) {
-		writeb((addr >> (alen * 8)) & 0xff, &i2c_regs->i2dr);
-
-		ret = i2c_imx_trx_complete();
-		if (ret)
-			break;
-
-		ret = i2c_imx_acked();
-		if (ret)
+		ret =  tx_byte(i2c_regs, (addr >> (alen * 8)) & 0xff);
+		if (ret < 0)
 			break;
 	}
 
@@ -322,13 +297,14 @@ int i2c_imx_set_reg_addr(uint addr, int alen)
  */
 int i2c_probe(uchar chip)
 {
+	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
 	int ret;
 
 	ret = i2c_imx_start();
 	if (ret)
 		return ret;
 
-	ret = i2c_imx_set_chip_addr(chip, 0);
+	ret = tx_byte(i2c_regs, chip << 1);
 	if (ret)
 		return ret;
 
@@ -352,7 +328,7 @@ int i2c_read(uchar chip, uint addr, int alen, uchar
 *buf, int len) return ret;
 
 	/* write slave address */
-	ret = i2c_imx_set_chip_addr(chip, 0);
+	ret = tx_byte(i2c_regs, chip << 1);
 	if (ret)
 		return ret;
 

drivers/i2c/mxc_i2c.c int i2c_read(uchar chip, uint addr, int alen, uchar · line 343

Quoted 5 lines
drivers/i2c/mxc_i2c.c +1 -1
@@ -364,7 +340,7 @@ int i2c_read(uchar chip, uint addr, int alen, uchar
 *buf, int len) temp |= I2CR_RSTA;
 	writeb(temp, &i2c_regs->i2cr);
 
-	ret = i2c_imx_set_chip_addr(chip, 1);
+	ret = tx_byte(i2c_regs, (chip << 1) | 1);

Isn't this | 1 and | 0 stuff #define-d somewhere? I think there was I2C_READ_SOMETHING in i2c.h and I2C_WRITE_SOMETHING...

drivers/i2c/mxc_i2c.c int i2c_imx_set_reg_addr(uint addr, int alen) · line 306

Quoted 3 lines
drivers/i2c/mxc_i2c.c +0 -0
 	if (ret)
 		return ret;
 

Otherwise

Best regards,
Marek Vasut

author
#5 lore raw

PATCH 3/24 mxc_i2c: create tx_byte function

Quoted 125 lines · Marek Vasut

Dear Troy Kisky,

Quoted 123 lines

Use tx_byte function instead of having 3 copies
of the code.

---
  drivers/i2c/mxc_i2c.c |   72

+++++++++++++++---------------------------------- 1 file changed, 21 insertions(+), 51 deletions(-)

drivers/i2c/mxc_i2c.c +17 -41
index 0b46c9c..0fd508a 100644
@@ -33,6 +33,7 @@
  #include <common.h>
  #include <asm/arch/clock.h>
  #include <asm/arch/imx-regs.h>
+#include <asm/errno.h>
  #include <asm/io.h>
  #include <i2c.h>
 
@@ -207,17 +208,21 @@ int i2c_imx_trx_complete(void)
  		udelay(1);
  	}
 
-	return 1;
+	return -ETIMEDOUT;
  }
 
-/*
- * Check if the transaction was ACKed
- */
-int i2c_imx_acked(void)
+static int tx_byte(struct mxc_i2c_regs *i2c_regs, u8 byte)
  {
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
+	unsigned ret;
 
-	return readb(&i2c_regs->i2sr) & I2SR_RX_NO_AK;
+	writeb(byte, &i2c_regs->i2dr);
+	ret = i2c_imx_trx_complete();
+	if (ret < 0)
+		return ret;
+	ret = readb(&i2c_regs->i2sr);
+	if (ret & I2SR_RX_NO_AK)
+		return -ENODEV;
+	return 0;
  }
 
  /*
@@ -271,30 +276,6 @@ void i2c_imx_stop(void)
  }
 
  /*
- * Set chip address and access mode
- *
- * read = 1: READ access
- * read = 0: WRITE access
- */
-int i2c_imx_set_chip_addr(uchar chip, int read)
-{
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
-	int ret;
-
-	writeb((chip << 1) | read, &i2c_regs->i2dr);
-
-	ret = i2c_imx_trx_complete();
-	if (ret)
-		return ret;
-
-	ret = i2c_imx_acked();
-	if (ret)
-		return ret;
-
-	return ret;
-}
-
-/*
   * Write register address
   */
  int i2c_imx_set_reg_addr(uint addr, int alen)
@@ -303,14 +284,8 @@ int i2c_imx_set_reg_addr(uint addr, int alen)
  	int ret = 0;
 
  	while (alen--) {
-		writeb((addr >> (alen * 8)) & 0xff, &i2c_regs->i2dr);
-
-		ret = i2c_imx_trx_complete();
-		if (ret)
-			break;
-
-		ret = i2c_imx_acked();
-		if (ret)
+		ret =  tx_byte(i2c_regs, (addr >> (alen * 8)) & 0xff);
+		if (ret < 0)
  			break;
  	}
 
@@ -322,13 +297,14 @@ int i2c_imx_set_reg_addr(uint addr, int alen)
   */
  int i2c_probe(uchar chip)
  {
+	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
  	int ret;
 
  	ret = i2c_imx_start();
  	if (ret)
  		return ret;
 
-	ret = i2c_imx_set_chip_addr(chip, 0);
+	ret = tx_byte(i2c_regs, chip << 1);
  	if (ret)
  		return ret;
 
@@ -352,7 +328,7 @@ int i2c_read(uchar chip, uint addr, int alen, uchar
 *buf, int len) return ret;
 
  	/* write slave address */
-	ret = i2c_imx_set_chip_addr(chip, 0);
+	ret = tx_byte(i2c_regs, chip << 1);
  	if (ret)
  		return ret;
 
Quoted 8 lines

drivers/i2c/mxc_i2c.c int i2c_read(uchar chip, uint addr, int alen, uchar · line 343

Quoted 6 lines
drivers/i2c/mxc_i2c.c +1 -1
@@ -364,7 +340,7 @@ int i2c_read(uchar chip, uint addr, int alen, uchar
 *buf, int len) temp |= I2CR_RSTA;
  	writeb(temp, &i2c_regs->i2cr);
 
-	ret = i2c_imx_set_chip_addr(chip, 1);
+	ret = tx_byte(i2c_regs, (chip << 1) | 1);

Isn't this | 1 and | 0 stuff #define-d somewhere? I think there was I2C_READ_SOMETHING in i2c.h and I2C_WRITE_SOMETHING...

I could not find what you are referring to. All drivers in i2c seem to use "| 1"
"| dir" and I2C_READ_BIT, I2C_WRITE_BIT

#define I2C_READ_BIT 1
#define I2C_WRITE_BIT 0

in fsl_i2c.c

But these are not defined in a header file.

Quoted 9 lines

drivers/i2c/mxc_i2c.c int i2c_imx_set_reg_addr(uint addr, int alen) · line 306

Quoted 3 lines
drivers/i2c/mxc_i2c.c +0 -0
  	if (ret)
  		return ret;
 

Otherwise

Best regards,
Marek Vasut

#6 lore raw

PATCH 3/24 mxc_i2c: create tx_byte function

Dear Troy Kisky,

[...]

Quoted 15 lines
Quoted 10 lines

drivers/i2c/mxc_i2c.c int i2c_read(uchar chip, uint addr, int alen, uchar · line 344

Quoted 7 lines
drivers/i2c/mxc_i2c.c +1 -1
@@ -364,7 +340,7 @@ int i2c_read(uchar chip, uint addr, int alen, uchar
 *buf, int len) temp |= I2CR_RSTA;
 
  	writeb(temp, &i2c_regs->i2cr);
 
-	ret = i2c_imx_set_chip_addr(chip, 1);
+	ret = tx_byte(i2c_regs, (chip << 1) | 1);

Isn't this | 1 and | 0 stuff #define-d somewhere? I think there was I2C_READ_SOMETHING in i2c.h and I2C_WRITE_SOMETHING...

I could not find what you are referring to. All drivers in i2c seem to use "| 1"
"| dir" and I2C_READ_BIT, I2C_WRITE_BIT

Quoted 7 lines

#define I2C_READ_BIT 1
#define I2C_WRITE_BIT 0

in fsl_i2c.c

But these are not defined in a header file.

You're right. I must have mistaken them for something else. Sorry!

Quoted 9 lines
Quoted 9 lines
Quoted 3 lines
(quoted diff) +0 -0
  	if (ret)
 

return ret;

Otherwise

Best regards,
Marek Vasut

Best regards,
Marek Vasut

author
#7 lore raw

PATCH 4/24 mxc_i2c: clear i2sr before waiting for bit

In reply to Troy Kisky

Let's clear the sr register before waiting for bit to be set, instead of clearing it after hardware sets it. No real operational difference here, but allows combining of i2c_imx_trx_complete and i2c_imx_bus_busy in later patches.

---
 drivers/i2c/mxc_i2c.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
drivers/i2c/mxc_i2c.c +2 -3
index 0fd508a..bae9335 100644
@@ -200,10 +200,8 @@ int i2c_imx_trx_complete(void)
 	int timeout = I2C_MAX_TIMEOUT;
 
 	while (timeout--) {
-		if (readb(&i2c_regs->i2sr) & I2SR_IIF) {
-			writeb(0, &i2c_regs->i2sr);
+		if (readb(&i2c_regs->i2sr) & I2SR_IIF)
 			return 0;
-		}
 
 		udelay(1);
 	}
@@ -215,6 +213,7 @@ static int tx_byte(struct mxc_i2c_regs *i2c_regs, u8 byte)
 {
 	unsigned ret;
 
+	writeb(0, &i2c_regs->i2sr);
 	writeb(byte, &i2c_regs->i2dr);
 	ret = i2c_imx_trx_complete();
 	if (ret < 0)
Signature · 1 line
1.7.9.5
#8 lore raw

PATCH 4/24 mxc_i2c: clear i2sr before waiting for bit

Dear Troy Kisky,

Quoted 28 lines

Let's clear the sr register before waiting for bit to be set, instead of clearing it after hardware sets it. No real operational difference here, but allows combining of i2c_imx_trx_complete and i2c_imx_bus_busy in later patches.

---
 drivers/i2c/mxc_i2c.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
drivers/i2c/mxc_i2c.c +1 -3
index 0fd508a..bae9335 100644
@@ -200,10 +200,8 @@ int i2c_imx_trx_complete(void)
 	int timeout = I2C_MAX_TIMEOUT;
 
 	while (timeout--) {
-		if (readb(&i2c_regs->i2sr) & I2SR_IIF) {
-			writeb(0, &i2c_regs->i2sr);
+		if (readb(&i2c_regs->i2sr) & I2SR_IIF)
 			return 0;
-		}
 
 		udelay(1);
 	}

drivers/i2c/mxc_i2c.c static int tx_byte(struct mxc_i2c_regs *i2c_regs, u8 · line 219

Quoted 7 lines
drivers/i2c/mxc_i2c.c +1 -0
@@ -215,6 +213,7 @@ static int tx_byte(struct mxc_i2c_regs *i2c_regs, u8
 byte) {
 	unsigned ret;
 
+	writeb(0, &i2c_regs->i2sr);
 	writeb(byte, &i2c_regs->i2dr);
 	ret = i2c_imx_trx_complete();
 	if (ret < 0)

If you could try this on mx51 and mx31, I'd be glad. I recall having some issues there.

If it works, add

Best regards,
Marek Vasut

author
#9 lore raw

PATCH 4/24 mxc_i2c: clear i2sr before waiting for bit

Quoted 40 lines · Marek Vasut

Dear Troy Kisky,

Quoted 28 lines

Let's clear the sr register before waiting for bit to be set, instead of clearing it after hardware sets it. No real operational difference here, but allows combining of i2c_imx_trx_complete and i2c_imx_bus_busy in later patches.

---
  drivers/i2c/mxc_i2c.c |    5 ++---
  1 file changed, 2 insertions(+), 3 deletions(-)
drivers/i2c/mxc_i2c.c +1 -3
index 0fd508a..bae9335 100644
@@ -200,10 +200,8 @@ int i2c_imx_trx_complete(void)
  	int timeout = I2C_MAX_TIMEOUT;
 
  	while (timeout--) {
-		if (readb(&i2c_regs->i2sr) & I2SR_IIF) {
-			writeb(0, &i2c_regs->i2sr);
+		if (readb(&i2c_regs->i2sr) & I2SR_IIF)
  			return 0;
-		}
 
  		udelay(1);
  	}

drivers/i2c/mxc_i2c.c static int tx_byte(struct mxc_i2c_regs *i2c_regs, u8 · line 219

Quoted 7 lines
drivers/i2c/mxc_i2c.c +1 -0
@@ -215,6 +213,7 @@ static int tx_byte(struct mxc_i2c_regs *i2c_regs, u8
 byte) {
  	unsigned ret;
 
+	writeb(0, &i2c_regs->i2sr);
  	writeb(byte, &i2c_regs->i2dr);
  	ret = i2c_imx_trx_complete();
  	if (ret < 0)

If you could try this on mx51 and mx31, I'd be glad. I recall having some issues there.

Quoted 6 lines

If it works, add

Best regards,
Marek Vasut

I can't try on a mx31, but will try on a mx51.

Thanks
Troy

author
#10 lore raw

PATCH 5/24 mxc_i2c: create i2c_init_transfer

In reply to Troy Kisky

Initial code of i2c_read and i2c_write
is identical, move to subroutine.

---
 drivers/i2c/mxc_i2c.c |   44 ++++++++++++++++++--------------------------
 1 file changed, 18 insertions(+), 26 deletions(-)
drivers/i2c/mxc_i2c.c +18 -26
index bae9335..626960d 100644
@@ -275,19 +275,29 @@ void i2c_imx_stop(void)
 }
 
 /*
- * Write register address
+ * Send start signal, chip address and
+ * write register address
  */
-int i2c_imx_set_reg_addr(uint addr, int alen)
+static int i2c_init_transfer(struct mxc_i2c_regs *i2c_regs,
+		uchar chip, uint addr, int alen)
 {
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
-	int ret = 0;
+	int ret = i2c_imx_start();
+	if (ret)
+		goto exit;
+
+	/* write slave address */
+	ret = tx_byte(i2c_regs, chip << 1);
+	if (ret < 0)
+		goto exit;
 
 	while (alen--) {
 		ret =  tx_byte(i2c_regs, (addr >> (alen * 8)) & 0xff);
 		if (ret < 0)
-			break;
+			goto exit;
 	}
-
+	return 0;
+exit:
+	i2c_imx_stop();
 	return ret;
 }
 
@@ -322,16 +332,7 @@ int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
 	unsigned int temp;
 	int i;
 
-	ret = i2c_imx_start();
-	if (ret)
-		return ret;
-
-	/* write slave address */
-	ret = tx_byte(i2c_regs, chip << 1);
-	if (ret)
-		return ret;
-
-	ret = i2c_imx_set_reg_addr(addr, alen);
+	ret = i2c_init_transfer(i2c_regs, chip, addr, alen);
 	if (ret)
 		return ret;
 
@@ -389,16 +390,7 @@ int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len)
 	int ret;
 	int i;
 
-	ret = i2c_imx_start();
-	if (ret)
-		return ret;
-
-	/* write slave address */
-	ret = tx_byte(i2c_regs, chip << 1);
-	if (ret)
-		return ret;
-
-	ret = i2c_imx_set_reg_addr(addr, alen);
+	ret = i2c_init_transfer(i2c_regs, chip, addr, alen);
 	if (ret)
 		return ret;
 
Signature · 1 line
1.7.9.5
#11 lore raw

PATCH 5/24 mxc_i2c: create i2c_init_transfer

Dear Troy Kisky,

Quoted 4 lines

Initial code of i2c_read and i2c_write
is identical, move to subroutine.

Quoted 69 lines
---
 drivers/i2c/mxc_i2c.c |   44 ++++++++++++++++++--------------------------
 1 file changed, 18 insertions(+), 26 deletions(-)
drivers/i2c/mxc_i2c.c +17 -19
index bae9335..626960d 100644
@@ -275,19 +275,29 @@ void i2c_imx_stop(void)
 }
 
 /*
- * Write register address
+ * Send start signal, chip address and
+ * write register address
  */
-int i2c_imx_set_reg_addr(uint addr, int alen)
+static int i2c_init_transfer(struct mxc_i2c_regs *i2c_regs,
+		uchar chip, uint addr, int alen)
 {
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
-	int ret = 0;
+	int ret = i2c_imx_start();
+	if (ret)
+		goto exit;
+
+	/* write slave address */
+	ret = tx_byte(i2c_regs, chip << 1);
+	if (ret < 0)
+		goto exit;
 
 	while (alen--) {
 		ret =  tx_byte(i2c_regs, (addr >> (alen * 8)) & 0xff);
 		if (ret < 0)
-			break;
+			goto exit;
 	}
-
+	return 0;
+exit:
+	i2c_imx_stop();
 	return ret;
 }
 
@@ -322,16 +332,7 @@ int i2c_read(uchar chip, uint addr, int alen, uchar
 *buf, int len) unsigned int temp;
 	int i;
 
-	ret = i2c_imx_start();
-	if (ret)
-		return ret;
-
-	/* write slave address */
-	ret = tx_byte(i2c_regs, chip << 1);
-	if (ret)
-		return ret;
-
-	ret = i2c_imx_set_reg_addr(addr, alen);
+	ret = i2c_init_transfer(i2c_regs, chip, addr, alen);
 	if (ret)
 		return ret;
 
@@ -389,16 +390,7 @@ int i2c_write(uchar chip, uint addr, int alen, uchar
 *buf, int len) int ret;
 	int i;
 
-	ret = i2c_imx_start();
-	if (ret)
-		return ret;

drivers/i2c/mxc_i2c.c int i2c_write(uchar chip, uint addr, int alen, uchar · line 395

Quoted 10 lines
drivers/i2c/mxc_i2c.c +1 -7
-
-	/* write slave address */
-	ret = tx_byte(i2c_regs, chip << 1);
-	if (ret)
-		return ret;
-
-	ret = i2c_imx_set_reg_addr(addr, alen);
+	ret = i2c_init_transfer(i2c_regs, chip, addr, alen);
 	if (ret)
 		return ret;

Best regards,
Marek Vasut

#12 lore raw

PATCH 5/24 mxc_i2c: create i2c_init_transfer

In reply to Troy Kisky
Quoted 5 lines · Troy Kisky

Initial code of i2c_read and i2c_write
is identical, move to subroutine.

---

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
author
#13 lore raw

PATCH 6/24 mxc_i2c.c: code i2c_probe as a 0 length i2c_write

In reply to Troy Kisky
---
 drivers/i2c/mxc_i2c.c |   29 ++++++++---------------------
 1 file changed, 8 insertions(+), 21 deletions(-)
drivers/i2c/mxc_i2c.c +8 -21
index 626960d..4f12b9e 100644
@@ -302,27 +302,6 @@ exit:
 }
 
 /*
- * Try if a chip add given address responds (probe the chip)
- */
-int i2c_probe(uchar chip)
-{
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
-	int ret;
-
-	ret = i2c_imx_start();
-	if (ret)
-		return ret;
-
-	ret = tx_byte(i2c_regs, chip << 1);
-	if (ret)
-		return ret;
-
-	i2c_imx_stop();
-
-	return ret;
-}
-
-/*
  * Read data from I2C device
  */
 int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
@@ -404,3 +383,11 @@ int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len)
 
 	return ret;
 }
+
+/*
+ * Try if a chip add given address responds (probe the chip)
+ */
+int i2c_probe(uchar chip)
+{
+	return i2c_write(chip, 0, 0, NULL, 0);
+}
Signature · 1 line
1.7.9.5
#14 lore raw

PATCH 6/24 mxc_i2c.c: code i2c_probe as a 0 length i2c_write

Dear Troy Kisky,

Quoted 1 line
Quoted 39 lines
---
 drivers/i2c/mxc_i2c.c |   29 ++++++++---------------------
 1 file changed, 8 insertions(+), 21 deletions(-)
drivers/i2c/mxc_i2c.c +0 -21
index 626960d..4f12b9e 100644
@@ -302,27 +302,6 @@ exit:
 }
 
 /*
- * Try if a chip add given address responds (probe the chip)
- */
-int i2c_probe(uchar chip)
-{
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
-	int ret;
-
-	ret = i2c_imx_start();
-	if (ret)
-		return ret;
-
-	ret = tx_byte(i2c_regs, chip << 1);
-	if (ret)
-		return ret;
-
-	i2c_imx_stop();
-
-	return ret;
-}
-
-/*
  * Read data from I2C device
  */
 int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
@@ -404,3 +383,11 @@ int i2c_write(uchar chip, uint addr, int alen, uchar
 *buf, int len)
 

drivers/i2c/mxc_i2c.c int i2c_write(uchar chip, uint addr, int alen, uchar · line 393

Quoted 10 lines
drivers/i2c/mxc_i2c.c +8 -0
 	return ret;
 }
+
+/*
+ * Try if a chip add given address responds (probe the chip)
+ */
+int i2c_probe(uchar chip)
+{
+	return i2c_write(chip, 0, 0, NULL, 0);
+}

Best regards,
Marek Vasut

#15 lore raw

PATCH 6/24 mxc_i2c.c: code i2c_probe as a 0 length i2c_write

In reply to Troy Kisky
Quoted 40 lines · Troy Kisky
---
 drivers/i2c/mxc_i2c.c |   29 ++++++++---------------------
 1 file changed, 8 insertions(+), 21 deletions(-)
drivers/i2c/mxc_i2c.c +0 -21
index 626960d..4f12b9e 100644
@@ -302,27 +302,6 @@ exit:
 }
 
 /*
- * Try if a chip add given address responds (probe the chip)
- */
-int i2c_probe(uchar chip)
-{
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
-	int ret;
-
-	ret = i2c_imx_start();
-	if (ret)
-		return ret;
-
-	ret = tx_byte(i2c_regs, chip << 1);
-	if (ret)
-		return ret;
-
-	i2c_imx_stop();
-
-	return ret;
-}
-
-/*
  * Read data from I2C device
  */
 int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
@@ -404,3 +383,11 @@ int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len)
 

drivers/i2c/mxc_i2c.c int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len) · line 393

Quoted 10 lines
drivers/i2c/mxc_i2c.c +8 -0
 	return ret;
 }
+
+/*
+ * Try if a chip add given address responds (probe the chip)
+ */
+int i2c_probe(uchar chip)
+{
+	return i2c_write(chip, 0, 0, NULL, 0);
+}

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
author
#16 lore raw

PATCH 7/24 mxc_i2c: combine i2c_imx_bus_busy and i2c_imx_trx_complete into wait_for_sr_state

In reply to Troy Kisky

Not using udelay gives a more accurate timeout. The current implementation of udelay in imx-common does not seem to wait at all for a udelay(1).

---
 drivers/i2c/mxc_i2c.c |   71 ++++++++++++++++---------------------------------
 1 file changed, 23 insertions(+), 48 deletions(-)
drivers/i2c/mxc_i2c.c +23 -48
index 4f12b9e..7b1b75c 100644
@@ -63,8 +63,6 @@ struct mxc_i2c_regs {
 #error "define CONFIG_SYS_I2C_BASE to use the mxc_i2c driver"
 #endif
 
-#define I2C_MAX_TIMEOUT		10000
-
 static u16 i2c_clk_div[50][2] = {
 	{ 22,	0x20 }, { 24,	0x21 }, { 26,	0x22 }, { 28,	0x23 },
 	{ 30,	0x00 }, { 32,	0x24 }, { 36,	0x25 }, { 40,	0x26 },
@@ -164,48 +162,25 @@ unsigned int i2c_get_bus_speed(void)
 	return mxc_get_clock(MXC_IPG_PERCLK) / i2c_clk_div[clk_div][0];
 }
 
-/*
- * Wait for bus to be busy (or free if for_busy = 0)
- *
- * for_busy = 1: Wait for IBB to be asserted
- * for_busy = 0: Wait for IBB to be de-asserted
- */
-int i2c_imx_bus_busy(int for_busy)
-{
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
-	unsigned int temp;
-
-	int timeout = I2C_MAX_TIMEOUT;
-
-	while (timeout--) {
-		temp = readb(&i2c_regs->i2sr);
+#define ST_BUS_IDLE (0 | (I2SR_IBB << 8))
+#define ST_BUS_BUSY (I2SR_IBB | (I2SR_IBB << 8))
+#define ST_IIF (I2SR_IIF | (I2SR_IIF << 8))
 
-		if (for_busy && (temp & I2SR_IBB))
-			return 0;
-		if (!for_busy && !(temp & I2SR_IBB))
-			return 0;
-
-		udelay(1);
-	}
-
-	return 1;
-}
-
-/*
- * Wait for transaction to complete
- */
-int i2c_imx_trx_complete(void)
+static unsigned wait_for_sr_state(struct mxc_i2c_regs *i2c_regs, unsigned state)
 {
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
-	int timeout = I2C_MAX_TIMEOUT;
-
-	while (timeout--) {
-		if (readb(&i2c_regs->i2sr) & I2SR_IIF)
-			return 0;
-
-		udelay(1);
+	unsigned sr;
+	ulong elapsed;
+	ulong start_time = get_timer(0);
+	for (;;) {
+		sr = readb(&i2c_regs->i2sr);
+		if ((sr & (state >> 8)) == (unsigned char)state)
+			return sr;
+		elapsed = get_timer(start_time);
+		if (elapsed > (CONFIG_SYS_HZ / 10))	/* .1 seconds */
+			break;
 	}
-
+	printf("%s: failed sr=%x cr=%x state=%x\n", __func__,
+			sr, readb(&i2c_regs->i2cr), state);
 	return -ETIMEDOUT;
 }
 
@@ -215,7 +190,7 @@ static int tx_byte(struct mxc_i2c_regs *i2c_regs, u8 byte)
 
 	writeb(0, &i2c_regs->i2sr);
 	writeb(byte, &i2c_regs->i2dr);
-	ret = i2c_imx_trx_complete();
+	ret = wait_for_sr_state(i2c_regs, ST_IIF);
 	if (ret < 0)
 		return ret;
 	ret = readb(&i2c_regs->i2sr);
@@ -245,8 +220,8 @@ int i2c_imx_start(void)
 	temp |= I2CR_MSTA;
 	writeb(temp, &i2c_regs->i2cr);
 
-	result = i2c_imx_bus_busy(1);
-	if (result)
+	result = wait_for_sr_state(i2c_regs, ST_BUS_BUSY);
+	if (result < 0)
 		return result;
 
 	temp |= I2CR_MTX | I2CR_TX_NO_AK;
@@ -268,7 +243,7 @@ void i2c_imx_stop(void)
 	temp &= ~(I2CR_MSTA | I2CR_MTX);
 	writeb(temp, &i2c_regs->i2cr);
 
-	i2c_imx_bus_busy(0);
+	wait_for_sr_state(i2c_regs, ST_BUS_IDLE);
 
 	/* Disable I2C controller */
 	writeb(0, &i2c_regs->i2cr);
@@ -333,8 +308,8 @@ int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
 
 	/* read data */
 	for (i = 0; i < len; i++) {
-		ret = i2c_imx_trx_complete();
-		if (ret)
+		ret = wait_for_sr_state(i2c_regs, ST_IIF);
+		if (ret < 0)
 			return ret;
 
 		/*
@@ -345,7 +320,7 @@ int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
 			temp = readb(&i2c_regs->i2cr);
 			temp &= ~(I2CR_MSTA | I2CR_MTX);
 			writeb(temp, &i2c_regs->i2cr);
-			i2c_imx_bus_busy(0);
+			wait_for_sr_state(i2c_regs, ST_BUS_IDLE);
 		} else if (i == (len - 2)) {
 			temp = readb(&i2c_regs->i2cr);
 			temp |= I2CR_TX_NO_AK;
Signature · 1 line
1.7.9.5
#17 lore raw

PATCH 7/24 mxc_i2c: combine i2c_imx_bus_busy and i2c_imx_trx_complete into wait_for_sr_state

Dear Troy Kisky,

Quoted 3 lines

Not using udelay gives a more accurate timeout. The current implementation of udelay in imx-common does not seem to wait at all for a udelay(1).

Add WATCHDOG_RESET() please.

Quoted 2 lines

---

Best regards,
Marek Vasut

author
#18 lore raw

PATCH 7/24 mxc_i2c: combine i2c_imx_bus_busy and i2c_imx_trx_complete into wait_for_sr_state

Quoted 6 lines · Marek Vasut

Dear Troy Kisky,

Quoted 3 lines

Not using udelay gives a more accurate timeout. The current implementation of udelay in imx-common does not seem to wait at all for a udelay(1).

Add WATCHDOG_RESET() please.

Are you sure it is needed for a 0.1 second max delay?

Quoted 5 lines
Quoted 2 lines

---

Best regards,
Marek Vasut

author
#19 lore raw

PATCH 7/24 mxc_i2c: combine i2c_imx_bus_busy and i2c_imx_trx_complete into wait_for_sr_state

Quoted 10 lines · Troy Kisky
Quoted 7 lines · Marek Vasut

Dear Troy Kisky,

Quoted 4 lines

Not using udelay gives a more accurate timeout. The current implementation
of udelay in imx-common does not seem to wait at all for a udelay(1).

Add WATCHDOG_RESET() please.

Are you sure it is needed for a 0.1 second max delay?

Hmm, I agree. Retries could increase the delay.

Quoted 8 lines
Quoted 6 lines
Quoted 3 lines

---

Best regards,
Marek Vasut

Thanks for the reviews Marek.

Troy

author
#20 lore raw

PATCH 8/24 mxc_i2c: remove redundant read

In reply to Troy Kisky

wait_for_sr_state returns i2sr on success
so no need to read again.

---
 drivers/i2c/mxc_i2c.c |    1 -
 1 file changed, 1 deletion(-)
drivers/i2c/mxc_i2c.c +0 -1
index 7b1b75c..9063d1e 100644
@@ -193,7 +193,6 @@ static int tx_byte(struct mxc_i2c_regs *i2c_regs, u8 byte)
 	ret = wait_for_sr_state(i2c_regs, ST_IIF);
 	if (ret < 0)
 		return ret;
-	ret = readb(&i2c_regs->i2sr);
 	if (ret & I2SR_RX_NO_AK)
 		return -ENODEV;
 	return 0;
Signature · 1 line
1.7.9.5
#21 lore raw

PATCH 8/24 mxc_i2c: remove redundant read

Dear Troy Kisky,

Quoted 4 lines

wait_for_sr_state returns i2sr on success
so no need to read again.

Quoted 9 lines
---
 drivers/i2c/mxc_i2c.c |    1 -
 1 file changed, 1 deletion(-)
drivers/i2c/mxc_i2c.c +0 -0
index 7b1b75c..9063d1e 100644

drivers/i2c/mxc_i2c.c static int tx_byte(struct mxc_i2c_regs *i2c_regs, u8 · line 198

Quoted 7 lines
drivers/i2c/mxc_i2c.c +0 -1
@@ -193,7 +193,6 @@ static int tx_byte(struct mxc_i2c_regs *i2c_regs, u8
 byte) ret = wait_for_sr_state(i2c_regs, ST_IIF);
 	if (ret < 0)
 		return ret;
-	ret = readb(&i2c_regs->i2sr);
 	if (ret & I2SR_RX_NO_AK)
 		return -ENODEV;
 	return 0;

Best regards,
Marek Vasut

author
#22 lore raw

PATCH 9/24 mxc_i2c: place imx_start code inline

In reply to Troy Kisky

imx_start is only referenced once so
move to that location.

---
 drivers/i2c/mxc_i2c.c |   53 +++++++++++++++++++------------------------------
 1 file changed, 20 insertions(+), 33 deletions(-)
drivers/i2c/mxc_i2c.c +20 -33
index 9063d1e..ac91872 100644
@@ -199,37 +199,6 @@ static int tx_byte(struct mxc_i2c_regs *i2c_regs, u8 byte)
 }
 
 /*
- * Start the controller
- */
-int i2c_imx_start(void)
-{
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
-	unsigned int temp = 0;
-	int result;
-
-	/* Enable I2C controller */
-	writeb(0, &i2c_regs->i2sr);
-	writeb(I2CR_IEN, &i2c_regs->i2cr);
-
-	/* Wait controller to be stable */
-	udelay(50);
-
-	/* Start I2C transaction */
-	temp = readb(&i2c_regs->i2cr);
-	temp |= I2CR_MSTA;
-	writeb(temp, &i2c_regs->i2cr);
-
-	result = wait_for_sr_state(i2c_regs, ST_BUS_BUSY);
-	if (result < 0)
-		return result;
-
-	temp |= I2CR_MTX | I2CR_TX_NO_AK;
-	writeb(temp, &i2c_regs->i2cr);
-
-	return 0;
-}
-
-/*
  * Stop the controller
  */
 void i2c_imx_stop(void)
@@ -255,10 +224,28 @@ void i2c_imx_stop(void)
 static int i2c_init_transfer(struct mxc_i2c_regs *i2c_regs,
 		uchar chip, uint addr, int alen)
 {
-	int ret = i2c_imx_start();
-	if (ret)
+	unsigned int temp;
+	int ret;
+
+	/* Enable I2C controller */
+	writeb(0, &i2c_regs->i2sr);
+	writeb(I2CR_IEN, &i2c_regs->i2cr);
+
+	/* Wait for controller to be stable */
+	udelay(50);
+
+	/* Start I2C transaction */
+	temp = readb(&i2c_regs->i2cr);
+	temp |= I2CR_MSTA;
+	writeb(temp, &i2c_regs->i2cr);
+
+	ret = wait_for_sr_state(i2c_regs, ST_BUS_BUSY);
+	if (ret < 0)
 		goto exit;
 
+	temp |= I2CR_MTX | I2CR_TX_NO_AK;
+	writeb(temp, &i2c_regs->i2cr);
+
 	/* write slave address */
 	ret = tx_byte(i2c_regs, chip << 1);
 	if (ret < 0)
Signature · 1 line
1.7.9.5
#23 lore raw

PATCH 9/24 mxc_i2c: place imx_start code inline

Dear Troy Kisky,

Quoted 4 lines

imx_start is only referenced once so
move to that location.

Quoted 68 lines
---
 drivers/i2c/mxc_i2c.c |   53

+++++++++++++++++++------------------------------ 1 file changed, 20 insertions(+), 33 deletions(-)

drivers/i2c/mxc_i2c.c +15 -33
index 9063d1e..ac91872 100644
@@ -199,37 +199,6 @@ static int tx_byte(struct mxc_i2c_regs *i2c_regs, u8
 byte) }
 
 /*
- * Start the controller
- */
-int i2c_imx_start(void)
-{
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
-	unsigned int temp = 0;
-	int result;
-
-	/* Enable I2C controller */
-	writeb(0, &i2c_regs->i2sr);
-	writeb(I2CR_IEN, &i2c_regs->i2cr);
-
-	/* Wait controller to be stable */
-	udelay(50);
-
-	/* Start I2C transaction */
-	temp = readb(&i2c_regs->i2cr);
-	temp |= I2CR_MSTA;
-	writeb(temp, &i2c_regs->i2cr);
-
-	result = wait_for_sr_state(i2c_regs, ST_BUS_BUSY);
-	if (result < 0)
-		return result;
-
-	temp |= I2CR_MTX | I2CR_TX_NO_AK;
-	writeb(temp, &i2c_regs->i2cr);
-
-	return 0;
-}
-
-/*
  * Stop the controller
  */
 void i2c_imx_stop(void)
@@ -255,10 +224,28 @@ void i2c_imx_stop(void)
 static int i2c_init_transfer(struct mxc_i2c_regs *i2c_regs,
 		uchar chip, uint addr, int alen)
 {
-	int ret = i2c_imx_start();
-	if (ret)
+	unsigned int temp;
+	int ret;
+
+	/* Enable I2C controller */
+	writeb(0, &i2c_regs->i2sr);
+	writeb(I2CR_IEN, &i2c_regs->i2cr);
+
+	/* Wait for controller to be stable */
+	udelay(50);
+
+	/* Start I2C transaction */
+	temp = readb(&i2c_regs->i2cr);
+	temp |= I2CR_MSTA;
+	writeb(temp, &i2c_regs->i2cr);
+

drivers/i2c/mxc_i2c.c void i2c_imx_stop(void) · line 251

Quoted 10 lines
drivers/i2c/mxc_i2c.c +5 -0
+	ret = wait_for_sr_state(i2c_regs, ST_BUS_BUSY);
+	if (ret < 0)
 		goto exit;
 
+	temp |= I2CR_MTX | I2CR_TX_NO_AK;
+	writeb(temp, &i2c_regs->i2cr);
+
 	/* write slave address */
 	ret = tx_byte(i2c_regs, chip << 1);
 	if (ret < 0)

Best regards,
Marek Vasut

author
#24 lore raw

PATCH 10/24 mxc_i2c: place i2c_reset code inline

In reply to Troy Kisky

imx_reset is only referenced once so
move to that location.

---
 drivers/i2c/mxc_i2c.c |   15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)
drivers/i2c/mxc_i2c.c +3 -12
index ac91872..2ef7b92 100644
@@ -113,17 +113,6 @@ static uint8_t i2c_imx_get_clk(unsigned int rate)
 }
 
 /*
- * Reset I2C Controller
- */
-void i2c_reset(void)
-{
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
-
-	writeb(0, &i2c_regs->i2cr);	/* Reset module */
-	writeb(0, &i2c_regs->i2sr);
-}
-
-/*
  * Init I2C Bus
  */
 void i2c_init(int speed, int unused)
@@ -135,7 +124,9 @@ void i2c_init(int speed, int unused)
 	/* Store divider value */
 	writeb(idx, &i2c_regs->ifdr);
 
-	i2c_reset();
+	/* Reset module */
+	writeb(0, &i2c_regs->i2cr);
+	writeb(0, &i2c_regs->i2sr);
 }
 
 /*
Signature · 1 line
1.7.9.5
#25 lore raw

PATCH 10/24 mxc_i2c: place i2c_reset code inline

Dear Troy Kisky,

Quoted 4 lines

imx_reset is only referenced once so
move to that location.

Quoted 27 lines
---
 drivers/i2c/mxc_i2c.c |   15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)
drivers/i2c/mxc_i2c.c +0 -11
index ac91872..2ef7b92 100644
@@ -113,17 +113,6 @@ static uint8_t i2c_imx_get_clk(unsigned int rate)
 }
 
 /*
- * Reset I2C Controller
- */
-void i2c_reset(void)
-{
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
-
-	writeb(0, &i2c_regs->i2cr);	/* Reset module */
-	writeb(0, &i2c_regs->i2sr);
-}
-
-/*
  * Init I2C Bus
  */
 void i2c_init(int speed, int unused)

drivers/i2c/mxc_i2c.c void i2c_init(int speed, int unused) · line 132

Quoted 10 lines
drivers/i2c/mxc_i2c.c +3 -1
@@ -135,7 +124,9 @@ void i2c_init(int speed, int unused)
 	/* Store divider value */
 	writeb(idx, &i2c_regs->ifdr);
 
-	i2c_reset();
+	/* Reset module */
+	writeb(0, &i2c_regs->i2cr);
+	writeb(0, &i2c_regs->i2sr);
 }
 
 /*

Best regards,
Marek Vasut

author
#26 lore raw

PATCH 11/24 mxc_i2c: don't disable controller after every transaction

In reply to Troy Kisky

This helps in a multiple bus master environment which is why I also added a wait for bus idle.

---
 drivers/i2c/mxc_i2c.c |   29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)
drivers/i2c/mxc_i2c.c +13 -16
index 2ef7b92..e433312 100644
@@ -190,22 +190,16 @@ static int tx_byte(struct mxc_i2c_regs *i2c_regs, u8 byte)
 }
 
 /*
- * Stop the controller
+ * Stop I2C transaction
  */
 void i2c_imx_stop(void)
 {
 	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
-	unsigned int temp = 0;
+	unsigned int temp = readb(&i2c_regs->i2cr);
 
-	/* Stop I2C transaction */
-	temp = readb(&i2c_regs->i2cr);
 	temp &= ~(I2CR_MSTA | I2CR_MTX);
 	writeb(temp, &i2c_regs->i2cr);
-
 	wait_for_sr_state(i2c_regs, ST_BUS_IDLE);
-
-	/* Disable I2C controller */
-	writeb(0, &i2c_regs->i2cr);
 }
 
 /*
@@ -219,11 +213,15 @@ static int i2c_init_transfer(struct mxc_i2c_regs *i2c_regs,
 	int ret;
 
 	/* Enable I2C controller */
+	if (!(readb(&i2c_regs->i2cr) & I2CR_IEN)) {
+		writeb(I2CR_IEN, &i2c_regs->i2cr);
+		/* Wait for controller to be stable */
+		udelay(50);
+	}
 	writeb(0, &i2c_regs->i2sr);
-	writeb(I2CR_IEN, &i2c_regs->i2cr);
-
-	/* Wait for controller to be stable */
-	udelay(50);
+	ret = wait_for_sr_state(i2c_regs, ST_BUS_IDLE);
+	if (ret < 0)
+		goto exit;
 
 	/* Start I2C transaction */
 	temp = readb(&i2c_regs->i2cr);
@@ -250,6 +248,8 @@ static int i2c_init_transfer(struct mxc_i2c_regs *i2c_regs,
 	return 0;
 exit:
 	i2c_imx_stop();
+	/* Disable I2C controller */
+	writeb(0, &i2c_regs->i2cr);
 	return ret;
 }
 
@@ -294,10 +294,7 @@ int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
 		 * controller from generating another clock cycle
 		 */
 		if (i == (len - 1)) {
-			temp = readb(&i2c_regs->i2cr);
-			temp &= ~(I2CR_MSTA | I2CR_MTX);
-			writeb(temp, &i2c_regs->i2cr);
-			wait_for_sr_state(i2c_regs, ST_BUS_IDLE);
+			i2c_imx_stop(i2c_regs);
 		} else if (i == (len - 2)) {
 			temp = readb(&i2c_regs->i2cr);
 			temp |= I2CR_TX_NO_AK;
Signature · 1 line
1.7.9.5
author
#27 lore raw

PATCH 12/24 mxc_i2c: change slave addr if conflicts with destination.

In reply to Troy Kisky

The i2c controller cannot be both master and slave in the same transaction.

---
 drivers/i2c/mxc_i2c.c |    2 ++
 1 file changed, 2 insertions(+)
drivers/i2c/mxc_i2c.c +2 -0
index e433312..2bff2b8 100644
@@ -218,6 +218,8 @@ static int i2c_init_transfer(struct mxc_i2c_regs *i2c_regs,
 		/* Wait for controller to be stable */
 		udelay(50);
 	}
+	if (readb(&i2c_regs->iadr) == (chip << 1))
+		writeb((chip << 1) ^ 2, &i2c_regs->iadr);
 	writeb(0, &i2c_regs->i2sr);
 	ret = wait_for_sr_state(i2c_regs, ST_BUS_IDLE);
 	if (ret < 0)
Signature · 1 line
1.7.9.5
author
#28 lore raw

PATCH 13/24 mxc_i2c: check for arbitration lost

In reply to Troy Kisky

No need to continue waiting if arbitration lost.

---
 drivers/i2c/mxc_i2c.c |    7 +++++++
 1 file changed, 7 insertions(+)
drivers/i2c/mxc_i2c.c +7 -0
index 2bff2b8..df033ea 100644
@@ -54,6 +54,7 @@ struct mxc_i2c_regs {
 
 #define I2SR_ICF	(1 << 7)
 #define I2SR_IBB	(1 << 5)
+#define I2SR_IAL	(1 << 4)
 #define I2SR_IIF	(1 << 1)
 #define I2SR_RX_NO_AK	(1 << 0)
 
@@ -164,6 +165,12 @@ static unsigned wait_for_sr_state(struct mxc_i2c_regs *i2c_regs, unsigned state)
 	ulong start_time = get_timer(0);
 	for (;;) {
 		sr = readb(&i2c_regs->i2sr);
+		if (sr & I2SR_IAL) {
+			writeb(sr & ~I2SR_IAL, &i2c_regs->i2sr);
+			printf("%s: Arbitration lost sr=%x cr=%x state=%x\n",
+				__func__, sr, readb(&i2c_regs->i2cr), state);
+			return -ERESTART;
+		}
 		if ((sr & (state >> 8)) == (unsigned char)state)
 			return sr;
 		elapsed = get_timer(start_time);
Signature · 1 line
1.7.9.5
#29 lore raw

PATCH 13/24 mxc_i2c: check for arbitration lost

Dear Troy Kisky,

Quoted 3 lines

No need to continue waiting if arbitration lost.

Quoted 19 lines
---
 drivers/i2c/mxc_i2c.c |    7 +++++++
 1 file changed, 7 insertions(+)
drivers/i2c/mxc_i2c.c +1 -0
index 2bff2b8..df033ea 100644
@@ -54,6 +54,7 @@ struct mxc_i2c_regs {
 
 #define I2SR_ICF	(1 << 7)
 #define I2SR_IBB	(1 << 5)
+#define I2SR_IAL	(1 << 4)
 #define I2SR_IIF	(1 << 1)
 #define I2SR_RX_NO_AK	(1 << 0)
 
@@ -164,6 +165,12 @@ static unsigned wait_for_sr_state(struct mxc_i2c_regs
 *i2c_regs, unsigned state) ulong start_time = get_timer(0);
 	for (;;) {

drivers/i2c/mxc_i2c.c static unsigned wait_for_sr_state(struct mxc_i2c_regs · line 176

Quoted 10 lines
drivers/i2c/mxc_i2c.c +6 -0
 		sr = readb(&i2c_regs->i2sr);
+		if (sr & I2SR_IAL) {
+			writeb(sr & ~I2SR_IAL, &i2c_regs->i2sr);
+			printf("%s: Arbitration lost sr=%x cr=%x state=%x\n",
+				__func__, sr, readb(&i2c_regs->i2cr), state);
+			return -ERESTART;
+		}
 		if ((sr & (state >> 8)) == (unsigned char)state)
 			return sr;
 		elapsed = get_timer(start_time);

Best regards,
Marek Vasut

author
#30 lore raw

PATCH 14/24 mxc_i2c: add retries

In reply to Troy Kisky

Retry unexpected hardware errors. This
will not retry a received NAK.

---
 drivers/i2c/mxc_i2c.c |   36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)
drivers/i2c/mxc_i2c.c +27 -9
index df033ea..802f70f 100644
@@ -213,7 +213,7 @@ void i2c_imx_stop(void)
  * Send start signal, chip address and
  * write register address
  */
-static int i2c_init_transfer(struct mxc_i2c_regs *i2c_regs,
+static int i2c_init_transfer_(struct mxc_i2c_regs *i2c_regs,
 		uchar chip, uint addr, int alen)
 {
 	unsigned int temp;
@@ -230,7 +230,7 @@ static int i2c_init_transfer(struct mxc_i2c_regs *i2c_regs,
 	writeb(0, &i2c_regs->i2sr);
 	ret = wait_for_sr_state(i2c_regs, ST_BUS_IDLE);
 	if (ret < 0)
-		goto exit;
+		return ret;
 
 	/* Start I2C transaction */
 	temp = readb(&i2c_regs->i2cr);
@@ -239,7 +239,7 @@ static int i2c_init_transfer(struct mxc_i2c_regs *i2c_regs,
 
 	ret = wait_for_sr_state(i2c_regs, ST_BUS_BUSY);
 	if (ret < 0)
-		goto exit;
+		return ret;
 
 	temp |= I2CR_MTX | I2CR_TX_NO_AK;
 	writeb(temp, &i2c_regs->i2cr);
@@ -247,18 +247,36 @@ static int i2c_init_transfer(struct mxc_i2c_regs *i2c_regs,
 	/* write slave address */
 	ret = tx_byte(i2c_regs, chip << 1);
 	if (ret < 0)
-		goto exit;
+		return ret;
 
 	while (alen--) {
 		ret =  tx_byte(i2c_regs, (addr >> (alen * 8)) & 0xff);
 		if (ret < 0)
-			goto exit;
+			return ret;
 	}
 	return 0;
-exit:
-	i2c_imx_stop();
-	/* Disable I2C controller */
-	writeb(0, &i2c_regs->i2cr);
+}
+
+static int i2c_init_transfer(struct mxc_i2c_regs *i2c_regs,
+		uchar chip, uint addr, int alen)
+{
+	int retry;
+	int ret;
+	for (retry = 0; retry < 3; retry++) {
+		ret = i2c_init_transfer_(i2c_regs, chip, addr, alen);
+		if (ret >= 0)
+			return ret;
+		i2c_imx_stop(i2c_regs);
+		if (ret == -ENODEV)
+			return ret;
+
+		printf("%s: failed for chip 0x%x retry=%d\n", __func__, chip,
+				retry);
+		if (ret != -ERESTART)
+			writeb(0, &i2c_regs->i2cr);	/* Disable controller */
+		udelay(100);
+	}
+	printf("%s: give up i2c_regs=%p\n", __func__, i2c_regs);
 	return ret;
 }
 
Signature · 1 line
1.7.9.5
#31 lore raw

PATCH 14/24 mxc_i2c: add retries

Dear Troy Kisky,

Quoted 4 lines

Retry unexpected hardware errors. This
will not retry a received NAK.

Why do you need this? Are you getting transmission errors? I think it's because you broke something in the driver, I was getting errors, but managed to hack it together in such a manner, that they didn't happen anymore.

Quoted 68 lines
---
 drivers/i2c/mxc_i2c.c |   36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)
drivers/i2c/mxc_i2c.c +3 -3
index df033ea..802f70f 100644
@@ -213,7 +213,7 @@ void i2c_imx_stop(void)
  * Send start signal, chip address and
  * write register address
  */
-static int i2c_init_transfer(struct mxc_i2c_regs *i2c_regs,
+static int i2c_init_transfer_(struct mxc_i2c_regs *i2c_regs,
 		uchar chip, uint addr, int alen)
 {
 	unsigned int temp;
@@ -230,7 +230,7 @@ static int i2c_init_transfer(struct mxc_i2c_regs
 *i2c_regs, writeb(0, &i2c_regs->i2sr);
 	ret = wait_for_sr_state(i2c_regs, ST_BUS_IDLE);
 	if (ret < 0)
-		goto exit;
+		return ret;
 
 	/* Start I2C transaction */
 	temp = readb(&i2c_regs->i2cr);
@@ -239,7 +239,7 @@ static int i2c_init_transfer(struct mxc_i2c_regs
 *i2c_regs,
 
 	ret = wait_for_sr_state(i2c_regs, ST_BUS_BUSY);
 	if (ret < 0)
-		goto exit;
+		return ret;
 
 	temp |= I2CR_MTX | I2CR_TX_NO_AK;
drivers/i2c/mxc_i2c.c +16 -6
 	writeb(temp, &i2c_regs->i2cr);
@@ -247,18 +247,36 @@ static int i2c_init_transfer(struct mxc_i2c_regs
 *i2c_regs, /* write slave address */
 	ret = tx_byte(i2c_regs, chip << 1);
 	if (ret < 0)
-		goto exit;
+		return ret;
 
 	while (alen--) {
 		ret =  tx_byte(i2c_regs, (addr >> (alen * 8)) & 0xff);
 		if (ret < 0)
-			goto exit;
+			return ret;
 	}
 	return 0;
-exit:
-	i2c_imx_stop();
-	/* Disable I2C controller */
-	writeb(0, &i2c_regs->i2cr);
+}
+
+static int i2c_init_transfer(struct mxc_i2c_regs *i2c_regs,
+		uchar chip, uint addr, int alen)
+{
+	int retry;
+	int ret;
+	for (retry = 0; retry < 3; retry++) {
+		ret = i2c_init_transfer_(i2c_regs, chip, addr, alen);
+		if (ret >= 0)
+			return ret;
+		i2c_imx_stop(i2c_regs);
+		if (ret == -ENODEV)
+			return ret;

drivers/i2c/mxc_i2c.c static int i2c_init_transfer(struct mxc_i2c_regs · line 281

Quoted 10 lines
drivers/i2c/mxc_i2c.c +8 -0
+
+		printf("%s: failed for chip 0x%x retry=%d\n", __func__, chip,
+				retry);
+		if (ret != -ERESTART)
+			writeb(0, &i2c_regs->i2cr);	/* Disable controller */
+		udelay(100);
+	}
+	printf("%s: give up i2c_regs=%p\n", __func__, i2c_regs);
 	return ret;
 }

Best regards,
Marek Vasut

author
#32 lore raw

PATCH 14/24 mxc_i2c: add retries

Quoted 9 lines · Marek Vasut

Dear Troy Kisky,

Quoted 4 lines

Retry unexpected hardware errors. This
will not retry a received NAK.

Why do you need this? Are you getting transmission errors? I think it's because you broke something in the driver, I was getting errors, but managed to hack it together in such a manner, that they didn't happen anymore.

I have an hdmi display that holds the SCL line low for ~3 seconds every ~9 seconds.
I added this code while trying to debug that. Granted it is the display's fault, but bus
recovery is important.

Quoted 81 lines
Quoted 68 lines
---
  drivers/i2c/mxc_i2c.c |   36 +++++++++++++++++++++++++++---------
  1 file changed, 27 insertions(+), 9 deletions(-)
drivers/i2c/mxc_i2c.c +3 -3
index df033ea..802f70f 100644
@@ -213,7 +213,7 @@ void i2c_imx_stop(void)
   * Send start signal, chip address and
   * write register address
   */
-static int i2c_init_transfer(struct mxc_i2c_regs *i2c_regs,
+static int i2c_init_transfer_(struct mxc_i2c_regs *i2c_regs,
  		uchar chip, uint addr, int alen)
  {
  	unsigned int temp;
@@ -230,7 +230,7 @@ static int i2c_init_transfer(struct mxc_i2c_regs
 *i2c_regs, writeb(0, &i2c_regs->i2sr);
  	ret = wait_for_sr_state(i2c_regs, ST_BUS_IDLE);
  	if (ret < 0)
-		goto exit;
+		return ret;
 
  	/* Start I2C transaction */
  	temp = readb(&i2c_regs->i2cr);
@@ -239,7 +239,7 @@ static int i2c_init_transfer(struct mxc_i2c_regs
 *i2c_regs,
 
  	ret = wait_for_sr_state(i2c_regs, ST_BUS_BUSY);
  	if (ret < 0)
-		goto exit;
+		return ret;
 
  	temp |= I2CR_MTX | I2CR_TX_NO_AK;
  	writeb(temp, &i2c_regs->i2cr);
drivers/i2c/mxc_i2c.c +16 -6
@@ -247,18 +247,36 @@ static int i2c_init_transfer(struct mxc_i2c_regs
 *i2c_regs, /* write slave address */
  	ret = tx_byte(i2c_regs, chip << 1);
  	if (ret < 0)
-		goto exit;
+		return ret;
 
  	while (alen--) {
  		ret =  tx_byte(i2c_regs, (addr >> (alen * 8)) & 0xff);
  		if (ret < 0)
-			goto exit;
+			return ret;
  	}
  	return 0;
-exit:
-	i2c_imx_stop();
-	/* Disable I2C controller */
-	writeb(0, &i2c_regs->i2cr);
+}
+
+static int i2c_init_transfer(struct mxc_i2c_regs *i2c_regs,
+		uchar chip, uint addr, int alen)
+{
+	int retry;
+	int ret;
+	for (retry = 0; retry < 3; retry++) {
+		ret = i2c_init_transfer_(i2c_regs, chip, addr, alen);
+		if (ret >= 0)
+			return ret;
+		i2c_imx_stop(i2c_regs);
+		if (ret == -ENODEV)
+			return ret;

drivers/i2c/mxc_i2c.c static int i2c_init_transfer(struct mxc_i2c_regs · line 281

Quoted 10 lines
drivers/i2c/mxc_i2c.c +8 -0
+
+		printf("%s: failed for chip 0x%x retry=%d\n", __func__, chip,
+				retry);
+		if (ret != -ERESTART)
+			writeb(0, &i2c_regs->i2cr);	/* Disable controller */
+		udelay(100);
+	}
+	printf("%s: give up i2c_regs=%p\n", __func__, i2c_regs);
  	return ret;
  }

Best regards,
Marek Vasut

author
#33 lore raw

PATCH 15/24 mxc_i2c: add i2c_regs argument to i2c_imx_stop

In reply to Troy Kisky

This is prep work for CONFIG_I2C_MULTI_BUS.

---
 drivers/i2c/mxc_i2c.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/i2c/mxc_i2c.c +3 -4
index 802f70f..cb061f7 100644
@@ -199,9 +199,8 @@ static int tx_byte(struct mxc_i2c_regs *i2c_regs, u8 byte)
 /*
  * Stop I2C transaction
  */
-void i2c_imx_stop(void)
+static void i2c_imx_stop(struct mxc_i2c_regs *i2c_regs)
 {
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
 	unsigned int temp = readb(&i2c_regs->i2cr);
 
 	temp &= ~(I2CR_MSTA | I2CR_MTX);
@@ -331,7 +330,7 @@ int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
 		buf[i] = readb(&i2c_regs->i2dr);
 	}
 
-	i2c_imx_stop();
+	i2c_imx_stop(i2c_regs);
 
 	return ret;
 }
@@ -355,7 +354,7 @@ int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len)
 			return ret;
 	}
 
-	i2c_imx_stop();
+	i2c_imx_stop(i2c_regs);
 
 	return ret;
 }
Signature · 1 line
1.7.9.5
#34 lore raw

PATCH 15/24 mxc_i2c: add i2c_regs argument to i2c_imx_stop

Dear Troy Kisky,

Quoted 3 lines

This is prep work for CONFIG_I2C_MULTI_BUS.

Quoted 29 lines
---
 drivers/i2c/mxc_i2c.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/i2c/mxc_i2c.c +0 -0
index 802f70f..cb061f7 100644
@@ -199,9 +199,8 @@ static int tx_byte(struct mxc_i2c_regs *i2c_regs, u8

byte) /*
* Stop I2C transaction
*/

drivers/i2c/mxc_i2c.c +2 -3
-void i2c_imx_stop(void)
+static void i2c_imx_stop(struct mxc_i2c_regs *i2c_regs)
 {
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
 	unsigned int temp = readb(&i2c_regs->i2cr);
 
 	temp &= ~(I2CR_MSTA | I2CR_MTX);
@@ -331,7 +330,7 @@ int i2c_read(uchar chip, uint addr, int alen, uchar
 *buf, int len) buf[i] = readb(&i2c_regs->i2dr);
 	}
 
-	i2c_imx_stop();
+	i2c_imx_stop(i2c_regs);
 
 	return ret;
 }

drivers/i2c/mxc_i2c.c int i2c_write(uchar chip, uint addr, int alen, uchar · line 360

Quoted 8 lines
drivers/i2c/mxc_i2c.c +1 -1
@@ -355,7 +354,7 @@ int i2c_write(uchar chip, uint addr, int alen, uchar
 *buf, int len) return ret;
 	}
 
-	i2c_imx_stop();
+	i2c_imx_stop(i2c_regs);
 
 	return ret;
 }

Best regards,
Marek Vasut

author
#35 lore raw

PATCH 16/24 mxc_i2c: prep work for multiple busses support

In reply to Troy Kisky
---
 drivers/i2c/mxc_i2c.c |  121 ++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 100 insertions(+), 21 deletions(-)
drivers/i2c/mxc_i2c.c +100 -21
index cb061f7..ec05798 100644
@@ -58,9 +58,7 @@ struct mxc_i2c_regs {
 #define I2SR_IIF	(1 << 1)
 #define I2SR_RX_NO_AK	(1 << 0)
 
-#ifdef CONFIG_SYS_I2C_BASE
-#define I2C_BASE	CONFIG_SYS_I2C_BASE
-#else
+#if defined(CONFIG_HARD_I2C) && !defined(CONFIG_SYS_I2C_BASE)
 #error "define CONFIG_SYS_I2C_BASE to use the mxc_i2c driver"
 #endif
 
@@ -114,11 +112,11 @@ static uint8_t i2c_imx_get_clk(unsigned int rate)
 }
 
 /*
- * Init I2C Bus
+ * Set I2C Bus speed
  */
-void i2c_init(int speed, int unused)
+int bus_i2c_set_bus_speed(void *base, int speed)
 {
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
+	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)base;
 	u8 clk_idx = i2c_imx_get_clk(speed);
 	u8 idx = i2c_clk_div[clk_idx][1];
 
@@ -128,23 +126,15 @@ void i2c_init(int speed, int unused)
 	/* Reset module */
 	writeb(0, &i2c_regs->i2cr);
 	writeb(0, &i2c_regs->i2sr);
-}
-
-/*
- * Set I2C Speed
- */
-int i2c_set_bus_speed(unsigned int speed)
-{
-	i2c_init(speed, 0);
 	return 0;
 }
 
 /*
  * Get I2C Speed
  */
-unsigned int i2c_get_bus_speed(void)
+unsigned int bus_i2c_get_bus_speed(void *base)
 {
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
+	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)base;
 	u8 clk_idx = readb(&i2c_regs->ifdr);
 	u8 clk_div;
 
@@ -282,12 +272,13 @@ static int i2c_init_transfer(struct mxc_i2c_regs *i2c_regs,
 /*
  * Read data from I2C device
  */
-int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
+int bus_i2c_read(void *base, uchar chip, uint addr, int alen, uchar *buf,
+		int len)
 {
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
 	int ret;
 	unsigned int temp;
 	int i;
+	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)base;
 
 	ret = i2c_init_transfer(i2c_regs, chip, addr, alen);
 	if (ret)
@@ -338,11 +329,12 @@ int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
 /*
  * Write data to I2C device
  */
-int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len)
+int bus_i2c_write(void *base, uchar chip, uint addr, int alen, uchar *buf,
+		int len)
 {
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
 	int ret;
 	int i;
+	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)base;
 
 	ret = i2c_init_transfer(i2c_regs, chip, addr, alen);
 	if (ret)
@@ -359,10 +351,97 @@ int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len)
 	return ret;
 }
 
+typedef void (*toggle_i2c_fn)(void *p);
+
+#ifdef CONFIG_I2C_MULTI_BUS
+static unsigned g_bus;
+#else
+#define g_bus 0
+#endif
+
+struct i2c_parms {
+	void *base;
+	void *toggle_data;
+	toggle_i2c_fn toggle_fn;
+};
+
+struct i2c_parms g_parms[3];
+
+void *get_base(void)
+{
+#ifndef CONFIG_SYS_I2C_BASE
+	return g_parms[g_bus].base;
+#elif defined(CONFIG_I2C_MULTI_BUS)
+	void *ret = g_parms[g_bus].base;
+	if (!ret)
+		ret = (void *)CONFIG_SYS_I2C_BASE;
+	return ret;
+#else
+	return (void *)CONFIG_SYS_I2C_BASE;
+#endif
+}
+
+int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
+{
+	return bus_i2c_read(get_base(), chip, addr, alen, buf, len);
+}
+
+int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len)
+{
+	return bus_i2c_write(get_base(), chip, addr, alen, buf, len);
+}
 /*
  * Try if a chip add given address responds (probe the chip)
  */
 int i2c_probe(uchar chip)
 {
-	return i2c_write(chip, 0, 0, NULL, 0);
+	return bus_i2c_write(get_base(), chip, 0, 0, NULL, 0);
+}
+
+void bus_i2c_init(void *base, int speed, int unused,
+		toggle_i2c_fn toggle_fn, void *toggle_data)
+{
+	int i = 0;
+	struct i2c_parms *p = g_parms;
+	if (!base)
+		return;
+	for (;;) {
+		if (!p->base || (p->base == base)) {
+			p->base = base;
+			if (toggle_data) {
+				p->toggle_data = toggle_data;
+				p->toggle_fn = toggle_fn;
+			}
+			break;
+		}
+		p++;
+		i++;
+		if (i >= ARRAY_SIZE(g_parms))
+			return;
+	}
+	bus_i2c_set_bus_speed(base, speed);
+}
+
+/*
+ * Init I2C Bus
+ */
+void i2c_init(int speed, int unused)
+{
+	bus_i2c_init(get_base(), speed, unused, NULL, NULL);
+}
+
+/*
+ * Set I2C Speed
+ */
+int i2c_set_bus_speed(unsigned int speed)
+{
+	return bus_i2c_set_bus_speed(get_base(), speed);
+}
+
+/*
+ * Get I2C Speed
+ */
+unsigned int i2c_get_bus_speed(void)
+{
+	return bus_i2c_get_bus_speed(get_base());
 }
Signature · 1 line
1.7.9.5
#36 lore raw

PATCH 16/24 mxc_i2c: prep work for multiple busses support

Dear Troy Kisky,

Quoted 94 lines
---
 drivers/i2c/mxc_i2c.c |  121

++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 100 insertions(+), 21 deletions(-)

drivers/i2c/mxc_i2c.c +12 -20
index cb061f7..ec05798 100644
@@ -58,9 +58,7 @@ struct mxc_i2c_regs {
 #define I2SR_IIF	(1 << 1)
 #define I2SR_RX_NO_AK	(1 << 0)
 
-#ifdef CONFIG_SYS_I2C_BASE
-#define I2C_BASE	CONFIG_SYS_I2C_BASE
-#else
+#if defined(CONFIG_HARD_I2C) && !defined(CONFIG_SYS_I2C_BASE)
 #error "define CONFIG_SYS_I2C_BASE to use the mxc_i2c driver"
 #endif
 
@@ -114,11 +112,11 @@ static uint8_t i2c_imx_get_clk(unsigned int rate)
 }
 
 /*
- * Init I2C Bus
+ * Set I2C Bus speed
  */
-void i2c_init(int speed, int unused)
+int bus_i2c_set_bus_speed(void *base, int speed)
 {
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
+	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)base;
 	u8 clk_idx = i2c_imx_get_clk(speed);
 	u8 idx = i2c_clk_div[clk_idx][1];
 
@@ -128,23 +126,15 @@ void i2c_init(int speed, int unused)
 	/* Reset module */
 	writeb(0, &i2c_regs->i2cr);
 	writeb(0, &i2c_regs->i2sr);
-}
-
-/*
- * Set I2C Speed
- */
-int i2c_set_bus_speed(unsigned int speed)
-{
-	i2c_init(speed, 0);
 	return 0;
 }
 
 /*
  * Get I2C Speed
  */
-unsigned int i2c_get_bus_speed(void)
+unsigned int bus_i2c_get_bus_speed(void *base)
 {
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
+	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)base;
 	u8 clk_idx = readb(&i2c_regs->ifdr);
 	u8 clk_div;
 
@@ -282,12 +272,13 @@ static int i2c_init_transfer(struct mxc_i2c_regs
 *i2c_regs, /*
  * Read data from I2C device
  */
-int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
+int bus_i2c_read(void *base, uchar chip, uint addr, int alen, uchar *buf,
+		int len)
 {
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
 	int ret;
 	unsigned int temp;
 	int i;
+	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)base;
 
 	ret = i2c_init_transfer(i2c_regs, chip, addr, alen);
 	if (ret)
@@ -338,11 +329,12 @@ int i2c_read(uchar chip, uint addr, int alen, uchar
 *buf, int len) /*
  * Write data to I2C device
  */
-int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len)
+int bus_i2c_write(void *base, uchar chip, uint addr, int alen, uchar *buf,
+		int len)
 {
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
 	int ret;
 	int i;
+	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)base;
 
 	ret = i2c_init_transfer(i2c_regs, chip, addr, alen);
 	if (ret)

drivers/i2c/mxc_i2c.c int i2c_write(uchar chip, uint addr, int alen, uchar · line 354

Quoted 4 lines
drivers/i2c/mxc_i2c.c +1 -0
@@ -359,10 +351,97 @@ int i2c_write(uchar chip, uint addr, int alen, uchar
 *buf, int len) return ret;
 }
 
+typedef void (*toggle_i2c_fn)(void *p);

I really don't like this kind of stuff ... who's supposed to look for this typedefs. It makes the code not obviously clear.

Quoted 5 lines
drivers/i2c/mxc_i2c.c +5 -0
+#ifdef CONFIG_I2C_MULTI_BUS
+static unsigned g_bus;
+#else
+#define g_bus 0
+#endif

drivers/i2c/mxc_i2c.c int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len) · line 370

Quoted 10 lines
drivers/i2c/mxc_i2c.c +10 -0
+
+struct i2c_parms {
+	void *base;
+	void *toggle_data;
+	toggle_i2c_fn toggle_fn;
+};
+
+struct i2c_parms g_parms[3];
+
+void *get_base(void)

static? Or this is part of i2c api?

Quoted 78 lines
drivers/i2c/mxc_i2c.c +71 -1
+{
+#ifndef CONFIG_SYS_I2C_BASE
+	return g_parms[g_bus].base;
+#elif defined(CONFIG_I2C_MULTI_BUS)
+	void *ret = g_parms[g_bus].base;
+	if (!ret)
+		ret = (void *)CONFIG_SYS_I2C_BASE;
+	return ret;
+#else
+	return (void *)CONFIG_SYS_I2C_BASE;
+#endif
+}
+
+int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
+{
+	return bus_i2c_read(get_base(), chip, addr, alen, buf, len);
+}
+
+int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len)
+{
+	return bus_i2c_write(get_base(), chip, addr, alen, buf, len);
+}
 /*
  * Try if a chip add given address responds (probe the chip)
  */
 int i2c_probe(uchar chip)
 {
-	return i2c_write(chip, 0, 0, NULL, 0);
+	return bus_i2c_write(get_base(), chip, 0, 0, NULL, 0);
+}
+
+void bus_i2c_init(void *base, int speed, int unused,
+		toggle_i2c_fn toggle_fn, void *toggle_data)
+{
+	int i = 0;
+	struct i2c_parms *p = g_parms;
+	if (!base)
+		return;
+	for (;;) {
+		if (!p->base || (p->base == base)) {
+			p->base = base;
+			if (toggle_data) {
+				p->toggle_data = toggle_data;
+				p->toggle_fn = toggle_fn;
+			}
+			break;
+		}
+		p++;
+		i++;
+		if (i >= ARRAY_SIZE(g_parms))
+			return;
+	}
+	bus_i2c_set_bus_speed(base, speed);
+}
+
+/*
+ * Init I2C Bus
+ */
+void i2c_init(int speed, int unused)
+{
+	bus_i2c_init(get_base(), speed, unused, NULL, NULL);
+}
+
+/*
+ * Set I2C Speed
+ */
+int i2c_set_bus_speed(unsigned int speed)
+{
+	return bus_i2c_set_bus_speed(get_base(), speed);
+}
+
+/*
+ * Get I2C Speed
+ */
+unsigned int i2c_get_bus_speed(void)
+{
+	return bus_i2c_get_bus_speed(get_base());
 }
#37 lore raw

PATCH 16/24 mxc_i2c: prep work for multiple busses support

In reply to Troy Kisky

Hello Troy,

Quoted 9 lines · Troy Kisky
---
  drivers/i2c/mxc_i2c.c |  121 ++++++++++++++++++++++++++++++++++++++++---------
  1 file changed, 100 insertions(+), 21 deletions(-)
drivers/i2c/mxc_i2c.c +0 -0
index cb061f7..ec05798 100644

[...]

drivers/i2c/mxc_i2c.c int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len) · line 357

Quoted 8 lines
drivers/i2c/mxc_i2c.c +4 -0
@@ -359,10 +351,97 @@ int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len)
  	return ret;
  }
 
+typedef void (*toggle_i2c_fn)(void *p);
+
+#ifdef CONFIG_I2C_MULTI_BUS
+static unsigned g_bus;

This is only valid after relocation ... and i2c is maybe used before relocation.

Try something (not tested) like that:

static unsigned int __attribute__((section (".data"))) g_bus = 0;

drivers/i2c/mxc_i2c.c int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len) · line 365

Quoted 8 lines
drivers/i2c/mxc_i2c.c +8 -0
+#else
+#define g_bus 0
+#endif
+
+struct i2c_parms {
+	void *base;
+	void *toggle_data;
+	toggle_i2c_fn toggle_fn;

For what is this toggle_* needed?

bye,
Heiko

Signature · 2 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
#38 lore raw

PATCH 16/24 mxc_i2c: prep work for multiple busses support

Dear Heiko Schocher,

Quoted 32 lines

Hello Troy,

Quoted 11 lines · Troy Kisky

---

  drivers/i2c/mxc_i2c.c |  121

++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 100 insertions(+), 21 deletions(-)

drivers/i2c/mxc_i2c.c +0 -0
index cb061f7..ec05798 100644

[...]

drivers/i2c/mxc_i2c.c int i2c_write(uchar chip, uint addr, int alen, · line 360

Quoted 11 lines
drivers/i2c/mxc_i2c.c +4 -0
@@ -359,10 +351,97 @@ int i2c_write(uchar chip, uint addr, int alen,
 uchar *buf, int len)
 
  	return ret;
 
  }
 
+typedef void (*toggle_i2c_fn)(void *p);
+
+#ifdef CONFIG_I2C_MULTI_BUS
+static unsigned g_bus;

This is only valid after relocation ... and i2c is maybe used before relocation.

Quoted 3 lines

Try something (not tested) like that:

static unsigned int __attribute__((section (".data"))) g_bus = 0;

This is a good knowledge to have, adding it amongst important emails :)

Quoted 12 lines

drivers/i2c/mxc_i2c.c int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len) · line 365

Quoted 9 lines
drivers/i2c/mxc_i2c.c +8 -0
+#else
+#define g_bus 0
+#endif
+
+struct i2c_parms {
+	void *base;
+	void *toggle_data;
+	toggle_i2c_fn toggle_fn;

For what is this toggle_* needed?

Quoted 2 lines

bye,
Heiko

Best regards,
Marek Vasut

#39 lore raw

PATCH 16/24 mxc_i2c: prep work for multiple busses support

Dear Marek Vasut,

Quoted 4 lines · In message <[email protected]> you wrote:
Quoted 2 lines

static unsigned int __attribute__((section (".data"))) g_bus = 0;

This is a good knowledge to have, adding it amongst important emails :)

Please add a note to it that whenever youneed doing this, chances are

Quoted 1 line

95% that you are doing something fundamally wrong.

Best regards,

Wolfgang Denk

Signature · 4 lines
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
"Go to Heaven for the climate, Hell for the company."    - Mark Twain
author
#40 lore raw

PATCH 17/24 mxc_i2c: add bus recovery support

In reply to Troy Kisky

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

---
 drivers/i2c/mxc_i2c.c |   26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)
drivers/i2c/mxc_i2c.c +26 -0
index ec05798..339bb6f 100644
@@ -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);
Signature · 1 line
1.7.9.5
#41 lore raw

PATCH 17/24 mxc_i2c: add bus recovery support

Hello Troy,

Quoted 49 lines · Troy Kisky

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

---
  drivers/i2c/mxc_i2c.c |   26 ++++++++++++++++++++++++++
  1 file changed, 26 insertions(+)
drivers/i2c/mxc_i2c.c +19 -0
index ec05798..339bb6f 100644
@@ -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)
+{

drivers/i2c/mxc_i2c.c void *get_base(void) · line 412

Quoted 10 lines
drivers/i2c/mxc_i2c.c +7 -0
+	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() for unblocking the i2c bus? And where is the function, which really toggles the SCL pin, as you described in the commit message?

bye,
Heiko

Signature · 2 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
author
#42 lore raw

PATCH 17/24 mxc_i2c: add bus recovery support

Quoted 59 lines · Heiko Schocher

Hello Troy,

Quoted 56 lines · Troy Kisky

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

---
  drivers/i2c/mxc_i2c.c |   26 ++++++++++++++++++++++++++
  1 file changed, 26 insertions(+)
drivers/i2c/mxc_i2c.c +2 -0
index ec05798..339bb6f 100644
@@ -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)
  {
drivers/i2c/mxc_i2c.c +1 -0
@@ -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;
drivers/i2c/mxc_i2c.c +21 -0
@@ -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);
Quoted 8 lines

drivers/i2c/mxc_i2c.c void *get_base(void) · line 412

Quoted 5 lines
drivers/i2c/mxc_i2c.c +2 -0
+}
+
  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?

Quoted 2 lines

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

author
#43 lore raw

PATCH 18/24 mxc_i2c.c: finish adding CONFIG_I2C_MULTI_BUS support

In reply to Troy Kisky
---
 drivers/i2c/mxc_i2c.c |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)
drivers/i2c/mxc_i2c.c +17 -0
index 339bb6f..5d18752 100644
@@ -407,6 +407,23 @@ static void toggle_i2c(void *base)
 		p->toggle_fn(p->toggle_data);
 }
 
+#ifdef CONFIG_I2C_MULTI_BUS
+unsigned int i2c_get_bus_num(void)
+{
+	return g_bus;
+}
+
+int i2c_set_bus_num(unsigned bus_idx)
+{
+	if (bus_idx >= ARRAY_SIZE(g_parms))
+		return -1;
+	if (!g_parms[bus_idx].base)
+		return -1;
+	g_bus = bus_idx;
+	return 0;
+}
+#endif
+
 int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
 {
 	return bus_i2c_read(get_base(), chip, addr, alen, buf, len);
Signature · 1 line
1.7.9.5
#44 lore raw

PATCH 18/24 mxc_i2c.c: finish adding CONFIG_I2C_MULTI_BUS support

Dear Troy Kisky,

Quoted 10 lines
---
 drivers/i2c/mxc_i2c.c |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)
drivers/i2c/mxc_i2c.c +0 -0
index 339bb6f..5d18752 100644

drivers/i2c/mxc_i2c.c static void toggle_i2c(void *base) · line 413

Quoted 7 lines
drivers/i2c/mxc_i2c.c +4 -0
@@ -407,6 +407,23 @@ static void toggle_i2c(void *base)
 		p->toggle_fn(p->toggle_data);
 }
 
+#ifdef CONFIG_I2C_MULTI_BUS
+unsigned int i2c_get_bus_num(void)
+{
+	return g_bus;

Is this global variable? If so, it won't work before relocation. And i2c can be enabled before relocation.

Quoted 6 lines
drivers/i2c/mxc_i2c.c +6 -0
+}
+
+int i2c_set_bus_num(unsigned bus_idx)
+{
+	if (bus_idx >= ARRAY_SIZE(g_parms))
+		return -1;

drivers/i2c/mxc_i2c.c static void toggle_i2c(void *base) · line 429

Quoted 10 lines
drivers/i2c/mxc_i2c.c +7 -0
+	if (!g_parms[bus_idx].base)
+		return -1;
+	g_bus = bus_idx;
+	return 0;
+}
+#endif
+
 int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
 {
 	return bus_i2c_read(get_base(), chip, addr, alen, buf, len);

Best regards,
Marek Vasut

author
#45 lore raw

PATCH 18/24 mxc_i2c.c: finish adding CONFIG_I2C_MULTI_BUS support

Quoted 15 lines · Marek Vasut

Dear Troy Kisky,

Quoted 13 lines
---
  drivers/i2c/mxc_i2c.c |   17 +++++++++++++++++
  1 file changed, 17 insertions(+)
drivers/i2c/mxc_i2c.c +0 -0
index 339bb6f..5d18752 100644
@@ -407,6 +407,23 @@ static void toggle_i2c(void *base)
  		p->toggle_fn(p->toggle_data);
  }
 
Quoted 6 lines

drivers/i2c/mxc_i2c.c static void toggle_i2c(void *base) · line 413

Quoted 4 lines
drivers/i2c/mxc_i2c.c +4 -0
+#ifdef CONFIG_I2C_MULTI_BUS
+unsigned int i2c_get_bus_num(void)
+{
+	return g_bus;

Is this global variable? If so, it won't work before relocation. And i2c can be enabled before relocation.

Correct and correct. If you need i2c working before relocation, you cannot enable MULTI_BUS.

Should I put this in struct global_data to remove this restriction?

Quoted 20 lines
Quoted 7 lines
drivers/i2c/mxc_i2c.c +6 -0
+}
+
+int i2c_set_bus_num(unsigned bus_idx)
+{
+	if (bus_idx >= ARRAY_SIZE(g_parms))
+		return -1;

drivers/i2c/mxc_i2c.c static void toggle_i2c(void *base) · line 429

Quoted 10 lines
drivers/i2c/mxc_i2c.c +7 -0
+	if (!g_parms[bus_idx].base)
+		return -1;
+	g_bus = bus_idx;
+	return 0;
+}
+#endif
+
  int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
  {
  	return bus_i2c_read(get_base(), chip, addr, alen, buf, len);

Best regards,
Marek Vasut

#46 lore raw

PATCH 18/24 mxc_i2c.c: finish adding CONFIG_I2C_MULTI_BUS support

Quoted 3 lines · Troy Kisky

Correct and correct. If you need i2c working before relocation, you cannot enable MULTI_BUS.

That sounds like a really strong limitation of MULTI_BUS. i2c is necessary for SPD, which is needed to initialize DDR.

Signature · 2 lines
Timur Tabi
Linux kernel developer at Freescale
author
#47 lore raw

PATCH 19/24 iomux-v3: remove include of mx6x_pins.h

In reply to Troy Kisky

This include is not needed.

---
 arch/arm/cpu/armv7/mx6/iomux-v3.c |    1 -
 1 file changed, 1 deletion(-)
arch/arm/cpu/armv7/mx6/iomux-v3.c +0 -1
index 8785532..a0c4b15 100644
@@ -23,7 +23,6 @@
 #include <common.h>
 #include <asm/io.h>
 #include <asm/arch/imx-regs.h>
-#include <asm/arch/mx6x_pins.h>
 #include <asm/arch/iomux-v3.h>
 
 static void *base = (void *)IOMUXC_BASE_ADDR;
Signature · 1 line
1.7.9.5
author
#48 lore raw

PATCH 20/24 i.mx: iomux-v3.h: move to imx-common include directory

In reply to Troy Kisky
---
 arch/arm/cpu/armv7/mx6/iomux-v3.c                  |    2 +-
 arch/arm/include/asm/arch-mx6/mx6x_pins.h          |    2 +-
 .../asm/{arch-mx6 => imx-common}/iomux-v3.h        |    0
 board/freescale/mx6qarm2/mx6qarm2.c                |    2 +-
 board/freescale/mx6qsabrelite/mx6qsabrelite.c      |    2 +-
 drivers/usb/host/ehci-mx6.c                        |    2 +-
 6 files changed, 5 insertions(+), 5 deletions(-)
 rename arch/arm/include/asm/{arch-mx6 => imx-common}/iomux-v3.h (100%)
arch/arm/cpu/armv7/mx6/iomux-v3.c +1 -1
index a0c4b15..da093fb 100644
@@ -23,7 +23,7 @@
 #include <common.h>
 #include <asm/io.h>
 #include <asm/arch/imx-regs.h>
-#include <asm/arch/iomux-v3.h>
+#include <asm/imx-common/iomux-v3.h>
 
 static void *base = (void *)IOMUXC_BASE_ADDR;
 
arch/arm/include/asm/arch-mx6/mx6x_pins.h +1 -1
index afaa068..c0bb494 100644
@@ -22,7 +22,7 @@
 #ifndef __ASM_ARCH_MX6_MX6X_PINS_H__
 #define __ASM_ARCH_MX6_MX6X_PINS_H__
 
-#include <asm/arch/iomux-v3.h>
+#include <asm/imx-common/iomux-v3.h>
 
 /* Use to set PAD control */
 #define PAD_CTL_HYS		(1 << 16)
arch/arm/include/asm/imx-common/iomux-v3.h renamed from arch/arm/include/asm/arch-mx6/iomux-v3.h +0 -0
similarity index 100%
rename from arch/arm/include/asm/arch-mx6/iomux-v3.h
rename to arch/arm/include/asm/imx-common/iomux-v3.h
board/freescale/mx6qarm2/mx6qarm2.c +1 -1
index 1367b88..340c4c4 100644
@@ -24,9 +24,9 @@
 #include <asm/io.h>
 #include <asm/arch/imx-regs.h>
 #include <asm/arch/mx6x_pins.h>
-#include <asm/arch/iomux-v3.h>
 #include <asm/errno.h>
 #include <asm/gpio.h>
+#include <asm/imx-common/iomux-v3.h>
 #include <mmc.h>
 #include <fsl_esdhc.h>
 #include <miiphy.h>
board/freescale/mx6qsabrelite/mx6qsabrelite.c +1 -1
index 72b4b54..74ce84c 100644
@@ -24,9 +24,9 @@
 #include <asm/io.h>
 #include <asm/arch/imx-regs.h>
 #include <asm/arch/mx6x_pins.h>
-#include <asm/arch/iomux-v3.h>
 #include <asm/errno.h>
 #include <asm/gpio.h>
+#include <asm/imx-common/iomux-v3.h>
 #include <mmc.h>
 #include <fsl_esdhc.h>
 #include <micrel.h>
drivers/usb/host/ehci-mx6.c +1 -1
index 5dec673..f2523f6 100644
@@ -22,7 +22,7 @@
 #include <asm/arch/imx-regs.h>
 #include <asm/arch/clock.h>
 #include <asm/arch/mx6x_pins.h>
-#include <asm/arch/iomux-v3.h>
+#include <asm/imx-common/iomux-v3.h>
 
 #include "ehci.h"
 #include "ehci-core.h"
Signature · 1 line
1.7.9.5
author
#49 lore raw

PATCH 21/24 i.mx: iomux-v3.c: move to imx-common directory

In reply to Troy Kisky
---
 arch/arm/cpu/armv7/imx-common/Makefile            |    2 +-
 arch/arm/cpu/armv7/{mx6 => imx-common}/iomux-v3.c |    0
 arch/arm/cpu/armv7/mx6/Makefile                   |    2 +-
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename arch/arm/cpu/armv7/{mx6 => imx-common}/iomux-v3.c (100%)
arch/arm/cpu/armv7/imx-common/Makefile +1 -1
index e5ff375..53296fa 100644
@@ -27,7 +27,7 @@ include $(TOPDIR)/config.mk
 
 LIB     = $(obj)libimx-common.o
 
-COBJS	= timer.o cpu.o speed.o
+COBJS	= iomux-v3.o timer.o cpu.o speed.o
 
 SRCS	:= $(SOBJS:.o=.S) $(COBJS:.o=.c)
 OBJS	:= $(addprefix $(obj),$(SOBJS) $(COBJS))
arch/arm/cpu/armv7/imx-common/iomux-v3.c renamed from arch/arm/cpu/armv7/mx6/iomux-v3.c +0 -0
similarity index 100%
rename from arch/arm/cpu/armv7/mx6/iomux-v3.c
rename to arch/arm/cpu/armv7/imx-common/iomux-v3.c
arch/arm/cpu/armv7/mx6/Makefile +1 -1
index b0da028..cbce411 100644
@@ -27,7 +27,7 @@ include $(TOPDIR)/config.mk
 
 LIB	= $(obj)lib$(SOC).o
 
-COBJS	= soc.o clock.o iomux-v3.o
+COBJS	= soc.o clock.o
 SOBJS   = lowlevel_init.o
 
 SRCS	:= $(SOBJS:.o=.S) $(COBJS:.o=.c)
Signature · 1 line
1.7.9.5
author
#50 lore raw

PATCH 22/24 i.mx53: add definition for I2C3_BASE_ADDR

In reply to Troy Kisky
---
 arch/arm/include/asm/arch-mx5/imx-regs.h |    1 +
 1 file changed, 1 insertion(+)
arch/arm/include/asm/arch-mx5/imx-regs.h +1 -0
index 4fa6658..caf5d21 100644
@@ -93,6 +93,7 @@
 #define GPIO5_BASE_ADDR         (AIPS1_BASE_ADDR + 0x000DC000)
 #define GPIO6_BASE_ADDR         (AIPS1_BASE_ADDR + 0x000E0000)
 #define GPIO7_BASE_ADDR         (AIPS1_BASE_ADDR + 0x000E4000)
+#define I2C3_BASE_ADDR		(AIPS1_BASE_ADDR + 0x000EC000)
 #endif
 /*
  * AIPS 2
Signature · 1 line
1.7.9.5
author
#51 lore raw

PATCH 23/24 imx-common: add i2c.c for bus recovery support

In reply to Troy Kisky
---
 arch/arm/cpu/armv7/imx-common/Makefile    |    4 +-
 arch/arm/cpu/armv7/imx-common/i2c.c       |   79 +++++++++++++++++++++++++++++
 arch/arm/cpu/armv7/mx5/clock.c            |   19 +++++++
 arch/arm/cpu/armv7/mx6/clock.c            |   19 +++++++
 arch/arm/include/asm/arch-mx5/clock.h     |    1 +
 arch/arm/include/asm/arch-mx6/clock.h     |    1 +
 arch/arm/include/asm/imx-common/mxc_i2c.h |   46 +++++++++++++++++
 7 files changed, 168 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/cpu/armv7/imx-common/i2c.c
 create mode 100644 arch/arm/include/asm/imx-common/mxc_i2c.h
arch/arm/cpu/armv7/imx-common/Makefile +3 -1
index 53296fa..bf36be5 100644
@@ -27,7 +27,9 @@ include $(TOPDIR)/config.mk
 
 LIB     = $(obj)libimx-common.o
 
-COBJS	= iomux-v3.o timer.o cpu.o speed.o
+COBJS-y	= iomux-v3.o timer.o cpu.o speed.o
+COBJS-$(CONFIG_I2C_MXC) += i2c.o
+COBJS	:= $(sort $(COBJS-y))
 
 SRCS	:= $(SOBJS:.o=.S) $(COBJS:.o=.c)
 OBJS	:= $(addprefix $(obj),$(SOBJS) $(COBJS))
arch/arm/cpu/armv7/imx-common/i2c.c new file +79 -0

79 lines not on this page · raw message

arch/arm/cpu/armv7/mx5/clock.c +19 -0
index e92f106..b5d2794 100644
@@ -80,6 +80,25 @@ void enable_usboh3_clk(unsigned char enable)
 	writel(reg, &mxc_ccm->CCGR2);
 }
 
+#ifdef CONFIG_I2C_MXC
+/* i2c_num can be from 0 - 2 */
+int enable_i2c_clock(unsigned char enable, unsigned i2c_num)
+{
+	u32 reg;
+	u32 mask;
+
+	if (i2c_num > 2)
+		return;
+	mask = MXC_CCM_CCGR_CG_MASK << ((i2c_num + 9) << 1);
+	reg = __raw_readl(&mxc_ccm->CCGR1);
+	if (enable)
+		reg |= mask;
+	else
+		reg &= ~mask;
+	__raw_writel(reg, &mxc_ccm->CCGR1);
+}
+#endif
+
 void set_usb_phy1_clk(void)
 {
 	unsigned int reg;
arch/arm/cpu/armv7/mx6/clock.c +19 -0
index ef98563..d31777b 100644
@@ -49,6 +49,25 @@ void enable_usboh3_clk(unsigned char enable)
 
 }
 
+#ifdef CONFIG_I2C_MXC
+/* i2c_num can be from 0 - 2 */
+int enable_i2c_clock(unsigned char enable, unsigned i2c_num)
+{
+	u32 reg;
+	u32 mask;
+
+	if (i2c_num > 2)
+		return;
+	mask = MXC_CCM_CCGR_CG_MASK << ((i2c_num + 3) << 1);
+	reg = __raw_readl(&imx_ccm->CCGR2);
+	if (enable)
+		reg |= mask;
+	else
+		reg &= ~mask;
+	__raw_writel(reg, &imx_ccm->CCGR2);
+}
+#endif
+
 static u32 decode_pll(enum pll_clocks pll, u32 infreq)
 {
 	u32 div;
arch/arm/include/asm/arch-mx5/clock.h +1 -0
index ea972a3..da604bf 100644
@@ -44,5 +44,6 @@ void set_usb_phy2_clk(void);
 void enable_usb_phy2_clk(unsigned char enable);
 void set_usboh3_clk(void);
 void enable_usboh3_clk(unsigned char enable);
+int enable_i2c_clk(unsigned char enable, unsigned i2c_num);
 
 #endif /* __ASM_ARCH_CLOCK_H */
arch/arm/include/asm/arch-mx6/clock.h +1 -0
index 613809b..04daf36 100644
@@ -47,5 +47,6 @@ u32 imx_get_uartclk(void);
 u32 imx_get_fecclk(void);
 unsigned int mxc_get_clock(enum mxc_clock clk);
 void enable_usboh3_clk(unsigned char enable);
+int enable_i2c_clk(unsigned char enable, unsigned i2c_num);
 
 #endif /* __ASM_ARCH_CLOCK_H */
arch/arm/include/asm/imx-common/mxc_i2c.h new file +46 -0

46 lines not on this page · raw message

Signature · 1 line
1.7.9.5
author
#52 lore raw

PATCH 24/24 mx6qsabrelite: add i2c multi-bus support

In reply to Troy Kisky

This includes bus recovery support.

---
 board/freescale/mx6qsabrelite/mx6qsabrelite.c |   50 +++++++++++++++++++++++--
 include/configs/mx6qsabrelite.h               |    6 +--
 2 files changed, 48 insertions(+), 8 deletions(-)
board/freescale/mx6qsabrelite/mx6qsabrelite.c +46 -4
index 74ce84c..29c6630 100644
@@ -27,6 +27,7 @@
 #include <asm/errno.h>
 #include <asm/gpio.h>
 #include <asm/imx-common/iomux-v3.h>
+#include <asm/imx-common/mxc_i2c.h>
 #include <mmc.h>
 #include <fsl_esdhc.h>
 #include <micrel.h>
@@ -72,9 +73,48 @@ iomux_v3_cfg_t uart2_pads[] = {
        MX6Q_PAD_EIM_D27__UART2_RXD | MUX_PAD_CTRL(UART_PAD_CTRL),
 };
 
-iomux_v3_cfg_t i2c3_pads[] = {
-	MX6Q_PAD_GPIO_5__I2C3_SCL | MUX_PAD_CTRL(I2C_PAD_CTRL),
-	MX6Q_PAD_GPIO_16__I2C3_SDA | MUX_PAD_CTRL(I2C_PAD_CTRL),
+#define PC MUX_PAD_CTRL(I2C_PAD_CTRL)
+
+/* I2C1, SGTL5000 */
+struct i2c_pads_info i2c_pad_info0 = {
+	.scl = {
+		.i2c_mode = MX6Q_PAD_EIM_D21__I2C1_SCL | PC,
+		.gpio_mode = MX6Q_PAD_EIM_D21__GPIO_3_21 | PC,
+		.gp = GPIO_NUMBER(3, 21)
+	},
+	.sda = {
+		.i2c_mode = MX6Q_PAD_EIM_D28__I2C1_SDA | PC,
+		.gpio_mode = MX6Q_PAD_EIM_D28__GPIO_3_28 | PC,
+		.gp = GPIO_NUMBER(3, 28)
+	}
+};
+
+/* I2C2 Camera, MIPI */
+struct i2c_pads_info i2c_pad_info1 = {
+	.scl = {
+		.i2c_mode = MX6Q_PAD_KEY_COL3__I2C2_SCL | PC,
+		.gpio_mode = MX6Q_PAD_KEY_COL3__GPIO_4_12 | PC,
+		.gp = GPIO_NUMBER(4, 12)
+	},
+	.sda = {
+		.i2c_mode = MX6Q_PAD_KEY_ROW3__I2C2_SDA | PC,
+		.gpio_mode = MX6Q_PAD_KEY_ROW3__GPIO_4_13 | PC,
+		.gp = GPIO_NUMBER(4, 13)
+	}
+};
+
+/* I2C3, J15 - RGB connector */
+struct i2c_pads_info i2c_pad_info2 = {
+	.scl = {
+		.i2c_mode = MX6Q_PAD_GPIO_5__I2C3_SCL | PC,
+		.gpio_mode = MX6Q_PAD_GPIO_5__GPIO_1_5 | PC,
+		.gp = GPIO_NUMBER(1, 5)
+	},
+	.sda = {
+		.i2c_mode = MX6Q_PAD_GPIO_16__I2C3_SDA | PC,
+		.gpio_mode = MX6Q_PAD_GPIO_16__GPIO_7_11 | PC,
+		.gp = GPIO_NUMBER(7, 11)
+	}
 };
 
 iomux_v3_cfg_t usdhc3_pads[] = {
@@ -292,7 +332,9 @@ int board_init(void)
 #ifdef CONFIG_MXC_SPI
 	setup_spi();
 #endif
-	imx_iomux_v3_setup_multiple_pads(i2c3_pads, ARRAY_SIZE(i2c3_pads));
+	setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f, &i2c_pad_info0);
+	setup_i2c(1, CONFIG_SYS_I2C_SPEED, 0x7f, &i2c_pad_info1);
+	setup_i2c(2, CONFIG_SYS_I2C_SPEED, 0x7f, &i2c_pad_info2);
 
        return 0;
 }
include/configs/mx6qsabrelite.h +2 -4
index df2b735..c3c6be2 100644
@@ -60,11 +60,9 @@
 
 /* I2C Configs */
 #define CONFIG_CMD_I2C
-#define CONFIG_HARD_I2C
+#define CONFIG_I2C_MULTI_BUS
 #define CONFIG_I2C_MXC
-#define CONFIG_SYS_I2C_BASE		I2C3_BASE_ADDR
-#define CONFIG_SYS_I2C_SPEED            100000
-#define CONFIG_SYS_I2C_SLAVE            0xfe
+#define CONFIG_SYS_I2C_SPEED		100000
 
 /* MMC Configs */
 #define CONFIG_FSL_ESDHC
Signature · 1 line
1.7.9.5
#53 lore raw

PATCH 1/24 mxc_i2c: fix i2c_imx_stop

In reply to Troy Kisky

Dear Troy Kisky,

Quoted 5 lines

Instead of clearing 2 bits, all the other
bits were set because '|=' was used instead
of '&='.

Quoted 15 lines
---
 drivers/i2c/mxc_i2c.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Note:

All patches in the series are based on the i2c/master branch even though only 1-18 will be applied there.

drivers/i2c/mxc_i2c.c +0 -0
index fc68062..c0c45fd 100644

drivers/i2c/mxc_i2c.c void i2c_imx_stop(void) · line 270

Quoted 8 lines
drivers/i2c/mxc_i2c.c +1 -1
@@ -264,7 +264,7 @@ void i2c_imx_stop(void)
 
 	/* Stop I2C transaction */
 	temp = readb(&i2c_regs->i2cr);
-	temp |= ~(I2CR_MSTA | I2CR_MTX);
+	temp &= ~(I2CR_MSTA | I2CR_MTX);
 	writeb(temp, &i2c_regs->i2cr);
 
 	i2c_imx_bus_busy(0);

Best regards,
Marek Vasut

#54 lore raw

PATCH 1/24 mxc_i2c: fix i2c_imx_stop

In reply to Troy Kisky
Quoted 21 lines · Troy Kisky

Instead of clearing 2 bits, all the other
bits were set because '|=' was used instead
of '&='.

---
 drivers/i2c/mxc_i2c.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Note:

All patches in the series are based on the i2c/master branch even though only 1-18 will be applied there.

drivers/i2c/mxc_i2c.c +0 -0
index fc68062..c0c45fd 100644

drivers/i2c/mxc_i2c.c void i2c_imx_stop(void) · line 270

Quoted 8 lines
drivers/i2c/mxc_i2c.c +1 -1
@@ -264,7 +264,7 @@ void i2c_imx_stop(void)
 
 	/* Stop I2C transaction */
 	temp = readb(&i2c_regs->i2cr);
-	temp |= ~(I2CR_MSTA | I2CR_MTX);
+	temp &= ~(I2CR_MSTA | I2CR_MTX);
 	writeb(temp, &i2c_regs->i2cr);
 
 	i2c_imx_bus_busy(0);

Good catch !

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================