Why can I not configure the PLL clock properly?

Question

I am currently trying to write a bootloader for an STM32F401 microcontroller using nothing but C++. In this project, I have removed all start files using -nostartfiles and have written all the startup code myself. The main issue I am currently having is setting the system clock from an external crystal source of 25MHz to get the system to approximately 84MHz. Using STM32CubeIDE I have confirmed that the clock settings I am attempting to configure are indeed valid values.

My main issue is that I cannot get the PLL ready bit to set to continue the execution of my program. I have changed the order of operations regarding HSE configuration and PLL configuration to no avail. Each peripheral is structured as a class where the new operator is overwritten to return the exact address in memory. The RCC::EnableExternalSystemClock() function is the first function I call in my boot_main() entry. Here are some snippets of the current clock configuration code I have now:

RCC.cpp

/**
 * @brief Sets the system clock to the specified TARGET_SYSTEM_CLOCK through
 * the external oscillator.
 *
 */
void RCC::EnableExternalSystemClock() {
    size_t timeout = 0;

    // Enable HSE
    this->CR.bits.HSEON = 1;
    while (this->CR.bits.HSERDY != 0b1 || timeout < this->HSE_TIMEOUT) {
        timeout++;
    }

    // Enable power controller
    this->APB1ENR.bits.PWREN = 1;

    // Configure voltage regulator scaling
    PWR &power = *new PWR();
    power.SetRegulatorScale(PWR::Scale::DIV2);

    // Enable flash prefetch & caches
    FLASH &flash = *new FLASH();
    flash.EnablePrefetchCache();

    // Calculate actual system clock
    constexpr auto pll_clocks =
        RCC::CalculatePLLClocks(RCC::HSE_CRYSTAL_FREQ_HZ, RCC::TARGET_SYSTEM_CLOCK);
    constexpr auto sys_clock = RCC::CalculateSysClock(RCC::HSE_CRYSTAL_FREQ_HZ, pll_clocks.pll_m,
                                                      pll_clocks.pll_n, pll_clocks.pll_p);

    // Set APB scalers
    constexpr auto lsapb2_prescaler = RCC::CalculateLSAPB2Prescaler(sys_clock);
    this->CFGR.bits.HPRE = 0b000;
    this->CFGR.bits.PPRE1 = lsapb2_prescaler; // calculates as 0x04
    this->CFGR.bits.PPRE2 = 0b000;

    // Enable system config click
    this->APB2ENR.bits.SYSCFGEN = 1;

    // Disable PLL
    this->CR.bits.PLLON = 0;
    while (this->CR.bits.PLLRDY != 0) {
        // Do nothing
    }

    // Set PLL scalers
    this->PLLCFGR.bits.PLLM = pll_clocks.pll_m; // calculates as 13
    this->PLLCFGR.bits.PLLN = pll_clocks.pll_n; // calclulates as 87
    this->PLLCFGR.bits.PLLP = pll_clocks.pll_p; // calculates as 0x00
    this->PLLCFGR.bits.PLLQ = 4;

    // Set PLL source
    this->PLLCFGR.bits.PLLSRC = 1;

    // Enable PLL
    this->CR.bits.PLLON = 1;
    while (this->CR.bits.PLLRDY == 0) { // currently stuck here
        // Do nothing
    }

    // Set clock source
    this->CFGR.bits.SW = 0b10;
    while (this->CFGR.bits.SWS != 0b10) {
        // Do nothing
    }
}

FLASH.hpp

void FLASH::EnablePrefetchCache() {
    this->ACR.bits.DCEN = 1;
    this->ACR.bits.ICEN = 1;
    this->ACR.bits.PRFTEN = 1;

    constexpr int sys_clock = RCC::GetSystemClock(RCC::HSE_CRYSTAL_FREQ_HZ);
    constexpr uint8_t latency = FLASH::CalculateFlashLatency(TARGET_SYSTEM_VOLTAGE, sys_clock);
    this->ACR.bits.LATENCY = latency;
}

The RCC.hpp file can be found here.

Here is a diagram of the intended system clock configuration in STM32CubeIDE:

What am I missing to get the system clock to set properly?

Answer 1

This works on the NUCLEO card which uses an 8MHz crystal (PUT32 is a str instruction and GET32 is an ldr instruction):

//PLLM aim for 2mhz so 8/2=4
#define PLLM 4
//PLLN input is 2, want >=100 and <=432 so between 50 and 216
//84*2 = 168
//84*4 = 336 <---
//84*6 = 504 
//PLLP = 4 encodes as 0b01
#define PLLP 1
//so PLLP is 4 VCO 336 so PLLN is 168
#define PLLN 168
//PLLQ  336 / 48  = 7
#define PLLQ 7
//The maximum frequency of the AHB domain is 84 MHz. 
//The maximum allowed frequency of the high-speed APB2 domain is 84 MHz. 
//The maximum allowed frequency of the low-speed APB1 domain is 42 MHz
//Bits 15:13 PPRE2: APB high-speed prescaler (APB2)
//0xx divide by 0
#define PPRE2 0
//Bits 12:10 PPRE1: APB Low speed prescaler (APB1)
//0b100 divide by 2
#define PPRE1 4
//Bits 7:4 HPRE: AHB prescaler
//0b0xxx: system clock not divided
#define HPRE 0
static int clock_init ( void )
{
    unsigned int ra;

    //switch to external clock.
    ra=GET32(RCC_CR);
    ra|=1<<16;
    PUT32(RCC_CR,ra);
    while(1) if(GET32(RCC_CR)&(1<<17)) break;
    if(1)
    {
        ra=GET32(RCC_CFGR);
        ra&=~3;
        ra|=1;
        PUT32(RCC_CFGR,ra);
        while(1) if(((GET32(RCC_CFGR)>>2)&3)==1) break;
    }
    //setup PLL
    ra=GET32(RCC_CFGR);
    ra&=(~(0x7<<13));
    ra|=(PPRE2<<13) ;
    ra&=(~(0x7<<10));
    ra|=(PPRE1<<10) ;
    ra&=(~(0xF<< 4));
    ra|=(HPRE << 4) ;
    PUT32(RCC_CFGR,ra);
    PUT32(RCC_PLLCFGR,
        0x20000000|
        (PLLQ<<24)|
        (1<<22)|
        (PLLP<<16)|
        (PLLN<<6)|
        (PLLM<<0)
    );
    ra=GET32(FLASH_ACR);
    ra&=(~(0xF<<0));
    ra|=(5<<0); //80+MHz
    PUT32(FLASH_ACR,ra);
    //start pll
    ra=GET32(RCC_CR);
    ra|=1<<24;
    PUT32(RCC_CR,ra);
    while(1) if(GET32(RCC_CR)&(1<<25)) break;
    //switch to PLL
    ra=GET32(RCC_CFGR);
    ra&=~3;
    ra|=2;
    PUT32(RCC_CFGR,ra);
    while(1) if(((GET32(RCC_CFGR)>>2)&3)==2) break;
    return(0);
}

As pointed out you could do this:

//PLLM aim for 2mhz but...25/25=1
#define PLLM 25
//PLLN input is 1, want >=100 and <=432
//84*2 = 168
//84*4 = 336 <---
//84*6 = 504 
//PLLP = 4 encodes as 0b01
#define PLLP 1
//so PLLP is 4 VCO 336 so PLLN is 336
#define PLLN 336
//PLLQ  336 / 48  = 7
#define PLLQ 7
//The maximum frequency of the AHB domain is 84 MHz. 
//The maximum allowed frequency of the high-speed APB2 domain is 84 MHz. 
//The maximum allowed frequency of the low-speed APB1 domain is 42 MHz
//Bits 15:13 PPRE2: APB high-speed prescaler (APB2)
//0xx divide by 0
#define PPRE2 0
//Bits 12:10 PPRE1: APB Low speed prescaler (APB1)
//0b100 divide by 2
#define PPRE1 4
//Bits 7:4 HPRE: AHB prescaler
//0b0xxx: system clock not divided
#define HPRE 0

but you need to set the flash before USING the HSE.

You have run using the external clock without issues?

//switch to external clock.
ra=GET32(RCC_CR);
ra|=1<<16;
PUT32(RCC_CR,ra);
while(1) if(GET32(RCC_CR)&(1<<17)) break;
if(1)
{
    ra=GET32(RCC_CFGR);
    ra&=~3;
    ra|=1;
    PUT32(RCC_CFGR,ra);
    while(1) if(((GET32(RCC_CFGR)>>2)&3)==1) break;
}

In your case you need to set the flash for 1 wait state before switching to 25mhz (or just set for 5 to start with). This is not required I just do this because the code was developed using external first no PLL then add to that to use the PLL.

If you want to use USB and get 48MHz then use 1mhz as the input as shown above and in another answer.

this->CFGR.bits.HPRE = 0b000;
this->CFGR.bits.PPRE1 = lsapb2_prescaler; // calculates as 0x04
this->CFGR.bits.PPRE2 = 0b000;

These are fine for 84 or close to it.

25/13 = 1.9231
25/13 * 87 = 167.3

my comments are wrong it is 192 to 432 both in the datasheet and the reference manual (as pointed out in another answer). So you are going to struggle to get a lock at all and from part to part (board to board).

336 / (25/13) = 174.72
(25/13) * 174 = 334.6
((25/13) * 174) / 4 = 83.65
((25/13) * 174) / 7 = 47.80

Fine for the CPU, I do not know what that does for the USB. The data sheet implies a wide range for the pll48out so probably okay.

So you want:

#define PLLM 13
#define PLLP 1
#define PLLN 174
#define PLLQ 7
#define PPRE2 0
#define PPRE1 4
#define HPRE 0

PLLN and PLLP being your primary issue.

It is really irrelevant what the generic STM32CubeIDE shows you. See comments below. You cannot trust their code nor tools, you have to go through it. (Actually you cannot trust the documentation either, some hacking is required, and dumb luck and faith). The documentation says 192 and...

