STMicroelectronics / STM32CubeF4

STM32Cube MCU Full Package for the STM32F4 series - (HAL + LL Drivers, CMSIS Core, CMSIS Device, MW libraries plus a set of Projects running on all boards provided by ST (Nucleo, Evaluation and Discovery Kits))
Other
869 stars 418 forks source link

Hardcoded address in use #67

Closed CanastraRF closed 3 years ago

CanastraRF commented 3 years ago

In stm32f4xx_hal_flash.h is a Hardcoded address in use

#define __HAL_FLASH_SET_LATENCY(__LATENCY__) (*(__IO uint8_t *)ACR_BYTE0_ADDRESS = (uint8_t)(__LATENCY__))
#define ACR_BYTE0_ADDRESS           0x40023C00UL

I think it is better to use the following code #define __HAL_FLASH_SET_LATENCY(__LATENCY__) (FLASH->ACR = (uint8_t)(__LATENCY__)) Can You explain your motivation?

Also other addresses shoud replaced / removed.

#define OPTCR_BYTE0_ADDRESS         0x40023C14UL
#define OPTCR_BYTE1_ADDRESS         0x40023C15UL
#define OPTCR_BYTE2_ADDRESS         0x40023C16UL
#define OPTCR_BYTE3_ADDRESS         0x40023C17UL

#define RCC_CR_BYTE2_ADDRESS       0x40023802UL
#define RCC_CIR_BYTE1_ADDRESS      ((uint32_t)(RCC_BASE + 0x0CU + 0x01U))
#define RCC_CIR_BYTE2_ADDRESS      ((uint32_t)(RCC_BASE + 0x0CU + 0x02U))
#define RCC_BDCR_BYTE0_ADDRESS     (PERIPH_BASE + RCC_BDCR_OFFSET)

Thanks ReFe

RKOUSTM commented 3 years ago

Hi @CanastraRF,

Thank you for your contribution. Your report will be forwarded to our development team. We will get back to you as soon as we have more details.

Thank you once more for this contribution.

With regards,

RKOUSTM commented 3 years ago

Hi @CanastraRF,

Thank you for your contribution. Regarding the first point, an internal tracker has been logged and a fix will be implemented and made available in the frame of a future release to update the definitions of the flash set latency. However, for the second point, actually your request can't be accepted in order to not break the compatibility of our HAL drivers and that Customers use the oldest version of our drivers.

With regards,

RKOUSTM commented 3 years ago

ST Internal Reference: 112635

RKOUSTM commented 3 years ago

Hi @CanastraRF,

I hope you are doing well. According to our development teams, the proposed enhancement in the hal_flash.h file to align with other STM32 series is not possible to implement because the DCRST and the ICRST bits in the FLASH access control register (FLASH_ACR) are read only:

DCRST_ICRST

Regarding the point for which you opened the issue, as it is irrelevant, allow me to close the issue. Thank you for your comprehension and thank you again for your contribution.

We are looking forward to reading from you again.

With regards,

CanastraRF commented 3 years ago

Hi @RKOUSTM Sorry I did not understand your explanation. #define __HAL_FLASH_SET_LATENCY(LATENCY) ((__IO uint8_t )ACR_BYTE0_ADDRESS = (uint8_t)(LATENCY)) and #define HAL_FLASH_SET_LATENCY(LATENCY) (FLASH->ACR = (uint8_t)(LATENCY__)) results in absolutely the same code. In both case is one byte written to ACR. This has now difference to DCRST or ICRST. The improvement of my version is, that no additional address is required and everything is depend of the FLASH struct. BTW. We often emulate STM32 code in a windows app. In this case we replace the _#define FLASH ((FLASH_TypeDef *) FLASH_RBASE) with #define FLASH (&sFLASH) and _FLASHTypeDef sFLASH Now read, write access the periphery struct is possible. But all hard coded address like ACR_BYTE0_ADDRESS must be handled separat. This should not be required. So please replace all hard coded address with the exist struct element.