[U-Boot] [PATCH 01/24] mxc_i2c: fix i2c_imx_stop

[U-Boot] [PATCH 01/24] mxc_i2c: fix i2c_imx_stop

[Troy Kisky]

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

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 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.


diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
index fc68062..c0c45fd 100644
--- a/drivers/i2c/mxc_i2c.c
+++ b/drivers/i2c/mxc_i2c.c
@@ -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);
-- 
1.7.9.5

[U-Boot] [PATCH 02/24] mxc_i2c: remove ifdef of CONFIG_HARD_I2C

[Troy Kisky]

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

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 drivers/i2c/mxc_i2c.c |    6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
index c0c45fd..0b46c9c 100644
--- a/drivers/i2c/mxc_i2c.c
+++ b/drivers/i2c/mxc_i2c.c
@@ -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 */
-- 
1.7.9.5

[U-Boot] [PATCH 02/24] mxc_i2c: remove ifdef of CONFIG_HARD_I2C

[Marek Vasut]

Dear Troy Kisky,

> This is always selected when CONFIG_I2C_MXC is
> selected, so it adds no value.
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

Acked-by: Marek Vasut <marex@denx.de>

> ---
>  drivers/i2c/mxc_i2c.c |    6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
> index c0c45fd..0b46c9c 100644
> --- a/drivers/i2c/mxc_i2c.c
> +++ b/drivers/i2c/mxc_i2c.c
> @@ -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 */

Best regards,
Marek Vasut

[U-Boot] [PATCH 03/24] mxc_i2c: create tx_byte function

[Troy Kisky]

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

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 drivers/i2c/mxc_i2c.c |   72 +++++++++++++++----------------------------------
 1 file changed, 21 insertions(+), 51 deletions(-)

diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
index 0b46c9c..0fd508a 100644
--- a/drivers/i2c/mxc_i2c.c
+++ b/drivers/i2c/mxc_i2c.c
@@ -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;
 	}
 
-- 
1.7.9.5

[U-Boot] [PATCH 03/24] mxc_i2c: create tx_byte function

[Marek Vasut]

Dear Troy Kisky,

> Use tx_byte function instead of having 3 copies
> of the code.
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> ---
>  drivers/i2c/mxc_i2c.c |   72
> +++++++++++++++---------------------------------- 1 file changed, 21
> insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
> index 0b46c9c..0fd508a 100644
> --- a/drivers/i2c/mxc_i2c.c
> +++ b/drivers/i2c/mxc_i2c.c
> @@ -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);

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

>  	if (ret)
>  		return ret;
> 

Otherwise
Acked-by: Marek Vasut <marex@denx.de>

Best regards,
Marek Vasut

[U-Boot] [PATCH 03/24] mxc_i2c: create tx_byte function

[Troy Kisky]

On 6/22/2012 9:58 AM, Marek Vasut wrote:
> Dear Troy Kisky,
>
>> Use tx_byte function instead of having 3 copies
>> of the code.
>>
>> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
>> ---
>>   drivers/i2c/mxc_i2c.c |   72
>> +++++++++++++++---------------------------------- 1 file changed, 21
>> insertions(+), 51 deletions(-)
>>
>> diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
>> index 0b46c9c..0fd508a 100644
>> --- a/drivers/i2c/mxc_i2c.c
>> +++ b/drivers/i2c/mxc_i2c.c
>> @@ -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);
> 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.

>>   	if (ret)
>>   		return ret;
>>
> Otherwise
> Acked-by: Marek Vasut <marex@denx.de>
>
> Best regards,
> Marek Vasut
>

[U-Boot] [PATCH 03/24] mxc_i2c: create tx_byte function

[Marek Vasut]

Dear Troy Kisky,

[...]

> >> @@ -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.

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

> >>   	if (ret)
> >>   	
> >>   		return ret;
> > 
> > Otherwise
> > Acked-by: Marek Vasut <marex@denx.de>
> > 
> > Best regards,
> > Marek Vasut

Best regards,
Marek Vasut

[U-Boot] [PATCH 04/24] mxc_i2c: clear i2sr before waiting for bit

[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.

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 drivers/i2c/mxc_i2c.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
index 0fd508a..bae9335 100644
--- a/drivers/i2c/mxc_i2c.c
+++ b/drivers/i2c/mxc_i2c.c
@@ -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)
-- 
1.7.9.5

