[DEV] STM32G4xx support

This forum is dedicated to feedback, discussions about ongoing or future developments, ideas and suggestions regarding the ChibiOS projects are welcome. This forum is NOT for support.
julester23
Posts: 6
Joined: Sat Jan 04, 2020 5:25 pm
Has thanked: 1 time

Re: [DEV] STM32G4xx support

Postby julester23 » Fri Mar 27, 2020 10:28 pm

Giovanni wrote:Hi Shevchenko,
@Julester23, any comment?
Giovanni


I'm afraid I didn't finish setting up mailboxes.

4) This is off by one. Mailbox is one-indexed in every other function, not zero-indexed. Setting mailbox to 0 (CAN_ANY_MAILBOX) will actually request sending TX Buffer 1, setting mailbox to 1 will send mailbox 2.

What to do with CAN_ANY_MAILBOX? Setting TXBAR to 7 doesn't make much sense - ANY does not imply ALL.

Code: Select all

// Validate mailbox > 0 and < 4
canp->fdcan->TXBAR = (uint32_t)1 << ((uint32_t)mailbox - 1U);

User avatar
alex31
Posts: 374
Joined: Fri May 25, 2012 10:23 am
Location: toulouse, france
Has thanked: 38 times
Been thanked: 61 times
Contact:

Re: [DEV] STM32G4xx support

Postby alex31 » Sat Mar 28, 2020 11:40 am

Shevchenko wrote:2)

Code: Select all

