[PATCH] i2c-mpc: work around missing-9th-clock-pulse bug

[PATCH] i2c-mpc: work around missing-9th-clock-pulse bug

[Domen Puncer]

Work around a problem reported on:
http://ozlabs.org/pipermail/linuxppc-embedded/2005-July/019038.html
Without this patch I2C on mpc5200 becomes unusable after a while.
Tested on mpc5200 based boards by Matthias and me.


Signed-off-by: Domen Puncer <domen.puncer@telargo.com>

---
 drivers/i2c/busses/i2c-mpc.c |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Index: work-powerpc.git/drivers/i2c/busses/i2c-mpc.c
===================================================================
--- work-powerpc.git.orig/drivers/i2c/busses/i2c-mpc.c
+++ work-powerpc.git/drivers/i2c/busses/i2c-mpc.c
@@ -74,6 +74,20 @@ static irqreturn_t mpc_i2c_isr(int irq, 
 	return IRQ_HANDLED;
 }
 
+static void mpc_i2c_fixup(struct mpc_i2c *i2c)
+{
+	writeccr(i2c, 0);
+	udelay(30);
+	writeccr(i2c, CCR_MEN);
+	udelay(30);
+	writeccr(i2c, CCR_MSTA | CCR_MTX);
+	udelay(30);
+	writeccr(i2c, CCR_MSTA | CCR_MTX | CCR_MEN);
+	udelay(30);
+	writeccr(i2c, CCR_MEN);
+	udelay(30);
+}
+
 static int i2c_wait(struct mpc_i2c *i2c, unsigned timeout, int writing)
 {
 	unsigned long orig_jiffies = jiffies;
@@ -153,6 +167,9 @@ static void mpc_i2c_start(struct mpc_i2c
 static void mpc_i2c_stop(struct mpc_i2c *i2c)
 {
 	writeccr(i2c, CCR_MEN);
+	mb();
+	writeccr(i2c, 0);
+	mb();
 }
 
 static int mpc_write(struct mpc_i2c *i2c, int target,
@@ -245,6 +262,8 @@ static int mpc_xfer(struct i2c_adapter *
 		}
 		if (time_after(jiffies, orig_jiffies + HZ)) {
 			pr_debug("I2C: timeout\n");
+			if (readb(i2c->base + MPC_I2C_SR) == (CSR_MCF | CSR_MBB | CSR_RXAK))
+				mpc_i2c_fixup(i2c);
 			return -EIO;
 		}
 		schedule();

Re: [PATCH] i2c-mpc: work around missing-9th-clock-pulse bug

[Grant Likely]

On 7/9/07, Domen Puncer <domen.puncer@telargo.com> wrote:
> Work around a problem reported on:
> http://ozlabs.org/pipermail/linuxppc-embedded/2005-July/019038.html
> Without this patch I2C on mpc5200 becomes unusable after a while.
> Tested on mpc5200 based boards by Matthias and me.
>
> Index: work-powerpc.git/drivers/i2c/busses/i2c-mpc.c
> ===================================================================
> --- work-powerpc.git.orig/drivers/i2c/busses/i2c-mpc.c
> +++ work-powerpc.git/drivers/i2c/busses/i2c-mpc.c
> @@ -153,6 +167,9 @@ static void mpc_i2c_start(struct mpc_i2c
>  static void mpc_i2c_stop(struct mpc_i2c *i2c)
>  {
>         writeccr(i2c, CCR_MEN);
> +       mb();
> +       writeccr(i2c, 0);
> +       mb();
>  }

Are the mb() calls necessary?  The writeccr path includes eieio; is
that not sufficient?

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

Re: [PATCH] i2c-mpc: work around missing-9th-clock-pulse bug

[Domen Puncer]

On 09/07/07 09:53 -0600, Grant Likely wrote:
> On 7/9/07, Domen Puncer <domen.puncer@telargo.com> wrote:
> >Work around a problem reported on:
> >http://ozlabs.org/pipermail/linuxppc-embedded/2005-July/019038.html
> >Without this patch I2C on mpc5200 becomes unusable after a while.
> >Tested on mpc5200 based boards by Matthias and me.
> >
> >Index: work-powerpc.git/drivers/i2c/busses/i2c-mpc.c
> >===================================================================
> >--- work-powerpc.git.orig/drivers/i2c/busses/i2c-mpc.c
> >+++ work-powerpc.git/drivers/i2c/busses/i2c-mpc.c
> >@@ -153,6 +167,9 @@ static void mpc_i2c_start(struct mpc_i2c
> > static void mpc_i2c_stop(struct mpc_i2c *i2c)
> > {
> >        writeccr(i2c, CCR_MEN);
> >+       mb();
> >+       writeccr(i2c, 0);
> >+       mb();
> > }
> 
> Are the mb() calls necessary?  The writeccr path includes eieio; is
> that not sufficient?

Right, v2:


Work around a problem reported on:
http://ozlabs.org/pipermail/linuxppc-embedded/2005-July/019038.html
Without this patch I2C on mpc5200 becomes unusable after a while.
Tested on mpc5200 boards by Matthias and me.


Signed-off-by: Domen Puncer <domen.puncer@telargo.com>

---
 drivers/i2c/busses/i2c-mpc.c |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Index: work-powerpc.git/drivers/i2c/busses/i2c-mpc.c
===================================================================
--- work-powerpc.git.orig/drivers/i2c/busses/i2c-mpc.c
+++ work-powerpc.git/drivers/i2c/busses/i2c-mpc.c
@@ -74,6 +74,20 @@ static irqreturn_t mpc_i2c_isr(int irq, 
 	return IRQ_HANDLED;
 }
 
+static void mpc_i2c_fixup(struct mpc_i2c *i2c)
+{
+	writeccr(i2c, 0);
+	udelay(30);
+	writeccr(i2c, CCR_MEN);
+	udelay(30);
+	writeccr(i2c, CCR_MSTA | CCR_MTX);
+	udelay(30);
+	writeccr(i2c, CCR_MSTA | CCR_MTX | CCR_MEN);
+	udelay(30);
+	writeccr(i2c, CCR_MEN);
+	udelay(30);
+}
+
 static int i2c_wait(struct mpc_i2c *i2c, unsigned timeout, int writing)
 {
 	unsigned long orig_jiffies = jiffies;
@@ -153,6 +167,7 @@ static void mpc_i2c_start(struct mpc_i2c
 static void mpc_i2c_stop(struct mpc_i2c *i2c)
 {
 	writeccr(i2c, CCR_MEN);
+	writeccr(i2c, 0);
 }
 
 static int mpc_write(struct mpc_i2c *i2c, int target,
@@ -245,6 +260,8 @@ static int mpc_xfer(struct i2c_adapter *
 		}
 		if (time_after(jiffies, orig_jiffies + HZ)) {
 			pr_debug("I2C: timeout\n");
+			if (readb(i2c->base + MPC_I2C_SR) == (CSR_MCF | CSR_MBB | CSR_RXAK))
+				mpc_i2c_fixup(i2c);
 			return -EIO;
 		}
 		schedule();

Re: [PATCH] i2c-mpc: work around missing-9th-clock-pulse bug

[Grant Likely]

On 7/10/07, Domen Puncer <domen.puncer@telargo.com> wrote:
> On 09/07/07 09:53 -0600, Grant Likely wrote:
> > On 7/9/07, Domen Puncer <domen.puncer@telargo.com> wrote:
> > >Work around a problem reported on:
> > >http://ozlabs.org/pipermail/linuxppc-embedded/2005-July/019038.html
> > >Without this patch I2C on mpc5200 becomes unusable after a while.
> > >Tested on mpc5200 based boards by Matthias and me.
> > >
> > >Index: work-powerpc.git/drivers/i2c/busses/i2c-mpc.c
> > >===================================================================
> > >--- work-powerpc.git.orig/drivers/i2c/busses/i2c-mpc.c
> > >+++ work-powerpc.git/drivers/i2c/busses/i2c-mpc.c
> > >@@ -153,6 +167,9 @@ static void mpc_i2c_start(struct mpc_i2c
> > > static void mpc_i2c_stop(struct mpc_i2c *i2c)
> > > {
> > >        writeccr(i2c, CCR_MEN);
> > >+       mb();
> > >+       writeccr(i2c, 0);
> > >+       mb();
> > > }
> >
> > Are the mb() calls necessary?  The writeccr path includes eieio; is
> > that not sufficient?
>
> Right, v2:
>
>
> Work around a problem reported on:
> http://ozlabs.org/pipermail/linuxppc-embedded/2005-July/019038.html
> Without this patch I2C on mpc5200 becomes unusable after a while.
> Tested on mpc5200 boards by Matthias and me.
>
>
> Signed-off-by: Domen Puncer <domen.puncer@telargo.com>

Looks good to me,

Acked-by: Grant Likely <grant.likely@secretlab.ca>

>
> ---
>  drivers/i2c/busses/i2c-mpc.c |   17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> Index: work-powerpc.git/drivers/i2c/busses/i2c-mpc.c
> ===================================================================
> --- work-powerpc.git.orig/drivers/i2c/busses/i2c-mpc.c
> +++ work-powerpc.git/drivers/i2c/busses/i2c-mpc.c
> @@ -74,6 +74,20 @@ static irqreturn_t mpc_i2c_isr(int irq,
>         return IRQ_HANDLED;
>  }
>
> +static void mpc_i2c_fixup(struct mpc_i2c *i2c)
> +{
> +       writeccr(i2c, 0);
> +       udelay(30);
> +       writeccr(i2c, CCR_MEN);
> +       udelay(30);
> +       writeccr(i2c, CCR_MSTA | CCR_MTX);
> +       udelay(30);
> +       writeccr(i2c, CCR_MSTA | CCR_MTX | CCR_MEN);
> +       udelay(30);
> +       writeccr(i2c, CCR_MEN);
> +       udelay(30);
> +}
> +
>  static int i2c_wait(struct mpc_i2c *i2c, unsigned timeout, int writing)
>  {
>         unsigned long orig_jiffies = jiffies;
> @@ -153,6 +167,7 @@ static void mpc_i2c_start(struct mpc_i2c
>  static void mpc_i2c_stop(struct mpc_i2c *i2c)
>  {
>         writeccr(i2c, CCR_MEN);
> +       writeccr(i2c, 0);
>  }
>
>  static int mpc_write(struct mpc_i2c *i2c, int target,
> @@ -245,6 +260,8 @@ static int mpc_xfer(struct i2c_adapter *
>                 }
>                 if (time_after(jiffies, orig_jiffies + HZ)) {
>                         pr_debug("I2C: timeout\n");
> +                       if (readb(i2c->base + MPC_I2C_SR) == (CSR_MCF | CSR_MBB | CSR_RXAK))
> +                               mpc_i2c_fixup(i2c);
>                         return -EIO;
>                 }
>                 schedule();
>


-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

Re: [PATCH] i2c-mpc: work around missing-9th-clock-pulse bug

[Jean Delvare]

Hi Grant, hi Domen,

On Tue, 10 Jul 2007 00:22:05 -0600, Grant Likely wrote:
> On 7/10/07, Domen Puncer <domen.puncer@telargo.com> wrote:
> > Work around a problem reported on:
> > http://ozlabs.org/pipermail/linuxppc-embedded/2005-July/019038.html
> > Without this patch I2C on mpc5200 becomes unusable after a while.
> > Tested on mpc5200 boards by Matthias and me.
> >
> >
> > Signed-off-by: Domen Puncer <domen.puncer@telargo.com>
> 
> Looks good to me,
> 
> Acked-by: Grant Likely <grant.likely@secretlab.ca>

OK, I will take this patch, but I'd like you to add a comment before
mpc_i2c_fixup() explaining what exactly the problem is and how it is
worked around. Otherwise it's a bit obscure what is going on.

I guess you want this patch in 2.6.23-rc1?

> > ---
> >  drivers/i2c/busses/i2c-mpc.c |   17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> >
> > Index: work-powerpc.git/drivers/i2c/busses/i2c-mpc.c
> > ===================================================================
> > --- work-powerpc.git.orig/drivers/i2c/busses/i2c-mpc.c
> > +++ work-powerpc.git/drivers/i2c/busses/i2c-mpc.c
> > @@ -74,6 +74,20 @@ static irqreturn_t mpc_i2c_isr(int irq,
> >         return IRQ_HANDLED;
> >  }
> >
> > +static void mpc_i2c_fixup(struct mpc_i2c *i2c)
> > +{
> > +       writeccr(i2c, 0);
> > +       udelay(30);
> > +       writeccr(i2c, CCR_MEN);
> > +       udelay(30);
> > +       writeccr(i2c, CCR_MSTA | CCR_MTX);
> > +       udelay(30);
> > +       writeccr(i2c, CCR_MSTA | CCR_MTX | CCR_MEN);
> > +       udelay(30);
> > +       writeccr(i2c, CCR_MEN);
> > +       udelay(30);
> > +}
> > +
> >  static int i2c_wait(struct mpc_i2c *i2c, unsigned timeout, int writing)
> >  {
> >         unsigned long orig_jiffies = jiffies;
> > @@ -153,6 +167,7 @@ static void mpc_i2c_start(struct mpc_i2c
> >  static void mpc_i2c_stop(struct mpc_i2c *i2c)
> >  {
> >         writeccr(i2c, CCR_MEN);
> > +       writeccr(i2c, 0);
> >  }
> >
> >  static int mpc_write(struct mpc_i2c *i2c, int target,
> > @@ -245,6 +260,8 @@ static int mpc_xfer(struct i2c_adapter *
> >                 }
> >                 if (time_after(jiffies, orig_jiffies + HZ)) {
> >                         pr_debug("I2C: timeout\n");
> > +                       if (readb(i2c->base + MPC_I2C_SR) == (CSR_MCF | CSR_MBB | CSR_RXAK))
> > +                               mpc_i2c_fixup(i2c);
> >                         return -EIO;
> >                 }
> >                 schedule();
> >

-- 
Jean Delvare

Re: [PATCH] i2c-mpc: work around missing-9th-clock-pulse bug

[Domen Puncer]

On 10/07/07 14:40 +0200, Jean Delvare wrote:
> Hi Grant, hi Domen,
> 
> On Tue, 10 Jul 2007 00:22:05 -0600, Grant Likely wrote:
> > On 7/10/07, Domen Puncer <domen.puncer@telargo.com> wrote:
> > > Work around a problem reported on:
> > > http://ozlabs.org/pipermail/linuxppc-embedded/2005-July/019038.html
> > > Without this patch I2C on mpc5200 becomes unusable after a while.
> > > Tested on mpc5200 boards by Matthias and me.
> > >
> > >
> > > Signed-off-by: Domen Puncer <domen.puncer@telargo.com>
> > 
> > Looks good to me,
> > 
> > Acked-by: Grant Likely <grant.likely@secretlab.ca>
> 
> OK, I will take this patch, but I'd like you to add a comment before
> mpc_i2c_fixup() explaining what exactly the problem is and how it is
> worked around. Otherwise it's a bit obscure what is going on.

OK.

> 
> I guess you want this patch in 2.6.23-rc1?

Yes.

So... v3:
<----------- cut ------------->

Work around a problem reported on:
http://ozlabs.org/pipermail/linuxppc-embedded/2005-July/019038.html
Without this patch I2C on mpc5200 becomes unusable after a while.
Tested on mpc5200 boards by Matthias Fechner and me.


Signed-off-by: Domen Puncer <domen.puncer@telargo.com>

---
 drivers/i2c/busses/i2c-mpc.c |   21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

Index: work-powerpc.git/drivers/i2c/busses/i2c-mpc.c
===================================================================
--- work-powerpc.git.orig/drivers/i2c/busses/i2c-mpc.c
+++ work-powerpc.git/drivers/i2c/busses/i2c-mpc.c
@@ -74,6 +74,24 @@ static irqreturn_t mpc_i2c_isr(int irq, 
 	return IRQ_HANDLED;
 }
 
+/* Sometimes 9th clock pulse isn't generated, so slave doesn't release
+ * the bus.  Documented and suggested workaround on
+ * http://ozlabs.org/pipermail/linuxppc-embedded/2005-July/019038.html
+ */
+static void mpc_i2c_fixup(struct mpc_i2c *i2c)
+{
+	writeccr(i2c, 0);
+	udelay(30);
+	writeccr(i2c, CCR_MEN);
+	udelay(30);
+	writeccr(i2c, CCR_MSTA | CCR_MTX);
+	udelay(30);
+	writeccr(i2c, CCR_MSTA | CCR_MTX | CCR_MEN);
+	udelay(30);
+	writeccr(i2c, CCR_MEN);
+	udelay(30);
+}
+
 static int i2c_wait(struct mpc_i2c *i2c, unsigned timeout, int writing)
 {
 	unsigned long orig_jiffies = jiffies;
@@ -153,6 +171,7 @@ static void mpc_i2c_start(struct mpc_i2c
 static void mpc_i2c_stop(struct mpc_i2c *i2c)
 {
 	writeccr(i2c, CCR_MEN);
+	writeccr(i2c, 0);
 }
 
 static int mpc_write(struct mpc_i2c *i2c, int target,
@@ -245,6 +264,8 @@ static int mpc_xfer(struct i2c_adapter *
 		}
 		if (time_after(jiffies, orig_jiffies + HZ)) {
 			pr_debug("I2C: timeout\n");
+			if (readb(i2c->base + MPC_I2C_SR) == (CSR_MCF | CSR_MBB | CSR_RXAK))
+				mpc_i2c_fixup(i2c);
 			return -EIO;
 		}
 		schedule();

Re: [PATCH] i2c-mpc: work around missing-9th-clock-pulse bug

[Grant Likely]

On 7/10/07, Domen Puncer <domen.puncer@telargo.com> wrote:
> On 10/07/07 14:40 +0200, Jean Delvare wrote:
> > Hi Grant, hi Domen,
> >
> > On Tue, 10 Jul 2007 00:22:05 -0600, Grant Likely wrote:
> > > On 7/10/07, Domen Puncer <domen.puncer@telargo.com> wrote:
> > > > Work around a problem reported on:
> > > > http://ozlabs.org/pipermail/linuxppc-embedded/2005-July/019038.html
> > > > Without this patch I2C on mpc5200 becomes unusable after a while.
> > > > Tested on mpc5200 boards by Matthias and me.
> > > >
> > > >
> > > > Signed-off-by: Domen Puncer <domen.puncer@telargo.com>
> > >
> > > Looks good to me,
> > >
> > > Acked-by: Grant Likely <grant.likely@secretlab.ca>
> >
> > OK, I will take this patch, but I'd like you to add a comment before
> > mpc_i2c_fixup() explaining what exactly the problem is and how it is
> > worked around. Otherwise it's a bit obscure what is going on.
>
> OK.
>
> >
> > I guess you want this patch in 2.6.23-rc1?
>
> Yes.
>
> So... v3:
> <----------- cut ------------->
>
> Work around a problem reported on:
> http://ozlabs.org/pipermail/linuxppc-embedded/2005-July/019038.html
> Without this patch I2C on mpc5200 becomes unusable after a while.
> Tested on mpc5200 boards by Matthias Fechner and me.
>
>
> Signed-off-by: Domen Puncer <domen.puncer@telargo.com>
>
> ---
>  drivers/i2c/busses/i2c-mpc.c |   21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> Index: work-powerpc.git/drivers/i2c/busses/i2c-mpc.c
> ===================================================================
> --- work-powerpc.git.orig/drivers/i2c/busses/i2c-mpc.c
> +++ work-powerpc.git/drivers/i2c/busses/i2c-mpc.c
> @@ -74,6 +74,24 @@ static irqreturn_t mpc_i2c_isr(int irq,
>         return IRQ_HANDLED;
>  }
>
> +/* Sometimes 9th clock pulse isn't generated, so slave doesn't release
> + * the bus.  Documented and suggested workaround on
> + * http://ozlabs.org/pipermail/linuxppc-embedded/2005-July/019038.html
> + */

I don't think it's a great idea to use a link;  You should copy the
important parts into the .c file.  Archives may not be forever, and
links cannot be read when offline.  If the text is too long for the
middle of the C file; then put the documentation at the top right
after the header block.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

Re: [PATCH] i2c-mpc: work around missing-9th-clock-pulse bug

[Domen Puncer]

On 11/07/07 00:03 -0600, Grant Likely wrote:
> On 7/10/07, Domen Puncer <domen.puncer@telargo.com> wrote:
> >On 10/07/07 14:40 +0200, Jean Delvare wrote:
...
> >+/* Sometimes 9th clock pulse isn't generated, so slave doesn't release
> >+ * the bus.  Documented and suggested workaround on
> >+ * http://ozlabs.org/pipermail/linuxppc-embedded/2005-July/019038.html
> >+ */
> 
> I don't think it's a great idea to use a link;  You should copy the
> important parts into the .c file.  Archives may not be forever, and
> links cannot be read when offline.  If the text is too long for the
> middle of the C file; then put the documentation at the top right
> after the header block.

And v4:
<----------- cut ------------->

Work around a problem reported on:
http://ozlabs.org/pipermail/linuxppc-embedded/2005-July/019038.html
Without this patch I2C on mpc5200 becomes unusable after a while.
Tested on mpc5200 boards by Matthias Fechner and me.


Signed-off-by: Domen Puncer <domen.puncer@telargo.com>

---
 drivers/i2c/busses/i2c-mpc.c |   22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

Index: work-powerpc.git/drivers/i2c/busses/i2c-mpc.c
===================================================================
--- work-powerpc.git.orig/drivers/i2c/busses/i2c-mpc.c
+++ work-powerpc.git/drivers/i2c/busses/i2c-mpc.c
@@ -74,6 +74,25 @@ static irqreturn_t mpc_i2c_isr(int irq, 
 	return IRQ_HANDLED;
 }
 
+/* Sometimes 9th clock pulse isn't generated, and slave doesn't release
+ * the bus, because it wants to send ACK.
+ * Following sequence of enabling/disabling and sending start/stop generates
+ * the pulse, so it's all OK.
+ */
+static void mpc_i2c_fixup(struct mpc_i2c *i2c)
+{
+	writeccr(i2c, 0);
+	udelay(30);
+	writeccr(i2c, CCR_MEN);
+	udelay(30);
+	writeccr(i2c, CCR_MSTA | CCR_MTX);
+	udelay(30);
+	writeccr(i2c, CCR_MSTA | CCR_MTX | CCR_MEN);
+	udelay(30);
+	writeccr(i2c, CCR_MEN);
+	udelay(30);
+}
+
 static int i2c_wait(struct mpc_i2c *i2c, unsigned timeout, int writing)
 {
 	unsigned long orig_jiffies = jiffies;
@@ -153,6 +172,7 @@ static void mpc_i2c_start(struct mpc_i2c
 static void mpc_i2c_stop(struct mpc_i2c *i2c)
 {
 	writeccr(i2c, CCR_MEN);
+	writeccr(i2c, 0);
 }
 
 static int mpc_write(struct mpc_i2c *i2c, int target,
@@ -245,6 +265,8 @@ static int mpc_xfer(struct i2c_adapter *
 		}
 		if (time_after(jiffies, orig_jiffies + HZ)) {
 			pr_debug("I2C: timeout\n");
+			if (readb(i2c->base + MPC_I2C_SR) == (CSR_MCF | CSR_MBB | CSR_RXAK))
+				mpc_i2c_fixup(i2c);
 			return -EIO;
 		}
 		schedule();

Re: [PATCH] i2c-mpc: work around missing-9th-clock-pulse bug

[Jean Delvare]

Hi Domen,

On Wed, 11 Jul 2007 08:33:35 +0200, Domen Puncer wrote:
> On 11/07/07 00:03 -0600, Grant Likely wrote:
> > I don't think it's a great idea to use a link;  You should copy the
> > important parts into the .c file.  Archives may not be forever, and
> > links cannot be read when offline.  If the text is too long for the
> > middle of the C file; then put the documentation at the top right
> > after the header block.
> 
> And v4:
> <----------- cut ------------->
> 
> Work around a problem reported on:
> http://ozlabs.org/pipermail/linuxppc-embedded/2005-July/019038.html
> Without this patch I2C on mpc5200 becomes unusable after a while.
> Tested on mpc5200 boards by Matthias Fechner and me.
> 
> 
> Signed-off-by: Domen Puncer <domen.puncer@telargo.com>

Applied, thanks.

-- 
Jean Delvare

Re: [i2c] [PATCH] i2c-mpc: work around missing-9th-clock-pulse bug

[Guennadi Liakhovetski]

On Mon, 9 Jul 2007, Domen Puncer wrote:

> Work around a problem reported on:
> http://ozlabs.org/pipermail/linuxppc-embedded/2005-July/019038.html
> Without this patch I2C on mpc5200 becomes unusable after a while.
> Tested on mpc5200 based boards by Matthias and me.

Domen, unfortunately, your suspicion, expressed on IRC, was right. It is 
this patch that breaks i2c on mpc8241 (linkstation). Reverting it fixes 
the problem. For reference, here's a rtc-debug log with this patch 
(linux-2.6 tree of 19.07):

rtc-rs5c372 0-0032: rs5c372_probe
rtc-rs5c372 0-0032: 52 28 21 (04) 19 07 07 (15), 04 00 6a, 08 00 3e; 00 20
rtc-rs5c372 0-0032: 28 21 04 (19) 07 07 15 (04), 00 6a 08, 00 3e 00; 20 52
rtc-rs5c372 0-0032: rs5c372_get_datetime: tm is secs=28, mins=21, hours=4, mday=7, mon=6, year=115, wday=1
rtc-rs5c372 0-0032: rs5c372b found, 24hr, driver version 0.5
rtc-rs5c372: dev (254:0)
rtc-rs5c372 0-0032: rtc core: registered rtc-rs5c372 as rtc0
rtc-rs5c372 0-0032: 21 04 19 (07) 07 15 04 (00), 6a 08 00, 3e 00 20; 52 28
rtc-rs5c372 0-0032: rs5c372_get_datetime: tm is secs=21, mins=4, hours=19, mday=7, mon=14, year=104, wday=7
rtc-rs5c372 0-0032: hctosys: unable to read the hardware clock

Let me know when you'll have any patches to test.

Thanks
Guennadi

> 
> 
> Signed-off-by: Domen Puncer <domen.puncer@telargo.com>
> 
> ---
>  drivers/i2c/busses/i2c-mpc.c |   19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> Index: work-powerpc.git/drivers/i2c/busses/i2c-mpc.c
> ===================================================================
> --- work-powerpc.git.orig/drivers/i2c/busses/i2c-mpc.c
> +++ work-powerpc.git/drivers/i2c/busses/i2c-mpc.c
> @@ -74,6 +74,20 @@ static irqreturn_t mpc_i2c_isr(int irq, 
>  	return IRQ_HANDLED;
>  }
>  
> +static void mpc_i2c_fixup(struct mpc_i2c *i2c)
> +{
> +	writeccr(i2c, 0);
> +	udelay(30);
> +	writeccr(i2c, CCR_MEN);
> +	udelay(30);
> +	writeccr(i2c, CCR_MSTA | CCR_MTX);
> +	udelay(30);
> +	writeccr(i2c, CCR_MSTA | CCR_MTX | CCR_MEN);
> +	udelay(30);
> +	writeccr(i2c, CCR_MEN);
> +	udelay(30);
> +}
> +
>  static int i2c_wait(struct mpc_i2c *i2c, unsigned timeout, int writing)
>  {
>  	unsigned long orig_jiffies = jiffies;
> @@ -153,6 +167,9 @@ static void mpc_i2c_start(struct mpc_i2c
>  static void mpc_i2c_stop(struct mpc_i2c *i2c)
>  {
>  	writeccr(i2c, CCR_MEN);
> +	mb();
> +	writeccr(i2c, 0);
> +	mb();
>  }
>  
>  static int mpc_write(struct mpc_i2c *i2c, int target,
> @@ -245,6 +262,8 @@ static int mpc_xfer(struct i2c_adapter *
>  		}
>  		if (time_after(jiffies, orig_jiffies + HZ)) {
>  			pr_debug("I2C: timeout\n");
> +			if (readb(i2c->base + MPC_I2C_SR) == (CSR_MCF | CSR_MBB | CSR_RXAK))
> +				mpc_i2c_fixup(i2c);
>  			return -EIO;
>  		}
>  		schedule();
> 
> _______________________________________________
> i2c mailing list
> i2c@lm-sensors.org
> http://lists.lm-sensors.org/mailman/listinfo/i2c
> 

---
Guennadi Liakhovetski

[PATCH] i2c-mpc: don't disable I2C module on stop condition.

[Domen Puncer]

Disabling module on stop doesn't work on some CPUs (ie. mpc8241,
as reported by Guennadi Liakhovetski), so remove that.

Disable I2C module on errors/interrupts to prevent it from
locking up on mpc5200b.


Signed-off-by: Domen Puncer <domen.puncer@telargo.com>
---
Hi!

So I fixed i2c on one board, and broke it on another :-(
This patch works on both Guennadi's and mine (hey, it might break
a third one!).
Jean, can you please push this, if there are no objections
and "doesn't work for me" reports.


	Domen


 drivers/i2c/busses/i2c-mpc.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Index: work-powerpc.git/drivers/i2c/busses/i2c-mpc.c
===================================================================
--- work-powerpc.git.orig/drivers/i2c/busses/i2c-mpc.c
+++ work-powerpc.git/drivers/i2c/busses/i2c-mpc.c
@@ -105,6 +105,7 @@ static int i2c_wait(struct mpc_i2c *i2c,
 			schedule();
 			if (time_after(jiffies, orig_jiffies + timeout)) {
 				pr_debug("I2C: timeout\n");
+				writeccr(i2c, 0);
 				result = -EIO;
 				break;
 			}
@@ -116,10 +117,12 @@ static int i2c_wait(struct mpc_i2c *i2c,
 		result = wait_event_interruptible_timeout(i2c->queue,
 			(i2c->interrupt & CSR_MIF), timeout * HZ);
 
-		if (unlikely(result < 0))
+		if (unlikely(result < 0)) {
 			pr_debug("I2C: wait interrupted\n");
-		else if (unlikely(!(i2c->interrupt & CSR_MIF))) {
+			writeccr(i2c, 0);
+		} else if (unlikely(!(i2c->interrupt & CSR_MIF))) {
 			pr_debug("I2C: wait timeout\n");
+			writeccr(i2c, 0);
 			result = -ETIMEDOUT;
 		}
 
@@ -172,7 +175,6 @@ static void mpc_i2c_start(struct mpc_i2c
 static void mpc_i2c_stop(struct mpc_i2c *i2c)
 {
 	writeccr(i2c, CCR_MEN);
-	writeccr(i2c, 0);
 }
 
 static int mpc_write(struct mpc_i2c *i2c, int target,
@@ -261,6 +263,7 @@ static int mpc_xfer(struct i2c_adapter *
 	while (readb(i2c->base + MPC_I2C_SR) & CSR_MBB) {
 		if (signal_pending(current)) {
 			pr_debug("I2C: Interrupted\n");
+			writeccr(i2c, 0);
 			return -EINTR;
 		}
 		if (time_after(jiffies, orig_jiffies + HZ)) {

Re: [PATCH] i2c-mpc: don't disable I2C module on stop condition.

[Jean Delvare]

Hi Domen,

On Tue, 24 Jul 2007 07:14:31 +0200, Domen Puncer wrote:
> Disabling module on stop doesn't work on some CPUs (ie. mpc8241,
> as reported by Guennadi Liakhovetski), so remove that.
> 
> Disable I2C module on errors/interrupts to prevent it from
> locking up on mpc5200b.
> 
> 
> Signed-off-by: Domen Puncer <domen.puncer@telargo.com>
> ---
> Hi!
> 
> So I fixed i2c on one board, and broke it on another :-(
> This patch works on both Guennadi's and mine (hey, it might break
> a third one!).
> Jean, can you please push this, if there are no objections
> and "doesn't work for me" reports.

Queued for 2.6.23-rc3, thanks. Guennadi, can you please confirm that
this patch fixes your problem?

> 
> 
> 	Domen
> 
> 
>  drivers/i2c/busses/i2c-mpc.c |    9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> Index: work-powerpc.git/drivers/i2c/busses/i2c-mpc.c
> ===================================================================
> --- work-powerpc.git.orig/drivers/i2c/busses/i2c-mpc.c
> +++ work-powerpc.git/drivers/i2c/busses/i2c-mpc.c
> @@ -105,6 +105,7 @@ static int i2c_wait(struct mpc_i2c *i2c,
>  			schedule();
>  			if (time_after(jiffies, orig_jiffies + timeout)) {
>  				pr_debug("I2C: timeout\n");
> +				writeccr(i2c, 0);
>  				result = -EIO;
>  				break;
>  			}
> @@ -116,10 +117,12 @@ static int i2c_wait(struct mpc_i2c *i2c,
>  		result = wait_event_interruptible_timeout(i2c->queue,
>  			(i2c->interrupt & CSR_MIF), timeout * HZ);
>  
> -		if (unlikely(result < 0))
> +		if (unlikely(result < 0)) {
>  			pr_debug("I2C: wait interrupted\n");
> -		else if (unlikely(!(i2c->interrupt & CSR_MIF))) {
> +			writeccr(i2c, 0);
> +		} else if (unlikely(!(i2c->interrupt & CSR_MIF))) {
>  			pr_debug("I2C: wait timeout\n");
> +			writeccr(i2c, 0);
>  			result = -ETIMEDOUT;
>  		}
>  
> @@ -172,7 +175,6 @@ static void mpc_i2c_start(struct mpc_i2c
>  static void mpc_i2c_stop(struct mpc_i2c *i2c)
>  {
>  	writeccr(i2c, CCR_MEN);
> -	writeccr(i2c, 0);
>  }
>  
>  static int mpc_write(struct mpc_i2c *i2c, int target,
> @@ -261,6 +263,7 @@ static int mpc_xfer(struct i2c_adapter *
>  	while (readb(i2c->base + MPC_I2C_SR) & CSR_MBB) {
>  		if (signal_pending(current)) {
>  			pr_debug("I2C: Interrupted\n");
> +			writeccr(i2c, 0);
>  			return -EINTR;
>  		}
>  		if (time_after(jiffies, orig_jiffies + HZ)) {


-- 
Jean Delvare

Re: [PATCH] i2c-mpc: don't disable I2C module on stop condition.

[Guennadi Liakhovetski]

On Wed, 8 Aug 2007, Jean Delvare wrote:

> Hi Domen,
> 
> On Tue, 24 Jul 2007 07:14:31 +0200, Domen Puncer wrote:
> > Disabling module on stop doesn't work on some CPUs (ie. mpc8241,
> > as reported by Guennadi Liakhovetski), so remove that.
> > 
> > Disable I2C module on errors/interrupts to prevent it from
> > locking up on mpc5200b.
> > 
> > 
> > Signed-off-by: Domen Puncer <domen.puncer@telargo.com>
> > ---
> > Hi!
> > 
> > So I fixed i2c on one board, and broke it on another :-(
> > This patch works on both Guennadi's and mine (hey, it might break
> > a third one!).
> > Jean, can you please push this, if there are no objections
> > and "doesn't work for me" reports.
> 
> Queued for 2.6.23-rc3, thanks. Guennadi, can you please confirm that
> this patch fixes your problem?

Yes, it does. Also tested with i2c bus debugging on and with interrupting 
hwclock, thereby messages

I2C: Interrupted

come, but the controller stays functional. Looks good so far:-)

Acked-by: G. Liakhovetski <g.liakhovetski@gmx.de>

Thanks
Guennadi
---
Guennadi Liakhovetski