STMicroelectronics/STM32CubeF7

Questionable logic when setting the number of flash wait states

hjuul opened this issue · 4 comments

hjuul commented

When setting the number of flash wait states, the following condition must be true to be able arrive at the highest wait states:

if(LL_PWR_IsEnabledOverDriveMode() != 0U)

So if this function is called with e.g. 216MHz as input but overdrive mode disabled, it will end up with LL_FLASH_LATENCY_5.

Although the input conditions would be invalid, both of the following solutions would be better, IMHO:

  • Return ERROR if this input condition occurs
  • Set LL_FLASH_LATENCY_7 (still not valid, but better than 5)

The same reasoning could be applied to the voltage scaling condition in the same function.

Hi @hjuul,

First thank you for your contribution.

The over-drive mode allows the CPU and the core logic to operate at a higher frequency than the normal mode for the voltage scaling scale 1 and scale 2.

Indeed, in order to reach an HCLK frequancy equal to 216MHz witch is the maximum frequency, the over driver mode must be activated. This is described in the Table 18. General operating conditions within the datasheet.

Thus, since the input of the function UTILS_SetFlashLatency is the HCLK_Frequency and for a frequency equal to 216MHz, the function will end up with LL_FLASH_LATENCY_7 since the over drive mode is imperatively activated to reach this frequency.

Therefore, the following part of the code is accessible only when the drive mode is active given that these HCLK frequencies are only reached when this mode is activated.

if(LL_PWR_IsEnabledOverDriveMode() != 0U)
{
if(HCLK_Frequency > UTILS_SCALE1_LATENCY7_FREQ)
{
/* 210 < HCLK <= 216 => 7WS (8 CPU cycles) */
latency = LL_FLASH_LATENCY_7;
}
else /* (HCLK_Frequency > UTILS_SCALE1_LATENCY6_FREQ) */
{
/* 180 < HCLK <= 210 => 6WS (7 CPU cycles) */
latency = LL_FLASH_LATENCY_6;
}
}

Hope that this will answer your question.
With regards,

hjuul commented

I understand that LATENCY_6 and LATENCY_7 should only be used when overdrive mode is enabled. But I don't think this function should assume that the caller actually did that.

In my opinion, if the caller wants 216MHz but hasn't enabled overdrive mode (and the correct voltage scale) he should get an error, not LATENCY_5.

Have a look at this related Zephyr issue. Zephyr calls UTILS_SetFlashLatency() with 216MHz without having enabled overdrive mode. If this function had returned an error on that condition, that bug would have been detected during development.

Hi @hjuul,

To ensure a proper use of the UTILS_SetFlashLatency function, correct parameters must be passed. Indeed, you have to pass the real value of HCLK_Frequency used by your system. In order to calculate the exact value of HCL_Frequancy, you may use the macro __LL_RCC_CALC_HCLK_FREQ.

Would you please try to use this macro to calculate the appropriate value of the HCLK_Frequancy and test back LL_SetFlashLatency function with the HCLK_Frequancy returned by the macro.

With regards,

hjuul commented

Never mind.
I know it will work if I provide the correct frequency, I was just suggesting you raise an error if I don't.
Feel free to close the issue if you don't want this function to handle such errors.