diff mbox

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

Message ID 1340338339-11626-17-git-send-email-troy.kisky@boundarydevices.com
State Superseded
Delegated to: Heiko Schocher
Headers show

Commit Message

Troy Kisky June 22, 2012, 4:12 a.m. UTC
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(+)

Comments

Heiko Schocher June 24, 2012, 8:51 a.m. UTC | #1
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
Troy Kisky June 26, 2012, 3:42 a.m. UTC | #2
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
diff mbox

Patch

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