Bug in LL_ADC_SetAnalogWDThresholds()

[mkm_ctv]

Enabling analog watchdog 1 for ADC3 on a STM32H723 in CubeMX with low threshold 0x8 and high threshold 0x80 generates

LL_ADC_SetAnalogWDThresholds(ADC3, LL_ADC_AWD1, 0x80, 0x8);

but the doc comment for LL_ADC_SetAnalogWDThresholds() says regarding the third parameter

 * @param AWDThresholdsHighLow This parameter can be one of the following values:
 * @arg @ref LL_ADC_AWD_THRESHOLD_HIGH
 * @arg @ref LL_ADC_AWD_THRESHOLD_LOW

indicating that the correct code to generate would be

 LL_ADC_SetAnalogWDThresholds(ADC3, LL_ADC_AWD1, LL_ADC_AWD_THRESHOLD_HIGH, 0x80);
 LL_ADC_SetAnalogWDThresholds(ADC3, LL_ADC_AWD1, LL_ADC_AWD_THRESHOLD_LOW, 0x8);

However neither produces the desired result. The code generated by CubeMX sets the lower threshold to 8, whereas the code I assumed to be correct sets it to 0x88. The high threshold is left at the reset value of 0xFFF in both cases.

Looking at LL_ADC_SetAnalogWDThresholds() the line

 MODIFY_REG(*preg,
 AWDThresholdsHighLow,
 AWDThresholdValue << ((AWDThresholdsHighLow & ADC_AWD_TRX_BIT_HIGH_MASK) >> ADC_AWD_TRX_BIT_HIGH_SHIFT4));

seems to assume that AWDThresholdsHighLow is a bit mask, i.e. either 0x00000FFF for LL_ADC_AWD_THRESHOLD_LOW where the threshold should be stored in bits 11:0 or 0x0FFF0000 for LL_ADC_AWD_THRESHOLD_HIGH that should be stored in bits 27:16. The latter would then give

 AWDThresholdsHighLow & ADC_AWD_TRX_BIT_HIGH_MASK ==

0x00000FFF & 0x00010000 = 0x00010000

and

  (AWDThresholdsHighLow & ADC_AWD_TRX_BIT_HIGH_MASK) >> ADC_AWD_TRX_BIT_HIGH_SHIFT4) ==

0x00010000 >> 12 == 16

so that the threshold value is correctly shifted from 11:0 to 27:16.

The ADCx != ADC3 case seems to depend on AWDThresholdsHighLow being 0 or 1 though, so changing LL_ADC_AWD_THRESHOLD_HIGH/LOW to bit masks wouldn't work. But the MODIFY_REG() for ADC3 could maybe be changed to something like

 uint32_t shift = AWDThresholdsHighLow << 4;
 MODIFY_REG(*preg, 0xFFF << shift, AWDThresholdValue << shift);

which seems to work.

Though to be honest, I'd rather have separate functions for high and low thresholds.

Using CubeMX version 6.7.0 and STM32Cube FW_H7 V1.11.0

[KDJEM.1]

Hello @Magnus Kristiansen-Modéer​,

Thank you for bringing these issues to our attention.

I confirm and I reported Internally.

Firmware related issue tracked under the number 143928. 

STM32CubeMX issue tracked in internal ticket 143921.

These are internal tracking numbers and are not accessible or usable by customers.

When your question is answered, please close this topic by choosing Select as Best. This will help other users find that answer faster.

Kaouthar

[Patrice LF]

Hello @Magnus Kristiansen-Modéer​ 

After the analysis of the Firmware internal ticket 143928, I am please to share with you the fix proposal.

In LL_ADC_SetAnalogWDThresholds(), please replace the following line

MODIFY_REG(*preg,
 AWDThresholdsHighLow,
 AWDThresholdValue << ((AWDThresholdsHighLow & ADC_AWD_TRX_BIT_HIGH_MASK) >> ADC_AWD_TRX_BIT_HIGH_SHIFT4));

with

MODIFY_REG(*preg,
 (ADC3_TR1_LT1 << (AWDThresholdsHighLow * ADC3_TR1_HT1_Pos)),
 AWDThresholdValue << (((AWDThresholdsHighLow * ADC3_TR1_HT1) & ADC_AWD_TRX_BIT_HIGH_MASK) >> ADC_AWD_TRX_BIT_HIGH_SHIFT4));

Thank you to share your feedback

Patrice

[KDJEM.1]

Hello @Magnus Kristiansen-Modéer​ ,

The STM32CubeMX issue is fixed in STM32CubeMX6.8.0.

Thank you.

Kaouthar

[mkm_ctv]

Sorry for being slow on this. Looks good, but isn't the second shift expression unnecessarily complicated? Isn't this equivalent?

 MODIFY_REG(*preg,
 (ADC3_TR1_LT1 << (AWDThresholdsHighLow * ADC3_TR1_HT1_Pos)),
 AWDThresholdValue << (AWDThresholdsHighLow * ADC3_TR1_HT1_Pos)));

Also, when starting a new project for NUCLEO-H723ZG in STM32CubeMX 6.8.0 with "STM32Cube FW_H7 V1.11.0" I can't see that anything has changed in LL_ADC_SetAnalogWDThresholds(). Am I doing something wrong?

[KDJEM.1]

Hello @magnus Kristiansen-Modéer​ ,

Concerning the issue related to STM32CubeMx and reported in 143921 ticket is fixed in STM32CubeMX.6.8.0 and the LL_ADC_SetAnalogWDThresholds is changed by these correct code lines:

I tested this by enabling analog watchdog 1 for ADC3 on a Nucleo-H723ZC in STM32CubeMX6.8.0 with low threshold 0x8 and high threshold 0x80 and I used STM32CubeIDE1.12.0 toolchain to generate the code.

Please let me know if the STM32CubeMx issue is solved!

Kaouthar

[mkm_ctv]

Sorry, I missed that. The generated code is indeed correct now but the code called (in "STM32Cube FW_H7 V1.11.0") seems to still be faulty.

[KDJEM.1]

Hi @mkm_ctv ,

Firmware issue is fixed in GitHub.

Thank you for your contribution in STCommunity :).

Kaouthar