[U-Boot] [PATCH 04/24] mxc_i2c: clear i2sr before waiting for bit

[Marek Vasut]

Dear 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.
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> ---
>  drivers/i2c/mxc_i2c.c |    5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
> index 0fd508a..bae9335 100644
> --- a/drivers/i2c/mxc_i2c.c
> +++ b/drivers/i2c/mxc_i2c.c
> @@ -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)

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

If it works, add
Acked-by: Marek Vasut <marex@denx.de>

Best regards,
Marek Vasut

[U-Boot] [PATCH 04/24] mxc_i2c: clear i2sr before waiting for bit

[Troy Kisky]

On 6/22/2012 9:59 AM, Marek Vasut wrote:
> Dear 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.
>>
>> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
>> ---
>>   drivers/i2c/mxc_i2c.c |    5 ++---
>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
>> index 0fd508a..bae9335 100644
>> --- a/drivers/i2c/mxc_i2c.c
>> +++ b/drivers/i2c/mxc_i2c.c
>> @@ -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)
> If you could try this on mx51 and mx31, I'd be glad. I recall having some issues
> there.
>
> If it works, add
> Acked-by: Marek Vasut <marex@denx.de>
>
> Best regards,
> Marek Vasut
>
I can't try on a mx31, but will try on a mx51.

Thanks
Troy

[U-Boot] [PATCH 05/24] mxc_i2c: create i2c_init_transfer

[Troy Kisky]

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

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 drivers/i2c/mxc_i2c.c |   44 ++++++++++++++++++--------------------------
 1 file changed, 18 insertions(+), 26 deletions(-)

diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
index bae9335..626960d 100644
--- a/drivers/i2c/mxc_i2c.c
+++ b/drivers/i2c/mxc_i2c.c
@@ -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;
 
-- 
1.7.9.5

[U-Boot] [PATCH 05/24] mxc_i2c: create i2c_init_transfer

[Marek Vasut]

Dear Troy Kisky,

> Initial code of i2c_read and i2c_write
> is identical, move to subroutine.
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

Acked-by: Marek Vasut <marex@denx.de>

> ---
>  drivers/i2c/mxc_i2c.c |   44 ++++++++++++++++++--------------------------
>  1 file changed, 18 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
> index bae9335..626960d 100644
> --- a/drivers/i2c/mxc_i2c.c
> +++ b/drivers/i2c/mxc_i2c.c
> @@ -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;

Best regards,
Marek Vasut

[U-Boot] [PATCH 05/24] mxc_i2c: create i2c_init_transfer

[Stefano Babic]

On 22/06/2012 06:12, Troy Kisky wrote:
> Initial code of i2c_read and i2c_write
> is identical, move to subroutine.
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> ---

Acked-by: Stefano Babic <sbabic@denx.de>

Best regards,
Stefano Babic



-- 
=====================================================================
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
=====================================================================

[U-Boot] [PATCH 06/24] mxc_i2c.c: code i2c_probe as a 0 length i2c_write

[Troy Kisky]

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 drivers/i2c/mxc_i2c.c |   29 ++++++++---------------------
 1 file changed, 8 insertions(+), 21 deletions(-)

diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
index 626960d..4f12b9e 100644
--- a/drivers/i2c/mxc_i2c.c
+++ b/drivers/i2c/mxc_i2c.c
@@ -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);
+}
-- 
1.7.9.5

[U-Boot] [PATCH 06/24] mxc_i2c.c: code i2c_probe as a 0 length i2c_write

[Marek Vasut]

Dear Troy Kisky,

> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

Acked-by: Marek Vasut <marex@denx.de>

> ---
>  drivers/i2c/mxc_i2c.c |   29 ++++++++---------------------
>  1 file changed, 8 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
> index 626960d..4f12b9e 100644
> --- a/drivers/i2c/mxc_i2c.c
> +++ b/drivers/i2c/mxc_i2c.c
> @@ -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);
> +}

Best regards,
Marek Vasut

[U-Boot] [PATCH 06/24] mxc_i2c.c: code i2c_probe as a 0 length i2c_write

[Stefano Babic]

On 22/06/2012 06:12, Troy Kisky wrote:
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> ---
>  drivers/i2c/mxc_i2c.c |   29 ++++++++---------------------
>  1 file changed, 8 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
> index 626960d..4f12b9e 100644
> --- a/drivers/i2c/mxc_i2c.c
> +++ b/drivers/i2c/mxc_i2c.c
> @@ -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);
> +}
> 

