STM32G431 - Interrupt fires twice, when optimization -O2 is used. How can i find the point of error

[Tobe]

Through an error, i found out, that the interrupt (FDCAN line 0) fires twice, when only one is activated. (See topic https://community.st.com/t5/stm32cubeide-mcus/compiler-optimized-program-has-errors/m-p/652965#M25387).

How can i now find the actual cause. My head is already spinning form all the investigation so far.

Update: trimmed the code:

Only the initializations are executed.

HAL_StatusTypeDef TK_FDCAN_GetRx0Message(
		FDCAN_HandleTypeDef *hfdcan,
		struct can_frame *cFrame)
{
	uint32_t *RxAddress;
	uint32_t GetIndex;

	// If no messages
	if ((hfdcan->Instance->RXF0S & FDCAN_RXF0S_F0FL) == 0U)
		return HAL_TIMEOUT;

	/* Calculate Rx FIFO 0 element address */
	GetIndex = ((hfdcan->Instance->RXF0S & FDCAN_RXF0S_F0GI) >> FDCAN_RXF0S_F0GI_Pos);
	RxAddress = (uint32_t *)(hfdcan->msgRam.RxFIFO0SA + (GetIndex * SRAMCAN_RF0_SIZE));

	/* Retrieve Identifier */
	cFrame->id = ((*RxAddress & FDCAN_ELEMENT_MASK_STDID) >> 18U);

	//    /* Retrieve ErrorStateIndicator */
	//    pRxHeader->ErrorStateIndicator = (*RxAddress & FDCAN_ELEMENT_MASK_ESI);

	/* Increment RxAddress pointer to second word of Rx FIFO element */
	RxAddress++;

// USE EXTERNALLY!
//	/* Retrieve RxTimestamp */
//	cFrame->timeMicros = micros();

	/* Retrieve DataLength - Yes its weird (bitshift)! Just ignore it!*/
	cFrame->count = (*RxAddress & FDCAN_ELEMENT_MASK_DLC) >> 16U;

	/* Increment RxAddress pointer to payload of Rx FIFO element */
	RxAddress++;

	/* Retrieve Rx payload */
	uint8_t  *pData = (uint8_t *)RxAddress;
	for (uint32_t ByteCounter = 0; ByteCounter < cFrame->count; ByteCounter++)
	{
		cFrame->data[ByteCounter] = pData[ByteCounter];
	}

	/* Acknowledge the Rx FIFO 0 that the oldest element is read so that it increments the GetIndex */
	hfdcan->Instance->RXF0A = GetIndex;

	return HAL_OK;
}

FDCAN_RX_FIFO0_MASK (FDCAN_IR_RF0L | FDCAN_IR_RF0F | FDCAN_IR_RF0N)

void clearFIFO0(FDCAN_HandleTypeDef *hfdcan){
	uint32_t RxFifo0ITs;
	RxFifo0ITs  = hfdcan->Instance->IR & FDCAN_RX_FIFO0_MASK;
	//RxFifo0ITs &= hfdcan->Instance->IE;
	__HAL_FDCAN_CLEAR_FLAG(hfdcan, RxFifo0ITs);
}


void TK_FDCAN1_IT0_IRQHandler(void)
{
	newMessageIRQ();
	clearFIFO0(&hfdcan1);	// clear interrupt flag
}

void FDCAN1_IT0_IRQHandler(void)
{
	TK_FDCAN1_IT0_IRQHandler();
}

[waclawek.jan]

I don't use CAN so have no intention to dig deeper, you should.

Read carefully the related chapter in RM, including the registers description. If you haven't found the answer, re read it. Follow meticulously what's written there.

> I do not reset the interrupt flags, 

RM tells you whether you must or shall not reset them. They may be reset by hardware upon some action e.g. reading from FIFO, but that action may or may not have consequences, including timing-related (which then brings us back to the"late interrupt flag clear results in false interrupt firings" case). As I've said, I haven't read given chapter in RM.

Write a stripped down (i.e. nothing but the CAN receiver) but compilable minimal code which exhibits the problem, and post.

JW

[Tobe]

I now have a minimal code that has the above said error:

/* USER CODE BEGIN Header */
/**
 ******************************************************************************
 * @file           : main.c D:\Programmieren\STM32\STM32CubeIDE_1.12.0\STM32CubeIDE\plugins\com.st.stm32cube.common.mx_6.8.0.202302231600\db\templates\tpl_main_c.ftl
 * @brief          : Main program body
 ******************************************************************************
 * @attention
 *
 * Copyright (c) 2023 STMicroelectronics.
 * All rights reserved.
 *
 * This software is licensed under terms that can be found in the LICENSE file
 * in the root directory of this software component.
 * If no LICENSE file comes with this software, it is provided AS-IS.
 *
 ******************************************************************************
 */
/* USER CODE END Header */
/* Includes ------------------------------------------------------------------*/
#include <boot_main.h>
#include "stm32g4xx_hal.h"
#include "stm32g4xx_hal_tim.h"

#include "stdbool.h"

#define MICROSECONDS_PER_TIMER0_OVERFLOW (0xFFFF)


/* USER CODE END Includes */

/* Private typedef -----------------------------------------------------------*/
/* USER CODE BEGIN PTD */

/* USER CODE END PTD */

/* Private define ------------------------------------------------------------*/
/* USER CODE BEGIN PD */

/* USER CODE END PD */

/* Private macro -------------------------------------------------------------*/
/* USER CODE BEGIN PM */

/* USER CODE END PM */

/* Private variables ---------------------------------------------------------*/
FDCAN_HandleTypeDef hfdcan1;

/* Private user code ---------------------------------------------------------*/
/* USER CODE BEGIN 0 */
bool usart3Init = false;

void setPin(GPIO_TypeDef  *GPIOx, uint32_t Pin, uint32_t Mode, uint32_t Pull){
	GPIO_InitTypeDef GPIO_InitStruct = {0};
	GPIO_InitStruct.Pin = Pin;
	GPIO_InitStruct.Mode = Mode;
	GPIO_InitStruct.Pull = Pull;
	HAL_GPIO_Init(GPIOx, &GPIO_InitStruct);		//TODO: remove this load of ***
}

void print(uint8_t b){
	if(usart3Init == false)
		return;

	// Wait until transmit data register is ready to take data
	while((USART3->ISR & USART_ISR_TC) == 0);

	//load data to transmit register
	USART3->TDR = b;
};

struct can_frame dummyFrame;

void newMessageIRQ(){
	print(66);
	TK_FDCAN_GetRx0Message(&hfdcan1, &dummyFrame);
	print(100);
}



void TK_FDCAN1_IT0_IRQHandler(void)
{
	//TK_FDCAN_GetRx0Message(&hfdcan1, &cFrame);

	newMessageIRQ();

	clearFIFO0(&hfdcan1);	// clear interrupt flag
}
/* USER CODE END 0 */

/**
 * @brief  The application entry point.
 * @retval int
 */
int main(void)
{
	/* Set Interrupt Group Priority */
	HAL_NVIC_SetPriorityGrouping(NVIC_PRIORITYGROUP_4);

	/* Use SysTick as time base source and configure 1ms tick (default clock after Reset is HSI) */
	HAL_InitTick(TICK_INT_PRIORITY);

	__HAL_RCC_SYSCFG_CLK_ENABLE();
	__HAL_RCC_PWR_CLK_ENABLE();

	/** Disable the internal Pull-Up in Dead Battery pins of UCPD peripheral*/
	HAL_PWREx_DisableUCPDDeadBattery();



	RCC_OscInitTypeDef RCC_OscInitStruct = {0};
	RCC_ClkInitTypeDef RCC_ClkInitStruct = {0};

	/** Configure the main internal regulator output voltage
	 */
	HAL_PWREx_ControlVoltageScaling(PWR_REGULATOR_VOLTAGE_SCALE1_BOOST);

	/** Initializes the RCC Oscillators according to the specified parameters
	 * in the RCC_OscInitTypeDef structure.
	 */
	RCC_OscInitStruct.OscillatorType = RCC_OSCILLATORTYPE_HSI;
	RCC_OscInitStruct.HSIState = RCC_HSI_ON;
	RCC_OscInitStruct.HSICalibrationValue = RCC_HSICALIBRATION_DEFAULT;
	RCC_OscInitStruct.PLL.PLLState = RCC_PLL_ON;
	RCC_OscInitStruct.PLL.PLLSource = RCC_PLLSOURCE_HSI;
	RCC_OscInitStruct.PLL.PLLM = RCC_PLLM_DIV4;
	RCC_OscInitStruct.PLL.PLLN = 85;
	RCC_OscInitStruct.PLL.PLLP = RCC_PLLP_DIV2;
	RCC_OscInitStruct.PLL.PLLQ = RCC_PLLQ_DIV2;
	RCC_OscInitStruct.PLL.PLLR = RCC_PLLR_DIV2;
	if (HAL_RCC_OscConfig(&RCC_OscInitStruct) != HAL_OK)
	{
		Error_Handler();
	}

	/** Initializes the CPU, AHB and APB buses clocks
	 */
	RCC_ClkInitStruct.ClockType = RCC_CLOCKTYPE_HCLK|RCC_CLOCKTYPE_SYSCLK
			|RCC_CLOCKTYPE_PCLK1|RCC_CLOCKTYPE_PCLK2;
	RCC_ClkInitStruct.SYSCLKSource = RCC_SYSCLKSOURCE_PLLCLK;
	RCC_ClkInitStruct.AHBCLKDivider = RCC_SYSCLK_DIV1;
	RCC_ClkInitStruct.APB1CLKDivider = RCC_HCLK_DIV1;
	RCC_ClkInitStruct.APB2CLKDivider = RCC_HCLK_DIV4;

	if (HAL_RCC_ClockConfig(&RCC_ClkInitStruct, FLASH_LATENCY_4) != HAL_OK)
	{
		Error_Handler();
	}


	GPIO_InitTypeDef GPIO_InitStruct = {0};

	/* GPIO Ports Clock Enable */
	__HAL_RCC_GPIOC_CLK_ENABLE();
	__HAL_RCC_GPIOF_CLK_ENABLE();
	__HAL_RCC_GPIOA_CLK_ENABLE();
	__HAL_RCC_GPIOB_CLK_ENABLE();

	/* Vdd DISPLAY */
	GPIO_InitStruct.Pin = GPIO_PIN_2;
	GPIO_InitStruct.Mode = GPIO_MODE_OUTPUT_PP;
	GPIO_InitStruct.Pull = GPIO_NOPULL;
	GPIO_InitStruct.Speed = GPIO_SPEED_FREQ_LOW;
	HAL_GPIO_Init(GPIOB, &GPIO_InitStruct);

	// output to keep system alive
	GPIO_InitStruct.Pin = GPIO_PIN_5;
	GPIO_InitStruct.Mode = GPIO_MODE_OUTPUT_PP;
	GPIO_InitStruct.Pull = GPIO_NOPULL;
	GPIO_InitStruct.Speed = GPIO_SPEED_FREQ_LOW;
	HAL_GPIO_Init(GPIOC, &GPIO_InitStruct);
	// Set output low keep system alive
	GPIOC->BRR |= (uint32_t) GPIO_PIN_5;

	/*Configure testpin (USART3) */
	GPIO_InitStruct.Pin = GPIO_PIN_10;
	GPIO_InitStruct.Mode = GPIO_MODE_AF_PP;
	GPIO_InitStruct.Pull = GPIO_NOPULL;
	GPIO_InitStruct.Speed = GPIO_SPEED_FREQ_LOW;
	GPIO_InitStruct.Alternate = GPIO_AF7_USART3;
	HAL_GPIO_Init(GPIOB, &GPIO_InitStruct);

	hfdcan1.Instance = FDCAN1;
	hfdcan1.Init.ClockDivider = FDCAN_CLOCK_DIV2;
	hfdcan1.Init.FrameFormat = FDCAN_FRAME_CLASSIC;
	hfdcan1.Init.Mode = FDCAN_MODE_NORMAL;
	hfdcan1.Init.AutoRetransmission = DISABLE;
	hfdcan1.Init.TransmitPause = DISABLE;
	hfdcan1.Init.ProtocolException = DISABLE;
	hfdcan1.Init.NominalPrescaler = 21;
	hfdcan1.Init.NominalSyncJumpWidth = 1;
	hfdcan1.Init.NominalTimeSeg1 = 4;
	hfdcan1.Init.NominalTimeSeg2 = 3;
	hfdcan1.Init.DataPrescaler = 21;
	hfdcan1.Init.DataSyncJumpWidth = 1;
	hfdcan1.Init.DataTimeSeg1 = 4;
	hfdcan1.Init.DataTimeSeg2 = 3;
	hfdcan1.Init.StdFiltersNbr = 0;
	hfdcan1.Init.ExtFiltersNbr = 0;
	hfdcan1.Init.TxFifoQueueMode = FDCAN_TX_FIFO_OPERATION;

	if (HAL_FDCAN_Init(&hfdcan1) != HAL_OK)
	{
		Error_Handler();
	}

	if( HAL_FDCAN_Start(&hfdcan1) != HAL_OK)
	{
		Error_Handler();
	}

	CLEAR_BIT(hfdcan1.Instance->IE, FDCAN_IE_TFEE); // no Interupt if empty buffer! Where the *** is this set up?
	SET_BIT(hfdcan1.Instance->ILE, FDCAN_INTERRUPT_LINE0);

	__HAL_FDCAN_ENABLE_IT(&hfdcan1, FDCAN_IT_RX_FIFO0_NEW_MESSAGE);

	__HAL_RCC_TIM16_CLK_ENABLE();
	TIM16->PSC = 85 - 1; // true prescaler 85 is value (85-1)
	TIM16->CR1 |= TIM_CR1_CEN | TIM_CR1_URS;
	TIM16->ARR = MICROSECONDS_PER_TIMER0_OVERFLOW - 1;
	TIM16->DIER |= TIM_DIER_UIE;

	//NVIC_EnableIRQ(TIM1_UP_TIM16_IRQn);

	/* Disable TRC */
	CoreDebug->DEMCR &= ~CoreDebug_DEMCR_TRCENA_Msk; // ~0x01000000;
	/* Enable TRC */
	CoreDebug->DEMCR |=  CoreDebug_DEMCR_TRCENA_Msk; // 0x01000000;

	/* Disable clock cycle counter */
	DWT->CTRL &= ~DWT_CTRL_CYCCNTENA_Msk; //~0x00000001;
	/* Enable  clock cycle counter */
	DWT->CTRL |=  DWT_CTRL_CYCCNTENA_Msk; //0x00000001;

	/* Reset the clock cycle counter value */
	DWT->CYCCNT = 0;

	/* 3 NO OPERATION instructions */
	__ASM volatile ("NOP");
	__ASM volatile ("NOP");
	__ASM volatile ("NOP");


	// activate clock to access registers
	SET_BIT(RCC->APB1ENR1, RCC_APB1ENR1_USART3EN);

	//TODO: might need some wait for the clock

	/*  Ensure that Frequency clock is in the range [3 * baudrate, 4096 * baudrate]
	 *
	 * 	The maximum Baud Rate is derived from the maximum clock on G4 (i.e. 150 MHz)
	 * 	divided by the smallest oversampling used on the USART (i.e. 8) */

	//Prescaler: (1476 = 115200baud (OVER8 = 0)) (4096 * baud < usart_ker_ck_pres !!!)
	USART3->BRR = (1476);

	// 1 stop bit = 000

	//activate USART
	USART3->CR1 |= USART_CR1_UE;

	//send idle frame
	USART3->CR1 |= USART_CR1_TE;
	usart3Init = true;
}



/* USER CODE BEGIN 4 */

/* USER CODE END 4 */

/**
 * @brief  This function is executed in case of error occurrence.
 * @retval None
 */
void Error_Handler(void)
{
	/* USER CODE BEGIN Error_Handler_Debug */
	/* User can add his own implementation to report the HAL error return state */
	__disable_irq();
	while (1)
	{
	}
	/* USER CODE END Error_Handler_Debug */
}

#ifdef  USE_FULL_ASSERT
/**
 * @brief  Reports the name of the source file and the source line number
 *         where the assert_param error has occurred.
 * @PAram  file: pointer to the source file name
 * @PAram  line: assert_param error line source number
 * @retval None
 */
void assert_failed(uint8_t *file, uint32_t line)
{
	/* USER CODE BEGIN 6 */
	/* User can add his own implementation to report the file name and line number,
     ex: printf("Wrong parameters value: file %s on line %d\r\n", file, line) */
	/* USER CODE END 6 */
}
#endif /* USE_FULL_ASSERT */

[Pavel A.]

@Tobe Spurious interrupts can occur because the "reset" action takes time to propagate thru the buses to the interrupt source and back to NVIC. Jan has explained this several times. If you get only one or two spurious interrupts (not endless loop) - ignore them.

[Tobe · Accepted solution]

I now can confirm, that it is as you said. The problem was, i didnt realize it, as the clearing was hidden below a lot of commented code. But i would now call them spurious, as they happen EVERY time.

I should have trimmed the code way earlier than i did! I even found a few things that were twice in the code.

A simple, but yet ugly solution looks like this:

void TK_FDCAN1_IT0_IRQHandler(void)
{
	//TK_FDCAN_GetRx0Message(&hfdcan1, &cFrame);

	//clearFIFO0(&hfdcan1);	// clear interrupt flag
	newMessageIRQ();
	clearFIFO0(&hfdcan1);	// clear interrupt flag
	delay(x);

}

[Pavel A.]

> delay(x);

Indeed ugly. Depends what is delay(). A long delay (more than few cycles) will block other interrupts of same and lower priority, without any good reason. But ok.

[Tobe]

I tried it out. One "__ASM volatile ("NOP");" is enough for all optimization levels. But i added another one to be save.

[waclawek.jan]

>Indeed ugly.

+1.

If there is no reasonable code to execute instead of doing delay, it may be better to qualify the interrupt sources and ignore "false interrupts". I believe I discussed that exhaustively in the linked text. 

[Tobe]

When it is possible, to prevent false interrupts, i find it ugly to qualify those.

[Tobe]

Also i had qualyfied those, but still it slipped through, as i checked only the buffer, which held old values ;)