CCS C Software and Maintenance Offers
FAQFAQ   FAQForum Help   FAQOfficial CCS Support   SearchSearch  RegisterRegister 

ProfileProfile   Log in to check your private messagesLog in to check your private messages   Log inLog in 

CCS does not monitor this forum on a regular basis.

Please do not post bug reports on this forum. Send them to support@ccsinfo.com

SOLVED? i2c_isr_state() and transfers larger than 127 bytes
Goto page 1, 2  Next
 
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion
View previous topic :: View next topic  
Author Message
allenhuffman



Joined: 17 Jun 2019
Posts: 537
Location: Des Moines, Iowa, USA

View user's profile Send private message Visit poster's website

SOLVED? i2c_isr_state() and transfers larger than 127 bytes
PostPosted: Thu Apr 13, 2023 1:59 pm     Reply with quote

This week I have been revising and improving some I2C code at work. It talks between various PIC24 processors and a PC via the FTDI USB interface.

The i2c_isr_state() help entry lists the state values as:

Quote:
0 - Address match received with R/W bit clear, perform i2c_read( ) to read the I2C address.

1-0x7F - Master has written data; i2c_read() will immediately return the data

0x80 - Address match received with R/W bit set; perform i2c_read( ) to read the I2C address, and use i2c_write( ) to pre-load the transmit buffer for the next transaction (next I2C read performed by master will read this byte).

0x81-0xFF - Transmission completed and acknowledged; respond with i2c_write() to pre-load the transmit buffer for the next transition (the next I2C read performed by master will read this byte).


I noticed the FTDI documentation for their FT4222_I2CMaster_Write() routine said it could write up to 65535 bytes. I was curious to see what the PIC24 would do with that, so I tried it.

On the Saleae analyzer, I wrote out a packet of values (00, 01, 02 ... fd, fe, ff, 00, 01, 02 ... etc.) and looked at what it did.

As expected, I see the write address byte, followed by 00, 01, 02 on up to 73... The Saleae reports each byte ACK'd but leaves SDA high so the master never sends any more.

At this point, the bus is hung with the clock held low. At least I know a way to hange my system ;-) (I actually started looking at this after finding out you could crash systems by sending unexpectedly large I2C messages, since code often assumes some size limit, but different things use different sizes -- such as FTDI and 65535 bytes.)

I don't know why anyone would want to do this, but it did make me wonder if there is even a way to use the CCS API to read more than address+127 bytes in an I2C transaction.

At the very least, I want to make sure my boards don't hang if this happens.

**This started when I expanded the size of a message we use, and bad things happen. The message was small, but when going through our multiplexer device, it adds it's own wrapper to the message and was making it too big to send.
_________________
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 ?


Last edited by allenhuffman on Mon Apr 17, 2023 11:28 am; edited 3 times in total
temtronic



Joined: 01 Jul 2010
Posts: 9097
Location: Greensville,Ontario

View user's profile Send private message

PostPosted: Thu Apr 13, 2023 4:42 pm     Reply with quote

From what I recall from Philips, decades ago, there is no 'limit' with respect to hardware, it's just whoever wrote the 'driver' put a limit. I've seen 32, 127 and 65535 as 'maximum' sizes for the data transfers.
So, apparently CCS decided on 127.... probably a 'magical' number ? Easy to code ??
Probably similar to their 'RS-232' buffer. I do suggest some form of 'timeout' escape and a 'reset the I2C bus'.....
No reason you can't code your own driver or API. whatever is the "PC" term for software these days...
allenhuffman



Joined: 17 Jun 2019
Posts: 537
Location: Des Moines, Iowa, USA

View user's profile Send private message Visit poster's website

PostPosted: Thu Apr 13, 2023 5:08 pm     Reply with quote

temtronic wrote:
From what I recall from Philips, decades ago, there is no 'limit' with respect to hardware, it's just whoever wrote the 'driver' put a limit. I've seen 32, 127 and 65535 as 'maximum' sizes for the data transfers.
So, apparently CCS decided on 127.... probably a 'magical' number ? Easy to code ??
Probably similar to their 'RS-232' buffer. I do suggest some form of 'timeout' escape and a 'reset the I2C bus'.....
No reason you can't code your own driver or API. whatever is the "PC" term for software these days...


What did you do at Phillips? A company I worked for did an operating system they used in their CD-I systems, oh so long ago.

