public inbox for linux-i2c@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH V1 1/3] i2c: imx: check busy bit when START/STOP
@ 2009-10-17  9:46 Richard Zhao
       [not found] ` <1255772784-11387-1-git-send-email-linuxzsc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Richard Zhao @ 2009-10-17  9:46 UTC (permalink / raw)
  To: kernel-bIcnvbaLZ9MEGnE8C9+IrQ
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA, Richard Zhao

The controller can't do anything else before it actually generates START/STOP.
So we check busy bit to make sure START/STOP is successfully finished.

If we don't check busy bit, START/STOP may fail on some fast CPUs.

Signed-off-by: Richard Zhao <linuxzsc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 4afba3e..6055e92 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -120,19 +120,25 @@ struct imx_i2c_struct {
 	wait_queue_head_t	queue;
 	unsigned long		i2csr;
 	unsigned int 		disable_delay;
+	int			stopped;
 };
 
 /** Functions for IMX I2C adapter driver ***************************************
 *******************************************************************************/
 
-static int i2c_imx_bus_busy(struct imx_i2c_struct *i2c_imx)
+static int i2c_imx_bus_busy(struct imx_i2c_struct *i2c_imx, int for_busy)
 {
 	unsigned long orig_jiffies = jiffies;
+	unsigned int temp;
 
 	dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
 
-	/* wait for bus not busy */
-	while (readb(i2c_imx->base + IMX_I2C_I2SR) & I2SR_IBB) {
+	while (1) {
+		temp = readb(i2c_imx->base + IMX_I2C_I2SR);
+		if (for_busy && (temp & I2SR_IBB))
+			break;
+		if (!for_busy && !(temp & I2SR_IBB))
+			break;
 		if (signal_pending(current)) {
 			dev_dbg(&i2c_imx->adapter.dev,
 				"<%s> I2C Interrupted\n", __func__);
@@ -179,39 +185,55 @@ static int i2c_imx_acked(struct imx_i2c_struct *i2c_imx)
 	return 0;
 }
 
-static void i2c_imx_start(struct imx_i2c_struct *i2c_imx)
+static int i2c_imx_start(struct imx_i2c_struct *i2c_imx)
 {
 	unsigned int temp = 0;
+	int result;
 
 	dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
 
 	/* Enable I2C controller */
+	writeb(0, i2c_imx->base + IMX_I2C_I2SR);
 	writeb(I2CR_IEN, i2c_imx->base + IMX_I2C_I2CR);
+
+	/* Wait controller to be stable */
+	udelay(50);
+
 	/* Start I2C transaction */
 	temp = readb(i2c_imx->base + IMX_I2C_I2CR);
 	temp |= I2CR_MSTA;
 	writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
+	result = i2c_imx_bus_busy(i2c_imx, 1);
+	if (result)
+		return result;
+	i2c_imx->stopped = 0;
+
 	temp |= I2CR_IIEN | I2CR_MTX | I2CR_TXAK;
 	writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
+	return result;
 }
 
 static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx)
 {
 	unsigned int temp = 0;
 
-	/* Stop I2C transaction */
-	dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
-	temp = readb(i2c_imx->base + IMX_I2C_I2CR);
-	temp &= ~I2CR_MSTA;
-	writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
-	/* setup chip registers to defaults */
-	writeb(I2CR_IEN, i2c_imx->base + IMX_I2C_I2CR);
-	writeb(0, i2c_imx->base + IMX_I2C_I2SR);
+	if (!i2c_imx->stopped) {
+		/* Stop I2C transaction */
+		dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
+		temp = readb(i2c_imx->base + IMX_I2C_I2CR);
+		temp &= ~(I2CR_MSTA | I2CR_MTX);
+		writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
+		i2c_imx->stopped = 1;
+	}
 	/*
 	 * This delay caused by an i.MXL hardware bug.
 	 * If no (or too short) delay, no "STOP" bit will be generated.
 	 */
 	udelay(i2c_imx->disable_delay);
+
+	if (!i2c_imx->stopped)
+		i2c_imx_bus_busy(i2c_imx, 0);
+
 	/* Disable I2C controller */
 	writeb(0, i2c_imx->base + IMX_I2C_I2CR);
 }
@@ -341,11 +363,15 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
 		if (result)
 			return result;
 		if (i == (msgs->len - 1)) {
+			/* It must generate STOP before read I2DR to prevent
+			   controller from generating another clock cycle */
 			dev_dbg(&i2c_imx->adapter.dev,
 				"<%s> clear MSTA\n", __func__);
 			temp = readb(i2c_imx->base + IMX_I2C_I2CR);
-			temp &= ~I2CR_MSTA;
+			temp &= ~(I2CR_MSTA | I2CR_MTX);
 			writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
+			i2c_imx_bus_busy(i2c_imx, 0);
+			i2c_imx->stopped = 1;
 		} else if (i == (msgs->len - 2)) {
 			dev_dbg(&i2c_imx->adapter.dev,
 				"<%s> set TXAK\n", __func__);
@@ -370,14 +396,11 @@ static int i2c_imx_xfer(struct i2c_adapter *adapter,
 
 	dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
 
-	/* Check if i2c bus is not busy */
-	result = i2c_imx_bus_busy(i2c_imx);
+	/* Start I2C transfer */
+	result = i2c_imx_start(i2c_imx);
 	if (result)
 		goto fail0;
 
-	/* Start I2C transfer */
-	i2c_imx_start(i2c_imx);
-
 	/* read/write data */
 	for (i = 0; i < num; i++) {
 		if (i) {
@@ -386,6 +409,9 @@ static int i2c_imx_xfer(struct i2c_adapter *adapter,
 			temp = readb(i2c_imx->base + IMX_I2C_I2CR);
 			temp |= I2CR_RSTA;
 			writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
+			result =  i2c_imx_bus_busy(i2c_imx, 1);
+			if (result)
+				goto fail0;
 		}
 		dev_dbg(&i2c_imx->adapter.dev,
 			"<%s> transfer message: %d\n", __func__, i);
-- 
1.6.0.4

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

* [PATCH V1 2/3] i2c: imx: only imx1 needs disable delay
       [not found] ` <1255772784-11387-1-git-send-email-linuxzsc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2009-10-17  9:46   ` Richard Zhao
       [not found]     ` <1255772784-11387-2-git-send-email-linuxzsc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2009-10-20 10:03   ` [PATCH V1 1/3] i2c: imx: check busy bit when START/STOP Richard Zhao
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Richard Zhao @ 2009-10-17  9:46 UTC (permalink / raw)
  To: kernel-bIcnvbaLZ9MEGnE8C9+IrQ
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA, Richard Zhao

check cpu_is_mx1() when disable delay.

Signed-off-by: Richard Zhao <linuxzsc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 6055e92..671d37c 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -225,11 +225,13 @@ static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx)
 		writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
 		i2c_imx->stopped = 1;
 	}
-	/*
-	 * This delay caused by an i.MXL hardware bug.
-	 * If no (or too short) delay, no "STOP" bit will be generated.
-	 */
-	udelay(i2c_imx->disable_delay);
+	if (cpu_is_mx1()) {
+		/*
+		 * This delay caused by an i.MXL hardware bug.
+		 * If no (or too short) delay, no "STOP" bit will be generated.
+		 */
+		udelay(i2c_imx->disable_delay);
+	}
 
 	if (!i2c_imx->stopped)
 		i2c_imx_bus_busy(i2c_imx, 0);
-- 
1.6.0.4

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

* [PATCH V1 3/3] i2c: imx: disable clock when it's possible to save power.
       [not found]     ` <1255772784-11387-2-git-send-email-linuxzsc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2009-10-17  9:46       ` Richard Zhao
  0 siblings, 0 replies; 8+ messages in thread
From: Richard Zhao @ 2009-10-17  9:46 UTC (permalink / raw)
  To: kernel-bIcnvbaLZ9MEGnE8C9+IrQ
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA, Richard Zhao

Enable clock before START, disable it after STOP.

Signed-off-by: Richard Zhao <linuxzsc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 671d37c..e3654d6 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -121,6 +121,7 @@ struct imx_i2c_struct {
 	unsigned long		i2csr;
 	unsigned int 		disable_delay;
 	int			stopped;
+	unsigned int		ifdr; /* IMX_I2C_IFDR */
 };
 
 /** Functions for IMX I2C adapter driver ***************************************
@@ -192,6 +193,8 @@ static int i2c_imx_start(struct imx_i2c_struct *i2c_imx)
 
 	dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
 
+	clk_enable(i2c_imx->clk);
+	writeb(i2c_imx->ifdr, i2c_imx->base + IMX_I2C_IFDR);
 	/* Enable I2C controller */
 	writeb(0, i2c_imx->base + IMX_I2C_I2SR);
 	writeb(I2CR_IEN, i2c_imx->base + IMX_I2C_I2CR);
@@ -238,6 +241,7 @@ static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx)
 
 	/* Disable I2C controller */
 	writeb(0, i2c_imx->base + IMX_I2C_I2CR);
+	clk_disable(i2c_imx->clk);
 }
 
 static void __init i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx,
@@ -257,8 +261,8 @@ static void __init i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx,
 	else
 		for (i = 0; i2c_clk_div[i][0] < div; i++);
 
-	/* Write divider value to register */
-	writeb(i2c_clk_div[i][1], i2c_imx->base + IMX_I2C_IFDR);
+	/* Store divider value */
+	i2c_imx->ifdr = i2c_clk_div[i][1];
 
 	/*
 	 * There dummy delay is calculated.
@@ -528,7 +532,6 @@ static int __init i2c_imx_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "can't get I2C clock\n");
 		goto fail3;
 	}
-	clk_enable(i2c_imx->clk);
 
 	/* Request IRQ */
 	ret = request_irq(i2c_imx->irq, i2c_imx_isr, 0, pdev->name, i2c_imx);
@@ -577,7 +580,6 @@ static int __init i2c_imx_probe(struct platform_device *pdev)
 fail5:
 	free_irq(i2c_imx->irq, i2c_imx);
 fail4:
-	clk_disable(i2c_imx->clk);
 	clk_put(i2c_imx->clk);
 fail3:
 	release_mem_region(i2c_imx->res->start, resource_size(res));
@@ -614,8 +616,6 @@ static int __exit i2c_imx_remove(struct platform_device *pdev)
 	if (pdata && pdata->exit)
 		pdata->exit(&pdev->dev);
 
-	/* Disable I2C clock */
-	clk_disable(i2c_imx->clk);
 	clk_put(i2c_imx->clk);
 
 	release_mem_region(i2c_imx->res->start, resource_size(i2c_imx->res));
-- 
1.6.0.4

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

* Re: [PATCH V1 1/3] i2c: imx: check busy bit when START/STOP
       [not found] ` <1255772784-11387-1-git-send-email-linuxzsc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2009-10-17  9:46   ` [PATCH V1 2/3] i2c: imx: only imx1 needs disable delay Richard Zhao
@ 2009-10-20 10:03   ` Richard Zhao
  2009-10-21 14:11   ` Richard Zhao
  2009-10-30  0:40   ` Ben Dooks
  3 siblings, 0 replies; 8+ messages in thread