typedef struct {
  /**
   * @brief   Frame header.
   */
  union {
    struct {
      union {
        uint32_t              EID:29;     /**< @brief Extended Identifier.    */
        struct {
          uint32_t            _R1:18;
          uint32_t            SID:11;     /**< @brief Standard identifier.    */
          uint32_t            RTR:1;      /**< @brief Remote transmit request.*/
          uint32_t            XTD:1;      /**< @brief Extended identifier.    */
          uint32_t            ESI:1;      /**< @brief Error state indicator.  */
        };
      };
      uint16_t                RXTS:16;    /**< @brief TX time stamp.          */
      uint8_t                 DLC:4;      /**< @brief Data length code.       */
      uint8_t                 BRS:1;      /**< @brief Bit rate switch.        */
      uint8_t                 FDF:1;      /**< @brief FDCAN frame format.     */
      uint8_t                 _R2:2;
      uint8_t                 FIDX:7;     /**< @brief Filter index.           */
      uint8_t                 ANMF:1;     /**< @brief Accepted non-matching
                                                      frame.                  */
    };
  uint32_t                  header32[2];
  };



Hello,

Beware that memory layout of C bit-field is compiler dependant. This code will probably work with gcc, but without guaranties that it works with any compilers or future versions of gcc. It's a case of undefined behaviour. This code can't be MISRA or CERT certified.

3

The C11 standard says:

An implementation may allocate any addressable storage unit large enough to hold a bitfield. If enough space remains, a bit-field that immediately follows another bit-field in a structure shall be packed into adjacent bits of the same unit. If insufficient space remains, whether a bit-field that does not fit is put into the next unit or overlaps adjacent units is implementation-defined. The order of allocation of bit-fields within a unit (high-order to low-order or low-order to high-order) is implementation-defined.


Alexandre

Shevchenko
Posts: 11
Joined: Tue Nov 13, 2018 9:25 am
Has thanked: 1 time
Been thanked: 3 times

Re: [DEV] STM32G4xx support

Postby Shevchenko » Mon May 25, 2020 11:58 am

Maybe to change the CAN sending and receiving function and structure as:
1. CANTxFrame:

Code: Select all

typedef struct {
  /**
   * @brief   Frame header.
   */
  union {
    uint32_t            EID:29;     /**< @brief Extended Identifier.    */
    struct {
      uint32_t            _R1:18;
      uint32_t            SID:11;     /**< @brief Standard identifier.    */
    };
  };
  uint32_t          RTR:1;      /**< @brief Remote transmit request.*/
  uint32_t          XTD:1;      /**< @brief Extended identifier.    */
  uint32_t          ESI:1;      /**< @brief Error state indicator.  */
  uint32_t          DLC:4;      /**< @brief Data length code.       */
  uint32_t          BPS:1;      /**< @brief Accepted non-matching
                                            frame.                  */
  uint32_t          FDF:1;      /**< @brief FDCAN frame format.     */
  uint32_t          EFC:1;      /**< @brief Event FIFO control.     */
  uint32_t          MM:8;       /**< @brief Message event marker.   */

  /**
   * @brief   Frame data.
   */
  union {
    uint8_t                 data8[CAN_MAX_DLC_BYTES];
    uint16_t                data16[CAN_MAX_DLC_BYTES / 2];
    uint32_t                data32[CAN_MAX_DLC_BYTES / 4];
  };
} CANTxFrame;


2. CANRxFrame:

Code: Select all

typedef struct {
  /**
   * @brief   Frame header.
   */
  union {
    uint32_t            EID:29;     /**< @brief Extended Identifier.    */
    struct {
      uint32_t            _R1:18;
      uint32_t            SID:11;     /**< @brief Standard identifier.    */
    };
  };
  uint32_t            RTR:1;      /**< @brief Remote transmit request.*/
  uint32_t            XTD:1;      /**< @brief Extended identifier.    */
  uint32_t            ESI:1;      /**< @brief Error state indicator.  */
  uint16_t            RXTS:16;    /**< @brief TX time stamp.          */
  uint8_t             DLC:4;      /**< @brief Data length code.       */
  uint8_t             BRS:1;      /**< @brief Bit rate switch.        */
  uint8_t             FDF:1;      /**< @brief FDCAN frame format.     */
  uint8_t             FIDX:7;     /**< @brief Filter index.           */
  uint8_t             ANMF:1;     /**< @brief Accepted non-matching
                                                  frame.               */
  /**
   * @brief   Frame data.
   */
  union {
    uint8_t                 data8[CAN_MAX_DLC_BYTES];
    uint16_t                data16[CAN_MAX_DLC_BYTES / 2];
    uint32_t                data32[CAN_MAX_DLC_BYTES / 4];
  };
} CANRxFrame;


3. can_lld_transmit:

Code: Select all

void can_lld_transmit(CANDriver *canp,
                      canmbx_t mailbox,
                      const CANTxFrame *ctfp) {
  uint32_t *tx_address;
  (void)mailbox;

  osalDbgCheck(dlc_to_bytes[ctfp->DLC] <= CAN_MAX_DLC_BYTES);

  /* Writing frame.*/
  tx_address = canp->ram_base + (SRAMCAN_TBSA / sizeof (uint32_t));
 
  if (ctfp->XTD == 0) {
    *tx_address = (ctfp->SID << 18);
  }
  else {
    *tx_address = (ctfp->EID << 0) | (ctfp->XTD << 30);
  }
  *tx_address++ |=  (ctfp->RTR << 29) | (ctfp->ESI << 31);

  *tx_address++ |= (ctfp->DLC << 16) | (ctfp->BPS << 20) | \
                (ctfp->FDF << 21) | (ctfp->EFC << 23) | \
                (ctfp->MM << 24);
 
  for (unsigned i = 0U; i < dlc_to_bytes[ctfp->DLC]; i += 4U) {
    *tx_address++ = ctfp->data32[i / 4U];
  }

  /* Starting transmission.*/
  canp->fdcan->TXBAR = (uint32_t)1 << (uint32_t)mailbox;
}


4. can_lld_receive:

Code: Select all

#define CAN_RX_FRAME_MASK_STDID ((uint32_t)0x1FFC0000U) /* Standard Identifier         */
#define CAN_RX_FRAME_MASK_EXTID ((uint32_t)0x1FFFFFFFU) /* Extended Identifier         */
#define CAN_RX_FRAME_MASK_RTR   ((uint32_t)0x20000000U) /* Remote Transmission Request */
#define CAN_RX_FRAME_MASK_XTD   ((uint32_t)0x40000000U) /* Extended Identifier         */
#define CAN_RX_FRAME_MASK_ESI   ((uint32_t)0x80000000U) /* Error State Indicator       */
#define CAN_RX_FRAME_MASK_RXTS  ((uint32_t)0x0000FFFFU) /* Timestamp                   */
#define CAN_RX_FRAME_MASK_DLC   ((uint32_t)0x000F0000U) /* Data Length Code            */
#define CAN_RX_FRAME_MASK_BRS   ((uint32_t)0x00100000U) /* Bit Rate Switch             */
#define CAN_RX_FRAME_MASK_FDF   ((uint32_t)0x00200000U) /* FD Format                   */
#define CAN_RX_FRAME_MASK_FIDX  ((uint32_t)0x7F000000U) /* Filter Index                */
#define CAN_RX_FRAME_MASK_ANMF  ((uint32_t)0x80000000U) /* Accepted Non-matching Frame */

void can_lld_receive(CANDriver *canp, canmbx_t mailbox, CANRxFrame *crfp) {
  uint32_t get_index;
  uint32_t *rx_address;

  if (mailbox == CAN_ANY_MAILBOX) {
    if (can_lld_is_rx_nonempty(canp, 1U)) {
      mailbox = 1U;
    }
    else if (can_lld_is_rx_nonempty(canp, 2U)) {
      mailbox = 2U;
    }
    else {
      return;
    }
  }

  /* GET index, add it and the length to the rx_address.*/
  get_index = (canp->fdcan->RXF0S & FDCAN_RXF0S_F0GI_Msk) >> FDCAN_RXF0S_F0GI_Pos;
  rx_address = canp->ram_base + (SRAMCAN_RF0SA +
                                 (get_index * SRAMCAN_RF0_SIZE)) / sizeof (uint32_t);
 
  crfp->XTD = *rx_address & CAN_RX_FRAME_MASK_XTD;
  if (crfp->XTD == 0) {
    crfp->SID = ((*rx_address & CAN_RX_FRAME_MASK_STDID) >> 18);
  }
  else {
    crfp->EID = ((*rx_address & CAN_RX_FRAME_MASK_EXTID) >> 0);
  }
  crfp->RTR = ((*rx_address & CAN_RX_FRAME_MASK_RTR) >> 29);
  crfp->ESI = ((*rx_address & CAN_RX_FRAME_MASK_ESI) >> 31);
  rx_address++;
  crfp->RXTS = ((*rx_address & CAN_RX_FRAME_MASK_RXTS) >> 0);
  crfp->DLC = ((*rx_address & CAN_RX_FRAME_MASK_DLC) >> 16);
  crfp->BRS = ((*rx_address & CAN_RX_FRAME_MASK_BRS) >> 20);
  crfp->FDF = ((*rx_address & CAN_RX_FRAME_MASK_FDF) >> 21);
  crfp->FIDX = ((*rx_address & CAN_RX_FRAME_MASK_FIDX) >> 24);
  crfp->ANMF = ((*rx_address & CAN_RX_FRAME_MASK_ANMF) >> 31);
  rx_address++;

  /* Copy message from FDCAN peripheral's SRAM to structure. RAM is restricted
     to word aligned accesses, so up to 3 extra bytes may be copied.*/
  for (unsigned i = 0U; i < dlc_to_bytes[crfp->DLC]; i += 4U) {
    crfp->data32[i / 4U] = *rx_address++;
  }

  /* Acknowledge receipt by writing the get-index to the acknowledge
     register RXFxA then re-enable RX FIFO message arrived interrupt once
     the FIFO is emptied.*/
  if (mailbox == 1U) {
    uint32_t rxf0a = canp->fdcan->RXF0A;
    rxf0a &= ~FDCAN_RXF0A_F0AI_Msk;
    rxf0a |= get_index << FDCAN_RXF0A_F0AI_Pos;
    canp->fdcan->RXF0A = rxf0a;

    if (!can_lld_is_rx_nonempty(canp, mailbox)) {
//      canp->fdcan->IR  = FDCAN_IR_RF0N;
      canp->fdcan->IE |= FDCAN_IE_RF0NE;
    }
  }
  else {
    uint32_t rxf1a = canp->fdcan->RXF1A;
    rxf1a &= ~FDCAN_RXF1A_F1AI_Msk;
    rxf1a |= get_index << FDCAN_RXF1A_F1AI_Pos;
    canp->fdcan->RXF1A = rxf1a;

    if (!can_lld_is_rx_nonempty(canp, mailbox)) {
//      canp->fdcan->IR  = FDCAN_IR_RF1N;
      canp->fdcan->IE |= FDCAN_IE_RF1NE;
    }
  }
}

Shevchenko
Posts: 11
Joined: Tue Nov 13, 2018 9:25 am
Has thanked: 1 time
Been thanked: 3 times

Re: [DEV] STM32G4xx support

Postby Shevchenko » Tue May 26, 2020 3:10 pm

To run FDCAN filters, I suggest:
1. Add the RXGFC variable to the CANConfig structure. Because write access by the bits is possible only when the bit 1 [CCE] and bit 0 [INIT] of CCCR register are set to 1. And eventually add definition:

Code: Select all

#define FDCAN_SET_RXGFC_LSS(value) (value << FDCAN_RXGFC_LSS_Pos)
#define FDCAN_SET_RXGFC_LSE(value) (value << FDCAN_RXGFC_LSE_Pos)
#define FDCAN_SET_RXGFC_ANFS(value) (value << FDCAN_RXGFC_ANFS_Pos)
#define FDCAN_SET_RXGFC_ANFE(value) (value << FDCAN_RXGFC_ANFE_Pos)
#define FDCAN_SET_RXGFC_RRFS(value) (value << FDCAN_RXGFC_RRFS_Pos)
#define FDCAN_SET_RXGFC_RRFE(value)(value << FDCAN_RXGFC_RRFE_Pos)

But stm32h7xx has not FDCAN_SET_RXGFC_LSS and FDCAN_SET_RXGFC_LSE.

2. To make filter configuration similar to previous versions of CAN drivers:

Code: Select all

typedef struct {
  /**
   * @brief   Filter scale.
   * @note    This bit associated to this
   *          filter (0=16 bits mode, 1=32 bits mode).
   */
  uint32_t                  scale:1;

  /**
   * @brief   Number of the filter to be programmed.
   */
  uint32_t                  filter;

  /**
   * @brief   Filter Type.
   */
  union {
  /**
    * @brief  Standard filter type.
    * @note   – 00: Range filter from SFID1 to SFID2
    *         – 01: Dual ID filter for SFID1 or SFID2
    *         – 10: Classic filter: SFID1 = filter, SFID2 = mask
    *         – 11: Filter element disabled
    */
    uint8_t               SFT:2;

  /**
    * @brief  Extended filter type.
    * @note   – 00: Range filter from EF1ID to EF2ID (EF2ID >= EF1ID)
    *         – 01: Dual ID filter for EF1ID or EF2ID
    *         – 10: Classic filter: EF1ID = filter, EF2ID = mask
    *         – 11: Range filter from EF1ID to EF2ID (EF2ID >= EF1ID), XIDAM mask not applied
    */
    uint8_t               EFT:2;
  };

  /**
   * @brief   Standard or Extended filter element configuration.
   * @note    – 000: Disable filter element
   *          – 001: Store in Rx FIFO 0 if filter matches
   *          – 010: Store in Rx FIFO 1 if filter matches
   *          – 011: Reject ID if filter matches
   *          – 100: Set priority if filter matches
   *          – 101: Set priority and store in FIFO 0 if filter matches
   *          – 110: Set priority and store in FIFO 1 if filter matches
   *          – 111: Not used
   */
  union {
    uint8_t               SFEC:3;
    uint8_t               EFEC:3;
  };
  /**
   * @brief   Standard or Extended filter ID 1 (identifier).
   */
  union {
    uint16_t              SFID1:11;
    uint32_t              EFID1:29;
  };
  /**
   * @brief   Standard or Extended filter ID 2. (mask/identifier depending on mode=0/1).
   */
  union {
    uint16_t              SFID2:11;
    uint32_t              EFID2:29;
  };
} CANFilter;

And function:

Code: Select all

static void can_lld_set_filters(CANDriver* canp,
                                uint32_t can2sb,
                                uint32_t num,
                                const CANFilter *cfp) {
  (void) can2sb;
 
  if (num > 0) {
    uint32_t filter_field1;
    uint32_t filter_field2;
    uint32_t *filter_address;
    for (size_t i = 0; i < num; i++) {
      if (cfp->scale == 0) {

        osalDbgCheck(cfp->filter <= STM32_FDCAN_FLS_NBR);

        filter_field1 = ((cfp->SFT << 30U) | (cfp->SFEC << 27U) |
                          (cfp->SFID1 << 16U) | cfp->SFID2);
        /* Calculate filter address */
        filter_address = (uint32_t *)(canp->ram_base + SRAMCAN_FLSSA + (cfp->filter * SRAMCAN_FLS_SIZE));

        /* Write filter element to the message RAM */
        *filter_address = filter_field1;
      }
      else {

        osalDbgCheck(cfp->filter <= STM32_FDCAN_FLE_NBR);

        /* Build first word of filter element */
        filter_field1 = ((cfp->EFEC << 29U) | cfp->EFID1);

        /* Build second word of filter element */
        filter_field2 = ((cfp->EFT << 30U) | cfp->EFID2);

        /* Calculate filter address */
        filter_address = (uint32_t *)(canp->ram_base + SRAMCAN_FLESA + (cfp->filter * SRAMCAN_FLS_SIZE));

        /* Write filter element to the message RAM */
        *filter_address = filter_field1;
        filter_address++;
        *filter_address = filter_field2;
      }
      cfp++;
    }
   
  }
}

Code: Select all

void canSTM32SetFilters(CANDriver *canp, uint32_t can2sb,
                        uint32_t num, const CANFilter *cfp) {
 
  osalDbgAssert(canp->state == CAN_READY, "invalid state");

  can_lld_set_filters(canp, can2sb, num, cfp);
}

I'm tested this on stm32g474

User avatar
Giovanni
Site Admin
Posts: 14444
Joined: Wed May 27, 2009 8:48 am
Location: Salerno, Italy
Has thanked: 1074 times
Been thanked: 921 times
Contact:

Re: [DEV] STM32G4xx support

Postby Giovanni » Tue May 26, 2020 4:04 pm

Anyone willing to collect all the changes required by the driver on trunk? I got lost in all suggestions.

About bit field, true it is not portable and MISRA complains but it is compatible among all ARM compilers and that is good enough for this use case, the structures are defined in the LLD because of this, I would not accept those in the HLD.

About G4 and H7, differences should be addressed in the registry ideally, most of it are offsets in driver memory and current macros should take care of it.

Giovanni

steved
Posts: 823
Joined: Fri Nov 09, 2012 2:22 pm
Has thanked: 12 times
Been thanked: 135 times

Re: [DEV] STM32G4xx support

Postby steved » Tue May 26, 2020 4:28 pm

Giovanni wrote:About bit field, true it is not portable and MISRA complains but it is compatible among all ARM compilers and that is good enough for this use case, the structures are defined in the LLD because of this, I would not accept those in the HLD.

For this type of application, where processor and peripherals are irrevocable bound together on one piece of silicon, there doesn't seem to be any problem at all with this approach. It may be more problematic to support peripherals on an external bus in a universal manner.

In my limited experience bitfield order is purely down to whether the ARM is running big-endian or little-endian; I have a big-endian system where the bitfield order is exactly reversed, so it would be possible to generate portable code by testing endianness. (I've seen code by others which does this, so it would appear to be a fairly common practice.)
Its slightly tedious because two lists of bitfields have to be generated, in opposite orders, with scope for mistyping or failing to fully implement mods.

All the more recent ARM configurations I've seen are little-endian; is this now universal?

User avatar
Giovanni
Site Admin
Posts: 14444
Joined: Wed May 27, 2009 8:48 am
Location: Salerno, Italy
Has thanked: 1074 times
Been thanked: 921 times
Contact:

Re: [DEV] STM32G4xx support

Postby Giovanni » Sun Jun 07, 2020 10:40 am

Hi,

I committed more changes to the FDCAN driver, some taken from this thread others submitted via PM. I have not yet addressed filtering, that is still missing.

Could you please assess the state of the driver as it is now on repository? and please, if you think changes are required could you explain the problem in addition to the required fixes? I have not understood some of the changes posted.

Giovanni

Shevchenko
Posts: 11
Joined: Tue Nov 13, 2018 9:25 am
Has thanked: 1 time
Been thanked: 3 times

Re: [DEV] STM32G4xx support

Postby Shevchenko » Mon Jun 15, 2020 12:19 pm

As for stopping the FDCAN clock. In datasheet written:
Power down (Sleep mode)
The FDCAN can be set into power down mode controlled by clock stop request input via CC control register CCCR[CSR]. As long as the clock stop request is active, bit CCCR[CSR] is read as 1.
When all pending transmission requests have completed, the FDCAN waits until bus idle state is detected. Then the FDCAN sets then CCCR[INIT] to 1 to prevent any further CAN transfers. Now the FDCAN acknowledges that it is ready for power down by setting CCCR[CSA] to 1.
In this state, before the clocks are switched off, further register accesses can be made. A write access to CCCR[INIT] has no effect. Now the module clock inputs may be switched off.
To leave power down mode, the application has to turn on the module clocks before resetting CC control register flag CCCR.CSR. The FDCAN
acknowledges this by resetting CCCR[CSA]. Afterwards, the application can restart CAN communication by resetting bit CCCR[INIT].


It follows that the function should look like this:

Code: Select all

static bool fdcan_clock_stop(CANDriver *canp) {
  systime_t start, end;

  /* Requesting clock stop then waiting for it to happen.*/
  canp->fdcan->CCCR |= FDCAN_CCCR_CSR;
  start = osalOsGetSystemTimeX();
  end = osalTimeAddX(start, TIME_MS2I(TIMEOUT_INIT_MS));
  while ((canp->fdcan->CCCR & FDCAN_CCCR_CSA) == 0U) {
    if (!osalTimeIsInRangeX(osalOsGetSystemTimeX(), start, end)) {
      return true;
    }
    osalThreadSleepS(1);
  }

  return false;
}


Also in the function can_lld_start need to start the clock by resetting the FDCAN_CCCR_CSR bit because it will not be possible to reset the FDCAN_CCCR_INIT (fdcan_clock_start instead of fdcan_clock_stop).

From datasheet:
Access to the FDCAN configuration registers is only enabled when both INIT bit in FDCAN_CCCR register and CCE bit in FDCAN_CCCR register are set.


It follows that need:
canp->fdcan->CCCR |= FDCAN_CCCR_CCE;

Stapelzeiger
Posts: 7
Joined: Mon Apr 06, 2015 1:08 pm

Re: [DEV] STM32G4xx support

Postby Stapelzeiger » Sun Oct 11, 2020 8:58 pm

Hi,
what is the status of the FDCAN driver? It looks like @Shevchenko last fix is not in the repo.

User avatar
Giovanni
Site Admin
Posts: 14444
Joined: Wed May 27, 2009 8:48 am
Location: Salerno, Italy
Has thanked: 1074 times
Been thanked: 921 times
Contact:

Re: [DEV] STM32G4xx support

Postby Giovanni » Tue Oct 13, 2020 4:13 pm

Hi,

I need to restart on this, lost contact with all changes.

Giovanni


Return to “Development and Feedback”

Who is online

Users browsing this forum: No registered users and 17 guests