can_lld_set_filters (stm32 CANv1) clobbers filters if called multiple times

[faisal]

If you call canSTM32SetFilters multiple times, even without overlapping filter numbers in the passed in CANFilter struct, can_lld_set_filters clears our previously configured filters. This does not allow you to incrementally add to the configured filters. This is strange because the loop in can_lld_set_filters where it writes to the filter registers actually respects the filter number that you passed in thru CANFilter. The following is a small reordering which will 'fix' it.

Code: Select all

static void can_lld_set_filters(CANDriver* canp,
                                uint32_t can2sb,
                                uint32_t num,
                                const CANFilter *cfp) {

    uint32_t i;
#if STM32_CAN_USE_CAN2
  if (canp == &CAND2) {
    /* Set handle to CAN1, because CAN1 manages the filters of CAN2.*/
    canp = &CAND1;
  }
#endif

  /* Temporarily enabling CAN clock.*/
#if STM32_CAN_USE_CAN1
  if (canp == &CAND1) {
    rccEnableCAN1(true);
    /* Filters initialization.*/
    canp->can->FMR = (canp->can->FMR & 0xFFFF0000) | CAN_FMR_FINIT;
    canp->can->FMR = (canp->can->FMR & 0xFFFF0000) | (can2sb << 8) | CAN_FMR_FINIT;
  }
#endif

#if STM32_CAN_USE_CAN3
  if (canp == &CAND3) {
    rccEnableCAN3(true);
    /* Filters initialization.*/
    canp->can->FMR = (canp->can->FMR & 0xFFFF0000) | CAN_FMR_FINIT;
  }
#endif

  if (num > 0) {
    uint32_t fmask;

    /* Scanning the filters array.*/
    for (i = 0; i < num; i++) {
      fmask = 1 << cfp->filter;
      if (cfp->mode)
        canp->can->FM1R |= fmask;
      if (cfp->scale)
        canp->can->FS1R |= fmask;
      if (cfp->assignment)
        canp->can->FFA1R |= fmask;
      canp->can->sFilterRegister[cfp->filter].FR1 = cfp->register1;
      canp->can->sFilterRegister[cfp->filter].FR2 = cfp->register2;
      canp->can->FA1R |= fmask;
      cfp++;
    }
  }
  else {
    /* All filters cleared.*/
    canp->can->FA1R = 0;
    canp->can->FM1R = 0;
    canp->can->FS1R = 0;
    canp->can->FFA1R = 0;

#if STM32_CAN_USE_CAN1
    if(canp == &CAND1) {
      for (i = 0; i < STM32_CAN_MAX_FILTERS; i++) {
        canp->can->sFilterRegister[i].FR1 = 0;
        canp->can->sFilterRegister[i].FR2 = 0;
      }
    }
#endif

#if STM32_CAN_USE_CAN3
    if(canp == &CAND3) {
      for (i = 0; i < STM32_CAN3_MAX_FILTERS; i++) {
        canp->can->sFilterRegister[i].FR1 = 0;
        canp->can->sFilterRegister[i].FR2 = 0;
      }
    }
#endif

    /* Setting up a single default filter that enables everything for both
       CANs.*/
    canp->can->sFilterRegister[0].FR1 = 0;
    canp->can->sFilterRegister[0].FR2 = 0;
#if STM32_CAN_USE_CAN2
    if (canp == &CAND1) {
      canp->can->sFilterRegister[can2sb].FR1 = 0;
      canp->can->sFilterRegister[can2sb].FR2 = 0;
    }
#endif
    canp->can->FM1R = 0;
    canp->can->FFA1R = 0;
    canp->can->FS1R = 1;
    canp->can->FA1R = 1;
#if STM32_CAN_USE_CAN2
    if (canp == &CAND1) {
      canp->can->FS1R |= 1 << can2sb;
      canp->can->FA1R |= 1 << can2sb;
    }
#endif
  }
  canp->can->FMR &= ~CAN_FMR_FINIT;

  /* Clock disabled, it will be enabled again in can_lld_start().*/
  /* Temporarily enabling CAN clock.*/
#if STM32_CAN_USE_CAN1
  if (canp == &CAND1) {
    rccDisableCAN1();
  }
#endif
#if STM32_CAN_USE_CAN3
  if (canp == &CAND3) {
    rccDisableCAN3();
  }
#endif
}

[Giovanni]

Hi,

I am not sure about this, this function is a "set" filters, not an "add" filters". Not saying that it would not be useful but it would also require something to clear filters and start over.

Giovanni

[steved]

Have a look at the attached; think it contains some useful routines for setting single filters.
(Don't adopt it wholesale; it's not been brought up to date with latest Chibi version, and may have other surprises; but it does work)

[faisal]

Hi,

I am not sure about this, this function is a "set" filters, not an "add" filters". Not saying that it would not be useful but it would also require something to clear filters and start over.

Giovanni

Okay, so maybe add another function to clear out all the filters?