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

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

[Alok Chauhan]

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.

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.
+	 */
+	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

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

[Ben Dooks]

On Mon, Dec 26, 2011 at 04:44:41PM +0530, Alok Chauhan 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.
> 
> 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.
> +	 */
> +	if (i2c_dev->msg_err == I2C_ERR_NO_ACK)
> +		udelay(DIV_ROUND_UP(2 * 1000000, i2c_dev->bus_clk_rate));
> +

Is a delay here good, would it be better to sleep so that some other
process can gain cpu time?

  	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-i2c" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Ben Dooks, ben-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org, http://www.fluff.org/ben/

Large Hadron Colada: A large Pina Colada that makes the universe disappear.

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

[Alok Chauhan]

On Mon, Dec 26, 2011 at 04:44:41PM +0530, Alok Chauhan wrote:
> From: Alok Chauhan <alokc@nvidia.com>
> 
> 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.
> 
> Signed-off-by: Alok Chauhan <alokc@nvidia.com>
> ---
> 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.
> +	 */
> +	if (i2c_dev->msg_err == I2C_ERR_NO_ACK)
> +		udelay(DIV_ROUND_UP(2 * 1000000, i2c_dev->bus_clk_rate));
> +

>Is a delay here good, would it be better to sleep so that some other process can gain cpu time?
We mostly used 100 khz i2c clock frequency and delay in that case will be 20 usec. Would it be 
ok to sleep for such a smaller time? Won't it increase any other overhead?

  	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-i2c" 
> in the body of a message to majordomo@vger.kernel.org More majordomo 
> info at  http://vger.kernel.org/majordomo-info.html

--
Ben Dooks, ben@fluff.org, http://www.fluff.org/ben/

Large Hadron Colada: A large Pina Colada that makes the universe disappear.

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

[Alok Chauhan]

> On Mon, Dec 26, 2011 at 04:44:41PM +0530, Alok Chauhan wrote:
>> 
> +	/*
> +	 * 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.
> +	 */
> +	if (i2c_dev->msg_err == I2C_ERR_NO_ACK)
> +		udelay(DIV_ROUND_UP(2 * 1000000, i2c_dev->bus_clk_rate));
> +

>Is a delay here good, would it be better to sleep so that some other process can gain cpu time?
>>We mostly used 100 khz i2c clock frequency and delay in that case will be 20 usec. Would it be ok to sleep for such a smaller time? Won't it increase any other overhead?

Please let me know if sleep is better option here instead of waiting.

Thanks
Alok

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Shubhrajyoti Datta]

Hello Alok,

On Mon, Dec 26, 2011 at 4:44 PM, Alok Chauhan <alokc@nvidia.com> wrote:
> From: Alok Chauhan <alokc@nvidia.com>
>
> 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.
>
> Signed-off-by: Alok Chauhan <alokc@nvidia.com>
> ---
> 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.
> +       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-i2c" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Alok Chauhan]

Hi Shubhrajyot,

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.
>
> 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
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" 
> in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo 
> info at  http://vger.kernel.org/majordomo-info.html

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

[Alok Chauhan]

Hi all,

If there are no more concern then can we merge this patch to i2c tegra driver? 

Thanks
Alok

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.
>
> 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
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" 
> in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Stephen Warren]

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

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

[Dylan Reid]

On Thu, Mar 8, 2012 at 8:30 AM, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
> 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.
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-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
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Alok Chauhan]

NACK interrupt generated before I2C controller generates
the STOP condition on bus. In Software, because of this
reset of controller is happening before I2C controller could
complete STOP condition. So wait for some time before resetting
the controller so that STOP condition has delivered properly on bus.

Added delay of 2 clock period before reset the
controller in case of NACK error.

Signed-off-by: Alok Chauhan <alokc-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
Added the more descriptive commit message about issue in case
of NACK error condition. Changed the comment in code also

 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 e978635..dfb850a8 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -516,6 +516,14 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
 	if (likely(i2c_dev->msg_err == I2C_ERR_NONE))
 		return 0;
 
+	/*
+	 * NACK interrupt generated before I2C controller generates the STOP
+	 * condition on bus. So wait for some time before reset the controller
+	 * so that STOP condition has delivered properly on bus.
+	 */
+	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

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

[Alok Chauhan]

Stephen,

I've updated the commit message and comment in the code as per your suggestion. Tegra I2C controller doesn't have idle bit so delay is required before reset the controller in case of NACK error.  This delay is calculated purely based on clock period of that particular i2c bus and not passed as hardcoded value. I2C SCL clock-stretching won't affect this calculated delay.