That makes sense. I have a simple deadlock check I was using, where when I decide I need to clock stretch with i2c_read(STREAM,2) I set a flag. In my main loop if that flag is set, and a counter increments far enough, I write out a byte to release the clock stretch.

Is there a better way?

I did create an I2C.c file that is reconstructions of the CCS Code, created by looking at the assembly and recreating them in C using register BITS and such where needed.

I suppose once the ISR fires, I can do anything I want if I am not relying on their ISR call.
_________________
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 ?
allenhuffman



Joined: 17 Jun 2019
Posts: 537
Location: Des Moines, Iowa, USA

View user's profile Send private message Visit poster's website

PostPosted: Thu Apr 13, 2023 5:11 pm     Reply with quote

Oh! It just dawned on me. Phillips created I2C, didn’t they? I had a computer called an MM/1, only a few hundred made, which was based on Philips chips (68070 and the video chip) used in the CD-I players. It had an I2C network bus but back then I had no idea what it was for.

Wish I knew then what I know now.
_________________
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 ?
temtronic



Joined: 01 Jul 2010
Posts: 9097
Location: Greensville,Ontario

View user's profile Send private message

PostPosted: Thu Apr 13, 2023 6:53 pm     Reply with quote

I didn't work for them, just bought a LOT of semis, chips, caps and res from them. Probably 1/2 my library was donated by local distributor and he always had interesting samples for me.
Great, now I really feel old...... Confused
Ttelmah



Joined: 11 Mar 2010
Posts: 19215

View user's profile Send private message

PostPosted: Thu Apr 13, 2023 11:10 pm     Reply with quote

If you look back, the CCS I2C_transfer functions appeared when Microchip
launched PIC's with the I2C peripheral, rather than the MSSP. These do
block transfers, so CCS wrote a function to support this. They then
added the same function to the MSSP implementation as a wrapper,
that uses the standard I2C_xfer calls to do the same thing. However
they made the transfer size that was supported, match what the PIC
I2C hardware peripheral did.
So, if you are on a PIC that doesn't have the I2C peripheral, you just
have to write your own wrapper to do the same job, but use an int16
counter instead of the signed int8 that the wrapper uses in the standard
implementation. If however you are using a PIC with the I2C peripheral,
you have a problem, since this supports 128 byes total in the transfer
(the address byte counts as one, so 127 bytes of data).
The functions abilities were written to support the hardware abilities on
some particular PIC peripherals...
allenhuffman



Joined: 17 Jun 2019
Posts: 537
Location: Des Moines, Iowa, USA

View user's profile Send private message Visit poster's website

PostPosted: Fri Apr 14, 2023 10:07 am     Reply with quote

Ttelmah wrote:
If however you are using a PIC with the I2C peripheral,
you have a problem, since this supports 128 byes total in the transfer
(the address byte counts as one, so 127 bytes of data).
The functions abilities were written to support the hardware abilities on
some particular PIC peripherals...


Very interesting -- Is this 128 byte limit a hardware limit? If I was doing polled mode, could I not i2c_read() as much as I wanted? (Assume FORCE_HW rather than bitbanging.)

Or is it a i2c_isr_stat() software limit? I can see that i2c_isr_state() is not set up to handle more than data bytes 0x1-0x7f, but if you write past that, I don't know what happens.

Time to do some more testing...
_________________
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 ?
allenhuffman



Joined: 17 Jun 2019
Posts: 537
Location: Des Moines, Iowa, USA

View user's profile Send private message Visit poster's website

PostPosted: Fri Apr 14, 2023 11:20 am     Reply with quote

Heh. From testing, it does seem that the incoming "state" value keeps incrementing. I see it go from...

127 128 129 ... then a lock

At 128 (0x80) the ISR example thinks the master is doing a READ, and ends up trying to i2c_write() data out, when the master is still writing.

Perhaps a code workaround is to just use 0x00 and 0x80 to be the start of a read or write, set a flag, and then only use that flag/mode until the STOP BIT is seen.

I already monitor the STOP bit to know when a write has completed:

Code:
// Wait for either a stop bit, or another incoming data byte.
while ((P == 0) && (I2C_SYSTEM_BUS_IRQ_PENDING_BIT == 0));

if (P != 0) // I2C Stop bit seen.
{
    ...
}


...with "I2C_SYSTEM_BUS_IRQ_PENDING_BIT" being defined in a header file for the specific PIC24 and project. (We have five different boards, using three different PIC24 chips):

