linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] i2c: tegra: Add delay before reset the controller
@ 2011-12-26 11:14 Alok Chauhan
  2012-01-06 16:18 ` Ben Dooks
  2012-02-10  9:35 ` Shubhrajyoti Datta
  0 siblings, 2 replies; 16+ messages in thread
From: Alok Chauhan @ 2011-12-26 11:14 UTC (permalink / raw)
  To: khali, ben-linux, swarren, olof, bones, paul.gortmaker, dgreid,
	ldewangan
  Cc: linux-tegra, linux-i2c, linux-kernel, Alok Chauhan

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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] i2c: tegra: Add delay before reset the controller
  2011-12-26 11:14 [PATCH v2] i2c: tegra: Add delay before reset the controller Alok Chauhan
@ 2012-01-06 16:18 ` Ben Dooks
  2012-01-10  4:12   ` Alok Chauhan
  2012-02-10  9:35 ` Shubhrajyoti Datta
  1 sibling, 1 reply; 16+ messages in thread
From: Ben Dooks @ 2012-01-06 16:18 UTC (permalink / raw)
  To: Alok Chauhan
  Cc: khali, ben-linux, swarren, olof, bones, paul.gortmaker, dgreid,
	ldewangan, linux-tegra, linux-i2c, linux-kernel

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?

  	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.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH v2] i2c: tegra: Add delay before reset the controller
  2012-01-06 16:18 ` Ben Dooks
@ 2012-01-10  4:12   ` Alok Chauhan
  2012-02-10  9:20     ` Alok Chauhan
  0 siblings, 1 reply; 16+ messages in thread
From: Alok Chauhan @ 2012-01-10  4:12 UTC (permalink / raw)
  To: Ben Dooks
  Cc: khali, ben-linux, Stephen Warren, olof, bones, paul.gortmaker,
	dgreid, Laxman Dewangan, linux-tegra, linux-i2c, linux-kernel

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.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH v2] i2c: tegra: Add delay before reset the controller
  2012-01-10  4:12   ` Alok Chauhan
@ 2012-02-10  9:20     ` Alok Chauhan
  0 siblings, 0 replies; 16+ messages in thread
From: Alok Chauhan @ 2012-02-10  9:20 UTC (permalink / raw)
  To: Alok Chauhan, Ben Dooks
  Cc: khali, ben-linux, Stephen Warren, olof, bones, paul.gortmaker,
	dgreid, Laxman Dewangan, linux-tegra, linux-i2c, linux-kernel

> 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@vger.kernel.org More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] i2c: tegra: Add delay before reset the controller
  2011-12-26 11:14 [PATCH v2] i2c: tegra: Add delay before reset the controller Alok Chauhan
  2012-01-06 16:18 ` Ben Dooks
@ 2012-02-10  9:35 ` Shubhrajyoti Datta
  2012-02-14  5:49   ` Alok Chauhan
  1 sibling, 1 reply; 16+ messages in thread
From: Shubhrajyoti Datta @ 2012-02-10  9:35 UTC (permalink / raw)
  To: Alok Chauhan
  Cc: khali, ben-linux, swarren, olof, bones, paul.gortmaker, dgreid,
	ldewangan, linux-tegra, linux-i2c, linux-kernel

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH v2] i2c: tegra: Add delay before reset the controller
  2012-02-10  9:35 ` Shubhrajyoti Datta
@ 2012-02-14  5:49   ` Alok Chauhan
  2012-03-08  6:29     ` Alok Chauhan
  0 siblings, 1 reply; 16+ messages in thread
From: Alok Chauhan @ 2012-02-14  5:49 UTC (permalink / raw)
  To: Shubhrajyoti Datta
  Cc: khali, ben-linux, Stephen Warren, olof, bones, paul.gortmaker,
	dgreid, Laxman Dewangan, linux-tegra, linux-i2c, linux-kernel

Hi Shubhrajyot,

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.
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@vger.kernel.org More majordomo 
> info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH v2] i2c: tegra: Add delay before reset the controller
  2012-02-14  5:49   ` Alok Chauhan
@ 2012-03-08  6:29     ` Alok Chauhan
  0 siblings, 0 replies; 16+ messages in thread
From: Alok Chauhan @ 2012-03-08  6:29 UTC (permalink / raw)
  To: Alok Chauhan, Shubhrajyoti Datta
  Cc: khali, ben-linux, Stephen Warren, olof, bones, paul.gortmaker,
	dgreid, Laxman Dewangan, linux-tegra, linux-i2c, linux-kernel

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@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.
>>>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@vger.kernel.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@vger.kernel.org More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] i2c: tegra: Add delay before reset the controller
  2012-04-18 20:02   ` Stephen Warren
@ 2012-04-18 20:20     ` Wolfram Sang
  0 siblings, 0 replies; 16+ messages in thread
