Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clock issues for STM32F103RE custom board #32382

Closed
ycsin opened this issue Feb 17, 2021 · 14 comments · Fixed by #32393
Closed

Clock issues for STM32F103RE custom board #32382

ycsin opened this issue Feb 17, 2021 · 14 comments · Fixed by #32393
Assignees
Labels
bug The issue is a bug, or the PR is fixing a bug platform: STM32 ST Micro STM32 priority: low Low impact/importance bug

Comments

@ycsin
Copy link
Member

ycsin commented Feb 17, 2021

CONFIG_CLOCK_STM32_PLL_PREDIV1 in CUSTOM_BOARD_defconfig for my custom board based on STM32F103RET6 doesn't seem to be working.

My board is using a 16 MHz HSE. To support my custom board, I copied the nucleo_f103rb board and made necessary changes to the board files. However, the nucleo_f103rb does not use HSE but the 8MHz clock from the STLINK, therefore for my case I will have to divide the HSE output by 2 before inputting into the PLL Source Mux, which will be X9 at its output to obtain 72 Mhz at SYSCLK.

I've met two issues when I try to configure this:

  1. The program will just halt somewhere randomly if I set CONFIG_CLOCK_STM32_PLL_MULTIPLIER to 9, setting CONFIG_CLOCK_STM32_PLL_MULTIPLIER to a value lower than 9 works, but the k_msleep(SLEEP_TIME_MS) in Blinky example doesn't match the specified time.
  2. Setting CONFIG_CLOCK_STM32_PLL_PREDIV1 has no effect to the program.

After checking drivers\clock_control\clock_stm32f1.c's config_pll_init(LL_UTILS_PLLInitTypeDef *pllinit) and RCC_PLLConfig(u32 RCC_PLLSource, u32 RCC_PLLMul) of STM32HAL's stm32f10x_rcc.h, I guess the config_pll_init(LL_UTILS_PLLInitTypeDef *pllinit) is missing something for the pllinit->Prediv.

I've then edited config_pll_init(LL_UTILS_PLLInitTypeDef *pllinit) to the following:

/**
 * @brief fill in pll configuration structure
 */
void config_pll_init(LL_UTILS_PLLInitTypeDef *pllinit)
{
	/*
	 * 2  -> LL_RCC_PLL_MUL_2  -> 0x00000000
	 * 3  -> LL_RCC_PLL_MUL_3  -> 0x00040000
	 * 4  -> LL_RCC_PLL_MUL_4  -> 0x00080000
	 * ...
	 * 16 -> LL_RCC_PLL_MUL_16 -> 0x00380000
	 *
	 */
	pllinit->PLLMul = ((CONFIG_CLOCK_STM32_PLL_MULTIPLIER - 2)
					<< RCC_CFGR_PLLMULL_Pos);

	/*
	 * SOC_STM32F10X_CONNECTIVITY_LINE_DEVICE
	 * 1  -> LL_RCC_PREDIV_DIV_1  -> 0x00000000
	 * 2  -> LL_RCC_PREDIV_DIV_2  -> 0x00000001
	 * 3  -> LL_RCC_PREDIV_DIV_3  -> 0x00000002
	 * ...
	 * 16 -> LL_RCC_PREDIV_DIV_16 -> 0x0000000F
	 */

    #if CONFIG_SOC_STM32F103XE
        #if CONFIG_CLOCK_STM32_PLL_SRC_HSE
            #if (CONFIG_CLOCK_STM32_PLL_PREDIV1 == 1)
                #define PLL_PREDIV 0x00010000
            #elif (CONFIG_CLOCK_STM32_PLL_PREDIV1 == 2)
                #define PLL_PREDIV 0x00030000
            #else
                #error PLL predivider for HSE can only be 1 or 2
            #endif
        #elif CONFIG_CLOCK_STM32_PLL_SRC_HSE
            #define PLL_PREDIV 0x00000000
        #else
            #error PLL source must be either HSE or HSI
        #endif
    #else
        #define PLL_PREDIV CONFIG_CLOCK_STM32_PLL_PREDIV1 - 1
    #endif

	pllinit->Prediv = PLL_PREDIV;
}

My def_config file:

# SPDX-License-Identifier: Apache-2.0

CONFIG_SOC_SERIES_STM32F1X=y
CONFIG_SOC_STM32F103XE=y
# 72MHz system clock
CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC=72000000

# enable uart driver
CONFIG_SERIAL=y
# enable console
CONFIG_CONSOLE=y
CONFIG_UART_CONSOLE=y

# enable pinmux
CONFIG_PINMUX=y

# enable GPIO
CONFIG_GPIO=y

CONFIG_CLOCK_STM32_LSE=y
CONFIG_COUNTER_RTC_STM32_CLOCK_LSE=y

# clock configuration
CONFIG_CLOCK_CONTROL=y
CONFIG_CLOCK_CONTROL_STM32_CUBE=y