Code:

#word IFS1 = getenv("SFR:IFS1")
#bit I2C_SYSTEM_BUS_IRQ_PENDING_BIT = IFS1.0    // bit 0 - slave I2C1 IRQ status

#word IFS3 = getenv("SFR:IFS3")
#bit I2C_SYSTEM_BUS_IRQ_PENDING_BIT = IFS3.1    // bit 1 - slave I2C2 IRQ status

#word IFS5 = getenv("SFR:IFS5")
#bit I2C_SYSTEM_BUS_IRQ_PENDING_BIT = IFS5.4    // bit 4 - slave I2C3 IRQ status


I just know it would be nice to have a system that doesn't crash (it hangs, writing when it should still be reading and then the watchdog resets) just because a message got 1 byte too long.
_________________
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 ?
allenhuffman



Joined: 17 Jun 2019
Posts: 537
Location: Des Moines, Iowa, USA

View user's profile Send private message Visit poster's website

PostPosted: Fri Apr 14, 2023 12:30 pm     Reply with quote

It looks like it is easy to workaround. Rather than relying on the state from i2c_isr_state() which is only set up to handle 0-127 bytes, you can check for the initial state of 0x00 (master is WRITING) or 0x80 (master is READING) and just set a static state flag (tri-state, read/write/idle). Then, if that flag is set to read, handle until you see a stop bit then reset the flag. If it is set to write, handle until you see a stop bit then reset the flag.

I have lightly tested it and blasted 1000 bytes to my board with no more issues.

Here is some PSEUDO code -- all the error checking and buffer checking and stuff I am using is removed for brevity.

I left in the comments of the original i2c_isr_state() example on how it works (0x0, 0x1-0x7f, 0x80, x081-0xff values).

