On Thu, Mar 8, 2012 at 8:30 AM, Stephen Warren <swarren@xxxxxxxxxxxxx> wrote:
> On Mon, Dec 26, 2011 at 4:44 PM, Alok Chauhan <alokc@xxxxxxxxxx> wrote:
>> From: Alok Chauhan <alokc@xxxxxxxxxx>
>>
>> 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.
The TPM issue was from the inital patchset, this current patch works
with those slow TPMs. The change to make the sleep duration dependent
on the i2c clock rate fixed it. Sorry it took so long for me to test
it.
>
> P.S. Top-posting makes for confusing emails.
>
> Thanks.
>
>>> Signed-off-by: Alok Chauhan <alokc@xxxxxxxxxx>
>>> ---
>>> 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
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