From: Wolfram Sang @ 2012-04-18 20:20 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Alok Chauhan, khali, ben-linux, swarren, olof, bones,
	omaplinuxkernel, ccross, ldewangan, linux-tegra, linux-i2c,
	dgreid, linux-kernel

[-- 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 --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] i2c: tegra: Add delay before reset the controller
  2012-04-18 14:27 ` Wolfram Sang
@ 2012-04-18 20:02   ` Stephen Warren
  2012-04-18 20:20     ` Wolfram Sang
  0 siblings, 1 reply; 16+ messages in thread
From: Stephen Warren @ 2012-04-18 20:02 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Alok Chauhan, khali, ben-linux, swarren, olof, bones,
	omaplinuxkernel, ccross, ldewangan, linux-tegra, linux-i2c,
	dgreid, linux-kernel

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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] i2c: tegra: Add delay before reset the controller
  2012-04-02  5:53 Alok Chauhan
  2012-04-02  5:59 ` Alok Chauhan
  2012-04-02 19:58 ` Stephen Warren
@ 2012-04-18 14:27 ` Wolfram Sang
  2012-04-18 20:02   ` Stephen Warren
  2 siblings, 1 reply; 16+ messages in thread
From: Wolfram Sang @ 2012-04-18 14:27 UTC (permalink / raw)
  To: Alok Chauhan
  Cc: swarren, khali, ben-linux, swarren, olof, bones, omaplinuxkernel,
	ccross, ldewangan, linux-tegra, linux-i2c, dgreid, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 851 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@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.

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

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] i2c: tegra: Add delay before reset the controller
  2012-04-02 19:58 ` Stephen Warren
@ 2012-04-13 22:51   ` Stephen Warren
  0 siblings, 0 replies; 16+ messages in thread
From: Stephen Warren @ 2012-04-13 22:51 UTC (permalink / raw)
  To: ben-linux, w.sang
  Cc: Alok Chauhan, khali, olof, bones, omaplinuxkernel, ccross,
	ldewangan, linux-tegra, linux-i2c, dgreid, linux-kernel

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@nvidia.com>
> 
> Acked-by: Stephen Warren <swarren@wwwdotorg.org>

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] i2c: tegra: Add delay before reset the controller
  2012-04-02  5:53 Alok Chauhan
  2012-04-02  5:59 ` Alok Chauhan
@ 2012-04-02 19:58 ` Stephen Warren
  2012-04-13 22:51   ` Stephen Warren
  2012-04-18 14:27 ` Wolfram Sang
  2 siblings, 1 reply; 16+ messages in thread
From: Stephen Warren @ 2012-04-02 19:58 UTC (permalink / raw)
  To: Alok Chauhan
  Cc: khali, ben-linux, w.sang, olof, bones, omaplinuxkernel, ccross,
	ldewangan, linux-tegra, linux-i2c, dgreid, linux-kernel

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@nvidia.com>

Acked-by: Stephen Warren <swarren@wwwdotorg.org>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH v2] i2c: tegra: Add delay before reset the controller
  2012-04-02 10:19   ` Shubhrajyoti Datta
@ 2012-04-02 12:06     ` Alok Chauhan
  0 siblings, 0 replies; 16+ messages in thread
From: Alok Chauhan @ 2012-04-02 12:06 UTC (permalink / raw)
  To: Shubhrajyoti Datta
  Cc: Stephen Warren, linux-kernel, khali, ben-linux, w.sang,
	Stephen Warren, olof, bones, ccross, Laxman Dewangan,
	linux-tegra, linux-i2c, dgreid

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@gmail.com] 
Sent: Monday, April 02, 2012 3:49 PM
To: Alok Chauhan
Cc: Stephen Warren; linux-kernel@vger.kernel.org; khali@linux-fr.org; ben-linux@fluff.org; w.sang@pengutronix.de; Stephen Warren; olof@lixom.net; bones@secretlab.ca; ccross@android.com; Laxman Dewangan; linux-tegra@vger.kernel.org; linux-i2c@vger.kernel.org; dgreid@chromium.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@nvidia.com> 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@nvidia.com]
> Sent: Monday, April 02, 2012 11:23 AM
> To: swarren@wwwdotorg.org; khali@linux-fr.org; ben-linux@fluff.org; w.sang@pengutronix.de; Stephen Warren; olof@lixom.net; bones@secretlab.ca; omaplinuxkernel@gmail.com; ccross@android.com; Laxman Dewangan; linux-tegra@vger.kernel.org; linux-i2c@vger.kernel.org; dgreid@chromium.org
> Cc: Alok Chauhan; linux-kernel@vger.kernel.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@nvidia.com>
> ---
> 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(-)
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] i2c: tegra: Add delay before reset the controller
  2012-04-02  5:59 ` Alok Chauhan
