search

CommunityGD32Cores / gd32-pio-spl-package

Improved repackaging of GD32 SPL code for PlatformIO
9 stars 13 forks source link

Better clock selection #4

Open maxgerhardt opened 3 years ago

maxgerhardt commented 3 years ago

The original GD32 SPL code sadly does a static clock source selection in the system_xxx.c file.

https://github.com/CommunityGD32Cores/gd32-pio-spl-package/blob/d8af9a4b5dde1418cc432c1e44c130055d40f2b8/gd32/cmsis/variants/gd32f30x/system_gd32f30x.c#L42-L55

This means that users who want to change which clock their firmware uses need to edit SPL package files. This is not how it's supposed to be, the SPL files are supposed to be constant but externally user-configurable.

A solution must be engineered that allows, e.g. via global macro setting, to influence the clock selection logic of the SPL framework easily.

Idea: global CLOCK_SOURCE macro with numeric values selecting the source? Or better a more dynamic "target frequency" macro, HSE or HSI clock flag, PLL factors macros, ...?

It should also be a choice to not compile in the clock selection code, but have the user provide it via one of the project files (and the linker).

PetteriAimonen commented 2 years ago

As a workaround in the meanwhile, one can define e.g. __SYSTEM_CLOCK_24M_PLL_HXTAL define in platformio.ini build_flags.

The default define __SYSTEM_CLOCK_120M_PLL_HXTAL is last in the selection logic so any other definition will override it.

maxgerhardt commented 2 years ago

Hmm yes this could work.. the builder script in platform-gd32 could then be expanded to accept some board_build.clock_source = <setting name> value and converts it into the necessary define, say -D__SYSTEM_CLOCK_48M_PLL_HXTAL=48000000U.. Will do some experimenting.

PetteriAimonen commented 2 years ago

Looks like the default code in system_gd32xxx.c makes some assumptions about the external crystal frequency. Not perfect, but usable at least.

Also for some reason I had to call SystemCoreClockUpdate() in main() to have correct value in SystemCoreClock variable.

maxgerhardt commented 2 years ago

No, actually this can't be right. The variable has an initial value that should be correct according to its clock settings..

https://github.com/CommunityGD32Cores/gd32-pio-spl-package/blob/d8af9a4b5dde1418cc432c1e44c130055d40f2b8/gd32/cmsis/variants/gd32f30x/system_gd32f30x.c#L64-L96

something else must going on in your example.

You did define your clock macro __SYSTEM_CLOCK_24M_PLL_HXTAL to a value too, like 24000000U, instead of just defining it without a value (implicit 0), right?

PetteriAimonen commented 2 years ago

Oh, I missed that. Indeed I just defined it to '1' for enable.