Duplicate/unneeded code in premain?

[matthijskooijman]

Looking around the code, I noticed some code in premain that seems duplicate:

Arduino_Core_STM32/cores/arduino/main.cpp

Lines 34 to 46 in db1b90f

	// Required by FreeRTOS, see http://www.freertos.org/RTOS-Cortex-M3-M4.html
	#ifdef NVIC_PRIORITYGROUP_4
	HAL_NVIC_SetPriorityGrouping(NVIC_PRIORITYGROUP_4);
	#endif
	#if (__CORTEX_M == 0x07U)
	// Defined in CMSIS core_cm7.h
	#ifndef I_CACHE_DISABLED
	SCB_EnableICache();
	#endif
	#ifndef D_CACHE_DISABLED
	SCB_EnableDCache();
	#endif
	#endif

The same things seem to be done by HAL_Init() already, e.g. on F4:

Arduino_Core_STM32/system/Drivers/STM32F4xx_HAL_Driver/Src/stm32f4xx_hal.c

Lines 160 to 173 in db1b90f

	#if (INSTRUCTION_CACHE_ENABLE != 0U)
	__HAL_FLASH_INSTRUCTION_CACHE_ENABLE();
	#endif /* INSTRUCTION_CACHE_ENABLE */
	#if (DATA_CACHE_ENABLE != 0U)
	__HAL_FLASH_DATA_CACHE_ENABLE();
	#endif /* DATA_CACHE_ENABLE */
	#if (PREFETCH_ENABLE != 0U)
	__HAL_FLASH_PREFETCH_BUFFER_ENABLE();
	#endif /* PREFETCH_ENABLE */
	/* Set Interrupt Group Priority */
	HAL_NVIC_SetPriorityGrouping(NVIC_PRIORITYGROUP_4);

This seems rather pointless, though perhaps when using RTOS the init() function might be replaced and HAL_Init() is not called? But then maybe the RTOS init() should just set up this NVIC stuff it needs by itself?

For the caches, it seems there are now two separate macros to control them, which could also be a bit confusing?

[fpistm]

#if (__CORTEX_M == 0x07U)
F4 is not an M7.

[matthijskooijman]

Ah, good point. Looking at the F7 (which does have an M7 AFAIU), it indeed does not initialize the caches in HAL_Init():

Arduino_Core_STM32/system/Drivers/STM32F7xx_HAL_Driver/Src/stm32f7xx_hal.c

Lines 138 to 161 in db1b90f

	HAL_StatusTypeDef HAL_Init(void)
	{
	/* Configure Instruction cache through ART accelerator */
	#if (ART_ACCLERATOR_ENABLE != 0)
	__HAL_FLASH_ART_ENABLE();
	#endif /* ART_ACCLERATOR_ENABLE */
	/* Configure Flash prefetch */
	#if (PREFETCH_ENABLE != 0U)
	__HAL_FLASH_PREFETCH_BUFFER_ENABLE();
	#endif /* PREFETCH_ENABLE */
	/* 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);
	/* Init the low level hardware */
	HAL_MspInit();
	/* Return function status */
	return HAL_OK;
	}

. Any idea why this is different for different MCU series?

Still, it seems the NVIC enable is still duplicate?

[fpistm]

Any idea why this is different for different MCU series?

No

Still, it seems the NVIC enable is still duplicate?

Right. Looking deeply it seems properly set for all STM32 series with NVIC_PRIORITYGROUP_4.
So it could probably removed without any issue while it is set before the RTOS is started.

[fpistm]

So, what did you expect on this?

[matthijskooijman]

It was mostly something I noticed while reading the code that looked like it could be cleaned up. I wanted to share that observation, but it seems cleaning it up might be more involved (i.e. needs more research and testing), so perhaps it should just be left as-is for now. Feel free to close this if you agree :-)

[fpistm]

I'm always ready for made some clean.
In this case I think you spotted the right thing , it will require some study.