[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: 288
Joined: Fri May 25, 2012 10:23 am
Location: toulouse, france
Has thanked: 24 times
Been thanked: 36 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: 10
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: 10
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: 12748
Joined: Wed May 27, 2009 8:48 am
Location: Salerno, Italy
Has thanked: 696 times
Been thanked: 573 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: 671
Joined: Fri Nov 09, 2012 2:22 pm
Has thanked: 8 times
Been thanked: 96 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?


Return to “Development and Feedback”

Who is online

Users browsing this forum: No registered users and 5 guests