Some can_17xx_40xx.c functions don't support multiple CAN controllers

cancel
Showing results for 
Show  only  | Search instead for 
Did you mean: 

Some can_17xx_40xx.c functions don't support multiple CAN controllers

640 Views
jkingeet
Contributor I

I'm working on a project that uses the CAN1 and CAN2 interfaces on the LPC1778. I pulled up some example code to figure out how the acceptance filtering works and I got it working properly with just one interface (CAN1) using the following modified AFLUT (Acceptance Filter Look-Up Table):

#define CAN1 0
#define CAN2 1

CAN_EXT_ID_RANGE_ENTRY_T EffGrpSection[] = {
 {{CAN1, 0}, {CAN1, 0x1FFFFFFF}}
};

CANAF_LUT_T AFSections = {
 NULL, 0, // No fullCAN section
 NULL, 0, // No Standard explicit section
 NULL, 0, // No Standard range section
 NULL, 0, // No Extended explicit section
 EffGrpSection, sizeof(EffGrpSection) / sizeof(CAN_EXT_ID_RANGE_ENTRY_T)
};‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍

I then call the Chip_CAN_SetAFLUT() function thusly:

Chip_CAN_SetAFLUT(LPC_CANAF, LPC_CANAF_RAM, &AFSections);

And it works out. The single entry from my EFFGrpSection array gets added to the LPC_CANAF_RAM section under MASK[0] and MASK[1] (0x1FFFFFFF = 536870911):
pastedImage_4.png

Just as a note before I move on: I'm using an extended range that encompasses the entire 29 bit address space instead of using CAN_AF_BYBASS_MODE to avoid receiving my own transmissions.

Problems arise when I expand my EEFGrpSection array to include the second controller:

#define CAN1 0
#define CAN2 1

CAN_EXT_ID_RANGE_ENTRY_T EffGrpSection[] = {
 {{CAN1, 0}, {CAN1, 0x1FFFFFFF}},
 {{CAN2, 0}, {CAN2, 0x1FFFFFFF}}
};‍‍‍‍‍‍‍‍‍‍‍‍‍‍

Now when I called Chip_CAN_SetAFLUT(), I would expect to see MASK[2] and MASK[3] populated, but I don't:
pastedImage_5.png

Why not? Well, the problem exists in a function within Chip_CAN_SetAFLUT() called setupEXTRangeSection():

/* Setup Group Extended ID section */
STATIC Status setupEXTRangeSection(uint32_t *pCANAFRamAddr,
 CAN_EXT_ID_RANGE_ENTRY_T *pExtRangeCANSec,
 uint16_t EntryNum)
{
 return setupEXTSection(pCANAFRamAddr, (CAN_EXT_ID_ENTRY_T *) pExtRangeCANSec, EntryNum * 2);
}
‍‍‍‍‍‍‍

This function contains another function called setupEXTSection() and this is where the problem truly lies:

/* Setup the Extended ID Section */
STATIC Status setupEXTSection(uint32_t *pCANAFRamAddr, CAN_EXT_ID_ENTRY_T *pExtCANSec, uint16_t EntryNum)
{
 uint16_t i;
 uint32_t CurID = 0;
 uint32_t Entry;
 uint16_t EntryCnt = 0;

 /* Setup FullCAN section */
 for (i = 0; i < EntryNum; i++) {
 if (CurID > pExtCANSec[i].ID_29) {
 return ERROR;
 }
 CurID = pExtCANSec[i].ID_29;
 Entry = createExtIDEntry(&pExtCANSec[i]);
 pCANAFRamAddr[EntryCnt] = Entry;
 EntryCnt++;
 }
 return SUCCESS;

}‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍

The underlying problem is that this function doesn't take the controller# into consideration. Only the .ID_29 field of the CAN_EXT_ID_ENTRY_T structure is used. This is a problem because of line 11 in the code snippet above. The conditional compares a 29 bit ID to the previous 29 bit ID in the sequence before adding it to LPC_CANAF_RAM. It does this because, according to the user manual (UM10470):

the Extended Range must be arranged in ascending numerical order

The 32-bit numbers to put in ascending numerical order are supposed to take the form:

pastedImage_12.png

As I said, the conditional in line 11 doesn't take the controller# into consideration, which makes my array of values look like {0, 0x1FFFFFFF, 0, 0x1FFFFFFF}. This makes sense why it only adds 2 entries to the LPC_CANAF_RAM, because the third entry is smaller than the second, so this function executes the "return ERROR" line.

What this array should look like is: {0, 0x1FFFFFFF, 0x20000000, 0x3FFFFFFF}. The 0x20000000 and 0x3FFFFFFFF are the result of shifting the controller# 29 bits to the left before ORing with the 29 bit address. Something like:

val = (pExtCANSec.CtrlNo << 29) | pExtCANSec.ID_29‍‍‍‍

In order to accommodate this, the setupEXTSection() function should look something like this:

#define CAN_EXT_ENTRY_CTRL_NO_POS (29 ) // This is actually defined in can_17xx_40xx.h

/* Setup the Extended ID Section */
STATIC Status setupEXTSection(uint32_t *pCANAFRamAddr, CAN_EXT_ID_ENTRY_T *pExtCANSec, uint16_t EntryNum)
{
 uint16_t i;
 uint32_t CurID = 0;
 uint32_t Entry;
 uint16_t EntryCnt = 0;

 /* Setup FullCAN section */
 for (i = 0; i < EntryNum; i++) {
 if (CurID > ((pExtCANSec[i].CtrlNo << CAN_EXT_ENTRY_CTRL_NO_POS) | pExtCANSec[i].ID_29)) {
 return ERROR;
 }
 CurID = (pExtCANSec[i].CtrlNo << CAN_EXT_ENTRY_CTRL_NO_POS) | pExtCANSec[i].ID_29;
 Entry = createExtIDEntry(&pExtCANSec[i]);
 pCANAFRamAddr[EntryCnt] = Entry;
 EntryCnt++;
 }
 return SUCCESS;

}‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍

And this produces the entries in the LPC_CANAF_RAM as expected:
pastedImage_13.png

Whoever wrote this function wasn't thinking about multiple controllers at the time of writing. This same error exists in the setupSTDSection() function in the same file, which would cause the same issue when using non-extended IDs with multiple controllers.

There may be more issues with this. I didn't comb over the entire library, so I can't be sure. At the very least, the sample code isn't consistent with the chip's capabilities.

I don't know if I'm the first person to notice this and post about it.

 

What's a little funny is that the erroneous function contains another function called createExtIDEntry(), which it uses to create a 32-bit number to store in LPC_CANAF_RAM. This function properly uses the .CtrlNo field when creating the 32-bit number, but the erroneous function that calls it doesn't :smileylaugh:

0 Kudos
1 Reply

568 Views
Alexis_A
NXP TechSupport
NXP TechSupport

Thanks a lot for your feedback!

You're right, there's some problems with the driver and the use of the two CAN channels. I will report this to the applications team.

Best Regards,

Alexis Andalon

0 Kudos