Acked-by: Stefano Babic <sbabic@denx.de>

Best regards,
Stefano Babic



-- 
=====================================================================
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
=====================================================================

[U-Boot] [PATCH 07/24] mxc_i2c: combine i2c_imx_bus_busy and i2c_imx_trx_complete into wait_for_sr_state

[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).

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 drivers/i2c/mxc_i2c.c |   71 ++++++++++++++++---------------------------------
 1 file changed, 23 insertions(+), 48 deletions(-)

diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
index 4f12b9e..7b1b75c 100644
--- a/drivers/i2c/mxc_i2c.c
+++ b/drivers/i2c/mxc_i2c.c
@@ -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;
-- 
1.7.9.5

[U-Boot] [PATCH 07/24] mxc_i2c: combine i2c_imx_bus_busy and i2c_imx_trx_complete into wait_for_sr_state

[Marek Vasut]

Dear 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).
> 

Add WATCHDOG_RESET() please.

> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> ---

Best regards,
Marek Vasut

[U-Boot] [PATCH 07/24] mxc_i2c: combine i2c_imx_bus_busy and i2c_imx_trx_complete into wait_for_sr_state

[Troy Kisky]

On 6/22/2012 10:01 AM, Marek Vasut wrote:
> Dear 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).
>>
> Add WATCHDOG_RESET() please.

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

>> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
>> ---
> Best regards,
> Marek Vasut
>

[U-Boot] [PATCH 07/24] mxc_i2c: combine i2c_imx_bus_busy and i2c_imx_trx_complete into wait_for_sr_state

[Troy Kisky]

On 6/22/2012 10:34 AM, Troy Kisky wrote:
> On 6/22/2012 10:01 AM, Marek Vasut wrote:
>> Dear 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).
>>>
>> 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.


>
>>> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
>>> ---
>> Best regards,
>> Marek Vasut
>>
>
>

Thanks for the reviews Marek.

Troy

[U-Boot] [PATCH 08/24] mxc_i2c: remove redundant read

[Troy Kisky]

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

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 drivers/i2c/mxc_i2c.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
index 7b1b75c..9063d1e 100644
--- a/drivers/i2c/mxc_i2c.c
+++ b/drivers/i2c/mxc_i2c.c
@@ -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;
-- 
1.7.9.5

[U-Boot] [PATCH 08/24] mxc_i2c: remove redundant read

[Marek Vasut]

Dear Troy Kisky,

> wait_for_sr_state returns i2sr on success
> so no need to read again.
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

Acked-by: Marek Vasut <marex@denx.de>

> ---
>  drivers/i2c/mxc_i2c.c |    1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
> index 7b1b75c..9063d1e 100644
> --- a/drivers/i2c/mxc_i2c.c
> +++ b/drivers/i2c/mxc_i2c.c
> @@ -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

[U-Boot] [PATCH 09/24] mxc_i2c: place imx_start code inline

[Troy Kisky]

imx_start is only referenced once so
move to that location.

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 drivers/i2c/mxc_i2c.c |   53 +++++++++++++++++++------------------------------
 1 file changed, 20 insertions(+), 33 deletions(-)

diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
index 9063d1e..ac91872 100644
--- a/drivers/i2c/mxc_i2c.c
+++ b/drivers/i2c/mxc_i2c.c
@@ -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)
-- 
1.7.9.5

[U-Boot] [PATCH 09/24] mxc_i2c: place imx_start code inline

[Marek Vasut]

Dear Troy Kisky,

> imx_start is only referenced once so
> move to that location.
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

Acked-by: Marek Vasut <marex@denx.de>

> ---
>  drivers/i2c/mxc_i2c.c |   53
> +++++++++++++++++++------------------------------ 1 file changed, 20
> insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
> index 9063d1e..ac91872 100644
> --- a/drivers/i2c/mxc_i2c.c
> +++ b/drivers/i2c/mxc_i2c.c
> @@ -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)

Best regards,
Marek Vasut

[U-Boot] [PATCH 10/24] mxc_i2c: place i2c_reset code inline

[Troy Kisky]

