Erratic MPC8248 CPM2 I2C behaviour

Erratic MPC8248 CPM2 I2C behaviour

[Laurent Pinchart]

Hi everybody,

I've been struggling with a weird CPM2 I2C behaviour for the past few days 
without much success.

Some background first. The platform is an MPC8248 (HiP7 Rev 14, Mask 1.0 
1K50M) system with an I2C bus on which two peripherals are connected. Back to 
the ppc architecture days, Heiko Schocher's i2c-mpc8260 out of tree driver 
worked like a charm.

After switching to the powerpc architecture and Jochen Friedrich's driver, 
I've experienced erratic transfer timeouts. Quite frequent at first (2.6.26), 
the problems disappeared on the road to 2.6.27 and have now resurfaced.

As the problem never occured with a ppc kernel I'm pretty sure we can rule out 
hardware issues. Bad hardware initialisation (either in U-Boot or in the Linux 
kernel) is possible but in my opinion very unlikely, as timeouts occur about 
once every 5 boots at most.

I've added debugging messages to the i2c-cpm driver to track down the issue. 
Here's a sample successful I2C transaction (1 byte write followed by 1 byte 
read) with comments inlined:

-----------------------------------------------------------------------------
> R: 0 T: 0
> Parsing write message (addr 0x90 len 1)
> R: 0 T: 1
> Parsing read message (addr 0x91 len 1)

The two messages making up the transaction are parsed. The driver fills a TX 
buffer descriptor for the first one, and a TX and an RX buffer descriptor for 
the second one.

> IRQ 0 - 00 00 00 00 00 00 00 00

No I2C IRQ occured so far, more on this later.

> rbase 0x01e0 tbase 0x01c0 rfcr 0x30 tfcr 0x30 mrblr 0x0201
> rstate 0x00000000 rptr 0x00000000 rbptr 0x01e0 rcount 0x0000 rtmp 0x00000000
> tstate 0x00000000 tptr 0x00000000 tbptr 0x01c0 tcount 0x0000 ttmp 0x00000000
> i2mod 0x00, i2add 0xfe i2brg 0x03 i2com 0x01 i2cer 0x00 i2cmr 0x00
> rx 0 sc 0x9000 tx 0 sc 0x9400
> rx 1 sc 0x7da4 tx 1 sc 0xac00
> rx 2 sc 0xefef tx 2 sc 0x743b
> rx 3 sc 0xf264 tx 3 sc 0x10e6

I2C controller registers and buffer descriptors state dump.

> Waiting on message 0.

The driver waits with wait_event_interruptible_timeout() for the end of the 
first message.

> IRQ 2 - 02 01 00 00 00 00 00 00

Two I2C interrupts occured, one for each message.

> rbase 0x01e0 tbase 0x01c0 rfcr 0x30 tfcr 0x30 mrblr 0x0201
> rstate 0x30000000 rptr 0x03905001 rbptr 0x01e8 rcount 0x0200 rtmp 0x00000000
> tstate 0x30000000 tptr 0x03920002 tbptr 0x01c0 tcount 0x0000 ttmp 0x91000000
> i2mod 0x01, i2add 0xfe i2brg 0x03 i2com 0x01 i2cer 0x00 i2cmr 0x13
> rx 0 sc 0x1800 tx 0 sc 0x1400
> rx 1 sc 0x7da4 tx 1 sc 0x2c00
> rx 2 sc 0xefef tx 2 sc 0x743b
> rx 3 sc 0xf264 tx 3 sc 0x10e6
> Message 0 completed.
> tx sc 0x1400
> Waiting on message 1.
> IRQ 2 - 02 01 00 00 00 00 00 00
> rbase 0x01e0 tbase 0x01c0 rfcr 0x30 tfcr 0x30 mrblr 0x0201
> rstate 0x30000000 rptr 0x03905001 rbptr 0x01e8 rcount 0x0200 rtmp 0x00000000
> tstate 0x30000000 tptr 0x03920002 tbptr 0x01c0 tcount 0x0000 ttmp 0x91000000
> i2mod 0x01, i2add 0xfe i2brg 0x03 i2com 0x01 i2cer 0x00 i2cmr 0x13
> rx 0 sc 0x1800 tx 0 sc 0x1400
> rx 1 sc 0x7da4 tx 1 sc 0x2c00
> rx 2 sc 0xefef tx 2 sc 0x743b
> rx 3 sc 0xf264 tx 3 sc 0x10e6
> Message 1 completed.
> tx sc 0x2c00, rx sc 0x1800

Both message have been successfully transmitted.
-----------------------------------------------------------------------------

I've rebooted the system, and the same transaction failed:

-----------------------------------------------------------------------------
> R: 0 T: 0
> Parsing write message (addr 0x90 len 1)
> R: 0 T: 1
> Parsing read message (addr 0x91 len 1)
> IRQ 0 - 00 00 00 00 00 00 00 00
> rbase 0x01e0 tbase 0x01c0 rfcr 0x30 tfcr 0x30 mrblr 0x0201
> rstate 0x00000000 rptr 0x00000000 rbptr 0x01e0 rcount 0x0000 rtmp 0x00000000
> tstate 0x00000000 tptr 0x00000000 tbptr 0x01c0 tcount 0x0000 ttmp 0x00000000
> i2mod 0x00, i2add 0xfe i2brg 0x03 i2com 0x01 i2cer 0x00 i2cmr 0x00
> rx 0 sc 0x9000 tx 0 sc 0x9400
> rx 1 sc 0x7da4 tx 1 sc 0xac00
> rx 2 sc 0xeeef tx 2 sc 0x74bb
> rx 3 sc 0x7264 tx 3 sc 0x50e6
> Waiting on message 0.
> IRQ 0 - 00 00 00 00 00 00 00 00
> rbase 0x01e0 tbase 0x01c0 rfcr 0x30 tfcr 0x30 mrblr 0x0201
> rstate 0x00000000 rptr 0x00000000 rbptr 0x01e0 rcount 0x0000 rtmp 0x00000000
> tstate 0x30e00000 tptr 0x03914002 tbptr 0x01c0 tcount 0x0001 ttmp 0x90010000
> i2mod 0x01, i2add 0xfe i2brg 0x03 i2com 0x01 i2cer 0x00 i2cmr 0x13
> rx 0 sc 0x9000 tx 0 sc 0x9400
> rx 1 sc 0x7da4 tx 1 sc 0xac00
> rx 2 sc 0xeeef tx 2 sc 0x74bb
> rx 3 sc 0x7264 tx 3 sc 0x50e6
> Message 0 timed out

