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

Troy Kisky
2012-06-22T17:40:00Z

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
>