MPC85xx i2c interface bug

* MPC85xx i2c interface bug
@ 2005-11-29 20:42 dwilson
  2005-11-30  6:14 ` Kumar Gala
  0 siblings, 1 reply; 6+ messages in thread
From: dwilson @ 2005-11-29 20:42 UTC (permalink / raw)
  To: linuxppc-embedded

We have found a problem in the i2c-mpc.c file (linux 2.4 kernel).  The
logic for the i2c transfer termination seems to have been inverted.

The original function is:

/* Perform one i2c_msg transfer.  It could be a read, or a write.
 * This function assumes that the address byte has been sent
 * already, and only handles the message contents */
static int mpc_i2c_xfer_bytes(struct mpc_i2c_private* priv, struct
i2c_msg* pm)
{
	volatile struct mpc_i2c *regs = priv->regs;
	char *buf = pm->buf;
	int len = pm->len;
	int i;
	int ret = 0;

	if(pm->flags & I2C_M_RD) {
		/* Change to read mode */
		priv->write(&regs->i2ccr, 0, MPC_I2CCR_MTX);

		/* If there is only one byte, we need to TXAK now */
		if(len == 1)
			priv->write(&regs->i2ccr, 0, MPC_I2CCR_TXAK);

		/* Do a dummy read, to initiate the first read */
		priv->read(&regs->i2cdr);
	}

	for(i = 0; i < len; ++i) {
		/* If this is a read, read a byte, otherwise, write a byte */
		if(pm->flags & I2C_M_RD) {
			/* Wait for the previous read to finish */
			ret = wait_for_txcomplete(priv);

			if (ret)
				return ret;

			/* If this is the 2nd to last byte, send
			 * the TXAK signal */
			if(i == len - 2) {
				priv->write(&regs->i2ccr, 0, MPC_I2CCR_TXAK);
			}

			/* If this is the last byte, send STOP */
			if (i == len - 1)
				mpc_i2c_stop(priv);

			buf[i] = priv->read(&regs->i2cdr);

		} else {
			/* Otherwise, it's a write */
			priv->write(&regs->i2cdr, buf[i], MPC_I2C_FULLREG);

			/* Wait for the transaction to complete */
			ret = wait_for_txcomplete(priv);

			if(ret)
				return ret;

			/* Return error if we didn't get the ack */
			if((priv->read(&regs->i2csr) & MPC_I2CSR_RXAK)) {
				printk(KERN_DEBUG "NO ACK!\n");
				ret = -EREMOTEIO;
			}
		}

		/* If there was an error, abort */
		if(ret)
			return ret;
	}

	return ret;
}

The problem is that the TXAK bit is set incorrectly: it should be set to a
zero when acks are to be sent and then set to a 1 just before reading the
next-to-last byte in the transfer (8555 Processor Reference Manual
(MPC8555ERM rev1), section 11.3.1.3 and also Figure 11-8).

Changes to the above code are thus:

302c302
<               priv->write(&regs->i2ccr, 0, MPC_I2CCR_MTX);
---
>               priv->write(&regs->i2ccr, 0, MPC_I2CCR_MTX | MPC_I2CCR_TXAK);
306c306
<                       priv->write(&regs->i2ccr, 0, MPC_I2CCR_TXAK);
---
>                       priv->write(&regs->i2ccr, MPC_I2CCR_TXAK,
MPC_I2CCR_TXAK);
324c324
<                               priv->write(&regs->i2ccr, 0, MPC_I2CCR_TXAK);
---
>                               priv->write(&regs->i2ccr, MPC_I2CCR_TXAK,
MPC_I2CCR_TXAK);

Thanks,

Dan.

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