Transmission timeout after one second. The first TX buffer descriptor status 
hasn't been modified by the CPM. The CPM state dump shows that processing of 
the first TX buffer has started, but the CPM stopped after the first byte 
(tcount is equal to 0x0001 and ttmp contains the 2 bytes to be transmitted). I 
assume the first byte has been written by the CPM to the I2C controller's 
register. Processing then stopped for an unknown reason.

> cpm_i2c_force_close()
-----------------------------------------------------------------------------

While trying to debug the issue I've added a dev_dbg() in the I2C interrupt 
handler. With that function being called by the interrupt handler I haven't 
been able to reproduce the problem. I thus removed the dev_dbg() call and 
replaced that with an interrupt counter and a table storing the first 8 
interrupts events, and printed all those information outside the interrupt 
handler after wait_for_event_interruptible.

Timing issues come to mind, but what I've found really puzzling is that the 
above log shows that the interrupt handler hasn't been called at all 
(confirmed by /proc/interrupts). Why would a call to dev_dbg() in a function 
that is never called modify execution timings ? This might just be a 
coincidence.

Has anyone run into the same kind of problem with the CPM2 I2C controller ? 
All help will really be appreciated.

Best regards,

-- 
Laurent Pinchart
CSE Semaphore Belgium

Chaussee de Bruxelles, 732A
B-1410 Waterloo
Belgium

T +32 (2) 387 42 59
F +32 (2) 387 42 75

Re: Erratic MPC8248 CPM2 I2C behaviour

[Wolfram Sang]

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

Hello Laurent,

> After switching to the powerpc architecture and Jochen Friedrich's driver, 
> I've experienced erratic transfer timeouts. Quite frequent at first (2.6.26), 
> the problems disappeared on the road to 2.6.27 and have now resurfaced.

Now == current linus.git?

Kind regards,

   Wolfram

-- 
  Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry

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

Re: Erratic MPC8248 CPM2 I2C behaviour

[Laurent Pinchart]

On Saturday 29 November 2008 06:41:53 Wolfram Sang wrote:
> Hello Laurent,
>
> > After switching to the powerpc architecture and Jochen Friedrich's
> > driver, I've experienced erratic transfer timeouts. Quite frequent at
> > first (2.6.26), the problems disappeared on the road to 2.6.27 and have
> > now resurfaced.
>
> Now == current linus.git?

Now = both 2.6.27 and 2.6.28-rc8.

Best regards,

-- 
Laurent Pinchart
CSE Semaphore Belgium

Chaussee de Bruxelles, 732A
B-1410 Waterloo
Belgium

T +32 (2) 387 42 59
F +32 (2) 387 42 75

Re: Erratic MPC8248 CPM2 I2C behaviour

[Mike Ditto]

Laurent Pinchart <laurentp@cse-semaphore.com> wrote:
> Transmission timeout after one second. The first TX buffer descriptor status 
> hasn't been modified by the CPM. The CPM state dump shows that processing of 
...

This sounds very similar to a problem I have seen on MPC8247 and
MPC8248.

It could be a variation of the CPM bug documented by Freescale as
erratum CPM98.  But the key difference is that we see a persistent
failure, while the erratum only mentions a problem with "the next
transaction".  What we see is that once the I2C engine gets confused
by some anomaly on the SCL signal, it stops processing buffer
descriptors entirely until it is turned off and back on.  You can
observe this bug by momentarily grounding the SCL line (it's an
open-collector bus) with a jumper while you attempt an I/O.

Our production equipment is using Linux 2.6 with the out-of-tree
i2c-algo-8260.c by Dan Malek and Brad Parker.  I modified this
driver to shut off the I2C controller and turn it back on when it
hits this problem, and now it can recover from this condition.

However, this does not explain how the controller gets into the
frozen state in the first place.  We see it very rarely in production
units and have not been able to identify the cause.  If it is
similar to erratum CPM98 then it could be noise on the SCL signal or
perhaps an I2C device that doesn't conform to the protocol perfectly.
Also beware, if you are using some kind of multiplexer, that you don't
direct the multiplexer to switch to a different bus (segment) while a
transaction is in progress.

In testing with the current (2.6.27) i2c-cpm.c driver, I found that
it is sufficient to #define I2C_CHIP_ERRATA to allow it to recover
from the CPM I2C freeze.  However, I don't know if I like this
approach because it turns off the I2C engine after every transfer,
even successful ones, and I don't know if this will affect reliability
or performance.  And I don't know if this will actually prevent the
freeze from happening, although I presume that it would protect the
I2C engine from getting confused by a glitch that happens while no
transfer is in progress.

