8xx i2c refers to unspecified chip errata

Hi - I've been looking at the i2c driver for MPC8260.  It's the first linux driver I have studied.  I have a couple of questions.

What experimental results or theory are behind the timeout values chosen when waiting for an interrupt from a write transaction?

Also: in the drivers/i2c/i2c-algo-cpm.c or i2c-algo-8xx.c file, I see the following a few times.  What two chip errata are the comments referring to?  A specific errata number would be enlightening.

---------snip
/* Chip errata, clear enable.
*/
i2c->i2c_i2mod = 0;

---------snip
/* Chip bug, set enable here */
save_flags(flags); cli();
i2c->i2c_i2cmr = 0x13;	/* Enable some interupts */
i2c->i2c_i2cer = 0xff;
i2c->i2c_i2mod = 1;	/* Enable */
i2c->i2c_i2com = 0x81;	/* Start master */
----------snip

Thanks for your attention -

--
Michael D. Barker, Motorola SPS Canada
This email along with any attachments is classified as:
[x]  General business information


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Also: in the drivers/i2c/i2c-algo-cpm.c or i2c-algo-8xx.c file, I see the following a few times.  What two chip errata
are the comments referring to?  A specific errata number would be enlightening.

I think these refer to older revision of mpc8xx CPU. In the mpc860 only the latest CPU rev(D4)
has all relevant I2C bugs fixed. The fix below does not work well in my MPC860(rev D4). I think
it's because I2C is disabled before it had a chence to generate a proper STOP condition
and that hangs my DS1337 RTC clock badly(need to reboot to fix it). The driver(i2c-algo-8xx.c) that
comes with the kernel has other severe bugs(memory corruption etc.) as well.

---------snip
/* Chip errata, clear enable.
*/
i2c->i2c_i2mod = 0;

---------snip
/* Chip bug, set enable here */
save_flags(flags); cli();
i2c->i2c_i2cmr = 0x13;	/* Enable some interupts */
i2c->i2c_i2cer = 0xff;
i2c->i2c_i2mod = 1;	/* Enable */
i2c->i2c_i2com = 0x81;	/* Start master */
----------snip

Thanks for your attention -

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Joakim Tjernlund wrote:

 > .... The driver(i2c-algo-8xx.c) that

comes with the kernel has other severe bugs(memory corruption etc.) as well.

Just toggle the bits in software.  I never saw an application where the I2C
controller provided a performance advantage.  It's lots of software to execute,
along with all of the interrupt handling and stuff, so you just spend lots of
overhead not making any other progress.

Make sure you carefully analyze the entire system performance advantages.
Using the controller assitance just because it is there isn't always the
proper solution.  Same is true for the SPI.


	-- Dan


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Joakim Tjernlund wrote:

 > .... The driver(i2c-algo-8xx.c) that

quoted

comes with the kernel has other severe bugs(memory corruption etc.) as well.

Dan Wrote

Just toggle the bits in software.  I never saw an application where the I2C
controller provided a performance advantage.  It's lots of software to execute,
along with all of the interrupt handling and stuff, so you just spend lots of
overhead not making any other progress.

Make sure you carefully analyze the entire system performance advantages.
Using the controller assitance just because it is there isn't always the
proper solution.  Same is true for the SPI.

Here's a link to my book's I2C bit banging code for 8xx
http://www.embeddedlinuxinterfacing.com/chapters/10/SAA1064_rpxcllf.c

You can pull the I2C routines from this code.

Let me know if you have any questions.
Craig


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Hi Dan

Joakim Tjernlund wrote:

 > .... The driver(i2c-algo-8xx.c) that

quoted

comes with the kernel has other severe bugs(memory corruption etc.) as well.

Just toggle the bits in software.  I never saw an application where the I2C
controller provided a performance advantage.  It's lots of software to execute,
along with all of the interrupt handling and stuff, so you just spend lots of
overhead not making any other progress.

I2C speed is not very important to me, but freeing up the CPU to do other things is. If
there are just a few bytes data to read/write it probably does not matter, but if you
read/write I2C memories(>128 bytes) it will matter and the I2C controller will make
a difference. One could also adjust the i2c-algo-8xx.c to busy wait for small reads/writes
instead of using interrupts if the interrupt handling overhead is significant.

Anyway I have fixed the bugs in i2c-algo-8xx.c driver and it's working great for me
so I think I will stick with it for the time being.

  Thanks

       Jocke

Make sure you carefully analyze the entire system performance advantages.
Using the controller assitance just because it is there isn't always the
proper solution.  Same is true for the SPI.

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Hi Joakim,

Anyway I have fixed the bugs in i2c-algo-8xx.c driver and it's working great for me
so I think I will stick with it for the time being.

Could you please post the fixes on the list? I also might want to use them.

Thanks,
	Bart

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

OK, here it is.

Perhaps someone can add it to the ppctree?

 Jocke

Hi Joakim,

quoted

Anyway I have fixed the bugs in i2c-algo-8xx.c driver and it's working great for me
so I think I will stick with it for the time being.

Could you please post the fixes on the list? I also might want to use them.

Thanks,
	Bart

Hi Joakim,

OK, here it is.

Thanks

Perhaps someone can add it to the ppctree?

