Bug in "fsl_flexcan.c" / wrong treatment of the 'FDRATE'-bit

[buescher]

Greetings all,

Just about to get my feet wet with CAN FD in an RT1064, and had a first look at what was provided by NXP.

I think there's a bug in the treatment of the 'FDRATE'-bit (31) in register 'FDCTRL', module "fsl_flexcan.c" (at least with the one used by the "canfd_interrupt_transfer" demo provided for the RT1064 evaluation board). It's in this function:

void FLEXCAN_FDInit( CAN_Type *base, const flexcan_config_t *config, uint32_t sourceClock_Hz, flexcan_mb_size_t dataSize, bool brs)

Here, "bool brs" means "bitrate switch enable" (TRUE=allow transmission and reception of CAN FD messages, FALSE=only allow normal "CAN" frames regardless of the mailbox/message-buffer configuration). I guess this should be mapped to register 'FDCTRL', bit 'FDRATE' ("brs" = "FDRATE", oh well...) .

Here is what I think is wrong - C++ style comments were added by myself, when trying to understand what this code was supposed to do:

/* Enable FD operation and set bitrate switch. */
    if (brs)
     { 
        base->FDCTRL &= CAN_FDCTRL_FDRATE_MASK;  // CLEAR anything except 'FDRATE' (bit 31) ? ? ?
     }
    else // disable "FD"-operation (allow "normal" CAN frames only)
     {
        base->FDCTRL &= ~CAN_FDCTRL_FDRATE_MASK; // CLEAR the 'FDRATE'-bit (31)
     }

What *I guess* should be done in FLEXCAN_FDInit() is :

  /* Enable FD operation and set bitrate switch. */
    if (brs)
     { 
        base->FDCTRL |= CAN_FDCTRL_FDRATE_MASK;  // SET 'FDRATE' but leave other bits unchanged
     }
    else // disable "FD"-operation (allow "normal" CAN frames only)
     {
        base->FDCTRL &= ~CAN_FDCTRL_FDRATE_MASK; // CLEAR the 'FDRATE'-bit (31)
     }

Strangely, even with the original code (which either clears ANYTHING BUT 'FDRATE' or clears only 'FDRATE', but never sets that bit), I am able to transmit "FD"-frames besides normal CAN frames.

Maybe NXP can correct this, or (if the strange bitwise-and to clear ANYTHING BUT 'FDRATE' was intended this way), add a helpful hint / comment in the sourcecode, documenting why the bit isn't bitwise OR-ed to "set" it in Mr. FlexCAN(3).

  Cheers, Wolfgang .

[kerryzhou · Accepted solution]

Hi Wolfgang Buescher ,

  Thanks a lot for your bug report.

  I totally agree with you that the code is wrong, it should be:

  /* Enable FD operation and set bitrate switch. */
    if (brs)
     { 
        base->FDCTRL |= CAN_FDCTRL_FDRATE_MASK;  // SET 'FDRATE' but leave other bits unchanged
     }

You can make it works, because the FDRATE bit is 1 after reset.

I will report this bug to our related department, and it will be fixed in the next SDK version.

Thanks again for the valuable contribution.

Have a great day,
Kerry

-------------------------------------------------------------------------------
Note:
- If this post answers your question, please click the "Mark Correct" button. Thank you!

- We are following threads for 7 weeks after the last post, later replies are ignored
Please open a new thread and refer to the closed one, if you have a related question at a later point in time.
-------------------------------------------------------------------------------

[buescher]

Thanks Kerry. The reset-value "set" = FDRATE enabled perfectly explains why the code works, as long as you never turned 'FDRATE' off in your application (and that's what happened here, and caused a bit of head-scratching).

The next issue (in another thread, #503656) about not being able to access certain 'Enhanced' timing registers seems a tougher nut to crack. But that's OT here. Case closed.

Have a nice day,

   Wolf .