Universal Boot Loader general discussion ()

Troy Kisky | 22 Jun 2012 06:12

[PATCH 14/24] mxc_i2c: add retries

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

Signed-off-by: Troy Kisky <troy.kisky <at> 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
 <at>  <at>  -213,7 +213,7  <at>  <at>  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;
 <at>  <at>  -230,7 +230,7  <at>  <at>  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);

(Continue reading)

headers

Marek Vasut | 22 Jun 2012 19:06

Re: [PATCH 14/24] mxc_i2c: add retries

Dear Troy Kisky,

> Retry unexpected hardware errors. This
> will not retry a received NAK.
> 
> Signed-off-by: Troy Kisky <troy.kisky <at> 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
>  <at>  <at>  -213,7 +213,7  <at>  <at>  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;
>  <at>  <at>  -230,7 +230,7  <at>  <at>  static int i2c_init_transfer(struct mxc_i2c_regs
> *i2c_regs, writeb(0, &i2c_regs->i2sr);

(Continue reading)

headers

Troy Kisky | 22 Jun 2012 19:40

Re: [PATCH 14/24] mxc_i2c: add retries

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 <at> 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
>>  <at>  <at>  -213,7 +213,7  <at>  <at>  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,

(Continue reading)

headers

Troy Kisky | 22 Jun 2012 06:11

[PATCH 03/24] mxc_i2c: create tx_byte function

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

Signed-off-by: Troy Kisky <troy.kisky <at> 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
 <at>  <at>  -33,6 +33,7  <at>  <at> 
 #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>

 <at>  <at>  -207,17 +208,21  <at>  <at>  int i2c_imx_trx_complete(void)
 		udelay(1);
 	}

-	return 1;
+	return -ETIMEDOUT;
 }

