 |
 |
View previous topic :: View next topic |
Author |
Message |
allenhuffman
Joined: 17 Jun 2019 Posts: 623 Location: Des Moines, Iowa, USA
|
FTDI LibFT4222 1.4.7 improved but broke my PIC24 code ;-) |
Posted: Thu Dec 26, 2024 4:40 pm |
|
|
Passing this along in case anyone else uses the FTDI I2C Windows drive to talk to a PIC24.
I have had I2C code running the past few years that suddenly started having issues after updating the FTDI drive to 1.4.7. The release notes have no details, but I observe when things break the STOP bit happens right after the last clock pulse of the final byte the Master is READING. When this happens that quick, my code that looks for the STOP/IRQ bit and is doing enough that it misses it.
In earlier drivers, the timing has always been sporadic with "random" gaps between bytes now and then. Normally there is a small gap after the final byte is READ by the FTDI driver and when the STOP bit is seen. When there is a gap before the STOP is seen, it works great as it has the past few years.
Now the gap seems to be gone "most" of the time, and those break my code. I am working on improving my code now.
Just sharing this in case anyone else has jumped from 1.4.5 to 1.4.7 like I just did and ran into issue... _________________ Allen C. Huffman, Sub-Etha Software (est. 1990) http://www.subethasoftware.com
Embedded C, Arduino, MSP430, ESP8266/32, BASIC Stamp and PIC24 programmer.
http://www.whywouldyouwanttodothat.com ?
Using: 24FJ256GA106, 24EP256GP202 and 24FJ64GA002. |
|
 |
temtronic
Joined: 01 Jul 2010 Posts: 9525 Location: Greensville,Ontario
|
|
Posted: Thu Dec 26, 2024 6:47 pm |
|
|
so, the cynic in me has to ask... WHY did you update something that has worked for YEARS ??? |
|
 |
allenhuffman
Joined: 17 Jun 2019 Posts: 623 Location: Des Moines, Iowa, USA
|
|
Posted: Thu Dec 26, 2024 7:58 pm |
|
|
temtronic wrote: | so, the cynic in me has to ask... WHY did you update something that has worked for YEARS ??? |
The current version has issues where the interface locks up and required the PC to be power cycled. _________________ Allen C. Huffman, Sub-Etha Software (est. 1990) http://www.subethasoftware.com
Embedded C, Arduino, MSP430, ESP8266/32, BASIC Stamp and PIC24 programmer.
http://www.whywouldyouwanttodothat.com ?
Using: 24FJ256GA106, 24EP256GP202 and 24FJ64GA002. |
|
 |