imx_reset is only referenced once so
move to that location.

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 drivers/i2c/mxc_i2c.c |   15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
index ac91872..2ef7b92 100644
--- a/drivers/i2c/mxc_i2c.c
+++ b/drivers/i2c/mxc_i2c.c
@@ -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);
 }
 
 /*
-- 
1.7.9.5

[U-Boot] [PATCH 10/24] mxc_i2c: place i2c_reset code inline

[Marek Vasut]

Dear Troy Kisky,

> imx_reset is only referenced once so
> move to that location.
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

Acked-by: Marek Vasut <marex@denx.de>

> ---
>  drivers/i2c/mxc_i2c.c |   15 +++------------
>  1 file changed, 3 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
> index ac91872..2ef7b92 100644
> --- a/drivers/i2c/mxc_i2c.c
> +++ b/drivers/i2c/mxc_i2c.c
> @@ -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);
>  }
> 
>  /*

Best regards,
Marek Vasut

[U-Boot] [PATCH 11/24] mxc_i2c: don't disable controller after every transaction

[Troy Kisky]

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

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 drivers/i2c/mxc_i2c.c |   29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
index 2ef7b92..e433312 100644
--- a/drivers/i2c/mxc_i2c.c
+++ b/drivers/i2c/mxc_i2c.c
@@ -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;
-- 
1.7.9.5

[U-Boot] [PATCH 12/24] mxc_i2c: change slave addr if conflicts with destination.

[Troy Kisky]

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

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 drivers/i2c/mxc_i2c.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
index e433312..2bff2b8 100644
--- a/drivers/i2c/mxc_i2c.c
+++ b/drivers/i2c/mxc_i2c.c
@@ -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)
-- 
1.7.9.5

[U-Boot] [PATCH 13/24] mxc_i2c: check for arbitration lost

[Troy Kisky]

No need to continue waiting if arbitration lost.

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 drivers/i2c/mxc_i2c.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
index 2bff2b8..df033ea 100644
--- a/drivers/i2c/mxc_i2c.c
+++ b/drivers/i2c/mxc_i2c.c
@@ -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);
-- 
1.7.9.5

[U-Boot] [PATCH 13/24] mxc_i2c: check for arbitration lost

[Marek Vasut]

Dear Troy Kisky,

> No need to continue waiting if arbitration lost.
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

Acked-by: Marek Vasut <marex@denx.de>

> ---
>  drivers/i2c/mxc_i2c.c |    7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
> index 2bff2b8..df033ea 100644
> --- a/drivers/i2c/mxc_i2c.c
> +++ b/drivers/i2c/mxc_i2c.c
> @@ -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);

Best regards,
Marek Vasut

[U-Boot] [PATCH 14/24] mxc_i2c: add retries

[Troy Kisky]

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

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 drivers/i2c/mxc_i2c.c |   36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
index df033ea..802f70f 100644
--- a/drivers/i2c/mxc_i2c.c
+++ b/drivers/i2c/mxc_i2c.c
@@ -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;
 }
 
-- 
1.7.9.5

[U-Boot] [PATCH 14/24] mxc_i2c: add retries

[Marek Vasut]

Dear Troy Kisky,

> Retry unexpected hardware errors. This
> will not retry a received NAK.
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

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.

> ---
>  drivers/i2c/mxc_i2c.c |   36 +++++++++++++++++++++++++++---------
>  1 file changed, 27 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
> index df033ea..802f70f 100644
> --- a/drivers/i2c/mxc_i2c.c
> +++ b/drivers/i2c/mxc_i2c.c
> @@ -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;
>  }

Best regards,
Marek Vasut

[U-Boot] [PATCH 14/24] mxc_i2c: add retries

[Troy Kisky]

On 6/22/2012 10:06 AM, Marek Vasut wrote:
> Dear Troy Kisky,
>
>> Retry unexpected hardware errors. This
>> will not retry a received NAK.
>>
>> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> 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.

>> ---
>>   drivers/i2c/mxc_i2c.c |   36 +++++++++++++++++++++++++++---------
>>   1 file changed, 27 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
>> index df033ea..802f70f 100644
>> --- a/drivers/i2c/mxc_i2c.c
>> +++ b/drivers/i2c/mxc_i2c.c
>> @@ -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;
>>   }
> Best regards,
> Marek Vasut
>

[U-Boot] [PATCH 15/24] mxc_i2c: add i2c_regs argument to i2c_imx_stop

[Troy Kisky]

This is prep work for CONFIG_I2C_MULTI_BUS.

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 drivers/i2c/mxc_i2c.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
index 802f70f..cb061f7 100644
--- a/drivers/i2c/mxc_i2c.c
+++ b/drivers/i2c/mxc_i2c.c
@@ -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;
 }
-- 
1.7.9.5

[U-Boot] [PATCH 15/24] mxc_i2c: add i2c_regs argument to i2c_imx_stop

[Marek Vasut]

Dear Troy Kisky,

> This is prep work for CONFIG_I2C_MULTI_BUS.
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

Acked-by: Marek Vasut <marex@denx.de>


> ---
>  drivers/i2c/mxc_i2c.c |    7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
> index 802f70f..cb061f7 100644
> --- a/drivers/i2c/mxc_i2c.c
> +++ b/drivers/i2c/mxc_i2c.c
> @@ -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;
>  }

Best regards,
Marek Vasut

[U-Boot] [PATCH 16/24] mxc_i2c: prep work for multiple busses support

[Troy Kisky]

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 drivers/i2c/mxc_i2c.c |  121 ++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 100 insertions(+), 21 deletions(-)

diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
index cb061f7..ec05798 100644
--- a/drivers/i2c/mxc_i2c.c
+++ b/drivers/i2c/mxc_i2c.c
@@ -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());
 }
-- 
1.7.9.5

[U-Boot] [PATCH 16/24] mxc_i2c: prep work for multiple busses support

[Marek Vasut]

Dear Troy Kisky,

> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> ---
>  drivers/i2c/mxc_i2c.c |  121
> ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 100
> insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
> index cb061f7..ec05798 100644
> --- a/drivers/i2c/mxc_i2c.c
> +++ b/drivers/i2c/mxc_i2c.c
> @@ -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);

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

> +#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)

static? Or this is part of i2c api?

> +{
> +#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());
>  }

[U-Boot] [PATCH 16/24] mxc_i2c: prep work for multiple busses support

[Heiko Schocher]

Hello Troy,

On 22.06.2012 06:12, Troy Kisky wrote:
> Signed-off-by: Troy Kisky<troy.kisky@boundarydevices.com>
> ---
>   drivers/i2c/mxc_i2c.c |  121 ++++++++++++++++++++++++++++++++++++++++---------
>   1 file changed, 100 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
> index cb061f7..ec05798 100644
> --- a/drivers/i2c/mxc_i2c.c
> +++ b/drivers/i2c/mxc_i2c.c
[...]
> @@ -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;

> +#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
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH 16/24] mxc_i2c: prep work for multiple busses support

[Marek Vasut]

Dear Heiko Schocher,

> Hello Troy,
> 
> On 22.06.2012 06:12, Troy Kisky wrote:
> > Signed-off-by: Troy Kisky<troy.kisky@boundarydevices.com>
> > ---
> > 
> >   drivers/i2c/mxc_i2c.c |  121
> >   ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 100
> >   insertions(+), 21 deletions(-)
> > 
> > diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
> > index cb061f7..ec05798 100644
> > --- a/drivers/i2c/mxc_i2c.c
> > +++ b/drivers/i2c/mxc_i2c.c
> 
> [...]
> 
> > @@ -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;

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

> 
> > +#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

Best regards,
Marek Vasut

[U-Boot] [PATCH 16/24] mxc_i2c: prep work for multiple busses support

[Wolfgang Denk]

Dear Marek Vasut,

In message <201206242208.51305.marek.vasut@gmail.com> you wrote:
> 
> > 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
>95% that you are doing something fundamally wrong.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Go to Heaven for the climate, Hell for the company."    - Mark Twain

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

[Troy Kisky]

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

Signed-off-by: Troy Kisky <troy.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);
-- 
1.7.9.5

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

[Heiko Schocher]

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 Kisky<troy.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()
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
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

[Troy Kisky]

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 Kisky<troy.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

[U-Boot] [PATCH 18/24] mxc_i2c.c: finish adding CONFIG_I2C_MULTI_BUS support

[Troy Kisky]

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 drivers/i2c/mxc_i2c.c |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
index 339bb6f..5d18752 100644
--- a/drivers/i2c/mxc_i2c.c
+++ b/drivers/i2c/mxc_i2c.c
@@ -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);
-- 
1.7.9.5

[U-Boot] [PATCH 18/24] mxc_i2c.c: finish adding CONFIG_I2C_MULTI_BUS support

[Marek Vasut]

Dear Troy Kisky,

> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> ---
>  drivers/i2c/mxc_i2c.c |   17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
> index 339bb6f..5d18752 100644
> --- a/drivers/i2c/mxc_i2c.c
> +++ b/drivers/i2c/mxc_i2c.c
> @@ -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.

> +}
> +
> +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);

Best regards,
Marek Vasut

[U-Boot] [PATCH 18/24] mxc_i2c.c: finish adding CONFIG_I2C_MULTI_BUS support

[Troy Kisky]

On 6/22/2012 10:09 AM, Marek Vasut wrote:
> Dear Troy Kisky,
>
>> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
>> ---
>>   drivers/i2c/mxc_i2c.c |   17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
>> index 339bb6f..5d18752 100644
>> --- a/drivers/i2c/mxc_i2c.c
>> +++ b/drivers/i2c/mxc_i2c.c
>> @@ -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.

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?

>
>> +}
>> +
>> +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);
> Best regards,
> Marek Vasut
>

[U-Boot] [PATCH 18/24] mxc_i2c.c: finish adding CONFIG_I2C_MULTI_BUS support

[Tabi Timur-B04825]

On Fri, Jun 22, 2012 at 2:41 PM, Troy Kisky
<troy.kisky@boundarydevices.com> wrote:
>
> 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.

-- 
Timur Tabi
Linux kernel developer at Freescale

[U-Boot] [PATCH 19/24] iomux-v3: remove include of mx6x_pins.h

[Troy Kisky]

This include is not needed.

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 arch/arm/cpu/armv7/mx6/iomux-v3.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/cpu/armv7/mx6/iomux-v3.c b/arch/arm/cpu/armv7/mx6/iomux-v3.c
index 8785532..a0c4b15 100644
--- a/arch/arm/cpu/armv7/mx6/iomux-v3.c
+++ b/arch/arm/cpu/armv7/mx6/iomux-v3.c
@@ -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;
-- 
1.7.9.5

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

[Troy Kisky]

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 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%)

diff --git a/arch/arm/cpu/armv7/mx6/iomux-v3.c b/arch/arm/cpu/armv7/mx6/iomux-v3.c
index a0c4b15..da093fb 100644
--- a/arch/arm/cpu/armv7/mx6/iomux-v3.c
+++ b/arch/arm/cpu/armv7/mx6/iomux-v3.c
@@ -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;
 
diff --git a/arch/arm/include/asm/arch-mx6/mx6x_pins.h b/arch/arm/include/asm/arch-mx6/mx6x_pins.h
index afaa068..c0bb494 100644
--- a/arch/arm/include/asm/arch-mx6/mx6x_pins.h
+++ b/arch/arm/include/asm/arch-mx6/mx6x_pins.h
@@ -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)
diff --git a/arch/arm/include/asm/arch-mx6/iomux-v3.h b/arch/arm/include/asm/imx-common/iomux-v3.h
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
diff --git a/board/freescale/mx6qarm2/mx6qarm2.c b/board/freescale/mx6qarm2/mx6qarm2.c
index 1367b88..340c4c4 100644
--- a/board/freescale/mx6qarm2/mx6qarm2.c
+++ b/board/freescale/mx6qarm2/mx6qarm2.c
@@ -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>
diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
index 72b4b54..74ce84c 100644
--- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
+++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
@@ -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>
diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c
index 5dec673..f2523f6 100644
--- a/drivers/usb/host/ehci-mx6.c
+++ b/drivers/usb/host/ehci-mx6.c
@@ -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"
-- 
1.7.9.5

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

[Troy Kisky]

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 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%)

diff --git a/arch/arm/cpu/armv7/imx-common/Makefile b/arch/arm/cpu/armv7/imx-common/Makefile
index e5ff375..53296fa 100644
--- a/arch/arm/cpu/armv7/imx-common/Makefile
+++ b/arch/arm/cpu/armv7/imx-common/Makefile
@@ -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))
diff --git a/arch/arm/cpu/armv7/mx6/iomux-v3.c b/arch/arm/cpu/armv7/imx-common/iomux-v3.c
similarity index 100%
rename from arch/arm/cpu/armv7/mx6/iomux-v3.c
rename to arch/arm/cpu/armv7/imx-common/iomux-v3.c
diff --git a/arch/arm/cpu/armv7/mx6/Makefile b/arch/arm/cpu/armv7/mx6/Makefile
index b0da028..cbce411 100644
--- a/arch/arm/cpu/armv7/mx6/Makefile
+++ b/arch/arm/cpu/armv7/mx6/Makefile
@@ -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)
-- 
1.7.9.5

[U-Boot] [PATCH 22/24] i.mx53: add definition for I2C3_BASE_ADDR

[Troy Kisky]

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 arch/arm/include/asm/arch-mx5/imx-regs.h |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/include/asm/arch-mx5/imx-regs.h b/arch/arm/include/asm/arch-mx5/imx-regs.h
index 4fa6658..caf5d21 100644
--- a/arch/arm/include/asm/arch-mx5/imx-regs.h
+++ b/arch/arm/include/asm/arch-mx5/imx-regs.h
@@ -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
-- 
1.7.9.5

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

[Troy Kisky]

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 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

diff --git a/arch/arm/cpu/armv7/imx-common/Makefile b/arch/arm/cpu/armv7/imx-common/Makefile
index 53296fa..bf36be5 100644
--- a/arch/arm/cpu/armv7/imx-common/Makefile
+++ b/arch/arm/cpu/armv7/imx-common/Makefile
@@ -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))
diff --git a/arch/arm/cpu/armv7/imx-common/i2c.c b/arch/arm/cpu/armv7/imx-common/i2c.c
new file mode 100644
index 0000000..a57d393
--- /dev/null
+++ b/arch/arm/cpu/armv7/imx-common/i2c.c
@@ -0,0 +1,79 @@
+/*
+ * Copyright (C) 2012 Boundary Devices Inc.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+#include <common.h>
+#include <asm/arch/clock.h>
+#include <asm/arch/imx-regs.h>
+#include <asm/errno.h>
+#include <asm/gpio.h>
+#include <asm/imx-common/mxc_i2c.h>
+#include <asm/io.h>
+#include <i2c.h>
+
+static void toggle_scl(void *priv)
+{
+	struct i2c_pads_info *p = (struct i2c_pads_info *)priv;
+	int i;
+	gpio_direction_input(p->sda.gp);
+	gpio_direction_input(p->scl.gp);
+
+	imx_iomux_v3_setup_pad(p->sda.gpio_mode);
+	imx_iomux_v3_setup_pad(p->scl.gpio_mode);
+
+	printf("%s sda=%i scl=%i sda.gp=0x%x scl.gp=0x%x\n", __func__,
+		gpio_get_value(p->sda.gp), gpio_get_value(p->scl.gp),
+		p->sda.gp, p->scl.gp);
+	/* Send high and low on the SCL line */
+	for (i = 0; i < 9; i++) {
+		gpio_direction_output(p->scl.gp, 0);
+		udelay(50);
+		gpio_direction_input(p->scl.gp);
+		udelay(50);
+		printf("%s sda=%i scl=%i\n", __func__,
+			gpio_get_value(p->sda.gp), gpio_get_value(p->scl.gp));
+		udelay(50);
+	}
+	imx_iomux_v3_setup_pad(p->sda.i2c_mode);
+	imx_iomux_v3_setup_pad(p->scl.i2c_mode);
+}
+
+static const unsigned i2c_bases[] = {
+	I2C1_BASE_ADDR,
+	I2C2_BASE_ADDR,
+#ifdef I2C3_BASE_ADDR
+	I2C3_BASE_ADDR,
+#endif
+};
+
+/* i2c_index can be from 0 - 2 */
+void setup_i2c(unsigned i2c_index, int speed, int slave_addr,
+		struct i2c_pads_info *p)
+{
+	unsigned reg;
+
+	if (i2c_index >= ARRAY_SIZE(i2c_bases))
+		return;
+	imx_iomux_v3_setup_pad(p->sda.i2c_mode);
+	imx_iomux_v3_setup_pad(p->scl.i2c_mode);
+	/* Enable i2c clock */
+	enable_i2c_clock(1, i2c_index);
+	bus_i2c_init(i2c_bases[i2c_index], speed, slave_addr, toggle_scl, p);
+}
diff --git a/arch/arm/cpu/armv7/mx5/clock.c b/arch/arm/cpu/armv7/mx5/clock.c
index e92f106..b5d2794 100644
--- a/arch/arm/cpu/armv7/mx5/clock.c
+++ b/arch/arm/cpu/armv7/mx5/clock.c
@@ -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;
diff --git a/arch/arm/cpu/armv7/mx6/clock.c b/arch/arm/cpu/armv7/mx6/clock.c
index ef98563..d31777b 100644
--- a/arch/arm/cpu/armv7/mx6/clock.c
+++ b/arch/arm/cpu/armv7/mx6/clock.c
@@ -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;
diff --git a/arch/arm/include/asm/arch-mx5/clock.h b/arch/arm/include/asm/arch-mx5/clock.h
index ea972a3..da604bf 100644
--- a/arch/arm/include/asm/arch-mx5/clock.h
+++ b/arch/arm/include/asm/arch-mx5/clock.h
@@ -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 */
diff --git a/arch/arm/include/asm/arch-mx6/clock.h b/arch/arm/include/asm/arch-mx6/clock.h
index 613809b..04daf36 100644
--- a/arch/arm/include/asm/arch-mx6/clock.h
+++ b/arch/arm/include/asm/arch-mx6/clock.h
@@ -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 */
diff --git a/arch/arm/include/asm/imx-common/mxc_i2c.h b/arch/arm/include/asm/imx-common/mxc_i2c.h
new file mode 100644
index 0000000..2a8f729
--- /dev/null
+++ b/arch/arm/include/asm/imx-common/mxc_i2c.h
@@ -0,0 +1,46 @@
+/*
+ * Copyright (C) 2011 Freescale Semiconductor, Inc. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+#ifndef __ASM_ARCH_MXC_MXC_I2C_H__
+#define __ASM_ARCH_MXC_MXC_I2C_H__
+#include <asm/imx-common/iomux-v3.h>
+
+typedef void (*toggle_i2c_fn)(void *p);
+struct mxc_i2c_regs;
+
+
+struct i2c_pin_ctrl {
+	iomux_v3_cfg_t i2c_mode;
+	iomux_v3_cfg_t gpio_mode;
+	unsigned char gp;
+	unsigned char spare;
+};
+
+struct i2c_pads_info {
+	struct i2c_pin_ctrl scl;
+	struct i2c_pin_ctrl sda;
+};
+
+void setup_i2c(unsigned i2c_index, int speed, int slave_addr,
+		struct i2c_pads_info *p);
+void bus_i2c_init(void *base, int speed, int slave_addr, toggle_i2c_fn tog,
+		void *p);
+int bus_i2c_read(void *base, 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);
+#endif
-- 
1.7.9.5

[U-Boot] [PATCH 24/24] mx6qsabrelite: add i2c multi-bus support

[Troy Kisky]

This includes bus recovery support.

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 board/freescale/mx6qsabrelite/mx6qsabrelite.c |   50 +++++++++++++++++++++++--
 include/configs/mx6qsabrelite.h               |    6 +--
 2 files changed, 48 insertions(+), 8 deletions(-)

diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
index 74ce84c..29c6630 100644
--- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
+++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
@@ -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;
 }
diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h
index df2b735..c3c6be2 100644
--- a/include/configs/mx6qsabrelite.h
+++ b/include/configs/mx6qsabrelite.h
@@ -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
-- 
1.7.9.5

[U-Boot] [PATCH 01/24] mxc_i2c: fix i2c_imx_stop

[Marek Vasut]

Dear Troy Kisky,

> Instead of clearing 2 bits, all the other
> bits were set because '|=' was used instead
> of '&='.
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

Acked-by: Marek Vasut <marex@denx.de>

> ---
>  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.
> 
> 
> diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
> index fc68062..c0c45fd 100644
> --- a/drivers/i2c/mxc_i2c.c
> +++ b/drivers/i2c/mxc_i2c.c
> @@ -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

[U-Boot] [PATCH 01/24] mxc_i2c: fix i2c_imx_stop

[Stefano Babic]

On 22/06/2012 06:11, Troy Kisky wrote:
> Instead of clearing 2 bits, all the other
> bits were set because '|=' was used instead
> of '&='.
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> ---
>  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.
> 
> 
> diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
> index fc68062..c0c45fd 100644
> --- a/drivers/i2c/mxc_i2c.c
> +++ b/drivers/i2c/mxc_i2c.c
> @@ -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 !

Acked-by: Stefano Babic <sbabic@denx.de>

Best regards,
Stefano Babic


-- 
=====================================================================
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
=====================================================================