Thanks
Alok


-----Original Message-----
From: Alok Chauhan [mailto:alokc-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org] 
Sent: Monday, April 02, 2012 11:23 AM
To: swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org; khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org; ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org; w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org; Stephen Warren; olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org; bones-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org; omaplinuxkernel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org; Laxman Dewangan; linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; dgreid-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org
Cc: Alok Chauhan; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: [PATCH v2] i2c: tegra: Add delay before reset the controller

NACK interrupt generated before I2C controller generates the STOP condition on bus. In Software, because of this reset of controller is happening before I2C controller could complete STOP condition. So wait for some time before resetting the controller so that STOP condition has delivered properly on bus.

Added delay of 2 clock period before reset the controller in case of NACK error.

Signed-off-by: Alok Chauhan <alokc-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
Added the more descriptive commit message about issue in case of NACK error condition. Changed the comment in code also

 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 e978635..dfb850a8 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -516,6 +516,14 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
 	if (likely(i2c_dev->msg_err == I2C_ERR_NONE))
 		return 0;
 
+	/*
+	 * NACK interrupt generated before I2C controller generates the STOP
+	 * condition on bus. So wait for some time before reset the controller
+	 * so that STOP condition has delivered properly on bus.
+	 */
+	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

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

[Shubhrajyoti Datta]

Hi Alok,
Few / doubts /questions.


On Mon, Apr 2, 2012 at 11:29 AM, Alok Chauhan <alokc-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> Stephen,
>
> I've updated the commit message and comment in the code as per your suggestion. Tegra I2C controller doesn't have idle bit so delay is required before reset the controller in case of NACK error.  This delay is calculated purely based on clock period of that particular i2c bus and not passed as hardcoded value. I2C SCL clock-stretching won't affect this calculated delay.

Why is it clock cycle dependent?

Is it possible to poll if the stop is sent by controller ?

>
> Thanks
> Alok
>
>
> -----Original Message-----
> From: Alok Chauhan [mailto:alokc-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org]
> Sent: Monday, April 02, 2012 11:23 AM
> To: swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org; khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org; ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org; w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org; Stephen Warren; olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org; bones@secretlab.ca; omaplinuxkernel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org; Laxman Dewangan; linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; dgreid@chromium.org
> Cc: Alok Chauhan; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: [PATCH v2] i2c: tegra: Add delay before reset the controller
>
> NACK interrupt generated before I2C controller generates the STOP condition on bus. In Software, because of this reset of controller is happening before I2C controller could complete STOP condition. So wait for some time before resetting the controller so that STOP condition has delivered properly on bus.
>
> Added delay of 2 clock period
How did you calculate the 2 clock cycles?


before reset the controller in case of NACK error.
>
> Signed-off-by: Alok Chauhan <alokc-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
> Added the more descriptive commit message about issue in case of NACK error condition. Changed the comment in code also
>
>  drivers/i2c/busses/i2c-tegra.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
>

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

[Alok Chauhan]

Hi Shubhrajyoti,

> Why is it clock cycle dependent?
I think you got me wrong. What I meant to say that delay is calculated based on clock period of that bus. So in this way I don't have to hardcode the delay value.

> Is it possible to poll if the stop is sent by controller ?
No, Tegra i2c controller doesn't have any register for STOP condition completion status bit. So delay is the only option here.

Thanks
Alok

-----Original Message-----
From: Shubhrajyoti Datta [mailto:omaplinuxkernel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org] 
Sent: Monday, April 02, 2012 3:49 PM
To: Alok Chauhan
Cc: Stephen Warren; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org; ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org; w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org; Stephen Warren; olof@lixom.net; bones-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org; ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org; Laxman Dewangan; linux-tegra@vger.kernel.org; linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; dgreid-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org
Subject: Re: [PATCH v2] i2c: tegra: Add delay before reset the controller

Hi Alok,
Few / doubts /questions.


On Mon, Apr 2, 2012 at 11:29 AM, Alok Chauhan <alokc-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> Stephen,
>
> I've updated the commit message and comment in the code as per your suggestion. Tegra I2C controller doesn't have idle bit so delay is required before reset the controller in case of NACK error.  This delay is calculated purely based on clock period of that particular i2c bus and not passed as hardcoded value. I2C SCL clock-stretching won't affect this calculated delay.