@ 2012-04-02 10:19   ` Shubhrajyoti Datta
  2012-04-02 12:06     ` Alok Chauhan
  0 siblings, 1 reply; 16+ messages in thread
From: Shubhrajyoti Datta @ 2012-04-02 10:19 UTC (permalink / raw)
  To: Alok Chauhan
  Cc: Stephen Warren, linux-kernel, khali, ben-linux, w.sang,
	Stephen Warren, olof, bones, ccross, Laxman Dewangan,
	linux-tegra, linux-i2c, dgreid

Hi Alok,
Few / doubts /questions.


On Mon, Apr 2, 2012 at 11:29 AM, Alok Chauhan <alokc@nvidia.com> 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@nvidia.com]
> Sent: Monday, April 02, 2012 11:23 AM
> To: swarren@wwwdotorg.org; khali@linux-fr.org; ben-linux@fluff.org; w.sang@pengutronix.de; Stephen Warren; olof@lixom.net; bones@secretlab.ca; omaplinuxkernel@gmail.com; ccross@android.com; Laxman Dewangan; linux-tegra@vger.kernel.org; linux-i2c@vger.kernel.org; dgreid@chromium.org
> Cc: Alok Chauhan; linux-kernel@vger.kernel.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@nvidia.com>
> ---
> 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(-)
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH v2] i2c: tegra: Add delay before reset the controller
  2012-04-02  5:53 Alok Chauhan
@ 2012-04-02  5:59 ` Alok Chauhan
  2012-04-02 10:19   ` Shubhrajyoti Datta
  2012-04-02 19:58 ` Stephen Warren
  2012-04-18 14:27 ` Wolfram Sang
  2 siblings, 1 reply; 16+ messages in thread
From: Alok Chauhan @ 2012-04-02  5:59 UTC (permalink / raw)
  To: Stephen Warren
  Cc: linux-kernel, Alok Chauhan, swarren, khali, ben-linux, w.sang,
	Stephen Warren, olof, bones, omaplinuxkernel, ccross,
	Laxman Dewangan, linux-tegra, linux-i2c, dgreid

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@nvidia.com] 
Sent: Monday, April 02, 2012 11:23 AM
To: swarren@wwwdotorg.org; khali@linux-fr.org; ben-linux@fluff.org; w.sang@pengutronix.de; Stephen Warren; olof@lixom.net; bones@secretlab.ca; omaplinuxkernel@gmail.com; ccross@android.com; Laxman Dewangan; linux-tegra@vger.kernel.org; linux-i2c@vger.kernel.org; dgreid@chromium.org
Cc: Alok Chauhan; linux-kernel@vger.kernel.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@nvidia.com>
---
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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2] i2c: tegra: Add delay before reset the controller
@ 2012-04-02  5:53 Alok Chauhan
  2012-04-02  5:59 ` Alok Chauhan
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Alok Chauhan @ 2012-04-02  5:53 UTC (permalink / raw)
  To: swarren, khali, ben-linux, w.sang, swarren, olof, bones,
	omaplinuxkernel, ccross, ldewangan, linux-tegra, linux-i2c,
	dgreid
  Cc: alokc, linux-kernel

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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2012-04-18 20:21 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-26 11:14 [PATCH v2] i2c: tegra: Add delay before reset the controller Alok Chauhan
2012-01-06 16:18 ` Ben Dooks
2012-01-10  4:12   ` Alok Chauhan
2012-02-10  9:20     ` Alok Chauhan
2012-02-10  9:35 ` Shubhrajyoti Datta
2012-02-14  5:49   ` Alok Chauhan
2012-03-08  6:29     ` Alok Chauhan
2012-04-02  5:53 Alok Chauhan
2012-04-02  5:59 ` Alok Chauhan
2012-04-02 10:19   ` Shubhrajyoti Datta
2012-04-02 12:06     ` Alok Chauhan
2012-04-02 19:58 ` Stephen Warren
2012-04-13 22:51   ` Stephen Warren
2012-04-18 14:27 ` Wolfram Sang
2012-04-18 20:02   ` Stephen Warren
2012-04-18 20:20     ` Wolfram Sang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).