Code:
// Processes the I2C interrupt for communications with the host.
//
// This must be defined like:
//
// #INT_SI2C3 NOCLEAR LEVEL=7
//
// NOCLEAR because we want to manually clear it inside the ISR and be able to
// detect the P stop bit. The P bit does not seem to be detectable otherwise.
static int s_mode = 0; // IDLE
void i2cISR (void)
{
    int state = i2c_isr_state (SYSTEM_BUS);

    I2C_SYSTEM_BUS_IRQ_PENDING_BIT = 0; // NOCLEAR, so we manually clear.

...indention removed for easier viewing...

/*-----------------------------------------------------------------------*/
// I2C Master WRITING to us (master writes address byte):
// Address byte needs to be read.
/*-----------------------------------------------------------------------*/
//if (state == RECEIVE_I2C) // I2C address match, R/W clear (writing).
if ((s_mode == 0) && (state == RECEIVE_I2C)) // I2C address match, R/W clear (writing).
{
    s_mode = 1; // Master is WRITING.

    indexRX = 0; // Reset receive buffer index.

    // Read byte which is the address.
    i2c_read (SYSTEM_BUS);
}
/*-----------------------------------------------------------------------*/
// I2C Master 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).
else if ((s_mode == 0) && (state == TRANSMIT_I2C)) // I2C address match, R/W set (reading).
{
    s_mode = -1; // Master is READING.

    indexTX = 0; // Reset transmit buffer index.

    // Read byte which is the address.
    i2c_read (SYSTEM_BUS);

    // Write out the first byte of data.
    i2c_write (SYSTEM_BUS, txBuf[indexTX++]);
}
/*-----------------------------------------------------------------------*/
// I2C Master READING from us (master reads data bytes):
// Transmits a message to the master.
/*-----------------------------------------------------------------------*/
//else if (state > TRANSMIT_I2C)
else if (s_mode < 0)
{
    // Send the data in the TX buffer to the master.
    byteToSend = txBuf[indexTX++];

    // This code is used to detect when the Master is done READING from us
    // so we can flag the response message as successfully sent:

    // Wait for either a stop bit, or another incoming data byte.
    while ((P == 0) && (I2C_SYSTEM_BUS_IRQ_PENDING_BIT == 0));

    if (P != 0) // I2C Stop bit seen.
    {
        LED_SlaveTXOff ();

        // Set flag indicating a message is waiting to be processed
        // in the main loop.
        // MessageReadyToBeProcessed = true;

        s_mode = 0; // Back to IDLE.
    }
}
/*-----------------------------------------------------------------------*/
// I2C Master WRITING to us (master writes data bytes):
// Receives a message from the master.
/*-----------------------------------------------------------------------*/
//else if (state > RECEIVE_I2C)
else if (s_mode > 0)
{
    // Prevent buffer overrun.
    if _indexRX < sizeof(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:

    // Wait for either a stop bit, or another incoming data byte.
    while ((I2C_SYSTEM_BUS_IRQ_PENDING_BIT == 0) && (P == 0));

    if (P != 0) // I2C Stop bit seen.
    {
        s_mode = 0; // Back to IDLE.
    }
}
else
{
    // Considered...
}

_________________
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 ?
temtronic



Joined: 01 Jul 2010
Posts: 9097
Location: Greensville,Ontario

View user's profile Send private message

PostPosted: Sat Apr 15, 2023 5:12 am     Reply with quote

Glad you found a solution !!
in the good old dinosaur dayze, I had to figure this stuff out myself as there was no internet..just databooks, teletypes and lots of pots of coffee Smile
allenhuffman



Joined: 17 Jun 2019
Posts: 537
Location: Des Moines, Iowa, USA

View user's profile Send private message Visit poster's website

PostPosted: Sat Apr 15, 2023 12:11 pm     Reply with quote

temtronic wrote:
Glad you found a solution !!
in the good old dinosaur dayze, I had to figure this stuff out myself as there was no internet..just databooks, teletypes and lots of pots of coffee Smile


I used to rely on my Pocket C book for programming… today, I don’t know if I could do anything unless I had Google!

Have you tested ChatGPT? It can make CCS PIC programs!
_________________
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 ?
allenhuffman



Joined: 17 Jun 2019
Posts: 537
Location: Des Moines, Iowa, USA

View user's profile Send private message Visit poster's website

PostPosted: Mon Apr 17, 2023 11:28 am     Reply with quote

temtronic wrote:
Glad you found a solution !!


I think I really need to look at the generated assembly and create our own ISR "state" handler that goes beyond 7-bits. In over-weekend testing, on a system with 27 boards all communicating using this code, it mostly works. I still have some issues to work out -- like how I handle when to clock stretch and my I2C Deadlock Detection code.

But, at least I can now write more than 127 bytes to the board without it freezing the I2C bus.
_________________
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 ?
allenhuffman



Joined: 17 Jun 2019
Posts: 537
Location: Des Moines, Iowa, USA

View user's profile Send private message Visit poster's website

PostPosted: Wed Apr 26, 2023 11:20 am     Reply with quote

CCS also sent me a modified version of their I2C ISR example code, which uses a similar workaround to what I came up with, so I guess we're okay.
_________________
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 ?
Ttelmah



Joined: 11 Mar 2010
Posts: 19215

View user's profile Send private message

PostPosted: Thu Apr 27, 2023 1:58 am     Reply with quote

Might be worth posting this.

I remember when you raised this that I had done transfers bigger than
127 bytes on a PIC18 before. Went and looked back, and found this was
what I did:
Code:

#byte SSP1STAT=getenv("SFR:SSP1STAT") //status register
#bit DATADDR=SSP1STAT.5 //data or address bit
#bit RDWR=SSP1STAT.2 //read/write bit

//16bit version of I2C_ISR_STATE uses 0x8000 for address match received
//instead 0f 0x80, and counts as I2C_ISR_STATE, but 1...7FFF and 0x8001...0xFFFF
#INLINE
unsigned int16 i2c_isr_state16(void)
{
   static int16 ctr=0;
   if (DATADDR==0)
   {
      ctr=0; //reset counter;
      ///address byte received
   }
   else
      ctr++;
   if (RDWR!=0)
   {
      bit_set(ctr,15);
   }     
   return(ctr);
}


Now this would need tweaking to use the right bit references for the PIC24
but shows what the function has to do.
allenhuffman



Joined: 17 Jun 2019
Posts: 537
Location: Des Moines, Iowa, USA

View user's profile Send private message Visit poster's website

PostPosted: Thu Apr 27, 2023 5:47 pm     Reply with quote

Ttelmah wrote:
Might be worth posting this.
...
unsigned int16 i2c_isr_state16(void)
[/code]

Now this would need tweaking to use the right bit references for the PIC24
but shows what the function has to do.


Doing the 16-bit version is something I thought about doing -- thanks for sharing.
_________________
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 ?
Display posts from previous:   
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion All times are GMT - 6 Hours
Goto page 1, 2  Next
Page 1 of 2

 
Jump to:  
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