[i2c] I2C on AT91RM9200 receives extra byte

[Mike Wolfram]

On Thursday 07 September 2006 13:16, Andrew Victor wrote:
> > When I reviewed this driver back in July, I remember asking Andrew to
> > check that the logic around the STOP condition matched the flowchart in
> > the data- sheet.  AFAICT, it didn't; in his latest revision it did. 
> > Looking again at the datasheet this morning, it actually says *nothing*
> > about when to read RHR.
> >
> > So Mike: it wouldn't surprise me if such a change were necessary.  But
> > just to be sure... please point out which of Andrew's revisions you've
> > started with (a link is fine) and also send a patch with your change.
> >
> > Andrew: do you have an update for this driver?  Any other comments?
>
> Since Mike mentions the 2.6.18-rc4 patches I guess he's referring to my
> latest AT91 patch:
>    http://maxim.org.za/AT91RM9200/2.6/
>
> That includes the updated I2C driver that *should* match the flowcharts
> in the datasheet.  Since I actually have a working I2C interface on my
> board, the change hasn't actually been tested though.

Hi,

yes, I took that patch. I also read the flowchart in the latest datasheet when 
I read about the changes in the driver.

It seems to me that if the byte is first read from RHR and then STOP is 
written, that another byte sampling is already in progress and won't be 
stopped when writing the STOP to the CR. So if reading RHR again in the next 
transfer the extra byte from the previous received message becomes the first 
of the next.

In my setup I found this when I was trying to get the RTC8564 driver running. 
In 2.6.17 it worked fine but the rtc module refused to load in 2.6.18. There 
is a validation check when loading the module to check if all registers are 
in range. The first register was ok, but every other got a FF. Changing the 
xfer_read in i2c-at91.c to the version below solved the problem.

Currently I have no time to prepare a patch but this is my currently working 
version. There is one ugly thing left that we now write the STOP twice, once 
before reading last byte and once after in the calling function.

static int xfer_read(struct i2c_adapter *adap, unsigned char *buf, int length)
{
	/* Send Start */
	at91_twi_write(AT91_TWI_CR, AT91_TWI_START);

	/* Read data */
	while (length--) {
	    if (!length)
	        at91_twi_write(AT91_TWI_CR, AT91_TWI_STOP);
		if (!at91_poll_status(AT91_TWI_RXRDY)) {
			dev_dbg(&adap->dev, "RXRDY timeout\n");
			return -ETIMEDOUT;
		}
		*buf++ = (at91_twi_read(AT91_TWI_RHR) & 0xff);
		dev_dbg(&adap->dev, "read %02X\n", *(buf - 1));
	}

	return 0;
}

--
Mike Wolfram
-- 
MicroDatec GmbH
Zeulenrodaer Str. 17, 99091 Erfurt, Germany
fon +49-361-77821-0, fax +49-361-77821-30
mobile +49-179-4902190