From: Richard Zhao @ 2009-10-20 10:03 UTC (permalink / raw)
  To: kernel-bIcnvbaLZ9MEGnE8C9+IrQ, Sascha Hauer
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA, Richard Zhao

On Sat, Oct 17, 2009 at 5:46 PM, Richard Zhao <linuxzsc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> The controller can't do anything else before it actually generates START/STOP.
> So we check busy bit to make sure START/STOP is successfully finished.
>
> If we don't check busy bit, START/STOP may fail on some fast CPUs.
>
> Signed-off-by: Richard Zhao <linuxzsc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> index 4afba3e..6055e92 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -120,19 +120,25 @@ struct imx_i2c_struct {
>        wait_queue_head_t       queue;
>        unsigned long           i2csr;
>        unsigned int            disable_delay;
> +       int                     stopped;
>  };
>
>  /** Functions for IMX I2C adapter driver ***************************************
>  *******************************************************************************/
>
> -static int i2c_imx_bus_busy(struct imx_i2c_struct *i2c_imx)
> +static int i2c_imx_bus_busy(struct imx_i2c_struct *i2c_imx, int for_busy)
>  {
>        unsigned long orig_jiffies = jiffies;
> +       unsigned int temp;
>
>        dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
>
> -       /* wait for bus not busy */
> -       while (readb(i2c_imx->base + IMX_I2C_I2SR) & I2SR_IBB) {
> +       while (1) {
> +               temp = readb(i2c_imx->base + IMX_I2C_I2SR);
> +               if (for_busy && (temp & I2SR_IBB))
> +                       break;
> +               if (!for_busy && !(temp & I2SR_IBB))
> +                       break;
>                if (signal_pending(current)) {
>                        dev_dbg(&i2c_imx->adapter.dev,
>                                "<%s> I2C Interrupted\n", __func__);
> @@ -179,39 +185,55 @@ static int i2c_imx_acked(struct imx_i2c_struct *i2c_imx)
>        return 0;
>  }
>
> -static void i2c_imx_start(struct imx_i2c_struct *i2c_imx)
> +static int i2c_imx_start(struct imx_i2c_struct *i2c_imx)
>  {
>        unsigned int temp = 0;
> +       int result;
>
>        dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
>
>        /* Enable I2C controller */
> +       writeb(0, i2c_imx->base + IMX_I2C_I2SR);
>        writeb(I2CR_IEN, i2c_imx->base + IMX_I2C_I2CR);
> +
> +       /* Wait controller to be stable */
> +       udelay(50);
> +
>        /* Start I2C transaction */
>        temp = readb(i2c_imx->base + IMX_I2C_I2CR);
>        temp |= I2CR_MSTA;
>        writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
> +       result = i2c_imx_bus_busy(i2c_imx, 1);
> +       if (result)
> +               return result;
> +       i2c_imx->stopped = 0;
> +
>        temp |= I2CR_IIEN | I2CR_MTX | I2CR_TXAK;
>        writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
> +       return result;
>  }
>
>  static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx)
>  {
>        unsigned int temp = 0;
>
> -       /* Stop I2C transaction */
> -       dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
> -       temp = readb(i2c_imx->base + IMX_I2C_I2CR);
> -       temp &= ~I2CR_MSTA;
> -       writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
> -       /* setup chip registers to defaults */
> -       writeb(I2CR_IEN, i2c_imx->base + IMX_I2C_I2CR);
> -       writeb(0, i2c_imx->base + IMX_I2C_I2SR);
> +       if (!i2c_imx->stopped) {
> +               /* Stop I2C transaction */
> +               dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
> +               temp = readb(i2c_imx->base + IMX_I2C_I2CR);
> +               temp &= ~(I2CR_MSTA | I2CR_MTX);
> +               writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
> +               i2c_imx->stopped = 1;
> +       }
>        /*
>         * This delay caused by an i.MXL hardware bug.
>         * If no (or too short) delay, no "STOP" bit will be generated.
>         */
>        udelay(i2c_imx->disable_delay);
> +
> +       if (!i2c_imx->stopped)
> +               i2c_imx_bus_busy(i2c_imx, 0);
> +
>        /* Disable I2C controller */
>        writeb(0, i2c_imx->base + IMX_I2C_I2CR);
>  }
> @@ -341,11 +363,15 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
>                if (result)
>                        return result;
>                if (i == (msgs->len - 1)) {
> +                       /* It must generate STOP before read I2DR to prevent
> +                          controller from generating another clock cycle */
>                        dev_dbg(&i2c_imx->adapter.dev,
>                                "<%s> clear MSTA\n", __func__);
>                        temp = readb(i2c_imx->base + IMX_I2C_I2CR);
> -                       temp &= ~I2CR_MSTA;
> +                       temp &= ~(I2CR_MSTA | I2CR_MTX);
>                        writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
> +                       i2c_imx_bus_busy(i2c_imx, 0);
> +                       i2c_imx->stopped = 1;
>                } else if (i == (msgs->len - 2)) {
>                        dev_dbg(&i2c_imx->adapter.dev,
>                                "<%s> set TXAK\n", __func__);
> @@ -370,14 +396,11 @@ static int i2c_imx_xfer(struct i2c_adapter *adapter,
>
>        dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
>
> -       /* Check if i2c bus is not busy */
> -       result = i2c_imx_bus_busy(i2c_imx);
> +       /* Start I2C transfer */
> +       result = i2c_imx_start(i2c_imx);
>        if (result)
>                goto fail0;
>
> -       /* Start I2C transfer */
> -       i2c_imx_start(i2c_imx);
> -
>        /* read/write data */
>        for (i = 0; i < num; i++) {
>                if (i) {
> @@ -386,6 +409,9 @@ static int i2c_imx_xfer(struct i2c_adapter *adapter,
>                        temp = readb(i2c_imx->base + IMX_I2C_I2CR);
>                        temp |= I2CR_RSTA;
>                        writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
> +                       result =  i2c_imx_bus_busy(i2c_imx, 1);
> +                       if (result)
> +                               goto fail0;
>                }
>                dev_dbg(&i2c_imx->adapter.dev,
>                        "<%s> transfer message: %d\n", __func__, i);
> --
> 1.6.0.4
>
>
Hello Sascha,

Could you please review this series of patch?

Thanks
Richard

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

* Re: [PATCH V1 1/3] i2c: imx: check busy bit when START/STOP
       [not found] ` <1255772784-11387-1-git-send-email-linuxzsc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2009-10-17  9:46   ` [PATCH V1 2/3] i2c: imx: only imx1 needs disable delay Richard Zhao
  2009-10-20 10:03   ` [PATCH V1 1/3] i2c: imx: check busy bit when START/STOP Richard Zhao
@ 2009-10-21 14:11   ` Richard Zhao
  2009-10-30  0:40   ` Ben Dooks
  3 siblings, 0 replies; 8+ messages in thread
From: Richard Zhao @ 2009-10-21 14:11 UTC (permalink / raw)
  To: kernel-bIcnvbaLZ9MEGnE8C9+IrQ, Ben Dooks (embedded platforms)
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA, Richard Zhao

On Sat, Oct 17, 2009 at 5:46 PM, Richard Zhao <linuxzsc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> The controller can't do anything else before it actually generates START/STOP.
> So we check busy bit to make sure START/STOP is successfully finished.
>
> If we don't check busy bit, START/STOP may fail on some fast CPUs.
>
> Signed-off-by: Richard Zhao <linuxzsc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> index 4afba3e..6055e92 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -120,19 +120,25 @@ struct imx_i2c_struct {
>        wait_queue_head_t       queue;
>        unsigned long           i2csr;
>        unsigned int            disable_delay;
> +       int                     stopped;
>  };
>
>  /** Functions for IMX I2C adapter driver ***************************************
>  *******************************************************************************/
>
> -static int i2c_imx_bus_busy(struct imx_i2c_struct *i2c_imx)
> +static int i2c_imx_bus_busy(struct imx_i2c_struct *i2c_imx, int for_busy)
>  {
>        unsigned long orig_jiffies = jiffies;
> +       unsigned int temp;
>
>        dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
>
> -       /* wait for bus not busy */
> -       while (readb(i2c_imx->base + IMX_I2C_I2SR) & I2SR_IBB) {
> +       while (1) {
> +               temp = readb(i2c_imx->base + IMX_I2C_I2SR);
> +               if (for_busy && (temp & I2SR_IBB))
> +                       break;
> +               if (!for_busy && !(temp & I2SR_IBB))
> +                       break;
>                if (signal_pending(current)) {
>                        dev_dbg(&i2c_imx->adapter.dev,
>                                "<%s> I2C Interrupted\n", __func__);
> @@ -179,39 +185,55 @@ static int i2c_imx_acked(struct imx_i2c_struct *i2c_imx)
>        return 0;
>  }
>
> -static void i2c_imx_start(struct imx_i2c_struct *i2c_imx)
> +static int i2c_imx_start(struct imx_i2c_struct *i2c_imx)
>  {
>        unsigned int temp = 0;
> +       int result;
>
>        dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
>
>        /* Enable I2C controller */
> +       writeb(0, i2c_imx->base + IMX_I2C_I2SR);
>        writeb(I2CR_IEN, i2c_imx->base + IMX_I2C_I2CR);
> +
> +       /* Wait controller to be stable */
> +       udelay(50);
> +
>        /* Start I2C transaction */
>        temp = readb(i2c_imx->base + IMX_I2C_I2CR);
>        temp |= I2CR_MSTA;
>        writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
> +       result = i2c_imx_bus_busy(i2c_imx, 1);
> +       if (result)
> +               return result;
> +       i2c_imx->stopped = 0;
> +
>        temp |= I2CR_IIEN | I2CR_MTX | I2CR_TXAK;
>        writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
> +       return result;
>  }
>
>  static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx)
>  {
>        unsigned int temp = 0;
>
> -       /* Stop I2C transaction */
> -       dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
> -       temp = readb(i2c_imx->base + IMX_I2C_I2CR);
> -       temp &= ~I2CR_MSTA;
> -       writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
> -       /* setup chip registers to defaults */
> -       writeb(I2CR_IEN, i2c_imx->base + IMX_I2C_I2CR);
> -       writeb(0, i2c_imx->base + IMX_I2C_I2SR);
> +       if (!i2c_imx->stopped) {
> +               /* Stop I2C transaction */
> +               dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
> +               temp = readb(i2c_imx->base + IMX_I2C_I2CR);
> +               temp &= ~(I2CR_MSTA | I2CR_MTX);
> +               writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
> +               i2c_imx->stopped = 1;
> +       }
>        /*
>         * This delay caused by an i.MXL hardware bug.
>         * If no (or too short) delay, no "STOP" bit will be generated.
>         */
>        udelay(i2c_imx->disable_delay);
> +
> +       if (!i2c_imx->stopped)
> +               i2c_imx_bus_busy(i2c_imx, 0);
> +
>        /* Disable I2C controller */
>        writeb(0, i2c_imx->base + IMX_I2C_I2CR);
>  }
> @@ -341,11 +363,15 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
>                if (result)
>                        return result;
>                if (i == (msgs->len - 1)) {
> +                       /* It must generate STOP before read I2DR to prevent
> +                          controller from generating another clock cycle */
>                        dev_dbg(&i2c_imx->adapter.dev,
>                                "<%s> clear MSTA\n", __func__);
>                        temp = readb(i2c_imx->base + IMX_I2C_I2CR);
> -                       temp &= ~I2CR_MSTA;
> +                       temp &= ~(I2CR_MSTA | I2CR_MTX);
>                        writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
> +                       i2c_imx_bus_busy(i2c_imx, 0);
> +                       i2c_imx->stopped = 1;
>                } else if (i == (msgs->len - 2)) {
>                        dev_dbg(&i2c_imx->adapter.dev,
>                                "<%s> set TXAK\n", __func__);
> @@ -370,14 +396,11 @@ static int i2c_imx_xfer(struct i2c_adapter *adapter,
>
>        dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
>
> -       /* Check if i2c bus is not busy */
> -       result = i2c_imx_bus_busy(i2c_imx);
> +       /* Start I2C transfer */
> +       result = i2c_imx_start(i2c_imx);
>        if (result)
>                goto fail0;
>
> -       /* Start I2C transfer */
> -       i2c_imx_start(i2c_imx);
> -
>        /* read/write data */
>        for (i = 0; i < num; i++) {
>                if (i) {
> @@ -386,6 +409,9 @@ static int i2c_imx_xfer(struct i2c_adapter *adapter,
>                        temp = readb(i2c_imx->base + IMX_I2C_I2CR);
>                        temp |= I2CR_RSTA;
>                        writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
> +                       result =  i2c_imx_bus_busy(i2c_imx, 1);
> +                       if (result)
> +                               goto fail0;
>                }
>                dev_dbg(&i2c_imx->adapter.dev,
>                        "<%s> transfer message: %d\n", __func__, i);
> --
> 1.6.0.4
>
>

Adding Ben ...

Thanks
Richard

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

* Re: [PATCH V1 1/3] i2c: imx: check busy bit when START/STOP
       [not found] ` <1255772784-11387-1-git-send-email-linuxzsc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
                     ` (2 preceding siblings ...)
  2009-10-21 14:11   ` Richard Zhao
@ 2009-10-30  0:40   ` Ben Dooks
       [not found]     ` <20091030004025.GG13398-elnMNo+KYs3pIgCt6eIbzw@public.gmane.org>
  3 siblings, 1 reply; 8+ messages in thread
From: Ben Dooks @ 2009-10-30  0:40 UTC (permalink / raw)
  To: Richard Zhao
  Cc: kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA

Does anyone have any comments on these, or shall I look at merging
them as fixes?

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

  'a smiley only costs 4 bytes'

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

* Re: [PATCH V1 1/3] i2c: imx: check busy bit when START/STOP
       [not found]     ` <20091030004025.GG13398-elnMNo+KYs3pIgCt6eIbzw@public.gmane.org>
@ 2009-10-31  2:47       ` Richard Zhao
       [not found]         ` <4e090d470910301947y2cd4e95x88f500c6c008d97d-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Richard Zhao @ 2009-10-31  2:47 UTC (permalink / raw)
  To: Ben Dooks
  Cc: kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA

On Fri, Oct 30, 2009 at 8:40 AM, Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org> wrote:
> Does anyone have any comments on these, or shall I look at merging
> them as fixes?
>
> --
> Ben (ben-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org, http://www.fluff.org/)
>
>  'a smiley only costs 4 bytes'
>

Thanks, Ben! I've test these patch on imx31 pdk and imx51 pdk.

  Richard

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

* Re: [PATCH V1 1/3] i2c: imx: check busy bit when START/STOP
       [not found]         ` <4e090d470910301947y2cd4e95x88f500c6c008d97d-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2009-11-10  2:55           ` Richard Zhao
  0 siblings, 0 replies; 8+ messages in thread
From: Richard Zhao @ 2009-11-10  2:55 UTC (permalink / raw)
  To: Ben Dooks
  Cc: kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA

On Sat, Oct 31, 2009 at 10:47 AM, Richard Zhao <linuxzsc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Fri, Oct 30, 2009 at 8:40 AM, Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org> wrote:
>> Does anyone have any comments on these, or shall I look at merging
>> them as fixes?
>>
>> --
>> Ben (ben-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org, http://www.fluff.org/)
>>
>>  'a smiley only costs 4 bytes'
>>
>
> Thanks, Ben! I've test these patch on imx31 pdk and imx51 pdk.
>
>  Richard
>
Hi Guys,

May I know what the series of patch is pending on?

Thanks
Richard

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

end of thread, other threads:[~2009-11-10  2:55 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-10-17  9:46 [PATCH V1 1/3] i2c: imx: check busy bit when START/STOP Richard Zhao
     [not found] ` <1255772784-11387-1-git-send-email-linuxzsc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2009-10-17  9:46   ` [PATCH V1 2/3] i2c: imx: only imx1 needs disable delay Richard Zhao
     [not found]     ` <1255772784-11387-2-git-send-email-linuxzsc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2009-10-17  9:46       ` [PATCH V1 3/3] i2c: imx: disable clock when it's possible to save power Richard Zhao
2009-10-20 10:03   ` [PATCH V1 1/3] i2c: imx: check busy bit when START/STOP Richard Zhao
2009-10-21 14:11   ` Richard Zhao
2009-10-30  0:40   ` Ben Dooks
     [not found]     ` <20091030004025.GG13398-elnMNo+KYs3pIgCt6eIbzw@public.gmane.org>
2009-10-31  2:47       ` Richard Zhao
     [not found]         ` <4e090d470910301947y2cd4e95x88f500c6c008d97d-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2009-11-10  2:55           ` Richard Zhao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox