cpu/stm32-L4: fix ADC initialization #20756
Conversation
github-actions
bot
added
Platform: ARM
|
I just looked at the defines in the RIOT/build/stm32/cmsis/l4/Include/stm32l476xx.h lines 2564-2568: #define ADC_CCR_CKMODE_Pos (16U)
#define ADC_CCR_CKMODE_Msk (0x3UL << ADC_CCR_CKMODE_Pos) /*!< 0x00030000 */
#define ADC_CCR_CKMODE ADC_CCR_CKMODE_Msk /*!< ADC common clock source and prescaler (prescaler only for clock source synchronous) */
#define ADC_CCR_CKMODE_0 (0x1UL << ADC_CCR_CKMODE_Pos) /*!< 0x00010000 */
#define ADC_CCR_CKMODE_1 (0x2UL << ADC_CCR_CKMODE_Pos) /*!< 0x00020000 */When you untangle the code from the PR of @gschorcht that was mentioned in the issue you get the following: #define ADC_CCR_CKMODE_HCLK_1 (ADC_CCR_CKMODE_0)
...
ADC->CCR |= ADC_CCR_CKMODE_HCLK_1 << ADC_CCR_CKMODE_Pos;
=>
ADC->CCR |= ADR_CCR_CKMODE_0 << ADC_CCR_CKMODE_Pos;
=>
ADC->CCR |= (0x1UL << ADC_CCR_CKMODE_Pos) << ADC_CCR_CKMODE_Pos;
=>
ADC->CCR |= (0x1UL << 16) << 16;
=>
ADC->CCR |= 0x1UL << 32;
=>
ADC->CCR |= 0;That means that the bit is shifted TWICE, which means it is shifted out of the register, so essentially the code ORs the register with 0. This means the change in the commit form @gschorcht is just wrong. I would propose the following code to replace the code in lines 146-167 (and remove the defines of ADC_CCR_CKMODE_HCLK_x in lines 67 and 68): ADC->CCR &= ~(ADC_CCR_CKMODE);
/* Setting ADC clock to HCLK/1 is only allowed if AHB clock prescaler is 1*/
if (!(RCC->CFGR & RCC_CFGR_HPRE_3)) {
/* set ADC clock to HCLK/1 */
ADC->CCR |= (ADC_CCR_CKMODE_0);
}
else {
/* set ADC clock to HCLK/2 otherwise */
ADC->CCR |= (ADC_CCR_CKMODE_1);
}The proposed change is untested because I don't have any L4 Nucleos here (the L452 board definition doesn't have the ADC added yet). |
krzysztof-cabaj
force-pushed
the
nucleo-l4xxx-fix-ADC
branch
from
c6f250b
to
f6e1ce7
Compare
|
@crasbe thanks for detailed analysis. I checked provided code using Updated code pushed. |
|
Good to hear that we could find a universal and elegant fix :) |
@crasbe Could you test ADC using this PR and |
I tested it and it hangs (as expected, because this PR is not applied to your branch). When manually applying the fix from this PR, I get the following output: |
|
I think you can rename this PR and commit to "cpu/stm32: fix L4 ADC initialization" (or similar), since the fix is related to the STM32L4 CPU family and not really to the Nucleo boards. |
krzysztof-cabaj
changed the title
|
@crasbe thanks for your tests. They show that this PR fix problem with ADC initialization and that ADC configuration for The note about renaming is very good - I changed PR name. I hope we could add this code to the RIOT |
benpicco
added
Type: bug
Contribution description
This PR fixes ADC initialization for
nucleo-l4xxxboards (more details in issue #20748).I tested this PR using
nucleo-l476rg,nucleo-l496zgandnucleo-l4r5zi.Testing procedure
Flash mentioned boards (or maybe other from L4xxx family) using
tests/periph/adcand observe console.With this PR you should see new measurements each 100 ms:
Issues/PRs references
Fixes #20748