Skip to content

Misleading bitoperation in OEMiROT_Boot clock configuration #31

@little-playground

Description

@little-playground

Describe the bug
In the OEMiROT_Boot application (Projects/*/Applications/ROT/OEMiROT_Boot) there is this block in SetSysClock:

MODIFY_REG(RCC->PLL1CFGR, RCC_PLL1CFGR_PLL1RGE, RCC_PLL1CFGR_PLL1SRC_1 << RCC_PLL1CFGR_PLL1RGE_Pos);
MODIFY_REG(RCC->PLL1CFGR, RCC_PLL1CFGR_PLL1VCOSEL, RCC_PLL1VCOWIDE << RCC_PLL1CFGR_PLL1VCOSEL_Pos);

PLL1RGE and PLL1SRC are unrelated fields in the register. Also RCC_PLL1CFGR_PLL1SRC_1 is already shifted to the right position in stm32h503xx.h

#define RCC_PLL1CFGR_PLL1SRC_1 (0x2UL << RCC_PLL1CFGR_PLL1SRC_Pos) /*!< 0x00000002 */

The line generates the correct bit pattern, but purely by chance.

The shift in the 2nd line is also incorrect. Since RCC_PLL1VCOWIDE = 0, it does not do anything. But if you would use RCC_PLL1VCOMEDIUM instead, it would expand to this:

(0x1UL << RCC_PLL1CFGR_PLL1VCOSEL_Pos) << RCC_PLL1CFGR_PLL1VCOSEL_Pos

Yet again, the line generates the correct bit pattern, but purely by chance.

Suggested Fix
It should be replaced with a clearer alternative:

MODIFY_REG(RCC->PLL1CFGR, RCC_PLL1CFGR_PLL1RGE, RCC_PLL1CFGR_PLL1RGE_1);
MODIFY_REG(RCC->PLL1CFGR, RCC_PLL1CFGR_PLL1VCOSEL, RCC_PLL1VCOWIDE);

or

MODIFY_REG(RCC->PLL1CFGR, RCC_PLL1CFGR_PLL1RGE, RCC_PLL1_VCIRANGE_2);
MODIFY_REG(RCC->PLL1CFGR, RCC_PLL1CFGR_PLL1VCOSEL, RCC_PLL1VCOWIDE);

Metadata

Metadata

Assignees

Labels

enhancementNew feature or requesthalHAL-LL driver-related issue or pull-request.rccReset and Clock Controller

Projects

Status

To do

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions