CAN filters

[panacekcz]

Hi,

I wanted to set a single CAN filter on STM32F107 like this:

Code: Select all

canSTM32SetFilters(1, 1, &comm_can_filter);

That means one filter for CAN1 and no filters for CAN2. However, this causes panic in canSTM32SetFilters.

Code: Select all

chDbgCheck((can2sb > 1)...

I don't see why can2sb cannot be 1.
The reference manual says to CAN2SB[5:0]:

They define the start bank for the CAN2 interface (Slave) in the range 1 to 27.

Also, the check (num < STM32_CAN_MAX_FILTERS) does not allow num to be 28, which should be ok (to use all filters).

So I think the check should be:

Code: Select all

 chDbgCheck(
             (can2sb >= 1) &&
             (can2sb < STM32_CAN_MAX_FILTERS) &&
             (num <= STM32_CAN_MAX_FILTERS),
             "canSTM32SetFilters");

Additionally, I think that can_lld_set_filters is incorrect when there is no CAN2 (for example STM32F103):
can_lld_init calls

Code: Select all

can_lld_set_filters(STM32_CAN_MAX_FILTERS, 0, NULL);

so can2sb is 14 and

Code: Select all

CAN1->sFilterRegister[can2sb].FR1 = 0;
CAN1->sFilterRegister[can2sb].FR2 = 0;

writes out of bounds of sFilterRegister[14].

Which could be fixed like this:

Code: Select all

    CAN1->sFilterRegister[0].FR1 = 0;
    CAN1->sFilterRegister[0].FR2 = 0;
#if STM32_HAS_CAN2
    CAN1->sFilterRegister[can2sb].FR1 = 0;
    CAN1->sFilterRegister[can2sb].FR2 = 0;
#endif
    CAN1->FM1R = 0;
    CAN1->FFA1R = 0;
#if STM32_HAS_CAN2
    CAN1->FS1R = 1 | (1 << can2sb);
    CAN1->FA1R = 1 | (1 << can2sb);
#else
    CAN1->FS1R = 1;
    CAN1->FA1R = 1;
#endif

Maybe all references to can2sb (the assignment to CAN1->FMR and the checks in canSTM32SetFilters) could be elliminated for the devices with no CAN2?

Vlastimil Dort

[efuentes]

I agree that 1 should be a valid start filter and I also agree that num <= STM32_CAN_MAX_FILTERS.

In devices with no CAN2, I think you meant to say that can2sb is equal to 28, which is out of bounds.

The fixes you are proposing seem reasonable.

[Giovanni]

Hi,

I will merge this fix tomorrow in both branches, thanks.

Giovanni

[Giovanni]

Hi,

Fixed in repository, could you please confirm it is working as expected now?

Giovanni

[panacekcz]

Hi,
thanks for the fix,

I can only test it on STM32F107 with ChibiOS 2.6, which seems ok (but I haven't actually run it yet).

However, in the trunk, there is (num <? STM32_CAN_MAX_FILTERS) instead of (num <= STM32_CAN_MAX_FILTERS),
and I am also not sure why you omitted the #else part, this way the FS1R and FA1R would be untouched when there is no CAN2.

Vlastimil Dort

[Giovanni]

Now it should be ok, missed that part.

Giovanni