CubeL4 ADC HAL_ADC_MspInit and HAL_ADC_MspDeInit issue

[MarkusVogel]

Dear all,

I found an interesting effect using the CubeL4 and CubeMX generated files for a STM32L496 and use ADC1 and ADC3 (not simultaneously):

What we do:

• We turn the ADCs on only when we sample data, otherwise they are turned off. 
• To turn the ADC off, we use HAL_ADC_DeInit which calls HAL_ADC_MspDeInit.

Issue:

HAL_ADC_MspInit and HAL_ADC_MspDeInit keep track of active ADCs through static variable HAL_RCC_ADC_CLK_ENABLED. Init increases the count, deinit decreases it. If, however, init and fails but we still call deinit the count wraps around to 0xFFFF FFFF - upon next init it wraps around to 0, but then the ADC clock is not turned on and initialization fails again. See the code snippets below:

HAL_ADC_MspInit

HAL_RCC_ADC_CLK_ENABLED++;
if(HAL_RCC_ADC_CLK_ENABLED==1){
__HAL_RCC_ADC_CLK_ENABLE();
}

 

HAL_ADC_MspDeInit

HAL_RCC_ADC_CLK_ENABLED--;
if(HAL_RCC_ADC_CLK_ENABLED==0){
__HAL_RCC_ADC_CLK_DISABLE();
}

 

Suggestion:

• Check for underflow of HAL_RCC_ADC_CLK_ENABLED in HAL_ADC_MspDeInit
• In HAL_ADC_MspInit add a check if ADC-clock is off - then turn it on -> that is what we want to do with initialization -> alternatively only check for ADC-clock and disregard the count completely.

Versions:

• I checked with CubeMX Version 6.17 and CubeL4 version 1.18.2 (latest as of today)

 

What do you think? Let me know if you need additional information. 

Looking forward to your opinion on this case, best regards

Markus

[Ozone]

> HAL_ADC_MspInit and HAL_ADC_MspDeInit keep track of active ADCs through static variable HAL_RCC_ADC_CLK_ENABLED. Init increases the count, deinit decreases it. If, however, init and fails but we still call deinit the count wraps around to 0xFFFF FFFF - upon next init it wraps around to 0, but then the ADC clock is not turned on and initialization fails again. 

Sounds like quite a bug to me.
I don't use Cube/HAL, and this resource management seems way overblown and superfluous to me on this occasion.

Anyway, why don't you just disable the ADC when not using ?

[MarkusVogel]

We do turn it off - using the HAL code (HAL_ADC_DeInit). It clears the ADEN bit in the ADC's CR-register and further also switches off the clock (which was the issue here) to save even more power. What it could do but does not is even to active Deeppower down mode - then calibration would have to be redone and startup takes longer. That is why we did not add deeppower down code ourselves. 

[Ghofrane GSOURI]

Hello @MarkusVogel 

Could you please provide your IOC?

I will be waiting for your feedback.

THX

Ghofrane

[MarkusVogel]

Hi @Ghofrane GSOURI ,

I am not sure what IOC stands for - but I assume you could mean a fix version :)

If so, here is what I added now to our code:

HAL_ADC_MspInit

HAL_RCC_ADC_CLK_ENABLED++;
if((HAL_RCC_ADC_CLK_ENABLED==1) || __HAL_RCC_ADC_IS_CLK_DISABLED())
{
__HAL_RCC_ADC_CLK_ENABLE();
}

 

HAL_ADC_MspDeInit:

if (HAL_RCC_ADC_CLK_ENABLED > 0)
{
HAL_RCC_ADC_CLK_ENABLED--;
}

if(HAL_RCC_ADC_CLK_ENABLED==0)
{
__HAL_RCC_ADC_CLK_DISABLE();
}

 

Best regards Markus

[Ozone]

> I am not sure what IOC stands for - but I assume you could mean a fix version :)

No. It is a kind of meta - project file, generated by CubeMX AFAIK, and containing the pin assignments and software components to include for a project, amongst others.
I suppose the suffix "*.IOC" stands for "IO Configuration".

I'm not using CubeIDE and it's tools, but I assume it is basically XML.
Which means you can inspect it with any text editor.

[MarkusVogel]

Hello ​@Ghofrane GSOURI,

could you regenerate the effect on your side and maybe fix the issue in CubeL4?

I have fixed the issue in our code myself, but would be could if future users do not fall over it.

Best regards

Markus

[Ghofrane GSOURI]

Hello ​@MarkusVogel ​@Ozone 

Thank you all for your contributions.

Issue has been escalated to dev team under internal ticket ID # 0064173 for further investigation.

THX

Ghofrane