I am not aware of any documentation for what exactly the I2C_CHIP_ERRATA
conditional code in i2c-cpm.c is meant to work around.  The comment
mentions "earlier than rev D4" but I'm not aware of any such rev for
8260 or 8272 chip families, and if it is related to erratum CPM98,
Freescale seems to say that this erratum is in all revs of these chips
and has no plan to fix it, so it seems that the workaround should be
enabled by default.

					-=] Mike [=-

Re: Erratic MPC8248 CPM2 I2C behaviour

[Mike Ditto]

I wrote:
> Our production equipment is using Linux 2.6 with the out-of-tree
> i2c-algo-8260.c by Dan Malek and Brad Parker.

Oops, I meant to say Linux 2.4.20 (MontaVista).

					-=] Mike [=-

Re: Erratic MPC8248 CPM2 I2C behaviour

[Mike Ditto]

(replying to myself again)

I wrote:
> But the key difference is that we see a persistent failure, while the
> erratum only mentions a problem with "the next transaction".

I think the timeout recovery code is not adequate for these CPM errors,
and that is why a transient error becomes a persistent one.  The
function cpm_i2c_force_close in i2c-cpm.c sends a CPM_CR_CLOSE_RX_BD
command, which will abort any receive transaction in progress, but if
it's the transmit phase that got the CPM hung up, there is no code to
abort that (and there does not exist a CP command to do so, as far as
I can tell).

So the I2C disable/enable seems to be an appropriate fix for the
persistent failure part of the problem.  Again, this doesn't explain
how it gets hung up the first time.

					-=] Mike [=-

Re: Erratic MPC8248 CPM2 I2C behaviour

[Joakim Tjernlund]

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

On Mon, 2008-12-01 at 15:28 -0800, Mike Ditto wrote:
> Laurent Pinchart <laurentp@cse-semaphore.com> wrote:
> > Transmission timeout after one second. The first TX buffer descriptor status 
> > hasn't been modified by the CPM. The CPM state dump shows that processing of 
> ...
> 
> This sounds very similar to a problem I have seen on MPC8247 and
> MPC8248.
> 
> It could be a variation of the CPM bug documented by Freescale as
> erratum CPM98.  But the key difference is that we see a persistent
> failure, while the erratum only mentions a problem with "the next
> transaction".  What we see is that once the I2C engine gets confused
> by some anomaly on the SCL signal, it stops processing buffer
> descriptors entirely until it is turned off and back on.  You can
> observe this bug by momentarily grounding the SCL line (it's an
> open-collector bus) with a jumper while you attempt an I/O.
> 
> Our production equipment is using Linux 2.6 with the out-of-tree
> i2c-algo-8260.c by Dan Malek and Brad Parker.  I modified this
> driver to shut off the I2C controller and turn it back on when it
> hits this problem, and now it can recover from this condition.
> 
> However, this does not explain how the controller gets into the
> frozen state in the first place.  We see it very rarely in production
> units and have not been able to identify the cause.  If it is
> similar to erratum CPM98 then it could be noise on the SCL signal or
> perhaps an I2C device that doesn't conform to the protocol perfectly.
> Also beware, if you are using some kind of multiplexer, that you don't
> direct the multiplexer to switch to a different bus (segment) while a
> transaction is in progress.
> 
> In testing with the current (2.6.27) i2c-cpm.c driver, I found that
> it is sufficient to #define I2C_CHIP_ERRATA to allow it to recover
> from the CPM I2C freeze.  However, I don't know if I like this
> approach because it turns off the I2C engine after every transfer,
> even successful ones, and I don't know if this will affect reliability
> or performance.  And I don't know if this will actually prevent the
> freeze from happening, although I presume that it would protect the
> I2C engine from getting confused by a glitch that happens while no
> transfer is in progress.
> 
> I am not aware of any documentation for what exactly the I2C_CHIP_ERRATA
> conditional code in i2c-cpm.c is meant to work around.

The I2C_CHIP_ERRATA is mine and refers to mpc8xx, mpc860 in my case. The
driver was adapted by me some years ago in 2.4 and now someone has
ported it to 2.6 it seems. From memory, you should check that mrblr is
one byte bigger than your max transfer size.
I have include my old driver that we still use in 2.4, as I recall, not
all bits made into the kernel.

 Jocke

[-- Attachment #2: i2c-algo-8xx.c --]
[-- Type: text/x-csrc, Size: 17460 bytes --]

/*
 * i2c-algo-8xx.c i2x driver algorithms for MPC8XX CPM
 * Copyright (c) 1999 Dan Malek (dmalek@jlc.net).
 *
 * moved into proper i2c interface; separated out platform specific 
 * parts into ic-rpx.c
 * Brad Parker (brad@heeltoe.com)
 */

/* $Id: i2c-algo-8xx.c,v 1.3 2001/11/21 17:20:22 jocke Exp $ */

#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/delay.h>
#include <linux/slab.h>
#include <linux/version.h>
#include <linux/init.h>
#include <asm/uaccess.h>
#include <linux/ioport.h>
#include <linux/errno.h>
#include <linux/sched.h>

#include <asm/mpc8xx.h>
#include <asm/commproc.h>

#include <linux/i2c.h>
#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;

static int cpm_scan = 0;
static int cpm_debug = 0;

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);
#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_i2cmr = 0;

	/* Get 'me going again.
	*/
#ifdef INTERRUPTIBLE
	wake_up_interruptible(&iic_wait);
#else
	wake_up(&iic_wait);
#endif
}

