tonton81 / FlexCAN_T4

FlexCAN (CAN 2.0 / CANFD) Library for Teensy 3.x and 4.0
https://forum.pjrc.com/threads/56035-FlexCAN_T4-FlexCAN-for-Teensy-4
MIT License
192 stars 63 forks source link

writeIFLAGBit() is clearing ALL bits, not just one #62

Closed bsundahl1 closed 1 year ago

bsundahl1 commented 1 year ago

I am trying to setup an "available()" function to improve the non-interrupt read() performance. I wrote some test code that reads frames at a very slow rate so that I can see multiple frames being handled by the mailboxes, and to see if the IFLAGs can be used to count available frames. But it wasn't working properly. I can see multiple frames being picked up up by multiple RX mailboxes and the IFLAGs being set, but when I call readMB() once, all of the IFLAGs were being cleared. I tracked this to a bug in writeIFLAGBit() in FlexCAN_T4.tpp, which is clearing 32-bits at a time, rather than just the one it is supposed to.

FCTP_FUNC void FCTP_OPT::writeIFLAGBit(uint8_t mb_num) {
  if ( mb_num < 32 ) FLEXCANb_IFLAG1(_bus) |= (1UL << mb_num);
  else FLEXCANb_IFLAG2(_bus) |= (1UL << (mb_num - 32));
}

A read-modify-write does not work on this register. Writing a "1" clears the flag, so reading, then writing the whole register, will clear ALL of the flags in the register. OR-ing in the MB flag does nothing. Changing the "|=" to "=" makes this behave as I would expect.

tonton81 commented 1 year ago

" I can see multiple frames being picked up up by multiple RX mailboxes and the IFLAGs being set, but when I call readMB() once, all of the IFLAGs were being cleared. I tracked this to a bug in writeIFLAGBit() in FlexCAN_T4.tpp, which is clearing 32-bits at a time, rather than just the one it is supposed to."

true, just notiuced that a boolean value can clear the IFLAG register, but if the mailboxes still have data the iflags will be active again. even if you read one mailbox, you'll have to clear the flag, which clears all, as it seems, but the iflags of the mailboxes still unread will still be active.

the quote above is kinda contradicting, writing any value clears it, so not a bug. but the unaccessed mailbox flags will still be intact.

tonton81 commented 1 year ago

if you read one or many mailboxes, either way the clear is set, the function only reads one mailbox, the interrupt reads all

if you read a mailbox and set the flag that opens the available spot to a new message to drop in and can reactivate the iflag bit again, if the code is in a loop check then that would be the intended effect

bsundahl1 commented 1 year ago

"true, just notiuced that a boolean value can clear the IFLAG register, but if the mailboxes still have data the iflags will be active again. even if you read one mailbox, you'll have to clear the flag, which clears all, as it seems, but the iflags of the mailboxes still unread will still be active."

My point is that clearing one flag should not clear all. This is a special type of register and the usual read-modify-write operation does not work. Writing a pattern to this register does not write the pattern to IFLAG, rather each "1" in the pattern clears the respective flag. If you use the following code (using "=" instead of "|=", then it will clear a single flag as expected.

 if ( mb_num < 32 ) FLEXCANb_IFLAG1(_bus) = (1UL << mb_num);
 else FLEXCANb_IFLAG2(_bus) = (1UL << (mb_num - 32));

Maybe this does not cause any issues in your driver, but it fixed the problem that I was having.

tonton81 commented 1 year ago

ok now i understand :) if you want to submit a fix as a contributor feel free, if not i'll edit it this weekend

thank you

tonton81 commented 1 year ago

https://github.com/tonton81/FlexCAN_T4/commit/5f434c4dba6984bad89dc70362b7c1bda93ce40c