search

CommunityGD32Cores / ArduinoCore-GD32

Arduino core for GD32 devices, community developed, based on original GigaDevice's core
Other
86 stars 33 forks source link

Move assembler startup code into variants directory. #36

Closed bjc closed 2 years ago

bjc commented 2 years ago

When this code lived in the ‘core/gd32’ directory, all variants were being automatically compiled and linked in to the resulting ‘core.a’, which resulted in name collisions and symbols not resolving correctly for the target architecture unless you got lucky at link time.

Moving the startup code into ‘variants’ resolves this issue, as only the code matching the variant currently in use is compiled and linked in.

maxgerhardt commented 2 years ago

This is definitely an error in the core, but moving the startup files to the variants will lead to extreme duplication in the future as more variants are added (in a possibly automated way).

The STM32 core uses the same "include correct startup file" with

https://github.com/stm32duino/Arduino_Core_STM32/blob/master/cores/arduino/stm32/startup_stm32yyxx.S

but their startup files are outside the core in system/ so that they are not built. (https://github.com/stm32duino/Arduino_Core_STM32/tree/master/system/Drivers/CMSIS/Device/ST/STM32F1xx/Source/Templates/gcc)

I much more of a fan of putting all startup files in a new GCC folder https://github.com/CommunityGD32Cores/ArduinoCore-GD32/tree/main/system/GD32F30x_firmware/CMSIS/GD/GD32F30x/Source, which is then added to the include path and can then be included. What do you think of that?

bjc commented 2 years ago

I'd thought of using system as well, but couldn't think of a good path for it off the top of my head and variants seemed to me to fit the bill, but I agree it will lead to duplication.

Since I still don't want to put anything in system/GD32F30x_firmware, as that's the upstream library, what do you think about adding a new directory, say system/startup?

maxgerhardt commented 2 years ago

Indeed ideally the startup files should be in the GigaDevice's supplied firmware library folder, but only ARM and IAR starup files are there right now. I'll definitely request that to be added.

And sure, until then we can use system/startup.

bjc commented 2 years ago

Sorry about the delay, but here are the changes we've talked about.

maxgerhardt commented 2 years ago

Merged and expanded for PlatformIO in 214a1d3510c31c54a8cb02ddf3d140a3c3cbfa11, CI was retriggered and goes through.