static void
cpm_iic_init(struct i2c_algo_8xx_data *cpm_adap)
{
	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");

	/* Initialize the parameter ram.
	 * We need to make sure many things are initialized to zero,
	 * especially in the case of a microcode patch.
	 */
	iip->iic_rstate = 0;
	iip->iic_rdp = 0;
	iip->iic_rbptr = 0;
	iip->iic_rbc = 0;
	iip->iic_rxtmp = 0;
	iip->iic_tstate = 0;
	iip->iic_tdp = 0;
	iip->iic_tbptr = 0;
	iip->iic_tbc = 0;
	iip->iic_txtmp = 0;

	/* Set up the IIC parameters in the parameter ram.
	*/
	iip->iic_tbase = r_tbase = cpm_adap->dp_addr;
	iip->iic_rbase = r_rbase = cpm_adap->dp_addr + sizeof(cbd_t)*2;

	iip->iic_tfcr = SMC_EB;
	iip->iic_rfcr = SMC_EB;

	/* Set maximum receive size.
	*/
	iip->iic_mrblr = CPM_MAX_READ;

	/* Initialize Tx/Rx parameters.
	*/
	if (cpm_adap->reloc == 0) {
		volatile cpm8xx_t *cp = cpm_adap->cp;

		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 = 0xfe;

	/* Make clock run at 80 KHz.
	*/
	brg = (unsigned char) (bd->bi_intfreq/(32*2*80000) -3);
	i2c->i2c_i2brg = brg;

	i2c->i2c_i2mod = 0x00; 
	i2c->i2c_i2com = 0x01; /* Master mode */

	/* Disable interrupts.
	*/
	i2c->i2c_i2cmr = 0;
	i2c->i2c_i2cer = 0xff;

	init_waitqueue_head(&iic_wait);

	/* Install interrupt handler.
	*/
	cpm_install_handler(CPMVEC_I2C, cpm_iic_interrupt, (void *)i2c);
}


static int
cpm_iic_shutdown(struct i2c_algo_8xx_data *cpm_adap)
{
	volatile i2c8xx_t *i2c = cpm_adap->i2c;

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

	return(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)
{
	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");
		cp->cp_cpcr =
			mk_cr_cmd(CPM_CR_CH_I2C, CPM_CR_CLOSE_RXBD) |
			CPM_CR_FLG;

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

#define __wait_event_interruptible_tmo(wq, condition, ret)		\
do {									\
	wait_queue_t __wait;						\
	init_waitqueue_entry(&__wait, current);				\
									\
	add_wait_queue(&wq, &__wait);					\
	for (;;) {							\
		set_current_state(TASK_INTERRUPTIBLE);			\
		if (condition)						\
			break;						\
		if (!signal_pending(current)) {				\
			ret = schedule_timeout(ret);			\
			if (!ret)					\
				break;					\
			continue;					\
		}							\
		ret = -ERESTARTSYS;					\
		break;							\
	}								\
	current->state = TASK_RUNNING;					\
	remove_wait_queue(&wq, &__wait);				\
} while (0)
	
#define wait_event_interruptible_tmo(wq, condition, tmo)		\
({									\
	long __ret = tmo;						\
	if (!(condition))						\
		__wait_event_interruptible_tmo(wq, condition, __ret);	\
	__ret;								\
})

void
cpm_reset_i2c(volatile iic_t *iip, volatile i2c8xx_t *i2c)
{
	// Disable interrupts.
	i2c->i2c_i2cmr = 0;
	i2c->i2c_i2cer = 0xff;
	// Clear enable
	i2c->i2c_i2mod &= ~1;
	// Reset internal state
	iip->iic_rstate = 0;
	iip->iic_tstate = 0;
	iip->iic_rbptr = iip->iic_rbase;
	iip->iic_tbptr = iip->iic_tbase;
}
/* Read from IIC...
 * abyte = address byte, with r/w flag already set
 */
static int
cpm_iic_read(struct i2c_algo_8xx_data *cpm, u_char abyte, char *buf, int count)
{
	volatile iic_t *iip = cpm->iip;
	volatile i2c8xx_t *i2c = cpm->i2c;
	volatile cpm8xx_t *cp = cpm->cp;
	volatile cbd_t	*tbdf, *rbdf;
	long tmo;

	if (count >= CPM_MAX_READ)
		return -EINVAL;

	tbdf = (cbd_t *)&cp->cp_dpmem[iip->iic_tbase];
	rbdf = (cbd_t *)&cp->cp_dpmem[iip->iic_rbase];

	/* To read, we need an empty buffer of the proper length.
	 * All that is used is the first byte for address, the remainder
	 * is just used for timing (and doesn't really have to exist).
	 */

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

	tbdf->cbd_bufaddr = __pa(&abyte); /* Device address byte w/rw flag */
	tbdf->cbd_datlen = count + 1;
	tbdf->cbd_sc =	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| BD_SC_INTRPT;
	/* Chip bug, set enable here */
	i2c->i2c_i2cer = 0xff;  /* Clear all pending interrupts */
	i2c->i2c_i2cmr = 0x15;	/* Enable some interrupts */
	i2c->i2c_i2mod |= 1;	/* Enable */
	i2c->i2c_i2com |= 0x80 | 0x01;	/* Begin transmission and force master.
					 * Some errors(CL) clears the M/S bit */
	/* Wait for IIC transfer */
#ifdef INTERRUPTIBLE
	tmo = wait_event_interruptible_tmo(iic_wait, i2c->i2c_i2cer, HZ);
#else
	tmo = sleep_on_timeout(&iic_wait,1*HZ);
#endif
#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

#ifdef INTERRUPTIBLE
	if (signal_pending(current)){
		force_close(cpm);
		return -EIO;
	}
#endif
	if (!tmo) {
		if (cpm_debug)
			printk("IIC read: timeout\n");
		cpm_reset_i2c(iip, i2c);
		
		//return -ETIMEDOUT; /* timeout */
		return -EREMOTEIO;
	}
	if (tbdf->cbd_sc & (BD_SC_READY | BD_SC_NAK | BD_SC_UN | BD_SC_CL)) {
		if (cpm_debug && tbdf->cbd_sc & BD_SC_READY)
			printk("IIC read; complete but tbuf ready\n");
		if (cpm_debug && tbdf->cbd_sc & BD_SC_NAK)
			printk("IIC read; no ack\n");
		if (cpm_debug && tbdf->cbd_sc & BD_SC_UN)
			printk("IIC read; TX underrun\n");
		if (cpm_debug && tbdf->cbd_sc & BD_SC_CL)
			printk("IIC read; TX collision\n");
		cpm_reset_i2c(iip, i2c);

		return -EREMOTEIO;
	}
	if (rbdf->cbd_sc & (BD_SC_EMPTY | BD_SC_OV)) {
		if (cpm_debug && rbdf->cbd_sc & BD_SC_EMPTY)
			printk("IIC read; complete but rbuf empty\n");
		if (cpm_debug && rbdf->cbd_sc &  BD_SC_OV)
			printk("IIC read; Overrun\n");
		cpm_reset_i2c(iip, i2c);

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

		return -EREMOTEIO;
	}
	return count;
}

/* Write to IIC...
 * addr = address byte, with r/w flag already set
 */
static int
cpm_iic_write(struct i2c_algo_8xx_data *cpm, u_char abyte, char *buf,int count)
{
	volatile iic_t *iip = cpm->iip;
	volatile i2c8xx_t *i2c = cpm->i2c;
	volatile cpm8xx_t *cp = cpm->cp;
	volatile cbd_t	*tbdf;
	long tmo;

	clean_dcache_range((unsigned long)&abyte, (unsigned long)(&abyte+1));
	flush_dcache_range((unsigned long)buf, (unsigned long)(buf+count));

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

	/* set up 2 descriptors */
	tbdf = (cbd_t *)&cp->cp_dpmem[iip->iic_tbase];

	tbdf[0].cbd_bufaddr = __pa(&abyte);
	tbdf[0].cbd_datlen = 1;
	tbdf[0].cbd_sc = BD_SC_READY | BD_IIC_START;

	tbdf[1].cbd_bufaddr = __pa(buf);
	tbdf[1].cbd_datlen = count;
	tbdf[1].cbd_sc = BD_SC_READY | BD_SC_INTRPT | BD_SC_LAST | BD_SC_WRAP;

	i2c->i2c_i2cer = 0xff;
	i2c->i2c_i2cmr = 0x16;	/* Enable some interupts */
	/* Chip bug, set enable here */
	i2c->i2c_i2mod |= 1;	/* Enable */
	i2c->i2c_i2com |= 0x80 | 0x01;	/* Begin transmission and force master.
					 * Some errors(CL) clears the M/S bit */
	/* Wait for IIC transfer */
#ifdef INTERRUPTIBLE
	tmo = wait_event_interruptible_tmo(iic_wait, i2c->i2c_i2cer, HZ);
#else
	tmo = sleep_on_timeout(&iic_wait,1*HZ);
#endif
#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
#ifdef INTERRUPTIBLE
	if (signal_pending(current)){
		force_close(cpm);
		return -EIO;
	}
#endif
	if (!tmo) {
		if (cpm_debug)
			printk("IIC write: timeout\n");
		cpm_reset_i2c(iip, i2c);

		//return -ETIMEDOUT; /* timeout */
		return -EREMOTEIO;
	}

	if (tbdf->cbd_sc & (BD_SC_READY | BD_SC_NAK | BD_SC_UN | BD_SC_CL)) {
		if (cpm_debug && tbdf->cbd_sc & BD_SC_READY) 
			printk("IIC write; complete but tbuf ready\n");
		if (cpm_debug && tbdf->cbd_sc & BD_SC_NAK) 
			printk("IIC write; no ack\n");
		if (cpm_debug && tbdf->cbd_sc & BD_SC_UN)
			printk("IIC write; TX underrun\n");
		if (cpm_debug && tbdf->cbd_sc & BD_SC_CL)
			printk("IIC write; TX collision\n");
		cpm_reset_i2c(iip, i2c);
		return -EREMOTEIO;
	}	  
	if (tbdf[1].cbd_sc & (BD_SC_READY | BD_SC_NAK | BD_SC_UN | BD_SC_CL)) {
		if (cpm_debug && tbdf[1].cbd_sc & BD_SC_READY) 
			printk("IIC write2; complete but tbuf ready\n");
		if (cpm_debug && tbdf[1].cbd_sc & BD_SC_NAK) 
			printk("IIC write2; no ack\n");
		if (cpm_debug && tbdf->cbd_sc & BD_SC_UN)
			printk("IIC write2; TX underrun\n");
		if (cpm_debug && tbdf->cbd_sc & BD_SC_CL)
			printk("IIC write2; TX collision\n");
		cpm_reset_i2c(iip, i2c);
		return -EREMOTEIO;
	}	  
	return count;
}

/* See if an IIC address exists..
 * addr = 7 bit address, unshifted
 */
static int
cpm_iic_tryaddress(struct i2c_algo_8xx_data *cpm, int addr)
{
	volatile iic_t *iip = cpm->iip;
	volatile i2c8xx_t *i2c = cpm->i2c;
	volatile cpm8xx_t *cp = cpm->cp;
	volatile cbd_t *tbdf, *rbdf;
	u_char *tb;
	unsigned long len;

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

	if (cpm_debug && addr == 0) {
		printk("iip %p, dp_addr 0x%x\n", cpm->iip, cpm->dp_addr);
		printk("iic_tbase %d, r_tbase %d\n", iip->iic_tbase, r_tbase);
	}

	tbdf = (cbd_t *)&cp->cp_dpmem[iip->iic_tbase];
	rbdf = (cbd_t *)&cp->cp_dpmem[iip->iic_rbase];

	tb = cpm->temp;

	/* do a simple read */
	tb[0] = (addr << 1) | 1;	/* device address (+ read) */
	len = 2;

	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_LAST |
		BD_SC_WRAP | BD_IIC_START;

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

	i2c->i2c_i2cmr = 0x0;	/* Disable all interupts */
	i2c->i2c_i2cer = 0xff;
	i2c->i2c_i2mod |= 1;	/* Enable */
	i2c->i2c_i2com |= 0x80;	/* Begin transmission */

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

	/* wait for IIC transfer */
	while(!i2c->i2c_i2cer); /* Busy wait*/
#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)){
		force_close(cpm);
		return -EIO;
	}

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

	if (tbdf->cbd_sc & BD_SC_NAK) {
		if (cpm_debug > 1) printk("IIC try; no ack\n");
		return 0;
	}
	  
	if (tbdf->cbd_sc & BD_SC_READY) {
		printk("IIC try; complete but tbuf ready\n");
	}
	
	return 1;
}

static int cpm_xfer(struct i2c_adapter *i2c_adap,
		    struct i2c_msg msgs[], 
		    int num)
{
	struct i2c_algo_8xx_data *adap = i2c_adap->algo_data;
	struct i2c_msg *pmsg;
	int i, ret;
	u_char addr;
	bd_t *bd = (bd_t *)__res;
    
	for (i = 0; i < num; i++) {
		pmsg = &msgs[i];

		if (cpm_debug)
			printk("cmp_xfer: "
			       "#%d addr=0x%x flags=0x%x len=%d\n buf=%lx\n",
			       i, pmsg->addr, pmsg->flags, pmsg->len, (unsigned long)pmsg->buf);
		if (bd->board_type == CU_BOARDTYPE || bd->board_type == CU2_BOARDTYPE) {
			if (pmsg->addr & 0x8000)
				cpmp->cp_pbdat |=  0x00000002;
			else
				cpmp->cp_pbdat &= ~0x00000002;
		} else {
			cpmp->cp_pbdat &= ~0x00000600; /* set to low */
			if (pmsg->addr & 0x8000)
				cpmp->cp_pbdat |= 0x00000400;
			if (pmsg->addr & 0x4000)
				cpmp->cp_pbdat |= 0x00000200;
		}
		addr = (pmsg->addr & ~0xc000) << 1;
		if (pmsg->flags & I2C_M_RD )
			addr |= 1;
		if (pmsg->flags & I2C_M_REV_DIR_ADDR )
			addr ^= 1;
    
		if (!(pmsg->flags & I2C_M_NOSTART)) {
		}
		if (pmsg->flags & I2C_M_RD ) {
			/* read bytes into buffer*/
			ret = cpm_iic_read(adap, addr, pmsg->buf, pmsg->len);
			if (cpm_debug)
				printk("i2c-algo-8xx.o: read %d bytes\n", ret);
			if (ret < pmsg->len ) {
				return (ret<0)? ret : -EREMOTEIO;
			}
		} else {
			/* write bytes from buffer */
			ret = cpm_iic_write(adap, addr, pmsg->buf, pmsg->len);
			if (cpm_debug)
				printk("i2c-algo-8xx.o: wrote %d\n", ret);
			if (ret < pmsg->len ) {
				return (ret<0) ? ret : -EREMOTEIO;
			}
		}
	}
	return (num);
}

#define I2C_BITRATE 0x070f
static int algo_control(struct i2c_adapter *adapter, 
	unsigned int cmd, unsigned long arg)
{
	struct i2c_algo_8xx_data *adap = adapter->algo_data;
	volatile i2c8xx_t *i2c = adap->i2c;
	unsigned char brg;
	bd_t *bd = (bd_t *)__res;

	switch(cmd){
	case I2C_BITRATE:
		brg = (unsigned char) (bd->bi_intfreq/(32*2*arg) -3);
		printk("IntFreq: %lu    I2C brg: %lu\n", bd->bi_intfreq, (unsigned long)brg);
		i2c->i2c_i2brg = brg;
		return 0;
		break;
	default:
		return -EINVAL;
	}
}

static u32 cpm_func(struct i2c_adapter *adap)
{
	return I2C_FUNC_SMBUS_EMUL | I2C_FUNC_10BIT_ADDR | 
	       I2C_FUNC_PROTOCOL_MANGLING; 
}

/* -----exported algorithm data: -------------------------------------	*/

static struct i2c_algorithm cpm_algo = {
	"MPX8XX CPM algorithm",
	I2C_ALGO_MPC8XX,
	cpm_xfer,
	NULL,
	NULL,				/* slave_xmit		*/
	NULL,				/* slave_recv		*/
	algo_control,			/* ioctl		*/
	cpm_func,			/* functionality	*/
};

/* 
 * registering functions to load algorithms at runtime 
 */
int i2c_8xx_add_bus(struct i2c_adapter *adap)
{
	int i;
	struct i2c_algo_8xx_data *cpm_adap = adap->algo_data;

	if (cpm_debug)
		printk("i2c-algo-8xx.o: hw routines for %s registered.\n",
		       adap->name);

	/* register new adapter to i2c module... */

	adap->id |= cpm_algo.id;
	adap->algo = &cpm_algo;

#ifdef MODULE
	MOD_INC_USE_COUNT;
#endif

	i2c_add_adapter(adap);
	cpm_iic_init(cpm_adap);

	/* scan bus */
	if (cpm_scan) {
		printk(KERN_INFO " i2c-algo-8xx.o: scanning bus %s...\n",
		       adap->name);
		for (i = 0; i < 128; i++) {
			if (cpm_iic_tryaddress(cpm_adap, i)) {
				printk("(%02x)",i<<1); 
			}
		}
		printk("\n");
	}
	return 0;
}


int i2c_8xx_del_bus(struct i2c_adapter *adap)
{
	int res;
	struct i2c_algo_8xx_data *cpm_adap = adap->algo_data;

	cpm_iic_shutdown(cpm_adap);

	if ((res = i2c_del_adapter(adap)) < 0)
		return res;

	printk("i2c-algo-8xx.o: adapter unregistered: %s\n",adap->name);

#ifdef MODULE
	MOD_DEC_USE_COUNT;
#endif
	return 0;
}

EXPORT_SYMBOL(i2c_8xx_add_bus);
EXPORT_SYMBOL(i2c_8xx_del_bus);

int __init i2c_algo_8xx_init (void)
{
	printk("i2c-algo-8xx.o: i2c mpc8xx algorithm module\n");
	return 0;
}


#ifdef MODULE
MODULE_AUTHOR("Brad Parker <brad@heeltoe.com>");
MODULE_DESCRIPTION("I2C-Bus MPC8XX algorithm");

int init_module(void) 
{
	return i2c_algo_8xx_init();
}

void cleanup_module(void) 
{
}
#endif

Re: Erratic MPC8248 CPM2 I2C behaviour

[Laurent Pinchart]

Hi Joakim,

On Tuesday 02 December 2008 09:39:59 Joakim Tjernlund wrote:
> On Mon, 2008-12-01 at 15:28 -0800, Mike Ditto wrote:
> > Laurent Pinchart <laurentp@cse-semaphore.com> wrote:
> > > Transmission timeout after one second. The first TX buffer descriptor
> > > status hasn't been modified by the CPM. The CPM state dump shows that
> > > processing of
> >
> > ...
> >
> > This sounds very similar to a problem I have seen on MPC8247 and
> > MPC8248.
> >
> > It could be a variation of the CPM bug documented by Freescale as
> > erratum CPM98.  But the key difference is that we see a persistent
> > failure, while the erratum only mentions a problem with "the next
> > transaction".  What we see is that once the I2C engine gets confused
> > by some anomaly on the SCL signal, it stops processing buffer
> > descriptors entirely until it is turned off and back on.  You can
> > observe this bug by momentarily grounding the SCL line (it's an
> > open-collector bus) with a jumper while you attempt an I/O.
> >
> > Our production equipment is using Linux 2.6 with the out-of-tree
> > i2c-algo-8260.c by Dan Malek and Brad Parker.  I modified this
> > driver to shut off the I2C controller and turn it back on when it
> > hits this problem, and now it can recover from this condition.
> >
> > However, this does not explain how the controller gets into the
> > frozen state in the first place.  We see it very rarely in production
> > units and have not been able to identify the cause.  If it is
> > similar to erratum CPM98 then it could be noise on the SCL signal or
> > perhaps an I2C device that doesn't conform to the protocol perfectly.
> > Also beware, if you are using some kind of multiplexer, that you don't
> > direct the multiplexer to switch to a different bus (segment) while a
> > transaction is in progress.
> >
> > In testing with the current (2.6.27) i2c-cpm.c driver, I found that
> > it is sufficient to #define I2C_CHIP_ERRATA to allow it to recover
> > from the CPM I2C freeze.  However, I don't know if I like this
> > approach because it turns off the I2C engine after every transfer,
> > even successful ones, and I don't know if this will affect reliability
> > or performance.  And I don't know if this will actually prevent the
> > freeze from happening, although I presume that it would protect the
> > I2C engine from getting confused by a glitch that happens while no
> > transfer is in progress.
> >
> > I am not aware of any documentation for what exactly the I2C_CHIP_ERRATA
> > conditional code in i2c-cpm.c is meant to work around.
>
> The I2C_CHIP_ERRATA is mine and refers to mpc8xx, mpc860 in my case. The
> driver was adapted by me some years ago in 2.4 and now someone has
> ported it to 2.6 it seems. From memory, you should check that mrblr is
> one byte bigger than your max transfer size.

I've checked that, there's no problem there. The transfer that fails doesn't 
involve any rx buffer anyway, so mrblr is irrelevant here.

> I have include my old driver that we still use in 2.4, as I recall, not
> all bits made into the kernel.

Thanks. The out-of-tree i2c-mpc8260.c driver that I was using with the ppc 
architecture seems to have been a port of your code. It has been replaced by 
the i2c-cpm.c driver in the powerpc architecture. Both drivers use a similar 
approach, but the i2c-cpm.c driver allocates rx/tx buffers differently.

I spent some more time last Friday investigating the problem, and it seems the 
baud rate generator configuration plays an important role. The default 
configuration (60kHz nominal => 65.104kHz using a 25MHz brg clock and a /32 
predivider) leads to timeouts, while I haven't been able to reproduce the 
problem with the i2c-mpc8260.c configuration (100kHz nominal => 104.167kHz 
using a 25MHz brg clock and a /8 predivider).

This might be a coincidence, and I can't verify any of the results right now 
as my system decide to work correctly even at 60kHz :-/ I hope I won't have to 
through clock jitter and system temperature into the mix, otherwise this is 
going to be very complex to debug.

Best regards,

-- 
Laurent Pinchart
CSE Semaphore Belgium

Chaussee de Bruxelles, 732A
B-1410 Waterloo
Belgium

T +32 (2) 387 42 59
F +32 (2) 387 42 75

Re: Erratic MPC8248 CPM2 I2C behaviour

[Laurent Pinchart]

Hi Mike,

On Tuesday 02 December 2008 00:28:23 Mike Ditto wrote:
> Laurent Pinchart <laurentp@cse-semaphore.com> wrote:
> > Transmission timeout after one second. The first TX buffer descriptor
> > status hasn't been modified by the CPM. The CPM state dump shows that
> > processing of
>
> ...
>
> This sounds very similar to a problem I have seen on MPC8247 and
> MPC8248.
>
> It could be a variation of the CPM bug documented by Freescale as
> erratum CPM98. But the key difference is that we see a persistent
> failure, while the erratum only mentions a problem with "the next
> transaction".  What we see is that once the I2C engine gets confused
> by some anomaly on the SCL signal, it stops processing buffer
> descriptors entirely until it is turned off and back on.  You can
> observe this bug by momentarily grounding the SCL line (it's an
> open-collector bus) with a jumper while you attempt an I/O.

The I2C controller seems to be split into two parts, a firmware CPM code that 
processes buffers and a hardware shift register to handle the actual I2C 
transfers. My guess is that the hardware seems to get confused for some reason 
and times out. The CPM parameter RAM shows that the CPM started processing the 
tx buffer and stopped, probably waiting for the hardware to transfer the first 
byte.

> Our production equipment is using Linux 2.6 with the out-of-tree
> i2c-algo-8260.c by Dan Malek and Brad Parker.  I modified this
> driver to shut off the I2C controller and turn it back on when it
> hits this problem, and now it can recover from this condition.

I'm running Linux 2.6.27 with the in-tree i2c-cpm.c driver.

> However, this does not explain how the controller gets into the
> frozen state in the first place.  We see it very rarely in production
> units and have not been able to identify the cause.  If it is
> similar to erratum CPM98 then it could be noise on the SCL signal or
> perhaps an I2C device that doesn't conform to the protocol perfectly.

I don't believe noise is the cause here. I've been able to reproduce the 
problem tens of times in a row last Friday in a clean environment.

What bothers me though is that earlier experiments with an oscilloscope showed 
the problem would disappear when the scope probes were connected to the SCL 
and SDA signals. This made the problem more complex to debug, and it could 
have been a coincidence.

> Also beware, if you are using some kind of multiplexer, that you don't
> direct the multiplexer to switch to a different bus (segment) while a
> transaction is in progress.

There's no multiplexer, but thanks for the hint.

> In testing with the current (2.6.27) i2c-cpm.c driver, I found that
> it is sufficient to #define I2C_CHIP_ERRATA to allow it to recover
> from the CPM I2C freeze.  However, I don't know if I like this
> approach because it turns off the I2C engine after every transfer,
> even successful ones, and I don't know if this will affect reliability
> or performance.  And I don't know if this will actually prevent the
> freeze from happening,

It won't prevent the problem from happening, as I've been able to reproduce 
the issue on the very first I2C transfer.

> although I presume that it would protect the
> I2C engine from getting confused by a glitch that happens while no
> transfer is in progress.

Good point. I haven't thought about the controller becoming confused by SCL 
glitches outside of I2C transfers.

> I am not aware of any documentation for what exactly the I2C_CHIP_ERRATA
> conditional code in i2c-cpm.c is meant to work around.  The comment
> mentions "earlier than rev D4" but I'm not aware of any such rev for
> 8260 or 8272 chip families, and if it is related to erratum CPM98,
> Freescale seems to say that this erratum is in all revs of these chips
> and has no plan to fix it, so it seems that the workaround should be
> enabled by default.

While the problem seems to be similar to CPM98, I don't understand how it 
could happen on the first character of the first I2C transfer.

As explained in my previous mail to Joakim, I spent some more time last Friday 
investigating the problem, and it seems the baud rate generator configuration 
plays an important role. The default configuration (60kHz nominal => 65.104kHz 
using a 25MHz brg clock and a /32 predivider) leads to timeouts, while I 
haven't been able to reproduce the problem with the i2c-mpc8260.c 
configuration (100kHz nominal => 104.167kHz using a 25MHz brg clock and a /8 
predivider).

This might be a coincidence, and I can't verify any of the results right now 
as my system decide to work correctly even at 60kHz :-/ I hope I won't have to 
through clock jitter and system temperature into the mix, otherwise this is 
going to be very complex to debug.

Best regards,

-- 
Laurent Pinchart
CSE Semaphore Belgium

Chaussee de Bruxelles, 732A
B-1410 Waterloo
Belgium

T +32 (2) 387 42 59
F +32 (2) 387 42 75

Re: Erratic MPC8248 CPM2 I2C behaviour

[Joakim Tjernlund]

On Tue, 2008-12-02 at 11:50 +0100, Laurent Pinchart wrote:
> Hi Joakim,
> 
> On Tuesday 02 December 2008 09:39:59 Joakim Tjernlund wrote:
> > On Mon, 2008-12-01 at 15:28 -0800, Mike Ditto wrote:
> > > Laurent Pinchart <laurentp@cse-semaphore.com> wrote:
> > > > Transmission timeout after one second. The first TX buffer descriptor
> > > > status hasn't been modified by the CPM. The CPM state dump shows that
> > > > processing of
> > >
> > > ...
> > >
> > > This sounds very similar to a problem I have seen on MPC8247 and
> > > MPC8248.
> > >
> > > It could be a variation of the CPM bug documented by Freescale as
> > > erratum CPM98.  But the key difference is that we see a persistent
> > > failure, while the erratum only mentions a problem with "the next
> > > transaction".  What we see is that once the I2C engine gets confused
> > > by some anomaly on the SCL signal, it stops processing buffer
> > > descriptors entirely until it is turned off and back on.  You can
> > > observe this bug by momentarily grounding the SCL line (it's an
> > > open-collector bus) with a jumper while you attempt an I/O.
> > >
> > > Our production equipment is using Linux 2.6 with the out-of-tree
> > > i2c-algo-8260.c by Dan Malek and Brad Parker.  I modified this
> > > driver to shut off the I2C controller and turn it back on when it
> > > hits this problem, and now it can recover from this condition.
> > >
> > > However, this does not explain how the controller gets into the
> > > frozen state in the first place.  We see it very rarely in production
> > > units and have not been able to identify the cause.  If it is
> > > similar to erratum CPM98 then it could be noise on the SCL signal or
> > > perhaps an I2C device that doesn't conform to the protocol perfectly.
> > > Also beware, if you are using some kind of multiplexer, that you don't
> > > direct the multiplexer to switch to a different bus (segment) while a
> > > transaction is in progress.
> > >
> > > In testing with the current (2.6.27) i2c-cpm.c driver, I found that
> > > it is sufficient to #define I2C_CHIP_ERRATA to allow it to recover
> > > from the CPM I2C freeze.  However, I don't know if I like this
> > > approach because it turns off the I2C engine after every transfer,
> > > even successful ones, and I don't know if this will affect reliability
> > > or performance.  And I don't know if this will actually prevent the
> > > freeze from happening, although I presume that it would protect the
> > > I2C engine from getting confused by a glitch that happens while no
> > > transfer is in progress.
> > >
> > > I am not aware of any documentation for what exactly the I2C_CHIP_ERRATA
> > > conditional code in i2c-cpm.c is meant to work around.
> >
> > The I2C_CHIP_ERRATA is mine and refers to mpc8xx, mpc860 in my case. The
> > driver was adapted by me some years ago in 2.4 and now someone has
> > ported it to 2.6 it seems. From memory, you should check that mrblr is
> > one byte bigger than your max transfer size.
> 
> I've checked that, there's no problem there. The transfer that fails doesn't 
> involve any rx buffer anyway, so mrblr is irrelevant here.

hmm, as I recall that doesn't matter. Mrblr should always be > transfer
size. This was for 8xx though so maybe it doesn't matter for you.
Try setting mrblr = transfer size, see if that trigger any problems.

 Jocke

Re: Erratic MPC8248 CPM2 I2C behaviour

[Mike Ditto]

Hi, Laurent,

> While the problem seems to be similar to CPM98, I don't understand how it 
> could happen on the first character of the first I2C transfer.

I agree, that is hard to explain given that i2c-cpm keeps the controller
shut off until the very moment of the first transfer.  Can you check that
the bus is really idle (SCL is high) when the timeout happens?  If one of
the slave devices is in a bad state and pulling SCL low, I think the
behavior you see is expected (CPM will wait forever for the bus to be
idle).  The same is probably true if a GPIO pin is mis-configured.

> As explained in my previous mail to Joakim, I spent some more time last Friday 
> investigating the problem, and it seems the baud rate generator configuration 
> plays an important role. The default configuration (60kHz nominal => 65.104kHz 
> using a 25MHz brg clock and a /32 predivider) leads to timeouts, while I 
> haven't been able to reproduce the problem with the i2c-mpc8260.c 
> configuration (100kHz nominal => 104.167kHz using a 25MHz brg clock and a /8 
> predivider).

It would be interesting to try each driver with the other's clock settings.

					-=] Mike [=-

Re: Erratic MPC8248 CPM2 I2C behaviour

[Mike Ditto]

Jocke,

> The I2C_CHIP_ERRATA is mine and refers to mpc8xx, mpc860 in my case. The
> driver was adapted by me some years ago in 2.4 and now someone has
> ported it to 2.6 it seems.

Thanks for the background info.  Any idea which particular errata it
was meant to fix?  I took a quick look at MPC860 errata document and it
seems that CPM5 and CPM6 both suggest this workaround.

I don't think any of the 8xx CPM errata are present on 82xx, but it
would be good to enable the same or similar workaround on CPM2 for
82xx erratum CPM98, probably even when I2C_CHIP_ERRATA is not defined.

					-=] Mike [=-

Re: Erratic MPC8248 CPM2 I2C behaviour

[Jochen Friedrich]

Hi Laurent,

> The two messages making up the transaction are parsed. The driver fills a TX 
> buffer descriptor for the first one, and a TX and an RX buffer descriptor for 
> the second one.
> 
>> rbase 0x01e0 tbase 0x01c0 rfcr 0x30 tfcr 0x30 mrblr 0x0201
>> rstate 0x00000000 rptr 0x00000000 rbptr 0x01e0 rcount 0x0000 rtmp 0x00000000
>> tstate 0x00000000 tptr 0x00000000 tbptr 0x01c0 tcount 0x0000 ttmp 0x00000000
>> i2mod 0x00, i2add 0xfe i2brg 0x03 i2com 0x01 i2cer 0x00 i2cmr 0x00
>> rx 0 sc 0x9000 tx 0 sc 0x9400
>> rx 1 sc 0x7da4 tx 1 sc 0xac00
>> rx 2 sc 0xefef tx 2 sc 0x743b
>> rx 3 sc 0xf264 tx 3 sc 0x10e6

Just to rule out some possible issues with the rx queue,
could you check if this patch improves anything?

Thanks,
Jochen

diff --git a/drivers/i2c/busses/i2c-cpm.c b/drivers/i2c/busses/i2c-cpm.c
index 228f757..b83b733 100644
--- a/drivers/i2c/busses/i2c-cpm.c
+++ b/drivers/i2c/busses/i2c-cpm.c
@@ -231,12 +231,6 @@ static void cpm_i2c_parse_message(struct i2c_adapter *adap,

 		dev_dbg(&adap->dev, "cpm_i2c_read(abyte=0x%x)\n", addr);

-		out_be16(&rbdf->cbd_datlen, 0);
-		out_be16(&rbdf->cbd_sc, BD_SC_EMPTY | BD_SC_INTRPT);
-
-		if (rx + 1 == CPM_MAXBD)
-			setbits16(&rbdf->cbd_sc, BD_SC_WRAP);
-
 		eieio();
 		setbits16(&tbdf->cbd_sc, BD_SC_READY);
 	} else {
@@ -328,6 +322,16 @@ static int cpm_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 			return -EINVAL;
 	}

+	/* Initialize rx queue */
+	for (i = 0; i < CPM_MAXBD; i++) {
+		rbdf = cpm->rbase + i;
+		out_be16(&rbdf->cbd_datlen, 0);
+		out_be16(&rbdf->cbd_sc, BD_SC_EMPTY | BD_SC_INTRPT);
+
+		if (i + 1 == CPM_MAXBD)
+			setbits16(&rbdf->cbd_sc, BD_SC_WRAP);
+	}
+
 	/* Reset to use first buffer */
 	out_be16(&i2c_ram->rbptr, in_be16(&i2c_ram->rbase));
 	out_be16(&i2c_ram->tbptr, in_be16(&i2c_ram->tbase));