-/*
- * Check if the transaction was ACKed

(Continue reading)

headers

Marek Vasut | 22 Jun 2012 18:58

Re: [PATCH 03/24] mxc_i2c: create tx_byte function

Dear Troy Kisky,

> Use tx_byte function instead of having 3 copies
> of the code.
> 
> Signed-off-by: Troy Kisky <troy.kisky <at> 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
>  <at>  <at>  -33,6 +33,7  <at>  <at> 
>  #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>
> 
>  <at>  <at>  -207,17 +208,21  <at>  <at>  int i2c_imx_trx_complete(void)
>  		udelay(1);
>  	}
> 
> -	return 1;
> +	return -ETIMEDOUT;
>  }

(Continue reading)

headers

Troy Kisky | 23 Jun 2012 01:05

Re: [PATCH 03/24] mxc_i2c: create tx_byte function

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 <at> 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
>>  <at>  <at>  -33,6 +33,7  <at>  <at> 
>>   #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>
>>
>>  <at>  <at>  -207,17 +208,21  <at>  <at>  int i2c_imx_trx_complete(void)
>>   		udelay(1);
>>   	}
>>
>> -	return 1;
>> +	return -ETIMEDOUT;

(Continue reading)

headers

Marek Vasut | 23 Jun 2012 03:51

Re: [PATCH 03/24] mxc_i2c: create tx_byte function

Dear Troy Kisky,

[...]

> >>  <at>  <at>  -364,7 +340,7  <at>  <at>  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)

(Continue reading)

headers

Troy Kisky | 22 Jun 2012 06:12

[PATCH 05/24] mxc_i2c: create i2c_init_transfer

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

Signed-off-by: Troy Kisky <troy.kisky <at> 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
 <at>  <at>  -275,19 +275,29  <at>  <at>  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;
+

(Continue reading)

headers

Marek Vasut | 22 Jun 2012 18:59

Re: [PATCH 05/24] mxc_i2c: create i2c_init_transfer

Dear Troy Kisky,

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

Acked-by: Marek Vasut <marex <at> 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
>  <at>  <at>  -275,19 +275,29  <at>  <at>  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;

(Continue reading)

headers

Stefano Babic | 24 Jun 2012 10:06

Re: [PATCH 05/24] mxc_i2c: create i2c_init_transfer

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 <at> boundarydevices.com>
> ---

Acked-by: Stefano Babic <sbabic <at> 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
=====================================================================

headers

Troy Kisky | 22 Jun 2012 06:12

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

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

Signed-off-by: Troy Kisky <troy.kisky <at> 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
 <at>  <at>  -218,6 +218,8  <at>  <at>  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

headers

Troy Kisky | 22 Jun 2012 06:11

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

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 <at> 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
 <at>  <at>  -200,10 +200,8  <at>  <at>  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);
 	}
 <at>  <at>  -215,6 +213,7  <at>  <at>  static int tx_byte(struct mxc_i2c_regs *i2c_regs, u8 byte)
 {
 	unsigned ret;

(Continue reading)

headers

Marek Vasut | 22 Jun 2012 18:59

Re: [PATCH 04/24] mxc_i2c: clear i2sr before waiting for bit

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 <at> 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
>  <at>  <at>  -200,10 +200,8  <at>  <at>  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);
>  	}
>  <at>  <at>  -215,6 +213,7  <at>  <at>  static int tx_byte(struct mxc_i2c_regs *i2c_regs, u8

(Continue reading)

headers

Troy Kisky | 24 Jun 2012 02:18

Re: [PATCH 04/24] mxc_i2c: clear i2sr before waiting for bit

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 <at> 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
>>  <at>  <at>  -200,10 +200,8  <at>  <at>  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);
>>   	}

(Continue reading)

headers

Troy Kisky | 22 Jun 2012 06:12

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

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 <at> 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
 <at>  <at>  -63,8 +63,6  <at>  <at>  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 },
 <at>  <at>  -164,48 +162,25  <at>  <at>  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

(Continue reading)

headers

Marek Vasut | 22 Jun 2012 19:01

Re: [PATCH 07/24] mxc_i2c: combine i2c_imx_bus_busy and i2c_imx_trx_complete into wait_for_sr_state

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 <at> boundarydevices.com>
> ---

Best regards,
Marek Vasut

headers

Troy Kisky | 22 Jun 2012 19:34

Re: [PATCH 07/24] mxc_i2c: combine i2c_imx_bus_busy and i2c_imx_trx_complete into wait_for_sr_state

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 <at> boundarydevices.com>
>> ---
> Best regards,
> Marek Vasut
>

headers

Troy Kisky | 22 Jun 2012 20:01

Re: [PATCH 07/24] mxc_i2c: combine i2c_imx_bus_busy and i2c_imx_trx_complete into wait_for_sr_state

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 <at> boundarydevices.com>
>>> ---
>> Best regards,
>> Marek Vasut
>>
>
>

Thanks for the reviews Marek.

Troy

headers

Troy Kisky | 22 Jun 2012 06:12

[PATCH 08/24] mxc_i2c: remove redundant read

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

Signed-off-by: Troy Kisky <troy.kisky <at> 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
 <at>  <at>  -193,7 +193,6  <at>  <at>  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

headers

Marek Vasut | 22 Jun 2012 19:02

Re: [PATCH 08/24] mxc_i2c: remove redundant read

Dear Troy Kisky,

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

Acked-by: Marek Vasut <marex <at> 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
>  <at>  <at>  -193,7 +193,6  <at>  <at>  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

headers

Troy Kisky | 22 Jun 2012 06:12

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

Signed-off-by: Troy Kisky <troy.kisky <at> 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
 <at>  <at>  -302,27 +302,6  <at>  <at>  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();
-

(Continue reading)

headers

Marek Vasut | 22 Jun 2012 19:00

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

Dear Troy Kisky,

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

Acked-by: Marek Vasut <marex <at> 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
>  <at>  <at>  -302,27 +302,6  <at>  <at>  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);

(Continue reading)

headers

Stefano Babic | 24 Jun 2012 10:03

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

On 22/06/2012 06:12, Troy Kisky wrote:
> Signed-off-by: Troy Kisky <troy.kisky <at> 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
>  <at>  <at>  -302,27 +302,6  <at>  <at>  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();

(Continue reading)

headers

Troy Kisky | 22 Jun 2012 06:12

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

No need to continue waiting if arbitration lost.

Signed-off-by: Troy Kisky <troy.kisky <at> 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
 <at>  <at>  -54,6 +54,7  <at>  <at>  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)

 <at>  <at>  -164,6 +165,12  <at>  <at>  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)

(Continue reading)

headers

Marek Vasut | 22 Jun 2012 19:04

Re: [PATCH 13/24] mxc_i2c: check for arbitration lost

Dear Troy Kisky,

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

Acked-by: Marek Vasut <marex <at> 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
>  <at>  <at>  -54,6 +54,7  <at>  <at>  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)
> 
>  <at>  <at>  -164,6 +165,12  <at>  <at>  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);

(Continue reading)

headers

Troy Kisky | 22 Jun 2012 06:11

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

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

Signed-off-by: Troy Kisky <troy.kisky <at> 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
 <at>  <at>  -31,12 +31,9  <at>  <at> 
  */

 #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 {
 <at>  <at>  -446,4 +443,3  <at>  <at>  int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len)

 	return ret;
 }

(Continue reading)

headers

Marek Vasut | 22 Jun 2012 18:55

Re: [PATCH 02/24] mxc_i2c: remove ifdef of CONFIG_HARD_I2C

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 <at> boundarydevices.com>