# Clock configuration for Cube Clock control driver
CONFIG_CLOCK_STM32_HSE_CLOCK=16000000
CONFIG_CLOCK_STM32_SYSCLK_SRC_PLL=y

# use HSE as PLL input
CONFIG_CLOCK_STM32_PLL_SRC_HSE=y

# produce 72MHz clock at PLL output
CONFIG_CLOCK_STM32_PLL_MULTIPLIER=9
CONFIG_CLOCK_STM32_PLL_PREDIV1=2

CONFIG_CLOCK_STM32_AHB_PRESCALER=1

# APB1 clock must not to exceed 36MHz limit
CONFIG_CLOCK_STM32_APB1_PRESCALER=2
CONFIG_CLOCK_STM32_APB2_PRESCALER=1

And now the program is able to work, and counting up like so:

while (1) {
    printk("%d\n", a++);
    k_msleep(SLEEP_TIME_MS);
}

Seems to match my stopwatch pretty well, with ~2 seconds off after ~10 minutes. But I'm not sure if I'm doing it correctly.

Environment:

  • OS: Windows 10
  • Toolchain: I'm using Platform.IO with Zephyr 2.4.0, not really sure what the default toolchain is

Additional context
Highlighted in green is my CONFIG_CLOCK_STM32_PLL_PREDIV1, and can only choose from /1 or /2 according to STM32CubeMX.
image

@ycsin ycsin added the bug The issue is a bug, or the PR is fixing a bug label Feb 17, 2021
@ycsin
Copy link
Member Author

ycsin commented Feb 17, 2021

I think the timer is still off, I tried counting using the Mbed OS blinky example:

int main()
{
    // Initialise the digital pin LED1 as an output
    DigitalOut led(LED1);
    int a = 0;
    while (true) {
        led = !led;
        printf("%d\n", a++);
        ThisThread::sleep_for(BLINKING_RATE);
    }
}

And the timing matches stopwatch perfectly, so there must be something wrong with my setup.

@erwango erwango added the platform: STM32 ST Micro STM32 label Feb 17, 2021
@erwango
Copy link
Member

erwango commented Feb 17, 2021

Thanks for reporting.
This might be due to this modification that removed PLLXTPRE support : 6b72fba

We need to resinstantiate it.

@erwango erwango self-assigned this Feb 17, 2021
@nashif nashif added the priority: low Low impact/importance bug label Feb 17, 2021
erwango added a commit to erwango/zephyr that referenced this issue Feb 17, 2021
This reverts commit "drivers/clock_control: Remove useless
CLOCK_STM32_PLL_XTPRE config" 9be1f7e.

Fixes zephyrproject-rtos#32382

Signed-off-by: Erwan Gouriou <[email protected]>
@erwango
Copy link
Member

erwango commented Feb 17, 2021

@ycsin When you have sometime, #32393 and let me know if this is fixing the issue you're seeing

@ycsin
Copy link
Member Author

ycsin commented Feb 18, 2021

@erwango Hi, I've added the commit manually in PlatformIO's Zephyr package and it seems to be working? With the same ~2s slower than Windows 10 stopwatch after 10mins.

I'm not sure if this is normal?

@erwango
Copy link
Member

erwango commented Feb 18, 2021

With the same ~2s slower than Windows 10 stopwatch after 10mins.

I'm not sure if this is normal?

Can you clarify the question ? What are you measuring ? What is the clock source ?

@narodo
Copy link

narodo commented Feb 18, 2021

Hi @ycsin, I was also working on this bug and I think I have pretty much the same HW setup as you. 16MHz external clock and running the sytem at 72MHz on an STM32F103C8T6. The pull-request fixed it for me and the clock configuration looks correct now.

I tried to reproduce the clock drift issue with your code snipped (I picked SLEEP_TIME_MS = 1000) but for me it tracks the system time of my PC well. After about 2500 seconds I have no visible drift in the seconds range (granted that this is a fairly crude measruement).

Can you check the content of your RCC_CFGR register and see if it matches the configuration you expect? Especially if the HSE clock is really selected?
As a reference, this is the content of my RCC_CFGR register for a div/2 on HSE and PLL multiplication of 9 configured with the new patch:
RCC_CFGR: 0x001F040A

@ycsin
Copy link
Member Author

ycsin commented Feb 18, 2021

Hey @erwango, I was counting up a variable every second like so in the main.c with nothing else running:

while (1) {
    printk("%d\n", a++);
    k_msleep(1000);
}

and compared the value with a stopwatch to determine the timer precision. But the value was 1 second slower than a stopwatch after every 5 minutes. (slower clock)

I've tried to compile a similar program using Mbed OS that I mentioned earlier, but the Mbed OS example doesn't have the "slower clock" issue.

Therefore I wondered if this is an expected outcome.

The clock source that I'm using for the RTC is a 32.768kHz LSE.

I've tried to setup the defconfig to use LSE for the RTC:

CONFIG_CLOCK_STM32_LSE=y
CONFIG_COUNTER_RTC_STM32_CLOCK_LSE=y