Ttelmah
Joined: 11 Mar 2010 Posts: 19895
|
|
Posted: Sun Dec 29, 2024 4:40 am |
|
|
Comment.
If your device is reading, it should be stretching the clock until the read has
been done. It sounds as if possibly your code is relying on the default
CCS behaviour, and resetting the IRQ, when it exits the subroutine, rather
than handling this yourself?. Problem is that if the clock is released, and then
there are a few uSec delay before you exit the sub, another event can
arrive in this interval, and the IRQ for this then gets reset.... :(
On my own code I handle resetting the IRQ the very next instruction to
releasing the clock. If another event then occurs the IRQ gets called
again to handle this. |
|
 |
allenhuffman
Joined: 17 Jun 2019 Posts: 623 Location: Des Moines, Iowa, USA
|
|
Posted: Sat Jan 04, 2025 6:21 pm |
|
|
I am using an ISR with NOCLEAR since I needed to check for the stop bit and that apparently was required. I handle the byte then spin in a loop checking for the next incoming byte (IRQ bit) or a stop (P bit). This was a bit wasteful for CPU cycles, but it let me know when a message was complete. (Original code would look at a special header byte in the message and then read that many more. Worked fine, until you got a bad byte ;-)
My code apparently was missing the now-faster (sometimes) stop bit, so I removed it. I still have it in there for master writes (so master can write messages of different sizes) but I was only using it in the master read code to toggle an LED on/off during the TX session. Removing it was enough to get past this. _________________ Allen C. Huffman, Sub-Etha Software (est. 1990) http://www.subethasoftware.com
Embedded C, Arduino, MSP430, ESP8266/32, BASIC Stamp and PIC24 programmer.
http://www.whywouldyouwanttodothat.com ?
Using: 24FJ256GA106, 24EP256GP202 and 24FJ64GA002. |
|
 |
Ttelmah
Joined: 11 Mar 2010 Posts: 19895
|
|
Posted: Sun Jan 05, 2025 2:15 am |
|
|
OK, so where do you clear the interrupt, relative to where you release the
clock????. The key point is that once the clock is released, then the stop bit
can come. Ideally you want the interrupt cleared ASAP of this point. That
way the subsequent interrupt can still be detected. |
|
 |
allenhuffman
Joined: 17 Jun 2019 Posts: 623 Location: Des Moines, Iowa, USA
|
|
Posted: Sun Jan 05, 2025 9:49 am |
|
|
Ttelmah wrote: | OK, so where do you clear the interrupt, relative to where you release the
clock????. The key point is that once the clock is released, then the stop bit
can come. Ideally you want the interrupt cleared ASAP of this point. That
way the subsequent interrupt can still be detected. |
Yeah I think I clear at the top of the ISR, so perhaps a "fix" might be to not clear until the end before I need to check for P. _________________ Allen C. Huffman, Sub-Etha Software (est. 1990) http://www.subethasoftware.com
Embedded C, Arduino, MSP430, ESP8266/32, BASIC Stamp and PIC24 programmer.
http://www.whywouldyouwanttodothat.com ?
Using: 24FJ256GA106, 24EP256GP202 and 24FJ64GA002. |
|
 |
allenhuffman
Joined: 17 Jun 2019 Posts: 623 Location: Des Moines, Iowa, USA
|
|
Posted: Fri Jul 11, 2025 8:45 am |
|
|
Bumping an old topic, because of more FTDI I2C issues that just started popping up along with some significant code changes on our PC Master I2C side of things...
Our I2C code is common and used with a few different types of PIC24. To make that work, I define some of the I2C bits like this, so the common code can get to them:
Code: | #word IFS5 = getenv("SFR:IFS5")
#bit I2C_SYSTEM_BUS_IRQ_PENDING_BIT = IFS5.4 // bit 4 - slave I2Cx IRQ status
|
In the I2C routine, it looks like this:
Code: | void i2cISR (void)
{
I2C_SYSTEM_BUS_IRQ_PENDING_BIT = 0; // NOCLEAR, so we manually clear.
int state = i2c_isr_state (SYSTEM_BUS);
/*-----------------------------------------------------------------------*/
// I2C Master BEGINS WRITING to us (master writes address byte):
// Address byte needs to be read.
/*-----------------------------------------------------------------------*/
if (state == RECEIVE_I2C) // I2C address match, R/W clear (writing).
{
LED_SlaveRXOn ();
|
My question is about when I should clear that IRQ PENDING bit.
At the end of handling the byte coming in from a master WRITE, I poll for either another incoming byte or the stop (P) bit:
Code: | /*-----------------------------------------------------------------------*/
// I2C Master continues WRITING to us (master writes data bytes):
// Receives a message from the master.
/*-----------------------------------------------------------------------*/
else if (state > RECEIVE_I2C)
{
// Prevent buffer overrun.
if (g_indexRX < sizeof(g_rxBuf))
{
// Store the message in the RX buffer
g_rxBuf[g_indexRX++] = i2c_read (SYSTEM_BUS);
}
else
{
// Read and discard extra bytes.
i2c_read (SYSTEM_BUS);
}
// This code is used to detect when the Master is done WRITING to us
// so we can flag that a message is ready to verify/process:
while ((P == 0) &&
(I2C_SYSTEM_BUS_IRQ_PENDING_BIT == 0))
{
restart_wdt();
}
if (P != 0) // I2C Stop bit seen.
{
LED_SlaveRXOff ();
// Message is only received when expected bytes have come in.
runTimeVariables.msgStatus = RX_MESSAGE_VERIFY;
// The master has written something to us.
g_expectMasterToReadResponse = true;
}
} |
This lets us handle incoming messages of different sizes, and also toggle an LED on at the start and stop of the transaction, for detecting bus lockups.
I had similar code in the master READ loop, but had to remove that "check for stop bit" loop because it was happening faster than the code could get to the check (seemingly).
For some time, I had updated this code to allow message payloads >127 bytes (bypassing the i2c_isr_state() with my own code, based on examples other shared, and one from CCS), but that was reverted due to use never having larger messages and unrelated issues.
Here is the full routine, with a few things left out that are not related.
When a message is received, we flag a state machine to know there is something in the buffer to process. If the master tries to READ before a response is ready, it will be blocked by clock stretching. When the state machine has a response ready in the TX buffer, it calls a function that, if the master is already blocked (clock stretching) on a read, it just pushes out the first byte to disable clock stretching and lets the ISR handle the rest. If the master is not already blocked waiting, it just continues.
It works well. In testing, I've seen it process 34+ million messages with zero errors.
But I'd really like to get my LED working again for debugging the current situation, and wonder if I could just move the "clear IRQ pending bit" to right before it checks for a stop bit and have that help. Since we are not getting IRQs on start/stop on these PIC24s, I am not sure how that would help. Those pulses are coming in no matter what, I would think?
This is also a "release the clock" check in our main loop that looks to see if clock stretching has been held too long, and will flush out a byte to release the bus in case something gets out-of-sync with the host. Not even sure if that is needed, but "belt and suspenders."
Code: | void i2cISR (void)
{
I2C_SYSTEM_BUS_IRQ_PENDING_BIT = 0; // NOCLEAR, so we manually clear.
int state = i2c_isr_state (SYSTEM_BUS);
/*-----------------------------------------------------------------------*/
// I2C Master BEGINS WRITING to us (master writes address byte):
// Address byte needs to be read.
/*-----------------------------------------------------------------------*/
if (state == RECEIVE_I2C) // I2C address match, R/W clear (writing).
{
LED_SlaveRXOn ();
// Get a byte of data which is the address of the board.
// (The Multiplexer uses this in RXMessageProc to relay broadcast
// messages to the control boards.)
g_boardAddress = i2c_read (SYSTEM_BUS);
// We can confirm the master is no longer reading.
g_masterIsReading = false;
// New write, so discard any old waiting response message.
g_slaveResponseIsReady = false;
// Reset receive buffer point to first byte of RX buffer.
g_indexRX = 0;
// A write message has been received, so clear the time-out counter.
g_commTimeoutCounter = 0;
// TODO: What happens if the Master writes the address, but never
// writes out any data? Do we need STOP/IRQ detection code here, too?
//DEBUG_PUTC ('W'); // Write began.
}
/*-----------------------------------------------------------------------*/
// I2C Master BEGINS READING from us (master wrote address byte):
// Address byte ready to be read, then write out first data byte so it
// will be ready for the Master's next read.
/*-----------------------------------------------------------------------*/
else if (state == TRANSMIT_I2C) // I2C address match, R/W set (reading).
{
// We no longer check for the master Read STOP bit due to it coming
// too fast (with FT4222 1.4.7) and we miss it, so we do not know
// when to turn this off.
//LED_SlaveTXOn ();
g_masterIsReading = true;
// Reset transmit buffer pointe to first byte of buffer.
g_indexTX = 0;
// A read message has been received, so clear the time-out counter.
g_commTimeoutCounter = 0;
// If this read was not expected...
if (g_expectMasterToReadResponse == false)
{
// Get a byte of data which is the address of the board.
// Clock stretching is used to work around Eratta 38 on the
// Universal board. It seems to make the problem go away.
i2c_read (SYSTEM_BUS, 2); // CLOCK STRETCH
// Write out first data byte.
i2c_write (SYSTEM_BUS, 0x01);
}
else // Master previously wrote, responses is expected.
{
// Read the address but clock stretch while we get wait for
// the response to be ready.
i2c_read (SYSTEM_BUS, 2); // CLOCK STRETCH
// Only write out the first data byte if a response message
// is ready to be sent.
if (g_slaveResponseIsReady == true) // If response buffer is ready...
{
g_slaveResponseIsReady = false; // Reset.
// Write out the first byte of data.
i2c_write (SYSTEM_BUS, g_txBuf[g_indexTX++]);
}
else
{
// Start the clock stretching timeout timer.
// We are in an ISR, so just access directly.
g_clockStretchingTimer = S_Tick; // Start timer.
//DEBUG_PUTC ('C');
}
// Else, we will manually write the first byte in
// i2cSlaveResponseIsReady () when the transmit buffer is ready.
}
//DEBUG_PUTC ('R'); // Read began.
}
/*-----------------------------------------------------------------------*/
// I2C Master continues READING from us (master reads data bytes):
// Transmits a message to the master.
/*-----------------------------------------------------------------------*/
else if (state > TRANSMIT_I2C)
{
int byteToSend;
// If we are expecting to be sending a response, and our index is
// within the size of that response...
if (g_expectMasterToReadResponse == false)
{
byteToSend = 0x02;
}
else if (g_indexTX < g_txMsgData.numBytes)
{
// Send the data in the TX buffer to the master.
byteToSend = g_txBuf[g_indexTX++];
}
else
{
// Pad extra bytes if the master reads more than we are writing.
// This can happen if we are NAKing something and the master
// expects a longer message back.
byteToSend = 0x03;
}
i2c_write (SYSTEM_BUS, byteToSend);
// No need to wait for anything. We have no control over how many bytes
// the master reads from us.
//DEBUG_PUTC ('r');
}
/*-----------------------------------------------------------------------*/
// I2C Master continues WRITING to us (master writes data bytes):
// Receives a message from the master.
/*-----------------------------------------------------------------------*/
else if (state > RECEIVE_I2C)
{
// Prevent buffer overrun.
if (g_indexRX < sizeof(g_rxBuf))
{
// Store the message in the RX buffer
g_rxBuf[g_indexRX++] = i2c_read (SYSTEM_BUS);
}
else
{
// Read and discard extra bytes.
i2c_read (SYSTEM_BUS);
}
// This code is used to detect when the Master is done WRITING to us
// so we can flag that a message is ready to verify/process:
while ((P == 0) &&
(I2C_SYSTEM_BUS_IRQ_PENDING_BIT == 0))
{
restart_wdt();
}
if (P != 0) // I2C Stop bit seen.
{
LED_SlaveRXOff ();
// Message is only received when expected bytes have come in.
runTimeVariables.msgStatus = RX_MESSAGE_VERIFY;
// The master has written something to us.
g_expectMasterToReadResponse = true;
//DEBUG_PUTC ('w');
}
}
else
{
// Do nothing.
}
}
void i2cSlaveResponseIsReady (void)
{
// Since this is checked in the ISR, we need to mask interrupts to make
// it atomic.
DISABLE_I2C_INTERRUPTS;
int masterIsReading = g_masterIsReading;
//ENABLE_I2C_INTERRUPTS;
if (masterIsReading == false) // Master has not tried to read yet.
{
g_clockStretchingTimer = 0; // Disable timer.
g_slaveResponseIsReady = true; // Let ISR know data is ready.
}
else // Master is reading and is blocked due to clock stretching.
{
g_clockStretchingTimer = 0; // Disable timer.
//DEBUG_PUTC ('c');
i2c_write (SYSTEM_BUS, g_txBuf[g_indexTX++]);
// The rest will be handled by the ISR.
}
ENABLE_I2C_INTERRUPTS;
} // end of i2cSlaveResponseIsReady () |
_________________ Allen C. Huffman, Sub-Etha Software (est. 1990) http://www.subethasoftware.com
Embedded C, Arduino, MSP430, ESP8266/32, BASIC Stamp and PIC24 programmer.
http://www.whywouldyouwanttodothat.com ?
Using: 24FJ256GA106, 24EP256GP202 and 24FJ64GA002. |
|
 |
Ttelmah
Joined: 11 Mar 2010 Posts: 19895
|
|
Posted: Mon Jul 14, 2025 1:31 am |
|
|
OK.
I don't think moving the IRQ releases would help. If you look at the I2C
family reference manual it always clears this ASAP.
Are you saying that you are not seeng the P bit getting set and the S bit
cleared when the stop comes?.
Thing I'm thinking is that state seems to be being set at the entry to the
IRQ. Since you are holding inside the IRQ, this may not meet the >
RECEIVE_I2C test, which would then result in the P test not being reached.
You presumably have the PCIE bit set?. You won't get interrupts on starts/
stop if mote. There is a comment that PIIE is not available on all devices.
I'd have to check your devices data sheet for this. |
|
 |
allenhuffman
Joined: 17 Jun 2019 Posts: 623 Location: Des Moines, Iowa, USA
|
|
Posted: Mon Jul 14, 2025 10:04 am |
|
|
I *believe* it is just happening quicker than my code reaches it because of "all the other junk" we have crammed in this ISR.
Do you happen to know what clears that P bit in the register? Is it just high during the pulse, and then goes back low? _________________ Allen C. Huffman, Sub-Etha Software (est. 1990) http://www.subethasoftware.com
Embedded C, Arduino, MSP430, ESP8266/32, BASIC Stamp and PIC24 programmer.
http://www.whywouldyouwanttodothat.com ?
Using: 24FJ256GA106, 24EP256GP202 and 24FJ64GA002. |
|
 |
Ttelmah
Joined: 11 Mar 2010 Posts: 19895
|
|
Posted: Mon Jul 14, 2025 10:56 am |
|
|
It'll clear when the next event happens. So when the next start comes.
Specified in I2C to not be less than 1.3uSec.
Do you have nested interrupts enabled?. Any possibility the I2C service
is being delayed by something else?.
Thinking more about this. You don't say your PIC24 speed?. However on
most 1.3uSec should be short enough for a response, but probably worth
doing some things:
1) use a FAST interrupt handler for this routine. Go through the assembler
and work out what registers need saving, and just save these yourself.
2) Raise the priority of this routine over your other interrupts. Beware the
caveat in the 'known issues' forum about using nested interrupts. CCS seem
never to have 100% fixed this, so use static variables in all your interrupt
handlers to ensure there are no problems from this.
3) I'd rethink the logic of where your test for the P bit. If you have interrupts
on the start and stop enabled, this test needs to be done ASAP coming into
the handler, not after the other work. I think some careful thought about the
timing of this detection is needed, |
|
 |
|
|
You cannot post new topics in this forum You cannot reply to topics in this forum You cannot edit your posts in this forum You cannot delete your posts in this forum You cannot vote in polls in this forum
|
Powered by phpBB © 2001, 2005 phpBB Group
|