build_block_buffer() does not end I2C read sequence with NACK

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Tested with SFP optical transceiver, although it might be reproducible using 
EEPROMs. Adapt the i2ceeprom example code to read a block of data from an I2C 
peripheral device.
2. Run the example with the peripheral device. Verify it reads meaningful data.
3. Then, rerun the example without resetting the peripheral.

What is the expected output? What do you see instead?
Depending on the data contents being read, in some cases the peripheral will 
fail the second read, usually returning all 0xFF bytes.

What version of the product are you using? On what operating system?
SVN version as of 02/27/2012, OS is CentOS 6.2 with libftdi1 and libusb1.

Please provide any additional information below.
Scope shows that the error is caused by the peripheral device's failure to 
release control of the SDA line after we are finished reading from it. When the 
problem occurs, the SDA line remains pulled low, as is the peripheral is trying 
to clock out more data for the master. That is indeed the case. The problem is 
caused by the build_block_buffer() function in support.c module, which ends the 
final byte read from I2C with ACK instead of NACK. See the recommendation at 
http://www.nxp.com/documents/user_manual/UM10204.pdf chapter 3.1.6: "...When 
SDA remains HIGH during this ninth clock pulse, this is defined as the Not 
Acknowledge signal. The master can then generate either a STOP condition to 
abort the transfer, or a repeated START condition to start a new transfer."
Patching the support.c to include something like this at line 169:
//buf[i++] = mpsse->tack;
buf[i++] = (j < num_blocks - 1) ? 0x00 : 0xFF; //NACK the last byte
resolves the problem, at least in my case. The STOP sequence is then properly 
executed and the program can be successfully restarted many times without 
resetting the peripheral.

Original issue reported on code.google.com by mii...@gmail.com on 27 Feb 2012 at 7:50

[GoogleCodeExporter]

On further investigation, this could be fixed by a sequence of reads like this:
Read(any amount of data);
...
Read(any amount of data);
SetAck(1);
Read(less than or equal to I2C_TRANSFER_SIZE);
Stop();
The condition is that we don't Read() more than I2C_TRANSFER_SIZE bytes in the 
last Read() call, otherwise the build_block_buffer() will NACK the peripheral 
before all the data is read. It seems that for I2C transfers, Read() and/or 
build_block_buffer() should somehow be made aware that this will be the last 
read before Stop(), so that they can NACK the final chunk. One possible (ugly) 
hack is to increase I2C_TRANSFER_SIZE to a maximum value needed, so that there 
is only one chunk of data read and then NACK the last byte in chunk every time.

Original comment by mii...@gmail.com on 28 Feb 2012 at 10:18

[GoogleCodeExporter]

I'll double check this on the scope when I get a chance, but the following 
*should* work:

Read(all data except the last byte);
SetAck(0);
Read(1);
Stop();

Passing a 1 to SetAck enables ACKs, while passing a 0 disables ACKs (aka, 
enables NACKs). This is a bit confusing as it is the reverse of the logic 
levels used on the bus, so perhaps this should be changed.

Let me know if the above works for you, or if you still have problems.

Original comment by heffne...@gmail.com on 5 Mar 2012 at 1:42

[GoogleCodeExporter]

Yes, that sequence is ok. Once I grasped the logic of it, it worked as 
advertised.

I think an update to i2ceeprom example would go a long way towards explaining 
this, including the ACK/NACK logic. As it stands now, the example reads in all 
the data and then attempts to Stop(), without toggling the Ack before the last 
byte. This does leave peripherals in a rather unfortunate state of still trying 
to clock out data.

Thank you for your assistance and a very good library!

Original comment by mii...@gmail.com on 5 Mar 2012 at 6:51

[GoogleCodeExporter]

Yes, EEPROMs are pretty forgiving while other I2C devices aren't. Looking over 
this I also noticed that the documentation in README.I2C is wrong as well, so 
I'll be sure to update that. Sorry for the confusion!

Original comment by heffne...@gmail.com on 6 Mar 2012 at 1:32

[GoogleCodeExporter]

Updated I2C examples and documentation. Also updated SetAck such that passing 
it a 1 sends NACKs and passing it a 0 sends ACKs; this corresponds with the ACK 
bit actually sent on the bus, as well as the ACK status returned by GetAck.

Thanks!

Original comment by heffne...@gmail.com on 9 Mar 2012 at 2:57

• Changed state: Fixed