84
2 42
2 2 21
2 2 3 7

48
2 24
2 2 12
2 2 2 6
2 2 2 2 3

2 2 2 2 3 7 336

336 is the only vco that works for 84 and 48 mhz (if using the USB) so do 336. Also as a general rule to reduce jitter, aim for the higher end of the vco range not the lower. So....336, or higher.

84*2 = 168
84*4 = 336
84*6 = 504 

You only have one VCO choice for 84-ishMHz and that is 336, so:

#define PLLM 13
#define PLLP 1
#define PLLN 174
#define PLLQ 7

Are your only valid choices for this part per the datasheet (again I don't care what the library or some vendor tool does, have you looked at chip vendors library code?):

when VOS[1:0] = 0x10, the maximum value of fHCLK = 84 MHz.

reset value 00

00: Reserved (Scale 3 mode selected)
01: Scale 3 mode
10: Scale 2 mode
11: Reserved (Scale 2 mode selected)

You set this I did not, I got lucky. Maxing out the clock speeds just burns power, only go as fast as you need. The flash is your performance factor unless you run your code from ram, so bumping the core clocks up, means the cpu waits longer for fetches. Their little cache thing works pretty good, sure, but still you have to ask yourself what am I actually gaining here? The answer is usually more power consumption. (I have no reason to run that fast, since the internal RC clock is 16Mhz on this part, I use the external clock to get a better 16MHz clock and the same firmware is happy with or without the external (well the uart is much happier)). I only ran 84MHz to write this SO answer, had it not worked I would have set that, for a product would have set that. (for a product I would generally have no use for 84mhz, I would do 48 if using USB or 24 if I could get away with it).

PWREN seems to be for the high speed audio serial interface? If you are using that, same goes for which clock you select for that. So maybe you need that set.

As a habit it is best to avoid multiple read-modify-write cycles to a control and status register where the fields are related. Unless you carefully research each step. Changing the control bits for gpio field to zero then changing it to the final value rather than the much better habit of read the whole register, do the read-modify-writes locally in a variable, write the whole register, now on the same clock cycle all the control bits have changed, you have not turned an input into an alternate function or ADC or whatever and then back to whatever you wanted (not just ST parts but look at every field in every register of every vendor).

The PLL config register on this part does have some reserved bits and those bits are non-zero per the documentation. In my example I just write it rather than do the list of read-modify-writes. For production code, I think I would, hmm, probably just write it for this one and see what testing shows across a lot of boards. I do not know I would have to think about it, safe bet would be read, isolate the reserved bits, orr in the rest of the register in this case, and then write it back.

Bottom line, looks like your problem is the VCO is too low, 336 is better even if the datasheet and reference manual are wrong (and 192 is not the minimum) for jitter. And if you want to use USB at some point that is your only choice. You might get it to lock eventually on some parts at some temperatures and voltages, etc, but I would not expect every part to work when pushed that far out of spec.

The easy one is to use 1mhz as the pll input, I know it says go for 2 but it also says 1-2 is the spec. Math is easy and straight forward and whole numbers.

I would have also gone with an 8MHz or maybe 16mhz but not a 25. I do not know off hand what the price and availability is for the different speeds. Maybe if not using the PLL at all (and no USB)... based on the flash spec I would not go above 16Mhz on the external clock and keep it at zero wait states, 2 cycles per fetch. (yes ST does have a very good flash cache (that you cannot turn off in general) but it is jerky so you will get those stalls waiting on fetches, these cores only fetch one or two instructions at a time).

Note

Using bitfields and in particular pointing structures (or perhaps worse, unions) across compile domains is just a bad idea, unless you are doing it for job security reasons (firmware fails to build/execute every so many years requiring your special few months of magic to keep the product alive). (Vendor-supplied libraries are poorly designed/written these days, remember they have a legal disclaimer to avoid liability. It only takes a few minutes of inspecting their code to see that you have to inspect all of the code you wish to use (or just write your own and reduce the risk)).

Edit

Based on a comment 2.10 is shown as the upper limit on the PLL input, and 0.95 but the 0.95 has a note that says it is not tested but only by design, that not is not on the 2.10.

336 / (25/12) = 161.28
(25/12) * 161 = 335.4

#define PLLM 12
#define PLLP 1
#define PLLN 161
#define PLLQ 7

Answer 2

You have violated both PLLM and PLLN configurations. Reference manual RM0368. RCC sections, PLL configuration register (page 105):

Read the Caution part. Basically, it says after PLLM clock division the output has to be within 1MHz-2MHz range.

Next the PLLN:

Coming up with with a coefficient combination is a little math problem in its own. My proposed solution is:

1. PLLM is 25, so VCO input is 1MHz.
2. PLLN is 84*4 = 336.
3. PLLP divider is 4, so PLLP output is 336MHz/4=84MHz.

Also, you may need to enable overdrive/overdrive switch in PWR peripheral, if you're pushing MCU to the max. You only set the scaling.