Acked-by: Marek Vasut <marex <at> 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
>  <at>  <at>  -31,12 +31,9  <at>  <at> 
>   */
> 
>  #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>
>

(Continue reading)

headers

Troy Kisky | 22 Jun 2012 06:12

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

This is prep work for CONFIG_I2C_MULTI_BUS.

Signed-off-by: Troy Kisky <troy.kisky <at> 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
 <at>  <at>  -199,9 +199,8  <at>  <at>  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);
 <at>  <at>  -331,7 +330,7  <at>  <at>  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;

(Continue reading)

headers

Marek Vasut | 22 Jun 2012 19:06

Re: [PATCH 15/24] mxc_i2c: add i2c_regs argument to i2c_imx_stop

Dear Troy Kisky,

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

Acked-by: Marek Vasut <marex <at> 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
>  <at>  <at>  -199,9 +199,8  <at>  <at>  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);
>  <at>  <at>  -331,7 +330,7  <at>  <at>  int i2c_read(uchar chip, uint addr, int alen, uchar
> *buf, int len) buf[i] = readb(&i2c_regs->i2dr);
>  	}

(Continue reading)

headers

Troy Kisky | 22 Jun 2012 06:12

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

imx_start is only referenced once so
move to that location.

Signed-off-by: Troy Kisky <troy.kisky <at> 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
 <at>  <at>  -199,37 +199,6  <at>  <at>  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);

(Continue reading)

headers

Marek Vasut | 22 Jun 2012 19:02

Re: [PATCH 09/24] mxc_i2c: place imx_start code inline

Dear Troy Kisky,

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

Acked-by: Marek Vasut <marex <at> 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
>  <at>  <at>  -199,37 +199,6  <at>  <at>  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;
> -

(Continue reading)

headers

Troy Kisky | 22 Jun 2012 06:12

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

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 <at> 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
 <at>  <at>  -190,22 +190,16  <at>  <at>  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);
-

(Continue reading)

headers

Troy Kisky | 22 Jun 2012 06:12

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

imx_reset is only referenced once so
move to that location.

Signed-off-by: Troy Kisky <troy.kisky <at> 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
 <at>  <at>  -113,17 +113,6  <at>  <at>  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)

(Continue reading)

headers

Marek Vasut | 22 Jun 2012 19:03

Re: [PATCH 10/24] mxc_i2c: place i2c_reset code inline

Dear Troy Kisky,

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

Acked-by: Marek Vasut <marex <at> 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
>  <at>  <at>  -113,17 +113,6  <at>  <at>  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);
> -}

(Continue reading)

headers

Troy Kisky | 22 Jun 2012 06:12

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

Signed-off-by: Troy Kisky <troy.kisky <at> 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
 <at>  <at>  -27,7 +27,7  <at>  <at>  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
 <at>  <at>  -27,7 +27,7  <at>  <at>  include $(TOPDIR)/config.mk

(Continue reading)

headers

Troy Kisky | 22 Jun 2012 06:12

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

Signed-off-by: Troy Kisky <troy.kisky <at> 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
 <at>  <at>  -407,6 +407,23  <at>  <at>  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
+

(Continue reading)

headers

Marek Vasut | 22 Jun 2012 19:09

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

Dear Troy Kisky,

> Signed-off-by: Troy Kisky <troy.kisky <at> 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
>  <at>  <at>  -407,6 +407,23  <at>  <at>  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)

(Continue reading)

headers

Troy Kisky | 22 Jun 2012 21:41

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

On 6/22/2012 10:09 AM, Marek Vasut wrote:
> Dear Troy Kisky,
>
>> Signed-off-by: Troy Kisky <troy.kisky <at> 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
>>  <at>  <at>  -407,6 +407,23  <at>  <at>  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?

>
>> +}

(Continue reading)

headers

Tabi Timur-B04825 | 25 Jun 2012 05:34

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

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

headers

Troy Kisky | 22 Jun 2012 06:12

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

Signed-off-by: Troy Kisky <troy.kisky <at> 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
 <at>  <at>  -58,9 +58,7  <at>  <at>  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

 <at>  <at>  -114,11 +112,11  <at>  <at>  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)
 {

(Continue reading)

headers

Marek Vasut | 22 Jun 2012 19:08

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

Dear Troy Kisky,

> Signed-off-by: Troy Kisky <troy.kisky <at> 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
>  <at>  <at>  -58,9 +58,7  <at>  <at>  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
> 
>  <at>  <at>  -114,11 +112,11  <at>  <at>  static uint8_t i2c_imx_get_clk(unsigned int rate)
>  }
> 
>  /*
> - * Init I2C Bus
> + * Set I2C Bus speed
>   */