As far as I understand is the i2c not 'realy' a part of the Linux source (I think
on http://www2.lm-sensors.nu you find the I2C maintainers which results in all kind
off problems with the drivers :(

Cheers,
	Bart

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Hi,

I had a look at you code. Some of the changes I already did in my code. :)
I couldn't find any thing that looks like it's fixing the memory problem
that you mentioned. I'm looking for a memory problem by my self and I wonder
what you might have figured out. Could you tell me more details?

Regards, Stephan

-----Original Message-----
From: owner-linuxppc-embedded@lists.linuxppc.org
[mailto:owner-linuxppc-embedded@lists.linuxppc.org]On Behalf Of Joakim
Tjernlund
Sent: Freitag, 11. Oktober 2002 11:12
To: bart@ardistech.com
Cc: linuxppc-embedded@lists.linuxppc.org
Subject: RE: 8xx i2c refers to unspecified chip errata


OK, here it is.

Perhaps someone can add it to the ppctree?

 Jocke

quoted

Hi Joakim,

quoted

Anyway I have fixed the bugs in i2c-algo-8xx.c driver and

it's working great for me

quoted

quoted

so I think I will stick with it for the time being.

Could you please post the fixes on the list? I also might want

to use them.

quoted

Thanks,
	Bart

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Hi Stephan,

I had a look at you code. Some of the changes I already did in my code. :)
I couldn't find any thing that looks like it's fixing the memory problem
that you mentioned. I'm looking for a memory problem by my self and I wonder
what you might have figured out.

You're sure it not a cache invalidate problem you're looking at?

Cheers,
	Bart

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Hi Bart and Joakim,

Well. No, I'm not shure. But I see what you mean Bart. :) I'll see what
happens now.

And I have a question to Joakim. In your code you wrote (cpm_iic_init):

	i2c->i2c_i2mod = 0x80; /* Filter clock */

I think this is not what you intended. I didn't find that bit (bit 0) in my
862 manual. But there's the FLT flag (bit 4) that matches your comment. So I
think It should be:

	i2c->i2c_i2mod = 0x08; /* Filter clock */

Thanks, Stephan

-----Original Message-----
From: bart [mailto:bart]On Behalf Of bart@ardistech.com
Sent: Freitag, 11. Oktober 2002 14:15
To: Stephan Linke
Cc: Linuxppc-Embedded
Subject: Re: 8xx i2c refers to unspecified chip errata


Hi Stephan,

quoted

I had a look at you code. Some of the changes I already did in

my code. :)

quoted

I couldn't find any thing that looks like it's fixing the memory problem
that you mentioned. I'm looking for a memory problem by my self

and I wonder

quoted

what you might have figured out.

You're sure it not a cache invalidate problem you're looking at?

Cheers,
	Bart

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Hi,

Look in cpm_iic_read() for:
/* flush will invalidate too. */
flush_dcache_range((unsigned long)buf, (unsigned long)(buf+count));

You can't just do invalidate_dcache_range() since it will invalidate more than your buffer
if it's not cache line aligned. Therefore you must flush to memory first and then invalidate.
But you don't need an explicit invalidate_dcache_range() since flush_dcache_range() will flush to
memory AND invalidate.

  Jocke

Hi,

I had a look at you code. Some of the changes I already did in my code. :)
I couldn't find any thing that looks like it's fixing the memory problem
that you mentioned. I'm looking for a memory problem by my self and I wonder
what you might have figured out. Could you tell me more details?

Regards, Stephan

quoted

-----Original Message-----
From: owner-linuxppc-embedded@lists.linuxppc.org
[mailto:owner-linuxppc-embedded@lists.linuxppc.org]On Behalf Of Joakim
Tjernlund
Sent: Freitag, 11. Oktober 2002 11:12
To: bart@ardistech.com
Cc: linuxppc-embedded@lists.linuxppc.org
Subject: RE: 8xx i2c refers to unspecified chip errata


OK, here it is.

Perhaps someone can add it to the ppctree?

 Jocke

quoted

Hi Joakim,

quoted

Anyway I have fixed the bugs in i2c-algo-8xx.c driver and

it's working great for me

quoted

quoted

so I think I will stick with it for the time being.

Could you please post the fixes on the list? I also might want

to use them.

quoted

Thanks,
	Bart

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Hi Bart and Joakim,

Well. No, I'm not shure. But I see what you mean Bart. :) I'll see what
happens now.

And I have a question to Joakim. In your code you wrote (cpm_iic_init):

	i2c->i2c_i2mod = 0x80; /* Filter clock */

I think this is not what you intended. I didn't find that bit (bit 0) in my
862 manual. But there's the FLT flag (bit 4) that matches your comment. So I
think It should be:

	i2c->i2c_i2mod = 0x08; /* Filter clock */

You are right

  Jocke


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Hi,

Look in cpm_iic_read() for:
/* flush will invalidate too. */
flush_dcache_range((unsigned long)buf, (unsigned long)(buf+count));

You can't just do invalidate_dcache_range() since it will invalidate more than your buffer