And also these flags to enable the RTC counter.

CONFIG_COUNTER=y
CONFIG_COUNTER_RTC_STM32=y

After further digging I guess this issue is likely due to this #32431

@ycsin
Copy link
Member Author

ycsin commented Feb 18, 2021

Hey @narodo, I'll check it asap, do you mind sharing me your BOARD_defconfig file please? Mine currently is set as such:

I've added LSE and COUNTER/RTC related flags but they are disabled as they are not working.

# SPDX-License-Identifier: Apache-2.0

CONFIG_SOC_SERIES_STM32F1X=y
CONFIG_SOC_STM32F103XE=y
# 72MHz system clock
CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC=72000000

# enable uart driver
CONFIG_SERIAL=y
# enable console
CONFIG_CONSOLE=y
CONFIG_UART_CONSOLE=y

# enable pinmux
CONFIG_PINMUX=y

# enable GPIO
CONFIG_GPIO=y

# CONFIG_CLOCK_STM32_LSE=y
# CONFIG_COUNTER_RTC_STM32_CLOCK_LSE=y
CONFIG_CLOCK_STM32_PLL_XTPRE=y

# clock configuration
CONFIG_CLOCK_CONTROL=y
CONFIG_CLOCK_CONTROL_STM32_CUBE=y

# Clock configuration for Cube Clock control driver
CONFIG_CLOCK_STM32_HSE_CLOCK=16000000
CONFIG_CLOCK_STM32_SYSCLK_SRC_PLL=y

# use HSE as PLL input
CONFIG_CLOCK_STM32_PLL_SRC_HSE=y

# produce 72MHz clock at PLL output
CONFIG_CLOCK_STM32_PLL_MULTIPLIER=9
# CONFIG_CLOCK_STM32_PLL_PREDIV1=2

CONFIG_CLOCK_STM32_AHB_PRESCALER=1

# APB1 clock must not to exceed 36MHz limit
CONFIG_CLOCK_STM32_APB1_PRESCALER=2
CONFIG_CLOCK_STM32_APB2_PRESCALER=1
# CONFIG_COUNTER=y
# CONFIG_COUNTER_RTC_STM32=y

@ycsin
Copy link
Member Author

ycsin commented Feb 18, 2021

@narodo This is my RCC register's content:
It matches yours.

image

This is the result of my comparison:

image

image

@narodo
Copy link

narodo commented Feb 18, 2021

Hi @ycsin, I let my board run for a lot longer and I can also see some drift now. But it took me about ~5000 seconds to get to the 2s(ish). So a factor of 10 slower than your test.

In my opinion the clock tree is configured correctly now. I don't think this patch is related to this behaviour. I don't see how it could introduce this drifting behaviour if the clocks are setup correctly (which they seem to be according to the RCC_CFGR register).

I think the problem is that the printk time execution time has to be added to the loop time (plus any other overhead), and that's why this test if off by so much as this error accumulates.
In my opinion the right way to achieve this tight control over the timing is to periodically schedule your routine based on a timer. Could you try to see if that gets rid of this drift?

@erwango
Copy link
Member

erwango commented Feb 18, 2021

@ycsin If at least the configuration issue is solved, then you can approve #32393. It will help getting it merged.

@ycsin ycsin closed this as completed Feb 18, 2021
@ycsin
Copy link
Member Author

ycsin commented Feb 18, 2021

@ycsin If at least the configuration issue is solved, then you can approve #32393. It will help getting it merged.

Thanks for the help!

@ycsin ycsin reopened this Feb 18, 2021
@ycsin ycsin closed this as completed Feb 18, 2021
@erwango
Copy link
Member

erwango commented Feb 18, 2021

@ycsin Can you approve #32393 in github UI ?
"Add your review" > "Review changes" > "Approve"

@ycsin
Copy link
Member Author

ycsin commented Feb 18, 2021

@ycsin Can you approve #32393 in github UI ?
"Add your review" > "Review changes" > "Approve"

Done!

galak pushed a commit that referenced this issue Feb 19, 2021
This reverts commit "drivers/clock_control: Remove useless
CLOCK_STM32_PLL_XTPRE config" 9be1f7e.

Fixes #32382

Signed-off-by: Erwan Gouriou <[email protected]>
zephyrbot pushed a commit that referenced this issue Feb 19, 2021
This reverts commit "drivers/clock_control: Remove useless
CLOCK_STM32_PLL_XTPRE config" 9be1f7e.

Fixes #32382

Signed-off-by: Erwan Gouriou <[email protected]>
nashif pushed a commit that referenced this issue Feb 19, 2021
This reverts commit "drivers/clock_control: Remove useless
CLOCK_STM32_PLL_XTPRE config" 9be1f7e.

Fixes #32382

Signed-off-by: Erwan Gouriou <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue is a bug, or the PR is fixing a bug platform: STM32 ST Micro STM32 priority: low Low impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants