Re: [PATCH v2] i2c: tegra: Add delay before reset the controller

[Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>]

On Mon, Dec 26, 2011 at 4:44 PM, Alok Chauhan <alokc-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> From: Alok Chauhan <alokc-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>
> In NACK error condition, I2C controller violates clock-to-data setup 
> time before stop. In Software, because of this reset of controller is 
> happening before I2C controller could complete STOP condition.
>
> Added delay of 2 clock period before reset the controller in case of 
> NACK error.

(CC list cut down to people who were active in the original patch
posting thread)

On 03/07/2012 11:29 PM, Alok Chauhan wrote:
> Hi all,
> 
> If there are no more concern then can we merge this patch to i2c tegra driver? 

Alok, a couple comments:

1) The set of I2C maintainers has changed since you last posted this
patch. You should resend the whole patch to make sure the new
maintainers have a copy. I've CC'd Wolfram on this message, but you
still need to repost since there's no way Wolfram can extract the patch
from this email.

2) Wasn't there an outstanding question about the length of the delay
here, and whether it negatively interfered with the TPM chips on some
boards, since they always NACK the first transaction, which then needs
to be retried quickly so the TPM doesn't go back to sleep? I don't
recall you answering addressing that issue. There's no point reposting
without addressing all the issues. Also, I'm not sure whether
Shubhrajyot or Ben consider the answers you gave to their questions
satisfactory or not.

P.S. Top-posting makes for confusing emails.

Thanks.

>> Signed-off-by: Alok Chauhan <alokc-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> ---
>> Instead of setting constant value for delay as was in previous patch, 
>> now in the current modification delay will be calculated based on clock frequency of the bus.
>>
>>  drivers/i2c/busses/i2c-tegra.c |    8 ++++++++
>>  1 files changed, 8 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-tegra.c 
>> b/drivers/i2c/busses/i2c-tegra.c index 6381604..62e197c 100644
>> --- a/drivers/i2c/busses/i2c-tegra.c
>> +++ b/drivers/i2c/busses/i2c-tegra.c
>> @@ -517,6 +517,14 @@ static int tegra_i2c_xfer_msg(struct 
>> tegra_i2c_dev *i2c_dev,
>>        if (likely(i2c_dev->msg_err == I2C_ERR_NONE))
>>                return 0;
>>
>> +       /*
>> +        * In NACK error condition resetting of I2C controller happens
>> +        * before STOP condition is properly completed by I2C 
>> + controller,
>> +        * so wait for 2 clock cycle to complete STOP condition.
>> +        */
> 
>>>>> Why do you need to reset the controller in case of a NACK.
>>>> This is required because of hardware limitations. Without reset we can't flus the internal hardware registers.
> 
> 
>> +       if (i2c_dev->msg_err == I2C_ERR_NO_ACK)
>> +               udelay(DIV_ROUND_UP(2 * 1000000, 
>> + i2c_dev->bus_clk_rate));
>> +
>>        tegra_i2c_init(i2c_dev);
>>        if (i2c_dev->msg_err == I2C_ERR_NO_ACK) {
>>                if (msg->flags & I2C_M_IGNORE_NAK)
>> --
>> 1.7.4.1