In my case it did invalidate the stack :(

if it's not cache line aligned. Therefore you must flush to memory first and then invalidate.
But you don't need an explicit invalidate_dcache_range() since flush_dcache_range() will flush to
memory AND invalidate.

Better would be to have a special DMA region for this. Or as Dan suggest forget the
whole CPM IIC and do it by software.

Cheers,
	Bart

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Hi,

quoted

Look in cpm_iic_read() for:
/* flush will invalidate too. */
flush_dcache_range((unsigned long)buf, (unsigned long)(buf+count));

You can't just do invalidate_dcache_range() since it will invalidate more than your buffer

In my case it did invalidate the stack :(

Me too, it was a major PITA to hunt it down.

    Jocke


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

On Fri, Oct 11, 2002 at 11:12:02AM +0200, Joakim Tjernlund wrote:

OK, here it is.

Perhaps someone can add it to the ppctree?

If you can submit this as a patch vs what's in the i2c CVS tree or the
ppctree, than yes, this can be committed there, and to the ppctrees.

--
Tom Rini (TR1265)
http://gate.crashing.org/~trini/

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

On Fri, Oct 11, 2002 at 11:12:02AM +0200, Joakim Tjernlund wrote:

quoted

OK, here it is.

Perhaps someone can add it to the ppctree?

If you can submit this as a patch vs what's in the i2c CVS tree or the
ppctree, than yes, this can be committed there, and to the ppctrees.

Here is a patch against the PPC 2.4 devel tree for the i2c-algo-8xx.c

Tom, do you look after 8xx_io/enet.c? I sent a patch to the emmbedded list
yesterday that does away with an expensive memcpy.

 Jocke

--- i2c-algo-8xx.c	Mon Sep 30 16:20:42 2002
+++ i2c-algo-8xx.c.clean	Tue Oct 22 10:49:33 2002

@@ -7,15 +7,12 @@
  * Brad Parker (brad@heeltoe.com)
  */

-// XXX todo
-// timeout sleep?
-
 /* $Id$ */

 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/delay.h>
-#include <linux/malloc.h>
+#include <linux/slab.h>
 #include <linux/version.h>
 #include <linux/init.h>
 #include <asm/uaccess.h>

@@ -30,25 +27,25 @@
 #include <linux/i2c-algo-8xx.h>

 #define CPM_MAX_READ	513
-
+/* #define I2C_CHIP_ERRATA */ /* Try uncomment this if you have an older CPU(earlier than rev D4) */
 static wait_queue_head_t iic_wait;
 static ushort r_tbase, r_rbase;

 int cpm_scan = 0;
-int cpm_debug = 1;
+int cpm_debug = 0;

-static void
-cpm_iic_interrupt(void *dev_id)
+static  void
+cpm_iic_interrupt(void *dev_id, struct pt_regs *regs)
 {
 	volatile i2c8xx_t *i2c = (i2c8xx_t *)dev_id;
-
 	if (cpm_debug > 1)
 		printk("cpm_iic_interrupt(dev_id=%p)\n", dev_id);
-
-	/* Chip errata, clear enable.
-	*/
-	i2c->i2c_i2mod = 0;
-
+#if 0
+	/* Chip errata, clear enable. This is not needed on rev D4 CPUs */
+        /* This should probably be removed and replaced by I2C_CHIP_ERRATA stuff */
+        /* Someone with a buggy CPU needs to confirm that */
+	i2c->i2c_i2mod &= ~1;
+#endif
 	/* Clear interrupt.
 	*/
 	i2c->i2c_i2cer = 0xff;

@@ -63,6 +60,8 @@
 {
 	volatile iic_t		*iip = cpm_adap->iip;
 	volatile i2c8xx_t	*i2c = cpm_adap->i2c;
+	unsigned char brg;
+	bd_t *bd = (bd_t *)__res;

 	if (cpm_debug) printk("cpm_iic_init()\n");

@@ -101,15 +100,24 @@
 		cp->cp_cpcr =
 			mk_cr_cmd(CPM_CR_CH_I2C, CPM_CR_INIT_TRX) | CPM_CR_FLG;
 		while (cp->cp_cpcr & CPM_CR_FLG);
+	} else {
+		iip->iic_rbptr = iip->iic_rbase;
+		iip->iic_tbptr = iip->iic_tbase;
+		iip->iic_rstate	= 0;
+		iip->iic_tstate	= 0;
 	}

 	/* Select an arbitrary address.  Just make sure it is unique.
 	*/
-	i2c->i2c_i2add = 0x34;
+	i2c->i2c_i2add = 0xfe;

-	/* Make clock run maximum slow.
+	/* Make clock run at 60 KHz.
 	*/
-	i2c->i2c_i2brg = 7;
+	brg = (unsigned char) (bd->bi_intfreq/(32*2*60000) -3);
+	i2c->i2c_i2brg = brg;
+
+	i2c->i2c_i2mod = 0x00;
+	i2c->i2c_i2com = 0x01; /* Master mode */

 	/* Disable interrupts.
 	*/

@@ -131,7 +139,7 @@

 	/* Shut down IIC.
 	*/
-	i2c->i2c_i2mod = 0;
+	i2c->i2c_i2mod &= ~1;
 	i2c->i2c_i2cmr = 0;
 	i2c->i2c_i2cer = 0xff;

@@ -151,22 +159,24 @@

 	iip->iic_rstate = 0;
 	iip->iic_rdp = 0;
-	iip->iic_rbptr = 0;
+	iip->iic_rbptr = iip->iic_rbase;
 	iip->iic_rbc = 0;
 	iip->iic_rxtmp = 0;
 	iip->iic_tstate = 0;
 	iip->iic_tdp = 0;
-	iip->iic_tbptr = 0;
+	iip->iic_tbptr = iip->iic_tbase;
 	iip->iic_tbc = 0;
 	iip->iic_txtmp = 0;
 }

 #define BD_SC_NAK		((ushort)0x0004) /* NAK - did not respond */
+#define BD_SC_OV		((ushort)0x0002) /* OV - receive overrun */
 #define CPM_CR_CLOSE_RXBD	((ushort)0x0007)

 static void force_close(struct i2c_algo_8xx_data *cpm)
 {
-	if (cpm->reloc == 0) {
+	volatile i2c8xx_t *i2c = cpm->i2c;
+	if (cpm->reloc == 0) { /* micro code disabled */
 		volatile cpm8xx_t *cp = cpm->cp;

 		if (cpm_debug) printk("force_close()\n");

@@ -176,6 +186,8 @@

 		while (cp->cp_cpcr & CPM_CR_FLG);
 	}
+	i2c->i2c_i2cmr = 0x00;	/* Disable all interrupts */
+	i2c->i2c_i2cer = 0xff;
 }

@@ -190,7 +202,7 @@
 	volatile cpm8xx_t *cp = cpm->cp;
 	volatile cbd_t	*tbdf, *rbdf;
 	u_char *tb;
-	unsigned long flags;
+	unsigned long flags, tmo;

 	if (count >= CPM_MAX_READ)
 		return -EINVAL;

@@ -207,68 +219,104 @@
 	 * All that is used is the first byte for address, the remainder
 	 * is just used for timing (and doesn't really have to exist).
 	 */
-	if (cpm->reloc) {
-		cpm_reset_iic_params(iip);
-	}
 	tb = cpm->temp;
 	tb = (u_char *)(((uint)tb + 15) & ~15);
 	tb[0] = abyte;		/* Device address byte w/rw flag */

-	flush_dcache_range(tb, tb+1);
-
+	flush_dcache_range((unsigned long)tb, (unsigned long)(tb+1));
 	if (cpm_debug) printk("cpm_iic_read(abyte=0x%x)\n", abyte);

 	tbdf->cbd_bufaddr = __pa(tb);
 	tbdf->cbd_datlen = count + 1;
 	tbdf->cbd_sc =
-		BD_SC_READY | BD_SC_INTRPT | BD_SC_LAST |
+		BD_SC_READY | BD_SC_LAST |
 		BD_SC_WRAP | BD_IIC_START;

+	iip->iic_mrblr = count +1; /* prevent excessive read, +1
+				      is needed otherwise will the
+				      RXB interrupt come too early */
+
+	/* flush will invalidate too. */
+	flush_dcache_range((unsigned long)buf, (unsigned long)(buf+count));
+
 	rbdf->cbd_datlen = 0;
 	rbdf->cbd_bufaddr = __pa(buf);
-	rbdf->cbd_sc = BD_SC_EMPTY | BD_SC_WRAP;
-
-	/* Chip bug, set enable here */
-	save_flags(flags); cli();
-	i2c->i2c_i2cmr = 0x13;	/* Enable some interupts */
-	i2c->i2c_i2cer = 0xff;
-	i2c->i2c_i2mod = 1;	/* Enable */
-	i2c->i2c_i2com = 0x81;	/* Start master */
+	rbdf->cbd_sc = BD_SC_EMPTY | BD_SC_WRAP| BD_SC_INTRPT;
+	if(count > 16){
+		/* Chip bug, set enable here */
+		local_irq_save(flags);
+		i2c->i2c_i2cmr = 0x13;	/* Enable some interupts */
+		i2c->i2c_i2cer = 0xff;
+		i2c->i2c_i2mod |= 1;	/* Enable */
+		i2c->i2c_i2com |= 0x80;	/* Begin transmission */
+
+		/* Wait for IIC transfer */
+		tmo = interruptible_sleep_on_timeout(&iic_wait,1*HZ);
+		local_irq_restore(flags);
+	} else { /* busy wait for small transfers, its faster */
+		i2c->i2c_i2cmr = 0x00;	/* Disable I2C interupts */
+		i2c->i2c_i2cer = 0xff;
+		i2c->i2c_i2mod |= 1;	/* Enable */
+		i2c->i2c_i2com |= 0x80;	/* Begin transmission */
+		tmo = jiffies + 1*HZ;
+		while(!(i2c->i2c_i2cer & 0x11 || time_after(jiffies, tmo))); /* Busy wait, with a timeout */
+	}

-	/* Wait for IIC transfer */
-	interruptible_sleep_on(&iic_wait);
-	restore_flags(flags);
-	if (signal_pending(current))
+	if (signal_pending(current) || !tmo){
+		force_close(cpm);
+		if(cpm_debug)
+			printk("IIC read: timeout!\n");
 		return -EIO;
-
+	}
+#ifdef I2C_CHIP_ERRATA
+	/* Chip errata, clear enable. This is not needed on rev D4 CPUs.
+	 Disabling I2C too early may cause too short stop condition */
+	udelay(4);
+	i2c->i2c_i2mod &= ~1;
+#endif
 	if (cpm_debug) {
 		printk("tx sc %04x, rx sc %04x\n",
 		       tbdf->cbd_sc, rbdf->cbd_sc);
 	}

+	if (tbdf->cbd_sc & BD_SC_READY) {
+		printk("IIC read; complete but tbuf ready\n");
+		force_close(cpm);
+		printk("tx sc %04x, rx sc %04x\n",
+		       tbdf->cbd_sc, rbdf->cbd_sc);
+	}
+
 	if (tbdf->cbd_sc & BD_SC_NAK) {
-		printk("IIC read; no ack\n");
-		return 0;
+		if (cpm_debug)
+			printk("IIC read; no ack\n");
+		return -EREMOTEIO;
 	}

 	if (rbdf->cbd_sc & BD_SC_EMPTY) {
-		printk("IIC read; complete but rbuf empty\n");
-		force_close(cpm);
-		printk("tx sc %04x, rx sc %04x\n",
-		       tbdf->cbd_sc, rbdf->cbd_sc);
+		/* force_close(cpm); */
+		if (cpm_debug){
+			printk("IIC read; complete but rbuf empty\n");
+			printk("tx sc %04x, rx sc %04x\n",
+			       tbdf->cbd_sc, rbdf->cbd_sc);
+		}
+		return -EREMOTEIO;
+	}
+
+	if (rbdf->cbd_sc & BD_SC_OV) {
+		if (cpm_debug)
+			printk("IIC read; Overrun\n");
+		return -EREMOTEIO;;
 	}

 	if (cpm_debug) printk("read %d bytes\n", rbdf->cbd_datlen);

 	if (rbdf->cbd_datlen < count) {
-		printk("IIC read; short, wanted %d got %d\n",
-		       count, rbdf->cbd_datlen);
+		if (cpm_debug)
+			printk("IIC read; short, wanted %d got %d\n",
+			       count, rbdf->cbd_datlen);
 		return 0;
 	}

-
-	invalidate_dcache_range(buf, buf+count);
-
 	return count;
 }

@@ -283,7 +331,7 @@
 	volatile cpm8xx_t *cp = cpm->cp;
 	volatile cbd_t	*tbdf;
 	u_char *tb;
-	unsigned long flags;
+	unsigned long flags, tmo;

 	/* check for and use a microcode relocation patch */
 	if (cpm->reloc) {

@@ -293,8 +341,8 @@
 	tb = (u_char *)(((uint)tb + 15) & ~15);
 	*tb = abyte;		/* Device address byte w/rw flag */

-	flush_dcache_range(tb, tb+1);
-	flush_dcache_range(buf, buf+count);
+	flush_dcache_range((unsigned long)tb, (unsigned long)(tb+1));
+	flush_dcache_range((unsigned long)buf, (unsigned long)(buf+count));

 	if (cpm_debug) printk("cpm_iic_write(abyte=0x%x)\n", abyte);

@@ -309,31 +357,53 @@
 	tbdf[1].cbd_datlen = count;
 	tbdf[1].cbd_sc = BD_SC_READY | BD_SC_INTRPT | BD_SC_LAST | BD_SC_WRAP;

-	/* Chip bug, set enable here */
-	save_flags(flags); cli();
-	i2c->i2c_i2cmr = 0x13;	/* Enable some interupts */
-	i2c->i2c_i2cer = 0xff;
-	i2c->i2c_i2mod = 1;	/* Enable */
-	i2c->i2c_i2com = 0x81;	/* Start master */
+	if(count > 16){
+		/* Chip bug, set enable here */
+		local_irq_save(flags);
+		i2c->i2c_i2cmr = 0x13;	/* Enable some interupts */
+		i2c->i2c_i2cer = 0xff;
+		i2c->i2c_i2mod |= 1;	/* Enable */
+		i2c->i2c_i2com |= 0x80;	/* Begin transmission */
+
+		/* Wait for IIC transfer */
+		tmo = interruptible_sleep_on_timeout(&iic_wait,1*HZ);
+		local_irq_restore(flags);
+	} else {  /* busy wait for small transfers, its faster */
+		i2c->i2c_i2cmr = 0x00;	/* Disable I2C interupts */
+		i2c->i2c_i2cer = 0xff;
+		i2c->i2c_i2mod |= 1;	/* Enable */
+		i2c->i2c_i2com |= 0x80;	/* Begin transmission */
+		tmo = jiffies + 1*HZ;
+		while(!(i2c->i2c_i2cer & 0x12 || time_after(jiffies, tmo))); /* Busy wait, with a timeout */
+	}

-	/* Wait for IIC transfer */
-	interruptible_sleep_on(&iic_wait);
-	restore_flags(flags);
-	if (signal_pending(current))
+	if (signal_pending(current) || !tmo){
+		force_close(cpm);
+		if(cpm_debug && !tmo)
+			printk("IIC write: timeout!\n");
 		return -EIO;
-
+	}
+
+#if I2C_CHIP_ERRATA
+	/* Chip errata, clear enable. This is not needed on rev D4 CPUs.
+	 Disabling I2C too early may cause too short stop condition */
+	udelay(4);
+	i2c->i2c_i2mod &= ~1;
+#endif
 	if (cpm_debug) {
 		printk("tx0 sc %04x, tx1 sc %04x\n",
 		       tbdf[0].cbd_sc, tbdf[1].cbd_sc);
 	}

 	if (tbdf->cbd_sc & BD_SC_NAK) {
-		printk("IIC write; no ack\n");
+		if (cpm_debug)
+			printk("IIC write; no ack\n");
 		return 0;
 	}

 	if (tbdf->cbd_sc & BD_SC_READY) {
-		printk("IIC write; complete but tbuf ready\n");
+		if (cpm_debug)
+			printk("IIC write; complete but tbuf ready\n");
 		return 0;
 	}

@@ -351,7 +421,7 @@
 	volatile cpm8xx_t *cp = cpm->cp;
 	volatile cbd_t *tbdf, *rbdf;
 	u_char *tb;
-	unsigned long flags, len;
+	unsigned long flags, len, tmo;

 	if (cpm_debug > 1)
 		printk("cpm_iic_tryaddress(cpm=%p,addr=%d)\n", cpm, addr);

@@ -376,31 +446,43 @@
 	tb[0] = (addr << 1) | 1;	/* device address (+ read) */
 	len = 2;

-	flush_dcache_range(tb, tb+1);
+	flush_dcache_range((unsigned long)tb, (unsigned long)(tb+2));

 	tbdf->cbd_bufaddr = __pa(tb);
 	tbdf->cbd_datlen = len;
 	tbdf->cbd_sc =
-		BD_SC_READY | BD_SC_INTRPT | BD_SC_LAST |
+		BD_SC_READY | BD_SC_LAST |
 		BD_SC_WRAP | BD_IIC_START;

 	rbdf->cbd_datlen = 0;
 	rbdf->cbd_bufaddr = __pa(tb+2);
-	rbdf->cbd_sc = BD_SC_EMPTY | BD_SC_WRAP;
+	rbdf->cbd_sc = BD_SC_EMPTY | BD_SC_WRAP | BD_SC_INTRPT;

-	save_flags(flags); cli();
+	local_irq_save(flags);
 	i2c->i2c_i2cmr = 0x13;	/* Enable some interupts */
 	i2c->i2c_i2cer = 0xff;
-	i2c->i2c_i2mod = 1;	/* Enable */
-	i2c->i2c_i2com = 0x81;	/* Start master */
+	i2c->i2c_i2mod |= 1;	/* Enable */
+	i2c->i2c_i2com |= 0x80;	/* Begin transmission */

 	if (cpm_debug > 1) printk("about to sleep\n");

 	/* wait for IIC transfer */
-	interruptible_sleep_on(&iic_wait);
-	restore_flags(flags);
-	if (signal_pending(current))
+	tmo = interruptible_sleep_on_timeout(&iic_wait,1*HZ);
+	local_irq_restore(flags);
+
+#ifdef I2C_CHIP_ERRATA
+	/* Chip errata, clear enable. This is not needed on rev D4 CPUs.
+	 Disabling I2C too early may cause too short stop condition */
+	udelay(4);
+	i2c->i2c_i2mod &= ~1;
+#endif
+
+	if (signal_pending(current) || !tmo){
+		force_close(cpm);
+		if(cpm_debug && !tmo)
+			printk("IIC tryaddress: timeout!\n");
 		return -EIO;
+	}

 	if (cpm_debug > 1) printk("back from sleep\n");

@@ -430,8 +512,8 @@

 		if (cpm_debug)
 			printk("i2c-algo-8xx.o: "
-			       "#%d addr=0x%x flags=0x%x len=%d\n",
-			       i, pmsg->addr, pmsg->flags, pmsg->len);
+			       "#%d addr=0x%x flags=0x%x len=%d\n buf=%lx\n",
+			       i, pmsg->addr, pmsg->flags, pmsg->len, (unsigned long)pmsg->buf);

 		addr = pmsg->addr << 1;
 		if (pmsg->flags & I2C_M_RD )

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

On Tue, Oct 22, 2002 at 11:15:38AM +0200, Joakim Tjernlund wrote:

quoted

On Fri, Oct 11, 2002 at 11:12:02AM +0200, Joakim Tjernlund wrote:

quoted

OK, here it is.

Perhaps someone can add it to the ppctree?

If you can submit this as a patch vs what's in the i2c CVS tree or the
ppctree, than yes, this can be committed there, and to the ppctrees.

Here is a patch against the PPC 2.4 devel tree for the i2c-algo-8xx.c

Can you please split this into logical chunks, or give me a list of each
fix in here?  Thanks.

--
Tom Rini (TR1265)
http://gate.crashing.org/~trini/

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

quoted

Here is a patch against the PPC 2.4 devel tree for the i2c-algo-8xx.c

Can you please split this into logical chunks, or give me a list of each
fix in here?  Thanks.

ohh, kind of hard to remember all that went into that patch. The path
has evoled over many days, but I can try summarize. I have tested this code
pretty well it does not fail for me. Speed has been between 10-150KHz. The
old code fails as soon you start to stress it a little.

 - replace invalidate_dcache_range with flush_dcache_range, since buffers are
   NOT cache aligned. flush will write to memory AND invalidate the cache.

 - move the chip errata stuff from irq routine into read/write routines. Made
   it default off since it causes lock ups on my I2C device. I think it causes
   a too short STOP condition. If enabled I2C will behave better than before, but
   may still cause problems if the read/write is interrupted with a signal while
   microcode is enabled.

 - set default speed to 60 KHz instead.

 - missing/faulty initialization of parameter ram when I2C micro patch is active.

 - replaced assingments with mask operations with relevant bits. Example:
      /* Shut down IIC. */
      i2c->i2c_i2mod = 0;
	i2c->i2c_i2mod &= ~1;

 - When reading from I2C device, let the receive BD generate interrupt instead of
   the dummy trasmit. This is important since the TX interrupt will be too early sometimes, before
   the RX BD has closed. There is one case where the RX irq is before the TX irq, if iic_mrblr is set
   to match the number of bytes to read. Therefore must the iic_mrblr be one byte larger than
   the expected number of bytes.

 - busy wait for small transfers since it's faster.

 - save_flags(flags); cli(); cleanups

 - interruptible_sleep_on_timeout() instead of interruptible_sleep_on() so it won't hang forever if an
   irq is lost.

 Jocke


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Hi Tom

Will you apply this patch? No problems reported so far.

 Jocke

quoted

quoted

Here is a patch against the PPC 2.4 devel tree for the i2c-algo-8xx.c

Can you please split this into logical chunks, or give me a list of each
fix in here?  Thanks.

ohh, kind of hard to remember all that went into that patch. The path
has evoled over many days, but I can try summarize. I have tested this code
pretty well it does not fail for me. Speed has been between 10-150KHz. The
old code fails as soon you start to stress it a little.

 - replace invalidate_dcache_range with flush_dcache_range, since buffers are
   NOT cache aligned. flush will write to memory AND invalidate the cache.

 - move the chip errata stuff from irq routine into read/write routines. Made
   it default off since it causes lock ups on my I2C device. I think it causes
   a too short STOP condition. If enabled I2C will behave better than before, but
   may still cause problems if the read/write is interrupted with a signal while
   microcode is enabled.

 - set default speed to 60 KHz instead.

 - missing/faulty initialization of parameter ram when I2C micro patch is active.

 - replaced assingments with mask operations with relevant bits. Example:
      /* Shut down IIC. */
      i2c->i2c_i2mod = 0;
	i2c->i2c_i2mod &= ~1;

 - When reading from I2C device, let the receive BD generate interrupt instead of
   the dummy trasmit. This is important since the TX interrupt will be too early sometimes, before
   the RX BD has closed. There is one case where the RX irq is before the TX irq, if iic_mrblr is set
   to match the number of bytes to read. Therefore must the iic_mrblr be one byte larger than
   the expected number of bytes.

 - busy wait for small transfers since it's faster.

 - save_flags(flags); cli(); cleanups

 - interruptible_sleep_on_timeout() instead of interruptible_sleep_on() so it won't hang forever if an
   irq is lost.

 Jocke

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

On Fri, Nov 01, 2002 at 12:01:45PM +0100, Joakim Tjernlund wrote:

Hi Tom

Will you apply this patch? No problems reported so far.

I will try and do this next week.

--
Tom Rini (TR1265)
http://gate.crashing.org/~trini/

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Tom + Joakim,
we're doing a release soon over here at sensors/i2c,
do you want this checked in as-is, with mods, or not at all?

Joakim Tjernlund wrote:

quoted

quoted

Here is a patch against the PPC 2.4 devel tree for the i2c-algo-8xx.c

Can you please split this into logical chunks, or give me a list of each
fix in here?  Thanks.

ohh, kind of hard to remember all that went into that patch. The path
has evoled over many days, but I can try summarize. I have tested this code
pretty well it does not fail for me. Speed has been between 10-150KHz. The
old code fails as soon you start to stress it a little.

 - replace invalidate_dcache_range with flush_dcache_range, since buffers are
   NOT cache aligned. flush will write to memory AND invalidate the cache.

 - move the chip errata stuff from irq routine into read/write routines. Made
   it default off since it causes lock ups on my I2C device. I think it causes
   a too short STOP condition. If enabled I2C will behave better than before, but
   may still cause problems if the read/write is interrupted with a signal while
   microcode is enabled.

 - set default speed to 60 KHz instead.

 - missing/faulty initialization of parameter ram when I2C micro patch is active.

 - replaced assingments with mask operations with relevant bits. Example:
      /* Shut down IIC. */
      i2c->i2c_i2mod = 0;
        i2c->i2c_i2mod &= ~1;

 - When reading from I2C device, let the receive BD generate interrupt instead of
   the dummy trasmit. This is important since the TX interrupt will be too early sometimes, before
   the RX BD has closed. There is one case where the RX irq is before the TX irq, if iic_mrblr is set
   to match the number of bytes to read. Therefore must the iic_mrblr be one byte larger than
   the expected number of bytes.

 - busy wait for small transfers since it's faster.

 - save_flags(flags); cli(); cleanups

 - interruptible_sleep_on_timeout() instead of interruptible_sleep_on() so it won't hang forever if an
   irq is lost.

 Jocke

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

As-is for me. No problems reported to me.

         Joakim

Tom + Joakim,
we're doing a release soon over here at sensors/i2c,
do you want this checked in as-is, with mods, or not at all?

Joakim Tjernlund wrote:

quoted

quoted

quoted

Here is a patch against the PPC 2.4 devel tree for the i2c-algo-8xx.c

Can you please split this into logical chunks, or give me a list of each
fix in here?  Thanks.

ohh, kind of hard to remember all that went into that patch. The path
has evoled over many days, but I can try summarize. I have tested this code
pretty well it does not fail for me. Speed has been between 10-150KHz. The
old code fails as soon you start to stress it a little.

 - replace invalidate_dcache_range with flush_dcache_range, since buffers are
   NOT cache aligned. flush will write to memory AND invalidate the cache.

 - move the chip errata stuff from irq routine into read/write routines. Made
   it default off since it causes lock ups on my I2C device. I think it causes
   a too short STOP condition. If enabled I2C will behave better than before, but
   may still cause problems if the read/write is interrupted with a signal while
   microcode is enabled.

 - set default speed to 60 KHz instead.

 - missing/faulty initialization of parameter ram when I2C micro patch is active.

 - replaced assingments with mask operations with relevant bits. Example:
      /* Shut down IIC. */
      i2c->i2c_i2mod = 0;
        i2c->i2c_i2mod &= ~1;

 - When reading from I2C device, let the receive BD generate interrupt instead of
   the dummy trasmit. This is important since the TX interrupt will be too early sometimes, before
   the RX BD has closed. There is one case where the RX irq is before the TX irq, if iic_mrblr is set
   to match the number of bytes to read. Therefore must the iic_mrblr be one byte larger than
   the expected number of bytes.

 - busy wait for small transfers since it's faster.

 - save_flags(flags); cli(); cleanups

 - interruptible_sleep_on_timeout() instead of interruptible_sleep_on() so it won't hang forever if an
   irq is lost.

 Jocke

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

On Sun, Nov 17, 2002 at 10:44:40PM +0100, Joakim Tjernlund wrote:

quoted

Tom + Joakim,
we're doing a release soon over here at sensors/i2c,
do you want this checked in as-is, with mods, or not at all?

As-is for me. No problems reported to me.

Works for me then.

--
Tom Rini (TR1265)
http://gate.crashing.org/~trini/

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

whatever your patch was against didn't match our tree very well. would
you please
resolve the differences and generate a new patch against our tree?
thanks
mds

Tom Rini wrote:

On Sun, Nov 17, 2002 at 10:44:40PM +0100, Joakim Tjernlund wrote:

quoted

quoted

Tom + Joakim,
we're doing a release soon over here at sensors/i2c,
do you want this checked in as-is, with mods, or not at all?

As-is for me. No problems reported to me.

Works for me then.

--
Tom Rini (TR1265)
http://gate.crashing.org/~trini/

whatever your patch was against didn't match our tree very well. would
you please
resolve the differences and generate a new patch against our tree?
thanks
mds

Patch was(and still is, I think) against linuxppc_2_4_devel. Don't know your tree, is
it not the same as linuxppc? Anyhow, too much time has passed for me
to remember all details and I don't have the time to go back and do it all over again.
I suggest you take the linuxppc version, patch it and adopt it to your tree.

  Regards
          Jocke


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

On Mon, Nov 18, 2002 at 08:24:32PM +0100, Joakim Tjernlund wrote:

quoted

whatever your patch was against didn't match our tree very well. would
you please
resolve the differences and generate a new patch against our tree?
thanks
mds

Patch was(and still is, I think) against linuxppc_2_4_devel. Don't know your tree, is
it not the same as linuxppc? Anyhow, too much time has passed for me
to remember all details and I don't have the time to go back and do it all over again.
I suggest you take the linuxppc version, patch it and adopt it to your tree.

I'll go create a proper patch vs the i2c tree later today.

--
Tom Rini (TR1265)
http://gate.crashing.org/~trini/

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Patch was(and still is, I think) against linuxppc_2_4_devel. Don't
know your tree, is it not the same as linuxppc? Anyhow, too much time
has passed for me to remember all details and I don't have the time to
go back and do it all over again. I suggest you take the linuxppc
version, patch it and adopt it to your tree.

Why the hell are there two different trees? :'( Can't we work all
together for a better result?

--
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

On Mon, Nov 18, 2002 at 08:42:48PM +0100, Jean Delvare wrote:

quoted

Patch was(and still is, I think) against linuxppc_2_4_devel. Don't
know your tree, is it not the same as linuxppc? Anyhow, too much time
has passed for me to remember all details and I don't have the time to
go back and do it all over again. I suggest you take the linuxppc
version, patch it and adopt it to your tree.

Why the hell are there two different trees? :'( Can't we work all
together for a better result?

Because the linuxppc tree doesn't track the whole i2c tree, as that would
be quite silly. :)

--
Tom Rini (TR1265)
http://gate.crashing.org/~trini/

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/