(Continue reading)

headers

Heiko Schocher | 24 Jun 2012 10:47

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

Hello Troy,

On 22.06.2012 06:12, Troy Kisky wrote:
> Signed-off-by: Troy Kisky<troy.kisky <at> 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
[...]
>  <at>  <at>  -359,10 +351,97  <at>  <at>  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

(Continue reading)

headers

Marek Vasut | 24 Jun 2012 22:08

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

Dear Heiko Schocher,

> Hello Troy,
> 
> On 22.06.2012 06:12, Troy Kisky wrote:
> > Signed-off-by: Troy Kisky<troy.kisky <at> 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
> 
> [...]
> 
> >  <at>  <at>  -359,10 +351,97  <at>  <at>  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;

(Continue reading)

headers

Wolfgang Denk | 25 Jun 2012 00:09

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

Dear Marek Vasut,

In message <201206242208.51305.marek.vasut <at> 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

headers

Troy Kisky | 22 Jun 2012 06:12

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

This include is not needed.

Signed-off-by: Troy Kisky <troy.kisky <at> 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
 <at>  <at>  -23,7 +23,6  <at>  <at> 
 #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

headers

Troy Kisky | 22 Jun 2012 06:12

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

Signed-off-by: Troy Kisky <troy.kisky <at> 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
 <at>  <at>  -93,6 +93,7  <at>  <at> 
 #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

headers

Troy Kisky | 22 Jun 2012 06:12

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

This includes bus recovery support.

Signed-off-by: Troy Kisky <troy.kisky <at> 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
 <at>  <at>  -27,6 +27,7  <at>  <at> 
 #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>
 <at>  <at>  -72,9 +73,48  <at>  <at>  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 */

(Continue reading)

headers

Troy Kisky | 22 Jun 2012 06:12

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

Signed-off-by: Troy Kisky <troy.kisky <at> 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
 <at>  <at>  -23,7 +23,7  <at>  <at> 
 #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
 <at>  <at>  -22,7 +22,7  <at>  <at> 
 #ifndef __ASM_ARCH_MX6_MX6X_PINS_H__

(Continue reading)

headers

Troy Kisky | 22 Jun 2012 06:12

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

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 <at> 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
 <at>  <at>  -246,6 +246,8  <at>  <at>  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)
 {
 <at>  <at>  -264,6 +266,7  <at>  <at>  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;
 <at>  <at>  -381,6 +384,29  <at>  <at>  void *get_base(void)

(Continue reading)

headers

Heiko Schocher | 24 Jun 2012 10:51

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

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 <at> 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
>  <at>  <at>  -246,6 +246,8  <at>  <at>  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)
>   {
>  <at>  <at>  -264,6 +266,7  <at>  <at>  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);
>   	}

(Continue reading)

headers

Troy Kisky | 26 Jun 2012 05:42

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

On 6/24/2012 1:51 AM, Heiko Schocher wrote:
> Hello Troy,
>
> On 22.06.2012 06:12, Troy Kisky wrote:
>> Toggling the scl line 9 clocks is the standard
>> way of returning a locked up bus to idle condition.
>>
>> Signed-off-by: Troy Kisky<troy.kisky <at> 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
>>  <at>  <at>  -246,6 +246,8  <at>  <at>  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)
>>   {
>>  <at>  <at>  -264,6 +266,7  <at>  <at>  static int i2c_init_transfer(struct mxc_i2c_regs 
>> *i2c_regs,
>>           if (ret != -ERESTART)
>>               writeb(0,&i2c_regs->i2cr);    /* Disable controller */

(Continue reading)

headers

Troy Kisky | 22 Jun 2012 06:12

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

Signed-off-by: Troy Kisky <troy.kisky <at> 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
 <at>  <at>  -27,7 +27,9  <at>  <at>  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

(Continue reading)

headers

Marek Vasut | 22 Jun 2012 18:55

Re: [PATCH 01/24] mxc_i2c: fix i2c_imx_stop

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 <at> boundarydevices.com>

Acked-by: Marek Vasut <marex <at> 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
>  <at>  <at>  -264,7 +264,7  <at>  <at>  void i2c_imx_stop(void)
> 
>  	/* Stop I2C transaction */
>  	temp = readb(&i2c_regs->i2cr);
> -	temp |= ~(I2CR_MSTA | I2CR_MTX);
> +	temp &= ~(I2CR_MSTA | I2CR_MTX);

(Continue reading)

headers

Stefano Babic | 24 Jun 2012 09:49

Re: [PATCH 01/24] mxc_i2c: fix i2c_imx_stop

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 <at> 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
>  <at>  <at>  -264,7 +264,7  <at>  <at>  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);
>

(Continue reading)