Why is it clock cycle dependent?

Is it possible to poll if the stop is sent by controller ?

>
> Thanks
> Alok
>
>
> -----Original Message-----
> From: Alok Chauhan [mailto:alokc-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org]
> Sent: Monday, April 02, 2012 11:23 AM
> To: swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org; khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org; ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org; w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org; Stephen Warren; olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org; bones@secretlab.ca; omaplinuxkernel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org; Laxman Dewangan; linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; dgreid@chromium.org
> Cc: Alok Chauhan; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: [PATCH v2] i2c: tegra: Add delay before reset the controller
>
> NACK interrupt generated before I2C controller generates the STOP condition on bus. In Software, because of this reset of controller is happening before I2C controller could complete STOP condition. So wait for some time before resetting the controller so that STOP condition has delivered properly on bus.
>
> Added delay of 2 clock period
How did you calculate the 2 clock cycles?


before reset the controller in case of NACK error.
>
> Signed-off-by: Alok Chauhan <alokc-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
> Added the more descriptive commit message about issue in case of NACK error condition. Changed the comment in code also
>
>  drivers/i2c/busses/i2c-tegra.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
>

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

[Stephen Warren]

On 04/01/2012 11:53 PM, Alok Chauhan wrote:
> NACK interrupt generated before I2C controller generates
> the STOP condition on bus. In Software, because of this
> reset of controller is happening before I2C controller could
> complete STOP condition. So wait for some time before resetting
> the controller so that STOP condition has delivered properly on bus.
> 
> Added delay of 2 clock period before reset the
> controller in case of NACK error.
> 
> Signed-off-by: Alok Chauhan <alokc-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Acked-by: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>

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

[Stephen Warren]

On 04/02/2012 01:58 PM, Stephen Warren wrote:
> On 04/01/2012 11:53 PM, Alok Chauhan wrote:
>> NACK interrupt generated before I2C controller generates
>> the STOP condition on bus. In Software, because of this
>> reset of controller is happening before I2C controller could
>> complete STOP condition. So wait for some time before resetting
>> the controller so that STOP condition has delivered properly on bus.
>>
>> Added delay of 2 clock period before reset the
>> controller in case of NACK error.
>>
>> Signed-off-by: Alok Chauhan <alokc-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> Acked-by: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>

Wolfram, Ben, can this be picked up for 3.5?

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

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 880 bytes --]

On Mon, Apr 02, 2012 at 11:23:02AM +0530, Alok Chauhan wrote:
> NACK interrupt generated before I2C controller generates
> the STOP condition on bus. In Software, because of this
> reset of controller is happening before I2C controller could
> complete STOP condition. So wait for some time before resetting
> the controller so that STOP condition has delivered properly on bus.
> 
> Added delay of 2 clock period before reset the
> controller in case of NACK error.
> 
> Signed-off-by: Alok Chauhan <alokc-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Applied with Stephen's ACK and slightly reworded comment and commit msg.

Wondering a bit that you want it for 3.5, not for 3.4. Looks like a
bugfix to me.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

[Stephen Warren]

On 04/18/2012 08:27 AM, Wolfram Sang wrote:
> On Mon, Apr 02, 2012 at 11:23:02AM +0530, Alok Chauhan wrote:
>> NACK interrupt generated before I2C controller generates
>> the STOP condition on bus. In Software, because of this
>> reset of controller is happening before I2C controller could
>> complete STOP condition. So wait for some time before resetting
>> the controller so that STOP condition has delivered properly on bus.
>>
>> Added delay of 2 clock period before reset the
>> controller in case of NACK error.
>>
>> Signed-off-by: Alok Chauhan <alokc@nvidia.com>
> 
> Applied with Stephen's ACK and slightly reworded comment and commit msg.
> 
> Wondering a bit that you want it for 3.5, not for 3.4. Looks like a
> bugfix to me.

Yes, applying this to 3.4 would make sense. When I said "for 3.5"
earlier, I meant I'd like it to at least get into 3.5 not later.

Thanks.

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

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 510 bytes --]


> > Applied with Stephen's ACK and slightly reworded comment and commit msg.
> > 
> > Wondering a bit that you want it for 3.5, not for 3.4. Looks like a
> > bugfix to me.
> 
> Yes, applying this to 3.4 would make sense. When I said "for 3.5"
> earlier, I meant I'd like it to at least get into 3.5 not later.

Done, thanks for clarifying.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]